Keeping channels enabled when they're unused is only causing problems:
Extra interrupts harm performance, and extra nested interrupts could even
have caused worse problems. However, on all Intel hardware I looked at
closely, a 0->1 transition of the enable bit causes an immediate IRQ.
Hence disabling channels isn't a good idea there. Set a "long" timeout
instead.
Along with that also "clear" the channel's "next event", for it to be
properly written by whatever the next user is going to want (possibly
avoiding too early an IRQ).
Further, along the same lines, don't enable channels early when starting
up an IRQ. This doesn't need to happen earlier than from
set_channel_irq_affinity() (once a channel goes into use the very first
time). This eliminates a single instance of
(XEN) [VT-D]INTR-REMAP: Request device [0000:00:1f.0] fault index 0
(XEN) [VT-D]INTR-REMAP: reason 25 - Blocked a compatibility format interrupt request
during boot. (Why exactly there's only one instance, when we use multiple
counters and hence multiple IRQs, I can't tell. My understanding would be
that this was due to __hpet_setup_msi_irq() being called only after
request_irq() [and hence the .startup handler], yet that should have
affected all channels.)
Fixes: 3ba523ff957c ("CPUIDLE: enable MSI capable HPET for timer broadcast")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Release-Acked-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>
---
A window still remains for IRQs to be caused by stale comparator values:
hpet_attach_channel() is called ahead of reprogram_hpet_evt_channel().
Should we also write the comparator to "far into the future"?
Furthermore this prolongues the window until "old" vectors may be released
again, as this way we potentially (and intentionally) delay the ocurrence
of the next IRQ for the channel in question. (This issue will disappear
once we switch to a fixed, global vector.)
---
v3: Don't disable channels, only set long timeouts.
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -295,12 +295,6 @@ static int hpet_msi_write(struct hpet_ev
return 0;
}
-static unsigned int cf_check hpet_msi_startup(struct irq_desc *desc)
-{
- hpet_msi_unmask(desc);
- return 0;
-}
-
#define hpet_msi_shutdown hpet_msi_mask
static void cf_check hpet_msi_set_affinity(
@@ -326,7 +320,7 @@ static void cf_check hpet_msi_set_affini
*/
static hw_irq_controller hpet_msi_type = {
.typename = "HPET-MSI",
- .startup = hpet_msi_startup,
+ .startup = irq_startup_none,
.shutdown = hpet_msi_shutdown,
.enable = hpet_msi_unmask,
.disable = hpet_msi_mask,
@@ -526,6 +520,8 @@ static void hpet_detach_channel(unsigned
spin_unlock_irq(&ch->lock);
else if ( (next = cpumask_first(ch->cpumask)) >= nr_cpu_ids )
{
+ hpet_write32(hpet_read32(HPET_COUNTER), HPET_Tn_CMP(ch->idx));
+ ch->next_event = STIME_MAX;
ch->cpu = -1;
clear_bit(HPET_EVT_USED_BIT, &ch->flags);
spin_unlock_irq(&ch->lock);