* [Qemu-devel] [PATCH RFC qom-cpu-next] apic: QOM'ify APICCommonState casts
@ 2013-04-28 11:22 Andreas Färber
2013-04-28 13:03 ` Jan Kiszka
2013-04-29 10:29 ` Igor Mammedov
0 siblings, 2 replies; 6+ messages in thread
From: Andreas Färber @ 2013-04-28 11:22 UTC (permalink / raw)
To: qemu-devel
Cc: ehabkost, gleb, jan.kiszka, anthony, imammedo,
Andreas Färber
Necessary to change the name of ICCDevice's parent object field.
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
Could any of the APIC experts please review whether I'm touching any hot path?
Not sure if this was a mix of pre- and post-QOM code or intentional... Thanks.
hw/i386/kvm/apic.c | 4 ++--
hw/intc/apic.c | 20 +++++++++++---------
hw/intc/apic_common.c | 10 +++++-----
include/hw/cpu/icc_bus.h | 2 +-
4 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 8f80425..0fe31ae 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -27,7 +27,7 @@ static inline uint32_t kvm_apic_get_reg(struct kvm_lapic_state *kapic,
void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
{
- APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+ APICCommonState *s = APIC_COMMON(d);
int i;
memset(kapic, 0, sizeof(*kapic));
@@ -53,7 +53,7 @@ void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
{
- APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+ APICCommonState *s = APIC_COMMON(d);
int i, v;
s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 756dff0..f171620 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -173,7 +173,7 @@ static void apic_local_deliver(APICCommonState *s, int vector)
void apic_deliver_pic_intr(DeviceState *d, int level)
{
- APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+ APICCommonState *s = APIC_COMMON(d);
if (level) {
apic_local_deliver(s, APIC_LVT_LINT0);
@@ -484,7 +484,7 @@ static void apic_startup(APICCommonState *s, int vector_num)
void apic_sipi(DeviceState *d)
{
- APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+ APICCommonState *s = APIC_COMMON(d);
cpu_reset_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
@@ -498,7 +498,7 @@ static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
uint8_t delivery_mode, uint8_t vector_num,
uint8_t trigger_mode)
{
- APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+ APICCommonState *s = APIC_COMMON(d);
uint32_t deliver_bitmask[MAX_APIC_WORDS];
int dest_shorthand = (s->icr[0] >> 18) & 3;
APICCommonState *apic_iter;
@@ -544,16 +544,18 @@ static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
static bool apic_check_pic(APICCommonState *s)
{
- if (!apic_accept_pic_intr(&s->busdev.qdev) || !pic_get_output(isa_pic)) {
+ DeviceState *dev = DEVICE(s);
+
+ if (!apic_accept_pic_intr(dev) || !pic_get_output(isa_pic)) {
return false;
}
- apic_deliver_pic_intr(&s->busdev.qdev, 1);
+ apic_deliver_pic_intr(dev, 1);
return true;
}
int apic_get_interrupt(DeviceState *d)
{
- APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+ APICCommonState *s = APIC_COMMON(d);
int intno;
/* if the APIC is installed or enabled, we let the 8259 handle the
@@ -587,7 +589,7 @@ int apic_get_interrupt(DeviceState *d)
int apic_accept_pic_intr(DeviceState *d)
{
- APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+ APICCommonState *s = APIC_COMMON(d);
uint32_t lvt0;
if (!s)
@@ -666,7 +668,7 @@ static uint32_t apic_mem_readl(void *opaque, hwaddr addr)
if (!d) {
return 0;
}
- s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+ s = APIC_COMMON(d);
index = (addr >> 4) & 0xff;
switch(index) {
@@ -769,7 +771,7 @@ static void apic_mem_writel(void *opaque, hwaddr addr, uint32_t val)
if (!d) {
return;
}
- s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+ s = APIC_COMMON(d);
trace_apic_mem_writel(addr, val);
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index b03e904..0087a14 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -82,7 +82,7 @@ uint8_t cpu_get_apic_tpr(DeviceState *d)
void apic_enable_tpr_access_reporting(DeviceState *d, bool enable)
{
- APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+ APICCommonState *s = APIC_COMMON(d);
APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
apic_report_tpr_access = enable;
@@ -93,7 +93,7 @@ void apic_enable_tpr_access_reporting(DeviceState *d, bool enable)
void apic_enable_vapic(DeviceState *d, hwaddr paddr)
{
- APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+ APICCommonState *s = APIC_COMMON(d);
APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
s->vapic_paddr = paddr;
@@ -103,7 +103,7 @@ void apic_enable_vapic(DeviceState *d, hwaddr paddr)
void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
TPRAccess access)
{
- APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+ APICCommonState *s = APIC_COMMON(d);
vapic_report_tpr_access(s->vapic, CPU(s->cpu), ip, access);
}
@@ -172,7 +172,7 @@ bool apic_next_timer(APICCommonState *s, int64_t current_time)
void apic_init_reset(DeviceState *d)
{
- APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+ APICCommonState *s = APIC_COMMON(d);
int i;
if (!s) {
@@ -215,7 +215,7 @@ void apic_designate_bsp(DeviceState *d)
static void apic_reset_common(DeviceState *d)
{
- APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+ APICCommonState *s = APIC_COMMON(d);
APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
bool bsp;
diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
index b550070..f2c8a50 100644
--- a/include/hw/cpu/icc_bus.h
+++ b/include/hw/cpu/icc_bus.h
@@ -51,7 +51,7 @@ typedef struct ICCBus {
*/
typedef struct ICCDevice {
/*< private >*/
- DeviceState qdev;
+ DeviceState parent_obj;
/*< public >*/
} ICCDevice;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH RFC qom-cpu-next] apic: QOM'ify APICCommonState casts
2013-04-28 11:22 [Qemu-devel] [PATCH RFC qom-cpu-next] apic: QOM'ify APICCommonState casts Andreas Färber
@ 2013-04-28 13:03 ` Jan Kiszka
2013-04-28 13:53 ` Andreas Färber
2013-04-29 10:29 ` Igor Mammedov
1 sibling, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2013-04-28 13:03 UTC (permalink / raw)
To: Andreas Färber; +Cc: imammedo, gleb, qemu-devel, anthony, ehabkost
[-- Attachment #1: Type: text/plain, Size: 6979 bytes --]
On 2013-04-28 13:22, Andreas Färber wrote:
> Necessary to change the name of ICCDevice's parent object field.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
> Could any of the APIC experts please review whether I'm touching any hot path?
> Not sure if this was a mix of pre- and post-QOM code or intentional... Thanks.
How "hot" does it have to be before we notice the type check overhead?
Did anyone already measure this, given that they are getting very common
now?
But none of the touched functions are used during normal operation when
the KVM APIC is active. With emulated APIC, they are frequently used, of
course.
Jan
>
> hw/i386/kvm/apic.c | 4 ++--
> hw/intc/apic.c | 20 +++++++++++---------
> hw/intc/apic_common.c | 10 +++++-----
> include/hw/cpu/icc_bus.h | 2 +-
> 4 files changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
> index 8f80425..0fe31ae 100644
> --- a/hw/i386/kvm/apic.c
> +++ b/hw/i386/kvm/apic.c
> @@ -27,7 +27,7 @@ static inline uint32_t kvm_apic_get_reg(struct kvm_lapic_state *kapic,
>
> void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + APICCommonState *s = APIC_COMMON(d);
> int i;
>
> memset(kapic, 0, sizeof(*kapic));
> @@ -53,7 +53,7 @@ void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
>
> void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + APICCommonState *s = APIC_COMMON(d);
> int i, v;
>
> s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
> diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> index 756dff0..f171620 100644
> --- a/hw/intc/apic.c
> +++ b/hw/intc/apic.c
> @@ -173,7 +173,7 @@ static void apic_local_deliver(APICCommonState *s, int vector)
>
> void apic_deliver_pic_intr(DeviceState *d, int level)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + APICCommonState *s = APIC_COMMON(d);
>
> if (level) {
> apic_local_deliver(s, APIC_LVT_LINT0);
> @@ -484,7 +484,7 @@ static void apic_startup(APICCommonState *s, int vector_num)
>
> void apic_sipi(DeviceState *d)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + APICCommonState *s = APIC_COMMON(d);
>
> cpu_reset_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
>
> @@ -498,7 +498,7 @@ static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
> uint8_t delivery_mode, uint8_t vector_num,
> uint8_t trigger_mode)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + APICCommonState *s = APIC_COMMON(d);
> uint32_t deliver_bitmask[MAX_APIC_WORDS];
> int dest_shorthand = (s->icr[0] >> 18) & 3;
> APICCommonState *apic_iter;
> @@ -544,16 +544,18 @@ static void apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
>
> static bool apic_check_pic(APICCommonState *s)
> {
> - if (!apic_accept_pic_intr(&s->busdev.qdev) || !pic_get_output(isa_pic)) {
> + DeviceState *dev = DEVICE(s);
> +
> + if (!apic_accept_pic_intr(dev) || !pic_get_output(isa_pic)) {
> return false;
> }
> - apic_deliver_pic_intr(&s->busdev.qdev, 1);
> + apic_deliver_pic_intr(dev, 1);
> return true;
> }
>
> int apic_get_interrupt(DeviceState *d)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + APICCommonState *s = APIC_COMMON(d);
> int intno;
>
> /* if the APIC is installed or enabled, we let the 8259 handle the
> @@ -587,7 +589,7 @@ int apic_get_interrupt(DeviceState *d)
>
> int apic_accept_pic_intr(DeviceState *d)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + APICCommonState *s = APIC_COMMON(d);
> uint32_t lvt0;
>
> if (!s)
> @@ -666,7 +668,7 @@ static uint32_t apic_mem_readl(void *opaque, hwaddr addr)
> if (!d) {
> return 0;
> }
> - s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + s = APIC_COMMON(d);
>
> index = (addr >> 4) & 0xff;
> switch(index) {
> @@ -769,7 +771,7 @@ static void apic_mem_writel(void *opaque, hwaddr addr, uint32_t val)
> if (!d) {
> return;
> }
> - s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + s = APIC_COMMON(d);
>
> trace_apic_mem_writel(addr, val);
>
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index b03e904..0087a14 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -82,7 +82,7 @@ uint8_t cpu_get_apic_tpr(DeviceState *d)
>
> void apic_enable_tpr_access_reporting(DeviceState *d, bool enable)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + APICCommonState *s = APIC_COMMON(d);
> APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
>
> apic_report_tpr_access = enable;
> @@ -93,7 +93,7 @@ void apic_enable_tpr_access_reporting(DeviceState *d, bool enable)
>
> void apic_enable_vapic(DeviceState *d, hwaddr paddr)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + APICCommonState *s = APIC_COMMON(d);
> APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
>
> s->vapic_paddr = paddr;
> @@ -103,7 +103,7 @@ void apic_enable_vapic(DeviceState *d, hwaddr paddr)
> void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
> TPRAccess access)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + APICCommonState *s = APIC_COMMON(d);
>
> vapic_report_tpr_access(s->vapic, CPU(s->cpu), ip, access);
> }
> @@ -172,7 +172,7 @@ bool apic_next_timer(APICCommonState *s, int64_t current_time)
>
> void apic_init_reset(DeviceState *d)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + APICCommonState *s = APIC_COMMON(d);
> int i;
>
> if (!s) {
> @@ -215,7 +215,7 @@ void apic_designate_bsp(DeviceState *d)
>
> static void apic_reset_common(DeviceState *d)
> {
> - APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> + APICCommonState *s = APIC_COMMON(d);
> APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
> bool bsp;
>
> diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
> index b550070..f2c8a50 100644
> --- a/include/hw/cpu/icc_bus.h
> +++ b/include/hw/cpu/icc_bus.h
> @@ -51,7 +51,7 @@ typedef struct ICCBus {
> */
> typedef struct ICCDevice {
> /*< private >*/
> - DeviceState qdev;
> + DeviceState parent_obj;
> /*< public >*/
> } ICCDevice;
>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH RFC qom-cpu-next] apic: QOM'ify APICCommonState casts
2013-04-28 13:03 ` Jan Kiszka
@ 2013-04-28 13:53 ` Andreas Färber
2013-04-28 14:13 ` Jan Kiszka
0 siblings, 1 reply; 6+ messages in thread
From: Andreas Färber @ 2013-04-28 13:53 UTC (permalink / raw)
To: Jan Kiszka; +Cc: imammedo, gleb, qemu-devel, anthony, ehabkost
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Am 28.04.2013 15:03, schrieb Jan Kiszka:
> On 2013-04-28 13:22, Andreas Färber wrote:
>> Necessary to change the name of ICCDevice's parent object field.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de> --- Could any of
>> the APIC experts please review whether I'm touching any hot
>> path? Not sure if this was a mix of pre- and post-QOM code or
>> intentional... Thanks.
>
> How "hot" does it have to be before we notice the type check
> overhead? Did anyone already measure this, given that they are
> getting very common now?
Heh, if I had a conclusive benchmark I wouldn't ask. ;)
In general init, reset and MMIO were considered cold paths in this
regard. Timer and interrupt code paths by contrast I've usually tried
to spare.
> But none of the touched functions are used during normal operation
> when the KVM APIC is active. With emulated APIC, they are
> frequently used, of course.
Where in doubt, we could just use (APICCommonState *) or a macro
hiding that.
Andreas
>> hw/i386/kvm/apic.c | 4 ++-- hw/intc/apic.c | 20
>> +++++++++++--------- hw/intc/apic_common.c | 10 +++++-----
>> include/hw/cpu/icc_bus.h | 2 +- 4 files changed, 19
>> insertions(+), 17 deletions(-)
>>
>> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c index
>> 8f80425..0fe31ae 100644 --- a/hw/i386/kvm/apic.c +++
>> b/hw/i386/kvm/apic.c @@ -27,7 +27,7 @@ static inline uint32_t
>> kvm_apic_get_reg(struct kvm_lapic_state *kapic,
>>
>> void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state
>> *kapic) { - APICCommonState *s = DO_UPCAST(APICCommonState,
>> busdev.qdev, d); + APICCommonState *s = APIC_COMMON(d); int
>> i;
>>
>> memset(kapic, 0, sizeof(*kapic)); @@ -53,7 +53,7 @@ void
>> kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state
>> *kapic)
>>
>> void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state
>> *kapic) { - APICCommonState *s = DO_UPCAST(APICCommonState,
>> busdev.qdev, d); + APICCommonState *s = APIC_COMMON(d); int i,
>> v;
>>
>> s->id = kvm_apic_get_reg(kapic, 0x2) >> 24; diff --git
>> a/hw/intc/apic.c b/hw/intc/apic.c index 756dff0..f171620 100644
>> --- a/hw/intc/apic.c +++ b/hw/intc/apic.c @@ -173,7 +173,7 @@
>> static void apic_local_deliver(APICCommonState *s, int vector)
>>
>> void apic_deliver_pic_intr(DeviceState *d, int level) { -
>> APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
>> + APICCommonState *s = APIC_COMMON(d);
>>
>> if (level) { apic_local_deliver(s, APIC_LVT_LINT0); @@ -484,7
>> +484,7 @@ static void apic_startup(APICCommonState *s, int
>> vector_num)
>>
>> void apic_sipi(DeviceState *d) { - APICCommonState *s =
>> DO_UPCAST(APICCommonState, busdev.qdev, d); + APICCommonState
>> *s = APIC_COMMON(d);
>>
>> cpu_reset_interrupt(CPU(s->cpu), CPU_INTERRUPT_SIPI);
>>
>> @@ -498,7 +498,7 @@ static void apic_deliver(DeviceState *d,
>> uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode, uint8_t
>> vector_num, uint8_t trigger_mode) { - APICCommonState *s =
>> DO_UPCAST(APICCommonState, busdev.qdev, d); + APICCommonState
>> *s = APIC_COMMON(d); uint32_t deliver_bitmask[MAX_APIC_WORDS];
>> int dest_shorthand = (s->icr[0] >> 18) & 3; APICCommonState
>> *apic_iter; @@ -544,16 +544,18 @@ static void
>> apic_deliver(DeviceState *d, uint8_t dest, uint8_t dest_mode,
>>
>> static bool apic_check_pic(APICCommonState *s) { - if
>> (!apic_accept_pic_intr(&s->busdev.qdev) ||
>> !pic_get_output(isa_pic)) { + DeviceState *dev = DEVICE(s); +
>> + if (!apic_accept_pic_intr(dev) || !pic_get_output(isa_pic))
>> { return false; } - apic_deliver_pic_intr(&s->busdev.qdev,
>> 1); + apic_deliver_pic_intr(dev, 1); return true; }
>>
>> int apic_get_interrupt(DeviceState *d) { - APICCommonState *s
>> = DO_UPCAST(APICCommonState, busdev.qdev, d); +
>> APICCommonState *s = APIC_COMMON(d); int intno;
>>
>> /* if the APIC is installed or enabled, we let the 8259 handle
>> the @@ -587,7 +589,7 @@ int apic_get_interrupt(DeviceState *d)
>>
>> int apic_accept_pic_intr(DeviceState *d) { - APICCommonState
>> *s = DO_UPCAST(APICCommonState, busdev.qdev, d); +
>> APICCommonState *s = APIC_COMMON(d); uint32_t lvt0;
>>
>> if (!s) @@ -666,7 +668,7 @@ static uint32_t apic_mem_readl(void
>> *opaque, hwaddr addr) if (!d) { return 0; } - s =
>> DO_UPCAST(APICCommonState, busdev.qdev, d); + s =
>> APIC_COMMON(d);
>>
>> index = (addr >> 4) & 0xff; switch(index) { @@ -769,7 +771,7 @@
>> static void apic_mem_writel(void *opaque, hwaddr addr, uint32_t
>> val) if (!d) { return; } - s = DO_UPCAST(APICCommonState,
>> busdev.qdev, d); + s = APIC_COMMON(d);
>>
>> trace_apic_mem_writel(addr, val);
>>
>> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c index
>> b03e904..0087a14 100644 --- a/hw/intc/apic_common.c +++
>> b/hw/intc/apic_common.c @@ -82,7 +82,7 @@ uint8_t
>> cpu_get_apic_tpr(DeviceState *d)
>>
>> void apic_enable_tpr_access_reporting(DeviceState *d, bool
>> enable) { - APICCommonState *s = DO_UPCAST(APICCommonState,
>> busdev.qdev, d); + APICCommonState *s = APIC_COMMON(d);
>> APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
>>
>> apic_report_tpr_access = enable; @@ -93,7 +93,7 @@ void
>> apic_enable_tpr_access_reporting(DeviceState *d, bool enable)
>>
>> void apic_enable_vapic(DeviceState *d, hwaddr paddr) { -
>> APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
>> + APICCommonState *s = APIC_COMMON(d); APICCommonClass *info =
>> APIC_COMMON_GET_CLASS(s);
>>
>> s->vapic_paddr = paddr; @@ -103,7 +103,7 @@ void
>> apic_enable_vapic(DeviceState *d, hwaddr paddr) void
>> apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
>> TPRAccess access) { - APICCommonState *s =
>> DO_UPCAST(APICCommonState, busdev.qdev, d); + APICCommonState
>> *s = APIC_COMMON(d);
>>
>> vapic_report_tpr_access(s->vapic, CPU(s->cpu), ip, access); } @@
>> -172,7 +172,7 @@ bool apic_next_timer(APICCommonState *s, int64_t
>> current_time)
>>
>> void apic_init_reset(DeviceState *d) { - APICCommonState *s =
>> DO_UPCAST(APICCommonState, busdev.qdev, d); + APICCommonState
>> *s = APIC_COMMON(d); int i;
>>
>> if (!s) { @@ -215,7 +215,7 @@ void apic_designate_bsp(DeviceState
>> *d)
>>
>> static void apic_reset_common(DeviceState *d) { -
>> APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
>> + APICCommonState *s = APIC_COMMON(d); APICCommonClass *info =
>> APIC_COMMON_GET_CLASS(s); bool bsp;
>>
>> diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
>> index b550070..f2c8a50 100644 --- a/include/hw/cpu/icc_bus.h +++
>> b/include/hw/cpu/icc_bus.h @@ -51,7 +51,7 @@ typedef struct
>> ICCBus { */ typedef struct ICCDevice { /*< private >*/ -
>> DeviceState qdev; + DeviceState parent_obj; /*< public >*/ }
>> ICCDevice;
>>
>>
>
>
- --
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
iQIcBAEBAgAGBQJRfSnoAAoJEPou0S0+fgE/NcgP/iJ99zuWVy+OG13Zr+gpGUu7
gL+GjPjKAo1QYTGBEtpB1g8veyXaVbDWCHzkGsitQRtIRqCJnwQcORcWes395J9N
ffy/fWUSwdzOdv4DP+hPoy8RaFdxKWr4JqUsoSLERM7vefQ86xieWvE7RW6Xcy+r
SDE4Qm67WWyGN5c875deF1/9LlT08vv0KCXzaqMLdm5y17c5td7h82JNNlYfYF9S
QeyPYOtWvEFR4uUaLqgoMbWpMjvobjt5G/tbSss1wfds9fiBM4nU+Blk51eF2N0v
VeNCO5NyNnQyiNR7znwYYnljNvg7Web9ITenCQGPj0eUDdAyb1kBwqCDZo5giqp4
ifvTVi4LAq0vCmBArKDeHpi0xZXPXDLm2QREdO80Qwnt9y2fieV3mMMTXl1X6vSp
R0dmGSbomgEoigQMXSHOdiz4M6CHphpUicG8/05op9Sjf5DVkfaDt7isvvzKifNL
K+vFogG3DLRyL6iw10H2B9A7e4mY/b6VCw+XvzhZa5iiZ7qa/ZVEt84uJQJNXxnv
Wd0rR5nlCdUJBq8IbDyJm80P+bnzgKU+Wfa4wOzxeDEWLJj4bRPdZd+I9mbFEgkf
rtseRM+vjooQhyTE6l/uphX94kzoOAVtwJefo9GXhhduZ26fwVepVsqH4q1Bie+G
xu50SmEmd2N4aw9VaRF9
=qCIU
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH RFC qom-cpu-next] apic: QOM'ify APICCommonState casts
2013-04-28 13:53 ` Andreas Färber
@ 2013-04-28 14:13 ` Jan Kiszka
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kiszka @ 2013-04-28 14:13 UTC (permalink / raw)
To: Andreas Färber; +Cc: imammedo, gleb, qemu-devel, anthony, ehabkost
[-- Attachment #1: Type: text/plain, Size: 1628 bytes --]
On 2013-04-28 15:53, Andreas Färber wrote:
> Am 28.04.2013 15:03, schrieb Jan Kiszka:
>> On 2013-04-28 13:22, Andreas Färber wrote:
>>> Necessary to change the name of ICCDevice's parent object field.
>>>
>>> Signed-off-by: Andreas Färber <afaerber@suse.de> --- Could any of
>>> the APIC experts please review whether I'm touching any hot
>>> path? Not sure if this was a mix of pre- and post-QOM code or
>>> intentional... Thanks.
>
>> How "hot" does it have to be before we notice the type check
>> overhead? Did anyone already measure this, given that they are
>> getting very common now?
>
> Heh, if I had a conclusive benchmark I wouldn't ask. ;)
Do object_class_dynamic_cast & friends show up higher in profiling
results when using your patch with an emulated APIC?
>
> In general init, reset and MMIO were considered cold paths in this
> regard. Timer and interrupt code paths by contrast I've usually tried
> to spare.
...but not in this patch. There are things like "deliver",
"get_interupt", "accept", and the APIC MMIO handlers are stressed at
least once per IRQ as well (EOI). When emulating in userspace.
>
>> But none of the touched functions are used during normal operation
>> when the KVM APIC is active. With emulated APIC, they are
>> frequently used, of course.
>
> Where in doubt, we could just use (APICCommonState *) or a macro
> hiding that.
Actually, I would prefer making the typical type check (class up- and
down-casts) so light-weight that we do not have to worry. Something that
ideally boils down to comparing two magic numbers inline.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH RFC qom-cpu-next] apic: QOM'ify APICCommonState casts
2013-04-28 11:22 [Qemu-devel] [PATCH RFC qom-cpu-next] apic: QOM'ify APICCommonState casts Andreas Färber
2013-04-28 13:03 ` Jan Kiszka
@ 2013-04-29 10:29 ` Igor Mammedov
2013-04-29 11:30 ` Andreas Färber
1 sibling, 1 reply; 6+ messages in thread
From: Igor Mammedov @ 2013-04-29 10:29 UTC (permalink / raw)
To: Andreas Färber; +Cc: anthony, jan.kiszka, qemu-devel, gleb, ehabkost
On Sun, 28 Apr 2013 13:22:10 +0200
Andreas Färber <afaerber@suse.de> wrote:
> Necessary to change the name of ICCDevice's parent object field.
>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
> Could any of the APIC experts please review whether I'm touching any hot path?
> Not sure if this was a mix of pre- and post-QOM code or intentional... Thanks.
>
> hw/i386/kvm/apic.c | 4 ++--
> hw/intc/apic.c | 20 +++++++++++---------
> hw/intc/apic_common.c | 10 +++++-----
> include/hw/cpu/icc_bus.h | 2 +-
> 4 files changed, 19 insertions(+), 17 deletions(-)
>
[...]
>
> diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
> index b550070..f2c8a50 100644
> --- a/include/hw/cpu/icc_bus.h
> +++ b/include/hw/cpu/icc_bus.h
> @@ -51,7 +51,7 @@ typedef struct ICCBus {
> */
> typedef struct ICCDevice {
> /*< private >*/
> - DeviceState qdev;
> + DeviceState parent_obj;
This hunk doesn't apply to qpm-cpu-next.
It looks like it's accidentally crept in
https://github.com/afaerber/qemu-cpu/commit/a7ae7cd073cc2b20de3372edc84ec5a01e88e55d
> /*< public >*/
> } ICCDevice;
>
> --
> 1.8.1.4
>
>
--
Regards,
Igor
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH RFC qom-cpu-next] apic: QOM'ify APICCommonState casts
2013-04-29 10:29 ` Igor Mammedov
@ 2013-04-29 11:30 ` Andreas Färber
0 siblings, 0 replies; 6+ messages in thread
From: Andreas Färber @ 2013-04-29 11:30 UTC (permalink / raw)
To: Igor Mammedov; +Cc: anthony, jan.kiszka, qemu-devel, gleb, ehabkost
Am 29.04.2013 12:29, schrieb Igor Mammedov:
> On Sun, 28 Apr 2013 13:22:10 +0200
> Andreas Färber <afaerber@suse.de> wrote:
>
>> Necessary to change the name of ICCDevice's parent object field.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>> Could any of the APIC experts please review whether I'm touching any hot path?
>> Not sure if this was a mix of pre- and post-QOM code or intentional... Thanks.
>>
>> hw/i386/kvm/apic.c | 4 ++--
>> hw/intc/apic.c | 20 +++++++++++---------
>> hw/intc/apic_common.c | 10 +++++-----
>> include/hw/cpu/icc_bus.h | 2 +-
>> 4 files changed, 19 insertions(+), 17 deletions(-)
>>
> [...]
>>
>> diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h
>> index b550070..f2c8a50 100644
>> --- a/include/hw/cpu/icc_bus.h
>> +++ b/include/hw/cpu/icc_bus.h
>> @@ -51,7 +51,7 @@ typedef struct ICCBus {
>> */
>> typedef struct ICCDevice {
>> /*< private >*/
>> - DeviceState qdev;
>> + DeviceState parent_obj;
> This hunk doesn't apply to qpm-cpu-next.
> It looks like it's accidentally crept in
> https://github.com/afaerber/qemu-cpu/commit/a7ae7cd073cc2b20de3372edc84ec5a01e88e55d
Yeah, sorry, forgot to push latest qom-cpu-next (but today that includes
preliminary APIC QOM'ification not yet ready for qom-cpu).
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-29 11:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-28 11:22 [Qemu-devel] [PATCH RFC qom-cpu-next] apic: QOM'ify APICCommonState casts Andreas Färber
2013-04-28 13:03 ` Jan Kiszka
2013-04-28 13:53 ` Andreas Färber
2013-04-28 14:13 ` Jan Kiszka
2013-04-29 10:29 ` Igor Mammedov
2013-04-29 11:30 ` Andreas Färber
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).