Skip to contentSkip to conversation

KVM: Add reading critical region for kvm_io_bus_read/write

PATCH In review Sheng Yang 2 participants 3 messages Dec 6, 2010 – Dec 7, 2010 KVM

lore
Share & tools
lore

Sheng Yang

author
#0 lore raw

PATCH 1/1 KVM: Add reading critical region for kvm_io_bus_read/write

Seems we missed it.

Signed-off-by Sheng Yang <[email protected]>

---
Do we need this, or slot_lock covered this?

 virt/kvm/kvm_main.c |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)
virt/kvm/kvm_main.c +16 -8
index c4ee364..3e71b91 100644
@@ -2228,28 +2228,36 @@ static void kvm_io_bus_destroy(struct kvm_io_bus *bus)
 int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 		     int len, const void *val)
 {
-	int i;
+	int i, idx, r = -EOPNOTSUPP;
 	struct kvm_io_bus *bus;
 
+	idx = srcu_read_lock(&kvm->srcu);
 	bus = srcu_dereference(kvm->buses[bus_idx], &kvm->srcu);
 	for (i = 0; i < bus->dev_count; i++)
-		if (!kvm_iodevice_write(bus->devs[i], addr, len, val))
-			return 0;
-	return -EOPNOTSUPP;
+		if (!kvm_iodevice_write(bus->devs[i], addr, len, val)) {
+			r = 0;
+			break;
+		}
+	srcu_read_unlock(&kvm->srcu, idx);
+	return r;
 }
 
 /* kvm_io_bus_read - called under kvm->slots_lock */
 int kvm_io_bus_read(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 		    int len, void *val)
 {
-	int i;
+	int i, idx, r = -EOPNOTSUPP;
 	struct kvm_io_bus *bus;
 
+	idx = srcu_read_lock(&kvm->srcu);
 	bus = srcu_dereference(kvm->buses[bus_idx], &kvm->srcu);
 	for (i = 0; i < bus->dev_count; i++)
-		if (!kvm_iodevice_read(bus->devs[i], addr, len, val))
-			return 0;
-	return -EOPNOTSUPP;
+		if (!kvm_iodevice_read(bus->devs[i], addr, len, val)) {
+			r = 0;
+			break;
+		}
+	srcu_read_unlock(&kvm->srcu, idx);
+	return r;
 }
 
 /* Caller must hold slots_lock. */
Signature · 1 line
1.7.0.1
#1 lore raw
Quoted 24 lines · Sheng Yang

Seems we missed it.

Signed-off-by Sheng Yang<[email protected]>

---
Do we need this, or slot_lock covered this?

  virt/kvm/kvm_main.c |   24 ++++++++++++++++--------
  1 files changed, 16 insertions(+), 8 deletions(-)
virt/kvm/kvm_main.c +2 -1
index c4ee364..3e71b91 100644
@@ -2228,28 +2228,36 @@ static void kvm_io_bus_destroy(struct kvm_io_bus *bus)
  int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
  		     int len, const void *val)
  {
-	int i;
+	int i, idx, r = -EOPNOTSUPP;
  	struct kvm_io_bus *bus;
 
+	idx = srcu_read_lock(&kvm->srcu);
  	bus = srcu_dereference(kvm->buses[bus_idx],&kvm->srcu);
  	for (i = 0; i<  bus->dev_count; i++)

virt/kvm/kvm_main.c static void kvm_io_bus_destroy(struct kvm_io_bus *bus) · line 2243

Quoted 10 lines
virt/kvm/kvm_main.c +6 -3
-		if (!kvm_iodevice_write(bus->devs[i], addr, len, val))
-			return 0;
-	return -EOPNOTSUPP;
+		if (!kvm_iodevice_write(bus->devs[i], addr, len, val)) {
+			r = 0;
+			break;
+		}
+	srcu_read_unlock(&kvm->srcu, idx);
+	return r;
  }

Isn't this already taken care of by srcu_read_lock() in vcpu_enter_guest(), just before calling ->handle_exit()?

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#2 lore raw
Quoted 43 lines · On Monday 06 December 2010 20:58:10 Avi Kivity wrote:
Quoted 42 lines · Sheng Yang

Seems we missed it.

Signed-off-by Sheng Yang<[email protected]>

---
Do we need this, or slot_lock covered this?

  virt/kvm/kvm_main.c |   24 ++++++++++++++++--------
  1 files changed, 16 insertions(+), 8 deletions(-)
virt/kvm/kvm_main.c +8 -4
index c4ee364..3e71b91 100644
@@ -2228,28 +2228,36 @@ static void kvm_io_bus_destroy(struct kvm_io_bus
 *bus)
 
  int kvm_io_bus_write(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr,
 
  		     int len, const void *val)
 
  {
 
-	int i;
+	int i, idx, r = -EOPNOTSUPP;
 
  	struct kvm_io_bus *bus;
 
+	idx = srcu_read_lock(&kvm->srcu);
 
  	bus = srcu_dereference(kvm->buses[bus_idx],&kvm->srcu);
  	for (i = 0; i<  bus->dev_count; i++)
 
-		if (!kvm_iodevice_write(bus->devs[i], addr, len, val))
-			return 0;
-	return -EOPNOTSUPP;
+		if (!kvm_iodevice_write(bus->devs[i], addr, len, val)) {
+			r = 0;
+			break;
+		}
+	srcu_read_unlock(&kvm->srcu, idx);
+	return r;
Quoted 4 lines

virt/kvm/kvm_main.c static void kvm_io_bus_destroy(struct kvm_io_bus · line 2252

Quoted 1 line
virt/kvm/kvm_main.c +0 -0
  }

Isn't this already taken care of by srcu_read_lock() in vcpu_enter_guest(), just before calling ->handle_exit()?

Yes, finally found it. So big reading critical region...

--
regards
Yang, Sheng