| Message ID | 20200305102702.31512-1-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] hw/i386/intel_iommu: Simplify vtd_find_as_from_bus_num() logic | expand |
On Thu, Mar 05, 2020 at 11:27:02AM +0100, Philippe Mathieu-Daudé wrote: > The vtd_find_as_from_bus_num() function was introduced (in commit > dbaabb25f) in a code format that could return an incorrect pointer, > which was later fixed by commit a2e1cd41ccf. > We could have avoided this by writing the if() statement differently. > Do it now, in case this function is re-used. The code is easier to > review (harder to miss bugs). > > Reviewed-by: Peter Xu <peterx@redhat.com> > Reviewed-by: Eric Auger <eric.auger@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> I'll queues this. > --- > Since v1: Re-worded commit description, patch sent without -w. > Since v2: patch send without diff > Since v3: Fix typo in description and comment (Eric Auger) > > This patch is easier to review with 'git-diff -w' (--ignore-all-space) > --- > hw/i386/intel_iommu.c | 34 ++++++++++++++++++---------------- > 1 file changed, 18 insertions(+), 16 deletions(-) > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 6258c58ac9..204b6841ec 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -987,24 +987,26 @@ static bool vtd_slpte_nonzero_rsvd(uint64_t slpte, uint32_t level) > static VTDBus *vtd_find_as_from_bus_num(IntelIOMMUState *s, uint8_t bus_num) > { > VTDBus *vtd_bus = s->vtd_as_by_bus_num[bus_num]; > - if (!vtd_bus) { > - /* > - * Iterate over the registered buses to find the one which > - * currently hold this bus number, and update the bus_num > - * lookup table: > - */ > - GHashTableIter iter; > + GHashTableIter iter; > > - g_hash_table_iter_init(&iter, s->vtd_as_by_busptr); > - while (g_hash_table_iter_next(&iter, NULL, (void **)&vtd_bus)) { > - if (pci_bus_num(vtd_bus->bus) == bus_num) { > - s->vtd_as_by_bus_num[bus_num] = vtd_bus; > - return vtd_bus; > - } > - } > - vtd_bus = NULL; > + if (vtd_bus) { > + return vtd_bus; > } > - return vtd_bus; > + > + /* > + * Iterate over the registered buses to find the one which > + * currently holds this bus number and update the bus_num > + * lookup table. > + */ > + g_hash_table_iter_init(&iter, s->vtd_as_by_busptr); > + while (g_hash_table_iter_next(&iter, NULL, (void **)&vtd_bus)) { > + if (pci_bus_num(vtd_bus->bus) == bus_num) { > + s->vtd_as_by_bus_num[bus_num] = vtd_bus; > + return vtd_bus; > + } > + } > + > + return NULL; > } > > /* Given the @iova, get relevant @slptep. @slpte_level will be the last level > -- > 2.21.1
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 6258c58ac9..204b6841ec 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -987,24 +987,26 @@ static bool vtd_slpte_nonzero_rsvd(uint64_t slpte, uint32_t level) static VTDBus *vtd_find_as_from_bus_num(IntelIOMMUState *s, uint8_t bus_num) { VTDBus *vtd_bus = s->vtd_as_by_bus_num[bus_num]; - if (!vtd_bus) { - /* - * Iterate over the registered buses to find the one which - * currently hold this bus number, and update the bus_num - * lookup table: - */ - GHashTableIter iter; + GHashTableIter iter; - g_hash_table_iter_init(&iter, s->vtd_as_by_busptr); - while (g_hash_table_iter_next(&iter, NULL, (void **)&vtd_bus)) { - if (pci_bus_num(vtd_bus->bus) == bus_num) { - s->vtd_as_by_bus_num[bus_num] = vtd_bus; - return vtd_bus; - } - } - vtd_bus = NULL; + if (vtd_bus) { + return vtd_bus; } - return vtd_bus; + + /* + * Iterate over the registered buses to find the one which + * currently holds this bus number and update the bus_num + * lookup table. + */ + g_hash_table_iter_init(&iter, s->vtd_as_by_busptr); + while (g_hash_table_iter_next(&iter, NULL, (void **)&vtd_bus)) { + if (pci_bus_num(vtd_bus->bus) == bus_num) { + s->vtd_as_by_bus_num[bus_num] = vtd_bus; + return vtd_bus; + } + } + + return NULL; } /* Given the @iova, get relevant @slptep. @slpte_level will be the last level