Re: [PATCH v9 00/22] Enable FRED with KVM VMX

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[Maciej Wieczor-Retman]

Hello!


On 2026-04-23 at 15:56:54 -0700, Xin Li wrote:
>> On Apr 23, 2026, at 7:35 AM, David Woodhouse <dwmw2@infradead.org> wrote:
>> Here's one to get you started (untested as I haven't found suitable
>> hardware to test it on).
>
>Same here for me now :(

I ran David's selftest on a PTL laptop and ran into a couple of issues.

>> 
>> From bd465aabebcb124e09a26fe9f4c861354febabe4 Mon Sep 17 00:00:00 2001
>> From: David Woodhouse <dwmw@amazon.co.uk>
>> Date: Thu, 23 Apr 2026 15:20:11 +0100
>> Subject: [PATCH] KVM: selftests: Add FRED event type classification test
>> 
>> +static void __used fred_handler(struct fred_stack_frame *frame)
>> +{
>> + fred_ss_value = frame->ss;
>> + fred_saved_rip = frame->rip;
>> + fred_handler_called = true;
>> +}

fred_handler() has problems getting linked:

	/usr/bin/ld: /home/maciej/linux/tools/testing/selftests/kvm/x86/int1_fred_test.o: in function `fred_entrypoint_kernel':
	int1_fred_test.c:(.text+0x104): undefined reference to `fred_handler'
	collect2: error: ld returned 1 exit status

I guess the .pushsection below makes it a different translation unit? Because
getting rid of the static keyword takes care of the problem for me.

>> +
>> +/*
>> + * FRED entry points. MSR_IA32_FRED_CONFIG points to the page-aligned
>> + * base. Ring 3 events enter at base+0, ring 0 events at base+0x100.
>> + * Since ICEBP executes in ring 0, the CPU enters at fred_entrypoint
>> + * + 256 = fred_entrypoint_kernel.
>> + */
>> +extern void fred_entrypoint(void);
>> +
>> +asm(
>> + ".pushsection .text\n"
>> + ".global fred_entrypoint\n"
>> + ".balign 4096\n"
>> +"fred_entrypoint:\n"
>> + /* Ring 3 entry — unused, no userspace in this test */
>> + "ud2\n"
>> + /* Pad to +256 for ring 0 entry */
>> + ".org fred_entrypoint + 256, 0xcc\n"
>> +"fred_entrypoint_kernel:\n"
>> + "movq %rsp, %rdi\n"
>> + "call fred_handler\n"
>> + ".byte 0xf2, 0x0f, 0x01, 0xca\n" /* ERETS */
>> + ".popsection\n"
>> +);
>> +
...
>> +
>> + /* Test 1: ICEBP (INT1) — should be EVENT_TYPE_PRIV_SWEXC (5) */
>> + fred_handler_called = false;
>> + asm volatile("lea 1f(%%rip), %0\n\t"
>> +     ".byte 0xf1\n\t"
>> +     "1:" : "=r"(expected_rip) :: "memory");
>> + check_fred_event(expected_rip, DB_VECTOR, EVENT_TYPE_PRIV_SWEXC,
>> + "ICEBP");
>> + GUEST_SYNC(0);

The above event type test seems to fail and return 0x3 instead of 0x5:

Random seed: 0x6b8b4567
Testing FRED event types with EPT fault on stack
==== Test Assertion Failure ====
  x86/int1_fred_test.c:120: event_type == expected_type
  pid=16646 tid=16646 errno=4 - Interrupted system call
     1  0x0000000000413349: assert_on_unhandled_exception at processor.c:659
     2  0x0000000000407d36: _vcpu_run at kvm_util.c:1703
     3   (inlined by) vcpu_run at kvm_util.c:1714
     4  0x0000000000403104: main at int1_fred_test.c:207
     5  0x00007ff8d4c2a1c9: ?? ??:0
     6  0x00007ff8d4c2a28a: ?? ??:0
     7  0x0000000000403314: _start at ??:?
  0x3 != 0x5 (event_type != expected_type)

after a little digging I think the issue could be this in arch/x86/kvm/x86.h:

	static inline bool kvm_exception_is_soft(unsigned int nr)
	{
		return (nr == BP_VECTOR) || (nr == OF_VECTOR);
	}

Since ICEBP(INT1) results in a DB_VECTOR it's not take into account and the
check fails. Then in vmx_inject_exception() INTR_TYPE_HARD_EXCEPTION is picked
which is 0x3 when decoded.

I think you'd need to add another check in vmx_inject_exception() to handle that
DB_VECTOR too. Simply changing the event type if the vector is of DB_VECTOR type
fixes that problem but then the selftest fails in other places (assert
fred_handler_called and saved rip vs expected_rip). I didn't yet have the time
to figure out what could be wrong there, maybe you would have more of an idea :)

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[Andrew Cooper]

On 05/05/2026 7:04 pm, Maciej Wieczor-Retman wrote:
> Hello!
>
>
> On 2026-04-23 at 15:56:54 -0700, Xin Li wrote:
>>> On Apr 23, 2026, at 7:35 AM, David Woodhouse <dwmw2@infradead.org> wrote:
>>> Here's one to get you started (untested as I haven't found suitable
>>> hardware to test it on).
>> Same here for me now :(
> I ran David's selftest on a PTL laptop and ran into a couple of issues.
>
>>> From bd465aabebcb124e09a26fe9f4c861354febabe4 Mon Sep 17 00:00:00 2001
>>> From: David Woodhouse <dwmw@amazon.co.uk>
>>> Date: Thu, 23 Apr 2026 15:20:11 +0100
>>> Subject: [PATCH] KVM: selftests: Add FRED event type classification test
>>>
>>> +static void __used fred_handler(struct fred_stack_frame *frame)
>>> +{
>>> + fred_ss_value = frame->ss;
>>> + fred_saved_rip = frame->rip;
>>> + fred_handler_called = true;
>>> +}
> fred_handler() has problems getting linked:
>
> 	/usr/bin/ld: /home/maciej/linux/tools/testing/selftests/kvm/x86/int1_fred_test.o: in function `fred_entrypoint_kernel':
> 	int1_fred_test.c:(.text+0x104): undefined reference to `fred_handler'
> 	collect2: error: ld returned 1 exit status
>
> I guess the .pushsection below makes it a different translation unit? Because
> getting rid of the static keyword takes care of the problem for me.

The problem is, being static, fred_handler() is eligible to be optimised
away, because the compiler can't see that the asm() refers to it.

Dropping static is the right fix to make.

GCC 15 can now do references out of global asm() to identify the symbols
they use, but it's going to be years before this capability is safe to
use generally.

>
>>> +
>>> +/*
>>> + * FRED entry points. MSR_IA32_FRED_CONFIG points to the page-aligned
>>> + * base. Ring 3 events enter at base+0, ring 0 events at base+0x100.
>>> + * Since ICEBP executes in ring 0, the CPU enters at fred_entrypoint
>>> + * + 256 = fred_entrypoint_kernel.
>>> + */
>>> +extern void fred_entrypoint(void);
>>> +
>>> +asm(
>>> + ".pushsection .text\n"
>>> + ".global fred_entrypoint\n"
>>> + ".balign 4096\n"
>>> +"fred_entrypoint:\n"
>>> + /* Ring 3 entry — unused, no userspace in this test */
>>> + "ud2\n"
>>> + /* Pad to +256 for ring 0 entry */
>>> + ".org fred_entrypoint + 256, 0xcc\n"
>>> +"fred_entrypoint_kernel:\n"
>>> + "movq %rsp, %rdi\n"
>>> + "call fred_handler\n"
>>> + ".byte 0xf2, 0x0f, 0x01, 0xca\n" /* ERETS */
>>> + ".popsection\n"
>>> +);
>>> +
> ...
>>> +
>>> + /* Test 1: ICEBP (INT1) — should be EVENT_TYPE_PRIV_SWEXC (5) */
>>> + fred_handler_called = false;
>>> + asm volatile("lea 1f(%%rip), %0\n\t"
>>> +     ".byte 0xf1\n\t"
>>> +     "1:" : "=r"(expected_rip) :: "memory");
>>> + check_fred_event(expected_rip, DB_VECTOR, EVENT_TYPE_PRIV_SWEXC,
>>> + "ICEBP");
>>> + GUEST_SYNC(0);
> The above event type test seems to fail and return 0x3 instead of 0x5:
>
> Random seed: 0x6b8b4567
> Testing FRED event types with EPT fault on stack
> ==== Test Assertion Failure ====
>   x86/int1_fred_test.c:120: event_type == expected_type
>   pid=16646 tid=16646 errno=4 - Interrupted system call
>      1  0x0000000000413349: assert_on_unhandled_exception at processor.c:659
>      2  0x0000000000407d36: _vcpu_run at kvm_util.c:1703
>      3   (inlined by) vcpu_run at kvm_util.c:1714
>      4  0x0000000000403104: main at int1_fred_test.c:207
>      5  0x00007ff8d4c2a1c9: ?? ??:0
>      6  0x00007ff8d4c2a28a: ?? ??:0
>      7  0x0000000000403314: _start at ??:?
>   0x3 != 0x5 (event_type != expected_type)
>
> after a little digging I think the issue could be this in arch/x86/kvm/x86.h:
>
> 	static inline bool kvm_exception_is_soft(unsigned int nr)
> 	{
> 		return (nr == BP_VECTOR) || (nr == OF_VECTOR);
> 	}
>
> Since ICEBP(INT1) results in a DB_VECTOR it's not take into account and the
> check fails. Then in vmx_inject_exception() INTR_TYPE_HARD_EXCEPTION is picked
> which is 0x3 when decoded.

That's a real bug then.

> I think you'd need to add another check in vmx_inject_exception() to handle that
> DB_VECTOR too. Simply changing the event type if the vector is of DB_VECTOR type
> fixes that problem but then the selftest fails in other places (assert
> fred_handler_called and saved rip vs expected_rip). I didn't yet have the time
> to figure out what could be wrong there, maybe you would have more of an idea :)

#DB is intercepted to mitigate CVE-2015-8104 (systemwide DoS).  But, to
start with, check that the test passes when #DB is not intercepted. 
That's the basecase for architectural behaviour.

When #DB is intercepted, the type in EXIT_INTR_INFO needs preserving and
forwarding into ENTRY_INTR_INFO, because that is what distinguishes an
ICEBP #DB from other #DBs.  There's no way of recovering this detail
after the fact.

On the injection side, some #DB's are traps and some are faults.  ICEBP
will have a fault-like VMExit but need trap semantics, so like other
soft interrupts, need INSN_LEN adding to %rip.  But, type=3 #DBs need to
leave %rip unchanged.

~Andrew

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[H. Peter Anvin]

On May 5, 2026 11:30:21 AM PDT, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>On 05/05/2026 7:04 pm, Maciej Wieczor-Retman wrote:
>> Hello!
>>
>>
>> On 2026-04-23 at 15:56:54 -0700, Xin Li wrote:
>>>> On Apr 23, 2026, at 7:35 AM, David Woodhouse <dwmw2@infradead.org> wrote:
>>>> Here's one to get you started (untested as I haven't found suitable
>>>> hardware to test it on).
>>> Same here for me now :(
>> I ran David's selftest on a PTL laptop and ran into a couple of issues.
>>
>>>> From bd465aabebcb124e09a26fe9f4c861354febabe4 Mon Sep 17 00:00:00 2001
>>>> From: David Woodhouse <dwmw@amazon.co.uk>
>>>> Date: Thu, 23 Apr 2026 15:20:11 +0100
>>>> Subject: [PATCH] KVM: selftests: Add FRED event type classification test
>>>>
>>>> +static void __used fred_handler(struct fred_stack_frame *frame)
>>>> +{
>>>> + fred_ss_value = frame->ss;
>>>> + fred_saved_rip = frame->rip;
>>>> + fred_handler_called = true;
>>>> +}
>> fred_handler() has problems getting linked:
>>
>> 	/usr/bin/ld: /home/maciej/linux/tools/testing/selftests/kvm/x86/int1_fred_test.o: in function `fred_entrypoint_kernel':
>> 	int1_fred_test.c:(.text+0x104): undefined reference to `fred_handler'
>> 	collect2: error: ld returned 1 exit status
>>
>> I guess the .pushsection below makes it a different translation unit? Because
>> getting rid of the static keyword takes care of the problem for me.
>
>The problem is, being static, fred_handler() is eligible to be optimised
>away, because the compiler can't see that the asm() refers to it.
>
>Dropping static is the right fix to make.
>
>GCC 15 can now do references out of global asm() to identify the symbols
>they use, but it's going to be years before this capability is safe to
>use generally.
>
>>
>>>> +
>>>> +/*
>>>> + * FRED entry points. MSR_IA32_FRED_CONFIG points to the page-aligned
>>>> + * base. Ring 3 events enter at base+0, ring 0 events at base+0x100.
>>>> + * Since ICEBP executes in ring 0, the CPU enters at fred_entrypoint
>>>> + * + 256 = fred_entrypoint_kernel.
>>>> + */
>>>> +extern void fred_entrypoint(void);
>>>> +
>>>> +asm(
>>>> + ".pushsection .text\n"
>>>> + ".global fred_entrypoint\n"
>>>> + ".balign 4096\n"
>>>> +"fred_entrypoint:\n"
>>>> + /* Ring 3 entry — unused, no userspace in this test */
>>>> + "ud2\n"
>>>> + /* Pad to +256 for ring 0 entry */
>>>> + ".org fred_entrypoint + 256, 0xcc\n"
>>>> +"fred_entrypoint_kernel:\n"
>>>> + "movq %rsp, %rdi\n"
>>>> + "call fred_handler\n"
>>>> + ".byte 0xf2, 0x0f, 0x01, 0xca\n" /* ERETS */
>>>> + ".popsection\n"
>>>> +);
>>>> +
>> ...
>>>> +
>>>> + /* Test 1: ICEBP (INT1) — should be EVENT_TYPE_PRIV_SWEXC (5) */
>>>> + fred_handler_called = false;
>>>> + asm volatile("lea 1f(%%rip), %0\n\t"
>>>> +     ".byte 0xf1\n\t"
>>>> +     "1:" : "=r"(expected_rip) :: "memory");
>>>> + check_fred_event(expected_rip, DB_VECTOR, EVENT_TYPE_PRIV_SWEXC,
>>>> + "ICEBP");
>>>> + GUEST_SYNC(0);
>> The above event type test seems to fail and return 0x3 instead of 0x5:
>>
>> Random seed: 0x6b8b4567
>> Testing FRED event types with EPT fault on stack
>> ==== Test Assertion Failure ====
>>   x86/int1_fred_test.c:120: event_type == expected_type
>>   pid=16646 tid=16646 errno=4 - Interrupted system call
>>      1  0x0000000000413349: assert_on_unhandled_exception at processor.c:659
>>      2  0x0000000000407d36: _vcpu_run at kvm_util.c:1703
>>      3   (inlined by) vcpu_run at kvm_util.c:1714
>>      4  0x0000000000403104: main at int1_fred_test.c:207
>>      5  0x00007ff8d4c2a1c9: ?? ??:0
>>      6  0x00007ff8d4c2a28a: ?? ??:0
>>      7  0x0000000000403314: _start at ??:?
>>   0x3 != 0x5 (event_type != expected_type)
>>
>> after a little digging I think the issue could be this in arch/x86/kvm/x86.h:
>>
>> 	static inline bool kvm_exception_is_soft(unsigned int nr)
>> 	{
>> 		return (nr == BP_VECTOR) || (nr == OF_VECTOR);
>> 	}
>>
>> Since ICEBP(INT1) results in a DB_VECTOR it's not take into account and the
>> check fails. Then in vmx_inject_exception() INTR_TYPE_HARD_EXCEPTION is picked
>> which is 0x3 when decoded.
>
>That's a real bug then.
>
>> I think you'd need to add another check in vmx_inject_exception() to handle that
>> DB_VECTOR too. Simply changing the event type if the vector is of DB_VECTOR type
>> fixes that problem but then the selftest fails in other places (assert
>> fred_handler_called and saved rip vs expected_rip). I didn't yet have the time
>> to figure out what could be wrong there, maybe you would have more of an idea :)
>
>#DB is intercepted to mitigate CVE-2015-8104 (systemwide DoS).  But, to
>start with, check that the test passes when #DB is not intercepted. 
>That's the basecase for architectural behaviour.
>
>When #DB is intercepted, the type in EXIT_INTR_INFO needs preserving and
>forwarding into ENTRY_INTR_INFO, because that is what distinguishes an
>ICEBP #DB from other #DBs.  There's no way of recovering this detail
>after the fact.
>
>On the injection side, some #DB's are traps and some are faults.  ICEBP
>will have a fault-like VMExit but need trap semantics, so like other
>soft interrupts, need INSN_LEN adding to %rip.  But, type=3 #DBs need to
>leave %rip unchanged.
>
>~Andrew

Also, a function that is accessed only from assembly should have the __visible annotation.

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[Maciej Wieczor-Retman]

On 2026-05-05 at 19:30:21 +0100, Andrew Cooper wrote:
>On 05/05/2026 7:04 pm, Maciej Wieczor-Retman wrote:
>> Hello!
>>
>>
>> On 2026-04-23 at 15:56:54 -0700, Xin Li wrote:
>>>> On Apr 23, 2026, at 7:35 AM, David Woodhouse <dwmw2@infradead.org> wrote:
>>>> Here's one to get you started (untested as I haven't found suitable
>>>> hardware to test it on).
>>> Same here for me now :(
>> I ran David's selftest on a PTL laptop and ran into a couple of issues.
>>
>> ...
>>>> +
>>>> + /* Test 1: ICEBP (INT1) — should be EVENT_TYPE_PRIV_SWEXC (5) */
>>>> + fred_handler_called = false;
>>>> + asm volatile("lea 1f(%%rip), %0\n\t"
>>>> +     ".byte 0xf1\n\t"
>>>> +     "1:" : "=r"(expected_rip) :: "memory");
>>>> + check_fred_event(expected_rip, DB_VECTOR, EVENT_TYPE_PRIV_SWEXC,
>>>> + "ICEBP");
>>>> + GUEST_SYNC(0);
>> The above event type test seems to fail and return 0x3 instead of 0x5:
>>
>> Random seed: 0x6b8b4567
>> Testing FRED event types with EPT fault on stack
>> ==== Test Assertion Failure ====
>>   x86/int1_fred_test.c:120: event_type == expected_type
>>   pid=16646 tid=16646 errno=4 - Interrupted system call
>>      1  0x0000000000413349: assert_on_unhandled_exception at processor.c:659
>>      2  0x0000000000407d36: _vcpu_run at kvm_util.c:1703
>>      3   (inlined by) vcpu_run at kvm_util.c:1714
>>      4  0x0000000000403104: main at int1_fred_test.c:207
>>      5  0x00007ff8d4c2a1c9: ?? ??:0
>>      6  0x00007ff8d4c2a28a: ?? ??:0
>>      7  0x0000000000403314: _start at ??:?
>>   0x3 != 0x5 (event_type != expected_type)
>>
>> after a little digging I think the issue could be this in arch/x86/kvm/x86.h:
>>
>> 	static inline bool kvm_exception_is_soft(unsigned int nr)
>> 	{
>> 		return (nr == BP_VECTOR) || (nr == OF_VECTOR);
>> 	}
>>
>> Since ICEBP(INT1) results in a DB_VECTOR it's not take into account and the
>> check fails. Then in vmx_inject_exception() INTR_TYPE_HARD_EXCEPTION is picked
>> which is 0x3 when decoded.
>
>That's a real bug then.
>
>> I think you'd need to add another check in vmx_inject_exception() to handle that
>> DB_VECTOR too. Simply changing the event type if the vector is of DB_VECTOR type
>> fixes that problem but then the selftest fails in other places (assert
>> fred_handler_called and saved rip vs expected_rip). I didn't yet have the time
>> to figure out what could be wrong there, maybe you would have more of an idea :)
>
>#DB is intercepted to mitigate CVE-2015-8104 (systemwide DoS).  But, to
>start with, check that the test passes when #DB is not intercepted. 
>That's the basecase for architectural behaviour.

I take it you mean dropping the ICEBP selftest test case and just checking INT3
and INT $0x20? In that case the other two tests pass after a minor change -
namely in guest_code() the expected_rip needs to be volatile as well. Otherwise
there is a RIP mismatch.

Or did you mean I should check something else?

>When #DB is intercepted, the type in EXIT_INTR_INFO needs preserving and
>forwarding into ENTRY_INTR_INFO, because that is what distinguishes an
>ICEBP #DB from other #DBs.  There's no way of recovering this detail
>after the fact.
>
>On the injection side, some #DB's are traps and some are faults.  ICEBP
>will have a fault-like VMExit but need trap semantics, so like other
>soft interrupts, need INSN_LEN adding to %rip.  But, type=3 #DBs need to
>leave %rip unchanged.
>
>~Andrew

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[Andrew Cooper]

On 05/05/2026 9:20 pm, Maciej Wieczor-Retman wrote:
> On 2026-05-05 at 19:30:21 +0100, Andrew Cooper wrote:
>> On 05/05/2026 7:04 pm, Maciej Wieczor-Retman wrote:
>>
>>> I think you'd need to add another check in vmx_inject_exception() to handle that
>>> DB_VECTOR too. Simply changing the event type if the vector is of DB_VECTOR type
>>> fixes that problem but then the selftest fails in other places (assert
>>> fred_handler_called and saved rip vs expected_rip). I didn't yet have the time
>>> to figure out what could be wrong there, maybe you would have more of an idea :)
>> #DB is intercepted to mitigate CVE-2015-8104 (systemwide DoS).  But, to
>> start with, check that the test passes when #DB is not intercepted. 
>> That's the basecase for architectural behaviour.
> I take it you mean dropping the ICEBP selftest test case and just checking INT3
> and INT $0x20? In that case the other two tests pass after a minor change -
> namely in guest_code() the expected_rip needs to be volatile as well. Otherwise
> there is a RIP mismatch.
>
> Or did you mean I should check something else?

The selftest is correct AIUI.  You should be able to prove this by
disabling #DB interception, and observing the test to pass.

Then, there's a logic bug to fix to cause the test to pass even when #DB
interception is active (which is necessary due to CVE-2015-8104).

~Andrew

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[Maciej Wieczor-Retman]

On 2026-05-05 at 21:27:12 +0100, Andrew Cooper wrote:
>On 05/05/2026 9:20 pm, Maciej Wieczor-Retman wrote:
>> On 2026-05-05 at 19:30:21 +0100, Andrew Cooper wrote:
>>> On 05/05/2026 7:04 pm, Maciej Wieczor-Retman wrote:
>>>
>>>> I think you'd need to add another check in vmx_inject_exception() to handle that
>>>> DB_VECTOR too. Simply changing the event type if the vector is of DB_VECTOR type
>>>> fixes that problem but then the selftest fails in other places (assert
>>>> fred_handler_called and saved rip vs expected_rip). I didn't yet have the time
>>>> to figure out what could be wrong there, maybe you would have more of an idea :)
>>> #DB is intercepted to mitigate CVE-2015-8104 (systemwide DoS).  But, to
>>> start with, check that the test passes when #DB is not intercepted. 
>>> That's the basecase for architectural behaviour.
>> I take it you mean dropping the ICEBP selftest test case and just checking INT3
>> and INT $0x20? In that case the other two tests pass after a minor change -
>> namely in guest_code() the expected_rip needs to be volatile as well. Otherwise
>> there is a RIP mismatch.
>>
>> Or did you mean I should check something else?
>
>The selftest is correct AIUI.  You should be able to prove this by
>disabling #DB interception, and observing the test to pass.

Okay, I commented out '(1u << DB_VECTOR)' from vmx_update_exception_bitmap(),
and booting with that and applying the two changes to the selftest that I
mentioned [1] it passes as you said.

>Then, there's a logic bug to fix to cause the test to pass even when #DB
>interception is active (which is necessary due to CVE-2015-8104).

I'll dig around the #DB thing, maybe I'll figure out what's wrong there.

>
>~Andrew

[1] Changes being making fred_handler() non-static and making expected_rip
volatile.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

On Tue, 2026-05-05 at 22:20 +0200, Maciej Wieczor-Retman wrote:
> 
> I take it you mean dropping the ICEBP selftest test case and just checking INT3
> and INT $0x20? In that case the other two tests pass after a minor change -
> namely in guest_code() the expected_rip needs to be volatile as well. Otherwise
> there is a RIP mismatch.

I don't understand the part about making expected_rip volatile. Are the
asm constraints there not correct? If not I'd rather *fix* them than
use 'volatile' to paper over it. I can't see the issue though.

Can you show the generated asm both with and without it?

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[Maciej Wieczor-Retman]

On 2026-05-07 at 08:49:27 +0100, David Woodhouse wrote:
>On Tue, 2026-05-05 at 22:20 +0200, Maciej Wieczor-Retman wrote:
>> 
>> I take it you mean dropping the ICEBP selftest test case and just checking INT3
>> and INT $0x20? In that case the other two tests pass after a minor change -
>> namely in guest_code() the expected_rip needs to be volatile as well. Otherwise
>> there is a RIP mismatch.
>
>I don't understand the part about making expected_rip volatile. Are the
>asm constraints there not correct? If not I'd rather *fix* them than
>use 'volatile' to paper over it. I can't see the issue though.
>
>Can you show the generated asm both with and without it?

---------- not volatile ---------------- | -------------- volatile -----------------
#APP						#APP
# 156 "x86/int1_fred_test.c" 1			# 156 "x86/int1_fred_test.c" 1
	lea 1f(%rip), %rdi		      |		lea 1f(%rip), %rax
	int3						int3
	1:						1:
# 0 "" 2					# 0 "" 2
.LVL42:					      <
	.loc 3 159 2 view .LVU146	      <
#NO_APP						#NO_APP
					      >		movq	%rax, 8(%rsp)
					      >		.loc 3 159 2 view .LVU146
					      >		movq	8(%rsp), %rdi
	movl	$6, %edx				movl	$6, %edx
	movl	$3, %esi				movl	$3, %esi
	call	check_fred_event.isra.0			call	check_fred_event.isra.0

I think that when the FRED event happens it doesn't save RDI and overwrites it
before going into check_fred_event. In the volatile case it is saved to the
stack before check_fred_event() (.loc 159 is the check_fred_event() call).

The below does work, it generates pretty much the same assembly as the volatile
expected_rip. Do you like it? I'm not sure if it can be simplified better?

	asm volatile("lea 1f(%%rip), %%rax\n\t"
		     "movq %%rax, %0\n\t"
		     "int3\n\t"
		     "1:" : "=m"(expected_rip) :: "memory", "rax");

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2468 bytes --]

On Thu, 2026-05-07 at 14:59 +0200, Maciej Wieczor-Retman wrote:
> On 2026-05-07 at 08:49:27 +0100, David Woodhouse wrote:
> > On Tue, 2026-05-05 at 22:20 +0200, Maciej Wieczor-Retman wrote:
> > > 
> > > I take it you mean dropping the ICEBP selftest test case and just checking INT3
> > > and INT $0x20? In that case the other two tests pass after a minor change -
> > > namely in guest_code() the expected_rip needs to be volatile as well. Otherwise
> > > there is a RIP mismatch.
> > 
> > I don't understand the part about making expected_rip volatile. Are the
> > asm constraints there not correct? If not I'd rather *fix* them than
> > use 'volatile' to paper over it. I can't see the issue though.
> > 
> > Can you show the generated asm both with and without it?
> 
> ---------- not volatile ---------------- | -------------- volatile -----------------
> #APP						#APP
> # 156 "x86/int1_fred_test.c" 1			# 156 "x86/int1_fred_test.c" 1
> 	lea 1f(%rip), %rdi		      |		lea 1f(%rip), %rax
> 	int3						int3
> 	1:						1:
> # 0 "" 2					# 0 "" 2
> .LVL42:					      <
> 	.loc 3 159 2 view .LVU146	      <
> #NO_APP						#NO_APP
> 					      >		movq	%rax, 8(%rsp)
> 					      >		.loc 3 159 2 view .LVU146
> 					      >		movq	8(%rsp), %rdi
> 	movl	$6, %edx				movl	$6, %edx
> 	movl	$3, %esi				movl	$3, %esi
> 	call	check_fred_event.isra.0			call	check_fred_event.isra.0
> 
> I think that when the FRED event happens it doesn't save RDI and overwrites it
> before going into check_fred_event. In the volatile case it is saved to the
> stack before check_fred_event() (.loc 159 is the check_fred_event() call).

That looks OK to me. When it calls check_fred_event(), expected_rip is
the first argument and thus lives in %rdi. On the right hand side where
it's volatile, it gets explicitly loaded again from 8(%rsp) for the
call to check_fred_event().

On the left hand side, the compiler doesn't mess with it at all; just
chooses %rdi as the register to use for the output %0 of the inline
assembly. So it's loaded *directly* into %rdi by our 'lea' and it goes
straight from there to the check_fred_event() function as it should.

Your version turns it into a memory operand and forces it to get
written out to the stack, after which the compiler has to load it
again, so yes it'll look a lot more like your right hand side.

But I don't see anything actually wrong with the original.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[Maciej Wieczor-Retman]

On 2026-05-07 at 14:35:41 +0100, David Woodhouse wrote:
>On Thu, 2026-05-07 at 14:59 +0200, Maciej Wieczor-Retman wrote:
>> On 2026-05-07 at 08:49:27 +0100, David Woodhouse wrote:
>> > On Tue, 2026-05-05 at 22:20 +0200, Maciej Wieczor-Retman wrote:
>> > > 
>> > > I take it you mean dropping the ICEBP selftest test case and just checking INT3
>> > > and INT $0x20? In that case the other two tests pass after a minor change -
>> > > namely in guest_code() the expected_rip needs to be volatile as well. Otherwise
>> > > there is a RIP mismatch.
>> > 
>> > I don't understand the part about making expected_rip volatile. Are the
>> > asm constraints there not correct? If not I'd rather *fix* them than
>> > use 'volatile' to paper over it. I can't see the issue though.
>> > 
>> > Can you show the generated asm both with and without it?
>> 
>> ---------- not volatile ---------------- | -------------- volatile -----------------
>> #APP						#APP
>> # 156 "x86/int1_fred_test.c" 1			# 156 "x86/int1_fred_test.c" 1
>> 	lea 1f(%rip), %rdi		      |		lea 1f(%rip), %rax
>> 	int3						int3
>> 	1:						1:
>> # 0 "" 2					# 0 "" 2
>> .LVL42:					      <
>> 	.loc 3 159 2 view .LVU146	      <
>> #NO_APP						#NO_APP
>> 					      >		movq	%rax, 8(%rsp)
>> 					      >		.loc 3 159 2 view .LVU146
>> 					      >		movq	8(%rsp), %rdi
>> 	movl	$6, %edx				movl	$6, %edx
>> 	movl	$3, %esi				movl	$3, %esi
>> 	call	check_fred_event.isra.0			call	check_fred_event.isra.0
>> 
>> I think that when the FRED event happens it doesn't save RDI and overwrites it
>> before going into check_fred_event. In the volatile case it is saved to the
>> stack before check_fred_event() (.loc 159 is the check_fred_event() call).
>
>That looks OK to me. When it calls check_fred_event(), expected_rip is
>the first argument and thus lives in %rdi. On the right hand side where
>it's volatile, it gets explicitly loaded again from 8(%rsp) for the
>call to check_fred_event().
>
>On the left hand side, the compiler doesn't mess with it at all; just
>chooses %rdi as the register to use for the output %0 of the inline
>assembly. So it's loaded *directly* into %rdi by our 'lea' and it goes
>straight from there to the check_fred_event() function as it should.
>
>Your version turns it into a memory operand and forces it to get
>written out to the stack, after which the compiler has to load it
>again, so yes it'll look a lot more like your right hand side.
>
>But I don't see anything actually wrong with the original.

My theory is that after 'int3' call the FRED event is handled elsewhere and %rdi
is not preserved. So the original version of the assembly looks okay but I was
thinking that int3 has side effects.

Below is the test output of the RIPs not matching when running the original,
does that help in any way?

Random seed: 0x6b8b4567
Testing FRED event types with EPT fault on stack
==== Test Assertion Failure ====
  x86/int1_fred_test.c:114: fred_saved_rip == expected_rip
  pid=193114 tid=193114 errno=4 - Interrupted system call
     1  0x0000000000413319: assert_on_unhandled_exception at processor.c:659
     2  0x0000000000407d06: _vcpu_run at kvm_util.c:1703
     3   (inlined by) vcpu_run at kvm_util.c:1714
     4  0x0000000000403104: main at int1_fred_test.c:209
     5  0x00007f90b3c2a1c9: ?? ??:0
     6  0x00007f90b3c2a28a: ?? ??:0
     7  0x0000000000403314: _start at ??:?
  0x40446b != 0xabaf80 (fred_saved_rip != expected_rip)

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 684 bytes --]

On Thu, 2026-05-07 at 15:53 +0200, Maciej Wieczor-Retman wrote:
> 
> My theory is that after 'int3' call the FRED event is handled elsewhere and %rdi
> is not preserved. So the original version of the assembly looks okay but I was
> thinking that int3 has side effects.
> 
> Below is the test output of the RIPs not matching when running the original,
> does that help in any way?

Ah right, of course!

+"fred_entrypoint_kernel:\n"
+	"movq %rsp, %rdi\n"
+	"call fred_handler\n"
+	".byte 0xf2, 0x0f, 0x01, 0xca\n"	/* ERETS */
+	".popsection\n"

That needs to save the %rdi it's scribbling on, plus all the callee-
clobbered registers that fred_handler() might use.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2017 bytes --]

On Thu, 2026-05-07 at 15:53 +0200, Maciej Wieczor-Retman wrote:
> 
> My theory is that after 'int3' call the FRED event is handled elsewhere and %rdi
> is not preserved. So the original version of the assembly looks okay but I was
> thinking that int3 has side effects.

Please could you try the version at
https://git.infradead.org/?p=users/dwmw2/linux.git;a=commitdiff;h=fred
or this incremental patch:

diff --git a/tools/testing/selftests/kvm/x86/int1_fred_test.c b/tools/testing/selftests/kvm/x86/int1_fred_test.c
index 7ffb54b56047..b91600abda62 100644
--- a/tools/testing/selftests/kvm/x86/int1_fred_test.c
+++ b/tools/testing/selftests/kvm/x86/int1_fred_test.c
@@ -74,7 +74,7 @@ static volatile uint64_t fred_ss_value;
 static volatile uint64_t fred_saved_rip;
 static volatile bool fred_handler_called;
 
-static void __used fred_handler(struct fred_stack_frame *frame)
+static __attribute__((used)) void fred_handler(struct fred_stack_frame *frame)
 {
 	fred_ss_value = frame->ss;
 	fred_saved_rip = frame->rip;
@@ -99,8 +99,41 @@ asm(
 	/* Pad to +256 for ring 0 entry */
 	".org fred_entrypoint + 256, 0xcc\n"
 "fred_entrypoint_kernel:\n"
-	"movq %rsp, %rdi\n"
+	/* Save all GPRs — exception must be transparent to interrupted code */
+	"pushq %rax\n"
+	"pushq %rcx\n"
+	"pushq %rdx\n"
+	"pushq %rbx\n"
+	"pushq %rbp\n"
+	"pushq %rsi\n"
+	"pushq %rdi\n"
+	"pushq %r8\n"
+	"pushq %r9\n"
+	"pushq %r10\n"
+	"pushq %r11\n"
+	"pushq %r12\n"
+	"pushq %r13\n"
+	"pushq %r14\n"
+	"pushq %r15\n"
+	/* Pass pointer to FRED stack frame (above our saved regs) */
+	"leaq 120(%rsp), %rdi\n"
 	"call fred_handler\n"
+	/* Restore all GPRs */
+	"popq %r15\n"
+	"popq %r14\n"
+	"popq %r13\n"
+	"popq %r12\n"
+	"popq %r11\n"
+	"popq %r10\n"
+	"popq %r9\n"
+	"popq %r8\n"
+	"popq %rdi\n"
+	"popq %rsi\n"
+	"popq %rbp\n"
+	"popq %rbx\n"
+	"popq %rdx\n"
+	"popq %rcx\n"
+	"popq %rax\n"
 	".byte 0xf2, 0x0f, 0x01, 0xca\n"	/* ERETS */
 	".popsection\n"
 );


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[Maciej Wieczor-Retman]

On 2026-05-08 at 00:00:21 +0100, David Woodhouse wrote:
>On Thu, 2026-05-07 at 15:53 +0200, Maciej Wieczor-Retman wrote:
>> 
>> My theory is that after 'int3' call the FRED event is handled elsewhere and %rdi
>> is not preserved. So the original version of the assembly looks okay but I was
>> thinking that int3 has side effects.
>
>Please could you try the version at
>https://git.infradead.org/?p=users/dwmw2/linux.git;a=commitdiff;h=fred
>or this incremental patch:
...
>

Just tested it and now it works fine :)
(aside from the ICEBP thing of course but that's on the kernel side)

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 413 bytes --]

On Fri, 2026-05-08 at 16:25 +0200, Maciej Wieczor-Retman wrote:
> 
> Just tested it and now it works fine :)

Great, thanks. Including the __attribute__((used)) part?

> (aside from the ICEBP thing of course but that's on the kernel side)

Well yes, that was kind of the point in generating the selftest.

I don't have access to hardware right now, but I can do test driven
development by proxy... :)

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

Re: [PATCH v9 00/22] Enable FRED with KVM VMX

[Maciej Wieczor-Retman]

On 2026-05-08 at 15:46:41 +0100, David Woodhouse wrote:
>On Fri, 2026-05-08 at 16:25 +0200, Maciej Wieczor-Retman wrote:
>> 
>> Just tested it and now it works fine :)
>
>Great, thanks. Including the __attribute__((used)) part?

Yeah, I didn't have to modify anything aside from commenting out the ICEBP part
for the test to pass.

>> (aside from the ICEBP thing of course but that's on the kernel side)
>
>Well yes, that was kind of the point in generating the selftest.
>
>I don't have access to hardware right now, but I can do test driven
>development by proxy... :)

Anytime :b

-- 
Kind regards
Maciej Wieczór-Retman