From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58515) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YRDnO-0007EB-3U for qemu-devel@nongnu.org; Fri, 27 Feb 2015 00:49:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YRDnE-0002zU-4U for qemu-devel@nongnu.org; Fri, 27 Feb 2015 00:49:06 -0500 Date: Fri, 27 Feb 2015 16:14:57 +1100 From: David Gibson Message-ID: <20150227051457.GC29409@voom.fritz.box> References: <1424096872-29868-1-git-send-email-mdroth@linux.vnet.ibm.com> <1424096872-29868-16-git-send-email-mdroth@linux.vnet.ibm.com> <20150225031129.GD15695@voom.fritz.box> <20150225051724.31752.99805@loki> <20150225064026.31752.8222@loki> <20150226200604.31752.68449@loki> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="GZVR6ND4mMseVXL/" Content-Disposition: inline In-Reply-To: <20150226200604.31752.68449@loki> Subject: Re: [Qemu-devel] [PATCH v5 15/16] spapr_pci: enable basic hotplug operations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth Cc: aik@ozlabs.ru, qemu-devel@nongnu.org, agraf@suse.de, ncmike@ncultra.org, qemu-ppc@nongnu.org, tyreld@linux.vnet.ibm.com, bharata.rao@gmail.com, nfont@linux.vnet.ibm.com --GZVR6ND4mMseVXL/ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Feb 26, 2015 at 02:06:04PM -0600, Michael Roth wrote: > Quoting Michael Roth (2015-02-25 00:40:26) > > Quoting Michael Roth (2015-02-24 23:17:24) > > > Quoting David Gibson (2015-02-24 21:11:29) > > > > On Mon, Feb 16, 2015 at 08:27:51AM -0600, Michael Roth wrote: [snip] > > > > > + } > > > > > + > > > > > + assigned =3D &rp->assigned.fields[assigned_idx++]; > > > > > + assigned->phys_hi =3D cpu_to_be32(reg->phys_hi | b_n(1)); > > > > > + assigned->phys_mid =3D cpu_to_be32(d->io_regions[i].addr= >> 32); > > > > > + assigned->phys_lo =3D cpu_to_be32(d->io_regions[i].addr); > > > >=20 > > > > Not sure if I understood the comment above properly; should these > > > > addresses actually be translated through an mappings above the PHB? > > > > Not that there are any such translations on sPAPR, but... > > >=20 > > > The io_regions[i].addr values are relative to the IO/MEM address spac= es for > > > the device, which correspond to IO/MEM windows for the PHB. So I thin= k the > > > memory API should handle any translation above that... > > >=20 > > > Or do you mean because of the n=3D0/relocatable IO regions described = by 'reg'? > > >=20 > > > IIUC, when n=3D0, reg->phys_{mid,lo} can be used to encode a starting= offset, > > > so a guest can re-assign/re-program an IO region that's already been = assigned > > > and described via the correspond fields in 'assigned-addresses', so l= ong as > > > uses an addr above reg->phys{mid,lo}. > > >=20 > > > So, we could use those reg->phys_{mid,lo} values as an alternative to= the > > > PHBs IO/MEM windows (I guess for platforms that don't provide these w= indows > > > and just expose the full address space?). > > >=20 > > > But since we have those windows, we end up not needing this, so we al= ways do > > > n=3D0, reg->phys_mid =3D reg->phys_lo =3D 0, so the values in > > > assigned->phys_{mid,lo} end up just being offsets into the IO/MEM win= dows, > > > which correspond directly to io_regions[n].addr. > >=20 > > Argh, not sure what I was thinking. io_regions[n].addr is relative to t= he > > IO/MEM window, so from the guest perspective .addr is indeed a different > > value... > >=20 > > So maybe reg->phys_mid/reg->phys_lo do in fact need to reflect the wind= ow > > offsets so that the relocatable region assignments are offset > > accordingly... > >=20 > > Will look into it more. >=20 > So, maybe my first response wasn't as misguided as I thought. I haven't > found any documentation that lays out the specifics, but the kernel code = makes > it fairly clear that the IO/MEM addresses in assigned-addresses are relat= ive > to PHB's IO/MEM windows. >=20 > The code basically does this: >=20 > arch/powerpc/kernel/pci_of_scan.c: of_pci_parse_addrs() > addrs =3D of_get('assigned-addresses') > config_offset =3D addrs[0] && 0xff (phys.hi) > pci_bar_num =3D (config_offset - PCI_BASE_ADDRESS_0) / 4 > base =3D &addrs[1:2] (phys.mid/phys.lo) > size =3D &addrs[3:4] (size.hi, size.lo) > region.start =3D base > region.end =3D base + size -1 > res =3D &dev->resource[pci_bar_num]; > pcibios_bus_to_resource(dev->bus, res, ®ion) > drivers/pci/host-bridge.c:pcibios_bus_to_resource(bus, bar_res, bar_reg= ion) > bridge =3D pci_host_bridge(bus) > for window in bridge->windows: > # skip if window type (IO/MEM), doesn't match device BAR's res) > bus_region.start =3D window->res->start - window->offset > bus_region.end =3D window->res->end - window->offset > if bus_region contains device BAR region: > offset =3D window->offset > break > //take the region, add the window offset, and use that as the resourc= e addr > res->start =3D region->start + offset > res->end =3D region->end + offset >=20 > I tested this with the rpaphp hotplug module, which honors > assigned-addresses (current guest hotplug uses generic pci rescan instead > due to a long-standing issue with rpaphp only being able to handle > 1 slot per PHB if there are devices present at module-load time), by addi= ng > some test code to pre-assign the BARs during hotplug (with addresses rela= tive > to PHB's IO/MEM windows) to make sure assigned-addresses values get prope= rly > translated to the correct offset in guest's io/system address space. >=20 > Will add a note in the comments. Ok, sounds like the only problem was a confusing description in the comment that made it seem like the assigned-address values were GPAs rather than relative to the PHB windows. --=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 --GZVR6ND4mMseVXL/ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJU7/1RAAoJEGw4ysog2bOSc5oQAKnhyHN07q+VmI00sUNIyBde 1jq38PqEUzsxgVV/hJ/7k2jpGlwtim92Dw89WDYj5ANQN8H63xtLmikA6KzQtD7G FfK3ofuUnoXDwqHDDIqR+5m5zWAYL4B8WZdgHJS/H3K9V4XTLWOS19JwiAYzSThi MwJI1PaSn8I44yc0DlT01zmDjvV9AmLPdAwmfpCoyw4HLu8bdlCXsqp1g5VFEYsI JuQk6laoVzY9AEN1kchqQAlEFT8xC2y+HPBQymzbEpNOTBxXSwiCm6PM5Ogb92Ir 8n6wabwe+AaJihYD+gyZZdHVorPKqCeyKlZ2WwilmrnAwWOi1XV6x9W4HaFb+80+ RSJqq1kbjgwG/rUL1avE1wt1N6J5bH6G3DP2GEOrDkY5ua4wy+9HWXB5uBrNorrr Y7AgpI1FBYJb6H/OZEunFzcO0/Yy3xgvo614KA/ByPZ7TVBNcKu6zuNCMPX+J309 YRvrgMbxX8GSGOUafODClnm8SLku6vaZ/O+CCtIHv29ukdvn2DZRaEKB80VfHati r99QIC0mS2w9r4XiuXUcSMrfBBOns0bdfhvvttg0FGrdVN+vSbXwb6pv4QNmwwPh Smu+AjuKJJpw7noYc2LV2ApZzjlBWOVHiUXA/8cGhPCJsWljj53KC4vBY7hxBndv o1YlizYkkAV4PQQKlf4F =N6uG -----END PGP SIGNATURE----- --GZVR6ND4mMseVXL/--