x86/HPET: fix FSB interrupt masking

HPET_TN_FSB is not really suitable for masking interrupts - it merely
switches between the two delivery methods. The right way of masking is
through the HPET_TN_ENABLE bit (which really is an interrupt enable,
not a counter enable or some such). This is even more so with certain
chip sets not even allowing HPET_TN_FSB to be cleared on some of the
channels.

Further, all the setup of the channel should happen before actually
enabling the interrupt, which requires splitting legacy and FSB logic.

Finally this also fixes an S3 resume problem (HPET_TN_FSB did not get
set in hpet_broadcast_resume(), and hpet_msi_unmask() doesn't get
called from the general resume code either afaict).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>

1 files changed, 19 insertions, 21 deletions

diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
index 49f85bcc75..fb875523f0 100644
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -237,7 +237,7 @@ static void hpet_msi_unmask(struct irq_desc *desc)
     struct hpet_event_channel *ch = desc->action->dev_id;
 
     cfg = hpet_read32(HPET_Tn_CFG(ch->idx));
-    cfg |= HPET_TN_FSB;
+    cfg |= HPET_TN_ENABLE;
     hpet_write32(cfg, HPET_Tn_CFG(ch->idx));
     ch->msi.msi_attrib.masked = 0;
 }
@@ -248,7 +248,7 @@ static void hpet_msi_mask(struct irq_desc *desc)
     struct hpet_event_channel *ch = desc->action->dev_id;
 
     cfg = hpet_read32(HPET_Tn_CFG(ch->idx));
-    cfg &= ~HPET_TN_FSB;
+    cfg &= ~HPET_TN_ENABLE;
     hpet_write32(cfg, HPET_Tn_CFG(ch->idx));
     ch->msi.msi_attrib.masked = 1;
 }
@@ -328,6 +328,7 @@ static void __hpet_setup_msi_irq(struct irq_desc *desc)
 static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
 {
     int ret;
+    u32 cfg = hpet_read32(HPET_Tn_CFG(ch->idx));
     irq_desc_t *desc = irq_to_desc(ch->msi.irq);
 
     if ( iommu_intremap )
@@ -338,6 +339,11 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
             return ret;
     }
 
+    /* set HPET Tn as oneshot */
+    cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC);
+    cfg |= HPET_TN_FSB | HPET_TN_32BIT;
+    hpet_write32(cfg, HPET_Tn_CFG(ch->idx));
+
     desc->handler = &hpet_msi_type;
     ret = request_irq(ch->msi.irq, hpet_interrupt_handler, 0, "HPET", ch);
     if ( ret < 0 )
@@ -393,17 +399,6 @@ static void __init hpet_fsb_cap_lookup(void)
         if ( !(cfg & HPET_TN_FSB_CAP) )
             continue;
 
-        /* hpet_setup(), via hpet_resume(), attempted to clear HPET_TN_FSB, so
-         * if it is still set... */
-        if ( !(cfg & HPET_TN_FSB) )
-            ch->msi.msi_attrib.maskbit = 1;
-        else
-        {
-            ch->msi.msi_attrib.maskbit = 0;
-            printk(XENLOG_WARNING "HPET: channel %u is not maskable (%04x)\n",
-                   i, cfg);
-        }
-
         if ( !zalloc_cpumask_var(&ch->cpumask) )
         {
             if ( !num_hpets_used )
@@ -570,11 +565,14 @@ void __init hpet_broadcast_init(void)
 
     for ( i = 0; i < n; i++ )
     {
-        /* set HPET Tn as oneshot */
-        cfg = hpet_read32(HPET_Tn_CFG(hpet_events[i].idx));
-        cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC);
-        cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
-        hpet_write32(cfg, HPET_Tn_CFG(hpet_events[i].idx));
+        if ( i == 0 && (cfg & HPET_CFG_LEGACY) )
+        {
+            /* set HPET T0 as oneshot */
+            cfg = hpet_read32(HPET_Tn_CFG(0));
+            cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC);
+            cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
+            hpet_write32(cfg, HPET_Tn_CFG(0));
+        }
 
         /*
          * The period is a femto seconds value. We need to calculate the scaled
@@ -588,6 +586,7 @@ void __init hpet_broadcast_init(void)
         wmb();
         hpet_events[i].event_handler = handle_hpet_broadcast;
 
+        hpet_events[i].msi.msi_attrib.maskbit = 1;
         hpet_events[i].msi.msi_attrib.pos = MSI_TYPE_HPET;
     }
 
@@ -633,6 +632,8 @@ void hpet_broadcast_resume(void)
         cfg = hpet_read32(HPET_Tn_CFG(hpet_events[i].idx));
         cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC);
         cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
+        if ( !(hpet_events[i].flags & HPET_EVT_LEGACY) )
+            cfg |= HPET_TN_FSB;
         hpet_write32(cfg, HPET_Tn_CFG(hpet_events[i].idx));
 
         hpet_events[i].next_event = STIME_MAX;
@@ -811,10 +812,7 @@ void hpet_resume(u32 *boot_cfg)
     {
         cfg = hpet_read32(HPET_Tn_CFG(i));
         if ( boot_cfg )
-        {
             boot_cfg[i + 1] = cfg;
-            cfg &= ~HPET_TN_FSB;
-        }
         cfg &= ~HPET_TN_ENABLE;
         if ( cfg & HPET_TN_RESERVED )
         {