Re: [PATCH 2/4] x86/vmware: Add a header file for hypercall definitions

[Thomas HellstrÃm (VMware)]

On 8/18/19 10:19 PM, Borislav Petkov wrote:
> On Sun, Aug 18, 2019 at 04:33:14PM +0200, Thomas HellstrÃm (VMware) wrote:
> > From: Thomas Hellstrom <thellstrom@xxxxxxxxxx>
> >
> > This is intended to be used by drivers using the backdoor, and
> > we follow the kvm example using alternatives self-patching to
> > choose between vmcall, vmmcall and inl instructions.
> >
> > diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> > index 998c2cc08363..69cecc3bc9cb 100644
> > --- a/arch/x86/include/asm/cpufeatures.h
> > +++ b/arch/x86/include/asm/cpufeatures.h
> > @@ -232,6 +232,8 @@
> >   #define X86_FEATURE_VMMCALL		( 8*32+15) /* Prefer VMMCALL to VMCALL */
> >   #define X86_FEATURE_XENPV		( 8*32+16) /* "" Xen paravirtual guest */
> >   #define X86_FEATURE_EPT_AD		( 8*32+17) /* Intel Extended Page Table access-dirty bit */
> > +#define X86_FEATURE_VMW_VMCALL		( 8*32+18) /* VMware prefers VMCALL hypercall instruction */
> > +#define X86_FEATURE_VMW_VMMCALL		( 8*32+19) /* VMware prefers VMMCALL hypercall instruction */
> Those are not set anywhere in the patchset. Please send them with their
> user.

Thanks for reviewing. Hmm, That hunk appears to have been lost. I'll 
restore in v2.

> Then, there already is X86_FEATURE_VMMCALL which you can use too, I'd
> guess.

Unfortunately we can't use it, because it's unconditionally set on AMD 
even if the VMware hypervisor
doesn't support it (by version or by configuration).

I was thinking of having the VMware platform code clear it if vmmcall 
wasn't supported but that would most probably conflict with other 
hypervisors using the vmmouse driver, still using the old "inl" 
interface for the vmmouse backdoor, but otherwise requiring vmmcall for 
other hypercalls.

> Also, I don't see a reason to show the synthetic bit in /proc/cpuinfo
> so when you define it, add "":
>
> #define X86_FEATURE_VMW_VMCALL		( 8*32+18) /* "" VMware prefers VMCALL hypercall instruction */
> 						      ^
> 						      |
> 						      |
> 						      this here.

Ah. I wasn't aware of that. I'll add. Thanks.

Thomas


> Thx.