linux-kernel - Re: [PATCH v9 3/3] x86/snp: Convert shared memory back to private on kexec

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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.
>> +		 */
>>