Re: [PATCH 08/16] x86/P2M: PoD, altp2m, and nested-p2m are HVM-only

[George Dunlap <George.Dunlap@xxxxxxxxxx>]

> On Jul 5, 2021, at 5:09 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> 
> There's no need to initialize respective data for PV domains. Note that
> p2m_teardown_{alt,nested}p2m() will handle the lack-of-initialization
> case fine.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -102,6 +102,9 @@ static int p2m_initialise(struct domain
>     p2m->default_access = p2m_access_rwx;
>     p2m->p2m_class = p2m_host;
> 
> +    if ( !is_hvm_domain(d) )
> +        return 0;
> +
>     p2m_pod_init(p2m);
>     p2m_nestedp2m_init(p2m);


> 
> @@ -259,7 +262,7 @@ int p2m_init(struct domain *d)
>     int rc;
> 
>     rc = p2m_init_hostp2m(d);
> -    if ( rc )
> +    if ( rc || !is_hvm_domain(d) )
>         return rc;
> 
> #ifdef CONFIG_HVM
> --- a/xen/arch/x86/mm/p2m.h
> +++ b/xen/arch/x86/mm/p2m.h
> @@ -17,6 +17,8 @@
> 
> #include <xen/mem_access.h>
> 
> +void p2m_pod_init(struct p2m_domain *p2m);
> +
> int p2m_add_identity_entry(struct domain *d, unsigned long gfn,
>                            p2m_access_t p2ma, unsigned int flag);
> int p2m_remove_identity_entry(struct domain *d, unsigned long gfn);
> --- a/xen/arch/x86/mm/p2m-pod.c
> +++ b/xen/arch/x86/mm/p2m-pod.c
> @@ -1135,6 +1135,12 @@ p2m_pod_demand_populate(struct p2m_domai
>     mfn_t mfn;
>     unsigned long i;
> 
> +    if ( !p2m_is_hostp2m(p2m) )
> +    {
> +        ASSERT_UNREACHABLE();
> +        return false;
> +    }
> +
>     ASSERT(gfn_locked_by_me(p2m, gfn));
>     pod_lock(p2m);

Why this check rather than something which explicitly says HVM?

If you really mean to check for HVM here but are just using this as a shortcut, 
it needs a comment.

With that addressed:

Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>

 -George