Re: [PATCH v2 2/8] iommu/arm-smmu-v3: Use S2FWB when available

Jason Gunthorpe
2024-08-28T18:31:57Z

On Tue, Aug 27, 2024 at 12:48:13PM -0700, Nicolin Chen wrote:
> Hi Jason,
>
> On Tue, Aug 27, 2024 at 12:51:32PM -0300, Jason Gunthorpe wrote:
> > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> > index f5d9fd1f45bf49..9b3658aae21005 100644
> > --- a/drivers/iommu/io-pgtable-arm.c
> > +++ b/drivers/iommu/io-pgtable-arm.c
> > @@ -106,6 +106,18 @@
> > #define ARM_LPAE_PTE_HAP_FAULT (((arm_lpae_iopte)0) << 6)
> > #define ARM_LPAE_PTE_HAP_READ (((arm_lpae_iopte)1) << 6)
> > #define ARM_LPAE_PTE_HAP_WRITE (((arm_lpae_iopte)2) << 6)
> > +/*
> > + * For !FWB these code to:
> > + * 1111 = Normal outer write back cachable / Inner Write Back Cachable
> > + * Permit S1 to override
> > + * 0101 = Normal Non-cachable / Inner Non-cachable
> > + * 0001 = Device / Device-nGnRE
> > + * For S2FWB these code:
> > + * 0110 Force Normal Write Back
> > + * 0101 Normal* is forced Normal-NC, Device unchanged
> > + * 0001 Force Device-nGnRE
> > + */
> > +#define ARM_LPAE_PTE_MEMATTR_FWB_WB (((arm_lpae_iopte)0x6) << 2)
>
> The other part looks good. Yet, would you mind sharing the location
> that defines this 0x6 explicitly?

I'm looking at an older one ARM DDI 0487F.c

D5.5.5 Stage 2 memory region type and Cacheability attributes when FEAT_S2FWB is implemented

The text talks about the bits in the PTE, not relative to the MEMATTR
field, so 6 << 2 encodes to:

543210
011000

Then see table D5-40 Effect of bit[4] == 1 on Cacheability and Memory Type)

Bit[5] = 0 = is RES0.
Bit[4] = 1 = determines the interpretation of bits [3:2].
Bits[3:2] == 10 == Normal Write-Back

Here Bit means 'bit of the PTE' because the MemAttr does not have 5
bits.

> Where it has the followings in D8.6.6:
> "For stage 2 translations, if FEAT_MTE_PERM is not implemented, then
> FEAT_S2FWB has all of the following effects on the MemAttr[3:2] bits:
> - MemAttr[3] is RES0.
> - The value of MemAttr[2] determines the interpretation of the
> MemAttr[1:0] bits.

And here the text switches from talking about the PTE bits to the
Field bits. MemAttr[3] == PTE[5], and the above text matches D5.5

The use of numbering schemes relative to the start of the field and
also relative to the PTE is tricky.

Jason