From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A02C7C432BE for ; Thu, 19 Aug 2021 20:45:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8803A61029 for ; Thu, 19 Aug 2021 20:45:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235328AbhHSUqN (ORCPT ); Thu, 19 Aug 2021 16:46:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46962 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230052AbhHSUqK (ORCPT ); Thu, 19 Aug 2021 16:46:10 -0400 Received: from mail-pf1-x433.google.com (mail-pf1-x433.google.com [IPv6:2607:f8b0:4864:20::433]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1B69CC061575 for ; Thu, 19 Aug 2021 13:45:34 -0700 (PDT) Received: by mail-pf1-x433.google.com with SMTP id g14so6627941pfm.1 for ; Thu, 19 Aug 2021 13:45:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=ciANhO2rI2yAVYRk89xhZMU4nhupjSlELscJz8Yo+Qw=; b=Fc7Daih9rnQh6iUId4QRiL4FivuwPAhyAMFNkO2VUAzl9fh8RJcdflKXRtvDtW22Dw 62K1Lh5y+Qh18QpKDce59XAwAQFsi2e0hbHHmHWSE+qcn9qT3B9yFSbUFCyvttMdHGw+ esZwVHbFPkPORlaIpJ6YSZYBtd2H0+g+bmQpEwGu2xNO8350bSN031MHRkAeb+QqSZRt kxhVmaya5nZ29ErR9EpIiA5i+9h+qYWjLCusY4XlNb4fu/PXgnT5K1yyQYlaSF3U2d/n aHGseXBbhe63iUKx2Jwwe4vKVlFrBXk9fBrak+iasFVoPH/GhT70RVjSleulYwixImdi 1C+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=ciANhO2rI2yAVYRk89xhZMU4nhupjSlELscJz8Yo+Qw=; b=LnM07JUVnQWcrj02gxUe5/78c7b40mDevmF52o69OitJkd4xrxC99W0yhqYg3twqkR 4ly9aH5NO17XTE8YkiYoGVmJAW6ZfdtTSJcyt+Qv9e6MN+LGsbDMytfcDOqW5kTuJ4Ey hAG5+A4JyduWFBBVxct7fZmqFfQqcYLhLc9SYEEFzdfdmXEC7dmQj2YWyug94PbxtrQc zg1oDIg4VbqR01/LuE7gqZ0QyoGVeF5ExXUPEZgf1amoaIPaoF2gOBde/L/lQNZgLLIF wxOrf5ay61K7spCBm1XoI1nr9K1hAiV+Yi9cw2vxFzPcOXFHW0O9iRFgtqtoZphdedb7 uiFg== X-Gm-Message-State: AOAM530fmjezxjdnFLkOv0/s93MW1KQlOqxIXouvWksA63HzNIRiVGsj Qn6uD6uIq2z43ylrjd6r6BW8rg== X-Google-Smtp-Source: ABdhPJy/YEvyPot1aTu59VL50ZY1wh0+PMMs7LuvoXrs8FoMJhsEul2LVsCbAJktJ+VCm+UP0FBlng== X-Received: by 2002:a62:86c4:0:b0:3e0:f216:81bc with SMTP id x187-20020a6286c4000000b003e0f21681bcmr16271484pfd.27.1629405933408; Thu, 19 Aug 2021 13:45:33 -0700 (PDT) Received: from google.com (157.214.185.35.bc.googleusercontent.com. [35.185.214.157]) by smtp.gmail.com with ESMTPSA id pj14sm3625802pjb.35.2021.08.19.13.45.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Aug 2021 13:45:32 -0700 (PDT) Date: Thu, 19 Aug 2021 20:45:27 +0000 From: Sean Christopherson To: Ashish Kalra Cc: pbonzini@redhat.com, tglx@linutronix.de, bp@alien8.de, mingo@redhat.com, hpa@zytor.com, joro@8bytes.org, Thomas.Lendacky@amd.com, x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, srutherford@google.com, brijesh.singh@amd.com, linux-efi@vger.kernel.org Subject: Re: [PATCH v3 2/5] KVM: x86: invert KVM_HYPERCALL to default to VMMCALL Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Preferred shortlog prefix for KVM guest changes is "x86/kvm". "KVM: x86" is for host changes. On Tue, Jun 08, 2021, Ashish Kalra wrote: > From: Ashish Kalra > > KVM hypercall framework relies on alternative framework to patch the > VMCALL -> VMMCALL on AMD platform. If a hypercall is made before > apply_alternative() is called then it defaults to VMCALL. The approach > works fine on non SEV guest. A VMCALL would causes #UD, and hypervisor > will be able to decode the instruction and do the right things. But > when SEV is active, guest memory is encrypted with guest key and > hypervisor will not be able to decode the instruction bytes. > > So invert KVM_HYPERCALL and X86_FEATURE_VMMCALL to default to VMMCALL > and opt into VMCALL. The changelog needs to explain why SEV hypercalls need to be made before apply_alternative(), why it's ok to make Intel CPUs take #UDs on the unknown VMMCALL, and why this is not creating the same conundrum for TDX. Actually, I don't think making Intel CPUs take #UDs is acceptable. This patch breaks Linux on upstream KVM on Intel due a bug in upstream KVM. KVM attempts to patch the "wrong" hypercall to the "right" hypercall, but stupidly does so via an emulated write. I.e. KVM honors the guest page table permissions and injects a !WRITABLE #PF on the VMMCALL RIP if the kernel code is mapped RX. In other words, trusting the VMM to not screw up the #UD is a bad idea. This also makes documenting the "why does SEV need super early hypercalls" extra important. This patch doesn't work because X86_FEATURE_VMCALL is a synthetic flag and is only set by VMware paravirt code, which is why the patching doesn't happen as would be expected. The obvious solution would be to manually set X86_FEATURE_VMCALL where appropriate, but given that defaulting to VMCALL has worked for years, defaulting to VMMCALL makes me nervous, e.g. even if we splatter X86_FEATURE_VMCALL into Intel, Centaur, and Zhaoxin, there's a possibility we'll break existing VMs that run on hypervisors that do something weird with the vendor string. Rather than look for X86_FEATURE_VMCALL, I think it makes sense to have this be a "pure" inversion, i.e. patch in VMCALL if VMMCALL is not supported, as opposed to patching in VMCALL if VMCALL is supproted. diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h index 69299878b200..61641e69cfda 100644 --- a/arch/x86/include/asm/kvm_para.h +++ b/arch/x86/include/asm/kvm_para.h @@ -17,7 +17,7 @@ static inline bool kvm_check_and_clear_guest_paused(void) #endif /* CONFIG_KVM_GUEST */ #define KVM_HYPERCALL \ - ALTERNATIVE("vmcall", "vmmcall", X86_FEATURE_VMMCALL) + ALTERNATIVE("vmmcall", "vmcall", ALT_NOT(X86_FEATURE_VMMCALL)) /* For KVM hypercalls, a three-byte sequence of either the vmcall or the vmmcall * instruction. The hypervisor may replace it with something else but only the > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: "H. Peter Anvin" > Cc: Paolo Bonzini > Cc: Joerg Roedel > Cc: Borislav Petkov > Cc: Tom Lendacky > Cc: x86@kernel.org > Cc: kvm@vger.kernel.org > Cc: linux-kernel@vger.kernel.org Suggested-by: Sean Christopherson > Signed-off-by: Brijesh Singh Is Brijesh the author? Co-developed-by for a one-line change would be odd... > Signed-off-by: Ashish Kalra > --- > arch/x86/include/asm/kvm_para.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h > index 69299878b200..0267bebb0b0f 100644 > --- a/arch/x86/include/asm/kvm_para.h > +++ b/arch/x86/include/asm/kvm_para.h > @@ -17,7 +17,7 @@ static inline bool kvm_check_and_clear_guest_paused(void) > #endif /* CONFIG_KVM_GUEST */ > > #define KVM_HYPERCALL \ > - ALTERNATIVE("vmcall", "vmmcall", X86_FEATURE_VMMCALL) > + ALTERNATIVE("vmmcall", "vmcall", X86_FEATURE_VMCALL) > > /* For KVM hypercalls, a three-byte sequence of either the vmcall or the vmmcall > * instruction. The hypervisor may replace it with something else but only the > -- > 2.17.1 >