From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43929) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yclxc-0007fd-AV for qemu-devel@nongnu.org; Mon, 30 Mar 2015 22:31:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yclxa-0006h1-Ty for qemu-devel@nongnu.org; Mon, 30 Mar 2015 22:31:24 -0400 Date: Tue, 31 Mar 2015 13:29:54 +1100 From: David Gibson Message-ID: <20150331022954.GV9908@voom.fritz.box> References: <1427713337-4295-1-git-send-email-nikunj@linux.vnet.ibm.com> <5519FFD9.2040209@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="wwU9tsYnHnYeRAKj" Content-Disposition: inline In-Reply-To: <5519FFD9.2040209@ozlabs.ru> Subject: Re: [Qemu-devel] [PATCH v3] spapr: populate ibm,loc-code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org, Nikunj A Dadhania , agraf@suse.de --wwU9tsYnHnYeRAKj Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Mar 31, 2015 at 01:00:57PM +1100, Alexey Kardashevskiy wrote: > On 03/30/2015 10:02 PM, Nikunj A Dadhania wrote: > >Each hardware instance has a platform unique location code. The OF > >device tree that describes a part of a hardware entity must include > >the =E2=80=9Cibm,loc-code=E2=80=9D property with a value that represents= the location > >code for that hardware entity. > > > >Introduce an rtas call to populate ibm,loc-code. > >1) PCI passthru devices need to identify with its own ibm,loc-code > > available on the host. > >2) Emulated devices encode as following: > > qemu_::. > > > >Signed-off-by: Nikunj A Dadhania > >--- > > > > > >Changelog > >v2: > >* Using rtas call for getting ibm,loc-code > >* Added sPAPRPHBState::get_loc_code > >* Refactored the return type of get_loc_code > >* Drop stat(), and rely on g_file_get_contents > > return type for file existence > > > >v1: > >* Dropped is_vfio patch and using TYPE_SPAPR_PCI_VFIO_HOST_BRIDGE > > to recognise vfio devices > >* Removed wrapper for hcall > >* Added sPAPRPHBClass::get_loc_code > > > > hw/ppc/spapr_pci.c | 70 +++++++++++++++++++++++++++++++++++++= ++++++++ > > hw/ppc/spapr_pci_vfio.c | 40 ++++++++++++++++++++++++++ > > include/hw/pci-host/spapr.h | 1 + > > include/hw/ppc/spapr.h | 3 +- > > 4 files changed, 113 insertions(+), 1 deletion(-) > > > >diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > >index 05f4fac..fe6dfd5 100644 > >--- a/hw/ppc/spapr_pci.c > >+++ b/hw/ppc/spapr_pci.c > >@@ -580,6 +580,50 @@ param_error_exit: > > rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > > } > > > >+static void rtas_ibm_get_loc_code(PowerPCCPU *cpu, > >+ sPAPREnvironment *spapr, > >+ uint32_t token, uint32_t nargs, > >+ target_ulong args, uint32_t nret, > >+ target_ulong rets) > >+{ > >+ sPAPRPHBState *sphb =3D NULL; > >+ sPAPRPHBClass *spc =3D NULL; > >+ char *buf =3D NULL; > >+ PCIDevice *pdev; > >+ uint64_t buid; > >+ uint32_t config_addr, loc_code, size; > >+ > >+ if ((nargs !=3D 5) || (nret !=3D 1)) { > >+ goto param_error_exit; > >+ } > >+ > >+ config_addr =3D rtas_ld(args, 0); > >+ buid =3D ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2); > >+ loc_code =3D rtas_ld(args, 3); > >+ size =3D rtas_ld(args, 4); > >+ > >+ sphb =3D find_phb(spapr, buid); > >+ pdev =3D find_dev(spapr, buid, config_addr); > >+ > >+ if (!sphb || !pdev) { > >+ goto param_error_exit; > >+ } > >+ > >+ spc =3D SPAPR_PCI_HOST_BRIDGE_GET_CLASS(sphb); > >+ if (spc->get_loc_code && spc->get_loc_code(sphb, pdev, &buf)) { > >+ uint32_t loc_len =3D strlen(buf); > >+ > >+ loc_len =3D (loc_len > size) ? size : loc_len; > >+ cpu_physical_memory_write(loc_code, buf, loc_len); > >+ g_free(buf); > >+ rtas_st(rets, 0, RTAS_OUT_SUCCESS); > >+ return; > >+ } > >+ > >+param_error_exit: > >+ rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR); > >+} > >+ > > static void rtas_ibm_configure_pe(PowerPCCPU *cpu, > > sPAPREnvironment *spapr, > > uint32_t token, uint32_t nargs, > >@@ -909,6 +953,27 @@ static void spapr_phb_finish_realize(sPAPRPHBState = *sphb, Error **errp) > > spapr_tce_get_iommu(tcet)); > > } > > > >+static bool spapr_phb_get_loc_code(sPAPRPHBState *sphb, PCIDevice *pde= v, > >+ char **loc_code) > >+{ > >+ char *path =3D g_malloc(PATH_MAX); > >+ > >+ if (!path) { > >+ return false; > >+ } > >+ > >+ /* > >+ * For non-vfio devices and failures make up the location code out > >+ * of the name, slot and function. > >+ * > >+ * qemu_::. > >+ */ > >+ snprintf(path, PATH_MAX, "qemu_%s:%02d:%02d.%1d", pdev->name, > >+ sphb->index, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn)); > >+ *loc_code =3D path; > >+ return true; > >+} > >+ > > static int spapr_phb_children_reset(Object *child, void *opaque) > > { > > DeviceState *dev =3D (DeviceState *) object_dynamic_cast(child, TY= PE_DEVICE); > >@@ -1058,6 +1123,7 @@ static void spapr_phb_class_init(ObjectClass *klas= s, void *data) > > set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); > > dc->cannot_instantiate_with_device_add_yet =3D false; > > spc->finish_realize =3D spapr_phb_finish_realize; > >+ spc->get_loc_code =3D spapr_phb_get_loc_code; > > } > > > > static const TypeInfo spapr_phb_info =3D { > >@@ -1245,6 +1311,10 @@ void spapr_pci_rtas_init(void) > > spapr_rtas_register(RTAS_IBM_SLOT_ERROR_DETAIL, > > "ibm,slot-error-detail", > > rtas_ibm_slot_error_detail); > >+ spapr_rtas_register(RTAS_IBM_GET_LOC_CODE, > >+ "ibm,get-loc-code", >=20 >=20 > s/ibm,get-loc-code/qemu,get-loc-code/ as it is not from sPAPR. Agreed. > btw we could make it even simpler and just put a property under PHB which > would contain a map slot:function<->loc-code, the map would be per PHB and > SLOF would just parse it and not call RTAS or do a hypercall. When we get > PCI scan in QEMU, we will just remove this chunk from the device tree (and > put loc-code from it to device nodes), and we won't have polluted RTAS to= ken > namespace. The current thing looks too complicated for such a simple > function. Dunno... I think that's a good idea. Introducing a callback to get device information seems pretty odd, when the device tree exists to communicate static device information to the guest. If we're not able to go straight to qemu doing the PCI scan, I think a PHB property listing this information is a better option than an RTAS callback or hcall. --=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 --wwU9tsYnHnYeRAKj Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJVGgaiAAoJEGw4ysog2bOSNZ4QAL1K7crCPiHJ8TLJVnn3RWA4 EkKXWbapkpZDV1yDmJaztgzxyNxq3xvVFNftpYPdiMylaSTT4NOxWoM2SNv416ft WC7eswrzO1XTmLWfeh6gvQEzvSo6HP0ElaB91Ktu2Vr+TKWTWXk75Ni8VH6TLrgq i0S1iXQ0AqVsh8/Mz4TLeOe6yEOc4cX3gS2XXxDvwMdRF5aQCx3U0k0bn2DxU6Wh joJY0RW9cRn5i3fSpX4jsHWzBqLgEI4WEWb0pXCojzcRP1wuUHXRDPtIKdgmmC1f OrqODMghnC8+PBL3WNt+XDpZwprMHymDXm0/oR+IkbJNEOOon2ftFPf0h+O1yj7q aMgyD2g0d940T1MghVnIKC3GmLRnITCuIsWO1kjdtB2F71w2KK2dktxOyu1oM2HD qEqr9CSS4cT1GhCvLrI1355Wt5RENSrHX7jl5Hqn5RUigMxSNiLJtwNeKkNX4Bty fNfW3Ok9QkfnQy0P7s6pbhT2r0vXm7+jy5fpNV5DHygyCVhP/CyqhvVGFZfkUHNO WKbFWLeR8HAactQHFOEJViriVpyCjmaJgRDX7MM2a1jcRbQ64ttdsGLH84KFJHDg I2msxMCqaz0gNASfdJ4sScBPoOHBKXRaRJEQZmFuyea7UAxtbml2k6OIR5zql8uk 0YoJl46qZEvJfDo/yc6q =3wtC -----END PGP SIGNATURE----- --wwU9tsYnHnYeRAKj--