On Thu, 16 Jul 2026 at 04:12, Jim Shu <[email protected]> wrote: > > On Thu, Jul 16, 2026 at 10:47 AM Jim Shu <[email protected]> wrote: > > > > CC: [email protected] > > > > > > > > > > On Wed, Jul 15, 2026 at 5:57 PM Peter Maydell <[email protected]> > > wrote: > > > > > > On Wed, 15 Jul 2026 at 07:52, Jim Shu <[email protected]> wrote: > > > > > > > > Hi all, > > > > > > > > I'd like to discuss this issue again. > > > > > > > > On Wed, Mar 18, 2026 at 2:42 PM Philippe Mathieu-Daudé > > > > <[email protected]> wrote: > > > >> > > > >> On 18/3/26 05:40, Jim Shu wrote: > > > >> > On Tue, Feb 10, 2026 at 8:25 AM Richard Henderson > > > >> > <[email protected]> wrote: > > > >> > ... > > > >> >> Hmm. This really overlaps the secure and space fields from arm, > > > >> >> and possibly some of the > > > >> >> others as well (e.g. user, requester_id, pid). > > > >> >> > > > >> >> I don't really have a good suggestion for that right now, but it > > > >> >> would be nice to not keep > > > >> >> expanding the count of these sorts of fields that somehow specify > > > >> >> the originator, but > > > >> >> clearly cannot overlap. > > > >> >> > > > >> >> I'm reasonably sure we've had this discussion before, but nothing > > > >> >> has come of it. > > > >> >> > > > >> >> Time to paint the bikeshed again? > > > >> > > > >> Last discussion IIRC: > > > >> https://lore.kernel.org/qemu-devel/CAFEAcA8vKNkfKgp_Yymo9NA1=e2xjyxamtgo3z6q6dhgqka...@mail.gmail.com/ > > > > > > > > > > > > Follow the idea in the above thread. I'd plan to add the 'src_cpu_id' > > > > field to 'MemTxAttr', so we can get the CPUState from MemTxAttr. Thus, > > > > we can retrieve the RISC-V world_id from CPU and we don't need to add > > > > world_id to 'MemTxAttr'. If other security attributes are stored in CPU > > > > states, they can also reuse this w/o adding more data to 'MemTxAttr'. > > > > > > We already have a requester_id field, which is basically > > > "what is the thing that is sending this request?". We > > > shouldn't have memory transactions that happen to be > > > from CPUs indicate the source in a totally different way. > > Thanks for mentioning it! I think re-use requester_id is better. > Then, I think we only need a boolean flag `src_is_cpu` and we can > rename it to `requester_is_cpu` to match the naming of `requester_id`.
But, what do you now want it for? If we put the world ID in the MemTxAttrs, do you still need a CPU ID? (There are some situations where it might be useful, but if we don't need it right now we shouldn't put doing that rework in the path of implementing the worldguard work you want to do.) > > > > The whole plan is to add the following 2 fields > > > > :: > > > > unsigned int src_is_cpu:1; > > > > uint16_t src_cpu_id; > > > > > > > > src_cpu_id is the CPU ID from 'CPUState->cpu_index'. src_is_cpu is a > > > > boolean flag to check if the transaction is from CPU. > > > > > > > > Moreover, I think this idea is extensible. DMA device transactions can > > > > also have security attributes like world_id. We can also add src_device > > > > field to 'MemTxAttr' to store the 'DeviceState *', so we can get the > > > > DeviceState from MemTxAttr to get security attributes. If 'DeviceState > > > > *' is too large to add to 'MemTxAttr', re-use requester_id as DMA > > > > device ID is another possible method to support this. > > > > > > I don't think that having the thing that receives the memory > > > transaction get or get hold of a pointer to the source of the > > > transaction that it then uses to get the world ID is a good idea. > > > (Especially not putting a DeviceState* into MemTxAttrs: the > > > size of the type needs to be kept small so we can conveniently > > > pass it around by value: we currently assert that it is 8 bytes.) > > OK, I think store the DMA device ID in the `requester_id` is another > way to keep the small size. > However, IIUC, the current SysbusDevice doesn't have the concept of a > device ID. We need to add an additional device ID to the device to > support this idea. What exactly is it that you need a device ID for? Earlier you suggest that you would want it to get the world ID -- but we should just put the world ID directly in the MemTxAttrs. thanks -- PMM
