On 5/7/24 2:58 AM, Borislav Petkov wrote:
On Mon, May 06, 2024 at 02:53:00PM -0700, Alexey Makhalov wrote:
+#define VMWARE_HYPERCALL \
+ ALTERNATIVE_3("cmpb $" \
+ __stringify(CPUID_VMWARE_FEATURES_ECX_VMMCALL) \
+ ", %[mode]\n\t" \
+ "jg 2f\n\t" \
+ "je 1f\n\t" \
+ "movw %[port], %%dx\n\t" \
+ "inl (%%dx), %%eax\n\t" \
+ "jmp 3f\n\t" \
+ "1: vmmcall\n\t" \
+ "jmp 3f\n\t" \
+ "2: vmcall\n\t" \
+ "3:\n\t", \
+ "movw %[port], %%dx\n\t" \
+ "inl (%%dx), %%eax", X86_FEATURE_HYPERVISOR, \
That's a bunch of insns and their size would inadvertently go into the final
image.
What you should try to do is something like this:
ALTERNATIVE_3("jmp .Lend_legacy_call", "", X86_FEATURE_HYPERVISOR,
"vmcall; jmp .Lend_legacy_call", X86_FEATURE_VMCALL,
"vmmcall; jmp .Lend_legacy_call", X86_FEATURE_VMW_VMMCALL)
/* bunch of conditional branches and INs and V*MCALLs, etc go here */
.Lend_legacy_call:
so that you don't have these 26 bytes, as you say, of alternatives to patch but
only the JMPs and the VM*CALLs.
See for an example the macros in arch/x86/entry/calling.h which simply jump
over the code when not needed.
Good idea!
Also, you could restructure the alternative differently so that that bunch of
insns call is completely out-of-line because all current machines support
VM*CALL so you won't even need to patch. You only get to patch when running on
some old rust and there you can just as well go completely out-of-line.
Alternatives patching has not been performed at platform detection time.
And platform detection hypercalls should work on all machines.
That is the reason we have IN as a default hypercall behavior.
Something along those lines, anyway.
- * The high bandwidth in call. The low word of edx is presumed to have the
- * HB bit set.
+ * High bandwidth calls are not supported on encrypted memory guests.
+ * The caller should check cc_platform_has(CC_ATTR_MEM_ENCRYPT) and use
+ * low bandwidth hypercall it memory encryption is set.
s/it/if/
Acked.
-#define VMWARE_PORT(cmd, eax, ebx, ecx, edx) \
- __asm__("inl (%%dx), %%eax" : \
- "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) : \
- "a"(VMWARE_HYPERVISOR_MAGIC), \
- "c"(VMWARE_CMD_##cmd), \
- "d"(VMWARE_HYPERVISOR_PORT), "b"(UINT_MAX) : \
- "memory")
-
-#define VMWARE_VMCALL(cmd, eax, ebx, ecx, edx) \
- __asm__("vmcall" : \
- "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) : \
- "a"(VMWARE_HYPERVISOR_MAGIC), \
- "c"(VMWARE_CMD_##cmd), \
- "d"(0), "b"(UINT_MAX) : \
- "memory")
-
-#define VMWARE_VMMCALL(cmd, eax, ebx, ecx, edx) \
- __asm__("vmmcall" : \
- "=a"(eax), "=c"(ecx), "=d"(edx), "=b"(ebx) : \
- "a"(VMWARE_HYPERVISOR_MAGIC), \
- "c"(VMWARE_CMD_##cmd), \
- "d"(0), "b"(UINT_MAX) : \
- "memory")
-
-#define VMWARE_CMD(cmd, eax, ebx, ecx, edx) do { \
- switch (vmware_hypercall_mode) { \
- case CPUID_VMWARE_FEATURES_ECX_VMCALL: \
- VMWARE_VMCALL(cmd, eax, ebx, ecx, edx); \
- break; \
- case CPUID_VMWARE_FEATURES_ECX_VMMCALL: \
- VMWARE_VMMCALL(cmd, eax, ebx, ecx, edx); \
- break; \
- default: \
- VMWARE_PORT(cmd, eax, ebx, ecx, edx); \
- break; \
- } \
- } while (0)
You're kidding, right?
You went to all that trouble in patch 1 to move those to the header only to
*remove* them here?
You do realize that that is a unnecessary churn for no good reason, right?
So that set needs to be restructured differently.
* first patch introduces those new API calls.
* follow-on patches convert the callers to the new API
* last patch removes the old API.
Ok?
My intention was to have a implementation transformation from locals
macro through common macros to common API.
What you are suggesting will eliminate unnecessary patches. It makes sense.
Will perform this restructuring in v10.
And when you redo them, make sure you drop all Reviewed-by tags because the new
versions are not reviewed anymore.
Noted.
Thanks again,
--Alexey
