From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35419) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1but66-0006X8-8e for qemu-devel@nongnu.org; Thu, 13 Oct 2016 23:23:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1but62-0002Qt-29 for qemu-devel@nongnu.org; Thu, 13 Oct 2016 23:23:49 -0400 Date: Fri, 14 Oct 2016 14:02:31 +1100 From: David Gibson Message-ID: <20161014030231.GB28562@umbus> References: <1476314039-9520-1-git-send-email-mdroth@linux.vnet.ibm.com> <1476314039-9520-3-git-send-email-mdroth@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Pd0ReVV5GZGQvF3a" Content-Disposition: inline In-Reply-To: <1476314039-9520-3-git-send-email-mdroth@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH 02/11] spapr_hcall: use spapr_ovec_* interfaces for CAS options List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, bharata@linux.vnet.ibm.com, nfont@linux.vnet.ibm.com, jallen@linux.vnet.ibm.com --Pd0ReVV5GZGQvF3a Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Oct 12, 2016 at 06:13:50PM -0500, Michael Roth wrote: > Currently we access individual bytes of an option vector via > ldub_phys() to test for the presence of a particular capability > within that byte. Currently this is only done for the "dynamic > reconfiguration memory" capability bit. If that bit is present, > we pass a boolean value to spapr_h_cas_compose_response() > to generate a modified device tree segment with the additional > properties required to enable this functionality. >=20 > As more capability bits are added, will would need to modify the > code to add additional option vector accesses and extend the > param list for spapr_h_cas_compose_response() to include similar > boolean values for these parameters. >=20 > Avoid this by switching to spapr_ovec_* helpers so we can do all > the parsing in one shot and then test for these additional bits > within spapr_h_cas_compose_response() directly. >=20 > Cc: Bharata B Rao > Signed-off-by: Michael Roth Reviewed-by: David Gibson > --- > hw/ppc/spapr.c | 10 ++++++-- > hw/ppc/spapr_hcall.c | 56 ++++++++++++---------------------------= ------ > include/hw/ppc/spapr.h | 5 +++- > include/hw/ppc/spapr_ovec.h | 3 +++ > 4 files changed, 30 insertions(+), 44 deletions(-) >=20 > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 03e3803..934d6b2 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -856,7 +856,7 @@ out: > =20 > int spapr_h_cas_compose_response(sPAPRMachineState *spapr, > target_ulong addr, target_ulong size, > - bool cpu_update, bool memory_update) > + bool cpu_update) > { > void *fdt, *fdt_skel; > sPAPRDeviceTreeUpdateHeader hdr =3D { .version_id =3D 1 }; > @@ -880,7 +880,8 @@ int spapr_h_cas_compose_response(sPAPRMachineState *s= papr, > } > =20 > /* Generate ibm,dynamic-reconfiguration-memory node if required */ > - if (memory_update && smc->dr_lmb_enabled) { > + if (spapr_ovec_test(spapr->ov5_cas, OV5_DRCONF_MEMORY)) { > + g_assert(smc->dr_lmb_enabled); > _FDT((spapr_populate_drconf_memory(spapr, fdt))); > } > =20 > @@ -1769,7 +1770,12 @@ static void ppc_spapr_init(MachineState *machine) > DIV_ROUND_UP(max_cpus * smt, smp_thre= ads), > XICS_IRQS_SPAPR, &error_fatal); > =20 > + /* Set up containers for ibm,client-set-architecture negotiated opti= ons */ > + spapr->ov5 =3D spapr_ovec_new(); > + spapr->ov5_cas =3D spapr_ovec_new(); > + > if (smc->dr_lmb_enabled) { > + spapr_ovec_set(spapr->ov5, OV5_DRCONF_MEMORY); > spapr_validate_node_memory(machine, &error_fatal); > } > =20 > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index c5e7e8c..f1d081b 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -11,6 +11,7 @@ > #include "trace.h" > #include "sysemu/kvm.h" > #include "kvm_ppc.h" > +#include "hw/ppc/spapr_ovec.h" > =20 > struct SPRSyncState { > int spr; > @@ -880,32 +881,6 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, sPAP= RMachineState *spapr, > return ret; > } > =20 > -/* > - * Return the offset to the requested option vector @vector in the > - * option vector table @table. > - */ > -static target_ulong cas_get_option_vector(int vector, target_ulong table) > -{ > - int i; > - char nr_vectors, nr_entries; > - > - if (!table) { > - return 0; > - } > - > - nr_vectors =3D (ldl_phys(&address_space_memory, table) >> 24) + 1; > - if (!vector || vector > nr_vectors) { > - return 0; > - } > - table++; /* skip nr option vectors */ > - > - for (i =3D 0; i < vector - 1; i++) { > - nr_entries =3D ldl_phys(&address_space_memory, table) >> 24; > - table +=3D nr_entries + 2; > - } > - return table; > -} > - > typedef struct { > uint32_t cpu_version; > Error *err; > @@ -961,23 +936,21 @@ static void cas_handle_compat_cpu(PowerPCCPUClass *= pcc, uint32_t pvr, > } > } > =20 > -#define OV5_DRCONF_MEMORY 0x20 > - > static target_ulong h_client_architecture_support(PowerPCCPU *cpu_, > sPAPRMachineState *spa= pr, > target_ulong opcode, > target_ulong *args) > { > target_ulong list =3D ppc64_phys_to_real(args[0]); > - target_ulong ov_table, ov5; > + target_ulong ov_table; > PowerPCCPUClass *pcc =3D POWERPC_CPU_GET_CLASS(cpu_); > CPUState *cs; > - bool cpu_match =3D false, cpu_update =3D true, memory_update =3D fal= se; > + bool cpu_match =3D false, cpu_update =3D true; > unsigned old_cpu_version =3D cpu_->cpu_version; > unsigned compat_lvl =3D 0, cpu_version =3D 0; > unsigned max_lvl =3D get_compat_level(cpu_->max_compat); > int counter; > - char ov5_byte2; > + sPAPROptionVector *ov5_guest; > =20 > /* Parse PVR list */ > for (counter =3D 0; counter < 512; ++counter) { > @@ -1033,19 +1006,20 @@ static target_ulong h_client_architecture_support= (PowerPCCPU *cpu_, > /* For the future use: here @ov_table points to the first option vec= tor */ > ov_table =3D list; > =20 > - ov5 =3D cas_get_option_vector(5, ov_table); > - if (!ov5) { > - return H_SUCCESS; > - } > + ov5_guest =3D spapr_ovec_parse_vector(ov_table, 5); > =20 > - /* @list now points to OV 5 */ > - ov5_byte2 =3D ldub_phys(&address_space_memory, ov5 + 2); > - if (ov5_byte2 & OV5_DRCONF_MEMORY) { > - memory_update =3D true; > - } > + /* NOTE: there are actually a number of ov5 bits where input from the > + * guest is always zero, and the platform/QEMU enables them independ= ently > + * of guest input. To model these properly we'd want some sort of ma= sk, > + * but since they only currently apply to memory migration as defined > + * by LoPAPR 1.1, 14.5.4.8, which QEMU doesn't implement, we don't n= eed > + * to worry about this. > + */ > + spapr_ovec_intersect(spapr->ov5_cas, spapr->ov5, ov5_guest); > + spapr_ovec_cleanup(ov5_guest); > =20 > if (spapr_h_cas_compose_response(spapr, args[1], args[2], > - cpu_update, memory_update)) { > + cpu_update)) { > qemu_system_reset_request(); > } > =20 > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 39dadaa..6c20d28 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -6,6 +6,7 @@ > #include "hw/ppc/xics.h" > #include "hw/ppc/spapr_drc.h" > #include "hw/mem/pc-dimm.h" > +#include "hw/ppc/spapr_ovec.h" > =20 > struct VIOsPAPRBus; > struct sPAPRPHBState; > @@ -66,6 +67,8 @@ struct sPAPRMachineState { > uint64_t rtc_offset; /* Now used only during incoming migration */ > struct PPCTimebase tb; > bool has_graphics; > + sPAPROptionVector *ov5; > + sPAPROptionVector *ov5_cas; > =20 > uint32_t check_exception_irq; > Notifier epow_notifier; > @@ -577,7 +580,7 @@ void spapr_events_init(sPAPRMachineState *sm); > void spapr_events_fdt_skel(void *fdt, uint32_t epow_irq); > int spapr_h_cas_compose_response(sPAPRMachineState *sm, > target_ulong addr, target_ulong size, > - bool cpu_update, bool memory_update); > + bool cpu_update); > sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn); > void spapr_tce_table_enable(sPAPRTCETable *tcet, > uint32_t page_shift, uint64_t bus_offset, > diff --git a/include/hw/ppc/spapr_ovec.h b/include/hw/ppc/spapr_ovec.h > index fba2d98..09afd59 100644 > --- a/include/hw/ppc/spapr_ovec.h > +++ b/include/hw/ppc/spapr_ovec.h > @@ -42,6 +42,9 @@ typedef struct sPAPROptionVector sPAPROptionVector; > =20 > #define OV_BIT(byte, bit) ((byte - 1) * BITS_PER_BYTE + bit) > =20 > +/* option vector 5 */ > +#define OV5_DRCONF_MEMORY OV_BIT(2, 2) > + > /* interfaces */ > sPAPROptionVector *spapr_ovec_new(void); > sPAPROptionVector *spapr_ovec_clone(sPAPROptionVector *ov_orig); --=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 --Pd0ReVV5GZGQvF3a Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYAErEAAoJEGw4ysog2bOSQsoQALKpYLLfjFTeXeX9tPFLotg1 5XqszOzT/hZaKRQP20imXg4Am+G3GmnswpvPTWx/sJ1uHvuEx01YS6oMd1+br/17 iVb7pE8X2UPRl58uj0LvxUpvBej1afZht7DI2F11iqR1cAurstG9c5aD45ll8K6a v3W64Yw9S188X/eptAWHIoa3Mimm/0O9JFVXreBIGEunwitjufGTu3Dt9XDXgu0l eX3gP/lcX/LMs2Hc64v1OVqpNiJQMu46Mzt6MipBdXu3F27/O0Rg6cxpcN+Lq1F7 NaugI+alGChq6VA03MvRoY40efjcIhzKrnwFFMSkoqC95YxC8CoYKoxzvtsMaFMC G6e0ABNWkd92rW6BFgbKYqQ5JIk7eX+fCWfJJV7X6yhrjG891AWSBERerj3XyWVx KosrbJkrRleFgbfpPyswQ/UG2EMQsHdS3Ud+oFduI/i3C9EoDLD3tvqqJHVI3nLK rwrYG8wUyLOeYj1ntEfTuQvGQ8yIYvkvNQceRdzwHt0DbXZjiQsMwqxRzcTMzbYJ 1UloFTokbcx0bHWZNkuBrgFIcViOwj0KEFB7g1xTep5HQPnKVQBWVENFPOGzM/iD zgSZCJjOecZZnur4ZAkoLYsZ+V+wQ2kUU4BFAofK0VOkbjbF+iGYxeJ5OOFWcms6 3Xv18MsbdhmRQmgPJ9ZG =+17s -----END PGP SIGNATURE----- --Pd0ReVV5GZGQvF3a--