Re: [PATCH 0/3] hw/arm/smmuv3: Advertise SMMUv3.1-XNX

[Mostafa Saleh <smostafa@google.com>]

Hi Peter,

On Thu, Sep 14, 2023 at 03:57:02PM +0100, Peter Maydell wrote:
> The SMMUv3.1-XNX feature is mandatory for an SMMUv3.1 if S2P is
> supported, so we should theoretically have implemented it as part of
> the recent S2P work.  Fortunately, for us the implementation is a
> no-op.
Oh, I missed that, thanks for spotting it.

> This feature is about interpretation of the stage 2 page table
> descriptor XN bits, which control execute permissions.
> 
> For QEMU, the permission bits passed to an IOMMU (via MemTxAttrs and
> IOMMUAccessFlags) only indicate read and write; we do not distinguish
> data reads from instruction reads outside the CPU proper.  In the
> SMMU architecture's terms, our interconnect between the client device
> and the SMMU doesn't have the ability to convey the INST attribute,
> and we therefore use the default value of "data" for this attribute.
> 
> We also do not support the bits in the Stream Table Entry that can
> override the on-the-bus transaction attribute permissions (we do not
> set SMMU_IDR1.ATTR_PERMS_OVR=1).
> 
> These two things together mean that for our implementation, it never
> has to deal with transactions with the INST attribute, and so it can
> correctly ignore the XN bits entirely.  So we already implement
> FEAT_XNX's "XN field is now 2 bits, not 1" behaviour to the extent
> that we need to.
> 
> Patches 1 and 2 in this series do a little bit of tidy up
> on the ID register bit code. Patch 3 is the one-liner to
> advertise SMMUv3.1-XNX in the ID register.
> 
> thanks
> -- PMM
> 
> Peter Maydell (3):
>   hw/arm/smmuv3: Update ID register bit field definitions
>   hw/arm/smmuv3: Sort ID register setting into field order
>   hw/arm/smmuv3: Advertise SMMUv3.1-XNX feature
> 
>  hw/arm/smmuv3-internal.h | 38 ++++++++++++++++++++++++++++++++++++++
>  hw/arm/smmuv3.c          |  5 +++--
>  2 files changed, 41 insertions(+), 2 deletions(-)
> 
> -- 
> 2.34.1

I left a comment/question on the last patch, otherwise

Reviewed-by: Mostafa Saleh <smostafa@google.com>

Thanks,
Mostafa