Re: [Xen-devel] [PATCH] x86/xen: Disable APIC PM for Xen PV guests

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 03/04/2014 05:25 AM, David Vrabel wrote:
> On 04/03/14 02:40, Boris Ostrovsky wrote:
> > Xen PV guests support only few APIC registers and writes to
> > unsupported registers result in WARN_ONs. Most APIC accesses in these
> > guests  have been eliminated; however, lapic_suspend/resume are still
> > called (on 32-bit kernels).
> >
> > We can disable APIC power management in xen_smp_prepare_boot_cpu()
> > (which is called after APIC has been initialized).
> Having looked at another APIC related problem recently, I wonder if Xen
> would be better served by having its own APIC driver instead of trying
> to piggy-back of some hardware one which doesn't do what we want.
Possibly, although I don't think it will help with this particular issue 
since this patch is trying to address a problem in non-privileged 
guests. Such guests don't have ACPI enabled and so no APIC probing is 
done for them.
-boris

> David
>
> > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> > ---
> >   arch/x86/include/asm/apic.h |    2 ++
> >   arch/x86/kernel/apic/apic.c |    6 ++++++
> >   arch/x86/xen/smp.c          |    4 ++++
> >   3 files changed, 12 insertions(+)
> >
> >
> > An alternative could be to add another op to stuct apic. The advantage would be
> > the fact that we never have a window of time when APIC PM is enabled. The 
> > downside
> > is having yet another op (of which there are already plenty).
> >
> > We could also avoid loading lapic_syscore_ops but I couldn't think of a good way
> > to figure out when not to do it.
> >
> >
> >
> > diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> > index 1d2091a..1f100f2 100644
> > --- a/arch/x86/include/asm/apic.h
> > +++ b/arch/x86/include/asm/apic.h
> > @@ -687,6 +687,8 @@ extern int default_cpu_present_to_apicid(int mps_cpu);
> >   extern int default_check_phys_apicid_present(int phys_apicid);
> >   #endif
> >   
> > +extern void apic_pm_deactivate(void);
> > +
> >   #endif /* CONFIG_X86_LOCAL_APIC */
> >   extern void irq_enter(void);
> >   extern void irq_exit(void);
> > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> > index 7f26c9a..60607b3 100644
> > --- a/arch/x86/kernel/apic/apic.c
> > +++ b/arch/x86/kernel/apic/apic.c
> > @@ -2424,6 +2424,11 @@ static void apic_pm_activate(void)
> >         apic_pm_state.active = 1;
> >   }
> >   
> > +void apic_pm_deactivate(void)
> > +{
> > +       apic_pm_state.active = 0;
> > +}
> > +
> >   static int __init init_lapic_sysfs(void)
> >   {
> >         /* XXX: remove suspend/resume procs if !apic_pm_state.active? */
> > @@ -2439,6 +2444,7 @@ core_initcall(init_lapic_sysfs);
> >   #else /* CONFIG_PM */
> >   
> >   static void apic_pm_activate(void) { }
> > +void apic_pm_deactivate(void) { }
> >   
> >   #endif	/* CONFIG_PM */
> >   
> > diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
> > index a18eadd..33115d8 100644
> > --- a/arch/x86/xen/smp.c
> > +++ b/arch/x86/xen/smp.c
> > @@ -298,6 +298,10 @@ static void __init xen_smp_prepare_boot_cpu(void)
> >   
> >   		xen_filter_cpu_maps();
> >                 xen_setup_vcpu_info_placement();
> > +
> > +#ifdef CONFIG_X86_LOCAL_APIC
> > +               apic_pm_deactivate();
> > +#endif
> >         }
> >         /*
> >          * The alternative logic (which patches the unlock/lock) runs before

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