On Thu, Mar 21, 2019 at 12:09:46AM -0700, Krish Sadhukhan wrote:
>
>
> On 03/19/2019 08:33 AM, Sean Christopherson wrote:
> >On Mon, Mar 18, 2019 at 09:46:24PM -0400, Krish Sadhukhan wrote:
> >>1. According to section "Checks on Host Control Registers and MSRs" in Intel
> >> SDM vol 3C, the following check is performed on vmentry of L2 guests:
> >Heh, "of L2 guests" again.
>
> We are testing VMLAUNCH of L2 in this test.
Right, but you're quoting SDM and the SDM has no concept of L2. You could
instead word the changelog as:
[PATCH 3/3][kvm-unit-test nVMX]: Check "load IA32_PAT" on vmentry of L2 guests
...to verify KVM performs the appropriate consistency checks for loading
IA32_PAT as part of running a nested guest.
According to section ...
>
> >> If the “load IA32_PAT” VM-exit control is 1, the value of the field
> >> for the IA32_PAT MSR must be one that could be written by WRMSR
> >> without fault at CPL 0. Specifically, each of the 8 bytes in the
> >> field must have one of the values 0 (UC), 1 (WC), 4 (WT), 5 (WP),
> >> 6 (WB), or 7 (UC-).
> >>
> >>2. According to section "CHECKING AND LOADING GUEST STATE" in Intel
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > Why use all caps here?
> >
> >> SDM vol 3C, the following check is performed on vmentry of L2 guests:
> >>
> >> If the "load IA32_PAT" VM-entry control is 1, the value of the field
> >> for the IA32_PAT MSR must be one that could be written by WRMSR
> >> without fault at CPL 0. Specifically, each of the 8 bytes in the
> >> field must have one of the values 0 (UC), 1 (WC), 4 (WT), 5 (WP),
> >> 6 (WB), or 7 (UC-).
> >IMO this is a bit gratuitous. The paragraphs are identical except for
> >"exit" vs. "entry". A little consolidation:
> >
> >According to sections "Checks on Host Control Registers and MSRs" and
> >"Checking and Loading Guest State" in Intel SDM vol 3c, the following
> >checks are performed on VM-Entry:
> >
> > If the "load IA32_PAT" VM-{entry,exit} control is 1, the value of
> > the field for the IA32_PAT MSR must be one that could be written by
> > WRMSR without fault at CPL 0. Specifically, each of the 8 bytes in
> > the field must have one of the values 0 (UC), 1 (WC), 4 (WT), 5 (WP),
> > 6 (WB), or 7 (UC-).
> >
> >>Signed-off-by: Krish Sadhukhan<krish.sadhukhan@xxxxxxxxxx>
> >>Reviewed-by: Karl Heubaum<karl.heubaum@xxxxxxxxxx>
...
> One related issue I notice is that the exit reason and the exit
> qualification are the same for both a true VM-exit and a VM-exit with
> VM-entry failure during guest state checks. For example, for a valid or an
> invalid value in the bytes of HOST_PAT/GUEST_PAT, I see the following in
> both cases:
>
> Exit Reason: 0x80000021
> Exit Qualification: 0
>
> We are setting bit 31 in Exit Reason even when all guest state checks have
> passed successfully and L2 has returned to L1 via a true VM-exit. What
> criterion can I use to differentiate between a true VM-exit vs. a VM-exit
> with VM-entry failure ?
Because it's not actually hitting a "true" VM-Exit. See the comment above
vmlaunch_succeeds():
/*
* Test for early VMLAUNCH failure. Returns true if VMLAUNCH makes it
* at least as far as the guest-state checks. Returns false if the
* VMLAUNCH fails early and execution falls through to the next
* instruction.
*/
static bool vmlaunch_succeeds(void)
In other words, vmlaunch_succeeds() considers "success" to be reaching
the point where VMLAUNCH starts to load guest state, i.e. it doesn't
ensure guest state is valid and so can't be used to test an invalid
GUEST_PAT.
