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

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 5470 bytes --]

On 2012-01-13 22:18, Anthony Liguori wrote:
> 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.

Yep. Will also help with other common settings.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]