Re: [PATCH v1 5/6] xen/riscv: add new p2m types and helper macros for type classification

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 5/20/25 5:11 PM, Jan Beulich wrote:

> On 09.05.2025 17:57, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/include/asm/p2m.h
> > +++ b/xen/arch/riscv/include/asm/p2m.h
> > @@ -80,8 +80,36 @@ struct p2m_domain {
> >  typedef enum {
> >      p2m_invalid = 0,    /* Nothing mapped here */
> >      p2m_ram_rw,         /* Normal read/write domain RAM */
> > +    p2m_ram_ro,         /* Read-only; writes are silently dropped */
> >       
> This is pretty special a type, which imo better wouldn't be introduced
> without there being proper support for it. (I don't suppose RISC-V
> hardware alone can effect this type?)
>     
It is possible to make ro by using r, w, x bits of page table entry in the
same way Arm does that:
    case p2m_ram_ro:
        e->p2m.xn = 0;
        e->p2m.write = 0;
        break;

> > +    p2m_mmio_direct_dev,/* Read/write mapping of genuine Device MMIO area */
> > +    p2m_map_foreign_rw, /* Read/write RAM pages from foreign domain */
> > +    p2m_map_foreign_ro, /* Read-only RAM pages from foreign domain */
> >       
> Aiui you took these from Arm. Looking at its sole use, I'm not convinced
> it's used correctly. If it is, the same comment as for p2m_ram_ro above
> would apply here, too.
>     
p2m_mmio_direct_dev - this one is defintely needed as it is used for device
pass through to guest domain to map device's MMIO. It seems to me like it is
correctly used.

Others we don't really use now in private branches but it seems like they could be
useful, so I added them now.

I can drop them now and return back if such functionality which uses them will be
introduced for RISC-V, and at that moment I think it will be
more clear if it is used correctly or not.
Right now, I am not sure if it is.

~ Oleksii