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

[Philippe Mathieu-Daudé]

On 6/12/22 13:30, Dr. David Alan Gilbert wrote:
> * Philippe Mathieu-Daudé (philmd@linaro.org) 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.

Easy to test :) If unspecified and there is some ISA-to-XXX bridge, then 
I expect this to be implementation dependent of the bridge.

> 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.
All I/O accesses are gated thru access_with_adjusted_size() in 
softmmu/memory.c.

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?

[1] 
https://lore.kernel.org/qemu-devel/20170630030058.28943-1-andrew@aj.id.au/
[2] 
https://lore.kernel.org/qemu-devel/20210619172626.875885-15-richard.henderson@linaro.org/
[3] 
https://lore.kernel.org/qemu-devel/CAFEAcA-fMUrwnpu90Qf1LWGsQ36M-PmX2uC1+kenT__otLxjTg@mail.gmail.com/