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

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

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.

> 
>> +    struct kvm_lapic_state kapic;
>> +    int i;
>> +
>> +    if (s && kvm_enabled() && kvm_irqchip_in_kernel()) {
>> +        memset(&kapic, 0, sizeof(kapic));
>> +        kvm_apic_set_reg(&kapic, 0x2, s->id << 24);
>> +        kvm_apic_set_reg(&kapic, 0x8, s->tpr);
>> +        kvm_apic_set_reg(&kapic, 0xd, s->log_dest << 24);
>> +        kvm_apic_set_reg(&kapic, 0xe, s->dest_mode << 28 | 0x0fffffff);
>> +        kvm_apic_set_reg(&kapic, 0xf, s->spurious_vec);
>> +        for (i = 0; i < 8; i++) {
>> +            kvm_apic_set_reg(&kapic, 0x10 + i, s->isr[i]);
>> +            kvm_apic_set_reg(&kapic, 0x18 + i, s->tmr[i]);
>> +            kvm_apic_set_reg(&kapic, 0x20 + i, s->irr[i]);
>> +        }
>> +        kvm_apic_set_reg(&kapic, 0x28, s->esr);
>> +        kvm_apic_set_reg(&kapic, 0x30, s->icr[0]);
>> +        kvm_apic_set_reg(&kapic, 0x31, s->icr[1]);
>> +        for (i = 0; i < APIC_LVT_NB; i++) {
>> +            kvm_apic_set_reg(&kapic, 0x32 + i, s->lvt[i]);
>> +        }
>> +        kvm_apic_set_reg(&kapic, 0x38, s->initial_count);
>> +        kvm_apic_set_reg(&kapic, 0x3e, s->divide_conf);
>> +
>> +        return kvm_vcpu_ioctl(env, KVM_SET_LAPIC, &kapic);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +int kvm_get_apic(CPUState *env)
> 
> Same here.
> 
>> +{
>> +    APICState *s = DO_UPCAST(APICState, busdev.qdev, env->apic_state);
>> +    struct kvm_lapic_state kapic;
>> +    int ret, i, v;
>> +
>> +    if (s && kvm_enabled() && kvm_irqchip_in_kernel()) {
>> +        ret = kvm_vcpu_ioctl(env, KVM_GET_LAPIC, &kapic);
>> +        if (ret < 0) {
>> +            return ret;
>> +        }
>> +
>> +        s->id = kvm_apic_get_reg(&kapic, 0x2) >> 24;
>> +        s->tpr = kvm_apic_get_reg(&kapic, 0x8);
>> +        s->arb_id = kvm_apic_get_reg(&kapic, 0x9);
>> +        s->log_dest = kvm_apic_get_reg(&kapic, 0xd) >> 24;
>> +        s->dest_mode = kvm_apic_get_reg(&kapic, 0xe) >> 28;
>> +        s->spurious_vec = kvm_apic_get_reg(&kapic, 0xf);
>> +        for (i = 0; i < 8; i++) {
>> +            s->isr[i] = kvm_apic_get_reg(&kapic, 0x10 + i);
>> +            s->tmr[i] = kvm_apic_get_reg(&kapic, 0x18 + i);
>> +            s->irr[i] = kvm_apic_get_reg(&kapic, 0x20 + i);
>> +        }
>> +        s->esr = kvm_apic_get_reg(&kapic, 0x28);
>> +        s->icr[0] = kvm_apic_get_reg(&kapic, 0x30);
>> +        s->icr[1] = kvm_apic_get_reg(&kapic, 0x31);
>> +        for (i = 0; i < APIC_LVT_NB; i++) {
>> +            s->lvt[i] = kvm_apic_get_reg(&kapic, 0x32 + i);
>> +        }
>> +        s->initial_count = kvm_apic_get_reg(&kapic, 0x38);
>> +        s->divide_conf = kvm_apic_get_reg(&kapic, 0x3e);
>> +
>> +        v = (s->divide_conf & 3) | ((s->divide_conf >> 1) & 4);
>> +        s->count_shift = (v + 1) & 7;
>> +
>> +        s->initial_count_load_time = qemu_get_clock_ns(vm_clock);
>> +        apic_next_timer(s, s->initial_count_load_time);
>> +    }
>> +    return 0;
>> +}
>> +
>> +static void kvm_apic_set_base(APICState *s, uint64_t val)
>> +{
>> +    s->apicbase = val;
>> +}
>> +
>> +static void kvm_apic_set_tpr(APICState *s, uint8_t val)
>> +{
>> +    s->tpr = (val & 0x0f) << 4;
>> +}
>> +
>> +static void do_inject_external_nmi(void *data)
>> +{
>> +    APICState *s = data;
>> +    CPUState *env = s->cpu_env;
>> +    uint32_t lvt;
>> +    int ret;
>> +
>> +    cpu_synchronize_state(env);
>> +
>> +    lvt = s->lvt[APIC_LVT_LINT1];
>> +    if (!(lvt & APIC_LVT_MASKED) && ((lvt >> 8) & 7) == APIC_DM_NMI) {
>> +        ret = kvm_vcpu_ioctl(env, KVM_NMI);
>> +        if (ret < 0) {
>> +            fprintf(stderr, "KVM: injection failed, NMI lost (%s)\n",
>> +                    strerror(-ret));
>> +        }
>> +    }
>> +}
>> +
>> +static void kvm_apic_external_nmi(APICState *s)
>> +{
>> +    run_on_cpu(s->cpu_env, do_inject_external_nmi, s);
> 
> Here probably CPUState would make more sense.

But here I think it's more consistent to stay with APICState (for the
hook) and DeviceState (for apic_deliver_nmi).

Jan

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