Hi Sean,
Thank you for reviewing my patches.
On 8/26/2024 9:36 PM, Sean Christopherson wrote:
On Mon, Aug 26, 2024, Manali Shukla wrote:
+struct buslock_test {
- unsigned char pad[126];
- atomic_long_t val;
+} __packed;
+struct buslock_test test __cacheline_aligned;
+static __always_inline void buslock_atomic_add(int i, atomic_long_t *v) +{
- asm volatile(LOCK_PREFIX "addl %1,%0"
: "+m" (v->counter): "ir" (i) : "memory");+}
+static void buslock_add(void) +{
- /*
* Increment a cache unaligned variable atomically.* This should generate a bus lock exit.So... this test doesn't actually verify that a bus lock exit occurs. The userspace side will eat an exit if one occurs, but there's literally not a single TEST_ASSERT() in here.
Agreed, How about doing following?
for (;;) {struct ucall uc;vcpu_run(vcpu);if (run->exit_reason == KVM_EXIT_IO) {switch (get_ucall(vcpu, &uc)) {case UCALL_ABORT:REPORT_GUEST_ASSERT(uc);/* NOT REACHED */case UCALL_SYNC:break;case UCALL_DONE:goto done;default:TEST_FAIL("Unknown ucall 0x%lx.", uc.cmd);}}TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_X86_BUS_LOCK);I doubt this works, the UCALL_SYNC above will fallthrough to this assert. I assume run->exit_reason needs a continue for UCALL_SYNC.
I agree, there should be a continue for UCALL_SYNC in place of break. I will correct it in V2.
I didn't observe this issue because UCALL_SYNC is invoked, when GUEST_SYNC() is called from the guest code. Since GUEST_SYNC() is not present in the guest code used in bus lock test case, UCALL_SYNC was never triggered.
TEST_ASSERT_EQ(run->flags, KVM_RUN_X86_BUS_LOCK);run->flags &= ~KVM_RUN_X86_BUS_LOCK;No need, KVM should clear the flag if the exit isn't due to a bus lock.
Sure I will remove this.
run->exit_reason = 0;Again, no need, KVM should take care of resetting exit_reason.
Ack.
}
- Manali