> On 23 Mar 2022, at 12:40, Robin Murphy <[email protected]> wrote: > > On 2022-03-23 12:54, Miguel Luis wrote: >> Allows userspace to check for SMMUv3 features. > > What will userspace do with that information? > > It hardly matters what fancy new features might be present, if the kernel > and/or the abstracted interfaces available to userspace aren't using them. > Any functionality which is supported by a usable interface should ideally be > represented via that interface itself. >
The inspiration was the same that Intel (cap/ecap) and AMD (cap/features) took exposing it's iommu feature registers on sysfs. It's an easy way to understand which features are supported by the hardware regardless of what the kernel supports. For example I could print the smmu->features and that would cover kernel supported features but wouldn't help when new hardware arrives to know which features are supported by the hardware. > Furthermore many of the raw SMMU features depend on other system components > and/or firmware, so the ID registers alone don't tell the full story anyway. > Would you mind to elaborate a bit more on that please? Would that mean that if a feature bit is set it doesn’t really tell that the feature is supported? >> Expose the following RO registers related to ARM SMMUv3 via sysfs: >> SMMU_IDR0 >> SMMU_IDR1 >> SMMU_IDR2 >> SMMU_IDR3 >> SMMU_IDR4 >> SMMU_IDR5 >> SMMU_IDR6 >> SMMU_IIDR >> SMMU_AIDR >> # find /sys | grep arm-iommu >> /sys/devices/platform/9050000.smmuv3/iommu/smmu3.0x0000000009050000/arm-iommu > > Nit: my main comment above notwithstanding, is this level of hierarchcy > meaningful or useful? "arm-iommu" isn't an established name for anything as > far as I'm aware. > I've followed the existent convention in other IOMMUs but I'm totally open to alternatives/suggestions. Miguel > Robin. > >> /sys/devices/platform/9050000.smmuv3/iommu/smmu3.0x0000000009050000/arm-iommu/smmu_idr5 >> /sys/devices/platform/9050000.smmuv3/iommu/smmu3.0x0000000009050000/arm-iommu/smmu_idr3 >> /sys/devices/platform/9050000.smmuv3/iommu/smmu3.0x0000000009050000/arm-iommu/smmu_idr1 >> /sys/devices/platform/9050000.smmuv3/iommu/smmu3.0x0000000009050000/arm-iommu/smmu_aidr >> /sys/devices/platform/9050000.smmuv3/iommu/smmu3.0x0000000009050000/arm-iommu/smmu_idr6 >> /sys/devices/platform/9050000.smmuv3/iommu/smmu3.0x0000000009050000/arm-iommu/smmu_idr4 >> /sys/devices/platform/9050000.smmuv3/iommu/smmu3.0x0000000009050000/arm-iommu/smmu_iidr >> /sys/devices/platform/9050000.smmuv3/iommu/smmu3.0x0000000009050000/arm-iommu/smmu_idr2 >> /sys/devices/platform/9050000.smmuv3/iommu/smmu3.0x0000000009050000/arm-iommu/smmu_idr0 >> Signed-off-by: Miguel Luis <[email protected]> >> --- >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 199 ++++++++++++++++---- >> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 27 +++ >> 2 files changed, 191 insertions(+), 35 deletions(-) >> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> index 6dc6d8b6b368..7f779d3f88f2 100644 >> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c >> @@ -3424,17 +3424,16 @@ static int arm_smmu_device_reset(struct >> arm_smmu_device *smmu, bool bypass) >> static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu) >> { >> - u32 reg; >> bool coherent = smmu->features & ARM_SMMU_FEAT_COHERENCY; >> /* IDR0 */ >> - reg = readl_relaxed(smmu->base + ARM_SMMU_IDR0); >> + smmu->idr0 = readl_relaxed(smmu->base + ARM_SMMU_IDR0); >> /* 2-level structures */ >> - if (FIELD_GET(IDR0_ST_LVL, reg) == IDR0_ST_LVL_2LVL) >> + if (FIELD_GET(IDR0_ST_LVL, smmu->idr0) == IDR0_ST_LVL_2LVL) >> smmu->features |= ARM_SMMU_FEAT_2_LVL_STRTAB; >> - if (reg & IDR0_CD2L) >> + if (smmu->idr0 & IDR0_CD2L) >> smmu->features |= ARM_SMMU_FEAT_2_LVL_CDTAB; >> /* >> @@ -3442,7 +3441,7 @@ static int arm_smmu_device_hw_probe(struct >> arm_smmu_device *smmu) >> * We currently require the same endianness as the CPU, but this >> * could be changed later by adding a new IO_PGTABLE_QUIRK. >> */ >> - switch (FIELD_GET(IDR0_TTENDIAN, reg)) { >> + switch (FIELD_GET(IDR0_TTENDIAN, smmu->idr0)) { >> case IDR0_TTENDIAN_MIXED: >> smmu->features |= ARM_SMMU_FEAT_TT_LE | ARM_SMMU_FEAT_TT_BE; >> break; >> @@ -3461,22 +3460,22 @@ static int arm_smmu_device_hw_probe(struct >> arm_smmu_device *smmu) >> } >> /* Boolean feature flags */ >> - if (IS_ENABLED(CONFIG_PCI_PRI) && reg & IDR0_PRI) >> + if (IS_ENABLED(CONFIG_PCI_PRI) && smmu->idr0 & IDR0_PRI) >> smmu->features |= ARM_SMMU_FEAT_PRI; >> - if (IS_ENABLED(CONFIG_PCI_ATS) && reg & IDR0_ATS) >> + if (IS_ENABLED(CONFIG_PCI_ATS) && smmu->idr0 & IDR0_ATS) >> smmu->features |= ARM_SMMU_FEAT_ATS; >> - if (reg & IDR0_SEV) >> + if (smmu->idr0 & IDR0_SEV) >> smmu->features |= ARM_SMMU_FEAT_SEV; >> - if (reg & IDR0_MSI) { >> + if (smmu->idr0 & IDR0_MSI) { >> smmu->features |= ARM_SMMU_FEAT_MSI; >> if (coherent && !disable_msipolling) >> smmu->options |= ARM_SMMU_OPT_MSIPOLL; >> } >> - if (reg & IDR0_HYP) { >> + if (smmu->idr0 & IDR0_HYP) { >> smmu->features |= ARM_SMMU_FEAT_HYP; >> if (cpus_have_cap(ARM64_HAS_VIRT_HOST_EXTN)) >> smmu->features |= ARM_SMMU_FEAT_E2H; >> @@ -3486,11 +3485,11 @@ static int arm_smmu_device_hw_probe(struct >> arm_smmu_device *smmu) >> * The coherency feature as set by FW is used in preference to the ID >> * register, but warn on mismatch. >> */ >> - if (!!(reg & IDR0_COHACC) != coherent) >> + if (!!(smmu->idr0 & IDR0_COHACC) != coherent) >> dev_warn(smmu->dev, "IDR0.COHACC overridden by FW configuration >> (%s)\n", >> coherent ? "true" : "false"); >> - switch (FIELD_GET(IDR0_STALL_MODEL, reg)) { >> + switch (FIELD_GET(IDR0_STALL_MODEL, smmu->idr0)) { >> case IDR0_STALL_MODEL_FORCE: >> smmu->features |= ARM_SMMU_FEAT_STALL_FORCE; >> fallthrough; >> @@ -3498,19 +3497,19 @@ static int arm_smmu_device_hw_probe(struct >> arm_smmu_device *smmu) >> smmu->features |= ARM_SMMU_FEAT_STALLS; >> } >> - if (reg & IDR0_S1P) >> + if (smmu->idr0 & IDR0_S1P) >> smmu->features |= ARM_SMMU_FEAT_TRANS_S1; >> - if (reg & IDR0_S2P) >> + if (smmu->idr0 & IDR0_S2P) >> smmu->features |= ARM_SMMU_FEAT_TRANS_S2; >> - if (!(reg & (IDR0_S1P | IDR0_S2P))) { >> + if (!(smmu->idr0 & (IDR0_S1P | IDR0_S2P))) { >> dev_err(smmu->dev, "no translation support!\n"); >> return -ENXIO; >> } >> /* We only support the AArch64 table format at present */ >> - switch (FIELD_GET(IDR0_TTF, reg)) { >> + switch (FIELD_GET(IDR0_TTF, smmu->idr0)) { >> case IDR0_TTF_AARCH32_64: >> smmu->ias = 40; >> fallthrough; >> @@ -3522,19 +3521,20 @@ static int arm_smmu_device_hw_probe(struct >> arm_smmu_device *smmu) >> } >> /* ASID/VMID sizes */ >> - smmu->asid_bits = reg & IDR0_ASID16 ? 16 : 8; >> - smmu->vmid_bits = reg & IDR0_VMID16 ? 16 : 8; >> + smmu->asid_bits = smmu->idr0 & IDR0_ASID16 ? 16 : 8; >> + smmu->vmid_bits = smmu->idr0 & IDR0_VMID16 ? 16 : 8; >> /* IDR1 */ >> - reg = readl_relaxed(smmu->base + ARM_SMMU_IDR1); >> - if (reg & (IDR1_TABLES_PRESET | IDR1_QUEUES_PRESET | IDR1_REL)) { >> + smmu->idr1 = readl_relaxed(smmu->base + ARM_SMMU_IDR1); >> + >> + if (smmu->idr1 & (IDR1_TABLES_PRESET | IDR1_QUEUES_PRESET | IDR1_REL)) { >> dev_err(smmu->dev, "embedded implementation not supported\n"); >> return -ENXIO; >> } >> /* Queue sizes, capped to ensure natural alignment */ >> smmu->cmdq.q.llq.max_n_shift = min_t(u32, CMDQ_MAX_SZ_SHIFT, >> - FIELD_GET(IDR1_CMDQS, reg)); >> + FIELD_GET(IDR1_CMDQS, smmu->idr1)); >> if (smmu->cmdq.q.llq.max_n_shift <= ilog2(CMDQ_BATCH_ENTRIES)) { >> /* >> * We don't support splitting up batches, so one batch of >> @@ -3548,13 +3548,13 @@ static int arm_smmu_device_hw_probe(struct >> arm_smmu_device *smmu) >> } >> smmu->evtq.q.llq.max_n_shift = min_t(u32, EVTQ_MAX_SZ_SHIFT, >> - FIELD_GET(IDR1_EVTQS, reg)); >> + FIELD_GET(IDR1_EVTQS, smmu->idr1)); >> smmu->priq.q.llq.max_n_shift = min_t(u32, PRIQ_MAX_SZ_SHIFT, >> - FIELD_GET(IDR1_PRIQS, reg)); >> + FIELD_GET(IDR1_PRIQS, smmu->idr1)); >> /* SID/SSID sizes */ >> - smmu->ssid_bits = FIELD_GET(IDR1_SSIDSIZE, reg); >> - smmu->sid_bits = FIELD_GET(IDR1_SIDSIZE, reg); >> + smmu->ssid_bits = FIELD_GET(IDR1_SSIDSIZE, smmu->idr1); >> + smmu->sid_bits = FIELD_GET(IDR1_SIDSIZE, smmu->idr1); >> /* >> * If the SMMU supports fewer bits than would fill a single L2 stream >> @@ -3563,31 +3563,37 @@ static int arm_smmu_device_hw_probe(struct >> arm_smmu_device *smmu) >> if (smmu->sid_bits <= STRTAB_SPLIT) >> smmu->features &= ~ARM_SMMU_FEAT_2_LVL_STRTAB; >> + /* IDR2 */ >> + smmu->idr2 = readl_relaxed(smmu->base + ARM_SMMU_IDR2); >> + >> /* IDR3 */ >> - reg = readl_relaxed(smmu->base + ARM_SMMU_IDR3); >> - if (FIELD_GET(IDR3_RIL, reg)) >> + smmu->idr3 = readl_relaxed(smmu->base + ARM_SMMU_IDR3); >> + if (FIELD_GET(IDR3_RIL, smmu->idr3)) >> smmu->features |= ARM_SMMU_FEAT_RANGE_INV; >> + /* IDR4 */ >> + smmu->idr4 = readl_relaxed(smmu->base + ARM_SMMU_IDR4); >> + >> /* IDR5 */ >> - reg = readl_relaxed(smmu->base + ARM_SMMU_IDR5); >> + smmu->idr5 = readl_relaxed(smmu->base + ARM_SMMU_IDR5); >> /* Maximum number of outstanding stalls */ >> - smmu->evtq.max_stalls = FIELD_GET(IDR5_STALL_MAX, reg); >> + smmu->evtq.max_stalls = FIELD_GET(IDR5_STALL_MAX, smmu->idr5); >> /* Page sizes */ >> - if (reg & IDR5_GRAN64K) >> + if (smmu->idr5 & IDR5_GRAN64K) >> smmu->pgsize_bitmap |= SZ_64K | SZ_512M; >> - if (reg & IDR5_GRAN16K) >> + if (smmu->idr5 & IDR5_GRAN16K) >> smmu->pgsize_bitmap |= SZ_16K | SZ_32M; >> - if (reg & IDR5_GRAN4K) >> + if (smmu->idr5 & IDR5_GRAN4K) >> smmu->pgsize_bitmap |= SZ_4K | SZ_2M | SZ_1G; >> /* Input address size */ >> - if (FIELD_GET(IDR5_VAX, reg) == IDR5_VAX_52_BIT) >> + if (FIELD_GET(IDR5_VAX, smmu->idr5) == IDR5_VAX_52_BIT) >> smmu->features |= ARM_SMMU_FEAT_VAX; >> /* Output address size */ >> - switch (FIELD_GET(IDR5_OAS, reg)) { >> + switch (FIELD_GET(IDR5_OAS, smmu->idr5)) { >> case IDR5_OAS_32_BIT: >> smmu->oas = 32; >> break; >> @@ -3632,6 +3638,16 @@ static int arm_smmu_device_hw_probe(struct >> arm_smmu_device *smmu) >> dev_info(smmu->dev, "ias %lu-bit, oas %lu-bit (features 0x%08x)\n", >> smmu->ias, smmu->oas, smmu->features); >> + >> + /* IDR6 */ >> + smmu->idr6 = readl_relaxed(smmu->base + ARM_SMMU_IDR6); >> + >> + /* IIDR */ >> + smmu->iidr = readl_relaxed(smmu->base + ARM_SMMU_IIDR); >> + >> + /* AIDR */ >> + smmu->aidr = readl_relaxed(smmu->base + ARM_SMMU_AIDR); >> + >> return 0; >> } >> @@ -3752,6 +3768,119 @@ static void __iomem *arm_smmu_ioremap(struct device >> *dev, resource_size_t start, >> return devm_ioremap_resource(dev, &res); >> } >> +static ssize_t smmu_idr0_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct arm_smmu_device *smmu = dev_to_arm_smmu_device(dev); >> + >> + return sprintf(buf, "%x\n", smmu->idr0); >> +} >> +static DEVICE_ATTR_RO(smmu_idr0); >> + >> +static ssize_t smmu_idr1_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct arm_smmu_device *smmu = dev_to_arm_smmu_device(dev); >> + >> + return sprintf(buf, "%x\n", smmu->idr1); >> +} >> +static DEVICE_ATTR_RO(smmu_idr1); >> + >> +static ssize_t smmu_idr2_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct arm_smmu_device *smmu = dev_to_arm_smmu_device(dev); >> + >> + return sprintf(buf, "%x\n", smmu->idr2); >> +} >> +static DEVICE_ATTR_RO(smmu_idr2); >> + >> +static ssize_t smmu_idr3_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct arm_smmu_device *smmu = dev_to_arm_smmu_device(dev); >> + >> + return sprintf(buf, "%x\n", smmu->idr3); >> +} >> +static DEVICE_ATTR_RO(smmu_idr3); >> + >> +static ssize_t smmu_idr4_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct arm_smmu_device *smmu = dev_to_arm_smmu_device(dev); >> + >> + return sprintf(buf, "%x\n", smmu->idr4); >> +} >> +static DEVICE_ATTR_RO(smmu_idr4); >> + >> +static ssize_t smmu_idr5_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct arm_smmu_device *smmu = dev_to_arm_smmu_device(dev); >> + >> + return sprintf(buf, "%x\n", smmu->idr5); >> +} >> +static DEVICE_ATTR_RO(smmu_idr5); >> + >> +static ssize_t smmu_idr6_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct arm_smmu_device *smmu = dev_to_arm_smmu_device(dev); >> + >> + return sprintf(buf, "%x\n", smmu->idr6); >> +} >> +static DEVICE_ATTR_RO(smmu_idr6); >> + >> +static ssize_t smmu_iidr_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct arm_smmu_device *smmu = dev_to_arm_smmu_device(dev); >> + >> + return sprintf(buf, "%x\n", smmu->iidr); >> +} >> +static DEVICE_ATTR_RO(smmu_iidr); >> + >> +static ssize_t smmu_aidr_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct arm_smmu_device *smmu = dev_to_arm_smmu_device(dev); >> + >> + return sprintf(buf, "%x\n", smmu->aidr); >> +} >> +static DEVICE_ATTR_RO(smmu_aidr); >> + >> +static struct attribute *arm_iommu_attrs[] = { >> + &dev_attr_smmu_idr0.attr, >> + &dev_attr_smmu_idr1.attr, >> + &dev_attr_smmu_idr2.attr, >> + &dev_attr_smmu_idr3.attr, >> + &dev_attr_smmu_idr4.attr, >> + &dev_attr_smmu_idr5.attr, >> + &dev_attr_smmu_idr6.attr, >> + &dev_attr_smmu_iidr.attr, >> + &dev_attr_smmu_aidr.attr, >> + NULL, >> +}; >> + >> +static struct attribute_group arm_iommu_group = { >> + .name = "arm-iommu", >> + .attrs = arm_iommu_attrs, >> +}; >> + >> +static const struct attribute_group *arm_iommu_groups[] = { >> + &arm_iommu_group, >> + NULL, >> +}; >> + >> static int arm_smmu_device_probe(struct platform_device *pdev) >> { >> int irq, ret; >> @@ -3839,7 +3968,7 @@ static int arm_smmu_device_probe(struct >> platform_device *pdev) >> return ret; >> /* And we're up. Go go go! */ >> - ret = iommu_device_sysfs_add(&smmu->iommu, dev, NULL, >> + ret = iommu_device_sysfs_add(&smmu->iommu, dev, arm_iommu_groups, >> "smmu3.%pa", &ioaddr); >> if (ret) >> return ret; >> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h >> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h >> index cd48590ada30..ffa1123bf37c 100644 >> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h >> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h >> @@ -50,9 +50,13 @@ >> #define IDR1_SSIDSIZE GENMASK(10, 6) >> #define IDR1_SIDSIZE GENMASK(5, 0) >> +#define ARM_SMMU_IDR2 0x8 >> + >> #define ARM_SMMU_IDR3 0xc >> #define IDR3_RIL (1 << 10) >> +#define ARM_SMMU_IDR4 0x10 >> + >> #define ARM_SMMU_IDR5 0x14 >> #define IDR5_STALL_MAX GENMASK(31, 16) >> #define IDR5_GRAN64K (1 << 6) >> @@ -69,6 +73,9 @@ >> #define IDR5_VAX GENMASK(11, 10) >> #define IDR5_VAX_52_BIT 1 >> +#define ARM_SMMU_IIDR 0x18 >> +#define ARM_SMMU_AIDR 0x1C >> + >> #define ARM_SMMU_CR0 0x20 >> #define CR0_ATSCHK (1 << 4) >> #define CR0_CMDQEN (1 << 3) >> @@ -154,6 +161,8 @@ >> #define ARM_SMMU_REG_SZ 0xe00 >> +#define ARM_SMMU_IDR6 0x0190 >> + >> /* Common MSI config fields */ >> #define MSI_CFG0_ADDR_MASK GENMASK_ULL(51, 2) >> #define MSI_CFG2_SH GENMASK(5, 4) >> @@ -669,6 +678,17 @@ struct arm_smmu_device { >> struct arm_smmu_strtab_cfg strtab_cfg; >> + /* SMMU feature registers */ >> + u32 idr0; >> + u32 idr1; >> + u32 idr2; >> + u32 idr3; >> + u32 idr4; >> + u32 idr5; >> + u32 idr6; >> + u32 iidr; >> + u32 aidr; >> + >> /* IOMMU core code handle */ >> struct iommu_device iommu; >> @@ -676,6 +696,13 @@ struct arm_smmu_device { >> struct mutex streams_mutex; >> }; >> +static inline struct arm_smmu_device *dev_to_arm_smmu_device(struct device >> *dev) >> +{ >> + struct iommu_device *iommu = dev_to_iommu_device(dev); >> + >> + return container_of(iommu, struct arm_smmu_device, iommu); >> +} >> + >> struct arm_smmu_stream { >> u32 id; >> struct arm_smmu_master *master; _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
