| Message ID | 20200123134805.1993-17-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add reassignable BARs and PCIE 1.1 support | expand |
On Thu, 23 Jan 2020 13:47:51 +0000 Alexandru Elisei <alexandru.elisei@arm.com> wrote: Hi, > An error returned by device__register, kvm__register_mmio and > ioport__register means that the device will > not be emulated properly. Annotate the functions with __must_check, so we > get a compiler warning when this error is ignored. > > And fix several instances where the caller returns 0 even if the > function failed. > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> Looks alright, one minor nit below, with that fixed: Reviewed-by: Andre Przywara <andre.przywara@arm.com> > --- > arm/ioport.c | 3 +- > hw/i8042.c | 12 ++++++-- > hw/vesa.c | 4 ++- > include/kvm/devices.h | 3 +- > include/kvm/ioport.h | 6 ++-- > include/kvm/kvm.h | 6 ++-- > ioport.c | 23 ++++++++------- > mips/kvm.c | 3 +- > powerpc/ioport.c | 3 +- > virtio/mmio.c | 13 +++++++-- > x86/ioport.c | 66 ++++++++++++++++++++++++++++++++----------- > 11 files changed, 100 insertions(+), 42 deletions(-) > > diff --git a/arm/ioport.c b/arm/ioport.c > index bdd30b6fe812..2f0feb9ab69f 100644 > --- a/arm/ioport.c > +++ b/arm/ioport.c > @@ -1,8 +1,9 @@ > #include "kvm/ioport.h" > #include "kvm/irq.h" > > -void ioport__setup_arch(struct kvm *kvm) > +int ioport__setup_arch(struct kvm *kvm) > { > + return 0; > } > > void ioport__map_irq(u8 *irq) > diff --git a/hw/i8042.c b/hw/i8042.c > index 2d8c96e9c7e6..37a99a2dc6b8 100644 > --- a/hw/i8042.c > +++ b/hw/i8042.c > @@ -349,10 +349,18 @@ static struct ioport_operations kbd_ops = { > > int kbd__init(struct kvm *kvm) > { > + int r; > + > kbd_reset(); > state.kvm = kvm; > - ioport__register(kvm, I8042_DATA_REG, &kbd_ops, 2, NULL); > - ioport__register(kvm, I8042_COMMAND_REG, &kbd_ops, 2, NULL); > + r = ioport__register(kvm, I8042_DATA_REG, &kbd_ops, 2, NULL); > + if (r < 0) > + return r; > + r = ioport__register(kvm, I8042_COMMAND_REG, &kbd_ops, 2, NULL); > + if (r < 0) { > + ioport__unregister(kvm, I8042_DATA_REG); > + return r; > + } > > return 0; > } > diff --git a/hw/vesa.c b/hw/vesa.c > index d8d91aa9c873..b92cc990b730 100644 > --- a/hw/vesa.c > +++ b/hw/vesa.c > @@ -70,7 +70,9 @@ struct framebuffer *vesa__init(struct kvm *kvm) > > vesa_base_addr = (u16)r; > vesa_pci_device.bar[0] = cpu_to_le32(vesa_base_addr | PCI_BASE_ADDRESS_SPACE_IO); > - device__register(&vesa_device); > + r = device__register(&vesa_device); > + if (r < 0) > + return ERR_PTR(r); > > mem = mmap(NULL, VESA_MEM_SIZE, PROT_RW, MAP_ANON_NORESERVE, -1, 0); > if (mem == MAP_FAILED) > diff --git a/include/kvm/devices.h b/include/kvm/devices.h > index 405f19521977..e445db6f56b1 100644 > --- a/include/kvm/devices.h > +++ b/include/kvm/devices.h > @@ -3,6 +3,7 @@ > > #include <linux/rbtree.h> > #include <linux/types.h> > +#include <linux/compiler.h> > > enum device_bus_type { > DEVICE_BUS_PCI, > @@ -18,7 +19,7 @@ struct device_header { > struct rb_node node; > }; > > -int device__register(struct device_header *dev); > +int __must_check device__register(struct device_header *dev); > void device__unregister(struct device_header *dev); > struct device_header *device__find_dev(enum device_bus_type bus_type, > u8 dev_num); > diff --git a/include/kvm/ioport.h b/include/kvm/ioport.h > index 8c86b7151f25..62a719327e3f 100644 > --- a/include/kvm/ioport.h > +++ b/include/kvm/ioport.h > @@ -33,11 +33,11 @@ struct ioport_operations { > enum irq_type)); > }; > > -void ioport__setup_arch(struct kvm *kvm); > +int ioport__setup_arch(struct kvm *kvm); > void ioport__map_irq(u8 *irq); > > -int ioport__register(struct kvm *kvm, u16 port, struct ioport_operations *ops, > - int count, void *param); > +int __must_check ioport__register(struct kvm *kvm, u16 port, struct ioport_operations *ops, > + int count, void *param); > int ioport__unregister(struct kvm *kvm, u16 port); > int ioport__init(struct kvm *kvm); > int ioport__exit(struct kvm *kvm); > diff --git a/include/kvm/kvm.h b/include/kvm/kvm.h > index c6dc6ef72d11..50119a8672eb 100644 > --- a/include/kvm/kvm.h > +++ b/include/kvm/kvm.h > @@ -128,9 +128,9 @@ static inline int kvm__reserve_mem(struct kvm *kvm, u64 guest_phys, u64 size) > KVM_MEM_TYPE_RESERVED); > } > > -int kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool coalesce, > - void (*mmio_fn)(struct kvm_cpu *vcpu, u64 addr, u8 *data, u32 len, u8 is_write, void *ptr), > - void *ptr); > +int __must_check kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool coalesce, > + void (*mmio_fn)(struct kvm_cpu *vcpu, u64 addr, u8 *data, u32 len, u8 is_write, void *ptr), > + void *ptr); > bool kvm__deregister_mmio(struct kvm *kvm, u64 phys_addr); > void kvm__reboot(struct kvm *kvm); > void kvm__pause(struct kvm *kvm); > diff --git a/ioport.c b/ioport.c > index a72e4035881a..d224819c6e43 100644 > --- a/ioport.c > +++ b/ioport.c > @@ -91,16 +91,21 @@ int ioport__register(struct kvm *kvm, u16 port, struct ioport_operations *ops, i > }; > > r = ioport_insert(&ioport_tree, entry); > - if (r < 0) { > - free(entry); > - br_write_unlock(kvm); > - return r; > - } > - > - device__register(&entry->dev_hdr); > + if (r < 0) > + goto out_free; > + r = device__register(&entry->dev_hdr); > + if (r < 0) > + goto out_erase; > br_write_unlock(kvm); > > return port; > + > +out_erase: > + rb_int_erase(&ioport_tree, &entry->node); To keep the abstraction, shouldn't that rather be ioport_remove() instead? Cheers, Andre. > +out_free: > + free(entry); > + br_write_unlock(kvm); > + return r; > } > > int ioport__unregister(struct kvm *kvm, u16 port) > @@ -196,9 +201,7 @@ out: > > int ioport__init(struct kvm *kvm) > { > - ioport__setup_arch(kvm); > - > - return 0; > + return ioport__setup_arch(kvm); > } > dev_base_init(ioport__init); > > diff --git a/mips/kvm.c b/mips/kvm.c > index 211770da0d85..26355930d3b6 100644 > --- a/mips/kvm.c > +++ b/mips/kvm.c > @@ -100,8 +100,9 @@ void kvm__irq_trigger(struct kvm *kvm, int irq) > die_perror("KVM_IRQ_LINE ioctl"); > } > > -void ioport__setup_arch(struct kvm *kvm) > +int ioport__setup_arch(struct kvm *kvm) > { > + return 0; > } > > bool kvm__arch_cpu_supports_vm(void) > diff --git a/powerpc/ioport.c b/powerpc/ioport.c > index 58dc625c54fe..0c188b61a51a 100644 > --- a/powerpc/ioport.c > +++ b/powerpc/ioport.c > @@ -12,9 +12,10 @@ > > #include <stdlib.h> > > -void ioport__setup_arch(struct kvm *kvm) > +int ioport__setup_arch(struct kvm *kvm) > { > /* PPC has no legacy ioports to set up */ > + return 0; > } > > void ioport__map_irq(u8 *irq) > diff --git a/virtio/mmio.c b/virtio/mmio.c > index 03cecc366292..5537c39367d6 100644 > --- a/virtio/mmio.c > +++ b/virtio/mmio.c > @@ -292,13 +292,16 @@ int virtio_mmio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev, > int device_id, int subsys_id, int class) > { > struct virtio_mmio *vmmio = vdev->virtio; > + int r; > > vmmio->addr = virtio_mmio_get_io_space_block(VIRTIO_MMIO_IO_SIZE); > vmmio->kvm = kvm; > vmmio->dev = dev; > > - kvm__register_mmio(kvm, vmmio->addr, VIRTIO_MMIO_IO_SIZE, > - false, virtio_mmio_mmio_callback, vdev); > + r = kvm__register_mmio(kvm, vmmio->addr, VIRTIO_MMIO_IO_SIZE, > + false, virtio_mmio_mmio_callback, vdev); > + if (r < 0) > + return r; > > vmmio->hdr = (struct virtio_mmio_hdr) { > .magic = {'v', 'i', 'r', 't'}, > @@ -313,7 +316,11 @@ int virtio_mmio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev, > .data = generate_virtio_mmio_fdt_node, > }; > > - device__register(&vmmio->dev_hdr); > + r = device__register(&vmmio->dev_hdr); > + if (r < 0) { > + kvm__deregister_mmio(kvm, vmmio->addr); > + return r; > + } > > /* > * Instantiate guest virtio-mmio devices using kernel command line > diff --git a/x86/ioport.c b/x86/ioport.c > index 8572c758ed4f..7ad7b8f3f497 100644 > --- a/x86/ioport.c > +++ b/x86/ioport.c > @@ -69,50 +69,84 @@ void ioport__map_irq(u8 *irq) > { > } > > -void ioport__setup_arch(struct kvm *kvm) > +int ioport__setup_arch(struct kvm *kvm) > { > + int r; > + > /* Legacy ioport setup */ > > /* 0000 - 001F - DMA1 controller */ > - ioport__register(kvm, 0x0000, &dummy_read_write_ioport_ops, 32, NULL); > + r = ioport__register(kvm, 0x0000, &dummy_read_write_ioport_ops, 32, NULL); > + if (r < 0) > + return r; > > /* 0x0020 - 0x003F - 8259A PIC 1 */ > - ioport__register(kvm, 0x0020, &dummy_read_write_ioport_ops, 2, NULL); > + r = ioport__register(kvm, 0x0020, &dummy_read_write_ioport_ops, 2, NULL); > + if (r < 0) > + return r; > > /* PORT 0040-005F - PIT - PROGRAMMABLE INTERVAL TIMER (8253, 8254) */ > - ioport__register(kvm, 0x0040, &dummy_read_write_ioport_ops, 4, NULL); > + r = ioport__register(kvm, 0x0040, &dummy_read_write_ioport_ops, 4, NULL); > + if (r < 0) > + return r; > > /* 0092 - PS/2 system control port A */ > - ioport__register(kvm, 0x0092, &ps2_control_a_ops, 1, NULL); > + r = ioport__register(kvm, 0x0092, &ps2_control_a_ops, 1, NULL); > + if (r < 0) > + return r; > > /* 0x00A0 - 0x00AF - 8259A PIC 2 */ > - ioport__register(kvm, 0x00A0, &dummy_read_write_ioport_ops, 2, NULL); > + r = ioport__register(kvm, 0x00A0, &dummy_read_write_ioport_ops, 2, NULL); > + if (r < 0) > + return r; > > /* 00C0 - 001F - DMA2 controller */ > - ioport__register(kvm, 0x00C0, &dummy_read_write_ioport_ops, 32, NULL); > + r = ioport__register(kvm, 0x00C0, &dummy_read_write_ioport_ops, 32, NULL); > + if (r < 0) > + return r; > > /* PORT 00E0-00EF are 'motherboard specific' so we use them for our > internal debugging purposes. */ > - ioport__register(kvm, IOPORT_DBG, &debug_ops, 1, NULL); > + r = ioport__register(kvm, IOPORT_DBG, &debug_ops, 1, NULL); > + if (r < 0) > + return r; > > /* PORT 00ED - DUMMY PORT FOR DELAY??? */ > - ioport__register(kvm, 0x00ED, &dummy_write_only_ioport_ops, 1, NULL); > + r = ioport__register(kvm, 0x00ED, &dummy_write_only_ioport_ops, 1, NULL); > + if (r < 0) > + return r; > > /* 0x00F0 - 0x00FF - Math co-processor */ > - ioport__register(kvm, 0x00F0, &dummy_write_only_ioport_ops, 2, NULL); > + r = ioport__register(kvm, 0x00F0, &dummy_write_only_ioport_ops, 2, NULL); > + if (r < 0) > + return r; > > /* PORT 0278-027A - PARALLEL PRINTER PORT (usually LPT1, sometimes LPT2) */ > - ioport__register(kvm, 0x0278, &dummy_read_write_ioport_ops, 3, NULL); > + r = ioport__register(kvm, 0x0278, &dummy_read_write_ioport_ops, 3, NULL); > + if (r < 0) > + return r; > > /* PORT 0378-037A - PARALLEL PRINTER PORT (usually LPT2, sometimes LPT3) */ > - ioport__register(kvm, 0x0378, &dummy_read_write_ioport_ops, 3, NULL); > + r = ioport__register(kvm, 0x0378, &dummy_read_write_ioport_ops, 3, NULL); > + if (r < 0) > + return r; > > /* PORT 03D4-03D5 - COLOR VIDEO - CRT CONTROL REGISTERS */ > - ioport__register(kvm, 0x03D4, &dummy_read_write_ioport_ops, 1, NULL); > - ioport__register(kvm, 0x03D5, &dummy_write_only_ioport_ops, 1, NULL); > + r = ioport__register(kvm, 0x03D4, &dummy_read_write_ioport_ops, 1, NULL); > + if (r < 0) > + return r; > + r = ioport__register(kvm, 0x03D5, &dummy_write_only_ioport_ops, 1, NULL); > + if (r < 0) > + return r; > > - ioport__register(kvm, 0x402, &seabios_debug_ops, 1, NULL); > + r = ioport__register(kvm, 0x402, &seabios_debug_ops, 1, NULL); > + if (r < 0) > + return r; > > /* 0510 - QEMU BIOS configuration register */ > - ioport__register(kvm, 0x510, &dummy_read_write_ioport_ops, 2, NULL); > + r = ioport__register(kvm, 0x510, &dummy_read_write_ioport_ops, 2, NULL); > + if (r < 0) > + return r; > + > + return 0; > }
Hi, On 1/30/20 2:51 PM, Andre Przywara wrote: > On Thu, 23 Jan 2020 13:47:51 +0000 > Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > Hi, > >> An error returned by device__register, kvm__register_mmio and >> ioport__register means that the device will >> not be emulated properly. Annotate the functions with __must_check, so we >> get a compiler warning when this error is ignored. >> >> And fix several instances where the caller returns 0 even if the >> function failed. >> >> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > Looks alright, one minor nit below, with that fixed: > > Reviewed-by: Andre Przywara <andre.przywara@arm.com> [..] >> diff --git a/ioport.c b/ioport.c >> index a72e4035881a..d224819c6e43 100644 >> --- a/ioport.c >> +++ b/ioport.c >> @@ -91,16 +91,21 @@ int ioport__register(struct kvm *kvm, u16 port, struct ioport_operations *ops, i >> }; >> >> r = ioport_insert(&ioport_tree, entry); >> - if (r < 0) { >> - free(entry); >> - br_write_unlock(kvm); >> - return r; >> - } >> - >> - device__register(&entry->dev_hdr); >> + if (r < 0) >> + goto out_free; >> + r = device__register(&entry->dev_hdr); >> + if (r < 0) >> + goto out_erase; >> br_write_unlock(kvm); >> >> return port; >> + >> +out_erase: >> + rb_int_erase(&ioport_tree, &entry->node); > To keep the abstraction, shouldn't that rather be ioport_remove() instead? ioport__register already uses rb_int_erase to remove a node (at the beginning, if the requested port is already allocated). But you're right, it should use ioport_remove in both cases, like ioport__unregister{,_all} does. Thanks, Alex
diff --git a/arm/ioport.c b/arm/ioport.c index bdd30b6fe812..2f0feb9ab69f 100644 --- a/arm/ioport.c +++ b/arm/ioport.c @@ -1,8 +1,9 @@ #include "kvm/ioport.h" #include "kvm/irq.h" -void ioport__setup_arch(struct kvm *kvm) +int ioport__setup_arch(struct kvm *kvm) { + return 0; } void ioport__map_irq(u8 *irq) diff --git a/hw/i8042.c b/hw/i8042.c index 2d8c96e9c7e6..37a99a2dc6b8 100644 --- a/hw/i8042.c +++ b/hw/i8042.c @@ -349,10 +349,18 @@ static struct ioport_operations kbd_ops = { int kbd__init(struct kvm *kvm) { + int r; + kbd_reset(); state.kvm = kvm; - ioport__register(kvm, I8042_DATA_REG, &kbd_ops, 2, NULL); - ioport__register(kvm, I8042_COMMAND_REG, &kbd_ops, 2, NULL); + r = ioport__register(kvm, I8042_DATA_REG, &kbd_ops, 2, NULL); + if (r < 0) + return r; + r = ioport__register(kvm, I8042_COMMAND_REG, &kbd_ops, 2, NULL); + if (r < 0) { + ioport__unregister(kvm, I8042_DATA_REG); + return r; + } return 0; } diff --git a/hw/vesa.c b/hw/vesa.c index d8d91aa9c873..b92cc990b730 100644 --- a/hw/vesa.c +++ b/hw/vesa.c @@ -70,7 +70,9 @@ struct framebuffer *vesa__init(struct kvm *kvm) vesa_base_addr = (u16)r; vesa_pci_device.bar[0] = cpu_to_le32(vesa_base_addr | PCI_BASE_ADDRESS_SPACE_IO); - device__register(&vesa_device); + r = device__register(&vesa_device); + if (r < 0) + return ERR_PTR(r); mem = mmap(NULL, VESA_MEM_SIZE, PROT_RW, MAP_ANON_NORESERVE, -1, 0); if (mem == MAP_FAILED) diff --git a/include/kvm/devices.h b/include/kvm/devices.h index 405f19521977..e445db6f56b1 100644 --- a/include/kvm/devices.h +++ b/include/kvm/devices.h @@ -3,6 +3,7 @@ #include <linux/rbtree.h> #include <linux/types.h> +#include <linux/compiler.h> enum device_bus_type { DEVICE_BUS_PCI, @@ -18,7 +19,7 @@ struct device_header { struct rb_node node; }; -int device__register(struct device_header *dev); +int __must_check device__register(struct device_header *dev); void device__unregister(struct device_header *dev); struct device_header *device__find_dev(enum device_bus_type bus_type, u8 dev_num); diff --git a/include/kvm/ioport.h b/include/kvm/ioport.h index 8c86b7151f25..62a719327e3f 100644 --- a/include/kvm/ioport.h +++ b/include/kvm/ioport.h @@ -33,11 +33,11 @@ struct ioport_operations { enum irq_type)); }; -void ioport__setup_arch(struct kvm *kvm); +int ioport__setup_arch(struct kvm *kvm); void ioport__map_irq(u8 *irq); -int ioport__register(struct kvm *kvm, u16 port, struct ioport_operations *ops, - int count, void *param); +int __must_check ioport__register(struct kvm *kvm, u16 port, struct ioport_operations *ops, + int count, void *param); int ioport__unregister(struct kvm *kvm, u16 port); int ioport__init(struct kvm *kvm); int ioport__exit(struct kvm *kvm); diff --git a/include/kvm/kvm.h b/include/kvm/kvm.h index c6dc6ef72d11..50119a8672eb 100644 --- a/include/kvm/kvm.h +++ b/include/kvm/kvm.h @@ -128,9 +128,9 @@ static inline int kvm__reserve_mem(struct kvm *kvm, u64 guest_phys, u64 size) KVM_MEM_TYPE_RESERVED); } -int kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool coalesce, - void (*mmio_fn)(struct kvm_cpu *vcpu, u64 addr, u8 *data, u32 len, u8 is_write, void *ptr), - void *ptr); +int __must_check kvm__register_mmio(struct kvm *kvm, u64 phys_addr, u64 phys_addr_len, bool coalesce, + void (*mmio_fn)(struct kvm_cpu *vcpu, u64 addr, u8 *data, u32 len, u8 is_write, void *ptr), + void *ptr); bool kvm__deregister_mmio(struct kvm *kvm, u64 phys_addr); void kvm__reboot(struct kvm *kvm); void kvm__pause(struct kvm *kvm); diff --git a/ioport.c b/ioport.c index a72e4035881a..d224819c6e43 100644 --- a/ioport.c +++ b/ioport.c @@ -91,16 +91,21 @@ int ioport__register(struct kvm *kvm, u16 port, struct ioport_operations *ops, i }; r = ioport_insert(&ioport_tree, entry); - if (r < 0) { - free(entry); - br_write_unlock(kvm); - return r; - } - - device__register(&entry->dev_hdr); + if (r < 0) + goto out_free; + r = device__register(&entry->dev_hdr); + if (r < 0) + goto out_erase; br_write_unlock(kvm); return port; + +out_erase: + rb_int_erase(&ioport_tree, &entry->node); +out_free: + free(entry); + br_write_unlock(kvm); + return r; } int ioport__unregister(struct kvm *kvm, u16 port) @@ -196,9 +201,7 @@ out: int ioport__init(struct kvm *kvm) { - ioport__setup_arch(kvm); - - return 0; + return ioport__setup_arch(kvm); } dev_base_init(ioport__init); diff --git a/mips/kvm.c b/mips/kvm.c index 211770da0d85..26355930d3b6 100644 --- a/mips/kvm.c +++ b/mips/kvm.c @@ -100,8 +100,9 @@ void kvm__irq_trigger(struct kvm *kvm, int irq) die_perror("KVM_IRQ_LINE ioctl"); } -void ioport__setup_arch(struct kvm *kvm) +int ioport__setup_arch(struct kvm *kvm) { + return 0; } bool kvm__arch_cpu_supports_vm(void) diff --git a/powerpc/ioport.c b/powerpc/ioport.c index 58dc625c54fe..0c188b61a51a 100644 --- a/powerpc/ioport.c +++ b/powerpc/ioport.c @@ -12,9 +12,10 @@ #include <stdlib.h> -void ioport__setup_arch(struct kvm *kvm) +int ioport__setup_arch(struct kvm *kvm) { /* PPC has no legacy ioports to set up */ + return 0; } void ioport__map_irq(u8 *irq) diff --git a/virtio/mmio.c b/virtio/mmio.c index 03cecc366292..5537c39367d6 100644 --- a/virtio/mmio.c +++ b/virtio/mmio.c @@ -292,13 +292,16 @@ int virtio_mmio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev, int device_id, int subsys_id, int class) { struct virtio_mmio *vmmio = vdev->virtio; + int r; vmmio->addr = virtio_mmio_get_io_space_block(VIRTIO_MMIO_IO_SIZE); vmmio->kvm = kvm; vmmio->dev = dev; - kvm__register_mmio(kvm, vmmio->addr, VIRTIO_MMIO_IO_SIZE, - false, virtio_mmio_mmio_callback, vdev); + r = kvm__register_mmio(kvm, vmmio->addr, VIRTIO_MMIO_IO_SIZE, + false, virtio_mmio_mmio_callback, vdev); + if (r < 0) + return r; vmmio->hdr = (struct virtio_mmio_hdr) { .magic = {'v', 'i', 'r', 't'}, @@ -313,7 +316,11 @@ int virtio_mmio_init(struct kvm *kvm, void *dev, struct virtio_device *vdev, .data = generate_virtio_mmio_fdt_node, }; - device__register(&vmmio->dev_hdr); + r = device__register(&vmmio->dev_hdr); + if (r < 0) { + kvm__deregister_mmio(kvm, vmmio->addr); + return r; + } /* * Instantiate guest virtio-mmio devices using kernel command line diff --git a/x86/ioport.c b/x86/ioport.c index 8572c758ed4f..7ad7b8f3f497 100644 --- a/x86/ioport.c +++ b/x86/ioport.c @@ -69,50 +69,84 @@ void ioport__map_irq(u8 *irq) { } -void ioport__setup_arch(struct kvm *kvm) +int ioport__setup_arch(struct kvm *kvm) { + int r; + /* Legacy ioport setup */ /* 0000 - 001F - DMA1 controller */ - ioport__register(kvm, 0x0000, &dummy_read_write_ioport_ops, 32, NULL); + r = ioport__register(kvm, 0x0000, &dummy_read_write_ioport_ops, 32, NULL); + if (r < 0) + return r; /* 0x0020 - 0x003F - 8259A PIC 1 */ - ioport__register(kvm, 0x0020, &dummy_read_write_ioport_ops, 2, NULL); + r = ioport__register(kvm, 0x0020, &dummy_read_write_ioport_ops, 2, NULL); + if (r < 0) + return r; /* PORT 0040-005F - PIT - PROGRAMMABLE INTERVAL TIMER (8253, 8254) */ - ioport__register(kvm, 0x0040, &dummy_read_write_ioport_ops, 4, NULL); + r = ioport__register(kvm, 0x0040, &dummy_read_write_ioport_ops, 4, NULL); + if (r < 0) + return r; /* 0092 - PS/2 system control port A */ - ioport__register(kvm, 0x0092, &ps2_control_a_ops, 1, NULL); + r = ioport__register(kvm, 0x0092, &ps2_control_a_ops, 1, NULL); + if (r < 0) + return r; /* 0x00A0 - 0x00AF - 8259A PIC 2 */ - ioport__register(kvm, 0x00A0, &dummy_read_write_ioport_ops, 2, NULL); + r = ioport__register(kvm, 0x00A0, &dummy_read_write_ioport_ops, 2, NULL); + if (r < 0) + return r; /* 00C0 - 001F - DMA2 controller */ - ioport__register(kvm, 0x00C0, &dummy_read_write_ioport_ops, 32, NULL); + r = ioport__register(kvm, 0x00C0, &dummy_read_write_ioport_ops, 32, NULL); + if (r < 0) + return r; /* PORT 00E0-00EF are 'motherboard specific' so we use them for our internal debugging purposes. */ - ioport__register(kvm, IOPORT_DBG, &debug_ops, 1, NULL); + r = ioport__register(kvm, IOPORT_DBG, &debug_ops, 1, NULL); + if (r < 0) + return r; /* PORT 00ED - DUMMY PORT FOR DELAY??? */ - ioport__register(kvm, 0x00ED, &dummy_write_only_ioport_ops, 1, NULL); + r = ioport__register(kvm, 0x00ED, &dummy_write_only_ioport_ops, 1, NULL); + if (r < 0) + return r; /* 0x00F0 - 0x00FF - Math co-processor */ - ioport__register(kvm, 0x00F0, &dummy_write_only_ioport_ops, 2, NULL); + r = ioport__register(kvm, 0x00F0, &dummy_write_only_ioport_ops, 2, NULL); + if (r < 0) + return r; /* PORT 0278-027A - PARALLEL PRINTER PORT (usually LPT1, sometimes LPT2) */ - ioport__register(kvm, 0x0278, &dummy_read_write_ioport_ops, 3, NULL); + r = ioport__register(kvm, 0x0278, &dummy_read_write_ioport_ops, 3, NULL); + if (r < 0) + return r; /* PORT 0378-037A - PARALLEL PRINTER PORT (usually LPT2, sometimes LPT3) */ - ioport__register(kvm, 0x0378, &dummy_read_write_ioport_ops, 3, NULL); + r = ioport__register(kvm, 0x0378, &dummy_read_write_ioport_ops, 3, NULL); + if (r < 0) + return r; /* PORT 03D4-03D5 - COLOR VIDEO - CRT CONTROL REGISTERS */ - ioport__register(kvm, 0x03D4, &dummy_read_write_ioport_ops, 1, NULL); - ioport__register(kvm, 0x03D5, &dummy_write_only_ioport_ops, 1, NULL); + r = ioport__register(kvm, 0x03D4, &dummy_read_write_ioport_ops, 1, NULL); + if (r < 0) + return r; + r = ioport__register(kvm, 0x03D5, &dummy_write_only_ioport_ops, 1, NULL); + if (r < 0) + return r; - ioport__register(kvm, 0x402, &seabios_debug_ops, 1, NULL); + r = ioport__register(kvm, 0x402, &seabios_debug_ops, 1, NULL); + if (r < 0) + return r; /* 0510 - QEMU BIOS configuration register */ - ioport__register(kvm, 0x510, &dummy_read_write_ioport_ops, 2, NULL); + r = ioport__register(kvm, 0x510, &dummy_read_write_ioport_ops, 2, NULL); + if (r < 0) + return r; + + return 0; }
An error returned by device__register, kvm__register_mmio and ioport__register means that the device will not be emulated properly. Annotate the functions with __must_check, so we get a compiler warning when this error is ignored. And fix several instances where the caller returns 0 even if the function failed. Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- arm/ioport.c | 3 +- hw/i8042.c | 12 ++++++-- hw/vesa.c | 4 ++- include/kvm/devices.h | 3 +- include/kvm/ioport.h | 6 ++-- include/kvm/kvm.h | 6 ++-- ioport.c | 23 ++++++++------- mips/kvm.c | 3 +- powerpc/ioport.c | 3 +- virtio/mmio.c | 13 +++++++-- x86/ioport.c | 66 ++++++++++++++++++++++++++++++++----------- 11 files changed, 100 insertions(+), 42 deletions(-)