On 15.05.2024 11:03, Sergiy Kibrik wrote:
> --- 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);
> +        }

With less code churn and less indentation:

    rc = hvm_altp2m_supported() ? p2m_init_altp2m(d) : 0;
    if ( rc )
    {
        p2m_teardown_hostp2m(d);
        p2m_teardown_nestedp2m(d);
    }

>      }
> -
>      return rc;
>  }

Please don't remove the blank line ahead of the main return of a function.

> --- 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())

I agree with Stefano's ordering comment here, btw.

Jan

Reply via email to