On 25/01/2019 10:25, Jan Beulich wrote:
>>>> On 24.01.19 at 19:28, <andrew.coop...@citrix.com> 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?
I do, yes.
>
>> condition is also wrong.
> What about the similar conditions in the handling of
> HVMOP_altp2m_vcpu_{en,dis}able_notify then?
In hindsight, I think that restriction (which was after the fact) was a
poor choice. I certainly wasn't aware of the emulated support at the time.
>
>> --- 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?
Why? There is a paragraph in the commit message explaining why this
check is wrong even when it isn't an out-of-bounds read.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
