Re: [PATCH] arm64/kvm: correct the error report in kvm_handle_guest_abort

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2021-01-28 03:01, Jianyong Wu wrote:
Hi Marc,


 From 8f2a919d6f13d36445974794c76821fbb6b40f88 Mon Sep 17 00:00:00
2001
 From: Marc Zyngier <maz@xxxxxxxxxx>
Date: Sat, 16 Jan 2021 10:53:21 +0000
Subject: [PATCH] CMO on RO memslot

Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
---
  arch/arm64/kvm/mmu.c | 51 +++++++++++++++++++++++++++++++++----
-------
  1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index
7d2257cc5438..3c176b5b0a28 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -760,7 +760,15 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
phys_addr_t fault_ipa,
  	struct kvm_pgtable *pgt;

  	fault_granule = 1UL <<
ARM64_HW_PGTABLE_LEVEL_SHIFT(fault_level);
-	write_fault = kvm_is_write_fault(vcpu);
+	/*
+	 * Treat translation faults on CMOs as read faults. Should
+	 * this further generate a permission fault, it will be caught
+	 * in kvm_handle_guest_abort(), with prejudice...
+	 */
+	if (fault_status == FSC_FAULT && kvm_vcpu_dabt_is_cm(vcpu))
+		write_fault = false;
+	else
+		write_fault = kvm_is_write_fault(vcpu);
  	exec_fault = kvm_vcpu_trap_is_exec_fault(vcpu);
  	VM_BUG_ON(write_fault && exec_fault);

@@ -1013,19 +1021,37 @@ int kvm_handle_guest_abort(struct kvm_vcpu
*vcpu)
  		}

  		/*
-		 * Check for a cache maintenance operation. Since we
-		 * ended-up here, we know it is outside of any memory
-		 * slot. But we can't find out if that is for a device,
-		 * or if the guest is just being stupid. The only thing
-		 * we know for sure is that this range cannot be cached.
+		 * Check for a cache maintenance operation. Two cases:
+		 *
+		 * - It is outside of any memory slot. But we can't find out
+		 *   if that is for a device, or if the guest is just being
+		 *   stupid. The only thing we know for sure is that this
+		 *   range cannot be cached.  So let's assume that the guest
+		 *   is just being cautious, and skip the instruction.
+		 *
+		 * - Otherwise, check whether this is a permission fault.
+		 *   If so, that's a DC IVAC on a R/O memslot, which is a
+		 *   pretty bad idea, and we tell the guest so.
  		 *
-		 * So let's assume that the guest is just being
-		 * cautious, and skip the instruction.
+		 * - If this wasn't a permission fault, pass it along for
+                 *   further handling (including faulting the page in
if it
+                 *   was a translation fault).
  		 */
-		if (kvm_is_error_hva(hva) && kvm_vcpu_dabt_is_cm(vcpu))
{
-			kvm_incr_pc(vcpu);
-			ret = 1;
-			goto out_unlock;
+		if (kvm_vcpu_dabt_is_cm(vcpu)) {
+			if (kvm_is_error_hva(hva)) {
+				kvm_incr_pc(vcpu);
+				ret = 1;
+				goto out_unlock;
+			}
+
+			if (fault_status == FSC_PERM) {
+				/* DC IVAC on a R/O memslot */
+				kvm_inject_dabt(vcpu,
kvm_vcpu_get_hfar(vcpu));

One question:
In general, the "DC" ops show up very early in guest. So what if the
guest do this before interrupt initialization? If so, the guest may
stuck here.

I don't understand your question. Do you mean "what if the guest
does this without being able to handle an exception"?

If that's your question, then the answer is "don't do that".
The architecture is clear that DC IVAC needs write permission, and
will result in an abort being delivered if there is no writable
mapping (and there can't be, the memslot is R/O).

DC CIVAC doesn't have that requirement, and will not generate an
exception.

Thanks,

        M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm



[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux