Hi Eric,

在 2022/4/15 0:02, Eric Auger 写道:
Hi Chenxiang,

On 4/7/22 9:57 AM, chenxiang via wrote:
From: Xiang Chen <chenxian...@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.

Thank you for your comments.
I will change the commit message in next version.



Pass the real perm to returned IOMMUTLBEntry to avoid the issue.

Signed-off-by: Xiang Chen <chenxian...@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.au...@redhat.com>

Ok, thanks


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;
.



Reply via email to