Re: [RFC PATCH V5 11/15] x86/sev: Add a #HV exception handler

Gupta, Pankaj
2023-05-11T13:28:42Z

On 5/5/2023 12:59 PM, Gupta, Pankaj wrote:
> Hi Tianyu,
>
> I tried to understand some details of this patch. Please find below
> some comments/questions.
>
> Thanks,
>
>
>> Add a #HV exception handler that uses IST stack.
>>
>> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
>> ---
>> Change since RFC V2:
>>         * Remove unnecessary line in the change log.
>> ---
>>   arch/x86/entry/entry_64.S             | 22 +++++++----
>>   arch/x86/include/asm/cpu_entry_area.h |  6 +++
>>   arch/x86/include/asm/idtentry.h       | 40 +++++++++++++++++++-
>>   arch/x86/include/asm/page_64_types.h  |  1 +
>>   arch/x86/include/asm/trapnr.h         |  1 +
>>   arch/x86/include/asm/traps.h          |  1 +
>>   arch/x86/kernel/cpu/common.c          |  1 +
>>   arch/x86/kernel/dumpstack_64.c        |  9 ++++-
>>   arch/x86/kernel/idt.c                 |  1 +
>>   arch/x86/kernel/sev.c                 | 53 +++++++++++++++++++++++++++
>>   arch/x86/kernel/traps.c               | 40 ++++++++++++++++++++
>>   arch/x86/mm/cpu_entry_area.c          |  2 +
>>   12 files changed, 165 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
>> index eccc3431e515..653b1f10699b 100644
>> --- a/arch/x86/entry/entry_64.S
>> +++ b/arch/x86/entry/entry_64.S
>> @@ -496,7 +496,7 @@ SYM_CODE_END(\asmsym)
>>   #ifdef CONFIG_AMD_MEM_ENCRYPT
>>   /**
>> - * idtentry_vc - Macro to generate entry stub for #VC
>> + * idtentry_sev - Macro to generate entry stub for #VC
>>    * @vector:        Vector number
>>    * @asmsym:        ASM symbol for the entry point
>>    * @cfunc:        C function to be called
>> @@ -515,14 +515,18 @@ SYM_CODE_END(\asmsym)
>>    *
>>    * The macro is only used for one vector, but it is planned to be
>> extended in
>>    * the future for the #HV exception.
>> - */
>> -.macro idtentry_vc vector asmsym cfunc
>> +*/
>> +.macro idtentry_sev vector asmsym cfunc has_error_code:req
>>   SYM_CODE_START(\asmsym)
>>       UNWIND_HINT_IRET_REGS
>>       ENDBR
>>       ASM_CLAC
>>       cld
>> +    .if \vector == X86_TRAP_HV
>> +        pushq    $-1            /* ORIG_RAX: no syscall */
>> +    .endif
>> +
>>       /*
>>        * If the entry is from userspace, switch stacks and treat it as
>>        * a normal entry.
>> @@ -545,7 +549,12 @@ SYM_CODE_START(\asmsym)
>>        * stack.
>>        */
>>       movq    %rsp, %rdi        /* pt_regs pointer */
>> -    call    vc_switch_off_ist
>> +    .if \vector == X86_TRAP_VC
>> +        call    vc_switch_off_ist
>
> I think the stack switching logic is similar for #VC & #HV.
> So, we can use common function. Just the corresponding fallback
> stack switching is different. Maybe we can pass the hint as an
> argument (%rsi?) to something like "sev_switch_off_ist()", and use
> the corresponding (#HV or #VC) fallbacks stack?

Also, Please include the below patch from Ashish for #HV
reentrancy check.

https://github.com/ashkalra/linux/commit/6975484094b7cb8d703c45066780dd85043cd040

Thanks,
Pankaj