Re: [Xen-devel] [PATCH for-4.12] x86/p2m: Drop erroneous #VE-enabled check in ept_set_entry()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.01.19 at 19:28, <andrew.cooper3@xxxxxxxxxx> wrote:
> Code clearing the "Suppress VE" bit in an EPT entry isn't nececsserily running
> in current context.  In ALTP2M_external mode, it definitely is not, and in PV
> context, vcpu_altp2m(current) acts upon the HVM union.
> 
> Even if we could sensibly resolve the target vCPU, it may legitimately not be
> fully set up at this point, so rejecting the EPT modification would be buggy.
> 
> There is a path in hvm_hap_nested_page_fault() which explicitly emulates #VE
> in the cpu_has_vmx_virt_exceptions case, so the -EOPNOTSUPP part of this

ITYM "in the !cpu_has_vmx_virt_exceptions case" here?

> condition is also wrong.

What about the similar conditions in the handling of
HVMOP_altp2m_vcpu_{en,dis}able_notify then?

> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -702,16 +702,6 @@ ept_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t 
> mfn,
>  
>      ASSERT(ept);
>  
> -    if ( !sve )
> -    {
> -        if ( !cpu_has_vmx_virt_exceptions )
> -            return -EOPNOTSUPP;
> -
> -        /* #VE should be enabled for this vcpu. */
> -        if ( gfn_eq(vcpu_altp2m(current).veinfo_gfn, INVALID_GFN) )
> -            return -ENXIO;
> -    }

How about retaining the latter, but qualifying it with
current->domain == d?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel