On 06/15/2015 04:22 PM, 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." > > In practice, a real Q35/ICH9 responds to 1, 2, 4 and 8 byte reads > regardless of alignment. Windows 7 can also be observed making 1 byte > reads to the middle of 32 bit registers. > > Introduce a wrapper to supper unaligned accesses to AHCI.
s/supper/support/ > 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 <js...@redhat.com> > --- > hw/ide/ahci.c | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index 9e5d862..55779fb 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -331,8 +331,7 @@ static void ahci_port_write(AHCIState *s, int port, int > offset, uint32_t val) > } > } > > -static uint64_t ahci_mem_read(void *opaque, hwaddr addr, > - unsigned size) > +static uint64_t ahci_mem_read_32(void *opaque, hwaddr addr) > { > AHCIState *s = opaque; > uint32_t val = 0; > @@ -368,6 +367,24 @@ static uint64_t ahci_mem_read(void *opaque, hwaddr addr, > } > > > +static uint64_t ahci_mem_read(void *opaque, hwaddr addr, unsigned size) > +{ > + hwaddr aligned = addr & ~0x3; > + int ofst = addr - aligned; > + uint64_t lo = ahci_mem_read_32(opaque, aligned); > + uint64_t hi; > + > + /* if 1/2/4 byte read does not cross 4 byte boundary */ > + if (ofst + size <= 4) { > + return lo >> (ofst * 8); > + } At this point, we could assert(size > 1). > + > + /* If the 64bit read is unaligned, we will produce undefined > + * results. AHCI does not support unaligned 64bit reads. */ > + hi = ahci_mem_read_32(opaque, aligned + 4); > + return (hi << 32) | lo; This makes no effort to support an unaligned 2 byte (16bit) or 4 byte (32bit) read that crosses 4-byte boundary. Is that intentional? I know it is intentional that you don't care about unaligned 64bit reads; conversely, while your commit message mentioned Windows doing 1-byte reads in the middle of 32-bit registers, you didn't mention whether Windows does unaligned 2- or 4-byte reads. So either the comment should be broadened, or the code needs further tuning. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature