From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48962) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buubw-0008Us-Hk for qemu-devel@nongnu.org; Fri, 14 Oct 2016 01:00:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1buubt-00039b-T3 for qemu-devel@nongnu.org; Fri, 14 Oct 2016 01:00:47 -0400 Date: Fri, 14 Oct 2016 15:20:32 +1100 From: David Gibson Message-ID: <20161014042032.GF28562@umbus> References: <1476314039-9520-1-git-send-email-mdroth@linux.vnet.ibm.com> <1476314039-9520-3-git-send-email-mdroth@linux.vnet.ibm.com> <20161014030231.GB28562@umbus> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Zi0sgQQBxRFxMTsj" Content-Disposition: inline In-Reply-To: <20161014030231.GB28562@umbus> 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 --Zi0sgQQBxRFxMTsj Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Oct 14, 2016 at 02:02:31PM +1100, David Gibson wrote: > 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 >=20 > Reviewed-by: David Gibson That said.. some comments making the overall scheme here might be helpful. Specifically.. [snip] > > 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; IIUC, the ov5 represents all the features qemu is capable of supporting, and ov5_cas records the ones that were actually negotiated during CAS. Some descriptions here could make that much easier to follow. > > 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 --=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 --Zi0sgQQBxRFxMTsj Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYAF0QAAoJEGw4ysog2bOSuhsP/3IVOcfQh6vRZxj8O5j/1GHt O04KVOk+W8bB1LU16XkLpxWoAtwL+yF4w+aDJm0SHE6a2mqqIvZrYPcoDXQz1e0w dXqzcmosCSv1SWxhkfFeHB2L7tTH4fh7vxBovWtQe/YRSwOckdb84Us0vU1IB654 K3q7wJb9bRTPAjq3cRiVY+B6PP3u6Yv1uK9jYUAon8b54DxDzNqurLpUKHDry0EK 6kNlkbdOtGE0HOiSocthm64UZBfcstLfLUSV38GE0Kr5dMkfi+f6bj84SqsAZON5 vKRSME+7tzaFHzJv3jYOE+ODULMTqd3lRa2I7tH3lohejWoYDc1NSG/lYM7jxef/ X0KuiBrpGK1hf+rOuK6UWv/qYbv/NzBEWeotHfGujSeGmVh04ltJzWjAl3uiIgC8 FLWN5vdLuqKQTuXlCgfu+VL1sw/PceK1cgSkMHAPVFkRPRflwLiOFeiGtZ0G+m1r iCEBvBny4rvQwGyp71eKA5juSIWD/7sh21AsNsrwpwpMWvy4vUdbK8W/xO9P44LB 7NBVO/IcvwDldQIcr08Tl6o/fanKdyMsOYI21rqfV9mshLE5EoiC72baLbUjH9UM DN8KIcoSmFsMI3pfysvpfhEJxNRdDIm0xqdPtujKZxZ6BtUiSqeEspTfztJlGFog u7Gc4bXU8MUg5JnFY3sI =egQx -----END PGP SIGNATURE----- --Zi0sgQQBxRFxMTsj--