[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/4] vga: make PCI devices optional
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/4] vga: make PCI devices optional |
Date: |
Mon, 17 Oct 2011 09:17:08 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
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 <address@hidden>
> ---
> 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