[PATCH 0/2] KVM: arm64: Fix splat relating to idregs debugfs

[PATCH 0/2] KVM: arm64: Fix splat relating to idregs debugfs

[Oliver Upton]

Looks like we're calling in to debugfs w/ preemption disabled, which
explodes when trying to take the parent inode's rwsem.

This is easy enough to work around, and we can just initialize at the
time of VM creation. No idreg values yet? Too bad, just kick an error
back at the user. All two of them.

Applies to kvmarm/next.

Oliver Upton (2):
  KVM: arm64: Fail the idreg iterator if idregs aren't initialized
  KVM: arm64: Don't initialize idreg debugfs w/ preemption disabled

 arch/arm64/include/asm/kvm_host.h |  1 +
 arch/arm64/kvm/arm.c              |  4 ++++
 arch/arm64/kvm/sys_regs.c         | 16 ++++++++++------
 3 files changed, 15 insertions(+), 6 deletions(-)


base-commit: bec0d120679f0033cca017e8f9e0f3782d261dc7
-- 
2.44.0.rc1.240.g4c46232300-goog

[PATCH 1/2] KVM: arm64: Fail the idreg iterator if idregs aren't initialized

[Oliver Upton]

Return an error to userspace if the VM's ID register values haven't been
initialized in preparation for changing the debugfs file initialization
order.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/sys_regs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 9a0b3a22a9d4..1ebc483c2b0c 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -3434,7 +3434,8 @@ static void *idregs_debug_start(struct seq_file *s, loff_t *pos)
 	mutex_lock(&kvm->arch.config_lock);
 
 	iter = &kvm->arch.idreg_debugfs_iter;
-	if (*iter == (u8)~0) {
+	if (test_bit(KVM_ARCH_FLAG_ID_REGS_INITIALIZED, &kvm->arch.flags) &&
+	    *iter == (u8)~0) {
 		*iter = *pos;
 		if (*iter >= KVM_ARM_ID_REG_NUM)
 			iter = NULL;
-- 
2.44.0.rc1.240.g4c46232300-goog

[PATCH 2/2] KVM: arm64: Don't initialize idreg debugfs w/ preemption disabled

[Oliver Upton]

Testing KVM with DEBUG_ATOMIC_SLEEP enabled doesn't get far before hitting the
first splat:

  BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1578
  in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 13062, name: vgic_lpi_stress
  preempt_count: 1, expected: 0
  2 locks held by vgic_lpi_stress/13062:
   #0: ffff080084553240 (&vcpu->mutex){+.+.}-{3:3}, at: kvm_vcpu_ioctl+0xc0/0x13f0
   #1: ffff800080485f08 (&kvm->arch.config_lock){+.+.}-{3:3}, at: kvm_arch_vcpu_ioctl+0xd60/0x1788
  CPU: 19 PID: 13062 Comm: vgic_lpi_stress Tainted: G        W  O       6.8.0-dbg-DEV #1
  Call trace:
   dump_backtrace+0xf8/0x148
   show_stack+0x20/0x38
   dump_stack_lvl+0xb4/0xf8
   dump_stack+0x18/0x40
   __might_resched+0x248/0x2a0
   __might_sleep+0x50/0x88
   down_write+0x30/0x150
   start_creating+0x90/0x1a0
   __debugfs_create_file+0x5c/0x1b0
   debugfs_create_file+0x34/0x48
   kvm_reset_sys_regs+0x120/0x1e8
   kvm_reset_vcpu+0x148/0x270
   kvm_arch_vcpu_ioctl+0xddc/0x1788
   kvm_vcpu_ioctl+0xb6c/0x13f0
   __arm64_sys_ioctl+0x98/0xd8
   invoke_syscall+0x48/0x108
   el0_svc_common+0xb4/0xf0
   do_el0_svc+0x24/0x38
   el0_svc+0x54/0x128
   el0t_64_sync_handler+0x68/0xc0
   el0t_64_sync+0x1a8/0x1b0

kvm_reset_vcpu() disables preemption as it needs to unload vCPU state
from the CPU to twiddle with it, which subsequently explodes when
taking the parent inode's rwsem while creating the idreg debugfs file.

Fix it by moving the initialization to kvm_arch_create_vm_debugfs().

Fixes: 891766581dea ("KVM: arm64: Add debugfs file for guest's ID registers")
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_host.h |  1 +
 arch/arm64/kvm/arm.c              |  4 ++++
 arch/arm64/kvm/sys_regs.c         | 13 ++++++++-----
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 181fef12e8e8..6883963bbc3a 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -1102,6 +1102,7 @@ int kvm_handle_cp15_64(struct kvm_vcpu *vcpu);
 int kvm_handle_sys_reg(struct kvm_vcpu *vcpu);
 int kvm_handle_cp10_id(struct kvm_vcpu *vcpu);
 
+void kvm_sys_regs_create_debugfs(struct kvm *kvm);
 void kvm_reset_sys_regs(struct kvm_vcpu *vcpu);
 
 int __init kvm_sys_reg_table_init(void);
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 0df8507f66e3..3dee5490eea9 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -190,6 +190,10 @@ vm_fault_t kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf)
 	return VM_FAULT_SIGBUS;
 }
 
+void kvm_arch_create_vm_debugfs(struct kvm *kvm)
+{
+	kvm_sys_regs_create_debugfs(kvm);
+}
 
 /**
  * kvm_arch_destroy_vm - destroy the VM data structure
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 1ebc483c2b0c..8e60aa4a8dfb 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -3502,6 +3502,14 @@ static const struct seq_operations idregs_debug_sops = {
 
 DEFINE_SEQ_ATTRIBUTE(idregs_debug);
 
+void kvm_sys_regs_create_debugfs(struct kvm *kvm)
+{
+	kvm->arch.idreg_debugfs_iter = ~0;
+
+	debugfs_create_file("idregs", 0444, kvm->debugfs_dentry, kvm,
+			    &idregs_debug_fops);
+}
+
 static void kvm_reset_id_regs(struct kvm_vcpu *vcpu)
 {
 	const struct sys_reg_desc *idreg = first_idreg;
@@ -3521,11 +3529,6 @@ static void kvm_reset_id_regs(struct kvm_vcpu *vcpu)
 		id = reg_to_encoding(idreg);
 	}
 
-	kvm->arch.idreg_debugfs_iter = ~0;
-
-	debugfs_create_file("idregs", 0444, kvm->debugfs_dentry, kvm,
-			    &idregs_debug_fops);
-
 	set_bit(KVM_ARCH_FLAG_ID_REGS_INITIALIZED, &kvm->arch.flags);
 }
 
-- 
2.44.0.rc1.240.g4c46232300-goog

Re: [PATCH 0/2] KVM: arm64: Fix splat relating to idregs debugfs

[Marc Zyngier]

On Tue, 27 Feb 2024 09:41:13 +0000,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> Looks like we're calling in to debugfs w/ preemption disabled, which
> explodes when trying to take the parent inode's rwsem.
> 
> This is easy enough to work around, and we can just initialize at the
> time of VM creation. No idreg values yet? Too bad, just kick an error
> back at the user. All two of them.
> 
> Applies to kvmarm/next.
> 
> Oliver Upton (2):
>   KVM: arm64: Fail the idreg iterator if idregs aren't initialized
>   KVM: arm64: Don't initialize idreg debugfs w/ preemption disabled

Thanks for tackling this!

Reviewed-by: Marc Zyngier <maz@kernel.org>

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 0/2] KVM: arm64: Fix splat relating to idregs debugfs

[Oliver Upton]

On Tue, 27 Feb 2024 09:41:13 +0000, Oliver Upton wrote:
> Looks like we're calling in to debugfs w/ preemption disabled, which
> explodes when trying to take the parent inode's rwsem.
> 
> This is easy enough to work around, and we can just initialize at the
> time of VM creation. No idreg values yet? Too bad, just kick an error
> back at the user. All two of them.
> 
> [...]

Applied to kvmarm/next, thanks!

[1/2] KVM: arm64: Fail the idreg iterator if idregs aren't initialized
      https://git.kernel.org/kvmarm/kvmarm/c/29ef55cec33d
[2/2] KVM: arm64: Don't initialize idreg debugfs w/ preemption disabled
      https://git.kernel.org/kvmarm/kvmarm/c/5c1ebe9ada19

--
Best,
Oliver