From 5c00acebb6fb92ff169b322c9e74d06d8b922232 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Apr 2024 12:24:20 +0200 Subject: [PATCH 1/6] vga: merge conditionals on shift control register MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are two sets of conditionals using the shift control bits: one to verify the palette and adjust disp_width, one to compute the "v" and "bits" variables. Merge them into one, with the extra benefit that we now have the "bits" value available early and can use it to compute region_end. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- hw/display/vga.c | 89 +++++++++++++++++++++++------------------------- 1 file changed, 42 insertions(+), 47 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index bc5b83421b..4795a0012e 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1546,12 +1546,54 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) } if (shift_control == 0) { + full_update |= update_palette16(s); if (sr(s, VGA_SEQ_CLOCK_MODE) & 8) { disp_width <<= 1; + v = VGA_DRAW_LINE4D2; + } else { + v = VGA_DRAW_LINE4; } + bits = 4; + } else if (shift_control == 1) { + full_update |= update_palette16(s); if (sr(s, VGA_SEQ_CLOCK_MODE) & 8) { disp_width <<= 1; + v = VGA_DRAW_LINE2D2; + } else { + v = VGA_DRAW_LINE2; + } + bits = 4; + + } else { + switch (depth) { + default: + case 0: + full_update |= update_palette256(s); + v = VGA_DRAW_LINE8D2; + bits = 4; + break; + case 8: + full_update |= update_palette256(s); + v = VGA_DRAW_LINE8; + bits = 8; + break; + case 15: + v = s->big_endian_fb ? VGA_DRAW_LINE15_BE : VGA_DRAW_LINE15_LE; + bits = 16; + break; + case 16: + v = s->big_endian_fb ? VGA_DRAW_LINE16_BE : VGA_DRAW_LINE16_LE; + bits = 16; + break; + case 24: + v = s->big_endian_fb ? VGA_DRAW_LINE24_BE : VGA_DRAW_LINE24_LE; + bits = 24; + break; + case 32: + v = s->big_endian_fb ? VGA_DRAW_LINE32_BE : VGA_DRAW_LINE32_LE; + bits = 32; + break; } } @@ -1607,53 +1649,6 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) } } - if (shift_control == 0) { - full_update |= update_palette16(s); - if (sr(s, VGA_SEQ_CLOCK_MODE) & 8) { - v = VGA_DRAW_LINE4D2; - } else { - v = VGA_DRAW_LINE4; - } - bits = 4; - } else if (shift_control == 1) { - full_update |= update_palette16(s); - if (sr(s, VGA_SEQ_CLOCK_MODE) & 8) { - v = VGA_DRAW_LINE2D2; - } else { - v = VGA_DRAW_LINE2; - } - bits = 4; - } else { - switch(s->get_bpp(s)) { - default: - case 0: - full_update |= update_palette256(s); - v = VGA_DRAW_LINE8D2; - bits = 4; - break; - case 8: - full_update |= update_palette256(s); - v = VGA_DRAW_LINE8; - bits = 8; - break; - case 15: - v = s->big_endian_fb ? VGA_DRAW_LINE15_BE : VGA_DRAW_LINE15_LE; - bits = 16; - break; - case 16: - v = s->big_endian_fb ? VGA_DRAW_LINE16_BE : VGA_DRAW_LINE16_LE; - bits = 16; - break; - case 24: - v = s->big_endian_fb ? VGA_DRAW_LINE24_BE : VGA_DRAW_LINE24_LE; - bits = 24; - break; - case 32: - v = s->big_endian_fb ? VGA_DRAW_LINE32_BE : VGA_DRAW_LINE32_LE; - bits = 32; - break; - } - } vga_draw_line = vga_draw_line_table[v]; if (!is_buffer_shared(surface) && s->cursor_invalidate) { From 3826a372e4aafac1dba9ba3434e7c2f76775de42 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Apr 2024 12:25:34 +0200 Subject: [PATCH 2/6] vga: move computation of dirty memory region later MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the computation of region_start and region_end after the value of "bits" is known. This makes it possible to distinguish modes that support horizontal pel panning from modes that do not. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- hw/display/vga.c | 50 ++++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 4795a0012e..b4ceff70eb 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1501,31 +1501,6 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) disp_width = width; depth = s->get_bpp(s); - region_start = (s->params.start_addr * 4); - region_end = region_start + (ram_addr_t)s->params.line_offset * height; - region_end += width * depth / 8; /* scanline length */ - region_end -= s->params.line_offset; - if (region_end > s->vbe_size || depth == 0 || depth == 15) { - /* - * We land here on: - * - wraps around (can happen with cirrus vbe modes) - * - depth == 0 (256 color palette video mode) - * - depth == 15 - * - * Take the safe and slow route: - * - create a dirty bitmap snapshot for all vga memory. - * - force shadowing (so all vga memory access goes - * through vga_read_*() helpers). - * - * Given this affects only vga features which are pretty much - * unused by modern guests there should be no performance - * impact. - */ - region_start = 0; - region_end = s->vbe_size; - force_shadow = true; - } - /* bits 5-6: 0 = 16-color mode, 1 = 4-color mode, 2 = 256-color mode. */ shift_control = (s->gr[VGA_GFX_MODE] >> 5) & 3; double_scan = (s->cr[VGA_CRTC_MAX_SCAN] >> 7); @@ -1597,6 +1572,31 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) } } + region_start = (s->params.start_addr * 4); + region_end = region_start + (ram_addr_t)s->params.line_offset * height; + region_end += width * depth / 8; /* scanline length */ + region_end -= s->params.line_offset; + if (region_end > s->vbe_size || depth == 0 || depth == 15) { + /* + * We land here on: + * - wraps around (can happen with cirrus vbe modes) + * - depth == 0 (256 color palette video mode) + * - depth == 15 + * + * Take the safe and slow route: + * - create a dirty bitmap snapshot for all vga memory. + * - force shadowing (so all vga memory access goes + * through vga_read_*() helpers). + * + * Given this affects only vga features which are pretty much + * unused by modern guests there should be no performance + * impact. + */ + region_start = 0; + region_end = s->vbe_size; + force_shadow = true; + } + /* * Check whether we can share the surface with the backend * or whether we need a shadow surface. We share native From 3b6d2b1962b23295c463f010ff88eb5a594f2ef9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Apr 2024 12:25:57 +0200 Subject: [PATCH 3/6] vga: adjust dirty memory region if pel panning is active MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When pel panning is active, one more byte is read from each of the VGA memory planes. This has to be accounted in the computation of region_end, otherwise vga_draw_graphic() fails an assertion: qemu-system-i386: ../system/physmem.c:946: cpu_physical_memory_snapshot_get_dirty: Assertion `start + length <= snap->end' failed. Reported-by: Helge Konetzka Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2244 Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- hw/display/vga.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index b4ceff70eb..40acd19e72 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1571,11 +1571,15 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) break; } } + hpel = bits <= 8 ? s->params.hpel : 0; region_start = (s->params.start_addr * 4); region_end = region_start + (ram_addr_t)s->params.line_offset * height; region_end += width * depth / 8; /* scanline length */ region_end -= s->params.line_offset; + if (hpel) { + region_end += 4; + } if (region_end > s->vbe_size || depth == 0 || depth == 15) { /* * We land here on: @@ -1660,7 +1664,6 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) width, height, v, line_offset, s->cr[9], s->cr[VGA_CRTC_MODE], s->params.line_compare, sr(s, VGA_SEQ_CLOCK_MODE)); #endif - hpel = bits <= 8 ? s->params.hpel : 0; addr1 = (s->params.start_addr * 4); bwidth = DIV_ROUND_UP(width * bits, 8); if (hpel) { From 1d1ee7e0a1b7041804e8c5f8c2453fdc2df0407e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 2 Apr 2024 12:26:36 +0200 Subject: [PATCH 4/6] vga: do not treat horiz pel panning value of 8 as "enabled" Horizontal pel panning bit 3 is only used in text mode. In graphics mode, it can be treated as if it was zero, thus not extending the dirty memory region. Signed-off-by: Paolo Bonzini --- hw/display/vga.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/display/vga.c b/hw/display/vga.c index 40acd19e72..77f59e8c11 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -1571,7 +1571,9 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) break; } } - hpel = bits <= 8 ? s->params.hpel : 0; + + /* Horizontal pel panning bit 3 is only used in text mode. */ + hpel = bits <= 8 ? s->params.hpel & 7 : 0; region_start = (s->params.start_addr * 4); region_end = region_start + (ram_addr_t)s->params.line_offset * height; From e497e6a55786a62ffe009a3fe2fa6d40e6080210 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Sun, 31 Mar 2024 20:04:41 +0200 Subject: [PATCH 5/6] lsi53c895a: avoid out of bounds access to s->msg[] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If no bytes are there to process in the message in phase, the input data latch (s->sidl) is set to s->msg[-1]. Just do nothing since no DMA is performed. Reported-by: Chuhong Yuan Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- hw/scsi/lsi53c895a.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c index 71f759a59d..eb9828dd5e 100644 --- a/hw/scsi/lsi53c895a.c +++ b/hw/scsi/lsi53c895a.c @@ -927,13 +927,18 @@ static void lsi_do_msgin(LSIState *s) assert(len > 0 && len <= LSI_MAX_MSGIN_LEN); if (len > s->dbc) len = s->dbc; - pci_dma_write(PCI_DEVICE(s), s->dnad, s->msg, len); - /* Linux drivers rely on the last byte being in the SIDL. */ - s->sidl = s->msg[len - 1]; - s->msg_len -= len; - if (s->msg_len) { - memmove(s->msg, s->msg + len, s->msg_len); - } else { + + if (len) { + pci_dma_write(PCI_DEVICE(s), s->dnad, s->msg, len); + /* Linux drivers rely on the last byte being in the SIDL. */ + s->sidl = s->msg[len - 1]; + s->msg_len -= len; + if (s->msg_len) { + memmove(s->msg, s->msg + len, s->msg_len); + } + } + + if (!s->msg_len) { /* ??? Check if ATN (not yet implemented) is asserted and maybe switch to PHASE_MO. */ switch (s->msg_action) { From 8fc4bdc537d901c200e43122e32bcb40dc8fed37 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 25 Mar 2024 14:58:46 +0100 Subject: [PATCH 6/6] pc_q35: remove unnecessary m->alias assignment The assignment is already inherited from pc-q35-8.2. Signed-off-by: Paolo Bonzini --- hw/i386/pc_q35.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index b5922b44af..c7bc8a2041 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -393,7 +393,6 @@ static void pc_q35_8_1_machine_options(MachineClass *m) { PCMachineClass *pcmc = PC_MACHINE_CLASS(m); pc_q35_8_2_machine_options(m); - m->alias = NULL; pcmc->broken_32bit_mem_addr_check = true; compat_props_add(m->compat_props, hw_compat_8_1, hw_compat_8_1_len); compat_props_add(m->compat_props, pc_compat_8_1, pc_compat_8_1_len);