Re: [Qemu-devel] [PATCH 1/3] ppc: Generalize the kvmppc_get_clockfreq() function

[Alexander Graf]

Am 30.09.2011 um 09:50 schrieb David Gibson <address@hidden>:

> 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 <address@hidden>
> ---
> 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/address@hidden", "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
>