thr3ads.net - Xen devel - [PATCH] x86/HPET: don''t disable interrupt delivery right after setting it up [Sep 2012]

If this information is useful, please help other people find it:
Share via:

Jan Beulich

2012-Sep-27 14:40 UTC

[PATCH] x86/HPET: don''t disable interrupt delivery right after setting it up

We shouldn''t clear HPET_TN_FSB right after we (indirectly, via
request_irq()) enabled it for the channels we intend to use for
broadcasts.

This fixes a regression introduced by c/s 25103:0b0e42dc4f0a.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
This fixes the MWAIT idle driver problem on the box that I was able to
(artificially) reproduce it with. Hence I subsequently intend to revert
25960:6bf8b882df8f, but I''m not intending to do this before getting a
push there.

I''m surprised that this didn''t hit anyone so far with the
ACPI-based
idle driver...

--- 2012-09-21.orig/xen/arch/x86/hpet.c	2012-09-27 16:10:35.000000000 +0200
+++ 2012-09-21/xen/arch/x86/hpet.c	2012-09-27 16:10:07.000000000 +0200
@@ -533,7 +533,7 @@ void __init hpet_broadcast_init(void)
     {
         /* set HPET Tn as oneshot */
         cfg = hpet_read32(HPET_Tn_CFG(hpet_events[i].idx));
-        cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC | HPET_TN_FSB);
+        cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC);
         cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
         hpet_write32(cfg, HPET_Tn_CFG(hpet_events[i].idx));
 
@@ -590,7 +590,7 @@ void hpet_broadcast_resume(void)
 
         /* set HPET Tn as oneshot */
         cfg = hpet_read32(HPET_Tn_CFG(hpet_events[i].idx));
-        cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC | HPET_TN_FSB);
+        cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC);
         cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
         hpet_write32(cfg, HPET_Tn_CFG(hpet_events[i].idx));
 





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Malcolm Crossley

2012-Sep-27 14:49 UTC

head link

Re: [PATCH] x86/HPET: don''t disable interrupt delivery right after setting it up

On 27/09/12 15:40, Jan Beulich wrote:> We shouldn''t clear HPET_TN_FSB right after we (indirectly, via
> request_irq()) enabled it for the channels we intend to use for
> broadcasts.
>
> This fixes a regression introduced by c/s 25103:0b0e42dc4f0a.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> This fixes the MWAIT idle driver problem on the box that I was able to
> (artificially) reproduce it with. Hence I subsequently intend to revert
> 25960:6bf8b882df8f, but I''m not intending to do this before
getting a
> push there.
>
> I''m surprised that this didn''t hit anyone so far with the
ACPI-based
> idle driver...Good catch Jan, Does this fix need backporting to Xen 4.2 and
4.1?> --- 2012-09-21.orig/xen/arch/x86/hpet.c	2012-09-27 16:10:35.000000000 +0200
> +++ 2012-09-21/xen/arch/x86/hpet.c	2012-09-27 16:10:07.000000000 +0200
> @@ -533,7 +533,7 @@ void __init hpet_broadcast_init(void)
>       {
>           /* set HPET Tn as oneshot */
>           cfg = hpet_read32(HPET_Tn_CFG(hpet_events[i].idx));
> -        cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC | HPET_TN_FSB);
> +        cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC);
>           cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
>           hpet_write32(cfg, HPET_Tn_CFG(hpet_events[i].idx));
>   
> @@ -590,7 +590,7 @@ void hpet_broadcast_resume(void)
>   
>           /* set HPET Tn as oneshot */
>           cfg = hpet_read32(HPET_Tn_CFG(hpet_events[i].idx));
> -        cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC | HPET_TN_FSB);
> +        cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC);
>           cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
>           hpet_write32(cfg, HPET_Tn_CFG(hpet_events[i].idx));
>   
>

Jan Beulich

2012-Sep-27 14:57 UTC

head link

Re: [PATCH] x86/HPET: don''t disable interrupt delivery right after setting it up

>>> On 27.09.12 at 16:49, Malcolm Crossley
<malcolm.crossley@citrix.com> wrote:
> On 27/09/12 15:40, Jan Beulich wrote:
>> We shouldn''t clear HPET_TN_FSB right after we (indirectly, via
>> request_irq()) enabled it for the channels we intend to use for
>> broadcasts.
>>
>> This fixes a regression introduced by c/s 25103:0b0e42dc4f0a.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> This fixes the MWAIT idle driver problem on the box that I was able to
>> (artificially) reproduce it with. Hence I subsequently intend to revert
>> 25960:6bf8b882df8f, but I''m not intending to do this before
getting a
>> push there.
>>
>> I''m surprised that this didn''t hit anyone so far with
the ACPI-based
>> idle driver...
> Good catch Jan, Does this fix need backporting to Xen 4.2 and 4.1?
Definitely to 4.2. Didn''t check yet whether the offending patch got
backported to 4.1.

Jan

Keir Fraser

2012-Sep-27 15:28 UTC

head link

Re: [PATCH] x86/HPET: don''t disable interrupt delivery right after setting it up

On 27/09/2012 15:40, "Jan Beulich" <JBeulich@suse.com> wrote:
> We shouldn''t clear HPET_TN_FSB right after we (indirectly, via
> request_irq()) enabled it for the channels we intend to use for
> broadcasts.
> 
> This fixes a regression introduced by c/s 25103:0b0e42dc4f0a.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
> ---
> This fixes the MWAIT idle driver problem on the box that I was able to
> (artificially) reproduce it with. Hence I subsequently intend to revert
> 25960:6bf8b882df8f, but I''m not intending to do this before
getting a
> push there.
> 
> I''m surprised that this didn''t hit anyone so far with the
ACPI-based
> idle driver...
> 
> --- 2012-09-21.orig/xen/arch/x86/hpet.c 2012-09-27 16:10:35.000000000 +0200
> +++ 2012-09-21/xen/arch/x86/hpet.c 2012-09-27 16:10:07.000000000 +0200
> @@ -533,7 +533,7 @@ void __init hpet_broadcast_init(void)
>      {
>          /* set HPET Tn as oneshot */
>          cfg = hpet_read32(HPET_Tn_CFG(hpet_events[i].idx));
> -        cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC | HPET_TN_FSB);
> +        cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC);
>          cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
>          hpet_write32(cfg, HPET_Tn_CFG(hpet_events[i].idx));
>  
> @@ -590,7 +590,7 @@ void hpet_broadcast_resume(void)
>  
>          /* set HPET Tn as oneshot */
>          cfg = hpet_read32(HPET_Tn_CFG(hpet_events[i].idx));
> -        cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC | HPET_TN_FSB);
> +        cfg &= ~(HPET_TN_LEVEL | HPET_TN_PERIODIC);
>          cfg |= HPET_TN_ENABLE | HPET_TN_32BIT;
>          hpet_write32(cfg, HPET_Tn_CFG(hpet_events[i].idx));
>  
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Xen devel - Sep 2012 - [PATCH] x86/HPET: don't disable interrupt delivery right after setting it up

[PATCH] x86/HPET: don''t disable interrupt delivery right after setting it up

Re: [PATCH] x86/HPET: don''t disable interrupt delivery right after setting it up

Re: [PATCH] x86/HPET: don''t disable interrupt delivery right after setting it up

Re: [PATCH] x86/HPET: don''t disable interrupt delivery right after setting it up