On Thu, Apr 7, 2016 at 10:18 PM, Juergen Gross <jgross@xxxxxxxx> 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@xxxxxxxxxx> >>>> Signed-off-by: Luis R. Rodriguez <mcgrof@xxxxxxxxxx> >> >> <-- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html