[RFC v4 13/31] hw/arm/smmu-common: Add sec_sid field to TLB entries

Tao Tang posted 31 patches 1 month, 2 weeks ago
Maintainers: Eric Auger <eric.auger@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
[RFC v4 13/31] hw/arm/smmu-common: Add sec_sid field to TLB entries
Posted by Tao Tang 1 month, 2 weeks ago
This is a non-functional preparation step that adds storage for
resolved security state in SMMUTLBEntry.

Together with the earlier commits that added NSCFG handling and
PTE NS/NSTable helpers, the plumbing is complete and we can now
refactor the PTW flow to handle Secure state.

Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
---
 include/hw/arm/smmu-common.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index bd88e599c77..b0a02e12fe6 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -91,6 +91,7 @@ typedef struct SMMUTLBEntry {
     uint8_t level;
     uint8_t granule;
     IOMMUAccessFlags parent_perm;
+    SMMUSecSID sec_sid;
 } SMMUTLBEntry;
 
 /* Stage-2 configuration. */
-- 
2.34.1
Re: [RFC v4 13/31] hw/arm/smmu-common: Add sec_sid field to TLB entries
Posted by Eric Auger 1 month, 1 week ago

On 2/21/26 11:16 AM, Tao Tang wrote:
> This is a non-functional preparation step that adds storage for
> resolved security state in SMMUTLBEntry.

Add the "why"in the commit description.
>
> Together with the earlier commits that added NSCFG handling and
> PTE NS/NSTable helpers, the plumbing is complete and we can now
> refactor the PTW flow to handle Secure state.
>
> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
> ---
>  include/hw/arm/smmu-common.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index bd88e599c77..b0a02e12fe6 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -91,6 +91,7 @@ typedef struct SMMUTLBEntry {
>      uint8_t level;
>      uint8_t granule;
>      IOMMUAccessFlags parent_perm;
> +    SMMUSecSID sec_sid;
>  } SMMUTLBEntry;
what does it need to be part of the actual entry and not only of the key?

Eric
>  
>  /* Stage-2 configuration. */
Re: [RFC v4 13/31] hw/arm/smmu-common: Add sec_sid field to TLB entries
Posted by Tao Tang 1 month, 1 week ago
Hi Eric,

On 2026/3/3 01:34, Eric Auger wrote:
>
> On 2/21/26 11:16 AM, Tao Tang wrote:
>> This is a non-functional preparation step that adds storage for
>> resolved security state in SMMUTLBEntry.
> Add the "why"in the commit description.
>> Together with the earlier commits that added NSCFG handling and
>> PTE NS/NSTable helpers, the plumbing is complete and we can now
>> refactor the PTW flow to handle Secure state.
>>
>> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
>> ---
>>   include/hw/arm/smmu-common.h | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
>> index bd88e599c77..b0a02e12fe6 100644
>> --- a/include/hw/arm/smmu-common.h
>> +++ b/include/hw/arm/smmu-common.h
>> @@ -91,6 +91,7 @@ typedef struct SMMUTLBEntry {
>>       uint8_t level;
>>       uint8_t granule;
>>       IOMMUAccessFlags parent_perm;
>> +    SMMUSecSID sec_sid;
>>   } SMMUTLBEntry;
> what does it need to be part of the actual entry and not only of the key?


sec_sid does not need to live in SMMUTLBEntry. After rechecking the 
code, SMMUTLBEntry.sec_sid is only consumed in:

smmu_ptw_64_s1

         if (current_ns) {
             tlbe->sec_sid = SMMU_SEC_SID_NS;
         } else {
             tlbe->sec_sid = PTE_NS(pte) ? SMMU_SEC_SID_NS : SMMU_SEC_SID_S;
         }
         tlbe->entry.target_as = (tlbe->sec_sid == SMMU_SEC_SID_S)
                                 ? &bs->secure_memory_as : &bs->memory_as;
         tlbe->entry.translated_addr = gpa;



and smmu_ptw_64_s2:

         tlbe->sec_sid = SMMU_SEC_SID_NS;
         tlbe->entry.target_as = cfg->ns_as;
         tlbe->entry.translated_addr = gpa;


to get the target_as. So keeping it in the cached entry is unnecessary 
if we use a local sec_sid instead.



I’ll drop this field and keep the security-state handling local to the 
walk instead. If we later need it for TLB disambiguation, it belongs in 
the lookup key rather than the entry payload.


Thanks for the catch.

Tao



Re: [RFC v4 13/31] hw/arm/smmu-common: Add sec_sid field to TLB entries
Posted by Mostafa Saleh 1 month, 1 week ago
On Sat, Feb 21, 2026 at 06:16:40PM +0800, Tao Tang wrote:
> This is a non-functional preparation step that adds storage for
> resolved security state in SMMUTLBEntry.
> 

I am not sure about this, when I added stage-2 I re-used the same
TLB instance as it might be used for nesting, and we can have
end-to-end cached entires.
For secure state, I think it’s cleaner to instantiate a new TLB, as
both states can never mix, and in this case we can re-use the same
functions without adding the secure bit in the TLB.

Thanks,
Mostafa

> Together with the earlier commits that added NSCFG handling and
> PTE NS/NSTable helpers, the plumbing is complete and we can now
> refactor the PTW flow to handle Secure state.
> 
> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
> ---
>  include/hw/arm/smmu-common.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index bd88e599c77..b0a02e12fe6 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -91,6 +91,7 @@ typedef struct SMMUTLBEntry {
>      uint8_t level;
>      uint8_t granule;
>      IOMMUAccessFlags parent_perm;
> +    SMMUSecSID sec_sid;
>  } SMMUTLBEntry;
>  
>  /* Stage-2 configuration. */
> -- 
> 2.34.1
> 
> 

Re: [RFC v4 13/31] hw/arm/smmu-common: Add sec_sid field to TLB entries
Posted by Tao Tang 1 month, 1 week ago
Hi Mostafa,

On 2026/2/27 PM10:45, Mostafa Saleh wrote:
> On Sat, Feb 21, 2026 at 06:16:40PM +0800, Tao Tang wrote:
>> This is a non-functional preparation step that adds storage for
>> resolved security state in SMMUTLBEntry.
>>
> I am not sure about this, when I added stage-2 I re-used the same
> TLB instance as it might be used for nesting, and we can have
> end-to-end cached entires.
> For secure state, I think it’s cleaner to instantiate a new TLB, as
> both states can never mix, and in this case we can re-use the same
> functions without adding the secure bit in the TLB.


Thanks for the suggestion.

I agree that keeping separate TLB instances per security state is 
cleaner and avoids having to carry a sec_sid tag through every cache path.

The only downside is a bit more plumbing: we’ll need to make sure the 
translation/PTW paths always pick the right per-bank TLB (otherwise some 
call sites may still default to the NS one). This also means touching 
the common layer since smmu-common currently assumes a single s->iotlb.


Although some refactoring work is still needed, I’m happy to follow your 
approach and will rework this in V5.


>
> Thanks,
> Mostafa


Best regards,

Tao


Re: [RFC v4 13/31] hw/arm/smmu-common: Add sec_sid field to TLB entries
Posted by Mostafa Saleh 1 month, 1 week ago
On Sun, Mar 1, 2026 at 3:08 PM Tao Tang <tangtao1634@phytium.com.cn> wrote:
>
> Hi Mostafa,
>
> On 2026/2/27 PM10:45, Mostafa Saleh wrote:
> > On Sat, Feb 21, 2026 at 06:16:40PM +0800, Tao Tang wrote:
> >> This is a non-functional preparation step that adds storage for
> >> resolved security state in SMMUTLBEntry.
> >>
> > I am not sure about this, when I added stage-2 I re-used the same
> > TLB instance as it might be used for nesting, and we can have
> > end-to-end cached entires.
> > For secure state, I think it’s cleaner to instantiate a new TLB, as
> > both states can never mix, and in this case we can re-use the same
> > functions without adding the secure bit in the TLB.
>
>
> Thanks for the suggestion.
>
> I agree that keeping separate TLB instances per security state is
> cleaner and avoids having to carry a sec_sid tag through every cache path.
>
> The only downside is a bit more plumbing: we’ll need to make sure the
> translation/PTW paths always pick the right per-bank TLB (otherwise some
> call sites may still default to the NS one). This also means touching
> the common layer since smmu-common currently assumes a single s->iotlb.
>

I see, I will need to think more about this. My initial thought was
that the iotlb seems to be exclusively used in smmu_iotlb_*()
So, I was wondering if we can change those functions to pass the iotlb
as a function argument. We should have enough context from translation
or invalidation paths to pass the correct iotlb instance.
But, I agree, it might not be as simple as I imagine, although I think
that would be cleaner. I will look more into it.

Thanks,
Mostafa

>
> Although some refactoring work is still needed, I’m happy to follow your
> approach and will rework this in V5.
>
>
> >
> > Thanks,
> > Mostafa
>
>
> Best regards,
>
> Tao
>
Re: [RFC v4 13/31] hw/arm/smmu-common: Add sec_sid field to TLB entries
Posted by Pierrick Bouvier 1 month, 2 weeks ago
On 2/21/26 2:16 AM, Tao Tang wrote:
> This is a non-functional preparation step that adds storage for
> resolved security state in SMMUTLBEntry.
> 
> Together with the earlier commits that added NSCFG handling and
> PTE NS/NSTable helpers, the plumbing is complete and we can now
> refactor the PTW flow to handle Secure state.
> 
> Signed-off-by: Tao Tang <tangtao1634@phytium.com.cn>
> ---
>   include/hw/arm/smmu-common.h | 1 +
>   1 file changed, 1 insertion(+)
> 

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>