Re: [Qemu-devel] [Qemu-ppc] [QEMU-PPC] [RFC 1/3] hw/ppc/spapr_caps: Rework spapr_caps to use uint8 internal representation

[Suraj Jitindar Singh]

On Tue, 2018-01-09 at 09:13 -0200, Murilo Opsfelder Araújo wrote:
> On 01/09/2018 07:21 AM, Suraj Jitindar Singh wrote:
> > Currently spapr_caps are tied to boolean values (on or off). This
> > patch
> > reworks the caps so that they can have any value between 0 and 127,
> > inclusive. This allows more capabilities with various values to be
> > represented in the same way internally. Capabilities are numbered
> > in
> > ascending order. The internal representation of capability values
> > is an
> > array of uint8s in the sPAPRMachineState, indexed by capability
> > number.
> > Note: The MSB (0x80) of a capability is reserved to track whether
> > the
> >       capability was set from the command line.
> > 
> > Capabilities can have their own name, description, options, getter
> > and
> > setter functions, type and allow functions. They also each have
> > their own
> > section in the migration stream. Capabilities are only migrated if
> > they
> > were explictly set on the command line, with the assumption that
> > otherwise the default will match.
> > 
> > On migration we ensure that the capability value on the destination
> > is greater than or equal to the capability value from the source.
> > So
> > long at this remains the case then the migration is considered
> > compatible and allowed to continue.
> > 
> > This patch implements generic getter and setter functions for
> > boolean
> > capabilities. It also converts the existings cap-htm, cap-vsx and
> > cap-dfp capabilities to this new format.
> 
> Hi, Suraj.
> 
> I've got the impression that this patch does a lot of things. What
> about
> splitting this patch into the following?
> 
> - rename spapr_has_cap() -> spapr_get_cap()
> - introduce each spapr_cap_[gs]et_bool() in separate patches
> - make use of spapr_cap[gs]et_bool()
> - convert capabilities internal representation to uint8
> - add each new capability separately

Absolutely. This was an RFC to get the code out there for comment.
I'll try to split it up further as suggested for the actual patch
series :)

> 
> Perhaps it can be broken into even smaller changes.
> 
> Cheers
> Murilo
>