[Qemu-devel] [PATCH v3 0/6] vga: dirty log cleanup, more linear mapping

[Qemu-devel] [PATCH v3 0/6] vga: dirty log cleanup, more linear mapping

[Jan Kiszka]

Rebased over latest qemu.git, addressing Avi's feedback and also fixing
a regression of the vmware reset patch.

Patch 1 is still a candidate for stable.

PS: Just accidentally ran checkpatch against recent top of qemu.git as
well - looks like not many maintainers use that tool themselves...
(not really helpful when asking contributors to do this)

Cc: Andrzej Zaborowski <balrogg@gmail.com>
CC: Andrzej Zaborowski <balrogg@gmail.com>
CC: Avi Kivity <avi@redhat.com>
CC: Gerd Hoffmann <kraxel@redhat.com>

Jan Kiszka (6):
  vmware-vga: Register reset service
  vmware-vga: Disable verbose mode
  vmware-vga: Remove dead DIRECT_VRAM mode
  vmware-vga: Eliminate vga_dirty_log_restart
  vga: Use linear mapping + dirty logging in chain 4 memory access mode
  vga: Drop some unused fields

 hw/vga.c        |   57 +++++++++++--
 hw/vga_int.h    |    5 +-
 hw/vmware_vga.c |  242 +++++++++++++++----------------------------------------
 3 files changed, 118 insertions(+), 186 deletions(-)

-- 
1.7.3.4

[Qemu-devel] [PATCH v3 1/6] vmware-vga: Register reset service

[Jan Kiszka]

Fixes cold reset in vmware graphic modes. We need to split up the reset
function for this purpose, breaking out init-once bits.

Cc: Andrzej Zaborowski <balrogg@gmail.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/vmware_vga.c |   66 +++++++++++++++++++++++++++++-------------------------
 1 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 5396a6c..4fac38b 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -998,46 +998,21 @@ static void vmsvga_update_display(void *opaque)
     }
 }
 
-static void vmsvga_reset(struct vmsvga_state_s *s)
+static void vmsvga_reset(DeviceState *dev)
 {
+    struct pci_vmsvga_state_s *pci =
+        DO_UPCAST(struct pci_vmsvga_state_s, card.qdev, dev);
+    struct vmsvga_state_s *s = &pci->chip;
+
     s->index = 0;
     s->enable = 0;
     s->config = 0;
     s->width = -1;
     s->height = -1;
     s->svgaid = SVGA_ID;
-    s->depth = ds_get_bits_per_pixel(s->vga.ds);
-    s->bypp = ds_get_bytes_per_pixel(s->vga.ds);
     s->cursor.on = 0;
     s->redraw_fifo_first = 0;
     s->redraw_fifo_last = 0;
-    switch (s->depth) {
-    case 8:
-        s->wred   = 0x00000007;
-        s->wgreen = 0x00000038;
-        s->wblue  = 0x000000c0;
-        break;
-    case 15:
-        s->wred   = 0x0000001f;
-        s->wgreen = 0x000003e0;
-        s->wblue  = 0x00007c00;
-        break;
-    case 16:
-        s->wred   = 0x0000001f;
-        s->wgreen = 0x000007e0;
-        s->wblue  = 0x0000f800;
-        break;
-    case 24:
-        s->wred   = 0x00ff0000;
-        s->wgreen = 0x0000ff00;
-        s->wblue  = 0x000000ff;
-        break;
-    case 32:
-        s->wred   = 0x00ff0000;
-        s->wgreen = 0x0000ff00;
-        s->wblue  = 0x000000ff;
-        break;
-    }
     s->syncing = 0;
 
     vga_dirty_log_start(&s->vga);
@@ -1227,7 +1202,35 @@ static void vmsvga_init(struct vmsvga_state_s *s, int vga_ram_size,
     vga_init(&s->vga, address_space);
     vmstate_register(NULL, 0, &vmstate_vga_common, &s->vga);
 
-    vmsvga_reset(s);
+    s->depth = ds_get_bits_per_pixel(s->vga.ds);
+    s->bypp = ds_get_bytes_per_pixel(s->vga.ds);
+    switch (s->depth) {
+    case 8:
+        s->wred   = 0x00000007;
+        s->wgreen = 0x00000038;
+        s->wblue  = 0x000000c0;
+        break;
+    case 15:
+        s->wred   = 0x0000001f;
+        s->wgreen = 0x000003e0;
+        s->wblue  = 0x00007c00;
+        break;
+    case 16:
+        s->wred   = 0x0000001f;
+        s->wgreen = 0x000007e0;
+        s->wblue  = 0x0000f800;
+        break;
+    case 24:
+        s->wred   = 0x00ff0000;
+        s->wgreen = 0x0000ff00;
+        s->wblue  = 0x000000ff;
+        break;
+    case 32:
+        s->wred   = 0x00ff0000;
+        s->wgreen = 0x0000ff00;
+        s->wblue  = 0x000000ff;
+        break;
+    }
 }
 
 static uint64_t vmsvga_io_read(void *opaque, target_phys_addr_t addr,
@@ -1312,6 +1315,7 @@ static PCIDeviceInfo vmsvga_info = {
     .qdev.name    = "vmware-svga",
     .qdev.size    = sizeof(struct pci_vmsvga_state_s),
     .qdev.vmsd    = &vmstate_vmware_vga,
+    .qdev.reset   = vmsvga_reset,
     .no_hotplug   = 1,
     .init         = pci_vmsvga_initfn,
     .romfile      = "vgabios-vmware.bin",
-- 
1.7.3.4

[Qemu-devel] [PATCH v3 2/6] vmware-vga: Disable verbose mode

[Jan Kiszka]

Elimiates 'vmsvga_value_write: guest runs Linux.' messages from the
console.

CC: Andrzej Zaborowski <balrogg@gmail.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/vmware_vga.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 4fac38b..8c63e48 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -27,7 +27,7 @@
 #include "pci.h"
 #include "vmware_vga.h"
 
-#define VERBOSE
+#undef VERBOSE
 #undef DIRECT_VRAM
 #define HW_RECT_ACCEL
 #define HW_FILL_ACCEL
-- 
1.7.3.4

[Qemu-devel] [PATCH v3 3/6] vmware-vga: Remove dead DIRECT_VRAM mode

[Jan Kiszka]

The code was disabled since day 1 of vmware-vga, and now it does not
even build anymore. Time for a cleanup.

CC: Andrzej Zaborowski <balrogg@gmail.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/vmware_vga.c |  172 ++++++++++---------------------------------------------
 1 files changed, 30 insertions(+), 142 deletions(-)

diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 8c63e48..95c9897 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -28,7 +28,6 @@
 #include "vmware_vga.h"
 
 #undef VERBOSE
-#undef DIRECT_VRAM
 #define HW_RECT_ACCEL
 #define HW_FILL_ACCEL
 #define HW_MOUSE_ACCEL
@@ -292,7 +291,6 @@ enum {
 static inline void vmsvga_update_rect(struct vmsvga_state_s *s,
                 int x, int y, int w, int h)
 {
-#ifndef DIRECT_VRAM
     int line;
     int bypl;
     int width;
@@ -323,23 +321,17 @@ static inline void vmsvga_update_rect(struct vmsvga_state_s *s,
 
     for (; line > 0; line --, src += bypl, dst += bypl)
         memcpy(dst, src, width);
-#endif
 
     dpy_update(s->vga.ds, x, y, w, h);
 }
 
 static inline void vmsvga_update_screen(struct vmsvga_state_s *s)
 {
-#ifndef DIRECT_VRAM
-    memcpy(ds_get_data(s->vga.ds), s->vga.vram_ptr, s->bypp * s->width * s->height);
-#endif
-
+    memcpy(ds_get_data(s->vga.ds), s->vga.vram_ptr,
+           s->bypp * s->width * s->height);
     dpy_update(s->vga.ds, 0, 0, s->width, s->height);
 }
 
-#ifdef DIRECT_VRAM
-# define vmsvga_update_rect_delayed	vmsvga_update_rect
-#else
 static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
                 int x, int y, int w, int h)
 {
@@ -350,7 +342,6 @@ static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
     rect->w = w;
     rect->h = h;
 }
-#endif
 
 static inline void vmsvga_update_rect_flush(struct vmsvga_state_s *s)
 {
@@ -372,32 +363,23 @@ static inline void vmsvga_update_rect_flush(struct vmsvga_state_s *s)
 static inline void vmsvga_copy_rect(struct vmsvga_state_s *s,
                 int x0, int y0, int x1, int y1, int w, int h)
 {
-# ifdef DIRECT_VRAM
-    uint8_t *vram = ds_get_data(s->ds);
-# else
     uint8_t *vram = s->vga.vram_ptr;
-# endif
     int bypl = s->bypp * s->width;
     int width = s->bypp * w;
     int line = h;
     uint8_t *ptr[2];
 
-# ifdef DIRECT_VRAM
-    if (s->ds->dpy_copy)
-        qemu_console_copy(s->ds, x0, y0, x1, y1, w, h);
-    else
-# endif
-    {
-        if (y1 > y0) {
-            ptr[0] = vram + s->bypp * x0 + bypl * (y0 + h - 1);
-            ptr[1] = vram + s->bypp * x1 + bypl * (y1 + h - 1);
-            for (; line > 0; line --, ptr[0] -= bypl, ptr[1] -= bypl)
-                memmove(ptr[1], ptr[0], width);
-        } else {
-            ptr[0] = vram + s->bypp * x0 + bypl * y0;
-            ptr[1] = vram + s->bypp * x1 + bypl * y1;
-            for (; line > 0; line --, ptr[0] += bypl, ptr[1] += bypl)
-                memmove(ptr[1], ptr[0], width);
+    if (y1 > y0) {
+        ptr[0] = vram + s->bypp * x0 + bypl * (y0 + h - 1);
+        ptr[1] = vram + s->bypp * x1 + bypl * (y1 + h - 1);
+        for (; line > 0; line --, ptr[0] -= bypl, ptr[1] -= bypl) {
+            memmove(ptr[1], ptr[0], width);
+        }
+    } else {
+        ptr[0] = vram + s->bypp * x0 + bypl * y0;
+        ptr[1] = vram + s->bypp * x1 + bypl * y1;
+        for (; line > 0; line --, ptr[0] += bypl, ptr[1] += bypl) {
+            memmove(ptr[1], ptr[0], width);
         }
     }
 
@@ -409,11 +391,7 @@ static inline void vmsvga_copy_rect(struct vmsvga_state_s *s,
 static inline void vmsvga_fill_rect(struct vmsvga_state_s *s,
                 uint32_t c, int x, int y, int w, int h)
 {
-# ifdef DIRECT_VRAM
-    uint8_t *vram = ds_get_data(s->ds);
-# else
     uint8_t *vram = s->vga.vram_ptr;
-# endif
     int bypp = s->bypp;
     int bypl = bypp * s->width;
     int width = bypp * w;
@@ -424,31 +402,25 @@ static inline void vmsvga_fill_rect(struct vmsvga_state_s *s,
     uint8_t *src;
     uint8_t col[4];
 
-# ifdef DIRECT_VRAM
-    if (s->ds->dpy_fill)
-        s->ds->dpy_fill(s->ds, x, y, w, h, c);
-    else
-# endif
-    {
-        col[0] = c;
-        col[1] = c >> 8;
-        col[2] = c >> 16;
-        col[3] = c >> 24;
-
-        if (line --) {
-            dst = fst;
-            src = col;
-            for (column = width; column > 0; column --) {
-                *(dst ++) = *(src ++);
-                if (src - col == bypp)
-                    src = col;
-            }
-            dst = fst;
-            for (; line > 0; line --) {
-                dst += bypl;
-                memcpy(dst, fst, width);
+    col[0] = c;
+    col[1] = c >> 8;
+    col[2] = c >> 16;
+    col[3] = c >> 24;
+
+    if (line--) {
+        dst = fst;
+        src = col;
+        for (column = width; column > 0; column--) {
+            *(dst++) = *(src++);
+            if (src - col == bypp) {
+                src = col;
             }
         }
+        dst = fst;
+        for (; line > 0; line--) {
+            dst += bypl;
+            memcpy(dst, fst, width);
+        }
     }
 
     vmsvga_update_rect_delayed(s, x, y, w, h);
@@ -1055,82 +1027,6 @@ static void vmsvga_text_update(void *opaque, console_ch_t *chardata)
         s->vga.text_update(&s->vga, chardata);
 }
 
-#ifdef DIRECT_VRAM
-static uint32_t vmsvga_vram_readb(void *opaque, target_phys_addr_t addr)
-{
-    struct vmsvga_state_s *s = opaque;
-    if (addr < s->fb_size)
-        return *(uint8_t *) (ds_get_data(s->ds) + addr);
-    else
-        return *(uint8_t *) (s->vram_ptr + addr);
-}
-
-static uint32_t vmsvga_vram_readw(void *opaque, target_phys_addr_t addr)
-{
-    struct vmsvga_state_s *s = opaque;
-    if (addr < s->fb_size)
-        return *(uint16_t *) (ds_get_data(s->ds) + addr);
-    else
-        return *(uint16_t *) (s->vram_ptr + addr);
-}
-
-static uint32_t vmsvga_vram_readl(void *opaque, target_phys_addr_t addr)
-{
-    struct vmsvga_state_s *s = opaque;
-    if (addr < s->fb_size)
-        return *(uint32_t *) (ds_get_data(s->ds) + addr);
-    else
-        return *(uint32_t *) (s->vram_ptr + addr);
-}
-
-static void vmsvga_vram_writeb(void *opaque, target_phys_addr_t addr,
-                uint32_t value)
-{
-    struct vmsvga_state_s *s = opaque;
-    if (addr < s->fb_size)
-        *(uint8_t *) (ds_get_data(s->ds) + addr) = value;
-    else
-        *(uint8_t *) (s->vram_ptr + addr) = value;
-}
-
-static void vmsvga_vram_writew(void *opaque, target_phys_addr_t addr,
-                uint32_t value)
-{
-    struct vmsvga_state_s *s = opaque;
-    if (addr < s->fb_size)
-        *(uint16_t *) (ds_get_data(s->ds) + addr) = value;
-    else
-        *(uint16_t *) (s->vram_ptr + addr) = value;
-}
-
-static void vmsvga_vram_writel(void *opaque, target_phys_addr_t addr,
-                uint32_t value)
-{
-    struct vmsvga_state_s *s = opaque;
-    if (addr < s->fb_size)
-        *(uint32_t *) (ds_get_data(s->ds) + addr) = value;
-    else
-        *(uint32_t *) (s->vram_ptr + addr) = value;
-}
-
-static const MemoryRegionOps vmsvga_vram_io_ops = {
-    .old_mmio = {
-        .read = {
-            vmsvga_vram_readb,
-            vmsvga_vram_readw,
-            vmsvga_vram_readl,
-        },
-        .write = {
-            vmsvga_vram_writeb,
-            vmsvga_vram_writew,
-            vmsvga_vram_writel,
-        },
-    },
-    .endianness = DEVICE_NATIVE_ENDIAN,
-}
-
-#endif
-
 static int vmsvga_post_load(void *opaque, int version_id)
 {
     struct vmsvga_state_s *s = opaque;
@@ -1277,15 +1173,7 @@ static int pci_vmsvga_initfn(PCIDevice *dev)
         DO_UPCAST(struct pci_vmsvga_state_s, card, dev);
     MemoryRegion *iomem;
 
-#ifdef DIRECT_VRAM
-    DirectMem *directmem = g_malloc(sizeof(*directmem));
-
-    iomem = &directmem->mr;
-    memory_region_init_io(iomem, &vmsvga_vram_io_ops, &s->chip, "vmsvga",
-                          memory_region_size(&s->chip.vga.vram));
-#else
     iomem = &s->chip.vga.vram;
-#endif
 
     vga_dirty_log_restart(&s->chip.vga);
 
-- 
1.7.3.4

[Qemu-devel] [PATCH v3 4/6] vmware-vga: Eliminate vga_dirty_log_restart

[Jan Kiszka]

After the conversion to the new Memory API, vga_dirty_log_restart became
seriously pointless. Remove it from vmware-vga and and then finally drop
the service.

CC: Andrzej Zaborowski <balrogg@gmail.com>
CC: Avi Kivity <avi@redhat.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/vga.c        |    6 ------
 hw/vga_int.h    |    1 -
 hw/vmware_vga.c |    2 --
 3 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/hw/vga.c b/hw/vga.c
index b0371d5..432d2cb 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1532,12 +1532,6 @@ void vga_dirty_log_stop(VGACommonState *s)
     memory_region_set_log(&s->vram, false, DIRTY_MEMORY_VGA);
 }
 
-void vga_dirty_log_restart(VGACommonState *s)
-{
-    vga_dirty_log_stop(s);
-    vga_dirty_log_start(s);
-}
-
 /*
  * graphic modes
  */
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 7e4ed71..d2dd7dd 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -193,7 +193,6 @@ void vga_common_reset(VGACommonState *s);
 
 void vga_dirty_log_start(VGACommonState *s);
 void vga_dirty_log_stop(VGACommonState *s);
-void vga_dirty_log_restart(VGACommonState *s);
 
 extern const VMStateDescription vmstate_vga_common;
 uint32_t vga_ioport_read(void *opaque, uint32_t addr);
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 95c9897..52d2d26 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -1175,8 +1175,6 @@ static int pci_vmsvga_initfn(PCIDevice *dev)
 
     iomem = &s->chip.vga.vram;
 
-    vga_dirty_log_restart(&s->chip.vga);
-
     s->card.config[PCI_CACHE_LINE_SIZE]	= 0x08;		/* Cache line size */
     s->card.config[PCI_LATENCY_TIMER] = 0x40;		/* Latency timer */
     s->card.config[PCI_INTERRUPT_LINE] = 0xff;		/* End */
-- 
1.7.3.4

[Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

Most VGA memory access modes require MMIO handling as they demand weird
logic to get a byte from or into the video RAM. However, there is one
exception: chain 4 mode with all memory planes enabled for writing. This
mode actually allows lineary mapping, which can then be combined with
dirty logging to accelerate KVM.

This patch accelerates specifically VBE accesses like they are used by
grub in graphical mode. Not only the standard VGA adapter benefits from
this, also vmware and spice in VGA mode.

CC: Gerd Hoffmann <kraxel@redhat.com>
CC: Avi Kivity <avi@redhat.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/vga.c     |   51 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/vga_int.h |    2 ++
 2 files changed, 52 insertions(+), 1 deletions(-)

diff --git a/hw/vga.c b/hw/vga.c
index 432d2cb..851fd68 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -152,6 +152,48 @@ static void vga_screen_dump(void *opaque, const char *filename);
 static char *screen_dump_filename;
 static DisplayChangeListener *screen_dump_dcl;
 
+static void vga_update_memory_access(VGACommonState *s)
+{
+    MemoryRegion *region, *old_region = s->chain4_alias;
+    target_phys_addr_t base, offset, size;
+
+    s->chain4_alias = NULL;
+
+    if ((s->sr[0x02] & 0xf) == 0xf && s->sr[0x04] & 0x08) {
+        offset = 0;
+        switch ((s->gr[6] >> 2) & 3) {
+        case 0:
+            base = 0xa0000;
+            size = 0x20000;
+            break;
+        case 1:
+            base = 0xa0000;
+            size = 0x10000;
+            offset = s->bank_offset;
+            break;
+        case 2:
+            base = 0xb0000;
+            size = 0x8000;
+            break;
+        case 3:
+            base = 0xb8000;
+            size = 0x8000;
+            break;
+        }
+        region = g_malloc(sizeof(*region));
+        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
+        memory_region_add_subregion_overlap(s->legacy_address_space, base,
+                                            region, 2);
+        s->chain4_alias = region;
+    }
+    if (old_region) {
+        memory_region_del_subregion(s->legacy_address_space, old_region);
+        memory_region_destroy(old_region);
+        g_free(old_region);
+        s->plane_updated = 0xf;
+    }
+}
+
 static void vga_dumb_update_retrace_info(VGACommonState *s)
 {
     (void) s;
@@ -445,6 +487,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 #endif
         s->sr[s->sr_index] = val & sr_mask[s->sr_index];
         if (s->sr_index == 1) s->update_retrace_info(s);
+        vga_update_memory_access(s);
         break;
     case 0x3c7:
         s->dac_read_index = val;
@@ -472,6 +515,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
         printf("vga: write GR%x = 0x%02x\n", s->gr_index, val);
 #endif
         s->gr[s->gr_index] = val & gr_mask[s->gr_index];
+        vga_update_memory_access(s);
         break;
     case 0x3b4:
     case 0x3d4:
@@ -605,6 +649,7 @@ static void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val)
             }
             s->vbe_regs[s->vbe_index] = val;
             s->bank_offset = (val << 16);
+            vga_update_memory_access(s);
             break;
         case VBE_DISPI_INDEX_ENABLE:
             if ((val & VBE_DISPI_ENABLED) &&
@@ -664,6 +709,7 @@ static void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val)
             }
             s->dac_8bit = (val & VBE_DISPI_8BIT_DAC) > 0;
             s->vbe_regs[s->vbe_index] = val;
+            vga_update_memory_access(s);
             break;
         case VBE_DISPI_INDEX_VIRT_WIDTH:
             {
@@ -1238,7 +1284,7 @@ static void vga_draw_text(VGACommonState *s, int full_update)
         s->font_offsets[1] = offset;
         full_update = 1;
     }
-    if (s->plane_updated & (1 << 2)) {
+    if (s->plane_updated & (1 << 2) || s->chain4_alias) {
         /* if the plane 2 was modified since the last display, it
            indicates the font may have been modified */
         s->plane_updated = 0;
@@ -1885,6 +1931,7 @@ void vga_common_reset(VGACommonState *s)
         memset(&s->retrace_info, 0, sizeof (s->retrace_info));
         break;
     }
+    vga_update_memory_access(s);
 }
 
 static void vga_reset(void *opaque)
@@ -2242,6 +2289,8 @@ void vga_init(VGACommonState *s, MemoryRegion *address_space)
 
     s->bank_offset = 0;
 
+    s->legacy_address_space = address_space;
+
     vga_io_memory = vga_init_io(s);
     memory_region_add_subregion_overlap(address_space,
                                         isa_mem_base + 0x000a0000,
diff --git a/hw/vga_int.h b/hw/vga_int.h
index d2dd7dd..28b9287 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -105,11 +105,13 @@ typedef uint8_t (* vga_retrace_fn)(struct VGACommonState *s);
 typedef void (* vga_update_retrace_info_fn)(struct VGACommonState *s);
 
 typedef struct VGACommonState {
+    MemoryRegion *legacy_address_space;
     uint8_t *vram_ptr;
     MemoryRegion vram;
     uint32_t vram_size;
     uint32_t latch;
     uint32_t lfb_vram_mapped; /* whether 0xa0000 is mapped as ram */
+    MemoryRegion *chain4_alias;
     uint8_t sr_index;
     uint8_t sr[256];
     uint8_t gr_index;
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 08/22/2011 08:12 PM, Jan Kiszka wrote:
> Most VGA memory access modes require MMIO handling as they demand weird
> logic to get a byte from or into the video RAM. However, there is one
> exception: chain 4 mode with all memory planes enabled for writing. This
> mode actually allows lineary mapping, which can then be combined with
> dirty logging to accelerate KVM.
>
> This patch accelerates specifically VBE accesses like they are used by
> grub in graphical mode. Not only the standard VGA adapter benefits from
> this, also vmware and spice in VGA mode.
>
>

On which version of grub does this work?  This isn't having any effect 
on my Fedora grub.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-08-25 09:19, Avi Kivity wrote:
> On 08/22/2011 08:12 PM, Jan Kiszka wrote:
>> Most VGA memory access modes require MMIO handling as they demand weird
>> logic to get a byte from or into the video RAM. However, there is one
>> exception: chain 4 mode with all memory planes enabled for writing. This
>> mode actually allows lineary mapping, which can then be combined with
>> dirty logging to accelerate KVM.
>>
>> This patch accelerates specifically VBE accesses like they are used by
>> grub in graphical mode. Not only the standard VGA adapter benefits from
>> this, also vmware and spice in VGA mode.
>>
>>
> 
> On which version of grub does this work?  This isn't having any effect 
> on my Fedora grub.

It's both grub1 (0.97) in graphical mode as used by OpenSUSE 11.4 and
grub2 (1.99-rc1) of Ubuntu 11.04.

Is Fedora's grub still slow or was it already fast before? Which Fedora?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 08/25/2011 12:07 PM, Jan Kiszka wrote:
> >>
> >
> >  On which version of grub does this work?  This isn't having any effect
> >  on my Fedora grub.
>
> It's both grub1 (0.97) in graphical mode as used by OpenSUSE 11.4 and
> grub2 (1.99-rc1) of Ubuntu 11.04.
>
> Is Fedora's grub still slow or was it already fast before? Which Fedora?
>

Still slow.  I tried an old F11 image I had lying around, and -snapshot 
/dev/sda, but this laptop was installed many years ago.  I'll download 
some more images and try.


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-08-25 11:16, Avi Kivity wrote:
> On 08/25/2011 12:07 PM, Jan Kiszka wrote:
>>>>
>>>
>>>  On which version of grub does this work?  This isn't having any effect
>>>  on my Fedora grub.
>>
>> It's both grub1 (0.97) in graphical mode as used by OpenSUSE 11.4 and
>> grub2 (1.99-rc1) of Ubuntu 11.04.
>>
>> Is Fedora's grub still slow or was it already fast before? Which Fedora?
>>
> 
> Still slow.  I tried an old F11 image I had lying around, and -snapshot 
> /dev/sda, but this laptop was installed many years ago.  I'll download 
> some more images and try.

You may also want to instrument vga_update_memory_access if some
requirement for linear mapping is not fulfilled.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 08/25/2011 12:21 PM, Jan Kiszka wrote:
> >
> >  Still slow.  I tried an old F11 image I had lying around, and -snapshot
> >  /dev/sda, but this laptop was installed many years ago.  I'll download
> >  some more images and try.
>
> You may also want to instrument vga_update_memory_access if some
> requirement for linear mapping is not fulfilled.
>

Plain F15 is slow.  SR2 = SR4 = 0.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-08-25 12:45, Avi Kivity wrote:
> On 08/25/2011 12:21 PM, Jan Kiszka wrote:
>>>
>>>  Still slow.  I tried an old F11 image I had lying around, and -snapshot
>>>  /dev/sda, but this laptop was installed many years ago.  I'll download
>>>  some more images and try.
>>
>> You may also want to instrument vga_update_memory_access if some
>> requirement for linear mapping is not fulfilled.
>>
> 
> Plain F15 is slow.  SR2 = SR4 = 0.

So it's not using chain4 mode. Can you check what mode the adapter is
actually in and how VRAM is accessed? Likely, there is nothing we can do
about it. /me just wonders what makes F15 grub behave differently from
other distro's versions...

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 08/25/2011 01:51 PM, Jan Kiszka wrote:
> >
> >  Plain F15 is slow.  SR2 = SR4 = 0.
>
> So it's not using chain4 mode. Can you check what mode the adapter is
> actually in and how VRAM is accessed? Likely, there is nothing we can do
> about it. /me just wonders what makes F15 grub behave differently from
> other distro's versions...
>

Do you remember offhand which registers I need to look at?

We need a collection of gdb scripts to find a qdev, dump the memory 
hierarchy, look up gpas, and get info about specific devices.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-08-25 13:19, Avi Kivity wrote:
> On 08/25/2011 01:51 PM, Jan Kiszka wrote:
>>>
>>>  Plain F15 is slow.  SR2 = SR4 = 0.
>>
>> So it's not using chain4 mode. Can you check what mode the adapter is
>> actually in and how VRAM is accessed? Likely, there is nothing we can do
>> about it. /me just wonders what makes F15 grub behave differently from
>> other distro's versions...
>>
> 
> Do you remember offhand which registers I need to look at?

No, just that there are quite a few. You could check e.g.
http://www.osdever.net/FreeVGA/vga/vga.htm for descriptions. Also check
what paths the mmio handlers take.

> 
> We need a collection of gdb scripts to find a qdev, dump the memory 
> hierarchy, look up gpas, and get info about specific devices.

...or finally have a "device-show" monitor command to dump the device
state. More stable /wrt QEMU internals.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

Jan Kiszka wrote:
> Most VGA memory access modes require MMIO handling as they demand weird
> logic to get a byte from or into the video RAM. However, there is one
> exception: chain 4 mode with all memory planes enabled for writing. This
> mode actually allows lineary mapping, which can then be combined with
> dirty logging to accelerate KVM.
>
> This patch accelerates specifically VBE accesses like they are used by
> grub in graphical mode. Not only the standard VGA adapter benefits from
> this, also vmware and spice in VGA mode.
>
> CC: Gerd Hoffmann <kraxel@redhat.com>
> CC: Avi Kivity <avi@redhat.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>   

[...]

> +static void vga_update_memory_access(VGACommonState *s)
> +{
> +    MemoryRegion *region, *old_region = s->chain4_alias;
> +    target_phys_addr_t base, offset, size;
> +
> +    s->chain4_alias = NULL;
> +
> +    if ((s->sr[0x02] & 0xf) == 0xf && s->sr[0x04] & 0x08) {
> +        offset = 0;
> +        switch ((s->gr[6] >> 2) & 3) {
> +        case 0:
> +            base = 0xa0000;
> +            size = 0x20000;
> +            break;
> +        case 1:
> +            base = 0xa0000;
> +            size = 0x10000;
> +            offset = s->bank_offset;
> +            break;
> +        case 2:
> +            base = 0xb0000;
> +            size = 0x8000;
> +            break;
> +        case 3:
> +            base = 0xb8000;
> +            size = 0x8000;
> +            break;
> +        }
> +        region = g_malloc(sizeof(*region));
> +        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
> +        memory_region_add_subregion_overlap(s->legacy_address_space, base,
> +                                            region, 2);
>   

This one eventually gives me the following in info mtree with -M g3beige
on qemu-system-ppc:

(qemu) info mtree
memory
system addr 00000000 off 00000000 size 7fffffffffffffff
-vga.chain4 addr 000a0000 off 00000000 size 10000
-macio addr 80880000 off 00000000 size 80000
--macio-nvram addr 00060000 off 00000000 size 20000
--pmac-ide addr 00020000 off 00000000 size 1000
--cuda addr 00016000 off 00000000 size 2000
--escc-bar addr 00013000 off 00000000 size 40
--dbdma addr 00008000 off 00000000 size 1000
--heathrow-pic addr 00000000 off 00000000 size 1000
-vga.rom addr 80800000 off 00000000 size 10000
-vga.vram addr 80000000 off 00000000 size 800000
-vga-lowmem addr 800a0000 off 00000000 size 20000
-escc addr 80013000 off 00000000 size 40
-isa-mmio addr fe000000 off 00000000 size 200000
I/O
io addr 00000000 off 00000000 size 10000
-cmd646-bmdma addr 00000700 off 00000000 size 10
--cmd646-bmdma-ioport addr 0000000c off 00000000 size 4
--cmd646-bmdma-bus addr 00000008 off 00000000 size 4
--cmd646-bmdma-ioport addr 00000004 off 00000000 size 4
--cmd646-bmdma-bus addr 00000000 off 00000000 size 4
-cmd646-cmd addr 00000680 off 00000000 size 4
-cmd646-data addr 00000600 off 00000000 size 8
-cmd646-cmd addr 00000580 off 00000000 size 4
-cmd646-data addr 00000500 off 00000000 size 8
-ne2000 addr 00000400 off 00000000 size 100

This ends up overmapping 0xa0000, effectively overwriting kernel data.
If I #if 0 the offending chunk out, everything is fine. I would assume
that chain4 really needs to be inside of lowmem? No idea about VGA, but
I'm sure you know what's going on :).


Alex

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-09-12 17:20, Alexander Graf wrote:
> Jan Kiszka wrote:
>> Most VGA memory access modes require MMIO handling as they demand weird
>> logic to get a byte from or into the video RAM. However, there is one
>> exception: chain 4 mode with all memory planes enabled for writing. This
>> mode actually allows lineary mapping, which can then be combined with
>> dirty logging to accelerate KVM.
>>
>> This patch accelerates specifically VBE accesses like they are used by
>> grub in graphical mode. Not only the standard VGA adapter benefits from
>> this, also vmware and spice in VGA mode.
>>
>> CC: Gerd Hoffmann <kraxel@redhat.com>
>> CC: Avi Kivity <avi@redhat.com>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>   
> 
> [...]
> 
>> +static void vga_update_memory_access(VGACommonState *s)
>> +{
>> +    MemoryRegion *region, *old_region = s->chain4_alias;
>> +    target_phys_addr_t base, offset, size;
>> +
>> +    s->chain4_alias = NULL;
>> +
>> +    if ((s->sr[0x02] & 0xf) == 0xf && s->sr[0x04] & 0x08) {
>> +        offset = 0;
>> +        switch ((s->gr[6] >> 2) & 3) {
>> +        case 0:
>> +            base = 0xa0000;
>> +            size = 0x20000;
>> +            break;
>> +        case 1:
>> +            base = 0xa0000;
>> +            size = 0x10000;
>> +            offset = s->bank_offset;
>> +            break;
>> +        case 2:
>> +            base = 0xb0000;
>> +            size = 0x8000;
>> +            break;
>> +        case 3:
>> +            base = 0xb8000;
>> +            size = 0x8000;
>> +            break;
>> +        }
>> +        region = g_malloc(sizeof(*region));
>> +        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>> +        memory_region_add_subregion_overlap(s->legacy_address_space, base,
>> +                                            region, 2);
>>   
> 
> This one eventually gives me the following in info mtree with -M g3beige
> on qemu-system-ppc:
> 
> (qemu) info mtree
> memory
> system addr 00000000 off 00000000 size 7fffffffffffffff
> -vga.chain4 addr 000a0000 off 00000000 size 10000
> -macio addr 80880000 off 00000000 size 80000
> --macio-nvram addr 00060000 off 00000000 size 20000
> --pmac-ide addr 00020000 off 00000000 size 1000
> --cuda addr 00016000 off 00000000 size 2000
> --escc-bar addr 00013000 off 00000000 size 40
> --dbdma addr 00008000 off 00000000 size 1000
> --heathrow-pic addr 00000000 off 00000000 size 1000
> -vga.rom addr 80800000 off 00000000 size 10000
> -vga.vram addr 80000000 off 00000000 size 800000
> -vga-lowmem addr 800a0000 off 00000000 size 20000
> -escc addr 80013000 off 00000000 size 40
> -isa-mmio addr fe000000 off 00000000 size 200000
> I/O
> io addr 00000000 off 00000000 size 10000
> -cmd646-bmdma addr 00000700 off 00000000 size 10
> --cmd646-bmdma-ioport addr 0000000c off 00000000 size 4
> --cmd646-bmdma-bus addr 00000008 off 00000000 size 4
> --cmd646-bmdma-ioport addr 00000004 off 00000000 size 4
> --cmd646-bmdma-bus addr 00000000 off 00000000 size 4
> -cmd646-cmd addr 00000680 off 00000000 size 4
> -cmd646-data addr 00000600 off 00000000 size 8
> -cmd646-cmd addr 00000580 off 00000000 size 4
> -cmd646-data addr 00000500 off 00000000 size 8
> -ne2000 addr 00000400 off 00000000 size 100
> 
> This ends up overmapping 0xa0000, effectively overwriting kernel data.
> If I #if 0 the offending chunk out, everything is fine. I would assume
> that chain4 really needs to be inside of lowmem? No idea about VGA, but
> I'm sure you know what's going on :).

Does this help?

diff --git a/hw/vga.c b/hw/vga.c
index 125fb29..0a0c5a6 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -181,6 +181,7 @@ static void vga_update_memory_access(VGACommonState *s)
             size = 0x8000;
             break;
         }
+        base += isa_mem_base;
         region = g_malloc(sizeof(*region));
         memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
         memory_region_add_subregion_overlap(s->legacy_address_space, base,

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Andreas Färber]

Am 12.09.2011 17:33, schrieb Jan Kiszka:
> On 2011-09-12 17:20, Alexander Graf wrote:
>> Jan Kiszka wrote:
>>> Most VGA memory access modes require MMIO handling as they demand weird
>>> logic to get a byte from or into the video RAM. However, there is one
>>> exception: chain 4 mode with all memory planes enabled for writing. This
>>> mode actually allows lineary mapping, which can then be combined with
>>> dirty logging to accelerate KVM.
>>>
>>> This patch accelerates specifically VBE accesses like they are used by
>>> grub in graphical mode. Not only the standard VGA adapter benefits from
>>> this, also vmware and spice in VGA mode.
>>>
>>> CC: Gerd Hoffmann <kraxel@redhat.com>
>>> CC: Avi Kivity <avi@redhat.com>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>   
>> [...]
>>
>>> +static void vga_update_memory_access(VGACommonState *s)
>>> +{
>>> +    MemoryRegion *region, *old_region = s->chain4_alias;
>>> +    target_phys_addr_t base, offset, size;
>>> +
>>> +    s->chain4_alias = NULL;
>>> +
>>> +    if ((s->sr[0x02] & 0xf) == 0xf && s->sr[0x04] & 0x08) {
>>> +        offset = 0;
>>> +        switch ((s->gr[6] >> 2) & 3) {
>>> +        case 0:
>>> +            base = 0xa0000;
>>> +            size = 0x20000;
>>> +            break;
>>> +        case 1:
>>> +            base = 0xa0000;
>>> +            size = 0x10000;
>>> +            offset = s->bank_offset;
>>> +            break;
>>> +        case 2:
>>> +            base = 0xb0000;
>>> +            size = 0x8000;
>>> +            break;
>>> +        case 3:
>>> +            base = 0xb8000;
>>> +            size = 0x8000;
>>> +            break;
>>> +        }
>>> +        region = g_malloc(sizeof(*region));
>>> +        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>>> +        memory_region_add_subregion_overlap(s->legacy_address_space, base,
>>> +                                            region, 2);
>>>   
>> This one eventually gives me the following in info mtree with -M g3beige
>> on qemu-system-ppc:
>>
>> (qemu) info mtree
>> memory
>> system addr 00000000 off 00000000 size 7fffffffffffffff
>> -vga.chain4 addr 000a0000 off 00000000 size 10000
>> -macio addr 80880000 off 00000000 size 80000
>> --macio-nvram addr 00060000 off 00000000 size 20000
>> --pmac-ide addr 00020000 off 00000000 size 1000
>> --cuda addr 00016000 off 00000000 size 2000
>> --escc-bar addr 00013000 off 00000000 size 40
>> --dbdma addr 00008000 off 00000000 size 1000
>> --heathrow-pic addr 00000000 off 00000000 size 1000
>> -vga.rom addr 80800000 off 00000000 size 10000
>> -vga.vram addr 80000000 off 00000000 size 800000
>> -vga-lowmem addr 800a0000 off 00000000 size 20000
>> -escc addr 80013000 off 00000000 size 40
>> -isa-mmio addr fe000000 off 00000000 size 200000
>> I/O
>> io addr 00000000 off 00000000 size 10000
>> -cmd646-bmdma addr 00000700 off 00000000 size 10
>> --cmd646-bmdma-ioport addr 0000000c off 00000000 size 4
>> --cmd646-bmdma-bus addr 00000008 off 00000000 size 4
>> --cmd646-bmdma-ioport addr 00000004 off 00000000 size 4
>> --cmd646-bmdma-bus addr 00000000 off 00000000 size 4
>> -cmd646-cmd addr 00000680 off 00000000 size 4
>> -cmd646-data addr 00000600 off 00000000 size 8
>> -cmd646-cmd addr 00000580 off 00000000 size 4
>> -cmd646-data addr 00000500 off 00000000 size 8
>> -ne2000 addr 00000400 off 00000000 size 100
>>
>> This ends up overmapping 0xa0000, effectively overwriting kernel data.
>> If I #if 0 the offending chunk out, everything is fine. I would assume
>> that chain4 really needs to be inside of lowmem? No idea about VGA, but
>> I'm sure you know what's going on :).
> Does this help?
>
> diff --git a/hw/vga.c b/hw/vga.c
> index 125fb29..0a0c5a6 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -181,6 +181,7 @@ static void vga_update_memory_access(VGACommonState *s)
>              size = 0x8000;
>              break;
>          }
> +        base += isa_mem_base;
>          region = g_malloc(sizeof(*region));
>          memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>          memory_region_add_subregion_overlap(s->legacy_address_space, base,

No longer oopses, but the screen looks chaotic now (black bar at bottom,
part of contents at top etc.).

Andreas

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-09-12 17:45, Andreas Färber wrote:
> Am 12.09.2011 17:33, schrieb Jan Kiszka:
>> On 2011-09-12 17:20, Alexander Graf wrote:
>>> Jan Kiszka wrote:
>>>> Most VGA memory access modes require MMIO handling as they demand weird
>>>> logic to get a byte from or into the video RAM. However, there is one
>>>> exception: chain 4 mode with all memory planes enabled for writing. This
>>>> mode actually allows lineary mapping, which can then be combined with
>>>> dirty logging to accelerate KVM.
>>>>
>>>> This patch accelerates specifically VBE accesses like they are used by
>>>> grub in graphical mode. Not only the standard VGA adapter benefits from
>>>> this, also vmware and spice in VGA mode.
>>>>
>>>> CC: Gerd Hoffmann <kraxel@redhat.com>
>>>> CC: Avi Kivity <avi@redhat.com>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>   
>>> [...]
>>>
>>>> +static void vga_update_memory_access(VGACommonState *s)
>>>> +{
>>>> +    MemoryRegion *region, *old_region = s->chain4_alias;
>>>> +    target_phys_addr_t base, offset, size;
>>>> +
>>>> +    s->chain4_alias = NULL;
>>>> +
>>>> +    if ((s->sr[0x02] & 0xf) == 0xf && s->sr[0x04] & 0x08) {
>>>> +        offset = 0;
>>>> +        switch ((s->gr[6] >> 2) & 3) {
>>>> +        case 0:
>>>> +            base = 0xa0000;
>>>> +            size = 0x20000;
>>>> +            break;
>>>> +        case 1:
>>>> +            base = 0xa0000;
>>>> +            size = 0x10000;
>>>> +            offset = s->bank_offset;
>>>> +            break;
>>>> +        case 2:
>>>> +            base = 0xb0000;
>>>> +            size = 0x8000;
>>>> +            break;
>>>> +        case 3:
>>>> +            base = 0xb8000;
>>>> +            size = 0x8000;
>>>> +            break;
>>>> +        }
>>>> +        region = g_malloc(sizeof(*region));
>>>> +        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>>>> +        memory_region_add_subregion_overlap(s->legacy_address_space, base,
>>>> +                                            region, 2);
>>>>   
>>> This one eventually gives me the following in info mtree with -M g3beige
>>> on qemu-system-ppc:
>>>
>>> (qemu) info mtree
>>> memory
>>> system addr 00000000 off 00000000 size 7fffffffffffffff
>>> -vga.chain4 addr 000a0000 off 00000000 size 10000
>>> -macio addr 80880000 off 00000000 size 80000
>>> --macio-nvram addr 00060000 off 00000000 size 20000
>>> --pmac-ide addr 00020000 off 00000000 size 1000
>>> --cuda addr 00016000 off 00000000 size 2000
>>> --escc-bar addr 00013000 off 00000000 size 40
>>> --dbdma addr 00008000 off 00000000 size 1000
>>> --heathrow-pic addr 00000000 off 00000000 size 1000
>>> -vga.rom addr 80800000 off 00000000 size 10000
>>> -vga.vram addr 80000000 off 00000000 size 800000
>>> -vga-lowmem addr 800a0000 off 00000000 size 20000
>>> -escc addr 80013000 off 00000000 size 40
>>> -isa-mmio addr fe000000 off 00000000 size 200000
>>> I/O
>>> io addr 00000000 off 00000000 size 10000
>>> -cmd646-bmdma addr 00000700 off 00000000 size 10
>>> --cmd646-bmdma-ioport addr 0000000c off 00000000 size 4
>>> --cmd646-bmdma-bus addr 00000008 off 00000000 size 4
>>> --cmd646-bmdma-ioport addr 00000004 off 00000000 size 4
>>> --cmd646-bmdma-bus addr 00000000 off 00000000 size 4
>>> -cmd646-cmd addr 00000680 off 00000000 size 4
>>> -cmd646-data addr 00000600 off 00000000 size 8
>>> -cmd646-cmd addr 00000580 off 00000000 size 4
>>> -cmd646-data addr 00000500 off 00000000 size 8
>>> -ne2000 addr 00000400 off 00000000 size 100
>>>
>>> This ends up overmapping 0xa0000, effectively overwriting kernel data.
>>> If I #if 0 the offending chunk out, everything is fine. I would assume
>>> that chain4 really needs to be inside of lowmem? No idea about VGA, but
>>> I'm sure you know what's going on :).
>> Does this help?
>>
>> diff --git a/hw/vga.c b/hw/vga.c
>> index 125fb29..0a0c5a6 100644
>> --- a/hw/vga.c
>> +++ b/hw/vga.c
>> @@ -181,6 +181,7 @@ static void vga_update_memory_access(VGACommonState *s)
>>              size = 0x8000;
>>              break;
>>          }
>> +        base += isa_mem_base;
>>          region = g_malloc(sizeof(*region));
>>          memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>>          memory_region_add_subregion_overlap(s->legacy_address_space, base,
> 
> No longer oopses, but the screen looks chaotic now (black bar at bottom,
> part of contents at top etc.).

Does this PPC machine map the ISA range and forward VGA accesses to the
adapter in general?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-09-12 17:49, Jan Kiszka wrote:
> On 2011-09-12 17:45, Andreas Färber wrote:
>> Am 12.09.2011 17:33, schrieb Jan Kiszka:
>>> On 2011-09-12 17:20, Alexander Graf wrote:
>>>> Jan Kiszka wrote:
>>>>> Most VGA memory access modes require MMIO handling as they demand weird
>>>>> logic to get a byte from or into the video RAM. However, there is one
>>>>> exception: chain 4 mode with all memory planes enabled for writing. This
>>>>> mode actually allows lineary mapping, which can then be combined with
>>>>> dirty logging to accelerate KVM.
>>>>>
>>>>> This patch accelerates specifically VBE accesses like they are used by
>>>>> grub in graphical mode. Not only the standard VGA adapter benefits from
>>>>> this, also vmware and spice in VGA mode.
>>>>>
>>>>> CC: Gerd Hoffmann <kraxel@redhat.com>
>>>>> CC: Avi Kivity <avi@redhat.com>
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>   
>>>> [...]
>>>>
>>>>> +static void vga_update_memory_access(VGACommonState *s)
>>>>> +{
>>>>> +    MemoryRegion *region, *old_region = s->chain4_alias;
>>>>> +    target_phys_addr_t base, offset, size;
>>>>> +
>>>>> +    s->chain4_alias = NULL;
>>>>> +
>>>>> +    if ((s->sr[0x02] & 0xf) == 0xf && s->sr[0x04] & 0x08) {
>>>>> +        offset = 0;
>>>>> +        switch ((s->gr[6] >> 2) & 3) {
>>>>> +        case 0:
>>>>> +            base = 0xa0000;
>>>>> +            size = 0x20000;
>>>>> +            break;
>>>>> +        case 1:
>>>>> +            base = 0xa0000;
>>>>> +            size = 0x10000;
>>>>> +            offset = s->bank_offset;
>>>>> +            break;
>>>>> +        case 2:
>>>>> +            base = 0xb0000;
>>>>> +            size = 0x8000;
>>>>> +            break;
>>>>> +        case 3:
>>>>> +            base = 0xb8000;
>>>>> +            size = 0x8000;
>>>>> +            break;
>>>>> +        }
>>>>> +        region = g_malloc(sizeof(*region));
>>>>> +        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>>>>> +        memory_region_add_subregion_overlap(s->legacy_address_space, base,
>>>>> +                                            region, 2);
>>>>>   
>>>> This one eventually gives me the following in info mtree with -M g3beige
>>>> on qemu-system-ppc:
>>>>
>>>> (qemu) info mtree
>>>> memory
>>>> system addr 00000000 off 00000000 size 7fffffffffffffff
>>>> -vga.chain4 addr 000a0000 off 00000000 size 10000
>>>> -macio addr 80880000 off 00000000 size 80000
>>>> --macio-nvram addr 00060000 off 00000000 size 20000
>>>> --pmac-ide addr 00020000 off 00000000 size 1000
>>>> --cuda addr 00016000 off 00000000 size 2000
>>>> --escc-bar addr 00013000 off 00000000 size 40
>>>> --dbdma addr 00008000 off 00000000 size 1000
>>>> --heathrow-pic addr 00000000 off 00000000 size 1000
>>>> -vga.rom addr 80800000 off 00000000 size 10000
>>>> -vga.vram addr 80000000 off 00000000 size 800000
>>>> -vga-lowmem addr 800a0000 off 00000000 size 20000
>>>> -escc addr 80013000 off 00000000 size 40
>>>> -isa-mmio addr fe000000 off 00000000 size 200000
>>>> I/O
>>>> io addr 00000000 off 00000000 size 10000
>>>> -cmd646-bmdma addr 00000700 off 00000000 size 10
>>>> --cmd646-bmdma-ioport addr 0000000c off 00000000 size 4
>>>> --cmd646-bmdma-bus addr 00000008 off 00000000 size 4
>>>> --cmd646-bmdma-ioport addr 00000004 off 00000000 size 4
>>>> --cmd646-bmdma-bus addr 00000000 off 00000000 size 4
>>>> -cmd646-cmd addr 00000680 off 00000000 size 4
>>>> -cmd646-data addr 00000600 off 00000000 size 8
>>>> -cmd646-cmd addr 00000580 off 00000000 size 4
>>>> -cmd646-data addr 00000500 off 00000000 size 8
>>>> -ne2000 addr 00000400 off 00000000 size 100
>>>>
>>>> This ends up overmapping 0xa0000, effectively overwriting kernel data.
>>>> If I #if 0 the offending chunk out, everything is fine. I would assume
>>>> that chain4 really needs to be inside of lowmem? No idea about VGA, but
>>>> I'm sure you know what's going on :).
>>> Does this help?
>>>
>>> diff --git a/hw/vga.c b/hw/vga.c
>>> index 125fb29..0a0c5a6 100644
>>> --- a/hw/vga.c
>>> +++ b/hw/vga.c
>>> @@ -181,6 +181,7 @@ static void vga_update_memory_access(VGACommonState *s)
>>>              size = 0x8000;
>>>              break;
>>>          }
>>> +        base += isa_mem_base;
>>>          region = g_malloc(sizeof(*region));
>>>          memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>>>          memory_region_add_subregion_overlap(s->legacy_address_space, base,
>>
>> No longer oopses, but the screen looks chaotic now (black bar at bottom,
>> part of contents at top etc.).
> 
> Does this PPC machine map the ISA range and forward VGA accesses to the
> adapter in general?

If it does, please post a dump of the VGACommonState while the screen is
corrupted (gdb or via device_show [1]. Maybe I missed some condition
that prevents chain4 optimizations, and your guest triggers this.

Jan

[1] http://thread.gmane.org/gmane.comp.emulators.qemu/114853

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

On 12.09.2011, at 17:57, Jan Kiszka wrote:

> On 2011-09-12 17:49, Jan Kiszka wrote:
>> On 2011-09-12 17:45, Andreas Färber wrote:
>>> Am 12.09.2011 17:33, schrieb Jan Kiszka:
>>>> On 2011-09-12 17:20, Alexander Graf wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Most VGA memory access modes require MMIO handling as they demand weird
>>>>>> logic to get a byte from or into the video RAM. However, there is one
>>>>>> exception: chain 4 mode with all memory planes enabled for writing. This
>>>>>> mode actually allows lineary mapping, which can then be combined with
>>>>>> dirty logging to accelerate KVM.
>>>>>> 
>>>>>> This patch accelerates specifically VBE accesses like they are used by
>>>>>> grub in graphical mode. Not only the standard VGA adapter benefits from
>>>>>> this, also vmware and spice in VGA mode.
>>>>>> 
>>>>>> CC: Gerd Hoffmann <kraxel@redhat.com>
>>>>>> CC: Avi Kivity <avi@redhat.com>
>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>> 
>>>>> [...]
>>>>> 
>>>>>> +static void vga_update_memory_access(VGACommonState *s)
>>>>>> +{
>>>>>> +    MemoryRegion *region, *old_region = s->chain4_alias;
>>>>>> +    target_phys_addr_t base, offset, size;
>>>>>> +
>>>>>> +    s->chain4_alias = NULL;
>>>>>> +
>>>>>> +    if ((s->sr[0x02] & 0xf) == 0xf && s->sr[0x04] & 0x08) {
>>>>>> +        offset = 0;
>>>>>> +        switch ((s->gr[6] >> 2) & 3) {
>>>>>> +        case 0:
>>>>>> +            base = 0xa0000;
>>>>>> +            size = 0x20000;
>>>>>> +            break;
>>>>>> +        case 1:
>>>>>> +            base = 0xa0000;
>>>>>> +            size = 0x10000;
>>>>>> +            offset = s->bank_offset;
>>>>>> +            break;
>>>>>> +        case 2:
>>>>>> +            base = 0xb0000;
>>>>>> +            size = 0x8000;
>>>>>> +            break;
>>>>>> +        case 3:
>>>>>> +            base = 0xb8000;
>>>>>> +            size = 0x8000;
>>>>>> +            break;
>>>>>> +        }
>>>>>> +        region = g_malloc(sizeof(*region));
>>>>>> +        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>>>>>> +        memory_region_add_subregion_overlap(s->legacy_address_space, base,
>>>>>> +                                            region, 2);
>>>>>> 
>>>>> This one eventually gives me the following in info mtree with -M g3beige
>>>>> on qemu-system-ppc:
>>>>> 
>>>>> (qemu) info mtree
>>>>> memory
>>>>> system addr 00000000 off 00000000 size 7fffffffffffffff
>>>>> -vga.chain4 addr 000a0000 off 00000000 size 10000
>>>>> -macio addr 80880000 off 00000000 size 80000
>>>>> --macio-nvram addr 00060000 off 00000000 size 20000
>>>>> --pmac-ide addr 00020000 off 00000000 size 1000
>>>>> --cuda addr 00016000 off 00000000 size 2000
>>>>> --escc-bar addr 00013000 off 00000000 size 40
>>>>> --dbdma addr 00008000 off 00000000 size 1000
>>>>> --heathrow-pic addr 00000000 off 00000000 size 1000
>>>>> -vga.rom addr 80800000 off 00000000 size 10000
>>>>> -vga.vram addr 80000000 off 00000000 size 800000
>>>>> -vga-lowmem addr 800a0000 off 00000000 size 20000
>>>>> -escc addr 80013000 off 00000000 size 40
>>>>> -isa-mmio addr fe000000 off 00000000 size 200000
>>>>> I/O
>>>>> io addr 00000000 off 00000000 size 10000
>>>>> -cmd646-bmdma addr 00000700 off 00000000 size 10
>>>>> --cmd646-bmdma-ioport addr 0000000c off 00000000 size 4
>>>>> --cmd646-bmdma-bus addr 00000008 off 00000000 size 4
>>>>> --cmd646-bmdma-ioport addr 00000004 off 00000000 size 4
>>>>> --cmd646-bmdma-bus addr 00000000 off 00000000 size 4
>>>>> -cmd646-cmd addr 00000680 off 00000000 size 4
>>>>> -cmd646-data addr 00000600 off 00000000 size 8
>>>>> -cmd646-cmd addr 00000580 off 00000000 size 4
>>>>> -cmd646-data addr 00000500 off 00000000 size 8
>>>>> -ne2000 addr 00000400 off 00000000 size 100
>>>>> 
>>>>> This ends up overmapping 0xa0000, effectively overwriting kernel data.
>>>>> If I #if 0 the offending chunk out, everything is fine. I would assume
>>>>> that chain4 really needs to be inside of lowmem? No idea about VGA, but
>>>>> I'm sure you know what's going on :).
>>>> Does this help?
>>>> 
>>>> diff --git a/hw/vga.c b/hw/vga.c
>>>> index 125fb29..0a0c5a6 100644
>>>> --- a/hw/vga.c
>>>> +++ b/hw/vga.c
>>>> @@ -181,6 +181,7 @@ static void vga_update_memory_access(VGACommonState *s)
>>>>             size = 0x8000;
>>>>             break;
>>>>         }
>>>> +        base += isa_mem_base;
>>>>         region = g_malloc(sizeof(*region));
>>>>         memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>>>>         memory_region_add_subregion_overlap(s->legacy_address_space, base,
>>> 
>>> No longer oopses, but the screen looks chaotic now (black bar at bottom,
>>> part of contents at top etc.).
>> 
>> Does this PPC machine map the ISA range and forward VGA accesses to the
>> adapter in general?
> 
> If it does, please post a dump of the VGACommonState while the screen is
> corrupted (gdb or via device_show [1]. Maybe I missed some condition
> that prevents chain4 optimizations, and your guest triggers this.

Picture: http://dl.dropbox.com/u/8976842/qemu/Screen%20shot%202011-09-13%20at%2009.37.15.png

(qemu) info qtree 
bus: main-system-bus
  type System
  dev: fw_cfg, id "#8"
    dev-prop: ctl_iobase = 0x0
    dev-prop: data_iobase = 0x0
    irq 0
    mmio f0000510/00000002
    mmio f0000512/00000002
  dev: escc, id "#4"
    dev-prop: frequency = 3686400
    dev-prop: it_shift = 4
    dev-prop: disabled = 0
    dev-prop: disabled = 0
    dev-prop: chnBtype = 0
    dev-prop: chnAtype = 0
    dev-prop: chrB = <null>
    dev-prop: chrA = serial0
    irq 2
    mmio 80013000/00000040
  dev: grackle, id "#1"
    irq 0
    mmio fec00000/00001000
    mmio fee00000/00001000
    bus: pci
      type PCI
      dev: cmd646-ide, id "#6"
        dev-prop: secondary = 0
        bus-prop: addr = 03.0
        bus-prop: romfile = <null>
        bus-prop: rombar = 1
        bus-prop: multifunction = off
        bus-prop: command_serr_enable = on
        class IDE controller, addr 00:03.0, pci id 1095:0646 (sub 1af4:1100)
        bar 0: i/o at 0x500 [0x507]
        bar 1: i/o at 0x580 [0x583]
        bar 2: i/o at 0x600 [0x607]
        bar 3: i/o at 0x680 [0x683]
        bar 4: i/o at 0x700 [0x70f]
        bus: ide.1
          type IDE
        bus: ide.0
          type IDE
          dev: ide-cd, id "#7"
            dev-prop: drive = ide1-cd0
            dev-prop: logical_block_size = 512
            dev-prop: physical_block_size = 512
            dev-prop: min_io_size = 0
            dev-prop: opt_io_size = 0
            dev-prop: bootindex = -1
            dev-prop: discard_granularity = 0
            dev-prop: ver = "0.15.50"
            dev-prop: serial = "QM00003"
            bus-prop: unit = 0
      dev: ne2k_pci, id "#5"
        dev-prop: mac = 52:54:00:12:34:56
        dev-prop: vlan = 0
        dev-prop: netdev = <null>
        dev-prop: bootindex = -1
        bus-prop: addr = 02.0
        bus-prop: romfile = <null>
        bus-prop: rombar = 1
        bus-prop: multifunction = off
        bus-prop: command_serr_enable = on
        class Ethernet controller, addr 00:02.0, pci id 10ec:8029 (sub 1af4:1100)
        bar 0: i/o at 0x400 [0x4ff]
      dev: VGA, id "#3"
        bus-prop: addr = 01.0
        bus-prop: romfile = "vgabios-stdvga.bin"
        bus-prop: rombar = 1
        bus-prop: multifunction = off
        bus-prop: command_serr_enable = on
        class VGA controller, addr 00:01.0, pci id 1234:1111 (sub 1af4:1100)
        bar 0: mem at 0x80000000 [0x807fffff]
        bar 6: mem at 0x80800000 [0x8080ffff]
      dev: grackle, id "#2"
        bus-prop: addr = 00.0
        bus-prop: romfile = <null>
        bus-prop: rombar = 1
        bus-prop: multifunction = off
        bus-prop: command_serr_enable = on
        class Host bridge, addr 00:00.0, pci id 1057:0002 (sub 1af4:1100)



(qemu) device_show #3
dev: VGA, id "#3", version 2
  dev.
    version_id:         00000002
    config:             00 00 00 00 10 d1 cf 20 - 00 00 00 00 10 d1 d0 30
                        ...
    irq_state:          00 00 00 00 00 00 00 10 - 00 00 00 00 00 00 00 00
  vga.
    latch:              00000000
    sr_index:           00
    sr:                 00 00 0f 00 08 00 00 00
    gr_index:           00
    gr:                 00 00 00 00 00 40 05 00 - 00 00 00 00 00 00 00 00
    ar_index:           20
    ar:                 00 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00
                        ...
    ar_flip_flop:       00000001
    cr_index:           00
    cr:                 00 63 00 00 00 00 00 50 - 00 40 00 00 00 00 00 00
                        ...
    msr:                00
    fcr:                00
    st00:               00
    st01:               00
    dac_state:          00
    dac_sub_index:      00
    dac_read_index:     00
    dac_write_index:    10
    dac_cache:          3f 3f 3f
    palette:            00 00 00 00 00 2a 00 2a - 00 00 2a 2a 2a 00 00 2a
                        ...
    bank_offset:        00000000
    is_vbe_vmstate:     01
    vbe_index:          0004
    vbe_regs[00]:       b0c5
    vbe_regs[01]:       0320
    vbe_regs[02]:       0258
    vbe_regs[03]:       000f
    vbe_regs[04]:       0001
    vbe_regs[05]:       0000
    vbe_regs[06]:       0320
    vbe_regs[07]:       0258
    vbe_regs[08]:       0000
    vbe_regs[09]:       0000
    vbe_start_addr:     00000000
    vbe_line_offset:    00000640
    vbe_bank_mask:      0000007f



Thanks!

Alex

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-09-13 09:39, Alexander Graf wrote:
> 
> On 12.09.2011, at 17:57, Jan Kiszka wrote:
> 
>> On 2011-09-12 17:49, Jan Kiszka wrote:
>>> On 2011-09-12 17:45, Andreas Färber wrote:
>>>> Am 12.09.2011 17:33, schrieb Jan Kiszka:
>>>>> On 2011-09-12 17:20, Alexander Graf wrote:
>>>>>> Jan Kiszka wrote:
>>>>>>> Most VGA memory access modes require MMIO handling as they demand weird
>>>>>>> logic to get a byte from or into the video RAM. However, there is one
>>>>>>> exception: chain 4 mode with all memory planes enabled for writing. This
>>>>>>> mode actually allows lineary mapping, which can then be combined with
>>>>>>> dirty logging to accelerate KVM.
>>>>>>>
>>>>>>> This patch accelerates specifically VBE accesses like they are used by
>>>>>>> grub in graphical mode. Not only the standard VGA adapter benefits from
>>>>>>> this, also vmware and spice in VGA mode.
>>>>>>>
>>>>>>> CC: Gerd Hoffmann <kraxel@redhat.com>
>>>>>>> CC: Avi Kivity <avi@redhat.com>
>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>
>>>>>> [...]
>>>>>>
>>>>>>> +static void vga_update_memory_access(VGACommonState *s)
>>>>>>> +{
>>>>>>> +    MemoryRegion *region, *old_region = s->chain4_alias;
>>>>>>> +    target_phys_addr_t base, offset, size;
>>>>>>> +
>>>>>>> +    s->chain4_alias = NULL;
>>>>>>> +
>>>>>>> +    if ((s->sr[0x02] & 0xf) == 0xf && s->sr[0x04] & 0x08) {
>>>>>>> +        offset = 0;
>>>>>>> +        switch ((s->gr[6] >> 2) & 3) {
>>>>>>> +        case 0:
>>>>>>> +            base = 0xa0000;
>>>>>>> +            size = 0x20000;
>>>>>>> +            break;
>>>>>>> +        case 1:
>>>>>>> +            base = 0xa0000;
>>>>>>> +            size = 0x10000;
>>>>>>> +            offset = s->bank_offset;
>>>>>>> +            break;
>>>>>>> +        case 2:
>>>>>>> +            base = 0xb0000;
>>>>>>> +            size = 0x8000;
>>>>>>> +            break;
>>>>>>> +        case 3:
>>>>>>> +            base = 0xb8000;
>>>>>>> +            size = 0x8000;
>>>>>>> +            break;
>>>>>>> +        }
>>>>>>> +        region = g_malloc(sizeof(*region));
>>>>>>> +        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>>>>>>> +        memory_region_add_subregion_overlap(s->legacy_address_space, base,
>>>>>>> +                                            region, 2);
>>>>>>>
>>>>>> This one eventually gives me the following in info mtree with -M g3beige
>>>>>> on qemu-system-ppc:
>>>>>>
>>>>>> (qemu) info mtree
>>>>>> memory
>>>>>> system addr 00000000 off 00000000 size 7fffffffffffffff
>>>>>> -vga.chain4 addr 000a0000 off 00000000 size 10000
>>>>>> -macio addr 80880000 off 00000000 size 80000
>>>>>> --macio-nvram addr 00060000 off 00000000 size 20000
>>>>>> --pmac-ide addr 00020000 off 00000000 size 1000
>>>>>> --cuda addr 00016000 off 00000000 size 2000
>>>>>> --escc-bar addr 00013000 off 00000000 size 40
>>>>>> --dbdma addr 00008000 off 00000000 size 1000
>>>>>> --heathrow-pic addr 00000000 off 00000000 size 1000
>>>>>> -vga.rom addr 80800000 off 00000000 size 10000
>>>>>> -vga.vram addr 80000000 off 00000000 size 800000
>>>>>> -vga-lowmem addr 800a0000 off 00000000 size 20000
>>>>>> -escc addr 80013000 off 00000000 size 40
>>>>>> -isa-mmio addr fe000000 off 00000000 size 200000
>>>>>> I/O
>>>>>> io addr 00000000 off 00000000 size 10000
>>>>>> -cmd646-bmdma addr 00000700 off 00000000 size 10
>>>>>> --cmd646-bmdma-ioport addr 0000000c off 00000000 size 4
>>>>>> --cmd646-bmdma-bus addr 00000008 off 00000000 size 4
>>>>>> --cmd646-bmdma-ioport addr 00000004 off 00000000 size 4
>>>>>> --cmd646-bmdma-bus addr 00000000 off 00000000 size 4
>>>>>> -cmd646-cmd addr 00000680 off 00000000 size 4
>>>>>> -cmd646-data addr 00000600 off 00000000 size 8
>>>>>> -cmd646-cmd addr 00000580 off 00000000 size 4
>>>>>> -cmd646-data addr 00000500 off 00000000 size 8
>>>>>> -ne2000 addr 00000400 off 00000000 size 100
>>>>>>
>>>>>> This ends up overmapping 0xa0000, effectively overwriting kernel data.
>>>>>> If I #if 0 the offending chunk out, everything is fine. I would assume
>>>>>> that chain4 really needs to be inside of lowmem? No idea about VGA, but
>>>>>> I'm sure you know what's going on :).
>>>>> Does this help?
>>>>>
>>>>> diff --git a/hw/vga.c b/hw/vga.c
>>>>> index 125fb29..0a0c5a6 100644
>>>>> --- a/hw/vga.c
>>>>> +++ b/hw/vga.c
>>>>> @@ -181,6 +181,7 @@ static void vga_update_memory_access(VGACommonState *s)
>>>>>             size = 0x8000;
>>>>>             break;
>>>>>         }
>>>>> +        base += isa_mem_base;
>>>>>         region = g_malloc(sizeof(*region));
>>>>>         memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>>>>>         memory_region_add_subregion_overlap(s->legacy_address_space, base,
>>>>
>>>> No longer oopses, but the screen looks chaotic now (black bar at bottom,
>>>> part of contents at top etc.).
>>>
>>> Does this PPC machine map the ISA range and forward VGA accesses to the
>>> adapter in general?
>>
>> If it does, please post a dump of the VGACommonState while the screen is
>> corrupted (gdb or via device_show [1]. Maybe I missed some condition
>> that prevents chain4 optimizations, and your guest triggers this.
> 
> Picture: http://dl.dropbox.com/u/8976842/qemu/Screen%20shot%202011-09-13%20at%2009.37.15.png
> 
> (qemu) info qtree 
> bus: main-system-bus
>   type System
>   dev: fw_cfg, id "#8"
>     dev-prop: ctl_iobase = 0x0
>     dev-prop: data_iobase = 0x0
>     irq 0
>     mmio f0000510/00000002
>     mmio f0000512/00000002
>   dev: escc, id "#4"
>     dev-prop: frequency = 3686400
>     dev-prop: it_shift = 4
>     dev-prop: disabled = 0
>     dev-prop: disabled = 0
>     dev-prop: chnBtype = 0
>     dev-prop: chnAtype = 0
>     dev-prop: chrB = <null>
>     dev-prop: chrA = serial0
>     irq 2
>     mmio 80013000/00000040
>   dev: grackle, id "#1"
>     irq 0
>     mmio fec00000/00001000
>     mmio fee00000/00001000
>     bus: pci
>       type PCI
>       dev: cmd646-ide, id "#6"
>         dev-prop: secondary = 0
>         bus-prop: addr = 03.0
>         bus-prop: romfile = <null>
>         bus-prop: rombar = 1
>         bus-prop: multifunction = off
>         bus-prop: command_serr_enable = on
>         class IDE controller, addr 00:03.0, pci id 1095:0646 (sub 1af4:1100)
>         bar 0: i/o at 0x500 [0x507]
>         bar 1: i/o at 0x580 [0x583]
>         bar 2: i/o at 0x600 [0x607]
>         bar 3: i/o at 0x680 [0x683]
>         bar 4: i/o at 0x700 [0x70f]
>         bus: ide.1
>           type IDE
>         bus: ide.0
>           type IDE
>           dev: ide-cd, id "#7"
>             dev-prop: drive = ide1-cd0
>             dev-prop: logical_block_size = 512
>             dev-prop: physical_block_size = 512
>             dev-prop: min_io_size = 0
>             dev-prop: opt_io_size = 0
>             dev-prop: bootindex = -1
>             dev-prop: discard_granularity = 0
>             dev-prop: ver = "0.15.50"
>             dev-prop: serial = "QM00003"
>             bus-prop: unit = 0
>       dev: ne2k_pci, id "#5"
>         dev-prop: mac = 52:54:00:12:34:56
>         dev-prop: vlan = 0
>         dev-prop: netdev = <null>
>         dev-prop: bootindex = -1
>         bus-prop: addr = 02.0
>         bus-prop: romfile = <null>
>         bus-prop: rombar = 1
>         bus-prop: multifunction = off
>         bus-prop: command_serr_enable = on
>         class Ethernet controller, addr 00:02.0, pci id 10ec:8029 (sub 1af4:1100)
>         bar 0: i/o at 0x400 [0x4ff]
>       dev: VGA, id "#3"
>         bus-prop: addr = 01.0
>         bus-prop: romfile = "vgabios-stdvga.bin"
>         bus-prop: rombar = 1
>         bus-prop: multifunction = off
>         bus-prop: command_serr_enable = on
>         class VGA controller, addr 00:01.0, pci id 1234:1111 (sub 1af4:1100)
>         bar 0: mem at 0x80000000 [0x807fffff]
>         bar 6: mem at 0x80800000 [0x8080ffff]
>       dev: grackle, id "#2"
>         bus-prop: addr = 00.0
>         bus-prop: romfile = <null>
>         bus-prop: rombar = 1
>         bus-prop: multifunction = off
>         bus-prop: command_serr_enable = on
>         class Host bridge, addr 00:00.0, pci id 1057:0002 (sub 1af4:1100)
> 
> 
> 
> (qemu) device_show #3
> dev: VGA, id "#3", version 2
>   dev.
>     version_id:         00000002
>     config:             00 00 00 00 10 d1 cf 20 - 00 00 00 00 10 d1 d0 30
>                         ...
>     irq_state:          00 00 00 00 00 00 00 10 - 00 00 00 00 00 00 00 00
>   vga.
>     latch:              00000000
>     sr_index:           00
>     sr:                 00 00 0f 00 08 00 00 00
>     gr_index:           00
>     gr:                 00 00 00 00 00 40 05 00 - 00 00 00 00 00 00 00 00
>     ar_index:           20
>     ar:                 00 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00
>                         ...
>     ar_flip_flop:       00000001
>     cr_index:           00
>     cr:                 00 63 00 00 00 00 00 50 - 00 40 00 00 00 00 00 00
>                         ...
>     msr:                00
>     fcr:                00
>     st00:               00
>     st01:               00
>     dac_state:          00
>     dac_sub_index:      00
>     dac_read_index:     00
>     dac_write_index:    10
>     dac_cache:          3f 3f 3f
>     palette:            00 00 00 00 00 2a 00 2a - 00 00 2a 2a 2a 00 00 2a
>                         ...
>     bank_offset:        00000000
>     is_vbe_vmstate:     01
>     vbe_index:          0004
>     vbe_regs[00]:       b0c5
>     vbe_regs[01]:       0320
>     vbe_regs[02]:       0258
>     vbe_regs[03]:       000f
>     vbe_regs[04]:       0001
>     vbe_regs[05]:       0000
>     vbe_regs[06]:       0320
>     vbe_regs[07]:       0258
>     vbe_regs[08]:       0000
>     vbe_regs[09]:       0000
>     vbe_start_addr:     00000000
>     vbe_line_offset:    00000640
>     vbe_bank_mask:      0000007f
> 

Makes no sense, must work with this setup. Maybe it's dynamic effect
when switching modes of bank offsets. Do you have some test image for me?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

[-- Attachment #1: Type: text/plain, Size: 10729 bytes --]


On 13.09.2011, at 10:14, Jan Kiszka wrote:

> On 2011-09-13 09:39, Alexander Graf wrote:
>> 
>> On 12.09.2011, at 17:57, Jan Kiszka wrote:
>> 
>>> On 2011-09-12 17:49, Jan Kiszka wrote:
>>>> On 2011-09-12 17:45, Andreas Färber wrote:
>>>>> Am 12.09.2011 17:33, schrieb Jan Kiszka:
>>>>>> On 2011-09-12 17:20, Alexander Graf wrote:
>>>>>>> Jan Kiszka wrote:
>>>>>>>> Most VGA memory access modes require MMIO handling as they demand weird
>>>>>>>> logic to get a byte from or into the video RAM. However, there is one
>>>>>>>> exception: chain 4 mode with all memory planes enabled for writing. This
>>>>>>>> mode actually allows lineary mapping, which can then be combined with
>>>>>>>> dirty logging to accelerate KVM.
>>>>>>>> 
>>>>>>>> This patch accelerates specifically VBE accesses like they are used by
>>>>>>>> grub in graphical mode. Not only the standard VGA adapter benefits from
>>>>>>>> this, also vmware and spice in VGA mode.
>>>>>>>> 
>>>>>>>> CC: Gerd Hoffmann <kraxel@redhat.com>
>>>>>>>> CC: Avi Kivity <avi@redhat.com>
>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>> 
>>>>>>> [...]
>>>>>>> 
>>>>>>>> +static void vga_update_memory_access(VGACommonState *s)
>>>>>>>> +{
>>>>>>>> +    MemoryRegion *region, *old_region = s->chain4_alias;
>>>>>>>> +    target_phys_addr_t base, offset, size;
>>>>>>>> +
>>>>>>>> +    s->chain4_alias = NULL;
>>>>>>>> +
>>>>>>>> +    if ((s->sr[0x02] & 0xf) == 0xf && s->sr[0x04] & 0x08) {
>>>>>>>> +        offset = 0;
>>>>>>>> +        switch ((s->gr[6] >> 2) & 3) {
>>>>>>>> +        case 0:
>>>>>>>> +            base = 0xa0000;
>>>>>>>> +            size = 0x20000;
>>>>>>>> +            break;
>>>>>>>> +        case 1:
>>>>>>>> +            base = 0xa0000;
>>>>>>>> +            size = 0x10000;
>>>>>>>> +            offset = s->bank_offset;
>>>>>>>> +            break;
>>>>>>>> +        case 2:
>>>>>>>> +            base = 0xb0000;
>>>>>>>> +            size = 0x8000;
>>>>>>>> +            break;
>>>>>>>> +        case 3:
>>>>>>>> +            base = 0xb8000;
>>>>>>>> +            size = 0x8000;
>>>>>>>> +            break;
>>>>>>>> +        }
>>>>>>>> +        region = g_malloc(sizeof(*region));
>>>>>>>> +        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>>>>>>>> +        memory_region_add_subregion_overlap(s->legacy_address_space, base,
>>>>>>>> +                                            region, 2);
>>>>>>>> 
>>>>>>> This one eventually gives me the following in info mtree with -M g3beige
>>>>>>> on qemu-system-ppc:
>>>>>>> 
>>>>>>> (qemu) info mtree
>>>>>>> memory
>>>>>>> system addr 00000000 off 00000000 size 7fffffffffffffff
>>>>>>> -vga.chain4 addr 000a0000 off 00000000 size 10000
>>>>>>> -macio addr 80880000 off 00000000 size 80000
>>>>>>> --macio-nvram addr 00060000 off 00000000 size 20000
>>>>>>> --pmac-ide addr 00020000 off 00000000 size 1000
>>>>>>> --cuda addr 00016000 off 00000000 size 2000
>>>>>>> --escc-bar addr 00013000 off 00000000 size 40
>>>>>>> --dbdma addr 00008000 off 00000000 size 1000
>>>>>>> --heathrow-pic addr 00000000 off 00000000 size 1000
>>>>>>> -vga.rom addr 80800000 off 00000000 size 10000
>>>>>>> -vga.vram addr 80000000 off 00000000 size 800000
>>>>>>> -vga-lowmem addr 800a0000 off 00000000 size 20000
>>>>>>> -escc addr 80013000 off 00000000 size 40
>>>>>>> -isa-mmio addr fe000000 off 00000000 size 200000
>>>>>>> I/O
>>>>>>> io addr 00000000 off 00000000 size 10000
>>>>>>> -cmd646-bmdma addr 00000700 off 00000000 size 10
>>>>>>> --cmd646-bmdma-ioport addr 0000000c off 00000000 size 4
>>>>>>> --cmd646-bmdma-bus addr 00000008 off 00000000 size 4
>>>>>>> --cmd646-bmdma-ioport addr 00000004 off 00000000 size 4
>>>>>>> --cmd646-bmdma-bus addr 00000000 off 00000000 size 4
>>>>>>> -cmd646-cmd addr 00000680 off 00000000 size 4
>>>>>>> -cmd646-data addr 00000600 off 00000000 size 8
>>>>>>> -cmd646-cmd addr 00000580 off 00000000 size 4
>>>>>>> -cmd646-data addr 00000500 off 00000000 size 8
>>>>>>> -ne2000 addr 00000400 off 00000000 size 100
>>>>>>> 
>>>>>>> This ends up overmapping 0xa0000, effectively overwriting kernel data.
>>>>>>> If I #if 0 the offending chunk out, everything is fine. I would assume
>>>>>>> that chain4 really needs to be inside of lowmem? No idea about VGA, but
>>>>>>> I'm sure you know what's going on :).
>>>>>> Does this help?
>>>>>> 
>>>>>> diff --git a/hw/vga.c b/hw/vga.c
>>>>>> index 125fb29..0a0c5a6 100644
>>>>>> --- a/hw/vga.c
>>>>>> +++ b/hw/vga.c
>>>>>> @@ -181,6 +181,7 @@ static void vga_update_memory_access(VGACommonState *s)
>>>>>>            size = 0x8000;
>>>>>>            break;
>>>>>>        }
>>>>>> +        base += isa_mem_base;
>>>>>>        region = g_malloc(sizeof(*region));
>>>>>>        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>>>>>>        memory_region_add_subregion_overlap(s->legacy_address_space, base,
>>>>> 
>>>>> No longer oopses, but the screen looks chaotic now (black bar at bottom,
>>>>> part of contents at top etc.).
>>>> 
>>>> Does this PPC machine map the ISA range and forward VGA accesses to the
>>>> adapter in general?
>>> 
>>> If it does, please post a dump of the VGACommonState while the screen is
>>> corrupted (gdb or via device_show [1]. Maybe I missed some condition
>>> that prevents chain4 optimizations, and your guest triggers this.
>> 
>> Picture: http://dl.dropbox.com/u/8976842/qemu/Screen%20shot%202011-09-13%20at%2009.37.15.png
>> 
>> (qemu) info qtree 
>> bus: main-system-bus
>>  type System
>>  dev: fw_cfg, id "#8"
>>    dev-prop: ctl_iobase = 0x0
>>    dev-prop: data_iobase = 0x0
>>    irq 0
>>    mmio f0000510/00000002
>>    mmio f0000512/00000002
>>  dev: escc, id "#4"
>>    dev-prop: frequency = 3686400
>>    dev-prop: it_shift = 4
>>    dev-prop: disabled = 0
>>    dev-prop: disabled = 0
>>    dev-prop: chnBtype = 0
>>    dev-prop: chnAtype = 0
>>    dev-prop: chrB = <null>
>>    dev-prop: chrA = serial0
>>    irq 2
>>    mmio 80013000/00000040
>>  dev: grackle, id "#1"
>>    irq 0
>>    mmio fec00000/00001000
>>    mmio fee00000/00001000
>>    bus: pci
>>      type PCI
>>      dev: cmd646-ide, id "#6"
>>        dev-prop: secondary = 0
>>        bus-prop: addr = 03.0
>>        bus-prop: romfile = <null>
>>        bus-prop: rombar = 1
>>        bus-prop: multifunction = off
>>        bus-prop: command_serr_enable = on
>>        class IDE controller, addr 00:03.0, pci id 1095:0646 (sub 1af4:1100)
>>        bar 0: i/o at 0x500 [0x507]
>>        bar 1: i/o at 0x580 [0x583]
>>        bar 2: i/o at 0x600 [0x607]
>>        bar 3: i/o at 0x680 [0x683]
>>        bar 4: i/o at 0x700 [0x70f]
>>        bus: ide.1
>>          type IDE
>>        bus: ide.0
>>          type IDE
>>          dev: ide-cd, id "#7"
>>            dev-prop: drive = ide1-cd0
>>            dev-prop: logical_block_size = 512
>>            dev-prop: physical_block_size = 512
>>            dev-prop: min_io_size = 0
>>            dev-prop: opt_io_size = 0
>>            dev-prop: bootindex = -1
>>            dev-prop: discard_granularity = 0
>>            dev-prop: ver = "0.15.50"
>>            dev-prop: serial = "QM00003"
>>            bus-prop: unit = 0
>>      dev: ne2k_pci, id "#5"
>>        dev-prop: mac = 52:54:00:12:34:56
>>        dev-prop: vlan = 0
>>        dev-prop: netdev = <null>
>>        dev-prop: bootindex = -1
>>        bus-prop: addr = 02.0
>>        bus-prop: romfile = <null>
>>        bus-prop: rombar = 1
>>        bus-prop: multifunction = off
>>        bus-prop: command_serr_enable = on
>>        class Ethernet controller, addr 00:02.0, pci id 10ec:8029 (sub 1af4:1100)
>>        bar 0: i/o at 0x400 [0x4ff]
>>      dev: VGA, id "#3"
>>        bus-prop: addr = 01.0
>>        bus-prop: romfile = "vgabios-stdvga.bin"
>>        bus-prop: rombar = 1
>>        bus-prop: multifunction = off
>>        bus-prop: command_serr_enable = on
>>        class VGA controller, addr 00:01.0, pci id 1234:1111 (sub 1af4:1100)
>>        bar 0: mem at 0x80000000 [0x807fffff]
>>        bar 6: mem at 0x80800000 [0x8080ffff]
>>      dev: grackle, id "#2"
>>        bus-prop: addr = 00.0
>>        bus-prop: romfile = <null>
>>        bus-prop: rombar = 1
>>        bus-prop: multifunction = off
>>        bus-prop: command_serr_enable = on
>>        class Host bridge, addr 00:00.0, pci id 1057:0002 (sub 1af4:1100)
>> 
>> 
>> 
>> (qemu) device_show #3
>> dev: VGA, id "#3", version 2
>>  dev.
>>    version_id:         00000002
>>    config:             00 00 00 00 10 d1 cf 20 - 00 00 00 00 10 d1 d0 30
>>                        ...
>>    irq_state:          00 00 00 00 00 00 00 10 - 00 00 00 00 00 00 00 00
>>  vga.
>>    latch:              00000000
>>    sr_index:           00
>>    sr:                 00 00 0f 00 08 00 00 00
>>    gr_index:           00
>>    gr:                 00 00 00 00 00 40 05 00 - 00 00 00 00 00 00 00 00
>>    ar_index:           20
>>    ar:                 00 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00
>>                        ...
>>    ar_flip_flop:       00000001
>>    cr_index:           00
>>    cr:                 00 63 00 00 00 00 00 50 - 00 40 00 00 00 00 00 00
>>                        ...
>>    msr:                00
>>    fcr:                00
>>    st00:               00
>>    st01:               00
>>    dac_state:          00
>>    dac_sub_index:      00
>>    dac_read_index:     00
>>    dac_write_index:    10
>>    dac_cache:          3f 3f 3f
>>    palette:            00 00 00 00 00 2a 00 2a - 00 00 2a 2a 2a 00 00 2a
>>                        ...
>>    bank_offset:        00000000
>>    is_vbe_vmstate:     01
>>    vbe_index:          0004
>>    vbe_regs[00]:       b0c5
>>    vbe_regs[01]:       0320
>>    vbe_regs[02]:       0258
>>    vbe_regs[03]:       000f
>>    vbe_regs[04]:       0001
>>    vbe_regs[05]:       0000
>>    vbe_regs[06]:       0320
>>    vbe_regs[07]:       0258
>>    vbe_regs[08]:       0000
>>    vbe_regs[09]:       0000
>>    vbe_start_addr:     00000000
>>    vbe_line_offset:    00000640
>>    vbe_bank_mask:      0000007f
>> 
> 
> Makes no sense, must work with this setup. Maybe it's dynamic effect
> when switching modes of bank offsets. Do you have some test image for me?

No need for an image - the black bar already occurs in firmware. If you want to check that everything really does work, Aurelien has a great collection of QEMU Debian images:

  http://people.debian.org/~aurel32/qemu/powerpc/


Thanks for looking into this :)

Alex


[-- Attachment #2: Type: text/html, Size: 47821 bytes --]

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Andreas Färber]

Am 13.09.2011 um 10:14 schrieb Jan Kiszka:

> On 2011-09-13 09:39, Alexander Graf wrote:
>> (qemu) device_show #3
>> dev: VGA, id "#3", version 2
>>  dev.
>>    version_id:         00000002
>>    config:             00 00 00 00 10 d1 cf 20 - 00 00 00 00 10 d1  
>> d0 30
>>                        ...
>>    irq_state:          00 00 00 00 00 00 00 10 - 00 00 00 00 00 00  
>> 00 00
>>  vga.
>>    latch:              00000000
>>    sr_index:           00
>>    sr:                 00 00 0f 00 08 00 00 00
>>    gr_index:           00
>>    gr:                 00 00 00 00 00 40 05 00 - 00 00 00 00 00 00  
>> 00 00
>>    ar_index:           20
>>    ar:                 00 00 00 00 00 00 00 00 - 00 00 00 00 00 00  
>> 00 00
>>                        ...
>>    ar_flip_flop:       00000001
>>    cr_index:           00
>>    cr:                 00 63 00 00 00 00 00 50 - 00 40 00 00 00 00  
>> 00 00
>>                        ...
>>    msr:                00
>>    fcr:                00
>>    st00:               00
>>    st01:               00
>>    dac_state:          00
>>    dac_sub_index:      00
>>    dac_read_index:     00
>>    dac_write_index:    10
>>    dac_cache:          3f 3f 3f
>>    palette:            00 00 00 00 00 2a 00 2a - 00 00 2a 2a 2a 00  
>> 00 2a
>>                        ...
>>    bank_offset:        00000000
>>    is_vbe_vmstate:     01
>>    vbe_index:          0004
>>    vbe_regs[00]:       b0c5
>>    vbe_regs[01]:       0320
>>    vbe_regs[02]:       0258
>>    vbe_regs[03]:       000f
>>    vbe_regs[04]:       0001
>>    vbe_regs[05]:       0000
>>    vbe_regs[06]:       0320
>>    vbe_regs[07]:       0258
>>    vbe_regs[08]:       0000
>>    vbe_regs[09]:       0000
>>    vbe_start_addr:     00000000
>>    vbe_line_offset:    00000640
>>    vbe_bank_mask:      0000007f
>>
>
> Makes no sense, must work with this setup. Maybe it's dynamic effect
> when switching modes of bank offsets. Do you have some test image  
> for me?

I've been using a Debian netinst image, but businesscard should do as  
well:
http://cdimage.debian.org/debian-cd/6.0.2.1/powerpc/iso-cd/debian-6.0.2.1-powerpc-businesscard.iso

qemu-system-ppc -boot d -cdrom path/to.iso # at yaboot prompt type:  
install

Then in addition to the black bar in Alex' image, the penguin gets  
beheaded.

Andreas

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

On 13.09.2011, at 10:14, Jan Kiszka wrote:

> On 2011-09-13 09:39, Alexander Graf wrote:
>> 
>> On 12.09.2011, at 17:57, Jan Kiszka wrote:
>> 
>>> On 2011-09-12 17:49, Jan Kiszka wrote:
>>>> On 2011-09-12 17:45, Andreas Färber wrote:
>>>>> Am 12.09.2011 17:33, schrieb Jan Kiszka:
>>>>>> On 2011-09-12 17:20, Alexander Graf wrote:
>>>>>>> Jan Kiszka wrote:
>>>>>>>> Most VGA memory access modes require MMIO handling as they demand weird
>>>>>>>> logic to get a byte from or into the video RAM. However, there is one
>>>>>>>> exception: chain 4 mode with all memory planes enabled for writing. This
>>>>>>>> mode actually allows lineary mapping, which can then be combined with
>>>>>>>> dirty logging to accelerate KVM.
>>>>>>>> 
>>>>>>>> This patch accelerates specifically VBE accesses like they are used by
>>>>>>>> grub in graphical mode. Not only the standard VGA adapter benefits from
>>>>>>>> this, also vmware and spice in VGA mode.
>>>>>>>> 
>>>>>>>> CC: Gerd Hoffmann <kraxel@redhat.com>
>>>>>>>> CC: Avi Kivity <avi@redhat.com>
>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>> 
>>>>>>> [...]
>>>>>>> 
>>>>>>>> +static void vga_update_memory_access(VGACommonState *s)
>>>>>>>> +{
>>>>>>>> +    MemoryRegion *region, *old_region = s->chain4_alias;
>>>>>>>> +    target_phys_addr_t base, offset, size;
>>>>>>>> +
>>>>>>>> +    s->chain4_alias = NULL;
>>>>>>>> +
>>>>>>>> +    if ((s->sr[0x02] & 0xf) == 0xf && s->sr[0x04] & 0x08) {
>>>>>>>> +        offset = 0;
>>>>>>>> +        switch ((s->gr[6] >> 2) & 3) {
>>>>>>>> +        case 0:
>>>>>>>> +            base = 0xa0000;
>>>>>>>> +            size = 0x20000;
>>>>>>>> +            break;
>>>>>>>> +        case 1:
>>>>>>>> +            base = 0xa0000;
>>>>>>>> +            size = 0x10000;
>>>>>>>> +            offset = s->bank_offset;
>>>>>>>> +            break;
>>>>>>>> +        case 2:
>>>>>>>> +            base = 0xb0000;
>>>>>>>> +            size = 0x8000;
>>>>>>>> +            break;
>>>>>>>> +        case 3:
>>>>>>>> +            base = 0xb8000;
>>>>>>>> +            size = 0x8000;
>>>>>>>> +            break;
>>>>>>>> +        }
>>>>>>>> +        region = g_malloc(sizeof(*region));
>>>>>>>> +        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>>>>>>>> +        memory_region_add_subregion_overlap(s->legacy_address_space, base,
>>>>>>>> +                                            region, 2);
>>>>>>>> 
>>>>>>> This one eventually gives me the following in info mtree with -M g3beige
>>>>>>> on qemu-system-ppc:
>>>>>>> 
>>>>>>> (qemu) info mtree
>>>>>>> memory
>>>>>>> system addr 00000000 off 00000000 size 7fffffffffffffff
>>>>>>> -vga.chain4 addr 000a0000 off 00000000 size 10000
>>>>>>> -macio addr 80880000 off 00000000 size 80000
>>>>>>> --macio-nvram addr 00060000 off 00000000 size 20000
>>>>>>> --pmac-ide addr 00020000 off 00000000 size 1000
>>>>>>> --cuda addr 00016000 off 00000000 size 2000
>>>>>>> --escc-bar addr 00013000 off 00000000 size 40
>>>>>>> --dbdma addr 00008000 off 00000000 size 1000
>>>>>>> --heathrow-pic addr 00000000 off 00000000 size 1000
>>>>>>> -vga.rom addr 80800000 off 00000000 size 10000
>>>>>>> -vga.vram addr 80000000 off 00000000 size 800000
>>>>>>> -vga-lowmem addr 800a0000 off 00000000 size 20000
>>>>>>> -escc addr 80013000 off 00000000 size 40
>>>>>>> -isa-mmio addr fe000000 off 00000000 size 200000
>>>>>>> I/O
>>>>>>> io addr 00000000 off 00000000 size 10000
>>>>>>> -cmd646-bmdma addr 00000700 off 00000000 size 10
>>>>>>> --cmd646-bmdma-ioport addr 0000000c off 00000000 size 4
>>>>>>> --cmd646-bmdma-bus addr 00000008 off 00000000 size 4
>>>>>>> --cmd646-bmdma-ioport addr 00000004 off 00000000 size 4
>>>>>>> --cmd646-bmdma-bus addr 00000000 off 00000000 size 4
>>>>>>> -cmd646-cmd addr 00000680 off 00000000 size 4
>>>>>>> -cmd646-data addr 00000600 off 00000000 size 8
>>>>>>> -cmd646-cmd addr 00000580 off 00000000 size 4
>>>>>>> -cmd646-data addr 00000500 off 00000000 size 8
>>>>>>> -ne2000 addr 00000400 off 00000000 size 100
>>>>>>> 
>>>>>>> This ends up overmapping 0xa0000, effectively overwriting kernel data.
>>>>>>> If I #if 0 the offending chunk out, everything is fine. I would assume
>>>>>>> that chain4 really needs to be inside of lowmem? No idea about VGA, but
>>>>>>> I'm sure you know what's going on :).
>>>>>> Does this help?
>>>>>> 
>>>>>> diff --git a/hw/vga.c b/hw/vga.c
>>>>>> index 125fb29..0a0c5a6 100644
>>>>>> --- a/hw/vga.c
>>>>>> +++ b/hw/vga.c
>>>>>> @@ -181,6 +181,7 @@ static void vga_update_memory_access(VGACommonState *s)
>>>>>>            size = 0x8000;
>>>>>>            break;
>>>>>>        }
>>>>>> +        base += isa_mem_base;
>>>>>>        region = g_malloc(sizeof(*region));
>>>>>>        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>>>>>>        memory_region_add_subregion_overlap(s->legacy_address_space, base,
>>>>> 
>>>>> No longer oopses, but the screen looks chaotic now (black bar at bottom,
>>>>> part of contents at top etc.).
>>>> 
>>>> Does this PPC machine map the ISA range and forward VGA accesses to the
>>>> adapter in general?
>>> 
>>> If it does, please post a dump of the VGACommonState while the screen is
>>> corrupted (gdb or via device_show [1]. Maybe I missed some condition
>>> that prevents chain4 optimizations, and your guest triggers this.
>> 
>> Picture: http://dl.dropbox.com/u/8976842/qemu/Screen%20shot%202011-09-13%20at%2009.37.15.png
>> 
>> (qemu) info qtree 
>> bus: main-system-bus
>>  type System
>>  dev: fw_cfg, id "#8"
>>    dev-prop: ctl_iobase = 0x0
>>    dev-prop: data_iobase = 0x0
>>    irq 0
>>    mmio f0000510/00000002
>>    mmio f0000512/00000002
>>  dev: escc, id "#4"
>>    dev-prop: frequency = 3686400
>>    dev-prop: it_shift = 4
>>    dev-prop: disabled = 0
>>    dev-prop: disabled = 0
>>    dev-prop: chnBtype = 0
>>    dev-prop: chnAtype = 0
>>    dev-prop: chrB = <null>
>>    dev-prop: chrA = serial0
>>    irq 2
>>    mmio 80013000/00000040
>>  dev: grackle, id "#1"
>>    irq 0
>>    mmio fec00000/00001000
>>    mmio fee00000/00001000
>>    bus: pci
>>      type PCI
>>      dev: cmd646-ide, id "#6"
>>        dev-prop: secondary = 0
>>        bus-prop: addr = 03.0
>>        bus-prop: romfile = <null>
>>        bus-prop: rombar = 1
>>        bus-prop: multifunction = off
>>        bus-prop: command_serr_enable = on
>>        class IDE controller, addr 00:03.0, pci id 1095:0646 (sub 1af4:1100)
>>        bar 0: i/o at 0x500 [0x507]
>>        bar 1: i/o at 0x580 [0x583]
>>        bar 2: i/o at 0x600 [0x607]
>>        bar 3: i/o at 0x680 [0x683]
>>        bar 4: i/o at 0x700 [0x70f]
>>        bus: ide.1
>>          type IDE
>>        bus: ide.0
>>          type IDE
>>          dev: ide-cd, id "#7"
>>            dev-prop: drive = ide1-cd0
>>            dev-prop: logical_block_size = 512
>>            dev-prop: physical_block_size = 512
>>            dev-prop: min_io_size = 0
>>            dev-prop: opt_io_size = 0
>>            dev-prop: bootindex = -1
>>            dev-prop: discard_granularity = 0
>>            dev-prop: ver = "0.15.50"
>>            dev-prop: serial = "QM00003"
>>            bus-prop: unit = 0
>>      dev: ne2k_pci, id "#5"
>>        dev-prop: mac = 52:54:00:12:34:56
>>        dev-prop: vlan = 0
>>        dev-prop: netdev = <null>
>>        dev-prop: bootindex = -1
>>        bus-prop: addr = 02.0
>>        bus-prop: romfile = <null>
>>        bus-prop: rombar = 1
>>        bus-prop: multifunction = off
>>        bus-prop: command_serr_enable = on
>>        class Ethernet controller, addr 00:02.0, pci id 10ec:8029 (sub 1af4:1100)
>>        bar 0: i/o at 0x400 [0x4ff]
>>      dev: VGA, id "#3"
>>        bus-prop: addr = 01.0
>>        bus-prop: romfile = "vgabios-stdvga.bin"
>>        bus-prop: rombar = 1
>>        bus-prop: multifunction = off
>>        bus-prop: command_serr_enable = on
>>        class VGA controller, addr 00:01.0, pci id 1234:1111 (sub 1af4:1100)
>>        bar 0: mem at 0x80000000 [0x807fffff]
>>        bar 6: mem at 0x80800000 [0x8080ffff]
>>      dev: grackle, id "#2"
>>        bus-prop: addr = 00.0
>>        bus-prop: romfile = <null>
>>        bus-prop: rombar = 1
>>        bus-prop: multifunction = off
>>        bus-prop: command_serr_enable = on
>>        class Host bridge, addr 00:00.0, pci id 1057:0002 (sub 1af4:1100)
>> 
>> 
>> 
>> (qemu) device_show #3
>> dev: VGA, id "#3", version 2
>>  dev.
>>    version_id:         00000002
>>    config:             00 00 00 00 10 d1 cf 20 - 00 00 00 00 10 d1 d0 30
>>                        ...
>>    irq_state:          00 00 00 00 00 00 00 10 - 00 00 00 00 00 00 00 00
>>  vga.
>>    latch:              00000000
>>    sr_index:           00
>>    sr:                 00 00 0f 00 08 00 00 00
>>    gr_index:           00
>>    gr:                 00 00 00 00 00 40 05 00 - 00 00 00 00 00 00 00 00
>>    ar_index:           20
>>    ar:                 00 00 00 00 00 00 00 00 - 00 00 00 00 00 00 00 00
>>                        ...
>>    ar_flip_flop:       00000001
>>    cr_index:           00
>>    cr:                 00 63 00 00 00 00 00 50 - 00 40 00 00 00 00 00 00
>>                        ...
>>    msr:                00
>>    fcr:                00
>>    st00:               00
>>    st01:               00
>>    dac_state:          00
>>    dac_sub_index:      00
>>    dac_read_index:     00
>>    dac_write_index:    10
>>    dac_cache:          3f 3f 3f
>>    palette:            00 00 00 00 00 2a 00 2a - 00 00 2a 2a 2a 00 00 2a
>>                        ...
>>    bank_offset:        00000000
>>    is_vbe_vmstate:     01
>>    vbe_index:          0004
>>    vbe_regs[00]:       b0c5
>>    vbe_regs[01]:       0320
>>    vbe_regs[02]:       0258
>>    vbe_regs[03]:       000f
>>    vbe_regs[04]:       0001
>>    vbe_regs[05]:       0000
>>    vbe_regs[06]:       0320
>>    vbe_regs[07]:       0258
>>    vbe_regs[08]:       0000
>>    vbe_regs[09]:       0000
>>    vbe_start_addr:     00000000
>>    vbe_line_offset:    00000640
>>    vbe_bank_mask:      0000007f
>> 
> 
> Makes no sense, must work with this setup. Maybe it's dynamic effect
> when switching modes of bank offsets. Do you have some test image for me?

Btw, it still tries to execute invalid code even with your patch. #if 0'ing out the memory region updates at least get the guest booting for me. Btw, to get it working you also need a patch for the interrupt controller (another breakage thanks to memory api).

diff --git a/hw/heathrow_pic.c b/hw/heathrow_pic.c
index 51996ab..16f48d1 100644
--- a/hw/heathrow_pic.c
+++ b/hw/heathrow_pic.c
@@ -126,7 +126,7 @@ static uint64_t pic_read(void *opaque, target_phys_addr_t addr,
 static const MemoryRegionOps heathrow_pic_ops = {
     .read = pic_read,
     .write = pic_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
 static void heathrow_pic_set_irq(void *opaque, int num, int level)


Alex

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-09-13 10:40, Alexander Graf wrote:
> Btw, it still tries to execute invalid code even with your patch. #if 0'ing out the memory region updates at least get the guest booting for me. Btw, to get it working you also need a patch for the interrupt controller (another breakage thanks to memory api).
> 
> diff --git a/hw/heathrow_pic.c b/hw/heathrow_pic.c
> index 51996ab..16f48d1 100644
> --- a/hw/heathrow_pic.c
> +++ b/hw/heathrow_pic.c
> @@ -126,7 +126,7 @@ static uint64_t pic_read(void *opaque, target_phys_addr_t addr,
>  static const MemoryRegionOps heathrow_pic_ops = {
>      .read = pic_read,
>      .write = pic_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
>  };
>  
>  static void heathrow_pic_set_irq(void *opaque, int num, int level)
> 

With out without this fix, with or without active chain-4 optimization,
I just get an empty yellow screen when firing up qemu-system-ppc (also
when using the Debian ISO). Do I need to specify a specific machine type?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

On 13.09.2011, at 11:00, Jan Kiszka wrote:

> On 2011-09-13 10:40, Alexander Graf wrote:
>> Btw, it still tries to execute invalid code even with your patch. #if 0'ing out the memory region updates at least get the guest booting for me. Btw, to get it working you also need a patch for the interrupt controller (another breakage thanks to memory api).
>> 
>> diff --git a/hw/heathrow_pic.c b/hw/heathrow_pic.c
>> index 51996ab..16f48d1 100644
>> --- a/hw/heathrow_pic.c
>> +++ b/hw/heathrow_pic.c
>> @@ -126,7 +126,7 @@ static uint64_t pic_read(void *opaque, target_phys_addr_t addr,
>> static const MemoryRegionOps heathrow_pic_ops = {
>>     .read = pic_read,
>>     .write = pic_write,
>> -    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>> };
>> 
>> static void heathrow_pic_set_irq(void *opaque, int num, int level)
>> 
> 
> With out without this fix, with or without active chain-4 optimization,
> I just get an empty yellow screen when firing up qemu-system-ppc (also
> when using the Debian ISO). Do I need to specify a specific machine type?

Ugh. No, you only need this patch:

  [PATCH] PPC: Fix via-cuda memory registration

which fixes another recently introduced regression :)


Alex

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-09-13 11:42, Alexander Graf wrote:
> 
> On 13.09.2011, at 11:00, Jan Kiszka wrote:
> 
>> On 2011-09-13 10:40, Alexander Graf wrote:
>>> Btw, it still tries to execute invalid code even with your patch. #if 0'ing out the memory region updates at least get the guest booting for me. Btw, to get it working you also need a patch for the interrupt controller (another breakage thanks to memory api).
>>>
>>> diff --git a/hw/heathrow_pic.c b/hw/heathrow_pic.c
>>> index 51996ab..16f48d1 100644
>>> --- a/hw/heathrow_pic.c
>>> +++ b/hw/heathrow_pic.c
>>> @@ -126,7 +126,7 @@ static uint64_t pic_read(void *opaque, target_phys_addr_t addr,
>>> static const MemoryRegionOps heathrow_pic_ops = {
>>>     .read = pic_read,
>>>     .write = pic_write,
>>> -    .endianness = DEVICE_NATIVE_ENDIAN,
>>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>>> };
>>>
>>> static void heathrow_pic_set_irq(void *opaque, int num, int level)
>>>
>>
>> With out without this fix, with or without active chain-4 optimization,
>> I just get an empty yellow screen when firing up qemu-system-ppc (also
>> when using the Debian ISO). Do I need to specify a specific machine type?
> 
> Ugh. No, you only need this patch:
> 
>   [PATCH] PPC: Fix via-cuda memory registration
> 
> which fixes another recently introduced regression :)

That works now - and allowed me to identify the bug after enhancing info
mtree a bit:

(qemu) info mtree
memory
  addr 00000000 prio 0 size 7fffffffffffffff system
    addr 80880000 prio 1 size 80000 macio
      addr 808e0000 prio 0 size 20000 macio-nvram
      addr 808a0000 prio 0 size 1000 pmac-ide
      addr 80896000 prio 0 size 2000 cuda
      addr 80893000 prio 0 size 40 escc-bar
      addr 80888000 prio 0 size 1000 dbdma
      addr 80880000 prio 0 size 1000 heathrow-pic
    addr 80000000 prio 1 size 800000 vga.vram
    addr 800a0000 prio 1 size 20000 vga-lowmem
    ...

Here is the problem: Both the vram and the ISA range get mapped into
system address space, but the former eclipses the latter as it shows up
earlier in the list and has the same priority. This picture changes with
the chain-4 alias which has prio 2, thus maps over the vram.

It looks to me like the ISA address space is either misplaced at
0x80000000 or is not supposed to be mapped at all on PPC. Comments?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Blue Swirl]

On Tue, Sep 13, 2011 at 11:34 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2011-09-13 11:42, Alexander Graf wrote:
>>
>> On 13.09.2011, at 11:00, Jan Kiszka wrote:
>>
>>> On 2011-09-13 10:40, Alexander Graf wrote:
>>>> Btw, it still tries to execute invalid code even with your patch. #if 0'ing out the memory region updates at least get the guest booting for me. Btw, to get it working you also need a patch for the interrupt controller (another breakage thanks to memory api).
>>>>
>>>> diff --git a/hw/heathrow_pic.c b/hw/heathrow_pic.c
>>>> index 51996ab..16f48d1 100644
>>>> --- a/hw/heathrow_pic.c
>>>> +++ b/hw/heathrow_pic.c
>>>> @@ -126,7 +126,7 @@ static uint64_t pic_read(void *opaque, target_phys_addr_t addr,
>>>> static const MemoryRegionOps heathrow_pic_ops = {
>>>>     .read = pic_read,
>>>>     .write = pic_write,
>>>> -    .endianness = DEVICE_NATIVE_ENDIAN,
>>>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>>>> };
>>>>
>>>> static void heathrow_pic_set_irq(void *opaque, int num, int level)
>>>>
>>>
>>> With out without this fix, with or without active chain-4 optimization,
>>> I just get an empty yellow screen when firing up qemu-system-ppc (also
>>> when using the Debian ISO). Do I need to specify a specific machine type?
>>
>> Ugh. No, you only need this patch:
>>
>>   [PATCH] PPC: Fix via-cuda memory registration
>>
>> which fixes another recently introduced regression :)
>
> That works now - and allowed me to identify the bug after enhancing info
> mtree a bit:
>
> (qemu) info mtree
> memory
>  addr 00000000 prio 0 size 7fffffffffffffff system
>    addr 80880000 prio 1 size 80000 macio
>      addr 808e0000 prio 0 size 20000 macio-nvram
>      addr 808a0000 prio 0 size 1000 pmac-ide
>      addr 80896000 prio 0 size 2000 cuda
>      addr 80893000 prio 0 size 40 escc-bar
>      addr 80888000 prio 0 size 1000 dbdma
>      addr 80880000 prio 0 size 1000 heathrow-pic
>    addr 80000000 prio 1 size 800000 vga.vram
>    addr 800a0000 prio 1 size 20000 vga-lowmem
>    ...
>
> Here is the problem: Both the vram and the ISA range get mapped into
> system address space, but the former eclipses the latter as it shows up
> earlier in the list and has the same priority. This picture changes with
> the chain-4 alias which has prio 2, thus maps over the vram.
>
> It looks to me like the ISA address space is either misplaced at
> 0x80000000 or is not supposed to be mapped at all on PPC. Comments?

Since there is no PCI-ISA bridge, ISA address space shouldn't exist.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 09/13/2011 10:39 PM, Blue Swirl wrote:
> >
> >  Here is the problem: Both the vram and the ISA range get mapped into
> >  system address space, but the former eclipses the latter as it shows up
> >  earlier in the list and has the same priority. This picture changes with
> >  the chain-4 alias which has prio 2, thus maps over the vram.
> >
> >  It looks to me like the ISA address space is either misplaced at
> >  0x80000000 or is not supposed to be mapped at all on PPC. Comments?
>
> Since there is no PCI-ISA bridge, ISA address space shouldn't exist.

Where does the vga device sit then?

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

On 14.09.2011, at 09:11, Avi Kivity wrote:

> On 09/13/2011 10:39 PM, Blue Swirl wrote:
>> >
>> >  Here is the problem: Both the vram and the ISA range get mapped into
>> >  system address space, but the former eclipses the latter as it shows up
>> >  earlier in the list and has the same priority. This picture changes with
>> >  the chain-4 alias which has prio 2, thus maps over the vram.
>> >
>> >  It looks to me like the ISA address space is either misplaced at
>> >  0x80000000 or is not supposed to be mapped at all on PPC. Comments?
>> 
>> Since there is no PCI-ISA bridge, ISA address space shouldn't exist.
> 
> Where does the vga device sit then?

On the PCI bus? :)


Alex

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-09-14 09:42, Alexander Graf wrote:
> 
> On 14.09.2011, at 09:11, Avi Kivity wrote:
> 
>> On 09/13/2011 10:39 PM, Blue Swirl wrote:
>>>>
>>>>  Here is the problem: Both the vram and the ISA range get mapped into
>>>>  system address space, but the former eclipses the latter as it shows up
>>>>  earlier in the list and has the same priority. This picture changes with
>>>>  the chain-4 alias which has prio 2, thus maps over the vram.
>>>>
>>>>  It looks to me like the ISA address space is either misplaced at
>>>>  0x80000000 or is not supposed to be mapped at all on PPC. Comments?
>>>
>>> Since there is no PCI-ISA bridge, ISA address space shouldn't exist.
>>
>> Where does the vga device sit then?
> 
> On the PCI bus? :)

Then make sure that the container for ISA resources is a dummy region -
or even NULL so that VGA will know that it's supposed to skip ISA
registrations.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 09/14/2011 10:42 AM, Alexander Graf wrote:
> On 14.09.2011, at 09:11, Avi Kivity wrote:
>
> >  On 09/13/2011 10:39 PM, Blue Swirl wrote:
> >>  >
> >>  >   Here is the problem: Both the vram and the ISA range get mapped into
> >>  >   system address space, but the former eclipses the latter as it shows up
> >>  >   earlier in the list and has the same priority. This picture changes with
> >>  >   the chain-4 alias which has prio 2, thus maps over the vram.
> >>  >
> >>  >   It looks to me like the ISA address space is either misplaced at
> >>  >   0x80000000 or is not supposed to be mapped at all on PPC. Comments?
> >>
> >>  Since there is no PCI-ISA bridge, ISA address space shouldn't exist.
> >
> >  Where does the vga device sit then?
>
> On the PCI bus? :)
>

I thought it was std vga, which is an ISA device.

Anyway PCI supports the vga region at 0xa0000-0xc0000.  Where is it 
supposed to be mapped?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-09-14 10:17, Avi Kivity wrote:
> On 09/14/2011 10:42 AM, Alexander Graf wrote:
>> On 14.09.2011, at 09:11, Avi Kivity wrote:
>>
>>>  On 09/13/2011 10:39 PM, Blue Swirl wrote:
>>>>  >
>>>>  >   Here is the problem: Both the vram and the ISA range get mapped into
>>>>  >   system address space, but the former eclipses the latter as it shows up
>>>>  >   earlier in the list and has the same priority. This picture changes with
>>>>  >   the chain-4 alias which has prio 2, thus maps over the vram.
>>>>  >
>>>>  >   It looks to me like the ISA address space is either misplaced at
>>>>  >   0x80000000 or is not supposed to be mapped at all on PPC. Comments?
>>>>
>>>>  Since there is no PCI-ISA bridge, ISA address space shouldn't exist.
>>>
>>>  Where does the vga device sit then?
>>
>> On the PCI bus? :)
>>
> 
> I thought it was std vga, which is an ISA device.

There are both types (ISA-only and PCI).

> 
> Anyway PCI supports the vga region at 0xa0000-0xc0000.  Where is it 
> supposed to be mapped?

...but not all PCI bridges make use of this feature / forward legacy
requests.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 09/14/2011 11:20 AM, Jan Kiszka wrote:
> >
> >  Anyway PCI supports the vga region at 0xa0000-0xc0000.  Where is it
> >  supposed to be mapped?
>
> ...but not all PCI bridges make use of this feature / forward legacy
> requests.
>

Then this should be fixed in the bridge?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-09-14 10:22, Avi Kivity wrote:
> On 09/14/2011 11:20 AM, Jan Kiszka wrote:
>>>
>>>  Anyway PCI supports the vga region at 0xa0000-0xc0000.  Where is it
>>>  supposed to be mapped?
>>
>> ...but not all PCI bridges make use of this feature / forward legacy
>> requests.
>>
> 
> Then this should be fixed in the bridge?

Yes, it's a PPC bug.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

On 14.09.2011, at 10:24, Jan Kiszka wrote:

> On 2011-09-14 10:22, Avi Kivity wrote:
>> On 09/14/2011 11:20 AM, Jan Kiszka wrote:
>>>> 
>>>> Anyway PCI supports the vga region at 0xa0000-0xc0000.  Where is it
>>>> supposed to be mapped?
>>> 
>>> ...but not all PCI bridges make use of this feature / forward legacy
>>> requests.
>>> 
>> 
>> Then this should be fixed in the bridge?
> 
> Yes, it's a PPC bug.

So how does the bridge not forward it then?


Alex

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Jan Kiszka]

On 2011-09-14 10:27, Alexander Graf wrote:
> 
> On 14.09.2011, at 10:24, Jan Kiszka wrote:
> 
>> On 2011-09-14 10:22, Avi Kivity wrote:
>>> On 09/14/2011 11:20 AM, Jan Kiszka wrote:
>>>>>
>>>>> Anyway PCI supports the vga region at 0xa0000-0xc0000.  Where is it
>>>>> supposed to be mapped?
>>>>
>>>> ...but not all PCI bridges make use of this feature / forward legacy
>>>> requests.
>>>>
>>>
>>> Then this should be fixed in the bridge?
>>
>> Yes, it's a PPC bug.
> 
> So how does the bridge not forward it then?

On real HW, by keeping the "VGA Enable" bit off. Or just not issuing
requests to the a0000..bffff range.

Under QEMU, I would simply provide the VGA model a memory region for
legacy stuff that remains unregistered.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 09/14/2011 11:27 AM, Alexander Graf wrote:
> On 14.09.2011, at 10:24, Jan Kiszka wrote:
>
> >  On 2011-09-14 10:22, Avi Kivity wrote:
> >>  On 09/14/2011 11:20 AM, Jan Kiszka wrote:
> >>>>
> >>>>  Anyway PCI supports the vga region at 0xa0000-0xc0000.  Where is it
> >>>>  supposed to be mapped?
> >>>
> >>>  ...but not all PCI bridges make use of this feature / forward legacy
> >>>  requests.
> >>>
> >>
> >>  Then this should be fixed in the bridge?
> >
> >  Yes, it's a PPC bug.
>
> So how does the bridge not forward it then?
>

I expect that currently vga adds the region to pci_address_space().  We 
need to create a pci_address_space_vga() function that returns a region 
for vga to use.  Then add or remove the region to pci_address_space(), 
within the bridge code, depending on whether the bridge forwards vga 
accesses or not.

(assuming I understood the problem correctly - not sure)

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Blue Swirl]

On Wed, Sep 14, 2011 at 8:35 AM, Avi Kivity <avi@redhat.com> wrote:
> On 09/14/2011 11:27 AM, Alexander Graf wrote:
>>
>> On 14.09.2011, at 10:24, Jan Kiszka wrote:
>>
>> >  On 2011-09-14 10:22, Avi Kivity wrote:
>> >>  On 09/14/2011 11:20 AM, Jan Kiszka wrote:
>> >>>>
>> >>>>  Anyway PCI supports the vga region at 0xa0000-0xc0000.  Where is it
>> >>>>  supposed to be mapped?
>> >>>
>> >>>  ...but not all PCI bridges make use of this feature / forward legacy
>> >>>  requests.
>> >>>
>> >>
>> >>  Then this should be fixed in the bridge?
>> >
>> >  Yes, it's a PPC bug.
>>
>> So how does the bridge not forward it then?
>>
>
> I expect that currently vga adds the region to pci_address_space().  We need
> to create a pci_address_space_vga() function that returns a region for vga
> to use.  Then add or remove the region to pci_address_space(), within the
> bridge code, depending on whether the bridge forwards vga accesses or not.

Similar treatment should be also needed for VGA IO ports 0x3b0 etc.

> (assuming I understood the problem correctly - not sure)

I think you did.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

On 14.09.2011, at 22:06, Blue Swirl wrote:

> On Wed, Sep 14, 2011 at 8:35 AM, Avi Kivity <avi@redhat.com> wrote:
>> On 09/14/2011 11:27 AM, Alexander Graf wrote:
>>> 
>>> On 14.09.2011, at 10:24, Jan Kiszka wrote:
>>> 
>>>>  On 2011-09-14 10:22, Avi Kivity wrote:
>>>>>  On 09/14/2011 11:20 AM, Jan Kiszka wrote:
>>>>>>> 
>>>>>>>  Anyway PCI supports the vga region at 0xa0000-0xc0000.  Where is it
>>>>>>>  supposed to be mapped?
>>>>>> 
>>>>>>  ...but not all PCI bridges make use of this feature / forward legacy
>>>>>>  requests.
>>>>>> 
>>>>> 
>>>>>  Then this should be fixed in the bridge?
>>>> 
>>>>  Yes, it's a PPC bug.
>>> 
>>> So how does the bridge not forward it then?
>>> 
>> 
>> I expect that currently vga adds the region to pci_address_space().  We need
>> to create a pci_address_space_vga() function that returns a region for vga
>> to use.  Then add or remove the region to pci_address_space(), within the
>> bridge code, depending on whether the bridge forwards vga accesses or not.
> 
> Similar treatment should be also needed for VGA IO ports 0x3b0 etc.
> 
>> (assuming I understood the problem correctly - not sure)
> 
> I think you did.

Well I don't completely, so would anybody who feels reasonably savvy in messing with the new memory api like to step up and implement this? :)


Alex

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 09/14/2011 11:14 PM, Alexander Graf wrote:
> >>  (assuming I understood the problem correctly - not sure)
> >
> >  I think you did.
>
> Well I don't completely, so would anybody who feels reasonably savvy in messing with the new memory api like to step up and implement this? :)
>
>

Can you explain what the memory map looks like from the hardware point 
of view?

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

Am 14.09.2011 um 22:16 schrieb Avi Kivity <avi@redhat.com>:

> On 09/14/2011 11:14 PM, Alexander Graf wrote:
>> >>  (assuming I understood the problem correctly - not sure)
>> >
>> >  I think you did.
>> 
>> Well I don't completely, so would anybody who feels reasonably savvy in messing with the new memory api like to step up and implement this? :)
>> 
>> 
> 
> Can you explain what the memory map looks like from the hardware point of view?

If you can tell me where to find out :). I seriously have zero experience in VGA mapping - and it sounds as if Blue has a pretty good idea what's going on.

Alex

> 

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Richard Henderson]

On 09/14/2011 01:35 PM, Alexander Graf wrote:

>> Can you explain what the memory map looks like from the hardware point of view?
> 
> If you can tell me where to find out :). I seriously have zero experience in VGA mapping - and it sounds as if Blue has a pretty good idea what's going on.

He's not interested in the VGA bits, but in the PPC board bits.
How are addresses forwarded from the main system bus to the
PCI host bridge, for instance?


r~

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Andreas Färber]

Am 14.09.2011 um 22:42 schrieb Richard Henderson:

> On 09/14/2011 01:35 PM, Alexander Graf wrote:
>
>>> Can you explain what the memory map looks like from the hardware  
>>> point of view?
>>
>> If you can tell me where to find out :). I seriously have zero  
>> experience in VGA mapping - and it sounds as if Blue has a pretty  
>> good idea what's going on.
>
> He's not interested in the VGA bits, but in the PPC board bits.
> How are addresses forwarded from the main system bus to the
> PCI host bridge, for instance?

Does this help?

http://tibit.org/ppc/imac-333.html

The historical dev trees at http://penguinppc.org/historical/dev-trees-html/trees-index.html 
  seem to have disappeared, this iMac seems closest: The ATI VGA card  
sits directly on /pci.

Later models of PowerMac G3 had AGP graphics, the final PowerMac G5  
models had PCIe graphics.
Note that ATI graphic cards were sold as special Mac Edition, so I  
wouldn't completely rule out deviations from PC standards...

Andreas

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

On 14.09.2011, at 22:42, Richard Henderson wrote:

> On 09/14/2011 01:35 PM, Alexander Graf wrote:
> 
>>> Can you explain what the memory map looks like from the hardware point of view?
>> 
>> If you can tell me where to find out :). I seriously have zero experience in VGA mapping - and it sounds as if Blue has a pretty good idea what's going on.
> 
> He's not interested in the VGA bits, but in the PPC board bits.
> How are addresses forwarded from the main system bus to the
> PCI host bridge, for instance?

Yeah, and what I'm trying to tell you is that I know about as much about the g3 beige as you do. However, Ben might now a bit more here. Let's ask him :).


Alex

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Benjamin Herrenschmidt]

On Wed, 2011-09-14 at 23:41 +0200, Alexander Graf wrote:
> On 14.09.2011, at 22:42, Richard Henderson wrote:
> 
> > On 09/14/2011 01:35 PM, Alexander Graf wrote:
> > 
> >>> Can you explain what the memory map looks like from the hardware
> point of view?
> >> 
> >> If you can tell me where to find out :). I seriously have zero
> experience in VGA mapping - and it sounds as if Blue has a pretty good
> idea what's going on.
> > 
> > He's not interested in the VGA bits, but in the PPC board bits.
> > How are addresses forwarded from the main system bus to the
> > PCI host bridge, for instance?
> 
> Yeah, and what I'm trying to tell you is that I know about as much
> about the g3 beige as you do. However, Ben might now a bit more here.
> Let's ask him :).

Can I have a bit of context please ? :-)

Cheers,
Ben.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 09/15/2011 04:24 AM, Benjamin Herrenschmidt wrote:
> On Wed, 2011-09-14 at 23:41 +0200, Alexander Graf wrote:
> >  On 14.09.2011, at 22:42, Richard Henderson wrote:
> >
> >  >  On 09/14/2011 01:35 PM, Alexander Graf wrote:
> >  >
> >  >>>  Can you explain what the memory map looks like from the hardware
> >  point of view?
> >  >>
> >  >>  If you can tell me where to find out :). I seriously have zero
> >  experience in VGA mapping - and it sounds as if Blue has a pretty good
> >  idea what's going on.
> >  >
> >  >  He's not interested in the VGA bits, but in the PPC board bits.
> >  >  How are addresses forwarded from the main system bus to the
> >  >  PCI host bridge, for instance?
> >
> >  Yeah, and what I'm trying to tell you is that I know about as much
> >  about the g3 beige as you do. However, Ben might now a bit more here.
> >  Let's ask him :).
>
> Can I have a bit of context please ? :-)

What does the memory map regarding a pci vga card looks like?  Right now 
it looks like some BAR has collided with the legacy vga range at 0xa0000 
(maped to 0x8000a0000) or  such.  What is supposed to be where?


-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

On 15.09.2011, at 03:24, Benjamin Herrenschmidt wrote:

> On Wed, 2011-09-14 at 23:41 +0200, Alexander Graf wrote:
>> On 14.09.2011, at 22:42, Richard Henderson wrote:
>> 
>>> On 09/14/2011 01:35 PM, Alexander Graf wrote:
>>> 
>>>>> Can you explain what the memory map looks like from the hardware
>> point of view?
>>>> 
>>>> If you can tell me where to find out :). I seriously have zero
>> experience in VGA mapping - and it sounds as if Blue has a pretty good
>> idea what's going on.
>>> 
>>> He's not interested in the VGA bits, but in the PPC board bits.
>>> How are addresses forwarded from the main system bus to the
>>> PCI host bridge, for instance?
>> 
>> Yeah, and what I'm trying to tell you is that I know about as much
>> about the g3 beige as you do. However, Ben might now a bit more here.
>> Let's ask him :).
> 
> Can I have a bit of context please ? :-)

Sure :). So the problem is that when emulating the G3 Beige machine in QEMU (default ppc32 target) we also add a PCI VGA adapter. Apparently, on x86 that PCI VGA adapter can map the special VGA regions to somewhere, namely 0xa0000. With the memory api overhaul, this also slipped into the PPC world where mapping 0xa0000 with VGA adapters is a pretty bad idea, as it's occupied by RAM.

Now the discussion was on which level that mapping would happen and which devices go through which buses which then would filter certain ranges from being mapped. Basically, which way does a memory request from the CPU go on a G3 Beige machine until it arrives the VGA adapter?

I hope that concludes the actual question. Avi, if I explained this wrong, please correct me.


Alex

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Benjamin Herrenschmidt]

> Sure :). So the problem is that when emulating the G3 Beige machine in
> QEMU (default ppc32 target) we also add a PCI VGA adapter. Apparently,
> on x86 that PCI VGA adapter can map the special VGA regions to
> somewhere, namely 0xa0000. With the memory api overhaul, this also
> slipped into the PPC world where mapping 0xa0000 with VGA adapters is
> a pretty bad idea, as it's occupied by RAM.
> 
> Now the discussion was on which level that mapping would happen and
> which devices go through which buses which then would filter certain
> ranges from being mapped. Basically, which way does a memory request
> from the CPU go on a G3 Beige machine until it arrives the VGA
> adapter?
> 
> I hope that concludes the actual question. Avi, if I explained this
> wrong, please correct me.

Ok so there's several things here.

First, the mapping from CPU addresses to PCI addresses. This depends on
the host bridge chip. The MPC106, used in the Beige G3, itself supports
different type of mappings.

>From memory, the way it's configured in a G3 is to have a 1:1 mapping of
80000000 CPU to 80000000 PCI.

That means that with this basic mapping, you cannot generate memory
accesses to low PCI addresses such as 0xa0000.

I don't remember (but it's possible) if it has another region which maps
some other (high address) part of the address space down to 0 PCI.
Typically that would be a smaller region which specifically allow access
to the "ISA hole" that way.

There is code in pci_process_bridge_OF_ranges() that  will detect such
an ISA hole, and while it cannot add it to the normal resource
management, it is remembered, so we -could- use it from the VGA code if
we wanted to (we don't today).

The problem is that most bridges used on Macs, typically the Apple ones
simply don't provide such a hole. In fact, most bridges used by IBM
aren't configured for that either.

Now back to your VGA adapter. The legacy VGA stuff does something called
"hard decoding", which means it decodes those legacy addresses, usually
without a BAR, but using a fixed address decoding. This is old ISA crap
emulated on top of PCI, exist only thanks to a "hack" in the PCI spec,
in order to be backward compatible with DOS and shit like that.

Ideally, you should provide a BAR to allow remapping that stuff
elsewhere and a setting to enable/disable the hard-decoding. That way,
on power, the firmware could just whack that setting and turn your VGA
device into something that behaves properly like a normal PCI device.

Macs or pSeries never really used the legacy crap. We always had drivers
that configured the cards into "native" mode, which means no hard
decoding (some old cards still hard decoded some IO ports but that went
away on anything modern), and just using proper MMIO register and linear
framebuffer from the driver. That does mean we never used text mode
though. It would be possible to still allow text mode by having a BAR in
the emulated card that can be used to move the VGA legacy crap around
tho if we really wanted to.

BTW. I haven't looked at the code, but I've been told that for some of
the splice stuff or other higher level additions, you have implemented
special IO ports in the card. This is totally ass backward. IOs are old
bad junk and must die. MMIO is semi-acceptable, commands in virtio would
be better (and perform better), but not IO ports, please ..... 

Cheers,
Ben.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 09/15/2011 01:01 PM, Benjamin Herrenschmidt wrote:
> >  Sure :). So the problem is that when emulating the G3 Beige machine in
> >  QEMU (default ppc32 target) we also add a PCI VGA adapter. Apparently,
> >  on x86 that PCI VGA adapter can map the special VGA regions to
> >  somewhere, namely 0xa0000. With the memory api overhaul, this also
> >  slipped into the PPC world where mapping 0xa0000 with VGA adapters is
> >  a pretty bad idea, as it's occupied by RAM.
> >
> >  Now the discussion was on which level that mapping would happen and
> >  which devices go through which buses which then would filter certain
> >  ranges from being mapped. Basically, which way does a memory request
> >  from the CPU go on a G3 Beige machine until it arrives the VGA
> >  adapter?
> >
> >  I hope that concludes the actual question. Avi, if I explained this
> >  wrong, please correct me.
>
> Ok so there's several things here.
>
> First, the mapping from CPU addresses to PCI addresses. This depends on
> the host bridge chip. The MPC106, used in the Beige G3, itself supports
> different type of mappings.
>
>  From memory, the way it's configured in a G3 is to have a 1:1 mapping of
> 80000000 CPU to 80000000 PCI.
>
> That means that with this basic mapping, you cannot generate memory
> accesses to low PCI addresses such as 0xa0000.

Alex, what this means (I think is) that: pci_grackle_init() needs to 
create a container memory region and pass it to pc_register_bus() as the 
pci address space, and create and alias starting at 0x80000000 of the 
pci address space, and map that alias at address 0x80000000 of the 
system address space.

See pc_init1() creating pci_memory and passing it to i440fx_init(), 
which then maps some aliases into the system address space and also 
gives it to pci_bus_new().  It's essentially the same thing with 
different details.

> I don't remember (but it's possible) if it has another region which maps
> some other (high address) part of the address space down to 0 PCI.
> Typically that would be a smaller region which specifically allow access
> to the "ISA hole" that way.

That would be done by mapping yet another alias.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Blue Swirl]

[-- Attachment #1: Type: text/plain, Size: 2567 bytes --]

On Thu, Sep 15, 2011 at 11:31 AM, Avi Kivity <avi@redhat.com> wrote:
> On 09/15/2011 01:01 PM, Benjamin Herrenschmidt wrote:
>>
>> >  Sure :). So the problem is that when emulating the G3 Beige machine in
>> >  QEMU (default ppc32 target) we also add a PCI VGA adapter. Apparently,
>> >  on x86 that PCI VGA adapter can map the special VGA regions to
>> >  somewhere, namely 0xa0000. With the memory api overhaul, this also
>> >  slipped into the PPC world where mapping 0xa0000 with VGA adapters is
>> >  a pretty bad idea, as it's occupied by RAM.
>> >
>> >  Now the discussion was on which level that mapping would happen and
>> >  which devices go through which buses which then would filter certain
>> >  ranges from being mapped. Basically, which way does a memory request
>> >  from the CPU go on a G3 Beige machine until it arrives the VGA
>> >  adapter?
>> >
>> >  I hope that concludes the actual question. Avi, if I explained this
>> >  wrong, please correct me.
>>
>> Ok so there's several things here.
>>
>> First, the mapping from CPU addresses to PCI addresses. This depends on
>> the host bridge chip. The MPC106, used in the Beige G3, itself supports
>> different type of mappings.
>>
>>  From memory, the way it's configured in a G3 is to have a 1:1 mapping of
>> 80000000 CPU to 80000000 PCI.
>>
>> That means that with this basic mapping, you cannot generate memory
>> accesses to low PCI addresses such as 0xa0000.
>
> Alex, what this means (I think is) that: pci_grackle_init() needs to create
> a container memory region and pass it to pc_register_bus() as the pci
> address space, and create and alias starting at 0x80000000 of the pci
> address space, and map that alias at address 0x80000000 of the system
> address space.
>
> See pc_init1() creating pci_memory and passing it to i440fx_init(), which
> then maps some aliases into the system address space and also gives it to
> pci_bus_new().  It's essentially the same thing with different details.

I think the attached patch (on top of ppc-next) should do it, but it
doesn't. Only the top area of the screen is shown, the rest is black.

>> I don't remember (but it's possible) if it has another region which maps
>> some other (high address) part of the address space down to 0 PCI.
>> Typically that would be a smaller region which specifically allow access
>> to the "ISA hole" that way.
>
> That would be done by mapping yet another alias.
>
> --
> error compiling committee.c: too many arguments to function
>
>

[-- Attachment #2: 0001-PPC-use-memory-API-to-construct-the-PCI-hole.patch --]
[-- Type: text/x-diff, Size: 4863 bytes --]

From c07f1116220cba7d2ee769b03de59b5a874b76db Mon Sep 17 00:00:00 2001
Message-Id: <c07f1116220cba7d2ee769b03de59b5a874b76db.1316295419.git.blauwirbel@gmail.com>
From: Blue Swirl <blauwirbel@gmail.com>
Date: Sat, 17 Sep 2011 20:30:50 +0000
Subject: [PATCH] PPC: use memory API to construct the PCI hole

Avoid vga.chain4 mapping by constructing a PCI hole for upper
2G of the PCI space.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 hw/grackle_pci.c  |   11 ++++++++++-
 hw/ppc_newworld.c |    2 --
 hw/ppc_oldworld.c |    2 --
 hw/unin_pci.c     |   18 ++++++++++++++++--
 4 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
index 9d3ff7d..94a608e 100644
--- a/hw/grackle_pci.c
+++ b/hw/grackle_pci.c
@@ -41,6 +41,8 @@
 typedef struct GrackleState {
     SysBusDevice busdev;
     PCIHostState host_state;
+    MemoryRegion pci_mmio;
+    MemoryRegion pci_hole;
 } GrackleState;
 
 /* Don't know if this matches real hardware, but it agrees with OHW.  */
@@ -73,11 +75,18 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic,
     qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     d = FROM_SYSBUS(GrackleState, s);
+
+    memory_region_init(&d->pci_mmio, "pci-mmio", 0x100000000ULL);
+    memory_region_init_alias(&d->pci_hole, "pci-hole", &d->pci_mmio,
+                             0x80000000ULL, 0x7e000000ULL);
+    memory_region_add_subregion(address_space_mem, 0x80000000ULL,
+                                &d->pci_hole);
+
     d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
                                          pci_grackle_set_irq,
                                          pci_grackle_map_irq,
                                          pic,
-                                         address_space_mem,
+                                         &d->pci_mmio,
                                          address_space_io,
                                          0, 4);
 
diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index 5fb9359..bcdc0a3 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -263,8 +263,6 @@ static void ppc_core99_init (ram_addr_t ram_size,
         }
     }
 
-    isa_mem_base = 0x80000000;
-
     /* Register 8 MB of ISA IO space */
     isa_mmio_init(0xf2000000, 0x00800000);
 
diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c
index 3857075..5c17944 100644
--- a/hw/ppc_oldworld.c
+++ b/hw/ppc_oldworld.c
@@ -208,8 +208,6 @@ static void ppc_heathrow_init (ram_addr_t ram_size,
         }
     }
 
-    isa_mem_base = 0x80000000;
-
     /* Register 2 MB of ISA IO space */
     isa_mmio_init(0xfe000000, 0x00200000);
 
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index 600cd1e..4299052 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -41,6 +41,8 @@ static const int unin_irq_line[] = { 0x1b, 0x1c, 0x1d, 0x1e };
 typedef struct UNINState {
     SysBusDevice busdev;
     PCIHostState host_state;
+    MemoryRegion pci_mmio;
+    MemoryRegion pci_hole;
 } UNINState;
 
 static int pci_unin_map_irq(PCIDevice *pci_dev, int irq_num)
@@ -215,10 +217,16 @@ PCIBus *pci_pmac_init(qemu_irq *pic,
     qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     d = FROM_SYSBUS(UNINState, s);
+    memory_region_init(&d->pci_mmio, "pci-mmio", 0x100000000ULL);
+    memory_region_init_alias(&d->pci_hole, "pci-hole", &d->pci_mmio,
+                             0x80000000ULL, 0x70000000ULL);
+    memory_region_add_subregion(address_space_mem, 0x80000000ULL,
+                                &d->pci_hole);
+
     d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
                                          pci_unin_set_irq, pci_unin_map_irq,
                                          pic,
-                                         address_space_mem,
+                                         &d->pci_mmio,
                                          address_space_io,
                                          PCI_DEVFN(11, 0), 4);
 
@@ -272,10 +280,16 @@ PCIBus *pci_pmac_u3_init(qemu_irq *pic,
     s = sysbus_from_qdev(dev);
     d = FROM_SYSBUS(UNINState, s);
 
+    memory_region_init(&d->pci_mmio, "pci-mmio", 0x100000000ULL);
+    memory_region_init_alias(&d->pci_hole, "pci-hole", &d->pci_mmio,
+                             0x80000000ULL, 0x70000000ULL);
+    memory_region_add_subregion(address_space_mem, 0x80000000ULL,
+                                &d->pci_hole);
+
     d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
                                          pci_unin_set_irq, pci_unin_map_irq,
                                          pic,
-                                         address_space_mem,
+                                         &d->pci_mmio,
                                          address_space_io,
                                          PCI_DEVFN(11, 0), 4);
 
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 09/18/2011 12:40 AM, Blue Swirl wrote:
> On Thu, Sep 15, 2011 at 11:31 AM, Avi Kivity<avi@redhat.com>  wrote:
> >  On 09/15/2011 01:01 PM, Benjamin Herrenschmidt wrote:
> >>
> >>  >    Sure :). So the problem is that when emulating the G3 Beige machine in
> >>  >    QEMU (default ppc32 target) we also add a PCI VGA adapter. Apparently,
> >>  >    on x86 that PCI VGA adapter can map the special VGA regions to
> >>  >    somewhere, namely 0xa0000. With the memory api overhaul, this also
> >>  >    slipped into the PPC world where mapping 0xa0000 with VGA adapters is
> >>  >    a pretty bad idea, as it's occupied by RAM.
> >>  >
> >>  >    Now the discussion was on which level that mapping would happen and
> >>  >    which devices go through which buses which then would filter certain
> >>  >    ranges from being mapped. Basically, which way does a memory request
> >>  >    from the CPU go on a G3 Beige machine until it arrives the VGA
> >>  >    adapter?
> >>  >
> >>  >    I hope that concludes the actual question. Avi, if I explained this
> >>  >    wrong, please correct me.
> >>
> >>  Ok so there's several things here.
> >>
> >>  First, the mapping from CPU addresses to PCI addresses. This depends on
> >>  the host bridge chip. The MPC106, used in the Beige G3, itself supports
> >>  different type of mappings.
> >>
> >>    From memory, the way it's configured in a G3 is to have a 1:1 mapping of
> >>  80000000 CPU to 80000000 PCI.
> >>
> >>  That means that with this basic mapping, you cannot generate memory
> >>  accesses to low PCI addresses such as 0xa0000.
> >
> >  Alex, what this means (I think is) that: pci_grackle_init() needs to create
> >  a container memory region and pass it to pc_register_bus() as the pci
> >  address space, and create and alias starting at 0x80000000 of the pci
> >  address space, and map that alias at address 0x80000000 of the system
> >  address space.
> >
> >  See pc_init1() creating pci_memory and passing it to i440fx_init(), which
> >  then maps some aliases into the system address space and also gives it to
> >  pci_bus_new().  It's essentially the same thing with different details.
>
> I think the attached patch (on top of ppc-next) should do it, but it
> doesn't. Only the top area of the screen is shown, the rest is black.

I can't see anything wrong with the patch either.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

On 17.09.2011, at 23:40, Blue Swirl wrote:

> On Thu, Sep 15, 2011 at 11:31 AM, Avi Kivity <avi@redhat.com> wrote:
>> On 09/15/2011 01:01 PM, Benjamin Herrenschmidt wrote:
>>> 
>>>>  Sure :). So the problem is that when emulating the G3 Beige machine in
>>>>  QEMU (default ppc32 target) we also add a PCI VGA adapter. Apparently,
>>>>  on x86 that PCI VGA adapter can map the special VGA regions to
>>>>  somewhere, namely 0xa0000. With the memory api overhaul, this also
>>>>  slipped into the PPC world where mapping 0xa0000 with VGA adapters is
>>>>  a pretty bad idea, as it's occupied by RAM.
>>>> 
>>>>  Now the discussion was on which level that mapping would happen and
>>>>  which devices go through which buses which then would filter certain
>>>>  ranges from being mapped. Basically, which way does a memory request
>>>>  from the CPU go on a G3 Beige machine until it arrives the VGA
>>>>  adapter?
>>>> 
>>>>  I hope that concludes the actual question. Avi, if I explained this
>>>>  wrong, please correct me.
>>> 
>>> Ok so there's several things here.
>>> 
>>> First, the mapping from CPU addresses to PCI addresses. This depends on
>>> the host bridge chip. The MPC106, used in the Beige G3, itself supports
>>> different type of mappings.
>>> 
>>>  From memory, the way it's configured in a G3 is to have a 1:1 mapping of
>>> 80000000 CPU to 80000000 PCI.
>>> 
>>> That means that with this basic mapping, you cannot generate memory
>>> accesses to low PCI addresses such as 0xa0000.
>> 
>> Alex, what this means (I think is) that: pci_grackle_init() needs to create
>> a container memory region and pass it to pc_register_bus() as the pci
>> address space, and create and alias starting at 0x80000000 of the pci
>> address space, and map that alias at address 0x80000000 of the system
>> address space.
>> 
>> See pc_init1() creating pci_memory and passing it to i440fx_init(), which
>> then maps some aliases into the system address space and also gives it to
>> pci_bus_new().  It's essentially the same thing with different details.
> 
> I think the attached patch (on top of ppc-next) should do it, but it
> doesn't. Only the top area of the screen is shown, the rest is black.

Without your patch:

(qemu) info mtree 
memory
00000000-fffffffe : system
  800a0000-800affff : vga.chain4
  80880000-808fffff : macio
    00060000-0007ffff : macio-nvram
    00020000-00020fff : pmac-ide
    00016000-00017fff : cuda
    00013000-0001303f : escc-bar
    00008000-00008fff : dbdma
    00000000-00000fff : heathrow-pic
  80800000-8080ffff : vga.rom
  80000000-807fffff : vga.vram
  800a0000-800bffff : vga-lowmem
  80013000-8001303f : escc
  fee00000-fee00fff : pci-data-idx
  fec00000-fec00fff : pci-conf-idx
  fe000000-fe1fffff : isa-mmio


With your patch:

(qemu) info mtree 
memory
00000000-fffffffe : system
  80013000-8001303f : escc
  fee00000-fee00fff : pci-data-idx
  fec00000-fec00fff : pci-conf-idx
  80000000-fdffffff : pci-hole
  fe000000-fe1fffff : isa-mmio



Since the VRAM is mapped to 0x80000000 which is now occupied by the hole and nothing behind it, nothing gets to write there? Not sure - I still haven't understood how the memory api works.


Alex

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 09/19/2011 12:15 PM, Alexander Graf wrote:
> On 17.09.2011, at 23:40, Blue Swirl wrote:
>
> >  On Thu, Sep 15, 2011 at 11:31 AM, Avi Kivity<avi@redhat.com>  wrote:
> >>  On 09/15/2011 01:01 PM, Benjamin Herrenschmidt wrote:
> >>>
> >>>>   Sure :). So the problem is that when emulating the G3 Beige machine in
> >>>>   QEMU (default ppc32 target) we also add a PCI VGA adapter. Apparently,
> >>>>   on x86 that PCI VGA adapter can map the special VGA regions to
> >>>>   somewhere, namely 0xa0000. With the memory api overhaul, this also
> >>>>   slipped into the PPC world where mapping 0xa0000 with VGA adapters is
> >>>>   a pretty bad idea, as it's occupied by RAM.
> >>>>
> >>>>   Now the discussion was on which level that mapping would happen and
> >>>>   which devices go through which buses which then would filter certain
> >>>>   ranges from being mapped. Basically, which way does a memory request
> >>>>   from the CPU go on a G3 Beige machine until it arrives the VGA
> >>>>   adapter?
> >>>>
> >>>>   I hope that concludes the actual question. Avi, if I explained this
> >>>>   wrong, please correct me.
> >>>
> >>>  Ok so there's several things here.
> >>>
> >>>  First, the mapping from CPU addresses to PCI addresses. This depends on
> >>>  the host bridge chip. The MPC106, used in the Beige G3, itself supports
> >>>  different type of mappings.
> >>>
> >>>    From memory, the way it's configured in a G3 is to have a 1:1 mapping of
> >>>  80000000 CPU to 80000000 PCI.
> >>>
> >>>  That means that with this basic mapping, you cannot generate memory
> >>>  accesses to low PCI addresses such as 0xa0000.
> >>
> >>  Alex, what this means (I think is) that: pci_grackle_init() needs to create
> >>  a container memory region and pass it to pc_register_bus() as the pci
> >>  address space, and create and alias starting at 0x80000000 of the pci
> >>  address space, and map that alias at address 0x80000000 of the system
> >>  address space.
> >>
> >>  See pc_init1() creating pci_memory and passing it to i440fx_init(), which
> >>  then maps some aliases into the system address space and also gives it to
> >>  pci_bus_new().  It's essentially the same thing with different details.
> >
> >  I think the attached patch (on top of ppc-next) should do it, but it
> >  doesn't. Only the top area of the screen is shown, the rest is black.
>
> Without your patch:
>
> (qemu) info mtree
> memory
> 00000000-fffffffe : system
>    800a0000-800affff : vga.chain4
This is here due to the isa_mem_base problem - I think we can get that 
part of the patch merged?

>    80880000-808fffff : macio
>      00060000-0007ffff : macio-nvram
>      00020000-00020fff : pmac-ide
>      00016000-00017fff : cuda
>      00013000-0001303f : escc-bar
>      00008000-00008fff : dbdma
>      00000000-00000fff : heathrow-pic
>    80800000-8080ffff : vga.rom
>    80000000-807fffff : vga.vram
>    800a0000-800bffff : vga-lowmem
>    80013000-8001303f : escc
>    fee00000-fee00fff : pci-data-idx
>    fec00000-fec00fff : pci-conf-idx
>    fe000000-fe1fffff : isa-mmio
>
>
> With your patch:
>
> (qemu) info mtree
> memory
> 00000000-fffffffe : system
>    80013000-8001303f : escc
>    fee00000-fee00fff : pci-data-idx
>    fec00000-fec00fff : pci-conf-idx
>    80000000-fdffffff : pci-hole
>    fe000000-fe1fffff : isa-mmio
>
>
>
> Since the VRAM is mapped to 0x80000000 which is now occupied by the hole and nothing behind it, nothing gets to write there?

The memory printer doesn't follow aliases, so this is incomplete.

> Not sure - I still haven't understood how the memory api works.
>

Thanks for reminding me to write up slides for tomorrow's code overview 
session.  I'll start with a general explanation.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

On 19.09.2011, at 11:22, Avi Kivity wrote:

> On 09/19/2011 12:15 PM, Alexander Graf wrote:
>> On 17.09.2011, at 23:40, Blue Swirl wrote:
>> 
>> >  On Thu, Sep 15, 2011 at 11:31 AM, Avi Kivity<avi@redhat.com>  wrote:
>> >>  On 09/15/2011 01:01 PM, Benjamin Herrenschmidt wrote:
>> >>>
>> >>>>   Sure :). So the problem is that when emulating the G3 Beige machine in
>> >>>>   QEMU (default ppc32 target) we also add a PCI VGA adapter. Apparently,
>> >>>>   on x86 that PCI VGA adapter can map the special VGA regions to
>> >>>>   somewhere, namely 0xa0000. With the memory api overhaul, this also
>> >>>>   slipped into the PPC world where mapping 0xa0000 with VGA adapters is
>> >>>>   a pretty bad idea, as it's occupied by RAM.
>> >>>>
>> >>>>   Now the discussion was on which level that mapping would happen and
>> >>>>   which devices go through which buses which then would filter certain
>> >>>>   ranges from being mapped. Basically, which way does a memory request
>> >>>>   from the CPU go on a G3 Beige machine until it arrives the VGA
>> >>>>   adapter?
>> >>>>
>> >>>>   I hope that concludes the actual question. Avi, if I explained this
>> >>>>   wrong, please correct me.
>> >>>
>> >>>  Ok so there's several things here.
>> >>>
>> >>>  First, the mapping from CPU addresses to PCI addresses. This depends on
>> >>>  the host bridge chip. The MPC106, used in the Beige G3, itself supports
>> >>>  different type of mappings.
>> >>>
>> >>>    From memory, the way it's configured in a G3 is to have a 1:1 mapping of
>> >>>  80000000 CPU to 80000000 PCI.
>> >>>
>> >>>  That means that with this basic mapping, you cannot generate memory
>> >>>  accesses to low PCI addresses such as 0xa0000.
>> >>
>> >>  Alex, what this means (I think is) that: pci_grackle_init() needs to create
>> >>  a container memory region and pass it to pc_register_bus() as the pci
>> >>  address space, and create and alias starting at 0x80000000 of the pci
>> >>  address space, and map that alias at address 0x80000000 of the system
>> >>  address space.
>> >>
>> >>  See pc_init1() creating pci_memory and passing it to i440fx_init(), which
>> >>  then maps some aliases into the system address space and also gives it to
>> >>  pci_bus_new().  It's essentially the same thing with different details.
>> >
>> >  I think the attached patch (on top of ppc-next) should do it, but it
>> >  doesn't. Only the top area of the screen is shown, the rest is black.
>> 
>> Without your patch:
>> 
>> (qemu) info mtree
>> memory
>> 00000000-fffffffe : system
>>   800a0000-800affff : vga.chain4
> This is here due to the isa_mem_base problem - I think we can get that part of the patch merged?
> 
>>   80880000-808fffff : macio
>>     00060000-0007ffff : macio-nvram
>>     00020000-00020fff : pmac-ide
>>     00016000-00017fff : cuda
>>     00013000-0001303f : escc-bar
>>     00008000-00008fff : dbdma
>>     00000000-00000fff : heathrow-pic
>>   80800000-8080ffff : vga.rom
>>   80000000-807fffff : vga.vram
>>   800a0000-800bffff : vga-lowmem

Well what I don't understand is how this corresponds to the lowmem region here. If I simply #if 0 the chain4 code out, everything works as expected.

>>   80013000-8001303f : escc
>>   fee00000-fee00fff : pci-data-idx
>>   fec00000-fec00fff : pci-conf-idx
>>   fe000000-fe1fffff : isa-mmio
>> 
>> 
>> With your patch:
>> 
>> (qemu) info mtree
>> memory
>> 00000000-fffffffe : system
>>   80013000-8001303f : escc
>>   fee00000-fee00fff : pci-data-idx
>>   fec00000-fec00fff : pci-conf-idx
>>   80000000-fdffffff : pci-hole
>>   fe000000-fe1fffff : isa-mmio
>> 
>> 
>> 
>> Since the VRAM is mapped to 0x80000000 which is now occupied by the hole and nothing behind it, nothing gets to write there?
> 
> The memory printer doesn't follow aliases, so this is incomplete.

Ugh :(.

> 
>> Not sure - I still haven't understood how the memory api works.
>> 
> 
> Thanks for reminding me to write up slides for tomorrow's code overview session.  I'll start with a general explanation.

What code overview session?


Alex

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 09/19/2011 12:36 PM, Alexander Graf wrote:
> >
> >>    80880000-808fffff : macio
> >>      00060000-0007ffff : macio-nvram
> >>      00020000-00020fff : pmac-ide
> >>      00016000-00017fff : cuda
> >>      00013000-0001303f : escc-bar
> >>      00008000-00008fff : dbdma
> >>      00000000-00000fff : heathrow-pic
> >>    80800000-8080ffff : vga.rom
> >>    80000000-807fffff : vga.vram
> >>    800a0000-800bffff : vga-lowmem
>
> Well what I don't understand is how this corresponds to the lowmem region here. If I simply #if 0 the chain4 code out, everything works as expected.

It's at the wrong address.  It should be at 000a0000, but not be visible 
since it's outside the pci hole.  IIUC.

> >
> >>  Not sure - I still haven't understood how the memory api works.
> >>
> >
> >  Thanks for reminding me to write up slides for tomorrow's code overview session.  I'll start with a general explanation.
>
> What code overview session?
>

At the usual kvm weekly time & place, I'll do a walkthrough of the 
memory API theory and practice.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 09/14/2011 11:06 PM, Blue Swirl wrote:
> On Wed, Sep 14, 2011 at 8:35 AM, Avi Kivity<avi@redhat.com>  wrote:
> >  On 09/14/2011 11:27 AM, Alexander Graf wrote:
> >>
> >>  On 14.09.2011, at 10:24, Jan Kiszka wrote:
> >>
> >>  >    On 2011-09-14 10:22, Avi Kivity wrote:
> >>  >>    On 09/14/2011 11:20 AM, Jan Kiszka wrote:
> >>  >>>>
> >>  >>>>    Anyway PCI supports the vga region at 0xa0000-0xc0000.  Where is it
> >>  >>>>    supposed to be mapped?
> >>  >>>
> >>  >>>    ...but not all PCI bridges make use of this feature / forward legacy
> >>  >>>    requests.
> >>  >>>
> >>  >>
> >>  >>    Then this should be fixed in the bridge?
> >>  >
> >>  >    Yes, it's a PPC bug.
> >>
> >>  So how does the bridge not forward it then?
> >>
> >
> >  I expect that currently vga adds the region to pci_address_space().  We need
> >  to create a pci_address_space_vga() function that returns a region for vga
> >  to use.  Then add or remove the region to pci_address_space(), within the
> >  bridge code, depending on whether the bridge forwards vga accesses or not.
>
> Similar treatment should be also needed for VGA IO ports 0x3b0 etc.
>
> >  (assuming I understood the problem correctly - not sure)
>
> I think you did.

Maybe, but the solution can't be right.  The bridge can't distinguish 
between a BAR mapped at 0xa0000 and the vga device claiming accesses to 
0xa0000.  Is this what is happening?

The current pci bridge implementation (440fx) uses an alias to 
instantiate pci 0xa0000-0xc0000 at the same address in the host address 
space.  If you disable it, those addresses map back to RAM - but there 
is no distinction between a BAR at that address and a VGA card at that 
address.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Blue Swirl]

On Wed, Sep 14, 2011 at 8:15 PM, Avi Kivity <avi@redhat.com> wrote:
> On 09/14/2011 11:06 PM, Blue Swirl wrote:
>>
>> On Wed, Sep 14, 2011 at 8:35 AM, Avi Kivity<avi@redhat.com>  wrote:
>> >  On 09/14/2011 11:27 AM, Alexander Graf wrote:
>> >>
>> >>  On 14.09.2011, at 10:24, Jan Kiszka wrote:
>> >>
>> >>  >    On 2011-09-14 10:22, Avi Kivity wrote:
>> >>  >>    On 09/14/2011 11:20 AM, Jan Kiszka wrote:
>> >>  >>>>
>> >>  >>>>    Anyway PCI supports the vga region at 0xa0000-0xc0000.  Where
>> >> is it
>> >>  >>>>    supposed to be mapped?
>> >>  >>>
>> >>  >>>    ...but not all PCI bridges make use of this feature / forward
>> >> legacy
>> >>  >>>    requests.
>> >>  >>>
>> >>  >>
>> >>  >>    Then this should be fixed in the bridge?
>> >>  >
>> >>  >    Yes, it's a PPC bug.
>> >>
>> >>  So how does the bridge not forward it then?
>> >>
>> >
>> >  I expect that currently vga adds the region to pci_address_space().  We
>> > need
>> >  to create a pci_address_space_vga() function that returns a region for
>> > vga
>> >  to use.  Then add or remove the region to pci_address_space(), within
>> > the
>> >  bridge code, depending on whether the bridge forwards vga accesses or
>> > not.
>>
>> Similar treatment should be also needed for VGA IO ports 0x3b0 etc.
>>
>> >  (assuming I understood the problem correctly - not sure)
>>
>> I think you did.
>
> Maybe, but the solution can't be right.  The bridge can't distinguish
> between a BAR mapped at 0xa0000 and the vga device claiming accesses to
> 0xa0000.  Is this what is happening?

If VGA enable is set, the bridge will forward the accesses to VGA
memory or ports to secondary interface. It doesn't care which device
uses them. This is described in VGA support chapter in the PCI bridge
spec.

> The current pci bridge implementation (440fx) uses an alias to instantiate
> pci 0xa0000-0xc0000 at the same address in the host address space.  If you
> disable it, those addresses map back to RAM - but there is no distinction
> between a BAR at that address and a VGA card at that address.
>
> --
> I have a truly marvellous patch that fixes the bug which this
> signature is too narrow to contain.
>
>

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 09/14/2011 11:25 PM, Blue Swirl wrote:
> On Wed, Sep 14, 2011 at 8:15 PM, Avi Kivity<avi@redhat.com>  wrote:
> >  On 09/14/2011 11:06 PM, Blue Swirl wrote:
> >>
> >>  On Wed, Sep 14, 2011 at 8:35 AM, Avi Kivity<avi@redhat.com>    wrote:
> >>  >    On 09/14/2011 11:27 AM, Alexander Graf wrote:
> >>  >>
> >>  >>    On 14.09.2011, at 10:24, Jan Kiszka wrote:
> >>  >>
> >>  >>    >      On 2011-09-14 10:22, Avi Kivity wrote:
> >>  >>    >>      On 09/14/2011 11:20 AM, Jan Kiszka wrote:
> >>  >>    >>>>
> >>  >>    >>>>      Anyway PCI supports the vga region at 0xa0000-0xc0000.  Where
> >>  >>  is it
> >>  >>    >>>>      supposed to be mapped?
> >>  >>    >>>
> >>  >>    >>>      ...but not all PCI bridges make use of this feature / forward
> >>  >>  legacy
> >>  >>    >>>      requests.
> >>  >>    >>>
> >>  >>    >>
> >>  >>    >>      Then this should be fixed in the bridge?
> >>  >>    >
> >>  >>    >      Yes, it's a PPC bug.
> >>  >>
> >>  >>    So how does the bridge not forward it then?
> >>  >>
> >>  >
> >>  >    I expect that currently vga adds the region to pci_address_space().  We
> >>  >  need
> >>  >    to create a pci_address_space_vga() function that returns a region for
> >>  >  vga
> >>  >    to use.  Then add or remove the region to pci_address_space(), within
> >>  >  the
> >>  >    bridge code, depending on whether the bridge forwards vga accesses or
> >>  >  not.
> >>
> >>  Similar treatment should be also needed for VGA IO ports 0x3b0 etc.
> >>
> >>  >    (assuming I understood the problem correctly - not sure)
> >>
> >>  I think you did.
> >
> >  Maybe, but the solution can't be right.  The bridge can't distinguish
> >  between a BAR mapped at 0xa0000 and the vga device claiming accesses to
> >  0xa0000.  Is this what is happening?
>
> If VGA enable is set, the bridge will forward the accesses to VGA
> memory or ports to secondary interface. It doesn't care which device
> uses them. This is described in VGA support chapter in the PCI bridge
> spec.

That doesn't match the original description, where it appeared that the 
vga window collided with a BAR.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Andreas Färber]

Am 13.09.2011 um 11:00 schrieb Jan Kiszka:

> On 2011-09-13 10:40, Alexander Graf wrote:
>> Btw, it still tries to execute invalid code even with your patch.  
>> #if 0'ing out the memory region updates at least get the guest  
>> booting for me. Btw, to get it working you also need a patch for  
>> the interrupt controller (another breakage thanks to memory api).
>>
>> diff --git a/hw/heathrow_pic.c b/hw/heathrow_pic.c
>> index 51996ab..16f48d1 100644
>> --- a/hw/heathrow_pic.c
>> +++ b/hw/heathrow_pic.c
>> @@ -126,7 +126,7 @@ static uint64_t pic_read(void *opaque,  
>> target_phys_addr_t addr,
>> static const MemoryRegionOps heathrow_pic_ops = {
>>     .read = pic_read,
>>     .write = pic_write,
>> -    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>> };
>>
>> static void heathrow_pic_set_irq(void *opaque, int num, int level)
>>
>
> With out without this fix, with or without active chain-4  
> optimization,
> I just get an empty yellow screen when firing up qemu-system-ppc (also
> when using the Debian ISO). Do I need to specify a specific machine  
> type?

No. Did you try with Alex' via-cuda patch? That's the only one I have  
on my branch for Linux host.

Andreas

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Blue Swirl]

On Mon, Sep 12, 2011 at 3:20 PM, Alexander Graf <agraf@suse.de> wrote:
> Jan Kiszka wrote:
>> Most VGA memory access modes require MMIO handling as they demand weird
>> logic to get a byte from or into the video RAM. However, there is one
>> exception: chain 4 mode with all memory planes enabled for writing. This
>> mode actually allows lineary mapping, which can then be combined with
>> dirty logging to accelerate KVM.
>>
>> This patch accelerates specifically VBE accesses like they are used by
>> grub in graphical mode. Not only the standard VGA adapter benefits from
>> this, also vmware and spice in VGA mode.
>>
>> CC: Gerd Hoffmann <kraxel@redhat.com>
>> CC: Avi Kivity <avi@redhat.com>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>
>
> [...]
>
>> +static void vga_update_memory_access(VGACommonState *s)
>> +{
>> +    MemoryRegion *region, *old_region = s->chain4_alias;
>> +    target_phys_addr_t base, offset, size;
>> +
>> +    s->chain4_alias = NULL;
>> +
>> +    if ((s->sr[0x02] & 0xf) == 0xf && s->sr[0x04] & 0x08) {
>> +        offset = 0;
>> +        switch ((s->gr[6] >> 2) & 3) {
>> +        case 0:
>> +            base = 0xa0000;
>> +            size = 0x20000;
>> +            break;
>> +        case 1:
>> +            base = 0xa0000;
>> +            size = 0x10000;
>> +            offset = s->bank_offset;
>> +            break;
>> +        case 2:
>> +            base = 0xb0000;
>> +            size = 0x8000;
>> +            break;
>> +        case 3:
>> +            base = 0xb8000;
>> +            size = 0x8000;
>> +            break;
>> +        }
>> +        region = g_malloc(sizeof(*region));
>> +        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>> +        memory_region_add_subregion_overlap(s->legacy_address_space, base,
>> +                                            region, 2);
>>
>
> This one eventually gives me the following in info mtree with -M g3beige
> on qemu-system-ppc:
>
> (qemu) info mtree

:-)

> memory
> system addr 00000000 off 00000000 size 7fffffffffffffff
> -vga.chain4 addr 000a0000 off 00000000 size 10000
> -macio addr 80880000 off 00000000 size 80000
> --macio-nvram addr 00060000 off 00000000 size 20000
> --pmac-ide addr 00020000 off 00000000 size 1000
> --cuda addr 00016000 off 00000000 size 2000
> --escc-bar addr 00013000 off 00000000 size 40
> --dbdma addr 00008000 off 00000000 size 1000
> --heathrow-pic addr 00000000 off 00000000 size 1000
> -vga.rom addr 80800000 off 00000000 size 10000
> -vga.vram addr 80000000 off 00000000 size 800000
> -vga-lowmem addr 800a0000 off 00000000 size 20000
> -escc addr 80013000 off 00000000 size 40
> -isa-mmio addr fe000000 off 00000000 size 200000
> I/O
> io addr 00000000 off 00000000 size 10000
> -cmd646-bmdma addr 00000700 off 00000000 size 10
> --cmd646-bmdma-ioport addr 0000000c off 00000000 size 4
> --cmd646-bmdma-bus addr 00000008 off 00000000 size 4
> --cmd646-bmdma-ioport addr 00000004 off 00000000 size 4
> --cmd646-bmdma-bus addr 00000000 off 00000000 size 4
> -cmd646-cmd addr 00000680 off 00000000 size 4
> -cmd646-data addr 00000600 off 00000000 size 8
> -cmd646-cmd addr 00000580 off 00000000 size 4
> -cmd646-data addr 00000500 off 00000000 size 8
> -ne2000 addr 00000400 off 00000000 size 100
>
> This ends up overmapping 0xa0000, effectively overwriting kernel data.
> If I #if 0 the offending chunk out, everything is fine. I would assume
> that chain4 really needs to be inside of lowmem? No idea about VGA, but
> I'm sure you know what's going on :).

I had similar problems with sun4u, fixed with
f69539b14bdba7a5cd22e1f4bed439b476b17286. I think also here, PCI
should be given a memory range at 0x80000000 and VGA should
automatically use that like.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

On 12.09.2011, at 22:21, Blue Swirl wrote:

> On Mon, Sep 12, 2011 at 3:20 PM, Alexander Graf <agraf@suse.de> wrote:
>> Jan Kiszka wrote:
>>> Most VGA memory access modes require MMIO handling as they demand weird
>>> logic to get a byte from or into the video RAM. However, there is one
>>> exception: chain 4 mode with all memory planes enabled for writing. This
>>> mode actually allows lineary mapping, which can then be combined with
>>> dirty logging to accelerate KVM.
>>> 
>>> This patch accelerates specifically VBE accesses like they are used by
>>> grub in graphical mode. Not only the standard VGA adapter benefits from
>>> this, also vmware and spice in VGA mode.
>>> 
>>> CC: Gerd Hoffmann <kraxel@redhat.com>
>>> CC: Avi Kivity <avi@redhat.com>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> 
>> 
>> [...]
>> 
>>> +static void vga_update_memory_access(VGACommonState *s)
>>> +{
>>> +    MemoryRegion *region, *old_region = s->chain4_alias;
>>> +    target_phys_addr_t base, offset, size;
>>> +
>>> +    s->chain4_alias = NULL;
>>> +
>>> +    if ((s->sr[0x02] & 0xf) == 0xf && s->sr[0x04] & 0x08) {
>>> +        offset = 0;
>>> +        switch ((s->gr[6] >> 2) & 3) {
>>> +        case 0:
>>> +            base = 0xa0000;
>>> +            size = 0x20000;
>>> +            break;
>>> +        case 1:
>>> +            base = 0xa0000;
>>> +            size = 0x10000;
>>> +            offset = s->bank_offset;
>>> +            break;
>>> +        case 2:
>>> +            base = 0xb0000;
>>> +            size = 0x8000;
>>> +            break;
>>> +        case 3:
>>> +            base = 0xb8000;
>>> +            size = 0x8000;
>>> +            break;
>>> +        }
>>> +        region = g_malloc(sizeof(*region));
>>> +        memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
>>> +        memory_region_add_subregion_overlap(s->legacy_address_space, base,
>>> +                                            region, 2);
>>> 
>> 
>> This one eventually gives me the following in info mtree with -M g3beige
>> on qemu-system-ppc:
>> 
>> (qemu) info mtree
> 
> :-)
> 
>> memory
>> system addr 00000000 off 00000000 size 7fffffffffffffff
>> -vga.chain4 addr 000a0000 off 00000000 size 10000
>> -macio addr 80880000 off 00000000 size 80000
>> --macio-nvram addr 00060000 off 00000000 size 20000
>> --pmac-ide addr 00020000 off 00000000 size 1000
>> --cuda addr 00016000 off 00000000 size 2000
>> --escc-bar addr 00013000 off 00000000 size 40
>> --dbdma addr 00008000 off 00000000 size 1000
>> --heathrow-pic addr 00000000 off 00000000 size 1000
>> -vga.rom addr 80800000 off 00000000 size 10000
>> -vga.vram addr 80000000 off 00000000 size 800000
>> -vga-lowmem addr 800a0000 off 00000000 size 20000
>> -escc addr 80013000 off 00000000 size 40
>> -isa-mmio addr fe000000 off 00000000 size 200000
>> I/O
>> io addr 00000000 off 00000000 size 10000
>> -cmd646-bmdma addr 00000700 off 00000000 size 10
>> --cmd646-bmdma-ioport addr 0000000c off 00000000 size 4
>> --cmd646-bmdma-bus addr 00000008 off 00000000 size 4
>> --cmd646-bmdma-ioport addr 00000004 off 00000000 size 4
>> --cmd646-bmdma-bus addr 00000000 off 00000000 size 4
>> -cmd646-cmd addr 00000680 off 00000000 size 4
>> -cmd646-data addr 00000600 off 00000000 size 8
>> -cmd646-cmd addr 00000580 off 00000000 size 4
>> -cmd646-data addr 00000500 off 00000000 size 8
>> -ne2000 addr 00000400 off 00000000 size 100
>> 
>> This ends up overmapping 0xa0000, effectively overwriting kernel data.
>> If I #if 0 the offending chunk out, everything is fine. I would assume
>> that chain4 really needs to be inside of lowmem? No idea about VGA, but
>> I'm sure you know what's going on :).
> 
> I had similar problems with sun4u, fixed with
> f69539b14bdba7a5cd22e1f4bed439b476b17286. I think also here, PCI
> should be given a memory range at 0x80000000 and VGA should
> automatically use that like.

Yeah, usually the ISA bus is behind an ISA-PCI bridge, so it should inherit the offset from its parent. Or do you mean something different?


Alex

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 09/13/2011 09:54 AM, Alexander Graf wrote:
> >
> >  I had similar problems with sun4u, fixed with
> >  f69539b14bdba7a5cd22e1f4bed439b476b17286. I think also here, PCI
> >  should be given a memory range at 0x80000000 and VGA should
> >  automatically use that like.
>
> Yeah, usually the ISA bus is behind an ISA-PCI bridge, so it should inherit the offset from its parent. Or do you mean something different?
>

He means that isa_mem_base should go away; instead isa_address_space() 
should be a subregion at offset 0x80000000.

Which vga variant are you using?

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Alexander Graf]

On 13.09.2011, at 09:51, Avi Kivity wrote:

> On 09/13/2011 09:54 AM, Alexander Graf wrote:
>> >
>> >  I had similar problems with sun4u, fixed with
>> >  f69539b14bdba7a5cd22e1f4bed439b476b17286. I think also here, PCI
>> >  should be given a memory range at 0x80000000 and VGA should
>> >  automatically use that like.
>> 
>> Yeah, usually the ISA bus is behind an ISA-PCI bridge, so it should inherit the offset from its parent. Or do you mean something different?
>> 
> 
> He means that isa_mem_base should go away; instead isa_address_space() should be a subregion at offset 0x80000000.

So we are talking about the same thing. Logically speaking, ISA devices are behind the ISA-PCI bridge, so the parent would be the bridge, right?

> Which vga variant are you using?

This is stdvga.


Alex

Re: [Qemu-devel] [PATCH v3 5/6] vga: Use linear mapping + dirty logging in chain 4 memory access mode

[Avi Kivity]

On 09/13/2011 10:54 AM, Alexander Graf wrote:
> >>
> >>  Yeah, usually the ISA bus is behind an ISA-PCI bridge, so it should inherit the offset from its parent. Or do you mean something different?
> >>
> >
> >  He means that isa_mem_base should go away; instead isa_address_space() should be a subregion at offset 0x80000000.
>
> So we are talking about the same thing. Logically speaking, ISA devices are behind the ISA-PCI bridge, so the parent would be the bridge, right?


Right.  system_memory -> pci_address_space() -> isa_address_space() -> 
various vga areas.

>
> >  Which vga variant are you using?
>
> This is stdvga.
>
>

Don't see the call to vga_init() in that path (this is what passes 
isa_address_space() to the vga core).

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

[Qemu-devel] [PATCH v3 6/6] vga: Drop some unused fields

[Jan Kiszka]

Memory region refactorings obsoleted them.

CC: Avi Kivity <avi@redhat.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/vga_int.h |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/hw/vga_int.h b/hw/vga_int.h
index 28b9287..99287dd 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -110,7 +110,6 @@ typedef struct VGACommonState {
     MemoryRegion vram;
     uint32_t vram_size;
     uint32_t latch;
-    uint32_t lfb_vram_mapped; /* whether 0xa0000 is mapped as ram */
     MemoryRegion *chain4_alias;
     uint8_t sr_index;
     uint8_t sr[256];
@@ -133,7 +132,6 @@ typedef struct VGACommonState {
     int dac_8bit;
     uint8_t palette[768];
     int32_t bank_offset;
-    MemoryRegion *vga_io_memory;
     int (*get_bpp)(struct VGACommonState *s);
     void (*get_offsets)(struct VGACommonState *s,
                         uint32_t *pline_offset,
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH v3 0/6] vga: dirty log cleanup, more linear mapping

[Anthony Liguori]

On 08/22/2011 12:12 PM, Jan Kiszka wrote:
> Rebased over latest qemu.git, addressing Avi's feedback and also fixing
> a regression of the vmware reset patch.
>
> Patch 1 is still a candidate for stable.

Applied all.  Thanks.

>
> PS: Just accidentally ran checkpatch against recent top of qemu.git as
> well - looks like not many maintainers use that tool themselves...
> (not really helpful when asking contributors to do this)

I'll bite, care to be more specific?

Regards,

Anthony Liguori

> Cc: Andrzej Zaborowski<balrogg@gmail.com>
> CC: Andrzej Zaborowski<balrogg@gmail.com>
> CC: Avi Kivity<avi@redhat.com>
> CC: Gerd Hoffmann<kraxel@redhat.com>
>
> Jan Kiszka (6):
>    vmware-vga: Register reset service
>    vmware-vga: Disable verbose mode
>    vmware-vga: Remove dead DIRECT_VRAM mode
>    vmware-vga: Eliminate vga_dirty_log_restart
>    vga: Use linear mapping + dirty logging in chain 4 memory access mode
>    vga: Drop some unused fields
>
>   hw/vga.c        |   57 +++++++++++--
>   hw/vga_int.h    |    5 +-
>   hw/vmware_vga.c |  242 +++++++++++++++----------------------------------------
>   3 files changed, 118 insertions(+), 186 deletions(-)
>

Re: [Qemu-devel] [PATCH v3 0/6] vga: dirty log cleanup, more linear mapping

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 512 bytes --]

On 2011-08-22 22:18, Anthony Liguori wrote:
>> PS: Just accidentally ran checkpatch against recent top of qemu.git as
>> well - looks like not many maintainers use that tool themselves...
>> (not really helpful when asking contributors to do this)
> 
> I'll bite, care to be more specific?

Try e.g.

git format-patch --stdout 0bf1dbdcc9..6e23063c46 | \
scripts/checkpatch.pl -

Looking closer, I guess one reason is that changes were done by scripts,
partly just "preserving" old issues.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]