On 5 July 2018 at 08:27, Eric Auger <[email protected]> wrote:
> smmu_iommu_mr() aims at returning the IOMMUMemoryRegion corresponding
> to a given sid. The function extracts both the PCIe bus number and
> the devfn to return this data. Current computation of devfn is wrong
> as it only returns the PCIe function instead of slot | function.
>
> Fixes 32cfd7f39e08 ("hw/arm/smmuv3: Cache/invalidate config data")
>
> Signed-off-by: Eric Auger <[email protected]>
> ---
> hw/arm/smmu-common.c | 2 +-
> include/hw/arm/smmu-common.h | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 3098915..55c75d6 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -351,7 +351,7 @@ IOMMUMemoryRegion *smmu_iommu_mr(SMMUState *s, uint32_t
> sid)
> bus_n = PCI_BUS_NUM(sid);
> smmu_bus = smmu_find_smmu_pcibus(s, bus_n);
> if (smmu_bus) {
> - devfn = sid & 0x7;
> + devfn = SMMU_PCI_DEVFN(sid);
> smmu = smmu_bus->pbdev[devfn];
> if (smmu) {
> return &smmu->iommu;
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index 50e2912..b07cadd 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -24,6 +24,7 @@
>
> #define SMMU_PCI_BUS_MAX 256
> #define SMMU_PCI_DEVFN_MAX 256
> +#define SMMU_PCI_DEVFN(sid) (sid & 0xFF)
>
> #define SMMU_MAX_VA_BITS 48
Applied to target-arm.next, thanks.
As I was reviewing this, I checked where we allocate the pbdev array
to confirm that it's big enough (which it is), and I noticed an oddity:
in include/hw/arm/smmu-common.h we define the SMMUPciBus struct like this:
typedef struct SMMUPciBus {
PCIBus *bus;
SMMUDevice *pbdev[0]; /* Parent array is sparse, so dynamically alloc */
} SMMUPciBus;
but in fact we don't ever have local variables of this type and the
only place we dynamically allocate them is in smmu_find_add_as(),
which does
sbus = g_malloc0(sizeof(SMMUPciBus) +
sizeof(SMMUDevice *) * SMMU_PCI_DEVFN_MAX);
Is there a reason I missed why we don't just define the struct
to have
SMMUDevice *pbdev[SMMU_PCI_DEVFN_MAX];
?
thanks
-- PMM
