| Message ID | 1459987594-5434-5-git-send-email-mcgrof@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/04/16 01:06, Luis R. Rodriguez wrote: > We have 4 types of x86 platforms that disable RTC: > > * Intel MID > * Lguest - uses paravirt > * Xen dom-U - uses paravirt > * x86 on legacy systems annotated with an ACPI legacy flag > > We can consolidate all of these into a platform specific legacy > quirk set early in boot through i386_start_kernel() and through > x86_64_start_reservations(). This deals with the RTC quirks which > we can rely on through the hardware subarch, the ACPI check can > be dealt with separately. Xen parts: Reviewed-by: David Vrabel <david.vrabel@citrix.com> David
On 04/06/2016 08:06 PM, Luis R. Rodriguez wrote: > We have 4 types of x86 platforms that disable RTC: > > * Intel MID > * Lguest - uses paravirt > * Xen dom-U - uses paravirt > * x86 on legacy systems annotated with an ACPI legacy flag > > We can consolidate all of these into a platform specific legacy > quirk set early in boot through i386_start_kernel() and through > x86_64_start_reservations(). This deals with the RTC quirks which > we can rely on through the hardware subarch, the ACPI check can > be dealt with separately. > > v2: split the subarch check from the ACPI check, clarify > on the ACPI change commit log why ordering works > > Suggested-by: Ingo Molnar <mingo@kernel.org> > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> > --- > arch/x86/Makefile | 1 + > arch/x86/include/asm/paravirt.h | 6 ------ > arch/x86/include/asm/paravirt_types.h | 5 ----- > arch/x86/include/asm/processor.h | 1 - > arch/x86/include/asm/x86_init.h | 13 +++++++++++++ > arch/x86/kernel/Makefile | 6 +++++- > arch/x86/kernel/head32.c | 2 ++ > arch/x86/kernel/head64.c | 1 + > arch/x86/kernel/platform-quirks.c | 18 ++++++++++++++++++ > arch/x86/kernel/rtc.c | 7 ++----- > arch/x86/lguest/boot.c | 1 - > arch/x86/xen/enlighten.c | 3 --- > 12 files changed, 42 insertions(+), 22 deletions(-) > create mode 100644 arch/x86/kernel/platform-quirks.c > > diff --git a/arch/x86/Makefile b/arch/x86/Makefile > index 4086abca0b32..f9ed8a7ce2b6 100644 > --- a/arch/x86/Makefile > +++ b/arch/x86/Makefile > @@ -209,6 +209,7 @@ endif > head-y := arch/x86/kernel/head_$(BITS).o > head-y += arch/x86/kernel/head$(BITS).o > head-y += arch/x86/kernel/head.o > +head-y += arch/x86/kernel/platform-quirks.o > > libs-y += arch/x86/lib/ > > diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h > index 601f1b8f9961..6c7a4a192032 100644 > --- a/arch/x86/include/asm/paravirt.h > +++ b/arch/x86/include/asm/paravirt.h > @@ -20,12 +20,6 @@ static inline int paravirt_enabled(void) > return pv_info.paravirt_enabled; > } > > -static inline int paravirt_has_feature(unsigned int feature) > -{ > - WARN_ON_ONCE(!pv_info.paravirt_enabled); > - return (pv_info.features & feature); > -} > - > static inline void load_sp0(struct tss_struct *tss, > struct thread_struct *thread) > { > diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h > index e8c2326478c8..6acc1b26cf40 100644 > --- a/arch/x86/include/asm/paravirt_types.h > +++ b/arch/x86/include/asm/paravirt_types.h > @@ -70,14 +70,9 @@ struct pv_info { > #endif > > int paravirt_enabled; > - unsigned int features; /* valid only if paravirt_enabled is set */ > const char *name; > }; > > -#define paravirt_has(x) paravirt_has_feature(PV_SUPPORTED_##x) > -/* Supported features */ > -#define PV_SUPPORTED_RTC (1<<0) > - > struct pv_init_ops { > /* > * Patch may replace one of the defined code sequences with > diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h > index 9264476f3d57..0c70c7daa6b8 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -474,7 +474,6 @@ static inline unsigned long current_top_of_stack(void) > #else > #define __cpuid native_cpuid > #define paravirt_enabled() 0 > -#define paravirt_has(x) 0 > > static inline void load_sp0(struct tss_struct *tss, > struct thread_struct *thread) > diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h > index 1ae89a2721d6..27d5c3fe5198 100644 > --- a/arch/x86/include/asm/x86_init.h > +++ b/arch/x86/include/asm/x86_init.h > @@ -142,6 +142,15 @@ struct x86_cpuinit_ops { > struct timespec; > > /** > + * struct x86_legacy_features - legacy x86 features > + * > + * @rtc: this device has a CMOS real-time clock present > + */ > +struct x86_legacy_features { > + int rtc; > +}; > + > +/** > * struct x86_platform_ops - platform specific runtime functions > * @calibrate_tsc: calibrate TSC > * @get_wallclock: get time from HW clock like RTC etc. > @@ -152,6 +161,7 @@ struct timespec; > * @save_sched_clock_state: save state for sched_clock() on suspend > * @restore_sched_clock_state: restore state for sched_clock() on resume > * @apic_post_init: adjust apic if neeeded > + * @legacy: legacy features > */ > struct x86_platform_ops { > unsigned long (*calibrate_tsc)(void); > @@ -165,6 +175,7 @@ struct x86_platform_ops { > void (*save_sched_clock_state)(void); > void (*restore_sched_clock_state)(void); > void (*apic_post_init)(void); > + struct x86_legacy_features legacy; > }; > > struct pci_dev; > @@ -186,6 +197,8 @@ extern struct x86_cpuinit_ops x86_cpuinit; > extern struct x86_platform_ops x86_platform; > extern struct x86_msi_ops x86_msi; > extern struct x86_io_apic_ops x86_io_apic_ops; > + > +extern void x86_early_init_platform_quirks(void); > extern void x86_init_noop(void); > extern void x86_init_uint_noop(unsigned int unused); > > diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile > index 89f8ade0bc7c..7a9e44d935de 100644 > --- a/arch/x86/kernel/Makefile > +++ b/arch/x86/kernel/Makefile > @@ -2,7 +2,11 @@ > # Makefile for the linux kernel. > # > > -extra-y := head_$(BITS).o head$(BITS).o head.o vmlinux.lds > +extra-y := head_$(BITS).o > +extra-y += head$(BITS).o > +extra-y += head.o > +extra-y += platform-quirks.o > +extra-y += vmlinux.lds > > CPPFLAGS_vmlinux.lds += -U$(UTS_MACHINE) > > diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c > index 2911ef3a9f1c..d784bb547a9d 100644 > --- a/arch/x86/kernel/head32.c > +++ b/arch/x86/kernel/head32.c > @@ -34,6 +34,8 @@ asmlinkage __visible void __init i386_start_kernel(void) > cr4_init_shadow(); > sanitize_boot_params(&boot_params); > > + x86_early_init_platform_quirks(); > + > /* Call the subarch specific early setup function */ > switch (boot_params.hdr.hardware_subarch) { > case X86_SUBARCH_INTEL_MID: > diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c > index 1f4422d5c8d0..b72fb0b71dd1 100644 > --- a/arch/x86/kernel/head64.c > +++ b/arch/x86/kernel/head64.c > @@ -182,6 +182,7 @@ void __init x86_64_start_reservations(char *real_mode_data) > if (!boot_params.hdr.version) > copy_bootdata(__va(real_mode_data)); > > + x86_early_init_platform_quirks(); > reserve_ebda_region(); > > switch (boot_params.hdr.hardware_subarch) { > diff --git a/arch/x86/kernel/platform-quirks.c b/arch/x86/kernel/platform-quirks.c > new file mode 100644 > index 000000000000..1b114ac5996f > --- /dev/null > +++ b/arch/x86/kernel/platform-quirks.c > @@ -0,0 +1,18 @@ > +#include <linux/kernel.h> > +#include <linux/init.h> > + > +#include <asm/setup.h> > +#include <asm/bios_ebda.h> > + > +void __init x86_early_init_platform_quirks(void) > +{ > + x86_platform.legacy.rtc = 1; > + > + switch (boot_params.hdr.hardware_subarch) { > + case X86_SUBARCH_XEN: > + case X86_SUBARCH_LGUEST: > + case X86_SUBARCH_INTEL_MID: > + x86_platform.legacy.rtc = 0; > + break; > + } > +} What about Xen dom0 (aka initial domain)? -boris > --- a/arch/x86/xen/enlighten.c > +++ b/arch/x86/xen/enlighten.c > @@ -1192,7 +1192,6 @@ static const struct pv_info xen_info __initconst = { > #ifdef CONFIG_X86_64 > .extra_user_64bit_cs = FLAT_USER_CS64, > #endif > - .features = 0, > .name = "Xen", > }; > > @@ -1525,8 +1524,6 @@ asmlinkage __visible void __init xen_start_kernel(void) > > /* Install Xen paravirt ops */ > pv_info = xen_info; > - if (xen_initial_domain()) > - pv_info.features |= PV_SUPPORTED_RTC; > pv_init_ops = xen_init_ops; > if (!xen_pvh_domain()) { > pv_cpu_ops = xen_cpu_ops;
On Thu, Apr 07, 2016 at 08:55:54AM -0400, Boris Ostrovsky wrote: > On 04/06/2016 08:06 PM, Luis R. Rodriguez wrote: > >We have 4 types of x86 platforms that disable RTC: > > > > * Intel MID > > * Lguest - uses paravirt > > * Xen dom-U - uses paravirt > > * x86 on legacy systems annotated with an ACPI legacy flag > > > >We can consolidate all of these into a platform specific legacy > >quirk set early in boot through i386_start_kernel() and through > >x86_64_start_reservations(). This deals with the RTC quirks which > >we can rely on through the hardware subarch, the ACPI check can > >be dealt with separately. > > > >v2: split the subarch check from the ACPI check, clarify > > on the ACPI change commit log why ordering works > > > >Suggested-by: Ingo Molnar <mingo@kernel.org> > >Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> <-- snip --> > >diff --git a/arch/x86/kernel/platform-quirks.c b/arch/x86/kernel/platform-quirks.c > >new file mode 100644 > >index 000000000000..1b114ac5996f > >--- /dev/null > >+++ b/arch/x86/kernel/platform-quirks.c > >@@ -0,0 +1,18 @@ > >+#include <linux/kernel.h> > >+#include <linux/init.h> > >+ > >+#include <asm/setup.h> > >+#include <asm/bios_ebda.h> > >+ > >+void __init x86_early_init_platform_quirks(void) > >+{ > >+ x86_platform.legacy.rtc = 1; > >+ > >+ switch (boot_params.hdr.hardware_subarch) { > >+ case X86_SUBARCH_XEN: > >+ case X86_SUBARCH_LGUEST: > >+ case X86_SUBARCH_INTEL_MID: > >+ x86_platform.legacy.rtc = 0; > >+ break; > >+ } > >+} > > What about Xen dom0 (aka initial domain)? Indeed, thanks for catching this, the hunk below removes the re-enablement of the the RTC for dom0: > >--- a/arch/x86/xen/enlighten.c > >+++ b/arch/x86/xen/enlighten.c > >@@ -1192,7 +1192,6 @@ static const struct pv_info xen_info __initconst = { > > #ifdef CONFIG_X86_64 > > .extra_user_64bit_cs = FLAT_USER_CS64, > > #endif > >- .features = 0, > > .name = "Xen", > > }; > >@@ -1525,8 +1524,6 @@ asmlinkage __visible void __init xen_start_kernel(void) > > /* Install Xen paravirt ops */ > > pv_info = xen_info; > >- if (xen_initial_domain()) > >- pv_info.features |= PV_SUPPORTED_RTC; > > pv_init_ops = xen_init_ops; > > if (!xen_pvh_domain()) { > > pv_cpu_ops = xen_cpu_ops; This should then break dom0 unless of course you have the respective next patch applied and that disabled the RTC due to an ACPI setting on your platform. Juergen, can you check to see if that was the case for your testing platform on dom0 ? This highlights a semantic gap issue. From a quick cursory review, I think we can address this temporarily by just using a check: void __init x86_early_init_platform_quirks(void) { x86_platform.legacy.rtc = 1; switch (boot_params.hdr.hardware_subarch) { case X86_SUBARCH_XEN: case X86_SUBARCH_LGUEST: case X86_SUBARCH_INTEL_MID: - x86_platform.legacy.rtc = 0; + if (x86_init.mpparse.get_smp_config != x86_init_uint_noop) + x86_platform.legacy.rtc = 0; break; } } This would work given x86_early_init_platform_quirks() is called prior to the any code that sets up x86_init.mpparse*, and the only code that would have set this is the PV guest path, and the dom0 override. Is would be replacing one hack with another though so I'm not exactly happy with it as a compromise. Another x86 standard thing dom0 populates on the PV path is &boot_params.screen_info, done through xen_init_vga() -- but it was not clear exactly what domU does with this ? Does domU never have VGA set up? Given that the PV path is the only thing that would ever set up the x86 zero page I take it domU leaves that empty, and dom0 *always* seems to set up the screen_info->orig_video_isVGA, so another mechanism might be something like: void __init x86_early_init_platform_quirks(void) { + struct screen_info *screen_info = &boot_params.screen_info; + x86_platform.legacy.rtc = 1; switch (boot_params.hdr.hardware_subarch) { case X86_SUBARCH_XEN: case X86_SUBARCH_LGUEST: case X86_SUBARCH_INTEL_MID: - x86_platform.legacy.rtc = 0; + if (!screen_info->orig_video_isVGA) + x86_platform.legacy.rtc = 0; break; } } If the semantics of requiring VGA through the x86 boot params suffice to annotate PV path dom0 then we have a win. Specially if this might be useful to other virtualization environments to do some of their own virtualization quirks in this path. Additionally -- if domU never sets the screen info stuff, should it or does it always set ACPI_FADT_NO_VGA as well ? Is Xen the only guest type we have that has a notion of dom0 and need dom0 type of quirks ? Also how would this work for HVMLite for domU and dom0 ? I think the ARM folks are doing some things with EFI configuration tables to pass the screen_info stuff, so perhaps that should be looked at. Luis
On 08/04/16 02:32, Luis R. Rodriguez wrote: > On Thu, Apr 07, 2016 at 08:55:54AM -0400, Boris Ostrovsky wrote: >> On 04/06/2016 08:06 PM, Luis R. Rodriguez wrote: >>> We have 4 types of x86 platforms that disable RTC: >>> >>> * Intel MID >>> * Lguest - uses paravirt >>> * Xen dom-U - uses paravirt >>> * x86 on legacy systems annotated with an ACPI legacy flag >>> >>> We can consolidate all of these into a platform specific legacy >>> quirk set early in boot through i386_start_kernel() and through >>> x86_64_start_reservations(). This deals with the RTC quirks which >>> we can rely on through the hardware subarch, the ACPI check can >>> be dealt with separately. >>> >>> v2: split the subarch check from the ACPI check, clarify >>> on the ACPI change commit log why ordering works >>> >>> Suggested-by: Ingo Molnar <mingo@kernel.org> >>> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> > > <-- snip --> > >>> diff --git a/arch/x86/kernel/platform-quirks.c b/arch/x86/kernel/platform-quirks.c >>> new file mode 100644 >>> index 000000000000..1b114ac5996f >>> --- /dev/null >>> +++ b/arch/x86/kernel/platform-quirks.c >>> @@ -0,0 +1,18 @@ >>> +#include <linux/kernel.h> >>> +#include <linux/init.h> >>> + >>> +#include <asm/setup.h> >>> +#include <asm/bios_ebda.h> >>> + >>> +void __init x86_early_init_platform_quirks(void) >>> +{ >>> + x86_platform.legacy.rtc = 1; >>> + >>> + switch (boot_params.hdr.hardware_subarch) { >>> + case X86_SUBARCH_XEN: >>> + case X86_SUBARCH_LGUEST: >>> + case X86_SUBARCH_INTEL_MID: >>> + x86_platform.legacy.rtc = 0; >>> + break; >>> + } >>> +} >> >> What about Xen dom0 (aka initial domain)? > > Indeed, thanks for catching this, the hunk below removes the re-enablement of > the the RTC for dom0: > >>> --- a/arch/x86/xen/enlighten.c >>> +++ b/arch/x86/xen/enlighten.c >>> @@ -1192,7 +1192,6 @@ static const struct pv_info xen_info __initconst = { >>> #ifdef CONFIG_X86_64 >>> .extra_user_64bit_cs = FLAT_USER_CS64, >>> #endif >>> - .features = 0, >>> .name = "Xen", >>> }; >>> @@ -1525,8 +1524,6 @@ asmlinkage __visible void __init xen_start_kernel(void) >>> /* Install Xen paravirt ops */ >>> pv_info = xen_info; >>> - if (xen_initial_domain()) >>> - pv_info.features |= PV_SUPPORTED_RTC; >>> pv_init_ops = xen_init_ops; >>> if (!xen_pvh_domain()) { >>> pv_cpu_ops = xen_cpu_ops; > > This should then break dom0 unless of course you have the respective next > patch applied and that disabled the RTC due to an ACPI setting on your > platform. Juergen, can you check to see if that was the case for your > testing platform on dom0 ? Are you sure it would break? Wouldn't it just fall back to another clock source, e.g. hpet? I looked into my test system: seems as if add_rtc_cmos() is returning before the .legacy.rtc test. > This highlights a semantic gap issue. From a quick cursory review, I think > we can address this temporarily by just using a check: > > void __init x86_early_init_platform_quirks(void) > { > x86_platform.legacy.rtc = 1; > > switch (boot_params.hdr.hardware_subarch) { > case X86_SUBARCH_XEN: > case X86_SUBARCH_LGUEST: > case X86_SUBARCH_INTEL_MID: > - x86_platform.legacy.rtc = 0; > + if (x86_init.mpparse.get_smp_config != x86_init_uint_noop) > + x86_platform.legacy.rtc = 0; No! Why don't you just use the explicit test xen_initial_domain() ? Juergen
On Thu, Apr 7, 2016 at 10:18 PM, Juergen Gross <jgross@suse.com> wrote: > On 08/04/16 02:32, Luis R. Rodriguez wrote: >> On Thu, Apr 07, 2016 at 08:55:54AM -0400, Boris Ostrovsky wrote: >>> On 04/06/2016 08:06 PM, Luis R. Rodriguez wrote: >>>> We have 4 types of x86 platforms that disable RTC: >>>> >>>> * Intel MID >>>> * Lguest - uses paravirt >>>> * Xen dom-U - uses paravirt >>>> * x86 on legacy systems annotated with an ACPI legacy flag >>>> >>>> We can consolidate all of these into a platform specific legacy >>>> quirk set early in boot through i386_start_kernel() and through >>>> x86_64_start_reservations(). This deals with the RTC quirks which >>>> we can rely on through the hardware subarch, the ACPI check can >>>> be dealt with separately. >>>> >>>> v2: split the subarch check from the ACPI check, clarify >>>> on the ACPI change commit log why ordering works >>>> >>>> Suggested-by: Ingo Molnar <mingo@kernel.org> >>>> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> >> >> <-- snip --> >> >>>> diff --git a/arch/x86/kernel/platform-quirks.c b/arch/x86/kernel/platform-quirks.c >>>> new file mode 100644 >>>> index 000000000000..1b114ac5996f >>>> --- /dev/null >>>> +++ b/arch/x86/kernel/platform-quirks.c >>>> @@ -0,0 +1,18 @@ >>>> +#include <linux/kernel.h> >>>> +#include <linux/init.h> >>>> + >>>> +#include <asm/setup.h> >>>> +#include <asm/bios_ebda.h> >>>> + >>>> +void __init x86_early_init_platform_quirks(void) >>>> +{ >>>> + x86_platform.legacy.rtc = 1; >>>> + >>>> + switch (boot_params.hdr.hardware_subarch) { >>>> + case X86_SUBARCH_XEN: >>>> + case X86_SUBARCH_LGUEST: >>>> + case X86_SUBARCH_INTEL_MID: >>>> + x86_platform.legacy.rtc = 0; >>>> + break; >>>> + } >>>> +} >>> >>> What about Xen dom0 (aka initial domain)? >> >> Indeed, thanks for catching this, the hunk below removes the re-enablement of >> the the RTC for dom0: >> >>>> --- a/arch/x86/xen/enlighten.c >>>> +++ b/arch/x86/xen/enlighten.c >>>> @@ -1192,7 +1192,6 @@ static const struct pv_info xen_info __initconst = { >>>> #ifdef CONFIG_X86_64 >>>> .extra_user_64bit_cs = FLAT_USER_CS64, >>>> #endif >>>> - .features = 0, >>>> .name = "Xen", >>>> }; >>>> @@ -1525,8 +1524,6 @@ asmlinkage __visible void __init xen_start_kernel(void) >>>> /* Install Xen paravirt ops */ >>>> pv_info = xen_info; >>>> - if (xen_initial_domain()) >>>> - pv_info.features |= PV_SUPPORTED_RTC; >>>> pv_init_ops = xen_init_ops; >>>> if (!xen_pvh_domain()) { >>>> pv_cpu_ops = xen_cpu_ops; >> >> This should then break dom0 unless of course you have the respective next >> patch applied and that disabled the RTC due to an ACPI setting on your >> platform. Juergen, can you check to see if that was the case for your >> testing platform on dom0 ? > > Are you sure it would break? No, suspected that it should though. > Wouldn't it just fall back to another > clock source, e.g. hpet? I suppose so. > I looked into my test system: seems as if add_rtc_cmos() is returning > before the .legacy.rtc test. OK thanks... >> This highlights a semantic gap issue. From a quick cursory review, I think >> we can address this temporarily by just using a check: >> >> void __init x86_early_init_platform_quirks(void) >> { >> x86_platform.legacy.rtc = 1; >> >> switch (boot_params.hdr.hardware_subarch) { >> case X86_SUBARCH_XEN: >> case X86_SUBARCH_LGUEST: >> case X86_SUBARCH_INTEL_MID: >> - x86_platform.legacy.rtc = 0; >> + if (x86_init.mpparse.get_smp_config != x86_init_uint_noop) >> + x86_platform.legacy.rtc = 0; > > No! Why don't you just use the explicit test xen_initial_domain() ? Because we don't want to sprinkle Xen specific code outside of Xen code. What do you think about the second possibility I listed? Otherwise, any other ideas? Luis
On 08/04/16 08:29, Luis R. Rodriguez wrote: > On Thu, Apr 7, 2016 at 10:18 PM, Juergen Gross <jgross@suse.com> wrote: >> On 08/04/16 02:32, Luis R. Rodriguez wrote: >>> This highlights a semantic gap issue. From a quick cursory review, I think >>> we can address this temporarily by just using a check: >>> >>> void __init x86_early_init_platform_quirks(void) >>> { >>> x86_platform.legacy.rtc = 1; >>> >>> switch (boot_params.hdr.hardware_subarch) { >>> case X86_SUBARCH_XEN: >>> case X86_SUBARCH_LGUEST: >>> case X86_SUBARCH_INTEL_MID: >>> - x86_platform.legacy.rtc = 0; >>> + if (x86_init.mpparse.get_smp_config != x86_init_uint_noop) >>> + x86_platform.legacy.rtc = 0; >> >> No! Why don't you just use the explicit test xen_initial_domain() ? > > Because we don't want to sprinkle Xen specific code outside of Xen > code. What do you think about the second possibility I listed? > Otherwise, any other ideas? Don't try to guess. In case you don't want to inject Xen internals here, just call a Xen function to either return the correct value, or to set all structure elements correctly. Thinking more about it: why not do that for all the subarchs? You'd have the specific settings where they belong: in a subarch specific source. Just do the default settings in x86_early_init_platform_quirks() and let the subarch functions set the non-default values. Juergen
On Thu, Apr 7, 2016 at 11:38 PM, Juergen Gross <jgross@suse.com> wrote: > On 08/04/16 08:29, Luis R. Rodriguez wrote: >> On Thu, Apr 7, 2016 at 10:18 PM, Juergen Gross <jgross@suse.com> wrote: >>> On 08/04/16 02:32, Luis R. Rodriguez wrote: >>>> This highlights a semantic gap issue. From a quick cursory review, I think >>>> we can address this temporarily by just using a check: >>>> >>>> void __init x86_early_init_platform_quirks(void) >>>> { >>>> x86_platform.legacy.rtc = 1; >>>> >>>> switch (boot_params.hdr.hardware_subarch) { >>>> case X86_SUBARCH_XEN: >>>> case X86_SUBARCH_LGUEST: >>>> case X86_SUBARCH_INTEL_MID: >>>> - x86_platform.legacy.rtc = 0; >>>> + if (x86_init.mpparse.get_smp_config != x86_init_uint_noop) >>>> + x86_platform.legacy.rtc = 0; >>> >>> No! Why don't you just use the explicit test xen_initial_domain() ? >> >> Because we don't want to sprinkle Xen specific code outside of Xen >> code. What do you think about the second possibility I listed? >> Otherwise, any other ideas? > > Don't try to guess. I can only do that given there is nothing at all to tell me what to expect here with regards to RTC on Xen guest, if there is some documentation that could help with that please let me know. > In case you don't want to inject Xen internals here, just call a Xen > function to either return the correct value, or to set all structure > elements correctly. I like the later as an option, in case there are further hardware subarch specific quirks which require internal logistics. What do others think? > Thinking more about it: why not do that for all the subarchs? I originally had went with that approach, but Ingo made the point that it would be best to instead move all quirk settings into one place. That lets a reader easily tell what is going on in one place, it also compartmentalizes the hardware subarch uses. > You'd > have the specific settings where they belong: in a subarch specific > source. Just do the default settings in x86_early_init_platform_quirks() > and let the subarch functions set the non-default values. This is a rather different approach than what I had originally tried. Bike shed thing -- someone just has to decide. Left up to me, I kind of really like centralizing the quirk settings in one place approach as it means a reader can easily tell what's going on regardless of platform in one place for odd settings. I prefer this given that we *already* have the semantics over hardware subarch in a generalized fashion. We *do not* have semantics for dom0 Vs domU -- if such a notion is generic to other virtualization environments it deserves consideration to new semantics to deal with that, otherwise the callback for handling further quirks is best, but I'd also highly discourage such callback to be used. Luis
On 08/04/16 08:56, Luis R. Rodriguez wrote: > On Thu, Apr 7, 2016 at 11:38 PM, Juergen Gross <jgross@suse.com> wrote: >> On 08/04/16 08:29, Luis R. Rodriguez wrote: >>> On Thu, Apr 7, 2016 at 10:18 PM, Juergen Gross <jgross@suse.com> wrote: >>>> On 08/04/16 02:32, Luis R. Rodriguez wrote: >>>>> This highlights a semantic gap issue. From a quick cursory review, I think >>>>> we can address this temporarily by just using a check: >>>>> >>>>> void __init x86_early_init_platform_quirks(void) >>>>> { >>>>> x86_platform.legacy.rtc = 1; >>>>> >>>>> switch (boot_params.hdr.hardware_subarch) { >>>>> case X86_SUBARCH_XEN: >>>>> case X86_SUBARCH_LGUEST: >>>>> case X86_SUBARCH_INTEL_MID: >>>>> - x86_platform.legacy.rtc = 0; >>>>> + if (x86_init.mpparse.get_smp_config != x86_init_uint_noop) >>>>> + x86_platform.legacy.rtc = 0; >>>> >>>> No! Why don't you just use the explicit test xen_initial_domain() ? >>> >>> Because we don't want to sprinkle Xen specific code outside of Xen >>> code. What do you think about the second possibility I listed? >>> Otherwise, any other ideas? >> >> Don't try to guess. > > I can only do that given there is nothing at all to tell me what to > expect here with regards to RTC on Xen guest, if there is some > documentation that could help with that please let me know. Only Xen inernals. :-) > >> In case you don't want to inject Xen internals here, just call a Xen >> function to either return the correct value, or to set all structure >> elements correctly. > > I like the later as an option, in case there are further hardware > subarch specific quirks which require internal logistics. What do > others think? > >> Thinking more about it: why not do that for all the subarchs? > > I originally had went with that approach, but Ingo made the point that > it would be best to instead move all quirk settings into one place. > That lets a reader easily tell what is going on in one place, it also > compartmentalizes the hardware subarch uses. Okay. Another idea (not sure whether this is really a good one): Add X86_SUBARCH_XEN_DOM0. As hardware_subarch is 32 bits wide I don't think the number of subarchs is a scarce resource. :-) I'd expect other quirks in future might have different settings for domU and dom0, too. >> You'd >> have the specific settings where they belong: in a subarch specific >> source. Just do the default settings in x86_early_init_platform_quirks() >> and let the subarch functions set the non-default values. > > This is a rather different approach than what I had originally tried. > Bike shed thing -- someone just has to decide. > > Left up to me, I kind of really like centralizing the quirk settings > in one place approach as it means a reader can easily tell what's > going on regardless of platform in one place for odd settings. I > prefer this given that we *already* have the semantics over hardware > subarch in a generalized fashion. We *do not* have semantics for dom0 > Vs domU -- if such a notion is generic to other virtualization That's not carved in stone - see above. :-) > environments it deserves consideration to new semantics to deal with > that, otherwise the callback for handling further quirks is best, but > I'd also highly discourage such callback to be used. Juergen
On Fri, Apr 8, 2016 at 12:13 AM, Juergen Gross <jgross@suse.com> wrote: > On 08/04/16 08:56, Luis R. Rodriguez wrote: >> On Thu, Apr 7, 2016 at 11:38 PM, Juergen Gross <jgross@suse.com> wrote: >>> On 08/04/16 08:29, Luis R. Rodriguez wrote: >>>> On Thu, Apr 7, 2016 at 10:18 PM, Juergen Gross <jgross@suse.com> wrote: >>>>> On 08/04/16 02:32, Luis R. Rodriguez wrote: >>>>>> This highlights a semantic gap issue. From a quick cursory review, I think >>>>>> we can address this temporarily by just using a check: >>>>>> >>>>>> void __init x86_early_init_platform_quirks(void) >>>>>> { >>>>>> x86_platform.legacy.rtc = 1; >>>>>> >>>>>> switch (boot_params.hdr.hardware_subarch) { >>>>>> case X86_SUBARCH_XEN: >>>>>> case X86_SUBARCH_LGUEST: >>>>>> case X86_SUBARCH_INTEL_MID: >>>>>> - x86_platform.legacy.rtc = 0; >>>>>> + if (x86_init.mpparse.get_smp_config != x86_init_uint_noop) >>>>>> + x86_platform.legacy.rtc = 0; >>>>> >>>>> No! Why don't you just use the explicit test xen_initial_domain() ? >>>> >>>> Because we don't want to sprinkle Xen specific code outside of Xen >>>> code. What do you think about the second possibility I listed? >>>> Otherwise, any other ideas? >>> >>> Don't try to guess. >> >> I can only do that given there is nothing at all to tell me what to >> expect here with regards to RTC on Xen guest, if there is some >> documentation that could help with that please let me know. > > Only Xen inernals. :-) Where can I look at this specifically on the Xen sources? No worries if you don't care -- as I don't really either. >>> In case you don't want to inject Xen internals here, just call a Xen >>> function to either return the correct value, or to set all structure >>> elements correctly. >> >> I like the later as an option, in case there are further hardware >> subarch specific quirks which require internal logistics. What do >> others think? >> >>> Thinking more about it: why not do that for all the subarchs? >> >> I originally had went with that approach, but Ingo made the point that >> it would be best to instead move all quirk settings into one place. >> That lets a reader easily tell what is going on in one place, it also >> compartmentalizes the hardware subarch uses. > > Okay. Another idea (not sure whether this is really a good one): > > Add X86_SUBARCH_XEN_DOM0. As hardware_subarch is 32 bits wide I don't > think the number of subarchs is a scarce resource. :-) This would mean bumping the x86 boot protocol, we shouldn't take that lightly, but given that in this case the new subarch would really only be set by the kernel (or future loaders for perhaps HVMLite) I'd think this is not such an intrusive alternative. > I'd expect other quirks in future might have different settings for > domU and dom0, too. Can you elaborate a bit more on this. I realize we expect domU and dom0 on HVMLite in the future, would HVMLite use the subarch ? From the last discussions on the HVMLite thread Boris noted HVMLite would use the PC subarch -- how would we do dom0 Vs domU quirk management? If it goes the EFI route, I gather Xen instead can use the EFI configuration tables for Xen specific tunings, however we may also need a generic x86 configuration table for generic quirks I think. We may need to provide a 1-1 mapping of these quirks there, if the subarch is not used. >>> You'd >>> have the specific settings where they belong: in a subarch specific >>> source. Just do the default settings in x86_early_init_platform_quirks() >>> and let the subarch functions set the non-default values. >> >> This is a rather different approach than what I had originally tried. >> Bike shed thing -- someone just has to decide. >> >> Left up to me, I kind of really like centralizing the quirk settings >> in one place approach as it means a reader can easily tell what's >> going on regardless of platform in one place for odd settings. I >> prefer this given that we *already* have the semantics over hardware >> subarch in a generalized fashion. We *do not* have semantics for dom0 >> Vs domU -- if such a notion is generic to other virtualization > > That's not carved in stone - see above. :-) Another subarch for Xen dom0 works well for me as well given the new subarch would just all set in the kernel. It does mean extending the x86 boot protocol though, and so for that I yield to hpa. Luis
On 08/04/16 09:36, Luis R. Rodriguez wrote: > On Fri, Apr 8, 2016 at 12:13 AM, Juergen Gross <jgross@suse.com> wrote: >> On 08/04/16 08:56, Luis R. Rodriguez wrote: >>> On Thu, Apr 7, 2016 at 11:38 PM, Juergen Gross <jgross@suse.com> wrote: >>>> On 08/04/16 08:29, Luis R. Rodriguez wrote: >>>>> On Thu, Apr 7, 2016 at 10:18 PM, Juergen Gross <jgross@suse.com> wrote: >>>>>> On 08/04/16 02:32, Luis R. Rodriguez wrote: >>>>>>> This highlights a semantic gap issue. From a quick cursory review, I think >>>>>>> we can address this temporarily by just using a check: >>>>>>> >>>>>>> void __init x86_early_init_platform_quirks(void) >>>>>>> { >>>>>>> x86_platform.legacy.rtc = 1; >>>>>>> >>>>>>> switch (boot_params.hdr.hardware_subarch) { >>>>>>> case X86_SUBARCH_XEN: >>>>>>> case X86_SUBARCH_LGUEST: >>>>>>> case X86_SUBARCH_INTEL_MID: >>>>>>> - x86_platform.legacy.rtc = 0; >>>>>>> + if (x86_init.mpparse.get_smp_config != x86_init_uint_noop) >>>>>>> + x86_platform.legacy.rtc = 0; >>>>>> >>>>>> No! Why don't you just use the explicit test xen_initial_domain() ? >>>>> >>>>> Because we don't want to sprinkle Xen specific code outside of Xen >>>>> code. What do you think about the second possibility I listed? >>>>> Otherwise, any other ideas? >>>> >>>> Don't try to guess. >>> >>> I can only do that given there is nothing at all to tell me what to >>> expect here with regards to RTC on Xen guest, if there is some >>> documentation that could help with that please let me know. >> >> Only Xen inernals. :-) > > Where can I look at this specifically on the Xen sources? No worries > if you don't care -- as I don't really either. Just look how xen_initial_domain() is defined. :-) >>>> In case you don't want to inject Xen internals here, just call a Xen >>>> function to either return the correct value, or to set all structure >>>> elements correctly. >>> >>> I like the later as an option, in case there are further hardware >>> subarch specific quirks which require internal logistics. What do >>> others think? >>> >>>> Thinking more about it: why not do that for all the subarchs? >>> >>> I originally had went with that approach, but Ingo made the point that >>> it would be best to instead move all quirk settings into one place. >>> That lets a reader easily tell what is going on in one place, it also >>> compartmentalizes the hardware subarch uses. >> >> Okay. Another idea (not sure whether this is really a good one): >> >> Add X86_SUBARCH_XEN_DOM0. As hardware_subarch is 32 bits wide I don't >> think the number of subarchs is a scarce resource. :-) > > This would mean bumping the x86 boot protocol, we shouldn't take that > lightly, but given that in this case the new subarch would really only > be set by the kernel (or future loaders for perhaps HVMLite) I'd think > this is not such an intrusive alternative. I think adding an own subarch for dom0 isn't that bad. It really is different from domU as dom0 has per default access to the real hardware (or at least to most of it). >> I'd expect other quirks in future might have different settings for >> domU and dom0, too. > > Can you elaborate a bit more on this. I guess we might want to add other quirks in order to switch on/off more features instead of doing this based on #ifdef or environment tests. I'm thinking of current use, not of HVMlite specific stuff. > I realize we expect domU and > dom0 on HVMLite in the future, would HVMLite use the subarch ? From > the last discussions on the HVMLite thread Boris noted HVMLite would > use the PC subarch -- how would we do dom0 Vs domU quirk management? This would have to be settled. I think it might be a good idea to initialize the quirks to their defaults statically in x86_init.c and just modify some as needed for HVMlite on early boot (e.g. in the HVMlite or EFI stub). This will enable us to either make use of subarch or not for HVMlite, just what fits best. > If it goes the EFI route, I gather Xen instead can use the EFI > configuration tables for Xen specific tunings, however we may also > need a generic x86 configuration table for generic quirks I think. We > may need to provide a 1-1 mapping of these quirks there, if the > subarch is not used. The EFI stub can set the quirks just according to it's needs. Going this route would _require_ HVMlite is using the PC subarch, though, in order to avoid overwriting the quirks in x86_early_init_platform_quirks() later. > >>>> You'd >>>> have the specific settings where they belong: in a subarch specific >>>> source. Just do the default settings in x86_early_init_platform_quirks() >>>> and let the subarch functions set the non-default values. >>> >>> This is a rather different approach than what I had originally tried. >>> Bike shed thing -- someone just has to decide. >>> >>> Left up to me, I kind of really like centralizing the quirk settings >>> in one place approach as it means a reader can easily tell what's >>> going on regardless of platform in one place for odd settings. I >>> prefer this given that we *already* have the semantics over hardware >>> subarch in a generalized fashion. We *do not* have semantics for dom0 >>> Vs domU -- if such a notion is generic to other virtualization >> >> That's not carved in stone - see above. :-) > > Another subarch for Xen dom0 works well for me as well given the new > subarch would just all set in the kernel. It does mean extending the > x86 boot protocol though, and so for that I yield to hpa. Fair enough. Juergen
On 04/08/2016 02:29 AM, Luis R. Rodriguez wrote: > On Thu, Apr 7, 2016 at 10:18 PM, Juergen Gross <jgross@suse.com> wrote: >> On 08/04/16 02:32, Luis R. Rodriguez wrote: >>> On Thu, Apr 07, 2016 at 08:55:54AM -0400, Boris Ostrovsky wrote: >>>> On 04/06/2016 08:06 PM, Luis R. Rodriguez wrote: >>>>> We have 4 types of x86 platforms that disable RTC: >>>>> >>>>> * Intel MID >>>>> * Lguest - uses paravirt >>>>> * Xen dom-U - uses paravirt >>>>> * x86 on legacy systems annotated with an ACPI legacy flag >>>>> >>>>> We can consolidate all of these into a platform specific legacy >>>>> quirk set early in boot through i386_start_kernel() and through >>>>> x86_64_start_reservations(). This deals with the RTC quirks which >>>>> we can rely on through the hardware subarch, the ACPI check can >>>>> be dealt with separately. >>>>> >>>>> v2: split the subarch check from the ACPI check, clarify >>>>> on the ACPI change commit log why ordering works >>>>> >>>>> Suggested-by: Ingo Molnar <mingo@kernel.org> >>>>> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> >>> <-- snip --> >>> >>>>> diff --git a/arch/x86/kernel/platform-quirks.c b/arch/x86/kernel/platform-quirks.c >>>>> new file mode 100644 >>>>> index 000000000000..1b114ac5996f >>>>> --- /dev/null >>>>> +++ b/arch/x86/kernel/platform-quirks.c >>>>> @@ -0,0 +1,18 @@ >>>>> +#include <linux/kernel.h> >>>>> +#include <linux/init.h> >>>>> + >>>>> +#include <asm/setup.h> >>>>> +#include <asm/bios_ebda.h> >>>>> + >>>>> +void __init x86_early_init_platform_quirks(void) >>>>> +{ >>>>> + x86_platform.legacy.rtc = 1; >>>>> + >>>>> + switch (boot_params.hdr.hardware_subarch) { >>>>> + case X86_SUBARCH_XEN: >>>>> + case X86_SUBARCH_LGUEST: >>>>> + case X86_SUBARCH_INTEL_MID: >>>>> + x86_platform.legacy.rtc = 0; >>>>> + break; >>>>> + } >>>>> +} >>>> What about Xen dom0 (aka initial domain)? >>> Indeed, thanks for catching this, the hunk below removes the re-enablement of >>> the the RTC for dom0: >>> >>>>> --- a/arch/x86/xen/enlighten.c >>>>> +++ b/arch/x86/xen/enlighten.c >>>>> @@ -1192,7 +1192,6 @@ static const struct pv_info xen_info __initconst = { >>>>> #ifdef CONFIG_X86_64 >>>>> .extra_user_64bit_cs = FLAT_USER_CS64, >>>>> #endif >>>>> - .features = 0, >>>>> .name = "Xen", >>>>> }; >>>>> @@ -1525,8 +1524,6 @@ asmlinkage __visible void __init xen_start_kernel(void) >>>>> /* Install Xen paravirt ops */ >>>>> pv_info = xen_info; >>>>> - if (xen_initial_domain()) >>>>> - pv_info.features |= PV_SUPPORTED_RTC; >>>>> pv_init_ops = xen_init_ops; >>>>> if (!xen_pvh_domain()) { >>>>> pv_cpu_ops = xen_cpu_ops; >>> This should then break dom0 unless of course you have the respective next >>> patch applied and that disabled the RTC due to an ACPI setting on your >>> platform. Juergen, can you check to see if that was the case for your >>> testing platform on dom0 ? >> Are you sure it would break? > No, suspected that it should though. > >> Wouldn't it just fall back to another >> clock source, e.g. hpet? > I suppose so. > >> I looked into my test system: seems as if add_rtc_cmos() is returning >> before the .legacy.rtc test. > OK thanks... It works because the clock must have been discovered by ACPI prior to add_rtc_cmos() call. It's PNP0b00 object, I believe. The rest of the routine is to handle the case when RTC is not found in ACPI tables for whatever reasons (I think). That's why we added paravirt_has(RTC) --- dom0 should be able to handle such cases, just like bare metal. -boris
On 04/08/2016 03:59 AM, Juergen Gross wrote: > On 08/04/16 09:36, Luis R. Rodriguez wrote: >> On Fri, Apr 8, 2016 at 12:13 AM, Juergen Gross <jgross@suse.com> wrote: >>> On 08/04/16 08:56, Luis R. Rodriguez wrote: >>>> On Thu, Apr 7, 2016 at 11:38 PM, Juergen Gross <jgross@suse.com> wrote: >>>> >>>> Okay. Another idea (not sure whether this is really a good one): >>>> >>>> Add X86_SUBARCH_XEN_DOM0. As hardware_subarch is 32 bits wide I don't >>>> think the number of subarchs is a scarce resource. :-) >> This would mean bumping the x86 boot protocol, we shouldn't take that >> lightly, but given that in this case the new subarch would really only >> be set by the kernel (or future loaders for perhaps HVMLite) I'd think >> this is not such an intrusive alternative. > I think adding an own subarch for dom0 isn't that bad. It really is > different from domU as dom0 has per default access to the real hardware > (or at least to most of it). Can we do this (overwrite quirks) in x86_init_ops.arch_setup? I'd really like to avoid adding a what essentially is a sub-subarch. -boris
On Fri, Apr 08, 2016 at 08:37:44AM -0400, Boris Ostrovsky wrote: > On 04/08/2016 03:59 AM, Juergen Gross wrote: > >On 08/04/16 09:36, Luis R. Rodriguez wrote: > >>On Fri, Apr 8, 2016 at 12:13 AM, Juergen Gross <jgross@suse.com> wrote: > >>>On 08/04/16 08:56, Luis R. Rodriguez wrote: > >>>>On Thu, Apr 7, 2016 at 11:38 PM, Juergen Gross <jgross@suse.com> wrote: > >>>> > >>>>Okay. Another idea (not sure whether this is really a good one): > >>>> > >>>>Add X86_SUBARCH_XEN_DOM0. As hardware_subarch is 32 bits wide I don't > >>>>think the number of subarchs is a scarce resource. :-) > >>This would mean bumping the x86 boot protocol, we shouldn't take that > >>lightly, but given that in this case the new subarch would really only > >>be set by the kernel (or future loaders for perhaps HVMLite) I'd think > >>this is not such an intrusive alternative. > >I think adding an own subarch for dom0 isn't that bad. It really is > >different from domU as dom0 has per default access to the real hardware > >(or at least to most of it). > > Can we do this (overwrite quirks) in x86_init_ops.arch_setup? I'd > really like to avoid adding a what essentially is a sub-subarch. I'm going with this. Will respin. Luis
On Thu, Apr 7, 2016 at 2:42 AM, David Vrabel <david.vrabel@citrix.com> wrote: > On 07/04/16 01:06, Luis R. Rodriguez wrote: >> We have 4 types of x86 platforms that disable RTC: >> >> * Intel MID >> * Lguest - uses paravirt >> * Xen dom-U - uses paravirt >> * x86 on legacy systems annotated with an ACPI legacy flag >> >> We can consolidate all of these into a platform specific legacy >> quirk set early in boot through i386_start_kernel() and through >> x86_64_start_reservations(). This deals with the RTC quirks which >> we can rely on through the hardware subarch, the ACPI check can >> be dealt with separately. > > Xen parts: > > Reviewed-by: David Vrabel <david.vrabel@citrix.com> So for instance, I dropped this Reviewed-by given that after this someone pointed out dom0 as an issue and then I addressed that. Hope to get a Reviewed-by or Acked-by for v6 then, which will also have the added __init on xen_dom0_set_legacy_features() as requested by Juergen. Luis
diff --git a/arch/x86/Makefile b/arch/x86/Makefile index 4086abca0b32..f9ed8a7ce2b6 100644 --- a/arch/x86/Makefile +++ b/arch/x86/Makefile @@ -209,6 +209,7 @@ endif head-y := arch/x86/kernel/head_$(BITS).o head-y += arch/x86/kernel/head$(BITS).o head-y += arch/x86/kernel/head.o +head-y += arch/x86/kernel/platform-quirks.o libs-y += arch/x86/lib/ diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h index 601f1b8f9961..6c7a4a192032 100644 --- a/arch/x86/include/asm/paravirt.h +++ b/arch/x86/include/asm/paravirt.h @@ -20,12 +20,6 @@ static inline int paravirt_enabled(void) return pv_info.paravirt_enabled; } -static inline int paravirt_has_feature(unsigned int feature) -{ - WARN_ON_ONCE(!pv_info.paravirt_enabled); - return (pv_info.features & feature); -} - static inline void load_sp0(struct tss_struct *tss, struct thread_struct *thread) { diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h index e8c2326478c8..6acc1b26cf40 100644 --- a/arch/x86/include/asm/paravirt_types.h +++ b/arch/x86/include/asm/paravirt_types.h @@ -70,14 +70,9 @@ struct pv_info { #endif int paravirt_enabled; - unsigned int features; /* valid only if paravirt_enabled is set */ const char *name; }; -#define paravirt_has(x) paravirt_has_feature(PV_SUPPORTED_##x) -/* Supported features */ -#define PV_SUPPORTED_RTC (1<<0) - struct pv_init_ops { /* * Patch may replace one of the defined code sequences with diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index 9264476f3d57..0c70c7daa6b8 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -474,7 +474,6 @@ static inline unsigned long current_top_of_stack(void) #else #define __cpuid native_cpuid #define paravirt_enabled() 0 -#define paravirt_has(x) 0 static inline void load_sp0(struct tss_struct *tss, struct thread_struct *thread) diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h index 1ae89a2721d6..27d5c3fe5198 100644 --- a/arch/x86/include/asm/x86_init.h +++ b/arch/x86/include/asm/x86_init.h @@ -142,6 +142,15 @@ struct x86_cpuinit_ops { struct timespec; /** + * struct x86_legacy_features - legacy x86 features + * + * @rtc: this device has a CMOS real-time clock present + */ +struct x86_legacy_features { + int rtc; +}; + +/** * struct x86_platform_ops - platform specific runtime functions * @calibrate_tsc: calibrate TSC * @get_wallclock: get time from HW clock like RTC etc. @@ -152,6 +161,7 @@ struct timespec; * @save_sched_clock_state: save state for sched_clock() on suspend * @restore_sched_clock_state: restore state for sched_clock() on resume * @apic_post_init: adjust apic if neeeded + * @legacy: legacy features */ struct x86_platform_ops { unsigned long (*calibrate_tsc)(void); @@ -165,6 +175,7 @@ struct x86_platform_ops { void (*save_sched_clock_state)(void); void (*restore_sched_clock_state)(void); void (*apic_post_init)(void); + struct x86_legacy_features legacy; }; struct pci_dev; @@ -186,6 +197,8 @@ extern struct x86_cpuinit_ops x86_cpuinit; extern struct x86_platform_ops x86_platform; extern struct x86_msi_ops x86_msi; extern struct x86_io_apic_ops x86_io_apic_ops; + +extern void x86_early_init_platform_quirks(void); extern void x86_init_noop(void); extern void x86_init_uint_noop(unsigned int unused); diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile index 89f8ade0bc7c..7a9e44d935de 100644 --- a/arch/x86/kernel/Makefile +++ b/arch/x86/kernel/Makefile @@ -2,7 +2,11 @@ # Makefile for the linux kernel. # -extra-y := head_$(BITS).o head$(BITS).o head.o vmlinux.lds +extra-y := head_$(BITS).o +extra-y += head$(BITS).o +extra-y += head.o +extra-y += platform-quirks.o +extra-y += vmlinux.lds CPPFLAGS_vmlinux.lds += -U$(UTS_MACHINE) diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c index 2911ef3a9f1c..d784bb547a9d 100644 --- a/arch/x86/kernel/head32.c +++ b/arch/x86/kernel/head32.c @@ -34,6 +34,8 @@ asmlinkage __visible void __init i386_start_kernel(void) cr4_init_shadow(); sanitize_boot_params(&boot_params); + x86_early_init_platform_quirks(); + /* Call the subarch specific early setup function */ switch (boot_params.hdr.hardware_subarch) { case X86_SUBARCH_INTEL_MID: diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c index 1f4422d5c8d0..b72fb0b71dd1 100644 --- a/arch/x86/kernel/head64.c +++ b/arch/x86/kernel/head64.c @@ -182,6 +182,7 @@ void __init x86_64_start_reservations(char *real_mode_data) if (!boot_params.hdr.version) copy_bootdata(__va(real_mode_data)); + x86_early_init_platform_quirks(); reserve_ebda_region(); switch (boot_params.hdr.hardware_subarch) { diff --git a/arch/x86/kernel/platform-quirks.c b/arch/x86/kernel/platform-quirks.c new file mode 100644 index 000000000000..1b114ac5996f --- /dev/null +++ b/arch/x86/kernel/platform-quirks.c @@ -0,0 +1,18 @@ +#include <linux/kernel.h> +#include <linux/init.h> + +#include <asm/setup.h> +#include <asm/bios_ebda.h> + +void __init x86_early_init_platform_quirks(void) +{ + x86_platform.legacy.rtc = 1; + + switch (boot_params.hdr.hardware_subarch) { + case X86_SUBARCH_XEN: + case X86_SUBARCH_LGUEST: + case X86_SUBARCH_INTEL_MID: + x86_platform.legacy.rtc = 0; + break; + } +} diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c index 4af8d063fb36..62c48da3889d 100644 --- a/arch/x86/kernel/rtc.c +++ b/arch/x86/kernel/rtc.c @@ -14,6 +14,7 @@ #include <asm/time.h> #include <asm/intel-mid.h> #include <asm/rtc.h> +#include <asm/setup.h> #ifdef CONFIG_X86_32 /* @@ -188,10 +189,6 @@ static __init int add_rtc_cmos(void) if (of_have_populated_dt()) return 0; - /* Intel MID platforms don't have ioport rtc */ - if (intel_mid_identify_cpu()) - return -ENODEV; - #ifdef CONFIG_ACPI if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) { /* This warning can likely go away again in a year or two. */ @@ -200,7 +197,7 @@ static __init int add_rtc_cmos(void) } #endif - if (paravirt_enabled() && !paravirt_has(RTC)) + if (!x86_platform.legacy.rtc) return -ENODEV; platform_device_register(&rtc_device); diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c index fd57d3ae7e16..f5497ee5fd2f 100644 --- a/arch/x86/lguest/boot.c +++ b/arch/x86/lguest/boot.c @@ -1414,7 +1414,6 @@ __init void lguest_init(void) pv_info.kernel_rpl = 1; /* Everyone except Xen runs with this set. */ pv_info.shared_kernel_pmd = 1; - pv_info.features = 0; /* * We set up all the lguest overrides for sensitive operations. These diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index 40487f1ecb4c..7449f268d687 100644 --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -1192,7 +1192,6 @@ static const struct pv_info xen_info __initconst = { #ifdef CONFIG_X86_64 .extra_user_64bit_cs = FLAT_USER_CS64, #endif - .features = 0, .name = "Xen", }; @@ -1525,8 +1524,6 @@ asmlinkage __visible void __init xen_start_kernel(void) /* Install Xen paravirt ops */ pv_info = xen_info; - if (xen_initial_domain()) - pv_info.features |= PV_SUPPORTED_RTC; pv_init_ops = xen_init_ops; if (!xen_pvh_domain()) { pv_cpu_ops = xen_cpu_ops;
We have 4 types of x86 platforms that disable RTC: * Intel MID * Lguest - uses paravirt * Xen dom-U - uses paravirt * x86 on legacy systems annotated with an ACPI legacy flag We can consolidate all of these into a platform specific legacy quirk set early in boot through i386_start_kernel() and through x86_64_start_reservations(). This deals with the RTC quirks which we can rely on through the hardware subarch, the ACPI check can be dealt with separately. v2: split the subarch check from the ACPI check, clarify on the ACPI change commit log why ordering works Suggested-by: Ingo Molnar <mingo@kernel.org> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> --- arch/x86/Makefile | 1 + arch/x86/include/asm/paravirt.h | 6 ------ arch/x86/include/asm/paravirt_types.h | 5 ----- arch/x86/include/asm/processor.h | 1 - arch/x86/include/asm/x86_init.h | 13 +++++++++++++ arch/x86/kernel/Makefile | 6 +++++- arch/x86/kernel/head32.c | 2 ++ arch/x86/kernel/head64.c | 1 + arch/x86/kernel/platform-quirks.c | 18 ++++++++++++++++++ arch/x86/kernel/rtc.c | 7 ++----- arch/x86/lguest/boot.c | 1 - arch/x86/xen/enlighten.c | 3 --- 12 files changed, 42 insertions(+), 22 deletions(-) create mode 100644 arch/x86/kernel/platform-quirks.c