Hi Chao,
Thank you for reviewing my patches.
On 5/28/2024 1:16 PM, Chao Gao wrote:
>> +static void guest_code(void)
>> +{
>> + uint32_t icr_val;
>> + int i;
>> +
>> + xapic_enable();
>> +
>> + icr_val = (APIC_DEST_SELF | APIC_INT_ASSERT | VINTR_VECTOR);
>> +
>> + for (i = 0; i < NUM_ITERATIONS; i++) {
>> + cli();
>> + xapic_write_reg(APIC_ICR, icr_val);
>> + safe_halt();
>> + GUEST_ASSERT(READ_ONCE(irq_received));
>> + WRITE_ONCE(irq_received, false);
>
> any reason to use READ/WRITE_ONCE here?
This is done to ensure that irq is already received at this point,
as irq_received is set to true in guest_vintr_handler.
>
>> + }
>> + GUEST_DONE();
>> +}
>> +
>> +static void guest_vintr_handler(struct ex_regs *regs)
>> +{
>> + WRITE_ONCE(irq_received, true);
>> + xapic_write_reg(APIC_EOI, 0x00);
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + struct kvm_vm *vm;
>> + struct kvm_vcpu *vcpu;
>> + struct ucall uc;
>> + uint64_t halt_exits, vintr_exits;
>> +
>> + /* Check the extension for binary stats */
>> + TEST_REQUIRE(this_cpu_has(X86_FEATURE_IDLE_HLT));
>
> IIUC, this test assumes that the IDLE_HLT feature is enabled for guests if it
> is supported by the CPU. But this isn't true in some cases:
>
I understand you are intending to create a capability for IDLE HLT intercept feature, but in my
opinion, the IDLE Halt intercept feature doesn't require user space to do anything for the feature
itself.
> 1. an old KVM runs on new hardware
> 2. the feature bit is somehow cleared, e.g., by "clearcpuid" cmdline
In both the case, the test case will fail.
In General, if the feature bit for the Idle halt feature is cleared somehow, or new KVM runs
on old hardware, the idle halt exits will be replaced with halt exits.
If the old KVM runs on new hardware, the idle halt feature is never enabled via KVM.
So, the presence of a pending V_INTR event during the execution of the halt instruction
won't result into the idle-halt exit; rather, it will result in a halt exit followed
by a vintr exit.
-Manali
> >> + TEST_REQUIRE(kvm_has_cap(KVM_CAP_BINARY_STATS_FD));
>> +
>> + vm = vm_create_with_one_vcpu(&vcpu, guest_code);
>> +
>> + vm_install_exception_handler(vm, VINTR_VECTOR, guest_vintr_handler);
>> + virt_pg_map(vm, APIC_DEFAULT_GPA, APIC_DEFAULT_GPA);
>> +
>> + vcpu_run(vcpu);
>> + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
>> +
>> + halt_exits = vcpu_get_stat(vcpu, HALT_EXITS);
>> + vintr_exits = vcpu_get_stat(vcpu, IRQ_WINDOW_EXITS);
>> +
>> + switch (get_ucall(vcpu, &uc)) {
>> + case UCALL_ABORT:
>> + REPORT_GUEST_ASSERT(uc);
>> + /* NOT REACHED */
>> + case UCALL_DONE:
>> + break;
>> +
>> + default:
>> + TEST_FAIL("Unknown ucall 0x%lx.", uc.cmd);
>> + }
>> +
>> + TEST_ASSERT_EQ(halt_exits, 0);
>> + pr_debug("Guest executed VINTR followed by halts: %d times.\n"
>> + "The guest exited due to halt: %ld times and number\n"
>> + "of vintr exits: %ld.\n",
>> + NUM_ITERATIONS, halt_exits, vintr_exits);
>> +
>> + kvm_vm_free(vm);
>> + return 0;
>> +}
>> --
>> 2.34.1
>>
>>
