Re: [PATCH v2 08/17] xen/riscv: add new p2m types and helper macros for type classification

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 6/30/25 5:50 PM, Jan Beulich wrote:

> On 30.06.2025 17:27, Oleksii Kurochko wrote:
> > On 6/30/25 4:45 PM, Jan Beulich wrote:
> > > On 30.06.2025 16:38, Oleksii Kurochko wrote:
> > > > On 6/30/25 4:33 PM, Oleksii Kurochko wrote:
> > > > > On 6/26/25 4:59 PM, Jan Beulich wrote:
> > > > > > On 10.06.2025 15:05, Oleksii Kurochko wrote:
> > > > > > > --- a/xen/arch/riscv/include/asm/p2m.h
> > > > > > > +++ b/xen/arch/riscv/include/asm/p2m.h
> > > > > > > @@ -61,8 +61,28 @@ 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 */
> > > > > > >                 
> > > > > > As indicated before - this type should be added when the special handling that
> > > > > > it requires is also introduced.
> > > > > >               
> > > > > Perhaps, I missed that. I will drop this type for now.
> > > > >
> > > > > > > +    p2m_mmio_direct_dev,/* Read/write mapping of genuine Device MMIO area */
> > > > > > >                 
> > > > > > What's the _dev suffix indicating here?
> > > > > >               
> > > > > It indicates that it is device memory, probably, it isn't so necessary in case of RISC-V as
> > > > > spec doesn't use such terminology. In RISC-V there is only available IO, NC. And we are
> > > > > |using PTE_PBMT_IO for |p2m_mmio_direct_dev.
> > > > >
> > > > > Maybe it would be better just to rename s/p2m_mmio_direct_dev/p2m_mmio_direct_io.
> > > > >             
> > > > I forgot that p2m_mmio_direct_dev is used by common code for dom0less code (handle_passthrough_prop())
> > > >           
> > > That'll want abstracting out, I think. I don't view it as helpful to clutter
> > > RISC-V (and later perhaps also PPC) with Arm-specific terminology.
> > >         
> > Would it be better then just rename it to p2m_device? Then it won't clear for Arm which type of MMIO p2m's
> > types is used as Arm has there MMIO types: *_dev, *_nc, *_c.
> >       
> I don't understand why Arm matters here. P2M types want naming in a way that makes
> sense for RISC-V.
>     
It doesn't matter.
But if we want to change the type name from p2m_mmio_direct_dev to p2m_mmio_direct or p2m_device then it will
affect Arm too as p2m_mmio_direct_dev is used in dom0less code which is also used by Arm.
I just re-used p2m_mmio_direct_dev as it looked for me pretty generic and clear for what this type is.

> > As an option (which I don't really like) it could be "#define p2m_mmio_direct_dev ARCH_specific_name" in
> > asm/p2m.h to not touch common code.
> >       
> A #define may be needed, but not one to _still_ introduce Arm naming into non-Arm
> code.
>     
As I mentioned above that p2m_mmio_direct_dev sounds pretty generic to me and I am okay to use it for
RISC-V. But if you have better suggestions I will be happy to consider it.

~ Oleksii