Re: [kvm-unit-tests PATCH 3/3] x86: nSVM: Add an exception test framework and tests

["Shukla, Manali" <mashukla@amd.com>]

On 2/15/2022 1:50 AM, Aaron Lewis wrote:
>> +static void svm_l2_nm_test(struct svm_test *svm)
>> +{
>> +    write_cr0(read_cr0() | X86_CR0_TS);
>> +    asm volatile("fnop");
>> +}
>> +
>> +static void svm_l2_of_test(struct svm_test *svm)
>> +{
>> +    struct far_pointer32 fp = {
>> +        .offset = (uintptr_t)&&into,
>> +        .selector = KERNEL_CS32,
>> +    };
>> +    uintptr_t rsp;
>> +
>> +    asm volatile ("mov %%rsp, %0" : "=r"(rsp));
>> +
>> +    if (fp.offset != (uintptr_t)&&into) {
>> +        printf("Codee address too high.\n");
> 
> Nit: Code
> 
>> +        return;
>> +    }
>> +
>> +    if ((u32)rsp != rsp) {
>> +        printf("Stack address too high.\n");
>> +    }
>> +
>> +    asm goto("lcall *%0" : : "m" (fp) : "rax" : into);
>> +    return;
>> +into:
>> +    asm volatile (".code32;"
>> +            "movl $0x7fffffff, %eax;"
>> +            "addl %eax, %eax;"
>> +            "into;"
>> +            "lret;"
>> +            ".code64");
>> +    __builtin_unreachable();
>> +}
>> +
> 
>> +static void svm_l2_ac_test(struct svm_test *test)
>> +{
>> +    bool hit_ac = false;
>> +
>> +    write_cr0(read_cr0() | X86_CR0_AM);
>> +    write_rflags(read_rflags() | X86_EFLAGS_AC);
>> +
>> +    run_in_user(usermode_callback, AC_VECTOR, 0, 0, 0, 0, &hit_ac);
>> +
>> +    report(hit_ac, "Usermode #AC handled in L2");
>> +    vmmcall();
>> +}
>> +
>> +static void svm_ac_init(void)
>> +{
>> +    set_user_mask_all(phys_to_virt(read_cr3()), PAGE_LEVEL);
>> +}
>> +
>> +static void svm_ac_uninit(void)
>> +{
>> +    clear_user_mask_all(phys_to_virt(read_cr3()), PAGE_LEVEL);
>> +}
>> +
>> +struct svm_exception_test {
>> +    u8 vector;
>> +    void (*guest_code)(struct svm_test*);
>> +    void (*init_test)(void);
>> +    void (*uninit_test)(void);
>> +};
>> +
>> +struct svm_exception_test svm_exception_tests[] = {
>> +    { GP_VECTOR, svm_l2_gp_test },
>> +    { UD_VECTOR, svm_l2_ud_test },
>> +    { DE_VECTOR, svm_l2_de_test },
>> +    { BP_VECTOR, svm_l2_bp_test },
>> +    { NM_VECTOR, svm_l2_nm_test },
>> +    { OF_VECTOR, svm_l2_of_test },
>> +    { DB_VECTOR, svm_l2_db_test },
>> +    { AC_VECTOR, svm_l2_ac_test, svm_ac_init, svm_ac_uninit },
>> +};
> 
> If you set and clear PT_USER_MASK in svm_l2_ac_test() before calling
> into userspace you can remove init_test and uninit_test from the
> framework all together.  That will simplify the code.
> 
If clear user mask is called after userspace code, when #AC exception is 
intercepted by L1, the control directly goes to L1 and it does not reach 
clear_user_mask_all() function (called after user space code function run_in_user()).

That is why I have added init_test and uninit_test function

> Further, it would be nice to then hoist this framework and the one in
> vmx into a common x86 file, but looking at this that may be something
> to think about in the future.  There would have to be wrappers when
> interacting with the vmc{s,b} and macros at the very least.
> 

Yeah we can think of this in future.

>> +
>> +static u8 svm_exception_test_vector;
>> +
>> +static void svm_exception_handler(struct ex_regs *regs)
>> +{
>> +    report(regs->vector == svm_exception_test_vector,
>> +            "Handling %s in L2's exception handler",
>> +            exception_mnemonic(svm_exception_test_vector));
>> +    vmmcall();
>> +}
>> +

-Manali