[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/arm/smmuv3: Pass the real perm to returned IOMMUTLBEntry
From: |
Eric Auger |
Subject: |
Re: [PATCH] hw/arm/smmuv3: Pass the real perm to returned IOMMUTLBEntry in smmuv3_translate() |
Date: |
Thu, 14 Apr 2022 18:02:04 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 |
Hi Chenxiang,
On 4/7/22 9:57 AM, chenxiang via wrote:
> From: Xiang Chen <chenxiang66@hisilicon.com>
>
> In function memory_region_iommu_replay(), it decides to notify() or not
> according to the perm of returned IOMMUTLBEntry. But for smmuv3, the
> returned perm is always IOMMU_NONE even if the translation success.
I think you should precise in the commit message that
memory_region_iommu_replay() always calls the IOMMU MR translate()
callback with flag=IOMMU_NONE and thus, currently, translate() returns
an IOMMUTLBEntry with perm set to IOMMU_NONE if the translation
succeeds, whereas it is expected to return the actual permission set in
the table entry.
> Pass the real perm to returned IOMMUTLBEntry to avoid the issue.
>
> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
> ---
> hw/arm/smmuv3.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 674623aabe..707eb430c2 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -760,7 +760,7 @@ epilogue:
> qemu_mutex_unlock(&s->mutex);
> switch (status) {
> case SMMU_TRANS_SUCCESS:
> - entry.perm = flag;
> + entry.perm = cached_entry->entry.perm;
With that clarification
Reviewed-by: Eric Auger <eric.auger@redhat.com>
the translate() doc in ./include/exec/memory.h states
"
If IOMMU_NONE is passed then the IOMMU must do the
* full page table walk and report the permissions in the returned
* IOMMUTLBEntry. (Note that this implies that an IOMMU may not
* return different mappings for reads and writes.)
"
Thanks
Eric
> entry.translated_addr = cached_entry->entry.translated_addr +
> (addr & cached_entry->entry.addr_mask);
> entry.addr_mask = cached_entry->entry.addr_mask;