On Mon, Nov 06, 2023 at 10:34:41AM +0000, Peter Maydell wrote:
> On Mon, 6 Nov 2023 at 01:57, Ethan Chen <etha...@andestech.com> wrote:
> >
> > On Fri, Nov 03, 2023 at 10:34:28AM +0000, Peter Maydell wrote:
> > > On Fri, 3 Nov 2023 at 03:29, Ethan Chen <etha...@andestech.com> wrote:
> > > >
> > > > On Thu, Nov 02, 2023 at 01:53:05PM +0000, Peter Maydell wrote:
> > > > > On Thu, 2 Nov 2023 at 13:49, Peter Xu <pet...@redhat.com> wrote:
> > > > > >
> > > > > > On Thu, Nov 02, 2023 at 05:40:12PM +0800, Ethan Chen wrote:
> > > > > > > Signed-off-by: Ethan Chen <etha...@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.
> > >
> > > Yes, but you don't pass the start and end address on the AXI
> > > bus, so they don't go in QEMU's MemTxAttrs either.
> >
> > I will add those AXI bus signals to MemTxAttrs instead of using start and
> > end
> > address in next revision.
>
> What AXI bus signals? You already get address and size in the
> actual memory transaction, they don't need to go in the MemTxAttrs.
>
A burst contains multiple continuous read or write operations. In current
transaction, I can only get the size and address of a single operation. IOPMP
checks not only a single operation but also the burst information. I propose
to add those signals to MemTxAttrs.
Following AXI signals are needed for IOPMP.
(AW/AR)ADDR: Address of the first beat(operation) of the burst
(AW/AR)LEN: Number of beats inside the burst
(AW/AR)SIZE: Size of each beat
(AW/AR)BURST: Type of the burst
Thanks,
Ethan Chen
