Re: [PATCH 3/6] Add the aehd-apic device type.

[Haitao Shan]

On Fri, Mar 3, 2023 at 1:59 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Hi Haitao,
>
> On 3/3/23 03:26, Haitao Shan wrote:
> > The aehd-apic device type represents the AEHD in kernel APIC.
> > The irqchips should be always in kernel when AEHD is used.
> >
> > Signed-off-by: Haitao Shan <hshan@google.com>
> > ---
> >   MAINTAINERS                  |   2 +
> >   hw/i386/aehd/apic.c          | 204 +++++++++++++++++++++++
> >   hw/i386/aehd/meson.build     |   4 +
> >   hw/i386/meson.build          |   1 +
> >   include/hw/core/cpu.h        |   7 +
> >   include/sysemu/aehd.h        |  52 ++++++
> >   target/i386/aehd/aehd-all.c  | 315 +++++++++++++++++++++++++++++++++++
> >   target/i386/aehd/aehd.c      |  88 ++++++++++
> >   target/i386/aehd/aehd_int.h  |  50 ++++++
> >   target/i386/aehd/meson.build |   4 +
> >   target/i386/cpu-sysemu.c     |   3 +
> >   target/i386/meson.build      |   1 +
> >   12 files changed, 731 insertions(+)
> >   create mode 100644 hw/i386/aehd/apic.c
> >   create mode 100644 hw/i386/aehd/meson.build
> >   create mode 100644 target/i386/aehd/aehd-all.c
> >   create mode 100644 target/i386/aehd/aehd.c
> >   create mode 100644 target/i386/aehd/aehd_int.h
> >   create mode 100644 target/i386/aehd/meson.build
>
>
> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > index fb5d9667ca..5ed0600504 100644
> > --- a/include/hw/core/cpu.h
> > +++ b/include/hw/core/cpu.h
> > @@ -400,6 +400,13 @@ struct CPUState {
> >       uint32_t kvm_fetch_index;
> >       uint64_t dirty_pages;
> >
> > +    /* Only used in AEHD */
> > +#ifdef _WIN32
> > +    HANDLE aehd_fd;
> > +    struct AEHDState *aehd_state;
> > +    struct aehd_run *aehd_run;
> > +#endif
>
> We are difficultly trying to make this structure generic
> since 2 years now. Since AEHD won't make it to the 8.0 release,
> I'd rather take the time to get this part well done.
Can I know when 8.0 will be released? I don't aim to push my patches
as soon as possible. But understanding the timeline can be helpful.

>
> IIRC we ought to use a AccelCPUState pointer here, allocated by
> AccelCPUClass::cpu_instance_init()
I would agree. Will this be addressed when 8.0 is released?

>
> >       /* Use by accel-block: CPU is executing an ioctl() */
> >       QemuLockCnt in_ioctl_lock;
> >
> > diff --git a/include/sysemu/aehd.h b/include/sysemu/aehd.h
> > index 7ba4234f60..87fa2f8362 100644
> > --- a/include/sysemu/aehd.h
> > +++ b/include/sysemu/aehd.h
> > @@ -14,6 +14,12 @@
> >   #ifndef QEMU_AEHD_H
> >   #define QEMU_AEHD_H
> >
> > +#include "qemu/queue.h"
> > +#include "qemu/accel.h"
> > +#include "hw/core/cpu.h"
> > +#include "exec/memattrs.h"
> > +#include "hw/irq.h"
> > +
> >   #ifdef NEED_CPU_H
> >   # ifdef CONFIG_AEHD
> >   #  define CONFIG_AEHD_IS_POSSIBLE
> > @@ -23,3 +29,49 @@
> >   #endif
> >
> >   #define aehd_enabled()           (0)
> > +
> > +struct aehd_run;
> > +struct aehd_lapic_state;
> > +struct aehd_irq_routing_entry;
> > +
> > +struct AEHDState;
> > +
> > +#define TYPE_AEHD_ACCEL ACCEL_CLASS_NAME("aehd")
> > +typedef struct AEHDState AEHDState;
> > +DECLARE_INSTANCE_CHECKER(AEHDState, AEHD_STATE,
> > +                         TYPE_AEHD_ACCEL)
> > +
> > +extern AEHDState *aehd_state;
> > +
> > +#ifdef NEED_CPU_H
> > +#include "cpu.h"
> > +
> > +/* internal API */
> > +
> > +int aehd_ioctl(AEHDState *s, int type, void *input, size_t input_size,
> > +               void *output, size_t output_size);
> > +int aehd_vm_ioctl(AEHDState *s, int type, void *input, size_t input_size,
> > +                  void *output, size_t output_size);
> > +int aehd_vcpu_ioctl(CPUState *cpu, int type, void *input, size_t 
> > input_size,
> > +                    void *output, size_t output_size);
> > +
> > +/* Arch specific hooks */
> > +
> > +/* Notify arch about newly added MSI routes */
> > +int aehd_arch_add_msi_route_post(struct aehd_irq_routing_entry *route,
> > +                                 int vector, PCIDevice *dev);
> > +/* Notify arch about released MSI routes */
> > +int aehd_arch_release_virq_post(int virq);
> > +
> > +int aehd_set_irq(AEHDState *s, int irq, int level);
> > +int aehd_irqchip_send_msi(AEHDState *s, MSIMessage msg);
> > +
> > +void aehd_put_apic_state(DeviceState *d, struct aehd_lapic_state *kapic);
> > +void aehd_get_apic_state(DeviceState *d, struct aehd_lapic_state *kapic);
> > +
> > +#endif /* NEED_CPU_H */
> > +
> > +void aehd_irqchip_commit_routes(AEHDState *s);
> > +void aehd_irqchip_release_virq(AEHDState *s, int virq);
> > +
> > +#endif
>


-- 
Haitao @Google