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

[Peter Maydell]

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.

thanks
-- PMM