Re: [RfC PATCH] vga: add mmio bar to standard vga

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Tue, 2012-09-18 at 11:51 +0200, Gerd Hoffmann wrote:
> This patch adds a mmio bar to the qemu standard vga which allows to
> access the standard vga registers and bochs dispi interface registers
> via mmio.

I had a patch like that somewhere (or is that it ? :-)

I dropped it in favor of a more interesting approach doing a virtio-vga,
which Anthony and I have been hacking on a bit, but due to time
constraints haven't really finished at this point.

In any case, I'm fine with this patch but does it help anybody ?

Cheers,
Ben.

> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/vga-pci.c |   97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/vga.c     |    6 ++--
>  hw/vga_int.h |    6 +++
>  3 files changed, 106 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/vga-pci.c b/hw/vga-pci.c
> index 9abbada..e05e2ef 100644
> --- a/hw/vga-pci.c
> +++ b/hw/vga-pci.c
> @@ -30,9 +30,36 @@
>  #include "qemu-timer.h"
>  #include "loader.h"
>  
> +/*
> + * QEMU Standard VGA -- MMIO area spec.
> + *
> + * Using PCI bar #2, keeping #1 free, which leaves the
> + * door open to upgrade bar #0 to 64bit.
> + *
> + * mmio area layout:
> + *   0x0000 -> 0x03ff  reserved, for possible virtio extension.
> + *   0x0400 -> 0x041f  vga ioports (0x3c0 -> 0x3df), remapped 1:1
> + *   0x0500 -> 0x0515  bochs dispi interface registers, mapped flat without
> + *                     index/data ports.  Use (index << 1) as offset for
> + *                     (16bit) register access.
> + */
> +#define PCI_VGA_IOPORT_OFFSET 0x400
> +#define PCI_VGA_IOPORT_SIZE   (0x3e0 - 0x3c0)
> +#define PCI_VGA_BOCHS_OFFSET  0x500
> +#define PCI_VGA_BOCHS_SIZE    (0x0b * 2)
> +#define PCI_VGA_MMIO_SIZE     0x1000
> +
> +enum vga_pci_flags {
> +    PCI_VGA_FLAG_ENABLE_MMIO = 1,
> +};
> +
>  typedef struct PCIVGAState {
>      PCIDevice dev;
>      VGACommonState vga;
> +    uint32_t flags;
> +    MemoryRegion mmio;
> +    MemoryRegion ioport;
> +    MemoryRegion bochs;
>  } PCIVGAState;
>  
>  static const VMStateDescription vmstate_vga_pci = {
> @@ -47,6 +74,60 @@ static const VMStateDescription vmstate_vga_pci = {
>      }
>  };
>  
> +static uint64_t pci_vga_ioport_read(void *ptr, target_phys_addr_t addr,
> +                                    unsigned size)
> +{
> +    PCIVGAState *d = ptr;
> +    return vga_ioport_read(&d->vga, addr);
> +}
> +
> +static void pci_vga_ioport_write(void *ptr, target_phys_addr_t addr,
> +                                 uint64_t val, unsigned size)
> +{
> +    PCIVGAState *d = ptr;
> +    vga_ioport_write(&d->vga, addr, val);
> +}
> +
> +static const MemoryRegionOps pci_vga_ioport_ops = {
> +    .read = pci_vga_ioport_read,
> +    .write = pci_vga_ioport_write,
> +    .valid.min_access_size = 1,
> +    .valid.max_access_size = 4,
> +    .impl.min_access_size = 1,
> +    .impl.max_access_size = 1,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static uint64_t pci_vga_bochs_read(void *ptr, target_phys_addr_t addr,
> +                                   unsigned size)
> +{
> +    PCIVGAState *d = ptr;
> +    int index = addr >> 1;
> +
> +    vbe_ioport_write_index(&d->vga, 0, index);
> +    return vbe_ioport_read_data(&d->vga, 0);
> +}
> +
> +static void pci_vga_bochs_write(void *ptr, target_phys_addr_t addr,
> +                                uint64_t val, unsigned size)
> +{
> +    PCIVGAState *d = ptr;
> +    int index = addr >> 1;
> +
> +    vbe_ioport_write_index(&d->vga, 0, index);
> +    vbe_ioport_write_data(&d->vga, 0, val);
> +}
> +
> +static const MemoryRegionOps pci_vga_bochs_ops = {
> +    .read = pci_vga_bochs_read,
> +    .write = pci_vga_bochs_write,
> +    .valid.min_access_size = 1,
> +    .valid.max_access_size = 4,
> +    .impl.min_access_size = 2,
> +    .impl.max_access_size = 2,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
>  static int pci_vga_initfn(PCIDevice *dev)
>  {
>       PCIVGAState *d = DO_UPCAST(PCIVGAState, dev, dev);
> @@ -62,6 +143,21 @@ static int pci_vga_initfn(PCIDevice *dev)
>       /* XXX: VGA_RAM_SIZE must be a power of two */
>       pci_register_bar(&d->dev, 0, PCI_BASE_ADDRESS_MEM_PREFETCH, &s->vram);
>  
> +     /* mmio bar for vga register access */
> +     if (d->flags & (1 << PCI_VGA_FLAG_ENABLE_MMIO)) {
> +         memory_region_init(&d->mmio, "vga.mmio", 4096);
> +         memory_region_init_io(&d->ioport, &pci_vga_ioport_ops, d,
> +                               "vga ioports remapped", PCI_VGA_IOPORT_SIZE);
> +         memory_region_init_io(&d->bochs, &pci_vga_bochs_ops, d,
> +                               "bochs dispi interface", PCI_VGA_BOCHS_SIZE);
> +
> +         memory_region_add_subregion(&d->mmio, PCI_VGA_IOPORT_OFFSET,
> +                                     &d->ioport);
> +         memory_region_add_subregion(&d->mmio, PCI_VGA_BOCHS_OFFSET,
> +                                     &d->bochs);
> +         pci_register_bar(&d->dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, &d->mmio);
> +     }
> +
>       if (!dev->rom_bar) {
>           /* compatibility with pc-0.13 and older */
>           vga_init_vbe(s, pci_address_space(dev));
> @@ -77,6 +173,7 @@ DeviceState *pci_vga_init(PCIBus *bus)
>  
>  static Property vga_pci_properties[] = {
>      DEFINE_PROP_UINT32("vgamem_mb", PCIVGAState, vga.vram_size_mb, 16),
> +    DEFINE_PROP_BIT("mmio", PCIVGAState, flags, PCI_VGA_FLAG_ENABLE_MMIO, true),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/vga.c b/hw/vga.c
> index ec4f0c5..053f89d 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -591,7 +591,7 @@ static uint32_t vbe_ioport_read_index(void *opaque, uint32_t addr)
>      return val;
>  }
>  
> -static uint32_t vbe_ioport_read_data(void *opaque, uint32_t addr)
> +uint32_t vbe_ioport_read_data(void *opaque, uint32_t addr)
>  {
>      VGACommonState *s = opaque;
>      uint32_t val;
> @@ -627,13 +627,13 @@ static uint32_t vbe_ioport_read_data(void *opaque, uint32_t addr)
>      return val;
>  }
>  
> -static void vbe_ioport_write_index(void *opaque, uint32_t addr, uint32_t val)
> +void vbe_ioport_write_index(void *opaque, uint32_t addr, uint32_t val)
>  {
>      VGACommonState *s = opaque;
>      s->vbe_index = val;
>  }
>  
> -static void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val)
> +void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val)
>  {
>      VGACommonState *s = opaque;
>  
> diff --git a/hw/vga_int.h b/hw/vga_int.h
> index 330a32f..5b68490 100644
> --- a/hw/vga_int.h
> +++ b/hw/vga_int.h
> @@ -208,7 +208,13 @@ void vga_invalidate_scanlines(VGACommonState *s, int y1, int y2);
>  void ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp);
>  
>  int vga_ioport_invalid(VGACommonState *s, uint32_t addr);
> +
> +#ifdef CONFIG_BOCHS_VBE
>  void vga_init_vbe(VGACommonState *s, MemoryRegion *address_space);
> +uint32_t vbe_ioport_read_data(void *opaque, uint32_t addr);
> +void vbe_ioport_write_index(void *opaque, uint32_t addr, uint32_t val);
> +void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val);
> +#endif
>  
>  extern const uint8_t sr_mask[8];
>  extern const uint8_t gr_mask[16];