The inner loop just needs to run from 0 to 7, covering all possible
combinations of the three bits. If an unsupported bit would be set,
skip that loop iteration.
On Tue, Jul 10, 2018 at 4:28 PM, Krish Sadhukhan
<krish.sadhukhan@xxxxxxxxxx> wrote:
> It becomes difficult to create the inner loop as the other three controls
> need to be tested individually, in pairs and then all together. But the
> outer loop makes sense. In fact, after your feedback, I realized that I
> hadn't run all the tests with "use TPR shadow" set. So I have created an
> outer loop (sort of) that runs twice - once with "use TPR shadow" unset and
> then with "use TPR shadow" set.
>
> I will send out the revised version.
>
> Thanks,
> Krish
>
>
> On 07/09/2018 10:31 AM, Jim Mattson wrote:
>>
>> Hi Krish,
>>
>> I think this would be more succint as two loops: an outer loop that
>> tested "use TPR shadow" values and an inner loop that tested
>> combinations of the other three controls. But this looks fine to me as
>> it is.
>>
>> Reviewed-by: Jim Mattson <jmattson@xxxxxxxxxx>
>>
>> On Fri, Jun 29, 2018 at 12:07 PM, Krish Sadhukhan
>> <krish.sadhukhan@xxxxxxxxxx> wrote:
>>>
>>> According to section "Checks on VMX Controls" in Intel SDM vol 3C, the
>>>
>>> following check needs to be enforced on vmentry of L2 guests:
>>>
>>> If the "use TPR shadow" VM-execution control is 0, the following
>>> VM-execution controls must also be 0: "virtualize x2APIC mode",
>>> "APIC-register virtualization" and "virtual-interrupt delivery".
>>>
>>> This unit-test validates the above vmentry check.
>>>
>>> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
>>> Reviewed-by: Karl Heubaum <karl.heubaum@xxxxxxxxxx>
>>> ---
>>> x86/vmx_tests.c | 108
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 106 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
>>> index 38f10f4..64be48f 100644
>>> --- a/x86/vmx_tests.c
>>> +++ b/x86/vmx_tests.c
>>> @@ -3580,6 +3580,111 @@ static void test_apic_access_addr(void)
>>> "virtualize APIC-accesses", true,
>>> false);
>>> }
>>> +/*
>>> + * If the "use TPR shadow" VM-execution control is 0, the following
>>> + * VM-execution controls must also be 0:
>>> + * - virtualize x2APIC mode
>>> + * - APIC-register virtualization
>>> + * - virtual-interrupt delivery
>>> + */
>>> +static void test_apic_virtual_ctls(void)
>>> +{
>>> + u32 saved_primary = vmcs_read(CPU_EXEC_CTRL0);
>>> + u32 saved_secondary = vmcs_read(CPU_EXEC_CTRL1);
>>> + u32 primary = saved_primary;
>>> + u32 secondary = saved_secondary;
>>> + bool cpu_has_virt_x2apic = 0;
>>> + bool cpu_has_apic_reg_virt = 0;
>>> + bool cpu_has_vintd = 0;
>>> +
>>> + if (!((ctrl_cpu_rev[0].clr & (CPU_SECONDARY | CPU_TPR_SHADOW)) ==
>>> + (CPU_SECONDARY | CPU_TPR_SHADOW)))
>>> + return;
>>> +
>>> + if ((ctrl_cpu_rev[1].clr & CPU_VIRT_X2APIC) == CPU_VIRT_X2APIC)
>>> + cpu_has_virt_x2apic = 1;
>>> + if ((ctrl_cpu_rev[1].clr & CPU_APIC_REG_VIRT) ==
>>> CPU_APIC_REG_VIRT)
>>> + cpu_has_apic_reg_virt = 1;
>>> + if ((ctrl_cpu_rev[1].clr & CPU_VINTD) == CPU_VINTD)
>>> + cpu_has_vintd = 1;
>>> +
>>> + primary |= CPU_SECONDARY;
>>> + vmcs_write(CPU_EXEC_CTRL0, primary & ~CPU_TPR_SHADOW);
>>> +
>>> + if (cpu_has_virt_x2apic) {
>>> + secondary &= ~(CPU_APIC_REG_VIRT | CPU_VINTD);
>>> + vmcs_write(CPU_EXEC_CTRL1, secondary | CPU_VIRT_X2APIC);
>>> + report_prefix_pushf("Use TPR shadow disabled; virtualize
>>> x2APIC mode enabled");
>>> + test_vmx_controls(false, false);
>>> + report_prefix_pop();
>>> +
>>> + if (cpu_has_apic_reg_virt) {
>>> + vmcs_write(CPU_EXEC_CTRL1, secondary |
>>> + CPU_APIC_REG_VIRT);
>>> + report_prefix_pushf("Use TPR shadow disabled;
>>> virtualize x2APIC mode enabled; APIC-register virtualization enabled");
>>> + test_vmx_controls(false, false);
>>> + report_prefix_pop();
>>> + }
>>> +
>>> + if (cpu_has_vintd) {
>>> + secondary &= ~CPU_APIC_REG_VIRT;
>>> + vmcs_write(CPU_EXEC_CTRL1, secondary |
>>> CPU_VINTD);
>>> + report_prefix_pushf("Use TPR shadow disabled;
>>> virtualize x2APIC mode enabled; virtual-interrupt delivery enabled");
>>> + test_vmx_controls(false, false);
>>> + report_prefix_pop();
>>> + }
>>> + }
>>> +
>>> + if (cpu_has_apic_reg_virt) {
>>> + secondary &= ~(~CPU_VIRT_X2APIC | CPU_VINTD);
>>> + vmcs_write(CPU_EXEC_CTRL1, secondary |
>>> CPU_APIC_REG_VIRT);
>>> + report_prefix_pushf("Use TPR shadow disabled;
>>> APIC-register
>>> virtualization enabled");
>>> + test_vmx_controls(false, false);
>>> + report_prefix_pop();
>>> +
>>> + if (cpu_has_vintd) {
>>> + secondary &= ~CPU_VIRT_X2APIC;
>>> + vmcs_write(CPU_EXEC_CTRL1, secondary |
>>> CPU_VINTD);
>>> + report_prefix_pushf("Use TPR shadow disabled;
>>> APIC-register virtualization enabled; virtual-interrupt delivery
>>> enabled");
>>> + test_vmx_controls(false, false);
>>> + report_prefix_pop();
>>> + }
>>> + }
>>> +
>>> + if (cpu_has_vintd) {
>>> + secondary &= ~(~CPU_VIRT_X2APIC | CPU_APIC_REG_VIRT);
>>> + vmcs_write(CPU_EXEC_CTRL1, secondary | CPU_VINTD);
>>> + report_prefix_pushf("Use TPR shadow disabled;
>>> virtual-interrupt delivery enabled");
>>> + test_vmx_controls(false, false);
>>> + report_prefix_pop();
>>> + }
>>> +
>>> + if (cpu_has_virt_x2apic && cpu_has_apic_reg_virt &&
>>> + cpu_has_vintd) {
>>> +
>>> + vmcs_write(CPU_EXEC_CTRL1, secondary | (CPU_VIRT_X2APIC |
>>> + CPU_APIC_REG_VIRT | CPU_VINTD));
>>> + report_prefix_pushf("Use TPR shadow disabled; virtualize
>>> x2APIC mode enabled; APIC-register virtualization enabled;
>>> virtual-interrupt
>>> delivery enabled");
>>> + test_vmx_controls(false, false);
>>> + report_prefix_pop();
>>> +
>>> + vmcs_write(CPU_EXEC_CTRL0, primary | CPU_TPR_SHADOW);
>>> + report_prefix_pushf("Use TPR shadow enabled; virtualize
>>> x2APIC mode enabled; APIC-register virtualization enabled;
>>> virtual-interrupt
>>> delivery enabled");
>>> + test_vmx_controls(true, false);
>>> + report_prefix_pop();
>>> + }
>>> +
>>> + vmcs_write(CPU_EXEC_CTRL0, saved_primary);
>>> + vmcs_write(CPU_EXEC_CTRL1, saved_secondary);
>>> +}
>>> +
>>> +static void test_apic_ctls(void)
>>> +{
>>> + test_apic_virt_addr();
>>> + test_apic_access_addr();
>>> + test_apic_virtual_ctls();
>>> +}
>>> +
>>> static void set_vtpr(unsigned vtpr)
>>> {
>>> *(u32 *)phys_to_virt(vmcs_read(APIC_VIRT_ADDR) + APIC_TASKPRI) =
>>> vtpr;
>>> @@ -3846,8 +3951,7 @@ static void vmx_controls_test(void)
>>> test_cr3_targets();
>>> test_io_bitmaps();
>>> test_msr_bitmap();
>>> - test_apic_virt_addr();
>>> - test_apic_access_addr();
>>> + test_apic_ctls();
>>> test_tpr_threshold();
>>> test_nmi_ctrls();
>>> }
>>> --
>>> 2.9.5
>>>
>
