* [PATCH] hw/arm/smmuv3: Pass the real perm to returned IOMMUTLBEntry in smmuv3_translate()
@ 2022-04-07 7:57 chenxiang via
2022-04-14 16:02 ` Eric Auger
0 siblings, 1 reply; 3+ messages in thread
From: chenxiang via @ 2022-04-07 7:57 UTC (permalink / raw)
To: eric.auger; +Cc: qemu-devel, linuxarm, Xiang Chen
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.
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;
entry.translated_addr = cached_entry->entry.translated_addr +
(addr & cached_entry->entry.addr_mask);
entry.addr_mask = cached_entry->entry.addr_mask;
--
2.33.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] hw/arm/smmuv3: Pass the real perm to returned IOMMUTLBEntry in smmuv3_translate()
2022-04-07 7:57 [PATCH] hw/arm/smmuv3: Pass the real perm to returned IOMMUTLBEntry in smmuv3_translate() chenxiang via
@ 2022-04-14 16:02 ` Eric Auger
2022-04-16 7:00 ` chenxiang (M) via
0 siblings, 1 reply; 3+ messages in thread
From: Eric Auger @ 2022-04-14 16:02 UTC (permalink / raw)
To: chenxiang; +Cc: qemu-devel, linuxarm
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;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] hw/arm/smmuv3: Pass the real perm to returned IOMMUTLBEntry in smmuv3_translate()
2022-04-14 16:02 ` Eric Auger
@ 2022-04-16 7:00 ` chenxiang (M) via
0 siblings, 0 replies; 3+ messages in thread
From: chenxiang (M) via @ 2022-04-16 7:00 UTC (permalink / raw)
To: eric.auger; +Cc: qemu-devel, linuxarm
Hi Eric,
在 2022/4/15 0:02, Eric Auger 写道:
> 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.
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 <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>
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;
> .
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-04-16 7:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-07 7:57 [PATCH] hw/arm/smmuv3: Pass the real perm to returned IOMMUTLBEntry in smmuv3_translate() chenxiang via
2022-04-14 16:02 ` Eric Auger
2022-04-16 7:00 ` chenxiang (M) via
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.