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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

> 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.

Jan