On 4/17/19 5:00 AM, Paolo Bonzini wrote:
On 17/04/19 02:11, Krish Sadhukhan wrote:
We will allow this behavior for KVM in some specific cases
(CR8 load/store exits enabled, virtualize APIC accesses
disabled).
I am wondering whether it will be useful to mention here the reason for allowing such an exception.
It's mentioned in the test: these are the only cases where it's actually
possible to emulate a virtual-APIC address that points outside RAM
(because in this case the processor will never use it and L0 can simply
ignore that L1 has set the TPR-shadow execution control).
Slightly unrelated topic. This test was the only one that was setting
'xfail'. With your changes, 'xfail' will not be used anywhere anymore.
Should we remove it then ?
static void test_apic_virt_addr(void)
{
+ /*
+ * Ensure the processor will never use the virtual-APIC page, since
+ * we will point it to invalid RAM. Otherwise KVM is puzzled about
+ * what we're trying to achieve and fails vmentry.
+ */
+ u32 cpu_ctrls0 = vmcs_read(CPU_EXEC_CTRL0);
+ vmcs_write(CPU_EXEC_CTRL0, cpu_ctrls0 | CPU_CR8_LOAD |
CPU_CR8_STORE);
Even though "virtualize APIC accesses" is not set by default, it might
be cleaner to unset it explicitly here in the test, in case someone sets
it prior to the execution of this piece of VMX controls test and forgets
to unset it.
If that happened, it would break a lot of things, I think. You're right
that it's not very clean, but the right way to do it would be to start
each test from scratch with an entirely new VMCS. Until then, I'm more
inclined to make things fail loudly if somebody doesn't respect the
invariants and leaves dirty control bits at the end of a test.
Thanks,
Paolo
Reviewed-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
