Re: [PATCH Part1 v5 07/38] x86/sev: Add support for hypervisor feature VMGEXIT

Brijesh Singh
2021-08-23T18:34:55Z

On 8/23/21 1:25 PM, Brijesh Singh wrote:
>
>
> On 8/23/21 4:47 AM, Borislav Petkov wrote:
>> On Fri, Aug 20, 2021 at 10:19:02AM -0500, Brijesh Singh wrote:
>>> Version 2 of GHCB specification introduced advertisement of a features
>>> that are supported by the hypervisor. Add support to query the HV
>>> features on boot.
>>>
>>> Version 2 of GHCB specification adds several new NAEs, most of them are
>>> optional except the hypervisor feature. Now that hypervisor feature NAE
>>> is implemented, so bump the GHCB maximum support protocol version.
>>>
>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>>> ---
>>>   arch/x86/include/asm/mem_encrypt.h |  2 ++
>>>   arch/x86/include/asm/sev-common.h  |  3 +++
>>>   arch/x86/include/asm/sev.h         |  2 +-
>>>   arch/x86/include/uapi/asm/svm.h    |  2 ++
>>>   arch/x86/kernel/sev-shared.c       | 23 +++++++++++++++++++++++
>>>   5 files changed, 31 insertions(+), 1 deletion(-)
>>
>> I think you can simplify more.
>>
>> The HV features are read twice - once in the decompressor stub and again
>> in kernel proper - but I guess that's not such a big deal.
>>
>> Also, sev_hv_features can be static.
>>
>> Diff ontop:
>>
>
> The sev_hv_features is also referred during the AP creation. By caching
> the value in sev-shared.c and exporting it to others, we wanted to
> minimize VMGEXITs during the AP creation.
>
> If we go with your patch below, then we will need to cache the
> sev_hv_features in sev.c, so that it can be later used by the AP
> creation code (see patch#22).
>

Let me take it back, I didn't realize that sev.c includes the
sev-shared.c. So your patch will work fine. sorry about the noise.