AMD IOMMU: fix debug console IOMMU intremap output
PATCH 1/1 AMD IOMMU: fix debug console IOMMU intremap output
When using the Xen debug console and printing the IOMMU intremap tables, it prints everything in the IVRS range regardless if it has an intr remap or not. Add some logic to cause an entry to only be printed if the intr remap table isn't empty.
---
CC: Paul Durrant <[email protected]>
CC: Roger Pau Monne <[email protected]>
xen/drivers/passthrough/amd/iommu_intr.c | 34 ++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-)
xen/drivers/passthrough/amd/iommu_intr.c +28 -6
@@ -665,6 +665,24 @@ int __init amd_setup_hpet_msi(struct msi_desc *msi_desc)return rc; } + +static bool intremap_table_empty (const u32 *table) +{ +u32 count; + +if ( !table ) +return true; + +for ( count =0 ; count <INTREMAP_ENTRIES ; count++ ) + { +if ( table[count] ) +return false; + } +return true; +} + + +static void dump_intremap_table (const u32 *table) {u32 count;
@@ -687,13 +705,17 @@ static int dump_intremap_mapping(u16 seg, struct ivrs_mappings *ivrs_mapping)if ( !ivrs_mapping )return 0 ; -printk (" %04x:%02x:%02x:%u:\n" , seg, -PCI_BUS (ivrs_mapping->dte_requestor_id), -PCI_SLOT (ivrs_mapping->dte_requestor_id), -PCI_FUNC (ivrs_mapping->dte_requestor_id)); -spin_lock_irqsave (&(ivrs_mapping->intremap_lock), flags); -dump_intremap_table (ivrs_mapping->intremap_table); + +if ( !intremap_table_empty (ivrs_mapping->intremap_table) ) { +printk (" %04x:%02x:%02x:%u:\n" , seg, +PCI_BUS (ivrs_mapping->dte_requestor_id), +PCI_SLOT (ivrs_mapping->dte_requestor_id), +PCI_FUNC (ivrs_mapping->dte_requestor_id)); + +dump_intremap_table (ivrs_mapping->intremap_table); + } +spin_unlock_irqrestore (&(ivrs_mapping->intremap_lock), flags);return 0 ;
Signature · 7 lines
2.11.0 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Quoted 29 lines
-----Original Message-----
From: Woods, Brian [mailto:[email protected]]
Sent: 04 December 2018 21:47
To: [email protected]
Cc: Suthikulpanit, Suravee <[email protected]>; Woods, Brian <[email protected]>; Paul Durrant <[email protected]>; Roger Pau Monne <[email protected]>
Subject: [PATCH] AMD IOMMU: fix debug console IOMMU intremap outputWhen using the Xen debug console and printing the IOMMU intremap tables, it prints everything in the IVRS range regardless if it has an intr remap or not. Add some logic to cause an entry to only be printed if the intr remap table isn't empty.
Signed-off-by Brian Woods <[email protected]>---
CC: Paul Durrant <[email protected]>
CC: Roger Pau Monne <[email protected]>xen/drivers/passthrough/amd/iommu_intr.c | 34 ++++++++++++++++++++++++++- ----- 1 file changed, 28 insertions(+), 6 deletions(-)xen/drivers/passthrough/amd/iommu_intr.c +0
-0
xen/drivers/passthrough/amd/iommu_intr.c int __init amd_setup_hpet_msi(struct msi_desc · line 672
Quoted 8 lines
xen/drivers/passthrough/amd/iommu_intr.c +4
-0@@ -665,6 +665,24 @@ int __init amd_setup_hpet_msi(struct msi_desc *msi_desc)return rc; } + +static bool intremap_table_empty (const u32 *table) +{ +u32 count;
s/u32/uint32_t in both cases above.
xen/drivers/passthrough/amd/iommu_intr.c int __init amd_setup_hpet_msi(struct msi_desc *msi_desc) · line 677
Quoted 6 lines
xen/drivers/passthrough/amd/iommu_intr.c +6
-0+ +if ( !table ) +return true; + +for ( count =0 ; count <INTREMAP_ENTRIES ; count++ ) + {
Unnecessary braces.
Quoted 15 lines
xen/drivers/passthrough/amd/iommu_intr.c +8
-0+if ( table[count] ) +return false; + } +return true; +} + + +static void dump_intremap_table (const u32 *table) {u32 count;xen/drivers/passthrough/amd/iommu_intr.c +0
-0@@ -687,13 +705,17 @@ static int dump_intremap_mapping(u16 seg, struct ivrs_mappings *ivrs_mapping)if ( !ivrs_mapping )return 0 ;
xen/drivers/passthrough/amd/iommu_intr.c static int dump_intremap_mapping(u16 seg, struct · line 711
Quoted 10 lines
xen/drivers/passthrough/amd/iommu_intr.c +2
-6-printk (" %04x:%02x:%02x:%u:\n" , seg, -PCI_BUS (ivrs_mapping->dte_requestor_id), -PCI_SLOT (ivrs_mapping->dte_requestor_id), -PCI_FUNC (ivrs_mapping->dte_requestor_id)); -spin_lock_irqsave (&(ivrs_mapping->intremap_lock), flags); -dump_intremap_table (ivrs_mapping->intremap_table); + +if ( !intremap_table_empty (ivrs_mapping->intremap_table) ) {
Style. The open brace should be on the line below.
The functionality looks fine though, so with those fixed...
Quoted 11 lines
xen/drivers/passthrough/amd/iommu_intr.c +8
-0+printk (" %04x:%02x:%02x:%u:\n" , seg, +PCI_BUS (ivrs_mapping->dte_requestor_id), +PCI_SLOT (ivrs_mapping->dte_requestor_id), +PCI_FUNC (ivrs_mapping->dte_requestor_id)); + +dump_intremap_table (ivrs_mapping->intremap_table); + } +spin_unlock_irqrestore (&(ivrs_mapping->intremap_lock), flags);return 0;
Quoted 2 lines
--
2.11.0
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
xen/drivers/passthrough/amd/iommu_intr.c int __init amd_setup_hpet_msi(struct msi_desc *msi_desc) · line 669
Quoted 9 lines
Quoted 1 line
Quoted 1 line · On 04.12.18 at 22:47, <[email protected]> wrote:
xen/drivers/passthrough/amd/iommu_intr.c +2
-0@@ -665,6 +665,24 @@ int __init amd_setup_hpet_msi(struct msi_desc *msi_desc)return rc; } + +static bool intremap_table_empty (const u32 *table)
uint32_t here please and ...
xen/drivers/passthrough/amd/iommu_intr.c int __init amd_setup_hpet_msi(struct msi_desc *msi_desc) · line 671
Quoted 2 lines
xen/drivers/passthrough/amd/iommu_intr.c +2
-0+{ +u32 count;
... since a fixed width type isn't needed here in the first place, unsigned int here. (This is notwithstanding the fact that I assume you've merely cloned dump_intremap_table().)
xen/drivers/passthrough/amd/iommu_intr.c int __init amd_setup_hpet_msi(struct msi_desc *msi_desc) · line 681
Quoted 9 lines
xen/drivers/passthrough/amd/iommu_intr.c +9
-0+if ( !table ) +return true; + +for ( count =0 ; count <INTREMAP_ENTRIES ; count++ ) + { +if ( table[count] ) +return false; + } +return true;
Blank line above here please.
xen/drivers/passthrough/amd/iommu_intr.c int __init amd_setup_hpet_msi(struct msi_desc *msi_desc) · line 686
Quoted 5 lines
xen/drivers/passthrough/amd/iommu_intr.c +4
-0+} + + +static void dump_intremap_table (const u32 *table)
No multiple consecutive blank lines in general please (there may be extremely limited cases where exceptions are possible).
xen/drivers/passthrough/amd/iommu_intr.c static int dump_intremap_mapping(u16 seg, struct ivrs_mappings *ivrs_mapping) · line 710
Quoted 13 lines
xen/drivers/passthrough/amd/iommu_intr.c +2
-6@@ -687,13 +705,17 @@ static int dump_intremap_mapping(u16 seg, struct ivrs_mappings *ivrs_mapping)if ( !ivrs_mapping )return 0 ; -printk (" %04x:%02x:%02x:%u:\n" , seg, -PCI_BUS (ivrs_mapping->dte_requestor_id), -PCI_SLOT (ivrs_mapping->dte_requestor_id), -PCI_FUNC (ivrs_mapping->dte_requestor_id)); -spin_lock_irqsave (&(ivrs_mapping->intremap_lock), flags); -dump_intremap_table (ivrs_mapping->intremap_table); + +if ( !intremap_table_empty (ivrs_mapping->intremap_table) ) {
Brace on its own line please.
xen/drivers/passthrough/amd/iommu_intr.c static int dump_intremap_mapping(u16 seg, struct ivrs_mappings *ivrs_mapping) · line 717
Quoted 7 lines
xen/drivers/passthrough/amd/iommu_intr.c +7
-0+printk (" %04x:%02x:%02x:%u:\n" , seg, +PCI_BUS (ivrs_mapping->dte_requestor_id), +PCI_SLOT (ivrs_mapping->dte_requestor_id), +PCI_FUNC (ivrs_mapping->dte_requestor_id)); + +dump_intremap_table (ivrs_mapping->intremap_table); + }
dump_intremap_table() already skips empty entries, so aiui it is just the headline above you omit. How much of a savings is this really?
Furthermore, instead of adding a second function with a second loop, did you consider moving the logging of the headline into dump_intremap_table(), issuing the line the first time you hit a non-empty entry?
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Quoted 12 lines · Jan Beulich
xen/drivers/passthrough/amd/iommu_intr.c int __init amd_setup_hpet_msi(struct msi_desc *msi_desc) · line 669
Quoted 9 lines
Quoted 1 line
Quoted 1 line · On 04.12.18 at 22:47, <[email protected]> wrote:
xen/drivers/passthrough/amd/iommu_intr.c +2
-0@@ -665,6 +665,24 @@ int __init amd_setup_hpet_msi(struct msi_desc *msi_desc)return rc; } + +static bool intremap_table_empty (const u32 *table)
uint32_there please and ...
Quoted 6 lines
xen/drivers/passthrough/amd/iommu_intr.c int __init amd_setup_hpet_msi(struct msi_desc *msi_desc) · line 671
Quoted 2 lines
xen/drivers/passthrough/amd/iommu_intr.c +2
-0+{ +u32 count;... since a fixed width type isn't needed here in the first place, unsigned int here. (This is notwithstanding the fact that I assume you've merely cloned
dump_intremap_table().)
Gah, I did copy/clone dump_intremap_table, if I keep the same code strucutre I'll use you ahd Paul's suggestions.
Quoted 49 lines
xen/drivers/passthrough/amd/iommu_intr.c int __init amd_setup_hpet_msi(struct msi_desc *msi_desc) · line 681
Quoted 9 lines
xen/drivers/passthrough/amd/iommu_intr.c +9
-0+if ( !table ) +return true; + +for ( count =0 ; count <INTREMAP_ENTRIES ; count++ ) + { +if ( table[count] ) +return false; + } +return true;Blank line above here please.
xen/drivers/passthrough/amd/iommu_intr.c int __init amd_setup_hpet_msi(struct msi_desc *msi_desc) · line 686
Quoted 5 lines
xen/drivers/passthrough/amd/iommu_intr.c +4
-0+} + + +static void dump_intremap_table (const u32 *table)No multiple consecutive blank lines in general please (there may be extremely limited cases where exceptions are possible).
xen/drivers/passthrough/amd/iommu_intr.c static int dump_intremap_mapping(u16 seg, struct ivrs_mappings *ivrs_mapping) · line 710
Quoted 13 lines
xen/drivers/passthrough/amd/iommu_intr.c +2
-6@@ -687,13 +705,17 @@ static int dump_intremap_mapping(u16 seg, struct ivrs_mappings *ivrs_mapping)if ( !ivrs_mapping )return 0 ; -printk (" %04x:%02x:%02x:%u:\n" , seg, -PCI_BUS (ivrs_mapping->dte_requestor_id), -PCI_SLOT (ivrs_mapping->dte_requestor_id), -PCI_FUNC (ivrs_mapping->dte_requestor_id)); -spin_lock_irqsave (&(ivrs_mapping->intremap_lock), flags); -dump_intremap_table (ivrs_mapping->intremap_table); + +if ( !intremap_table_empty (ivrs_mapping->intremap_table) ) {Brace on its own line please.
xen/drivers/passthrough/amd/iommu_intr.c static int dump_intremap_mapping(u16 seg, struct ivrs_mappings *ivrs_mapping) · line 717
Quoted 7 lines
xen/drivers/passthrough/amd/iommu_intr.c +7
-0+printk (" %04x:%02x:%02x:%u:\n" , seg, +PCI_BUS (ivrs_mapping->dte_requestor_id), +PCI_SLOT (ivrs_mapping->dte_requestor_id), +PCI_FUNC (ivrs_mapping->dte_requestor_id)); + +dump_intremap_table (ivrs_mapping->intremap_table); + }
dump_intremap_table()already skips empty entries, so aiui it is just the headline above you omit. How much of a savings is this really?
Quoted 7 lines
Furthermore, instead of adding a second function with a second loop, did you consider moving the logging of the headline into
dump_intremap_table(), issuing the line the first time you hit a non-empty entry?Jan
I did think about doing that also, but ended up going with this route. What I'll do is move printing the headline intp dump_intremap_table and see what you guys think (since it's such a small patch, it'll be easier to do that than talk about it).
Signature · 6 lines
Brian Woods _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel