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

[Philippe Mathieu-Daudé]

On 10/5/20 6:40 PM, Igor Mammedov wrote:
> 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.

OK will update the patch.

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