Re: [RFC PATCH v1 4/4] KVM: selftests: Add bus lock exit test

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.

•           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.

•           run->exit_reason = 0;
  

Again, no need, KVM should take care of resetting exit_reason.

•   }
  
  

• Manali