[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/arm/smmu-common: a fix to smmu_find_smmu_pcibus
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH] hw/arm/smmu-common: a fix to smmu_find_smmu_pcibus |
Date: |
Wed, 26 Feb 2020 18:42:35 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 |
On 2/26/20 6:37 PM, Philippe Mathieu-Daudé wrote:
Hi Eric,
On 2/26/20 6:26 PM, Eric Auger wrote:
Make sure a null SMMUPciBus is returned in case we were
not able to identify a pci bus matching the @bus_num.
This matches the fix done on intel iommu in commit:
a2e1cd41ccfe796529abfd1b6aeb1dd4393762a2
Signed-off-by: Eric Auger <address@hidden>
---
hw/arm/smmu-common.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 0f2573f004..67d7b2d0fd 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -301,6 +301,7 @@ SMMUPciBus *smmu_find_smmu_pcibus(SMMUState *s,
uint8_t bus_num)
return smmu_pci_bus;
}
}
+ smmu_pci_bus = NULL;
}
return smmu_pci_bus;
}
Patch is easy to review but code not. By inverting the if() statement I
find the code easier to review. The patch isn't however:
I used 'git-diff -W' instead of 'git-diff -w'. -w works better:
-- >8 --
@@ -290,10 +290,12 @@ inline int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t
iova, IOMMUAccessFlags perm,
SMMUPciBus *smmu_find_smmu_pcibus(SMMUState *s, uint8_t bus_num)
{
SMMUPciBus *smmu_pci_bus = s->smmu_pcibus_by_bus_num[bus_num];
-
- if (!smmu_pci_bus) {
GHashTableIter iter;
+ if (smmu_pci_bus) {
+ return smmu_pci_bus;
+ }
+
g_hash_table_iter_init(&iter, s->smmu_pcibus_by_busptr);
while (g_hash_table_iter_next(&iter, NULL, (void **)&smmu_pci_bus)) {
if (pci_bus_num(smmu_pci_bus->bus) == bus_num) {
@@ -301,8 +303,8 @@ SMMUPciBus *smmu_find_smmu_pcibus(SMMUState *s,
uint8_t bus_num)
return smmu_pci_bus;
}
}
- }
- return smmu_pci_bus;
+
+ return NULL;
}
---
The code is easier although:
SMMUPciBus *smmu_find_smmu_pcibus(SMMUState *s, uint8_t bus_num)
{
SMMUPciBus *smmu_pci_bus = s->smmu_pcibus_by_bus_num[bus_num];
GHashTableIter iter;
if (smmu_pci_bus) {
return smmu_pci_bus;
}
g_hash_table_iter_init(&iter, s->smmu_pcibus_by_busptr);
while (g_hash_table_iter_next(&iter, NULL, (void **)&smmu_pci_bus)) {
if (pci_bus_num(smmu_pci_bus->bus) == bus_num) {
s->smmu_pcibus_by_bus_num[bus_num] = smmu_pci_bus;
return smmu_pci_bus;
}
}
return NULL;
}
What do you think?