Re: [PATCH v2 1/4] exec/memattrs: Add iopmp source id, start address, end address to MemTxAttrs

[Ethan Chen]

On Thu, Nov 02, 2023 at 01:53:05PM +0000, Peter Maydell wrote:
> On Thu, 2 Nov 2023 at 13:49, Peter Xu <peterx@redhat.com> wrote:
> >
> > On Thu, Nov 02, 2023 at 05:40:12PM +0800, Ethan Chen wrote:
> > > Signed-off-by: Ethan Chen <ethan84@andestech.com>
> > > ---
> > >  include/exec/memattrs.h | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
> > > index d04170aa27..fc15e5d7d3 100644
> > > --- a/include/exec/memattrs.h
> > > +++ b/include/exec/memattrs.h
> > > @@ -64,6 +64,12 @@ typedef struct MemTxAttrs {
> > >      unsigned int target_tlb_bit0 : 1;
> > >      unsigned int target_tlb_bit1 : 1;
> > >      unsigned int target_tlb_bit2 : 1;
> > > +
> > > +    /* IOPMP support up to 65535 sources */
> > > +    unsigned int iopmp_sid:16;
> >
> > There's MemTxAttrs.requester_id, SID for pci, same length.  Reuse it?
> >
> > > +    /* Transaction infomation for IOPMP */
> > > +    unsigned long long iopmp_start_addr;
> > > +    unsigned long long iopmp_end_addr;
> >
> > PS: encoding addresses into memattrs is.. strange, but since I know nothing
> > about iopmp, I'll leave that for other reviewers.
> >
> > Currently MemTxAttrs are passed as a whole int on the stack, if it keeps
> > growing we may start to consider a pointer, but need to check the side
> > effects of unexpected fields modified within a call.
> 
> Yeah, this struct is intended to model the various attributes that
> get passed around on the bus alongside data in real hardware.
> I'm pretty sure no real hardware is passing around start and
> end transaction addresses on its bus with every read and
> write, which suggests that we should be doing this some other
> way than adding these fields to the MemTxAttrs struct.

For AXI bus ADDR, LEN, SIZE are signals in read/write address channel.
IOPMP will check that start address = ADDR, 
and end address = ADDR + LEN * SIZE.

Thanks,
Ethan Chen