Nathan Chen <[email protected]> writes:
> From: Nathan Chen <[email protected]>
>
> Introduce smmuv3_accel_auto_finalise() to resolve properties
> that are set to 'auto' for accelerated SMMUv3. This helper
> function allows properties such as ATS, RIL, SSIDSIZE, and OAS
> support to be resolved from host IOMMU values, while avoiding
> triggering auto-resolved values for hot-plugged devices.
>
> Auto mode requires at least one cold-plugged device to retrieve
> and finalise these properties, and we fail boot if that is not
> the case.
>
> Subsequent patches will make use of this helper to set the
> values when we convert the values to OnOffAuto. New auto_mode
> and auto_finalised bool members are added to SMMUv3AccelState.
> smmuv3_accel_init() will set auto_mode to true when 'auto' is
> detected for the accel SMMUv3 properties.
> smmuv3_accel_auto_finalise() will set auto_finalised to true
> after all 'auto' properties are resolved, and subsequent
> calls to this function will return early if auto_finalised is
> set to true.
>
> Suggested-by: Shameer Kolothum <[email protected]>
> Signed-off-by: Nathan Chen <[email protected]>
> ---
> hw/arm/smmuv3-accel.c | 38 +++++++++++++++++++++++++++++++++-----
> hw/arm/smmuv3-accel.h | 2 ++
> 2 files changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 17306cd04b..617629bacd 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -35,11 +35,34 @@ static int smmuv3_oas_bits(uint32_t oas)
> return map[oas];
> }
>
> +static void smmuv3_accel_auto_finalise(SMMUv3State *s, PCIDevice *pdev,
> + struct iommu_hw_info_arm_smmuv3
> *info) {
> + SMMUv3AccelState *accel = s->s_accel;
> +
> + /* Return if no auto for any or finalised already */
> + if (!accel->auto_mode || accel->auto_finalised) {
> + return;
> + }
> +
> + /* We can't update if device is hotplugged */
> + if (DEVICE(pdev)->hotplugged) {
> + warn_report("arm-smmuv3: 'auto' feature property detected, but host "
> + "value cannot be applied for hot-plugged device; using "
> + "existing value");
Why is this warning useful?
Does @auto's meaning depend on whether the device is cold- or
hot-plugged?
> + return;
> + }
> +
> + accel->auto_finalised = true;
> +}
> +
> static bool
> smmuv3_accel_check_hw_compatible(SMMUv3State *s,
> struct iommu_hw_info_arm_smmuv3 *info,
> + PCIDevice *pdev,
> Error **errp)
> {
> + smmuv3_accel_auto_finalise(s, pdev, info);
> +
> /* QEMU SMMUv3 supports both linear and 2-level stream tables */
> if (FIELD_EX32(info->idr[0], IDR0, STLEVEL) !=
> FIELD_EX32(s->idr[0], IDR0, STLEVEL)) {
> @@ -124,7 +147,7 @@ smmuv3_accel_check_hw_compatible(SMMUv3State *s,
>
> static bool
> smmuv3_accel_hw_compatible(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> - Error **errp)
> + PCIDevice *pdev, Error **errp)
> {
> struct iommu_hw_info_arm_smmuv3 info;
> uint32_t data_type;
> @@ -142,7 +165,7 @@ smmuv3_accel_hw_compatible(SMMUv3State *s,
> HostIOMMUDeviceIOMMUFD *idev,
> return false;
> }
>
> - if (!smmuv3_accel_check_hw_compatible(s, &info, errp)) {
> + if (!smmuv3_accel_check_hw_compatible(s, &info, pdev, errp)) {
> return false;
> }
> return true;
> @@ -595,6 +618,7 @@ static bool smmuv3_accel_set_iommu_device(PCIBus *bus,
> void *opaque, int devfn,
> SMMUv3State *s = ARM_SMMUV3(bs);
> SMMUPciBus *sbus = smmu_get_sbus(bs, bus);
> SMMUv3AccelDevice *accel_dev = smmuv3_accel_get_dev(bs, sbus, bus,
> devfn);
> + PCIDevice *pdev = pci_find_device(bus, pci_bus_num(bus), devfn);
>
> if (!idev) {
> return true;
> @@ -613,7 +637,7 @@ static bool smmuv3_accel_set_iommu_device(PCIBus *bus,
> void *opaque, int devfn,
> * Check the host SMMUv3 associated with the dev is compatible with the
> * QEMU SMMUv3 accel.
> */
> - if (!smmuv3_accel_hw_compatible(s, idev, errp)) {
> + if (!smmuv3_accel_hw_compatible(s, idev, pdev, errp)) {
> return false;
> }
>
> @@ -867,8 +891,12 @@ bool smmuv3_accel_attach_gbpa_hwpt(SMMUv3State *s, Error
> **errp)
>
> void smmuv3_accel_reset(SMMUv3State *s)
> {
> - /* Attach a HWPT based on GBPA reset value */
> - smmuv3_accel_attach_gbpa_hwpt(s, NULL);
> + if (s->s_accel && s->s_accel->auto_mode && !s->s_accel->auto_finalised) {
> + error_report("AUTO mode specified but properties not finalised.");
> + exit(1);
How can we get here?
> + }
> + /* Attach a HWPT based on GBPA reset value */
> + smmuv3_accel_attach_gbpa_hwpt(s, NULL);
> }
>
> static void smmuv3_accel_as_init(SMMUv3State *s)
> diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
> index dba6c71de5..3c1cd55714 100644
> --- a/hw/arm/smmuv3-accel.h
> +++ b/hw/arm/smmuv3-accel.h
> @@ -26,6 +26,8 @@ typedef struct SMMUv3AccelState {
> uint32_t bypass_hwpt_id;
> uint32_t abort_hwpt_id;
> QLIST_HEAD(, SMMUv3AccelDevice) device_list;
> + bool auto_mode;
> + bool auto_finalised;
> } SMMUv3AccelState;
>
> typedef struct SMMUS1Hwpt {
