On 6/19/2025 3:15 PM, Sohil Mehta wrote:

On 6/18/2025 10:02 PM, Xin Li wrote:

quoted

quoted

diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
index 29c5c32c16c3..1c9c6e036233 100644
--- a/arch/x86/entry/entry_64_fred.S
+++ b/arch/x86/entry/entry_64_fred.S

@@ -93,7 +93,7 @@ SYM_FUNC_START(asm_fred_entry_from_kvm)
   	 * +--------+-----------------+
   	 */
   	push $0				/* Reserved, must be 0 */
-	push $0				/* Event data, 0 for IRQ/NMI */
+	push %rsi			/* Event data for NMI */

Maybe a bit more accurate?

Actually, I am wondering if it might be better to make it less precise
than it is right now. How about

/* Event data passed in by the caller */

The problem with having precise comments for a generic implementation is
that the caller might get updated, but we would forget to update this
comment since nothing in this function needs to change.

No strong preference, I'm okay if you take this approach.

quoted

/* Event data, NMI-source bitmap only so far */

quoted

   	push %rdi			/* fred_ss handed in by the caller */
   	push %rbp
   	pushf

...

quoted

quoted

   /* Must be called from noinstr code, thus __always_inline */
-static __always_inline void fred_nmi_from_kvm(void)
+static __always_inline void fred_nmi_from_kvm(unsigned long edata)
   {
   	struct fred_ss ss = {
   		.ss	= __KERNEL_DS,

@@ -83,7 +83,7 @@ static __always_inline void fred_nmi_from_kvm(void)
   		.lm	= 1,
   	};
   
-	asm_fred_entry_from_kvm(ss);
+	asm_fred_entry_from_kvm(ss, edata);
   }
   
   static inline void fred_irq_from_kvm(unsigned int vector)

@@ -95,7 +95,8 @@ static inline void fred_irq_from_kvm(unsigned int vector)
   		.lm	= 1,
   	};
   
-	asm_fred_entry_from_kvm(ss);
+	/* Event data is always zero for IRQ */

/* Event data not used for IRQ thus 0 */

Event data "not used" might imply that the architecture provides
something, but the kernel is choosing to not use it. There is no event
data for IRQ, right?

I want to say that the event data for IRQ has to be zero until the
architecture changes — Similar to the /* Reserved, must be 0 */ comment
in asm_fred_entry_from_kvm().

FRED spec says:

For any other event, the event data are not currently defined and will 
be zero until they are.

So "Event data not defined for IRQ thus 0."