Re: [XEN PATCH v2 03/15] x86/p2m: guard altp2m routines

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 15 May 2024, Sergiy Kibrik wrote:
> Initialize and bring down altp2m only when it is supported by the platform,
> e.g. VMX. Also guard p2m_altp2m_propagate_change().
> The puspose of that is the possiblity to disable altp2m support and exclude 
> its
> code from the build completely, when it's not supported by the target 
> platform.
> 
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
> CC: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> ---
>  xen/arch/x86/mm/p2m-basic.c | 19 +++++++++++--------
>  xen/arch/x86/mm/p2m-ept.c   |  2 +-
>  2 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/x86/mm/p2m-basic.c b/xen/arch/x86/mm/p2m-basic.c
> index 8599bd15c6..90106997d7 100644
> --- a/xen/arch/x86/mm/p2m-basic.c
> +++ b/xen/arch/x86/mm/p2m-basic.c
> @@ -126,13 +126,15 @@ int p2m_init(struct domain *d)
>          return rc;
>      }
>  
> -    rc = p2m_init_altp2m(d);
> -    if ( rc )
> +    if ( hvm_altp2m_supported() )
>      {
> -        p2m_teardown_hostp2m(d);
> -        p2m_teardown_nestedp2m(d);
> +        rc = p2m_init_altp2m(d);
> +        if ( rc )
> +        {
> +            p2m_teardown_hostp2m(d);
> +            p2m_teardown_nestedp2m(d);
> +        }
>      }
> -
>      return rc;
>  }
>  
> @@ -195,11 +197,12 @@ void p2m_final_teardown(struct domain *d)
>  {
>      if ( is_hvm_domain(d) )
>      {
> +        if ( hvm_altp2m_supported() )
> +            p2m_teardown_altp2m(d);
>          /*
> -         * We must tear down both of them unconditionally because
> -         * we initialise them unconditionally.
> +         * We must tear down nestedp2m unconditionally because
> +         * we initialise it unconditionally.
>           */
> -        p2m_teardown_altp2m(d);
>          p2m_teardown_nestedp2m(d);
>      }
>  
> diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
> index f83610cb8c..d264df5b14 100644
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -986,7 +986,7 @@ out:
>      if ( is_epte_present(&old_entry) )
>          ept_free_entry(p2m, &old_entry, target);
>  
> -    if ( entry_written && p2m_is_hostp2m(p2m) )
> +    if ( entry_written && p2m_is_hostp2m(p2m) && hvm_altp2m_supported())
>      {
>          ret = p2m_altp2m_propagate_change(d, _gfn(gfn), mfn, order, p2mt, 
> p2ma);
>          if ( !rc )

This is a matter of taste but I would put hvm_altp2m_supported() first.

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>