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
