On 23.05.2025 13:34, Oleksii Kurochko wrote:
> 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;
That takes care of the r/o aspect, yes, but not of the "writes are silently
dropped" one.
>>> + 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.
My earlier comment was mainly about p2m_map_foreign_ro.
> 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.
Indeed. And maybe it'll just be p2m_map_foreign, as we have it on x86.
Jan
