Re: [Qemu-devel] [PATCH v6 06/18] apic: Factor out base class for KVM reuse

[Anthony Liguori <aliguori@us.ibm.com>]

On 01/13/2012 11:35 AM, Jan Kiszka wrote:
> The KVM in-kernel APIC model will reuse parts of the user space model
> while providing the same frontend view to guest and most management
> interfaces.
>
> Factor out an APIC base class to encapsulate those parts that will be
> shared by user space and KVM model. This class offers callback hooks for
> init, base/tpr setting, and the external NMI delivery that will be
> set via APICCommonInfo super-structure and implemented specifically in
> the subclasses.
>
> Furthermore, a common vmstate and a common list of qdev properties is
> provided with the base clase. Also the reset handler will be shared.
>
> diff --git a/hw/apic_internal.h b/hw/apic_internal.h
> new file mode 100644
> index 0000000..2ca18d4
> --- /dev/null
> +++ b/hw/apic_internal.h
> @@ -0,0 +1,121 @@
> +/*
> + *  APIC support - internal interfaces
> + *
> + *  Copyright (c) 2004-2005 Fabrice Bellard
> + *  Copyright (c) 2011      Jan Kiszka, Siemens AG
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see<http://www.gnu.org/licenses/>
> + */
> +#ifndef QEMU_APIC_INTERNAL_H
> +#define QEMU_APIC_INTERNAL_H
> +
> +#include "memory.h"
> +#include "sysbus.h"
> +#include "qemu-timer.h"
> +
> +/* APIC Local Vector Table */
> +#define APIC_LVT_TIMER                  0
> +#define APIC_LVT_THERMAL                1
> +#define APIC_LVT_PERFORM                2
> +#define APIC_LVT_LINT0                  3
> +#define APIC_LVT_LINT1                  4
> +#define APIC_LVT_ERROR                  5
> +#define APIC_LVT_NB                     6
> +
> +/* APIC delivery modes */
> +#define APIC_DM_FIXED                   0
> +#define APIC_DM_LOWPRI                  1
> +#define APIC_DM_SMI                     2
> +#define APIC_DM_NMI                     4
> +#define APIC_DM_INIT                    5
> +#define APIC_DM_SIPI                    6
> +#define APIC_DM_EXTINT                  7
> +
> +/* APIC destination mode */
> +#define APIC_DESTMODE_FLAT              0xf
> +#define APIC_DESTMODE_CLUSTER           1
> +
> +#define APIC_TRIGGER_EDGE               0
> +#define APIC_TRIGGER_LEVEL              1
> +
> +#define APIC_LVT_TIMER_PERIODIC         (1<<17)
> +#define APIC_LVT_MASKED                 (1<<16)
> +#define APIC_LVT_LEVEL_TRIGGER          (1<<15)
> +#define APIC_LVT_REMOTE_IRR             (1<<14)
> +#define APIC_INPUT_POLARITY             (1<<13)
> +#define APIC_SEND_PENDING               (1<<12)
> +
> +#define ESR_ILLEGAL_ADDRESS (1<<  7)
> +
> +#define APIC_SV_DIRECTED_IO             (1<<12)
> +#define APIC_SV_ENABLE                  (1<<8)
> +
> +#define MAX_APICS 255
> +
> +#define MSI_SPACE_SIZE                  0x100000
> +
> +typedef struct APICCommonState APICCommonState;
> +
> +struct APICCommonState {
> +    SysBusDevice busdev;
> +    MemoryRegion io_memory;
> +    void *cpu_env;
> +    uint32_t apicbase;
> +    uint8_t id;
> +    uint8_t arb_id;
> +    uint8_t tpr;
> +    uint32_t spurious_vec;
> +    uint8_t log_dest;
> +    uint8_t dest_mode;
> +    uint32_t isr[8];  /* in service register */
> +    uint32_t tmr[8];  /* trigger mode register */
> +    uint32_t irr[8]; /* interrupt request register */
> +    uint32_t lvt[APIC_LVT_NB];
> +    uint32_t esr; /* error register */
> +    uint32_t icr[2];
> +
> +    uint32_t divide_conf;
> +    int count_shift;
> +    uint32_t initial_count;
> +    int64_t initial_count_load_time;
> +    int64_t next_time;
> +    int idx;
> +    QEMUTimer *timer;
> +    int sipi_vector;
> +    int wait_for_sipi;
> +};
> +
> +typedef struct APICCommonInfo APICCommonInfo;
> +
> +struct APICCommonInfo {
> +    SysBusDeviceInfo busdev;
> +    void (*init)(APICCommonState *s);
> +    void (*set_base)(APICCommonState *s, uint64_t val);
> +    void (*set_tpr)(APICCommonState *s, uint8_t val);
> +    void (*external_nmi)(APICCommonState *s);
> +};
> +
> +#define APIC_VMSTATE_VERSION 3
> +
> +extern const VMStateDescription vmstate_apic_common;
> +
> +#define APIC_PROPERTIES_COMMON(cont_type, cont_var)         \
> +    DEFINE_PROP_UINT8("id", cont_type, cont_var.id, -1),    \
> +    DEFINE_PROP_PTR("cpu_env", cont_type, cont_var.cpu_env)
> +
> +void apic_report_irq_delivered(int delivered);
> +void apic_reset_common(DeviceState *d);
> +int apic_init_common(SysBusDevice *d);

I think you want to introduce an apic_qdev_register(APICCommonInfo *).  Then you 
can set info->qdev.vmsd = &vmstate_apic_common and avoid having to make it extern.

Regards,

Anthony Liguori

> +
> +#endif /* !QEMU_APIC_INTERNAL_H */