On 8/22/22 17:42, Sean Christopherson wrote:
>> On 8/22/22 00:06, Michal Luczaj wrote:
>> test can be simplified to something like
>>
>> asm volatile("push %[ss]\n\t"
>> __ASM_TRY(KVM_FEP "pop %%ss", "1f")
>> "ex_blocked: mov $1, %[success]\n\t"
>
> So I'm 99% certain this only passes because KVM doesn't emulate the `mov $1, %[success]`.
> kvm_vcpu_check_code_breakpoint() honors EFLAGS.RF, but not MOV/POP_SS blocking.
> I.e. I would expect this to fail if the `mov` were also tagged KVM_FEP.
I wasn't sure if I understood you correctly, so, FWIW, I've ran:
static void test_pop_ss_blocking_try(void)
{
int success;
write_dr7(DR7_FIXED_1 |
DR7_GLOBAL_ENABLE_DRx(0) |
DR7_EXECUTE_DRx(0) |
DR7_LEN_1_DRx(0));
#define POPSS(desc, fep1, fep2) \
asm volatile("mov $0, %[success]\n\t" \
"lea 1f, %%eax\n\r" \
"mov %%eax, %%dr0\n\r" \
"push %%ss\n\t" \
__ASM_TRY(fep1 "pop %%ss", "2f") \
"1:" fep2 "mov $1, %[success]\n\t" \
"2:" \
: [success] "=g" (success) \
: \
: "eax", "memory"); \
report(success, desc ": #DB suppressed after POP SS")
POPSS("no fep", "", "");
POPSS("fep pop-ss", KVM_FEP, "");
POPSS("fep mov-1", "", KVM_FEP);
POPSS("fep pop-ss/fep mov-1", KVM_FEP, KVM_FEP);
write_dr7(DR7_FIXED_1);
}
and got:
em_pop_sreg unpatched:
PASS: no fep: #DB suppressed after POP SS
FAIL: fep pop-ss: #DB suppressed after POP SS
PASS: fep mov-1: #DB suppressed after POP SS
FAIL: fep pop-ss/fep mov-1: #DB suppressed after POP SS
em_pop_sreg patched:
PASS: no fep: #DB suppressed after POP SS
PASS: fep pop-ss: #DB suppressed after POP SS
PASS: fep mov-1: #DB suppressed after POP SS
PASS: fep pop-ss/fep mov-1: #DB suppressed after POP SS
For the sake of completeness: basically the same, but MOV SS, i.e.
asm volatile("mov $0, %[success]\n\t" \
"lea 1f, %%eax\n\r" \
"mov %%eax, %%dr0\n\r" \
"mov %%ss, %%eax\n\t" \
__ASM_TRY(fep1 "mov %%eax, %%ss", "2f") \
"1:" fep2 "mov $1, %[success]\n\t" \
"2:" \
: [success] "=g" (success) \
: \
: "eax", "memory"); \
PASS: no fep: #DB suppressed after MOV SS
PASS: fep mov-ss: #DB suppressed after MOV SS
PASS: fep mov-1: #DB suppressed after MOV SS
PASS: fep mov-ss/fep mov-1: #DB suppressed after MOV SS
> The reason I say the setup_idt() change is a moot point is because I think it
> would be better to put this testcase into debug.c (where "this" testcase is really
> going to be multiple testcases). With macro shenanigans (or code patching), it
> should be fairly easy to run all testcases with both FEP=0 and FEP=1.
>
> Then it's "just" a matter of adding a code #DB testcase. __run_single_step_db_test()
> and run_ss_db_test() aren't fundamentally tied to single-step, i.e. can simply be
> renamed. For simplicity, I'd say just skip the usermode test for code #DBs, IMO
> it's not worth the extra coverage.
So that would involve a 32-bit segment for POP SS and making use of DR0/DR7 instead
of single stepping? I guess I can try.
Thanks,
Michal
