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

Reply via email to