[Qemu-devel] hw/display/vga: Why architecture specific code?

[Qemu-devel] hw/display/vga: Why architecture specific code?

[Philippe Mathieu-Daudé]

Hi Gerd,

I'm looking at:

static const MemoryRegionPortio vbe_portio_list[] = {
    { 0, 1, 2, .read = vbe_ioport_read_index, .write =
vbe_ioport_write_index },
# ifdef TARGET_I386
    { 1, 1, 2, .read = vbe_ioport_read_data, .write =
vbe_ioport_write_data },
# endif
    { 2, 1, 2, .read = vbe_ioport_read_data, .write =
vbe_ioport_write_data },
    PORTIO_END_OF_LIST(),
};

which comes from:

$ git show 09a79b4974f
commit 09a79b4974fbeee660660d79ab45bd37ec416741
Author: bellard <bellard@c046a42c-6fe2-441c-8c8c-71466251a162>
Date:   Wed May 26 22:58:01 2004 +0000

    partial big endian fixes - change VESA VBE ports for non i386
targets to avoid unaligned accesses

@@ -1774,19 +1804,27 @@ int vga_initialize(DisplayState *ds, uint8_t
*vga_ram_base,
 #ifdef CONFIG_BOCHS_VBE
     s->vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID0;
     s->vbe_bank_mask = ((s->vram_size >> 16) - 1);
-    register_ioport_read(0x1ce, 1, 2, vbe_ioport_read, s);
-    register_ioport_read(0x1cf, 1, 2, vbe_ioport_read, s);
+#if defined (TARGET_I386)
+    register_ioport_read(0x1ce, 1, 2, vbe_ioport_read_index, s);
+    register_ioport_read(0x1cf, 1, 2, vbe_ioport_read_data, s);

-    register_ioport_write(0x1ce, 1, 2, vbe_ioport_write, s);
-    register_ioport_write(0x1cf, 1, 2, vbe_ioport_write, s);
+    register_ioport_write(0x1ce, 1, 2, vbe_ioport_write_index, s);
+    register_ioport_write(0x1cf, 1, 2, vbe_ioport_write_data, s);

     /* old Bochs IO ports */
-    register_ioport_read(0xff80, 1, 2, vbe_ioport_read, s);
-    register_ioport_read(0xff81, 1, 2, vbe_ioport_read, s);
+    register_ioport_read(0xff80, 1, 2, vbe_ioport_read_index, s);
+    register_ioport_read(0xff81, 1, 2, vbe_ioport_read_data, s);

-    register_ioport_write(0xff80, 1, 2, vbe_ioport_write, s);
-    register_ioport_write(0xff81, 1, 2, vbe_ioport_write, s);
+    register_ioport_write(0xff80, 1, 2, vbe_ioport_write_index, s);
+    register_ioport_write(0xff81, 1, 2, vbe_ioport_write_data, s);
+#else
+    register_ioport_read(0x1ce, 1, 2, vbe_ioport_read_index, s);
+    register_ioport_read(0x1d0, 1, 2, vbe_ioport_read_data, s);
+
+    register_ioport_write(0x1ce, 1, 2, vbe_ioport_write_index, s);
+    register_ioport_write(0x1d0, 1, 2, vbe_ioport_write_data, s);
 #endif
+#endif /* CONFIG_BOCHS_VBE */

     vga_io_memory = cpu_register_io_memory(0, vga_mem_read, vga_mem_write);
     cpu_register_physical_memory(isa_mem_base + 0x000a0000, 0x20000,
@@ -1814,11 +1852,9 @@ int vga_initialize(DisplayState *ds, uint8_t
*vga_ram_base,
                                PCI_ADDRESS_SPACE_MEM_PREFETCH, vga_map);
     } else {
 #ifdef CONFIG_BOCHS_VBE
-#if defined (TARGET_I386)
         /* XXX: use optimized standard vga accesses */
         cpu_register_physical_memory(VBE_DISPI_LFB_PHYSICAL_ADDRESS,
                                      vga_ram_size, vga_ram_offset);
-#endif
 #endif
     }
     return 0;

What is X86-specific here?

Thanks,

Phil.

Re: [Qemu-devel] hw/display/vga: Why architecture specific code?

[Gerd Hoffmann]

On Thu, Jul 04, 2019 at 03:27:25PM +0200, Philippe Mathieu-Daudé wrote:
> Hi Gerd,
> 
> I'm looking at:
> 
> static const MemoryRegionPortio vbe_portio_list[] = {
>     { 0, 1, 2, .read = vbe_ioport_read_index, .write =
> vbe_ioport_write_index },
> # ifdef TARGET_I386
>     { 1, 1, 2, .read = vbe_ioport_read_data, .write =
> vbe_ioport_write_data },
> # endif
>     { 2, 1, 2, .read = vbe_ioport_read_data, .write =
> vbe_ioport_write_data },
>     PORTIO_END_OF_LIST(),
> };

Unaligned 16bit ioport access works on x86 only.

cheers,
  Gerd

Re: [Qemu-devel] hw/display/vga: Why architecture specific code?

[Paolo Bonzini]

On 04/07/19 15:55, Gerd Hoffmann wrote:
> On Thu, Jul 04, 2019 at 03:27:25PM +0200, Philippe Mathieu-Daudé wrote:
>> Hi Gerd,
>>
>> I'm looking at:
>>
>> static const MemoryRegionPortio vbe_portio_list[] = {
>>     { 0, 1, 2, .read = vbe_ioport_read_index, .write =
>> vbe_ioport_write_index },
>> # ifdef TARGET_I386
>>     { 1, 1, 2, .read = vbe_ioport_read_data, .write =
>> vbe_ioport_write_data },
>> # endif
>>     { 2, 1, 2, .read = vbe_ioport_read_data, .write =
>> vbe_ioport_write_data },
>>     PORTIO_END_OF_LIST(),
>> };
> 
> Unaligned 16bit ioport access works on x86 only.

But I'm not sure that it needs to be target-dependent now.  Worst case
you'd have a dead MemoryRegion.

Paolo