Re: [PATCH 6/6] KVM: selftests: Verify that faulting in private guest_memfd memory fails

[Ackerley Tng <ackerleytng@google.com>]

Sean Christopherson <seanjc@google.com> writes:

> On Tue, Sep 30, 2025, Ackerley Tng wrote:
>> Sean Christopherson <seanjc@google.com> writes:
>> 
>> [...snip...]
>> 
>> 
>> At some point, sigaction, sigsetjmp, etc could perhaps even be further
>> wrapped. For testing memory_failure() for guest_memfd we will want to
>> check for SIGBUS on memory failure injection instead of on host fault.
>> 
>> Would be nice if it looked like this (maybe not in this patch series):
>> 
>> + TEST_ASSERT_WILL_SIGBUS(READ_ONCE(mem[i]))
>> + TEST_ASSERT_WILL_SIGBUS(WRITE_ONCE(mem[i]))
>> + TEST_ASSERT_WILL_SIGBUS(madvise(MADV_HWPOISON))
>
> Ooh, me likey.  Definitely can do it now.  Using a macro means we can print out
> the actual action that didn't generate a SIGUBS, e.g. hacking the test to read
> byte 0 generates:
>
> 	'(void)READ_ONCE(mem[0])' should have triggered SIGBUS
>
> Hmm, how about TEST_EXPECT_SIGBUS?  TEST_ASSERT_xxx() typically asserts on a
> value, i.e. on the result of a previous action.  And s/WILL/EXPECT to make it
> clear that the action is expected to SIGBUS _now_.
>
> And if we use a descriptive global variable, we can extract the macro to e.g.
> test_util.h or kvm_util.h (not sure we want to do that right away; probably best
> left to the future).
>
> static sigjmp_buf expect_sigbus_jmpbuf;
> void fault_sigbus_handler(int signum)
> {
> 	siglongjmp(expect_sigbus_jmpbuf, 1);
> }
>
> #define TEST_EXPECT_SIGBUS(action)						\
> do {										\
> 	struct sigaction sa_old, sa_new = {					\
> 		.sa_handler = fault_sigbus_handler,				\
> 	};									\
> 										\
> 	sigaction(SIGBUS, &sa_new, &sa_old);					\
> 	if (sigsetjmp(expect_sigbus_jmpbuf, 1) == 0) {				\
> 		action;								\
> 		TEST_FAIL("'%s' should have triggered SIGBUS", #action);	\
> 	}									\
> 	sigaction(SIGBUS, &sa_old, NULL);					\
> } while (0)
>
> static void test_fault_sigbus(int fd, size_t accessible_size, size_t map_size)
> {
> 	const char val = 0xaa;
> 	char *mem;
> 	size_t i;
>
> 	mem = kvm_mmap(map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd);
>
> 	TEST_EXPECT_SIGBUS(memset(mem, val, map_size));
> 	TEST_EXPECT_SIGBUS((void)READ_ONCE(mem[accessible_size]));
>
> 	for (i = 0; i < accessible_size; i++)
> 		TEST_ASSERT_EQ(READ_ONCE(mem[i]), val);
>
> 	kvm_munmap(mem, map_size);
> }
>

Awesome! Thanks!

And thanks for the explanations on the other suggestions.

>> 
>> [...snip...]
>>