From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH V2 1/1] amd/iommu: Fix infinite loop due to ivrs_bdf_entries larger than 16-bit value Date: Mon, 30 Dec 2013 02:37:36 +0000 Message-ID: <52C0DC70.9040000@citrix.com> References: <1388335114-3593-1-git-send-email-suravee.suthikulpanit@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1388335114-3593-1-git-send-email-suravee.suthikulpanit@amd.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: suravee.suthikulpanit@amd.com, JBeulich@suse.com Cc: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 29/12/2013 16:38, suravee.suthikulpanit@amd.com wrote: > From: Suravee Suthikulpanit > > 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. > --- > V2: > - Fix in more places as pointed out by Andrew You are missing a Signed-off-by on this patch, However, for the content, Reviewed-by: Andrew Cooper > > xen/drivers/passthrough/amd/iommu_acpi.c | 5 +++-- > xen/drivers/passthrough/amd/iommu_init.c | 13 +++++++------ > 2 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c > index fca2037..634dee3 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 */ > diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c > index b431d16..c410465 100644 > --- a/xen/drivers/passthrough/amd/iommu_init.c > +++ b/xen/drivers/passthrough/amd/iommu_init.c > @@ -524,8 +524,8 @@ static hw_irq_controller iommu_maskable_msi_type = { > > static void parse_event_log_entry(struct amd_iommu *iommu, u32 entry[]) > { > - u16 domain_id, device_id, bdf, flags; > - u32 code; > + u16 domain_id, device_id, flags; > + u32 code, bdf; > u64 *addr; > int count = 0; > static const char *const event_str[] = { > @@ -1103,7 +1103,7 @@ int iterate_ivrs_entries(int (*handler)(u16 seg, struct ivrs_mappings *)) > > do { > struct ivrs_mappings *map; > - int bdf; > + u32 bdf; > > if ( !radix_tree_gang_lookup(&ivrs_maps, (void **)&map, seg, 1) ) > break; > @@ -1118,7 +1118,7 @@ int iterate_ivrs_entries(int (*handler)(u16 seg, struct ivrs_mappings *)) > static int __init alloc_ivrs_mappings(u16 seg) > { > struct ivrs_mappings *ivrs_mappings; > - int bdf; > + u32 bdf; > > BUG_ON( !ivrs_bdf_entries ); > > @@ -1156,7 +1156,7 @@ static int __init alloc_ivrs_mappings(u16 seg) > static int __init amd_iommu_setup_device_table( > u16 seg, struct ivrs_mappings *ivrs_mappings) > { > - int bdf; > + u32 bdf; > void *intr_tb, *dte; > > BUG_ON( (ivrs_bdf_entries == 0) ); > @@ -1306,7 +1306,8 @@ static void invalidate_all_domain_pages(void) > static int _invalidate_all_devices( > u16 seg, struct ivrs_mappings *ivrs_mappings) > { > - int bdf, req_id; > + u32 bdf; > + u16 req_id; > unsigned long flags; > struct amd_iommu *iommu; >