On 2/3/21 3:51 PM, Philippe Mathieu-Daudé wrote:
> On 2/3/21 3:49 PM, Claudio Fontana wrote:
>> On 2/3/21 3:27 PM, Philippe Mathieu-Daudé wrote:
>>> On 2/1/21 11:08 AM, Claudio Fontana wrote:
>>>> add a new optional interface to CPUClass, which allows accelerators
>>>> to extend the CPUClass with additional accelerator-specific
>>>> initializations.
>>>>
>>>> Signed-off-by: Claudio Fontana <cfont...@suse.de>
>>>> ---
>>>> include/hw/core/accel-cpu.h | 35 +++++++++++++++++++++++++++++
>>>> include/hw/core/cpu.h | 1 +
>>>> accel/accel-common.c | 44 +++++++++++++++++++++++++++++++++++++
>>>> MAINTAINERS | 1 +
>>>> 4 files changed, 81 insertions(+)
>>>> create mode 100644 include/hw/core/accel-cpu.h
>>>>
>>>> diff --git a/include/hw/core/accel-cpu.h b/include/hw/core/accel-cpu.h
>>>> new file mode 100644
>>>> index 0000000000..c7c137dc9a
>>>> --- /dev/null
>>>> +++ b/include/hw/core/accel-cpu.h
>>>> @@ -0,0 +1,35 @@
>>>> +/*
>>>> + * Accelerator interface, specializes CPUClass
>>>> + *
>>>> + * Copyright 2021 SUSE LLC
>>>> + *
>>>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>>>> later.
>>>> + * See the COPYING file in the top-level directory.
>>>> + */
>>>> +
>>>> +#ifndef ACCEL_CPU_H
>>>> +#define ACCEL_CPU_H
>>>> +
>>>> +/*
>>>> + * these defines cannot be in cpu.h, because we are using
>>>> + * CPU_RESOLVING_TYPE here.
>>>> + * Use this header to define your accelerator-specific
>>>> + * cpu-specific accelerator interfaces.
>>>> + */
>>>> +
This is the comment to read :-)
>>>> +#define TYPE_ACCEL_CPU "accel-" CPU_RESOLVING_TYPE
>>>> +#define ACCEL_CPU_NAME(name) (name "-" TYPE_ACCEL_CPU)
>>>> +typedef struct AccelCPUClass AccelCPUClass;
>>>> +DECLARE_CLASS_CHECKERS(AccelCPUClass, ACCEL_CPU, TYPE_ACCEL_CPU)
>>>> +
>>>> +typedef struct AccelCPUClass {
>>>> + /*< private >*/
>>>> + ObjectClass parent_class;
>>>> + /*< public >*/
>>>> +
>>>> + void (*cpu_class_init)(CPUClass *cc);
>>>> + void (*cpu_instance_init)(CPUState *cpu);
>>>> + void (*cpu_realizefn)(CPUState *cpu, Error **errp);
>>>> +} AccelCPUClass;
>>>
>>> This header only makes sense if you forward-declare CPUClass
>>> in "qemu/typedefs.h", so accelerators don't have to include
>>> "hw/core/cpu.h".
>>>
>>
>> Can you clarify what you mean? I don't see how it follows that this header
>> only makes sense if I forward-declare CPUClass.
>>
>> This is necessary for the accel-specific target-specific code that needs to
>> extend cpu classes with Accel CPU interfaces,
>> in this series f.e.:
>>
>> target/i386/kvm/kvm-cpu.c
>> target/i386/hvf/hvf-cpu.c
>> target/i386/tcg/tcg-cpu.c
>
> Why not keep theses declarations in "hw/core/cpu.h", rather than
> adding a new header? What is the point of the new header?
>
It is not possible (see comment above).
The header needs to be target-specific, and only can be included by target
code, or disaster is ensured.
The part that is in hw/core/cpu.h is the part that can be safely included by
both common and target-specific code.
The accel-cpu.h is target-specific.
This is one of the fallouts of our split of code between common,
target-specific modules.
It gains us quite a bit in compilation time,
but with the drawback that thinking about these things is quite convoluted
sometimes.
Ciao,
CLaudio
