On Wed, Mar 11, 2020 at 01:38:24PM -0700, Krish Sadhukhan wrote: > > On 3/11/20 8:05 AM, Sean Christopherson wrote: > >>+static void test_guest_segment_sel_fields(void) > >>+{ > >>+ u16 sel_saved; > >>+ u16 sel; > >>+ > >>+ sel_saved = vmcs_read(GUEST_SEL_TR); > >>+ sel = sel_saved | 0x4; > >>+ TEST_SEGMENT_SEL(GUEST_SEL_TR, "GUEST_SEL_TR", sel, sel_saved); > >>+ > >>+ sel_saved = vmcs_read(GUEST_SEL_LDTR); > >>+ sel = sel_saved | 0x4; > >>+ TEST_SEGMENT_SEL(GUEST_SEL_LDTR, "GUEST_SEL_LDTR", sel, sel_saved); > >>+ > >>+ if (!(vmcs_read(GUEST_RFLAGS) & X86_EFLAGS_VM) && > >>+ !(vmcs_read(CPU_SECONDARY) & CPU_URG)) { > >Rather than react to the environment, these tests should configure every > >relevant aspect and ignore the ones it can't change. E.g. the unit tests > >aren't going to randomly launch a vm86 guest. Ditto for the unusuable bit, > >it's unlikely to be set for most segments and would be something to test > >explicitly. > > > Just wanted to clarify on the "unusable bit" part of your comment. Do you > mean each of the segment register checks from the SDM should have two tests, > one with the "unusable bit" set and the other with that bit not set, > irrespective of the checks being conditional on the setting of that bit ? Sort of. In an ideal world, kvm-unit-tests would verify correctness of KVM for both unusable=1 and unusable=0. But, the unusable=1 validation space is enormous, i.e. there are a bazillion combinations of random garbage that can be thrown into GUEST_*S_{SE,ARBYTE,BASE}. So yeah, it could be as simple as running the same test as unusable=0, but expecting VM-Entry to succeed. That being said, I don't understand the motivation for these tests. KVM doesn't have any dedicated logic for checking guest segments, i.e. these tests are validating hardware behavior, not KVM behavior. The validation resources thrown at hardware dwarf what kvm-unit-tests can do, i.e. the odds of finding a silicon bug are tiny, and the odds of such a bug being exploitable aginst L0 are downright miniscule.