On Fri, 15 Aug 2025 at 07:02, Tao Tang <tangtao1...@phytium.com.cn> wrote:
> This leads me to two questions on the best path forward:
>
> 1. How should the SEC_SID be propagated from the device? The only thing
> that is certain is that the hardware transaction will carry a SEC_SID
> value that is consistent with the manual. My initial thought was to
> extend MemTxAttrs.secure to carry the four states, but I am unsure if
> this is the best approach.
>
> typedef struct MemTxAttrs {
>      /*
>       * ARM/AMBA: TrustZone Secure access
>       * x86: System Management Mode access
>       */
>      unsigned int secure:1;
>      /*
>       * ARM: ArmSecuritySpace.  This partially overlaps secure, but it is
>       * easier to have both fields to assist code that does not understand
>       * ARMv9 RME, or no specific knowledge of ARM at all (e.g. pflash).
>       */
>      unsigned int space:2;

The SEC_SID is the information in the transaction that tells
us whether the device that initiated the transaction is
NonSecure, Secure, or Realm, right? I think we can
straightforwardly use 'space' for this. (The mapping between
SEC_SID and space can be written "space = sec_sid ^ 1" but
that is perhaps being unnecessarily clever.)

"secure" is (as the comment notes) for the benefit of code
that doesn't understand Realm. It should be initialized to
arm_space_is_secure(space). (Currently that function is in
target/arm/cpu.h, which is an awkward place for it: we
should probably move it to somewhere that lets code that's
not tied to the CPU use it.)

> To accurately model the SMMU, our smmuv3_translate function must
> ultimately use this SEC_SID value. Taking the translate callback
> signature into consideration:
>
> IOMMUTLBEntry (*translate)(IOMMUMemoryRegion *iommu, hwaddr addr,
>                             IOMMUAccessFlags flags, int iommu_idx);
>
> My understanding is that we can use the iommu_idx parameter to carry the
> SEC_SID value for each transaction.

Yes. You also will need to implement the attrs_to_index method,
which is what converts from the attrs in the MemTxAttrs to
the iommu_idx that the translate method gets passed.

(We do actually have hw/misc/tz-mpc.c that uses the IOMMU
APIs to handle Secure vs NonSecure transactions differently,
though it's such a long way from a real SMMU that I don't
know that it's a very useful thing to look at.)

> 2. What should we use as the internal bank index within the SMMUv3
> model? I see two potential options:
> a) Use the existing ARMSecuritySpace as the internal index. This would
> require conversions from the SEC_SID carried by each transaction.
> b) Define a new, SMMU-specific enum that perfectly matches the SEC_SID
> hardware specification (e.g., enum SmmuSecSid { NS=0, S=1, REALM=2,
> ROOT=3 }). We would use this new enum as our bank index internally.
>
> I am leaning towards option (2b) as it seems to offer better
> encapsulation and fidelity to the hardware specification, but I would be
> very grateful to hear your opinions and guidance on this architectural
> choice.

Yes, I like 2b here. This is what I am doing for the GICv5 --
there the architecture specifies various "interrupt domains",
and there's an obvious encoding from those to 0..3, which is
what I use to pick the register bank to use. The MemTxAttrs
don't quite line up with the domain index, and they are something
we need to figure out less often than "which register bank", so
it's OK for that to be something we do via a function or
whatever.

For the GICv5 in fact I have opted to create the MemTxAttrs
ahead of time at the point where the guest enables things
and we know what they should be (roughly, the device state
caches various bits of info about the configured state
in a struct, and one of those fields is a MemTxAttrs; so
you can pass cfg[domain]->txattrs as your attrs rather than
computing it on the spot every time -- especially since you
need to initialize both .space and .secure this seemed
nicer to me).

thanks
-- PMM

Reply via email to