lkml.org 
[lkml]   [2009]   [Jun]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [KVM PATCH v8 1/3] KVM: make io_bus interface more robust
    Gregory Haskins wrote:
    > Today kvm_io_bus_regsiter_dev() returns void and will internally BUG_ON if it
    > fails. We want to create dynamic MMIO/PIO entries driven from userspace later
    > in the series, so we need to enhance the code to be more robust with the
    > following changes:
    >
    > 1) Add a return value to the registration function
    > 2) Fix up all the callsites to check the return code, handle any
    > failures, and percolate the error up to the caller.
    > 3) Add an unregister function that collapses holes in the array
    >
    > Signed-off-by: Gregory Haskins <ghaskins@novell.com>
    >

    Hi Avi,
    While the disposition on the iosignalfd patch w.r.t. current proposals
    by Michael is up in the air, this patch has not had and comments in a
    while. Please at least consider this one for merging ASAP so we can
    base some of the other io_bus cleanups on it.

    Thanks,
    -Greg

    > ---
    >
    > arch/x86/kvm/i8254.c | 22 ++++++++++++++++++++--
    > arch/x86/kvm/i8259.c | 9 ++++++++-
    > include/linux/kvm_host.h | 6 ++++--
    > virt/kvm/coalesced_mmio.c | 8 ++++++--
    > virt/kvm/ioapic.c | 9 +++++++--
    > virt/kvm/kvm_main.c | 23 +++++++++++++++++++++--
    > 6 files changed, 66 insertions(+), 11 deletions(-)
    >
    > diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
    > index 331705f..1c41715 100644
    > --- a/arch/x86/kvm/i8254.c
    > +++ b/arch/x86/kvm/i8254.c
    > @@ -586,6 +586,7 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags)
    > {
    > struct kvm_pit *pit;
    > struct kvm_kpit_state *pit_state;
    > + int ret;
    >
    > pit = kzalloc(sizeof(struct kvm_pit), GFP_KERNEL);
    > if (!pit)
    > @@ -620,14 +621,31 @@ 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);
    > + ret = kvm_io_bus_register_dev(&kvm->pio_bus, &pit->dev);
    > + if (ret < 0)
    > + goto fail;
    >
    > 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);
    > + ret = kvm_io_bus_register_dev(&kvm->pio_bus,
    > + &pit->speaker_dev);
    > + if (ret < 0)
    > + goto fail;
    > }
    >
    > return pit;
    > +
    > +fail:
    > + if (flags & KVM_PIT_SPEAKER_DUMMY)
    > + kvm_io_bus_unregister_dev(&kvm->pio_bus, &pit->speaker_dev);
    > +
    > + kvm_io_bus_unregister_dev(&kvm->pio_bus, &pit->dev);
    > +
    > + if (pit->irq_source_id >= 0)
    > + kvm_free_irq_source_id(kvm, pit->irq_source_id);
    > +
    > + kfree(pit);
    > + return NULL;
    > }
    >
    > void kvm_free_pit(struct kvm *kvm)
    > diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
    > index 148c52a..66e37c2 100644
    > --- a/arch/x86/kvm/i8259.c
    > +++ b/arch/x86/kvm/i8259.c
    > @@ -532,6 +532,8 @@ static const struct kvm_io_device_ops picdev_ops = {
    > struct kvm_pic *kvm_create_pic(struct kvm *kvm)
    > {
    > struct kvm_pic *s;
    > + int ret;
    > +
    > s = kzalloc(sizeof(struct kvm_pic), GFP_KERNEL);
    > if (!s)
    > return NULL;
    > @@ -548,6 +550,11 @@ 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);
    > + ret = kvm_io_bus_register_dev(&kvm->pio_bus, &s->dev);
    > + if (ret < 0) {
    > + kfree(s);
    > + return NULL;
    > + }
    > +
    > return s;
    > }
    > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
    > index e4e78db..eafa2b3 100644
    > --- a/include/linux/kvm_host.h
    > +++ b/include/linux/kvm_host.h
    > @@ -61,8 +61,10 @@ 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,
    > - struct kvm_io_device *dev);
    > +int kvm_io_bus_register_dev(struct kvm_io_bus *bus,
    > + struct kvm_io_device *dev);
    > +void kvm_io_bus_unregister_dev(struct kvm_io_bus *bus,
    > + struct kvm_io_device *dev);
    >
    > struct kvm_vcpu {
    > struct kvm *kvm;
    > diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c
    > index 397f419..3e89db8 100644
    > --- a/virt/kvm/coalesced_mmio.c
    > +++ b/virt/kvm/coalesced_mmio.c
    > @@ -94,6 +94,7 @@ static const struct kvm_io_device_ops coalesced_mmio_ops = {
    > int kvm_coalesced_mmio_init(struct kvm *kvm)
    > {
    > struct kvm_coalesced_mmio_dev *dev;
    > + int ret;
    >
    > dev = kzalloc(sizeof(struct kvm_coalesced_mmio_dev), GFP_KERNEL);
    > if (!dev)
    > @@ -102,9 +103,12 @@ 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);
    >
    > - return 0;
    > + ret = kvm_io_bus_register_dev(&kvm->mmio_bus, &dev->dev);
    > + if (ret < 0)
    > + kfree(dev);
    > +
    > + return ret;
    > }
    >
    > int kvm_vm_ioctl_register_coalesced_mmio(struct kvm *kvm,
    > diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
    > index d8b2eca..28adb05 100644
    > --- a/virt/kvm/ioapic.c
    > +++ b/virt/kvm/ioapic.c
    > @@ -335,6 +335,7 @@ static const struct kvm_io_device_ops ioapic_mmio_ops = {
    > int kvm_ioapic_init(struct kvm *kvm)
    > {
    > struct kvm_ioapic *ioapic;
    > + int ret;
    >
    > ioapic = kzalloc(sizeof(struct kvm_ioapic), GFP_KERNEL);
    > if (!ioapic)
    > @@ -343,7 +344,11 @@ 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);
    > - return 0;
    > +
    > + ret = kvm_io_bus_register_dev(&kvm->mmio_bus, &ioapic->dev);
    > + if (ret < 0)
    > + kfree(ioapic);
    > +
    > + return ret;
    > }
    >
    > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
    > index 9fab08e..6bd71f7 100644
    > --- a/virt/kvm/kvm_main.c
    > +++ b/virt/kvm/kvm_main.c
    > @@ -2481,11 +2481,30 @@ 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)
    > +/* assumes kvm->lock held */
    > +int kvm_io_bus_register_dev(struct kvm_io_bus *bus, struct kvm_io_device *dev)
    > {
    > - BUG_ON(bus->dev_count > (NR_IOBUS_DEVS-1));
    > + if (bus->dev_count > (NR_IOBUS_DEVS-1))
    > + return -ENOSPC;
    >
    > bus->devs[bus->dev_count++] = dev;
    > +
    > + return 0;
    > +}
    > +
    > +/* assumes kvm->lock held */
    > +void kvm_io_bus_unregister_dev(struct kvm_io_bus *bus,
    > + struct kvm_io_device *dev)
    > +{
    > + int i;
    > +
    > + for (i = 0; i < bus->dev_count; i++) {
    > +
    > + if (bus->devs[i] == dev) {
    > + bus->devs[i] = bus->devs[--bus->dev_count];
    > + return;
    > + }
    > + }
    > }
    >
    > static struct notifier_block kvm_cpu_notifier = {
    >
    > --
    > To unsubscribe from this list: send the line "unsubscribe kvm" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html
    >


    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2009-06-25 16:25    [W:5.846 / U:0.400 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site