From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51316) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bwioL-0003yc-EF 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-0001W8-Rc for qemu-devel@nongnu.org; Wed, 19 Oct 2016 00:49:05 -0400 Date: Wed, 19 Oct 2016 14:19:11 +1100 From: David Gibson Message-ID: <20161019031911.GI11140@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> <634b8b57-9069-a077-2ccf-53bfbd9ed35f@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="HuXIgs6JvY9hJs5C" Content-Disposition: inline In-Reply-To: <634b8b57-9069-a077-2ccf-53bfbd9ed35f@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 --HuXIgs6JvY9hJs5C Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Oct 18, 2016 at 06:28:26PM +0200, Laurent Vivier wrote: >=20 >=20 > On 18/10/2016 17:14, 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 pe= rfect > >> sense for the PCI config space accesses. However for IO, rather than = the > >> usual PCI approach of mapping a PCI BAR, then accessing that, it inste= ad > >> uses the legacy approach of fixed, known addresses in PCI IO space. > >> > >> That doesn't work very well with the qpci_io_{read,write} functions be= cause > >> we never use qpci_iomap() and so have to make assumptions about the > >> internal encoding of the address tokens iomap() returns. > >> > >> 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. > >> > >> Signed-off-by: David Gibson > >> --- > >> tests/tco-test.c | 87 ++++++++++++++++++++++++++++-------------------= --------- > >> 1 file changed, 44 insertions(+), 43 deletions(-) > >> > >> 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. > >=20 > >> + 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. >=20 > OK, I was watching the state of qpci_io_XXX() after the series, so you > can't do that here, but perhaps this patch should be moved to PATCH 8/8? No, this is quite deliberately before 8/8. The point of 8/8 is that QPCIBar is supposed to be opaque to the tests/drivers. Using the legacy addresses requires that the test/driver work directly with the actual IO address. Mixing the two - having the test case construct the base pointer makes the conversion to an opaque token more awkward. However.. another approach just occurred to me. I could define a special QPCIBar constant - exported as a global by libqos, or returned by a function - which references the legacy PIO space instead of a particular bar. The qpci_io_*() functions could be used with that. I'll look into that, and see how it works. --=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 --HuXIgs6JvY9hJs5C Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYBuYvAAoJEGw4ysog2bOSlqIQAKNnpH9pypsp3a8dUDng1OSW SCpixGIfJ3151kL4RcxKqX5eJyo1ALo5T5iuQnq6DiHLuFwx5hYD17aUJlxTt2WJ QTWy4XbfSKDX9bDtxrDbaoc23mb+lpHXvXK/LrLs8IeHYtPwZrcUTqOToeZ0EJUf Fj76MYLFVFBiEaqE9K027Ul8GOL8U4NH1L9UXEvRnhNM0h6EY7thbOW5b2e6JH3m hcLmsa1D4TEsVg6zlP4bPU4TdUjcfAVwhNvRsydqVkSlalw8UoXJzF4Sz7Rp+fRf mJxE4MMAd6yXUZtsGC9jH6dxTQonWbuLV7gM1oBH02ftgvKAFKUz1au0lbDTRUcD j7IP921gr51nFuj0kXD5n2wkRnMJ9RIOlaoSLgUpziSNLQvUhI9amXJ/1Ip5laNL PsTY7i9zSU/fNoGd2hPWgoX1YUaUFW6VbmgCITx9I/r7gh/D36gjVPo5vH6QJrJF 7lksnp0lRO0I8VdV/8HaLXfoiUE2dSWPXfIdGk+gyG9UgzkGLTj9OWD1urwSIE3x ntUKE/R+QHFbSFxYNguvTI60+SxYoeDJTQLfh1+vxYbwuhPlgdlAuEOkS1w1nt1a C3eZnavP2Swf5hcLt4wwWVmeQ35Rl/QIW6L+fYWjGOm/NJ09by5bvSXxezuM4y/F PJSc6ae3v1AIg32WVYVn =MfQT -----END PGP SIGNATURE----- --HuXIgs6JvY9hJs5C--