kvmarm.lists.cs.columbia.edu archive mirror
 help / color / mirror / Atom feed
* [GIT PULL] KVM/arm updates for 5.5-rc2
@ 2019-12-12 17:28 Marc Zyngier
  2019-12-12 17:28 ` [PATCH 1/8] KVM: arm/arm64: Get rid of unused arg in cpu_init_hyp_mode() Marc Zyngier
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Marc Zyngier @ 2019-12-12 17:28 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Jia He, kvm, Ard Biesheuvel, Will Deacon, Steven Price, kvmarm,
	linux-arm-kernel

Paolo, Radim,

This is the first set of fixes for 5.5-rc2. This time around,
a couple of MM fixes, a ONE_REG fix for an issue detected by
GCC-10, and a handful of cleanups.

Please pull,

	M.

The following changes since commit cd7056ae34af0e9424da97bbc7d2b38246ba8a2c:

  Merge remote-tracking branch 'kvmarm/misc-5.5' into kvmarm/next (2019-11-08 11:27:29 +0000)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git tags/kvmarm-fixes-5.5-1

for you to fetch changes up to 6d674e28f642e3ff676fbae2d8d1b872814d32b6:

  KVM: arm/arm64: Properly handle faulting of device mappings (2019-12-12 16:22:40 +0000)

----------------------------------------------------------------
KVM/arm fixes for .5.5, take #1

- Fix uninitialised sysreg accessor
- Fix handling of demand-paged device mappings
- Stop spamming the console on IMPDEF sysregs
- Relax mappings of writable memslots
- Assorted cleanups

----------------------------------------------------------------
Jia He (1):
      KVM: arm/arm64: Remove excessive permission check in kvm_arch_prepare_memory_region

Marc Zyngier (1):
      KVM: arm/arm64: Properly handle faulting of device mappings

Mark Rutland (2):
      KVM: arm64: Sanely ratelimit sysreg messages
      KVM: arm64: Don't log IMP DEF sysreg traps

Miaohe Lin (3):
      KVM: arm/arm64: Get rid of unused arg in cpu_init_hyp_mode()
      KVM: arm/arm64: vgic: Fix potential double free dist->spis in __kvm_vgic_destroy()
      KVM: arm/arm64: vgic: Use wrapper function to lock/unlock all vcpus in kvm_vgic_create()

Will Deacon (1):
      KVM: arm64: Ensure 'params' is initialised when looking up sys register

 arch/arm64/kvm/sys_regs.c     | 25 ++++++++++++++++++-------
 arch/arm64/kvm/sys_regs.h     | 17 +++++++++++++++--
 virt/kvm/arm/arm.c            |  4 ++--
 virt/kvm/arm/mmu.c            | 30 +++++++++++++++++-------------
 virt/kvm/arm/vgic/vgic-init.c | 20 +++++---------------
 5 files changed, 57 insertions(+), 39 deletions(-)
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/8] KVM: arm/arm64: Get rid of unused arg in cpu_init_hyp_mode()
  2019-12-12 17:28 [GIT PULL] KVM/arm updates for 5.5-rc2 Marc Zyngier
@ 2019-12-12 17:28 ` Marc Zyngier
  2019-12-12 17:28 ` [PATCH 2/8] KVM: arm/arm64: vgic: Fix potential double free dist->spis in __kvm_vgic_destroy() Marc Zyngier
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2019-12-12 17:28 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Jia He, kvm, Ard Biesheuvel, Will Deacon, Steven Price, kvmarm,
	linux-arm-kernel

From: Miaohe Lin <linmiaohe@huawei.com>

As arg dummy is not really needed, there's no need to pass
NULL when calling cpu_init_hyp_mode(). So clean it up.

Fixes: 67f691976662 ("arm64: kvm: allows kvm cpu hotplug")
Reviewed-by: Steven Price <steven.price@arm.com>
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/1574320559-5662-1-git-send-email-linmiaohe@huawei.com
---
 virt/kvm/arm/arm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 12e0280291ce..8de4daf25097 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -1352,7 +1352,7 @@ long kvm_arch_vm_ioctl(struct file *filp,
 	}
 }
 
-static void cpu_init_hyp_mode(void *dummy)
+static void cpu_init_hyp_mode(void)
 {
 	phys_addr_t pgd_ptr;
 	unsigned long hyp_stack_ptr;
@@ -1386,7 +1386,7 @@ static void cpu_hyp_reinit(void)
 	if (is_kernel_in_hyp_mode())
 		kvm_timer_init_vhe();
 	else
-		cpu_init_hyp_mode(NULL);
+		cpu_init_hyp_mode();
 
 	kvm_arm_init_debug();
 
-- 
2.20.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/8] KVM: arm/arm64: vgic: Fix potential double free dist->spis in __kvm_vgic_destroy()
  2019-12-12 17:28 [GIT PULL] KVM/arm updates for 5.5-rc2 Marc Zyngier
  2019-12-12 17:28 ` [PATCH 1/8] KVM: arm/arm64: Get rid of unused arg in cpu_init_hyp_mode() Marc Zyngier
@ 2019-12-12 17:28 ` Marc Zyngier
  2019-12-12 17:28 ` [PATCH 3/8] KVM: arm/arm64: vgic: Use wrapper function to lock/unlock all vcpus in kvm_vgic_create() Marc Zyngier
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2019-12-12 17:28 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Jia He, kvm, Ard Biesheuvel, Will Deacon, Steven Price, kvmarm,
	linux-arm-kernel

From: Miaohe Lin <linmiaohe@huawei.com>

In kvm_vgic_dist_init() called from kvm_vgic_map_resources(), if
dist->vgic_model is invalid, dist->spis will be freed without set
dist->spis = NULL. And in vgicv2 resources clean up path,
__kvm_vgic_destroy() will be called to free allocated resources.
And dist->spis will be freed again in clean up chain because we
forget to set dist->spis = NULL in kvm_vgic_dist_init() failed
path. So double free would happen.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Link: https://lore.kernel.org/r/1574923128-19956-1-git-send-email-linmiaohe@huawei.com
---
 virt/kvm/arm/vgic/vgic-init.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index b3c5de48064c..7c58112ae67c 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -177,6 +177,7 @@ static int kvm_vgic_dist_init(struct kvm *kvm, unsigned int nr_spis)
 			break;
 		default:
 			kfree(dist->spis);
+			dist->spis = NULL;
 			return -EINVAL;
 		}
 	}
-- 
2.20.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/8] KVM: arm/arm64: vgic: Use wrapper function to lock/unlock all vcpus in kvm_vgic_create()
  2019-12-12 17:28 [GIT PULL] KVM/arm updates for 5.5-rc2 Marc Zyngier
  2019-12-12 17:28 ` [PATCH 1/8] KVM: arm/arm64: Get rid of unused arg in cpu_init_hyp_mode() Marc Zyngier
  2019-12-12 17:28 ` [PATCH 2/8] KVM: arm/arm64: vgic: Fix potential double free dist->spis in __kvm_vgic_destroy() Marc Zyngier
@ 2019-12-12 17:28 ` Marc Zyngier
  2019-12-12 17:28 ` [PATCH 4/8] KVM: arm64: Sanely ratelimit sysreg messages Marc Zyngier
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2019-12-12 17:28 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Jia He, kvm, Ard Biesheuvel, Will Deacon, Steven Price, kvmarm,
	linux-arm-kernel

From: Miaohe Lin <linmiaohe@huawei.com>

Use wrapper function lock_all_vcpus()/unlock_all_vcpus()
in kvm_vgic_create() to remove duplicated code dealing
with locking and unlocking all vcpus in a vm.

Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Link: https://lore.kernel.org/r/1575081918-11401-1-git-send-email-linmiaohe@huawei.com
---
 virt/kvm/arm/vgic/vgic-init.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index 7c58112ae67c..a963b9d766b7 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -70,7 +70,7 @@ void kvm_vgic_early_init(struct kvm *kvm)
  */
 int kvm_vgic_create(struct kvm *kvm, u32 type)
 {
-	int i, vcpu_lock_idx = -1, ret;
+	int i, ret;
 	struct kvm_vcpu *vcpu;
 
 	if (irqchip_in_kernel(kvm))
@@ -86,17 +86,9 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
 		!kvm_vgic_global_state.can_emulate_gicv2)
 		return -ENODEV;
 
-	/*
-	 * Any time a vcpu is run, vcpu_load is called which tries to grab the
-	 * vcpu->mutex.  By grabbing the vcpu->mutex of all VCPUs we ensure
-	 * that no other VCPUs are run while we create the vgic.
-	 */
 	ret = -EBUSY;
-	kvm_for_each_vcpu(i, vcpu, kvm) {
-		if (!mutex_trylock(&vcpu->mutex))
-			goto out_unlock;
-		vcpu_lock_idx = i;
-	}
+	if (!lock_all_vcpus(kvm))
+		return ret;
 
 	kvm_for_each_vcpu(i, vcpu, kvm) {
 		if (vcpu->arch.has_run_once)
@@ -125,10 +117,7 @@ int kvm_vgic_create(struct kvm *kvm, u32 type)
 		INIT_LIST_HEAD(&kvm->arch.vgic.rd_regions);
 
 out_unlock:
-	for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) {
-		vcpu = kvm_get_vcpu(kvm, vcpu_lock_idx);
-		mutex_unlock(&vcpu->mutex);
-	}
+	unlock_all_vcpus(kvm);
 	return ret;
 }
 
-- 
2.20.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/8] KVM: arm64: Sanely ratelimit sysreg messages
  2019-12-12 17:28 [GIT PULL] KVM/arm updates for 5.5-rc2 Marc Zyngier
                   ` (2 preceding siblings ...)
  2019-12-12 17:28 ` [PATCH 3/8] KVM: arm/arm64: vgic: Use wrapper function to lock/unlock all vcpus in kvm_vgic_create() Marc Zyngier
@ 2019-12-12 17:28 ` Marc Zyngier
  2019-12-12 17:28 ` [PATCH 5/8] KVM: arm64: Don't log IMP DEF sysreg traps Marc Zyngier
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2019-12-12 17:28 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Jia He, kvm, Ard Biesheuvel, Will Deacon, Steven Price, kvmarm,
	linux-arm-kernel

From: Mark Rutland <mark.rutland@arm.com>

Currently kvm_pr_unimpl() is ratelimited, so print_sys_reg_instr() won't
spam the console. However, someof its callers try to print some
contextual information with kvm_err(), which is not ratelimited. This
means that in some cases the context may be printed without the sysreg
encoding, which isn't all that useful.

Let's ensure that both are consistently printed together and
ratelimited, by refactoring print_sys_reg_instr() so that some callers
can provide it with an arbitrary format string.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20191205180652.18671-2-mark.rutland@arm.com
---
 arch/arm64/kvm/sys_regs.c | 12 ++++++------
 arch/arm64/kvm/sys_regs.h | 17 +++++++++++++++--
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 2071260a275b..e8bf08e09f78 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2094,9 +2094,9 @@ static void unhandled_cp_access(struct kvm_vcpu *vcpu,
 		WARN_ON(1);
 	}
 
-	kvm_err("Unsupported guest CP%d access at: %08lx [%08lx]\n",
-		cp, *vcpu_pc(vcpu), *vcpu_cpsr(vcpu));
-	print_sys_reg_instr(params);
+	print_sys_reg_msg(params,
+			  "Unsupported guest CP%d access at: %08lx [%08lx]\n",
+			  cp, *vcpu_pc(vcpu), *vcpu_cpsr(vcpu));
 	kvm_inject_undefined(vcpu);
 }
 
@@ -2245,9 +2245,9 @@ static int emulate_sys_reg(struct kvm_vcpu *vcpu,
 	if (likely(r)) {
 		perform_access(vcpu, params, r);
 	} else {
-		kvm_err("Unsupported guest sys_reg access at: %lx [%08lx]\n",
-			*vcpu_pc(vcpu), *vcpu_cpsr(vcpu));
-		print_sys_reg_instr(params);
+		print_sys_reg_msg(params,
+				  "Unsupported guest sys_reg access at: %lx [%08lx]\n",
+				  *vcpu_pc(vcpu), *vcpu_cpsr(vcpu));
 		kvm_inject_undefined(vcpu);
 	}
 	return 1;
diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
index 9bca0312d798..5a6fc30f5989 100644
--- a/arch/arm64/kvm/sys_regs.h
+++ b/arch/arm64/kvm/sys_regs.h
@@ -62,11 +62,24 @@ struct sys_reg_desc {
 #define REG_HIDDEN_USER		(1 << 0) /* hidden from userspace ioctls */
 #define REG_HIDDEN_GUEST	(1 << 1) /* hidden from guest */
 
-static inline void print_sys_reg_instr(const struct sys_reg_params *p)
+static __printf(2, 3)
+inline void print_sys_reg_msg(const struct sys_reg_params *p,
+				       char *fmt, ...)
 {
+	va_list va;
+
+	va_start(va, fmt);
 	/* Look, we even formatted it for you to paste into the table! */
-	kvm_pr_unimpl(" { Op0(%2u), Op1(%2u), CRn(%2u), CRm(%2u), Op2(%2u), func_%s },\n",
+	kvm_pr_unimpl("%pV { Op0(%2u), Op1(%2u), CRn(%2u), CRm(%2u), Op2(%2u), func_%s },\n",
+		      &(struct va_format){ fmt, &va },
 		      p->Op0, p->Op1, p->CRn, p->CRm, p->Op2, p->is_write ? "write" : "read");
+	va_end(va);
+}
+
+static inline void print_sys_reg_instr(const struct sys_reg_params *p)
+{
+	/* GCC warns on an empty format string */
+	print_sys_reg_msg(p, "%s", "");
 }
 
 static inline bool ignore_write(struct kvm_vcpu *vcpu,
-- 
2.20.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/8] KVM: arm64: Don't log IMP DEF sysreg traps
  2019-12-12 17:28 [GIT PULL] KVM/arm updates for 5.5-rc2 Marc Zyngier
                   ` (3 preceding siblings ...)
  2019-12-12 17:28 ` [PATCH 4/8] KVM: arm64: Sanely ratelimit sysreg messages Marc Zyngier
@ 2019-12-12 17:28 ` Marc Zyngier
  2019-12-12 17:28 ` [PATCH 6/8] KVM: arm/arm64: Remove excessive permission check in kvm_arch_prepare_memory_region Marc Zyngier
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2019-12-12 17:28 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Jia He, kvm, Ard Biesheuvel, Will Deacon, Steven Price, kvmarm,
	linux-arm-kernel

From: Mark Rutland <mark.rutland@arm.com>

We don't intend to support IMPLEMENATION DEFINED system registers, but
have to trap them (and emulate them as UNDEFINED). These traps aren't
interesting to the system administrator or to the KVM developers, so
let's not bother logging when we do so.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20191205180652.18671-3-mark.rutland@arm.com
---
 arch/arm64/kvm/sys_regs.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index e8bf08e09f78..bd2ac3796d8d 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2229,6 +2229,12 @@ int kvm_handle_cp14_32(struct kvm_vcpu *vcpu, struct kvm_run *run)
 				NULL, 0);
 }
 
+static bool is_imp_def_sys_reg(struct sys_reg_params *params)
+{
+	// See ARM DDI 0487E.a, section D12.3.2
+	return params->Op0 == 3 && (params->CRn & 0b1011) == 0b1011;
+}
+
 static int emulate_sys_reg(struct kvm_vcpu *vcpu,
 			   struct sys_reg_params *params)
 {
@@ -2244,6 +2250,8 @@ static int emulate_sys_reg(struct kvm_vcpu *vcpu,
 
 	if (likely(r)) {
 		perform_access(vcpu, params, r);
+	} else if (is_imp_def_sys_reg(params)) {
+		kvm_inject_undefined(vcpu);
 	} else {
 		print_sys_reg_msg(params,
 				  "Unsupported guest sys_reg access at: %lx [%08lx]\n",
-- 
2.20.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/8] KVM: arm/arm64: Remove excessive permission check in kvm_arch_prepare_memory_region
  2019-12-12 17:28 [GIT PULL] KVM/arm updates for 5.5-rc2 Marc Zyngier
                   ` (4 preceding siblings ...)
  2019-12-12 17:28 ` [PATCH 5/8] KVM: arm64: Don't log IMP DEF sysreg traps Marc Zyngier
@ 2019-12-12 17:28 ` Marc Zyngier
  2019-12-12 17:28 ` [PATCH 7/8] KVM: arm64: Ensure 'params' is initialised when looking up sys register Marc Zyngier
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2019-12-12 17:28 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Jia He, kvm, Ard Biesheuvel, Will Deacon, Steven Price, kvmarm,
	linux-arm-kernel

From: Jia He <justin.he@arm.com>

In kvm_arch_prepare_memory_region, arm kvm regards the memory region as
writable if the flag has no KVM_MEM_READONLY, and the vm is readonly if
!VM_WRITE.

But there is common usage for setting kvm memory region as follows:
e.g. qemu side (see the PROT_NONE flag)
1. mmap(NULL, size, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
   memory_region_init_ram_ptr()
2. re mmap the above area with read/write authority.

Such example is used in virtio-fs qemu codes which hasn't been upstreamed
[1]. But seems we can't forbid this example.

Without this patch, it will cause an EPERM during kvm_set_memory_region()
and cause qemu boot crash.

As told by Ard, "the underlying assumption is incorrect, i.e., that the
value of vm_flags at this point in time defines how the VMA is used
during its lifetime. There may be other cases where a VMA is created
with VM_READ vm_flags that are changed to VM_READ|VM_WRITE later, and
we are currently rejecting this use case as well."

[1] https://gitlab.com/virtio-fs/qemu/blob/5a356e/hw/virtio/vhost-user-fs.c#L488

Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Jia He <justin.he@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
Link: https://lore.kernel.org/r/20191206020802.196108-1-justin.he@arm.com
---
 virt/kvm/arm/mmu.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index 38b4c910b6c3..a48994af70b8 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -2301,15 +2301,6 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
 		if (!vma || vma->vm_start >= reg_end)
 			break;
 
-		/*
-		 * Mapping a read-only VMA is only allowed if the
-		 * memory region is configured as read-only.
-		 */
-		if (writable && !(vma->vm_flags & VM_WRITE)) {
-			ret = -EPERM;
-			break;
-		}
-
 		/*
 		 * Take the intersection of this VMA with the memory region
 		 */
-- 
2.20.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/8] KVM: arm64: Ensure 'params' is initialised when looking up sys register
  2019-12-12 17:28 [GIT PULL] KVM/arm updates for 5.5-rc2 Marc Zyngier
                   ` (5 preceding siblings ...)
  2019-12-12 17:28 ` [PATCH 6/8] KVM: arm/arm64: Remove excessive permission check in kvm_arch_prepare_memory_region Marc Zyngier
@ 2019-12-12 17:28 ` Marc Zyngier
  2019-12-12 17:28 ` [PATCH 8/8] KVM: arm/arm64: Properly handle faulting of device mappings Marc Zyngier
  2019-12-18 16:48 ` [GIT PULL] KVM/arm updates for 5.5-rc2 Paolo Bonzini
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2019-12-12 17:28 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Jia He, kvm, Ard Biesheuvel, Will Deacon, stable, Steven Price,
	kvmarm, linux-arm-kernel

From: Will Deacon <will@kernel.org>

Commit 4b927b94d5df ("KVM: arm/arm64: vgic: Introduce find_reg_by_id()")
introduced 'find_reg_by_id()', which looks up a system register only if
the 'id' index parameter identifies a valid system register. As part of
the patch, existing callers of 'find_reg()' were ported over to the new
interface, but this breaks 'index_to_sys_reg_desc()' in the case that the
initial lookup in the vCPU target table fails because we will then call
into 'find_reg()' for the system register table with an uninitialised
'param' as the key to the lookup.

GCC 10 is bright enough to spot this (amongst a tonne of false positives,
but hey!):

  | arch/arm64/kvm/sys_regs.c: In function ‘index_to_sys_reg_desc.part.0.isra’:
  | arch/arm64/kvm/sys_regs.c:983:33: warning: ‘params.Op2’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  |   983 |   (u32)(x)->CRn, (u32)(x)->CRm, (u32)(x)->Op2);
  | [...]

Revert the hunk of 4b927b94d5df which breaks 'index_to_sys_reg_desc()' so
that the old behaviour of checking the index upfront is restored.

Fixes: 4b927b94d5df ("KVM: arm/arm64: vgic: Introduce find_reg_by_id()")
Signed-off-by: Will Deacon <will@kernel.org>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20191212094049.12437-1-will@kernel.org
---
 arch/arm64/kvm/sys_regs.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index bd2ac3796d8d..d78b726d4722 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2364,8 +2364,11 @@ static const struct sys_reg_desc *index_to_sys_reg_desc(struct kvm_vcpu *vcpu,
 	if ((id & KVM_REG_ARM_COPROC_MASK) != KVM_REG_ARM64_SYSREG)
 		return NULL;
 
+	if (!index_to_params(id, &params))
+		return NULL;
+
 	table = get_target_table(vcpu->arch.target, true, &num);
-	r = find_reg_by_id(id, &params, table, num);
+	r = find_reg(&params, table, num);
 	if (!r)
 		r = find_reg(&params, sys_reg_descs, ARRAY_SIZE(sys_reg_descs));
 
-- 
2.20.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 8/8] KVM: arm/arm64: Properly handle faulting of device mappings
  2019-12-12 17:28 [GIT PULL] KVM/arm updates for 5.5-rc2 Marc Zyngier
                   ` (6 preceding siblings ...)
  2019-12-12 17:28 ` [PATCH 7/8] KVM: arm64: Ensure 'params' is initialised when looking up sys register Marc Zyngier
@ 2019-12-12 17:28 ` Marc Zyngier
  2019-12-18 16:48 ` [GIT PULL] KVM/arm updates for 5.5-rc2 Paolo Bonzini
  8 siblings, 0 replies; 10+ messages in thread
From: Marc Zyngier @ 2019-12-12 17:28 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Jia He, kvm, Ard Biesheuvel, Will Deacon, stable, Steven Price,
	kvmarm, linux-arm-kernel

A device mapping is normally always mapped at Stage-2, since there
is very little gain in having it faulted in.

Nonetheless, it is possible to end-up in a situation where the device
mapping has been removed from Stage-2 (userspace munmaped the VFIO
region, and the MMU notifier did its job), but present in a userspace
mapping (userpace has mapped it back at the same address). In such
a situation, the device mapping will be demand-paged as the guest
performs memory accesses.

This requires to be careful when dealing with mapping size, cache
management, and to handle potential execution of a device mapping.

Reported-by: Alexandru Elisei <alexandru.elisei@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Tested-by: Alexandru Elisei <alexandru.elisei@arm.com>
Reviewed-by: James Morse <james.morse@arm.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20191211165651.7889-2-maz@kernel.org
---
 virt/kvm/arm/mmu.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
index a48994af70b8..0b32a904a1bb 100644
--- a/virt/kvm/arm/mmu.c
+++ b/virt/kvm/arm/mmu.c
@@ -38,6 +38,11 @@ static unsigned long io_map_base;
 #define KVM_S2PTE_FLAG_IS_IOMAP		(1UL << 0)
 #define KVM_S2_FLAG_LOGGING_ACTIVE	(1UL << 1)
 
+static bool is_iomap(unsigned long flags)
+{
+	return flags & KVM_S2PTE_FLAG_IS_IOMAP;
+}
+
 static bool memslot_is_logging(struct kvm_memory_slot *memslot)
 {
 	return memslot->dirty_bitmap && !(memslot->flags & KVM_MEM_READONLY);
@@ -1698,6 +1703,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 
 	vma_pagesize = vma_kernel_pagesize(vma);
 	if (logging_active ||
+	    (vma->vm_flags & VM_PFNMAP) ||
 	    !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
 		force_pte = true;
 		vma_pagesize = PAGE_SIZE;
@@ -1760,6 +1766,9 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 			writable = false;
 	}
 
+	if (exec_fault && is_iomap(flags))
+		return -ENOEXEC;
+
 	spin_lock(&kvm->mmu_lock);
 	if (mmu_notifier_retry(kvm, mmu_seq))
 		goto out_unlock;
@@ -1781,7 +1790,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa,
 	if (writable)
 		kvm_set_pfn_dirty(pfn);
 
-	if (fault_status != FSC_PERM)
+	if (fault_status != FSC_PERM && !is_iomap(flags))
 		clean_dcache_guest_page(pfn, vma_pagesize);
 
 	if (exec_fault)
@@ -1948,9 +1957,8 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run)
 	if (kvm_is_error_hva(hva) || (write_fault && !writable)) {
 		if (is_iabt) {
 			/* Prefetch Abort on I/O address */
-			kvm_inject_pabt(vcpu, kvm_vcpu_get_hfar(vcpu));
-			ret = 1;
-			goto out_unlock;
+			ret = -ENOEXEC;
+			goto out;
 		}
 
 		/*
@@ -1992,6 +2000,11 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, struct kvm_run *run)
 	ret = user_mem_abort(vcpu, fault_ipa, memslot, hva, fault_status);
 	if (ret == 0)
 		ret = 1;
+out:
+	if (ret == -ENOEXEC) {
+		kvm_inject_pabt(vcpu, kvm_vcpu_get_hfar(vcpu));
+		ret = 1;
+	}
 out_unlock:
 	srcu_read_unlock(&vcpu->kvm->srcu, idx);
 	return ret;
-- 
2.20.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] KVM/arm updates for 5.5-rc2
  2019-12-12 17:28 [GIT PULL] KVM/arm updates for 5.5-rc2 Marc Zyngier
                   ` (7 preceding siblings ...)
  2019-12-12 17:28 ` [PATCH 8/8] KVM: arm/arm64: Properly handle faulting of device mappings Marc Zyngier
@ 2019-12-18 16:48 ` Paolo Bonzini
  8 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2019-12-18 16:48 UTC (permalink / raw)
  To: Marc Zyngier, Radim Krčmář
  Cc: Jia He, kvm, Ard Biesheuvel, Will Deacon, Steven Price, kvmarm,
	linux-arm-kernel

On 12/12/19 18:28, Marc Zyngier wrote:
> Paolo, Radim,
> 
> This is the first set of fixes for 5.5-rc2. This time around,
> a couple of MM fixes, a ONE_REG fix for an issue detected by
> GCC-10, and a handful of cleanups.
> 
> Please pull,
> 
> 	M.
> 
> The following changes since commit cd7056ae34af0e9424da97bbc7d2b38246ba8a2c:
> 
>   Merge remote-tracking branch 'kvmarm/misc-5.5' into kvmarm/next (2019-11-08 11:27:29 +0000)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git tags/kvmarm-fixes-5.5-1
> 
> for you to fetch changes up to 6d674e28f642e3ff676fbae2d8d1b872814d32b6:
> 
>   KVM: arm/arm64: Properly handle faulting of device mappings (2019-12-12 16:22:40 +0000)

Pulled, thanks.

Paolo

> 
> ----------------------------------------------------------------
> KVM/arm fixes for .5.5, take #1
> 
> - Fix uninitialised sysreg accessor
> - Fix handling of demand-paged device mappings
> - Stop spamming the console on IMPDEF sysregs
> - Relax mappings of writable memslots
> - Assorted cleanups
> 
> ----------------------------------------------------------------
> Jia He (1):
>       KVM: arm/arm64: Remove excessive permission check in kvm_arch_prepare_memory_region
> 
> Marc Zyngier (1):
>       KVM: arm/arm64: Properly handle faulting of device mappings
> 
> Mark Rutland (2):
>       KVM: arm64: Sanely ratelimit sysreg messages
>       KVM: arm64: Don't log IMP DEF sysreg traps
> 
> Miaohe Lin (3):
>       KVM: arm/arm64: Get rid of unused arg in cpu_init_hyp_mode()
>       KVM: arm/arm64: vgic: Fix potential double free dist->spis in __kvm_vgic_destroy()
>       KVM: arm/arm64: vgic: Use wrapper function to lock/unlock all vcpus in kvm_vgic_create()
> 
> Will Deacon (1):
>       KVM: arm64: Ensure 'params' is initialised when looking up sys register
> 
>  arch/arm64/kvm/sys_regs.c     | 25 ++++++++++++++++++-------
>  arch/arm64/kvm/sys_regs.h     | 17 +++++++++++++++--
>  virt/kvm/arm/arm.c            |  4 ++--
>  virt/kvm/arm/mmu.c            | 30 +++++++++++++++++-------------
>  virt/kvm/arm/vgic/vgic-init.c | 20 +++++---------------
>  5 files changed, 57 insertions(+), 39 deletions(-)
> 

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-12-18 16:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-12 17:28 [GIT PULL] KVM/arm updates for 5.5-rc2 Marc Zyngier
2019-12-12 17:28 ` [PATCH 1/8] KVM: arm/arm64: Get rid of unused arg in cpu_init_hyp_mode() Marc Zyngier
2019-12-12 17:28 ` [PATCH 2/8] KVM: arm/arm64: vgic: Fix potential double free dist->spis in __kvm_vgic_destroy() Marc Zyngier
2019-12-12 17:28 ` [PATCH 3/8] KVM: arm/arm64: vgic: Use wrapper function to lock/unlock all vcpus in kvm_vgic_create() Marc Zyngier
2019-12-12 17:28 ` [PATCH 4/8] KVM: arm64: Sanely ratelimit sysreg messages Marc Zyngier
2019-12-12 17:28 ` [PATCH 5/8] KVM: arm64: Don't log IMP DEF sysreg traps Marc Zyngier
2019-12-12 17:28 ` [PATCH 6/8] KVM: arm/arm64: Remove excessive permission check in kvm_arch_prepare_memory_region Marc Zyngier
2019-12-12 17:28 ` [PATCH 7/8] KVM: arm64: Ensure 'params' is initialised when looking up sys register Marc Zyngier
2019-12-12 17:28 ` [PATCH 8/8] KVM: arm/arm64: Properly handle faulting of device mappings Marc Zyngier
2019-12-18 16:48 ` [GIT PULL] KVM/arm updates for 5.5-rc2 Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).