[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ba22b2d8-5621-4efc-8dc4-03b92408155f@amd.com>
Date: Mon, 24 Jun 2024 15:57:34 -0500
From: "Kalra, Ashish" <ashish.kalra@....com>
To: Borislav Petkov <bp@...en8.de>
Cc: tglx@...utronix.de, mingo@...hat.com, dave.hansen@...ux.intel.com,
x86@...nel.org, rafael@...nel.org, hpa@...or.com, peterz@...radead.org,
adrian.hunter@...el.com, sathyanarayanan.kuppuswamy@...ux.intel.com,
jun.nakajima@...el.com, rick.p.edgecombe@...el.com, thomas.lendacky@....com,
michael.roth@....com, seanjc@...gle.com, kai.huang@...el.com,
bhe@...hat.com, kirill.shutemov@...ux.intel.com, bdas@...hat.com,
vkuznets@...hat.com, dionnaglaze@...gle.com, anisinha@...hat.com,
jroedel@...e.de, ardb@...nel.org, dyoung@...hat.com,
kexec@...ts.infradead.org, linux-coco@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9 3/3] x86/snp: Convert shared memory back to private on
kexec
On 6/24/2024 1:26 PM, Borislav Petkov wrote:
>> @@ -92,6 +94,9 @@ static struct ghcb *boot_ghcb __section(".data");
>> /* Bitmap of SEV features supported by the hypervisor */
>> static u64 sev_hv_features __ro_after_init;
>>
>> +/* Last address to be switched to private during kexec */
>> +static unsigned long kexec_last_addr_to_make_private;
> This is particularly yucky.
>
> AFAIU, you want to:
>
> 1. Skip GHCB addresses when making all pages private
>
> 2. Once you're done converting, in snp_kexec_finish() go over the GHCB
> addresses and convert them explicitly, one-by-one, without this silly
> variable
it is simple to compare the current CPU's GHCB while walking the direct mapping, OTOH, comparing each and every address during unshare_all_memory() with all per-CPU GHCB addresses will just make it complicated and less performant too.
By skipping only the current CPU's GHCB and saving a reference to it, we are able to switch this (one) skipped GHCB page to private at the end of kexec_finish().
>
>> /* #VC handler runtime per-CPU data */
>> struct sev_es_runtime_data {
>> struct ghcb ghcb_page;
>> @@ -1010,6 +1015,169 @@ void snp_accept_memory(phys_addr_t start, phys_addr_t end)
>> set_pages_state(vaddr, npages, SNP_PAGE_STATE_PRIVATE);
>> }
>>
>> +static bool set_pte_enc(pte_t *kpte, int level, void *va)
> Code duplication: __set_clr_pte_enc().
>
> You need to refactor the code instead of adding just another, generically
> looking helper just because it is easier this way.
The issue with using __set_clr_pte_enc() is that it is called by early initialization code, it uses early_snp_set_memory_private() which uses the MSR protocol for making page state change calls as early_set_pages_state() can be called during early init code before the GHCB is established.
Hence, added simple static functions make_pte_private() and set_pte_enc() to make use of the more optimized snp_set_memory_private() to use the GHCB instead of the MSR protocol. Additionally, make_pte_private() adds check for GHCB addresses during unshare_all_memory() loop.
Thanks, Ashish
>> + * at the end of unshared loop so that we continue to use the
>> + * optimized GHCB protocol and not force the switch to
>> + * MSR protocol till the very end.
>> + */
>>
Powered by blists - more mailing lists