Received: by 2002:a05:6a11:4021:0:0:0:0 with SMTP id ky33csp308502pxb; Wed, 22 Sep 2021 02:40:53 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzczePywxckm1p7DvkJfyBxjsgvedabBVCPeohBzUOXv6Nr8wwBciAH6qDsVnzs16IYrUQG X-Received: by 2002:a05:6402:1808:: with SMTP id g8mr40538955edy.188.1632303652863; Wed, 22 Sep 2021 02:40:52 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1632303652; cv=none; d=google.com; s=arc-20160816; b=VQBQ8hmgmi7QOSCeytYN/v96GiGdXw+DM4omIFCJ/wPxcPpXqqHFh4mQaPCD6EOU06 XyPrOIeZ5n/lLmKCRmkJTvSNGNEJqScRXy7NupXvN0DJBDKZL4m6/p93W2bjgyef3m6d o8k3jcarPPU4DC0rxgdug8iYbgDW0WxjB2dw0zWKKo3PflqBQRJ8k4P1S7zeROL5LYcp koDNjSFFD+vmBvkm4olJ6aSuwh75q8Tt4Q+MLv5eTOSxgF7mIttrpwCsV2oHJjMQEt41 fbz/fTVbfZku8w+fcQNWySavUMi9UiYEbExmFHjGHQSnNQPINNqBkPdPd1xnU0t7y576 tHRg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=nehvbtp+iVTeyyMzkseD676nsbARQC8Fz0zTVaQUD18=; b=d+VPh6PbQ8CtwLl87FrTL2oTkZSV7SUIozTESCI0CY5cQPWzHxVXDm0P3oClXVDcXv 7uykRdF+4CdVDvYTRk6EftIfZENVe+f+IzdOHQ3NiK8AJg+5ABGenDJwqB6kBn4eaBy0 Thajzl8aZSkpIY/2b6hFCItFMs7iQqYJWHPyzDviF2S3jfHPJHw8iBVa24KM/exZ1hEq vvyYcjjPADa+LFWBDieun2HIv/O/PfvVlZi8WBThm0+2exrYkNln7rs8hyIxoBpOV6FO 1GkVu3MaHXipSjXjHK8wxnA6hTtHZigZe+hhcpxtDQUEpcLgoFIexkR8F1gS7q/DBTiH Vv9A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@alien8.de header.s=dkim header.b=Im+KSGCT; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=alien8.de Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id cx2si2010084edb.344.2021.09.22.02.40.28; Wed, 22 Sep 2021 02:40:52 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@alien8.de header.s=dkim header.b=Im+KSGCT; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=alien8.de Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234432AbhIVJkT (ORCPT + 99 others); Wed, 22 Sep 2021 05:40:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44686 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234418AbhIVJkK (ORCPT ); Wed, 22 Sep 2021 05:40:10 -0400 Received: from mail.skyhub.de (mail.skyhub.de [IPv6:2a01:4f8:190:11c2::b:1457]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6DBE7C061574; Wed, 22 Sep 2021 02:38:19 -0700 (PDT) Received: from zn.tnic (p200300ec2f0efa00e1b20f6d5f00f371.dip0.t-ipconnect.de [IPv6:2003:ec:2f0e:fa00:e1b2:f6d:5f00:f371]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.skyhub.de (SuperMail on ZX Spectrum 128k) with ESMTPSA id 8A4101EC04E4; Wed, 22 Sep 2021 11:38:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alien8.de; s=dkim; t=1632303493; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:in-reply-to:in-reply-to: references:references; bh=nehvbtp+iVTeyyMzkseD676nsbARQC8Fz0zTVaQUD18=; b=Im+KSGCT0T1VMIm8WO9h8Oks9HOlxtRwhQ2VGhoavv9UnIJXkRFGzamH4+IXhhr91N63Ig T/5OuKTubgkJFniabg0xpI4tI1Ug9MzWuyRJWRm0jpakGBieOuo2UNPhUqYrVoqe6s8/OS 5kRFMMx8KdGgGAVsjHy9KQLo2KiOfD4= Date: Wed, 22 Sep 2021 11:38:08 +0200 From: Borislav Petkov To: Sean Christopherson Cc: Ashish Kalra , Steve Rutherford , pbonzini@redhat.com, tglx@linutronix.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, brijesh.singh@amd.com, dovmurik@linux.ibm.com, tobin@linux.ibm.com, jejb@linux.ibm.com, dgilbert@redhat.com Subject: Re: [PATCH v6 1/5] x86/kvm: Add AMD SEV specific Hypercall3 Message-ID: References: <6fd25c749205dd0b1eb492c60d41b124760cc6ae.1629726117.git.ashish.kalra@amd.com> <20210921095838.GA17357@ashkalra_ubuntu_server> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Sep 21, 2021 at 04:07:03PM +0000, Sean Christopherson wrote: > init_hypervisor_platform(), after x86_init.hyper.init_platform() so that the > PV support can set the desired feature flags. Since kvm_hypercall*() is only > used by KVM guests, set_cpu_cap(c, X86_FEATURE_VMMCALL) can be moved out of > early_init_amd/hygon() and into kvm_init_platform(). See below. > Another option would be to refactor apply_alternatives() to allow > the caller to provide a different feature check mechanism than > boot_cpu_has(), which I think would let us drop X86_FEATURE_VMMCALL, > X86_FEATURE_VMCALL, and X86_FEATURE_VMW_VMMCALL from cpufeatures. That > might get more than a bit gross though. Uuuf. So here's what I'm seeing (line numbers given to show when stuff happens): start_kernel |-> 953: setup_arch |-> 794: early_cpu_init |-> 936: init_hypervisor_platform | |-> 1134: check_bugs |-> alternative_instructions at line 794 setup_arch() calls early_cpu_init() which would end up setting X86_FEATURE_VMMCALL on an AMD guest, based on CPUID information. init_hypervisor_platform() happens after that. The alternatives patching happens in check_bugs() at line 1134. Which means, if one would consider moving the patching up, one would have to audit all the code between line 953 and 1134, whether it does set_cpu_cap() or some of the other helpers to set or clear bits in boot_cpu_data which controls the patching. So for that I have only one thing to say: can'o'worms. We tried to move the memblock allocations placement in the boot process and generated at least 4 regressions. I'm still testing the fix for the fix for the 4th regression. So moving stuff in the fragile boot process makes my hair stand up. Refactoring apply_alternatives() to patch only for X86_FEATURE_VMMCALL and then patch again, I dunno, this stuff is fragile and it might cause some other similarly nasty fallout. And those are hard to debug because one does not see immediately when boot_cpu_data features are missing and functionality is behaving differently because of that. So what's wrong with: kvm_hypercall3: if (cpu_feature_enabled(X86_FEATURE_VMMCALL)) return kvm_sev_hypercall3(...); /* rest of code */ ? Dunno we probably had that already in those old versions and maybe that was shot down for another reason but it should get you what you want without having to test the world and more for regressions possibly happening from disturbing the house of cards called x86 boot order. IMHO, I'd say. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette