[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 12/15] kvm: x86: Add user space part for in-k
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [PATCH v4 12/15] kvm: x86: Add user space part for in-kernel APIC |
Date: |
Sat, 10 Dec 2011 16:58:38 +0100 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
On 2011-12-10 16:40, Blue Swirl wrote:
> On Fri, Dec 9, 2011 at 07:52, Jan Kiszka <address@hidden> wrote:
>> 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 <address@hidden> 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 <address@hidden>
>>>>> Signed-off-by: Jan Kiszka <address@hidden>
>>>>> ---
>>>>> 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 <address@hidden>
>>>>> + *
>>>>> + * 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.
>
> There's kvm_get_supported_msrs for example.
>
>> 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.
>
> It does, passing CPUState violates layering. Please split the
> functions so that the ioctl calls which need CPUState go to kvm.c. For
> example, the functions in kvm/apic.c could just perform copying from
> kvm_lapic_state fields to APICstate fields and vice versa.
That's a good idea.
>
> The KVM interface by the way does not look so clever. Why isn't there
> just an array of 32 bit fields so the casts can be avoided? Perhaps
> APICState should be (later) changed to match KVM version so that the
> structure can be passed directly without copying.
Wouldn't that complicate the use in the user space model again? At least
for registers that are used with both backends.
Jan
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v4 05/15] apic: Introduce backend/frontend infrastructure for KVM reuse, (continued)
- [Qemu-devel] [PATCH v4 05/15] apic: Introduce backend/frontend infrastructure for KVM reuse, Jan Kiszka, 2011/12/08
- [Qemu-devel] [PATCH v4 13/15] kvm: x86: Add user space part for in-kernel i8259, Jan Kiszka, 2011/12/08
- [Qemu-devel] [PATCH v4 06/15] apic: Open-code timer save/restore, Jan Kiszka, 2011/12/08
- [Qemu-devel] [PATCH v4 10/15] kvm: Introduce core services for in-kernel irqchip support, Jan Kiszka, 2011/12/08
- [Qemu-devel] [PATCH v4 03/15] apic: Stop timer on reset, Jan Kiszka, 2011/12/08
- [Qemu-devel] [PATCH v4 12/15] kvm: x86: Add user space part for in-kernel APIC, Jan Kiszka, 2011/12/08
[Qemu-devel] [PATCH v4 02/15] kvm: Move kvmclock into hw/kvm folder, Jan Kiszka, 2011/12/08
[Qemu-devel] [PATCH v4 04/15] apic: Inject external NMI events via LINT1, Jan Kiszka, 2011/12/08
Re: [Qemu-devel] [PATCH v4 00/15] uq/master: Introduce basic irqchip support, Blue Swirl, 2011/12/08
Re: [Qemu-devel] [PATCH v4 00/15] uq/master: Introduce basic irqchip support, Marcelo Tosatti, 2011/12/12