From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54615) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4doK-0005Ue-I5 for qemu-devel@nongnu.org; Mon, 15 Jun 2015 19:29:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z4doJ-0007tb-Cf for qemu-devel@nongnu.org; Mon, 15 Jun 2015 19:29:00 -0400 Message-ID: <557F5FAF.9030707@redhat.com> Date: Mon, 15 Jun 2015 17:28:47 -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> <557F57CF.2090102@redhat.com> <557F5B2D.4090408@redhat.com> In-Reply-To: <557F5B2D.4090408@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="QnOPTp228GU0B17aG3sfwFojb1eCKxkaD" 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) --QnOPTp228GU0B17aG3sfwFojb1eCKxkaD Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 06/15/2015 05:09 PM, John Snow wrote: >> >>> 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 >>> + >>> + /* 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 kn= ow >> 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 shou= ld >> be broadened, or the code needs further tuning. >> >=20 > Good catch. >=20 Oh, and one other comment - do we care about the contents in the remaining bytes beyond the requested size? > I have not observed any OS making 2 or 4 byte accesses across the > register boundary, and cannot think of a reason why you would want to, > though the AHCI spec technically doesn't discount your ability to do so= > and it does work on a real Q35. >=20 > I can do this: >=20 > return (hi << 32 | lo) >> (ofst * 8); >=20 > which will give us unaligned 2 and 4 byte reads, but will really get > very wacky for unaligned 8 byte reads -- which you really should > probably not be doing anyway. I don't mind wacky unaligned 8 byte reads - they are in clear violation of the spec you quoted, so the caller deserves any such garbage they get. But as the spec is silent on unaligned 4 byte reads, and real hardware supports it, it's probably best to support it ourselves even if we haven't observed clients exploiting it. Note that while this returns the desired 16 or 32 bits in the low order side of the result, it does not guarantee that the remaining upper bytes are all 0. I don't know if it matters to callers, or even what real hardware does, but you may want to mask things at both return statements, to guarantee a stable result limited to size bytes of information rather than leaking nearby bytes from the rest of the registers being read. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --QnOPTp228GU0B17aG3sfwFojb1eCKxkaD 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/ iQEcBAEBCAAGBQJVf1+wAAoJEKeha0olJ0Nq2kUH/Ao8r/gnbCOFq5M2Osd9c04l 4kJmWGv2APnFKCSFF5sjanh7axxBLY4pL2gys9mBTE5wPq/CYa69DXAsLPTpxCHK wP2VPqDv791z9xELy8228m/WVdUe9flGBnMRBbwKm3H1VkcoEpmVGiVdhacj811z hBD+HNzrqZAAI/7ibrCpjT79jseT+R2dZ2e65uf+B5Kg6iZij1BIncLBcpVz5Jba 6mCWRyrZFwGWilON6KWLx8mVXqCEIs8OfGN72JWKNemMRGrwWaeNjfCtJjlfG/9R 6vdd8LqZWSALR9YntnGSPfnq7FzacEYLveECou7KN33fVbN088o2Qiu9H3soX1o= =vxCs -----END PGP SIGNATURE----- --QnOPTp228GU0B17aG3sfwFojb1eCKxkaD--