[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [QEMU-PPC] [PATCH V5 2/7] target/ppc/spapr_caps: Add ma
From: |
Suraj Jitindar Singh |
Subject: |
Re: [Qemu-devel] [QEMU-PPC] [PATCH V5 2/7] target/ppc/spapr_caps: Add macro to generate spapr_caps migration vmstate |
Date: |
Mon, 29 Jan 2018 10:43:34 +1100 |
On Fri, 2018-01-19 at 16:18 +1100, Alexey Kardashevskiy wrote:
> On 19/01/18 16:00, Suraj Jitindar Singh wrote:
> > The vmstate description and the contained needed function for
> > migration
> > of spapr_caps is the same for each cap, with the name of the cap
> > substituted. As such introduce a macro to allow for easier
> > generation of
> > these.
> >
> > Convert the three existing spapr_caps (htm, vsx, and dfp) to use
> > this
> > macro.
> >
> > Signed-off-by: Suraj Jitindar Singh <address@hidden>
> >
> > ---
> >
> > V5:
> > - Patch added to series
> > ---
> > hw/ppc/spapr_caps.c | 78 +++++++++++++++++----------------------
> > --------------
> > 1 file changed, 24 insertions(+), 54 deletions(-)
> >
> > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> > index d5c9ce774a..5d52969bd5 100644
> > --- a/hw/ppc/spapr_caps.c
> > +++ b/hw/ppc/spapr_caps.c
> > @@ -228,62 +228,32 @@ int
> > spapr_caps_post_migration(sPAPRMachineState *spapr)
> > return ok ? 0 : -EINVAL;
> > }
> >
> > -static bool spapr_cap_htm_needed(void *opaque)
> > -{
> > - sPAPRMachineState *spapr = opaque;
> > -
> > - return spapr->cmd_line_caps[SPAPR_CAP_HTM] &&
> > - (spapr->eff.caps[SPAPR_CAP_HTM] != spapr-
> > >def.caps[SPAPR_CAP_HTM]);
> > -}
> > -
> > -const VMStateDescription vmstate_spapr_cap_htm = {
> > - .name = "spapr/cap/htm",
> > - .version_id = 1,
> > - .minimum_version_id = 1,
> > - .needed = spapr_cap_htm_needed,
> > - .fields = (VMStateField[]) {
> > - VMSTATE_UINT8(mig.caps[SPAPR_CAP_HTM], sPAPRMachineState),
> > - VMSTATE_END_OF_LIST()
> > - },
> > -};
> > -
> > -static bool spapr_cap_vsx_needed(void *opaque)
> > -{
> > - sPAPRMachineState *spapr = opaque;
> > -
> > - return spapr->cmd_line_caps[SPAPR_CAP_VSX] &&
> > - (spapr->eff.caps[SPAPR_CAP_VSX] != spapr-
> > >def.caps[SPAPR_CAP_VSX]);
> > +/* Used to generate the migration field and needed function for a
> > spapr cap */
> > +#define SPAPR_CAP_MIG_STATE(cap, ccap) \
> > +static bool spapr_cap_##cap##_needed(void *opaque) \
> > +{ \
> > + sPAPRMachineState *spapr = opaque; \
> > + \
> > + return spapr->cmd_line_caps[SPAPR_CAP_##ccap] && \
> > + (spapr->eff.caps[SPAPR_CAP_##ccap] != \
> > + spapr->def.caps[SPAPR_CAP_##ccap]); \
> > +} \
> > + \
> > +const VMStateDescription vmstate_spapr_cap_##cap = { \
> > + .name = "spapr/cap/" #cap, \
> > + .version_id = 1, \
> > + .minimum_version_id = 1, \
> > + .needed = spapr_cap_##cap##_needed, \
> > + .fields = (VMStateField[]) { \
> > + VMSTATE_UINT8(mig.caps[SPAPR_CAP_##ccap], \
> > + sPAPRMachineState), \
> > + VMSTATE_END_OF_LIST() \
> > + }, \
> > }
> >
> > -const VMStateDescription vmstate_spapr_cap_vsx = {
> > - .name = "spapr/cap/vsx",
> > - .version_id = 1,
> > - .minimum_version_id = 1,
> > - .needed = spapr_cap_vsx_needed,
> > - .fields = (VMStateField[]) {
> > - VMSTATE_UINT8(mig.caps[SPAPR_CAP_VSX], sPAPRMachineState),
> > - VMSTATE_END_OF_LIST()
> > - },
> > -};
> > -
> > -static bool spapr_cap_dfp_needed(void *opaque)
> > -{
> > - sPAPRMachineState *spapr = opaque;
> > -
> > - return spapr->cmd_line_caps[SPAPR_CAP_DFP] &&
> > - (spapr->eff.caps[SPAPR_CAP_DFP] != spapr-
> > >def.caps[SPAPR_CAP_DFP]);
> > -}
> > -
> > -const VMStateDescription vmstate_spapr_cap_dfp = {
> > - .name = "spapr/cap/dfp",
> > - .version_id = 1,
> > - .minimum_version_id = 1,
> > - .needed = spapr_cap_dfp_needed,
> > - .fields = (VMStateField[]) {
> > - VMSTATE_UINT8(mig.caps[SPAPR_CAP_DFP], sPAPRMachineState),
> > - VMSTATE_END_OF_LIST()
> > - },
> > -};
> > +SPAPR_CAP_MIG_STATE(htm, HTM);
> > +SPAPR_CAP_MIG_STATE(vsx, VSX);
> > +SPAPR_CAP_MIG_STATE(dfp, DFP);
>
>
> A nit: I really dislike this kind of parameters which you cannot
> really
> cscope/ctags for because they are joined to some other things.
>
> "no matches found for cscope query g HTM of HTM" says vim's cscope
> when I
> search for "HTM".
Would something like:
SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_HTM);
SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_VSX);
SPAPR_CAP_MIG_STATE(htm, SPAPR_CAP_DFP);
be better?
I could send a follow up
>
>
> >
> > void spapr_caps_reset(sPAPRMachineState *spapr)
> > {
> >
>
>
[Qemu-devel] [QEMU-PPC] [PATCH V5 3/7] target/ppc/spapr_caps: Add support for tristate spapr_capabilities, Suraj Jitindar Singh, 2018/01/19
[Qemu-devel] [QEMU-PPC] [PATCH V5 4/7] target/ppc/spapr_caps: Add new tristate cap safe_cache, Suraj Jitindar Singh, 2018/01/19
[Qemu-devel] [QEMU-PPC] [PATCH V5 5/7] target/ppc/spapr_caps: Add new tristate cap safe_bounds_check, Suraj Jitindar Singh, 2018/01/19
[Qemu-devel] [QEMU-PPC] [PATCH V5 6/7] target/ppc/spapr_caps: Add new tristate cap safe_indirect_branch, Suraj Jitindar Singh, 2018/01/19
[Qemu-devel] [QEMU-PPC] [PATCH V5 7/7] target/ppc/spapr: Add H-Call H_GET_CPU_CHARACTERISTICS, Suraj Jitindar Singh, 2018/01/19
Re: [Qemu-devel] [QEMU-PPC] [PATCH V5 0/7] target/ppc: Rework spapr_caps, no-reply, 2018/01/19
Re: [Qemu-devel] [QEMU-PPC] [PATCH V5 0/7] target/ppc: Rework spapr_caps, David Gibson, 2018/01/28