Hi Tao, On 2/21/26 11:14 AM, Tao Tang wrote: > Adapt the configuration cache to support multiple security states by > introducing a composite key, SMMUConfigKey. This key combines the > SMMUDevice with SEC_SID, preventing aliasing between Secure and > Non-secure configurations for the same device, also the future Realm and > Root configurations.
Looking at 27/31, the sec_sid of a device looks rather static, set by a property. However here you mention risk of aliasing between non secure and secure for a given sdev Please could you clarify? Thanks Eric > > The cache lookup, insertion, and invalidation mechanisms are updated > to use this new keying infrastructure. This change is critical for > ensuring correct translation when a device is active in more than one > security world. > > Signed-off-by: Tao Tang <[email protected]> > Reviewed-by: Eric Auger <[email protected]> > Link: > https://lore.kernel.org/qemu-devel/[email protected]/ > --- > hw/arm/smmu-common.c | 45 ++++++++++++++++++++++++++++++++++-- > hw/arm/smmuv3.c | 13 +++++++---- > include/hw/arm/smmu-common.h | 7 ++++++ > 3 files changed, 58 insertions(+), 7 deletions(-) > > diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c > index b320aec8c60..a732303b28b 100644 > --- a/hw/arm/smmu-common.c > +++ b/hw/arm/smmu-common.c > @@ -30,6 +30,26 @@ > #include "hw/arm/smmu-common.h" > #include "smmu-internal.h" > > +/* Configuration Cache Management */ > +static guint smmu_config_key_hash(gconstpointer key) > +{ > + const SMMUConfigKey *k = key; > + return g_direct_hash(k->sdev) ^ (guint)k->sec_sid; > +} > + > +static gboolean smmu_config_key_equal(gconstpointer a, gconstpointer b) > +{ > + const SMMUConfigKey *ka = a; > + const SMMUConfigKey *kb = b; > + return ka->sdev == kb->sdev && ka->sec_sid == kb->sec_sid; > +} > + > +SMMUConfigKey smmu_get_config_key(SMMUDevice *sdev, SMMUSecSID sec_sid) > +{ > + SMMUConfigKey key = {.sdev = sdev, .sec_sid = sec_sid}; > + return key; > +} > + > ARMSecuritySpace smmu_get_security_space(SMMUSecSID sec_sid) > { > switch (sec_sid) { > @@ -265,7 +285,8 @@ static gboolean smmu_hash_remove_by_vmid_ipa(gpointer > key, gpointer value, > static gboolean > smmu_hash_remove_by_sid_range(gpointer key, gpointer value, gpointer > user_data) > { > - SMMUDevice *sdev = (SMMUDevice *)key; > + SMMUConfigKey *config_key = (SMMUConfigKey *)key; > + SMMUDevice *sdev = config_key->sdev; > uint32_t sid = smmu_get_sid(sdev); > SMMUSIDRange *sid_range = (SMMUSIDRange *)user_data; > > @@ -283,6 +304,24 @@ void smmu_configs_inv_sid_range(SMMUState *s, > SMMUSIDRange sid_range) > &sid_range); > } > > +static gboolean smmu_hash_remove_by_sdev(gpointer key, gpointer value, > + gpointer user_data) > +{ > + SMMUConfigKey *config_key = (SMMUConfigKey *)key; > + SMMUDevice *target = (SMMUDevice *)user_data; > + > + if (config_key->sdev != target) { > + return false; > + } > + trace_smmu_config_cache_inv(smmu_get_sid(target)); > + return true; > +} > + > +void smmu_configs_inv_sdev(SMMUState *s, SMMUDevice *sdev) > +{ > + g_hash_table_foreach_remove(s->configs, smmu_hash_remove_by_sdev, sdev); > +} > + > void smmu_iotlb_inv_iova(SMMUState *s, int asid, int vmid, dma_addr_t iova, > uint8_t tg, uint64_t num_pages, uint8_t ttl) > { > @@ -978,7 +1017,9 @@ static void smmu_base_realize(DeviceState *dev, Error > **errp) > error_propagate(errp, local_err); > return; > } > - s->configs = g_hash_table_new_full(NULL, NULL, NULL, g_free); > + s->configs = g_hash_table_new_full(smmu_config_key_hash, > + smmu_config_key_equal, > + g_free, g_free); > s->iotlb = g_hash_table_new_full(smmu_iotlb_key_hash, > smmu_iotlb_key_equal, > g_free, g_free); > s->smmu_pcibus_by_busptr = g_hash_table_new(NULL, NULL); > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c > index d011357253e..aa1a95a0093 100644 > --- a/hw/arm/smmuv3.c > +++ b/hw/arm/smmuv3.c > @@ -904,10 +904,11 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, > SMMUTransCfg *cfg, > * > * @sdev: SMMUDevice handle > * @event: output event info > + * @sec_sid: StreamID Security state > * > * The configuration cache contains data resulting from both STE and CD > * decoding under the form of an SMMUTransCfg struct. The hash table is > indexed > - * by the SMMUDevice handle. > + * by a composite key of the SMMUDevice and the sec_sid. > */ > static SMMUTransCfg *smmuv3_get_config(SMMUDevice *sdev, SMMUEventInfo > *event, > SMMUSecSID sec_sid) > @@ -915,8 +916,9 @@ static SMMUTransCfg *smmuv3_get_config(SMMUDevice *sdev, > SMMUEventInfo *event, > SMMUv3State *s = sdev->smmu; > SMMUState *bc = &s->smmu_state; > SMMUTransCfg *cfg; > + SMMUConfigKey lookup_key = smmu_get_config_key(sdev, sec_sid); > > - cfg = g_hash_table_lookup(bc->configs, sdev); > + cfg = g_hash_table_lookup(bc->configs, &lookup_key); > if (cfg) { > sdev->cfg_cache_hits++; > trace_smmuv3_config_cache_hit(smmu_get_sid(sdev), > @@ -940,7 +942,9 @@ static SMMUTransCfg *smmuv3_get_config(SMMUDevice *sdev, > SMMUEventInfo *event, > g_assert(cfg->as && cfg->ns_as); > > if (!smmuv3_decode_config(&sdev->iommu, cfg, event)) { > - g_hash_table_insert(bc->configs, sdev, cfg); > + SMMUConfigKey *persistent_key = g_new(SMMUConfigKey, 1); > + *persistent_key = lookup_key; > + g_hash_table_insert(bc->configs, persistent_key, cfg); > } else { > g_free(cfg); > cfg = NULL; > @@ -954,8 +958,7 @@ static void smmuv3_flush_config(SMMUDevice *sdev) > SMMUv3State *s = sdev->smmu; > SMMUState *bc = &s->smmu_state; > > - trace_smmu_config_cache_inv(smmu_get_sid(sdev)); > - g_hash_table_remove(bc->configs, sdev); > + smmu_configs_inv_sdev(bc, sdev); > } > > /* Do translation with TLB lookup. */ > diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h > index 7944e8d1b64..9e44c9f7710 100644 > --- a/include/hw/arm/smmu-common.h > +++ b/include/hw/arm/smmu-common.h > @@ -162,6 +162,11 @@ typedef struct SMMUIOTLBKey { > uint8_t level; > } SMMUIOTLBKey; > > +typedef struct SMMUConfigKey { > + SMMUDevice *sdev; > + SMMUSecSID sec_sid; > +} SMMUConfigKey; > + > typedef struct SMMUSIDRange { > uint32_t start; > uint32_t end; > @@ -250,6 +255,7 @@ SMMUTLBEntry *smmu_iotlb_lookup(SMMUState *bs, > SMMUTransCfg *cfg, > void smmu_iotlb_insert(SMMUState *bs, SMMUTransCfg *cfg, SMMUTLBEntry > *entry); > SMMUIOTLBKey smmu_get_iotlb_key(int asid, int vmid, uint64_t iova, > uint8_t tg, uint8_t level); > +SMMUConfigKey smmu_get_config_key(SMMUDevice *sdev, SMMUSecSID sec_sid); > void smmu_iotlb_inv_all(SMMUState *s); > void smmu_iotlb_inv_asid_vmid(SMMUState *s, int asid, int vmid); > void smmu_iotlb_inv_vmid(SMMUState *s, int vmid); > @@ -259,6 +265,7 @@ void smmu_iotlb_inv_iova(SMMUState *s, int asid, int > vmid, dma_addr_t iova, > void smmu_iotlb_inv_ipa(SMMUState *s, int vmid, dma_addr_t ipa, uint8_t tg, > uint64_t num_pages, uint8_t ttl); > void smmu_configs_inv_sid_range(SMMUState *s, SMMUSIDRange sid_range); > +void smmu_configs_inv_sdev(SMMUState *s, SMMUDevice *sdev); > /* Unmap the range of all the notifiers registered to any IOMMU mr */ > void smmu_inv_notifiers_all(SMMUState *s); >
