From: Jan Kiszka <jan.kiszka@web.de>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, avi@redhat.com, mtosatti@redhat.com,
kvm@vger.kernel.org
Subject: Re: [PATCH 2/3] apic: add get/put methods
Date: Tue, 30 Oct 2012 19:17:13 +0100 [thread overview]
Message-ID: <509019A9.7060803@web.de> (raw)
In-Reply-To: <1351599394-24876-3-git-send-email-pbonzini@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 6110 bytes --]
On 2012-10-30 13:16, Paolo Bonzini wrote:
> Change the hard-coded references to kvm_get_apic_state and
> kvm_put_apic_state to methods in APICCommonClass. This makes it possible
> to reuse the methods in common code that cannot assume the presence
> of KVM.
The effect of patch 3 can be achieved using existing callbacks, and I
fail to see how this is nicer (the concept of "get" and "put" device
model states is not really generic). So I would skip this refactoring.
Jan
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/apic.h | 2 ++
> hw/apic_common.c | 32 ++++++++++++++++++++++++++++++++
> hw/apic_internal.h | 2 ++
> hw/kvm/apic.c | 8 ++++----
> kvm.h | 3 ---
> target-i386/kvm.c | 24 ++----------------------
> 6 files changed, 42 insertions(+), 29 deletions(-)
>
> diff --git a/hw/apic.h b/hw/apic.h
> index 1d48e02..f15d100 100644
> --- a/hw/apic.h
> +++ b/hw/apic.h
> @@ -12,6 +12,8 @@ void apic_deliver_nmi(DeviceState *d);
> int apic_get_interrupt(DeviceState *s);
> void apic_reset_irq_delivered(void);
> int apic_get_irq_delivered(void);
> +int cpu_get_apic_state(DeviceState *d);
> +int cpu_put_apic_state(DeviceState *d);
> void cpu_set_apic_base(DeviceState *s, uint64_t val);
> uint64_t cpu_get_apic_base(DeviceState *s);
> void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
> diff --git a/hw/apic_common.c b/hw/apic_common.c
> index d68116d..f373ba8 100644
> --- a/hw/apic_common.c
> +++ b/hw/apic_common.c
> @@ -312,6 +312,38 @@ static int apic_init_common(SysBusDevice *dev)
> return 0;
> }
>
> +int cpu_get_apic_state(DeviceState *apic)
> +{
> + APICCommonState *s;
> + APICCommonClass *info;
> + if (!apic) {
> + return 0;
> + }
> +
> + s = APIC_COMMON(apic);
> + info = APIC_COMMON_GET_CLASS(s);
> + if (info->get) {
> + return info->get(s);
> + }
> + return 0;
> +}
> +
> +int cpu_put_apic_state(DeviceState *apic)
> +{
> + APICCommonState *s;
> + APICCommonClass *info;
> + if (!apic) {
> + return 0;
> + }
> +
> + s = APIC_COMMON(apic);
> + info = APIC_COMMON_GET_CLASS(s);
> + if (info->put) {
> + return info->put(s);
> + }
> + return 0;
> +}
> +
> static void apic_dispatch_pre_save(void *opaque)
> {
> APICCommonState *s = APIC_COMMON(opaque);
> diff --git a/hw/apic_internal.h b/hw/apic_internal.h
> index 30932a3..256fb1a 100644
> --- a/hw/apic_internal.h
> +++ b/hw/apic_internal.h
> @@ -89,6 +89,8 @@ typedef struct APICCommonClass
> void (*enable_tpr_reporting)(APICCommonState *s, bool enable);
> void (*vapic_base_update)(APICCommonState *s);
> void (*external_nmi)(APICCommonState *s);
> + int (*get)(APICCommonState *s);
> + int (*put)(APICCommonState *s);
> void (*pre_save)(APICCommonState *s);
> void (*post_load)(APICCommonState *s);
> } APICCommonClass;
> diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c
> index ddf6b7d..35afe0c 100644
> --- a/hw/kvm/apic.c
> +++ b/hw/kvm/apic.c
> @@ -25,9 +25,8 @@ static inline uint32_t kvm_apic_get_reg(struct kvm_lapic_state *kapic,
> return *((uint32_t *)(kapic->regs + (reg_id << 4)));
> }
>
> -int kvm_put_apic_state(DeviceState *d)
> +static int kvm_put_apic_state(APICCommonState *s)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> struct kvm_lapic_state kapic;
> int i;
>
> @@ -54,9 +53,8 @@ int kvm_put_apic_state(DeviceState *d)
> return kvm_vcpu_ioctl(s->cpu_env, KVM_SET_LAPIC, &kapic);
> }
>
> -int kvm_get_apic_state(DeviceState *d)
> +static int kvm_get_apic_state(APICCommonState *s)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> struct kvm_lapic_state kapic;
> int i, v, ret;
>
> @@ -202,6 +200,8 @@ static void kvm_apic_class_init(ObjectClass *klass, void *data)
> k->enable_tpr_reporting = kvm_apic_enable_tpr_reporting;
> k->vapic_base_update = kvm_apic_vapic_base_update;
> k->external_nmi = kvm_apic_external_nmi;
> + k->get = kvm_get_apic_state;
> + k->put = kvm_put_apic_state;
> }
>
> static TypeInfo kvm_apic_info = {
> diff --git a/kvm.h b/kvm.h
> index 0056f92..83f7b05 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -191,9 +191,6 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg);
>
> void kvm_irqchip_add_irq_route(KVMState *s, int gsi, int irqchip, int pin);
>
> -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 092d4f1..0912e15 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1381,26 +1381,6 @@ static int kvm_get_mp_state(CPUX86State *env)
> return 0;
> }
>
> -static int kvm_get_apic(CPUX86State *env)
> -{
> - DeviceState *apic = env->apic_state;
> -
> - if (apic && kvm_irqchip_in_kernel()) {
> - return kvm_get_apic_state(apic);
> - }
> - return 0;
> -}
> -
> -static int kvm_put_apic(CPUX86State *env)
> -{
> - DeviceState *apic = env->apic_state;
> -
> - if (apic && kvm_irqchip_in_kernel()) {
> - return kvm_put_apic_state(apic);
> - }
> - return 0;
> -}
> -
> static int kvm_put_vcpu_events(CPUX86State *env, int level)
> {
> struct kvm_vcpu_events events;
> @@ -1576,7 +1556,7 @@ int kvm_arch_put_registers(CPUX86State *env, int level)
> if (ret < 0) {
> return ret;
> }
> - ret = kvm_put_apic(env);
> + ret = cpu_put_apic_state(env->apic_state);
> if (ret < 0) {
> return ret;
> }
> @@ -1627,7 +1607,7 @@ int kvm_arch_get_registers(CPUX86State *env)
> if (ret < 0) {
> return ret;
> }
> - ret = kvm_get_apic(env);
> + ret = cpu_get_apic_state(env->apic_state);
> if (ret < 0) {
> return ret;
> }
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
WARNING: multiple messages have this Message-ID (diff)
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 2/3] apic: add get/put methods
Date: Tue, 30 Oct 2012 19:17:13 +0100 [thread overview]
Message-ID: <509019A9.7060803@web.de> (raw)
In-Reply-To: <1351599394-24876-3-git-send-email-pbonzini@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 6110 bytes --]
On 2012-10-30 13:16, Paolo Bonzini wrote:
> Change the hard-coded references to kvm_get_apic_state and
> kvm_put_apic_state to methods in APICCommonClass. This makes it possible
> to reuse the methods in common code that cannot assume the presence
> of KVM.
The effect of patch 3 can be achieved using existing callbacks, and I
fail to see how this is nicer (the concept of "get" and "put" device
model states is not really generic). So I would skip this refactoring.
Jan
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/apic.h | 2 ++
> hw/apic_common.c | 32 ++++++++++++++++++++++++++++++++
> hw/apic_internal.h | 2 ++
> hw/kvm/apic.c | 8 ++++----
> kvm.h | 3 ---
> target-i386/kvm.c | 24 ++----------------------
> 6 files changed, 42 insertions(+), 29 deletions(-)
>
> diff --git a/hw/apic.h b/hw/apic.h
> index 1d48e02..f15d100 100644
> --- a/hw/apic.h
> +++ b/hw/apic.h
> @@ -12,6 +12,8 @@ void apic_deliver_nmi(DeviceState *d);
> int apic_get_interrupt(DeviceState *s);
> void apic_reset_irq_delivered(void);
> int apic_get_irq_delivered(void);
> +int cpu_get_apic_state(DeviceState *d);
> +int cpu_put_apic_state(DeviceState *d);
> void cpu_set_apic_base(DeviceState *s, uint64_t val);
> uint64_t cpu_get_apic_base(DeviceState *s);
> void cpu_set_apic_tpr(DeviceState *s, uint8_t val);
> diff --git a/hw/apic_common.c b/hw/apic_common.c
> index d68116d..f373ba8 100644
> --- a/hw/apic_common.c
> +++ b/hw/apic_common.c
> @@ -312,6 +312,38 @@ static int apic_init_common(SysBusDevice *dev)
> return 0;
> }
>
> +int cpu_get_apic_state(DeviceState *apic)
> +{
> + APICCommonState *s;
> + APICCommonClass *info;
> + if (!apic) {
> + return 0;
> + }
> +
> + s = APIC_COMMON(apic);
> + info = APIC_COMMON_GET_CLASS(s);
> + if (info->get) {
> + return info->get(s);
> + }
> + return 0;
> +}
> +
> +int cpu_put_apic_state(DeviceState *apic)
> +{
> + APICCommonState *s;
> + APICCommonClass *info;
> + if (!apic) {
> + return 0;
> + }
> +
> + s = APIC_COMMON(apic);
> + info = APIC_COMMON_GET_CLASS(s);
> + if (info->put) {
> + return info->put(s);
> + }
> + return 0;
> +}
> +
> static void apic_dispatch_pre_save(void *opaque)
> {
> APICCommonState *s = APIC_COMMON(opaque);
> diff --git a/hw/apic_internal.h b/hw/apic_internal.h
> index 30932a3..256fb1a 100644
> --- a/hw/apic_internal.h
> +++ b/hw/apic_internal.h
> @@ -89,6 +89,8 @@ typedef struct APICCommonClass
> void (*enable_tpr_reporting)(APICCommonState *s, bool enable);
> void (*vapic_base_update)(APICCommonState *s);
> void (*external_nmi)(APICCommonState *s);
> + int (*get)(APICCommonState *s);
> + int (*put)(APICCommonState *s);
> void (*pre_save)(APICCommonState *s);
> void (*post_load)(APICCommonState *s);
> } APICCommonClass;
> diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c
> index ddf6b7d..35afe0c 100644
> --- a/hw/kvm/apic.c
> +++ b/hw/kvm/apic.c
> @@ -25,9 +25,8 @@ static inline uint32_t kvm_apic_get_reg(struct kvm_lapic_state *kapic,
> return *((uint32_t *)(kapic->regs + (reg_id << 4)));
> }
>
> -int kvm_put_apic_state(DeviceState *d)
> +static int kvm_put_apic_state(APICCommonState *s)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> struct kvm_lapic_state kapic;
> int i;
>
> @@ -54,9 +53,8 @@ int kvm_put_apic_state(DeviceState *d)
> return kvm_vcpu_ioctl(s->cpu_env, KVM_SET_LAPIC, &kapic);
> }
>
> -int kvm_get_apic_state(DeviceState *d)
> +static int kvm_get_apic_state(APICCommonState *s)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> struct kvm_lapic_state kapic;
> int i, v, ret;
>
> @@ -202,6 +200,8 @@ static void kvm_apic_class_init(ObjectClass *klass, void *data)
> k->enable_tpr_reporting = kvm_apic_enable_tpr_reporting;
> k->vapic_base_update = kvm_apic_vapic_base_update;
> k->external_nmi = kvm_apic_external_nmi;
> + k->get = kvm_get_apic_state;
> + k->put = kvm_put_apic_state;
> }
>
> static TypeInfo kvm_apic_info = {
> diff --git a/kvm.h b/kvm.h
> index 0056f92..83f7b05 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -191,9 +191,6 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg);
>
> void kvm_irqchip_add_irq_route(KVMState *s, int gsi, int irqchip, int pin);
>
> -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 092d4f1..0912e15 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1381,26 +1381,6 @@ static int kvm_get_mp_state(CPUX86State *env)
> return 0;
> }
>
> -static int kvm_get_apic(CPUX86State *env)
> -{
> - DeviceState *apic = env->apic_state;
> -
> - if (apic && kvm_irqchip_in_kernel()) {
> - return kvm_get_apic_state(apic);
> - }
> - return 0;
> -}
> -
> -static int kvm_put_apic(CPUX86State *env)
> -{
> - DeviceState *apic = env->apic_state;
> -
> - if (apic && kvm_irqchip_in_kernel()) {
> - return kvm_put_apic_state(apic);
> - }
> - return 0;
> -}
> -
> static int kvm_put_vcpu_events(CPUX86State *env, int level)
> {
> struct kvm_vcpu_events events;
> @@ -1576,7 +1556,7 @@ int kvm_arch_put_registers(CPUX86State *env, int level)
> if (ret < 0) {
> return ret;
> }
> - ret = kvm_put_apic(env);
> + ret = cpu_put_apic_state(env->apic_state);
> if (ret < 0) {
> return ret;
> }
> @@ -1627,7 +1607,7 @@ int kvm_arch_get_registers(CPUX86State *env)
> if (ret < 0) {
> return ret;
> }
> - ret = kvm_get_apic(env);
> + ret = cpu_get_apic_state(env->apic_state);
> if (ret < 0) {
> return ret;
> }
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
next prev parent reply other threads:[~2012-10-30 18:17 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-30 12:16 [PATCH uq/master 0/3] Fix MSI injection at load time Paolo Bonzini
2012-10-30 12:16 ` [Qemu-devel] " Paolo Bonzini
2012-10-30 12:16 ` [PATCH 1/3] kvm: move KVM_GET_LAPIC/KVM_SET_LAPIC to hw/kvm/apic.c Paolo Bonzini
2012-10-30 12:16 ` [Qemu-devel] " Paolo Bonzini
2012-10-30 18:13 ` Jan Kiszka
2012-10-30 18:13 ` [Qemu-devel] " Jan Kiszka
2012-10-30 12:16 ` [PATCH 2/3] apic: add get/put methods Paolo Bonzini
2012-10-30 12:16 ` [Qemu-devel] " Paolo Bonzini
2012-10-30 18:17 ` Jan Kiszka [this message]
2012-10-30 18:17 ` Jan Kiszka
2012-10-30 12:16 ` [PATCH 3/3] apic: always update the in-kernel status after loading Paolo Bonzini
2012-10-30 12:16 ` [Qemu-devel] " Paolo Bonzini
2012-10-30 12:38 ` Avi Kivity
2012-10-30 12:38 ` [Qemu-devel] " Avi Kivity
2012-10-30 14:16 ` Paolo Bonzini
2012-10-30 14:16 ` [Qemu-devel] " Paolo Bonzini
2012-10-30 18:21 ` Jan Kiszka
2012-10-30 18:21 ` [Qemu-devel] " Jan Kiszka
2012-11-02 14:53 ` Paolo Bonzini
2012-11-02 14:53 ` [Qemu-devel] " Paolo Bonzini
2012-11-02 14:59 ` Jan Kiszka
2012-11-02 14:59 ` [Qemu-devel] " Jan Kiszka
2012-11-02 15:07 ` Gerd Hoffmann
2012-11-02 15:07 ` [Qemu-devel] " Gerd Hoffmann
2012-11-02 15:13 ` Paolo Bonzini
2012-11-02 15:13 ` [Qemu-devel] " Paolo Bonzini
2012-11-02 15:17 ` Gerd Hoffmann
2012-11-02 15:17 ` [Qemu-devel] " Gerd Hoffmann
2012-11-02 15:21 ` Paolo Bonzini
2012-11-02 15:21 ` [Qemu-devel] " Paolo Bonzini
2012-10-30 18:17 ` Jan Kiszka
2012-10-30 18:17 ` [Qemu-devel] " Jan Kiszka
2012-10-30 12:16 ` [PATCH 4/3] ioapic: change pre_save/post_load methods to get/put Paolo Bonzini
2012-10-30 12:16 ` [Qemu-devel] " Paolo Bonzini
2012-10-30 18:18 ` Jan Kiszka
2012-10-30 18:18 ` [Qemu-devel] " Jan Kiszka
2012-10-30 12:16 ` [PATCH 5/3] ioapic: unify reset callbacks Paolo Bonzini
2012-10-30 12:16 ` [Qemu-devel] " Paolo Bonzini
2012-10-30 16:47 ` [PATCH uq/master 0/3] Fix MSI injection at load time Paolo Bonzini
2012-10-30 16:47 ` [Qemu-devel] " Paolo Bonzini
2012-10-30 18:22 ` Jan Kiszka
2012-10-30 18:22 ` [Qemu-devel] " 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=509019A9.7060803@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.