On 17.02.2026 16:28, Orzel, Michal wrote:
>
>
> On 16/02/2026 16:20, Jan Beulich wrote:
>> Permitting writes when the P2M type says "read-only" can't be correct.
>>
>> Fixes: 1661158723a ("xen/arm: Extend copy_to_guest to support copying
>> from/to guest physical address")
>> Signed-off-by: Jan Beulich <[email protected]>
> Reviewed-by: Michal Orzel <[email protected]>
Thanks.
>> ---
>> What exactly p2m_ram_ro means on Arm is unclear: The comment next to its
>> definition says one thing, its use in get_page_from_gfn() says another.
>> (I remember raising this point before, i.e. it feels a little odd that the
>> ambiguity still exists.) The patch here assumes the comment is what is
>> wrong.
>>
>> --- a/xen/arch/arm/guestcopy.c
>> +++ b/xen/arch/arm/guestcopy.c
>> @@ -44,7 +44,7 @@ static struct page_info *translate_get_p
>> if ( !page )
>> return NULL;
>>
>> - if ( !p2m_is_ram(p2mt) )
>> + if ( write ? p2mt != p2m_ram_rw : !p2m_is_ram(p2mt) )
>> {
>> put_page(page);
>> return NULL;
>
> The ambiguity you mention is indeed problematic. This mixes page type with p2m
> type. The comment "The p2m_type is based on the type of the page" admits this
> conflation for DOMID_XEN.
>
> AFAICT, p2m_ram_ro is not used on Arm for normal domains. The only use is in
> get_page_from_gfn() for DOMID_XEN. Maybe we could change get_page_from_gfn()
> to
> always return p2m_ram_rw since DOMID_XEN has direct 1:1 access anyway?
But that's not correct for cases where share_xen_page_with_privileged_guest()
is passed SHARE_ro. XENMAPSPACE_gmfn_foreign requests have to result in r/o
mappings in that case.
Jan