[Qemu-devel] [PATCH] Correct computation of bytes per pixel from bits per pixel
Signed-off-by: BALATON Zoltan
---
console.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/console.c b/console.c
index 4525cc7..f698b77 100644
--- a/console.c
+++ b/console.c
@@ -1612,7 +1612,7 @@ PixelFormat qemu_different_endianness_pixelformat(int bpp)
memset(&pf, 0x00, sizeof(PixelFormat));
pf.bits_per_pixel = bpp;
-pf.bytes_per_pixel = bpp / 8;
+pf.bytes_per_pixel = (bpp + 7) >> 3;
pf.depth = bpp == 32 ? 24 : bpp;
switch (bpp) {
@@ -1661,7 +1661,7 @@ PixelFormat qemu_default_pixelformat(int bpp)
memset(&pf, 0x00, sizeof(PixelFormat));
pf.bits_per_pixel = bpp;
-pf.bytes_per_pixel = bpp / 8;
+pf.bytes_per_pixel = (bpp + 7) >> 3;
pf.depth = bpp == 32 ? 24 : bpp;
switch (bpp) {
--
1.7.10
[Qemu-devel] [PATCH] vmware_vga: Cleanup and allow simple drivers to work without the fifo
Detailed changes: Removing info available elsewhere from vmsvga_state.
Fix mixup between depth and bits per pixel. Return a value for FB_SIZE
even before enabled (according to the documentation, drivers should
read this value before enabling the device). Postpone stopping the
dirty log to the point where the command fifo is configured to allow
drivers which don't use the fifo to work too. (Without this the
picture rendered into the vram never got to the screen but the
DIRECT_VRAM option meant to support this case was removed a year ago.)
Signed-off-by: BALATON Zoltan
---
console.h | 20 +
hw/vga.c|2 +-
hw/vga_int.h|1 +
hw/vmware_vga.c | 243 ++-
4 files changed, 137 insertions(+), 129 deletions(-)
diff --git a/console.h b/console.h
index 4334db5..00baf5b 100644
--- a/console.h
+++ b/console.h
@@ -328,6 +328,26 @@ static inline int ds_get_bytes_per_pixel(DisplayState *ds)
return ds->surface->pf.bytes_per_pixel;
}
+static inline int ds_get_depth(DisplayState *ds)
+{
+return ds->surface->pf.depth;
+}
+
+static inline int ds_get_rmask(DisplayState *ds)
+{
+return ds->surface->pf.rmask;
+}
+
+static inline int ds_get_gmask(DisplayState *ds)
+{
+return ds->surface->pf.gmask;
+}
+
+static inline int ds_get_bmask(DisplayState *ds)
+{
+return ds->surface->pf.bmask;
+}
+
#ifdef CONFIG_CURSES
#include
typedef chtype console_ch_t;
diff --git a/hw/vga.c b/hw/vga.c
index f82ced8..7e6dc41 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1611,7 +1611,7 @@ void vga_invalidate_scanlines(VGACommonState *s, int y1,
int y2)
}
}
-static void vga_sync_dirty_bitmap(VGACommonState *s)
+void vga_sync_dirty_bitmap(VGACommonState *s)
{
memory_region_sync_dirty_bitmap(&s->vram);
}
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 8938093..16d53ef 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -195,6 +195,7 @@ MemoryRegion *vga_init_io(VGACommonState *s,
const MemoryRegionPortio **vbe_ports);
void vga_common_reset(VGACommonState *s);
+void vga_sync_dirty_bitmap(VGACommonState *s);
void vga_dirty_log_start(VGACommonState *s);
void vga_dirty_log_stop(VGACommonState *s);
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index f5e4f44..2b77766 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -32,16 +32,19 @@
#define HW_FILL_ACCEL
#define HW_MOUSE_ACCEL
-# include "vga_int.h"
+#include "vga_int.h"
+
+/* See http://vmware-svga.sf.net/ for some documentation on VMWare SVGA */
struct vmsvga_state_s {
VGACommonState vga;
-int width;
-int height;
+/* -*- The members marked are now unused and could be removed but they are
+ * contained in the VMState thus need special handling. Maybe they could
+ * be removed the next time a new machine type is added.
+ */
int invalidated;
-int depth;
-int bypp;
+int depth; /* -*- */
int enable;
int config;
struct {
@@ -58,11 +61,8 @@ struct vmsvga_state_s {
int new_height;
uint32_t guest;
uint32_t svgaid;
-uint32_t wred;
-uint32_t wgreen;
-uint32_t wblue;
int syncing;
-int fb_size;
+int fb_size; /* -*- */
MemoryRegion fifo_ram;
uint8_t *fifo_ptr;
@@ -298,40 +298,33 @@ static inline void vmsvga_update_rect(struct
vmsvga_state_s *s,
uint8_t *src;
uint8_t *dst;
-if (x + w > s->width) {
+if (x + w > ds_get_width(s->vga.ds)) {
fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
-__FUNCTION__, x, w);
-x = MIN(x, s->width);
-w = s->width - x;
+__func__, x, w);
+x = MIN(x, ds_get_width(s->vga.ds));
+w = ds_get_width(s->vga.ds) - x;
}
-if (y + h > s->height) {
+if (y + h > ds_get_height(s->vga.ds)) {
fprintf(stderr, "%s: update height too large y: %d, h: %d\n",
-__FUNCTION__, y, h);
-y = MIN(y, s->height);
-h = s->height - y;
+__func__, y, h);
+y = MIN(y, ds_get_height(s->vga.ds));
+h = ds_get_height(s->vga.ds) - y;
}
-line = h;
-bypl = s->bypp * s->width;
-width = s->bypp * w;
-start = s->bypp * x + bypl * y;
+bypl = ds_get_linesize(s->vga.ds);
+width = ds_get_bytes_per_pixel(s->vga.ds) * w;
+start = ds_get_bytes_per_pixel(s->vga.ds) * x + bypl * y;
src = s->vga.vram_ptr + start;
dst = ds_get_data(s->vga.ds) + start;
-for (; line > 0; line --, src += bypl, dst += bypl)
+for (line = h; line > 0; line--, src += bypl, dst += bypl) {
memcpy(dst, src, width);
+}
dpy_update(s->vga.ds, x, y, w, h);
}
-static inline void vmsvga_update_screen(struct vmsvga_state_s *s)
-{
-memcpy(ds_get_data(s->vga.ds),
[Qemu-devel] [PATCH] Fix copy&paste typos in documentation comments
Signed-off-by: BALATON Zoltan --- memory.h | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/memory.h b/memory.h index bd1bbae..aba5721 100644 --- a/memory.h +++ b/memory.h @@ -252,9 +252,9 @@ void memory_region_init_ram(MemoryRegion *mr, uint64_t size); /** - * memory_region_init_ram: Initialize RAM memory region from a user-provided. - * pointer. Accesses into the region will modify - * memory directly. + * memory_region_init_ram_ptr: Initialize RAM memory region from a + * user-provided pointer. Accesses into the + * region will modify memory directly. * * @mr: the #MemoryRegion to be initialized. * @name: the name of the region. @@ -581,7 +581,8 @@ void memory_region_add_subregion(MemoryRegion *mr, target_phys_addr_t offset, MemoryRegion *subregion); /** - * memory_region_add_subregion: Add a subregion to a container, with overlap. + * memory_region_add_subregion_overlap: Add a subregion to a container + * with overlap. * * Adds a subregion at @offset. The subregion may overlap with other * subregions. Conflicts are resolved by having a higher @priority hide a @@ -743,7 +744,7 @@ void memory_listener_unregister(MemoryListener *listener); void memory_global_dirty_log_start(void); /** - * memory_global_dirty_log_stop: begin dirty logging for all regions + * memory_global_dirty_log_stop: end dirty logging for all regions */ void memory_global_dirty_log_stop(void); -- 1.7.10
Re: [Qemu-devel] [PATCH] vmware_vga: Cleanup and allow simple drivers to work without the fifo
On Wed, 22 Aug 2012, Jan Kiszka wrote: This is a rather big patch. I strongly suspect you can break it up into smaller pieces that address separate aspects one-by-one. Also, it is definitely to heavy for "qemu-trivial". Despite its size the changes included are fairly simple but I can try to break it up. I've sent it to qemu-trivial because it may meet the "Do not fall under an actively maintained subsystem." category as I've found no maintainer for this part. Should I send the revisions only to qemu-devel then? Thanks, BALATON Zoltan
[Qemu-devel] [PATCH v2] Fix copy&paste typos in documentation comments
Signed-off-by: BALATON Zoltan --- memory.h | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) v2: indented memory_region_init_ram_ptr diff --git a/memory.h b/memory.h index bd1bbae..f6c8e32 100644 --- a/memory.h +++ b/memory.h @@ -252,9 +252,9 @@ void memory_region_init_ram(MemoryRegion *mr, uint64_t size); /** - * memory_region_init_ram: Initialize RAM memory region from a user-provided. - * pointer. Accesses into the region will modify - * memory directly. + * memory_region_init_ram_ptr: Initialize RAM memory region from a + * user-provided pointer. Accesses into the + * region will modify memory directly. * * @mr: the #MemoryRegion to be initialized. * @name: the name of the region. @@ -581,7 +581,8 @@ void memory_region_add_subregion(MemoryRegion *mr, target_phys_addr_t offset, MemoryRegion *subregion); /** - * memory_region_add_subregion: Add a subregion to a container, with overlap. + * memory_region_add_subregion_overlap: Add a subregion to a container + * with overlap. * * Adds a subregion at @offset. The subregion may overlap with other * subregions. Conflicts are resolved by having a higher @priority hide a @@ -743,7 +744,7 @@ void memory_listener_unregister(MemoryListener *listener); void memory_global_dirty_log_start(void); /** - * memory_global_dirty_log_stop: begin dirty logging for all regions + * memory_global_dirty_log_stop: end dirty logging for all regions */ void memory_global_dirty_log_stop(void); -- 1.7.10
[Qemu-devel] [PATCH v2] console: Correct computation of bytes per pixel from bits per pixel
Division with round up is the correct way to compute this even if the
only case where division with round down gives incorrect result is
probably 15 bpp. This case was explicitely patched up in one of these
functions but was unhandled in the other. (I'm not sure about setting
16 bpp for the 15bpp case either but I left that there for now.)
Signed-off-by: BALATON Zoltan
---
console.c |5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
v2: Use DIV_ROUND_UP and extended commit message
diff --git a/console.c b/console.c
index 4525cc7..9df1701 100644
--- a/console.c
+++ b/console.c
@@ -1612,7 +1612,7 @@ PixelFormat qemu_different_endianness_pixelformat(int bpp)
memset(&pf, 0x00, sizeof(PixelFormat));
pf.bits_per_pixel = bpp;
-pf.bytes_per_pixel = bpp / 8;
+pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8);
pf.depth = bpp == 32 ? 24 : bpp;
switch (bpp) {
@@ -1661,13 +1661,12 @@ PixelFormat qemu_default_pixelformat(int bpp)
memset(&pf, 0x00, sizeof(PixelFormat));
pf.bits_per_pixel = bpp;
-pf.bytes_per_pixel = bpp / 8;
+pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8);
pf.depth = bpp == 32 ? 24 : bpp;
switch (bpp) {
case 15:
pf.bits_per_pixel = 16;
-pf.bytes_per_pixel = 2;
pf.rmask = 0x7c00;
pf.gmask = 0x03E0;
pf.bmask = 0x001F;
--
1.7.10
[Qemu-devel] [PATCH v3] console: Cleanup computation of bytes per pixel and add missing cases
Division with round up is the correct way to compute this even if the
only case where division with round down gives incorrect result is
probably 15 bpp. This case was explicitely patched up in one of these
functions but was unhandled in the other. This patch also adds the
missing cases and aborts for invalid unhandled cases. (I'm not sure
about setting 16 bpp for the 15 bpp case so I left it there for now.)
Signed-off-by: BALATON Zoltan
---
console.c | 230 +++--
1 file changed, 131 insertions(+), 99 deletions(-)
v2: Use DIV_ROUND_UP and extended commit message
v3: Add missing cases to qemu_different_endianness_pixelformat (I
have no way to test if this is correct though),
abort() for the default: case
also reindented as suggested by scripts/checkpatch.pl
diff --git a/console.c b/console.c
index 4525cc7..21843cd 100644
--- a/console.c
+++ b/console.c
@@ -1612,44 +1612,75 @@ PixelFormat qemu_different_endianness_pixelformat(int
bpp)
memset(&pf, 0x00, sizeof(PixelFormat));
pf.bits_per_pixel = bpp;
-pf.bytes_per_pixel = bpp / 8;
+pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8);
pf.depth = bpp == 32 ? 24 : bpp;
switch (bpp) {
-case 24:
-pf.rmask = 0x00FF;
-pf.gmask = 0xFF00;
-pf.bmask = 0x00FF;
-pf.rmax = 255;
-pf.gmax = 255;
-pf.bmax = 255;
-pf.rshift = 0;
-pf.gshift = 8;
-pf.bshift = 16;
-pf.rbits = 8;
-pf.gbits = 8;
-pf.bbits = 8;
-break;
-case 32:
-pf.rmask = 0xFF00;
-pf.gmask = 0x00FF;
-pf.bmask = 0xFF00;
-pf.amask = 0x;
-pf.amax = 255;
-pf.rmax = 255;
-pf.gmax = 255;
-pf.bmax = 255;
-pf.ashift = 0;
-pf.rshift = 8;
-pf.gshift = 16;
-pf.bshift = 24;
-pf.rbits = 8;
-pf.gbits = 8;
-pf.bbits = 8;
-pf.abits = 8;
-break;
-default:
-break;
+case 0:
+break;
+case 15:
+pf.bits_per_pixel = 16; /* Is this correct? */
+pf.rmask = 0x001F;
+pf.gmask = 0x03E0;
+pf.bmask = 0x7C00;
+pf.rmax = 31;
+pf.gmax = 31;
+pf.bmax = 31;
+pf.rshift = 0;
+pf.gshift = 5;
+pf.bshift = 10;
+pf.rbits = 5;
+pf.gbits = 5;
+pf.bbits = 5;
+break;
+case 16:
+pf.rmask = 0x001F;
+pf.gmask = 0x07E0;
+pf.bmask = 0xF800;
+pf.rmax = 31;
+pf.gmax = 63;
+pf.bmax = 31;
+pf.rshift = 0;
+pf.gshift = 5;
+pf.bshift = 11;
+pf.rbits = 5;
+pf.gbits = 6;
+pf.bbits = 5;
+break;
+case 24:
+pf.rmask = 0x00FF;
+pf.gmask = 0xFF00;
+pf.bmask = 0x00FF;
+pf.rmax = 255;
+pf.gmax = 255;
+pf.bmax = 255;
+pf.rshift = 0;
+pf.gshift = 8;
+pf.bshift = 16;
+pf.rbits = 8;
+pf.gbits = 8;
+pf.bbits = 8;
+break;
+case 32:
+pf.rmask = 0xFF00;
+pf.gmask = 0x00FF;
+pf.bmask = 0xFF00;
+pf.amask = 0x;
+pf.amax = 255;
+pf.rmax = 255;
+pf.gmax = 255;
+pf.bmax = 255;
+pf.ashift = 0;
+pf.rshift = 8;
+pf.gshift = 16;
+pf.bshift = 24;
+pf.rbits = 8;
+pf.gbits = 8;
+pf.bbits = 8;
+pf.abits = 8;
+break;
+default:
+abort();
}
return pf;
}
@@ -1661,73 +1692,74 @@ PixelFormat qemu_default_pixelformat(int bpp)
memset(&pf, 0x00, sizeof(PixelFormat));
pf.bits_per_pixel = bpp;
-pf.bytes_per_pixel = bpp / 8;
+pf.bytes_per_pixel = DIV_ROUND_UP(bpp, 8);
pf.depth = bpp == 32 ? 24 : bpp;
switch (bpp) {
-case 15:
-pf.bits_per_pixel = 16;
-pf.bytes_per_pixel = 2;
-pf.rmask = 0x7c00;
-pf.gmask = 0x03E0;
-pf.bmask = 0x001F;
-pf.rmax = 31;
-pf.gmax = 31;
-pf.bmax = 31;
-pf.rshift = 10;
-pf.gshift = 5;
-pf.bshift = 0;
-pf.rbits = 5;
-pf.gbits = 5;
-pf.bbits = 5;
-break;
-case 16:
-pf.rmask = 0xF800;
-pf.gmask = 0x07E0;
-pf.bmask = 0x001F;
-pf.rmax = 31;
-pf.gmax = 63;
-pf.bmax = 31;
-pf.rshift = 11;
-pf.gshift = 5;
-pf.bshift = 0;
-pf.rbits = 5;
-pf.gbits = 6;
-pf.bbits = 5;
-break;
-case 24:
-pf.rmask
[Qemu-devel] [PATCH 1/3] vmware_vga: Cleanup and remove duplicated info from local state
Removed info from vmsvga_state that is available from elsewhere and
thus was duplicated here unnecessarily. Also includes some coding
style fixes suggested by checkpatch.pl.
Signed-off-by: BALATON Zoltan
---
console.h | 20 ++
hw/vmware_vga.c | 196 +++
2 files changed, 102 insertions(+), 114 deletions(-)
Split up version of previous [PATCH] vmware_vga: Cleanup and allow
simple drivers to work without the fifo
diff --git a/console.h b/console.h
index 4334db5..00baf5b 100644
--- a/console.h
+++ b/console.h
@@ -328,6 +328,26 @@ static inline int ds_get_bytes_per_pixel(DisplayState *ds)
return ds->surface->pf.bytes_per_pixel;
}
+static inline int ds_get_depth(DisplayState *ds)
+{
+return ds->surface->pf.depth;
+}
+
+static inline int ds_get_rmask(DisplayState *ds)
+{
+return ds->surface->pf.rmask;
+}
+
+static inline int ds_get_gmask(DisplayState *ds)
+{
+return ds->surface->pf.gmask;
+}
+
+static inline int ds_get_bmask(DisplayState *ds)
+{
+return ds->surface->pf.bmask;
+}
+
#ifdef CONFIG_CURSES
#include
typedef chtype console_ch_t;
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index f5e4f44..0a73c7a 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -32,16 +32,15 @@
#define HW_FILL_ACCEL
#define HW_MOUSE_ACCEL
-# include "vga_int.h"
+#include "vga_int.h"
+
+/* See http://vmware-svga.sf.net/ for some documentation on VMWare SVGA */
struct vmsvga_state_s {
VGACommonState vga;
-int width;
-int height;
int invalidated;
int depth;
-int bypp;
int enable;
int config;
struct {
@@ -58,9 +57,6 @@ struct vmsvga_state_s {
int new_height;
uint32_t guest;
uint32_t svgaid;
-uint32_t wred;
-uint32_t wgreen;
-uint32_t wblue;
int syncing;
int fb_size;
@@ -298,40 +294,33 @@ static inline void vmsvga_update_rect(struct
vmsvga_state_s *s,
uint8_t *src;
uint8_t *dst;
-if (x + w > s->width) {
+if (x + w > ds_get_width(s->vga.ds)) {
fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
-__FUNCTION__, x, w);
-x = MIN(x, s->width);
-w = s->width - x;
+__func__, x, w);
+x = MIN(x, ds_get_width(s->vga.ds));
+w = ds_get_width(s->vga.ds) - x;
}
-if (y + h > s->height) {
+if (y + h > ds_get_height(s->vga.ds)) {
fprintf(stderr, "%s: update height too large y: %d, h: %d\n",
-__FUNCTION__, y, h);
-y = MIN(y, s->height);
-h = s->height - y;
+__func__, y, h);
+y = MIN(y, ds_get_height(s->vga.ds));
+h = ds_get_height(s->vga.ds) - y;
}
-line = h;
-bypl = s->bypp * s->width;
-width = s->bypp * w;
-start = s->bypp * x + bypl * y;
+bypl = ds_get_linesize(s->vga.ds);
+width = ds_get_bytes_per_pixel(s->vga.ds) * w;
+start = ds_get_bytes_per_pixel(s->vga.ds) * x + bypl * y;
src = s->vga.vram_ptr + start;
dst = ds_get_data(s->vga.ds) + start;
-for (; line > 0; line --, src += bypl, dst += bypl)
+for (line = h; line > 0; line--, src += bypl, dst += bypl) {
memcpy(dst, src, width);
+}
dpy_update(s->vga.ds, x, y, w, h);
}
-static inline void vmsvga_update_screen(struct vmsvga_state_s *s)
-{
-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);
-}
-
static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
int x, int y, int w, int h)
{
@@ -364,20 +353,21 @@ static inline void vmsvga_copy_rect(struct vmsvga_state_s
*s,
int x0, int y0, int x1, int y1, int w, int h)
{
uint8_t *vram = s->vga.vram_ptr;
-int bypl = s->bypp * s->width;
-int width = s->bypp * w;
+int bypl = ds_get_linesize(s->vga.ds);
+int bypp = ds_get_bytes_per_pixel(s->vga.ds);
+int width = bypp * w;
int line = h;
uint8_t *ptr[2];
if (y1 > y0) {
-ptr[0] = vram + s->bypp * x0 + bypl * (y0 + h - 1);
-ptr[1] = vram + s->bypp * x1 + bypl * (y1 + h - 1);
+ptr[0] = vram + bypp * x0 + bypl * (y0 + h - 1);
+ptr[1] = vram + 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;
+ptr[0] = vram + bypp * x0 + bypl * y0;
+ptr[1] = vram + bypp * x1 + bypl * y1;
for (; line > 0; line --, ptr[0] += bypl, ptr[1] += bypl) {
memmove(ptr[1], ptr[0], wid
[Qemu-devel] [PATCH 2/3] vmware_vga: Return a value for FB_SIZE before the device is enabled
According to the documentation drivers using this device should read
FB_SIZE before enabling the device to know what memory to map. This
would not work if we return 0 before enabled.
Signed-off-by: BALATON Zoltan
---
hw/vmware_vga.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
Split up version of previous [PATCH] vmware_vga: Cleanup and allow
simple drivers to work without the fifo
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 0a73c7a..3c4beb9 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -39,8 +39,12 @@
struct vmsvga_state_s {
VGACommonState vga;
+/* -*- The members marked are now unused and could be removed but they are
+ * contained in the VMState thus need special handling. Maybe they could
+ * be removed the next time a new machine type is added.
+ */
int invalidated;
-int depth;
+int depth; /* -*- */
int enable;
int config;
struct {
@@ -58,7 +62,7 @@ struct vmsvga_state_s {
uint32_t guest;
uint32_t svgaid;
int syncing;
-int fb_size;
+int fb_size; /* -*- */
MemoryRegion fifo_ram;
uint8_t *fifo_ptr;
@@ -733,10 +737,10 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t
address)
return 0x0;
case SVGA_REG_VRAM_SIZE:
-return s->vga.vram_size;
+return s->vga.vram_size; /* No physical VRAM besides the framebuffer */
case SVGA_REG_FB_SIZE:
-return s->fb_size;
+return s->vga.vram_size;
case SVGA_REG_CAPABILITIES:
caps = SVGA_CAP_NONE;
@@ -821,7 +825,6 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
s->invalidated = 1;
s->vga.invalidate(&s->vga);
if (s->enable) {
-s->fb_size = ((s->depth + 7) >> 3) * s->new_width * s->new_height;
vga_dirty_log_stop(&s->vga);
} else {
vga_dirty_log_start(&s->vga);
--
1.7.10
[Qemu-devel] [PATCH 3/3] vmware_vga: Allow simple drivers to work without using the fifo
Postpone stopping the dirty log to the point where the command fifo is
configured to allow drivers which don't use the fifo to work too.
(Without this the picture rendered into the vram never got to the
screen and the DIRECT_VRAM option meant to support this case was
removed a year ago.)
Signed-off-by: BALATON Zoltan
---
hw/vga.c|2 +-
hw/vga_int.h|1 +
hw/vmware_vga.c | 34 +-
3 files changed, 27 insertions(+), 10 deletions(-)
Split up version of previous [PATCH] vmware_vga: Cleanup and allow
simple drivers to work without the fifo
diff --git a/hw/vga.c b/hw/vga.c
index f82ced8..7e6dc41 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1611,7 +1611,7 @@ void vga_invalidate_scanlines(VGACommonState *s, int y1,
int y2)
}
}
-static void vga_sync_dirty_bitmap(VGACommonState *s)
+void vga_sync_dirty_bitmap(VGACommonState *s)
{
memory_region_sync_dirty_bitmap(&s->vram);
}
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 8938093..16d53ef 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -195,6 +195,7 @@ MemoryRegion *vga_init_io(VGACommonState *s,
const MemoryRegionPortio **vbe_ports);
void vga_common_reset(VGACommonState *s);
+void vga_sync_dirty_bitmap(VGACommonState *s);
void vga_dirty_log_start(VGACommonState *s);
void vga_dirty_log_stop(VGACommonState *s);
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 3c4beb9..2b77766 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -820,11 +820,10 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
break;
case SVGA_REG_ENABLE:
-s->enable = value;
-s->config &= !!value;
+s->enable = !!value;
s->invalidated = 1;
s->vga.invalidate(&s->vga);
-if (s->enable) {
+if (s->enable && s->config) {
vga_dirty_log_stop(&s->vga);
} else {
vga_dirty_log_start(&s->vga);
@@ -860,15 +859,19 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
if (value) {
s->fifo = (uint32_t *) s->fifo_ptr;
/* Check range and alignment. */
-if ((CMD(min) | CMD(max) |
-CMD(next_cmd) | CMD(stop)) & 3)
+if ((CMD(min) | CMD(max) | CMD(next_cmd) | CMD(stop)) & 3) {
break;
-if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo)
+}
+if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo) {
break;
-if (CMD(max) > SVGA_FIFO_SIZE)
+}
+if (CMD(max) > SVGA_FIFO_SIZE) {
break;
-if (CMD(max) < CMD(min) + 10 * 1024)
+}
+if (CMD(max) < CMD(min) + 10 * 1024) {
break;
+}
+vga_dirty_log_stop(&s->vga);
}
s->config = !!value;
break;
@@ -949,6 +952,8 @@ static inline void vmsvga_check_size(struct vmsvga_state_s
*s)
static void vmsvga_update_display(void *opaque)
{
struct vmsvga_state_s *s = opaque;
+bool dirty = false;
+
if (!s->enable) {
s->vga.update(&s->vga);
return;
@@ -963,13 +968,24 @@ static void vmsvga_update_display(void *opaque)
* Is it more efficient to look at vram VGA-dirty bits or wait
* for the driver to issue SVGA_CMD_UPDATE?
*/
-if (s->invalidated) {
+if (memory_region_is_logging(&s->vga.vram)) {
+vga_sync_dirty_bitmap(&s->vga);
+dirty = memory_region_get_dirty(&s->vga.vram, 0,
+ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds),
+DIRTY_MEMORY_VGA);
+}
+if (s->invalidated || dirty) {
s->invalidated = 0;
memcpy(ds_get_data(s->vga.ds), s->vga.vram_ptr,
ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds));
dpy_update(s->vga.ds, 0, 0,
ds_get_width(s->vga.ds), ds_get_height(s->vga.ds));
}
+if (dirty) {
+memory_region_reset_dirty(&s->vga.vram, 0,
+ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds),
+DIRTY_MEMORY_VGA);
+}
}
static void vmsvga_reset(DeviceState *dev)
--
1.7.10
Re: [Qemu-devel] [PATCH v3] console: Cleanup computation of bytes per pixel and add missing cases
On Fri, 24 Aug 2012, Stefan Hajnoczi wrote: +case 15: +pf.bits_per_pixel = 16; /* Is this correct? */ A trivial patch can't have "Is this correct?" parts :). I already applied the simple and correct v2 patch. Feel free to follow up with additional cleanups but with resolved "Is this correct?" parts. The v2 contained all my originally intended changes so it's fine with me. During review it was discovered that this part of code probably has more problems so I was trying to provide a fix for those in v3, and then checkpatch.pl complained about formatting too so I included a fix for that too. I can resend these broken up into too patches but as I wrote in the comment I have no way to test if these are correct. The above bits_per_pixel=16 for 15 bpp case was there already I just added a comment marking it as suspicious but I'm not sure how to resolve that. Thanks, BALATON Zoltan
Re: [Qemu-devel] [PATCH v3] console: Cleanup computation of bytes per pixel and add missing cases
On Fri, 24 Aug 2012, Peter Maydell wrote: I think all this code needs careful auditing to (a) clearly define what "bpp" means and what "depth" means (assuming we need to distinguish; IME they are usually supposed to be synonyms!) and (b) make sure that everything that accesses one or the other is actually using the right one... Right, and I don't feel like I can do that for parts I don't know at all so that's what I meant by not being sure about it and chose to add the comment as a hint for others who know better where these are used. I think I've mostly fixed it in vmware_vga which was the part I've looked at. About depth vs. bpp, they are usually synonyms except for the 32 bits case where colour depth is actually 24 bits so distinguishing the two is proabably OK. (This was one of the things that the vmware_vga reported wrongly and the driver was patching it up with issuing a warning about it.) Regards, BALATON Zoltan
Re: [Qemu-devel] [PATCH 1/4 v3] vmware_vga: Coding style cleanup
Ping? http://patchwork.ozlabs.org/patch/189750/ http://patchwork.ozlabs.org/patch/189751/ http://patchwork.ozlabs.org/patch/189752/ http://patchwork.ozlabs.org/patch/189754/
Re: [Qemu-devel] [PATCH 1/4 v3] vmware_vga: Coding style cleanup
On Thu, 18 Oct 2012, BALATON Zoltan wrote: Ping? http://patchwork.ozlabs.org/patch/189750/ http://patchwork.ozlabs.org/patch/189751/ http://patchwork.ozlabs.org/patch/189752/ http://patchwork.ozlabs.org/patch/189754/ Is there anything else that needs to be done with these or it's ignored only because nobody is interested? Could someone please tell what to do to get these merged? Thanks, BALATON Zoltan
[Qemu-devel] [PATCH 1/4 v4] vmware_vga: Coding style cleanup
Fix coding style as suggested by checkpatch.pl
Signed-off-by: BALATON Zoltan
---
hw/vmware_vga.c | 283 ++-
1 file changed, 156 insertions(+), 127 deletions(-)
v4: rebased to apply to current
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index badaf7c..913c882 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -79,7 +79,7 @@ struct vmsvga_state_s {
} *cmd;
};
-#define REDRAW_FIFO_LEN512
+#define REDRAW_FIFO_LEN 512
struct vmsvga_rect_s {
int x, y, w, h;
} redraw_fifo[REDRAW_FIFO_LEN];
@@ -92,31 +92,31 @@ struct pci_vmsvga_state_s {
MemoryRegion io_bar;
};
-#define SVGA_MAGIC 0x90UL
-#define SVGA_MAKE_ID(ver) (SVGA_MAGIC << 8 | (ver))
-#define SVGA_ID_0 SVGA_MAKE_ID(0)
-#define SVGA_ID_1 SVGA_MAKE_ID(1)
-#define SVGA_ID_2 SVGA_MAKE_ID(2)
+#define SVGA_MAGIC 0x90UL
+#define SVGA_MAKE_ID(ver) (SVGA_MAGIC << 8 | (ver))
+#define SVGA_ID_0 SVGA_MAKE_ID(0)
+#define SVGA_ID_1 SVGA_MAKE_ID(1)
+#define SVGA_ID_2 SVGA_MAKE_ID(2)
-#define SVGA_LEGACY_BASE_PORT 0x4560
-#define SVGA_INDEX_PORT0x0
-#define SVGA_VALUE_PORT0x1
-#define SVGA_BIOS_PORT 0x2
+#define SVGA_LEGACY_BASE_PORT 0x4560
+#define SVGA_INDEX_PORT 0x0
+#define SVGA_VALUE_PORT 0x1
+#define SVGA_BIOS_PORT 0x2
#define SVGA_VERSION_2
#ifdef SVGA_VERSION_2
-# define SVGA_ID SVGA_ID_2
-# define SVGA_IO_BASE SVGA_LEGACY_BASE_PORT
-# define SVGA_IO_MUL 1
-# define SVGA_FIFO_SIZE0x1
-# define SVGA_PCI_DEVICE_IDPCI_DEVICE_ID_VMWARE_SVGA2
+# define SVGA_IDSVGA_ID_2
+# define SVGA_IO_BASE SVGA_LEGACY_BASE_PORT
+# define SVGA_IO_MUL1
+# define SVGA_FIFO_SIZE 0x1
+# define SVGA_PCI_DEVICE_ID PCI_DEVICE_ID_VMWARE_SVGA2
#else
-# define SVGA_ID SVGA_ID_1
-# define SVGA_IO_BASE SVGA_LEGACY_BASE_PORT
-# define SVGA_IO_MUL 4
-# define SVGA_FIFO_SIZE0x1
-# define SVGA_PCI_DEVICE_IDPCI_DEVICE_ID_VMWARE_SVGA
+# define SVGA_IDSVGA_ID_1
+# define SVGA_IO_BASE SVGA_LEGACY_BASE_PORT
+# define SVGA_IO_MUL4
+# define SVGA_FIFO_SIZE 0x1
+# define SVGA_PCI_DEVICE_ID PCI_DEVICE_ID_VMWARE_SVGA
#endif
enum {
@@ -128,7 +128,7 @@ enum {
SVGA_REG_MAX_WIDTH = 4,
SVGA_REG_MAX_HEIGHT = 5,
SVGA_REG_DEPTH = 6,
-SVGA_REG_BITS_PER_PIXEL = 7, /* Current bpp in the guest */
+SVGA_REG_BITS_PER_PIXEL = 7,/* Current bpp in the guest */
SVGA_REG_PSEUDOCOLOR = 8,
SVGA_REG_RED_MASK = 9,
SVGA_REG_GREEN_MASK = 10,
@@ -141,46 +141,46 @@ enum {
/* ID 1 and 2 registers */
SVGA_REG_CAPABILITIES = 17,
-SVGA_REG_MEM_START = 18, /* Memory for command FIFO */
+SVGA_REG_MEM_START = 18,/* Memory for command FIFO */
SVGA_REG_MEM_SIZE = 19,
-SVGA_REG_CONFIG_DONE = 20, /* Set when memory area configured */
-SVGA_REG_SYNC = 21,/* Write to force
synchronization */
-SVGA_REG_BUSY = 22,/* Read to check if sync is
done */
-SVGA_REG_GUEST_ID = 23,/* Set guest OS identifier */
-SVGA_REG_CURSOR_ID = 24, /* ID of cursor */
-SVGA_REG_CURSOR_X = 25,/* Set cursor X position */
-SVGA_REG_CURSOR_Y = 26,/* Set cursor Y position */
-SVGA_REG_CURSOR_ON = 27, /* Turn cursor on/off */
-SVGA_REG_HOST_BITS_PER_PIXEL = 28, /* Current bpp in the host */
-SVGA_REG_SCRATCH_SIZE = 29,/* Number of scratch registers
*/
-SVGA_REG_MEM_REGS = 30,/* Number of FIFO registers */
-SVGA_REG_NUM_DISPLAYS = 31,/* Number of guest displays */
-SVGA_REG_PITCHLOCK = 32, /* Fixed pitch for all modes */
-
-SVGA_PALETTE_BASE = 1024, /* Base of SVGA color map */
+SVGA_REG_CONFIG_DONE = 20, /* Set when memory area configured */
+SVGA_REG_SYNC = 21, /* Write to force synchronization */
+SVGA_REG_BUSY = 22, /* Read to check if sync is done */
+SVGA_REG_GUEST_ID = 23, /* Set guest OS identifier */
+SVGA_REG_CURSOR_ID = 24,/* ID of cursor */
+SVGA_REG_CURSOR_X = 25, /* Set cursor X position */
+SVGA_REG_CURSOR_Y = 26, /* Set cursor Y position */
+SVGA_REG_CURSOR_ON = 27,/* Turn cursor on/off */
+SVGA_REG_HOST_BITS_PER_PIXEL = 28, /* Current bpp in the host */
+SVGA_REG_SCRATCH_SIZE = 29, /* Number of scratch registers */
+SVGA_REG_MEM_REGS = 30, /* Number of FIFO registers */
+SVGA_REG_NUM_DISPLAYS = 31, /* Number of gu
[Qemu-devel] [PATCH 2/4 v4] vmware_vga: Remove duplicated info from local state
Removed info from vmsvga_state that is available from elsewhere and
thus was duplicated here unnecessarily.
Signed-off-by: BALATON Zoltan
---
console.h | 20 +++
hw/vmware_vga.c | 156 +++
2 files changed, 84 insertions(+), 92 deletions(-)
v4: rebased to apply to current
diff --git a/console.h b/console.h
index 6099d8d..6eb1c6d 100644
--- a/console.h
+++ b/console.h
@@ -330,6 +330,26 @@ static inline int ds_get_bytes_per_pixel(DisplayState *ds)
return ds->surface->pf.bytes_per_pixel;
}
+static inline int ds_get_depth(DisplayState *ds)
+{
+return ds->surface->pf.depth;
+}
+
+static inline int ds_get_rmask(DisplayState *ds)
+{
+return ds->surface->pf.rmask;
+}
+
+static inline int ds_get_gmask(DisplayState *ds)
+{
+return ds->surface->pf.gmask;
+}
+
+static inline int ds_get_bmask(DisplayState *ds)
+{
+return ds->surface->pf.bmask;
+}
+
#ifdef CONFIG_CURSES
#include
typedef chtype console_ch_t;
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 913c882..1aa6180 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -36,11 +36,8 @@
struct vmsvga_state_s {
VGACommonState vga;
-int width;
-int height;
int invalidated;
int depth;
-int bypp;
int enable;
int config;
struct {
@@ -57,9 +54,6 @@ struct vmsvga_state_s {
int new_height;
uint32_t guest;
uint32_t svgaid;
-uint32_t wred;
-uint32_t wgreen;
-uint32_t wblue;
int syncing;
int fb_size;
@@ -297,23 +291,23 @@ static inline void vmsvga_update_rect(struct
vmsvga_state_s *s,
uint8_t *src;
uint8_t *dst;
-if (x + w > s->width) {
+if (x + w > ds_get_width(s->vga.ds)) {
fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
__func__, x, w);
-x = MIN(x, s->width);
-w = s->width - x;
+x = MIN(x, ds_get_width(s->vga.ds));
+w = ds_get_width(s->vga.ds) - x;
}
-if (y + h > s->height) {
+if (y + h > ds_get_height(s->vga.ds)) {
fprintf(stderr, "%s: update height too large y: %d, h: %d\n",
__func__, y, h);
-y = MIN(y, s->height);
-h = s->height - y;
+y = MIN(y, ds_get_height(s->vga.ds));
+h = ds_get_height(s->vga.ds) - y;
}
-bypl = s->bypp * s->width;
-width = s->bypp * w;
-start = s->bypp * x + bypl * y;
+bypl = ds_get_linesize(s->vga.ds);
+width = ds_get_bytes_per_pixel(s->vga.ds) * w;
+start = ds_get_bytes_per_pixel(s->vga.ds) * x + bypl * y;
src = s->vga.vram_ptr + start;
dst = ds_get_data(s->vga.ds) + start;
@@ -326,8 +320,9 @@ static inline void vmsvga_update_rect(struct vmsvga_state_s
*s,
static inline void vmsvga_update_screen(struct vmsvga_state_s *s)
{
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);
+ ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds));
+dpy_update(s->vga.ds, 0, 0,
+ ds_get_width(s->vga.ds), ds_get_height(s->vga.ds));
}
static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
@@ -364,20 +359,21 @@ static inline void vmsvga_copy_rect(struct vmsvga_state_s
*s,
int x0, int y0, int x1, int y1, int w, int h)
{
uint8_t *vram = s->vga.vram_ptr;
-int bypl = s->bypp * s->width;
-int width = s->bypp * w;
+int bypl = ds_get_linesize(s->vga.ds);
+int bypp = ds_get_bytes_per_pixel(s->vga.ds);
+int width = bypp * w;
int line = h;
uint8_t *ptr[2];
if (y1 > y0) {
-ptr[0] = vram + s->bypp * x0 + bypl * (y0 + h - 1);
-ptr[1] = vram + s->bypp * x1 + bypl * (y1 + h - 1);
+ptr[0] = vram + bypp * x0 + bypl * (y0 + h - 1);
+ptr[1] = vram + 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;
+ptr[0] = vram + bypp * x0 + bypl * y0;
+ptr[1] = vram + bypp * x1 + bypl * y1;
for (; line > 0; line --, ptr[0] += bypl, ptr[1] += bypl) {
memmove(ptr[1], ptr[0], width);
}
@@ -391,13 +387,11 @@ 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)
{
-uint8_t *vram = s->vga.vram_ptr;
-int bypp = s->bypp;
-int bypl = bypp * s->width;
-int width = bypp * w;
+int bypl = ds_get_linesize(s->vga.ds);
+int width = ds_get_bytes_per_pixel(s-&
[Qemu-devel] [PATCH 3/4 v4] vmware_vga: Return a value for FB_SIZE before the device is enabled
According to the documentation drivers using this device should read
FB_SIZE before enabling the device to know what memory to map. This
would not work if we return 0 before enabled. The docs also mention
reading SVGA_REG_DEPTH but not writing it. (Only SVGA_REG_BITS_PER_PIXEL
can be written but we don't really support that either.)
Signed-off-by: BALATON Zoltan
---
hw/vmware_vga.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
v4: rebased to apply to current
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 1aa6180..deb9dda 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -31,13 +31,14 @@
#define HW_FILL_ACCEL
#define HW_MOUSE_ACCEL
-# include "vga_int.h"
+#include "vga_int.h"
+
+/* See http://vmware-svga.sf.net/ for some documentation on VMWare SVGA */
struct vmsvga_state_s {
VGACommonState vga;
int invalidated;
-int depth;
int enable;
int config;
struct {
@@ -55,7 +56,6 @@ struct vmsvga_state_s {
uint32_t guest;
uint32_t svgaid;
int syncing;
-int fb_size;
MemoryRegion fifo_ram;
uint8_t *fifo_ptr;
@@ -758,10 +758,10 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t
address)
return 0x0;
case SVGA_REG_VRAM_SIZE:
-return s->vga.vram_size;
+return s->vga.vram_size; /* No physical VRAM besides the framebuffer */
case SVGA_REG_FB_SIZE:
-return s->fb_size;
+return s->vga.vram_size;
case SVGA_REG_CAPABILITIES:
caps = SVGA_CAP_NONE;
@@ -850,7 +850,6 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
s->invalidated = 1;
s->vga.invalidate(&s->vga);
if (s->enable) {
-s->fb_size = ((s->depth + 7) >> 3) * s->new_width * s->new_height;
vga_dirty_log_stop(&s->vga);
} else {
vga_dirty_log_start(&s->vga);
@@ -875,10 +874,9 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
}
break;
-case SVGA_REG_DEPTH:
case SVGA_REG_BITS_PER_PIXEL:
if (value != ds_get_bits_per_pixel(s->vga.ds)) {
-printf("%s: Bad colour depth: %i bits\n", __func__, value);
+printf("%s: Bad bits per pixel: %i bits\n", __func__, value);
s->config = 0;
}
break;
@@ -941,6 +939,7 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
#endif
break;
+case SVGA_REG_DEPTH:
case SVGA_REG_MEM_REGS:
case SVGA_REG_NUM_DISPLAYS:
case SVGA_REG_PITCHLOCK:
@@ -1079,7 +1078,7 @@ static const VMStateDescription
vmstate_vmware_vga_internal = {
.minimum_version_id_old = 0,
.post_load = vmsvga_post_load,
.fields = (VMStateField[]) {
-VMSTATE_INT32_EQUAL(depth, struct vmsvga_state_s),
+VMSTATE_UNUSED(4), /* was depth */
VMSTATE_INT32(enable, struct vmsvga_state_s),
VMSTATE_INT32(config, struct vmsvga_state_s),
VMSTATE_INT32(cursor.id, struct vmsvga_state_s),
@@ -1094,7 +1093,7 @@ static const VMStateDescription
vmstate_vmware_vga_internal = {
VMSTATE_UINT32(guest, struct vmsvga_state_s),
VMSTATE_UINT32(svgaid, struct vmsvga_state_s),
VMSTATE_INT32(syncing, struct vmsvga_state_s),
-VMSTATE_INT32(fb_size, struct vmsvga_state_s),
+VMSTATE_UNUSED(4), /* was fb_size */
VMSTATE_END_OF_LIST()
}
};
--
1.7.10
[Qemu-devel] [PATCH 4/4 v4] vmware_vga: Allow simple drivers to work without using the fifo
Postpone stopping the dirty log to the point where the command fifo is
configured to allow drivers which don't use the fifo to work too.
(Without this the picture rendered into the vram never got to the
screen and the DIRECT_VRAM option meant to support this case was
removed a year ago.)
Signed-off-by: BALATON Zoltan
---
hw/vga.c|2 +-
hw/vga_int.h|1 +
hw/vmware_vga.c | 34 +-
3 files changed, 23 insertions(+), 14 deletions(-)
v4: rebased to apply to current
diff --git a/hw/vga.c b/hw/vga.c
index e4220df..a565002 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1607,7 +1607,7 @@ void vga_invalidate_scanlines(VGACommonState *s, int y1,
int y2)
}
}
-static void vga_sync_dirty_bitmap(VGACommonState *s)
+void vga_sync_dirty_bitmap(VGACommonState *s)
{
memory_region_sync_dirty_bitmap(&s->vram);
}
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 22f1706..722de6f 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -184,6 +184,7 @@ MemoryRegion *vga_init_io(VGACommonState *s,
const MemoryRegionPortio **vbe_ports);
void vga_common_reset(VGACommonState *s);
+void vga_sync_dirty_bitmap(VGACommonState *s);
void vga_dirty_log_start(VGACommonState *s);
void vga_dirty_log_stop(VGACommonState *s);
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index deb9dda..86586ce 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -317,14 +317,6 @@ static inline void vmsvga_update_rect(struct
vmsvga_state_s *s,
dpy_update(s->vga.ds, x, y, w, h);
}
-static inline void vmsvga_update_screen(struct vmsvga_state_s *s)
-{
-memcpy(ds_get_data(s->vga.ds), s->vga.vram_ptr,
- ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds));
-dpy_update(s->vga.ds, 0, 0,
- ds_get_width(s->vga.ds), ds_get_height(s->vga.ds));
-}
-
static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
int x, int y, int w, int h)
{
@@ -845,11 +837,10 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
break;
case SVGA_REG_ENABLE:
-s->enable = value;
-s->config &= !!value;
+s->enable = !!value;
s->invalidated = 1;
s->vga.invalidate(&s->vga);
-if (s->enable) {
+if (s->enable && s->config) {
vga_dirty_log_stop(&s->vga);
} else {
vga_dirty_log_start(&s->vga);
@@ -897,6 +888,7 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
if (CMD(max) < CMD(min) + 10 * 1024) {
break;
}
+vga_dirty_log_stop(&s->vga);
}
s->config = !!value;
break;
@@ -979,6 +971,8 @@ static inline void vmsvga_check_size(struct vmsvga_state_s
*s)
static void vmsvga_update_display(void *opaque)
{
struct vmsvga_state_s *s = opaque;
+bool dirty = false;
+
if (!s->enable) {
s->vga.update(&s->vga);
return;
@@ -993,9 +987,23 @@ static void vmsvga_update_display(void *opaque)
* Is it more efficient to look at vram VGA-dirty bits or wait
* for the driver to issue SVGA_CMD_UPDATE?
*/
-if (s->invalidated) {
+if (memory_region_is_logging(&s->vga.vram)) {
+vga_sync_dirty_bitmap(&s->vga);
+dirty = memory_region_get_dirty(&s->vga.vram, 0,
+ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds),
+DIRTY_MEMORY_VGA);
+}
+if (s->invalidated || dirty) {
s->invalidated = 0;
-vmsvga_update_screen(s);
+memcpy(ds_get_data(s->vga.ds), s->vga.vram_ptr,
+ ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds));
+dpy_update(s->vga.ds, 0, 0,
+ ds_get_width(s->vga.ds), ds_get_height(s->vga.ds));
+}
+if (dirty) {
+memory_region_reset_dirty(&s->vga.vram, 0,
+ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds),
+DIRTY_MEMORY_VGA);
}
}
--
1.7.10
Re: [Qemu-devel] [PATCH 1/4 v3] vmware_vga: Coding style cleanup
On Tue, 30 Oct 2012, Blue Swirl wrote: The patches look OK. But they don't apply anymore, please rebase. Sorry, I did not notice that as they merged all right for me. I've sent a rebased set to the list now. Thanks, BALATON Zoltan
Re: [Qemu-devel] [PATCH v2] vmware_vga: fix out of bounds and invalid rects updating
On Fri, 25 Jan 2013, Michael Tokarev wrote: But since vmsvga_update_rect() has other sanity checks already, I'm adding the missing ones there as well. Cc'ing BALATON Zoltan and Andrzej Zaborowski who shows in `git blame' output and may know something in this area. I've proposed this patch before: http://patchwork.ozlabs.org/patch/197904/ but then thought that later commit 4b4496dbccc5f286f0ef411f0ff702d67cb95145 fixed this and similar problems in other devices by clipping out of bounds updates further down the road so no further checks should be needed. There was some debate on where to put these checks but I'm not sure about the outcome so I'm not sure if we need more checks in devices now or not. I'm afraid I don't have more helpful information on this. Regards, BALATON Zoltan
[Qemu-devel] [PATCH 0/3] vmware_vga: Cleanup and allow simple drivers to work without the fifo
Resending it again as I got no comments and seems to have been ignored so far. These patches simplify the vmware_vga by removing duplicated info from its local state and make it work with more guest drivers (in particular with the very simple OpenStep VMWareFB driver) that do not use the fifo while it should have no effect on other drivers.
[Qemu-devel] [PATCH 1/3 v2] vmware_vga: Cleanup and remove duplicated info from local state
Removed info from vmsvga_state that is available from elsewhere and
thus was duplicated here unnecessarily. Also includes some coding
style fixes suggested by checkpatch.pl.
Signed-off-by: BALATON Zoltan
---
console.h | 20 ++
hw/vmware_vga.c | 196 +++
2 files changed, 102 insertions(+), 114 deletions(-)
v2: Rebase to apply to current
diff --git a/console.h b/console.h
index f990684..3bcbc86 100644
--- a/console.h
+++ b/console.h
@@ -330,6 +330,26 @@ static inline int ds_get_bytes_per_pixel(DisplayState *ds)
return ds->surface->pf.bytes_per_pixel;
}
+static inline int ds_get_depth(DisplayState *ds)
+{
+return ds->surface->pf.depth;
+}
+
+static inline int ds_get_rmask(DisplayState *ds)
+{
+return ds->surface->pf.rmask;
+}
+
+static inline int ds_get_gmask(DisplayState *ds)
+{
+return ds->surface->pf.gmask;
+}
+
+static inline int ds_get_bmask(DisplayState *ds)
+{
+return ds->surface->pf.bmask;
+}
+
#ifdef CONFIG_CURSES
#include
typedef chtype console_ch_t;
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index b68e883..20f4fb8 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -32,16 +32,15 @@
#define HW_FILL_ACCEL
#define HW_MOUSE_ACCEL
-# include "vga_int.h"
+#include "vga_int.h"
+
+/* See http://vmware-svga.sf.net/ for some documentation on VMWare SVGA */
struct vmsvga_state_s {
VGACommonState vga;
-int width;
-int height;
int invalidated;
int depth;
-int bypp;
int enable;
int config;
struct {
@@ -58,9 +57,6 @@ struct vmsvga_state_s {
int new_height;
uint32_t guest;
uint32_t svgaid;
-uint32_t wred;
-uint32_t wgreen;
-uint32_t wblue;
int syncing;
int fb_size;
@@ -298,40 +294,33 @@ static inline void vmsvga_update_rect(struct
vmsvga_state_s *s,
uint8_t *src;
uint8_t *dst;
-if (x + w > s->width) {
+if (x + w > ds_get_width(s->vga.ds)) {
fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
-__FUNCTION__, x, w);
-x = MIN(x, s->width);
-w = s->width - x;
+__func__, x, w);
+x = MIN(x, ds_get_width(s->vga.ds));
+w = ds_get_width(s->vga.ds) - x;
}
-if (y + h > s->height) {
+if (y + h > ds_get_height(s->vga.ds)) {
fprintf(stderr, "%s: update height too large y: %d, h: %d\n",
-__FUNCTION__, y, h);
-y = MIN(y, s->height);
-h = s->height - y;
+__func__, y, h);
+y = MIN(y, ds_get_height(s->vga.ds));
+h = ds_get_height(s->vga.ds) - y;
}
-line = h;
-bypl = s->bypp * s->width;
-width = s->bypp * w;
-start = s->bypp * x + bypl * y;
+bypl = ds_get_linesize(s->vga.ds);
+width = ds_get_bytes_per_pixel(s->vga.ds) * w;
+start = ds_get_bytes_per_pixel(s->vga.ds) * x + bypl * y;
src = s->vga.vram_ptr + start;
dst = ds_get_data(s->vga.ds) + start;
-for (; line > 0; line --, src += bypl, dst += bypl)
+for (line = h; line > 0; line--, src += bypl, dst += bypl) {
memcpy(dst, src, width);
+}
dpy_update(s->vga.ds, x, y, w, h);
}
-static inline void vmsvga_update_screen(struct vmsvga_state_s *s)
-{
-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);
-}
-
static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
int x, int y, int w, int h)
{
@@ -364,20 +353,21 @@ static inline void vmsvga_copy_rect(struct vmsvga_state_s
*s,
int x0, int y0, int x1, int y1, int w, int h)
{
uint8_t *vram = s->vga.vram_ptr;
-int bypl = s->bypp * s->width;
-int width = s->bypp * w;
+int bypl = ds_get_linesize(s->vga.ds);
+int bypp = ds_get_bytes_per_pixel(s->vga.ds);
+int width = bypp * w;
int line = h;
uint8_t *ptr[2];
if (y1 > y0) {
-ptr[0] = vram + s->bypp * x0 + bypl * (y0 + h - 1);
-ptr[1] = vram + s->bypp * x1 + bypl * (y1 + h - 1);
+ptr[0] = vram + bypp * x0 + bypl * (y0 + h - 1);
+ptr[1] = vram + 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;
+ptr[0] = vram + bypp * x0 + bypl * y0;
+ptr[1] = vram + bypp * x1 + bypl * y1;
for (; line > 0; line --, ptr[0] += bypl, ptr[1] += bypl) {
memmove(ptr[1], ptr[0], width);
}
@@ -391,13 +381,11 @@ static inline void vmsvga_copy
[Qemu-devel] [PATCH 2/3 v2] vmware_vga: Return a value for FB_SIZE before the device is enabled
According to the documentation drivers using this device should read
FB_SIZE before enabling the device to know what memory to map. This
would not work if we return 0 before enabled.
Signed-off-by: BALATON Zoltan
---
hw/vmware_vga.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
v2: Rebase to apply to current
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 20f4fb8..5e4786f 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -39,8 +39,12 @@
struct vmsvga_state_s {
VGACommonState vga;
+/* -*- The members marked are now unused and could be removed but they are
+ * contained in the VMState thus need special handling. Maybe they could
+ * be removed the next time a new machine type is added.
+ */
int invalidated;
-int depth;
+int depth; /* -*- */
int enable;
int config;
struct {
@@ -58,7 +62,7 @@ struct vmsvga_state_s {
uint32_t guest;
uint32_t svgaid;
int syncing;
-int fb_size;
+int fb_size; /* -*- */
MemoryRegion fifo_ram;
uint8_t *fifo_ptr;
@@ -733,10 +737,10 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t
address)
return 0x0;
case SVGA_REG_VRAM_SIZE:
-return s->vga.vram_size;
+return s->vga.vram_size; /* No physical VRAM besides the framebuffer */
case SVGA_REG_FB_SIZE:
-return s->fb_size;
+return s->vga.vram_size;
case SVGA_REG_CAPABILITIES:
caps = SVGA_CAP_NONE;
@@ -821,7 +825,6 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
s->invalidated = 1;
s->vga.invalidate(&s->vga);
if (s->enable) {
-s->fb_size = ((s->depth + 7) >> 3) * s->new_width * s->new_height;
vga_dirty_log_stop(&s->vga);
} else {
vga_dirty_log_start(&s->vga);
--
1.7.10
[Qemu-devel] [PATCH 3/3 v2] vmware_vga: Allow simple drivers to work without using the fifo
Postpone stopping the dirty log to the point where the command fifo is
configured to allow drivers which don't use the fifo to work too.
(Without this the picture rendered into the vram never got to the
screen and the DIRECT_VRAM option meant to support this case was
removed a year ago.)
Signed-off-by: BALATON Zoltan
---
hw/vga.c|2 +-
hw/vga_int.h|1 +
hw/vmware_vga.c | 34 +-
3 files changed, 27 insertions(+), 10 deletions(-)
v2: Rebase to apply to current
diff --git a/hw/vga.c b/hw/vga.c
index 80299ea..7939a9d 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1612,7 +1612,7 @@ void vga_invalidate_scanlines(VGACommonState *s, int y1,
int y2)
}
}
-static void vga_sync_dirty_bitmap(VGACommonState *s)
+void vga_sync_dirty_bitmap(VGACommonState *s)
{
memory_region_sync_dirty_bitmap(&s->vram);
}
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 330a32f..c01d07d 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -196,6 +196,7 @@ MemoryRegion *vga_init_io(VGACommonState *s,
const MemoryRegionPortio **vbe_ports);
void vga_common_reset(VGACommonState *s);
+void vga_sync_dirty_bitmap(VGACommonState *s);
void vga_dirty_log_start(VGACommonState *s);
void vga_dirty_log_stop(VGACommonState *s);
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 5e4786f..17b3140 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -820,11 +820,10 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
break;
case SVGA_REG_ENABLE:
-s->enable = value;
-s->config &= !!value;
+s->enable = !!value;
s->invalidated = 1;
s->vga.invalidate(&s->vga);
-if (s->enable) {
+if (s->enable && s->config) {
vga_dirty_log_stop(&s->vga);
} else {
vga_dirty_log_start(&s->vga);
@@ -860,15 +859,19 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
if (value) {
s->fifo = (uint32_t *) s->fifo_ptr;
/* Check range and alignment. */
-if ((CMD(min) | CMD(max) |
-CMD(next_cmd) | CMD(stop)) & 3)
+if ((CMD(min) | CMD(max) | CMD(next_cmd) | CMD(stop)) & 3) {
break;
-if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo)
+}
+if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo) {
break;
-if (CMD(max) > SVGA_FIFO_SIZE)
+}
+if (CMD(max) > SVGA_FIFO_SIZE) {
break;
-if (CMD(max) < CMD(min) + 10 * 1024)
+}
+if (CMD(max) < CMD(min) + 10 * 1024) {
break;
+}
+vga_dirty_log_stop(&s->vga);
}
s->config = !!value;
break;
@@ -949,6 +952,8 @@ static inline void vmsvga_check_size(struct vmsvga_state_s
*s)
static void vmsvga_update_display(void *opaque)
{
struct vmsvga_state_s *s = opaque;
+bool dirty = false;
+
if (!s->enable) {
s->vga.update(&s->vga);
return;
@@ -963,13 +968,24 @@ static void vmsvga_update_display(void *opaque)
* Is it more efficient to look at vram VGA-dirty bits or wait
* for the driver to issue SVGA_CMD_UPDATE?
*/
-if (s->invalidated) {
+if (memory_region_is_logging(&s->vga.vram)) {
+vga_sync_dirty_bitmap(&s->vga);
+dirty = memory_region_get_dirty(&s->vga.vram, 0,
+ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds),
+DIRTY_MEMORY_VGA);
+}
+if (s->invalidated || dirty) {
s->invalidated = 0;
memcpy(ds_get_data(s->vga.ds), s->vga.vram_ptr,
ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds));
dpy_update(s->vga.ds, 0, 0,
ds_get_width(s->vga.ds), ds_get_height(s->vga.ds));
}
+if (dirty) {
+memory_region_reset_dirty(&s->vga.vram, 0,
+ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds),
+DIRTY_MEMORY_VGA);
+}
}
static void vmsvga_reset(DeviceState *dev)
--
1.7.10
[Qemu-devel] [PATCH 1/4 v3] vmware_vga: Coding style cleanup
Fix coding style as suggested by checkpatch.pl
Signed-off-by: BALATON Zoltan
---
hw/vmware_vga.c | 283 ++-
1 file changed, 156 insertions(+), 127 deletions(-)
v3: More complete code style cleanup now in a separate patch
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index e815a04..6380ffe 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -80,7 +80,7 @@ struct vmsvga_state_s {
} *cmd;
};
-#define REDRAW_FIFO_LEN512
+#define REDRAW_FIFO_LEN 512
struct vmsvga_rect_s {
int x, y, w, h;
} redraw_fifo[REDRAW_FIFO_LEN];
@@ -93,31 +93,31 @@ struct pci_vmsvga_state_s {
MemoryRegion io_bar;
};
-#define SVGA_MAGIC 0x90UL
-#define SVGA_MAKE_ID(ver) (SVGA_MAGIC << 8 | (ver))
-#define SVGA_ID_0 SVGA_MAKE_ID(0)
-#define SVGA_ID_1 SVGA_MAKE_ID(1)
-#define SVGA_ID_2 SVGA_MAKE_ID(2)
+#define SVGA_MAGIC 0x90UL
+#define SVGA_MAKE_ID(ver) (SVGA_MAGIC << 8 | (ver))
+#define SVGA_ID_0 SVGA_MAKE_ID(0)
+#define SVGA_ID_1 SVGA_MAKE_ID(1)
+#define SVGA_ID_2 SVGA_MAKE_ID(2)
-#define SVGA_LEGACY_BASE_PORT 0x4560
-#define SVGA_INDEX_PORT0x0
-#define SVGA_VALUE_PORT0x1
-#define SVGA_BIOS_PORT 0x2
+#define SVGA_LEGACY_BASE_PORT 0x4560
+#define SVGA_INDEX_PORT 0x0
+#define SVGA_VALUE_PORT 0x1
+#define SVGA_BIOS_PORT 0x2
#define SVGA_VERSION_2
#ifdef SVGA_VERSION_2
-# define SVGA_ID SVGA_ID_2
-# define SVGA_IO_BASE SVGA_LEGACY_BASE_PORT
-# define SVGA_IO_MUL 1
-# define SVGA_FIFO_SIZE0x1
-# define SVGA_PCI_DEVICE_IDPCI_DEVICE_ID_VMWARE_SVGA2
+# define SVGA_IDSVGA_ID_2
+# define SVGA_IO_BASE SVGA_LEGACY_BASE_PORT
+# define SVGA_IO_MUL1
+# define SVGA_FIFO_SIZE 0x1
+# define SVGA_PCI_DEVICE_ID PCI_DEVICE_ID_VMWARE_SVGA2
#else
-# define SVGA_ID SVGA_ID_1
-# define SVGA_IO_BASE SVGA_LEGACY_BASE_PORT
-# define SVGA_IO_MUL 4
-# define SVGA_FIFO_SIZE0x1
-# define SVGA_PCI_DEVICE_IDPCI_DEVICE_ID_VMWARE_SVGA
+# define SVGA_IDSVGA_ID_1
+# define SVGA_IO_BASE SVGA_LEGACY_BASE_PORT
+# define SVGA_IO_MUL4
+# define SVGA_FIFO_SIZE 0x1
+# define SVGA_PCI_DEVICE_ID PCI_DEVICE_ID_VMWARE_SVGA
#endif
enum {
@@ -129,7 +129,7 @@ enum {
SVGA_REG_MAX_WIDTH = 4,
SVGA_REG_MAX_HEIGHT = 5,
SVGA_REG_DEPTH = 6,
-SVGA_REG_BITS_PER_PIXEL = 7, /* Current bpp in the guest */
+SVGA_REG_BITS_PER_PIXEL = 7,/* Current bpp in the guest */
SVGA_REG_PSEUDOCOLOR = 8,
SVGA_REG_RED_MASK = 9,
SVGA_REG_GREEN_MASK = 10,
@@ -142,46 +142,46 @@ enum {
/* ID 1 and 2 registers */
SVGA_REG_CAPABILITIES = 17,
-SVGA_REG_MEM_START = 18, /* Memory for command FIFO */
+SVGA_REG_MEM_START = 18,/* Memory for command FIFO */
SVGA_REG_MEM_SIZE = 19,
-SVGA_REG_CONFIG_DONE = 20, /* Set when memory area configured */
-SVGA_REG_SYNC = 21,/* Write to force
synchronization */
-SVGA_REG_BUSY = 22,/* Read to check if sync is
done */
-SVGA_REG_GUEST_ID = 23,/* Set guest OS identifier */
-SVGA_REG_CURSOR_ID = 24, /* ID of cursor */
-SVGA_REG_CURSOR_X = 25,/* Set cursor X position */
-SVGA_REG_CURSOR_Y = 26,/* Set cursor Y position */
-SVGA_REG_CURSOR_ON = 27, /* Turn cursor on/off */
-SVGA_REG_HOST_BITS_PER_PIXEL = 28, /* Current bpp in the host */
-SVGA_REG_SCRATCH_SIZE = 29,/* Number of scratch registers
*/
-SVGA_REG_MEM_REGS = 30,/* Number of FIFO registers */
-SVGA_REG_NUM_DISPLAYS = 31,/* Number of guest displays */
-SVGA_REG_PITCHLOCK = 32, /* Fixed pitch for all modes */
-
-SVGA_PALETTE_BASE = 1024, /* Base of SVGA color map */
+SVGA_REG_CONFIG_DONE = 20, /* Set when memory area configured */
+SVGA_REG_SYNC = 21, /* Write to force synchronization */
+SVGA_REG_BUSY = 22, /* Read to check if sync is done */
+SVGA_REG_GUEST_ID = 23, /* Set guest OS identifier */
+SVGA_REG_CURSOR_ID = 24,/* ID of cursor */
+SVGA_REG_CURSOR_X = 25, /* Set cursor X position */
+SVGA_REG_CURSOR_Y = 26, /* Set cursor Y position */
+SVGA_REG_CURSOR_ON = 27,/* Turn cursor on/off */
+SVGA_REG_HOST_BITS_PER_PIXEL = 28, /* Current bpp in the host */
+SVGA_REG_SCRATCH_SIZE = 29, /* Number of scratch registers */
+SVGA_REG_MEM_REGS = 30, /* Number of FIFO registers */
+SVGA_REG_NUM_D
[Qemu-devel] [PATCH 2/4 v3] vmware_vga: Remove duplicated info from local state
Removed info from vmsvga_state that is available from elsewhere and
thus was duplicated here unnecessarily.
Signed-off-by: BALATON Zoltan
---
console.h | 20 +++
hw/vmware_vga.c | 156 +++
2 files changed, 84 insertions(+), 92 deletions(-)
v3: Split off unrelated changes to other commits
diff --git a/console.h b/console.h
index f990684..3bcbc86 100644
--- a/console.h
+++ b/console.h
@@ -330,6 +330,26 @@ static inline int ds_get_bytes_per_pixel(DisplayState *ds)
return ds->surface->pf.bytes_per_pixel;
}
+static inline int ds_get_depth(DisplayState *ds)
+{
+return ds->surface->pf.depth;
+}
+
+static inline int ds_get_rmask(DisplayState *ds)
+{
+return ds->surface->pf.rmask;
+}
+
+static inline int ds_get_gmask(DisplayState *ds)
+{
+return ds->surface->pf.gmask;
+}
+
+static inline int ds_get_bmask(DisplayState *ds)
+{
+return ds->surface->pf.bmask;
+}
+
#ifdef CONFIG_CURSES
#include
typedef chtype console_ch_t;
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 6380ffe..c5a8909 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -37,11 +37,8 @@
struct vmsvga_state_s {
VGACommonState vga;
-int width;
-int height;
int invalidated;
int depth;
-int bypp;
int enable;
int config;
struct {
@@ -58,9 +55,6 @@ struct vmsvga_state_s {
int new_height;
uint32_t guest;
uint32_t svgaid;
-uint32_t wred;
-uint32_t wgreen;
-uint32_t wblue;
int syncing;
int fb_size;
@@ -298,23 +292,23 @@ static inline void vmsvga_update_rect(struct
vmsvga_state_s *s,
uint8_t *src;
uint8_t *dst;
-if (x + w > s->width) {
+if (x + w > ds_get_width(s->vga.ds)) {
fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
__func__, x, w);
-x = MIN(x, s->width);
-w = s->width - x;
+x = MIN(x, ds_get_width(s->vga.ds));
+w = ds_get_width(s->vga.ds) - x;
}
-if (y + h > s->height) {
+if (y + h > ds_get_height(s->vga.ds)) {
fprintf(stderr, "%s: update height too large y: %d, h: %d\n",
__func__, y, h);
-y = MIN(y, s->height);
-h = s->height - y;
+y = MIN(y, ds_get_height(s->vga.ds));
+h = ds_get_height(s->vga.ds) - y;
}
-bypl = s->bypp * s->width;
-width = s->bypp * w;
-start = s->bypp * x + bypl * y;
+bypl = ds_get_linesize(s->vga.ds);
+width = ds_get_bytes_per_pixel(s->vga.ds) * w;
+start = ds_get_bytes_per_pixel(s->vga.ds) * x + bypl * y;
src = s->vga.vram_ptr + start;
dst = ds_get_data(s->vga.ds) + start;
@@ -327,8 +321,9 @@ static inline void vmsvga_update_rect(struct vmsvga_state_s
*s,
static inline void vmsvga_update_screen(struct vmsvga_state_s *s)
{
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);
+ ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds));
+dpy_update(s->vga.ds, 0, 0,
+ ds_get_width(s->vga.ds), ds_get_height(s->vga.ds));
}
static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
@@ -365,20 +360,21 @@ static inline void vmsvga_copy_rect(struct vmsvga_state_s
*s,
int x0, int y0, int x1, int y1, int w, int h)
{
uint8_t *vram = s->vga.vram_ptr;
-int bypl = s->bypp * s->width;
-int width = s->bypp * w;
+int bypl = ds_get_linesize(s->vga.ds);
+int bypp = ds_get_bytes_per_pixel(s->vga.ds);
+int width = bypp * w;
int line = h;
uint8_t *ptr[2];
if (y1 > y0) {
-ptr[0] = vram + s->bypp * x0 + bypl * (y0 + h - 1);
-ptr[1] = vram + s->bypp * x1 + bypl * (y1 + h - 1);
+ptr[0] = vram + bypp * x0 + bypl * (y0 + h - 1);
+ptr[1] = vram + 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;
+ptr[0] = vram + bypp * x0 + bypl * y0;
+ptr[1] = vram + bypp * x1 + bypl * y1;
for (; line > 0; line --, ptr[0] += bypl, ptr[1] += bypl) {
memmove(ptr[1], ptr[0], width);
}
@@ -392,13 +388,11 @@ 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)
{
-uint8_t *vram = s->vga.vram_ptr;
-int bypp = s->bypp;
-int bypl = bypp * s->width;
-int width = bypp * w;
+int bypl = ds_get_linesize(s->vga.ds);
+int width = ds_get_bytes_p
[Qemu-devel] [PATCH 3/4 v3] vmware_vga: Return a value for FB_SIZE before the device is enabled
According to the documentation drivers using this device should read
FB_SIZE before enabling the device to know what memory to map. This
would not work if we return 0 before enabled. The docs also mention
reading SVGA_REG_DEPTH but not writing it. (Only SVGA_REG_BITS_PER_PIXEL
can be written but we don't really support that either.)
Signed-off-by: BALATON Zoltan
---
hw/vmware_vga.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
v3: Changes to address comments from Paolo Bonzini (Thanks for the review!)
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index c5a8909..c3ef924 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -32,13 +32,14 @@
#define HW_FILL_ACCEL
#define HW_MOUSE_ACCEL
-# include "vga_int.h"
+#include "vga_int.h"
+
+/* See http://vmware-svga.sf.net/ for some documentation on VMWare SVGA */
struct vmsvga_state_s {
VGACommonState vga;
int invalidated;
-int depth;
int enable;
int config;
struct {
@@ -56,7 +57,6 @@ struct vmsvga_state_s {
uint32_t guest;
uint32_t svgaid;
int syncing;
-int fb_size;
MemoryRegion fifo_ram;
uint8_t *fifo_ptr;
@@ -759,10 +759,10 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t
address)
return 0x0;
case SVGA_REG_VRAM_SIZE:
-return s->vga.vram_size;
+return s->vga.vram_size; /* No physical VRAM besides the framebuffer */
case SVGA_REG_FB_SIZE:
-return s->fb_size;
+return s->vga.vram_size;
case SVGA_REG_CAPABILITIES:
caps = SVGA_CAP_NONE;
@@ -851,7 +851,6 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
s->invalidated = 1;
s->vga.invalidate(&s->vga);
if (s->enable) {
-s->fb_size = ((s->depth + 7) >> 3) * s->new_width * s->new_height;
vga_dirty_log_stop(&s->vga);
} else {
vga_dirty_log_start(&s->vga);
@@ -876,10 +875,9 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
}
break;
-case SVGA_REG_DEPTH:
case SVGA_REG_BITS_PER_PIXEL:
if (value != ds_get_bits_per_pixel(s->vga.ds)) {
-printf("%s: Bad colour depth: %i bits\n", __func__, value);
+printf("%s: Bad bits per pixel: %i bits\n", __func__, value);
s->config = 0;
}
break;
@@ -942,6 +940,7 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
#endif
break;
+case SVGA_REG_DEPTH:
case SVGA_REG_MEM_REGS:
case SVGA_REG_NUM_DISPLAYS:
case SVGA_REG_PITCHLOCK:
@@ -1080,7 +1079,7 @@ static const VMStateDescription
vmstate_vmware_vga_internal = {
.minimum_version_id_old = 0,
.post_load = vmsvga_post_load,
.fields = (VMStateField[]) {
-VMSTATE_INT32_EQUAL(depth, struct vmsvga_state_s),
+VMSTATE_UNUSED(4), /* was depth */
VMSTATE_INT32(enable, struct vmsvga_state_s),
VMSTATE_INT32(config, struct vmsvga_state_s),
VMSTATE_INT32(cursor.id, struct vmsvga_state_s),
@@ -1095,7 +1094,7 @@ static const VMStateDescription
vmstate_vmware_vga_internal = {
VMSTATE_UINT32(guest, struct vmsvga_state_s),
VMSTATE_UINT32(svgaid, struct vmsvga_state_s),
VMSTATE_INT32(syncing, struct vmsvga_state_s),
-VMSTATE_INT32(fb_size, struct vmsvga_state_s),
+VMSTATE_UNUSED(4), /* was fb_size */
VMSTATE_END_OF_LIST()
}
};
--
1.7.10
[Qemu-devel] [PATCH 4/4 v3] vmware_vga: Allow simple drivers to work without using the fifo
Postpone stopping the dirty log to the point where the command fifo is
configured to allow drivers which don't use the fifo to work too.
(Without this the picture rendered into the vram never got to the
screen and the DIRECT_VRAM option meant to support this case was
removed a year ago.)
Signed-off-by: BALATON Zoltan
---
hw/vga.c|2 +-
hw/vga_int.h|1 +
hw/vmware_vga.c | 34 +-
3 files changed, 23 insertions(+), 14 deletions(-)
v3: Split off coding style fixes and explicitely inline update function here
diff --git a/hw/vga.c b/hw/vga.c
index afaef0d..8f7df0b 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1616,7 +1616,7 @@ void vga_invalidate_scanlines(VGACommonState *s, int y1,
int y2)
}
}
-static void vga_sync_dirty_bitmap(VGACommonState *s)
+void vga_sync_dirty_bitmap(VGACommonState *s)
{
memory_region_sync_dirty_bitmap(&s->vram);
}
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 330a32f..c01d07d 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -196,6 +196,7 @@ MemoryRegion *vga_init_io(VGACommonState *s,
const MemoryRegionPortio **vbe_ports);
void vga_common_reset(VGACommonState *s);
+void vga_sync_dirty_bitmap(VGACommonState *s);
void vga_dirty_log_start(VGACommonState *s);
void vga_dirty_log_stop(VGACommonState *s);
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index c3ef924..d46d5ba 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -318,14 +318,6 @@ static inline void vmsvga_update_rect(struct
vmsvga_state_s *s,
dpy_update(s->vga.ds, x, y, w, h);
}
-static inline void vmsvga_update_screen(struct vmsvga_state_s *s)
-{
-memcpy(ds_get_data(s->vga.ds), s->vga.vram_ptr,
- ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds));
-dpy_update(s->vga.ds, 0, 0,
- ds_get_width(s->vga.ds), ds_get_height(s->vga.ds));
-}
-
static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
int x, int y, int w, int h)
{
@@ -846,11 +838,10 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
break;
case SVGA_REG_ENABLE:
-s->enable = value;
-s->config &= !!value;
+s->enable = !!value;
s->invalidated = 1;
s->vga.invalidate(&s->vga);
-if (s->enable) {
+if (s->enable && s->config) {
vga_dirty_log_stop(&s->vga);
} else {
vga_dirty_log_start(&s->vga);
@@ -898,6 +889,7 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
if (CMD(max) < CMD(min) + 10 * 1024) {
break;
}
+vga_dirty_log_stop(&s->vga);
}
s->config = !!value;
break;
@@ -980,6 +972,8 @@ static inline void vmsvga_check_size(struct vmsvga_state_s
*s)
static void vmsvga_update_display(void *opaque)
{
struct vmsvga_state_s *s = opaque;
+bool dirty = false;
+
if (!s->enable) {
s->vga.update(&s->vga);
return;
@@ -994,9 +988,23 @@ static void vmsvga_update_display(void *opaque)
* Is it more efficient to look at vram VGA-dirty bits or wait
* for the driver to issue SVGA_CMD_UPDATE?
*/
-if (s->invalidated) {
+if (memory_region_is_logging(&s->vga.vram)) {
+vga_sync_dirty_bitmap(&s->vga);
+dirty = memory_region_get_dirty(&s->vga.vram, 0,
+ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds),
+DIRTY_MEMORY_VGA);
+}
+if (s->invalidated || dirty) {
s->invalidated = 0;
-vmsvga_update_screen(s);
+memcpy(ds_get_data(s->vga.ds), s->vga.vram_ptr,
+ ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds));
+dpy_update(s->vga.ds, 0, 0,
+ ds_get_width(s->vga.ds), ds_get_height(s->vga.ds));
+}
+if (dirty) {
+memory_region_reset_dirty(&s->vga.vram, 0,
+ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds),
+DIRTY_MEMORY_VGA);
}
}
--
1.7.10
Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register
On Tue, 4 Sep 2012, Avi Kivity wrote: On 09/04/2012 12:15 PM, Paolo Bonzini wrote: Il 04/09/2012 10:16, Avi Kivity ha scritto: But the point of subsections is to succeed migration in the common case, assuming there is more than one case that doesn't affect guest operation. According to the patch, if icw3 == 4 && !(eclr & 4), then behaviour will change. With the standard configuration, if two pci interrupts hit at once, then before the patch irr.2 will be clear, and afterwards set. So we do have a behavioural change. Is the rest of the code masking this change under the standard configuration? No, it is not masking the change. The assumption is that nothing should care about irr.2 or isr.2, because nothing attaches an handler to the cascade interrupt. Won't the next call to pic_get_irq() notice the difference in s->irr? You have to choose between assuming this, and breaking backwards migration. I would rather break backwards migration, but others disagree... Normally I'd agree, but if the only known breakee is a 1987 guest then I'd make an exception. Another one affected by this is OpenStep 4.2 (probably NeXTstep and Rhapsody too) which are not exactly recent either but there are more than only one "breakee". Regards, BALATON Zoltan
Re: [Qemu-devel] [PATCH 1/3] vmware_vga: Cleanup and remove duplicated info from local state
Ping!
http://patchwork.ozlabs.org/patch/179449/
On Thu, 23 Aug 2012, BALATON Zoltan wrote:
Removed info from vmsvga_state that is available from elsewhere and
thus was duplicated here unnecessarily. Also includes some coding
style fixes suggested by checkpatch.pl.
Signed-off-by: BALATON Zoltan
---
console.h | 20 ++
hw/vmware_vga.c | 196
+++
2 files changed, 102 insertions(+), 114 deletions(-)
Split up version of previous [PATCH] vmware_vga: Cleanup and allow
simple drivers to work without the fifo
diff --git a/console.h b/console.h
index 4334db5..00baf5b 100644
--- a/console.h
+++ b/console.h
@@ -328,6 +328,26 @@ static inline int ds_get_bytes_per_pixel(DisplayState
*ds)
return ds->surface->pf.bytes_per_pixel;
}
+static inline int ds_get_depth(DisplayState *ds)
+{
+return ds->surface->pf.depth;
+}
+
+static inline int ds_get_rmask(DisplayState *ds)
+{
+return ds->surface->pf.rmask;
+}
+
+static inline int ds_get_gmask(DisplayState *ds)
+{
+return ds->surface->pf.gmask;
+}
+
+static inline int ds_get_bmask(DisplayState *ds)
+{
+return ds->surface->pf.bmask;
+}
+
#ifdef CONFIG_CURSES
#include
typedef chtype console_ch_t;
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index f5e4f44..0a73c7a 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -32,16 +32,15 @@
#define HW_FILL_ACCEL
#define HW_MOUSE_ACCEL
-# include "vga_int.h"
+#include "vga_int.h"
+
+/* See http://vmware-svga.sf.net/ for some documentation on VMWare SVGA */
struct vmsvga_state_s {
VGACommonState vga;
-int width;
-int height;
int invalidated;
int depth;
-int bypp;
int enable;
int config;
struct {
@@ -58,9 +57,6 @@ struct vmsvga_state_s {
int new_height;
uint32_t guest;
uint32_t svgaid;
-uint32_t wred;
-uint32_t wgreen;
-uint32_t wblue;
int syncing;
int fb_size;
@@ -298,40 +294,33 @@ static inline void vmsvga_update_rect(struct
vmsvga_state_s *s,
uint8_t *src;
uint8_t *dst;
-if (x + w > s->width) {
+if (x + w > ds_get_width(s->vga.ds)) {
fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
-__FUNCTION__, x, w);
-x = MIN(x, s->width);
-w = s->width - x;
+__func__, x, w);
+x = MIN(x, ds_get_width(s->vga.ds));
+w = ds_get_width(s->vga.ds) - x;
}
-if (y + h > s->height) {
+if (y + h > ds_get_height(s->vga.ds)) {
fprintf(stderr, "%s: update height too large y: %d, h: %d\n",
-__FUNCTION__, y, h);
-y = MIN(y, s->height);
-h = s->height - y;
+__func__, y, h);
+y = MIN(y, ds_get_height(s->vga.ds));
+h = ds_get_height(s->vga.ds) - y;
}
-line = h;
-bypl = s->bypp * s->width;
-width = s->bypp * w;
-start = s->bypp * x + bypl * y;
+bypl = ds_get_linesize(s->vga.ds);
+width = ds_get_bytes_per_pixel(s->vga.ds) * w;
+start = ds_get_bytes_per_pixel(s->vga.ds) * x + bypl * y;
src = s->vga.vram_ptr + start;
dst = ds_get_data(s->vga.ds) + start;
-for (; line > 0; line --, src += bypl, dst += bypl)
+for (line = h; line > 0; line--, src += bypl, dst += bypl) {
memcpy(dst, src, width);
+}
dpy_update(s->vga.ds, x, y, w, h);
}
-static inline void vmsvga_update_screen(struct vmsvga_state_s *s)
-{
-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);
-}
-
static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
int x, int y, int w, int h)
{
@@ -364,20 +353,21 @@ static inline void vmsvga_copy_rect(struct
vmsvga_state_s *s,
int x0, int y0, int x1, int y1, int w, int h)
{
uint8_t *vram = s->vga.vram_ptr;
-int bypl = s->bypp * s->width;
-int width = s->bypp * w;
+int bypl = ds_get_linesize(s->vga.ds);
+int bypp = ds_get_bytes_per_pixel(s->vga.ds);
+int width = bypp * w;
int line = h;
uint8_t *ptr[2];
if (y1 > y0) {
-ptr[0] = vram + s->bypp * x0 + bypl * (y0 + h - 1);
-ptr[1] = vram + s->bypp * x1 + bypl * (y1 + h - 1);
+ptr[0] = vram + bypp * x0 + bypl * (y0 + h - 1);
+ptr[1] = vram + 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;
+ptr[0] = vram + bypp * x0 + bypl * y0;
+ptr[1] = vram + bypp * x1 + bypl * y1;
for (; line > 0; line --, ptr[0] += bypl, p
[Qemu-devel] [PATCH 2/3 v2] vmware_vga: Return a value for FB_SIZE before the device is enabled
According to the documentation drivers using this device should read
FB_SIZE before enabling the device to know what memory to map. This
would not work if we return 0 before enabled.
Signed-off-by: BALATON Zoltan
---
hw/vmware_vga.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
v2: Rebase to apply to current
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 20f4fb8..5e4786f 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -39,8 +39,12 @@
struct vmsvga_state_s {
VGACommonState vga;
+/* -*- The members marked are now unused and could be removed but they are
+ * contained in the VMState thus need special handling. Maybe they could
+ * be removed the next time a new machine type is added.
+ */
int invalidated;
-int depth;
+int depth; /* -*- */
int enable;
int config;
struct {
@@ -58,7 +62,7 @@ struct vmsvga_state_s {
uint32_t guest;
uint32_t svgaid;
int syncing;
-int fb_size;
+int fb_size; /* -*- */
MemoryRegion fifo_ram;
uint8_t *fifo_ptr;
@@ -733,10 +737,10 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t
address)
return 0x0;
case SVGA_REG_VRAM_SIZE:
-return s->vga.vram_size;
+return s->vga.vram_size; /* No physical VRAM besides the framebuffer */
case SVGA_REG_FB_SIZE:
-return s->fb_size;
+return s->vga.vram_size;
case SVGA_REG_CAPABILITIES:
caps = SVGA_CAP_NONE;
@@ -821,7 +825,6 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
s->invalidated = 1;
s->vga.invalidate(&s->vga);
if (s->enable) {
-s->fb_size = ((s->depth + 7) >> 3) * s->new_width * s->new_height;
vga_dirty_log_stop(&s->vga);
} else {
vga_dirty_log_start(&s->vga);
--
1.7.10
[Qemu-devel] [PATCH 3/3 v2] vmware_vga: Allow simple drivers to work without using the fifo
Postpone stopping the dirty log to the point where the command fifo is
configured to allow drivers which don't use the fifo to work too.
(Without this the picture rendered into the vram never got to the
screen and the DIRECT_VRAM option meant to support this case was
removed a year ago.)
Signed-off-by: BALATON Zoltan
---
hw/vga.c|2 +-
hw/vga_int.h|1 +
hw/vmware_vga.c | 34 +-
3 files changed, 27 insertions(+), 10 deletions(-)
v2: Rebase to apply to current
diff --git a/hw/vga.c b/hw/vga.c
index 80299ea..7939a9d 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1612,7 +1612,7 @@ void vga_invalidate_scanlines(VGACommonState *s, int y1,
int y2)
}
}
-static void vga_sync_dirty_bitmap(VGACommonState *s)
+void vga_sync_dirty_bitmap(VGACommonState *s)
{
memory_region_sync_dirty_bitmap(&s->vram);
}
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 330a32f..c01d07d 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -196,6 +196,7 @@ MemoryRegion *vga_init_io(VGACommonState *s,
const MemoryRegionPortio **vbe_ports);
void vga_common_reset(VGACommonState *s);
+void vga_sync_dirty_bitmap(VGACommonState *s);
void vga_dirty_log_start(VGACommonState *s);
void vga_dirty_log_stop(VGACommonState *s);
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 5e4786f..17b3140 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -820,11 +820,10 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
break;
case SVGA_REG_ENABLE:
-s->enable = value;
-s->config &= !!value;
+s->enable = !!value;
s->invalidated = 1;
s->vga.invalidate(&s->vga);
-if (s->enable) {
+if (s->enable && s->config) {
vga_dirty_log_stop(&s->vga);
} else {
vga_dirty_log_start(&s->vga);
@@ -860,15 +859,19 @@ static void vmsvga_value_write(void *opaque, uint32_t
address, uint32_t value)
if (value) {
s->fifo = (uint32_t *) s->fifo_ptr;
/* Check range and alignment. */
-if ((CMD(min) | CMD(max) |
-CMD(next_cmd) | CMD(stop)) & 3)
+if ((CMD(min) | CMD(max) | CMD(next_cmd) | CMD(stop)) & 3) {
break;
-if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo)
+}
+if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo) {
break;
-if (CMD(max) > SVGA_FIFO_SIZE)
+}
+if (CMD(max) > SVGA_FIFO_SIZE) {
break;
-if (CMD(max) < CMD(min) + 10 * 1024)
+}
+if (CMD(max) < CMD(min) + 10 * 1024) {
break;
+}
+vga_dirty_log_stop(&s->vga);
}
s->config = !!value;
break;
@@ -949,6 +952,8 @@ static inline void vmsvga_check_size(struct vmsvga_state_s
*s)
static void vmsvga_update_display(void *opaque)
{
struct vmsvga_state_s *s = opaque;
+bool dirty = false;
+
if (!s->enable) {
s->vga.update(&s->vga);
return;
@@ -963,13 +968,24 @@ static void vmsvga_update_display(void *opaque)
* Is it more efficient to look at vram VGA-dirty bits or wait
* for the driver to issue SVGA_CMD_UPDATE?
*/
-if (s->invalidated) {
+if (memory_region_is_logging(&s->vga.vram)) {
+vga_sync_dirty_bitmap(&s->vga);
+dirty = memory_region_get_dirty(&s->vga.vram, 0,
+ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds),
+DIRTY_MEMORY_VGA);
+}
+if (s->invalidated || dirty) {
s->invalidated = 0;
memcpy(ds_get_data(s->vga.ds), s->vga.vram_ptr,
ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds));
dpy_update(s->vga.ds, 0, 0,
ds_get_width(s->vga.ds), ds_get_height(s->vga.ds));
}
+if (dirty) {
+memory_region_reset_dirty(&s->vga.vram, 0,
+ds_get_linesize(s->vga.ds) * ds_get_height(s->vga.ds),
+DIRTY_MEMORY_VGA);
+}
}
static void vmsvga_reset(DeviceState *dev)
--
1.7.10
[Qemu-devel] [PATCH 1/3 v2] vmware_vga: Cleanup and remove duplicated info from local state
Removed info from vmsvga_state that is available from elsewhere and
thus was duplicated here unnecessarily. Also includes some coding
style fixes suggested by checkpatch.pl.
Signed-off-by: BALATON Zoltan
---
console.h | 20 ++
hw/vmware_vga.c | 196 +++
2 files changed, 102 insertions(+), 114 deletions(-)
v2: Rebase to apply to current
diff --git a/console.h b/console.h
index f990684..3bcbc86 100644
--- a/console.h
+++ b/console.h
@@ -330,6 +330,26 @@ static inline int ds_get_bytes_per_pixel(DisplayState *ds)
return ds->surface->pf.bytes_per_pixel;
}
+static inline int ds_get_depth(DisplayState *ds)
+{
+return ds->surface->pf.depth;
+}
+
+static inline int ds_get_rmask(DisplayState *ds)
+{
+return ds->surface->pf.rmask;
+}
+
+static inline int ds_get_gmask(DisplayState *ds)
+{
+return ds->surface->pf.gmask;
+}
+
+static inline int ds_get_bmask(DisplayState *ds)
+{
+return ds->surface->pf.bmask;
+}
+
#ifdef CONFIG_CURSES
#include
typedef chtype console_ch_t;
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index b68e883..20f4fb8 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -32,16 +32,15 @@
#define HW_FILL_ACCEL
#define HW_MOUSE_ACCEL
-# include "vga_int.h"
+#include "vga_int.h"
+
+/* See http://vmware-svga.sf.net/ for some documentation on VMWare SVGA */
struct vmsvga_state_s {
VGACommonState vga;
-int width;
-int height;
int invalidated;
int depth;
-int bypp;
int enable;
int config;
struct {
@@ -58,9 +57,6 @@ struct vmsvga_state_s {
int new_height;
uint32_t guest;
uint32_t svgaid;
-uint32_t wred;
-uint32_t wgreen;
-uint32_t wblue;
int syncing;
int fb_size;
@@ -298,40 +294,33 @@ static inline void vmsvga_update_rect(struct
vmsvga_state_s *s,
uint8_t *src;
uint8_t *dst;
-if (x + w > s->width) {
+if (x + w > ds_get_width(s->vga.ds)) {
fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
-__FUNCTION__, x, w);
-x = MIN(x, s->width);
-w = s->width - x;
+__func__, x, w);
+x = MIN(x, ds_get_width(s->vga.ds));
+w = ds_get_width(s->vga.ds) - x;
}
-if (y + h > s->height) {
+if (y + h > ds_get_height(s->vga.ds)) {
fprintf(stderr, "%s: update height too large y: %d, h: %d\n",
-__FUNCTION__, y, h);
-y = MIN(y, s->height);
-h = s->height - y;
+__func__, y, h);
+y = MIN(y, ds_get_height(s->vga.ds));
+h = ds_get_height(s->vga.ds) - y;
}
-line = h;
-bypl = s->bypp * s->width;
-width = s->bypp * w;
-start = s->bypp * x + bypl * y;
+bypl = ds_get_linesize(s->vga.ds);
+width = ds_get_bytes_per_pixel(s->vga.ds) * w;
+start = ds_get_bytes_per_pixel(s->vga.ds) * x + bypl * y;
src = s->vga.vram_ptr + start;
dst = ds_get_data(s->vga.ds) + start;
-for (; line > 0; line --, src += bypl, dst += bypl)
+for (line = h; line > 0; line--, src += bypl, dst += bypl) {
memcpy(dst, src, width);
+}
dpy_update(s->vga.ds, x, y, w, h);
}
-static inline void vmsvga_update_screen(struct vmsvga_state_s *s)
-{
-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);
-}
-
static inline void vmsvga_update_rect_delayed(struct vmsvga_state_s *s,
int x, int y, int w, int h)
{
@@ -364,20 +353,21 @@ static inline void vmsvga_copy_rect(struct vmsvga_state_s
*s,
int x0, int y0, int x1, int y1, int w, int h)
{
uint8_t *vram = s->vga.vram_ptr;
-int bypl = s->bypp * s->width;
-int width = s->bypp * w;
+int bypl = ds_get_linesize(s->vga.ds);
+int bypp = ds_get_bytes_per_pixel(s->vga.ds);
+int width = bypp * w;
int line = h;
uint8_t *ptr[2];
if (y1 > y0) {
-ptr[0] = vram + s->bypp * x0 + bypl * (y0 + h - 1);
-ptr[1] = vram + s->bypp * x1 + bypl * (y1 + h - 1);
+ptr[0] = vram + bypp * x0 + bypl * (y0 + h - 1);
+ptr[1] = vram + 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;
+ptr[0] = vram + bypp * x0 + bypl * y0;
+ptr[1] = vram + bypp * x1 + bypl * y1;
for (; line > 0; line --, ptr[0] += bypl, ptr[1] += bypl) {
memmove(ptr[1], ptr[0], width);
}
@@ -391,13 +381,11 @@ static inline void vmsvga_copy
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] mac99: Change memory layout to better match PowerMac3, 1
Ping! <http://patchwork.ozlabs.org/patch/355103/> On Sat, 12 Apr 2014, BALATON Zoltan wrote: Bring the memory map closer to a PowerMac3,1 model by removing unused areas and adding the VGA and network cards after the macio to let the latter be mapped from 0x8000 like on real hardware. (On real hardware the graphics and network cards are on separate buses but we don't model that yet.) Signed-off-by: BALATON Zoltan --- This patch is intended to bring memory layout closer to what's seen in these dumps: http://nandra.segv.jp/NetBSD/G4.dump-device-tree.txt http://raveland.org/ports/eeprom.txt http://mail-index.netbsd.org/port-macppc/2007/10/24/.html https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=604134 v2: Added back unin2_memory region that Darwin seems to like better --- hw/pci-host/uninorth.c | 2 +- hw/ppc/mac_newworld.c | 14 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c index e72fe2a..21f805f 100644 --- a/hw/pci-host/uninorth.c +++ b/hw/pci-host/uninorth.c @@ -230,7 +230,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic, d = UNI_NORTH_PCI_HOST_BRIDGE(dev); memory_region_init(&d->pci_mmio, OBJECT(d), "pci-mmio", 0x1ULL); memory_region_init_alias(&d->pci_hole, OBJECT(d), "pci-hole", &d->pci_mmio, - 0x8000ULL, 0x7000ULL); + 0x8000ULL, 0x1000ULL); memory_region_add_subregion(address_space_mem, 0x8000ULL, &d->pci_hole); diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index 4bdaa8d..a4e5044 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -371,18 +371,11 @@ static void ppc_core99_init(MachineState *machine) machine_arch = ARCH_MAC99; } /* init basic PC hardware */ -pci_vga_init(pci_bus); - escc_mem = escc_init(0, pic[0x25], pic[0x24], serial_hds[0], serial_hds[1], ESCC_CLOCK, 4); memory_region_init_alias(escc_bar, NULL, "escc-bar", escc_mem, 0, memory_region_size(escc_mem)); -for(i = 0; i < nb_nics; i++) -pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); - -ide_drive_get(hd, MAX_IDE_BUS); - macio = pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO); dev = DEVICE(macio); qdev_connect_gpio_out(dev, 0, pic[0x19]); /* CUDA */ @@ -393,6 +386,8 @@ static void ppc_core99_init(MachineState *machine) macio_init(macio, pic_mem, escc_bar); /* We only emulate 2 out of 3 IDE controllers for now */ +ide_drive_get(hd, MAX_IDE_BUS); + macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio), "ide[0]")); macio_ide_init_drives(macio_ide, hd); @@ -418,9 +413,14 @@ static void ppc_core99_init(MachineState *machine) } } +pci_vga_init(pci_bus); + if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) graphic_depth = 15; +for(i = 0; i < nb_nics; i++) +pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); + /* The NewWorld NVRAM is not located in the MacIO device */ dev = qdev_create(NULL, TYPE_MACIO_NVRAM); qdev_prop_set_uint32(dev, "size", 0x2000); -- 1.8.1.5
Re: [Qemu-devel] Help needed testing on ppc
On Wed, 7 May 2014, Alexander Graf wrote: On 05/07/2014 05:31 PM, Tom Musta wrote: On 5/6/2014 6:17 PM, BALATON Zoltan wrote: On Tue, 6 May 2014, Tom Musta wrote: (2) Your patch makes some store instructions compliant with the most recent ISAs but there are many other instructions that are not addressed by the patch. I think fixing only some will be a future source of confusion.>> Alex: do you have an opinion on this? Are you OK with changing masks for a few stores but not all instructions in general? I would like to see someone just test all those load/store instructions on old CPUs and see whether they fault. If none faults, we should just be consistent and remove them for all. If say a 750 really only ignores the Rc bit for stwx for some reason we should just model it accordingly. To get some answers to this and other questions that are still open I've made a test program by stripping down yaboot and adding tests to it so that it should be possible to run from Open Firmware as a boot loader. It can be found here: http://goliat.eik.bme.hu/~balaton/oftest/ The files there are: * oftest - an ELF executable that you can put on some device OF can read and run it if it were a boot loader ( e.g. 0> boot hd0,0:\oftest ) * oftest.hfs.xz - the same file on an 800k HFS volume that can be put on e.g. a USB drive or CD then used as the previous one * oftest-src.tar.xz - the source When run from Open Firmware it should print some information about memory layout, MSR setting, stack location, BAT registers and test the stwx opcode with and without reserved bit which should help us understand better the differences between QEMU and real hardware. I could only test it on QEMU though. I'd appreciate if you could run it on real hardware and take a picture of the output screen which you upload somewhere and send the URL to that (or if you cannot upload you can send the picture but in that case only to me not to the list please). If cannot be seen on the picture please also include the model of your machine that should appear at the beginning of the Open Firmware greeting line. Regards, BALATON Zoltan
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] mac99: Change memory layout to better match PowerMac3, 1
Ping again. This patch already missed two pull requests without getting any comments. I hope you can look at it now. Regards, BALATON Zoltan On Mon, 9 Jun 2014, BALATON Zoltan wrote: Ping! <http://patchwork.ozlabs.org/patch/355103/> On Sat, 12 Apr 2014, BALATON Zoltan wrote: Bring the memory map closer to a PowerMac3,1 model by removing unused areas and adding the VGA and network cards after the macio to let the latter be mapped from 0x8000 like on real hardware. (On real hardware the graphics and network cards are on separate buses but we don't model that yet.) Signed-off-by: BALATON Zoltan --- This patch is intended to bring memory layout closer to what's seen in these dumps: http://nandra.segv.jp/NetBSD/G4.dump-device-tree.txt http://raveland.org/ports/eeprom.txt http://mail-index.netbsd.org/port-macppc/2007/10/24/.html https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=604134 v2: Added back unin2_memory region that Darwin seems to like better --- hw/pci-host/uninorth.c | 2 +- hw/ppc/mac_newworld.c | 14 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c index e72fe2a..21f805f 100644 --- a/hw/pci-host/uninorth.c +++ b/hw/pci-host/uninorth.c @@ -230,7 +230,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic, d = UNI_NORTH_PCI_HOST_BRIDGE(dev); memory_region_init(&d->pci_mmio, OBJECT(d), "pci-mmio", 0x1ULL); memory_region_init_alias(&d->pci_hole, OBJECT(d), "pci-hole", &d->pci_mmio, - 0x8000ULL, 0x7000ULL); + 0x8000ULL, 0x1000ULL); memory_region_add_subregion(address_space_mem, 0x8000ULL, &d->pci_hole); diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index 4bdaa8d..a4e5044 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -371,18 +371,11 @@ static void ppc_core99_init(MachineState *machine) machine_arch = ARCH_MAC99; } /* init basic PC hardware */ -pci_vga_init(pci_bus); - escc_mem = escc_init(0, pic[0x25], pic[0x24], serial_hds[0], serial_hds[1], ESCC_CLOCK, 4); memory_region_init_alias(escc_bar, NULL, "escc-bar", escc_mem, 0, memory_region_size(escc_mem)); -for(i = 0; i < nb_nics; i++) -pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); - -ide_drive_get(hd, MAX_IDE_BUS); - macio = pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO); dev = DEVICE(macio); qdev_connect_gpio_out(dev, 0, pic[0x19]); /* CUDA */ @@ -393,6 +386,8 @@ static void ppc_core99_init(MachineState *machine) macio_init(macio, pic_mem, escc_bar); /* We only emulate 2 out of 3 IDE controllers for now */ +ide_drive_get(hd, MAX_IDE_BUS); + macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio), "ide[0]")); macio_ide_init_drives(macio_ide, hd); @@ -418,9 +413,14 @@ static void ppc_core99_init(MachineState *machine) } } +pci_vga_init(pci_bus); + if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) graphic_depth = 15; +for(i = 0; i < nb_nics; i++) +pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); + /* The NewWorld NVRAM is not located in the MacIO device */ dev = qdev_create(NULL, TYPE_MACIO_NVRAM); qdev_prop_set_uint32(dev, "size", 0x2000); -- 1.8.1.5
Re: [Qemu-devel] Help needed testing on ppc
On Thu, 12 Jun 2014, BALATON Zoltan wrote: On Wed, 7 May 2014, Alexander Graf wrote: On 05/07/2014 05:31 PM, Tom Musta wrote: On 5/6/2014 6:17 PM, BALATON Zoltan wrote: On Tue, 6 May 2014, Tom Musta wrote: (2) Your patch makes some store instructions compliant with the most recent ISAs but there are many other instructions that are not addressed by the patch. I think fixing only some will be a future source of confusion.>> Alex: do you have an opinion on this? Are you OK with changing masks for a few stores but not all instructions in general? I would like to see someone just test all those load/store instructions on old CPUs and see whether they fault. If none faults, we should just be consistent and remove them for all. If say a 750 really only ignores the Rc bit for stwx for some reason we should just model it accordingly. To get some answers to this and other questions that are still open I've made a test program by stripping down yaboot and adding tests to it so that it should be possible to run from Open Firmware as a boot loader. It can be found here: http://goliat.eik.bme.hu/~balaton/oftest/ The files there are: * oftest - an ELF executable that you can put on some device OF can read and run it if it were a boot loader ( e.g. 0> boot hd0,0:\oftest ) * oftest.hfs.xz - the same file on an 800k HFS volume that can be put on e.g. a USB drive or CD then used as the previous one * oftest-src.tar.xz - the source When run from Open Firmware it should print some information about memory layout, MSR setting, stack location, BAT registers and test the stwx opcode with and without reserved bit which should help us understand better the differences between QEMU and real hardware. I could only test it on QEMU though. I've got some results (but more are welcome) which can be seen here: http://goliat.eik.bme.hu/~balaton/oftest/results/ The results show that the stwx instruction with reserved bit set does not change status bits and does not generate an exception on any CPU tested (G3 and G4) so it is most probably just ignored as we thought. Regards, BALATON Zoltan
Re: [Qemu-devel] [Qemu-ppc] Help needed testing on ppc
On Tue, 17 Jun 2014, Alexander Graf wrote: On 17.06.14 01:42, BALATON Zoltan wrote: On Thu, 12 Jun 2014, BALATON Zoltan wrote: On Wed, 7 May 2014, Alexander Graf wrote: On 05/07/2014 05:31 PM, Tom Musta wrote: On 5/6/2014 6:17 PM, BALATON Zoltan wrote: On Tue, 6 May 2014, Tom Musta wrote: (2) Your patch makes some store instructions compliant with the most recent ISAs but there are many other instructions that are not addressed by the patch. I think fixing only some will be a future source of confusion.>> Alex: do you have an opinion on this? Are you OK with changing masks for a few stores but not all instructions in general? I would like to see someone just test all those load/store instructions on old CPUs and see whether they fault. If none faults, we should just be consistent and remove them for all. If say a 750 really only ignores the Rc bit for stwx for some reason we should just model it accordingly. To get some answers to this and other questions that are still open I've made a test program by stripping down yaboot and adding tests to it so that it should be possible to run from Open Firmware as a boot loader. It can be found here: http://goliat.eik.bme.hu/~balaton/oftest/ The files there are: * oftest - an ELF executable that you can put on some device OF can read and run it if it were a boot loader ( e.g. 0> boot hd0,0:\oftest ) * oftest.hfs.xz - the same file on an 800k HFS volume that can be put on e.g. a USB drive or CD then used as the previous one * oftest-src.tar.xz - the source When run from Open Firmware it should print some information about memory layout, MSR setting, stack location, BAT registers and test the stwx opcode with and without reserved bit which should help us understand better the differences between QEMU and real hardware. I could only test it on QEMU though. I've got some results (but more are welcome) which can be seen here: http://goliat.eik.bme.hu/~balaton/oftest/results/ The results show that the stwx instruction with reserved bit set does not change status bits and does not generate an exception on any CPU tested (G3 and G4) so it is most probably just ignored as we thought. [adding qemu-ppc and tom to CC] Tom already commented on this. Is there a pattern that matches all the indexed load/store instructions or is stwx a one-off? Is this a question to whom? If to me I don't understand it. Regards, BALATON Zoltan
Re: [Qemu-devel] [PATCH v2] mac99: Change memory layout to better match PowerMac3, 1
On Tue, 17 Jun 2014, Alexander Graf wrote: On 12.04.14 11:20, BALATON Zoltan wrote: Bring the memory map closer to a PowerMac3,1 model by removing unused areas and adding the VGA and network cards after the macio to let the latter be mapped from 0x8000 like on real hardware. (On real hardware the graphics and network cards are on separate buses but we don't model that yet.) Signed-off-by: BALATON Zoltan --- This patch is intended to bring memory layout closer to what's seen in these dumps: http://nandra.segv.jp/NetBSD/G4.dump-device-tree.txt http://raveland.org/ports/eeprom.txt http://mail-index.netbsd.org/port-macppc/2007/10/24/.html https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=604134 v2: Added back unin2_memory region that Darwin seems to like better --- hw/pci-host/uninorth.c | 2 +- hw/ppc/mac_newworld.c | 14 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c index e72fe2a..21f805f 100644 --- a/hw/pci-host/uninorth.c +++ b/hw/pci-host/uninorth.c @@ -230,7 +230,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic, d = UNI_NORTH_PCI_HOST_BRIDGE(dev); memory_region_init(&d->pci_mmio, OBJECT(d), "pci-mmio", 0x1ULL); memory_region_init_alias(&d->pci_hole, OBJECT(d), "pci-hole", &d->pci_mmio, - 0x8000ULL, 0x7000ULL); + 0x8000ULL, 0x1000ULL); Doesn't OpenBIOS need to know about this change so it can update its device tree? No. memory_region_add_subregion(address_space_mem, 0x8000ULL, &d->pci_hole); diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index 4bdaa8d..a4e5044 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -371,18 +371,11 @@ static void ppc_core99_init(MachineState *machine) machine_arch = ARCH_MAC99; } /* init basic PC hardware */ -pci_vga_init(pci_bus); - escc_mem = escc_init(0, pic[0x25], pic[0x24], serial_hds[0], serial_hds[1], ESCC_CLOCK, 4); memory_region_init_alias(escc_bar, NULL, "escc-bar", escc_mem, 0, memory_region_size(escc_mem)); -for(i = 0; i < nb_nics; i++) -pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); - -ide_drive_get(hd, MAX_IDE_BUS); - macio = pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO); dev = DEVICE(macio); qdev_connect_gpio_out(dev, 0, pic[0x19]); /* CUDA */ @@ -393,6 +386,8 @@ static void ppc_core99_init(MachineState *machine) macio_init(macio, pic_mem, escc_bar); /* We only emulate 2 out of 3 IDE controllers for now */ +ide_drive_get(hd, MAX_IDE_BUS); + macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio), "ide[0]")); macio_ide_init_drives(macio_ide, hd); @@ -418,9 +413,14 @@ static void ppc_core99_init(MachineState *machine) } } +pci_vga_init(pci_bus); + if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) graphic_depth = 15; +for(i = 0; i < nb_nics; i++) +pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); + /* The NewWorld NVRAM is not located in the MacIO device */ dev = qdev_create(NULL, TYPE_MACIO_NVRAM); qdev_prop_set_uint32(dev, "size", 0x2000); I presume all the changes above only change the devfn ordering? It changes the addresses assigned to devices which is needed for MorphOS to work as it hardcodes the mmio locations of some (actually most) devices. Regards, BALATON Zoltan
Re: [Qemu-devel] [PATCH v2] mac99: Change memory layout to better match PowerMac3, 1
On Tue, 17 Jun 2014, Alexander Graf wrote: On 17.06.14 11:36, BALATON Zoltan wrote: On Tue, 17 Jun 2014, Alexander Graf wrote: On 12.04.14 11:20, BALATON Zoltan wrote: Bring the memory map closer to a PowerMac3,1 model by removing unused areas and adding the VGA and network cards after the macio to let the latter be mapped from 0x8000 like on real hardware. (On real hardware the graphics and network cards are on separate buses but we don't model that yet.) Signed-off-by: BALATON Zoltan --- This patch is intended to bring memory layout closer to what's seen in these dumps: http://nandra.segv.jp/NetBSD/G4.dump-device-tree.txt http://raveland.org/ports/eeprom.txt http://mail-index.netbsd.org/port-macppc/2007/10/24/.html https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=604134 v2: Added back unin2_memory region that Darwin seems to like better --- hw/pci-host/uninorth.c | 2 +- hw/ppc/mac_newworld.c | 14 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c index e72fe2a..21f805f 100644 --- a/hw/pci-host/uninorth.c +++ b/hw/pci-host/uninorth.c @@ -230,7 +230,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic, d = UNI_NORTH_PCI_HOST_BRIDGE(dev); memory_region_init(&d->pci_mmio, OBJECT(d), "pci-mmio", 0x1ULL); memory_region_init_alias(&d->pci_hole, OBJECT(d), "pci-hole", &d->pci_mmio, - 0x8000ULL, 0x7000ULL); + 0x8000ULL, 0x1000ULL); Doesn't OpenBIOS need to know about this change so it can update its device tree? No. We don't expose the pci-hole size in device tree? We do but already as 0x1000. See: http://tracker.coreboot.org/trac/openbios/browser/trunk/openbios-devel/arch/ppc/qemu/init.c memory_region_add_subregion(address_space_mem, 0x8000ULL, &d->pci_hole); diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index 4bdaa8d..a4e5044 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -371,18 +371,11 @@ static void ppc_core99_init(MachineState *machine) machine_arch = ARCH_MAC99; } /* init basic PC hardware */ -pci_vga_init(pci_bus); - escc_mem = escc_init(0, pic[0x25], pic[0x24], serial_hds[0], serial_hds[1], ESCC_CLOCK, 4); memory_region_init_alias(escc_bar, NULL, "escc-bar", escc_mem, 0, memory_region_size(escc_mem)); -for(i = 0; i < nb_nics; i++) -pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); - -ide_drive_get(hd, MAX_IDE_BUS); - macio = pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO); dev = DEVICE(macio); qdev_connect_gpio_out(dev, 0, pic[0x19]); /* CUDA */ @@ -393,6 +386,8 @@ static void ppc_core99_init(MachineState *machine) macio_init(macio, pic_mem, escc_bar); /* We only emulate 2 out of 3 IDE controllers for now */ +ide_drive_get(hd, MAX_IDE_BUS); + macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio), "ide[0]")); macio_ide_init_drives(macio_ide, hd); @@ -418,9 +413,14 @@ static void ppc_core99_init(MachineState *machine) } } +pci_vga_init(pci_bus); + if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) graphic_depth = 15; +for(i = 0; i < nb_nics; i++) +pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); + /* The NewWorld NVRAM is not located in the MacIO device */ dev = qdev_create(NULL, TYPE_MACIO_NVRAM); qdev_prop_set_uint32(dev, "size", 0x2000); I presume all the changes above only change the devfn ordering? It changes the addresses assigned to devices which is needed for MorphOS to work as it hardcodes the mmio locations of some (actually most) devices. I don't see how the ordering change here could possibly change MMIO locations for anything? It's how OpenBIOS assigns MMIO addresses to pci devices. It does it by going through them in order and map them starting from the base address (with some allingment). I guess you could look at drivers/pci.c I think it's in there somewhere. Regards, BALATON Zoltan
Re: [Qemu-devel] [Qemu-ppc] Help needed testing on ppc
On Tue, 17 Jun 2014, Alexander Graf wrote: http://goliat.eik.bme.hu/~balaton/oftest/results/ The results show that the stwx instruction with reserved bit set does not change status bits and does not generate an exception on any CPU tested (G3 and G4) so it is most probably just ignored as we thought. [adding qemu-ppc and tom to CC] Tom already commented on this. Is there a pattern that matches all the indexed load/store instructions or is stwx a one-off? Is this a question to whom? If to me I don't understand it. stwx is part of a group of instructions. It's very rare that hardware only shows certain behavior (like ignore a reserved bit) for single instructions. Usually it happens on complete groups. Who would know that? The test only tested stwx and I assume the same that this should not behave differently than any other instruction. Also this is the only instruction that was used with the set reserved bit in MorphOS and the patch ignoring reserved bits for this group of instructions that we discussed earlier seems to fix it. (There's another case with an altivec instruction with a similar failure but I did not look at that yet if that's a reserved bit too or something else.) As to why it's in MorphOS I don't know, I got no answer from them. Regards, BALATON Zoltan
Re: [Qemu-devel] [Qemu-ppc] Help needed testing on ppc
On Tue, 17 Jun 2014, Tom Musta wrote:
I am looking at the test case source code and do not see how you are
setting the reserved bit. Maybe I am missing some cleverness in how the
test is built?
Probably I should have written it more straight-forward but I wanted it to
be possible to change it for other tests easily so it's a bit tricky.
Basically I get the code location by a bl then fetching the link register:
asm volatile("mfcr %0 \n\t"
"bl 1f \n\t"
"mfcr %1 \n\t"
"mflr 10 \n\t"
and then set the bit with the next three lines after testing the normal
case:
"lwz %0, 36(10)\n\t"
"ori %0, %0, 1 \n\t"
"stw %0, 36(10)\n\t"
Then test again with the bit set:
"mfcr %0 \n\t"
"bl 1f \n\t"
"mfcr %2 \n\t"
and exit:
"b 2f \n\t"
"1: stwx %0, %4, %6\n\t" <<<<<<<<<<<<< just a normal
stwx, right?
"blr \n\t"
"2:\n\t"
: "=&r"(cr), "=&r"(cr1), "=&r"(cr2), "=m"(val)
: "r"(&val), "m"(val), "r"(8)
: "r8", "r9", "r10", "cc", "memory");
prom_printf("old cr (mem):\t%#x\n", val);
prom_printf("old cr (reg):\t%#x\n", cr);
prom_printf("new cr1 (reg):\t%#x\n", cr1);
prom_printf("new cr2 (reg):\t%#x\n", cr2);
}
But the objdump of your test binary does not show that it is set either:
It should show in a debugger the second time the stwx is called (it did
for me).
Regards,
BALATON Zoltan
Re: [Qemu-devel] [Qemu-ppc] Help needed testing on ppc
On Wed, 18 Jun 2014, Tom Musta wrote:
On 6/17/2014 10:17 AM, BALATON Zoltan wrote:
On Tue, 17 Jun 2014, Tom Musta wrote:
I am looking at the test case source code and do not see how you are setting
the reserved bit. Maybe I am missing some cleverness in how the test is built?
Probably I should have written it more straight-forward but I wanted it to be
possible to change it for other tests easily so it's a bit tricky. Basically I
get the code location by a bl then fetching the link register:
asm volatile("mfcr %0 \n\t"
"bl 1f \n\t"
"mfcr %1 \n\t"
"mflr 10 \n\t"
and then set the bit with the next three lines after testing the normal case:
"lwz %0, 36(10)\n\t"
"ori %0, %0, 1 \n\t"
"stw %0, 36(10)\n\t"
Then test again with the bit set:
"mfcr %0 \n\t"
"bl 1f \n\t"
"mfcr %2 \n\t"
and exit:
"b 2f \n\t"
"1: stwx %0, %4, %6\n\t" <<<<<<<<<<<<< just a normal
stwx, right?
"blr \n\t"
"2:\n\t"
: "=&r"(cr), "=&r"(cr1), "=&r"(cr2), "=m"(val)
: "r"(&val), "m"(val), "r"(8)
: "r8", "r9", "r10", "cc", "memory");
prom_printf("old cr (mem):\t%#x\n", val);
prom_printf("old cr (reg):\t%#x\n", cr);
prom_printf("new cr1 (reg):\t%#x\n", cr1);
prom_printf("new cr2 (reg):\t%#x\n", cr2);
}
But the objdump of your test binary does not show that it is set either:
It should show in a debugger the second time the stwx is called (it did for me).
There should be an icbi after the ori/stw sequence to ensure that the
modified code gets into the instruction cache.
I've corrected the test accordingly and rerun on iMac,1. It did not change
the stwx test results, the cr values are still the same.
Regards,
BALATON Zoltan
Re: [Qemu-devel] [PULL 075/118] macio: handle non-block ATAPI DMA transfers
On Fri, 20 Jun 2014, Mark Cave-Ayland wrote: Zoltan, please can you test the attached patch to see if this still allows MorphOS to boot? Unfortunately it seems MorphOS cannot boot with this patch. It hangs while trying to read the TOC from the CD. Debug output with DEBUG_MACIO and DEBUG_DBDMA enabled shows: DBDMA: writel 0x0d0c <= 0x00e51970 DBDMA: channel 0x1a reg 0x3 DBDMA: dbdma_cmdptr_load 0x00e51970 DBDMA: DBDMA_run_bh DBDMA: writel 0x0d00 <= 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: status 0x8400 DBDMA: readl 0x0d00 => 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: DBDMA_run_bh DBDMA: channel_run dbdma_cmd 0x7f0997120f28 req_count 0x0324 command 0x3000 phy_addr 0x00e7b0bc cmd_dep 0x res_count 0x xfer_status 0x DBDMA: start_input DBDMA: addr 0xe7b0bc key 0x0 non-block ATAPI DMA transfer size: 804 io_buffer_size = 0 remainder: 0 io->len: 0 size: 20 end of DMA done DMA DBDMA: dbdma_end DBDMA: conditional_wait DBDMA: dbdma_cmdptr_save 0x00e51970 DBDMA: xfer_status 0x8400 res_count 0x DBDMA: conditional_interrupt DBDMA: conditional_branch DBDMA: dbdma_cmdptr_load 0x00e51980 DBDMA: channel_run dbdma_cmd 0x7f0997120f28 req_count 0x command 0x7000 phy_addr 0x cmd_dep 0x res_count 0x xfer_status 0x and no further ide activity from here whereas without the patch when it boots I see these logs: DBDMA: writel 0x0d0c <= 0x00e50090 DBDMA: channel 0x1a reg 0x3 DBDMA: dbdma_cmdptr_load 0x00e50090 DBDMA: DBDMA_run_bh DBDMA: writel 0x0d00 <= 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: status 0x8400 DBDMA: readl 0x0d00 => 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: DBDMA_run_bh DBDMA: channel_run dbdma_cmd 0x7f56695a7f28 req_count 0x0324 command 0x3000 phy_addr 0x00e4f8fc cmd_dep 0x res_count 0x xfer_status 0x DBDMA: start_input DBDMA: addr 0xe4f8fc key 0x0 non-block ATAPI DMA transfer size: 20 end of non-block ATAPI DMA transfer DBDMA: dbdma_end DBDMA: conditional_wait DBDMA: dbdma_cmdptr_save 0x00e50090 DBDMA: xfer_status 0x8400 res_count 0x0324 DBDMA: conditional_interrupt DBDMA: conditional_branch DBDMA: dbdma_cmdptr_load 0x00e500a0 DBDMA: channel_run dbdma_cmd 0x7f56695a7f28 req_count 0x command 0x7000 phy_addr 0x cmd_dep 0x res_count 0x xfer_status 0x DBDMA: writel 0x0d00 <= 0x9800 DBDMA: channel 0x1a reg 0x0 DBDMA: status 0x DBDMA: writel 0x0d0c <= 0x00e50090 DBDMA: channel 0x1a reg 0x3 DBDMA: dbdma_cmdptr_load 0x00e50090 DBDMA: writel 0x0d00 <= 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: status 0x8400 DBDMA: readl 0x0d00 => 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: DBDMA_run_bh DBDMA: channel_run dbdma_cmd 0x7f56695a7f28 req_count 0x0800 command 0x3000 phy_addr 0x00e8d7c0 cmd_dep 0x res_count 0x xfer_status 0x DBDMA: start_input DBDMA: addr 0xe8d7c0 key 0x0 io_buffer_size = 0 remainder: 0 io->len: 2048 size: 2048 io->len = 0x800 set remainder to: 0 sector_num=64 size=2048, cmd_cmd=0 io_buffer_size = 0x800 remainder: 0 io->len: 0 size: 0 end of transfer end of DMA done DMA DBDMA: dbdma_end DBDMA: conditional_wait DBDMA: dbdma_cmdptr_save 0x00e50090 DBDMA: xfer_status 0x8400 res_count 0x DBDMA: conditional_interrupt DBDMA: conditional_branch DBDMA: dbdma_cmdptr_load 0x00e500a0 DBDMA: channel_run dbdma_cmd 0x7f56695a7f28 req_count 0x command 0x7000 phy_addr 0x cmd_dep 0x res_count 0x xfer_status 0x and a lot of similar stuff after this. If this is not enough to understand the problem and you need more details please tell me what to look for. Regards, BALATON Zoltan
Re: [Qemu-devel] [PULL 075/118] macio: handle non-block ATAPI DMA transfers
_branch DBDMA: dbdma_cmdptr_load 0x00e89cf0 DBDMA: channel_run dbdma_cmd 0x7fc4dcef2228 req_count 0x command 0x7000 phy_addr 0x cmd_dep 0x res_count 0x xfer_status 0x DBDMA: writel 0x0d00 <= 0x9800 DBDMA: channel 0x1a reg 0x0 DBDMA: status 0x DBDMA: writel 0x0d0c <= 0x00e89ce0 DBDMA: channel 0x1a reg 0x3 DBDMA: dbdma_cmdptr_load 0x00e89ce0 ATAPI limit=0x8000 packet: 28 00 00 00 00 10 00 00 01 00 00 00 read dma: LBA=16 nb_sectors=1 The only difference in the common part I can see is the different transfer size. I'm afraid as you're the only person that can boot MorphOS this far then we need you to diagnose and suggest a suitable alternative by comparing the before and after output. Since MacOS is already a supported client then if no solution can be found then it is likely that this patch will be reverted :( Unfortunately I can't make sense of the mess in macio.c so I cannot suggest a solution as I don't understand what's happening and how should it work. I can help in testing and trying things you tell me to do but I cannot solve it by myself. (It would probably help if the unaligned hacks could be removed if this is already possible with the current block layer as that should simplify the code so it could be understood. I can't do that either though as I don't know what needs to be done for this.) I wish more people could boot MorphOS by now but that still needs the patches I've sent for OpenBIOS and QEMU that are not merged yet and there's also the problem with the MMU exceptions that we don't have a solution for yet. Regards, BALATON Zoltan
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] mac99: Change memory layout to better match PowerMac3, 1
Ping! On Tue, 17 Jun 2014, BALATON Zoltan wrote: On Tue, 17 Jun 2014, Alexander Graf wrote: On 17.06.14 11:36, BALATON Zoltan wrote: On Tue, 17 Jun 2014, Alexander Graf wrote: On 12.04.14 11:20, BALATON Zoltan wrote: Bring the memory map closer to a PowerMac3,1 model by removing unused areas and adding the VGA and network cards after the macio to let the latter be mapped from 0x8000 like on real hardware. (On real hardware the graphics and network cards are on separate buses but we don't model that yet.) Signed-off-by: BALATON Zoltan --- This patch is intended to bring memory layout closer to what's seen in these dumps: http://nandra.segv.jp/NetBSD/G4.dump-device-tree.txt http://raveland.org/ports/eeprom.txt http://mail-index.netbsd.org/port-macppc/2007/10/24/.html https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=604134 v2: Added back unin2_memory region that Darwin seems to like better --- hw/pci-host/uninorth.c | 2 +- hw/ppc/mac_newworld.c | 14 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c index e72fe2a..21f805f 100644 --- a/hw/pci-host/uninorth.c +++ b/hw/pci-host/uninorth.c @@ -230,7 +230,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic, d = UNI_NORTH_PCI_HOST_BRIDGE(dev); memory_region_init(&d->pci_mmio, OBJECT(d), "pci-mmio", 0x1ULL); memory_region_init_alias(&d->pci_hole, OBJECT(d), "pci-hole", &d->pci_mmio, - 0x8000ULL, 0x7000ULL); + 0x8000ULL, 0x1000ULL); Doesn't OpenBIOS need to know about this change so it can update its device tree? No. We don't expose the pci-hole size in device tree? We do but already as 0x1000. See: http://tracker.coreboot.org/trac/openbios/browser/trunk/openbios-devel/arch/ppc/qemu/init.c memory_region_add_subregion(address_space_mem, 0x8000ULL, &d->pci_hole); diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index 4bdaa8d..a4e5044 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -371,18 +371,11 @@ static void ppc_core99_init(MachineState *machine) machine_arch = ARCH_MAC99; } /* init basic PC hardware */ -pci_vga_init(pci_bus); - escc_mem = escc_init(0, pic[0x25], pic[0x24], serial_hds[0], serial_hds[1], ESCC_CLOCK, 4); memory_region_init_alias(escc_bar, NULL, "escc-bar", escc_mem, 0, memory_region_size(escc_mem)); -for(i = 0; i < nb_nics; i++) -pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); - -ide_drive_get(hd, MAX_IDE_BUS); - macio = pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO); dev = DEVICE(macio); qdev_connect_gpio_out(dev, 0, pic[0x19]); /* CUDA */ @@ -393,6 +386,8 @@ static void ppc_core99_init(MachineState *machine) macio_init(macio, pic_mem, escc_bar); /* We only emulate 2 out of 3 IDE controllers for now */ +ide_drive_get(hd, MAX_IDE_BUS); + macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio), "ide[0]")); macio_ide_init_drives(macio_ide, hd); @@ -418,9 +413,14 @@ static void ppc_core99_init(MachineState *machine) } } +pci_vga_init(pci_bus); + if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) graphic_depth = 15; +for(i = 0; i < nb_nics; i++) +pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); + /* The NewWorld NVRAM is not located in the MacIO device */ dev = qdev_create(NULL, TYPE_MACIO_NVRAM); qdev_prop_set_uint32(dev, "size", 0x2000); I presume all the changes above only change the devfn ordering? It changes the addresses assigned to devices which is needed for MorphOS to work as it hardcodes the mmio locations of some (actually most) devices. I don't see how the ordering change here could possibly change MMIO locations for anything? It's how OpenBIOS assigns MMIO addresses to pci devices. It does it by going through them in order and map them starting from the base address (with some allingment). I guess you could look at drivers/pci.c I think it's in there somewhere. Regards, BALATON Zoltan
Re: [Qemu-devel] [PATCH v2] mac99: Change memory layout to better match PowerMac3, 1
On Mon, 23 Jun 2014, Alexander Graf wrote: On 17.06.14 12:24, BALATON Zoltan wrote: On Tue, 17 Jun 2014, Alexander Graf wrote: On 17.06.14 11:36, BALATON Zoltan wrote: On Tue, 17 Jun 2014, Alexander Graf wrote: On 12.04.14 11:20, BALATON Zoltan wrote: Bring the memory map closer to a PowerMac3,1 model by removing unused areas and adding the VGA and network cards after the macio to let the latter be mapped from 0x8000 like on real hardware. (On real hardware the graphics and network cards are on separate buses but we don't model that yet.) Signed-off-by: BALATON Zoltan --- This patch is intended to bring memory layout closer to what's seen in these dumps: http://nandra.segv.jp/NetBSD/G4.dump-device-tree.txt http://raveland.org/ports/eeprom.txt http://mail-index.netbsd.org/port-macppc/2007/10/24/.html https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=604134 v2: Added back unin2_memory region that Darwin seems to like better --- hw/pci-host/uninorth.c | 2 +- hw/ppc/mac_newworld.c | 14 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c index e72fe2a..21f805f 100644 --- a/hw/pci-host/uninorth.c +++ b/hw/pci-host/uninorth.c @@ -230,7 +230,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic, d = UNI_NORTH_PCI_HOST_BRIDGE(dev); memory_region_init(&d->pci_mmio, OBJECT(d), "pci-mmio", 0x1ULL); memory_region_init_alias(&d->pci_hole, OBJECT(d), "pci-hole", &d->pci_mmio, - 0x8000ULL, 0x7000ULL); + 0x8000ULL, 0x1000ULL); Doesn't OpenBIOS need to know about this change so it can update its device tree? No. We don't expose the pci-hole size in device tree? We do but already as 0x1000. See: http://tracker.coreboot.org/trac/openbios/browser/trunk/openbios-devel/arch/ppc/qemu/init.c Ugh. Please send a single patch that fixes this with a proper patch description indicating that OpenBIOS already exposes the pci hole with 0x1000 size. Sent to the list. memory_region_add_subregion(address_space_mem, 0x8000ULL, &d->pci_hole); diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index 4bdaa8d..a4e5044 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -371,18 +371,11 @@ static void ppc_core99_init(MachineState *machine) machine_arch = ARCH_MAC99; } /* init basic PC hardware */ -pci_vga_init(pci_bus); - escc_mem = escc_init(0, pic[0x25], pic[0x24], serial_hds[0], serial_hds[1], ESCC_CLOCK, 4); memory_region_init_alias(escc_bar, NULL, "escc-bar", escc_mem, 0, memory_region_size(escc_mem)); -for(i = 0; i < nb_nics; i++) -pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); - -ide_drive_get(hd, MAX_IDE_BUS); - macio = pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO); dev = DEVICE(macio); qdev_connect_gpio_out(dev, 0, pic[0x19]); /* CUDA */ @@ -393,6 +386,8 @@ static void ppc_core99_init(MachineState *machine) macio_init(macio, pic_mem, escc_bar); /* We only emulate 2 out of 3 IDE controllers for now */ +ide_drive_get(hd, MAX_IDE_BUS); + macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio), "ide[0]")); macio_ide_init_drives(macio_ide, hd); @@ -418,9 +413,14 @@ static void ppc_core99_init(MachineState *machine) } } +pci_vga_init(pci_bus); + if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) graphic_depth = 15; +for(i = 0; i < nb_nics; i++) +pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); + /* The NewWorld NVRAM is not located in the MacIO device */ dev = qdev_create(NULL, TYPE_MACIO_NVRAM); qdev_prop_set_uint32(dev, "size", 0x2000); I presume all the changes above only change the devfn ordering? It changes the addresses assigned to devices which is needed for MorphOS to work as it hardcodes the mmio locations of some (actually most) devices. I don't see how the ordering change here could possibly change MMIO locations for anything? It's how OpenBIOS assigns MMIO addresses to pci devices. It does it by going through them in order and map them starting from the base address (with some allingment). I guess you could look at drivers/pci.c I think it's in there somewhere. I think it'd make more sense to just bolt the PCI devices to their respective devfns that they also have on real hardware. Depending on ordering magic that happens to give us different BAR maps by firmware doesn't really give me a lot of confidence. I don't understand what you mean. I don't want to rewrite P
[Qemu-devel] [PATCH] uninorth: Fix PCI hole size
Fix PCI hole size to match that what is found on real hardware. (OpenBIOS already uses the correct length.) Signed-off-by: BALATON Zoltan --- hw/pci-host/uninorth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c index e72fe2a..21f805f 100644 --- a/hw/pci-host/uninorth.c +++ b/hw/pci-host/uninorth.c @@ -230,7 +230,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic, d = UNI_NORTH_PCI_HOST_BRIDGE(dev); memory_region_init(&d->pci_mmio, OBJECT(d), "pci-mmio", 0x1ULL); memory_region_init_alias(&d->pci_hole, OBJECT(d), "pci-hole", &d->pci_mmio, - 0x8000ULL, 0x7000ULL); + 0x8000ULL, 0x1000ULL); memory_region_add_subregion(address_space_mem, 0x8000ULL, &d->pci_hole); -- 1.8.1.5
Re: [Qemu-devel] [PULL 075/118] macio: handle non-block ATAPI DMA transfers
On Mon, 23 Jun 2014, Alexander Graf wrote: On 20.06.14 21:27, Mark Cave-Ayland wrote: On 20/06/14 20:17, BALATON Zoltan wrote: On Fri, 20 Jun 2014, Mark Cave-Ayland wrote: Zoltan, please can you test the attached patch to see if this still allows MorphOS to boot? Unfortunately it seems MorphOS cannot boot with this patch. It hangs while trying to read the TOC from the CD. Debug output with DEBUG_MACIO and DEBUG_DBDMA enabled shows: And also with ATAPI debugging enabled? I suspect the problem is with the interaction between the DMA/ATAPI systems again. DBDMA: writel 0x0d0c <= 0x00e51970 DBDMA: channel 0x1a reg 0x3 DBDMA: dbdma_cmdptr_load 0x00e51970 DBDMA: DBDMA_run_bh DBDMA: writel 0x0d00 <= 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: status 0x8400 DBDMA: readl 0x0d00 => 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: DBDMA_run_bh DBDMA: channel_run dbdma_cmd 0x7f0997120f28 req_count 0x0324 command 0x3000 phy_addr 0x00e7b0bc cmd_dep 0x res_count 0x xfer_status 0x DBDMA: start_input DBDMA: addr 0xe7b0bc key 0x0 non-block ATAPI DMA transfer size: 804 io_buffer_size = 0 remainder: 0 io->len: 0 size: 20 end of DMA done DMA DBDMA: dbdma_end DBDMA: conditional_wait DBDMA: dbdma_cmdptr_save 0x00e51970 DBDMA: xfer_status 0x8400 res_count 0x DBDMA: conditional_interrupt DBDMA: conditional_branch DBDMA: dbdma_cmdptr_load 0x00e51980 DBDMA: channel_run dbdma_cmd 0x7f0997120f28 req_count 0x command 0x7000 phy_addr 0x cmd_dep 0x res_count 0x xfer_status 0x and no further ide activity from here whereas without the patch when it boots I see these logs: DBDMA: writel 0x0d0c <= 0x00e50090 DBDMA: channel 0x1a reg 0x3 DBDMA: dbdma_cmdptr_load 0x00e50090 DBDMA: DBDMA_run_bh DBDMA: writel 0x0d00 <= 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: status 0x8400 DBDMA: readl 0x0d00 => 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: DBDMA_run_bh DBDMA: channel_run dbdma_cmd 0x7f56695a7f28 req_count 0x0324 command 0x3000 phy_addr 0x00e4f8fc cmd_dep 0x res_count 0x xfer_status 0x DBDMA: start_input DBDMA: addr 0xe4f8fc key 0x0 non-block ATAPI DMA transfer size: 20 end of non-block ATAPI DMA transfer DBDMA: dbdma_end DBDMA: conditional_wait DBDMA: dbdma_cmdptr_save 0x00e50090 DBDMA: xfer_status 0x8400 res_count 0x0324 DBDMA: conditional_interrupt DBDMA: conditional_branch DBDMA: dbdma_cmdptr_load 0x00e500a0 DBDMA: channel_run dbdma_cmd 0x7f56695a7f28 req_count 0x command 0x7000 phy_addr 0x cmd_dep 0x res_count 0x xfer_status 0x DBDMA: writel 0x0d00 <= 0x9800 DBDMA: channel 0x1a reg 0x0 DBDMA: status 0x DBDMA: writel 0x0d0c <= 0x00e50090 DBDMA: channel 0x1a reg 0x3 DBDMA: dbdma_cmdptr_load 0x00e50090 DBDMA: writel 0x0d00 <= 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: status 0x8400 DBDMA: readl 0x0d00 => 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: DBDMA_run_bh DBDMA: channel_run dbdma_cmd 0x7f56695a7f28 req_count 0x0800 command 0x3000 phy_addr 0x00e8d7c0 cmd_dep 0x res_count 0x xfer_status 0x DBDMA: start_input DBDMA: addr 0xe8d7c0 key 0x0 io_buffer_size = 0 remainder: 0 io->len: 2048 size: 2048 io->len = 0x800 set remainder to: 0 sector_num=64 size=2048, cmd_cmd=0 io_buffer_size = 0x800 remainder: 0 io->len: 0 size: 0 end of transfer end of DMA done DMA DBDMA: dbdma_end DBDMA: conditional_wait DBDMA: dbdma_cmdptr_save 0x00e50090 DBDMA: xfer_status 0x8400 res_count 0x DBDMA: conditional_interrupt DBDMA: conditional_branch DBDMA: dbdma_cmdptr_load 0x00e500a0 DBDMA: channel_run dbdma_cmd 0x7f56695a7f28 req_count 0x command 0x7000 phy_addr 0x cmd_dep 0x res_count 0x xfer_status 0x and a lot of similar stuff after this. If this is not enough to understand the problem and you need more details please tell me what to look for. I'm afraid as you're the only person that can boot MorphOS this far then we need you to diagnose and suggest a suitable alternative by comparing the before and after output. Since MacOS is already a supported client then if no solution can be found then it is likely that this patch will be reverted :( So should I revert the patch for now? We're already in soft freeze. It would be nicer if it could be fixed instead of reverting. You could help detangling the macio.c code for a start. Regards, BALATON Zoltan
Re: [Qemu-devel] [PULL 075/118] macio: handle non-block ATAPI DMA transfers
On Fri, 20 Jun 2014, Mark Cave-Ayland wrote: On 04/06/14 13:44, Alexander Graf wrote: From: Mark Cave-Ayland Currently the macio DMA routines assume that all DMA requests are for read/write block transfers. This is not always the case for ATAPI, for example when requesting a TOC where the response is generated directly in the IDE buffer. Detect these non-block ATAPI DMA transfers (where no lba is specified in the command) and copy the results directly into RAM as indicated by the DBDMA descriptor. This fixes CDROM access under MorphOS. Signed-off-by: Mark Cave-Ayland Signed-off-by: Alexander Graf I've just done a complete round of OpenBIOS tests and it looks as if this patch introduces random hang failures (60% or so) into my Darwin image boot tests. The issue seems to be related as to the timing of the DMA callback relative to reception of the IDE command; if the IDE transfer is invoked first (as happens in Darwin) then we hang after sending the IDE command which is likely because the DMA is not being completed correctly. There is also another difference with MorphOS in that when the IDE transfer callback is invoked first then s->packet_transfer_size == 0 and so the memory copy would always copy 0 bytes. After some experimentation, I've come up with the attached patch which should retain the DMA memory copy, but instead invokes a further round of pmac_ide_atapi_transfer_cb() with io->len == 0 and s->io_buffer_size == 0 which should terminate the DMA request correctly. At the very least it seems to fix the hang on boot with my Darwin images. Zoltan, please can you test the attached patch to see if this still allows MorphOS to boot? Also this patch seems to slow down Finnix boot very much. Regards, BALATON Zoltan
[Qemu-devel] [PATCH] mac99: Add motherboard devices before PCI cards
Change the order of creating devices for New World Mac emulation so that devices on the motherboard are added first and PCI cards (VGA and NIC) come later. As a side effect, this also causes OpenBIOS to map the motherboard devices into the MMIO space to the same addresses as on real hardware and allow clients that hardcode these addresses (e.g. MorphOS) to find and use them until OpenBIOS is tought to map devices to specific addresses. (On real hardware the graphics and network cards are really on separate buses but we don't model that yet.) This brings the memory map closer to what is found on PowerMac3,1. Signed-off-by: BALATON Zoltan --- hw/ppc/mac_newworld.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index e493dc1..1a1e305 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -373,18 +373,11 @@ static void ppc_core99_init(MachineState *machine) machine_arch = ARCH_MAC99; } /* init basic PC hardware */ -pci_vga_init(pci_bus); - escc_mem = escc_init(0, pic[0x25], pic[0x24], serial_hds[0], serial_hds[1], ESCC_CLOCK, 4); memory_region_init_alias(escc_bar, NULL, "escc-bar", escc_mem, 0, memory_region_size(escc_mem)); -for(i = 0; i < nb_nics; i++) -pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); - -ide_drive_get(hd, MAX_IDE_BUS); - macio = pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO); dev = DEVICE(macio); qdev_connect_gpio_out(dev, 0, pic[0x19]); /* CUDA */ @@ -395,6 +388,8 @@ static void ppc_core99_init(MachineState *machine) macio_init(macio, pic_mem, escc_bar); /* We only emulate 2 out of 3 IDE controllers for now */ +ide_drive_get(hd, MAX_IDE_BUS); + macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio), "ide[0]")); macio_ide_init_drives(macio_ide, hd); @@ -420,9 +415,14 @@ static void ppc_core99_init(MachineState *machine) } } +pci_vga_init(pci_bus); + if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) graphic_depth = 15; +for(i = 0; i < nb_nics; i++) +pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); + /* The NewWorld NVRAM is not located in the MacIO device */ dev = qdev_create(NULL, TYPE_MACIO_NVRAM); qdev_prop_set_uint32(dev, "size", 0x2000); -- 1.8.1.5
Re: [Qemu-devel] [PATCH v2] mac99: Change memory layout to better match PowerMac3, 1
On Mon, 23 Jun 2014, Mark Cave-Ayland wrote: On 23/06/14 20:25, BALATON Zoltan wrote: It's how OpenBIOS assigns MMIO addresses to pci devices. It does it by going through them in order and map them starting from the base address (with some allingment). I guess you could look at drivers/pci.c I think it's in there somewhere. I think it'd make more sense to just bolt the PCI devices to their respective devfns that they also have on real hardware. Depending on ordering magic that happens to give us different BAR maps by firmware doesn't really give me a lot of confidence. I don't understand what you mean. I don't want to rewrite PCI handling code in OpenBIOS as that would have a higher chance of breaking something else (OpenBIOS is used by other archs as well). Also it would require more knowledge about the emulated hardware in OpenBIOS while it aims to be a generic implementation and wants to reduce special cases already in it. So I don't see a cleaner and easy way to do this. If I'm missing something please tell me. On the other hand you've said before that the mac99 machine is mostly a hack to be enough that some OS-es can run with it. Why reordering some devices to get the right BAR maps not fit in this hack? Since the MMIO address is calculated by rounding up to the next start address based on region size (where size tends to be more static compared to start address) then I'd be okay with this as a short term hack. Thanks. I've sent the other half of the patch then, that could be taken until a better solution will be available (it does not have to be reverted then either). Regards, BALATON Zoltan
[Qemu-devel] [PATCH v2] mac99: Add motherboard devices before PCI cards
Change the order of creating devices for New World Mac emulation so
that devices on the motherboard are added first and PCI cards (VGA and
NIC) come later. As a side effect, this also causes OpenBIOS to map
the motherboard devices into the MMIO space to the same addresses as
on real hardware and allow clients that hardcode these addresses (e.g.
MorphOS) to find and use them until OpenBIOS is tought to map devices
to specific addresses. (On real hardware the graphics and network
cards are really on separate buses but we don't model that yet.) This
brings the memory map closer to what is found on PowerMac3,1.
Signed-off-by: BALATON Zoltan
---
v2: style fixes suggested by checkpatch.pl
---
hw/ppc/mac_newworld.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index e493dc1..89d3cad 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -373,18 +373,11 @@ static void ppc_core99_init(MachineState *machine)
machine_arch = ARCH_MAC99;
}
/* init basic PC hardware */
-pci_vga_init(pci_bus);
-
escc_mem = escc_init(0, pic[0x25], pic[0x24],
serial_hds[0], serial_hds[1], ESCC_CLOCK, 4);
memory_region_init_alias(escc_bar, NULL, "escc-bar",
escc_mem, 0, memory_region_size(escc_mem));
-for(i = 0; i < nb_nics; i++)
-pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL);
-
-ide_drive_get(hd, MAX_IDE_BUS);
-
macio = pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO);
dev = DEVICE(macio);
qdev_connect_gpio_out(dev, 0, pic[0x19]); /* CUDA */
@@ -395,6 +388,8 @@ static void ppc_core99_init(MachineState *machine)
macio_init(macio, pic_mem, escc_bar);
/* We only emulate 2 out of 3 IDE controllers for now */
+ide_drive_get(hd, MAX_IDE_BUS);
+
macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio),
"ide[0]"));
macio_ide_init_drives(macio_ide, hd);
@@ -420,8 +415,15 @@ static void ppc_core99_init(MachineState *machine)
}
}
-if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8)
+pci_vga_init(pci_bus);
+
+if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) {
graphic_depth = 15;
+}
+
+for (i = 0; i < nb_nics; i++) {
+pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL);
+}
/* The NewWorld NVRAM is not located in the MacIO device */
dev = qdev_create(NULL, TYPE_MACIO_NVRAM);
--
1.8.1.5
Re: [Qemu-devel] [Qemu-ppc] [PATCH] mac99: Add motherboard devices before PCI cards
On Mon, 23 Jun 2014, Mark Cave-Ayland wrote: On 23/06/14 23:03, BALATON Zoltan wrote: Change the order of creating devices for New World Mac emulation so that devices on the motherboard are added first and PCI cards (VGA and NIC) come later. As a side effect, this also causes OpenBIOS to map the motherboard devices into the MMIO space to the same addresses as on real hardware and allow clients that hardcode these addresses (e.g. MorphOS) to find and use them until OpenBIOS is tought to map devices to specific addresses. (On real hardware the graphics and network cards are really on separate buses but we don't model that yet.) This brings the memory map closer to what is found on PowerMac3,1. Signed-off-by: BALATON Zoltan --- hw/ppc/mac_newworld.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index e493dc1..1a1e305 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -373,18 +373,11 @@ static void ppc_core99_init(MachineState *machine) machine_arch = ARCH_MAC99; } /* init basic PC hardware */ -pci_vga_init(pci_bus); - escc_mem = escc_init(0, pic[0x25], pic[0x24], serial_hds[0], serial_hds[1], ESCC_CLOCK, 4); memory_region_init_alias(escc_bar, NULL, "escc-bar", escc_mem, 0, memory_region_size(escc_mem)); -for(i = 0; i < nb_nics; i++) -pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); - -ide_drive_get(hd, MAX_IDE_BUS); - macio = pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO); dev = DEVICE(macio); qdev_connect_gpio_out(dev, 0, pic[0x19]); /* CUDA */ @@ -395,6 +388,8 @@ static void ppc_core99_init(MachineState *machine) macio_init(macio, pic_mem, escc_bar); /* We only emulate 2 out of 3 IDE controllers for now */ +ide_drive_get(hd, MAX_IDE_BUS); + macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio), "ide[0]")); macio_ide_init_drives(macio_ide, hd); @@ -420,9 +415,14 @@ static void ppc_core99_init(MachineState *machine) } } +pci_vga_init(pci_bus); + if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) graphic_depth = 15; Missing braces here. +for(i = 0; i < nb_nics; i++) +pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); + And here too. /* The NewWorld NVRAM is not located in the MacIO device */ dev = qdev_create(NULL, TYPE_MACIO_NVRAM); qdev_prop_set_uint32(dev, "size", 0x2000); Generally the rule with QEMU is that as you change parts of the code that haven't been touched for a while, you should update them to meet the new style guidelines. Did you run the diffs through scripts/checkpatch.pl at all as that should catch styling points like this? Not this one as I haven't expected it to break anything and I did not expect it to be broken. Sent a v2. Regards, BALATON Zoltan
Re: [Qemu-devel] [PULL 075/118] macio: handle non-block ATAPI DMA transfers
On Mon, 23 Jun 2014, Mark Cave-Ayland wrote: On 23/06/14 20:26, BALATON Zoltan wrote: (add Kevin to CC) I'm afraid as you're the only person that can boot MorphOS this far then we need you to diagnose and suggest a suitable alternative by comparing the before and after output. Since MacOS is already a supported client then if no solution can be found then it is likely that this patch will be reverted :( So should I revert the patch for now? We're already in soft freeze. Well let's see if Zoltan can make any headway with debugging over the next few days; if there's no progress by the weekend then sadly my recommendation would be to revert in time for -rc0 as this definitely causes intermittent boot failures in Darwin for me. It would be nicer if it could be fixed instead of reverting. You could help detangling the macio.c code for a start. Just to clarify here: the macio/DBDMA code is quite complicated, but this is because this device has to work around to the fact that currently the DMA I/O routines currently need sector alignment whereas macio requires byte-level alignment. There has been quite a lot of work at the lower levels to support byte-level alignment (see Kevin's series at http://lists.gnu.org/archive/html/qemu-devel/2014-01/msg02163.html) but until we can specify transfers to byte granularity in the dma_bdrv_*() APIs then there isn't much we can do to clean up the macio.c code. Kevin, are there any plans to bubble the byte-granularity block layer changes up to the dma_bdrv_*() APIs in the near future? Please bear in mind that QEMU supports a large number of OSs, and there is already an enthusiastic group of people using Alex's OS X work (see emaculation for many examples) so introducing an intermittent fault on a supported OS is not an option here. I should also re-emphasise that Alex/Andreas work on many different parts of QEMU, and my work is currently unsponsored so while we are all keen to improve QEMU to the point where it can emulate new OSs such as MorphOS, it's not the case that we can simply drop what we are doing at the time to focus on an issue that affects a single OS which is new and currently unsupported. I also work unsponsored on this and not sure how long can I still find time for it. I've already spent much more with this than I originally planned as I'm doing it since end of this February already. So I'd like my work so far to get upstream so that if I have to finish it's not lost and others could use and build on it. If there's no chance that this can be achieved by 2.1 then you could revert this patch and get back to it in 2.2 but that would delay things by months again. My patches are on the list for quite some time so it's not like I'm asking you to work on this in the last minute and this bug was reported on May 4th. I appreciate your help so far very much and don't exepct this to be highest priority but I'd like to make some progress too. Now I think it's fair to say that I've spent quite a few hours helping you and coming up with the original version of this patch, and I'm glad that you Now doubt about that, thank you very much again. are now seeing success with this. But what is important to us right now heading towards a release is that patches don't cause any regressions. All I can say is that debugging this stuff isn't easy, particularly with MorphOS which has some rather unusual behaviours. But what we really need from you now over the next few days is for you to compare the debug output between the working and non-working cases and figure out if we can fix this in time for the 2.1 release. You have everything you need (including my acceptance test of booting both MorphOS and Darwin ISOs), so time to take a deep breath and begin what should be a challenging yet ultimately rewarding debugging process :) I'm still working on finding a solution for the exception problems with OpenBIOS that prevent MorphOS from working and I failed to understand the whole working of macio, DBDMA and the whole block layer so far but I can try to debug it. Can you tell how to reproduce the problem with Darwin? The Darwin images don't seem to work with -M mac99 either before or after the patch so no regressions there. Regards, BALATON Zoltan
Re: [Qemu-devel] [PATCH 3/4] machine: Introduce QEMU_COMPAT_* macros
On Tue, 24 Jun 2014, Eduardo Habkost wrote:
The QEMU_COMPAT_* macros will contain compat properties that are not
specific to PC, and may be reused by other machine-types.
PC-specific properties were left on the PC_COMPAT_* macros.
Signed-off-by: Eduardo Habkost
---
include/hw/boards.h | 161 +++
include/hw/i386/pc.h | 150 ++-
2 files changed, 166 insertions(+), 145 deletions(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 605a970..709b582 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -134,4 +134,165 @@ struct MachineState {
const char *cpu_model;
};
+
+/* Macros for compat_props corresponding to specific QEMU versions: */
+
+#define QEMU_COMPAT_2_0 \
+{\
+.driver = "virtio-scsi-pci",\
+.property = "any_layout",\
+.value= "off",\
+},\
+{\
+.driver = "apic",\
+.property = "version",\
+.value= stringify(0x11),\
+},\
+{\
+.driver = "nec-usb-xhci",\
+.property = "superspeed-ports-first",\
+.value= "off",\
+},\
+{\
+.driver = "pci-serial",\
+.property = "prog_if",\
+.value= stringify(0),\
+},\
+{\
+.driver = "pci-serial-2x",\
+.property = "prof_if",\
Just noticed a typo now in this part which was added by me previously.
This should also be "prog_if" instead of "prof_if". Can you also correct
that while changing this or otherwise while merging or should I send a
separate patch with just this change?
Regards,
BALATON Zoltan
+.value= stringify(0),\
+},\
+{\
+.driver = "pci-serial-4x",\
+.property = "prog_if",\
+.value= stringify(0),\
+},\
+{\
+.driver = "virtio-net-pci",\
+.property = "guest_announce",\
+.value= "off",\
+}
+
+#define QEMU_COMPAT_1_7 \
Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report
On Wed, 7 May 2014, Mark Cave-Ayland wrote: On 07/05/14 18:00, BALATON Zoltan wrote: On Wed, 7 May 2014, Mark Cave-Ayland wrote: I'm not sure if your problem related to s->lba == -1 should be solved just for macio or higher up in the block layer, but the block people will be on qemu-devel and not qemu-ppc so you should definitely CC the main list to get their feedback on this. Cc'd the devel list now, the original question can be found here: http://lists.nongnu.org/archive/html/qemu-ppc/2014-05/msg3.html I think it's handled by the block layer but the translation in macio breaks it and converts it to -4 which causes an error so probably it should only be solved for macio. But I hope Alex can look at it and tell for sure or maybe solve it. Okay. So in that case it could just be a macio bug. Those ISOs only boot for me with the default -M g3beige, but darwin is a huge culprit for these unaligned accesses. My point was that if you are making changes in this area, if you can still boot the darwin images then that would be a good test that any changes you make are working correctly :) Ah, OK. They boot for me without a -M parameter with my changes (to the point saying "available for installation:" and then empty as I did not specify a hard disk image) too, but I was testing with -M mac99 as that was what I changed. I think they still "work" as before. But... the block patchset link I sent you has the potential to fix this anyway, since if the unaligned accesses can be passed directly up to the block layer then in theory this problem should go away as this whole chunk of code should no longer be required. A good test for this would be to try reverting this patch: https://github.com/agraf/qemu/commit/063a333911f1bb09fcc1a4925c8ebf5c88fc2b63 which is Alex's original alignment fix. If you find with that with this patch reverted darwin boots as before, it means that the block alignment patches in core are working correctly and hopefully you may find that it fixes your MorphOS problem. But again, you really should get this on qemu-devel to get the definitive answer from the block guys. That patch would be 80fc95d8bdaf3392106b131a97ca701fd374489a in QEMU master. I've tried reverting it and Darwin still boots (without -M mac99) up to the point where it asks to install as before but I don't know how good a test is this as I'm not sure it does unaligned accesses up to this point. I see accesses like this: pmac_ide_transfer(ATAPI) lba=, buffer_index=30, len=930 pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=930 pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=1, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=1, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=2, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=2, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=f2, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=f3, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=1ac, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=9ae, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=9af, buffer_index=0, len=400 pmac_ide_transfer(ATAPI) lba=9ae, buffer_index=0, len=400 pmac_ide_transfer(ATAPI) lba=9ae, buffer_index=400, len=400 pmac_ide_transfer(ATAPI) lba=9af, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=11ae, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=11af, buffer_index=0, len=1000 [...] pmac_ide_transfer(ATAPI) lba=3858e, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=3856e, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=38572, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=38590, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=38584, buffer_index=0, len=1000 pmac_ide_transfer(ATAPI) lba=38586, buffer_index=0, len=3000 pmac_ide_transfer(ATAPI) lba=3857e, buffer_index=0, len=3000 pmac_ide_transfer(ATAPI) lba=38566, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=38550, buffer_index=0, len=1000 pmac_ide_transfer(ATAPI) lba=38552, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=38536, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=3852c, buffer_index=0, len=5000 pmac_ide_transfer(ATAPI) lba=385a6, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=a724, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=3854a, buffer_index=0, len=3000 pmac_ide_transfer(ATAPI) lba=1654, buffer_index=0, len=1000 pmac_ide_transfer(ATAPI) lba=3b1cc, buffer_index=0, len=8000 pmac_ide_transfer(ATAPI) lba=3b3a8, buffer_index=0, len=8000 pmac_ide_transfer(ATAPI) lba=3b3a0, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=3b3a8, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=38546, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=3854a, buffer_index=0, len=8000 pmac_ide_transfer(ATAPI) lba=3b3d8, buffer_index=0, len=8000 pm
Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report
On Mon, 12 May 2014, Mark Cave-Ayland wrote:
On 10/05/14 13:30, BALATON Zoltan wrote:
That patch would be 80fc95d8bdaf3392106b131a97ca701fd374489a in QEMU
master. I've tried reverting it and Darwin still boots (without -M
mac99) up to the point where it asks to install as before but I don't
know how good a test is this as I'm not sure it does unaligned accesses
up to this point. I see accesses like this:
pmac_ide_transfer(ATAPI) lba=, buffer_index=30, len=930
pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=930
Here is an example of an unaligned access, where the length is not a multiple
of 0x200 (512 bytes) which was required for the block layer DMA APIs at the
time it was written. If you can get to the same point with the patch
reverted, then it means Kevin's patchset for byte-level rather than
sector-level granularity for block requests is working.
These logs were with the above commit reverted so it's probably not needed
any more. I've tried booting Darwin 6.0.2, Darwin 8.0.1 up to the
installation (did not install it) and Finnix 109 and they seem to work as
before.
What's important is the sum of the start_input size descriptors now matches
the size of the ATA request, so the dma_* callbacks can now be used directly
with byte precision rather than clobbering the memory up to the next multiple
of 0x200.
This however does not fix MorphOS which fails as before. I've tried this
patch too (on top of the previous revert) but that did not work either:
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index d3395f4..bce14fc 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -56,6 +56,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque,
int ret)
DBDMA_io *io = opaque;
MACIOIDEState *m = io->opaque;
IDEState *s = idebus_active_if(&m->bus);
+int64_t sector_num;
if (ret < 0) {
m->aiocb = NULL;
@@ -107,13 +108,16 @@ static void pmac_ide_atapi_transfer_cb(void
*opaque, int r
qemu_sglist_add(&s->sg, io->addr, io->len);
io->addr += io->len;
io->len = 0;
-
-MACIO_DPRINTF("sector_num=%d size=%d, cmd_cmd=%d\n",
- (s->lba << 2) + (s->io_buffer_index >> 9),
+if (s->lba >= 0)
+sector_num = (s->lba << 2) + (s->io_buffer_index >> 9);
+else
+sector_num = s->lba;
+MACIO_DPRINTF("sector_num=%ld size=%d, cmd_cmd=%d\n",
+ sector_num,
s->packet_transfer_size, s->dma_cmd);
m->aiocb = dma_bdrv_read(s->bs, &s->sg,
- (int64_t)(s->lba << 2) +
(s->io_buffer_index >> 9)
+ sector_num,
pmac_ide_atapi_transfer_cb, io);
return;
This still produces an error in MorphOS as before:
DBDMA: writel 0x0d0c <= 0x00e5ac80
DBDMA: channel 0x1a reg 0x3
DBDMA: dbdma_cmdptr_load 0x00e5ac80
ATAPI limit=0x8000 packet: 43 00 00 00 00 00 00 03 24 00 00 00
DBDMA: DBDMA_run_bh
DBDMA: writel 0x0d00 <= 0x80008000
DBDMA: channel 0x1a reg 0x0
DBDMA: status 0x8400
DBDMA: readl 0x0d00 => 0x80008000
DBDMA: channel 0x1a reg 0x0
DBDMA: DBDMA_run_bh
DBDMA: channel_run
dbdma_cmd 0x7f7dddff5ae0
req_count 0x0324
command 0x3000
phy_addr 0x00e7dddc
cmd_dep 0x
res_count 0x
xfer_status 0x
DBDMA: start_input
DBDMA: addr 0xe7dddc key 0x0
pmac_ide_transfer(ATAPI) lba=, buffer_index=0, len=324
^
io_buffer_size = 0
io->len = 0x324
sector_num=-1 size=20, cmd_cmd=0
atapi_cmd_error: sense=0x5 asc=0x21
done DMA
DBDMA: dbdma_end
Interestingly a similar read works for Darwin which does this:
ATAPI limit=0x0 packet: bb 00 ff ff 00 00 00 00 00 00 00 00
ATAPI limit=0xfffe packet: 43 02 00 00 00 00 00 ff fe 80 00 00
reply: tx_size=48 elem_tx_size=0 index=0
byte_count_limit=65534
status=0x58
reply: tx_size=0 elem_tx_size=0 index=48
status=0x50
ATAPI limit=0x30 packet: 43 02 00 00 00 00 00 00 30 80 00 00
reply: tx_size=48 elem_tx_size=0 index=0
byte_count_limit=48
status=0x58
reply: tx_size=0 elem_tx_size=0 index=48
status=0x50
DBDMA: readl 0x0d04 => 0x
DBDMA: channel 0x1a reg 0x1
DBDMA: writel 0x0d08 <= 0x
DBDMA: channel 0x1a reg 0x2
DBDMA: writel 0x0d0c <= 0x011f8010
DBDMA: channel 0x1a reg 0x3
DBDMA: dbdma_cmdptr_load 0x011f8010
DBDMA: writel 0x0d00 <= 0x90009000
DBDMA: channel 0x1a reg 0x0
DBDMA: status 0x9400
DBDMA: DBDMA_run_bh
DBDMA: channel_run
dbdma_cmd 0x7ffa05173c60
req_count 0x0930
command 0x2000
phy_addr 0x017cc000
cmd_dep 0x
res_count 0x
xfer_status 0x
DBDMA: start_input
DBDMA: addr 0x17cc000 key 0x0
pmac_ide_transfer(ATAPI) lba=, buffer_index=30, len=930
waiting for data (0
Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report
On Mon, 12 May 2014, Mark Cave-Ayland wrote:
On 12/05/14 20:32, BALATON Zoltan wrote:
(cut)
MorphOS and Darwin are definitely doing things differently. I hope
someone who understands what is happening can explain it why one of them
works while the other doesn't.
Which I hope is what I'm trying to do? From hw/ide/atapi.c you can see that
Yes, your comments and advise are helping a lot. Thank you.
command 0x43 is read the TOC which according to atapi_cmd_table should call
cmd_read_toc_pma_atip(). You can see that in your MorphOS case you are
getting a line with a "atapi_cmd_error" prefix which indicates that something
is calling ide_atapi_cmd_error() to return an error code instead of
ide_atapi_cmd_reply() which would output the "reply" prefix as seen in your
Darwin case. So you need to step through these functions in QEMU in order to
see why your ATAPI command is failing.
I've tried doing this and it seems that the cmd_read_toc_pma_atip function
returns all right (via the case 0 path) with a 20 byte result array and
calls ide_atapi_cmd_reply which takes the DMA path as s->atapi_dma is set
to 1 and the error comes from somewhere during trying to DMA the result
back to the client. I could not follow it so I've only got a backtrace
from where ide_atapi_cmd_error is called:
DBDMA: DBDMA_run_bh
DBDMA: writel 0x0d00 <= 0x80008000
DBDMA: channel 0x1a reg 0x0
DBDMA: status 0x8400
DBDMA: readl 0x0d00 => 0x80008000
DBDMA: channel 0x1a reg 0x0
DBDMA: DBDMA_run_bh
DBDMA: channel_run
dbdma_cmd 0x563cccb0
req_count 0x0324
command 0x3000
phy_addr 0x00e17e4c
cmd_dep 0x
res_count 0x
xfer_status 0x
DBDMA: start_input
DBDMA: addr 0xe17e4c key 0x0
pmac_ide_transfer(ATAPI) lba=, buffer_index=0, len=324
io_buffer_size = 0
io->len = 0x324
sector_num=-1 size=20, cmd_cmd=0
[Switching to Thread 0x77fc5900 (LWP 6462)]
Breakpoint 2, ide_atapi_cmd_error (s=0x563cb238, sense_key=5, asc=33)
at hw/ide/atapi.c:141
141 {
(gdb) bt
#0 ide_atapi_cmd_error (s=0x563cb238, sense_key=5, asc=33)
at hw/ide/atapi.c:141
#1 0x556cecf5 in ide_atapi_io_error (s=0x563cb238, ret=-5)
at hw/ide/atapi.c:160
#2 0x556d9d01 in pmac_ide_atapi_transfer_cb (opaque=0x563ccc68,
ret=-5) at hw/ide/macio.c:64
#3 0x556780d2 in dma_complete (dbs=0x563ab840, ret=-5)
at dma-helpers.c:121
#4 0x556781db in dma_bdrv_cb (opaque=0x563ab840, ret=-5)
at dma-helpers.c:149
#5 0x55614dd1 in bdrv_co_em_bh (opaque=0x563b5000) at block.c:4602
#6 0x555f8170 in aio_bh_poll (ctx=0x5637fc00) at async.c:81
#7 0x555f7dc9 in aio_poll (ctx=0x5637fc00, blocking=false)
at aio-posix.c:188
#8 0x555f8587 in aio_ctx_dispatch (source=0x5637fc00, callback=
0x0, user_data=0x0) at async.c:205
#9 0x778ca6d5 in g_main_context_dispatch ()
from /lib64/libglib-2.0.so.0
#10 0x557a0f42 in glib_pollfds_poll () at main-loop.c:190
#11 0x557a1042 in os_host_main_loop_wait (timeout=0) at main-loop.c:235
#12 0x557a1115 in main_loop_wait (nonblocking=1) at main-loop.c:484
#13 0x55844190 in main_loop () at vl.c:2075
#14 0x5584bc23 in main (argc=30, argv=0x7fffdc88, envp=
0x7fffdd80) at vl.c:4556
Do you have any idea how to debug this further or does this help to tell
where is the problem? (I think the question is where does the -5 return
value come from?)
Regards,
BALATON Zoltan
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2] serial-pci: Set prog interface field of pci config to 16550 compatible
On Thu, 8 May 2014, Michael Tokarev wrote: 27.02.2014 05:05, BALATON Zoltan wrote: Shoudl we actually make it machine-specific, to keep even prog-if value of these things the same as before for older machine types? I dunno. mst says we should, I think this is not a very important property to keep. So what's the decision? Can it be taken as it is now or do you want any more changes? (If you want it version specific then please tell me how to do that or show me an example because I don't know how to do that.) Regards, BALATON Zoltan Thanks, /mjt v2: resubmission after pc-2.1 is added with the multiport case hw/char/serial-pci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c index 991c99f..f3f5e07 100644 --- a/hw/char/serial-pci.c +++ b/hw/char/serial-pci.c @@ -60,6 +60,7 @@ static int serial_pci_init(PCIDevice *dev) return -1; } +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; s->irq = pci_allocate_irq(&pci->dev); @@ -101,6 +102,7 @@ static int multi_serial_pci_init(PCIDevice *dev) assert(pci->ports > 0); assert(pci->ports <= PCI_SERIAL_MAX_PORTS); +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; memory_region_init(&pci->iobar, OBJECT(pci), "multiserial", 8 * pci->ports); pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar);
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2] serial-pci: Set prog interface field of pci config to 16550 compatible
On Wed, 14 May 2014, Michael S. Tsirkin wrote: On Wed, May 14, 2014 at 01:39:03AM +0200, BALATON Zoltan wrote: On Thu, 8 May 2014, Michael Tokarev wrote: 27.02.2014 05:05, BALATON Zoltan wrote: Shoudl we actually make it machine-specific, to keep even prog-if value of these things the same as before for older machine types? I dunno. mst says we should, I think this is not a very important property to keep. So what's the decision? Can it be taken as it is now or do you want any more changes? (If you want it version specific then please tell me how to do that or show me an example because I don't know how to do that.) If you change it migration to old qemu breaks. This is unlikely as this pci serial card is not used by default in any machine version AFAIK. Is it still desired to keep backward compatibility for a device that can only be added by users explicitely? Regards, BALATON Zoltan See commit aa93200b88fb1071eaf21bf766711762ed4630e2 Author: Gabriel L. Somlo Date: Mon May 5 10:52:51 2014 -0400 apic: use emulated lapic version 0x14 on pc machines >= 2.1 as an example on how to do it. Thanks, /mjt v2: resubmission after pc-2.1 is added with the multiport case hw/char/serial-pci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c index 991c99f..f3f5e07 100644 --- a/hw/char/serial-pci.c +++ b/hw/char/serial-pci.c @@ -60,6 +60,7 @@ static int serial_pci_init(PCIDevice *dev) return -1; } +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; s->irq = pci_allocate_irq(&pci->dev); @@ -101,6 +102,7 @@ static int multi_serial_pci_init(PCIDevice *dev) assert(pci->ports > 0); assert(pci->ports <= PCI_SERIAL_MAX_PORTS); +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; memory_region_init(&pci->iobar, OBJECT(pci), "multiserial", 8 * pci->ports); pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar);
Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report
On Wed, 14 May 2014, Mark Cave-Ayland wrote: On 14/05/14 00:02, BALATON Zoltan wrote: command 0x43 is read the TOC which according to atapi_cmd_table should call cmd_read_toc_pma_atip(). You can see that in your MorphOS case you are getting a line with a "atapi_cmd_error" prefix which indicates that something is calling ide_atapi_cmd_error() to return an error code instead of ide_atapi_cmd_reply() which would output the "reply" prefix as seen in your Darwin case. So you need to step through these functions in QEMU in order to see why your ATAPI command is failing. I've tried doing this and it seems that the cmd_read_toc_pma_atip function returns all right (via the case 0 path) with a 20 byte result array and calls ide_atapi_cmd_reply which takes the DMA path as s->atapi_dma is set to 1 and the error comes from somewhere during trying to DMA the result back to the client. I could not follow it so I've only got a backtrace from where ide_atapi_cmd_error is called: So this basically confirms that the DMA errors out because s->lba == -1 in the macio callback. FWIW you should probably ensure that DEBUG_IDE_ATAPI is enabled when posting logs in future as this helps show the interaction between the two systems. The logs I've posted are with DEBUG_IDE_ATAPI, DEBUG_DBDMA and DEBUG_MACIO already enabled... Do you have any idea how to debug this further or does this help to tell where is the problem? (I think the question is where does the -5 return value come from?) Well from this the cause is fairly easy to spot: ide_atapi_cmd_reply() sets s->lba == -1 when called from cmd_read_toc_pma_atip(). And since as you correctly point out this is a DMA request, it invokes the start_dma function in macio's dbdma_ops (ide_dbdma_start), which kicks the DBDMA engine into life. I think the issue here is related to the fact that reading a TOC doesn't actually involve reading physical blocks from disk (as the TOC is placed directly in the buffer) and so the dma_bdrv_* functions shouldn't actually be invoked in the first place. And because of the DBDMA setup, ide_atapi_cmd_read_dma_cb() can't be used which is why pmac_ide_transfer_cb() needs to do a lot of similar work itself. Maybe you can find some clues there as to what the logic should be? I'm afraid I still can't understand it. Is there a way to trace the path after DBDMA engine is kicked? Where should I break to get some insight on what is happening? (Maybe it's a dbdma bug not a macio one.) Regards, BALATON Zoltan
[Qemu-devel] [PATCH v3] serial-pci: Set prog interface field of pci config to 16550 compatible
Signed-off-by: BALATON Zoltan
---
v2: resubmission after pc-2.1 is added with the multiport case
v3: added compatibility check to avoid changing earlier than pc-2.1
hw/char/serial-pci.c | 11 +++
include/hw/i386/pc.h | 15 +++
2 files changed, 26 insertions(+)
diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
index 991c99f..ae57098 100644
--- a/hw/char/serial-pci.c
+++ b/hw/char/serial-pci.c
@@ -34,6 +34,7 @@
typedef struct PCISerialState {
PCIDevice dev;
SerialState state;
+uint8_t compat;
} PCISerialState;
typedef struct PCIMultiSerialState {
@@ -44,6 +45,7 @@ typedef struct PCIMultiSerialState {
SerialState state[PCI_SERIAL_MAX_PORTS];
uint32_t level[PCI_SERIAL_MAX_PORTS];
qemu_irq *irqs;
+uint8_t compat;
} PCIMultiSerialState;
static int serial_pci_init(PCIDevice *dev)
@@ -60,6 +62,9 @@ static int serial_pci_init(PCIDevice *dev)
return -1;
}
+if (!pci->compat) {
+pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */
+}
pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
s->irq = pci_allocate_irq(&pci->dev);
@@ -101,6 +106,9 @@ static int multi_serial_pci_init(PCIDevice *dev)
assert(pci->ports > 0);
assert(pci->ports <= PCI_SERIAL_MAX_PORTS);
+if (!pci->compat) {
+pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */
+}
pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
memory_region_init(&pci->iobar, OBJECT(pci), "multiserial", 8 *
pci->ports);
pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar);
@@ -177,12 +185,14 @@ static const VMStateDescription vmstate_pci_multi_serial
= {
static Property serial_pci_properties[] = {
DEFINE_PROP_CHR("chardev", PCISerialState, state.chr),
+DEFINE_PROP_UINT8("compat", PCISerialState, compat, 0),
DEFINE_PROP_END_OF_LIST(),
};
static Property multi_2x_serial_pci_properties[] = {
DEFINE_PROP_CHR("chardev1", PCIMultiSerialState, state[0].chr),
DEFINE_PROP_CHR("chardev2", PCIMultiSerialState, state[1].chr),
+DEFINE_PROP_UINT8("compat", PCIMultiSerialState, compat, 0),
DEFINE_PROP_END_OF_LIST(),
};
@@ -191,6 +201,7 @@ static Property multi_4x_serial_pci_properties[] = {
DEFINE_PROP_CHR("chardev2", PCIMultiSerialState, state[1].chr),
DEFINE_PROP_CHR("chardev3", PCIMultiSerialState, state[2].chr),
DEFINE_PROP_CHR("chardev4", PCIMultiSerialState, state[3].chr),
+DEFINE_PROP_UINT8("compat", PCIMultiSerialState, compat, 0),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 32a7687..8fb8046 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -271,6 +271,21 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
.driver = "apic",\
.property = "version",\
.value= stringify(0x11),\
+},\
+{\
+.driver = "pci-serial",\
+.property = "compat",\
+.value= stringify(1),\
+},\
+{\
+.driver = "pci-serial-2x",\
+.property = "compat",\
+.value= stringify(1),\
+},\
+{\
+.driver = "pci-serial-4x",\
+.property = "compat",\
+.value= stringify(1),\
}
#define PC_COMPAT_1_7 \
--
1.8.1.5
Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report
On Wed, 14 May 2014, Mark Cave-Ayland wrote: On 14/05/14 12:10, BALATON Zoltan wrote: The logs I've posted are with DEBUG_IDE_ATAPI, DEBUG_DBDMA and DEBUG_MACIO already enabled... Well sure, but not the ones in your last email - I had to go back several mails back into the thread to pull them out. Bear in mind the high volume of these lists means that a lot of people who could help won't have the time to do this. All the logs I posted in this thread were with these debug options enabled. Maybe the beginning was missing as I only included the logs from the failing dma reply because before that I was tracing to see that the TOC was read and about to be returned so I did not include those logs again. (They were in the previous mail though.) I'm including them again below this time. Which part is it that's still confusing you? Putting breakpoints on pmac_ide_transfer() and pmac_ide_atapi_transfer_cb() will show you the iterations on each DMA request (be sure to compare against a "known good" example to understand how it should work first). If you can give more detail as to which bits are confusing, I will try my best to explain them. Looking at the backtrace: #0 ide_atapi_cmd_error (s=0x563cb238, sense_key=5, asc=33) at hw/ide/atapi.c:141 #1 0x556cecf5 in ide_atapi_io_error (s=0x563cb238, ret=-5) at hw/ide/atapi.c:160 #2 0x556d9d01 in pmac_ide_atapi_transfer_cb (opaque=0x563ccc68, ret=-5) at hw/ide/macio.c:64 #3 0x556780d2 in dma_complete (dbs=0x563ab840, ret=-5) at dma-helpers.c:121 #4 0x556781db in dma_bdrv_cb (opaque=0x563ab840, ret=-5) at dma-helpers.c:149 #5 0x55614dd1 in bdrv_co_em_bh (opaque=0x563b5000) at block.c:4602 #6 0x555f8170 in aio_bh_poll (ctx=0x5637fc00) at async.c:81 #7 0x555f7dc9 in aio_poll (ctx=0x5637fc00, blocking=false) at aio-posix.c:188 #8 0x555f8587 in aio_ctx_dispatch (source=0x5637fc00, callback= 0x0, user_data=0x0) at async.c:205 #9 0x778ca6d5 in g_main_context_dispatch () from /lib64/libglib-2.0.so.0 #10 0x557a0f42 in glib_pollfds_poll () at main-loop.c:190 #11 0x557a1042 in os_host_main_loop_wait (timeout=0) at main-loop.c:235 #12 0x557a1115 in main_loop_wait (nonblocking=1) at main-loop.c:484 #13 0x55844190 in main_loop () at vl.c:2075 #14 0x5584bc23 in main (argc=30, argv=0x7fffdc88, envp= 0x7fffdd80) at vl.c:4556 shows that pmac_ide_atapi_transfer_cb is called with ret=-5 which is why it fails, so putting a breakpoint there is too late. What I don't understand is where this -5 value comes from. I don't have a known good example because Darwin reads the TOC differently (probably before enabling DMA, I did not trace it more than the logs I've included earlier though) and MorphOS always fails. Here are the logs of all requests MorphOS does up to the failing ReadTOC again: ATAPI limit=0x8000 packet: 00 00 00 00 00 00 00 00 00 00 00 00 ATAPI limit=0x8000 packet: 12 00 00 00 24 00 00 00 00 00 00 00 reply: tx_size=36 elem_tx_size=0 index=0 byte_count_limit=32768 status=0x58 reply: tx_size=0 elem_tx_size=0 index=36 status=0x50 ATAPI limit=0x8000 packet: 1b 00 00 00 01 00 00 00 00 00 00 00 ATAPI limit=0x8000 packet: 00 00 00 00 00 00 00 00 00 00 00 00 ATAPI limit=0x8000 packet: 25 00 00 00 00 00 00 00 00 00 00 00 reply: tx_size=8 elem_tx_size=0 index=0 byte_count_limit=32768 status=0x58 reply: tx_size=0 elem_tx_size=0 index=8 status=0x50 ATAPI limit=0x8000 packet: 5a 00 05 00 00 00 00 00 30 00 00 00 atapi_cmd_error: sense=0x5 asc=0x24 ATAPI limit=0x8000 packet: 5a 00 04 00 00 00 00 00 28 00 00 00 atapi_cmd_error: sense=0x5 asc=0x24 ATAPI limit=0x8000 packet: 5a 00 03 00 00 00 00 00 28 00 00 00 atapi_cmd_error: sense=0x5 asc=0x24 ATAPI limit=0x8000 packet: 5a 00 3f 00 00 00 00 01 02 00 00 00 atapi_cmd_error: sense=0x5 asc=0x24 ATAPI limit=0x8000 packet: 51 00 00 00 00 00 00 00 22 00 00 00 reply: tx_size=34 elem_tx_size=0 index=0 byte_count_limit=32768 status=0x58 reply: tx_size=0 elem_tx_size=0 index=34 status=0x50 DBDMA: writel 0x0d0c <= 0x00e4e960 DBDMA: channel 0x1a reg 0x3 DBDMA: dbdma_cmdptr_load 0x00e4e960 ATAPI limit=0x8000 packet: 43 00 00 00 00 00 00 03 24 00 00 00 DBDMA: DBDMA_run_bh DBDMA: writel 0x0d00 <= 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: status 0x8400 DBDMA: readl 0x0d00 => 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: DBDMA_run_bh DBDMA: channel_run dbdma_cmd 0x7f8f93662ee0 req_count 0x0324 command 0x3000 phy_addr 0x00e4f30c cmd_dep 0x res_count 0x xfer_status 0x DBDMA: start_input DBDMA: addr 0xe4f30c key 0x0 pmac_ide_transfer(ATAPI) lba=, buffer_index=0, len=324 io_buffer_size = 0 io->len = 0x324 sector_num=-1 size=20, cmd_cmd=0 atapi_cmd_error: sense=0x5 asc=0x2
[Qemu-devel] [PATCH v4] serial-pci: Set prog interface field of pci config to 16550 compatible
Signed-off-by: BALATON Zoltan
---
v2: resubmission after pc-2.1 is added with the multiport case
v3: added compatibility check to avoid changing earlier than pc-2.1
v4: renamed compat property to prog_if
hw/char/serial-pci.c | 7 +++
include/hw/i386/pc.h | 15 +++
2 files changed, 22 insertions(+)
diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
index 991c99f..a9c 100644
--- a/hw/char/serial-pci.c
+++ b/hw/char/serial-pci.c
@@ -34,6 +34,7 @@
typedef struct PCISerialState {
PCIDevice dev;
SerialState state;
+uint8_t prog_if;
} PCISerialState;
typedef struct PCIMultiSerialState {
@@ -44,6 +45,7 @@ typedef struct PCIMultiSerialState {
SerialState state[PCI_SERIAL_MAX_PORTS];
uint32_t level[PCI_SERIAL_MAX_PORTS];
qemu_irq *irqs;
+uint8_t prog_if;
} PCIMultiSerialState;
static int serial_pci_init(PCIDevice *dev)
@@ -60,6 +62,7 @@ static int serial_pci_init(PCIDevice *dev)
return -1;
}
+pci->dev.config[PCI_CLASS_PROG] = pci->prog_if;
pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
s->irq = pci_allocate_irq(&pci->dev);
@@ -101,6 +104,7 @@ static int multi_serial_pci_init(PCIDevice *dev)
assert(pci->ports > 0);
assert(pci->ports <= PCI_SERIAL_MAX_PORTS);
+pci->dev.config[PCI_CLASS_PROG] = pci->prog_if;
pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
memory_region_init(&pci->iobar, OBJECT(pci), "multiserial", 8 *
pci->ports);
pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar);
@@ -177,12 +181,14 @@ static const VMStateDescription vmstate_pci_multi_serial
= {
static Property serial_pci_properties[] = {
DEFINE_PROP_CHR("chardev", PCISerialState, state.chr),
+DEFINE_PROP_UINT8("prog_if", PCISerialState, prog_if, 0x02),
DEFINE_PROP_END_OF_LIST(),
};
static Property multi_2x_serial_pci_properties[] = {
DEFINE_PROP_CHR("chardev1", PCIMultiSerialState, state[0].chr),
DEFINE_PROP_CHR("chardev2", PCIMultiSerialState, state[1].chr),
+DEFINE_PROP_UINT8("prog_if", PCIMultiSerialState, prog_if, 0x02),
DEFINE_PROP_END_OF_LIST(),
};
@@ -191,6 +197,7 @@ static Property multi_4x_serial_pci_properties[] = {
DEFINE_PROP_CHR("chardev2", PCIMultiSerialState, state[1].chr),
DEFINE_PROP_CHR("chardev3", PCIMultiSerialState, state[2].chr),
DEFINE_PROP_CHR("chardev4", PCIMultiSerialState, state[3].chr),
+DEFINE_PROP_UINT8("prog_if", PCIMultiSerialState, prog_if, 0x02),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 32a7687..552fbd8 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -271,6 +271,21 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
.driver = "apic",\
.property = "version",\
.value= stringify(0x11),\
+},\
+{\
+.driver = "pci-serial",\
+.property = "prog_if",\
+.value= stringify(0),\
+},\
+{\
+.driver = "pci-serial-2x",\
+.property = "prof_if",\
+.value= stringify(0),\
+},\
+{\
+.driver = "pci-serial-4x",\
+.property = "prog_if",\
+.value= stringify(0),\
}
#define PC_COMPAT_1_7 \
--
1.8.1.5
Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report
On Thu, 15 May 2014, Mark Cave-Ayland wrote:
On 15/05/14 00:21, BALATON Zoltan wrote:
Which part is it that's still confusing you? Putting breakpoints on
pmac_ide_transfer() and pmac_ide_atapi_transfer_cb() will show you the
iterations on each DMA request (be sure to compare against a "known
good" example to understand how it should work first). If you can give
more detail as to which bits are confusing, I will try my best to
explain them.
Looking at the backtrace:
#0 ide_atapi_cmd_error (s=0x563cb238, sense_key=5, asc=33)
at hw/ide/atapi.c:141
#1 0x556cecf5 in ide_atapi_io_error (s=0x563cb238, ret=-5)
at hw/ide/atapi.c:160
#2 0x556d9d01 in pmac_ide_atapi_transfer_cb
(opaque=0x563ccc68,
ret=-5) at hw/ide/macio.c:64
#3 0x556780d2 in dma_complete (dbs=0x563ab840, ret=-5)
at dma-helpers.c:121
#4 0x556781db in dma_bdrv_cb (opaque=0x563ab840, ret=-5)
at dma-helpers.c:149
#5 0x55614dd1 in bdrv_co_em_bh (opaque=0x563b5000) at
block.c:4602
#6 0x555f8170 in aio_bh_poll (ctx=0x5637fc00) at async.c:81
#7 0x555f7dc9 in aio_poll (ctx=0x5637fc00, blocking=false)
at aio-posix.c:188
#8 0x555f8587 in aio_ctx_dispatch (source=0x5637fc00,
callback=
0x0, user_data=0x0) at async.c:205
#9 0x778ca6d5 in g_main_context_dispatch ()
from /lib64/libglib-2.0.so.0
#10 0x557a0f42 in glib_pollfds_poll () at main-loop.c:190
#11 0x557a1042 in os_host_main_loop_wait (timeout=0) at
main-loop.c:235
#12 0x557a1115 in main_loop_wait (nonblocking=1) at main-loop.c:484
#13 0x55844190 in main_loop () at vl.c:2075
#14 0x5584bc23 in main (argc=30, argv=0x7fffdc88, envp=
0x7fffdd80) at vl.c:4556
shows that pmac_ide_atapi_transfer_cb is called with ret=-5 which is why
it fails, so putting a breakpoint there is too late. What I don't
understand is where this -5 value comes from. I don't have a known good
example because Darwin reads the TOC differently (probably before
enabling DMA, I did not trace it more than the logs I've included
earlier though) and MorphOS always fails.
Have you noticed that the dma_bdrv_*() functions use a function pointer to
pmac_ide_atapi_transfer_cb() as their callback function? The dma_bdrv_*()
functions operate in a separate thread and then invoke the callback function
when finished.
The breakpoint you are hitting above is the invocation of
pmac_ide_atapi_transfer_cb() as a result of the callback *after* the command
has already failed, and not the invocation of pmac_ide_atapi_transfer_cb()
which calls dma_bdrv_*() to setup the asynchronous transfer. Hence the DMA
has already failed and the -5 value is being returned from the IDE code. I
can only guess the reason this works with Darwin is because it is a non-DMA
request.
If you place a breakpoint on pmac_ide_transfer() then its invocation of
pmac_ide_atapi_transfer_cb() should be the first iteration which sets up the
DMA request, and the second invocation should be the (failing) callback from
the dma_bdrv_*() functions. But as I mentioned before, I think the problem is
that these functions shouldn't be called in the first place when requesting a
TOC.
OK, I've done that and stopped at the first invocation of
pmac_ide_atapi_transfer_cb. Here is a backtrace and the contents of some
data structures:
#0 pmac_ide_atapi_transfer_cb (opaque=0x563ccc68, ret=0)
at hw/ide/macio.c:55
#1 0x556da6d0 in pmac_ide_transfer (io=0x563ccc68)
at hw/ide/macio.c:225
#2 0x5562 in start_input (ch=0x563ccc18, key=0, addr=
14777932, req_count=804, is_last=1) at hw/misc/macio/mac_dbdma.c:334
#3 0x556ef444 in channel_run (ch=0x563ccc18)
at hw/misc/macio/mac_dbdma.c:489
#4 0x556ef599 in DBDMA_run (s=0x563cba40)
at hw/misc/macio/mac_dbdma.c:531
#5 0x556ef5f4 in DBDMA_run_bh (opaque=0x563cba40)
at hw/misc/macio/mac_dbdma.c:542
#6 0x555f8200 in aio_bh_poll (ctx=0x5637fc00) at async.c:81
#7 0x555f7e59 in aio_poll (ctx=0x5637fc00, blocking=false)
at aio-posix.c:188
#8 0x555f8617 in aio_ctx_dispatch (source=0x5637fc00, callback=
0x0, user_data=0x0) at async.c:205
#9 0x778ca6d5 in g_main_context_dispatch ()
from /lib64/libglib-2.0.so.0
#10 0x557a0fde in glib_pollfds_poll () at main-loop.c:190
#11 0x557a10de in os_host_main_loop_wait (timeout=15883632)
at main-loop.c:235
#12 0x557a11b1 in main_loop_wait (nonblocking=0) at main-loop.c:484
#13 0x5584422c in main_loop () at vl.c:2075
#14 0x5584bcbf in main (argc=32, argv=0x7fffdc48, envp=
0x7fffdd50) at vl.c:4556
(gdb) p *io
$5 = {opaque = 0x563c8db0, channel = 0x563ccc18, addr = 14777932,
len = 804, is_last = 1, is_dma_out = 0, dma_end =
0x556eebf7 , process
Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report
On Thu, 15 May 2014, Mark Cave-Ayland wrote:
On 15/05/14 18:28, BALATON Zoltan wrote:
#0 pmac_ide_atapi_transfer_cb (opaque=0x563ccc68, ret=0)
at hw/ide/macio.c:55
#1 0x556da6d0 in pmac_ide_transfer (io=0x563ccc68)
at hw/ide/macio.c:225
#2 0x5562 in start_input (ch=0x563ccc18, key=0, addr=
14777932, req_count=804, is_last=1) at hw/misc/macio/mac_dbdma.c:334
#3 0x556ef444 in channel_run (ch=0x563ccc18)
at hw/misc/macio/mac_dbdma.c:489
#4 0x556ef599 in DBDMA_run (s=0x563cba40)
at hw/misc/macio/mac_dbdma.c:531
#5 0x556ef5f4 in DBDMA_run_bh (opaque=0x563cba40)
at hw/misc/macio/mac_dbdma.c:542
#6 0x555f8200 in aio_bh_poll (ctx=0x5637fc00) at async.c:81
#7 0x555f7e59 in aio_poll (ctx=0x5637fc00, blocking=false)
at aio-posix.c:188
#8 0x555f8617 in aio_ctx_dispatch (source=0x5637fc00,
callback=
0x0, user_data=0x0) at async.c:205
#9 0x778ca6d5 in g_main_context_dispatch ()
from /lib64/libglib-2.0.so.0
#10 0x557a0fde in glib_pollfds_poll () at main-loop.c:190
#11 0x557a10de in os_host_main_loop_wait (timeout=15883632)
at main-loop.c:235
#12 0x557a11b1 in main_loop_wait (nonblocking=0) at main-loop.c:484
#13 0x5584422c in main_loop () at vl.c:2075
#14 0x5584bcbf in main (argc=32, argv=0x7fffdc48, envp=
0x7fffdd50) at vl.c:4556
(lots cut)
So that looks like the correct request based upon it's size so what happened
when you stepped into pmac_ide_atapi_transfer_cb()...?
Did not follow it to the end because we know it would fail and the sense
value returned seems to be just a remnant of previous failures as can be
seen from the structures I've printed.
My testing was done with commit 80fc95d8bdaf3392106b131a97ca701fd374489a
already reverted as that was established before that it is not needed
any more which simplifies pmac_ide_atapi_transfer_cb() quite a bit
Sadly I've now found that this isn't the case (email to the qemu-devel list
to follow, but I've run out of time today) :(
OK, I'm back to the HEAD version and testing with that with your patch
applied.
Well my understanding is that it's impossible to boot a MorphOS image
directly and requires all sorts of tricks with debuggers etc. Unless you can
provide a "run this and it breaks" test case, then the time barrier for
trying to fix bugs like this is exceptionally high.
Unfortunately it is not straightforward to test with MorphOS but I think
it could be reproduced with other OS-es if they can be made to do a read
TOC with DMA. I can try to find a way to reproduce it with Linux but
hopefully we can get to the end of it now. (I think we are close.)
You mention that you don't understand the fields and sizes, so explain which
ones you don't understand and ask. Search around for guides to how ATAPI/IDE
The ones used in pmac_ide_atapi_transfer_cb(). It's not clear to me where
is the source buffer where is the destination and what is the correct
lenght to use. Your patch helps but it's not correct yet. See below.
Please see above. FWIW based upon the your gdb output I've put together the
following patch which compiles, but that's all I can vouch for it. Further
testing and debugging will have to be done by you, although I will try and
respond to specific questions where possible.
Thanks a lot. I've tested your patch and it comes closer but not quite
there yet. Running it in a debugger I've seen this:
Breakpoint 1, pmac_ide_transfer (io=0x563d1068) at hw/ide/macio.c:340
340 {
(gdb) n
341 MACIOIDEState *m = io->opaque;
342 IDEState *s = idebus_active_if(&m->bus);
344 MACIO_DPRINTF("pmac_ide_transfer%s lba=%x, buffer_index=%x,
len=%x\n",
pmac_ide_transfer(ATAPI) lba=, buffer_index=0, len=324
348 s->io_buffer_size = 0;
349 if (s->drive_kind == IDE_CD) {
350 bdrv_acct_start(s->bs, &s->acct, io->len, BDRV_ACCT_READ);
351 pmac_ide_atapi_transfer_cb(io, 0);
(gdb) s
pmac_ide_atapi_transfer_cb (opaque=0x563d1068, ret=0) at hw/ide/macio.c:55
55 {
56 DBDMA_io *io = opaque;
57 MACIOIDEState *m = io->opaque;
58 IDEState *s = idebus_active_if(&m->bus);
61 if (ret < 0) {
69 if (!m->dma_active) {
77 MACIO_DPRINTF("io_buffer_size = %#x\n", s->io_buffer_size);
io_buffer_size = 0
79 if (s->io_buffer_size > 0) {
90 s->io_buffer_size = MIN(io->len, s->packet_transfer_size);
92 MACIO_DPRINTF("remainder: %d io->len: %d size: %d\n",
io->remainder_len,
remainder: 0 io->len: 804 size: 20
94 if (io->remainder_len && io->len) {
114 if (s->packe
Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report
On Thu, 15 May 2014, BALATON Zoltan wrote:
confusing.) Do you think that replacing io->len in your patch with
s->io_buffer_size would be the correct thing to do?
Probably that's not enough. I've tried it and then it gets to here:
end of non-IO ATAPI DMA transfer
122 if (!s->packet_transfer_size) {
(gdb) p s->packet_transfer_size
$1 = 0
(gdb) n
123 MACIO_DPRINTF("end of transfer\n");
end of transfer
124 ide_atapi_cmd_ok(s);
125 m->dma_active = false;
128 if (io->len == 0) {
(gdb) p io->len
$2 = 804
(gdb) l
123 MACIO_DPRINTF("end of transfer\n");
124 ide_atapi_cmd_ok(s);
125 m->dma_active = false;
126 }
127
128 if (io->len == 0) {
129 MACIO_DPRINTF("end of DMA\n");
130 goto done;
131 }
132
133 /* launch next transfer */
134
135 /* handle unaligned accesses first, get them over with and only do
the
136remaining bulk transfer using our async DMA helpers */
137 unaligned = io->len & 0x1ff;
138 if (unaligned) {
139 int sector_num = (s->lba << 2) + (s->io_buffer_index >> 9);
140 int nsector = io->len >> 9;
141
142 MACIO_DPRINTF("precopying unaligned %d bytes to %#" HWADDR_PRIx
"\n",
(gdb) c
Continuing.
precopying unaligned 292 bytes to 0xe4f50c
io->len = 0x200
set remainder to: -20
sector_num=-4 size=0, cmd_cmd=0
atapi_cmd_error: sense=0x5 asc=0x21
done DMA
DBDMA: dbdma_end
Regards,
BALATON Zoltan
Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report
On Thu, 15 May 2014, BALATON Zoltan wrote:
On Thu, 15 May 2014, BALATON Zoltan wrote:
confusing.) Do you think that replacing io->len in your patch with
s->io_buffer_size would be the correct thing to do?
Probably that's not enough. I've tried it and then it gets to here:
I should've also included these lines too to make it more clear what did I
change:
114 if (s->packet_transfer_size && s->lba == -1) {
115 MACIO_DPRINTF("non-IO ATAPI DMA transfer size: %d\n", io->len);
non-IO ATAPI DMA transfer size: 804
117 cpu_physical_memory_write(io->addr, s->io_buffer,
s->io_buffer_size);
118 s->packet_transfer_size -= s->io_buffer_size;
119 MACIO_DPRINTF("end of non-IO ATAPI DMA transfer\n");
end of non-IO ATAPI DMA transfer
122 if (!s->packet_transfer_size) {
(gdb) p s->packet_transfer_size
$1 = 0
(gdb) n
123 MACIO_DPRINTF("end of transfer\n");
end of transfer
124 ide_atapi_cmd_ok(s);
125 m->dma_active = false;
128 if (io->len == 0) {
(gdb) p io->len
$2 = 804
(gdb) l
123 MACIO_DPRINTF("end of transfer\n");
124 ide_atapi_cmd_ok(s);
125 m->dma_active = false;
126 }
127
128 if (io->len == 0) {
129 MACIO_DPRINTF("end of DMA\n");
130 goto done;
131 }
132
133 /* launch next transfer */
134
135 /* handle unaligned accesses first, get them over with and only
do the
136remaining bulk transfer using our async DMA helpers */
137 unaligned = io->len & 0x1ff;
138 if (unaligned) {
139 int sector_num = (s->lba << 2) + (s->io_buffer_index >> 9);
140 int nsector = io->len >> 9;
141
142 MACIO_DPRINTF("precopying unaligned %d bytes to %#"
HWADDR_PRIx "\n",
(gdb) c
Continuing.
precopying unaligned 292 bytes to 0xe4f50c
io->len = 0x200
set remainder to: -20
sector_num=-4 size=0, cmd_cmd=0
atapi_cmd_error: sense=0x5 asc=0x21
done DMA
DBDMA: dbdma_end
Regards,
BALATON Zoltan
Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report
On Fri, 16 May 2014, Mark Cave-Ayland wrote: Perhaps we need to assume for a non-IO DMA request that the result will only be a single ATAPI reply packet? Attached is another version of the patch for you to experiment with which makes your s->io_buffer_size change but also moves the logic into pmac_ide_transfer() so that we don't inadvertently drop into the unaligned code. This seems to have worked and now it mounts its boot cd and seems to boot afterwards. At least it generated a lot of logs that I'm currently looking at which end with an error about failing to open the display so it looks good so far. (I'm sure there are more problems ahead but hopefully I can now look at the display problem which is the next big thing.) Thank you very much for helping with this. You have saved me a lot of time by providing this patch so I did not have to dig up all the pieces that are scattered in the qemu sources that are involved in this DMA transfer. Understanding it conceptually is one thing but finding all the details in the several files involved to get the details is more difficult. Regards, BALATON Zoltan
Re: [Qemu-devel] Help needed testing on ppc
On Wed, 7 May 2014, Tom Musta wrote: On 5/6/2014 6:17 PM, BALATON Zoltan wrote: On Tue, 6 May 2014, Tom Musta wrote: On 5/6/2014 5:03 AM, BALATON Zoltan wrote: I'd appreciate some insight and help. [snip] (1) Why is MorphOS using this invalid instruction form? Would it be easier to fix the OS rather than QEMU? I don't know why is it used. I can ask the MorphOS developers but they did not seem to be too supportive so far and at least one of them expressed that they have no interest supporting other than their officially supported list of hardware at this time. So I assume it is easier to fix QEMU than MorphOS and if it works on a real Mac then it should also work on QEMU's emulation of that Mac hardware. Is there some undocumented processor behavior that the code is dependent upon (e.g. is it actually expected CR0 to be set?). This is what the testing was supposed to find out but MorphOS seems to run better with the quoted patch so I don't think it depends on any other undocumented behaviour other than ignoring reserved bits but I have no definitive answer. It still seems to me that setting a reserved instruction bit is an strange thing to do. It would be nice to at least have a justification from MorphOS. It is possible that no one even knows the answer. I've tried to ask them about this but all I got as an answer was that running on QEMU is not supported so I take it that they don't know or won't tell. But it's also very unlikely they would change it in MorphOS so it seems it is not easier to fix in MorphOS. (2) Your patch makes some store instructions compliant with the most recent ISAs but there are many other instructions that are not addressed by the patch. I think fixing only some will be a future source of confusion.>> Alex: do you have an opinion on this? Are you OK with changing masks for a few stores but not all instructions in general? I've got a bit further and came across another invalid instruction exception: invalid bits: 0200 for opcode: 1f - 16 - 0b (7e04caec) 1020574c I don't know if this is another case of using reserved bits or something else though. Regards, BALATON Zoltan
Re: [Qemu-devel] [PATCH 3/5] vmware-vga: use vmsvga_verify_rect in vmsvga_update_rect
On Tue, 14 Oct 2014, Gerd Hoffmann wrote:
Switch vmsvga_update_rect over to use vmsvga_verify_rect. Slight change
in behavior: We don't try to automatically fixup rectangles any more.
Invalid update requests will be ignored instead.
Are you sure this won't break clients? I remember that maybe Windows
drivers did produce requests with partially off screen rectangles for
objects that are partially visible. I don't recall if this was for windows
dragged off screen or mouse pointer near the screen but there was a reason
this fixup was added. Did you test this?
Regards,
BALATON Zoltan
Cc: qemu-sta...@nongnu.org
Signed-off-by: Gerd Hoffmann
---
hw/display/vmware_vga.c | 32 ++--
1 file changed, 2 insertions(+), 30 deletions(-)
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index fc0a2a7..7564f88 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -356,36 +356,8 @@ static inline void vmsvga_update_rect(struct
vmsvga_state_s *s,
uint8_t *src;
uint8_t *dst;
-if (x < 0) {
-fprintf(stderr, "%s: update x was < 0 (%d)\n", __func__, x);
-w += x;
-x = 0;
-}
-if (w < 0) {
-fprintf(stderr, "%s: update w was < 0 (%d)\n", __func__, w);
-w = 0;
-}
-if (x + w > surface_width(surface)) {
-fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
-__func__, x, w);
-x = MIN(x, surface_width(surface));
-w = surface_width(surface) - x;
-}
-
-if (y < 0) {
-fprintf(stderr, "%s: update y was < 0 (%d)\n", __func__, y);
-h += y;
-y = 0;
-}
-if (h < 0) {
-fprintf(stderr, "%s: update h was < 0 (%d)\n", __func__, h);
-h = 0;
-}
-if (y + h > surface_height(surface)) {
-fprintf(stderr, "%s: update height too large y: %d, h: %d\n",
-__func__, y, h);
-y = MIN(y, surface_height(surface));
-h = surface_height(surface) - y;
+if (!vmsvga_verify_rect(surface, __func__, x, y, w, h)) {
+return;
}
bypl = surface_stride(surface);
--
1.8.3.1
Re: [Qemu-devel] OVMF, Q35 and USB keyboard/mouse
On Thu, 11 Sep 2014, Gabriel L. Somlo wrote: On Thu, Sep 11, 2014 at 11:34:03PM +0200, Alexander Graf wrote: XNU also populates its device tree based on the DSDT. Maybe there's a subtle difference there? This was the low hanging fruit, so I checked it first :) Pulled the DSDT using the OS X version of "DSDTEditor" (found on insanelymac) on both the Chameleon q35 version which had all three UHCIs and from the OVMF q35 version which only showed UHCI3. The two DSDTs looked absolutely identical (no output from diff)... You may also look for differences in USB devices in the ouput of ioreg. Maybe that shows something. (It's basically what you can see in System Profiler but easier to compare with diff and may have additional data.) Regards, BALATON Zoltan
[Qemu-devel] commit 56bd9ea1a breaks display
Hello, It looks like that commit 56bd9ea1a37395012adecca8b9c4762da15b01e7 (console: reimplement qemu_default_pixelformat) breaks display with qemu-system-ppc on x86_64 host with SDL. The window just stays black after this commit (although the machine still runs, it cannot be seen). Can you check please? Regards, BALATON Zoltan
Re: [Qemu-devel] OVMF, Q35 and USB keyboard/mouse
On Mon, 15 Sep 2014, Gabriel L. Somlo wrote: mappings should end up being the same. I'll look for as close an equivalent of "cat /proc/interrupts" on os x as I can find, and try to sanity-check this assumption. I think you can find these in the ioreg output. Try ioreg -lfw0 and look for IOInterruptSpecifiers or you can try different IORegistryPlanes with the -p option (I don't have OS X at hand to test). Alternatively there's a GUI tool to get the same info called IORegistryExplorer that used to be part of older XCode versions, not sure where can you get it now. Regards, BALATON Zoltan
[Qemu-devel] [PATCH] mac99: Change memory layout to better match PowerMac3, 1
Bring the memory map closer to a PowerMac3,1 model by removing unused areas and adding the VGA and network cards after the macio to let the latter be mapped from 0x8000 like on real hardware. (On real hardware the graphics and network cards are on separate buses but we don't model that yet.) Signed-off-by: BALATON Zoltan --- This patch is intended to bring memory layout closer to what's seen in these dumps: http://nandra.segv.jp/NetBSD/G4.dump-device-tree.txt http://raveland.org/ports/eeprom.txt http://mail-index.netbsd.org/port-macppc/2007/10/24/.html https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=604134 --- hw/pci-host/uninorth.c | 2 +- hw/ppc/mac_newworld.c | 20 +++- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c index e72fe2a..21f805f 100644 --- a/hw/pci-host/uninorth.c +++ b/hw/pci-host/uninorth.c @@ -230,7 +230,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic, d = UNI_NORTH_PCI_HOST_BRIDGE(dev); memory_region_init(&d->pci_mmio, OBJECT(d), "pci-mmio", 0x1ULL); memory_region_init_alias(&d->pci_hole, OBJECT(d), "pci-hole", &d->pci_mmio, - 0x8000ULL, 0x7000ULL); + 0x8000ULL, 0x1000ULL); memory_region_add_subregion(address_space_mem, 0x8000ULL, &d->pci_hole); diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c index 5e79575..a533ec7 100644 --- a/hw/ppc/mac_newworld.c +++ b/hw/ppc/mac_newworld.c @@ -154,7 +154,9 @@ static void ppc_core99_init(QEMUMachineInitArgs *args) qemu_irq *pic, **openpic_irqs; MemoryRegion *isa = g_new(MemoryRegion, 1); MemoryRegion *unin_memory = g_new(MemoryRegion, 1); +#if 0 MemoryRegion *unin2_memory = g_new(MemoryRegion, 1); +#endif int linux_boot, i, j, k; MemoryRegion *ram = g_new(MemoryRegion, 1), *bios = g_new(MemoryRegion, 1); hwaddr kernel_base, initrd_base, cmdline_base = 0; @@ -296,10 +298,10 @@ static void ppc_core99_init(QEMUMachineInitArgs *args) /* UniN init: XXX should be a real device */ memory_region_init_io(unin_memory, NULL, &unin_ops, token, "unin", 0x1000); memory_region_add_subregion(get_system_memory(), 0xf800, unin_memory); - +#if 0 memory_region_init_io(unin2_memory, NULL, &unin_ops, token, "unin", 0x1000); memory_region_add_subregion(get_system_memory(), 0xf300, unin2_memory); - +#endif openpic_irqs = g_malloc0(smp_cpus * sizeof(qemu_irq *)); openpic_irqs[0] = g_malloc0(smp_cpus * sizeof(qemu_irq) * OPENPIC_OUTPUT_NB); @@ -371,18 +373,11 @@ static void ppc_core99_init(QEMUMachineInitArgs *args) machine_arch = ARCH_MAC99; } /* init basic PC hardware */ -pci_vga_init(pci_bus); - escc_mem = escc_init(0, pic[0x25], pic[0x24], serial_hds[0], serial_hds[1], ESCC_CLOCK, 4); memory_region_init_alias(escc_bar, NULL, "escc-bar", escc_mem, 0, memory_region_size(escc_mem)); -for(i = 0; i < nb_nics; i++) -pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); - -ide_drive_get(hd, MAX_IDE_BUS); - macio = pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO); dev = DEVICE(macio); qdev_connect_gpio_out(dev, 0, pic[0x19]); /* CUDA */ @@ -393,6 +388,8 @@ static void ppc_core99_init(QEMUMachineInitArgs *args) macio_init(macio, pic_mem, escc_bar); /* We only emulate 2 out of 3 IDE controllers for now */ +ide_drive_get(hd, MAX_IDE_BUS); + macio_ide = MACIO_IDE(object_resolve_path_component(OBJECT(macio), "ide[0]")); macio_ide_init_drives(macio_ide, hd); @@ -418,9 +415,14 @@ static void ppc_core99_init(QEMUMachineInitArgs *args) } } +pci_vga_init(pci_bus); + if (graphic_depth != 15 && graphic_depth != 32 && graphic_depth != 8) graphic_depth = 15; +for(i = 0; i < nb_nics; i++) +pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL); + /* The NewWorld NVRAM is not located in the MacIO device */ dev = qdev_create(NULL, TYPE_MACIO_NVRAM); qdev_prop_set_uint32(dev, "size", 0x2000); -- 1.8.1.5
Re: [Qemu-devel] [PATCH] po/hu.po: Hungarian translation for the GTK+ interface
On Sun, 5 May 2013, Laszlo Ersek wrote:
On 05/04/13 17:22, akoskov...@gmx.com wrote:
+#: ../ui/gtk.c:217
+msgid " [Paused]"
+msgstr " [Leállítva]"
Hrmph. Yet another overloaded word. "Leállítva" can just as well mean
"shut down" (adverb or adjective, not the verb). I'm tempted to
recommend "felfüggesztve" (~= suspended), but that of course overlaps
with the power management kind of "suspend".
I think " [Megállítva]" is better here or maybe "befagyasztva" could be
considered which means frozen that probably does not collide with other
usages like suspend. Your latest suggestion of "szüneteltetve",
"szüneteltetés" is also good and less confusing than "leállítva" and
even more distinct from it then "megállítva" so that may be even better.
IIRC the pause button on remote controls was explained as "pillanatállj"
in old manuals ("momentary stop"). Not bad from the meaning side but it
sounds super lame, plus I have no idea how to make an adverb/abjective
out of "pillanatállj" -- "egy pillanatra megállítva"? Tragic.
I have never ever heard anyone actually using that word other than in
translated manuals. Probably for a reason...
+#: ../ui/gtk.c:1312
+msgid "Grab On _Hover"
+msgstr "Automatikus _elfogás"
Good idea.
+
+#: ../ui/gtk.c:1315
+msgid "_Grab Input"
+msgstr "_Bemenet elfogása"
"Bemeneti eszközök kisajátítása" is more accurate but of course sounds
horrible...
How about "megragadás/a" for a translation of grab? But probably it sounds
more horrible than your suggestion which is not so bad IMO. The problem
with all of these is that they are not obvious to someone not
knowledgeable about the concept but that may be true for the English term
too.
Regards,
BALATON Zoltan
Re: [Qemu-devel] [PATCH v2] po/hu.po: Hungarian translation for the GTK+ interface
On Mon, 6 May 2013, akoskov...@gmx.com wrote: +#: ../ui/gtk.c:1312 +msgid "Grab On _Hover" +msgstr "Automatikus _elfogás" + +#: ../ui/gtk.c:1315 +msgid "_Grab Input" +msgstr "_Bemeneti eszközök megragadása" Sorry for nitpicking but for consistency your should use the same term for grab everywhere so "Grab On Hover" should probably better be translated as "Bemenet automatikus megragadása". However it's fine with me either way. Regards, BALATON Zoltan
[Qemu-devel] [PATCH] serial-pci: Set prog interface field of pci config to 16550 compatible
Signed-off-by: BALATON Zoltan --- hw/char/serial-pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c index 991c99f..e662b77 100644 --- a/hw/char/serial-pci.c +++ b/hw/char/serial-pci.c @@ -60,6 +60,7 @@ static int serial_pci_init(PCIDevice *dev) return -1; } +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; s->irq = pci_allocate_irq(&pci->dev); -- 1.8.1.5
[Qemu-devel] [PATCH] q35: Correct typo BRDIGE -> BRIDGE
Signed-off-by: BALATON Zoltan
---
hw/pci-host/q35.c | 6 +++---
include/hw/i386/ich9.h| 2 +-
include/hw/pci-host/q35.h | 24
3 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 4bc2e01..eda64c5 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -306,8 +306,8 @@ static void mch_write_config(PCIDevice *d,
mch_update_pciexbar(mch);
}
-if (ranges_overlap(address, len, MCH_HOST_BRDIGE_SMRAM,
- MCH_HOST_BRDIGE_SMRAM_SIZE)) {
+if (ranges_overlap(address, len, MCH_HOST_BRIDGE_SMRAM,
+ MCH_HOST_BRIDGE_SMRAM_SIZE)) {
mch_update_smram(mch);
}
}
@@ -347,7 +347,7 @@ static void mch_reset(DeviceState *qdev)
pci_set_quad(d->config + MCH_HOST_BRIDGE_PCIEXBAR,
MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT);
-d->config[MCH_HOST_BRDIGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT;
+d->config[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT;
mch_update(mch);
}
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 9e4a0e4..e191435 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -102,7 +102,7 @@ Object *ich9_lpc_find(void);
#define ICH9_USB_UHCI1_DEV 29
#define ICH9_USB_UHCI1_FUNC 0
-/* D30:F0 DMI-to-PCI brdige */
+/* D30:F0 DMI-to-PCI bridge */
#define ICH9_D2P_BRIDGE "ICH9 D2P BRIDGE"
#define ICH9_D2P_BRIDGE_SAVEVM_VERSION 0
diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index d0355b7..d9ee978 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -125,8 +125,8 @@ typedef struct Q35PCIHost {
#define MCH_HOST_BRIDGE_PAM_RE ((uint8_t)0x1)
#define MCH_HOST_BRIDGE_PAM_MASK ((uint8_t)0x3)
-#define MCH_HOST_BRDIGE_SMRAM 0x9d
-#define MCH_HOST_BRDIGE_SMRAM_SIZE 1
+#define MCH_HOST_BRIDGE_SMRAM 0x9d
+#define MCH_HOST_BRIDGE_SMRAM_SIZE 1
#define MCH_HOST_BRIDGE_SMRAM_DEFAULT ((uint8_t)0x2)
#define MCH_HOST_BRIDGE_SMRAM_D_OPEN ((uint8_t)(1 << 6))
#define MCH_HOST_BRIDGE_SMRAM_D_CLS((uint8_t)(1 << 5))
@@ -140,16 +140,16 @@ typedef struct Q35PCIHost {
#define MCH_HOST_BRIDGE_UPPER_SYSTEM_BIOS_END 0x10
#define MCH_HOST_BRIDGE_ESMRAMC0x9e
-#define MCH_HOST_BRDIGE_ESMRAMC_H_SMRAME ((uint8_t)(1 << 6))
-#define MCH_HOST_BRDIGE_ESMRAMC_E_SMERR((uint8_t)(1 << 5))
-#define MCH_HOST_BRDIGE_ESMRAMC_SM_CACHE ((uint8_t)(1 << 4))
-#define MCH_HOST_BRDIGE_ESMRAMC_SM_L1 ((uint8_t)(1 << 3))
-#define MCH_HOST_BRDIGE_ESMRAMC_SM_L2 ((uint8_t)(1 << 2))
-#define MCH_HOST_BRDIGE_ESMRAMC_TSEG_SZ_MASK ((uint8_t)(0x3 << 1))
-#define MCH_HOST_BRDIGE_ESMRAMC_TSEG_SZ_1MB((uint8_t)(0x0 << 1))
-#define MCH_HOST_BRDIGE_ESMRAMC_TSEG_SZ_2MB((uint8_t)(0x1 << 1))
-#define MCH_HOST_BRDIGE_ESMRAMC_TSEG_SZ_8MB((uint8_t)(0x2 << 1))
-#define MCH_HOST_BRDIGE_ESMRAMC_T_EN ((uint8_t)1)
+#define MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME ((uint8_t)(1 << 6))
+#define MCH_HOST_BRIDGE_ESMRAMC_E_SMERR((uint8_t)(1 << 5))
+#define MCH_HOST_BRIDGE_ESMRAMC_SM_CACHE ((uint8_t)(1 << 4))
+#define MCH_HOST_BRIDGE_ESMRAMC_SM_L1 ((uint8_t)(1 << 3))
+#define MCH_HOST_BRIDGE_ESMRAMC_SM_L2 ((uint8_t)(1 << 2))
+#define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_MASK ((uint8_t)(0x3 << 1))
+#define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_1MB((uint8_t)(0x0 << 1))
+#define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_2MB((uint8_t)(0x1 << 1))
+#define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_8MB((uint8_t)(0x2 << 1))
+#define MCH_HOST_BRIDGE_ESMRAMC_T_EN ((uint8_t)1)
/* D1:F0 PCIE* port*/
#define MCH_PCIE_DEV 1
--
1.8.1.5
[Qemu-devel] [PATCH v2] q35: Correct typo BRDIGE -> BRIDGE
Signed-off-by: BALATON Zoltan
---
v2: Sorry, I was too fast to send it. Found two more places to change.
hw/pci-host/q35.c | 10 +-
include/hw/i386/ich9.h| 2 +-
include/hw/pci-host/q35.h | 24
3 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 4bc2e01..8b8cc4e 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -272,7 +272,7 @@ static void mch_update_smram(MCHPCIState *mch)
PCIDevice *pd = PCI_DEVICE(mch);
memory_region_transaction_begin();
-smram_update(&mch->smram_region, pd->config[MCH_HOST_BRDIGE_SMRAM],
+smram_update(&mch->smram_region, pd->config[MCH_HOST_BRIDGE_SMRAM],
mch->smm_enabled);
memory_region_transaction_commit();
}
@@ -283,7 +283,7 @@ static void mch_set_smm(int smm, void *arg)
PCIDevice *pd = PCI_DEVICE(mch);
memory_region_transaction_begin();
-smram_set_smm(&mch->smm_enabled, smm, pd->config[MCH_HOST_BRDIGE_SMRAM],
+smram_set_smm(&mch->smm_enabled, smm, pd->config[MCH_HOST_BRIDGE_SMRAM],
&mch->smram_region);
memory_region_transaction_commit();
}
@@ -306,8 +306,8 @@ static void mch_write_config(PCIDevice *d,
mch_update_pciexbar(mch);
}
-if (ranges_overlap(address, len, MCH_HOST_BRDIGE_SMRAM,
- MCH_HOST_BRDIGE_SMRAM_SIZE)) {
+if (ranges_overlap(address, len, MCH_HOST_BRIDGE_SMRAM,
+ MCH_HOST_BRIDGE_SMRAM_SIZE)) {
mch_update_smram(mch);
}
}
@@ -347,7 +347,7 @@ static void mch_reset(DeviceState *qdev)
pci_set_quad(d->config + MCH_HOST_BRIDGE_PCIEXBAR,
MCH_HOST_BRIDGE_PCIEXBAR_DEFAULT);
-d->config[MCH_HOST_BRDIGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT;
+d->config[MCH_HOST_BRIDGE_SMRAM] = MCH_HOST_BRIDGE_SMRAM_DEFAULT;
mch_update(mch);
}
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 9e4a0e4..e191435 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -102,7 +102,7 @@ Object *ich9_lpc_find(void);
#define ICH9_USB_UHCI1_DEV 29
#define ICH9_USB_UHCI1_FUNC 0
-/* D30:F0 DMI-to-PCI brdige */
+/* D30:F0 DMI-to-PCI bridge */
#define ICH9_D2P_BRIDGE "ICH9 D2P BRIDGE"
#define ICH9_D2P_BRIDGE_SAVEVM_VERSION 0
diff --git a/include/hw/pci-host/q35.h b/include/hw/pci-host/q35.h
index d0355b7..d9ee978 100644
--- a/include/hw/pci-host/q35.h
+++ b/include/hw/pci-host/q35.h
@@ -125,8 +125,8 @@ typedef struct Q35PCIHost {
#define MCH_HOST_BRIDGE_PAM_RE ((uint8_t)0x1)
#define MCH_HOST_BRIDGE_PAM_MASK ((uint8_t)0x3)
-#define MCH_HOST_BRDIGE_SMRAM 0x9d
-#define MCH_HOST_BRDIGE_SMRAM_SIZE 1
+#define MCH_HOST_BRIDGE_SMRAM 0x9d
+#define MCH_HOST_BRIDGE_SMRAM_SIZE 1
#define MCH_HOST_BRIDGE_SMRAM_DEFAULT ((uint8_t)0x2)
#define MCH_HOST_BRIDGE_SMRAM_D_OPEN ((uint8_t)(1 << 6))
#define MCH_HOST_BRIDGE_SMRAM_D_CLS((uint8_t)(1 << 5))
@@ -140,16 +140,16 @@ typedef struct Q35PCIHost {
#define MCH_HOST_BRIDGE_UPPER_SYSTEM_BIOS_END 0x10
#define MCH_HOST_BRIDGE_ESMRAMC0x9e
-#define MCH_HOST_BRDIGE_ESMRAMC_H_SMRAME ((uint8_t)(1 << 6))
-#define MCH_HOST_BRDIGE_ESMRAMC_E_SMERR((uint8_t)(1 << 5))
-#define MCH_HOST_BRDIGE_ESMRAMC_SM_CACHE ((uint8_t)(1 << 4))
-#define MCH_HOST_BRDIGE_ESMRAMC_SM_L1 ((uint8_t)(1 << 3))
-#define MCH_HOST_BRDIGE_ESMRAMC_SM_L2 ((uint8_t)(1 << 2))
-#define MCH_HOST_BRDIGE_ESMRAMC_TSEG_SZ_MASK ((uint8_t)(0x3 << 1))
-#define MCH_HOST_BRDIGE_ESMRAMC_TSEG_SZ_1MB((uint8_t)(0x0 << 1))
-#define MCH_HOST_BRDIGE_ESMRAMC_TSEG_SZ_2MB((uint8_t)(0x1 << 1))
-#define MCH_HOST_BRDIGE_ESMRAMC_TSEG_SZ_8MB((uint8_t)(0x2 << 1))
-#define MCH_HOST_BRDIGE_ESMRAMC_T_EN ((uint8_t)1)
+#define MCH_HOST_BRIDGE_ESMRAMC_H_SMRAME ((uint8_t)(1 << 6))
+#define MCH_HOST_BRIDGE_ESMRAMC_E_SMERR((uint8_t)(1 << 5))
+#define MCH_HOST_BRIDGE_ESMRAMC_SM_CACHE ((uint8_t)(1 << 4))
+#define MCH_HOST_BRIDGE_ESMRAMC_SM_L1 ((uint8_t)(1 << 3))
+#define MCH_HOST_BRIDGE_ESMRAMC_SM_L2 ((uint8_t)(1 << 2))
+#define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_MASK ((uint8_t)(0x3 << 1))
+#define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_1MB((uint8_t)(0x0 << 1))
+#define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_2MB((uint8_t)(0x1 << 1))
+#define MCH_HOST_BRIDGE_ESMRAMC_TSEG_SZ_8MB((uint8_t)(0x2 << 1))
+#define MCH_HOST_BRIDGE_ESMRAMC_T_EN ((uint8_t)1)
/* D1:F0 PCIE* port*/
#define MCH_PCIE_DEV 1
--
1.8.1.5
Re: [Qemu-devel] [PATCH v2] cpu: implementing victim TLB for QEMUsystem emulated TLBB
On Fri, 24 Jan 2014, Alex Bennée wrote: trent.t...@gmail.com writes: Attaching data in excel which could not be sent with the patch at the same time. If you can attach the summary of the data as plain text that would be useful. Not all of us have access to a Windows box with Excell! Opens on LibreOffice as well but plain text is easier to read. I wonder how much of the measured performance increase is due to the increased effective cache size and how much is the reduction in number of instructions by the victim TLB mechanism. Wouldn't it be a more fair measurement to use a bigger TLB size for the TLB only case that matches the effective size of the TLB+victim case instead of using the same TLB size for both?
Re: [Qemu-devel] AmigaOS 4.x on QEMU
On Sun, 12 Jan 2014, Alpha Mule wrote: Hi. AmigaOS 4.x runs natively on some niche PowerPC boards. I was wondering about the viability of running AmigaOS 4.x on QEMU. Specifically, I was wondering if there has been any development on that and/or what needs to be done to get it fully working. Any information would be appreciated. Thanks. Looks like not much info is available on this... I think that the first difficulty is that AmigaOS 4.x is not freely available (AFAIK) so it's hard to do any testing with it. I've found that a demo version of MorphOS (which is not the same but runs on similar hardware) is available and runs on some Mac hardware that may be closer to what qemu already has support for so it may be an easier target to test. I've tried to boot it on qemu but haven't got very far and I think qemu is missing some features MorphOS needs so it may not be easy to make it work. Here's what I've found: 1. The CPU type MorphOS seems to prefer is G4 7447A. This CPU type may not be fully supported by qemu yet and specifying it with -cpu makes the VM hang in the openbios firmware before it even gets to boot the OS. By using the default G4 CPU without any -cpu option it gets further and hangs later in the OS's boot loader. 2. None of the video cards MorphOS can work with is emulated by qemu and I think those qemu has do not work with MorphOS. So even if it was running otherwise there would be no display. Theoretically this may be got around (for testing at least) with the PCI pass-through of an appropriate card from the host but I don't have such a video card at hand and don't know if this would be possible in practice at all. 3. The farthest I got is with the 'boot cd:,\mac_ppc32\boot.img' command line from openbios which seems to at least start to boot but hangs without any messages (even on the serial console) quite early and I don't know enough to debug this further. All I got was some debug logs which seem to go astray around here: IN: 0x00441b14: dcbtst r8,r10 0x00441b18: stw r5,4(r10) 0x00441b1c: stw r5,8(r10) 0x00441b20: stw r5,12(r10) 0x00441b24: stw r5,16(r10) 0x00441b28: stw r5,20(r10) 0x00441b2c: stw r5,24(r10) 0x00441b30: stw r5,28(r10) 0x00441b34: stwur5,32(r10) 0x00441b38: bdnz+ 0x441b14 Raise exception at 00441b18 => 0002 (00) invalid/unsupported opcode: 00 - 00 - 00 () 0080 0 IN: 0x0034: addir1,r1,16 0x0038: lwz r0,52(r1) 0x003c: mtlrr0 0x0040: lwz r0,56(r1) 0x0044: mtcrr0
Re: [Qemu-devel] osx bootloader
On Tue, 28 Jan 2014, Gabriel L. Somlo wrote: BTW, when you say "use Apple's EFI bootloader" what do you mean ? Is that some piece of software we'd have to download and build, like Chameleon? Or would TianoCore in theory be capable of doing what the Apple EFI Bios does and load XNU from the hard drive image directly, and there's some default bootloader placed there by the OS X installer, kinda like grub in the Linux world ? If the latter, I think understanding how Chameleon does that part would come in handy (for me at least, since I don't have a good mental map on how things work before the XNU kernel starts running). Here are some pages that might help enlightening the boot process: https://developer.apple.com/library/mac/documentation/Darwin/Conceptual/KernelProgramming/booting/booting.html http://homepage.ntlworld.com/jonathan.deboynepollard/FGA/efi-boot-process.html#Apple http://developer.apple.com./mac/library/DOCUMENTATION/Darwin/References/ManPages/man8/bless.8.html Regards, BALATON Zoltan
Re: [Qemu-devel] osx bootloader
On Tue, 28 Jan 2014, Gabriel L. Somlo wrote: But now I think it's starting to make sense. Chameleon's stage2 loader, named "boot" (which we're starting via "-kernel boot", and which is the only piece of Chameleon we need) is apparently equivalent to Apple's "boot.efi", which it bypasses, and which I think is what AFAIK Chameleon's boot is based on Apple's older, pre-EFI boot loader (http://opensource.apple.com/source/boot/boot-132/) that performs similar functions but does not require EFI and works on BIOS based machines. It is updated to work with recent EFI-only releases, also provides some EFI stuff that's needed by OS X (fake_efi) and has some means of patching up non-Apple hardware to work with OS X. To make sure it's not trying to patch anything try to remove all modules (may need to recompile to get rid of built-in modules) and make sure all *Enabler options are disabled. Alex was talking about (i.e., eventually getting TianoCore to pick up and run "boot.efi" as left behind by the vanilla OS X installer). I've also found this: http://sourceforge.net/apps/mediawiki/tianocore/index.php?title=OvmfPkg which might be what we need for using Apple's boot.efi but I don't know how ready is it and if it works with OS X. Regards, BALATON Zoltan
Re: [Qemu-devel] osx bootloader
On Tue, 28 Jan 2014, Gabriel L. Somlo wrote: Thanks again for the helpful links !!! Here are some more that can be helpful: http://www.rodsbooks.com/refind/drivers.html about some EFI drivers that can make an EFI implementation able to read an HFS+ volume (rEFInd is a boot manager for EFI that can select and load os boot loaders) http://clover-wiki.zetam.org/What-is-what about Clover boot process and components (Clover is similar to Chameleon but with a different approach providing an EFI environment first and having other functions in EFI programs thus if your firmware is EFI then the first part is not needed.) From these parts it should be possible to create a minimal boot loader for Qemu that can load OS X but it may not be completely trivial. Regards, BALATON Zoltan
Re: [Qemu-devel] osx bootloader
On Wed, 29 Jan 2014, Alexander Graf wrote: On 01/29/2014 03:53 PM, Gabriel L. Somlo wrote: I managed to boot OVMF following their wiki; It seems to work with kvm enabled, but not with -M q35. My current command line is: Did you use the snapshots on the wiki or compiled the latest version from source? It seems the snapshots are quite old and there have been recent commits since. (I don't know if those fixed or broke things though.) So I guess I have a bit more RTFM ahead of me. Maybe I can find a way to copy boot.efi over to the actual EFI partition, which I assume is the only one I can explore with OVMF from the entire disk image... Linux can read hfs+ file systems and the EFI partition can also be mounted and written to on OS X from the command line so it's easy to copy boot.efi over to the EFI partition either on Linux or OS X. You will need an EFI HFS+ driver (look at the refind pointer) because boot.efi will use EFI callbacks to read the kernel and kext cache. Right. Hopefully it does not rely on Apple's specific HFS+ support in their firmware or that's compatible with what (at least one of) the EFI drivers provide. Regards, BALATON Zoltan
Re: [Qemu-devel] osx bootloader
On Wed, 29 Jan 2014, Gabriel L. Somlo wrote: I managed to boot OVMF following their wiki; It seems to work with kvm enabled, but not with -M q35. My current command line is: I've tried with the version compiled from the edk2 HEAD and that also fails with -M q35. I've managed to get some debug output via the options described in OVMF's readme file and the error is around where initialising the graphics card. With q35 it stops with: ASSERT .../edk2/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c(163): (Port & 3) == 0 This function is IoRead32 and it says the port must be 32-bit aligned. I've tried to find what port it tries to access but qemu -d ioport option generates no output for me. Any hints on how to use this debug option of qemu? Regards, BALATON Zoltan
Re: [Qemu-devel] OVMF with q35 (was: osx bootloader)
0xFFF; Owner = PCI [00|1F|02:24] Base = 0x82021000; Length = 0x1000;Alignment = 0xFFF; Owner = PCI [00|01|00:14] [...] IoWrite32 CF8 80001020 IoRead32 CFC IoWrite32 CF8 IoRead32 CF8 IoWrite32 CF8 80001024 IoRead32 CFC IoWrite32 CF8 IoRead32 CF8 IoWrite32 CF8 80001004 IoWrite32 CF8 IoRead32 CF8 IoWrite32 CF8 80001004 IoWrite32 CF8 IoRead32 CF8 IoWrite32 CF8 80001004 IoWrite32 CF8 IoRead32 CF8 IoWrite32 CF8 80001004 IoWrite32 CF8 IoRead32 CF8 IoWrite32 CF8 80001004 IoWrite32 CF8 IoRead32 CF8 IoWrite32 CF8 80001004 IoWrite32 CF8 IoRead32 CF8 IoWrite32 CF8 80001004 IoWrite32 CF8 IoRead32 CF8 IoWrite32 CF8 8000100C IoWrite32 CF8 IoRead32 CF8 IoWrite32 CF8 8000100C IoWrite32 CF8 IoRead32 CF8 IoWrite32 CF8 8B40 IoRead32 CFC IoWrite32 CF8 IoRead32 6 ASSERT .../edk2/MdePkg/Library/BaseIoLibIntrinsic/IoLibGcc.c(164): (Port & 3) == 0 Does anybody have any hints on why this fails or how to debug it further? Regards, BALATON Zoltan
Re: [Qemu-devel] OVMF with q35 (was: osx bootloader)
On Sat, 1 Feb 2014, Alexander Graf wrote: Easiest is probably to attach gdb and get a backtrace to see who accesses that port. Only if I knew how to do that... I can start qemu with -s and attach gdb to it but how to get symbols for the OVMF.fd file and how to set the breakpoint? I've tried gdb OVMF.fd but it cannot load it directly. I've also found *.debug files in the edk2/Build directory but they are only for parts of the whole fw image and none of them seems to be for the part which defines the IoRead32 function. Does anyone know how to debug OVMF firmware running in qemu? Regards, BALATON Zoltan
Re: [Qemu-devel] osx bootloader
On Wed, 29 Jan 2014, Alexander Graf wrote: You will need an EFI HFS+ driver (look at the refind pointer) because boot.efi will use EFI callbacks to read the kernel and kext cache. Probably we will need more than that. I've copied boot.efi to the EFI partition and tried to start it from OMVF (without -M q35) but it only printed: Can not initialize console Boot failed, sleeping for 10 seconds before exiting... Regards, BALATON Zoltan
Re: [Qemu-devel] osx bootloader
On Sat, 1 Feb 2014, Paolo Bonzini wrote: The firmware has to be ported to each new chipset. OVMF was never ported to anything but PIIX. Looks like this is the case. Now I can see that OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c has hardcoded pci paths in it corresponding to the pc machine type with PIIX and it tries to access these devices unconditionally even if they do not exist (as on q35 machine). So this obviously won't work. I think I give up at this point because I don't know enough about Q35 and I don't see how could it be hacked into Ovmf. Additionally it seems that this alone would not be enough to use Apple's boot.efi verbatim but something more complex, very much resembling Clover would be needed to boot OS X with Ovmf EFI which is not much cleaner solution than using Chameleon with Seabios that already works now. Regards, BALATON Zoltan
Re: [Qemu-devel] [Bug 1276879] Re: lots of dma command 10, 14 not supported
On Thu, 6 Feb 2014, tyler knosis wrote: p.s. I tried Bochs 2.6.2, and it is not stuck at the same place. Did qemu take the bochs bios and change anything regarding the IDE drives? With OPENSTEP 4.2 a similar irq hang is happening due to a bug in the i8259 model which is fixed by the one liner patch referenced here: http://lists.nongnu.org/archive/html/qemu-devel/2013-10/msg03329.html so it's most probably not a BIOS problem. The fix was not merged though due to some risks with migration and maybe other bugs that could currently be masked by this one but I don't know if the risks are well understood. Some discussion can be found in this thread: http://lists.nongnu.org/archive/html/qemu-devel/2013-01/msg02396.html Could you please let us know if this fixes the irq timeout you are seeing with NeXTSTEP too? The dma commands are something else though that may not be related.
[Qemu-devel] [Bug 1276879] Re: lots of dma command 10, 14 not supported
http://lists.nongnu.org/archive/html/qemu-devel/2014-02/msg00889.html -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1276879 Title: lots of dma command 10, 14 not supported Status in QEMU: New Bug description: Trying to install NeXTSTEP 3.3 onto a 2GB file with QEMU 1.7.0. In the terminal that started QEMU, there are a lot of dma: command 10 not supported and dma: command 14 not supported messages. When the installation of NeXTSTEP gets to 'preparing disk for nextstep installation', there are a lot of messages that ATA command c5 failed and other info. The result is a failed installation. Is this a bug in QEMU? Is there a workaround, e.g. by disabling DMA altogether? thank you To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1276879/+subscriptions
[Qemu-devel] [Bug 1276879] Re: lots of dma command 10, 14 not supported
The other patch is for KVM. Here's a way to build a patched kvm module (I did not test it): http://www.contrib.andrew.cmu.edu/~somlo/OSXKVM/#sec_0_kvm_kmod_build A click in the window should be enough to grab the mouse. If it's not working it may be that NeXTSTEP is using a different driver than what QEMU emulates. Since NeXTSTEP is quite old you may have better luck with a serial mouse emulation such as msmouse or make sure a PS/2 mouse driver is selected in NeXTSTEP (it may not autodetect it). -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1276879 Title: lots of dma command 10, 14 not supported Status in QEMU: New Bug description: Trying to install NeXTSTEP 3.3 onto a 2GB file with QEMU 1.7.0. In the terminal that started QEMU, there are a lot of dma: command 10 not supported and dma: command 14 not supported messages. When the installation of NeXTSTEP gets to 'preparing disk for nextstep installation', there are a lot of messages that ATA command c5 failed and other info. The result is a failed installation. Is this a bug in QEMU? Is there a workaround, e.g. by disabling DMA altogether? thank you To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1276879/+subscriptions
[Qemu-devel] [Bug 1276879] Re: lots of dma command 10, 14 not supported
I don't know about Bochs but here's another page with resources on running OPENSTEP on a VM: http://www.zebpedersen.co.uk/?p=126 The VMWare drivers (both svga and mouse) should work with QEMU too but I don't know if they are compatible with NeXTSTEP. Now I remember there was some problem with mouse emulation with the PS/2 or serial driver also on VMWare which made the mouse pointer jump around and then freeze. This may also happen with QEMU but it was never debugged AFAIK. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1276879 Title: lots of dma command 10, 14 not supported Status in QEMU: New Bug description: Trying to install NeXTSTEP 3.3 onto a 2GB file with QEMU 1.7.0. In the terminal that started QEMU, there are a lot of dma: command 10 not supported and dma: command 14 not supported messages. When the installation of NeXTSTEP gets to 'preparing disk for nextstep installation', there are a lot of messages that ATA command c5 failed and other info. The result is a failed installation. Is this a bug in QEMU? Is there a workaround, e.g. by disabling DMA altogether? thank you To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1276879/+subscriptions
[Qemu-devel] [PATCH] mac99: Added FW_CFG_PPC_BUSFREQ to match CPUFREQ and TBFREQ already there
While there, also moved the hard coded value for CPUFREQ to a #define.
Signed-off-by: BALATON Zoltan
---
hw/ppc/mac_newworld.c | 5 -
include/hw/ppc/ppc.h | 2 ++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index a533ec7..c937ff1 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -72,6 +72,8 @@
#define MAX_IDE_BUS 2
#define CFG_ADDR 0xf510
#define TBFREQ (100UL * 1000UL * 1000UL)
+#define CLOCKFREQ (266UL * 1000UL * 1000UL)
+#define BUSFREQ (100UL * 1000UL * 1000UL)
/* debug UniNorth */
//#define DEBUG_UNIN
@@ -469,7 +471,8 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, TBFREQ);
}
/* Mac OS X requires a "known good" clock-frequency value; pass it one. */
-fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_CLOCKFREQ, 26600);
+fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_CLOCKFREQ, CLOCKFREQ);
+fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_BUSFREQ, BUSFREQ);
qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
}
diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
index d71bd07..7e16e2e 100644
--- a/include/hw/ppc/ppc.h
+++ b/include/hw/ppc/ppc.h
@@ -92,6 +92,8 @@ enum {
#define FW_CFG_PPC_IS_KVM (FW_CFG_ARCH_LOCAL + 0x05)
#define FW_CFG_PPC_KVM_HC (FW_CFG_ARCH_LOCAL + 0x06)
#define FW_CFG_PPC_KVM_PID (FW_CFG_ARCH_LOCAL + 0x07)
+/* OpenBIOS has FW_CFG_PPC_NVRAM_ADDR as +0x08 */
+#define FW_CFG_PPC_BUSFREQ (FW_CFG_ARCH_LOCAL + 0x09)
#define PPC_SERIAL_MM_BAUDBASE 399193
--
1.8.1.5
[Qemu-devel] [PATCH v2] mac99: Added FW_CFG_PPC_BUSFREQ to match CLOCKFREQ and TBFREQ already there
While there, also moved the hard coded value for CLOCKFREQ to a #define.
Signed-off-by: BALATON Zoltan
---
v2: Also include mac_oldworld that I missed in the first version and
fix commit message
hw/ppc/mac_newworld.c | 5 -
hw/ppc/mac_oldworld.c | 5 -
include/hw/ppc/ppc.h | 2 ++
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index a533ec7..c937ff1 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -72,6 +72,8 @@
#define MAX_IDE_BUS 2
#define CFG_ADDR 0xf510
#define TBFREQ (100UL * 1000UL * 1000UL)
+#define CLOCKFREQ (266UL * 1000UL * 1000UL)
+#define BUSFREQ (100UL * 1000UL * 1000UL)
/* debug UniNorth */
//#define DEBUG_UNIN
@@ -469,7 +471,8 @@ static void ppc_core99_init(QEMUMachineInitArgs *args)
fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, TBFREQ);
}
/* Mac OS X requires a "known good" clock-frequency value; pass it one. */
-fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_CLOCKFREQ, 26600);
+fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_CLOCKFREQ, CLOCKFREQ);
+fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_BUSFREQ, BUSFREQ);
qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
}
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 2f27754..5de9223 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -46,6 +46,8 @@
#define MAX_IDE_BUS 2
#define CFG_ADDR 0xf510
#define TBFREQ 1660UL
+#define CLOCKFREQ 26600UL
+#define BUSFREQ 6600UL
static int fw_cfg_boot_set(void *opaque, const char *boot_device)
{
@@ -337,7 +339,8 @@ static void ppc_heathrow_init(QEMUMachineInitArgs *args)
fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, TBFREQ);
}
/* Mac OS X requires a "known good" clock-frequency value; pass it one. */
-fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_CLOCKFREQ, 26600);
+fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_CLOCKFREQ, CLOCKFREQ);
+fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_BUSFREQ, BUSFREQ);
qemu_register_boot_set(fw_cfg_boot_set, fw_cfg);
}
diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
index d71bd07..7e16e2e 100644
--- a/include/hw/ppc/ppc.h
+++ b/include/hw/ppc/ppc.h
@@ -92,6 +92,8 @@ enum {
#define FW_CFG_PPC_IS_KVM (FW_CFG_ARCH_LOCAL + 0x05)
#define FW_CFG_PPC_KVM_HC (FW_CFG_ARCH_LOCAL + 0x06)
#define FW_CFG_PPC_KVM_PID (FW_CFG_ARCH_LOCAL + 0x07)
+/* OpenBIOS has FW_CFG_PPC_NVRAM_ADDR as +0x08 */
+#define FW_CFG_PPC_BUSFREQ (FW_CFG_ARCH_LOCAL + 0x09)
#define PPC_SERIAL_MM_BAUDBASE 399193
--
1.8.1.5
[Qemu-devel] Help needed testing on ppc
Hello,
As I got no reply on the qemu-ppc list so far I try here maybe there are
some people who read this list but don't follow the ppc one.
I don't have the necessary hardware to do the testing needed for the patch
below. Some context for the discussion can be found in this message:
http://lists.nongnu.org/archive/html/qemu-ppc/2014-04/msg00277.html
It seems we have some code that contains instructions with a reserved bit set
in an stwx instruction that works on real hardware but causes an invalid
instruction exception on QEMU.
I'd appreciate some insight and help.
Regards,
BALATON Zoltan
On Thu, 17 Apr 2014, Programmingkid wrote:
On Apr 17, 2014, at 3:16 AM, qemu-ppc-requ...@nongnu.org wrote:
On Wed, 16 Apr 2014, Alexander Graf wrote:
On 16.04.14 12:24, BALATON Zoltan wrote:
On Tue, 15 Apr 2014, Alexander Graf wrote:
Try to do the same with the _E macro. Be creative :)
This one did it:
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index e3fcb03..d1e175e 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -10341,7 +10341,7 @@ GEN_HANDLER(stop##u, opc, 0xFF, 0xFF, 0x,
type),
#define GEN_STUX(name, stop, opc2, opc3, type)
\
GEN_HANDLER(name##ux, 0x1F, opc2, opc3, 0x0001, type),
#define GEN_STX_E(name, stop, opc2, opc3, type, type2)
\
-GEN_HANDLER_E(name##x, 0x1F, opc2, opc3, 0x0001, type, type2),
+GEN_HANDLER_E(name##x, 0x1F, opc2, opc3, 0x, type, type2),
#define GEN_STS(name, stop, op, type)
\
GEN_ST(name, stop, op | 0x20, type)
\
GEN_STU(name, stop, op | 0x21, type)
\
Cool. Could you please write a small program similar to the one I sent you
that runs all of these instructions and checks that really none of them
trigger a program interrupt on real hardware? We can then remove the
reserved
1 bit from the mask.
Would something like this work? (You should be able to change the
instruction to test at the 1: label.) I can't test it though without a PPC
machine.
#include
int main(int argc, char **argv)
{
register unsigned long r8 asm("r8");
register unsigned long r9 asm("r9");
register unsigned long r10 asm("r10");
register unsigned long r11 asm("r11");
register unsigned long r12 asm("r12");
long val = 0;
r8 = 0;
r9 = (long)&val;
r10 = 0;
asm volatile("mfcr 8\n\t"
"bl 1f \n\t"
"mfcr 11 \n\t"
"mflr 0\n\t"
"lwz 8, 36(0) \n\t"
"ori 8, 8, 1 \n\t"
"stw 8, 36(0) \n\t"
"mfcr 8\n\t"
"bl 1f \n\t"
"mfcr 12 \n\t"
"b 2f \n\t"
"1: stwx 8, 9, 10 \n\t"
"blr \n\t"
"2:\n\t"
: "=r"(r8), "=r"(r11), "=r"(r12)
: "r"(r8), "r"(r9), "r"(r10)
: "cc");
printf("old cr (mem):\t%#lx\n", val);
printf("old cr (reg):\t%#lx\n", r8);
printf("new cr1 (reg):\t%#lx\n", r11);
printf("new cr2 (reg):\t%#lx\n", r12);
return 0;
}
Regards,
BALATON Zoltan
Just tried out your program on a Macintosh with a G3 processor. It doesn't
compile under Mac OS X. Under Linux it crashes with a segmentation fault.
Re: [Qemu-devel] Help needed testing on ppc
On Tue, 6 May 2014, Tom Musta wrote: On 5/6/2014 5:03 AM, BALATON Zoltan wrote: Hello, As I got no reply on the qemu-ppc list so far I try here maybe there are some people who read this list but don't follow the ppc one. I don't have the necessary hardware to do the testing needed for the patch below. Some context for the discussion can be found in this message: http://lists.nongnu.org/archive/html/qemu-ppc/2014-04/msg00277.html It seems we have some code that contains instructions with a reserved bit set in an stwx instruction that works on real hardware but causes an invalid instruction exception on QEMU. I'd appreciate some insight and help. Regards, BALATON Zoltan This is a bit tricky. You appear to have code that has a reserved bit set. Early forms of the PowerPC ISA (circa 1998) said this: "All reserved fields in instructions should be zero. If they are not, the instruction form is invalid. ... Any attempt to execute an invalid form of an instruction will cause the system illegal instruction error handler to be invoked or yield boundedly undefined results." QEMU, as a general rule, meets this requirement by causing illegal instruction exceptions. More modern versions of the ISA (circa 2006) say this: "Reserved fields in instructions are ignored by the processor. This is a requirement in the Server environment and is being phased into the Embedded environment. ... To maximize compatibility with future architecture extensions, software must ensure that reserved fields in instructions contain zero and that defined fields of instructions do not contain reserved values." Technically, QEMU does not comply with the requirement in the first sentence; and MorpOS does not comply with the third. The newer form of the ISA is compatible with the older one since ignoring reserved fields is a valid implementation of "boundedly undefined." Thanks for the exhaustive answer with definitive references. This is really very helpful. A few questions and comments: (1) Why is MorphOS using this invalid instruction form? Would it be easier to fix the OS rather than QEMU? I don't know why is it used. I can ask the MorphOS developers but they did not seem to be too supportive so far and at least one of them expressed that they have no interest supporting other than their officially supported list of hardware at this time. So I assume it is easier to fix QEMU than MorphOS and if it works on a real Mac then it should also work on QEMU's emulation of that Mac hardware. Is there some undocumented processor behavior that the code is dependent upon (e.g. is it actually expected CR0 to be set?). This is what the testing was supposed to find out but MorphOS seems to run better with the quoted patch so I don't think it depends on any other undocumented behaviour other than ignoring reserved bits but I have no definitive answer. (2) Your patch makes some store instructions compliant with the most recent ISAs but there are many other instructions that are not addressed by the patch. I think fixing only some will be a future source of confusion. (3) The change risks breaking behavior on older designs which may very well have taken the illegal instruction interrupt. Would it make more sense to leave the masks as-is and instead make a single, isolated change in the decoder (gen_intermediate_code_internal). This behavior could be made conditional (configuration item? processor family specific flag?). Unfortunately, the masks also catch some invalid forms that do not involve reserved fields (e.g. lq/stq to odd numbered registers). I don't know this code very well so not sure I can follow your suggestion. Are you proposing that the invalid masks could be ignored globally in gen_intermediate_code_internal (around target-ppc/traslate.c:11444) based on some condition for all opcodes? Since your quotes above show that QEMU does not implement the current specification and code relying on older behaviour would not run on newer processors so it's likely they will get fixed so I think the risk of breaking older designs is less than breaking software that rely on current specification so IMO it should be changed in QEMU if possible and only care about older designs when one is actually encountered. (4) In general, modeling undefined behavior is a slippery slope. I would much prefer to see the code fixed or justified before changing QEMU. I can try to ask on the MorphOS list but their previous answer to another question was that it works on the hardware they officially support. Regards, BALATON Zoltan
