Re: [PATCH v8 8/8] hw/riscv/virt: Add IOPMP support

[Alistair Francis]

On Fri, Aug 9, 2024 at 8:14 PM Ethan Chen <ethan84@andestech.com> wrote:
>
> On Thu, Aug 08, 2024 at 02:01:13PM +1000, Alistair Francis wrote:
> >
> > On Mon, Jul 15, 2024 at 8:15 PM Ethan Chen via <qemu-devel@nongnu.org> 
> > wrote:
> > >
> > > - Add 'iopmp=on' option to enable IOPMP. It adds an iopmp device virt 
> > > machine
> > >   to protect all regions of system memory, and configures RRID of CPU.
> > >
> > > Signed-off-by: Ethan Chen <ethan84@andestech.com>
> > > ---
> > >  docs/system/riscv/virt.rst |  5 +++
> > >  hw/riscv/Kconfig           |  1 +
> > >  hw/riscv/virt.c            | 63 ++++++++++++++++++++++++++++++++++++++
> > >  include/hw/riscv/virt.h    |  5 ++-
> > >  4 files changed, 73 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/docs/system/riscv/virt.rst b/docs/system/riscv/virt.rst
> > > index 9a06f95a34..9fd006ccc2 100644
> > > --- a/docs/system/riscv/virt.rst
> > > +++ b/docs/system/riscv/virt.rst
> > > @@ -116,6 +116,11 @@ The following machine-specific options are supported:
> > >    having AIA IMSIC (i.e. "aia=aplic-imsic" selected). When not specified,
> > >    the default number of per-HART VS-level AIA IMSIC pages is 0.
> > >
> > > +- iopmp=[on|off]
> > > +
> > > +  When this option is "on", an IOPMP device is added to machine. IOPMP 
> > > checks
> > > +  memory transcations in system memory. This option is assumed to be 
> > > "off".
> >
> > We probably should have a a little more here. You don't even mention
> > that this is the rapid-k model.
>
> I'll provide more details.
>
> >
> > It might be worth adding a `model` field, to make it easier to add
> > other models in the future. Thoughts?
> >
>
> I think the IOPMP model should be a device property and not
> configured here.

It should be a device property, but then how does a user configure
that? I guess users can globally set device props, but it's a bit
clunky

Alistair