"Michael Kelley (LINUX)" <mikelley@xxxxxxxxxxxxx> writes:
> From: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> Sent: Tuesday, June 6, 2023 8:49 AM
>>
>> Tianyu Lan <ltykernel@xxxxxxxxx> writes:
>>
>> > On 6/5/2023 8:13 PM, Vitaly Kuznetsov wrote:
>> >>> @@ -113,6 +114,11 @@ static int hv_cpu_init(unsigned int cpu)
>> >>>
>> >>> }
>> >>> if (!WARN_ON(!(*hvp))) {
>> >>> + if (hv_isolation_type_en_snp()) {
>> >>> + WARN_ON_ONCE(set_memory_decrypted((unsigned long)(*hvp), 1));
>> >>> + memset(*hvp, 0, PAGE_SIZE);
>> >>> + }
>> >> Why do we need to set the page as decrypted here and not when we
>> >> allocate the page (a few lines above)?
>> >
>> > If Linux root partition boots in the SEV-SNP guest, the page still needs
>> > to be decrypted.
>
> We have code in place that prevents this scenario. We don't allow Linux
> in the root partition to run in SEV-SNP mode. See commit f8acb24aaf89.
>
>> >
>>
>> I'd suggest we add a flag to indicate that VP assist page was actually
>> set (on the first invocation of hv_cpu_init() for guest partitions and
>> all invocations for root partition) and only call
>> set_memory_decrypted()/memset() then: that would both help with the
>> potential issue with KVM using enlightened vmcs and avoid the unneeded
>> hypercall.
>>
>
> I think there's actually a more immediate problem with the code as
> written. The VP assist page for a CPU is not re-encrypted or freed when
> a CPU goes offline (for reasons that have been discussed elsewhere). So
> if a CPU in an SEV-SNP VM goes offline and then comes back online, the
> originally allocated and already decrypted VP assist page will be reused.
> But bad things will happen if we try to decrypt the page again.
>
> Given that we disallow the root partition running in SEV-SNP mode,
> can we avoid the complexity of a flag, and just do the decryption and
> zero'ing when the page is allocated?
Sure, makes perfect sense but let's leave a [one line] comment why we
don't do any decryption for root partition then.
--
Vitaly
