Re: [Qemu-devel] [QEMU-PPC] [PATCH 1/4] target/ppc/spapr: Add SPAPR_CAP_LARGE_DECREMENTER

2019-02-26 Thread David Gibson
On Tue, Feb 26, 2019 at 05:26:10PM +1100, Suraj Jitindar Singh wrote:
> 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
> > > = {
> > >  _spapr_irq_map,
> > >  _spapr_cap_nested_kvm_hv,
> > >  _spapr_dtb,
> > > +_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...

Ok, and that difference is..?

> SPAPR_CAP_LARGE_DECR_NR_BITS?

That could work.

-- 
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


Re: [Qemu-devel] [QEMU-PPC] [PATCH 1/4] target/ppc/spapr: Add SPAPR_CAP_LARGE_DECREMENTER

2019-02-25 Thread Suraj Jitindar Singh
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
> > = {
> >  _spapr_irq_map,
> >  _spapr_cap_nested_kvm_hv,
> >  _spapr_dtb,
> > +_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, _abort);
> >  smc->irq = _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, , 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, , _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 */
> >  

Re: [Qemu-devel] [QEMU-PPC] [PATCH 1/4] target/ppc/spapr: Add SPAPR_CAP_LARGE_DECREMENTER

2019-02-25 Thread David Gibson
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 = {
>  _spapr_irq_map,
>  _spapr_cap_nested_kvm_hv,
>  _spapr_dtb,
> +_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.

>  spapr_caps_add_properties(smc, _abort);
>  smc->irq = _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, , 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, , _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 

[Qemu-devel] [QEMU-PPC] [PATCH 1/4] target/ppc/spapr: Add SPAPR_CAP_LARGE_DECREMENTER

2019-02-25 Thread Suraj Jitindar Singh
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 = {
 _spapr_irq_map,
 _spapr_cap_nested_kvm_hv,
 _spapr_dtb,
+_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;
 spapr_caps_add_properties(smc, _abort);
 smc->irq = _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, , 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, , _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)
 {
-- 
2.13.6