Re: [Xen-devel] [PATCH v3 10/11] x86/rtc: replace paravirt rtc check with x86 specific solution

[Luis R. Rodriguez]

On Tue, Feb 23, 2016 at 11:57:16AM +, David Vrabel wrote:
> On 23/02/16 07:24, Luis R. Rodriguez wrote:
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> [...]
> > @@ -1526,8 +1525,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;
> > diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> > index a0a4e554c6f1..fbfcb01015f0 100644
> > --- a/arch/x86/xen/time.c
> > +++ b/arch/x86/xen/time.c
> > @@ -446,8 +446,10 @@ void __init xen_init_time_ops(void)
> > x86_platform.calibrate_tsc = xen_tsc_khz;
> > x86_platform.get_wallclock = xen_get_wallclock;
> > /* Dom0 uses the native method to set the hardware RTC. */
> > -   if (!xen_initial_domain())
> > +   if (!xen_initial_domain()) {
> > +   x86_init.platform_flags |= X86_PLATFORM_NO_RTC;
> > x86_platform.set_wallclock = xen_set_wallclock;
> > +   }
> >  }
> 
> Is this an early enough point to set this flag?

I'm glad you asked, I should have explained how I confirmed this on the commit
log as well. The answer is yes, even though I haven't tested it, but logically
I've confirmed this given that rtc is initialized via 
device_initcall(add_rtc_cmos);
-- these get called late in boot, way after setup_arch() during rest_init().

  Luis

Re: [Xen-devel] [PATCH v3 10/11] x86/rtc: replace paravirt rtc check with x86 specific solution

[Luis R. Rodriguez]

On Tue, Feb 23, 2016 at 11:57:16AM +, David Vrabel wrote:
> On 23/02/16 07:24, Luis R. Rodriguez wrote:
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> [...]
> > @@ -1526,8 +1525,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;
> > diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> > index a0a4e554c6f1..fbfcb01015f0 100644
> > --- a/arch/x86/xen/time.c
> > +++ b/arch/x86/xen/time.c
> > @@ -446,8 +446,10 @@ void __init xen_init_time_ops(void)
> > x86_platform.calibrate_tsc = xen_tsc_khz;
> > x86_platform.get_wallclock = xen_get_wallclock;
> > /* Dom0 uses the native method to set the hardware RTC. */
> > -   if (!xen_initial_domain())
> > +   if (!xen_initial_domain()) {
> > +   x86_init.platform_flags |= X86_PLATFORM_NO_RTC;
> > x86_platform.set_wallclock = xen_set_wallclock;
> > +   }
> >  }
> 
> Is this an early enough point to set this flag?

I'm glad you asked, I should have explained how I confirmed this on the commit
log as well. The answer is yes, even though I haven't tested it, but logically
I've confirmed this given that rtc is initialized via 
device_initcall(add_rtc_cmos);
-- these get called late in boot, way after setup_arch() during rest_init().

  Luis

Re: [Xen-devel] [PATCH v3 10/11] x86/rtc: replace paravirt rtc check with x86 specific solution

[David Vrabel]

On 23/02/16 07:24, Luis R. Rodriguez wrote:
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
[...]
> @@ -1526,8 +1525,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;
> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> index a0a4e554c6f1..fbfcb01015f0 100644
> --- a/arch/x86/xen/time.c
> +++ b/arch/x86/xen/time.c
> @@ -446,8 +446,10 @@ void __init xen_init_time_ops(void)
>   x86_platform.calibrate_tsc = xen_tsc_khz;
>   x86_platform.get_wallclock = xen_get_wallclock;
>   /* Dom0 uses the native method to set the hardware RTC. */
> - if (!xen_initial_domain())
> + if (!xen_initial_domain()) {
> + x86_init.platform_flags |= X86_PLATFORM_NO_RTC;
>   x86_platform.set_wallclock = xen_set_wallclock;
> + }
>  }

Is this an early enough point to set this flag?

David

Re: [Xen-devel] [PATCH v3 10/11] x86/rtc: replace paravirt rtc check with x86 specific solution

[David Vrabel]

On 23/02/16 07:24, Luis R. Rodriguez wrote:
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
[...]
> @@ -1526,8 +1525,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;
> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> index a0a4e554c6f1..fbfcb01015f0 100644
> --- a/arch/x86/xen/time.c
> +++ b/arch/x86/xen/time.c
> @@ -446,8 +446,10 @@ void __init xen_init_time_ops(void)
>   x86_platform.calibrate_tsc = xen_tsc_khz;
>   x86_platform.get_wallclock = xen_get_wallclock;
>   /* Dom0 uses the native method to set the hardware RTC. */
> - if (!xen_initial_domain())
> + if (!xen_initial_domain()) {
> + x86_init.platform_flags |= X86_PLATFORM_NO_RTC;
>   x86_platform.set_wallclock = xen_set_wallclock;
> + }
>  }

Is this an early enough point to set this flag?

David

[PATCH v3 10/11] x86/rtc: replace paravirt rtc check with x86 specific solution

[Luis R. Rodriguez]

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 solution.

Signed-off-by: Luis R. Rodriguez 
---
 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 | 12 
 arch/x86/kernel/acpi/boot.c |  4 
 arch/x86/kernel/rtc.c   | 15 ++-
 arch/x86/lguest/boot.c  |  2 +-
 arch/x86/platform/intel-mid/intel-mid.c |  3 +++
 arch/x86/xen/enlighten.c|  3 ---
 arch/x86/xen/time.c |  4 +++-
 10 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index f6192502149e..c261402340e3 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -19,12 +19,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 77db5616a473..2489d6a08e89 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 20c11d1aa4cc..10f3614265c1 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -472,7 +472,6 @@ static inline unsigned long current_top_of_stack(void)
 #else
 #define __cpuidnative_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..0ef697e46f6e 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -1,6 +1,7 @@
 #ifndef _ASM_X86_PLATFORM_H
 #define _ASM_X86_PLATFORM_H
 
+#include 
 #include 
 
 struct mpc_bus;
@@ -116,6 +117,7 @@ struct x86_init_pci {
 /**
  * struct x86_init_ops - functions for platform specific setup
  *
+ * @platform_flags:bitmask of enum x86_platform_flags.
  */
 struct x86_init_ops {
struct x86_init_resources   resources;
@@ -126,6 +128,7 @@ struct x86_init_ops {
struct x86_init_timers  timers;
struct x86_init_iommu   iommu;
struct x86_init_pci pci;
+   u64 platform_flags;
 };
 
 /**
@@ -142,6 +145,15 @@ struct x86_cpuinit_ops {
 struct timespec;
 
 /**
+ * enum x86_platform_flags - x86 platform flags
+ *
+ * X86_PLATFORM_NO_RTC: set when platform has no CMOS real-time clock present
+ */
+enum x86_platform_flags {
+   X86_PLATFORM_NO_RTC = BIT(0),
+};
+
+/**
  * struct x86_platform_ops - platform specific runtime functions
  * @calibrate_tsc: calibrate TSC
  * @get_wallclock: get time from HW clock like RTC etc.
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index e75907601a41..d156f6cd2be3 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -913,6 +913,10 @@ late_initcall(hpet_insert_resource);
 
 static int __init acpi_parse_fadt(struct acpi_table_header *table)
 {
+   if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) {
+   pr_debug("ACPI: not registering RTC platform device\n");
+   x86_init.platform_flags |= X86_PLATFORM_NO_RTC;
+   }
 
 #ifdef CONFIG_X86_PM_TIMER
/* detect the location of the ACPI PM Timer */
diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
index 4af8d063fb36..5f95e42ecc68 100644
--- a/arch/x86/kernel/rtc.c
+++ b/arch/x86/kernel/rtc.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifdef CONFIG_X86_32
 /*
@@ -188,19 +189,7 @@ 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 

[PATCH v3 10/11] x86/rtc: replace paravirt rtc check with x86 specific solution

[Luis R. Rodriguez]

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 solution.

Signed-off-by: Luis R. Rodriguez 
---
 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 | 12 
 arch/x86/kernel/acpi/boot.c |  4 
 arch/x86/kernel/rtc.c   | 15 ++-
 arch/x86/lguest/boot.c  |  2 +-
 arch/x86/platform/intel-mid/intel-mid.c |  3 +++
 arch/x86/xen/enlighten.c|  3 ---
 arch/x86/xen/time.c |  4 +++-
 10 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index f6192502149e..c261402340e3 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -19,12 +19,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 77db5616a473..2489d6a08e89 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 20c11d1aa4cc..10f3614265c1 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -472,7 +472,6 @@ static inline unsigned long current_top_of_stack(void)
 #else
 #define __cpuidnative_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..0ef697e46f6e 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -1,6 +1,7 @@
 #ifndef _ASM_X86_PLATFORM_H
 #define _ASM_X86_PLATFORM_H
 
+#include 
 #include 
 
 struct mpc_bus;
@@ -116,6 +117,7 @@ struct x86_init_pci {
 /**
  * struct x86_init_ops - functions for platform specific setup
  *
+ * @platform_flags:bitmask of enum x86_platform_flags.
  */
 struct x86_init_ops {
struct x86_init_resources   resources;
@@ -126,6 +128,7 @@ struct x86_init_ops {
struct x86_init_timers  timers;
struct x86_init_iommu   iommu;
struct x86_init_pci pci;
+   u64 platform_flags;
 };
 
 /**
@@ -142,6 +145,15 @@ struct x86_cpuinit_ops {
 struct timespec;
 
 /**
+ * enum x86_platform_flags - x86 platform flags
+ *
+ * X86_PLATFORM_NO_RTC: set when platform has no CMOS real-time clock present
+ */
+enum x86_platform_flags {
+   X86_PLATFORM_NO_RTC = BIT(0),
+};
+
+/**
  * struct x86_platform_ops - platform specific runtime functions
  * @calibrate_tsc: calibrate TSC
  * @get_wallclock: get time from HW clock like RTC etc.
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index e75907601a41..d156f6cd2be3 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -913,6 +913,10 @@ late_initcall(hpet_insert_resource);
 
 static int __init acpi_parse_fadt(struct acpi_table_header *table)
 {
+   if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) {
+   pr_debug("ACPI: not registering RTC platform device\n");
+   x86_init.platform_flags |= X86_PLATFORM_NO_RTC;
+   }
 
 #ifdef CONFIG_X86_PM_TIMER
/* detect the location of the ACPI PM Timer */
diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
index 4af8d063fb36..5f95e42ecc68 100644
--- a/arch/x86/kernel/rtc.c
+++ b/arch/x86/kernel/rtc.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifdef CONFIG_X86_32
 /*
@@ -188,19 +189,7 @@ 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
-