Re: [Xen-devel] [PATCH 8/9] x86/rtc: replace paravirt_enabled() check with subarch check

["Luis R. Rodriguez" <mcgrof@xxxxxxxxxx>]

On Fri, Feb 19, 2016 at 03:48:41PM +0100, Luis R. Rodriguez wrote:
> On Fri, Feb 19, 2016 at 02:22:12PM +0100, Juergen Gross wrote:
> > On 19/02/16 14:08, Luis R. Rodriguez wrote:
> > > The current check is a super long winded way of asking if this
> > > is on lguest. The flags is used for legacy features, this is
> > 
> > What about Xen pv-domU? I wouldn't expect those to have PV_SUPPORTED_RTC
> > set.
> 
> Hrm, I see -- how do we check for that in a standard more clean way?

OK we have 4 types of x86 platforms that disable RTC:

  * Intel MID
  * Lguest
  * Xen dom-U
  * x86 on legacy systems annotated with an ACPI legacy flag

So it would seem its best to just generalize the disabling of
the RTC for any x86 platform, we could also fold the ACPI check
into the FADT parse table routine, which is called during
setup_arch().

So how about this:

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 __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..fe0d579b63e3 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -84,11 +84,14 @@ struct x86_init_paging {
  *                             boot cpu
  * @timer_init:                        initialize the platform timer (default 
PIT/HPET)
  * @wallclock_init:            init the wallclock device
+ * @no_cmos_rtc:               set when platform has no CMOS real-time clock
+ *                             present
  */
 struct x86_init_timers {
        void (*setup_percpu_clockev)(void);
        void (*timer_init)(void);
        void (*wallclock_init)(void);
+       bool no_cmos_rtc;
 };
 
 /**
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index e75907601a41..6b2cac0f276b 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.timers.no_cmos_rtc = true;
+       }
 
 #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..ef92aa84c2e3 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,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
-       if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) {
-               /* This warning can likely go away again in a year or two. */
-               pr_info("ACPI: not registering RTC platform device\n");
-               return -ENODEV;
-       }
-#endif
-
-       if (paravirt_enabled() && !paravirt_has(RTC))
+       if (x86_init.timers.no_cmos_rtc)
                return -ENODEV;
 
        platform_device_register(&rtc_device);
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index 4ba229ac3f4f..62535a869e06 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
@@ -1482,6 +1481,7 @@ __init void lguest_init(void)
        x86_init.resources.memory_setup = lguest_memory_setup;
        x86_init.irqs.intr_init = lguest_init_IRQ;
        x86_init.timers.timer_init = lguest_time_init;
+       x86_init.timers.no_cmos_rtc = true;
        x86_platform.calibrate_tsc = lguest_tsc_khz;
        x86_platform.get_wallclock =  lguest_get_wallclock;
 
diff --git a/arch/x86/platform/intel-mid/intel-mid.c 
b/arch/x86/platform/intel-mid/intel-mid.c
index 90bb997ed0a2..083369a6b215 100644
--- a/arch/x86/platform/intel-mid/intel-mid.c
+++ b/arch/x86/platform/intel-mid/intel-mid.c
@@ -175,6 +175,9 @@ void __init x86_intel_mid_early_setup(void)
        x86_init.timers.timer_init = intel_mid_time_init;
        x86_init.timers.setup_percpu_clockev = x86_init_noop;
 
+       /* Intel MID platforms don't have ioport rtc */
+       x86_init.timers.no_cmos_rtc = true;
+
        x86_init.irqs.pre_vector_init = x86_init_noop;
 
        x86_init.oem.arch_setup = intel_mid_arch_setup;
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 5b3f1c763806..5c06169bce27 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",
 };
 
@@ -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..963cdaf63496 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.timers.no_cmos_rtc = true;
                x86_platform.set_wallclock = xen_set_wallclock;
+       }
 }
 
 #ifdef CONFIG_XEN_PVHVM

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