Re: Thoughts on removing the TARGET_I386 part of hw/display/vga/vbe_portio_list[]

On 6/12/22 13:30, Dr. David Alan Gilbert wrote:

* Philippe Mathieu-Daudé ([email protected]) wrote:

Hi,

I'm trying to understand the x86 architecture-specific code in
hw/display/vga.c:

     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(),
     };

Having:

     typedef struct MemoryRegionPortio {
         uint32_t offset;
         uint32_t len;
         unsigned size;
         uint32_t (*read)(...);
         void (*write)(...);
         ...
     } MemoryRegionPortio;

So on x86 we can have 16-bit I/O accesses unaligned to 8-bit boundary?

Yes, like most things in x86 the requirement for alignment is a 'should'
followed by a description of what might happen if you don't:

 From intel arch manual 19.3:
  '..16-bit ports should be aligned to even addresses (0, 2, 4, ...) so that 
all 16 bits can be transferred in a
   single bus cycle. Likewise, 32-bit ports should be aligned to addresses that 
are multiples of four (0, 4, 8, ...). The
   processor supports data transfers to unaligned ports, but there is a 
performance penalty because one or more
   extra bus cycle must be used.'

So you confirm this is a architecture behavior, not a device one, thanks.

I think I've even seen it suggested that a 32bit access to ffff might be
defined - although I'm not sure if that's legal.

I don't know that bit of qemu well enough to know whether the cpu part
of qemu should be splitting the unaligned accesses or not.

There is an old access_with_adjusted_size_unaligned() version [1] from
Andrew and a more recent series [2] from Richard. Maybe the latter fixes
some long-standing bug [3] we have here?