From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suraj Jitindar Singh Date: Wed, 17 Aug 2016 04:57:02 +0000 Subject: Re: [kvm-unit-tests PATCH V3 4/5] lib/powerpc: Implement generic sleep function for use in unit test Message-Id: <1471409822.4989.20.camel@gmail.com> List-Id: References: <1471331895-29887-1-git-send-email-sjitindarsingh@gmail.com> <1471331895-29887-4-git-send-email-sjitindarsingh@gmail.com> <20160816124103.bse5scxxngitfoxh@kamzik.localdomain> In-Reply-To: <20160816124103.bse5scxxngitfoxh@kamzik.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: Andrew Jones Cc: kvm@vger.kernel.org, pbonzini@redhat.com, rkrcmar@redhat.com, kvm-ppc@vger.kernel.org, lvivier@redhat.com, thuth@redhat.com On Tue, 2016-08-16 at 14:41 +0200, Andrew Jones wrote: > On Tue, Aug 16, 2016 at 05:18:14PM +1000, Suraj Jitindar Singh wrote: > > > > It would be nice if we had a generic sleep function which could be > > used > > in unit tests, add one. > > > > Add the variable tb_hz used to store the time base frequency which > > is read > > from the device tree on setup. > > > > Add functions msleep, usleep and sleep in processor.c to sleep for > > a given > > number of milliseconds, microseconds and time base ticks > > respectively. > Above you talk about adding *sleep functions, but below you add > *delay > functions. Woops > > > > > > > Signed-off-by: Suraj Jitindar Singh > > --- > > > > Change Log: > > > > V2 -> V3: > > - Add patch to series > > > > Signed-off-by: Suraj Jitindar Singh > > --- > >  lib/powerpc/asm/processor.h | 19 +++++++++++++++++++ > >  lib/powerpc/asm/setup.h     |  2 ++ > >  lib/powerpc/processor.c     | 13 +++++++++++++ > >  lib/powerpc/setup.c         | 12 ++++++++++++ > >  4 files changed, 46 insertions(+) > > > > diff --git a/lib/powerpc/asm/processor.h > > b/lib/powerpc/asm/processor.h > > index 09692bd..9554e08 100644 > > --- a/lib/powerpc/asm/processor.h > > +++ b/lib/powerpc/asm/processor.h > > @@ -1,11 +1,30 @@ > >  #ifndef _ASMPOWERPC_PROCESSOR_H_ > >  #define _ASMPOWERPC_PROCESSOR_H_ > >   > > +#include > >  #include > >   > > +#define cpu_relax() asm volatile ("" : : : "memory") > Please add this to lib/asm-generic/barrier.h, then > lib/ppc64/asm/barrier.h > will pick it up from there and you'll just need to make sure you > include > asm/barrier.h in processor.c Will do > > > > > + > >  #ifndef __ASSEMBLY__ > >  void handle_exception(int trap, void (*func)(struct pt_regs *, > > void *), void *); > >  void do_handle_exception(struct pt_regs *regs); > >  #endif /* __ASSEMBLY__ */ > >   > > +static inline uint64_t get_tb(void) > > +{ > > + uint64_t tb; > > + asm volatile ("mfspr %[tb],268" : [tb] "=r" (tb)); > > + return tb; > > +} > > + > > +extern void delay(uint64_t cycles); > > +extern void udelay(uint64_t us); > > + > > +static inline void mdelay(uint64_t ms) > > +{ > > + while (ms--) > > + udelay(1000); > > +} > > + > >  #endif /* _ASMPOWERPC_PROCESSOR_H_ */ > > diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h > > index b1e1e5a..23b4156 100644 > > --- a/lib/powerpc/asm/setup.h > > +++ b/lib/powerpc/asm/setup.h > > @@ -11,6 +11,8 @@ > >  extern u32 cpus[NR_CPUS]; > >  extern int nr_cpus; > >   > > +extern uint64_t tb_hz; > > + > >  #define NR_MEM_REGIONS 8 > >  #define MR_F_PRIMARY (1U << 0) > >  struct mem_region { > > diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c > > index a78bc3c..c84bae5 100644 > > --- a/lib/powerpc/processor.c > > +++ b/lib/powerpc/processor.c > > @@ -5,6 +5,7 @@ > >  #include > >  #include > >  #include > > +#include > >   > >  static struct { > >   void (*func)(struct pt_regs *, void *data); > > @@ -36,3 +37,15 @@ void do_handle_exception(struct pt_regs *regs) > >   printf("unhandled cpu exception 0x%lx\n", regs->trap); > >   abort(); > >  } > > + > > +void delay(uint64_t cycles) > > +{ > > + uint64_t start = get_tb(); > > + while((get_tb() - start) < cycles) > > + cpu_relax(); > > +} > > + > > +void udelay(uint64_t us) > > +{ > > + delay((us * tb_hz) / 1000000); > > +} > > diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c > > index e3d2afa..e7fa525 100644 > > --- a/lib/powerpc/setup.c > > +++ b/lib/powerpc/setup.c > > @@ -24,6 +24,7 @@ extern void setup_args_progname(const char > > *args); > >   > >  u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) }; > >  int nr_cpus; > > +uint64_t tb_hz; > >   > >  struct mem_region mem_regions[NR_MEM_REGIONS]; > >  phys_addr_t __physical_start, __physical_end; > > @@ -51,6 +52,17 @@ static void cpu_set(int fdtnode, u32 regval, > > void *info) > >   } > >   cpus[cpu] = regval; > >   > > + /* set timebase frequency */ > > + if (!tb_hz) { > Instead of this condition, can't we tuck this DT parsing into the > if (!read_common_info) block we already have? Didn't read that properly to realise that block is only called once, I'll move it into there. > > > > > + const struct fdt_property *prop; > > + u32 *data; > > + prop = fdt_get_property(dt_fdt(), fdtnode, > > + "timebase-frequency", > > NULL); > > + assert(prop != NULL); > > + data = (u32 *)prop->data; > > + tb_hz = fdt32_to_cpu(*data); > > + } > > + > >   /* set exception stack address for this CPU (in SPGR0) */ > >   > >   asm volatile ("mtsprg0 %[addr]" :: From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suraj Jitindar Singh Subject: Re: [kvm-unit-tests PATCH V3 4/5] lib/powerpc: Implement generic sleep function for use in unit tests Date: Wed, 17 Aug 2016 14:57:02 +1000 Message-ID: <1471409822.4989.20.camel@gmail.com> References: <1471331895-29887-1-git-send-email-sjitindarsingh@gmail.com> <1471331895-29887-4-git-send-email-sjitindarsingh@gmail.com> <20160816124103.bse5scxxngitfoxh@kamzik.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Cc: kvm@vger.kernel.org, pbonzini@redhat.com, rkrcmar@redhat.com, kvm-ppc@vger.kernel.org, lvivier@redhat.com, thuth@redhat.com To: Andrew Jones Return-path: Received: from mail-pf0-f195.google.com ([209.85.192.195]:36098 "EHLO mail-pf0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750784AbcHQE5K (ORCPT ); Wed, 17 Aug 2016 00:57:10 -0400 In-Reply-To: <20160816124103.bse5scxxngitfoxh@kamzik.localdomain> Sender: kvm-owner@vger.kernel.org List-ID: On Tue, 2016-08-16 at 14:41 +0200, Andrew Jones wrote: > On Tue, Aug 16, 2016 at 05:18:14PM +1000, Suraj Jitindar Singh wrote: > > > > It would be nice if we had a generic sleep function which could be > > used > > in unit tests, add one. > > > > Add the variable tb_hz used to store the time base frequency which > > is read > > from the device tree on setup. > > > > Add functions msleep, usleep and sleep in processor.c to sleep for > > a given > > number of milliseconds, microseconds and time base ticks > > respectively. > Above you talk about adding *sleep functions, but below you add > *delay > functions. Woops > > > > > > > Signed-off-by: Suraj Jitindar Singh > > --- > > > > Change Log: > > > > V2 -> V3: > > - Add patch to series > > > > Signed-off-by: Suraj Jitindar Singh > > --- > >  lib/powerpc/asm/processor.h | 19 +++++++++++++++++++ > >  lib/powerpc/asm/setup.h     |  2 ++ > >  lib/powerpc/processor.c     | 13 +++++++++++++ > >  lib/powerpc/setup.c         | 12 ++++++++++++ > >  4 files changed, 46 insertions(+) > > > > diff --git a/lib/powerpc/asm/processor.h > > b/lib/powerpc/asm/processor.h > > index 09692bd..9554e08 100644 > > --- a/lib/powerpc/asm/processor.h > > +++ b/lib/powerpc/asm/processor.h > > @@ -1,11 +1,30 @@ > >  #ifndef _ASMPOWERPC_PROCESSOR_H_ > >  #define _ASMPOWERPC_PROCESSOR_H_ > >   > > +#include > >  #include > >   > > +#define cpu_relax() asm volatile ("" : : : "memory") > Please add this to lib/asm-generic/barrier.h, then > lib/ppc64/asm/barrier.h > will pick it up from there and you'll just need to make sure you > include > asm/barrier.h in processor.c Will do > > > > > + > >  #ifndef __ASSEMBLY__ > >  void handle_exception(int trap, void (*func)(struct pt_regs *, > > void *), void *); > >  void do_handle_exception(struct pt_regs *regs); > >  #endif /* __ASSEMBLY__ */ > >   > > +static inline uint64_t get_tb(void) > > +{ > > + uint64_t tb; > > + asm volatile ("mfspr %[tb],268" : [tb] "=r" (tb)); > > + return tb; > > +} > > + > > +extern void delay(uint64_t cycles); > > +extern void udelay(uint64_t us); > > + > > +static inline void mdelay(uint64_t ms) > > +{ > > + while (ms--) > > + udelay(1000); > > +} > > + > >  #endif /* _ASMPOWERPC_PROCESSOR_H_ */ > > diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h > > index b1e1e5a..23b4156 100644 > > --- a/lib/powerpc/asm/setup.h > > +++ b/lib/powerpc/asm/setup.h > > @@ -11,6 +11,8 @@ > >  extern u32 cpus[NR_CPUS]; > >  extern int nr_cpus; > >   > > +extern uint64_t tb_hz; > > + > >  #define NR_MEM_REGIONS 8 > >  #define MR_F_PRIMARY (1U << 0) > >  struct mem_region { > > diff --git a/lib/powerpc/processor.c b/lib/powerpc/processor.c > > index a78bc3c..c84bae5 100644 > > --- a/lib/powerpc/processor.c > > +++ b/lib/powerpc/processor.c > > @@ -5,6 +5,7 @@ > >  #include > >  #include > >  #include > > +#include > >   > >  static struct { > >   void (*func)(struct pt_regs *, void *data); > > @@ -36,3 +37,15 @@ void do_handle_exception(struct pt_regs *regs) > >   printf("unhandled cpu exception 0x%lx\n", regs->trap); > >   abort(); > >  } > > + > > +void delay(uint64_t cycles) > > +{ > > + uint64_t start = get_tb(); > > + while((get_tb() - start) < cycles) > > + cpu_relax(); > > +} > > + > > +void udelay(uint64_t us) > > +{ > > + delay((us * tb_hz) / 1000000); > > +} > > diff --git a/lib/powerpc/setup.c b/lib/powerpc/setup.c > > index e3d2afa..e7fa525 100644 > > --- a/lib/powerpc/setup.c > > +++ b/lib/powerpc/setup.c > > @@ -24,6 +24,7 @@ extern void setup_args_progname(const char > > *args); > >   > >  u32 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (~0U) }; > >  int nr_cpus; > > +uint64_t tb_hz; > >   > >  struct mem_region mem_regions[NR_MEM_REGIONS]; > >  phys_addr_t __physical_start, __physical_end; > > @@ -51,6 +52,17 @@ static void cpu_set(int fdtnode, u32 regval, > > void *info) > >   } > >   cpus[cpu] = regval; > >   > > + /* set timebase frequency */ > > + if (!tb_hz) { > Instead of this condition, can't we tuck this DT parsing into the > if (!read_common_info) block we already have? Didn't read that properly to realise that block is only called once, I'll move it into there. > > > > > + const struct fdt_property *prop; > > + u32 *data; > > + prop = fdt_get_property(dt_fdt(), fdtnode, > > + "timebase-frequency", > > NULL); > > + assert(prop != NULL); > > + data = (u32 *)prop->data; > > + tb_hz = fdt32_to_cpu(*data); > > + } > > + > >   /* set exception stack address for this CPU (in SPGR0) */ > >   > >   asm volatile ("mtsprg0 %[addr]" ::