Cc:     brijesh.singh@amd.com, x86@kernel.org,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        linux-efi@vger.kernel.org, platform-driver-x86@vger.kernel.org,
        linux-coco@lists.linux.dev, linux-mm@kvack.org,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Joerg Roedel <jroedel@suse.de>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        "H. Peter Anvin" <hpa@zytor.com>, Ard Biesheuvel <ardb@kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Sean Christopherson <seanjc@google.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Jim Mattson <jmattson@google.com>,
        Andy Lutomirski <luto@kernel.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Sergio Lopez <slp@redhat.com>, Peter Gonda <pgonda@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
        David Rientjes <rientjes@google.com>,
        Dov Murik <dovmurik@linux.ibm.com>,
        Tobin Feldman-Fitzthum <tobin@ibm.com>,
        Michael Roth <michael.roth@amd.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        "Kirill A . Shutemov" <kirill@shutemov.name>,
        Andi Kleen <ak@linux.intel.com>,
        "Dr . David Alan Gilbert" <dgilbert@redhat.com>,
        tony.luck@intel.com, marcorr@google.com,
        sathyanarayanan.kuppuswamy@linux.intel.com
Subject: Re: [PATCH v8 11/40] x86/sev: Register GHCB memory when SEV-SNP is
 active
To:     Borislav Petkov <bp@alien8.de>
References: <20211210154332.11526-1-brijesh.singh@amd.com>
 <20211210154332.11526-12-brijesh.singh@amd.com> <YcMlOOPp2rTFKkeW@zn.tnic>
From:   Brijesh Singh <brijesh.singh@amd.com>
Message-ID: <7cedae5f-96e1-cb37-1ed1-6f8e7a23868b@amd.com>
Date:   Wed, 22 Dec 2021 09:16:54 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
In-Reply-To: <YcMlOOPp2rTFKkeW@zn.tnic>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a21Wa2R6MEtacGx3czc3bndJYmRJSzFmNG1RZ1YxQzRLdWJFN09IbHJZcGND?=
 =?utf-8?B?MmZ3N05kMEpaNlQxbFZ6Q3JYZlhHck83ODJaYWV4TXNtWExsOTlKYW5OSCtL?=
 =?utf-8?B?SWFpK1NDKzQ2dUpxSkhJNkpoSFhnY1k5ZFdEaFV1cVViUjdIL1pCVW5SZUtF?=
 =?utf-8?B?Skl0TjVlc0pUNkZPOVhBN29CdkE3NktKanp0b3E3cnp4SUV5L2pWN2ExVll1?=
 =?utf-8?B?M0F3cnF5TG00dDhmdU16NGRWME5YWWRWVFQ2RkFLMDlJRlpoYnRxNXZFV1Zm?=
 =?utf-8?B?eTBPZUthUEN4WGNVY0hNVnRlS2hmYllaRmh1Mk5jZXdOL05vd25FdDFheTVr?=
 =?utf-8?B?U2M1M2o0OUdNR0lRQS9KS1ZhTjN0a2g5c3g4K3JXQzVHQ1VKUUg1WXdVbXBs?=
 =?utf-8?B?bGhPZUYrYWpFMWlKcGNSTFFqK2p2d3NPL1h1WVJrZlBpTThyMXpaZmxrQ0Rt?=
 =?utf-8?B?a3VkajhOa0psQ0hRK2l3cUFvRUtPQ0ZkZkw2N2VIZk93Ni9hN0pNNzNzTmxV?=
 =?utf-8?B?RVNqSGFJK1h5RzRqaU5TdisydllPa0I1Rm40Z3RINnRQL2Nsay8ydHRYTFFT?=
 =?utf-8?B?Mnc2OW9QNVVCT3Bvak10alVYendDYnRzZFJKdHFRYU5jNi8ybnh5ODZSQXg0?=
 =?utf-8?B?emExSjJ4Y1ZPaXozanJqaWVrb3JDNExtWUk0QnBubkluS2tHYVk5QXQ5SkFV?=
 =?utf-8?B?R09ESHpFdDFUWStpcDZuRjByRUprWnVGN1EzMUZJQUkySVhLd2I4RzFmZmlh?=
 =?utf-8?B?bHhpUmRiTncxTFR4Z1U0cGF5VGVIUnhvTEZpb0ZacHVlNG9iWnZyM2VjRWdz?=
 =?utf-8?B?K0JOVllzWUVIWXZwT3NINUFUY2ljWE01VFFPVkNGbU1lWG93cWJBTkYrT29G?=
 =?utf-8?B?elcvYnYwc2hVd3hsWE4rbHpyS1U1Y0x5TjdQUlB2NGF0TVN1b21GazdHMDlG?=
 =?utf-8?B?UE53NnR1RmxvZUhnT2hsMFJuaVp5WEVRcHlCaXl2NENFRzNmVlIzZTlZVkRG?=
 =?utf-8?B?VkR4NnJNa2N0cGFrZGZKdGFVVUdDeWVFdGx6bWpMVDRZREpPb3YwYkdlS0Fv?=
 =?utf-8?B?enVmZklKeVZIK2tWQjQ5ZG1abGczdUhxZ2QvZG1LU0l2RDQyUnVKZ291cnZn?=
 =?utf-8?B?RGhpSk14SFg0am05SjdmNXlQb081dTdROEJCMUFlaUhJRVpySXRjdjRWN0Jx?=
 =?utf-8?B?M2JNOGhtUWpURi9wdHdqVjJPcllkLyszMFdvMGtFeEJtRXVoam5hcm5ia0JE?=
 =?utf-8?B?SHQ4d2tGdGVrb29FMlRrUW5YdFUralZtQjdyQVdIcGg1Q21tOWhLRzJyclVv?=
 =?utf-8?B?QU1YeTV2U2QzcGFMVVNVVDlMcjMwODBLZVBuUW5BS2M3ajdYK08rdStCNS9S?=
 =?utf-8?B?aEI1b0RySDRwWG5vc1U2SGtLNDBiZEtlYko5UGdSb2FIUW03R1NUUHZPRkc0?=
 =?utf-8?B?R3R5RStWZzQ0Z2xGTll6b2lpdXVNdTdpbEEycm1TWS9ESWgvSjg1NytvTElT?=
 =?utf-8?B?MnRocFpmS09CY2F0M0dJbXE4R3hVVEFDZVJpRnZDNDF6ZXZhWDFvS2NqRWdq?=
 =?utf-8?B?UE5YWHFnVTJ2NlZPYUt2Qmd4aUtaTlp0SDVqRHhwdENRNzBORWxCUExTclow?=
 =?utf-8?B?SFM3Zm4waXMza1JHUWkrYmE4bFhjNFRYcnlsUzF6QjF3Z0EwbHRkNlBGOXhR?=
 =?utf-8?B?N09qNHpONWdJalFneDdNNTIvcmc3Z1A1dENmSGZhY1ZGK1BJREdTZXZiME0y?=
 =?utf-8?B?VkdxT2p2ZFUwckZxQktGcGJTUzMwWVQzNGhOdVRCYk53cVhOSGFEWjdwMFo4?=
 =?utf-8?B?eTlyRFpZY0ExdTVoc1JoTXlneE4weG9aNTRoMEgwY3NEbnBhazhrUHNxU3dW?=
 =?utf-8?B?dzJ6ZC84cklsQWV2eEhwaTRGTEJtcENxT3NQMWkrZnRVTGV6aXlNRHBqTThv?=
 =?utf-8?B?NGhYYWpOVE1zMHl5VVNzci9WTUdkZU04emloaFhjQ2x0WTU1V2w5N1JpRlJn?=
 =?utf-8?B?SUhtSm4rM1JHK3lxOUd6VHNNczN1dmtLTS9STXJ1YlZEN2l3dHBxeXJnZWp4?=
 =?utf-8?B?WkhKRFhhTGdzM2VTYXEyTWJMZVllU2RkSy9pbDl2R3lIUXdlRHhxYnowRXgx?=
 =?utf-8?B?RWg3VUcydXdYd1VmMjRyd0NxQnRmZkJ2NXZZdk1hNEJoaVJTZ2wyN3dyS0dR?=
 =?utf-8?Q?+XhNefrjgjdcavrTcJ6JaSs=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 45d970e0-2a05-45d5-769b-08d9c55e19c5
X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB2718.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Dec 2021 15:17:00.2583
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: +2QVGOzfyn1RbeVj+md0/bZwKik3QGWABT241WiWvBPgff0qpRcNKokLTDrZ0oX1ZB1I+V1M/lLq3tLROHu51g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR12MB2784
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 12/22/21 7:16 AM, Borislav Petkov wrote:
> On Fri, Dec 10, 2021 at 09:43:03AM -0600, Brijesh Singh wrote:
>> @@ -652,7 +652,7 @@ static enum es_result vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
>>    * This function runs on the first #VC exception after the kernel
>>    * switched to virtual addresses.
>>    */
>> -static bool __init sev_es_setup_ghcb(void)
>> +static bool __init setup_ghcb(void)
>>   {
>>   	/* First make sure the hypervisor talks a supported protocol. */
>>   	if (!sev_es_negotiate_protocol())
> 
> Ok, let me stare at this for a while:
> 
> This gets called by handle_vc_boot_ghcb() which gets set at build time:
> 
> arch/x86/kernel/head_64.S:372:SYM_DATA(initial_vc_handler,      .quad handle_vc_boot_ghcb)
> 
> initial_vc_handler() gets called by vc_boot_ghcb() which gets set in
> 
> early_setup_idt()
> 
> and that function already does sev_snp_register_ghcb().
> 
> So why don't you concentrate the work setup_ghcb() does before the first
> #VC and call it in early_setup_idt(), before the IDT is set?
> 
> And then you get rid of yet another setup-at-first-use case?
> 

I was following the existing SEV-ES implementation in which GHCB is 
setup on first #VC. But recently you recommended to move the setup 
outside of the VC handler for the decompression path and I was going to 
do the same for the kernel proper. I have tried moving the GHCB setup 
outside and it seems to be working okay with me (a limited testing so 
far). I will check Jorge to see if there was any reason for doing the 
GHCB setup inside the VC for the SEV-ES case.