Re: [PATCH 3/3] hw/arm/smmuv3: Advertise support for SMMUv3.2-BBML2

[Eric Auger <eric.auger@redhat.com>]

Hi Peter,

On 4/26/22 18:04, Peter Maydell wrote:
> The Arm SMMUv3 includes an optional feature equivalent to the CPU
> FEAT_BBM, which permits an OS to switch a range of memory between
> "covered by a huge page" and "covered by a sequence of normal pages"
> without having to engage in the traditional 'break-before-make'
> dance. (This is particularly important for the SMMU, because devices
> performing I/O through an SMMU are less likely to be able to cope with
> the window in the sequence where an access results in a translation
> fault.)  The SMMU spec explicitly notes that one of the valid ways to
> be a BBM level 2 compliant implementation is:
>  * if there are multiple entries in the TLB for an address,
>    choose one of them and use it, ignoring the others
>
> Our SMMU TLB implementation (unlike our CPU TLB) does allow multiple
> TLB entries for an address, because the translation table level is
> part of the SMMUIOTLBKey, and so our IOTLB hashtable can include
> entries for the same address where the leaf was at different levels
> (i.e. both hugepage and normal page). Our TLB lookup implementation in
> smmu_iotlb_lookup() will always find the entry with the lowest level
> (i.e. it prefers the hugepage over the normal page) and ignore any
> others. TLB invalidation correctly removes all TLB entries matching
> the specified address or address range (unless the guest specifies the
> leaf level explicitly, in which case it gets what it asked for). So we
"

unless the guest specifies the
leaf level explicitly, in which case it gets what it asked for

"
This is the less obvious part as the spec says:

"A TLB invalidation operation removes all matching TLB entries even if
overlapping entries exist for a given
address."

I failed to find further precisions about the range invalidation & BBML.

If you are confident about this, it looks good to me.
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric




> can validly advertise support for BBML level 2.
>
> Note that we still can't yet advertise ourselves as an SMMU v3.2,
> because v3.2 requires support for the S2FWB feature, which we don't
> yet implement.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/smmuv3-internal.h | 1 +
>  hw/arm/smmuv3.c          | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index d1885ae3f25..e9150a6ff33 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -56,6 +56,7 @@ REG32(IDR2,                0x8)
>  REG32(IDR3,                0xc)
>       FIELD(IDR3, HAD,         2, 1);
>       FIELD(IDR3, RIL,        10, 1);
> +     FIELD(IDR3, BBML,       11, 2);
>  REG32(IDR4,                0x10)
>  REG32(IDR5,                0x14)
>       FIELD(IDR5, OAS,         0, 3);
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 707eb430c23..74bc2e85ee4 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -259,6 +259,7 @@ static void smmuv3_init_regs(SMMUv3State *s)
>  
>      s->idr[3] = FIELD_DP32(s->idr[3], IDR3, RIL, 1);
>      s->idr[3] = FIELD_DP32(s->idr[3], IDR3, HAD, 1);
> +    s->idr[3] = FIELD_DP32(s->idr[3], IDR3, BBML, 2);
>  
>      /* 4K, 16K and 64K granule support */
>      s->idr[5] = FIELD_DP32(s->idr[5], IDR5, GRAN4K, 1);