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