linux-kernel - Re: [PATCH v2 3/3] x86/sev: Add SEV-SNP CipherTextHiding support

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]

Message-ID: <0a468f32-c586-4cfc-a606-89ab5c3e77c2@amd.com>
Date: Tue, 10 Dec 2024 15:32:05 -0600
From: "Kalra, Ashish" <ashish.kalra@....com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Tom Lendacky <thomas.lendacky@....com>, Peter Gonda <pgonda@...gle.com>,
 pbonzini@...hat.com, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, hpa@...or.com, herbert@...dor.apana.org.au,
 x86@...nel.org, john.allen@....com, davem@...emloft.net,
 michael.roth@....com, kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-crypto@...r.kernel.org
Subject: Re: [PATCH v2 3/3] x86/sev: Add SEV-SNP CipherTextHiding support

Hello Sean,

On 12/9/2024 7:30 PM, Sean Christopherson wrote:
> On Fri, Dec 06, 2024, Ashish Kalra wrote:
>> On 12/6/2024 4:30 PM, Sean Christopherson wrote:
>>>> This can reuse the current support (in KVM) to do SEV INIT implicitly when
>>>> the first SEV VM is run: sev_guest_init() -> sev_platform_init() 
>>>
>>> I don't love the implicit behavior, but assuming hotloading firmware can't be done
>>> after SEV_CMD_INIT{_EX}, that does seem like the least awful solution.
>>>
>>> To summarize, if the above assumptions hold:
>>>
>>>  1. Initialize SNP when kvm-amd.ko is loaded.
>>>  2. Define CipherTextHiding and ASID params kvm-amd.ko.
>>>  3. Initialize SEV+ at first use.
>>
>> Yes, the above summary is correct except for (3).
> 
> Heh, that wasn't a statement of fast, it was a suggestion for a possible
> implementation.
> 
>> The initial set of patches will initialize SNP and SEV both at kvm-amd.ko module load,
>> similar to PSP module load/probe time.
> 
> Why?  If SEV+ is initialized at kvm-amd.ko load, doesn't that prevent firmware
> hotloading?

Yes it does, i was thinking of fixing it as part of a series on top of these patches
to support SEV firmware hotloading.

> 
>> For backward compatibility, the PSP module parameter psp_init_on_probe will still be
>> supported, i believe it is used for INIT_EX support.
> 
> Again, why?  If the only use of psp_init_on_probe is to _disable_ that behavior,
> and we make the code never init-on-probe, then the param is unnecessary, no?

Yes, it makes sense to remove this param.

> 
>>> Just to triple check: that will allow firmware hotloading even if kvm-amd.ko is
>>> built-in, correct?  I.e. doesn't requires deferring kvm-amd.ko load until after
>>> firmware hotloading.
>>
>> Yes, this should work, for supporting firmware hotloading, the PSP driver's
>> psp_init_on_probe parameter will need to be set to false, which will ensure
>> that SEV INIT is not done during SEV/SNP platform initialization at KVM module
>> probe time and instead it will be done implicitly at first SEV/SEV-ES VM launch.
> 
> Please no.  I really, really don't want gunk like this in KVM:
> 
> 	init_args.probe = false;
> 	ret = sev_platform_init(&init_args);
> 
> That's inscrutable without a verbose comment, and all kinds of ugly.  Why can't
> we simply separate SNP initialization from SEV+ initialization?

Yes we can do that, by default KVM module load time will only do SNP initialization,
and then we will do SEV initialization if a SEV VM is being launched.

This will remove the probe parameter from init_args above, but will need to add another
parameter like VM type to specify if SNP or SEV initialization is to be performed with
the sev_platform_init() call.

Will work on re-posting my series with SNP initialization separated from SEV
initialization.

Thanks,
Ashish