Jan Beulich
2011-Feb-10 13:58 UTC
[Xen-devel] [PATCH] x86: suppress HPET broadcast initialization in the presence of ARAT
This follows Linux commit 39fe05e58c5e448601ce46e6b03900d5bf31c4b0,
noticing that all this setup is pointless when ARAT support is there,
and knowing that on SLED11''s native kernel it has actually caused S3
resume issues.
A question would be whether HPET legacy interrupts should be forced
off in this case (rather than leaving whatever came from firmware).
Signed-off-by: Jan Beulich <jbeulich@novell.com>
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -557,6 +557,9 @@ void hpet_broadcast_init(void)
u32 hpet_id, cfg;
int i;
+ if ( boot_cpu_has(X86_FEATURE_ARAT) )
+ return;
+
if ( irq_channel == NULL )
{
irq_channel = xmalloc_array(int, nr_irqs);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Wei, Gang
2011-Feb-12 07:15 UTC
RE: [Xen-devel] [PATCH] x86: suppress HPET broadcast initialization in the presence of ARAT
Jan Beulich wrote on 2011-02-10:> This follows Linux commit 39fe05e58c5e448601ce46e6b03900d5bf31c4b0, > noticing that all this setup is pointless when ARAT support is there, > and knowing that on SLED11''s native kernel it has actually caused S3 resume issues. > > A question would be whether HPET legacy interrupts should be forced > off in this case (rather than leaving whatever came from firmware). > > Signed-off-by: Jan Beulich <jbeulich@novell.com> > > --- a/xen/arch/x86/hpet.c > +++ b/xen/arch/x86/hpet.c > @@ -557,6 +557,9 @@ void hpet_broadcast_init(void) > u32 hpet_id, cfg; > int i; > + if ( boot_cpu_has(X86_FEATURE_ARAT) ) > + return; > + > if ( irq_channel == NULL ) > { > irq_channel = xmalloc_array(int, nr_irqs); >Although this patch was already checked in, I still have to say it is not necessary for Xen. Because hpet_broadcast_init() fn is only called if (xen_cpuidle && !boot_cpu_has(X86_FEATURE_ARAT)) in disable_pit_irq(). Of course I agree to keep it as a never used double check. Jimmy _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2011-Feb-12 07:35 UTC
Re: [Xen-devel] [PATCH] x86: suppress HPET broadcast initialization in the presence of ARAT
On 12/02/2011 07:15, "Wei, Gang" <gang.wei@intel.com> wrote:> Jan Beulich wrote on 2011-02-10: >> This follows Linux commit 39fe05e58c5e448601ce46e6b03900d5bf31c4b0, >> noticing that all this setup is pointless when ARAT support is there, >> and knowing that on SLED11''s native kernel it has actually caused S3 resume >> issues. >> > > Although this patch was already checked in, I still have to say it is not > necessary for Xen. Because hpet_broadcast_init() fn is only called if > (xen_cpuidle && !boot_cpu_has(X86_FEATURE_ARAT)) in disable_pit_irq(). Of > course I agree to keep it as a never used double check.Hmm I didn''t spot that. Actually it is part of a more complex series of checks in the caller, so I wonder whether repeating just that one check in the function itself really makes much sense. I''m somewhat inclibned to revert it. -- Keir> Jimmy > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Wei, Gang
2011-Feb-12 08:46 UTC
RE: [Xen-devel] [PATCH] x86: suppress HPET broadcast initialization in the presence of ARAT
Keir Fraser wrote on 2011-02-12:> On 12/02/2011 07:15, "Wei, Gang" <gang.wei@intel.com> wrote: >> Jan Beulich wrote on 2011-02-10: >>> This follows Linux commit 39fe05e58c5e448601ce46e6b03900d5bf31c4b0, >>> noticing that all this setup is pointless when ARAT support is >>> there, and knowing that on SLED11''s native kernel it has actually >>> caused S3 resume issues. >> >> Although this patch was already checked in, I still have to say it >> is not necessary for Xen. Because hpet_broadcast_init() fn is only >> called if (xen_cpuidle && !boot_cpu_has(X86_FEATURE_ARAT)) in >> disable_pit_irq(). Of course I agree to keep it as a never used double check. > > Hmm I didn''t spot that. Actually it is part of a more complex series > of checks in the caller, so I wonder whether repeating just that one > check in the function itself really makes much sense. I''m somewhat inclibned to revert it.Revert it would be better. Jimmy _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2011-Feb-14 08:15 UTC
Re: [Xen-devel] [PATCH] x86: suppress HPET broadcast initialization in the presence of ARAT
>>> On 12.02.11 at 08:35, Keir Fraser <keir@xen.org> wrote: > On 12/02/2011 07:15, "Wei, Gang" <gang.wei@intel.com> wrote: > >> Jan Beulich wrote on 2011-02-10: >>> This follows Linux commit 39fe05e58c5e448601ce46e6b03900d5bf31c4b0, >>> noticing that all this setup is pointless when ARAT support is there, >>> and knowing that on SLED11''s native kernel it has actually caused S3 resume >>> issues. >>> >> >> Although this patch was already checked in, I still have to say it is not >> necessary for Xen. Because hpet_broadcast_init() fn is only called if >> (xen_cpuidle && !boot_cpu_has(X86_FEATURE_ARAT)) in disable_pit_irq(). Of >> course I agree to keep it as a never used double check. > > Hmm I didn''t spot that. Actually it is part of a more complex series of > checks in the caller, so I wonder whether repeating just that one check in > the function itself really makes much sense. I''m somewhat inclibned to > revert it.Yes, revert it - somehow I managed to not notice we do this check already. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel