[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V3 1/1] amd/iommu: Fix infinite loop due to ivrs_bdf_entries larger than 16-bit value



On 12/30/2013 07:04 AM, Boris Ostrovsky wrote:
On 12/29/2013 06:34 PM, suravee.suthikulpanit@xxxxxxx wrote:
From: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>

Certain AMD systems could have upto 0x10000 ivrs_bdf_entries.
However, the loop variable (bdf) is declared as u16 which causes
inifinite loop when parsing IOMMU event log with IO_PAGE_FAULT event.
This patch changes the variable to u32 instead.

Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
V3:
    - More places found in iommu_acpi.c
    - Add signed off message.
V2:
    - Fix in more places as pointed out by Andrew
  xen/drivers/passthrough/amd/iommu_acpi.c |   17 +++++++++++------
  xen/drivers/passthrough/amd/iommu_init.c |   13 +++++++------
  2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c
b/xen/drivers/passthrough/amd/iommu_acpi.c
index fca2037..b396e0e 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -159,7 +159,7 @@ static int __init
register_exclusion_range_for_all_devices(
      int seg = 0; /* XXX */
      unsigned long range_top, iommu_top, length;
      struct amd_iommu *iommu;
-    u16 bdf;
+    u32 bdf;
      /* is part of exclusion range inside of IOMMU virtual address
space? */
      /* note: 'limit' parameter is assumed to be page-aligned */
@@ -237,7 +237,8 @@ static int __init
register_exclusion_range_for_iommu_devices(
      unsigned long base, unsigned long limit, u8 iw, u8 ir)
  {
      unsigned long range_top, iommu_top, length;
-    u16 bdf, req;
+    u32 bdf;
+    u16 req;
      /* is part of exclusion range inside of IOMMU virtual address
space? */
      /* note: 'limit' parameter is assumed to be page-aligned */
@@ -292,7 +293,8 @@ static int __init parse_ivmd_device_range(
      const struct acpi_ivrs_memory *ivmd_block,
      unsigned long base, unsigned long limit, u8 iw, u8 ir)
  {
-    u16 first_bdf, last_bdf, bdf;
+    u16 first_bdf, last_bdf;
+    u32 bdf;
      int error;

Shouldn't first_bdf and last_bdf be u32 as well?

There is, for example, a loop in this routine

     for ( bdf = first_bdf, error = 0; (bdf <= last_bdf) && !error; bdf++ )

And in routines below as well.


-boris

I am not expecting the first_bdf and last_bdf to be greater than 16-bit. However, for the bitwise logic comparisons, I can make them all 32-bit. I'll send out V4. Thanks for the review.

Suravee



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.