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. > > > > 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. > > > > 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. > > > > Cc: Bharata B Rao <bhar...@linux.vnet.ibm.com> > > Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com> > > Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
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"
> >
> > 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;
> >
> > #define OV_BIT(byte, bit) ((byte - 1) * BITS_PER_BYTE + bit)
> >
> > +/* option vector 5 */
> > +#define OV5_DRCONF_MEMORY OV_BIT(2, 2)
> > +
> > /* interfaces */
> > sPAPROptionVector *spapr_ovec_new(void);
> > sPAPROptionVector *spapr_ovec_clone(sPAPROptionVector *ov_orig);
>
--
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
signature.asc
Description: PGP signature
