Re: [PATCH Part1 RFC v3 11/22] x86/sev: Add helper for validating pages in early enc attribute changes

[Brijesh Singh]

On 6/14/21 2:03 PM, Borislav Petkov wrote:
> On Mon, Jun 14, 2021 at 07:45:11AM -0500, Brijesh Singh wrote:
>> IMO, there is no need to add a warning. This case should happen if its
>> either a hypervisor bug or hypervisor does not follow the GHCB
>> specification. I followed the SEV-ES vmgexit handling  and it does not
>> warn if the hypervisor returns a wrong response code. We simply
>> terminate the guest.
> This brings my regular user-friendliness question: will the guest user
> know what happened or will the guest simply disappear/freeze without any
> hint as to what has happened so that a post-mortem analysis would turn
> out hard to decipher?

When a guest requests to terminate then guest user (aka VMM) will be
notified through the hypervisor that guest has requested the
termination. KVM defines a fixed set of reason code that is passed to
the guest user, see
https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/kvm.h#L237.
In this particular case guest user probably get the KVM_EXIT_SHUTDOWN --
i.e guest asked to be terminated. If user wants to see the actual GHCB
reason code then they must look into the KVM log.

Now that we have to defined a Linux specific reason set, we could
potentially define a new error code "Invalid response code" and return
that instead of generic termination error in this particular case. So
that when user looks at KVM log they see the "invalid response code"
instead of the generic GHCB error.

If we go with that approach then I think it makes sense to cover it for
SEV-ES guests too.


>> I did thought about reusing the VMGEXIT defined macro
>> SNP_PAGE_STATE_{PRIVATE, SHARED} but I was not sure if you will be okay
>> with that.
> Yeah, I think that makes stuff simpler. Unless there's something
> speaking against it which we both are not thinking of right now.
>
>> Additionally now both the function name and macro name will
>> include the "SNP". The call will look like this:
>>
>> snp_prep_memory(paddr, SNP_PAGE_STATE_PRIVATE)
> Yap, looks ok to me.
>
> Thx.
>