Hi Shameer,
On 9/29/25 3:36 PM, Shameer Kolothum wrote:
> QEMU SMMUv3 does not enable ATS (Address Translation Services) by default.
> When accelerated mode is enabled and the host SMMUv3 supports ATS, it can
> be useful to report ATS capability to the guest so it can take advantage
> of it if the device also supports ATS.
>
> Note: ATS support cannot be reliably detected from the host SMMUv3 IDR
> registers alone, as firmware ACPI IORT tables may override them. The
> user must therefore ensure the support before enabling it.
This looks incomplete to me. ATS is a big topic in itself. I would
prefer we do not advertise it until we do not have full support for it
(including emulation). Comparing to
c049bf5bb9dd ("intel_iommu: Add support for ATS") which was recently
contributed we miss at least translation request implementations
(PCIIOMMUOps ats_request_translation callback implementation).See: https://lore.kernel.org/all/[email protected]/#r Also in SMMU spec I see other stuff like STE.EATS, ATS skipping stage 1, ... Here I only see SMMU_CMD_ATC_INV and this is only implemented for accel. I think I would rather stick to the minimum in this series, ie. reject in case of ATS mismatch (for testing purpose you will just need a small hack until we get ATS support), work on ATS enablement in parallel . Thanks Eric > > Signed-off-by: Shameer Kolothum <[email protected]> > --- > hw/arm/smmuv3-accel.c | 4 ++++ > hw/arm/smmuv3.c | 25 ++++++++++++++++++++++++- > hw/arm/virt-acpi-build.c | 10 ++++++++-- > include/hw/arm/smmuv3.h | 1 + > 4 files changed, 37 insertions(+), 3 deletions(-) > > diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c > index e8607b253e..eee54316bf 100644 > --- a/hw/arm/smmuv3-accel.c > +++ b/hw/arm/smmuv3-accel.c > @@ -644,6 +644,10 @@ void smmuv3_accel_idr_override(SMMUv3State *s) > if (!s->ril) { > s->idr[3] = FIELD_DP32(s->idr[3], IDR3, RIL, 0); > } > + /* QEMU SMMUv3 has no ATS. Update IDR0 if user has enabled it */ > + if (s->ats) { > + s->idr[0] = FIELD_DP32(s->idr[0], IDR0, ATS, 1); /* ATS */ > + } > } > > /* > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c > index 0f3a61646a..77d46a9cd6 100644 > --- a/hw/arm/smmuv3.c > +++ b/hw/arm/smmuv3.c > @@ -1510,13 +1510,28 @@ static int smmuv3_cmdq_consume(SMMUv3State *s) > */ > smmuv3_range_inval(bs, &cmd, SMMU_STAGE_2); > break; > + case SMMU_CMD_ATC_INV: > + { > + SMMUDevice *sdev = smmu_find_sdev(bs, CMD_SID(&cmd)); > + Error *local_err = NULL; > + > + if (!sdev) { > + break; > + } > + > + if (!smmuv3_accel_issue_inv_cmd(s, &cmd, sdev, &local_err)) { > + error_report_err(local_err); > + cmd_error = SMMU_CERROR_ILL; > + break; > + } > + break; > + } > case SMMU_CMD_TLBI_EL3_ALL: > case SMMU_CMD_TLBI_EL3_VA: > case SMMU_CMD_TLBI_EL2_ALL: > case SMMU_CMD_TLBI_EL2_ASID: > case SMMU_CMD_TLBI_EL2_VA: > case SMMU_CMD_TLBI_EL2_VAA: > - case SMMU_CMD_ATC_INV: > case SMMU_CMD_PRI_RESP: > case SMMU_CMD_RESUME: > case SMMU_CMD_STALL_TERM: > @@ -1934,6 +1949,10 @@ static bool smmu_validate_property(SMMUv3State *s, > Error **errp) > error_setg(errp, "ril can only be disabled if accel=on"); > return false; > } > + if (s->ats) { > + error_setg(errp, "ats can only be enabled if accel=on"); > + return false; > + } > return true; > } > > @@ -2058,6 +2077,7 @@ static const Property smmuv3_properties[] = { > DEFINE_PROP_BOOL("accel", SMMUv3State, accel, false), > /* RIL can be turned off for accel cases */ > DEFINE_PROP_BOOL("ril", SMMUv3State, ril, true), > + DEFINE_PROP_BOOL("ats", SMMUv3State, ats, false), > }; > > static void smmuv3_instance_init(Object *obj) > @@ -2087,6 +2107,9 @@ static void smmuv3_class_init(ObjectClass *klass, const > void *data) > "in nested mode for vfio-pci dev > assignment"); > object_class_property_set_description(klass, "ril", > "Enable/disable range invalidation support"); > + object_class_property_set_description(klass, "ats", > + "Enable/disable ATS support. Please ensure host platform has ATS " > + "support before enabling this"); > } > > static int smmuv3_notify_flag_changed(IOMMUMemoryRegion *iommu, > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index d0c1e10019..a53f0229b8 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -322,6 +322,7 @@ typedef struct AcpiIortSMMUv3Dev { > /* Offset of the SMMUv3 IORT Node relative to the start of the IORT */ > size_t offset; > bool accel; > + bool ats; > } AcpiIortSMMUv3Dev; > > /* > @@ -377,6 +378,7 @@ static int iort_smmuv3_devices(Object *obj, void *opaque) > > bus = PCI_BUS(object_property_get_link(obj, "primary-bus", > &error_abort)); > sdev.accel = object_property_get_bool(obj, "accel", &error_abort); > + sdev.ats = object_property_get_bool(obj, "ats", &error_abort); > pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev); > sbdev = SYS_BUS_DEVICE(obj); > sdev.base = platform_bus_get_mmio_addr(pbus, sbdev, 0); > @@ -511,6 +513,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, > VirtMachineState *vms) > int i, nb_nodes, rc_mapping_count; > AcpiIortSMMUv3Dev *sdev; > size_t node_size; > + bool ats_needed = false; > int num_smmus = 0; > uint32_t id = 0; > int rc_smmu_idmaps_len = 0; > @@ -546,6 +549,9 @@ build_iort(GArray *table_data, BIOSLinker *linker, > VirtMachineState *vms) > /* Calculate RMR nodes required. One per SMMUv3 with accelerated > mode */ > for (i = 0; i < num_smmus; i++) { > sdev = &g_array_index(smmuv3_devs, AcpiIortSMMUv3Dev, i); > + if (sdev->ats) { > + ats_needed = true; > + } > if (sdev->accel) { > nb_nodes++; > } > @@ -645,8 +651,8 @@ build_iort(GArray *table_data, BIOSLinker *linker, > VirtMachineState *vms) > build_append_int_noprefix(table_data, 0, 2); /* Reserved */ > /* Table 15 Memory Access Flags */ > build_append_int_noprefix(table_data, 0x3 /* CCA = CPM = DACS = 1 */, 1); > - > - build_append_int_noprefix(table_data, 0, 4); /* ATS Attribute */ > + /* ATS Attribute */ > + build_append_int_noprefix(table_data, (ats_needed ? 1 : 0), 4); > /* MCFG pci_segment */ > build_append_int_noprefix(table_data, 0, 4); /* PCI Segment number */ > > diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h > index c555ea684e..6f07baa144 100644 > --- a/include/hw/arm/smmuv3.h > +++ b/include/hw/arm/smmuv3.h > @@ -69,6 +69,7 @@ struct SMMUv3State { > struct SMMUv3AccelState *s_accel; > Error *migration_blocker; > bool ril; > + bool ats; > }; > > typedef enum {
