Re: [Qemu-devel] [PATCH v2 1/4] ahci: Do not ignore memory access read size

[John Snow <jsnow@redhat.com>]

On 06/16/2015 12:25 PM, Eric Blake wrote:
> On 06/16/2015 10:02 AM, John Snow wrote:
>> The only guidance the AHCI specification gives on memory access
>> is: "Register accesses shall have a maximum size of 64-bits;
>> 64-bit access must not cross an 8-byte alignment boundary."
>> 
>> I interpret this to mean that aligned or unaligned 1, 2 and 4
>> byte accesses should work, as well as aligned 8 byte accesses.
>> 
>> In practice, a real Q35/ICH9 responds to 1, 2, 4 and 8 byte
>> reads regardless of alignment. Windows 7 can be observed making 1
>> byte reads to the middle of 32 bit registers to fetch error
>> codes.
>> 
>> Introduce a wrapper to support unaligned accesses to AHCI. This
>> wrapper will support aligned 8 byte reads, but will make no
>> effort to support unaligned 8 byte reads, which although they 
>> will work on real hardware, are not guaranteed to work and do not
>> appear to be used by either Windows or Linux.
>> 
>> Signed-off-by: John Snow <jsnow@redhat.com> --- hw/ide/ahci.c |
>> 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+),
>> 2 deletions(-)
>> 
> 
>> +/** + * AHCI 1.3 section 3 ("HBA Memory Registers") + * Support
>> unaligned 8/16/32 bit reads, and 64 bit aligned reads. + * Caller
>> is responsible for masking unwanted higher order bytes. + */ 
>> +static uint64_t ahci_mem_read(void *opaque, hwaddr addr,
>> unsigned size) +{ +    hwaddr aligned = addr & ~0x3;
> 
> This actually supports 4-byte aligned 8-byte reads (which is an 
> unaligned 8-byte read).  Doesn't matter; no guest should be relying
> on it.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

Easier to say "indeterminate" than rely on wonko behavior, even if it
sometimes accidentally works :)

Thanks.