From: Jan Kiszka <jan.kiszka@web.de>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: mtosatti@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org,
avi@redhat.com
Subject: Re: [Qemu-devel] [PATCH 1/3] kvm: move KVM_GET_LAPIC/KVM_SET_LAPIC to hw/kvm/apic.c
Date: Tue, 30 Oct 2012 19:13:56 +0100 [thread overview]
Message-ID: <509018E4.9020001@web.de> (raw)
In-Reply-To: <1351599394-24876-2-git-send-email-pbonzini@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 7114 bytes --]
On 2012-10-30 13:16, Paolo Bonzini wrote:
> Leave knowledge of the KVM in-kernel LAPIC ioctls to hw/kvm/apic.c.
> The CPU doesn't need to know anything about kvm_lapic_state.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/kvm/apic.c | 76 ++++++++++++++++++++++++++++++-----------------------
> kvm.h | 4 +-
> target-i386/kvm.c | 14 +--------
> 3 files changed, 47 insertions(+), 47 deletions(-)
>
> diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c
> index dbac7ff..ddf6b7d 100644
> --- a/hw/kvm/apic.c
> +++ b/hw/kvm/apic.c
> @@ -25,62 +25,72 @@ static inline uint32_t kvm_apic_get_reg(struct kvm_lapic_state *kapic,
> return *((uint32_t *)(kapic->regs + (reg_id << 4)));
> }
>
> -void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
> +int kvm_put_apic_state(DeviceState *d)
> {
> APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + struct kvm_lapic_state kapic;
> int i;
>
> - 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);
> + 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, 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]);
> + 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, 0x32 + i, s->lvt[i]);
> }
> - kvm_apic_set_reg(kapic, 0x38, s->initial_count);
> - kvm_apic_set_reg(kapic, 0x3e, s->divide_conf);
> + kvm_apic_set_reg(&kapic, 0x38, s->initial_count);
> + kvm_apic_set_reg(&kapic, 0x3e, s->divide_conf);
> +
> + return kvm_vcpu_ioctl(s->cpu_env, KVM_SET_LAPIC, &kapic);
> }
>
> -void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
> +int kvm_get_apic_state(DeviceState *d)
> {
> APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> - int i, v;
> -
> - 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);
> + struct kvm_lapic_state kapic;
> + int i, v, ret;
> +
> + ret = kvm_vcpu_ioctl(s->cpu_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->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);
> + 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->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);
> + 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(APICCommonState *s, uint64_t val)
> diff --git a/kvm.h b/kvm.h
> index 2b26dcb..0056f92 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -191,8 +191,8 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg);
>
> void kvm_irqchip_add_irq_route(KVMState *s, int gsi, int irqchip, int pin);
>
> -void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic);
> -void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic);
> +int kvm_put_apic_state(DeviceState *d);
> +int kvm_get_apic_state(DeviceState *d);
>
> struct kvm_guest_debug;
> struct kvm_debug_exit_arch;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 3aa62b2..092d4f1 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1384,16 +1384,9 @@ static int kvm_get_mp_state(CPUX86State *env)
> static int kvm_get_apic(CPUX86State *env)
> {
> DeviceState *apic = env->apic_state;
> - struct kvm_lapic_state kapic;
> - int ret;
>
> if (apic && kvm_irqchip_in_kernel()) {
> - ret = kvm_vcpu_ioctl(env, KVM_GET_LAPIC, &kapic);
> - if (ret < 0) {
> - return ret;
> - }
> -
> - kvm_get_apic_state(apic, &kapic);
> + return kvm_get_apic_state(apic);
> }
> return 0;
> }
> @@ -1401,12 +1394,9 @@ static int kvm_get_apic(CPUX86State *env)
> static int kvm_put_apic(CPUX86State *env)
> {
> DeviceState *apic = env->apic_state;
> - struct kvm_lapic_state kapic;
>
> if (apic && kvm_irqchip_in_kernel()) {
> - kvm_put_apic_state(apic, &kapic);
> -
> - return kvm_vcpu_ioctl(env, KVM_SET_LAPIC, &kapic);
> + return kvm_put_apic_state(apic);
> }
> return 0;
> }
>
This refactoring is fine with me.
Acked-by: Jan Kiszka <jan.kiszka@siemens.com>
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
next prev parent reply other threads:[~2012-10-30 18:14 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-30 12:16 [Qemu-devel] [PATCH uq/master 0/3] Fix MSI injection at load time Paolo Bonzini
2012-10-30 12:16 ` [Qemu-devel] [PATCH 1/3] kvm: move KVM_GET_LAPIC/KVM_SET_LAPIC to hw/kvm/apic.c Paolo Bonzini
2012-10-30 18:13 ` Jan Kiszka [this message]
2012-10-30 12:16 ` [Qemu-devel] [PATCH 2/3] apic: add get/put methods Paolo Bonzini
2012-10-30 18:17 ` Jan Kiszka
2012-10-30 12:16 ` [Qemu-devel] [PATCH 3/3] apic: always update the in-kernel status after loading Paolo Bonzini
2012-10-30 12:38 ` Avi Kivity
2012-10-30 14:16 ` Paolo Bonzini
2012-10-30 18:21 ` Jan Kiszka
2012-11-02 14:53 ` Paolo Bonzini
2012-11-02 14:59 ` Jan Kiszka
2012-11-02 15:07 ` Gerd Hoffmann
2012-11-02 15:13 ` Paolo Bonzini
2012-11-02 15:17 ` Gerd Hoffmann
2012-11-02 15:21 ` Paolo Bonzini
2012-10-30 18:17 ` Jan Kiszka
2012-10-30 12:16 ` [Qemu-devel] [PATCH 4/3] ioapic: change pre_save/post_load methods to get/put Paolo Bonzini
2012-10-30 18:18 ` Jan Kiszka
2012-10-30 12:16 ` [Qemu-devel] [PATCH 5/3] ioapic: unify reset callbacks Paolo Bonzini
2012-10-30 16:47 ` [Qemu-devel] [PATCH uq/master 0/3] Fix MSI injection at load time Paolo Bonzini
2012-10-30 18:22 ` Jan Kiszka
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=509018E4.9020001@web.de \
--to=jan.kiszka@web.de \
--cc=avi@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=mtosatti@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).