Re: [PATCH v5 3/3] x86/altp2m: Add a hvmop for querying the suppress #VE bit

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 9/20/18 3:50 PM, George Dunlap wrote:
> On 09/03/2018 04:48 PM, Adrian Pop wrote:
>> Signed-off-by: Adrian Pop <apop@bitdefender.com>
>> +++ b/xen/arch/x86/mm/mem_access.c
>> @@ -550,6 +550,51 @@ out:
>>      return rc;
>>  }
>>  
>> +int p2m_get_suppress_ve(struct domain *d, gfn_t gfn, bool *suppress_ve,
>> +                        unsigned int altp2m_idx)
>> +{
>> +    struct p2m_domain *host_p2m = p2m_get_hostp2m(d);
>> +    struct p2m_domain *ap2m = NULL;
>> +    struct p2m_domain *p2m;
>> +    mfn_t mfn;
>> +    p2m_access_t a;
>> +    p2m_type_t t;
>> +
>> +    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;
> 
> Basically the same comments as for 2/3:  Move to p2m.c, and get rid of
> the vmx-ism.
> 
> Anothre idea is to get rid of these checks altogether -- returning
> 'false' when the feature isn't supported or enabled shouldn't be a big
> deal.  But I don't feel strongly enough about it to argue either way.

I've moved the functions to p2m.c and started moving the checks when it
occured to me that it might be cha to move them to ept_get_entry() /
ept_set_entry() in p2m-ept.c.

ept_get_entry() uses a bool_t *sve pointer, so I could check if the
pointer is not NULL, and if it's not perform the checks then, and so
require the caller to always pass a NULL pointer in non-#VE cases.
Still, ept_get_entry() returns a mfn_t, so all can do there is return
INVALID_MFN, which is not very helpful.

ept_set_entry() is more complicated: it takes "int sve" as a parameter
(which makes sense because it can also be -1) - but then really there's
no special value which should trigger a check. Any value could be a
legitimate attempt to set that bit - although 0 (clearing it) might be
viewed as a special case, like it is in the NPT code.

I could indeed remove all the checks and allow setting and getting that
bit regardless of whether #VE is enabled and / or available - if that's
acceptable (though I believe someone has required the checks at some
point in Adrian's work with the patches).


Thanks,
Razvan

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