Re: [PATCH] KVM: x86: Use gfn_to_pfn_cache for steal_time

[David Woodhouse <dwmw2@infradead.org>]

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

On Fri, 2024-08-02 at 13:03 +0100, David Woodhouse wrote:
> 
> I'm actually tempted to make user access *interruptible* though, and
> either add copy_{from,to}_user_interruptible() or change the semantics
> of the existing ones (which I believe are already killable).
> 
> That would require each architecture implementing interruptible
> exceptions, by doing an extable lookup before the retry. Not overly
> complex, but needs to be done for all architectures (although not at
> once; we could live with not-yet-done architectures just remaining
> killable).
> 
> Thoughts?

Utterly untested, hasn't even built yet and would need some cleanup
(and better thoughts about how to indicate -EFAULT vs. -EINTR instead
of having the caller check signal_pending() for itself). But should
demonstrate what I was thinking, at least...

diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
index 2bec0c89a95c..854ccd5f2342 100644
--- a/arch/x86/include/asm/asm.h
+++ b/arch/x86/include/asm/asm.h
@@ -229,6 +229,9 @@ register unsigned long current_stack_pointer asm(_ASM_SP);
 #define _ASM_EXTABLE_UA(from, to)				\
 	_ASM_EXTABLE_TYPE(from, to, EX_TYPE_UACCESS)
 
+#define _ASM_EXTABLE_UA_INTR(from, to)				\
+	_ASM_EXTABLE_TYPE(from, to, EX_TYPE_UACCESS_INTERRUPTIBLE)
+
 #define _ASM_EXTABLE_FAULT(from, to)				\
 	_ASM_EXTABLE_TYPE(from, to, EX_TYPE_FAULT)
 
diff --git a/arch/x86/include/asm/extable_fixup_types.h b/arch/x86/include/asm/extable_fixup_types.h
index 906b0d5541e8..651d42f39e9b 100644
--- a/arch/x86/include/asm/extable_fixup_types.h
+++ b/arch/x86/include/asm/extable_fixup_types.h
@@ -36,7 +36,7 @@
 #define	EX_TYPE_DEFAULT			 1
 #define	EX_TYPE_FAULT			 2
 #define	EX_TYPE_UACCESS			 3
-/* unused, was: #define EX_TYPE_COPY	 4 */
+#define	EX_TYPE_UACCESS_INTERRUPTIBLE	 4
 #define	EX_TYPE_CLEAR_FS		 5
 #define	EX_TYPE_FPU_RESTORE		 6
 #define	EX_TYPE_BPF			 7
diff --git a/arch/x86/include/asm/trapnr.h b/arch/x86/include/asm/trapnr.h
index 8d1154cdf787..9f6397bad398 100644
--- a/arch/x86/include/asm/trapnr.h
+++ b/arch/x86/include/asm/trapnr.h
@@ -41,4 +41,5 @@
 #define X86_TRAP_VC		29	/* VMM Communication Exception */
 #define X86_TRAP_IRET		32	/* IRET Exception */
 
+#define X86_TRAP_INTERRUPTIBLE 0x40000000	/* Internal, for interruptible exceptions */
 #endif
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cef729a25655..ab00150d360b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3819,12 +3819,15 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
 		asm volatile("1: xchgb %0, %2\n"
 			     "xor %1, %1\n"
 			     "2:\n"
-			     _ASM_EXTABLE_UA(1b, 2b)
+			     _ASM_EXTABLE_UA_INTR(1b, 2b)
 			     : "+q" (st_preempted),
 			       "+&r" (err),
 			       "+m" (st->preempted));
-		if (err)
+		if (err) {
+			if (signal_pending(current))
+				err = -EINTR;
 			goto out;
+		}
 
 		user_access_end();
 
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 51986e8a9d35..d2cef84042a5 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -325,6 +325,12 @@ int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,
 	reg  = FIELD_GET(EX_DATA_REG_MASK,  e->data);
 	imm  = FIELD_GET(EX_DATA_IMM_MASK,  e->data);
 
+	if (trapnr & X86_TRAP_INTERRUPTIBLE) {
+		trapnr &= ~X86_TRAP_INTERRUPTIBLE;
+		if (type != EX_TYPE_UACCESS_INTERRUPTIBLE)
+			return 0;
+	}
+
 	switch (type) {
 	case EX_TYPE_DEFAULT:
 	case EX_TYPE_DEFAULT_MCE_SAFE:
@@ -333,6 +339,7 @@ int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,
 	case EX_TYPE_FAULT_MCE_SAFE:
 		return ex_handler_fault(e, regs, trapnr);
 	case EX_TYPE_UACCESS:
+	case EX_TYPE_UACCESS_INTERRUPTIBLE:
 		return ex_handler_uaccess(e, regs, trapnr, fault_addr);
 	case EX_TYPE_CLEAR_FS:
 		return ex_handler_clear_fs(e, regs);
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index e6c469b323cc..4b32348dbb23 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1388,18 +1388,6 @@ void do_user_addr_fault(struct pt_regs *regs,
 	 */
 	fault = handle_mm_fault(vma, address, flags, regs);
 
-	if (fault_signal_pending(fault, regs)) {
-		/*
-		 * Quick path to respond to signals.  The core mm code
-		 * has unlocked the mm for us if we get here.
-		 */
-		if (!user_mode(regs))
-			kernelmode_fixup_or_oops(regs, error_code, address,
-						 SIGBUS, BUS_ADRERR,
-						 ARCH_DEFAULT_PKEY);
-		return;
-	}
-
 	/* The fault is fully completed (including releasing mmap lock) */
 	if (fault & VM_FAULT_COMPLETED)
 		return;
@@ -1410,6 +1398,28 @@ void do_user_addr_fault(struct pt_regs *regs,
 	 * that we made any progress. Handle this case first.
 	 */
 	if (unlikely(fault & VM_FAULT_RETRY)) {
+		if (signal_pending(current)) {
+			if (user_mode(regs))
+				return;
+
+			if (fatal_signal_pending(current)) {
+				kernelmode_fixup_or_oops(regs, error_code, address,
+							 SIGBUS, BUS_ADRERR,
+							 ARCH_DEFAULT_PKEY);
+				return;
+			}
+
+			/*
+			 * First time round, if woken by a signal, see if there
+			 * is an interruptible exception handler. If so, do it.
+			 * Else, switch off FAULT_FLAG_INTERRUPTIBLE.
+			 */
+			if (fixup_exception(regs, X86_TRAP_INTERRUPTIBLE | X86_TRAP_PF,
+					    error_code, address))
+				return;
+			flags &= ~FAULT_FLAG_INTERRUPTIBLE;
+		}
+
 		flags |= FAULT_FLAG_TRIED;
 		goto retry;
 	}


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