Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1740721 > unrolled thread

[PATCH v2 00/10] vITS Migration fixes and reset

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/10] vITS Migration fixes and reset Eric Auger <eric.auger@redhat.com> - 2017-09-27 15:30 +0200
    [PATCH v2 06/10] KVM: arm/arm64: vgic-its: Always attempt to save/restore device and collection tables Eric Auger <eric.auger@redhat.com> - 2017-09-27 15:30 +0200
    [PATCH v2 03/10] KVM: arm/arm64: vgic-its: Improve error reporting on device table save Eric Auger <eric.auger@redhat.com> - 2017-09-27 15:30 +0200
    [PATCH v2 04/10] KVM: arm/arm64: vgic-its: Check GITS_BASER Valid bit before saving tables Eric Auger <eric.auger@redhat.com> - 2017-09-27 15:30 +0200
    [PATCH v2 09/10] KVM: arm/arm64: Document KVM_DEV_ARM_ITS_CTRL_RESET Eric Auger <eric.auger@redhat.com> - 2017-09-27 15:30 +0200
    [PATCH v2 01/10] KVM: arm/arm64: vgic-its: fix return value for restore Eric Auger <eric.auger@redhat.com> - 2017-09-27 15:30 +0200
    [PATCH v2 05/10] KVM: arm/arm64: vgic-its: Check GITS_CBASER validity before processing commands Eric Auger <eric.auger@redhat.com> - 2017-09-27 15:30 +0200
    [PATCH v2 07/10] KVM: arm/arm64: vgic-its: new helper functions to free the caches Eric Auger <eric.auger@redhat.com> - 2017-09-27 15:40 +0200

#1740721 — [PATCH v2 00/10] vITS Migration fixes and reset

This series fixes various bugs observed when saving/restoring the
ITS state before the guest writes the ITS registers (on first boot or
after reset/reboot).

This is a follow up of Wanghaibin's series [1] plus additional
patches following additional code review. It also proposes one
ITS reset implementation. I dared to include 2 Wanghaibin patches,
taking into account the discussions held on the ML but obviously
they can live separately.

Currently, the in-kernel emulated ITS is not reset. After a
reset/reboot, the ITS register values and caches are left
unchanged. Registers may point to some tables in guest memory
which do not exist anymore. If an ITS state backup is initiated
before the guest re-writes the registers, the save fails
because inconsistencies are detected. Also restore of data saved
as such moment is failing.

Patches [1-6] attempt to fix the migration issues without
implementing the reset.
As such they may be candidate for stable:
- do not fail restore if device or ITT tables only contain invalid
  entries
- allow clearing GITS_CREADR/CWRITER whatever CBASER queue size
- limit the cases where we return -EINVAL in table save()
- never attempt to use GITS_BASER<n> and GITS_CBASER if they are
  not valid.
- systematically do both device and collection save/restore even if
  one fails.

Patches [7-10] implement a new ITS reset IOCTL
  - maybe patch "free caches when GITS_BASER Valid bit is cleared" can
    be omitted if we consider the caches should not be cleared when
    GITS_BASER<n>.Valid is cleared.

Best Regards

Eric

Git: complete series available at
https://github.com/eauger/linux/tree/v4.14-rc2-its-reset-v2

* Testing:
- on Cavium using a virtio-net-pci guest and various sequences of
  guest shutdown -r now, virsh reset, virsh suspend/resume,
  virsh reboot, virsh save.restore, virsh shutdown

References:
[1] [RFC PATCH 0/3] fix migrate failed when vm is in booting
https://www.spinics.net/lists/kvm-arm/msg27121.html

History:
v1 -> v2:
- added KVM: arm/arm64: vgic-its: Always attempt to save/restore
  device and collection tables

PATCH v1
- series including 2 modified patches of Wanghaibin


Eric Auger (8):
  KVM: arm/arm64: vgic-its: Always allow clearing GITS_CREADR/CWRITER
  KVM: arm/arm64: vgic-its: Improve error reporting on device table save
  KVM: arm/arm64: vgic-its: Check GITS_BASER Valid bit before saving
    tables
  KVM: arm/arm64: vgic-its: Check GITS_CBASER validity before processing
    commands
  KVM: arm/arm64: vgic-its: Always attempt to save/restore device and
    collection tables
  KVM: arm/arm64: vgic-its: free caches when GITS_BASER Valid bit is
    cleared
  KVM: arm/arm64: Document KVM_DEV_ARM_ITS_CTRL_RESET
  KVM: arm/arm64: vgic-its: Implement KVM_DEV_ARM_ITS_CTRL_RESET

wanghaibin (2):
  KVM: arm/arm64: vgic-its: fix return value for restore
  KVM: arm/arm64: vgic-its: new helper functions to free the caches

 Documentation/virtual/kvm/devices/arm-vgic-its.txt |  16 ++
 arch/arm/include/uapi/asm/kvm.h                    |   1 +
 arch/arm64/include/uapi/asm/kvm.h                  |   1 +
 virt/kvm/arm/vgic/vgic-its.c                       | 222 +++++++++++++--------
 4 files changed, 158 insertions(+), 82 deletions(-)

-- 
2.5.5

[toc] | [next] | [standalone]

#1740722 — [PATCH v2 06/10] KVM: arm/arm64: vgic-its: Always attempt to save/restore device and collection tables

In case the device table save fails, we currently do not
attempt to save the collection table. However it may
happen that the device table fails because the structures
in memory are inconsistent with device GITS_BASER however
this does not mean collection backup can't be performed and
wouldn't succeed. Same on restore path. Without this patch,
after a reset and in case the device table fails in case of
L1 entry not valid, the guest gets stuck on restore.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

candidate to be CC'ed stable
---
 virt/kvm/arm/vgic/vgic-its.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
index 720552c..9e6b556 100644
--- a/virt/kvm/arm/vgic/vgic-its.c
+++ b/virt/kvm/arm/vgic/vgic-its.c
@@ -2304,12 +2304,9 @@ static int vgic_its_save_tables_v0(struct vgic_its *its)
 	}
 
 	ret = vgic_its_save_device_tables(its);
-	if (ret)
-		goto out;
 
-	ret = vgic_its_save_collection_table(its);
+	ret |= vgic_its_save_collection_table(its);
 
-out:
 	unlock_all_vcpus(kvm);
 	mutex_unlock(&its->its_lock);
 	mutex_unlock(&kvm->lock);
@@ -2336,11 +2333,9 @@ static int vgic_its_restore_tables_v0(struct vgic_its *its)
 	}
 
 	ret = vgic_its_restore_collection_table(its);
-	if (ret)
-		goto out;
 
-	ret = vgic_its_restore_device_tables(its);
-out:
+	ret |= vgic_its_restore_device_tables(its);
+
 	unlock_all_vcpus(kvm);
 	mutex_unlock(&its->its_lock);
 	mutex_unlock(&kvm->lock);
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1740723 — [PATCH v2 03/10] KVM: arm/arm64: vgic-its: Improve error reporting on device table save

At the moment the device table save() returns -EINVAL if
vgic_its_check_id() fails to return the gpa of the entry
associated to the device/collection id. Let vgic_its_check_id()
return an int instead of a bool and return a more precised
error value:
- EINVAL in case the id is out of range
- EFAULT if the gpa is not provisionned or is not valid

We also check first the GITS_BASER<n> Valid bit is set.

This allows the userspace to discriminate failure reasons.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

need to CC stable
---
 virt/kvm/arm/vgic/vgic-its.c | 53 ++++++++++++++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 17 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
index 76bed2d..c1f7972 100644
--- a/virt/kvm/arm/vgic/vgic-its.c
+++ b/virt/kvm/arm/vgic/vgic-its.c
@@ -688,14 +688,24 @@ static int vgic_its_cmd_handle_movi(struct kvm *kvm, struct vgic_its *its,
 }
 
 /*
- * Check whether an ID can be stored into the corresponding guest table.
+ * vgic_its_check_id - Check whether an ID can be stored into
+ * the corresponding guest table.
+ *
  * For a direct table this is pretty easy, but gets a bit nasty for
  * indirect tables. We check whether the resulting guest physical address
  * is actually valid (covered by a memslot and guest accessible).
  * For this we have to read the respective first level entry.
+ *
+ * @its: its handle
+ * @baser: GITS_BASER<n> register
+ * @id: id of the device/collection
+ * @eaddr: output gpa of the corresponding table entry
+ *
+ * Return: 0 on success, -EINVAL if @id is out of range, -EFAULT if
+ * the address cannot be computed or is not valid
  */
-static bool vgic_its_check_id(struct vgic_its *its, u64 baser, u32 id,
-			      gpa_t *eaddr)
+static int vgic_its_check_id(struct vgic_its *its, u64 baser, u32 id,
+			     gpa_t *eaddr)
 {
 	int l1_tbl_size = GITS_BASER_NR_PAGES(baser) * SZ_64K;
 	u64 indirect_ptr, type = GITS_BASER_TYPE(baser);
@@ -703,50 +713,56 @@ static bool vgic_its_check_id(struct vgic_its *its, u64 baser, u32 id,
 	int index;
 	gfn_t gfn;
 
+	if (!(baser & GITS_BASER_VALID))
+		return -EFAULT;
+
 	switch (type) {
 	case GITS_BASER_TYPE_DEVICE:
 		if (id >= BIT_ULL(VITS_TYPER_DEVBITS))
-			return false;
+			return -EINVAL;
 		break;
 	case GITS_BASER_TYPE_COLLECTION:
 		/* as GITS_TYPER.CIL == 0, ITS supports 16-bit collection ID */
 		if (id >= BIT_ULL(16))
-			return false;
+			return -EINVAL;
 		break;
 	default:
-		return false;
+		return -EINVAL;
 	}
 
 	if (!(baser & GITS_BASER_INDIRECT)) {
 		phys_addr_t addr;
 
 		if (id >= (l1_tbl_size / esz))
-			return false;
+			return -EINVAL;
 
 		addr = BASER_ADDRESS(baser) + id * esz;
 		gfn = addr >> PAGE_SHIFT;
 
 		if (eaddr)
 			*eaddr = addr;
-		return kvm_is_visible_gfn(its->dev->kvm, gfn);
+		if (kvm_is_visible_gfn(its->dev->kvm, gfn))
+			return 0;
+		else
+			return -EFAULT;
 	}
 
 	/* calculate and check the index into the 1st level */
 	index = id / (SZ_64K / esz);
 	if (index >= (l1_tbl_size / sizeof(u64)))
-		return false;
+		return -EINVAL;
 
 	/* Each 1st level entry is represented by a 64-bit value. */
 	if (kvm_read_guest(its->dev->kvm,
 			   BASER_ADDRESS(baser) + index * sizeof(indirect_ptr),
 			   &indirect_ptr, sizeof(indirect_ptr)))
-		return false;
+		return -EFAULT;
 
 	indirect_ptr = le64_to_cpu(indirect_ptr);
 
 	/* check the valid bit of the first level entry */
 	if (!(indirect_ptr & BIT_ULL(63)))
-		return false;
+		return -EFAULT;
 
 	/*
 	 * Mask the guest physical address and calculate the frame number.
@@ -762,7 +778,10 @@ static bool vgic_its_check_id(struct vgic_its *its, u64 baser, u32 id,
 
 	if (eaddr)
 		*eaddr = indirect_ptr;
-	return kvm_is_visible_gfn(its->dev->kvm, gfn);
+	if (kvm_is_visible_gfn(its->dev->kvm, gfn))
+		return 0;
+	else
+		return -EFAULT;
 }
 
 static int vgic_its_alloc_collection(struct vgic_its *its,
@@ -771,7 +790,7 @@ static int vgic_its_alloc_collection(struct vgic_its *its,
 {
 	struct its_collection *collection;
 
-	if (!vgic_its_check_id(its, its->baser_coll_table, coll_id, NULL))
+	if (vgic_its_check_id(its, its->baser_coll_table, coll_id, NULL))
 		return E_ITS_MAPC_COLLECTION_OOR;
 
 	collection = kzalloc(sizeof(*collection), GFP_KERNEL);
@@ -943,7 +962,7 @@ static int vgic_its_cmd_handle_mapd(struct kvm *kvm, struct vgic_its *its,
 	gpa_t itt_addr = its_cmd_get_ittaddr(its_cmd);
 	struct its_device *device;
 
-	if (!vgic_its_check_id(its, its->baser_device_table, device_id, NULL))
+	if (vgic_its_check_id(its, its->baser_device_table, device_id, NULL))
 		return E_ITS_MAPD_DEVICE_OOR;
 
 	if (valid && num_eventid_bits > VITS_TYPER_IDBITS)
@@ -2060,9 +2079,9 @@ static int vgic_its_save_device_tables(struct vgic_its *its)
 		int ret;
 		gpa_t eaddr;
 
-		if (!vgic_its_check_id(its, baser,
-				       dev->device_id, &eaddr))
-			return -EINVAL;
+		ret = vgic_its_check_id(its, baser, dev->device_id, &eaddr);
+		if (ret)
+			return ret;
 
 		ret = vgic_its_save_itt(its, dev);
 		if (ret)
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1740725 — [PATCH v2 04/10] KVM: arm/arm64: vgic-its: Check GITS_BASER Valid bit before saving tables

At the moment we don't properly check the GITS_BASER<n>.Valid
bit before saving the collection and device tables.

On Collection table save() we use the gpa field whereas the Valid bit
should be used. On device table save() there is no check. This can
cause various bugs, among which a subsequent fault when accessing
the table in guest memory.

Let's systematically check the Valid bit before doing anything.

We also unifomize the code between save and restore.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 virt/kvm/arm/vgic/vgic-its.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
index c1f7972..60ecf91 100644
--- a/virt/kvm/arm/vgic/vgic-its.c
+++ b/virt/kvm/arm/vgic/vgic-its.c
@@ -2067,11 +2067,12 @@ static int vgic_its_device_cmp(void *priv, struct list_head *a,
 static int vgic_its_save_device_tables(struct vgic_its *its)
 {
 	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
+	u64 baser = its->baser_device_table;
 	struct its_device *dev;
 	int dte_esz = abi->dte_esz;
-	u64 baser;
 
-	baser = its->baser_device_table;
+	if (!(baser & GITS_BASER_VALID))
+		return 0;
 
 	list_sort(NULL, &its->device_list, vgic_its_device_cmp);
 
@@ -2217,17 +2218,17 @@ static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz)
 static int vgic_its_save_collection_table(struct vgic_its *its)
 {
 	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
+	u64 baser = its->baser_coll_table;
+	gpa_t gpa = BASER_ADDRESS(baser);
 	struct its_collection *collection;
 	u64 val;
-	gpa_t gpa;
 	size_t max_size, filled = 0;
 	int ret, cte_esz = abi->cte_esz;
 
-	gpa = BASER_ADDRESS(its->baser_coll_table);
-	if (!gpa)
+	if (!(baser & GITS_BASER_VALID))
 		return 0;
 
-	max_size = GITS_BASER_NR_PAGES(its->baser_coll_table) * SZ_64K;
+	max_size = GITS_BASER_NR_PAGES(baser) * SZ_64K;
 
 	list_for_each_entry(collection, &its->collection_list, coll_list) {
 		ret = vgic_its_save_cte(its, collection, gpa, cte_esz);
@@ -2258,17 +2259,18 @@ static int vgic_its_save_collection_table(struct vgic_its *its)
 static int vgic_its_restore_collection_table(struct vgic_its *its)
 {
 	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
+	u64 baser = its->baser_coll_table;
 	int cte_esz = abi->cte_esz;
 	size_t max_size, read = 0;
 	gpa_t gpa;
 	int ret;
 
-	if (!(its->baser_coll_table & GITS_BASER_VALID))
+	if (!(baser & GITS_BASER_VALID))
 		return 0;
 
-	gpa = BASER_ADDRESS(its->baser_coll_table);
+	gpa = BASER_ADDRESS(baser);
 
-	max_size = GITS_BASER_NR_PAGES(its->baser_coll_table) * SZ_64K;
+	max_size = GITS_BASER_NR_PAGES(baser) * SZ_64K;
 
 	while (read < max_size) {
 		ret = vgic_its_restore_cte(its, gpa, cte_esz);
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1740726 — [PATCH v2 09/10] KVM: arm/arm64: Document KVM_DEV_ARM_ITS_CTRL_RESET

At the moment, the in-kernel emulated ITS is not properly reset.
On guest restart/reset some registers keep their old values and
internal structures like device, ITE, collection lists are not freed.

This may lead to various bugs. Among them, we can have incorrect state
backup or failure when saving the ITS state at early guest boot stage.

This patch documents a new attribute, KVM_DEV_ARM_ITS_CTRL_RESET in
the KVM_DEV_ARM_VGIC_GRP_CTRL group.

Upon this action, we can reset registers and especially those
pointing to tables previously allocated by the guest and free
the internal data structures storing the list of devices, collections
and lpis.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reported-by: wanghaibin <wanghaibin.wang@huawei.com>

---

v1 -> v2:
- Describe architecturally-defined reset values
---
 Documentation/virtual/kvm/devices/arm-vgic-its.txt | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/virtual/kvm/devices/arm-vgic-its.txt b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
index eb06beb..047358c 100644
--- a/Documentation/virtual/kvm/devices/arm-vgic-its.txt
+++ b/Documentation/virtual/kvm/devices/arm-vgic-its.txt
@@ -33,6 +33,10 @@ Groups:
       request the initialization of the ITS, no additional parameter in
       kvm_device_attr.addr.
 
+    KVM_DEV_ARM_ITS_CTRL_RESET
+      reset the ITS, no additional parameter in kvm_device_attr.addr.
+      See "ITS Reset State" section.
+
     KVM_DEV_ARM_ITS_SAVE_TABLES
       save the ITS table data into guest RAM, at the location provisioned
       by the guest in corresponding registers/table entries.
@@ -157,3 +161,15 @@ Then vcpus can be started.
  - pINTID is the physical LPI ID; if zero, it means the entry is not valid
    and other fields are not meaningful.
  - ICID is the collection ID
+
+ ITS Reset State:
+ ----------------
+
+- the ITS is not enabled and quiescent:
+  GITS_CTLR.Enabled = 0 .Quiescent=1
+- caches are empty
+- No collection or device table is provisionned
+  GITS_BASER<n>.Valid = 0
+- the command queue is not allocated:
+  GITS_CBASER = 0, GITS_CREADR = 0, GITS_CWRITER = 0
+- The ABI version corresponds to the one set before reset
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1740727 — [PATCH v2 01/10] KVM: arm/arm64: vgic-its: fix return value for restore

From: wanghaibin <wanghaibin.wang@huawei.com>

This patch fix the migrate restore tables failure.

The same scene, at the destination, the restore tables
interface traversal guest memory, and check the dte/ite
is valid or not.  If all dtes/ites are invalid, we will do
try next one, and the last it will take the 1 return value,
but currently, it be treated as error. That's not correct.

This patch try to fix this problem.

Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com>

---

need to CC stable

v1 -> v2:
- if (ret > 0) ret = 0
---
 virt/kvm/arm/vgic/vgic-its.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
index f51c1e1..fbbc97b 100644
--- a/virt/kvm/arm/vgic/vgic-its.c
+++ b/virt/kvm/arm/vgic/vgic-its.c
@@ -2018,7 +2018,7 @@ static int vgic_its_restore_dte(struct vgic_its *its, u32 id,
 		return PTR_ERR(dev);
 
 	ret = vgic_its_restore_itt(its, dev);
-	if (ret) {
+	if (ret < 0) {
 		vgic_its_free_device(its->dev->kvm, dev);
 		return ret;
 	}
@@ -2141,7 +2141,7 @@ static int vgic_its_restore_device_tables(struct vgic_its *its)
 	}
 
 	if (ret > 0)
-		ret = -EINVAL;
+		ret = 0;
 
 	return ret;
 }
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1740728 — [PATCH v2 05/10] KVM: arm/arm64: vgic-its: Check GITS_CBASER validity before processing commands

At the moment vgic_its_process_commands() does not
check the CBASER is valid before processing any command.
Let's fix that.

Also rename cbaser local variable into cbaser_pa to avoid
any confusion with the full register.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 virt/kvm/arm/vgic/vgic-its.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
index 60ecf91..720552c 100644
--- a/virt/kvm/arm/vgic/vgic-its.c
+++ b/virt/kvm/arm/vgic/vgic-its.c
@@ -1301,17 +1301,20 @@ static void vgic_mmio_write_its_cbaser(struct kvm *kvm, struct vgic_its *its,
 /* Must be called with the cmd_lock held. */
 static void vgic_its_process_commands(struct kvm *kvm, struct vgic_its *its)
 {
-	gpa_t cbaser;
+	gpa_t cbaser_pa;
 	u64 cmd_buf[4];
 
-	/* Commands are only processed when the ITS is enabled. */
-	if (!its->enabled)
+	/*
+	 * Commands are only processed when the ITS is enabled and
+	 * CBASER is valid
+	 */
+	if (!its->enabled || (!(its->cbaser & GITS_CBASER_VALID)))
 		return;
 
-	cbaser = CBASER_ADDRESS(its->cbaser);
+	cbaser_pa = CBASER_ADDRESS(its->cbaser);
 
 	while (its->cwriter != its->creadr) {
-		int ret = kvm_read_guest(kvm, cbaser + its->creadr,
+		int ret = kvm_read_guest(kvm, cbaser_pa + its->creadr,
 					 cmd_buf, ITS_CMD_SIZE);
 		/*
 		 * If kvm_read_guest() fails, this could be due to the guest
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1740733 — [PATCH v2 07/10] KVM: arm/arm64: vgic-its: new helper functions to free the caches

From: wanghaibin <wanghaibin.wang@huawei.com>

We create 2 new functions that frees the device and
collection lists. this is currently called by vgic_its_destroy()
and we will add other callers in subsequent patches.

We also remove the check on its->device_list.next as it looks
unnecessary:

The kvm device is removed by kvm_destroy_devices which loops on
all the devices added to kvm->devices. kvm_ioctl_create_device
only adds the device to kvm_devices once the lists have been
initialized (in vgic_create_its).

We also move vgic_its_free_device to prepare for new callers.

Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>

---
[Eric] removed its->device_list.next which is not needed as
pointed out by Wanghaibin. Reword the commit message
---
 virt/kvm/arm/vgic/vgic-its.c | 76 ++++++++++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 35 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
index 9e6b556..0df6d5f 100644
--- a/virt/kvm/arm/vgic/vgic-its.c
+++ b/virt/kvm/arm/vgic/vgic-its.c
@@ -611,6 +611,45 @@ static void its_free_ite(struct kvm *kvm, struct its_ite *ite)
 	kfree(ite);
 }
 
+static void vgic_its_free_device(struct kvm *kvm, struct its_device *dev)
+{
+	struct its_ite *ite, *tmp;
+
+	list_for_each_entry_safe(ite, tmp, &dev->itt_head, ite_list)
+		its_free_ite(kvm, ite);
+	list_del(&dev->dev_list);
+	kfree(dev);
+}
+
+static void vgic_its_free_device_list(struct kvm *kvm, struct vgic_its *its)
+{
+	struct list_head *cur, *temp;
+
+	mutex_lock(&its->its_lock);
+	list_for_each_safe(cur, temp, &its->device_list) {
+		struct its_device *dev;
+
+		dev = list_entry(cur, struct its_device, dev_list);
+		vgic_its_free_device(kvm, dev);
+	}
+	mutex_unlock(&its->its_lock);
+}
+
+static void vgic_its_free_collection_list(struct kvm *kvm, struct vgic_its *its)
+{
+	struct list_head *cur, *temp;
+
+	list_for_each_safe(cur, temp, &its->collection_list) {
+		struct its_collection *coll;
+
+		coll = list_entry(cur, struct its_collection, coll_list);
+		list_del(cur);
+		kfree(coll);
+	}
+	mutex_unlock(&its->its_lock);
+}
+
+
 static u64 its_cmd_mask_field(u64 *its_cmd, int word, int shift, int size)
 {
 	return (le64_to_cpu(its_cmd[word]) >> shift) & (BIT_ULL(size) - 1);
@@ -1634,46 +1673,13 @@ static int vgic_its_create(struct kvm_device *dev, u32 type)
 	return vgic_its_set_abi(its, NR_ITS_ABIS - 1);
 }
 
-static void vgic_its_free_device(struct kvm *kvm, struct its_device *dev)
-{
-	struct its_ite *ite, *tmp;
-
-	list_for_each_entry_safe(ite, tmp, &dev->itt_head, ite_list)
-		its_free_ite(kvm, ite);
-	list_del(&dev->dev_list);
-	kfree(dev);
-}
-
 static void vgic_its_destroy(struct kvm_device *kvm_dev)
 {
 	struct kvm *kvm = kvm_dev->kvm;
 	struct vgic_its *its = kvm_dev->private;
-	struct list_head *cur, *temp;
-
-	/*
-	 * We may end up here without the lists ever having been initialized.
-	 * Check this and bail out early to avoid dereferencing a NULL pointer.
-	 */
-	if (!its->device_list.next)
-		return;
-
-	mutex_lock(&its->its_lock);
-	list_for_each_safe(cur, temp, &its->device_list) {
-		struct its_device *dev;
-
-		dev = list_entry(cur, struct its_device, dev_list);
-		vgic_its_free_device(kvm, dev);
-	}
-
-	list_for_each_safe(cur, temp, &its->collection_list) {
-		struct its_collection *coll;
-
-		coll = list_entry(cur, struct its_collection, coll_list);
-		list_del(cur);
-		kfree(coll);
-	}
-	mutex_unlock(&its->its_lock);
 
+	vgic_its_free_device_list(kvm, its);
+	vgic_its_free_collection_list(kvm, its);
 	kfree(its);
 }
 
-- 
2.5.5

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web