From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39200) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4dHk-0005mK-C4 for qemu-devel@nongnu.org; Mon, 15 Jun 2015 18:55:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z4dHj-0000Sg-AO for qemu-devel@nongnu.org; Mon, 15 Jun 2015 18:55:20 -0400 Message-ID: <557F57CF.2090102@redhat.com> Date: Mon, 15 Jun 2015 16:55:11 -0600 From: Eric Blake MIME-Version: 1.0 References: <1434406965-30883-1-git-send-email-jsnow@redhat.com> <1434406965-30883-2-git-send-email-jsnow@redhat.com> In-Reply-To: <1434406965-30883-2-git-send-email-jsnow@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="EMqt6K6oUJJAK5dcPTiGIffNEHigunPAo" Subject: Re: [Qemu-devel] [PATCH 1/4] ahci: Do not ignore memory access read size List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-block@nongnu.org Cc: kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --EMqt6K6oUJJAK5dcPTiGIffNEHigunPAo Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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." >=20 > 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. >=20 > 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. >=20 > Signed-off-by: John Snow > --- > hw/ide/ahci.c | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) >=20 > 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) > } > } > =20 > -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 =3D opaque; > uint32_t val =3D 0; > @@ -368,6 +367,24 @@ static uint64_t ahci_mem_read(void *opaque, hwaddr= addr, > } > =20 > =20 > +static uint64_t ahci_mem_read(void *opaque, hwaddr addr, unsigned size= ) > +{ > + hwaddr aligned =3D addr & ~0x3; > + int ofst =3D addr - aligned; > + uint64_t lo =3D ahci_mem_read_32(opaque, aligned); > + uint64_t hi; > + > + /* if 1/2/4 byte read does not cross 4 byte boundary */ > + if (ofst + size <=3D 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 =3D 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. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --EMqt6K6oUJJAK5dcPTiGIffNEHigunPAo Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJVf1fPAAoJEKeha0olJ0NqiKMH+wWcTBt8xX85UAI2Ib6l7luB a6WHQE7lQj1Zzbn/ZEWmM0Avk3k2sXPOjQtoPFgE2uzC7Msnb3B/McduVDn9gcn4 B7pqtoUGlkXz9E+rjXhUaTpWHR6qa18cbgSx/R82ye+D2gdaJzAg9zsbbburwKQk QEpiWcHo8LN88zczcpa46ua45pPTbRUUc67dO/AsUVtV5qQ1hlDrFrV70uapV0AH UCCwNaQjbTKD/Bub0tJofesnWr2fno2jIo1f3N6iwT8sXtirDKr9dYmWlr+oIZGq sSDMTVaI3uzpKCzKu4dCqRAFjpSlhaEJThgCkLArAMG05xodGxrISol6iRxWDn8= =5Kue -----END PGP SIGNATURE----- --EMqt6K6oUJJAK5dcPTiGIffNEHigunPAo--