| Message ID | 1304000700-3640-2-git-send-email-uobergfe@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Apr 28, 2011 at 5:24 PM, Ulrich Obergfell <uobergfe@redhat.com> wrote: > 'target_get_irq_delivered' and 'target_reset_irq_delivered' contain > entry addresses of functions that are utilized by update_irq() to > detect coalesced interrupts. apic code loads these pointers during > initialization. I'm not so happy with this approach, but probably then the i386 dependencies can be removed from RTC and it can be compiled only once for all targets. > This change can be replaced if a generic feedback infrastructure to > track coalesced IRQs for periodic, clock providing devices becomes > available. > > Signed-off-by: Ulrich Obergfell <uobergfe@redhat.com> > --- > hw/apic.c | 4 ++++ > sysemu.h | 3 +++ > vl.c | 3 +++ > 3 files changed, 10 insertions(+), 0 deletions(-) > > diff --git a/hw/apic.c b/hw/apic.c > index a45b57f..eb0f6d9 100644 > --- a/hw/apic.c > +++ b/hw/apic.c > @@ -24,6 +24,7 @@ > #include "sysbus.h" > #include "trace.h" > #include "kvm.h" > +#include "sysemu.h" > > /* APIC Local Vector Table */ > #define APIC_LVT_TIMER 0 > @@ -1143,6 +1144,9 @@ static SysBusDeviceInfo apic_info = { > > static void apic_register_devices(void) > { > + target_get_irq_delivered = apic_get_irq_delivered; > + target_reset_irq_delivered = apic_reset_irq_delivered; > + > sysbus_register_withprop(&apic_info); > } > > diff --git a/sysemu.h b/sysemu.h > index 07d85cd..75b0139 100644 > --- a/sysemu.h > +++ b/sysemu.h > @@ -98,6 +98,9 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f); > void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f); > int qemu_loadvm_state(QEMUFile *f); > > +extern int (*target_get_irq_delivered)(void); > +extern void (*target_reset_irq_delivered)(void); > + > /* SLIRP */ > void do_info_slirp(Monitor *mon); > > diff --git a/vl.c b/vl.c > index 0c24e07..7bab315 100644 > --- a/vl.c > +++ b/vl.c > @@ -233,6 +233,9 @@ const char *prom_envs[MAX_PROM_ENVS]; > const char *nvram = NULL; > int boot_menu; > > +int (*target_get_irq_delivered)(void) = 0; > +void (*target_reset_irq_delivered)(void) = 0; Instead of initializing with 0 (should be actually NULL in C), please define stub functions, which are used by default. Then the users don't need to check for NULL pointers.
On 2011-04-28 20:51, Blue Swirl wrote: > On Thu, Apr 28, 2011 at 5:24 PM, Ulrich Obergfell <uobergfe@redhat.com> wrote: >> 'target_get_irq_delivered' and 'target_reset_irq_delivered' contain >> entry addresses of functions that are utilized by update_irq() to >> detect coalesced interrupts. apic code loads these pointers during >> initialization. > > I'm not so happy with this approach, but probably then the i386 > dependencies can be removed from RTC and it can be compiled only once > for all targets. This whole series is really the minimalistic approach. The callbacks defined here must remain a temporary "shortcut". Just like proper abstraction of periodic tick compensation for reuse in other timers has to be added later on. And the limitation to edge-triggered legacy HPET INTs has to be removed. Jan
> On 2011-04-28 20:51, Blue Swirl wrote: >> On Thu, Apr 28, 2011 at 5:24 PM, Ulrich Obergfell wrote: >>> 'target_get_irq_delivered' and 'target_reset_irq_delivered' contain >>> entry addresses of functions that are utilized by update_irq() to >>> detect coalesced interrupts. apic code loads these pointers during >>> initialization. >> >> I'm not so happy with this approach, but probably then the i386 >> dependencies can be removed from RTC and it can be compiled only once >> for all targets. > > This whole series is really the minimalistic approach. The callbacks > defined here must remain a temporary "shortcut". Just like proper > abstraction of periodic tick compensation for reuse in other timers has > to be added later on. And the limitation to edge-triggered legacy HPET > INTs has to be removed. Since QEMU doesn't have a generic infrastructure to track interrupt delivery, I decided to reuse something that is currently available. The only mechanism that I'm aware of is the one that is utilized by RTC code ('rtc_td_hack'), i.e. apic_get_irq_delivered() etc. The changes that would be introduced by part 1/5 and part 4/5 of this patch series could be replaced if a generic infrastructure to track interrupt delivery becomes available. Regards, Uli
On 2011-04-29 11:45, Ulrich Obergfell wrote: > >> On 2011-04-28 20:51, Blue Swirl wrote: >>> On Thu, Apr 28, 2011 at 5:24 PM, Ulrich Obergfell wrote: >>>> 'target_get_irq_delivered' and 'target_reset_irq_delivered' contain >>>> entry addresses of functions that are utilized by update_irq() to >>>> detect coalesced interrupts. apic code loads these pointers during >>>> initialization. >>> >>> I'm not so happy with this approach, but probably then the i386 >>> dependencies can be removed from RTC and it can be compiled only once >>> for all targets. >> >> This whole series is really the minimalistic approach. The callbacks >> defined here must remain a temporary "shortcut". Just like proper >> abstraction of periodic tick compensation for reuse in other timers has >> to be added later on. And the limitation to edge-triggered legacy HPET >> INTs has to be removed. > > Since QEMU doesn't have a generic infrastructure to track interrupt > delivery, I decided to reuse something that is currently available. > The only mechanism that I'm aware of is the one that is utilized by > RTC code ('rtc_td_hack'), i.e. apic_get_irq_delivered() etc. > > The changes that would be introduced by part 1/5 and part 4/5 of > this patch series could be replaced if a generic infrastructure > to track interrupt delivery becomes available. Right, and I hope you have the resources to continue working on this topic in the foreseeable future. Jan
On Fri, Apr 29, 2011 at 12:45 PM, Ulrich Obergfell <uobergfe@redhat.com> wrote: > >> On 2011-04-28 20:51, Blue Swirl wrote: >>> On Thu, Apr 28, 2011 at 5:24 PM, Ulrich Obergfell wrote: >>>> 'target_get_irq_delivered' and 'target_reset_irq_delivered' contain >>>> entry addresses of functions that are utilized by update_irq() to >>>> detect coalesced interrupts. apic code loads these pointers during >>>> initialization. >>> >>> I'm not so happy with this approach, but probably then the i386 >>> dependencies can be removed from RTC and it can be compiled only once >>> for all targets. >> >> This whole series is really the minimalistic approach. The callbacks >> defined here must remain a temporary "shortcut". Just like proper >> abstraction of periodic tick compensation for reuse in other timers has >> to be added later on. And the limitation to edge-triggered legacy HPET >> INTs has to be removed. > > Since QEMU doesn't have a generic infrastructure to track interrupt > delivery, I decided to reuse something that is currently available. > The only mechanism that I'm aware of is the one that is utilized by > RTC code ('rtc_td_hack'), i.e. apic_get_irq_delivered() etc. I think your approach is slightly better (while not the best possible one), so it should be used also for RTC. Some more comments: - instead of global variables, there could be a setter function (maybe inline) - pc.h would be a better place instead of sysemu.h > The changes that would be introduced by part 1/5 and part 4/5 of > this patch series could be replaced if a generic infrastructure > to track interrupt delivery becomes available. OK.
diff --git a/hw/apic.c b/hw/apic.c index a45b57f..eb0f6d9 100644 --- a/hw/apic.c +++ b/hw/apic.c @@ -24,6 +24,7 @@ #include "sysbus.h" #include "trace.h" #include "kvm.h" +#include "sysemu.h" /* APIC Local Vector Table */ #define APIC_LVT_TIMER 0 @@ -1143,6 +1144,9 @@ static SysBusDeviceInfo apic_info = { static void apic_register_devices(void) { + target_get_irq_delivered = apic_get_irq_delivered; + target_reset_irq_delivered = apic_reset_irq_delivered; + sysbus_register_withprop(&apic_info); } diff --git a/sysemu.h b/sysemu.h index 07d85cd..75b0139 100644 --- a/sysemu.h +++ b/sysemu.h @@ -98,6 +98,9 @@ int qemu_savevm_state_complete(Monitor *mon, QEMUFile *f); void qemu_savevm_state_cancel(Monitor *mon, QEMUFile *f); int qemu_loadvm_state(QEMUFile *f); +extern int (*target_get_irq_delivered)(void); +extern void (*target_reset_irq_delivered)(void); + /* SLIRP */ void do_info_slirp(Monitor *mon); diff --git a/vl.c b/vl.c index 0c24e07..7bab315 100644 --- a/vl.c +++ b/vl.c @@ -233,6 +233,9 @@ const char *prom_envs[MAX_PROM_ENVS]; const char *nvram = NULL; int boot_menu; +int (*target_get_irq_delivered)(void) = 0; +void (*target_reset_irq_delivered)(void) = 0; + typedef struct FWBootEntry FWBootEntry; struct FWBootEntry {
'target_get_irq_delivered' and 'target_reset_irq_delivered' contain entry addresses of functions that are utilized by update_irq() to detect coalesced interrupts. apic code loads these pointers during initialization. This change can be replaced if a generic feedback infrastructure to track coalesced IRQs for periodic, clock providing devices becomes available. Signed-off-by: Ulrich Obergfell <uobergfe@redhat.com> --- hw/apic.c | 4 ++++ sysemu.h | 3 +++ vl.c | 3 +++ 3 files changed, 10 insertions(+), 0 deletions(-)