Re: [Qemu-devel] [PATCH v2 1/2] memory: Fix old portio word accesses

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 1990 bytes --]

On 2011-09-19 14:19, Avi Kivity wrote:
> On 09/18/2011 10:07 PM, Jan Kiszka wrote:
>> On 2011-09-18 18:49, Avi Kivity wrote:
>> >  On 09/18/2011 07:28 PM, Jan Kiszka wrote:
>> >>  >>
>> >>  >>   This is PIO, limited by the x86 address space to 16 bit. Will
>> add a
>> >>  >>   comment.
>> >>  >
>> >>  >   x86 PIO is not limited to 16 bits, just ISA, which memory.c knows
>> >>  >   nothing about.
>> >>
>> >>  Confused address and data, the former is limited 16, the latter
>> can be
>> >>  32 as well. But I guess only ISA models made use of the core's
>> split up
>> >>  service, and that's why QEMU limited itself accordingly.
>> >
>> >  Let's not bury such details in the core.
>>
>> It's already in the core (ioport), and would refrain from changing it in
>> this fix.
> 
> That's the bad old core we're trying to get away from.

We overcome it at the point the last portio user was converted and that
legacy is removed.

> 
>> >
>> >  I don't think this holds for pci; there the bus always generates
>> 32-bit
>> >  writes with separate byte enables for each lane.  The device need not
>> >  even be aware of a sub-word access, for reads.
>>
>> The problem is that once we "enhance" the core with such a support to
>> potentially help one use case, we need to validate all users again if
>> they depend on the old behavior. That's tricky as breakage may only show
>> up with odd guests that issue invalid but so far harmless requests.
> 
> It's opt-in.  If a device sets
> MemoryRegionOps::impl.{min,max}_access_size = 1, it will only be fed
> byte accesses (the core will take care of breaking apart larger
> writes).  If it sets MemoryRegionOps::impl.{min,max}_access_size = 4, it
> will only get long accesses (and the core will/should shift/mask or
> RMW).  Refusing illegal access sizes is done using
> MemoryRegionOps::valid.  Most of this is unimplemented unfortunately.

That makes sense (for non-old_portio users).

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]