Re: [RFC PATCH 8/8] hw/arm/smmuv3-accel: Introduce _AUTO support for OAS

[Markus Armbruster <armbru@redhat.com>]

Nathan Chen <nathanc@nvidia.com> writes:

> From: Nathan Chen <nathanc@nvidia.com>
>
> Allow accelerated SMMUv3 OAS property to be derived from host IOMMU
> capabilities. Derive host values using IOMMU_GET_HW_INFO, retrieving
> OAS from IDR5.
>
> Set the default oas value to auto. The default Output Address Size used
> to be 44-bit, but we change it to match what the host IOMMU properties
> report so that users do not have to introspect host IDR5 for the OAS.
> This keeps the OAS value advertised by the virtual SMMU compatible with
> the capabilities of the host SMMUv3, so that the intermediate physical
> addresses (IPA) consumed by host SMMU for stage-2 translation do not
> exceed the host's max supported IPA size.
>
> Signed-off-by: Nathan Chen <nathanc@nvidia.com>
> ---
>  hw/arm/smmuv3-accel.c   | 11 +++++++++--
>  hw/arm/smmuv3.c         | 11 ++++++-----
>  include/hw/arm/smmuv3.h |  2 +-
>  3 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index bd27b0da7c..03950a4cef 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -71,6 +71,12 @@ static void smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
>                                 FIELD_EX32(info->idr[1], IDR1, SSIDSIZE));
>      }
>  
> +    /* Update OAS if auto from info */
> +    if (s->oas == OAS_MODE_AUTO) {
> +        s->idr[5] = FIELD_DP32(s->idr[5], IDR5, OAS,
> +                               FIELD_EX32(info->idr[5], IDR5, OAS));
> +    }
> +
>      accel->auto_finalised = true;
>  }
>  
> @@ -898,7 +904,7 @@ void smmuv3_accel_idr_override(SMMUv3State *s)
>      }
>  
>      /* Advertise 48-bit OAS in IDR5 when requested (default is 44 bits). */
> -    if (s->oas == SMMU_OAS_48BIT) {
> +    if (s->oas == OAS_MODE_48) {
>          s->idr[5] = FIELD_DP32(s->idr[5], IDR5, OAS, SMMU_IDR5_OAS_48);
>      }
>  
> @@ -979,7 +985,8 @@ void smmuv3_accel_init(SMMUv3State *s)
>  
>      if (s->ats == ON_OFF_AUTO_AUTO ||
>          s->ril == ON_OFF_AUTO_AUTO ||
> -        s->ssidsize == SSID_SIZE_MODE_AUTO) {
> +        s->ssidsize == SSID_SIZE_MODE_AUTO ||
> +        s->oas == OAS_MODE_AUTO) {
>          s->s_accel->auto_mode = true;
>      }
>  }
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index bc03353759..4fc4ed2c06 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -1982,7 +1982,7 @@ static bool smmu_validate_property(SMMUv3State *s, Error **errp)
>              error_setg(errp, "ats can only be enabled if accel=on");
>              return false;
>          }
> -        if (s->oas != SMMU_OAS_44BIT) {
> +        if (s->oas > OAS_MODE_44) {
>              error_setg(errp, "OAS must be 44 bits when accel=off");
>              return false;
>          }
> @@ -2000,8 +2000,9 @@ static bool smmu_validate_property(SMMUv3State *s, Error **errp)
>          return false;
>      }
>  
> -    if (s->oas != SMMU_OAS_44BIT && s->oas != SMMU_OAS_48BIT) {
> -        error_setg(errp, "OAS can only be set to 44 or 48 bits");
> +    if (s->oas != OAS_MODE_AUTO && s->oas != OAS_MODE_44 &&
> +        s->oas != OAS_MODE_48) {
> +        error_setg(errp, "OAS can only be set to auto, 44 bits, or 48 bits");
>          return false;
>      }
>  
> @@ -2131,7 +2132,7 @@ static const Property smmuv3_properties[] = {
>      /* RIL can be turned off for accel cases */
>      DEFINE_PROP_ON_OFF_AUTO("ril", SMMUv3State, ril, ON_OFF_AUTO_AUTO),
>      DEFINE_PROP_ON_OFF_AUTO("ats", SMMUv3State, ats, ON_OFF_AUTO_AUTO),
> -    DEFINE_PROP_UINT8("oas", SMMUv3State, oas, 44),
> +    DEFINE_PROP_OAS_MODE("oas", SMMUv3State, oas, OAS_MODE_AUTO),


Is property "oas" accessible via QMP or JSON command line?  If yes, this
is an incompatible change: JSON integer values no longer work.

>      DEFINE_PROP_SSIDSIZE_MODE("ssidsize", SMMUv3State, ssidsize,
>                                SSID_SIZE_MODE_AUTO),
>  };
> @@ -2168,7 +2169,7 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
>          "platform has ATS support before enabling this");
>      object_class_property_set_description(klass, "oas",
>          "Specify Output Address Size (for accel=on). Supported values "
> -        "are 44 or 48 bits. Defaults to 44 bits");
> +        "are 44 or 48 bits.");
>      object_class_property_set_description(klass, "ssidsize",
>          "Number of bits used to represent SubstreamIDs (SSIDs). "
>          "A value of N allows SSIDs in the range [0 .. 2^N - 1]. "
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index ae8158a5c3..3bfee63396 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -72,7 +72,7 @@ struct SMMUv3State {
>      Error *migration_blocker;
>      OnOffAuto ril;
>      OnOffAuto ats;
> -    uint8_t oas;
> +    OasMode oas;
>      SsidSizeMode ssidsize;
>  };