From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34581) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XCqv5-0004ma-9I for qemu-devel@nongnu.org; Thu, 31 Jul 2014 10:01:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XCquz-0001OI-S6 for qemu-devel@nongnu.org; Thu, 31 Jul 2014 10:01:23 -0400 Received: from mail-we0-x22f.google.com ([2a00:1450:400c:c03::22f]:33015) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XCquz-0001O7-L6 for qemu-devel@nongnu.org; Thu, 31 Jul 2014 10:01:17 -0400 Received: by mail-we0-f175.google.com with SMTP id t60so2805017wes.34 for ; Thu, 31 Jul 2014 07:01:14 -0700 (PDT) Date: Thu, 31 Jul 2014 15:01:11 +0100 From: Stefan Hajnoczi Message-ID: <20140731140111.GC25929@stefanha-thinkpad.redhat.com> References: <1404757089-4836-1-git-send-email-jsnow@redhat.com> <1404757089-4836-27-git-send-email-jsnow@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="uBaQ+BsiqpBBIRFI" Content-Disposition: inline In-Reply-To: <1404757089-4836-27-git-send-email-jsnow@redhat.com> Subject: Re: [Qemu-devel] [PATCH 26/28] ahci: Add test_hba_spec to ahci-test. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, mst@redhat.com --uBaQ+BsiqpBBIRFI Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Jul 07, 2014 at 02:18:07PM -0400, John Snow wrote: > +/*** IO macros for the AHCI memory registers. ***/ > +#define void_incr(vptr, OFST) ((void *)((char *)(vptr) + (OFST))) I'm pretty sure QEMU takes advantage of GCC's void pointer arithmetic extension: https://gcc.gnu.org/onlinedocs/gcc-4.9.1/gcc/Pointer-Arith.html#Pointer-Arith In other words, vptr + OFST works and you don't need a macro. > +#define ahci_set(regno, mask) ahci_wreg((regno), ahci_rreg(regno) | (mask)) > +#define ahci_clr(regno, mask) ahci_wreg((regno), ahci_rreg(regno) & ~(mask)) Unused. Please move to the patch that actually uses them. > +#define px_set(port, reg, mask) px_wreg((port), (reg), \ > + px_rreg((port), (reg)) | (mask)); > +#define px_clr(port, reg, mask) px_wreg((port), (reg), \ > + px_rreg((port), (reg)) & ~(mask)); Unused. Please move to the patch that actually uses them. > + /* We need to know the size of the region, > + * but qpci_iomap doesn't save it. Recalculate it. */ It seems like many tests will want to check the BAR size. Please add an argument to qpci_iomap() so the caller gets the size. > + if (bitset(cap, AHCI_CAP_SAM)) { > + g_test_message("Supports AHCI-Only Mode: GHC_AE is Read-Only."); > + assert_bit_set(reg, AHCI_GHC_AE); > + } else { > + g_test_message("Supports AHCI/Legacy mix."); > + assert_bit_clear(reg, AHCI_GHC_AE); > + } Let's just assert what QEMU implements. > + /* 12 -- 23: Reserved */ > + g_test_message("Verifying HBA reserved area is empty."); Debugging message that can be removed? More elsewhere in this patch. --uBaQ+BsiqpBBIRFI Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJT2kwnAAoJEJykq7OBq3PILFEIAKY2GfLzZn/7YH1XkVaTA8F+ 3hALnZO8d7Co42eZsBdv80wpvbpJgI/VEAP71vqjC85Azkhyu3oHT7L9WUj3JZg7 QsjzervROoGM7K3kLGWbrXLX1rqms1dL/xZ5HT87tMXv0OMqdyqjGTFuYgdyiMwk IjFy39AbJWe0DZsoCSZL2nyWicj5vBK+hdUnHUVclF3cVyQnlyv7Y0sGpt7x1E0M zG7Qp7lIqFa8Arf+awLVbrbXDmDFGfBkl4kleuw7I80bzGBEdUFGIvffM9CsfOUH nRh+bjZTEkaQpl6LDaSszCfhiqESWQz2DrxBWvCZzDx4pdWTJt5egAQL8o2ipYo= =uLz/ -----END PGP SIGNATURE----- --uBaQ+BsiqpBBIRFI--