Re: [PATCH v2 04/18] exec: Add RISC-V WorldGuard WID to MemTxAttrs

[Jim Shu <jim.shu@sifive.com>]

Hi Daniel,

Both '_reserved1' and '_reserved2' fields are only for padding
MemTxAttrs struct to be 8-byte [1], so I remove a 1-byte reserved
field when adding 'world_id' field to it.
Is it ok for you? Or you think it is better to separate them.


[1]
commit 5014e33b1e00d330f13df33c09a3932ac88f8d94
Link: https://lore.kernel.org/r/20250121151322.171832-2-zhao1.liu@intel.com

Thanks!

On Sat, Aug 9, 2025 at 8:34 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
>
>
> On 4/17/25 7:52 AM, Jim Shu wrote:
> > RISC-V WorldGuard will add 5-bit world_id (WID) to the each memory
> > transaction on the bus. The wgChecker in front of RAM or peripherals
> > MMIO could do the access control based on the WID. It is similar to ARM
> > TrustZone NS bit, but the WID is 5-bit.
> >
> > The common implementation of WID is AXI4 AxUSER signal.
> >
> > Signed-off-by: Jim Shu <jim.shu@sifive.com>
> > ---
> >   include/exec/memattrs.h | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
> > index 8db1d30464..7a6866fa41 100644
> > --- a/include/exec/memattrs.h
> > +++ b/include/exec/memattrs.h
> > @@ -54,6 +54,11 @@ typedef struct MemTxAttrs {
> >        */
> >       unsigned int pid:8;
> >
> > +    /*
> > +     * RISC-V WorldGuard: the 5-bit WID field of memory access.
> > +     */
> > +    unsigned int world_id:5;
> > +
> >       /*
> >        * Bus masters which don't specify any attributes will get this
> >        * (via the MEMTXATTRS_UNSPECIFIED constant), so that we can
> > @@ -63,8 +68,7 @@ typedef struct MemTxAttrs {
> >        */
> >       bool unspecified;
> >
> > -    uint8_t _reserved1;
> > -    uint16_t _reserved2;
> > +    uint16_t _reserved1;
>
> Is 'reserved2' unused? Not sure why you ended up removing it in this patch.
>
> If it's really unused it's ok to remove it but this should be done in separate.
>
>
> Thanks,
>
> Daniel
>
>
> >   } MemTxAttrs;
> >
> >   QEMU_BUILD_BUG_ON(sizeof(MemTxAttrs) > 8);
>