Re: [PATCH 01/16] hw/core/cpu: Let CPU object have a clock source

[Igor Mammedov]

On Wed, 30 Sep 2020 12:16:53 +0200
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

> +arm/ppc/riscv folks
> 
> On 9/30/20 9:43 AM, Igor Mammedov wrote:
> > On Mon, 28 Sep 2020 19:15:24 +0200
> > Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >   
> >> Let CPUState have a clock source (named 'clk') and CPUClass
> >> have a clock_update() callback. The clock can be optionally
> >> set Using qdev_connect_clock_in() from the Clock API.
> >> If the clock changes, the optional clock_update() will be
> >> called.  
> > 
> > the sole user of it is mips cpu, so question is why
> > you are making it part of generic CPUm instead of
> > MIPSCPUClass/MIPSCPU?  
> 
> This is a feature of the CPU, regardless its architecture.
> 
> I expect the other archs to start using it soon.

if there aren't any plans to actually to do that,
I'd keep it to MIPS class and generalize later when there is demand.
 
> 
> >   
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >> ---
> >>  include/hw/core/cpu.h |  5 +++++
> >>  hw/core/cpu.c         | 12 ++++++++++++
> >>  2 files changed, 17 insertions(+)
> >>
> >> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> >> index 6c34798c8b3..6989d90c193 100644
> >> --- a/include/hw/core/cpu.h
> >> +++ b/include/hw/core/cpu.h
> >> @@ -31,6 +31,7 @@
> >>  #include "qemu/thread.h"
> >>  #include "qemu/plugin.h"
> >>  #include "qom/object.h"
> >> +#include "hw/clock.h"
> >>  
> >>  typedef int (*WriteCoreDumpFunction)(const void *buf, size_t size,
> >>                                       void *opaque);
> >> @@ -155,6 +156,7 @@ struct TranslationBlock;
> >>   * @disas_set_info: Setup architecture specific components of disassembly 
> >> info
> >>   * @adjust_watchpoint_address: Perform a target-specific adjustment to an
> >>   * address before attempting to match it against watchpoints.
> >> + * @clock_update: Callback for input clock changes
> >>   *
> >>   * Represents a CPU family or model.
> >>   */
> >> @@ -176,6 +178,7 @@ struct CPUClass {
> >>                                    unsigned size, MMUAccessType 
> >> access_type,
> >>                                    int mmu_idx, MemTxAttrs attrs,
> >>                                    MemTxResult response, uintptr_t 
> >> retaddr);
> >> +    void (*clock_update)(CPUState *cpu);
> >>      bool (*virtio_is_big_endian)(CPUState *cpu);
> >>      int (*memory_rw_debug)(CPUState *cpu, vaddr addr,
> >>                             uint8_t *buf, int len, bool is_write);
> >> @@ -316,6 +319,7 @@ struct qemu_work_item;
> >>   *   QOM parent.
> >>   * @nr_cores: Number of cores within this CPU package.
> >>   * @nr_threads: Number of threads within this CPU.
> >> + * @clock: this CPU source clock (an output clock of another device)
> >>   * @running: #true if CPU is currently running (lockless).
> >>   * @has_waiter: #true if a CPU is currently waiting for the cpu_exec_end;
> >>   * valid under cpu_list_lock.
> >> @@ -400,6 +404,7 @@ struct CPUState {
> >>      int num_ases;
> >>      AddressSpace *as;
> >>      MemoryRegion *memory;
> >> +    Clock *clock;
> >>  
> >>      void *env_ptr; /* CPUArchState */
> >>      IcountDecr *icount_decr_ptr;
> >> diff --git a/hw/core/cpu.c b/hw/core/cpu.c
> >> index c55c09f734c..37fcff3ec64 100644
> >> --- a/hw/core/cpu.c
> >> +++ b/hw/core/cpu.c
> >> @@ -30,6 +30,7 @@
> >>  #include "qemu/qemu-print.h"
> >>  #include "sysemu/tcg.h"
> >>  #include "hw/boards.h"
> >> +#include "hw/qdev-clock.h"
> >>  #include "hw/qdev-properties.h"
> >>  #include "trace/trace-root.h"
> >>  #include "qemu/plugin.h"
> >> @@ -247,6 +248,16 @@ void cpu_reset(CPUState *cpu)
> >>      trace_guest_cpu_reset(cpu);
> >>  }
> >>  
> >> +static void cpu_clk_update(void *opaque)
> >> +{
> >> +    CPUState *cpu = opaque;
> >> +    CPUClass *cc = CPU_GET_CLASS(cpu);
> >> +
> >> +    if (cc->clock_update) {
> >> +        cc->clock_update(cpu);
> >> +    }
> >> +}
> >> +
> >>  static void cpu_common_reset(DeviceState *dev)
> >>  {
> >>      CPUState *cpu = CPU(dev);
> >> @@ -367,6 +378,7 @@ static void cpu_common_initfn(Object *obj)
> >>      /* the default value is changed by qemu_init_vcpu() for softmmu */
> >>      cpu->nr_cores = 1;
> >>      cpu->nr_threads = 1;
> >> +    cpu->clock = qdev_init_clock_in(DEVICE(obj), "clk", cpu_clk_update, 
> >> cpu);
> >>  
> >>      qemu_mutex_init(&cpu->work_mutex);
> >>      QSIMPLEQ_INIT(&cpu->work_list);  
> >   
>