Re: [PATCH v1 3/3] hw/riscv: virt: Add optional ACLINT support to virt machine

From: Bin Meng <bmeng.cn@gmail.com>
To: Anup Patel <anup@brainfault.org>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	"open list:RISC-V" <qemu-riscv@nongnu.org>,
	Sagar Karandikar <sagark@eecs.berkeley.edu>,
	Anup Patel <anup.patel@wdc.com>,
	"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
	Atish Patra <atish.patra@wdc.com>,
	Alistair Francis <Alistair.Francis@wdc.com>,
	Palmer Dabbelt <palmer@dabbelt.com>
Subject: Re: [PATCH v1 3/3] hw/riscv: virt: Add optional ACLINT support to virt machine
Date: Tue, 13 Jul 2021 07:05:25 +0800	[thread overview]
Message-ID: <CAEUhbmWgmu3UKfAtq31E=BrBhJeWUSC+KhQ2FTCD2PBqnact_g@mail.gmail.com> (raw)
In-Reply-To: <CAAhSdy195JTXoZsk=XOZmQog2X4ZP2Npty-74SvorC8Kjjdm9g@mail.gmail.com>

On Mon, Jul 12, 2021 at 11:03 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Mon, Jul 12, 2021 at 6:41 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Mon, Jul 12, 2021 at 6:54 PM Anup Patel <anup@brainfault.org> wrote:
> > >
> > > On Mon, Jul 12, 2021 at 11:45 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > On Mon, Jul 12, 2021 at 1:39 PM Anup Patel <anup@brainfault.org> wrote:
> > > > >
> > > > > On Mon, Jun 14, 2021 at 5:52 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > >
> > > > > > On Sun, Jun 13, 2021 at 12:14 AM Anup Patel <anup.patel@wdc.com> wrote:
> > > > > > >
> > > > > > > We extend virt machine to emulate ACLINT devices only when "aclint=on"
> > > > > > > parameter is passed along with machine name in QEMU command-line.
> > > > > > >
> > > > > > > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> > > > > > > ---
> > > > > > >  hw/riscv/virt.c         | 110 +++++++++++++++++++++++++++++++++++++++-
> > > > > > >  include/hw/riscv/virt.h |   2 +
> > > > > > >  2 files changed, 111 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > > > > > > index 977d699753..a35f66af13 100644
> > > > > > > --- a/hw/riscv/virt.c
> > > > > > > +++ b/hw/riscv/virt.c
> > > > > > > @@ -50,6 +50,7 @@ static const MemMapEntry virt_memmap[] = {
> > > > > > >      [VIRT_TEST] =        {   0x100000,        0x1000 },
> > > > > > >      [VIRT_RTC] =         {   0x101000,        0x1000 },
> > > > > > >      [VIRT_CLINT] =       {  0x2000000,       0x10000 },
> > > > > > > +    [VIRT_ACLINT_SSWI] = {  0x2F00000,        0x4000 },
> > > > > >
> > > > > > How about we reuse the same register space to support both CLINT and
> > > > > > ACLINT? This saves some register space for future extension.
> > > > >
> > > > > The intention of placing ACLINT SSWI separate from ACLINT MTIMER and
> > > > > MSWI is to minimize PMP region usage.
> > > >
> > > > Okay, so this leaves spaces for 240 ACLINT MTIMER and MSWI devices in
> > > > total, if we put ACLINT SSWI at 0x2F00000, and we still have spaces
> > > > for 64 ACLINT SSWI devices. Is this enough?
> > >
> > > We just need one instance of MTIMER, MSWI, and SSWI per-socket.
> > > Current limit of max sockets in RISC-V virt machine is 8. We will be
> > > reducing this to 4 due space required by IMSICs. This means no matter
> > > what 8 instances of each MTIMER, MSWI, and SSWI is the max we
> > > can go for RISC-V virt machine. This limits are due to the fact that
> > > we want to fit devices in first 2GB space.
> > >
> >
> > Can you list the maximum ACLINT devices and their memory map we intend
> > to support and with that we can see how many PMP is used?
>
> For 4 sockets, we will have following layout:
> 0x2000000-0x200FFFF (Socket0 MTIMER and MSWI)
> 0x2010000-0x201FFFF (Socket1 MTIMER and MSWI)
> 0x2020000-0x202FFFF (Socket2 MTIMER and MSWI)
> 0x2030000-0x203FFFF (Socket3 MTIMER and MSWI)
> 0x2F00000-0x2F03FFF (Socket0 SSWI)
> 0x2F04000-0x2F07FFF (Socket1 SSWI)
> 0x2F08000-0x2F0bFFF (Socket2 SSWI)
> 0x2F0C000-0x2F0FFFF (Socket3 SSWI)
>
> OpenSBI will create one PMP region to protect all
> MTIMERs and MSWIs which is:
> 0x2000000-0x203FFFF

Thanks! This makes sense.

Regards,
Bin