[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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




> -----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?


> Jan

Thanks,
Feng

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.