From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51219) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bwioJ-0003yR-G8 for qemu-devel@nongnu.org; Wed, 19 Oct 2016 00:49:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bwioC-0001WE-SA for qemu-devel@nongnu.org; Wed, 19 Oct 2016 00:49:03 -0400 Date: Wed, 19 Oct 2016 14:09:51 +1100 From: David Gibson Message-ID: <20161019030951.GH11140@umbus.fritz.box> References: <1476787933-7180-1-git-send-email-david@gibson.dropbear.id.au> <1476787933-7180-5-git-send-email-david@gibson.dropbear.id.au> <44f479dd-27c7-2723-a551-3c7575f2889b@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="cW+P/jduATWpL925" Content-Disposition: inline In-Reply-To: <44f479dd-27c7-2723-a551-3c7575f2889b@redhat.com> Subject: Re: [Qemu-devel] [PATCH 4/8] tests: Better handle legacy IO addresses in tco-test List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Vivier Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, qemu-ppc@nongnu.org, agraf@suse.de, stefanha@redhat.com, mst@redhat.com, aik@ozlabs.ru, mdroth@linux.vnet.ibm.com, groug@kaod.org, thuth@redhat.com --cW+P/jduATWpL925 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Oct 18, 2016 at 05:14:04PM +0200, Laurent Vivier wrote: >=20 >=20 > On 18/10/2016 12:52, David Gibson wrote: > > tco_test uses the libqos PCI code to access the device. This makes per= fect > > sense for the PCI config space accesses. However for IO, rather than t= he > > usual PCI approach of mapping a PCI BAR, then accessing that, it instead > > uses the legacy approach of fixed, known addresses in PCI IO space. > >=20 > > That doesn't work very well with the qpci_io_{read,write} functions bec= ause > > we never use qpci_iomap() and so have to make assumptions about the > > internal encoding of the address tokens iomap() returns. > >=20 > > This patch avoids that, by directly using the bus's pio_{read,write} > > callbacks, which are defined to take addresses within the PCI IO space. > >=20 > > Signed-off-by: David Gibson > > --- > > tests/tco-test.c | 87 ++++++++++++++++++++++++++++--------------------= -------- > > 1 file changed, 44 insertions(+), 43 deletions(-) > >=20 > > diff --git a/tests/tco-test.c b/tests/tco-test.c > > index 0d201b1..e668630 100644 > > --- a/tests/tco-test.c > > +++ b/tests/tco-test.c > > @@ -40,13 +40,13 @@ enum { > > typedef struct { > > const char *args; > > bool noreboot; > > + QPCIBus *bus; > > QPCIDevice *dev; > > - void *tco_io_base; > > + uint16_t tco_io_base; > > } TestData; > > =20 > > static void test_init(TestData *d) > > { > > - QPCIBus *bus; > > QTestState *qs; > > char *s; > > =20 > > @@ -57,8 +57,8 @@ static void test_init(TestData *d) > > qtest_irq_intercept_in(qs, "ioapic"); > > g_free(s); > > =20 > > - bus =3D qpci_init_pc(NULL); > > - d->dev =3D qpci_device_find(bus, QPCI_DEVFN(0x1f, 0x00)); > > + d->bus =3D qpci_init_pc(NULL); >=20 > You can use qtest_pc_boot() now. I could, but that's not really in scope for this patch. > > + d->dev =3D qpci_device_find(d->bus, QPCI_DEVFN(0x1f, 0x00)); > > g_assert(d->dev !=3D NULL); > > =20 > > qpci_device_enable(d->dev); > > @@ -70,42 +70,42 @@ static void test_init(TestData *d) > > /* set Root Complex BAR */ > > qpci_config_writel(d->dev, ICH9_LPC_RCBA, RCBA_BASE_ADDR | 0x1); > > =20 > > - d->tco_io_base =3D (void *)((uintptr_t)PM_IO_BASE_ADDR + 0x60); > > + d->tco_io_base =3D PM_IO_BASE_ADDR + 0x60; >=20 > Why don't you use QPCIBar in TestData to store the address? > And you can call qpci_io_XXX() with it. As noted in your later patch, QPCIBar hasn't been introduced at this stage of the series. Further, storing the address in there would require the testcase to understand the QPCIBar's internal structure, which is exactly what I'm trying to avoid. Now, it's quite likely that the ICH9 device here has the same registers aliased in a PIO bar, which we could map in the normal way. However, if we did that, we wouldn't be testing quite the same thing - we'd be testing availability of the registers via the BAR address, rather than via the fixed legacy address. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --cW+P/jduATWpL925 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYBuP/AAoJEGw4ysog2bOSyesQAKzRMncFOdg4eyS97SEsY6TH ivBa8/ZllCAMEHBJveTWidmCmbr35gIWDRtFxVNRKvDv5u2QZ8gS4iW8BycCqUn8 mUmdkyPQeGF9RBp3782ChvxN6APBUwmPg6IvEhHnfpfoaymXkL3JNqmUVmrDnfbp IPHu8ZUf+qVqMdAP9huUCtfG8Qq0IKLGy3eCMp/cFKYw4ozhWey/ZqRxkq4JAUlQ vJbWUH3o8O8Hz1qU5ycYpxmDZtWexqhemLOye2C9EJNJghQnC4BLZ7MD3prXA7A+ txfwOWqakKf/u0BVH+4XDg3rg9XPvyCQVWYuDLmmSS+qbsLErXEFHWCBF8Flp5FL 73zlgpNqoQScENvVQyC7+38f8yAYgEZ8TVXKFKrgMiH5jw6+dL7a861rhvMgozlG DBrtY98C9zP+v1eBMyocSgwWweZq56Hi7odSpxR4JU2BYKIJJH8VgUIGAZacKb5J 74QgF1K4PdhJGQKYsv5O8DPmNzK2SBSVbVQgSBk5ue0IdavaIe4mqughx4K/Q2q4 aR6BTR8XV7ivGn3OwaQE1r4oo2ZKTjdX+vSZSKxoiPYTlo3tpkw0Rptgz1HX2uwt FiRGwQMPTlZaN5ZA/C38J3EHWm/jUlA644/G1Lm4vMlPHpcEswRXudW7ZKYpy03X 2/ux0yXzhqGJvQ+L6IgJ =ovzr -----END PGP SIGNATURE----- --cW+P/jduATWpL925--