Re: [PATCH] Add an isa device for SGA

[Markus Armbruster <armbru@redhat.com>]

Glauber Costa <glommer@redhat.com> writes:

> This patch adds a dummy legacy ISA device whose responsibility is to
> deploy sgabios, an option rom for a serial graphics adapter.
> The proposal is that this device is always-on when -nographics,
> but can otherwise be enable in any setup when -device sga is used.
>
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  Makefile.target |    2 +-
>  hw/pc.c         |    2 ++
>  hw/pc.h         |    3 +++
>  hw/sga.c        |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 55 insertions(+), 1 deletions(-)
>  create mode 100644 hw/sga.c
>
> diff --git a/Makefile.target b/Makefile.target
> index fdbdc6c..004ea7e 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -224,7 +224,7 @@ obj-$(CONFIG_KVM) += ivshmem.o
>  # Hardware support
>  obj-i386-y += vga.o
>  obj-i386-y += mc146818rtc.o i8259.o pc.o
> -obj-i386-y += cirrus_vga.o apic.o ioapic.o piix_pci.o
> +obj-i386-y += cirrus_vga.o sga.o apic.o ioapic.o piix_pci.o
>  obj-i386-y += vmport.o
>  obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>  obj-i386-y += extboot.o
> diff --git a/hw/pc.c b/hw/pc.c
> index 8d351ba..56c3887 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1096,6 +1096,8 @@ void pc_vga_init(PCIBus *pci_bus)
>              isa_vga_init();
>          }
>      }
> +
> +    isa_sga_init();


Please do this the qdev way (untested):

    if (display_type == DT_NOGRAPHIC) {
        dev = qdev_create(NULL, "sga");
        qdev_init_nofail(dev);
    }

>  }
>  
>  static void cpu_request_exit(void *opaque, int irq, int level)
> diff --git a/hw/pc.h b/hw/pc.h
> index 1291e2d..a00e054 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -219,6 +219,9 @@ int isa_vga_mm_init(target_phys_addr_t vram_base,
>  void pci_cirrus_vga_init(PCIBus *bus);
>  void isa_cirrus_vga_init(void);
>  
> +/* serial graphics */
> +void isa_sga_init(void);
> +
>  /* ne2000.c */
>  static inline bool isa_ne2000_init(int base, int irq, NICInfo *nd)
>  {
> diff --git a/hw/sga.c b/hw/sga.c
> new file mode 100644
> index 0000000..411191b
> --- /dev/null
> +++ b/hw/sga.c
> @@ -0,0 +1,49 @@
> +#include "pci.h"
> +#include "pc.h"
> +#include "loader.h"
> +#include "sysemu.h"
> +
> +#define SGABIOS_FILENAME "sgabios.bin"
> +
> +typedef struct ISAGAState {
> +    ISADevice dev;
> +} ISASGAState;
> +
> +/* We can have both -device, and the initfn called, so better
> + * avoid to have the rom loaded twice */
> +static void deploy_rom(void)
> +{
> +    static int rom_deployed = 0;
> +
> +    if (!rom_deployed++) {
> +        rom_add_vga(SGABIOS_FILENAME);
> +    }
> +}

Is rom_deployed still needed with isa_sga_init() gone?

> +
> +static int isa_cirrus_vga_initfn(ISADevice *dev)
> +{
> +    deploy_rom();
> +
> +    return 0;
> +}
> +
> +void isa_sga_init(void)
> +{
> +    if (display_type == DT_NOGRAPHIC) {
> +        deploy_rom();
> +    }
> +}
> +
> +static ISADeviceInfo sga_info = {
> +    .qdev.name    = "sga",
> +    .qdev.desc    = "Serial Graphics Adapter",
> +    .qdev.size    = sizeof(ISASGAState),
> +    .init         = isa_cirrus_vga_initfn,
> +};
> +
> +static void sga_register(void)
> +{
> +      isa_qdev_register(&sga_info);
> +}
> +
> +device_init(sga_register);