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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.04.14 at 10:54, <feng.wu@xxxxxxxxx> wrote:

> 
>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: Wednesday, April 16, 2014 4:47 PM
>> To: Wu, Feng
>> Cc: Ian.Campbell@xxxxxxxxxx; Dong, Eddie; Nakajima, Jun;
>> xen-devel@xxxxxxxxxxxxx 
>> Subject: RE: [PATCH v1 4/6] x86/hvm: Add SMAP support to HVM guest
>> 
>> >>> 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).
>> 
> 
> So, I need send out a patch to fix this SMEP issue, then make the SMAP patch 
> set
> based on it, right?

That would be the ideal flow, yes.

Jan


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