[PATCH 4/5] hw/arm/smmuv3: Use correct bit positions in EVT_SET_ADDR2 macro

Simon Veith posted 5 patches 6 years, 2 months ago
Maintainers: Eric Auger <eric.auger@redhat.com>, Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[PATCH 4/5] hw/arm/smmuv3: Use correct bit positions in EVT_SET_ADDR2 macro
Posted by Simon Veith 6 years, 2 months ago
The bit offsets in the EVT_SET_ADDR2 macro do not match those specified
in the ARM SMMUv3 Architecture Specification. In all events that use
this macro, e.g. F_WALK_EABT, the faulting fetch address or IPA actually
occupies the 32-bit words 6 and 7 in the event record contiguously, with
the upper and lower unused bits clear due to alignment or maximum
supported address bits. How many bits are clear depends on the
individual event type.

Update the macro to write to the correct words in the event record so
that guest drivers can obtain accurate address information on events.

ref. ARM IHI 0070C, sections 7.3.12 through 7.3.16.

Signed-off-by: Simon Veith <sveith@amazon.de>
Cc: Eric Auger <eric.auger@redhat.com>
Cc: qemu-devel@nongnu.org
Cc: qemu-arm@nongnu.org
---
 hw/arm/smmuv3-internal.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index d190181..eb275e2 100644
--- a/hw/arm/smmuv3-internal.h
+++ b/hw/arm/smmuv3-internal.h
@@ -461,8 +461,8 @@ typedef struct SMMUEventInfo {
     } while (0)
 #define EVT_SET_ADDR2(x, addr)                            \
     do {                                                  \
-            (x)->word[7] = deposit32((x)->word[7], 3, 29, addr >> 16);   \
-            (x)->word[7] = deposit32((x)->word[7], 0, 16, addr & 0xffff);\
+            (x)->word[7] = (uint32_t)(addr >> 32);        \
+            (x)->word[6] = (uint32_t)(addr & 0xffffffff); \
     } while (0)
 
 void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *event);
-- 
2.7.4


Re: [PATCH 4/5] hw/arm/smmuv3: Use correct bit positions in EVT_SET_ADDR2 macro
Posted by Auger Eric 6 years, 2 months ago
Hi Simon,

On 12/4/19 2:55 PM, Simon Veith wrote:
> The bit offsets in the EVT_SET_ADDR2 macro do not match those specified
> in the ARM SMMUv3 Architecture Specification. In all events that use
> this macro, e.g. F_WALK_EABT, the faulting fetch address or IPA actually
> occupies the 32-bit words 6 and 7 in the event record contiguously, with
> the upper and lower unused bits clear due to alignment or maximum
> supported address bits. How many bits are clear depends on the
> individual event type.
> 
> Update the macro to write to the correct words in the event record so
> that guest drivers can obtain accurate address information on events.
> 
> ref. ARM IHI 0070C, sections 7.3.12 through 7.3.16.
> 
> Signed-off-by: Simon Veith <sveith@amazon.de>
Acked-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: qemu-devel@nongnu.org
> Cc: qemu-arm@nongnu.org
> ---
>  hw/arm/smmuv3-internal.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index d190181..eb275e2 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -461,8 +461,8 @@ typedef struct SMMUEventInfo {
>      } while (0)
>  #define EVT_SET_ADDR2(x, addr)                            \
>      do {                                                  \
> -            (x)->word[7] = deposit32((x)->word[7], 3, 29, addr >> 16);   \
> -            (x)->word[7] = deposit32((x)->word[7], 0, 16, addr & 0xffff);\
> +            (x)->word[7] = (uint32_t)(addr >> 32);        \
> +            (x)->word[6] = (uint32_t)(addr & 0xffffffff); \
>      } while (0)
>  
>  void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo *event);
>