Re: [Xen-devel] [PATCH v1 4/6] x86/hvm: Add SMAP support to HVM guest

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.04.14 at 03:49, <feng.wu@xxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> >>> On 15.04.14 at 15:02, <feng.wu@xxxxxxxxx> wrote:
>> > @@ -371,6 +383,7 @@ static inline int hvm_event_pending(struct vcpu *v)
>> >          X86_CR4_MCE | X86_CR4_PGE | X86_CR4_PCE |       \
>> >          X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT |           \
>> >          (cpu_has_smep ? X86_CR4_SMEP : 0) |             \
>> > +        (hvm_cpuid_has_smap() ? X86_CR4_SMAP : 0) |     \
>> 
>> What's the reason for the asymmetry with SMEP here? Also, did you
>> verify that v == current in all call paths? And even if you did, passing
>> v into the function and adding a respective ASSERT() would seem
>> rather desirable.
> 
> My original thought is that "cpu_has_smap" reflects whether the host cpu has
> SMAP feature, however, here we need to check whether the VCPU of the hvm
> has this feature, so I used hvm_cpuid_has_smap() instead. But I thought 
> about
> this logic again yesterday, seems using "cpu_has_smap" here is okay, since 
> we
> always expose the SMAP feature to HVM guest if it exists on the host cpu. So
> I will change this to align with smep.

Please indeed do so initially, even if your argumentation is slightly
wrong: Via CPUID masking the guest may not see the SMEP and/or
SMAP features, and hence cpu_has_sm[ea]p isn't generally the
correct qualifier. So we'd need a fixup patch on top (or, alternatively
one preceding the SMAP additions, correcting the SMEP aspect here,
in which case then your change could remain as is).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel