Parse each PCI device's sec-sid property during SMMU device initialization
and cache it in SMMUDevice::sec_sid. Support "non-secure" and "secure",
default to non-secure when unspecified, and reject invalid values with an
explicit error. Use sdev->sec_sid in smmuv3_translate() to select the
register bank instead of hardcoding the non-secure context.
Keep sec-sid parsing in smmu-common, and add a SMMUv3-specific validation
hook to enforce architectural constraints: fail fast when sec-sid=secure
while SMMU_S_IDR1.SECURE_IMPL is 0 or secure AS is not available.
Typically, SEC_SID is a system-defined attribute (e.g. sideband or tied-off)
rather than something a PCIe endpoint can freely toggle in pre-RME scenario.
So this PCI sec-sid property is used as a static platform/testing knob to
drive the SMMU bank selection.
For future RME-DA + TDISP, this will need to become dynamic: the effective
state for PCIe requests is derived from PCIe IDE/TDISP T/XT
(e.g. SEC_SID = (XT || T) ? Realm : Non-secure), so we'll switch from a
static property to a runtime per-device state update when that plumbing
is added.
Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
---
hw/arm/smmu-common.c | 37 ++++++++++++++++++++++++++++++++++++
hw/arm/smmuv3.c | 34 ++++++++++++++++++++++++++++++++-
include/hw/arm/smmu-common.h | 2 ++
3 files changed, 72 insertions(+), 1 deletion(-)
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 5dece2024a4..b0a238abe93 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -21,6 +21,7 @@
#include "exec/target_page.h"
#include "hw/core/cpu.h"
#include "hw/pci/pci_bridge.h"
+#include "hw/pci/pci_device.h"
#include "hw/core/qdev-properties.h"
#include "qapi/error.h"
#include "qemu/jhash.h"
@@ -1071,14 +1072,50 @@ SMMUPciBus *smmu_find_smmu_pcibus(SMMUState *s, uint8_t bus_num)
return NULL;
}
+static SMMUSecSID smmu_parse_pci_sec_sid(PCIDevice *pdev, int bus_num,
+ int devfn)
+{
+ const char *sec_sid;
+
+ if (!pdev || !pdev->sec_sid) {
+ return SMMU_SEC_SID_NS;
+ }
+
+ sec_sid = pdev->sec_sid;
+ if (!strcmp(sec_sid, "non-secure")) {
+ return SMMU_SEC_SID_NS;
+ }
+ if (!strcmp(sec_sid, "secure")) {
+ return SMMU_SEC_SID_S;
+ }
+
+ error_report("Invalid sec-sid value '%s' for PCI device %02x:%02x.%x; "
+ "allowed values: non-secure or secure (case-sensitive)",
+ sec_sid, bus_num, PCI_SLOT(devfn), PCI_FUNC(devfn));
+ exit(EXIT_FAILURE);
+}
+
void smmu_init_sdev(SMMUState *s, SMMUDevice *sdev, PCIBus *bus, int devfn)
{
static unsigned int index;
g_autofree char *name = g_strdup_printf("%s-%d-%d", s->mrtypename, devfn,
index++);
+ SMMUBaseClass *sbc = ARM_SMMU_GET_CLASS(s);
+ PCIDevice *pdev;
+ int bus_num;
+
sdev->smmu = s;
sdev->bus = bus;
sdev->devfn = devfn;
+ sdev->sec_sid = SMMU_SEC_SID_NS;
+
+ bus_num = pci_bus_num(bus);
+ pdev = pci_find_device(bus, bus_num, devfn);
+ sdev->sec_sid = smmu_parse_pci_sec_sid(pdev, bus_num, devfn);
+ if (sbc->validate_sec_sid &&
+ !sbc->validate_sec_sid(s, sdev, bus_num)) {
+ exit(EXIT_FAILURE);
+ }
memory_region_init_iommu(&sdev->iommu, sizeof(sdev->iommu),
s->mrtypename, OBJECT(s), name, UINT64_MAX);
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 0b8ea922851..57a063b5e5d 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1116,7 +1116,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);
SMMUv3State *s = sdev->smmu;
uint32_t sid = smmu_get_sid(sdev);
- SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
+ SMMUSecSID sec_sid = sdev->sec_sid;
SMMUv3RegBank *bank = smmuv3_bank(s, sec_sid);
SMMUEventInfo event = {.type = SMMU_EVT_NONE,
.sid = sid,
@@ -1509,6 +1509,36 @@ static inline bool smmu_hw_secure_implemented(SMMUv3State *s)
return FIELD_EX32(s->bank[SMMU_SEC_SID_S].idr[1], S_IDR1, SECURE_IMPL);
}
+static bool smmuv3_validate_sec_sid(SMMUState *bs, SMMUDevice *sdev,
+ int bus_num)
+{
+ SMMUv3State *s = ARM_SMMUV3(bs);
+ bool secure_as_available = bs->secure_memory &&
+ bs->secure_memory_as.root != NULL;
+
+ if (sdev->sec_sid != SMMU_SEC_SID_S) {
+ return true;
+ }
+
+ if (!smmu_hw_secure_implemented(s)) {
+ error_report("Invalid sec-sid value 'secure' for PCI device "
+ "%02x:%02x.%x: S_IDR1.SECURE_IMPL is 0, so only "
+ "non-secure is allowed",
+ bus_num, PCI_SLOT(sdev->devfn), PCI_FUNC(sdev->devfn));
+ return false;
+ }
+
+ if (!secure_as_available) {
+ error_report("Invalid sec-sid value 'secure' for PCI device "
+ "%02x:%02x.%x: secure-memory address space is not "
+ "configured",
+ bus_num, PCI_SLOT(sdev->devfn), PCI_FUNC(sdev->devfn));
+ return false;
+ }
+
+ return true;
+}
+
static int smmuv3_cmdq_consume(SMMUv3State *s, Error **errp, SMMUSecSID sec_sid)
{
SMMUState *bs = ARM_SMMU(s);
@@ -2664,6 +2694,7 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
ResettableClass *rc = RESETTABLE_CLASS(klass);
SMMUv3Class *c = ARM_SMMUV3_CLASS(klass);
+ SMMUBaseClass *sbc = ARM_SMMU_CLASS(klass);
dc->vmsd = &vmstate_smmuv3;
resettable_class_set_parent_phases(rc, NULL, NULL, smmu_reset_exit,
@@ -2673,6 +2704,7 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
device_class_set_props(dc, smmuv3_properties);
dc->hotpluggable = false;
dc->user_creatable = true;
+ sbc->validate_sec_sid = smmuv3_validate_sec_sid;
object_class_property_set_description(klass, "accel",
"Enable SMMUv3 accelerator support. Allows host SMMUv3 to be "
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index d05cf6ae53b..c74f66a1bb9 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -144,6 +144,7 @@ typedef struct SMMUDevice {
void *smmu;
PCIBus *bus;
int devfn;
+ SMMUSecSID sec_sid;
IOMMUMemoryRegion iommu;
AddressSpace as;
uint32_t cfg_cache_hits;
@@ -204,6 +205,7 @@ struct SMMUBaseClass {
/*< public >*/
DeviceRealize parent_realize;
+ bool (*validate_sec_sid)(struct SMMUState *s, SMMUDevice *sdev, int bus_num);
};
--
2.34.1
On 2/21/26 11:19 AM, Tao Tang wrote:
> Parse each PCI device's sec-sid property during SMMU device initialization
> and cache it in SMMUDevice::sec_sid. Support "non-secure" and "secure",
> default to non-secure when unspecified, and reject invalid values with an
> explicit error. Use sdev->sec_sid in smmuv3_translate() to select the
> register bank instead of hardcoding the non-secure context.
>
> Keep sec-sid parsing in smmu-common, and add a SMMUv3-specific validation
> hook to enforce architectural constraints: fail fast when sec-sid=secure
> while SMMU_S_IDR1.SECURE_IMPL is 0 or secure AS is not available.
>
> Typically, SEC_SID is a system-defined attribute (e.g. sideband or tied-off)
> rather than something a PCIe endpoint can freely toggle in pre-RME scenario.
> So this PCI sec-sid property is used as a static platform/testing knob to
> drive the SMMU bank selection.
>
> For future RME-DA + TDISP, this will need to become dynamic: the effective
> state for PCIe requests is derived from PCIe IDE/TDISP T/XT
> (e.g. SEC_SID = (XT || T) ? Realm : Non-secure), so we'll switch from a
> static property to a runtime per-device state update when that plumbing
> is added.
When a translation is triggered, address_space_translate_iommu passes attrs.
MemTxAttrs.secure bit should tell you whether the translation is
requested in secure mode.
Shouldn't we pass that information the smmuv3 translate function()?
In address_space_translate_iommu I see:
if (imrc->attrs_to_index) {
iommu_idx = imrc->attrs_to_index(iommu_mr, attrs);
}
iotlb = imrc->translate(iommu_mr, addr, is_write ?
IOMMU_WO : IOMMU_RO, iommu_idx);
Thanks
Eric
>
> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
> ---
> hw/arm/smmu-common.c | 37 ++++++++++++++++++++++++++++++++++++
> hw/arm/smmuv3.c | 34 ++++++++++++++++++++++++++++++++-
> include/hw/arm/smmu-common.h | 2 ++
> 3 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 5dece2024a4..b0a238abe93 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -21,6 +21,7 @@
> #include "exec/target_page.h"
> #include "hw/core/cpu.h"
> #include "hw/pci/pci_bridge.h"
> +#include "hw/pci/pci_device.h"
> #include "hw/core/qdev-properties.h"
> #include "qapi/error.h"
> #include "qemu/jhash.h"
> @@ -1071,14 +1072,50 @@ SMMUPciBus *smmu_find_smmu_pcibus(SMMUState *s, uint8_t bus_num)
> return NULL;
> }
>
> +static SMMUSecSID smmu_parse_pci_sec_sid(PCIDevice *pdev, int bus_num,
> + int devfn)
> +{
> + const char *sec_sid;
> +
> + if (!pdev || !pdev->sec_sid) {
> + return SMMU_SEC_SID_NS;
> + }
> +
> + sec_sid = pdev->sec_sid;
> + if (!strcmp(sec_sid, "non-secure")) {
> + return SMMU_SEC_SID_NS;
> + }
> + if (!strcmp(sec_sid, "secure")) {
> + return SMMU_SEC_SID_S;
> + }
> +
> + error_report("Invalid sec-sid value '%s' for PCI device %02x:%02x.%x; "
> + "allowed values: non-secure or secure (case-sensitive)",
> + sec_sid, bus_num, PCI_SLOT(devfn), PCI_FUNC(devfn));
> + exit(EXIT_FAILURE);
> +}
> +
> void smmu_init_sdev(SMMUState *s, SMMUDevice *sdev, PCIBus *bus, int devfn)
> {
> static unsigned int index;
> g_autofree char *name = g_strdup_printf("%s-%d-%d", s->mrtypename, devfn,
> index++);
> + SMMUBaseClass *sbc = ARM_SMMU_GET_CLASS(s);
> + PCIDevice *pdev;
> + int bus_num;
> +
> sdev->smmu = s;
> sdev->bus = bus;
> sdev->devfn = devfn;
> + sdev->sec_sid = SMMU_SEC_SID_NS;
> +
> + bus_num = pci_bus_num(bus);
> + pdev = pci_find_device(bus, bus_num, devfn);
> + sdev->sec_sid = smmu_parse_pci_sec_sid(pdev, bus_num, devfn);
> + if (sbc->validate_sec_sid &&
> + !sbc->validate_sec_sid(s, sdev, bus_num)) {
> + exit(EXIT_FAILURE);
> + }
>
> memory_region_init_iommu(&sdev->iommu, sizeof(sdev->iommu),
> s->mrtypename, OBJECT(s), name, UINT64_MAX);
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 0b8ea922851..57a063b5e5d 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -1116,7 +1116,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);
> SMMUv3State *s = sdev->smmu;
> uint32_t sid = smmu_get_sid(sdev);
> - SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
> + SMMUSecSID sec_sid = sdev->sec_sid;
> SMMUv3RegBank *bank = smmuv3_bank(s, sec_sid);
> SMMUEventInfo event = {.type = SMMU_EVT_NONE,
> .sid = sid,
> @@ -1509,6 +1509,36 @@ static inline bool smmu_hw_secure_implemented(SMMUv3State *s)
> return FIELD_EX32(s->bank[SMMU_SEC_SID_S].idr[1], S_IDR1, SECURE_IMPL);
> }
>
> +static bool smmuv3_validate_sec_sid(SMMUState *bs, SMMUDevice *sdev,
> + int bus_num)
> +{
> + SMMUv3State *s = ARM_SMMUV3(bs);
> + bool secure_as_available = bs->secure_memory &&
> + bs->secure_memory_as.root != NULL;
> +
> + if (sdev->sec_sid != SMMU_SEC_SID_S) {
> + return true;
> + }
> +
> + if (!smmu_hw_secure_implemented(s)) {
> + error_report("Invalid sec-sid value 'secure' for PCI device "
> + "%02x:%02x.%x: S_IDR1.SECURE_IMPL is 0, so only "
> + "non-secure is allowed",
> + bus_num, PCI_SLOT(sdev->devfn), PCI_FUNC(sdev->devfn));
> + return false;
> + }
> +
> + if (!secure_as_available) {
> + error_report("Invalid sec-sid value 'secure' for PCI device "
> + "%02x:%02x.%x: secure-memory address space is not "
> + "configured",
> + bus_num, PCI_SLOT(sdev->devfn), PCI_FUNC(sdev->devfn));
> + return false;
> + }
> +
> + return true;
> +}
> +
> static int smmuv3_cmdq_consume(SMMUv3State *s, Error **errp, SMMUSecSID sec_sid)
> {
> SMMUState *bs = ARM_SMMU(s);
> @@ -2664,6 +2694,7 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> ResettableClass *rc = RESETTABLE_CLASS(klass);
> SMMUv3Class *c = ARM_SMMUV3_CLASS(klass);
> + SMMUBaseClass *sbc = ARM_SMMU_CLASS(klass);
>
> dc->vmsd = &vmstate_smmuv3;
> resettable_class_set_parent_phases(rc, NULL, NULL, smmu_reset_exit,
> @@ -2673,6 +2704,7 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
> device_class_set_props(dc, smmuv3_properties);
> dc->hotpluggable = false;
> dc->user_creatable = true;
> + sbc->validate_sec_sid = smmuv3_validate_sec_sid;
>
> object_class_property_set_description(klass, "accel",
> "Enable SMMUv3 accelerator support. Allows host SMMUv3 to be "
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index d05cf6ae53b..c74f66a1bb9 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -144,6 +144,7 @@ typedef struct SMMUDevice {
> void *smmu;
> PCIBus *bus;
> int devfn;
> + SMMUSecSID sec_sid;
> IOMMUMemoryRegion iommu;
> AddressSpace as;
> uint32_t cfg_cache_hits;
> @@ -204,6 +205,7 @@ struct SMMUBaseClass {
> /*< public >*/
>
> DeviceRealize parent_realize;
> + bool (*validate_sec_sid)(struct SMMUState *s, SMMUDevice *sdev, int bus_num);
>
> };
>
On 2/21/26 11:19 AM, Tao Tang wrote:
> Parse each PCI device's sec-sid property during SMMU device initialization
> and cache it in SMMUDevice::sec_sid. Support "non-secure" and "secure",
> default to non-secure when unspecified, and reject invalid values with an
> explicit error. Use sdev->sec_sid in smmuv3_translate() to select the
> register bank instead of hardcoding the non-secure context.
>
> Keep sec-sid parsing in smmu-common, and add a SMMUv3-specific validation
> hook to enforce architectural constraints: fail fast when sec-sid=secure
> while SMMU_S_IDR1.SECURE_IMPL is 0 or secure AS is not available.
>
> Typically, SEC_SID is a system-defined attribute (e.g. sideband or tied-off)
> rather than something a PCIe endpoint can freely toggle in pre-RME scenario.
> So this PCI sec-sid property is used as a static platform/testing knob to
> drive the SMMU bank selection.
>
> For future RME-DA + TDISP, this will need to become dynamic: the effective
> state for PCIe requests is derived from PCIe IDE/TDISP T/XT
> (e.g. SEC_SID = (XT || T) ? Realm : Non-secure), so we'll switch from a
> static property to a runtime per-device state update when that plumbing
> is added.
When a translation is triggered, address_space_translate_iommu passes attrs.
MemTxAttrs.secure bit should tell you whether the translation is
requested in secure mode.
Shouldn't we pass that information the smmuv3 translate function()?
In address_space_translate_iommu I see:
if (imrc->attrs_to_index) {
iommu_idx = imrc->attrs_to_index(iommu_mr, attrs);
}
iotlb = imrc->translate(iommu_mr, addr, is_write ?
IOMMU_WO : IOMMU_RO, iommu_idx);
Thanks
Eric
>
> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
> ---
> hw/arm/smmu-common.c | 37 ++++++++++++++++++++++++++++++++++++
> hw/arm/smmuv3.c | 34 ++++++++++++++++++++++++++++++++-
> include/hw/arm/smmu-common.h | 2 ++
> 3 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 5dece2024a4..b0a238abe93 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -21,6 +21,7 @@
> #include "exec/target_page.h"
> #include "hw/core/cpu.h"
> #include "hw/pci/pci_bridge.h"
> +#include "hw/pci/pci_device.h"
> #include "hw/core/qdev-properties.h"
> #include "qapi/error.h"
> #include "qemu/jhash.h"
> @@ -1071,14 +1072,50 @@ SMMUPciBus *smmu_find_smmu_pcibus(SMMUState *s, uint8_t bus_num)
> return NULL;
> }
>
> +static SMMUSecSID smmu_parse_pci_sec_sid(PCIDevice *pdev, int bus_num,
> + int devfn)
> +{
> + const char *sec_sid;
> +
> + if (!pdev || !pdev->sec_sid) {
> + return SMMU_SEC_SID_NS;
> + }
> +
> + sec_sid = pdev->sec_sid;
> + if (!strcmp(sec_sid, "non-secure")) {
> + return SMMU_SEC_SID_NS;
> + }
> + if (!strcmp(sec_sid, "secure")) {
> + return SMMU_SEC_SID_S;
> + }
> +
> + error_report("Invalid sec-sid value '%s' for PCI device %02x:%02x.%x; "
> + "allowed values: non-secure or secure (case-sensitive)",
> + sec_sid, bus_num, PCI_SLOT(devfn), PCI_FUNC(devfn));
> + exit(EXIT_FAILURE);
> +}
> +
> void smmu_init_sdev(SMMUState *s, SMMUDevice *sdev, PCIBus *bus, int devfn)
> {
> static unsigned int index;
> g_autofree char *name = g_strdup_printf("%s-%d-%d", s->mrtypename, devfn,
> index++);
> + SMMUBaseClass *sbc = ARM_SMMU_GET_CLASS(s);
> + PCIDevice *pdev;
> + int bus_num;
> +
> sdev->smmu = s;
> sdev->bus = bus;
> sdev->devfn = devfn;
> + sdev->sec_sid = SMMU_SEC_SID_NS;
> +
> + bus_num = pci_bus_num(bus);
> + pdev = pci_find_device(bus, bus_num, devfn);
> + sdev->sec_sid = smmu_parse_pci_sec_sid(pdev, bus_num, devfn);
> + if (sbc->validate_sec_sid &&
> + !sbc->validate_sec_sid(s, sdev, bus_num)) {
> + exit(EXIT_FAILURE);
> + }
>
> memory_region_init_iommu(&sdev->iommu, sizeof(sdev->iommu),
> s->mrtypename, OBJECT(s), name, UINT64_MAX);
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 0b8ea922851..57a063b5e5d 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -1116,7 +1116,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,
> SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);
> SMMUv3State *s = sdev->smmu;
> uint32_t sid = smmu_get_sid(sdev);
> - SMMUSecSID sec_sid = SMMU_SEC_SID_NS;
> + SMMUSecSID sec_sid = sdev->sec_sid;
> SMMUv3RegBank *bank = smmuv3_bank(s, sec_sid);
> SMMUEventInfo event = {.type = SMMU_EVT_NONE,
> .sid = sid,
> @@ -1509,6 +1509,36 @@ static inline bool smmu_hw_secure_implemented(SMMUv3State *s)
> return FIELD_EX32(s->bank[SMMU_SEC_SID_S].idr[1], S_IDR1, SECURE_IMPL);
> }
>
> +static bool smmuv3_validate_sec_sid(SMMUState *bs, SMMUDevice *sdev,
> + int bus_num)
> +{
> + SMMUv3State *s = ARM_SMMUV3(bs);
> + bool secure_as_available = bs->secure_memory &&
> + bs->secure_memory_as.root != NULL;
> +
> + if (sdev->sec_sid != SMMU_SEC_SID_S) {
> + return true;
> + }
> +
> + if (!smmu_hw_secure_implemented(s)) {
> + error_report("Invalid sec-sid value 'secure' for PCI device "
> + "%02x:%02x.%x: S_IDR1.SECURE_IMPL is 0, so only "
> + "non-secure is allowed",
> + bus_num, PCI_SLOT(sdev->devfn), PCI_FUNC(sdev->devfn));
> + return false;
> + }
> +
> + if (!secure_as_available) {
> + error_report("Invalid sec-sid value 'secure' for PCI device "
> + "%02x:%02x.%x: secure-memory address space is not "
> + "configured",
> + bus_num, PCI_SLOT(sdev->devfn), PCI_FUNC(sdev->devfn));
> + return false;
> + }
> +
> + return true;
> +}
> +
> static int smmuv3_cmdq_consume(SMMUv3State *s, Error **errp, SMMUSecSID sec_sid)
> {
> SMMUState *bs = ARM_SMMU(s);
> @@ -2664,6 +2694,7 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> ResettableClass *rc = RESETTABLE_CLASS(klass);
> SMMUv3Class *c = ARM_SMMUV3_CLASS(klass);
> + SMMUBaseClass *sbc = ARM_SMMU_CLASS(klass);
>
> dc->vmsd = &vmstate_smmuv3;
> resettable_class_set_parent_phases(rc, NULL, NULL, smmu_reset_exit,
> @@ -2673,6 +2704,7 @@ static void smmuv3_class_init(ObjectClass *klass, const void *data)
> device_class_set_props(dc, smmuv3_properties);
> dc->hotpluggable = false;
> dc->user_creatable = true;
> + sbc->validate_sec_sid = smmuv3_validate_sec_sid;
>
> object_class_property_set_description(klass, "accel",
> "Enable SMMUv3 accelerator support. Allows host SMMUv3 to be "
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index d05cf6ae53b..c74f66a1bb9 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -144,6 +144,7 @@ typedef struct SMMUDevice {
> void *smmu;
> PCIBus *bus;
> int devfn;
> + SMMUSecSID sec_sid;
> IOMMUMemoryRegion iommu;
> AddressSpace as;
> uint32_t cfg_cache_hits;
> @@ -204,6 +205,7 @@ struct SMMUBaseClass {
> /*< public >*/
>
> DeviceRealize parent_realize;
> + bool (*validate_sec_sid)(struct SMMUState *s, SMMUDevice *sdev, int bus_num);
>
> };
>
Hi Eric,
On 2026/3/3 18:47, Eric Auger wrote:
> On 2/21/26 11:19 AM, Tao Tang wrote:
>> Parse each PCI device's sec-sid property during SMMU device initialization
>> and cache it in SMMUDevice::sec_sid. Support "non-secure" and "secure",
>> default to non-secure when unspecified, and reject invalid values with an
>> explicit error. Use sdev->sec_sid in smmuv3_translate() to select the
>> register bank instead of hardcoding the non-secure context.
>>
>> Keep sec-sid parsing in smmu-common, and add a SMMUv3-specific validation
>> hook to enforce architectural constraints: fail fast when sec-sid=secure
>> while SMMU_S_IDR1.SECURE_IMPL is 0 or secure AS is not available.
>>
>> Typically, SEC_SID is a system-defined attribute (e.g. sideband or tied-off)
>> rather than something a PCIe endpoint can freely toggle in pre-RME scenario.
>> So this PCI sec-sid property is used as a static platform/testing knob to
>> drive the SMMU bank selection.
>>
>> For future RME-DA + TDISP, this will need to become dynamic: the effective
>> state for PCIe requests is derived from PCIe IDE/TDISP T/XT
>> (e.g. SEC_SID = (XT || T) ? Realm : Non-secure), so we'll switch from a
>> static property to a runtime per-device state update when that plumbing
>> is added.
> When a translation is triggered, address_space_translate_iommu passes attrs.
> MemTxAttrs.secure bit should tell you whether the translation is
> requested in secure mode.
> Shouldn't we pass that information the smmuv3 translate function()?
>
> In address_space_translate_iommu I see:
> if (imrc->attrs_to_index) {
> iommu_idx = imrc->attrs_to_index(iommu_mr, attrs);
> }
>
> iotlb = imrc->translate(iommu_mr, addr, is_write ?
> IOMMU_WO : IOMMU_RO, iommu_idx);
Thanks for pointing this out.
I agree that per-transaction request intent should eventually come from
MemTxAttrs or attrs_to_index().
However, the current producer side is not yet in a state where
transaction attrs alone can replace stream-level security wiring: many
PCI DMA paths still use MEMTXATTRS_UNSPECIFIED, and space cannot be
consumed safely without an explicit validity signal. For example, an
incorrect zero-initialized or otherwise unspecified .space value cannot
be treated as authoritative by itself as Pierrick mentioned in this thread:
https://lore.kernel.org/qemu-devel/dbc4d33e-3477-4f39-a745-4fdc0866fc08@linaro.org/
In the current codebase, this can easily blur the distinction between
“explicitly Secure” and “not specified at all”, especially for legacy or
non-Arm-aware producers. So I think there are really two separate
concerns here:
1. the per-transaction intent (which should indeed come from MemTxAttrs
/ attrs_to_index()); and
2. the stream-level capability / wiring, i.e. whether this SID is
allowed to reach the Secure programming interface in the first place.
My concern with the current patch is not that attrs-based routing is the
wrong long-term direction, but that today transaction attrs are not yet
propagated consistently enough to replace a stream-level model entirely.
So for now, I think a static sec-sid property is still useful as a
platform knob, until producer-side propagation becomes reliable enough.
Once that is in place, I agree that the bank selection should ultimately
be driven through the standard per-transaction attrs path. How do you
think about it?
Best regards,
Tao
On 3/6/26 5:30 AM, Tao Tang wrote:
> Hi Eric,
>
> On 2026/3/3 18:47, Eric Auger wrote:
>> On 2/21/26 11:19 AM, Tao Tang wrote:
>>> Parse each PCI device's sec-sid property during SMMU device initialization
>>> and cache it in SMMUDevice::sec_sid. Support "non-secure" and "secure",
>>> default to non-secure when unspecified, and reject invalid values with an
>>> explicit error. Use sdev->sec_sid in smmuv3_translate() to select the
>>> register bank instead of hardcoding the non-secure context.
>>>
>>> Keep sec-sid parsing in smmu-common, and add a SMMUv3-specific validation
>>> hook to enforce architectural constraints: fail fast when sec-sid=secure
>>> while SMMU_S_IDR1.SECURE_IMPL is 0 or secure AS is not available.
>>>
>>> Typically, SEC_SID is a system-defined attribute (e.g. sideband or tied-off)
>>> rather than something a PCIe endpoint can freely toggle in pre-RME scenario.
>>> So this PCI sec-sid property is used as a static platform/testing knob to
>>> drive the SMMU bank selection.
>>>
>>> For future RME-DA + TDISP, this will need to become dynamic: the effective
>>> state for PCIe requests is derived from PCIe IDE/TDISP T/XT
>>> (e.g. SEC_SID = (XT || T) ? Realm : Non-secure), so we'll switch from a
>>> static property to a runtime per-device state update when that plumbing
>>> is added.
>> When a translation is triggered, address_space_translate_iommu passes attrs.
>> MemTxAttrs.secure bit should tell you whether the translation is
>> requested in secure mode.
>> Shouldn't we pass that information the smmuv3 translate function()?
>>
>> In address_space_translate_iommu I see:
>> if (imrc->attrs_to_index) {
>> iommu_idx = imrc->attrs_to_index(iommu_mr, attrs);
>> }
>>
>> iotlb = imrc->translate(iommu_mr, addr, is_write ?
>> IOMMU_WO : IOMMU_RO, iommu_idx);
>
>
> Thanks for pointing this out.
>
> I agree that per-transaction request intent should eventually come from
> MemTxAttrs or attrs_to_index().
>
> However, the current producer side is not yet in a state where
> transaction attrs alone can replace stream-level security wiring: many
> PCI DMA paths still use MEMTXATTRS_UNSPECIFIED, and space cannot be
> consumed safely without an explicit validity signal. For example, an
> incorrect zero-initialized or otherwise unspecified .space value cannot
> be treated as authoritative by itself as Pierrick mentioned in this thread:
>
> https://lore.kernel.org/qemu-devel/dbc4d33e-3477-4f39-a745-4fdc0866fc08@linaro.org/
>
> In the current codebase, this can easily blur the distinction between
> “explicitly Secure” and “not specified at all”, especially for legacy or
> non-Arm-aware producers. So I think there are really two separate
> concerns here:
>
> 1. the per-transaction intent (which should indeed come from MemTxAttrs
> / attrs_to_index()); and
> 2. the stream-level capability / wiring, i.e. whether this SID is
> allowed to reach the Secure programming interface in the first place.
>
> My concern with the current patch is not that attrs-based routing is the
> wrong long-term direction, but that today transaction attrs are not yet
> propagated consistently enough to replace a stream-level model entirely.
>
> So for now, I think a static sec-sid property is still useful as a
> platform knob, until producer-side propagation becomes reliable enough.
> Once that is in place, I agree that the bank selection should ultimately
> be driven through the standard per-transaction attrs path. How do you
> think about it?
>
As a complement of information, SMMU specification mentions:
```
3.10.1 StreamID Security state (SEC_SID)
Note: Whether a stream is under Secure control or not is a different
property to the target PA space of a transaction.
```
At the time, we checked with spec authors at Arm what was the exact
meaning, and they confirmed it was a builtin (static) property of the
system. A secure device will always be secure through execution.
Realms have a different approach, and PCI transport has the T bit for
that, to make it a dynamic property.
Of course, we can implement things differently than what spec says, but
I definitely see it as a design hint that it should be a property of the
device, and not of the transaction itself.
My 2 cents,
Pierrick
On 2/21/26 2:19 AM, Tao Tang wrote: > Parse each PCI device's sec-sid property during SMMU device initialization > and cache it in SMMUDevice::sec_sid. Support "non-secure" and "secure", > default to non-secure when unspecified, and reject invalid values with an > explicit error. Use sdev->sec_sid in smmuv3_translate() to select the > register bank instead of hardcoding the non-secure context. > > Keep sec-sid parsing in smmu-common, and add a SMMUv3-specific validation > hook to enforce architectural constraints: fail fast when sec-sid=secure > while SMMU_S_IDR1.SECURE_IMPL is 0 or secure AS is not available. > > Typically, SEC_SID is a system-defined attribute (e.g. sideband or tied-off) > rather than something a PCIe endpoint can freely toggle in pre-RME scenario. > So this PCI sec-sid property is used as a static platform/testing knob to > drive the SMMU bank selection. > > For future RME-DA + TDISP, this will need to become dynamic: the effective > state for PCIe requests is derived from PCIe IDE/TDISP T/XT > (e.g. SEC_SID = (XT || T) ? Realm : Non-secure), so we'll switch from a > static property to a runtime per-device state update when that plumbing > is added. > > Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn> > --- > hw/arm/smmu-common.c | 37 ++++++++++++++++++++++++++++++++++++ > hw/arm/smmuv3.c | 34 ++++++++++++++++++++++++++++++++- > include/hw/arm/smmu-common.h | 2 ++ > 3 files changed, 72 insertions(+), 1 deletion(-) > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
On 2/21/26 2:19 AM, Tao Tang wrote: > Parse each PCI device's sec-sid property during SMMU device initialization > and cache it in SMMUDevice::sec_sid. Support "non-secure" and "secure", > default to non-secure when unspecified, and reject invalid values with an > explicit error. Use sdev->sec_sid in smmuv3_translate() to select the > register bank instead of hardcoding the non-secure context. > > Keep sec-sid parsing in smmu-common, and add a SMMUv3-specific validation > hook to enforce architectural constraints: fail fast when sec-sid=secure > while SMMU_S_IDR1.SECURE_IMPL is 0 or secure AS is not available. > > Typically, SEC_SID is a system-defined attribute (e.g. sideband or tied-off) > rather than something a PCIe endpoint can freely toggle in pre-RME scenario. > So this PCI sec-sid property is used as a static platform/testing knob to > drive the SMMU bank selection. > > For future RME-DA + TDISP, this will need to become dynamic: the effective > state for PCIe requests is derived from PCIe IDE/TDISP T/XT > (e.g. SEC_SID = (XT || T) ? Realm : Non-secure), so we'll switch from a > static property to a runtime per-device state update when that plumbing > is added. > > Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn> > --- > hw/arm/smmu-common.c | 37 ++++++++++++++++++++++++++++++++++++ > hw/arm/smmuv3.c | 34 ++++++++++++++++++++++++++++++++- > include/hw/arm/smmu-common.h | 2 ++ > 3 files changed, 72 insertions(+), 1 deletion(-) > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
© 2016 - 2026 Red Hat, Inc.