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

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 <[email protected]>
> ---
> 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.

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

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

>   /*
>    * The following flags are used to specify behavior of various get and
>    * put commands.  The first is also stored in page->partial_flags to
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -705,12 +705,14 @@ static XSM_INLINE int cf_check xsm_updat
>   
>   #endif /* CONFIG_PV */
>   
> +#if defined(CONFIG_PV) || defined(CONFIG_SHADOW_PAGING)
>   static XSM_INLINE int cf_check xsm_priv_mapping(
>       XSM_DEFAULT_ARG struct domain *d, struct domain *t)
>   {
>       XSM_ASSERT_ACTION(XSM_TARGET);
>       return xsm_default_action(action, d, t);
>   }
> +#endif
>   
>   static XSM_INLINE int cf_check xsm_ioport_permission(
>       XSM_DEFAULT_ARG struct domain *d, uint32_t s, uint32_t e, uint8_t allow)
> --- a/xen/include/xsm/hooks.h
> +++ b/xen/include/xsm/hooks.h
> @@ -141,7 +141,9 @@ XSM_HOOK(int, mmu_update, struct domain
>   XSM_HOOK(int, mmuext_op, struct domain *, struct domain *)
>   XSM_HOOK(int, update_va_mapping, struct domain *, struct domain *, l1_pgentry_t)
>   #endif /* CONFIG_PV */
> +#if defined(CONFIG_PV) || defined(CONFIG_SHADOW_PAGING)
>   XSM_HOOK(int, priv_mapping, struct domain *, struct domain *)
> +#endif
>   XSM_HOOK(int, ioport_permission, struct domain *, uint32_t, uint32_t, uint8_t)
>   XSM_HOOK(int, ioport_mapping, struct domain *, uint32_t, uint32_t, uint8_t)
>   XSM_HOOK(int, pmu_op, struct domain *, unsigned int)
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -1828,10 +1828,12 @@ static int cf_check flask_update_va_mapp
>   
>   #endif /* CONFIG_PV */
>   
> +#if defined(CONFIG_PV) || defined(CONFIG_SHADOW_PAGING)
>   static int cf_check flask_priv_mapping(struct domain *d, struct domain *t)
>   {
>       return domain_has_perm(d, t, SECCLASS_MMU, MMU__TARGET_HACK);
>   }
> +#endif
>   
>   static int cf_check flask_pmu_op(struct domain *d, unsigned int op)
>   {
> 

I think it would be a good defensive approach to condition out the 
header declaration. Otherwise,

Acked-by: Daniel P. Smith <[email protected]>