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

[Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>]

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