On 2011-10-16 23:21, Blue Swirl wrote: > Improve VGA selection logic, push check for device availabilty to vl.c. > Make PCI VGA devices optional. > > Signed-off-by: Blue Swirl <blauwir...@gmail.com> > --- > hw/cirrus_vga.c | 5 ----- > hw/pc.c | 6 +----- > hw/pc.h | 33 +++++++++++++++++++++++++++------ > hw/pci.c | 18 ++++++++++++++++++ > hw/pci.h | 4 ++++ > hw/qdev.c | 5 +++++ > hw/qdev.h | 1 + > hw/vga-pci.c | 6 ------ > vl.c | 33 +++++++++++++++++++++++++++------ > 9 files changed, 83 insertions(+), 28 deletions(-) > > diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c > index c7e365b..a11444c 100644 > --- a/hw/cirrus_vga.c > +++ b/hw/cirrus_vga.c > @@ -2955,11 +2955,6 @@ static int pci_cirrus_vga_initfn(PCIDevice *dev) > return 0; > } > > -void pci_cirrus_vga_init(PCIBus *bus) > -{ > - pci_create_simple(bus, -1, "cirrus-vga"); > -} > - > static PCIDeviceInfo cirrus_vga_info = { > .qdev.name = "cirrus-vga", > .qdev.desc = "Cirrus CLGD 54xx VGA", > diff --git a/hw/pc.c b/hw/pc.c > index f0802b7..057eb9c 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -1080,11 +1080,7 @@ void pc_vga_init(PCIBus *pci_bus) > } > } else if (vmsvga_enabled) { > if (pci_bus) { > - if (!pci_vmsvga_init(pci_bus)) { > - fprintf(stderr, "Warning: vmware_vga not available," > - " using standard VGA instead\n"); > - pci_vga_init(pci_bus); > - } > + pci_vmsvga_init(pci_bus); > } else { > fprintf(stderr, "%s: vmware_vga: no PCI bus\n", __FUNCTION__); > } > diff --git a/hw/pc.h b/hw/pc.h > index b8ad9a3..6c951e8 100644 > --- a/hw/pc.h > +++ b/hw/pc.h > @@ -9,6 +9,7 @@ > #include "net.h" > #include "memory.h" > #include "ioapic.h" > +#include "pci.h" > > /* PC-style peripherals (also used by other machines). */ > > @@ -203,26 +204,46 @@ enum vga_retrace_method { > > extern enum vga_retrace_method vga_retrace_method; > > -static inline int isa_vga_init(void) > +static inline bool isa_vga_init(void) > { > ISADevice *dev; > > dev = isa_try_create("isa-vga"); > if (!dev) { > - fprintf(stderr, "Warning: isa-vga not available\n"); > - return 0; > + return false; > } > qdev_init_nofail(&dev->qdev); > - return 1; > + return true; > +} > + > +/* vga-pci.c */ > +static inline bool pci_vga_init(PCIBus *bus) > +{ > + PCIDevice *dev; > + > + dev = pci_try_create_simple(bus, -1, "VGA"); > + if (!dev) { > + return false; > + } > + return true; > } > > -int pci_vga_init(PCIBus *bus); > int isa_vga_mm_init(target_phys_addr_t vram_base, > target_phys_addr_t ctrl_base, int it_shift, > MemoryRegion *address_space); > > /* cirrus_vga.c */ > -void pci_cirrus_vga_init(PCIBus *bus); > +static inline bool pci_cirrus_vga_init(PCIBus *bus) > +{ > + PCIDevice *dev; > + > + dev = pci_try_create_simple(bus, -1, "cirrus-vga"); > + if (!dev) { > + return false; > + } > + return true; > +} > + > void isa_cirrus_vga_init(MemoryRegion *address_space); > > /* ne2000.c */ > diff --git a/hw/pci.c b/hw/pci.c > index 749e8d8..46c01ac 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -1687,6 +1687,19 @@ PCIDevice > *pci_create_simple_multifunction(PCIBus *bus, int devfn, > return dev; > } > > +PCIDevice *pci_try_create_simple_multifunction(PCIBus *bus, int devfn, > + bool multifunction, > + const char *name) > +{ > + PCIDevice *dev = pci_try_create_multifunction(bus, devfn, multifunction, > + name); > + if (!dev) { > + return NULL; > + } > + qdev_init_nofail(&dev->qdev); > + return dev; > +} > + > PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name) > { > return pci_create_multifunction(bus, devfn, false, name); > @@ -1702,6 +1715,11 @@ PCIDevice *pci_try_create(PCIBus *bus, int > devfn, const char *name) > return pci_try_create_multifunction(bus, devfn, false, name); > } > > +PCIDevice *pci_try_create_simple(PCIBus *bus, int devfn, const char *name) > +{ > + return pci_try_create_simple_multifunction(bus, devfn, false, name); > +} > + > static int pci_find_space(PCIDevice *pdev, uint8_t size) > { > int config_size = pci_config_size(pdev); > diff --git a/hw/pci.h b/hw/pci.h > index 86a81c8..aa2e040 100644 > --- a/hw/pci.h > +++ b/hw/pci.h > @@ -473,9 +473,13 @@ PCIDevice *pci_create_simple_multifunction(PCIBus > *bus, int devfn, > PCIDevice *pci_try_create_multifunction(PCIBus *bus, int devfn, > bool multifunction, > const char *name); > +PCIDevice *pci_try_create_simple_multifunction(PCIBus *bus, int devfn, > + bool multifunction, > + const char *name); > PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name); > PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name); > PCIDevice *pci_try_create(PCIBus *bus, int devfn, const char *name); > +PCIDevice *pci_try_create_simple(PCIBus *bus, int devfn, const char *name); > > static inline int pci_is_express(const PCIDevice *d) > { > diff --git a/hw/qdev.c b/hw/qdev.c > index a223d41..dc0aa1c 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -80,6 +80,11 @@ static DeviceInfo *qdev_find_info(BusInfo > *bus_info, const char *name) > return NULL; > } > > +bool qdev_exists(const char *name) > +{ > + return !!qdev_find_info(NULL, name); > +} > + > static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info) > { > DeviceState *dev; > diff --git a/hw/qdev.h b/hw/qdev.h > index aa7ae36..a53ccb1 100644 > --- a/hw/qdev.h > +++ b/hw/qdev.h > @@ -123,6 +123,7 @@ typedef struct GlobalProperty { > > DeviceState *qdev_create(BusState *bus, const char *name); > DeviceState *qdev_try_create(BusState *bus, const char *name); > +bool qdev_exists(const char *name); > int qdev_device_help(QemuOpts *opts); > DeviceState *qdev_device_add(QemuOpts *opts); > int qdev_init(DeviceState *dev) QEMU_WARN_UNUSED_RESULT; > diff --git a/hw/vga-pci.c b/hw/vga-pci.c > index 14bfadb..68fddd9 100644 > --- a/hw/vga-pci.c > +++ b/hw/vga-pci.c > @@ -70,12 +70,6 @@ static int pci_vga_initfn(PCIDevice *dev) > return 0; > } > > -int pci_vga_init(PCIBus *bus) > -{ > - pci_create_simple(bus, -1, "VGA"); > - return 0; > -} > - > static PCIDeviceInfo vga_info = { > .qdev.name = "VGA", > .qdev.size = sizeof(PCIVGAState), > diff --git a/vl.c b/vl.c > index 2dce3ae..15fb0d1 100644 > --- a/vl.c > +++ b/vl.c > @@ -1654,11 +1654,26 @@ static void select_vgahw (const char *p) > default_vga = 0; > vga_interface_type = VGA_NONE; > if (strstart(p, "std", &opts)) { > - vga_interface_type = VGA_STD; > + if (qdev_exists("VGA") || qdev_exists("isa-vga")) { > + vga_interface_type = VGA_STD; > + } else { > + fprintf(stderr, "Error: standard VGA not available\n"); > + exit(0); > + } > } else if (strstart(p, "cirrus", &opts)) { > - vga_interface_type = VGA_CIRRUS; > + if (qdev_exists("cirrus-vga") || qdev_exists("isa-cirrus-vga")) { > + vga_interface_type = VGA_CIRRUS; > + } else { > + fprintf(stderr, "Error: Cirrus VGA not available\n"); > + exit(0); > + } > } else if (strstart(p, "vmware", &opts)) { > - vga_interface_type = VGA_VMWARE; > + if (qdev_exists("vmware-svga")) { > + vga_interface_type = VGA_VMWARE; > + } else { > + fprintf(stderr, "Error: VMWare SVGA not available\n"); > + exit(0); > + } > } else if (strstart(p, "xenfb", &opts)) { > vga_interface_type = VGA_XENFB; > } else if (strstart(p, "qxl", &opts)) { > @@ -2277,6 +2292,7 @@ int main(int argc, char **argv, char **envp) > const char *loadvm = NULL; > QEMUMachine *machine; > const char *cpu_model; > + const char *vga_model = NULL; > const char *pid_file = NULL; > const char *incoming = NULL; > #ifdef CONFIG_VNC > @@ -2694,7 +2710,7 @@ int main(int argc, char **argv, char **envp) > rtc_utc = 0; > break; > case QEMU_OPTION_vga: > - select_vgahw (optarg); > + vga_model = optarg; > break; > case QEMU_OPTION_g: > { > @@ -3252,8 +3268,6 @@ int main(int argc, char **argv, char **envp) > if (default_virtcon) > add_device_config(DEV_VIRTCON, "vc:80Cx24C"); > } > - if (default_vga) > - vga_interface_type = VGA_CIRRUS; > > socket_init(); > > @@ -3398,6 +3412,13 @@ int main(int argc, char **argv, char **envp) > > module_call_init(MODULE_INIT_DEVICE); > > + /* must be after qdev registration but before machine init */ > + if (vga_model) { > + select_vgahw(vga_model); > + } else if (default_vga) { > + select_vgahw("cirrus"); > + }
Nice change, except maybe for this detail: If the default model is disabled, we will bail out, no? Would it make sense to handle this case gracefully as well? Jan
signature.asc
Description: OpenPGP digital signature