From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:46338) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gyWCQ-0007xa-6o for qemu-devel@nongnu.org; Tue, 26 Feb 2019 01:26:43 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gyWCO-0001Rm-1z for qemu-devel@nongnu.org; Tue, 26 Feb 2019 01:26:42 -0500 Message-ID: <1551162370.2210.0.camel@gmail.com> From: Suraj Jitindar Singh Date: Tue, 26 Feb 2019 17:26:10 +1100 In-Reply-To: <20190226033914.GK6872@umbus.fritz.box> References: <20190226030531.9932-1-sjitindarsingh@gmail.com> <20190226033914.GK6872@umbus.fritz.box> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [QEMU-PPC] [PATCH 1/4] target/ppc/spapr: Add SPAPR_CAP_LARGE_DECREMENTER List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: qemu-ppc@nongnu.org, clg@kaod.org, qemu-devel@nongnu.org On Tue, 2019-02-26 at 14:39 +1100, David Gibson wrote: > On Tue, Feb 26, 2019 at 02:05:28PM +1100, Suraj Jitindar Singh wrote: > > Add spapr_cap SPAPR_CAP_LARGE_DECREMENTER to be used to control the > > availability and size of the large decrementer made available to > > the > > guest. > > > > Signed-off-by: Suraj Jitindar Singh > > --- > > hw/ppc/spapr.c | 2 ++ > > hw/ppc/spapr_caps.c | 45 > > +++++++++++++++++++++++++++++++++++++++++++++ > > include/hw/ppc/spapr.h | 5 ++++- > > 3 files changed, 51 insertions(+), 1 deletion(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index b6a571b6f1..acf62a2b9f 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -2077,6 +2077,7 @@ static const VMStateDescription vmstate_spapr > > = { > > &vmstate_spapr_irq_map, > > &vmstate_spapr_cap_nested_kvm_hv, > > &vmstate_spapr_dtb, > > + &vmstate_spapr_cap_large_decr, > > NULL > > } > > }; > > @@ -4288,6 +4289,7 @@ static void > > spapr_machine_class_init(ObjectClass *oc, void *data) > > smc->default_caps.caps[SPAPR_CAP_IBS] = SPAPR_CAP_BROKEN; > > smc->default_caps.caps[SPAPR_CAP_HPT_MAXPAGESIZE] = 16; /* > > 64kiB */ > > smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = > > SPAPR_CAP_OFF; > > + smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = 0; > > This looks basically fine, but the name kind of suggests it's a > boolean, whereas it's actually a number of bits. I wonder if just > calling it "decrementer bits" would be clearer, with it defaulting to > 32. Yes, except there's a difference between a decrementer with 32 bits and a large decrementer with 32 bits... SPAPR_CAP_LARGE_DECR_NR_BITS? > > > spapr_caps_add_properties(smc, &error_abort); > > smc->irq = &spapr_irq_xics; > > smc->dr_phb_enabled = true; > > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c > > index 64f98ae68d..1545a02729 100644 > > --- a/hw/ppc/spapr_caps.c > > +++ b/hw/ppc/spapr_caps.c > > @@ -182,6 +182,34 @@ static void spapr_cap_set_pagesize(Object > > *obj, Visitor *v, const char *name, > > spapr->eff.caps[cap->index] = val; > > } > > > > +static void spapr_cap_get_uint8(Object *obj, Visitor *v, const > > char *name, > > + void *opaque, Error **errp) > > +{ > > + sPAPRCapabilityInfo *cap = opaque; > > + sPAPRMachineState *spapr = SPAPR_MACHINE(obj); > > + uint8_t val = spapr_get_cap(spapr, cap->index); > > + > > + visit_type_uint8(v, name, &val, errp); > > +} > > + > > +static void spapr_cap_set_uint8(Object *obj, Visitor *v, const > > char *name, > > + void *opaque, Error **errp) > > +{ > > + sPAPRCapabilityInfo *cap = opaque; > > + sPAPRMachineState *spapr = SPAPR_MACHINE(obj); > > + Error *local_err = NULL; > > + uint8_t val; > > + > > + visit_type_uint8(v, name, &val, &local_err); > > + if (local_err) { > > + error_propagate(errp, local_err); > > + return; > > + } > > + > > + spapr->cmd_line_caps[cap->index] = true; > > + spapr->eff.caps[cap->index] = val; > > +} > > + > > static void cap_htm_apply(sPAPRMachineState *spapr, uint8_t val, > > Error **errp) > > { > > if (!val) { > > @@ -390,6 +418,13 @@ static void > > cap_nested_kvm_hv_apply(sPAPRMachineState *spapr, > > } > > } > > > > +static void cap_large_decr_apply(sPAPRMachineState *spapr, > > + uint8_t val, Error **errp) > > +{ > > + if (val) > > + error_setg(errp, "No large decrementer support, try cap- > > large-decr=0"); > > +} > > + > > sPAPRCapabilityInfo capability_table[SPAPR_CAP_NUM] = { > > [SPAPR_CAP_HTM] = { > > .name = "htm", > > @@ -468,6 +503,15 @@ sPAPRCapabilityInfo > > capability_table[SPAPR_CAP_NUM] = { > > .type = "bool", > > .apply = cap_nested_kvm_hv_apply, > > }, > > + [SPAPR_CAP_LARGE_DECREMENTER] = { > > + .name = "large-decr", > > + .description = "Size of Large Decrementer for the Guest > > (bits) 0=disabled", > > + .index = SPAPR_CAP_LARGE_DECREMENTER, > > + .get = spapr_cap_get_uint8, > > + .set = spapr_cap_set_uint8, > > + .type = "int", > > + .apply = cap_large_decr_apply, > > + }, > > }; > > > > static sPAPRCapabilities default_caps_with_cpu(sPAPRMachineState > > *spapr, > > @@ -596,6 +640,7 @@ SPAPR_CAP_MIG_STATE(cfpc, SPAPR_CAP_CFPC); > > SPAPR_CAP_MIG_STATE(sbbc, SPAPR_CAP_SBBC); > > SPAPR_CAP_MIG_STATE(ibs, SPAPR_CAP_IBS); > > SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV); > > +SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER); > > > > void spapr_caps_init(sPAPRMachineState *spapr) > > { > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > > index 59073a7579..8efc5e0779 100644 > > --- a/include/hw/ppc/spapr.h > > +++ b/include/hw/ppc/spapr.h > > @@ -74,8 +74,10 @@ typedef enum { > > #define SPAPR_CAP_HPT_MAXPAGESIZE 0x06 > > /* Nested KVM-HV */ > > #define SPAPR_CAP_NESTED_KVM_HV 0x07 > > +/* Large Decrementer */ > > +#define SPAPR_CAP_LARGE_DECREMENTER 0x08 > > /* Num Caps */ > > -#define SPAPR_CAP_NUM (SPAPR_CAP_NESTED_KVM_HV + > > 1) > > +#define > > SPAPR_CAP_NUM (SPAPR_CAP_LARGE_DECREMENTER + 1) > > > > /* > > * Capability Values > > @@ -828,6 +830,7 @@ extern const VMStateDescription > > vmstate_spapr_cap_cfpc; > > extern const VMStateDescription vmstate_spapr_cap_sbbc; > > extern const VMStateDescription vmstate_spapr_cap_ibs; > > extern const VMStateDescription vmstate_spapr_cap_nested_kvm_hv; > > +extern const VMStateDescription vmstate_spapr_cap_large_decr; > > > > static inline uint8_t spapr_get_cap(sPAPRMachineState *spapr, int > > cap) > > { > >