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

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 8/3/26 6:11 AM, Jan Beulich wrote:
> 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.
But it's not the same, the enforcement mechanism is different. FLASK is 
an evaluation of Subject/Object/Predicate. In this case the mechanism 
(software enforced access) that provides the Predicate has enough risk 
that it warranted itself a separate check to allow fine grained 
assignment of the operation to a specific domain which was driven by a 
specific use case.
> > I think the question is how to address the TARGET_HACK situation.
> I fear I don't really know what exactly you mean here.
Is this path still needed for the pvfb or is it now in use by other use 
cases. If the former, then close the ability otherwise TARGET_HACK 
should be renamed to something sensible for general case. Some code 
documentation might be necessary to help understand why/
> > > --- 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