From: Jan Kiszka <jan.kiszka@web.de>
To: Blue Swirl <blauwirbel@gmail.com>
Cc: qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] [PATCH v3 1/4] vga: make PCI devices optional
Date: Mon, 17 Oct 2011 09:17:08 +0200 [thread overview]
Message-ID: <4E9BD674.5050302@web.de> (raw)
In-Reply-To: <CAAu8pHtsxw3v4OrOmOVjYsaHyEECJJfdcj2z0NV=UUe6b9Z2gA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 9846 bytes --]
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 <blauwirbel@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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
next prev parent reply other threads:[~2011-10-17 7:19 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-16 21:21 [Qemu-devel] [PATCH v3 1/4] vga: make PCI devices optional Blue Swirl
2011-10-17 7:17 ` Jan Kiszka [this message]
2011-10-17 19:09 ` Blue Swirl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E9BD674.5050302@web.de \
--to=jan.kiszka@web.de \
--cc=blauwirbel@gmail.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.