Re: [PATCH 12/24] x86/mm: get_page_from_l1e() is PV-or-shadow-only

[Jan Beulich <jbeulich@xxxxxxxx>]

On 02.08.2026 17:55, Daniel P. Smith wrote:
> On 7/28/26 9:18 AM, Jan Beulich wrote:
>> Otherwise the function is unreachable, violating MISRA C:2012 rule 2.1.
>> With the function compiled out, its dedicated XSM hook also becomes
>> unreachable, so it is similarly guarded.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> It feels suspicious that the .priv_mapping() check is used for HVM guests
>> in shadow mode, but not for ones in HAP mode.
> 
> I believe a hint to it is laying in the comment,
> 
>   /*
>    * Let privileged domains transfer the right to map their target
>    * domain's pages. This is used to allow stub-domain pvfb export to
>    * dom0, until pvfb supports granted mappings. At that time this
>    * minor hack can go away.
>    */
> 
> Correct me if I am wrong, but get_page_from_l1e() is only used by PV and 
> HVM + Shadow. When in HVM + HAP is mapping a guest page, it is done 
> through p2m_get_foreign() which will then be covered by 
> xsm_map_gmfn_foreign(). So only HVM + Shadow can hit TARGET_HACK check.

Yes, sure; that wasn't the point of my comment. The point was that I'd
expect _the same_ hook to be used by the other path. Aiui if you make a
policy, you want same situations dealt with the same. Hence there shouldn't
be a need to express the same thing two ways.

> I think the question is how to address the TARGET_HACK situation.

I fear I don't really know what exactly you mean here.

>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -837,6 +837,8 @@ static int cf_check print_mmio_emul_rang
>>   }
>>   #endif
>>   
>> +#if defined(CONFIG_PV) || defined(CONFIG_SHADOW_PAGING)
>> +
>>   /*
>>    * get_page_from_l1e returns:
>>    *   0  => success (page not present also counts as such)
>> @@ -1038,6 +1040,8 @@ get_page_from_l1e(
>>       return -EBUSY;
>>   }
>>   
>> +#endif /* CONFIG_PV || CONFIG_SHADOW_PAGING */
>> +
> 
> Would it also not be prudent to #ifdef out the declaration in asm/mm.h?

Ah, yes, this looks possible for this function - the decl isn't needed for any
DCE-ing by the compiler.

> I think it would be a good defensive approach to condition out the 
> header declaration. Otherwise,
> 
> Acked-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>

Thanks, also for all the others.

Jan