linux-kernel - Re: [PATCH v10 1/8] x86/vmware: Introduce VMware hypercall API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240527170734.GCZlS91uXD68HRN1na@fat_crate.local>
Date: Mon, 27 May 2024 19:07:34 +0200
From: Borislav Petkov <bp@...en8.de>
To: Alexey Makhalov <alexey.makhalov@...adcom.com>
Cc: linux-kernel@...r.kernel.org, virtualization@...ts.linux.dev,
	hpa@...or.com, dave.hansen@...ux.intel.com, mingo@...hat.com,
	tglx@...utronix.de, x86@...nel.org, netdev@...r.kernel.org,
	richardcochran@...il.com, linux-input@...r.kernel.org,
	dmitry.torokhov@...il.com, zackr@...are.com,
	linux-graphics-maintainer@...are.com, pv-drivers@...are.com,
	timothym@...are.com, akaher@...are.com,
	dri-devel@...ts.freedesktop.org, daniel@...ll.ch, airlied@...il.com,
	tzimmermann@...e.de, mripard@...nel.org,
	maarten.lankhorst@...ux.intel.com, horms@...nel.org,
	kirill.shutemov@...ux.intel.com
Subject: Re: [PATCH v10 1/8] x86/vmware: Introduce VMware hypercall API

On Thu, May 23, 2024 at 12:14:39PM -0700, Alexey Makhalov wrote:
> +#define VMWARE_HYPERCALL						\
> +	ALTERNATIVE_3("",						\
> +		      "jmp .Lport_call%=", X86_FEATURE_HYPERVISOR,	\
> +		      "jmp .Lvmcall%=", X86_FEATURE_VMCALL,		\
> +		      "vmmcall\n\t"					\
> +		      "jmp .Lend%=", X86_FEATURE_VMW_VMMCALL)		\
> +		      "cmpb $"						\
> +			__stringify(CPUID_VMWARE_FEATURES_ECX_VMMCALL)	\
> +			", %[mode]\n\t"					\
> +		      "jg .Lvmcall%=\n\t"				\
> +		      "je .Lvmmcall%=\n\t"				\
> +		      ".Lport_call%=: movw %[port], %%dx\n\t"		\
> +		      "inl (%%dx), %%eax\n\t"				\
> +		      "jmp .Lend%=\n\t"					\
> +		      ".Lvmmcall%=: vmmcall\n\t"			\
> +		      "jmp .Lend%=\n\t"					\
> +		      ".Lvmcall%=: vmcall\n\t"				\
> +		      ".Lend%=:"

So applied (and with minor fixups for the proper indentation, see end of
this mail) this looks like this:

pushsection .altinstructions,"a"
 .long 661b - .
 .long 6641f - .
 .4byte ( 4*32+31)
 .byte 663b-661b
 .byte 6651f-6641f
 .long 661b - .
 .long 6642f - .
 .4byte ( 8*32+18)
 .byte 663b-661b
 .byte 6652f-6642f
 .long 661b - .
 .long 6643f - .
 .4byte ( 8*32+19)
 .byte 663b-661b
 .byte 6653f-6643f
popsection
pushsection .altinstr_replacement, "ax"
# ALT: replacement 1
6641:
        jmp .Lport_call72
6651:
# ALT: replacement 2
6642:
        jmp .Lvmcall72
6652:
# ALT: replacement 3
6643:
        vmmcall
        jmp .Lend72
6653:
popsection
        cmpb $((((1UL))) << (0)), vmware_hypercall_mode(%rip)   # vmware_hypercall_mode
        jg .Lvmcall72
        je .Lvmmcall72
Lport_call72:
        movw $22104, %dx        #
        inl (%dx), %eax
        jmp .Lend72
Lvmmcall72:
        vmmcall
        jmp .Lend72 
Lvmcall72:
        vmcall
Lend72:

---

so AFAICT, you want three things:

1. X86_FEATURE_HYPERVISOR - that is always set when running as a guest.
   For that it should do:

        movw $22104, %dx        #
        inl (%dx), %eax

2. X86_FEATURE_VMCALL:

	vmcall

3. X86_FEATURE_VMW_VMMCALL:

	vmmcall

So why don't you simply do that?

vmware_set_capabilities() sets vmware_hypercall_mode *and* those feature
flags at the same time.

And you either support VMCALL or VMMCALL so the first thing should be the
fallback for some ancient crap.

IOW, your hypercall alternative should simply be:

	ALTERNATIVE_2("vmcall", "vmmcall", X86_FEATURE_VMW_VMMCALL, "movw %[port], %%dx; "inl (%%dx), %%eax", X86_FEATURE_HYPERVISOR);

without any more silly dance?

Hmmm?

---

Fixup indentation for proper .s output:

diff --git a/arch/x86/include/asm/vmware.h b/arch/x86/include/asm/vmware.h
index 5114f4c75c54..8be877d8bb7c 100644
--- a/arch/x86/include/asm/vmware.h
+++ b/arch/x86/include/asm/vmware.h
@@ -70,17 +70,18 @@ extern u8 vmware_hypercall_mode;
 		      "jmp .Lvmcall%=", X86_FEATURE_VMCALL,		\
 		      "vmmcall\n\t"					\
 		      "jmp .Lend%=", X86_FEATURE_VMW_VMMCALL)		\
-		      "cmpb $"						\
-			__stringify(CPUID_VMWARE_FEATURES_ECX_VMMCALL)	\
-			", %[mode]\n\t"					\
+		      "\tcmpb $" __stringify(CPUID_VMWARE_FEATURES_ECX_VMMCALL) ", %[mode]\n\t" \
 		      "jg .Lvmcall%=\n\t"				\
-		      "je .Lvmmcall%=\n\t"				\
-		      ".Lport_call%=: movw %[port], %%dx\n\t"		\
+		      "je .Lvmmcall%=\n"				\
+		      ".Lport_call%=:\n\t"				\
+		      "movw %[port], %%dx\n\t"				\
 		      "inl (%%dx), %%eax\n\t"				\
-		      "jmp .Lend%=\n\t"					\
-		      ".Lvmmcall%=: vmmcall\n\t"			\
-		      "jmp .Lend%=\n\t"					\
-		      ".Lvmcall%=: vmcall\n\t"				\
+		      "jmp .Lend%=\n"					\
+		      ".Lvmmcall%=:\n\t"				\
+		      "vmmcall\n\t"					\
+		      "jmp .Lend%=\n"					\
+		      ".Lvmcall%=:\n\t"					\
+		      "vmcall\n"					\
 		      ".Lend%=:"
 
 static inline


-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette