All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Improve dirty ring warning report
@ 2023-01-19 23:44 Gavin Shan
  2023-01-19 23:44 ` [PATCH v2 1/3] KVM: arm64: Add helper vgic_write_guest_lock() Gavin Shan
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Gavin Shan @ 2023-01-19 23:44 UTC (permalink / raw)
  To: kvmarm
  Cc: kvmarm, kvm, linux-kernel, linux-doc, pbonzini, corbet, maz,
	oliver.upton, will, gshan, ricarkol, eric.auger, yuzhe,
	renzhengeek, reijiw, ardb, Julia.Lawall, yuzenghui, seanjc,
	shan.gavin

It has been known case where no running VCPU context exists when the
vgic/its tables are saved. There are other two unknown cases where we
don't have the running VCPU context: (a) restore vgic3 LPI pending
status. (b) restoring vgic3 pending tables.

PATCH[1]     adds unified helper vgic_write_guest_lock()
PATCH[2 - 3] allows no-running-vcpu context for (a) and (b)

v1: https://lore.kernel.org/kvmarm/20230116040405.260935-1-gshan@redhat.com/T/#t

Changelog
=========
v2:
  * Add unified helper vgic_write_guest_lock()                 (Oliver)
  * Dropped two patches to refactor mark_page_dirty_in_slot()  (Sean)

Gavin Shan (3):
  KVM: arm64: Add helper vgic_write_guest_lock()
  KVM: arm64: Allow no running vcpu on restoring vgic3 LPI pending
    status
  KVM: arm64: Allow no running vcpu on saving vgic3 pending table

 Documentation/virt/kvm/api.rst     | 10 +++++++---
 arch/arm64/kvm/vgic-sys-reg-v3.c   |  1 +
 arch/arm64/kvm/vgic/vgic-irqfd.c   |  1 +
 arch/arm64/kvm/vgic/vgic-its.c     | 13 +++++--------
 arch/arm64/kvm/vgic/vgic-mmio-v2.c |  1 +
 arch/arm64/kvm/vgic/vgic-mmio.c    |  1 +
 arch/arm64/kvm/vgic/vgic-v3.c      |  4 ++--
 arch/arm64/kvm/vgic/vgic-v4.c      |  1 +
 arch/arm64/kvm/vgic/vgic.c         |  1 +
 arch/arm64/kvm/vgic/vgic.h         | 13 +++++++++++++
 include/kvm/arm_vgic.h             |  2 +-
 11 files changed, 34 insertions(+), 14 deletions(-)

-- 
2.23.0


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

* [PATCH v2 1/3] KVM: arm64: Add helper vgic_write_guest_lock()
  2023-01-19 23:44 [PATCH v2 0/3] Improve dirty ring warning report Gavin Shan
@ 2023-01-19 23:44 ` Gavin Shan
  2023-01-26 20:20   ` Oliver Upton
  2023-01-19 23:44 ` [PATCH v2 2/3] KVM: arm64: Allow no running vcpu on restoring vgic3 LPI pending status Gavin Shan
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Gavin Shan @ 2023-01-19 23:44 UTC (permalink / raw)
  To: kvmarm
  Cc: kvmarm, kvm, linux-kernel, linux-doc, pbonzini, corbet, maz,
	oliver.upton, will, gshan, ricarkol, eric.auger, yuzhe,
	renzhengeek, reijiw, ardb, Julia.Lawall, yuzenghui, seanjc,
	shan.gavin

Currently, the unknown no-running-vcpu sites are reported when a
dirty page is tracked by mark_page_dirty_in_slot(). Until now, the
only known no-running-vcpu site is saving vgic/its tables through
KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_SAVE_TABLES} command on KVM device
"kvm-arm-vgic-its". Unfortunately, there are more unknown sites to
be handled and no-running-vcpu context will be allowed in these
sites: (1) KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_RESTORE_TABLES} command
on KVM device "kvm-arm-vgic-its" to restore vgic/its tables. The
vgic3 LPI pending status could be restored. (2) Save vgic3 pending
table through KVM_DEV_ARM_{VGIC_GRP_CTRL, VGIC_SAVE_PENDING_TABLES}
command on KVM device "kvm-arm-vgic-v3".

In order to handle those unknown cases, we need a unified helper
vgic_write_guest_lock(). struct vgic_dist::save_its_tables_in_progress
is also renamed to struct vgic_dist::save_tables_in_progress. Besides,
"asm/kvm_mmu.h" needs to be included for "vgic.h" for the definition
of kvm_write_guest_lock().

No functional change intended.

Suggested-by: Oliver Upton <oliver.upton@linux.dev>
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 arch/arm64/kvm/vgic-sys-reg-v3.c   |  1 +
 arch/arm64/kvm/vgic/vgic-irqfd.c   |  1 +
 arch/arm64/kvm/vgic/vgic-its.c     | 13 +++++--------
 arch/arm64/kvm/vgic/vgic-mmio-v2.c |  1 +
 arch/arm64/kvm/vgic/vgic-mmio.c    |  1 +
 arch/arm64/kvm/vgic/vgic-v4.c      |  1 +
 arch/arm64/kvm/vgic/vgic.c         |  1 +
 arch/arm64/kvm/vgic/vgic.h         | 13 +++++++++++++
 include/kvm/arm_vgic.h             |  2 +-
 9 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/kvm/vgic-sys-reg-v3.c b/arch/arm64/kvm/vgic-sys-reg-v3.c
index 9e7c486b48c2..215ce2dbdb7b 100644
--- a/arch/arm64/kvm/vgic-sys-reg-v3.c
+++ b/arch/arm64/kvm/vgic-sys-reg-v3.c
@@ -7,6 +7,7 @@
 #include <linux/kvm.h>
 #include <linux/kvm_host.h>
 #include <asm/kvm_emulate.h>
+#include <asm/kvm_mmu.h>
 #include "vgic/vgic.h"
 #include "sys_regs.h"
 
diff --git a/arch/arm64/kvm/vgic/vgic-irqfd.c b/arch/arm64/kvm/vgic/vgic-irqfd.c
index 475059bacedf..f2581fe6620c 100644
--- a/arch/arm64/kvm/vgic/vgic-irqfd.c
+++ b/arch/arm64/kvm/vgic/vgic-irqfd.c
@@ -7,6 +7,7 @@
 #include <linux/kvm_host.h>
 #include <trace/events/kvm.h>
 #include <kvm/arm_vgic.h>
+#include <asm/kvm_mmu.h>
 #include "vgic.h"
 
 /**
diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c
index 94a666dd1443..2fe2922533ad 100644
--- a/arch/arm64/kvm/vgic/vgic-its.c
+++ b/arch/arm64/kvm/vgic/vgic-its.c
@@ -2187,7 +2187,7 @@ static int vgic_its_save_ite(struct vgic_its *its, struct its_device *dev,
 	       ((u64)ite->irq->intid << KVM_ITS_ITE_PINTID_SHIFT) |
 		ite->collection->collection_id;
 	val = cpu_to_le64(val);
-	return kvm_write_guest_lock(kvm, gpa, &val, ite_esz);
+	return vgic_write_guest_lock(kvm, gpa, &val, ite_esz);
 }
 
 /**
@@ -2339,7 +2339,7 @@ static int vgic_its_save_dte(struct vgic_its *its, struct its_device *dev,
 	       (itt_addr_field << KVM_ITS_DTE_ITTADDR_SHIFT) |
 		(dev->num_eventid_bits - 1));
 	val = cpu_to_le64(val);
-	return kvm_write_guest_lock(kvm, ptr, &val, dte_esz);
+	return vgic_write_guest_lock(kvm, ptr, &val, dte_esz);
 }
 
 /**
@@ -2526,7 +2526,7 @@ static int vgic_its_save_cte(struct vgic_its *its,
 	       ((u64)collection->target_addr << KVM_ITS_CTE_RDBASE_SHIFT) |
 	       collection->collection_id);
 	val = cpu_to_le64(val);
-	return kvm_write_guest_lock(its->dev->kvm, gpa, &val, esz);
+	return vgic_write_guest_lock(its->dev->kvm, gpa, &val, esz);
 }
 
 /*
@@ -2607,7 +2607,7 @@ static int vgic_its_save_collection_table(struct vgic_its *its)
 	 */
 	val = 0;
 	BUG_ON(cte_esz > sizeof(val));
-	ret = kvm_write_guest_lock(its->dev->kvm, gpa, &val, cte_esz);
+	ret = vgic_write_guest_lock(its->dev->kvm, gpa, &val, cte_esz);
 	return ret;
 }
 
@@ -2743,7 +2743,6 @@ static int vgic_its_has_attr(struct kvm_device *dev,
 static int vgic_its_ctrl(struct kvm *kvm, struct vgic_its *its, u64 attr)
 {
 	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
-	struct vgic_dist *dist = &kvm->arch.vgic;
 	int ret = 0;
 
 	if (attr == KVM_DEV_ARM_VGIC_CTRL_INIT) /* Nothing to do */
@@ -2763,9 +2762,7 @@ static int vgic_its_ctrl(struct kvm *kvm, struct vgic_its *its, u64 attr)
 		vgic_its_reset(kvm, its);
 		break;
 	case KVM_DEV_ARM_ITS_SAVE_TABLES:
-		dist->save_its_tables_in_progress = true;
 		ret = abi->save_tables(its);
-		dist->save_its_tables_in_progress = false;
 		break;
 	case KVM_DEV_ARM_ITS_RESTORE_TABLES:
 		ret = abi->restore_tables(its);
@@ -2792,7 +2789,7 @@ bool kvm_arch_allow_write_without_running_vcpu(struct kvm *kvm)
 {
 	struct vgic_dist *dist = &kvm->arch.vgic;
 
-	return dist->save_its_tables_in_progress;
+	return dist->save_tables_in_progress;
 }
 
 static int vgic_its_set_attr(struct kvm_device *dev,
diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v2.c b/arch/arm64/kvm/vgic/vgic-mmio-v2.c
index e070cda86e12..90502ce5a97f 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v2.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v2.c
@@ -10,6 +10,7 @@
 
 #include <kvm/iodev.h>
 #include <kvm/arm_vgic.h>
+#include <asm/kvm_mmu.h>
 
 #include "vgic.h"
 #include "vgic-mmio.h"
diff --git a/arch/arm64/kvm/vgic/vgic-mmio.c b/arch/arm64/kvm/vgic/vgic-mmio.c
index b32d434c1d4a..36d851ee02d8 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio.c
@@ -12,6 +12,7 @@
 #include <kvm/iodev.h>
 #include <kvm/arm_arch_timer.h>
 #include <kvm/arm_vgic.h>
+#include <asm/kvm_mmu.h>
 
 #include "vgic.h"
 #include "vgic-mmio.h"
diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c
index ad06ba6c9b00..3396ad1b0f17 100644
--- a/arch/arm64/kvm/vgic/vgic-v4.c
+++ b/arch/arm64/kvm/vgic/vgic-v4.c
@@ -9,6 +9,7 @@
 #include <linux/irqdomain.h>
 #include <linux/kvm_host.h>
 #include <linux/irqchip/arm-gic-v3.h>
+#include <asm/kvm_mmu.h>
 
 #include "vgic.h"
 
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index d97e6080b421..a17a40802af7 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -11,6 +11,7 @@
 #include <linux/nospec.h>
 
 #include <asm/kvm_hyp.h>
+#include <asm/kvm_mmu.h>
 
 #include "vgic.h"
 
diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h
index 0c8da72953f0..d16180eb1a92 100644
--- a/arch/arm64/kvm/vgic/vgic.h
+++ b/arch/arm64/kvm/vgic/vgic.h
@@ -131,6 +131,19 @@ static inline bool vgic_irq_is_multi_sgi(struct vgic_irq *irq)
 	return vgic_irq_get_lr_count(irq) > 1;
 }
 
+static inline int vgic_write_guest_lock(struct kvm *kvm, gpa_t gpa,
+					const void *data, unsigned long len)
+{
+	struct vgic_dist *dist = &kvm->arch.vgic;
+	int ret;
+
+	dist->save_tables_in_progress = true;
+	ret = kvm_write_guest_lock(kvm, gpa, data, len);
+	dist->save_tables_in_progress = false;
+
+	return ret;
+}
+
 /*
  * This struct provides an intermediate representation of the fields contained
  * in the GICH_VMCR and ICH_VMCR registers, such that code exporting the GIC
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 9270cd87da3f..a0be53bc5703 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -263,7 +263,7 @@ struct vgic_dist {
 	struct vgic_io_device	dist_iodev;
 
 	bool			has_its;
-	bool			save_its_tables_in_progress;
+	bool			save_tables_in_progress;
 
 	/*
 	 * Contains the attributes and gpa of the LPI configuration table.
-- 
2.23.0


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

* [PATCH v2 2/3] KVM: arm64: Allow no running vcpu on restoring vgic3 LPI pending status
  2023-01-19 23:44 [PATCH v2 0/3] Improve dirty ring warning report Gavin Shan
  2023-01-19 23:44 ` [PATCH v2 1/3] KVM: arm64: Add helper vgic_write_guest_lock() Gavin Shan
@ 2023-01-19 23:44 ` Gavin Shan
  2023-01-19 23:44 ` [PATCH v2 3/3] KVM: arm64: Allow no running vcpu on saving vgic3 pending table Gavin Shan
  2023-01-26 20:21 ` [PATCH v2 0/3] Improve dirty ring warning report Oliver Upton
  3 siblings, 0 replies; 7+ messages in thread
From: Gavin Shan @ 2023-01-19 23:44 UTC (permalink / raw)
  To: kvmarm
  Cc: kvmarm, kvm, linux-kernel, linux-doc, pbonzini, corbet, maz,
	oliver.upton, will, gshan, ricarkol, eric.auger, yuzhe,
	renzhengeek, reijiw, ardb, Julia.Lawall, yuzenghui, seanjc,
	shan.gavin

We don't have a running VCPU context to restore vgic3 LPI pending status
due to command KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_RESTORE_TABLES} on KVM
device "kvm-arm-vgic-its".

Use vgic_write_guest_lock() to restore vgic3 LPI pending status.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 Documentation/virt/kvm/api.rst | 8 +++++---
 arch/arm64/kvm/vgic/vgic-v3.c  | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 9807b05a1b57..40ada313faa3 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -8070,9 +8070,11 @@ considering the state as complete. VMM needs to ensure that the dirty
 state is final and avoid missing dirty pages from another ioctl ordered
 after the bitmap collection.
 
-NOTE: One example of using the backup bitmap is saving arm64 vgic/its
-tables through KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_SAVE_TABLES} command on
-KVM device "kvm-arm-vgic-its" when dirty ring is enabled.
+NOTE: Multiple examples of using the backup bitmap: (1) save vgic/its
+tables through command KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_SAVE_TABLES} on
+KVM device "kvm-arm-vgic-its". (2) restore vgic/its tables through
+command KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_RESTORE_TABLES} on KVM device
+"kvm-arm-vgic-its". vgic3 LPI pending status is restored.
 
 8.30 KVM_CAP_XEN_HVM
 --------------------
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 2074521d4a8c..2e680d8a0a15 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -339,7 +339,7 @@ int vgic_v3_lpi_sync_pending_status(struct kvm *kvm, struct vgic_irq *irq)
 	if (status) {
 		/* clear consumed data */
 		val &= ~(1 << bit_nr);
-		ret = kvm_write_guest_lock(kvm, ptr, &val, 1);
+		ret = vgic_write_guest_lock(kvm, ptr, &val, 1);
 		if (ret)
 			return ret;
 	}
-- 
2.23.0


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

* [PATCH v2 3/3] KVM: arm64: Allow no running vcpu on saving vgic3 pending table
  2023-01-19 23:44 [PATCH v2 0/3] Improve dirty ring warning report Gavin Shan
  2023-01-19 23:44 ` [PATCH v2 1/3] KVM: arm64: Add helper vgic_write_guest_lock() Gavin Shan
  2023-01-19 23:44 ` [PATCH v2 2/3] KVM: arm64: Allow no running vcpu on restoring vgic3 LPI pending status Gavin Shan
@ 2023-01-19 23:44 ` Gavin Shan
  2023-01-26 20:21 ` [PATCH v2 0/3] Improve dirty ring warning report Oliver Upton
  3 siblings, 0 replies; 7+ messages in thread
From: Gavin Shan @ 2023-01-19 23:44 UTC (permalink / raw)
  To: kvmarm
  Cc: kvmarm, kvm, linux-kernel, linux-doc, pbonzini, corbet, maz,
	oliver.upton, will, gshan, ricarkol, eric.auger, yuzhe,
	renzhengeek, reijiw, ardb, Julia.Lawall, yuzenghui, seanjc,
	shan.gavin

We don't have a running VCPU context to save vgic3 pending table due
to KVM_DEV_ARM_VGIC_{GRP_CTRL, SAVE_PENDING_TABLES} command on KVM
device "kvm-arm-vgic-v3". The unknown case is caught by kvm-unit-tests.

   # ./kvm-unit-tests/tests/its-pending-migration
   WARNING: CPU: 120 PID: 7973 at arch/arm64/kvm/../../../virt/kvm/kvm_main.c:3325 \
   mark_page_dirty_in_slot+0x60/0xe0
    :
   mark_page_dirty_in_slot+0x60/0xe0
   __kvm_write_guest_page+0xcc/0x100
   kvm_write_guest+0x7c/0xb0
   vgic_v3_save_pending_tables+0x148/0x2a0
   vgic_set_common_attr+0x158/0x240
   vgic_v3_set_attr+0x4c/0x5c
   kvm_device_ioctl+0x100/0x160
   __arm64_sys_ioctl+0xa8/0xf0
   invoke_syscall.constprop.0+0x7c/0xd0
   el0_svc_common.constprop.0+0x144/0x160
   do_el0_svc+0x34/0x60
   el0_svc+0x3c/0x1a0
   el0t_64_sync_handler+0xb4/0x130
   el0t_64_sync+0x178/0x17c

Use vgic_write_guest_lock() to save vgic3 pending table.

Reported-by: Zenghui Yu <yuzenghui@huawei.com>
Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 Documentation/virt/kvm/api.rst | 4 +++-
 arch/arm64/kvm/vgic/vgic-v3.c  | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 40ada313faa3..07f07668995e 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -8074,7 +8074,9 @@ NOTE: Multiple examples of using the backup bitmap: (1) save vgic/its
 tables through command KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_SAVE_TABLES} on
 KVM device "kvm-arm-vgic-its". (2) restore vgic/its tables through
 command KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_RESTORE_TABLES} on KVM device
-"kvm-arm-vgic-its". vgic3 LPI pending status is restored.
+"kvm-arm-vgic-its". vgic3 LPI pending status is restored. (3) save
+vgic3 pending table through KVM_DEV_ARM_VGIC_{GRP_CTRL, SAVE_PENDING_TABLES}
+command on KVM device "kvm-arm-vgic-v3".
 
 8.30 KVM_CAP_XEN_HVM
 --------------------
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 2e680d8a0a15..fc2f24433076 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -437,7 +437,7 @@ int vgic_v3_save_pending_tables(struct kvm *kvm)
 		else
 			val &= ~(1 << bit_nr);
 
-		ret = kvm_write_guest_lock(kvm, ptr, &val, 1);
+		ret = vgic_write_guest_lock(kvm, ptr, &val, 1);
 		if (ret)
 			goto out;
 	}
-- 
2.23.0


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

* Re: [PATCH v2 1/3] KVM: arm64: Add helper vgic_write_guest_lock()
  2023-01-19 23:44 ` [PATCH v2 1/3] KVM: arm64: Add helper vgic_write_guest_lock() Gavin Shan
@ 2023-01-26 20:20   ` Oliver Upton
  2023-01-27  0:02     ` Gavin Shan
  0 siblings, 1 reply; 7+ messages in thread
From: Oliver Upton @ 2023-01-26 20:20 UTC (permalink / raw)
  To: Gavin Shan
  Cc: kvmarm, kvmarm, kvm, linux-kernel, linux-doc, pbonzini, corbet,
	maz, will, ricarkol, eric.auger, yuzhe, renzhengeek, reijiw, ardb,
	Julia.Lawall, yuzenghui, seanjc, shan.gavin

Hi Gavin,

On Fri, Jan 20, 2023 at 07:44:03AM +0800, Gavin Shan wrote:
> Currently, the unknown no-running-vcpu sites are reported when a
> dirty page is tracked by mark_page_dirty_in_slot(). Until now, the
> only known no-running-vcpu site is saving vgic/its tables through
> KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_SAVE_TABLES} command on KVM device
> "kvm-arm-vgic-its". Unfortunately, there are more unknown sites to
> be handled and no-running-vcpu context will be allowed in these
> sites: (1) KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_RESTORE_TABLES} command
> on KVM device "kvm-arm-vgic-its" to restore vgic/its tables. The
> vgic3 LPI pending status could be restored. (2) Save vgic3 pending
> table through KVM_DEV_ARM_{VGIC_GRP_CTRL, VGIC_SAVE_PENDING_TABLES}
> command on KVM device "kvm-arm-vgic-v3".
> 
> In order to handle those unknown cases, we need a unified helper
> vgic_write_guest_lock(). struct vgic_dist::save_its_tables_in_progress
> is also renamed to struct vgic_dist::save_tables_in_progress. Besides,
> "asm/kvm_mmu.h" needs to be included for "vgic.h" for the definition
> of kvm_write_guest_lock().
> 
> No functional change intended.
> 
> Suggested-by: Oliver Upton <oliver.upton@linux.dev>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>  arch/arm64/kvm/vgic-sys-reg-v3.c   |  1 +
>  arch/arm64/kvm/vgic/vgic-irqfd.c   |  1 +
>  arch/arm64/kvm/vgic/vgic-its.c     | 13 +++++--------
>  arch/arm64/kvm/vgic/vgic-mmio-v2.c |  1 +
>  arch/arm64/kvm/vgic/vgic-mmio.c    |  1 +
>  arch/arm64/kvm/vgic/vgic-v4.c      |  1 +
>  arch/arm64/kvm/vgic/vgic.c         |  1 +
>  arch/arm64/kvm/vgic/vgic.h         | 13 +++++++++++++
>  include/kvm/arm_vgic.h             |  2 +-
>  9 files changed, 25 insertions(+), 9 deletions(-)

You wouldn't have to add the include all around the shop if you instead
just stuck it in vgic.h...

Having said that, we really ought to get a fix in for this sooner rather
than later. I just hit it myself testing kvmarm/next.

Marc, could you take care of the include fix when applying?

-- 
Thanks,
Oliver

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

* Re: [PATCH v2 0/3] Improve dirty ring warning report
  2023-01-19 23:44 [PATCH v2 0/3] Improve dirty ring warning report Gavin Shan
                   ` (2 preceding siblings ...)
  2023-01-19 23:44 ` [PATCH v2 3/3] KVM: arm64: Allow no running vcpu on saving vgic3 pending table Gavin Shan
@ 2023-01-26 20:21 ` Oliver Upton
  3 siblings, 0 replies; 7+ messages in thread
From: Oliver Upton @ 2023-01-26 20:21 UTC (permalink / raw)
  To: Gavin Shan
  Cc: kvmarm, kvmarm, kvm, linux-kernel, linux-doc, pbonzini, corbet,
	maz, will, ricarkol, eric.auger, yuzhe, renzhengeek, reijiw, ardb,
	Julia.Lawall, yuzenghui, seanjc, shan.gavin

On Fri, Jan 20, 2023 at 07:44:02AM +0800, Gavin Shan wrote:
> It has been known case where no running VCPU context exists when the
> vgic/its tables are saved. There are other two unknown cases where we
> don't have the running VCPU context: (a) restore vgic3 LPI pending
> status. (b) restoring vgic3 pending tables.
> 
> PATCH[1]     adds unified helper vgic_write_guest_lock()
> PATCH[2 - 3] allows no-running-vcpu context for (a) and (b)

Besides the issue with the first patch, for the series:

Reviewed-by: Oliver Upton <oliver.upton@linux.dev>

-- 
Thanks,
Oliver

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

* Re: [PATCH v2 1/3] KVM: arm64: Add helper vgic_write_guest_lock()
  2023-01-26 20:20   ` Oliver Upton
@ 2023-01-27  0:02     ` Gavin Shan
  0 siblings, 0 replies; 7+ messages in thread
From: Gavin Shan @ 2023-01-27  0:02 UTC (permalink / raw)
  To: Oliver Upton
  Cc: kvmarm, kvmarm, kvm, linux-kernel, linux-doc, pbonzini, corbet,
	maz, will, ricarkol, eric.auger, yuzhe, renzhengeek, reijiw, ardb,
	Julia.Lawall, yuzenghui, seanjc, shan.gavin

Hi Oliver,

On 1/27/23 7:20 AM, Oliver Upton wrote:
> On Fri, Jan 20, 2023 at 07:44:03AM +0800, Gavin Shan wrote:
>> Currently, the unknown no-running-vcpu sites are reported when a
>> dirty page is tracked by mark_page_dirty_in_slot(). Until now, the
>> only known no-running-vcpu site is saving vgic/its tables through
>> KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_SAVE_TABLES} command on KVM device
>> "kvm-arm-vgic-its". Unfortunately, there are more unknown sites to
>> be handled and no-running-vcpu context will be allowed in these
>> sites: (1) KVM_DEV_ARM_{VGIC_GRP_CTRL, ITS_RESTORE_TABLES} command
>> on KVM device "kvm-arm-vgic-its" to restore vgic/its tables. The
>> vgic3 LPI pending status could be restored. (2) Save vgic3 pending
>> table through KVM_DEV_ARM_{VGIC_GRP_CTRL, VGIC_SAVE_PENDING_TABLES}
>> command on KVM device "kvm-arm-vgic-v3".
>>
>> In order to handle those unknown cases, we need a unified helper
>> vgic_write_guest_lock(). struct vgic_dist::save_its_tables_in_progress
>> is also renamed to struct vgic_dist::save_tables_in_progress. Besides,
>> "asm/kvm_mmu.h" needs to be included for "vgic.h" for the definition
>> of kvm_write_guest_lock().
>>
>> No functional change intended.
>>
>> Suggested-by: Oliver Upton <oliver.upton@linux.dev>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>>   arch/arm64/kvm/vgic-sys-reg-v3.c   |  1 +
>>   arch/arm64/kvm/vgic/vgic-irqfd.c   |  1 +
>>   arch/arm64/kvm/vgic/vgic-its.c     | 13 +++++--------
>>   arch/arm64/kvm/vgic/vgic-mmio-v2.c |  1 +
>>   arch/arm64/kvm/vgic/vgic-mmio.c    |  1 +
>>   arch/arm64/kvm/vgic/vgic-v4.c      |  1 +
>>   arch/arm64/kvm/vgic/vgic.c         |  1 +
>>   arch/arm64/kvm/vgic/vgic.h         | 13 +++++++++++++
>>   include/kvm/arm_vgic.h             |  2 +-
>>   9 files changed, 25 insertions(+), 9 deletions(-)
> 
> You wouldn't have to add the include all around the shop if you instead
> just stuck it in vgic.h...
> 
> Having said that, we really ought to get a fix in for this sooner rather
> than later. I just hit it myself testing kvmarm/next.
> 
> Marc, could you take care of the include fix when applying?
> 

I've posted v3 to have a separate PATCH[1/4] where the header file inclusions
are handled, to save Marc's valuable time. After 'kvm_mmu.h' is included to
'vgic.h', the duplicate inclusions of 'kvm_mmu.h' needs to be removed. A separate
patch would make the follow-up patches clean.

https://lore.kernel.org/kvmarm/20230126235451.469087-1-gshan@redhat.com/T/#t

Thanks,
Gavin


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

end of thread, other threads:[~2023-01-27  0:03 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-19 23:44 [PATCH v2 0/3] Improve dirty ring warning report Gavin Shan
2023-01-19 23:44 ` [PATCH v2 1/3] KVM: arm64: Add helper vgic_write_guest_lock() Gavin Shan
2023-01-26 20:20   ` Oliver Upton
2023-01-27  0:02     ` Gavin Shan
2023-01-19 23:44 ` [PATCH v2 2/3] KVM: arm64: Allow no running vcpu on restoring vgic3 LPI pending status Gavin Shan
2023-01-19 23:44 ` [PATCH v2 3/3] KVM: arm64: Allow no running vcpu on saving vgic3 pending table Gavin Shan
2023-01-26 20:21 ` [PATCH v2 0/3] Improve dirty ring warning report Oliver Upton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.