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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.07.2025 12:13, Oleksii Kurochko wrote:
> 
> 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.

As said - imo this needs abstracting away.

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

Well, the name we use on x86 (and I think this was quite obviously implied
by earlier replies of mine): p2m_mmio_direct.

Jan