Patchwork [1/3] ppc: Generalize the kvmppc_get_clockfreq() function

login
register
mail settings
Submitter David Gibson
Date Sept. 30, 2011, 7:50 a.m.
Message ID <1317369040-30437-2-git-send-email-david@gibson.dropbear.id.au>
Download mbox | patch
Permalink /patch/117053/
State New
Headers show

Comments

David Gibson - Sept. 30, 2011, 7:50 a.m.
Currently the kvmppc_get_clockfreq() function reads the host's clock
frequency from /proc/device-tree, which is useful to past to the guest
in KVM setups.  However, there are some other host properties
advertised in the device tree which can also be relevant to the
guests.

This patch, therefore, replaces kvmppc_get_clockfreq() which can
retrieve any named, single integer property from the host device
tree's CPU node.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc440_bamboo.c     |    2 +-
 hw/ppce500_mpc8544ds.c |    2 +-
 hw/spapr.c             |   13 ++++++++++++-
 target-ppc/kvm.c       |   30 +++++++++++++++++++-----------
 target-ppc/kvm_ppc.h   |    4 ++--
 5 files changed, 35 insertions(+), 16 deletions(-)
Alexander Graf - Sept. 30, 2011, 6:06 p.m.
Am 30.09.2011 um 09:50 schrieb David Gibson <david@gibson.dropbear.id.au>:

> Currently the kvmppc_get_clockfreq() function reads the host's clock
> frequency from /proc/device-tree, which is useful to past to the guest
> in KVM setups.  However, there are some other host properties
> advertised in the device tree which can also be relevant to the
> guests.
> 
> This patch, therefore, replaces kvmppc_get_clockfreq() which can
> retrieve any named, single integer property from the host device
> tree's CPU node.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/ppc440_bamboo.c     |    2 +-
> hw/ppce500_mpc8544ds.c |    2 +-
> hw/spapr.c             |   13 ++++++++++++-
> target-ppc/kvm.c       |   30 +++++++++++++++++++-----------
> target-ppc/kvm_ppc.h   |    4 ++--
> 5 files changed, 35 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
> index 1523764..df85da0 100644
> --- a/hw/ppc440_bamboo.c
> +++ b/hw/ppc440_bamboo.c
> @@ -83,7 +83,7 @@ static int bamboo_load_device_tree(target_phys_addr_t addr,
>      * the correct frequencies. */
>     if (kvm_enabled()) {
>         tb_freq = kvmppc_get_tbfreq();
> -        clock_freq = kvmppc_get_clockfreq();
> +        clock_freq = kvmppc_read_int_cpu_dt("clock-frequency");

Hrm. I was actually trying to abstract host dt handling away here. The idea was to use the helper inside of kvm.c, but still expose specific functions for specific properties.

>     }
> 
>     qemu_devtree_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
> diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
> index f00367e..eb37c3d 100644
> --- a/hw/ppce500_mpc8544ds.c
> +++ b/hw/ppce500_mpc8544ds.c
> @@ -112,7 +112,7 @@ static int mpc8544_load_device_tree(CPUState *env,
> 
>     if (kvm_enabled()) {
>         /* Read out host's frequencies */
> -        clock_freq = kvmppc_get_clockfreq();
> +        clock_freq = kvmppc_read_int_cpu_dt("clock-frequency");
>         tb_freq = kvmppc_get_tbfreq();
> 
>         /* indicate KVM hypercall interface */
> diff --git a/hw/spapr.c b/hw/spapr.c
> index 9a3a1ea..ea5690e 100644
> --- a/hw/spapr.c
> +++ b/hw/spapr.c
> @@ -185,7 +185,8 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
>         uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
>                            0xffffffff, 0xffffffff};
>         uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TIMEBASE_FREQ;
> -        uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 1000000000;
> +        uint32_t cpufreq = kvm_enabled() ?
> +            kvmppc_read_int_cpu_dt("clock-frequency") : 1000000000;
> 
>         if ((index % smt) != 0) {
>             continue;
> @@ -233,6 +234,16 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
>                                segs, sizeof(segs))));
>         }
> 
> +        /* Advertise VMX/VSX (vector extensions) if available */
> +        if (vmx) {
> +            _FDT((fdt_property_cell(fdt, "ibm,vmx", vmx)));
> +        }
> +
> +        /* Advertise DFP (Decimal Floating Point) if available */
> +        if (dfp) {
> +            _FDT((fdt_property_cell(fdt, "ibm,dfp", dfp)));
> +        }
> +

Please make sure that your patch set is bisectable :)

Alex

>         _FDT((fdt_end_node(fdt)));
>     }
> 
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 26165b6..db2326d 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -650,32 +650,40 @@ static int kvmppc_find_cpu_dt(char *buf, int buf_len)
>     return 0;
> }
> 
> -uint64_t kvmppc_get_clockfreq(void)
> +/* Read a CPU node property from the host device tree that's a single
> + * integer (32-bit or 64-bit).  Returns 0 if anything goes wrong
> + * (can't find or open the property, or doesn't understand the
> + * format) */
> +uint64_t kvmppc_read_int_cpu_dt(const char *propname)
> {
> -    char buf[512];
> -    uint32_t tb[2];
> +    char buf[PATH_MAX];
> +    union {
> +        uint32_t v32;
> +        uint64_t v64;
> +    } u;
>     FILE *f;
>     int len;
> 
>     if (kvmppc_find_cpu_dt(buf, sizeof(buf))) {
> -        return 0;
> +        return -1;
>     }
> 
> -    strncat(buf, "/clock-frequency", sizeof(buf) - strlen(buf));
> +    strncat(buf, "/", sizeof(buf) - strlen(buf));
> +    strncat(buf, propname, sizeof(buf) - strlen(buf));
> 
>     f = fopen(buf, "rb");
>     if (!f) {
>         return -1;
>     }
> 
> -    len = fread(tb, sizeof(tb[0]), 2, f);
> +    len = fread(&u, 1, sizeof(u), f);
>     fclose(f);
>     switch (len) {
> -    case 1:
> -        /* freq is only a single cell */
> -        return tb[0];
> -    case 2:
> -        return *(uint64_t*)tb;
> +    case 4:
> +        /* property is a 32-bit quantity */
> +        return be32_to_cpu(u.v32);
> +    case 8:
> +        return be64_to_cpu(u.v64);
>     }
> 
>     return 0;
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 9e8a7b5..0b9a58a 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -14,7 +14,7 @@ void kvmppc_init(void);
> #ifdef CONFIG_KVM
> 
> uint32_t kvmppc_get_tbfreq(void);
> -uint64_t kvmppc_get_clockfreq(void);
> +uint64_t kvmppc_read_int_cpu_dt(const char *propname);
> int kvmppc_get_hypercall(CPUState *env, uint8_t *buf, int buf_len);
> int kvmppc_set_interrupt(CPUState *env, int irq, int level);
> void kvmppc_set_papr(CPUState *env);
> @@ -30,7 +30,7 @@ static inline uint32_t kvmppc_get_tbfreq(void)
>     return 0;
> }
> 
> -static inline uint64_t kvmppc_get_clockfreq(void)
> +static inline uint64_t kvmppc_read_int_cpu_dt(const char *propname)
> {
>     return 0;
> }
> -- 
> 1.7.6.3
>
David Gibson - Oct. 11, 2011, 4:29 a.m.
On Fri, Sep 30, 2011 at 08:06:59PM +0200, Alexander Graf wrote:
> 
> Am 30.09.2011 um 09:50 schrieb David Gibson <david@gibson.dropbear.id.au>:
> 
> > Currently the kvmppc_get_clockfreq() function reads the host's clock
> > frequency from /proc/device-tree, which is useful to past to the guest
> > in KVM setups.  However, there are some other host properties
> > advertised in the device tree which can also be relevant to the
> > guests.
> > 
> > This patch, therefore, replaces kvmppc_get_clockfreq() which can
> > retrieve any named, single integer property from the host device
> > tree's CPU node.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> > hw/ppc440_bamboo.c     |    2 +-
> > hw/ppce500_mpc8544ds.c |    2 +-
> > hw/spapr.c             |   13 ++++++++++++-
> > target-ppc/kvm.c       |   30 +++++++++++++++++++-----------
> > target-ppc/kvm_ppc.h   |    4 ++--
> > 5 files changed, 35 insertions(+), 16 deletions(-)
> > 
> > diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
> > index 1523764..df85da0 100644
> > --- a/hw/ppc440_bamboo.c
> > +++ b/hw/ppc440_bamboo.c
> > @@ -83,7 +83,7 @@ static int bamboo_load_device_tree(target_phys_addr_t addr,
> >      * the correct frequencies. */
> >     if (kvm_enabled()) {
> >         tb_freq = kvmppc_get_tbfreq();
> > -        clock_freq = kvmppc_get_clockfreq();
> > +        clock_freq = kvmppc_read_int_cpu_dt("clock-frequency");
> 
> Hrm. I was actually trying to abstract host dt handling away
> here. The idea was to use the helper inside of kvm.c, but still
> expose specific functions for specific properties.

Ok, fair enough.

> >     }
> > 
> >     qemu_devtree_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
> > diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
> > index f00367e..eb37c3d 100644
> > --- a/hw/ppce500_mpc8544ds.c
> > +++ b/hw/ppce500_mpc8544ds.c
> > @@ -112,7 +112,7 @@ static int mpc8544_load_device_tree(CPUState *env,
> > 
> >     if (kvm_enabled()) {
> >         /* Read out host's frequencies */
> > -        clock_freq = kvmppc_get_clockfreq();
> > +        clock_freq = kvmppc_read_int_cpu_dt("clock-frequency");
> >         tb_freq = kvmppc_get_tbfreq();
> > 
> >         /* indicate KVM hypercall interface */
> > diff --git a/hw/spapr.c b/hw/spapr.c
> > index 9a3a1ea..ea5690e 100644
> > --- a/hw/spapr.c
> > +++ b/hw/spapr.c
> > @@ -185,7 +185,8 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
> >         uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
> >                            0xffffffff, 0xffffffff};
> >         uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TIMEBASE_FREQ;
> > -        uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 1000000000;
> > +        uint32_t cpufreq = kvm_enabled() ?
> > +            kvmppc_read_int_cpu_dt("clock-frequency") : 1000000000;
> > 
> >         if ((index % smt) != 0) {
> >             continue;
> > @@ -233,6 +234,16 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
> >                                segs, sizeof(segs))));
> >         }
> > 
> > +        /* Advertise VMX/VSX (vector extensions) if available */
> > +        if (vmx) {
> > +            _FDT((fdt_property_cell(fdt, "ibm,vmx", vmx)));
> > +        }
> > +
> > +        /* Advertise DFP (Decimal Floating Point) if available */
> > +        if (dfp) {
> > +            _FDT((fdt_property_cell(fdt, "ibm,dfp", dfp)));
> > +        }
> > +
> 
> Please make sure that your patch set is bisectable :)

Oops, bad split - that hunk was supposed to be in the other patch of
the series.  I'll resend with both the above fixed momentarily.

Patch

diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
index 1523764..df85da0 100644
--- a/hw/ppc440_bamboo.c
+++ b/hw/ppc440_bamboo.c
@@ -83,7 +83,7 @@  static int bamboo_load_device_tree(target_phys_addr_t addr,
      * the correct frequencies. */
     if (kvm_enabled()) {
         tb_freq = kvmppc_get_tbfreq();
-        clock_freq = kvmppc_get_clockfreq();
+        clock_freq = kvmppc_read_int_cpu_dt("clock-frequency");
     }
 
     qemu_devtree_setprop_cell(fdt, "/cpus/cpu@0", "clock-frequency",
diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index f00367e..eb37c3d 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -112,7 +112,7 @@  static int mpc8544_load_device_tree(CPUState *env,
 
     if (kvm_enabled()) {
         /* Read out host's frequencies */
-        clock_freq = kvmppc_get_clockfreq();
+        clock_freq = kvmppc_read_int_cpu_dt("clock-frequency");
         tb_freq = kvmppc_get_tbfreq();
 
         /* indicate KVM hypercall interface */
diff --git a/hw/spapr.c b/hw/spapr.c
index 9a3a1ea..ea5690e 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -185,7 +185,8 @@  static void *spapr_create_fdt_skel(const char *cpu_model,
         uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
                            0xffffffff, 0xffffffff};
         uint32_t tbfreq = kvm_enabled() ? kvmppc_get_tbfreq() : TIMEBASE_FREQ;
-        uint32_t cpufreq = kvm_enabled() ? kvmppc_get_clockfreq() : 1000000000;
+        uint32_t cpufreq = kvm_enabled() ?
+            kvmppc_read_int_cpu_dt("clock-frequency") : 1000000000;
 
         if ((index % smt) != 0) {
             continue;
@@ -233,6 +234,16 @@  static void *spapr_create_fdt_skel(const char *cpu_model,
                                segs, sizeof(segs))));
         }
 
+        /* Advertise VMX/VSX (vector extensions) if available */
+        if (vmx) {
+            _FDT((fdt_property_cell(fdt, "ibm,vmx", vmx)));
+        }
+
+        /* Advertise DFP (Decimal Floating Point) if available */
+        if (dfp) {
+            _FDT((fdt_property_cell(fdt, "ibm,dfp", dfp)));
+        }
+
         _FDT((fdt_end_node(fdt)));
     }
 
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 26165b6..db2326d 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -650,32 +650,40 @@  static int kvmppc_find_cpu_dt(char *buf, int buf_len)
     return 0;
 }
 
-uint64_t kvmppc_get_clockfreq(void)
+/* Read a CPU node property from the host device tree that's a single
+ * integer (32-bit or 64-bit).  Returns 0 if anything goes wrong
+ * (can't find or open the property, or doesn't understand the
+ * format) */
+uint64_t kvmppc_read_int_cpu_dt(const char *propname)
 {
-    char buf[512];
-    uint32_t tb[2];
+    char buf[PATH_MAX];
+    union {
+        uint32_t v32;
+        uint64_t v64;
+    } u;
     FILE *f;
     int len;
 
     if (kvmppc_find_cpu_dt(buf, sizeof(buf))) {
-        return 0;
+        return -1;
     }
 
-    strncat(buf, "/clock-frequency", sizeof(buf) - strlen(buf));
+    strncat(buf, "/", sizeof(buf) - strlen(buf));
+    strncat(buf, propname, sizeof(buf) - strlen(buf));
 
     f = fopen(buf, "rb");
     if (!f) {
         return -1;
     }
 
-    len = fread(tb, sizeof(tb[0]), 2, f);
+    len = fread(&u, 1, sizeof(u), f);
     fclose(f);
     switch (len) {
-    case 1:
-        /* freq is only a single cell */
-        return tb[0];
-    case 2:
-        return *(uint64_t*)tb;
+    case 4:
+        /* property is a 32-bit quantity */
+        return be32_to_cpu(u.v32);
+    case 8:
+        return be64_to_cpu(u.v64);
     }
 
     return 0;
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 9e8a7b5..0b9a58a 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -14,7 +14,7 @@  void kvmppc_init(void);
 #ifdef CONFIG_KVM
 
 uint32_t kvmppc_get_tbfreq(void);
-uint64_t kvmppc_get_clockfreq(void);
+uint64_t kvmppc_read_int_cpu_dt(const char *propname);
 int kvmppc_get_hypercall(CPUState *env, uint8_t *buf, int buf_len);
 int kvmppc_set_interrupt(CPUState *env, int irq, int level);
 void kvmppc_set_papr(CPUState *env);
@@ -30,7 +30,7 @@  static inline uint32_t kvmppc_get_tbfreq(void)
     return 0;
 }
 
-static inline uint64_t kvmppc_get_clockfreq(void)
+static inline uint64_t kvmppc_read_int_cpu_dt(const char *propname)
 {
     return 0;
 }