spapr_iommu: Fix xlate trace to print translated address

[Qemu-devel] [PATCH qemu] spapr_iommu: Fix xlate trace to print translated address

Posted by Alexey Kardashevskiy 4 years, 8 months ago

Currently we basically print IO address twice, fix this.

Fixes: 7e472264e9e2 ("PPC: spapr: iommu: rework traces")
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr_iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
index bd3d0256a65d..6fe57d799a10 100644
--- a/hw/ppc/spapr_iommu.c
+++ b/hw/ppc/spapr_iommu.c
@@ -137,7 +137,7 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(IOMMUMemoryRegion *iommu,
         ret.addr_mask = ~page_mask;
         ret.perm = spapr_tce_iommu_access_flags(tce);
     }
-    trace_spapr_iommu_xlate(tcet->liobn, addr, ret.iova, ret.perm,
+    trace_spapr_iommu_xlate(tcet->liobn, addr, ret.translated_addr, ret.perm,
                             ret.addr_mask);
 
     return ret;
-- 
2.17.1

Re: [Qemu-devel] [PATCH qemu] spapr_iommu: Fix xlate trace to print translated address

Posted by David Gibson 4 years, 8 months ago

On Mon, Aug 12, 2019 at 03:42:02PM +1000, Alexey Kardashevskiy wrote:
> Currently we basically print IO address twice, fix this.
> 
> Fixes: 7e472264e9e2 ("PPC: spapr: iommu: rework traces")
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Applied to ppc-for-4.2, thanks.

> ---
>  hw/ppc/spapr_iommu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index bd3d0256a65d..6fe57d799a10 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -137,7 +137,7 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(IOMMUMemoryRegion *iommu,
>          ret.addr_mask = ~page_mask;
>          ret.perm = spapr_tce_iommu_access_flags(tce);
>      }
> -    trace_spapr_iommu_xlate(tcet->liobn, addr, ret.iova, ret.perm,
> +    trace_spapr_iommu_xlate(tcet->liobn, addr, ret.translated_addr, ret.perm,
>                              ret.addr_mask);
>  
>      return ret;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [Qemu-devel] [PATCH qemu] spapr_iommu: Fix xlate trace to print translated address

Posted by Philippe Mathieu-Daudé 4 years, 8 months ago

Hi Alexey,

On 8/12/19 7:42 AM, Alexey Kardashevskiy wrote:
> Currently we basically print IO address twice, fix this.
> 
> Fixes: 7e472264e9e2 ("PPC: spapr: iommu: rework traces")
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  hw/ppc/spapr_iommu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index bd3d0256a65d..6fe57d799a10 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -137,7 +137,7 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(IOMMUMemoryRegion *iommu,
>          ret.addr_mask = ~page_mask;
>          ret.perm = spapr_tce_iommu_access_flags(tce);
>      }
> -    trace_spapr_iommu_xlate(tcet->liobn, addr, ret.iova, ret.perm,
> +    trace_spapr_iommu_xlate(tcet->liobn, addr, ret.translated_addr, ret.perm,
>                              ret.addr_mask);

But the trace format is:

spapr_iommu_xlate(uint64_t liobn, uint64_t ioba, uint64_t tce, unsigned
perm, unsigned pgsize) "liobn=%"PRIx64" 0x%"PRIx64" -> 0x%"PRIx64"
perm=%u mask=%x"

So this could be more appropriate:

  trace_spapr_iommu_xlate(tcet->liobn, ret.iova, ret.translated_addr,
                          ret.perm, ret.addr_mask);

Anyhow your patch is an improvment, so regardless addr/ret.iova:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>  
>      return ret;
>

Re: [Qemu-devel] [PATCH qemu] spapr_iommu: Fix xlate trace to print translated address

Posted by Alexey Kardashevskiy 4 years, 8 months ago


On 12/08/2019 19:01, Philippe Mathieu-Daudé wrote:
> Hi Alexey,
> 
> On 8/12/19 7:42 AM, Alexey Kardashevskiy wrote:
>> Currently we basically print IO address twice, fix this.
>>
>> Fixes: 7e472264e9e2 ("PPC: spapr: iommu: rework traces")
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>   hw/ppc/spapr_iommu.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
>> index bd3d0256a65d..6fe57d799a10 100644
>> --- a/hw/ppc/spapr_iommu.c
>> +++ b/hw/ppc/spapr_iommu.c
>> @@ -137,7 +137,7 @@ static IOMMUTLBEntry spapr_tce_translate_iommu(IOMMUMemoryRegion *iommu,
>>           ret.addr_mask = ~page_mask;
>>           ret.perm = spapr_tce_iommu_access_flags(tce);
>>       }
>> -    trace_spapr_iommu_xlate(tcet->liobn, addr, ret.iova, ret.perm,
>> +    trace_spapr_iommu_xlate(tcet->liobn, addr, ret.translated_addr, ret.perm,
>>                               ret.addr_mask);
> 
> But the trace format is:
> 
> spapr_iommu_xlate(uint64_t liobn, uint64_t ioba, uint64_t tce, unsigned
> perm, unsigned pgsize) "liobn=%"PRIx64" 0x%"PRIx64" -> 0x%"PRIx64"
> perm=%u mask=%x"
> 
> So this could be more appropriate:
> 
>    trace_spapr_iommu_xlate(tcet->liobn, ret.iova, ret.translated_addr,
>                            ret.perm, ret.addr_mask);
> 
> Anyhow your patch is an improvment, so regardless addr/ret.iova:


I'd rather want to see the raw input data than with some bits removed.

> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Thanks!

> 
>>   
>>       return ret;
>>

-- 
Alexey