kvm: document locking for kvm_io_device_ops
PATCH v5 1/6 kvm: document locking for kvm_io_device_ops
In reply to a message not in the archiveslots_lock is taken everywhere when device ops are called. Document this as we will use this to rework locking for io.
Signed-off-by Michael S. Tsirkin <[email protected]>
--- virt/kvm/iodev.h | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
virt/kvm/iodev.h +3 -0
@@ -20,6 +20,9 @@struct kvm_io_device ; +/** +* kvm_io_device_ops are called under kvm slots_lock. +**/ struct kvm_io_device_ops { void (*read)(struct kvm_io_device *this, gpa_t addr,
Signature · 1 line
1.6.2.2
PATCH v5 2/6 kvm: switch coalesced mmio changes to slots_lock
In reply to a message not in the archiveswitch coalesced mmio slots_lock. slots_lock is already taken for read everywhere, so we only need to take it for write when changing zones. This is in preparation to removing in_range and kvm->lock around it.
Signed-off-by Michael S. Tsirkin <[email protected]>
--- virt/kvm/coalesced_mmio.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
virt/kvm/coalesced_mmio.c +6 -6
@@ -102,7 +102,7 @@ int kvm_coalesced_mmio_init(struct kvm *kvm)kvm_iodevice_init (&dev->dev, &coalesced_mmio_ops); dev->kvm = kvm; kvm->coalesced_mmio_dev = dev; -kvm_io_bus_register_dev (&kvm->mmio_bus, &dev->dev); +kvm_io_bus_register_dev (kvm, &kvm->mmio_bus, &dev->dev);return 0 ; }
@@ -115,16 +115,16 @@ int kvm_vm_ioctl_register_coalesced_mmio(struct kvm *kvm,if (dev ==NULL )return -EINVAL ; -mutex_lock (&kvm->lock); +down_write (&kvm->slots_lock);if (dev->nb_zones >=KVM_COALESCED_MMIO_ZONE_MAX ) { -mutex_unlock (&kvm->lock); +up_write (&kvm->slots_lock);return -ENOBUFS ; } dev->zone[dev->nb_zones] = *zone; dev->nb_zones++; -mutex_unlock (&kvm->lock); +up_write (&kvm->slots_lock);return 0 ; }
@@ -138,7 +138,7 @@ int kvm_vm_ioctl_unregister_coalesced_mmio(struct kvm *kvm,if (dev ==NULL )return -EINVAL ; -mutex_lock (&kvm->lock); +down_write (&kvm->slots_lock); i = dev->nb_zones;while (i) {
@@ -156,7 +156,7 @@ int kvm_vm_ioctl_unregister_coalesced_mmio(struct kvm *kvm, i--; } -mutex_unlock (&kvm->lock); +up_write (&kvm->slots_lock);return 0 ; }
Signature · 1 line
1.6.2.2
PATCH v5 3/6 kvm: switch pit creation to slots_lock
In reply to a message not in the archiveswitch pit creation to slots_lock. slots_lock is already taken for read everywhere, so we only need to take it for write when creating pit. This is in preparation to removing in_range and kvm->lock around it.
Signed-off-by Michael S. Tsirkin <[email protected]>
--- arch/x86/kvm/x86.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/kvm/x86.c +2 -2
@@ -2115,7 +2115,7 @@ long kvm_arch_vm_ioctl(struct file *filp, sizeof(struct kvm_pit_config))) goto out; create_pit: -mutex_lock (&kvm->lock); +down_write (&kvm->slots_lock); r = -EEXIST ;if (kvm->arch.vpit) goto create_pit_unlock;
@@ -2124,7 +2124,7 @@ long kvm_arch_vm_ioctl(struct file *filp,if (kvm->arch.vpit) r =0 ; create_pit_unlock: -mutex_unlock (&kvm->lock); +up_write (&kvm->slots_lock);break ;case KVM_IRQ_LINE_STATUS :case KVM_IRQ_LINE : {
Signature · 1 line
1.6.2.2
PATCH v5 4/6 kvm: convert bus to slots_lock
In reply to a message not in the archiveUse slots_lock to protect device list on the bus. slots_lock is already taken for read everywhere, so we only need to take it for write when registering devices. This is in preparation to removing in_range and kvm->lock around it.
Signed-off-by Michael S. Tsirkin <[email protected]>
--- arch/x86/kvm/i8254.c | 5 +++-- arch/x86/kvm/i8259.c | 2 +- include/linux/kvm_host.h | 5 ++++- virt/kvm/ioapic.c | 2 +- virt/kvm/kvm_main.c | 12 +++++++++++- 5 files changed, 20 insertions(+), 6 deletions(-)
arch/x86/kvm/i8254.c +3 -2
@@ -582,6 +582,7 @@ static const struct kvm_io_device_ops speaker_dev_ops = { .in_range = speaker_in_range, }; +/* Caller must have writers lock on slots_lock */ struct kvm_pit *kvm_create_pit (struct kvm *kvm,u32 flags) {struct kvm_pit *pit;
@@ -620,11 +621,11 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)kvm_register_irq_mask_notifier (kvm,0 , &pit->mask_notifier);kvm_iodevice_init (&pit->dev, &pit_dev_ops); -kvm_io_bus_register_dev (&kvm->pio_bus, &pit->dev); +__kvm_io_bus_register_dev (&kvm->pio_bus, &pit->dev);if (flags &KVM_PIT_SPEAKER_DUMMY ) {kvm_iodevice_init (&pit->speaker_dev, &speaker_dev_ops); -kvm_io_bus_register_dev (&kvm->pio_bus, &pit->speaker_dev); +__kvm_io_bus_register_dev (&kvm->pio_bus, &pit->speaker_dev); }return pit;
arch/x86/kvm/i8259.c +1 -1
@@ -548,6 +548,6 @@ struct kvm_pic *kvm_create_pic(struct kvm *kvm)* Initialize PIO device */ kvm_iodevice_init (&s->dev, &picdev_ops); -kvm_io_bus_register_dev (&kvm->pio_bus, &s->dev); +kvm_io_bus_register_dev (kvm, &kvm->pio_bus, &s->dev);return s; }
include/linux/kvm_host.h +4 -1
@@ -42,6 +42,7 @@#define KVM_USERSPACE_IRQ_SOURCE_ID 0 +struct kvm ;struct kvm_vcpu ;extern struct kmem_cache *kvm_vcpu_cache;
@@ -61,7 +62,9 @@ void kvm_io_bus_init(struct kvm_io_bus *bus);void kvm_io_bus_destroy (struct kvm_io_bus *bus);struct kvm_io_device *kvm_io_bus_find_dev (struct kvm_io_bus *bus,gpa_t addr,int len,int is_write); -void kvm_io_bus_register_dev (struct kvm_io_bus *bus, +void __kvm_io_bus_register_dev (struct kvm_io_bus *bus, +struct kvm_io_device *dev); +void kvm_io_bus_register_dev (struct kvm *kvm,struct kvm_io_bus *bus,struct kvm_io_device *dev);struct kvm_vcpu {
virt/kvm/ioapic.c +1 -1
@@ -343,7 +343,7 @@ int kvm_ioapic_init(struct kvm *kvm)kvm_ioapic_reset (ioapic);kvm_iodevice_init (&ioapic->dev, &ioapic_mmio_ops); ioapic->kvm = kvm; -kvm_io_bus_register_dev (&kvm->mmio_bus, &ioapic->dev); +kvm_io_bus_register_dev (kvm, &kvm->mmio_bus, &ioapic->dev);return 0 ; }
virt/kvm/kvm_main.c +11 -1
@@ -2499,7 +2499,17 @@ struct kvm_io_device *kvm_io_bus_find_dev(struct kvm_io_bus *bus,return NULL ; } -void kvm_io_bus_register_dev (struct kvm_io_bus *bus,struct kvm_io_device *dev) +void kvm_io_bus_register_dev (struct kvm *kvm,struct kvm_io_bus *bus, +struct kvm_io_device *dev) +{ +down_write (&kvm->slots_lock); +__kvm_io_bus_register_dev (bus, dev); +up_write (&kvm->slots_lock); +} + +/* An unlocked version. Caller must have write lock on slots_lock. */ +void __kvm_io_bus_register_dev (struct kvm_io_bus *bus, +struct kvm_io_device *dev) {BUG_ON (bus->dev_count > (NR_IOBUS_DEVS -1 ));
Signature · 1 line
1.6.2.2
PATCH v5 5/6 kvm: remove in_range from io devices
In reply to a message not in the archiveThis changes bus accesses to use high-level kvm_io_bus_read/kvm_io_bus_write functions. in_range now becomes unused so it is removed from device ops in favor of read/write callbacks performing range checks internally.
This allows aliasing (mostly for in-kernel virtio), as well as better error handling by making it possible to pass errors up to userspace.
Signed-off-by Michael S. Tsirkin <[email protected]>
--- arch/ia64/kvm/kvm-ia64.c | 28 +++-------- arch/x86/kvm/i8254.c | 49 +++++++++++--------- arch/x86/kvm/i8259.c | 20 +++++--- arch/x86/kvm/lapic.c | 44 ++++++++---------- arch/x86/kvm/x86.c | 110 +++++++++++++-------------------------------- include/linux/kvm_host.h | 6 ++- virt/kvm/coalesced_mmio.c | 16 +++---- virt/kvm/ioapic.c | 22 +++++---- virt/kvm/iodev.h | 39 ++++++---------- virt/kvm/kvm_main.c | 26 +++++++---- 10 files changed, 152 insertions(+), 208 deletions(-)
arch/ia64/kvm/kvm-ia64.c +8 -20
@@ -210,16 +210,6 @@ int kvm_dev_ioctl_check_extension(long ext) } -static struct kvm_io_device *vcpu_find_mmio_dev (struct kvm_vcpu *vcpu, -gpa_t addr,int len,int is_write) -{ -struct kvm_io_device *dev; - - dev =kvm_io_bus_find_dev (&vcpu->kvm->mmio_bus, addr, len, is_write); - -return dev; -} -static int handle_vm_error (struct kvm_vcpu *vcpu,struct kvm_run *kvm_run) { kvm_run->exit_reason =KVM_EXIT_UNKNOWN ;
@@ -231,6 +221,7 @@ static int handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) {struct kvm_mmio_req *p;struct kvm_io_device *mmio_dev; +int r; p =kvm_get_vcpu_ioreq (vcpu);
@@ -247,16 +238,13 @@ static int handle_mmio(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) kvm_run->exit_reason =KVM_EXIT_MMIO ;return 0 ; mmio: - mmio_dev =vcpu_find_mmio_dev (vcpu, p->addr, p->size, !p->dir); -if (mmio_dev) { -if (!p->dir) -kvm_iodevice_write (mmio_dev, p->addr, p->size, - &p->data); -else -kvm_iodevice_read (mmio_dev, p->addr, p->size, - &p->data); - - }else +if (p->dir) + r =kvm_io_bus_read (&vcpu->kvm->mmio_bus, p->addr, + p->size, &p->data); +else + r =kvm_io_bus_write (&vcpu->kvm->mmio_bus, p->addr, + p->size, &p->data); +if (r)printk (KERN_ERR "kvm: No iodevice found! addr:%lx\n" , p->addr); p->state =STATE_IORESP_READY ;
arch/x86/kvm/i8254.c +26 -23
118 lines not on this page · raw message
arch/x86/kvm/i8259.c +12 -8
@@ -430,8 +430,7 @@ static u32 elcr_ioport_read(void *opaque, u32 addr1)return s->elcr; } -static int picdev_in_range (struct kvm_io_device *this,gpa_t addr, -int len,int is_write) +static int picdev_in_range (gpa_t addr) {switch (addr) {case 0x20 :
@@ -451,16 +450,18 @@ static inline struct kvm_pic *to_pic(struct kvm_io_device *dev) return container_of(dev, struct kvm_pic, dev); } -static void picdev_write (struct kvm_io_device *this, +static int picdev_write (struct kvm_io_device *this,gpa_t addr,int len,const void *val) {struct kvm_pic *s =to_pic (this);unsigned char data = *(unsigned char *)val; +if (!picdev_in_range (addr)) +return -EOPNOTSUPP ;if (len !=1 ) {if (printk_ratelimit ())printk (KERN_ERR "PIC: non byte write\n" ); -return ; +return 0 ; }pic_lock (s);switch (addr) {
@@ -476,18 +477,21 @@ static void picdev_write(struct kvm_io_device *this,break ; }pic_unlock (s); +return 0 ; } -static void picdev_read (struct kvm_io_device *this, -gpa_t addr,int len,void *val) +static int picdev_read (struct kvm_io_device *this, +gpa_t addr,int len,void *val) {struct kvm_pic *s =to_pic (this);unsigned char data =0 ; +if (!picdev_in_range (addr)) +return -EOPNOTSUPP ;if (len !=1 ) {if (printk_ratelimit ())printk (KERN_ERR "PIC: non byte read\n" ); -return ; +return 0 ; }pic_lock (s);switch (addr) {
@@ -504,6 +508,7 @@ static void picdev_read(struct kvm_io_device *this, } *(unsigned char *)val = data;pic_unlock (s); +return 0 ; }/*
@@ -526,7 +531,6 @@ static void pic_irq_request(void *opaque, int level)static const struct kvm_io_device_ops picdev_ops = { .read = picdev_read, .write = picdev_write, - .in_range = picdev_in_range, };struct kvm_pic *kvm_create_pic (struct kvm *kvm)
arch/x86/kvm/lapic.c +20 -24
100 lines not on this page · raw message
arch/x86/kvm/x86.c +32 -78
201 lines not on this page · raw message
include/linux/kvm_host.h +4 -2
@@ -60,8 +60,10 @@ struct kvm_io_bus {void kvm_io_bus_init (struct kvm_io_bus *bus);void kvm_io_bus_destroy (struct kvm_io_bus *bus); -struct kvm_io_device *kvm_io_bus_find_dev (struct kvm_io_bus *bus, -gpa_t addr,int len,int is_write); +int kvm_io_bus_write (struct kvm_io_bus *bus,gpa_t addr,int len, +const void *val); +int kvm_io_bus_read (struct kvm_io_bus *bus,gpa_t addr,int len, +void *val);void __kvm_io_bus_register_dev (struct kvm_io_bus *bus,struct kvm_io_device *dev); void kvm_io_bus_register_dev(struct kvm *kvm, struct kvm_io_bus *bus,
virt/kvm/coalesced_mmio.c +7 -9
@@ -19,18 +19,14 @@ static inline struct kvm_coalesced_mmio_dev *to_mmio(struct kvm_io_device *dev) return container_of(dev, struct kvm_coalesced_mmio_dev, dev); } -static int coalesced_mmio_in_range (struct kvm_io_device *this, -gpa_t addr,int len,int is_write) +static int coalesced_mmio_in_range (struct kvm_coalesced_mmio_dev *dev, +gpa_t addr,int len) { -struct kvm_coalesced_mmio_dev *dev =to_mmio (this);struct kvm_coalesced_mmio_zone *zone;struct kvm_coalesced_mmio_ring *ring;unsigned avail;int i; -if (!is_write) -return 0 ; -/* Are we able to batch it ? */ /* last is the first free entry
@@ -60,11 +56,13 @@ static int coalesced_mmio_in_range(struct kvm_io_device *this,return 0 ; } -static void coalesced_mmio_write (struct kvm_io_device *this, -gpa_t addr,int len,const void *val) +static int coalesced_mmio_write (struct kvm_io_device *this, +gpa_t addr,int len,const void *val) {struct kvm_coalesced_mmio_dev *dev =to_mmio (this);struct kvm_coalesced_mmio_ring *ring = dev->kvm->coalesced_mmio_ring; +if (!coalesced_mmio_in_range (dev, addr, len)) +return -EOPNOTSUPP ;spin_lock (&dev->lock);
@@ -76,6 +74,7 @@ static void coalesced_mmio_write(struct kvm_io_device *this,smp_wmb (); ring->last = (ring->last +1 ) %KVM_COALESCED_MMIO_MAX ;spin_unlock (&dev->lock); +return 0 ; }static void coalesced_mmio_destructor (struct kvm_io_device *this)
@@ -87,7 +86,6 @@ static void coalesced_mmio_destructor(struct kvm_io_device *this)static const struct kvm_io_device_ops coalesced_mmio_ops = { .write = coalesced_mmio_write, - .in_range = coalesced_mmio_in_range, .destructor = coalesced_mmio_destructor, };
virt/kvm/ioapic.c +12 -10
@@ -227,20 +227,19 @@ static inline struct kvm_ioapic *to_ioapic(struct kvm_io_device *dev) return container_of(dev, struct kvm_ioapic, dev); } -static int ioapic_in_range (struct kvm_io_device *this,gpa_t addr, -int len,int is_write) +static inline int ioapic_in_range (struct kvm_ioapic *ioapic,gpa_t addr) { -struct kvm_ioapic *ioapic =to_ioapic (this); -return ((addr >= ioapic->base_address && (addr < ioapic->base_address +IOAPIC_MEM_LENGTH ))); } -static void ioapic_mmio_read (struct kvm_io_device *this,gpa_t addr,int len, -void *val) +static int ioapic_mmio_read (struct kvm_io_device *this,gpa_t addr,int len, +void *val) {struct kvm_ioapic *ioapic =to_ioapic (this);u32 result; +if (!ioapic_in_range (ioapic, addr)) +return -EOPNOTSUPP ;ioapic_debug ("addr %lx\n" , (unsigned long )addr);ASSERT (!(addr &0xf ));/* check alignment */
@@ -273,13 +272,16 @@ static void ioapic_mmio_read(struct kvm_io_device *this, gpa_t addr, int len,printk (KERN_WARNING "ioapic: wrong length %d\n" , len); }mutex_unlock (&ioapic->kvm->irq_lock); +return 0 ; } -static void ioapic_mmio_write (struct kvm_io_device *this,gpa_t addr,int len, -const void *val) +static int ioapic_mmio_write (struct kvm_io_device *this,gpa_t addr,int len, +const void *val) {struct kvm_ioapic *ioapic =to_ioapic (this);u32 data; +if (!ioapic_in_range (ioapic, addr)) +return -EOPNOTSUPP ;ioapic_debug ("ioapic_mmio_write addr=%p len=%d val=%p\n" , (void *)addr, len, val);
@@ -290,7 +292,7 @@ static void ioapic_mmio_write(struct kvm_io_device *this, gpa_t addr, int len, data = *(u32 *) val; else {printk (KERN_WARNING "ioapic: Unsupported size %d\n" , len); -return ; +return 0 ; } addr &=0xff ;
@@ -312,6 +314,7 @@ static void ioapic_mmio_write(struct kvm_io_device *this, gpa_t addr, int len,break ; }mutex_unlock (&ioapic->kvm->irq_lock); +return 0 ; }void kvm_ioapic_reset (struct kvm_ioapic *ioapic)
@@ -329,7 +332,6 @@ void kvm_ioapic_reset(struct kvm_ioapic *ioapic)static const struct kvm_io_device_ops ioapic_mmio_ops = { .read = ioapic_mmio_read, .write = ioapic_mmio_write, - .in_range = ioapic_in_range, };int kvm_ioapic_init (struct kvm *kvm)
virt/kvm/iodev.h +15 -24
@@ -17,23 +17,24 @@#define __KVM_IODEV_H__#include <linux/kvm_types.h> +#include <asm/errno.h> struct kvm_io_device ;/** * kvm_io_device_ops are called under kvm slots_lock. +* read and write handlers return 0 if the transaction has been handled, +* or non-zero to have it passed to the next device. **/ struct kvm_io_device_ops { -void (*read)(struct kvm_io_device *this, +int (*read)(struct kvm_io_device *this, +gpa_t addr, +int len, +void *val); +int (*write)(struct kvm_io_device *this,gpa_t addr,int len, -void *val); -void (*write)(struct kvm_io_device *this, -gpa_t addr, -int len, -const void *val); -int (*in_range)(struct kvm_io_device *this,gpa_t addr,int len, -int is_write); +const void *val);void (*destructor)(struct kvm_io_device *this); };
@@ -48,26 +49,16 @@ static inline void kvm_iodevice_init(struct kvm_io_device *dev, dev->ops = ops; } -static inline void kvm_iodevice_read (struct kvm_io_device *dev, -gpa_t addr, -int len, -void *val) +static inline int kvm_iodevice_read (struct kvm_io_device *dev, +gpa_t addr,int l,void *v) { - dev->ops->read (dev, addr, len, val); +return dev->ops->read ? dev->ops->read (dev, addr, l, v) : -EOPNOTSUPP ; } -static inline void kvm_iodevice_write (struct kvm_io_device *dev, -gpa_t addr, -int len, -const void *val) +static inline int kvm_iodevice_write (struct kvm_io_device *dev, +gpa_t addr,int l,const void *v) { - dev->ops->write (dev, addr, len, val); -} - -static inline int kvm_iodevice_in_range (struct kvm_io_device *dev, -gpa_t addr,int len,int is_write) -{ -return dev->ops->in_range (dev, addr, len, is_write); +return dev->ops->write ? dev->ops->write (dev, addr, l, v) : -EOPNOTSUPP ; }static inline void kvm_iodevice_destructor (struct kvm_io_device *dev)
virt/kvm/kvm_main.c +16 -10
@@ -2484,19 +2484,25 @@ void kvm_io_bus_destroy(struct kvm_io_bus *bus) } } -struct kvm_io_device *kvm_io_bus_find_dev (struct kvm_io_bus *bus, -gpa_t addr,int len,int is_write) +/* kvm_io_bus_write - called under kvm->slots_lock */ +int kvm_io_bus_write (struct kvm_io_bus *bus,gpa_t addr, +int len,const void *val) {int i; +for (i =0 ; i < bus->dev_count; i++) +if (!kvm_iodevice_write (bus->devs[i], addr, len, val)) +return 0 ; +return -EOPNOTSUPP ; +} -for (i =0 ; i < bus->dev_count; i++) { -struct kvm_io_device *pos = bus->devs[i]; - -if (kvm_iodevice_in_range (pos, addr, len, is_write)) -return pos; - } - -return NULL ; +/* kvm_io_bus_read - called under kvm->slots_lock */ +int kvm_io_bus_read (struct kvm_io_bus *bus,gpa_t addr,int len,void *val) +{ +int i; +for (i =0 ; i < bus->dev_count; i++) +if (!kvm_iodevice_read (bus->devs[i], addr, len, val)) +return 0 ; +return -EOPNOTSUPP ; } void kvm_io_bus_register_dev(struct kvm *kvm, struct kvm_io_bus *bus,
Signature · 1 line
1.6.2.2
PATCH v5 6/6 kvm: document lock nesting rule
In reply to a message not in the archiveDocument kvm->lock nesting within kvm->slots_lock
Signed-off-by Michael S. Tsirkin <[email protected]>
--- virt/kvm/kvm_main.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
virt/kvm/kvm_main.c +1 -1
@@ -68,7 +68,7 @@ MODULE_LICENSE("GPL");/* * Ordering of locks: * -* kvm->lock --> kvm->irq_lock +* kvm->slots_lock --> kvm->lock --> kvm->irq_lock */ DEFINE_SPINLOCK (kvm_lock);
Signature · 1 line
1.6.2.2