Re: [PATCH v4 12/15] kvm: x86: Add user space part for in-kernel APIC

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2011-12-09 08:45, Jan Kiszka wrote:
> On 2011-12-08 22:16, Blue Swirl wrote:
>> On Thu, Dec 8, 2011 at 11:52, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> This introduces the alternative APIC backend which makes use of KVM's
>>> in-kernel device model. External NMI injection via LINT1 is emulated by
>>> checking the current state of the in-kernel APIC, only injecting a NMI
>>> into the VCPU if LINT1 is unmasked and configured to DM_NMI.
>>>
>>> MSI is not yet supported, so we disable this when the in-kernel model is
>>> in use.
>>>
>>> CC: Lai Jiangshan <laijs@cn.fujitsu.com>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>  Makefile.target   |    2 +-
>>>  hw/kvm/apic.c     |  154 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  hw/pc.c           |   15 ++++--
>>>  kvm.h             |    3 +
>>>  target-i386/kvm.c |    8 +++
>>>  5 files changed, 176 insertions(+), 6 deletions(-)
>>>  create mode 100644 hw/kvm/apic.c
>>>
>>> diff --git a/Makefile.target b/Makefile.target
>>> index b549988..76de485 100644
>>> --- a/Makefile.target
>>> +++ b/Makefile.target
>>> @@ -236,7 +236,7 @@ obj-i386-y += vmport.o
>>>  obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
>>>  obj-i386-y += debugcon.o multiboot.o
>>>  obj-i386-y += pc_piix.o
>>> -obj-i386-$(CONFIG_KVM) += kvm/clock.o
>>> +obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o
>>>  obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
>>>
>>>  # shared objects
>>> diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c
>>> new file mode 100644
>>> index 0000000..3924f9e
>>> --- /dev/null
>>> +++ b/hw/kvm/apic.c
>>> @@ -0,0 +1,154 @@
>>> +/*
>>> + * KVM in-kernel APIC support
>>> + *
>>> + * Copyright (c) 2011 Siemens AG
>>> + *
>>> + * Authors:
>>> + *  Jan Kiszka          <jan.kiszka@siemens.com>
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL version 2.
>>> + * See the COPYING file in the top-level directory.
>>> + */
>>> +#include "hw/apic_internal.h"
>>> +#include "kvm.h"
>>> +
>>> +static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
>>> +                                   int reg_id, uint32_t val)
>>> +{
>>> +    *((uint32_t *)(kapic->regs + (reg_id << 4))) = val;
>>> +}
>>> +
>>> +static inline uint32_t kvm_apic_get_reg(struct kvm_lapic_state *kapic,
>>> +                                       int reg_id)
>>> +{
>>> +    return *((uint32_t *)(kapic->regs + (reg_id << 4)));
>>> +}
>>> +
>>> +int kvm_put_apic(CPUState *env)
>>> +{
>>> +    APICState *s = DO_UPCAST(APICState, busdev.qdev, env->apic_state);
>>
>> Please pass APICState instead of CPUState.
> 
> DeviceState, I suppose. Yes, makes more sense, update will follow.

On second look: no, I'll keep it as is. All kvm_get/put_* helpers have
this kind of signature, i.e. are working against env. kvm_get/put_apic
just happens to be implemented outside of target-i386/kvm.c. And they
require both APIC and CPUState anyway, so it makes no difference.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux