[PATCH v4 00/14] x86: remove paravirt_enabled

[PATCH v4 00/14] x86: remove paravirt_enabled

[Luis R. Rodriguez]

Now that Andy's ASM paravirt_enabled() use is merged all we need is to address
the rest of the C code uses. This completes that work by providing proper
semantics for platform legacy settings and quirks as suggested by Ingo, this in
turn can also be extended later for benefit of further processing of ACPI
5.2.9.3 IA-PC Boot Architecture flags, which we currently don't take much
advantage of.  For instance the ACPI_FADT_NO_VGA can later be leveraged by bare
metal x86 *and* HVMLite, as HVMLite seems to plan to set this.

Also, hpa has noted both Intel MID and CE4100 can make use of disabling
pnpbios, we can do that separately after this, but it should now be a
trivial change, generic given this quirk stuff is all generic now.

This patches goes tested by 0-day, except for the last patch, for some reason
the branch that included that patch has had testing delayed for quite a
while now, but I can't think of anything there that should break anything.

I've also just run time tested this on bare metal only so far.

Luis R. Rodriguez (14):
  x86/boot: enumerate documentation for the x86 hardware_subarch
  x86/xen: use X86_SUBARCH_XEN for PV guest boots
  tools/lguest: make lguest launcher use X86_SUBARCH_LGUEST explicitly
  x86/rtc: replace paravirt rtc check with platform legacy quirk
  x86, ACPI: move ACPI_FADT_NO_CMOS_RTC check to ACPI boot code
  x86/init: use a platform legacy quirk for ebda
  tools/lguest: force disable tboot and apm
  apm32: remove paravirt_enabled() use
  x86/tboot: remove paravirt_enabled()
  x86/cpu/intel: remove not needed paravirt_enabled() for f00f work
    around
  pnpbios: replace paravirt_enabled() check with legacy device check
  x86, ACPI: parse ACPI_FADT_LEGACY_DEVICES
  x86/init: rename ebda code file
  x86/paravirt: remove paravirt_enabled()

 arch/x86/Makefile                     |  3 ++-
 arch/x86/include/asm/paravirt.h       | 11 ---------
 arch/x86/include/asm/paravirt_types.h |  6 -----
 arch/x86/include/asm/processor.h      |  2 --
 arch/x86/include/asm/x86_init.h       | 42 +++++++++++++++++++++++++++++++++++
 arch/x86/include/uapi/asm/bootparam.h | 36 +++++++++++++++++++++++++++++-
 arch/x86/kernel/Makefile              |  6 ++++-
 arch/x86/kernel/acpi/boot.c           |  9 ++++++++
 arch/x86/kernel/apm_32.c              |  2 +-
 arch/x86/kernel/cpu/intel.c           |  2 +-
 arch/x86/kernel/{head.c => ebda.c}    |  2 +-
 arch/x86/kernel/head32.c              |  2 ++
 arch/x86/kernel/head64.c              |  1 +
 arch/x86/kernel/kvm.c                 |  8 -------
 arch/x86/kernel/paravirt.c            |  1 -
 arch/x86/kernel/platform-quirks.c     | 32 ++++++++++++++++++++++++++
 arch/x86/kernel/rtc.c                 | 15 ++-----------
 arch/x86/kernel/tboot.c               |  6 -----
 arch/x86/lguest/boot.c                |  3 ---
 arch/x86/xen/enlighten.c              |  5 +----
 drivers/pnp/pnpbios/core.c            |  3 ++-
 include/linux/pnp.h                   |  2 ++
 tools/lguest/lguest.c                 | 10 +++++++--
 23 files changed, 146 insertions(+), 63 deletions(-)
 rename arch/x86/kernel/{head.c => ebda.c} (98%)
 create mode 100644 arch/x86/kernel/platform-quirks.c

-- 
2.7.2

[PATCH v4 01/14] x86/boot: enumerate documentation for the x86 hardware_subarch

[Luis R. Rodriguez]

Although hardware_subarch has been in place since the x86 boot
protocol 2.07 it hasn't been used much. Enumerate current possible
values to avoid misuses and help with semantics later at boot
time should this be used further.

These enums should only ever be used by architecture x86 code,
and all that code should be well contained and compartamentalized,
clarify that as well.

v2: updates documentation further -- be a bit more pedantic about
    annotating care and use of these guys.

Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 arch/x86/include/uapi/asm/bootparam.h | 36 ++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index 329254373479..d03e2e9eefca 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -157,7 +157,41 @@ struct boot_params {
 	__u8  _pad9[276];				/* 0xeec */
 } __attribute__((packed));
 
-enum {
+/**
+ * enum x86_hardware_subarch - x86 hardware subarchitecture
+ *
+ * The x86 hardware_subarch and hardware_subarch_data were added as of the x86
+ * boot protocol 2.07 to help distinguish and support custom x86 boot
+ * sequences. This enum represents accepted values for the x86
+ * hardware_subarch.  Custom x86 boot sequences (not X86_SUBARCH_PC) do not
+ * have or simply *cannot* make use of natural stubs like BIOS or EFI, the
+ * hardware_subarch can be used on the Linux entry path to revector to a
+ * subarchitecture stub when needed. This subarchitecture stub can be used to
+ * set up Linux boot parameters or for special care to account for nonstandard
+ * handling of page tables.
+ *
+ * These enums should only ever be used by x86 code, and the code that uses
+ * it should be well contained and compartamentalized.
+ *
+ * KVM and Xen HVM do not have a subarch as these are expected to follow
+ * standard x86 boot entries. If there is a genuine need for "hypervisor" type
+ * that should be considered separately in the future. Future guest types
+ * should seriously consider working with standard x86 boot stubs such as
+ * the BIOS or EFI boot stubs.
+ *
+ * @X86_SUBARCH_PC: Should be used if the hardware is enumerable using standard
+ *	PC mechanisms (PCI, ACPI) and doesn't need a special boot flow.
+ * @X86_SUBARCH_LGUEST: Used for x86 hypervisor demo, lguest
+ * @X86_SUBARCH_XEN: Used for Xen guest types which follow the PV boot path,
+ * 	which start at asm startup_xen() entry point and later jump to the C
+ * 	xen_start_kernel() entry point.
+ * @X86_SUBARCH_INTEL_MID: Used for Intel MID (Mobile Internet Device) platform
+ *	systems which do not have the PCI legacy interfaces.
+ * @X86_SUBARCH_CE4100: Used for Intel CE media processor (CE4100) SOC for
+ * 	for settop boxes and media devices, the use of a subarch for CE4100
+ * 	is more of a hack...
+ */
+enum x86_hardware_subarch {
 	X86_SUBARCH_PC = 0,
 	X86_SUBARCH_LGUEST,
 	X86_SUBARCH_XEN,
-- 
2.7.2

Re: [PATCH v4 01/14] x86/boot: enumerate documentation for the x86 hardware_subarch

[Andy Shevchenko]

On Wed, 2016-04-06 at 17:06 -0700, Luis R. Rodriguez wrote:
> Although hardware_subarch has been in place since the x86 boot
> protocol 2.07 it hasn't been used much. Enumerate current possible
> values to avoid misuses and help with semantics later at boot
> time should this be used further.
> 
> These enums should only ever be used by architecture x86 code,
> and all that code should be well contained and compartamentalized,
> clarify that as well.

Nitpick:

> + * @X86_SUBARCH_PC: Should be used if the hardware is enumerable
> using standard
> + *	PC mechanisms (PCI, ACPI) and doesn't need a special boot
> flow.
> + * @X86_SUBARCH_LGUEST: Used for x86 hypervisor demo, lguest
> + * @X86_SUBARCH_XEN: Used for Xen guest types which follow the PV
> boot path,
> + * 	which start at asm startup_xen() entry point and later
> jump to the C
> + * 	xen_start_kernel() entry point.
> + * @X86_SUBARCH_INTEL_MID: Used for Intel MID (Mobile Internet
> Device) platform
> + *	systems which do not have the PCI legacy interfaces.
> + * @X86_SUBARCH_CE4100: Used for Intel CE media processor (CE4100)
> SOC for

I think 'SoC' (without quotes) will be better.

> + * 	for settop boxes and media devices, the use of a subarch
> for CE4100
> + * 	is more of a hack...

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v4 01/14] x86/boot: enumerate documentation for the x86 hardware_subarch

[Luis R. Rodriguez]

On Thu, Apr 07, 2016 at 02:25:38PM +0300, Andy Shevchenko wrote:
> On Wed, 2016-04-06 at 17:06 -0700, Luis R. Rodriguez wrote:
> > Although hardware_subarch has been in place since the x86 boot
> > protocol 2.07 it hasn't been used much. Enumerate current possible
> > values to avoid misuses and help with semantics later at boot
> > time should this be used further.
> > 
> > These enums should only ever be used by architecture x86 code,
> > and all that code should be well contained and compartamentalized,
> > clarify that as well.
> 
> Nitpick:
> 
> > + * @X86_SUBARCH_PC: Should be used if the hardware is enumerable
> > using standard
> > + *	PC mechanisms (PCI, ACPI) and doesn't need a special boot
> > flow.
> > + * @X86_SUBARCH_LGUEST: Used for x86 hypervisor demo, lguest
> > + * @X86_SUBARCH_XEN: Used for Xen guest types which follow the PV
> > boot path,
> > + * 	which start at asm startup_xen() entry point and later
> > jump to the C
> > + * 	xen_start_kernel() entry point.
> > + * @X86_SUBARCH_INTEL_MID: Used for Intel MID (Mobile Internet
> > Device) platform
> > + *	systems which do not have the PCI legacy interfaces.
> > + * @X86_SUBARCH_CE4100: Used for Intel CE media processor (CE4100)
> > SOC for
> 
> I think 'SoC' (without quotes) will be better.

Amended, since I think I'll need a re-spin and since we may need to take
care of the dom0 Vs domU semantics I'll also make some changes to include
X86_SUBARCH_XEN documentation to annotate that PV guests can be of domU
or dom0 type...

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 02/14] x86/xen: use X86_SUBARCH_XEN for PV guest boots

[Luis R. Rodriguez]

The use of subarch should have no current effect on Xen
PV guests, as such this should have no current functional
effects.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 arch/x86/xen/enlighten.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 9b8f1eacc110..40487f1ecb4c 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1661,6 +1661,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
 	boot_params.hdr.ramdisk_image = initrd_start;
 	boot_params.hdr.ramdisk_size = xen_start_info->mod_len;
 	boot_params.hdr.cmd_line_ptr = __pa(xen_start_info->cmd_line);
+	boot_params.hdr.hardware_subarch = X86_SUBARCH_XEN;
 
 	if (!xen_initial_domain()) {
 		add_preferred_console("xenboot", 0, NULL);
-- 
2.7.2


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

Re: [Xen-devel] [PATCH v4 02/14] x86/xen: use X86_SUBARCH_XEN for PV guest boots

[David Vrabel]

On 07/04/16 01:06, Luis R. Rodriguez wrote:
> The use of subarch should have no current effect on Xen
> PV guests, as such this should have no current functional
> effects.

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David

[PATCH v4 03/14] tools/lguest: make lguest launcher use X86_SUBARCH_LGUEST explicitly

[Luis R. Rodriguez]

Be explicit and make use of X86_SUBARCH_LGUEST directly.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 tools/lguest/lguest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lguest/lguest.c b/tools/lguest/lguest.c
index 80159e6811c2..ff0aa580c6e1 100644
--- a/tools/lguest/lguest.c
+++ b/tools/lguest/lguest.c
@@ -3351,8 +3351,8 @@ int main(int argc, char *argv[])
 	/* Boot protocol version: 2.07 supports the fields for lguest. */
 	boot->hdr.version = 0x207;
 
-	/* The hardware_subarch value of "1" tells the Guest it's an lguest. */
-	boot->hdr.hardware_subarch = 1;
+	/* X86_SUBARCH_LGUEST tells the Guest it's an lguest. */
+	boot->hdr.hardware_subarch = X86_SUBARCH_LGUEST;
 
 	/* Tell the entry path not to try to reload segment registers. */
 	boot->hdr.loadflags |= KEEP_SEGMENTS;
-- 
2.7.2

[PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[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 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;
+	}
+}
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;
-- 
2.7.2


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

Re: [Xen-devel] [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[David Vrabel]

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

Re: [Xen-devel] [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Luis R. Rodriguez]

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

Re: [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Boris Ostrovsky]

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;


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

Re: [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Luis R. Rodriguez]

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

Re: [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Juergen Gross]

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

Re: [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Luis R. Rodriguez]

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

Re: [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Juergen Gross]

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

Re: [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Luis R. Rodriguez]

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

Re: [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Juergen Gross]

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

Re: [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Luis R. Rodriguez]

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

Re: [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Juergen Gross]

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

Re: [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Boris Ostrovsky]

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

Re: [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Luis R. Rodriguez]

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

Re: [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Boris Ostrovsky]

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

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

[PATCH v4 05/14] x86, ACPI: move ACPI_FADT_NO_CMOS_RTC check to ACPI boot code

[Luis R. Rodriguez]

This moves the ACPI specific check into the ACPI boot code,
it also takes advantage of the x86_platform.legacy.rtc which
is checked for already on the RTC initialization code. This
lets us remove the nasty #ifdefery and consolidate the checks
to use only one toggle to disable the RTC init code.

The works as RTC is initialized by device_initcall(add_rtc_cmos),
this will run late in boot on start_kernel() during rest_init(),
acpi_parse_fadt() gets called earlier during setup_arch().

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 arch/x86/kernel/acpi/boot.c | 4 ++++
 arch/x86/kernel/rtc.c       | 8 --------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 1db7e80c26f2..5e736e2108a1 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -911,6 +911,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_platform.legacy.rtc = 0;
+	}
 
 #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 62c48da3889d..ff4f4180fefd 100644
--- a/arch/x86/kernel/rtc.c
+++ b/arch/x86/kernel/rtc.c
@@ -189,14 +189,6 @@ static __init int add_rtc_cmos(void)
 	if (of_have_populated_dt())
 		return 0;
 
-#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 (!x86_platform.legacy.rtc)
 		return -ENODEV;
 
-- 
2.7.2


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

[PATCH v4 06/14] x86/init: use a platform legacy quirk for ebda

[Luis R. Rodriguez]

This replaces the paravirt_enabled() check with a
proper x86 legacy platform quirk.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 arch/x86/include/asm/x86_init.h   | 3 +++
 arch/x86/kernel/head.c            | 2 +-
 arch/x86/kernel/platform-quirks.c | 4 ++++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 27d5c3fe5198..f3f81122ae3b 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -145,9 +145,12 @@ struct timespec;
  * struct x86_legacy_features - legacy x86 features
  *
  * @rtc: this device has a CMOS real-time clock present
+ * @ebda_search: it's safe to search for the EBDA signature in the hardware's
+ * 	low RAM
  */
 struct x86_legacy_features {
 	int rtc;
+	int ebda_search;
 };
 
 /**
diff --git a/arch/x86/kernel/head.c b/arch/x86/kernel/head.c
index 992f442ca155..afe65dffee80 100644
--- a/arch/x86/kernel/head.c
+++ b/arch/x86/kernel/head.c
@@ -38,7 +38,7 @@ void __init reserve_ebda_region(void)
 	 * that the paravirt case can handle memory setup
 	 * correctly, without our help.
 	 */
-	if (paravirt_enabled())
+	if (!x86_platform.legacy.ebda_search)
 		return;
 
 	/* end of low (conventional) memory */
diff --git a/arch/x86/kernel/platform-quirks.c b/arch/x86/kernel/platform-quirks.c
index 1b114ac5996f..a871b6b0e35f 100644
--- a/arch/x86/kernel/platform-quirks.c
+++ b/arch/x86/kernel/platform-quirks.c
@@ -7,8 +7,12 @@
 void __init x86_early_init_platform_quirks(void)
 {
 	x86_platform.legacy.rtc = 1;
+	x86_platform.legacy.ebda_search = 0;
 
 	switch (boot_params.hdr.hardware_subarch) {
+	case X86_SUBARCH_PC:
+		x86_platform.legacy.ebda_search = 1;
+		break;
 	case X86_SUBARCH_XEN:
 	case X86_SUBARCH_LGUEST:
 	case X86_SUBARCH_INTEL_MID:
-- 
2.7.2


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

Re: [Xen-devel] [PATCH v4 06/14] x86/init: use a platform legacy quirk for ebda

[David Vrabel]

On 07/04/16 01:06, Luis R. Rodriguez wrote:
> 
> --- a/arch/x86/kernel/platform-quirks.c
> +++ b/arch/x86/kernel/platform-quirks.c
> @@ -7,8 +7,12 @@
>  void __init x86_early_init_platform_quirks(void)
>  {
>  	x86_platform.legacy.rtc = 1;
> +	x86_platform.legacy.ebda_search = 0;

You should make the default the setting for regular PC hardware, as you
have done for the .rtc bit.

David

Re: [Xen-devel] [PATCH v4 06/14] x86/init: use a platform legacy quirk for ebda

[Luis R. Rodriguez]

On Thu, Apr 07, 2016 at 10:44:50AM +0100, David Vrabel wrote:
> On 07/04/16 01:06, Luis R. Rodriguez wrote:
> > 
> > --- a/arch/x86/kernel/platform-quirks.c
> > +++ b/arch/x86/kernel/platform-quirks.c
> > @@ -7,8 +7,12 @@
> >  void __init x86_early_init_platform_quirks(void)
> >  {
> >  	x86_platform.legacy.rtc = 1;
> > +	x86_platform.legacy.ebda_search = 0;
> 
> You should make the default the setting for regular PC hardware, as you
> have done for the .rtc bit.

I went with with Ingo's suggested template [0]. Ingo, let me know what
you prefer.

[0] http://lkml.kernel.org/r/20160407205902.GS1990@wotan.suse.de

  Luis

[PATCH v4 07/14] tools/lguest: force disable tboot and apm

[Luis R. Rodriguez]

The paravirt_enabled() check is going away, the area tossed to
the kernel on lguest is not zerored out, so ensure lguest force
disables tboot and apm just in case the kernel file being read might
have this set for whatever reason.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 tools/lguest/lguest.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/lguest/lguest.c b/tools/lguest/lguest.c
index ff0aa580c6e1..0aa75af6e862 100644
--- a/tools/lguest/lguest.c
+++ b/tools/lguest/lguest.c
@@ -3357,6 +3357,12 @@ int main(int argc, char *argv[])
 	/* Tell the entry path not to try to reload segment registers. */
 	boot->hdr.loadflags |= KEEP_SEGMENTS;
 
+	/* We don't support tboot */
+	boot->tboot_addr = 0;
+
+	/* Ensure this is 0 to prevent apm from loading */
+	boot->apm_bios_info.version = 0;
+
 	/* We tell the kernel to initialize the Guest. */
 	tell_kernel(start);
 
-- 
2.7.2

Re: [PATCH v4 07/14] tools/lguest: force disable tboot and apm

[Rusty Russell]

"Luis R. Rodriguez" <mcgrof@kernel.org> writes:
> The paravirt_enabled() check is going away, the area tossed to
> the kernel on lguest is not zerored out, so ensure lguest force
> disables tboot and apm just in case the kernel file being read might
> have this set for whatever reason.
>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>

Nice, thanks!

Acked-by: Rusty Russell <rusty@rustcorp.com.au>

Cheers,
Rusty.

[PATCH v4 08/14] apm32: remove paravirt_enabled() use

[Luis R. Rodriguez]

There is already a check for apm_info.bios == 0, the
apm_info.bios is set from the boot_params.apm_bios_info.
Both Xen and lguest, which are also the only ones that set
paravirt_enabled to true, never set the apm_bios.info. The

Xen folks are sure force disable to 0 is not needed, we
recently forced disabled this on lguest. With this in place
the paravirt_enabled() check is simply not needed anymore.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 arch/x86/kernel/apm_32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
index 9307f182fe30..c7364bd633e1 100644
--- a/arch/x86/kernel/apm_32.c
+++ b/arch/x86/kernel/apm_32.c
@@ -2267,7 +2267,7 @@ static int __init apm_init(void)
 
 	dmi_check_system(apm_dmi_table);
 
-	if (apm_info.bios.version == 0 || paravirt_enabled() || machine_is_olpc()) {
+	if (apm_info.bios.version == 0 || machine_is_olpc()) {
 		printk(KERN_INFO "apm: BIOS not found.\n");
 		return -ENODEV;
 	}
-- 
2.7.2


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

Re: [PATCH v4 08/14] apm32: remove paravirt_enabled() use

[Boris Ostrovsky]

On 04/06/2016 08:06 PM, Luis R. Rodriguez wrote:
> There is already a check for apm_info.bios == 0, the
> apm_info.bios is set from the boot_params.apm_bios_info.
> Both Xen and lguest, which are also the only ones that set
> paravirt_enabled to true, never set the apm_bios.info. The
>
> Xen folks are sure force disable to 0 is not needed,

Because apm_info lives in .bss (which we recently made sure is cleared 
on Xen PV). May be worth mentioning in the commit message so that we 
don't forget why this is not needed.

I think you also have this statement in other patches.

-boris

> we
> recently forced disabled this on lguest. With this in place
> the paravirt_enabled() check is simply not needed anymore.
>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
>   arch/x86/kernel/apm_32.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
> index 9307f182fe30..c7364bd633e1 100644
> --- a/arch/x86/kernel/apm_32.c
> +++ b/arch/x86/kernel/apm_32.c
> @@ -2267,7 +2267,7 @@ static int __init apm_init(void)
>   
>   	dmi_check_system(apm_dmi_table);
>   
> -	if (apm_info.bios.version == 0 || paravirt_enabled() || machine_is_olpc()) {
> +	if (apm_info.bios.version == 0 || machine_is_olpc()) {
>   		printk(KERN_INFO "apm: BIOS not found.\n");
>   		return -ENODEV;
>   	}

Re: [PATCH v4 08/14] apm32: remove paravirt_enabled() use

[Luis R. Rodriguez]

On Thu, Apr 07, 2016 at 09:08:36AM -0400, Boris Ostrovsky wrote:
> On 04/06/2016 08:06 PM, Luis R. Rodriguez wrote:
> >There is already a check for apm_info.bios == 0, the
> >apm_info.bios is set from the boot_params.apm_bios_info.
> >Both Xen and lguest, which are also the only ones that set
> >paravirt_enabled to true, never set the apm_bios.info. The
> >
> >Xen folks are sure force disable to 0 is not needed,
> 
> Because apm_info lives in .bss (which we recently made sure is
> cleared on Xen PV). May be worth mentioning in the commit message so
> that we don't forget why this is not needed.

Thanks, I'll change that last paragraph with:

Xen folks are sure force disable to 0 is not needed because
apm_info lives in .bss, we recently forced disabled this on
lguest, and on the Xen side just to be sure Boris zeroed out
the .bss for PV guests through commit 04b6b4a56884327c1648
("xen/x86: Zero out .bss for PV guests"). With this care taken
into consideration the paravirt_enabled() check is simply not
needed anymore.

> I think you also have this statement in other patches.

Indeed, I'll highlight this on the tboot commit log as well.

  Luis

[PATCH v4 09/14] x86/tboot: remove paravirt_enabled()

[Luis R. Rodriguez]

There is already a check for boot_params.tboot_addr prior
to paravirt_enabled(). Both Xen and lguest, which are also the
only ones that set paravirt_enabled to true, never set the
boot_params.tboot_addr. The Xen folks are sure a force disable
to 0 is not needed, we recently forced disabled this on lguest.
With this in place this check is no longer needed.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 arch/x86/kernel/tboot.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c
index e72a07f20b05..9b0185fbe3eb 100644
--- a/arch/x86/kernel/tboot.c
+++ b/arch/x86/kernel/tboot.c
@@ -74,12 +74,6 @@ void __init tboot_probe(void)
 		return;
 	}
 
-	/* only a natively booted kernel should be using TXT */
-	if (paravirt_enabled()) {
-		pr_warning("non-0 tboot_addr but pv_ops is enabled\n");
-		return;
-	}
-
 	/* Map and check for tboot UUID. */
 	set_fixmap(FIX_TBOOT_BASE, boot_params.tboot_addr);
 	tboot = (struct tboot *)fix_to_virt(FIX_TBOOT_BASE);
-- 
2.7.2

[PATCH v4 10/14] x86/cpu/intel: remove not needed paravirt_enabled() for f00f work around

[Luis R. Rodriguez]

The X86_BUG_F00F work around is responsible for fixing up the error
generated on attempted F00F exploitation from an OOPS to a SIGILL.
There is no reason why this code should not be allowed to run on
PV guest on a F00F-affected CPU -- it would simply never trigger.
The pv_enabled() check was there only to avoid printing the f00f
workaround, so removing the check is purely a cosmetic change.

Suggested-by: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 arch/x86/kernel/cpu/intel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index f71a34944b56..66509285ffdd 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -233,7 +233,7 @@ static void intel_workarounds(struct cpuinfo_x86 *c)
 	 * The Quark is also family 5, but does not have the same bug.
 	 */
 	clear_cpu_bug(c, X86_BUG_F00F);
-	if (!paravirt_enabled() && c->x86 == 5 && c->x86_model < 9) {
+	if (c->x86 == 5 && c->x86_model < 9) {
 		static int f00f_workaround_enabled;
 
 		set_cpu_bug(c, X86_BUG_F00F);
-- 
2.7.2


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

[PATCH v4 11/14] pnpbios: replace paravirt_enabled() check with legacy device check

[Luis R. Rodriguez]

Since we are removing paravirt_enabled() replace it with a
logical equivalent. Even though PNPBIOS is x86 specific we
add an arch-specific type call, which can be implemented by
any architecture to show how other legacy attribute devices
can later be also checked for with other ACPI legacy attribute
flags.

This implicates the first ACPI 5.2.9.3 IA-PC Boot Architecture
ACPI_FADT_LEGACY_DEVICES flag device, and shows how to add more.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 arch/x86/include/asm/x86_init.h   | 26 ++++++++++++++++++++++++++
 arch/x86/kernel/platform-quirks.c | 10 ++++++++++
 drivers/pnp/pnpbios/core.c        |  3 ++-
 include/linux/pnp.h               |  2 ++
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index f3f81122ae3b..894cc29529f9 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -142,15 +142,41 @@ struct x86_cpuinit_ops {
 struct timespec;
 
 /**
+ * struct x86_legacy_devices - legacy x86 devices
+ *
+ * @pnpbios: this platform can have a PNPBIOS. If this is disabled the platform
+ * 	is known to never have a PNPBIOS.
+ *
+ * These are devices known to require LPC or ISA bus. The definition of legacy
+ * devices adheres to the ACPI 5.2.9.3 IA-PC Boot Architecture flag
+ * ACPI_FADT_LEGACY_DEVICES. These devices consist of user visible devices on
+ * the LPC or ISA bus. User visible devices are devices that have end-user
+ * accessible connectors (for example, LPT parallel port). Legacy devices on
+ * the LPC bus consist for example of serial and parallel ports, PS/2 keyboard
+ * / mouse, and the floppy disk controller. A system that lacks all known
+ * legacy devices can assume all devices can be detected exclusively via
+ * standard device enumeration mechanisms including the ACPI namespace.
+ *
+ * A system which has does not have ACPI_FADT_LEGACY_DEVICES enabled must not
+ * have any of the legacy devices enumerated below present.
+ */
+struct x86_legacy_devices {
+	int pnpbios;
+};
+
+/**
  * struct x86_legacy_features - legacy x86 features
  *
  * @rtc: this device has a CMOS real-time clock present
  * @ebda_search: it's safe to search for the EBDA signature in the hardware's
  * 	low RAM
+ * @devices: legacy x86 devices, refer to struct x86_legacy_devices
+ * 	documentation for further details.
  */
 struct x86_legacy_features {
 	int rtc;
 	int ebda_search;
+	struct x86_legacy_devices devices;
 };
 
 /**
diff --git a/arch/x86/kernel/platform-quirks.c b/arch/x86/kernel/platform-quirks.c
index a871b6b0e35f..742f879da871 100644
--- a/arch/x86/kernel/platform-quirks.c
+++ b/arch/x86/kernel/platform-quirks.c
@@ -8,6 +8,7 @@ void __init x86_early_init_platform_quirks(void)
 {
 	x86_platform.legacy.rtc = 1;
 	x86_platform.legacy.ebda_search = 0;
+	x86_platform.legacy.devices.pnpbios = 1;
 
 	switch (boot_params.hdr.hardware_subarch) {
 	case X86_SUBARCH_PC:
@@ -15,8 +16,17 @@ void __init x86_early_init_platform_quirks(void)
 		break;
 	case X86_SUBARCH_XEN:
 	case X86_SUBARCH_LGUEST:
+		x86_platform.legacy.devices.pnpbios = 0;
+	/* fall through */
 	case X86_SUBARCH_INTEL_MID:
 		x86_platform.legacy.rtc = 0;
 		break;
 	}
 }
+
+#if defined(CONFIG_PNPBIOS)
+bool __init arch_pnpbios_disabled(void)
+{
+	return x86_platform.legacy.devices.pnpbios == 0;
+}
+#endif
diff --git a/drivers/pnp/pnpbios/core.c b/drivers/pnp/pnpbios/core.c
index facd43b8516c..81603d99082b 100644
--- a/drivers/pnp/pnpbios/core.c
+++ b/drivers/pnp/pnpbios/core.c
@@ -521,10 +521,11 @@ static int __init pnpbios_init(void)
 	int ret;
 
 	if (pnpbios_disabled || dmi_check_system(pnpbios_dmi_table) ||
-	    paravirt_enabled()) {
+	    arch_pnpbios_disabled()) {
 		printk(KERN_INFO "PnPBIOS: Disabled\n");
 		return -ENODEV;
 	}
+
 #ifdef CONFIG_PNPACPI
 	if (!acpi_disabled && !pnpacpi_disabled) {
 		pnpbios_disabled = 1;
diff --git a/include/linux/pnp.h b/include/linux/pnp.h
index 5df733b8f704..2588ca6a9028 100644
--- a/include/linux/pnp.h
+++ b/include/linux/pnp.h
@@ -337,9 +337,11 @@ extern struct mutex pnp_res_mutex;
 
 #ifdef CONFIG_PNPBIOS
 extern struct pnp_protocol pnpbios_protocol;
+extern bool arch_pnpbios_disabled(void);
 #define pnp_device_is_pnpbios(dev) ((dev)->protocol == (&pnpbios_protocol))
 #else
 #define pnp_device_is_pnpbios(dev) 0
+#define arch_pnpbios_disabled()	false
 #endif
 
 #ifdef CONFIG_PNPACPI
-- 
2.7.2

Re: [Xen-devel] [PATCH v4 11/14] pnpbios: replace paravirt_enabled() check with legacy device check

[David Vrabel]

On 07/04/16 01:06, Luis R. Rodriguez wrote:
> Since we are removing paravirt_enabled() replace it with a
> logical equivalent. Even though PNPBIOS is x86 specific we
> add an arch-specific type call, which can be implemented by
> any architecture to show how other legacy attribute devices
> can later be also checked for with other ACPI legacy attribute
> flags.
> 
> This implicates the first ACPI 5.2.9.3 IA-PC Boot Architecture
> ACPI_FADT_LEGACY_DEVICES flag device, and shows how to add more.
[...]
> +struct x86_legacy_devices {
> +	int pnpbios;
> +};

It's not clear why pnpbios needs a new structure and why this structure
of devices does not have the bit for the rtc device.

David
_______________________________________________
Lguest mailing list
Lguest@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/lguest

Re: [Xen-devel] [PATCH v4 11/14] pnpbios: replace paravirt_enabled() check with legacy device check

[Luis R. Rodriguez]

On Thu, Apr 07, 2016 at 10:46:11AM +0100, David Vrabel wrote:
> On 07/04/16 01:06, Luis R. Rodriguez wrote:
> > Since we are removing paravirt_enabled() replace it with a
> > logical equivalent. Even though PNPBIOS is x86 specific we
> > add an arch-specific type call, which can be implemented by
> > any architecture to show how other legacy attribute devices
> > can later be also checked for with other ACPI legacy attribute
> > flags.
> > 
> > This implicates the first ACPI 5.2.9.3 IA-PC Boot Architecture
> > ACPI_FADT_LEGACY_DEVICES flag device, and shows how to add more.
> [...]
> > +struct x86_legacy_devices {
> > +	int pnpbios;
> > +};
> 
> It's not clear why pnpbios needs a new structure

I'm glad you asked. Dealing with placing pnpbios quirk in a more useful generic
fashion was perhaps the most difficult challenge in this series.  As I reviewed
possibilities to remove paravirt_enabled() the best prospect I found was to see
if Xen could instead use ACPI 5.2.9.3 IA-PC Boot Architecture flags to annotate
some quirks. It turns out that it is possible, but there are only so many flags,
and also, we didn't want to have a solution that incurred respective upstream
Xen hypervisor change, that would be silly.

To make this quirk more useful then this folds the pnpbios quirk as a sub
quirk under the more borad ACPI_FADT_LEGACY_DEVICES ACPI flag. What this
does, as can be seen by also looking at the next patch, "x86, ACPI: parse
ACPI_FADT_LEGACY_DEVICES" is it explicitly folds pnpbios as one of the
ACPI_FADT_LEGACY_DEVICES devices, but also paves the way for further known
main legacy components to added to the list.

> and why this structure of devices does not have the bit for the rtc device.

That's because ACPI has its own dedicated flag for it, so there already
is a one-to-one mapping available. All we needed to do to replace the
RTC hack was to provide a mechanism to unify both the paravirt RTC hack
with the ACPI RTC flag.

  Luis

[PATCH v4 12/14] x86, ACPI: parse ACPI_FADT_LEGACY_DEVICES

[Luis R. Rodriguez]

ACPI 5.2.9.3 IA-PC Boot Architecture flag ACPI_FADT_LEGACY_DEVICES
can be used to determine if a system has legacy devices LPC or
ISA devices. The x86 platform already has a struct which lists
known associated legacy devices, we start off careful only
by disabling root devices we should not regress with. The struct
and device list can be expanded with time to cover more root
legacy components.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 arch/x86/kernel/acpi/boot.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 5e736e2108a1..3cada5e8505a 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -911,6 +911,11 @@ late_initcall(hpet_insert_resource);
 
 static int __init acpi_parse_fadt(struct acpi_table_header *table)
 {
+	if (!(acpi_gbl_FADT.boot_flags & ACPI_FADT_LEGACY_DEVICES)) {
+		pr_debug("ACPI: no legacy devices present\n");
+		x86_platform.legacy.devices.pnpbios = 0;
+	}
+
 	if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) {
 		pr_debug("ACPI: not registering RTC platform device\n");
 		x86_platform.legacy.rtc = 0;
-- 
2.7.2

[PATCH v4 13/14] x86/init: rename ebda code file

[Luis R. Rodriguez]

This makes it clearer what this is.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 arch/x86/Makefile                  | 2 +-
 arch/x86/kernel/Makefile           | 2 +-
 arch/x86/kernel/{head.c => ebda.c} | 0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename arch/x86/kernel/{head.c => ebda.c} (100%)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index f9ed8a7ce2b6..6fce7f096b88 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -208,7 +208,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/ebda.o
 head-y += arch/x86/kernel/platform-quirks.o
 
 libs-y  += arch/x86/lib/
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 7a9e44d935de..0503f5bfb18d 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -4,7 +4,7 @@
 
 extra-y	:= head_$(BITS).o
 extra-y	+= head$(BITS).o
-extra-y	+= head.o
+extra-y	+= ebda.o
 extra-y	+= platform-quirks.o
 extra-y	+= vmlinux.lds
 
diff --git a/arch/x86/kernel/head.c b/arch/x86/kernel/ebda.c
similarity index 100%
rename from arch/x86/kernel/head.c
rename to arch/x86/kernel/ebda.c
-- 
2.7.2


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

[PATCH v4 14/14] x86/paravirt: remove paravirt_enabled()

[Luis R. Rodriguez]

That that paravirt_enabled() is replaced with proper
x86 semantics we can remove it.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 arch/x86/include/asm/paravirt.h       | 5 -----
 arch/x86/include/asm/paravirt_types.h | 1 -
 arch/x86/include/asm/processor.h      | 1 -
 arch/x86/kernel/kvm.c                 | 8 --------
 arch/x86/kernel/paravirt.c            | 1 -
 arch/x86/lguest/boot.c                | 2 --
 arch/x86/xen/enlighten.c              | 1 -
 7 files changed, 19 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 6c7a4a192032..dff26bc91b17 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -15,11 +15,6 @@
 #include <linux/cpumask.h>
 #include <asm/frame.h>
 
-static inline int paravirt_enabled(void)
-{
-	return pv_info.paravirt_enabled;
-}
-
 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 6acc1b26cf40..7fedf24bd811 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -69,7 +69,6 @@ struct pv_info {
 	u16 extra_user_64bit_cs;  /* __USER_CS if none */
 #endif
 
-	int paravirt_enabled;
 	const char *name;
 };
 
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 0c70c7daa6b8..8d326e822cb8 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -473,7 +473,6 @@ static inline unsigned long current_top_of_stack(void)
 #include <asm/paravirt.h>
 #else
 #define __cpuid			native_cpuid
-#define paravirt_enabled()	0
 
 static inline void load_sp0(struct tss_struct *tss,
 			    struct thread_struct *thread)
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index dc1207e2f193..eea2a6f72b31 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -285,14 +285,6 @@ static void __init paravirt_ops_setup(void)
 {
 	pv_info.name = "KVM";
 
-	/*
-	 * KVM isn't paravirt in the sense of paravirt_enabled.  A KVM
-	 * guest kernel works like a bare metal kernel with additional
-	 * features, and paravirt_enabled is about features that are
-	 * missing.
-	 */
-	pv_info.paravirt_enabled = 0;
-
 	if (kvm_para_has_feature(KVM_FEATURE_NOP_IO_DELAY))
 		pv_cpu_ops.io_delay = kvm_io_delay;
 
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index f08ac28b8136..71a2d8a05a66 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -294,7 +294,6 @@ enum paravirt_lazy_mode paravirt_get_lazy_mode(void)
 
 struct pv_info pv_info = {
 	.name = "bare hardware",
-	.paravirt_enabled = 0,
 	.kernel_rpl = 0,
 	.shared_kernel_pmd = 1,	/* Only used when CONFIG_X86_PAE is set */
 
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index f5497ee5fd2f..3847e736702e 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -1408,8 +1408,6 @@ __init void lguest_init(void)
 {
 	/* We're under lguest. */
 	pv_info.name = "lguest";
-	/* Paravirt is enabled. */
-	pv_info.paravirt_enabled = 1;
 	/* We're running at privilege level 1, not 0 as normal. */
 	pv_info.kernel_rpl = 1;
 	/* Everyone except Xen runs with this set. */
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 7449f268d687..d4bb70e9372e 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1186,7 +1186,6 @@ static unsigned xen_patch(u8 type, u16 clobbers, void *insnbuf,
 }
 
 static const struct pv_info xen_info __initconst = {
-	.paravirt_enabled = 1,
 	.shared_kernel_pmd = 0,
 
 #ifdef CONFIG_X86_64
-- 
2.7.2

Re: [Xen-devel] [PATCH v4 00/14] x86: remove paravirt_enabled

[Juergen Gross]

On 07/04/16 02:06, Luis R. Rodriguez wrote:
> Now that Andy's ASM paravirt_enabled() use is merged all we need is to address
> the rest of the C code uses. This completes that work by providing proper
> semantics for platform legacy settings and quirks as suggested by Ingo, this in
> turn can also be extended later for benefit of further processing of ACPI
> 5.2.9.3 IA-PC Boot Architecture flags, which we currently don't take much
> advantage of.  For instance the ACPI_FADT_NO_VGA can later be leveraged by bare
> metal x86 *and* HVMLite, as HVMLite seems to plan to set this.
> 
> Also, hpa has noted both Intel MID and CE4100 can make use of disabling
> pnpbios, we can do that separately after this, but it should now be a
> trivial change, generic given this quirk stuff is all generic now.
> 
> This patches goes tested by 0-day, except for the last patch, for some reason
> the branch that included that patch has had testing delayed for quite a
> while now, but I can't think of anything there that should break anything.
> 
> I've also just run time tested this on bare metal only so far.

FWIW: Xen dom0 is booting with the patches applied.


Juergen

> 
> Luis R. Rodriguez (14):
>   x86/boot: enumerate documentation for the x86 hardware_subarch
>   x86/xen: use X86_SUBARCH_XEN for PV guest boots
>   tools/lguest: make lguest launcher use X86_SUBARCH_LGUEST explicitly
>   x86/rtc: replace paravirt rtc check with platform legacy quirk
>   x86, ACPI: move ACPI_FADT_NO_CMOS_RTC check to ACPI boot code
>   x86/init: use a platform legacy quirk for ebda
>   tools/lguest: force disable tboot and apm
>   apm32: remove paravirt_enabled() use
>   x86/tboot: remove paravirt_enabled()
>   x86/cpu/intel: remove not needed paravirt_enabled() for f00f work
>     around
>   pnpbios: replace paravirt_enabled() check with legacy device check
>   x86, ACPI: parse ACPI_FADT_LEGACY_DEVICES
>   x86/init: rename ebda code file
>   x86/paravirt: remove paravirt_enabled()
> 
>  arch/x86/Makefile                     |  3 ++-
>  arch/x86/include/asm/paravirt.h       | 11 ---------
>  arch/x86/include/asm/paravirt_types.h |  6 -----
>  arch/x86/include/asm/processor.h      |  2 --
>  arch/x86/include/asm/x86_init.h       | 42 +++++++++++++++++++++++++++++++++++
>  arch/x86/include/uapi/asm/bootparam.h | 36 +++++++++++++++++++++++++++++-
>  arch/x86/kernel/Makefile              |  6 ++++-
>  arch/x86/kernel/acpi/boot.c           |  9 ++++++++
>  arch/x86/kernel/apm_32.c              |  2 +-
>  arch/x86/kernel/cpu/intel.c           |  2 +-
>  arch/x86/kernel/{head.c => ebda.c}    |  2 +-
>  arch/x86/kernel/head32.c              |  2 ++
>  arch/x86/kernel/head64.c              |  1 +
>  arch/x86/kernel/kvm.c                 |  8 -------
>  arch/x86/kernel/paravirt.c            |  1 -
>  arch/x86/kernel/platform-quirks.c     | 32 ++++++++++++++++++++++++++
>  arch/x86/kernel/rtc.c                 | 15 ++-----------
>  arch/x86/kernel/tboot.c               |  6 -----
>  arch/x86/lguest/boot.c                |  3 ---
>  arch/x86/xen/enlighten.c              |  5 +----
>  drivers/pnp/pnpbios/core.c            |  3 ++-
>  include/linux/pnp.h                   |  2 ++
>  tools/lguest/lguest.c                 | 10 +++++++--
>  23 files changed, 146 insertions(+), 63 deletions(-)
>  rename arch/x86/kernel/{head.c => ebda.c} (98%)
>  create mode 100644 arch/x86/kernel/platform-quirks.c
> 

[PATCH v4 13/14] x86/init: rename ebda code file

[Luis R. Rodriguez]

This makes it clearer what this is.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 arch/x86/Makefile                  | 2 +-
 arch/x86/kernel/Makefile           | 2 +-
 arch/x86/kernel/{head.c => ebda.c} | 0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename arch/x86/kernel/{head.c => ebda.c} (100%)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index f9ed8a7ce2b6..6fce7f096b88 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -208,7 +208,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/ebda.o
 head-y += arch/x86/kernel/platform-quirks.o
 
 libs-y  += arch/x86/lib/
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 7a9e44d935de..0503f5bfb18d 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -4,7 +4,7 @@
 
 extra-y	:= head_$(BITS).o
 extra-y	+= head$(BITS).o
-extra-y	+= head.o
+extra-y	+= ebda.o
 extra-y	+= platform-quirks.o
 extra-y	+= vmlinux.lds
 
diff --git a/arch/x86/kernel/head.c b/arch/x86/kernel/ebda.c
similarity index 100%
rename from arch/x86/kernel/head.c
rename to arch/x86/kernel/ebda.c
-- 
2.7.2

Re: [PATCH v4 00/14] x86: remove paravirt_enabled

[Luis R. Rodriguez]

On Wed, Apr 06, 2016 at 05:06:20PM -0700, Luis R. Rodriguez wrote:
> Now that Andy's ASM paravirt_enabled() use is merged 

Sorry I should have provided more context, I meant that now
that Andy's ASM paravirt_enabled() removal is merged:

This is the ASM hack that Andy removed:
https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/commit/?h=x86/fsgsbase&id=58a5aac5331388a175a42b6ed2154f0559cefb21

This puts a nail on coffin for the ASM hack:
https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/commit/?h=x86/fsgsbase&id=0dd0036f6e07f741a1356b424b84a3164b6e59cf

  Luis

Re: [PATCH v4 00/14] x86: remove paravirt_enabled

[Borislav Petkov]

On Fri, Apr 08, 2016 at 03:14:43AM +0200, Luis R. Rodriguez wrote:
> On Wed, Apr 06, 2016 at 05:06:20PM -0700, Luis R. Rodriguez wrote:
> > Now that Andy's ASM paravirt_enabled() use is merged 
> 
> Sorry I should have provided more context, I meant that now
> that Andy's ASM paravirt_enabled() removal is merged:
> 
> This is the ASM hack that Andy removed:
> https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/commit/?h=x86/fsgsbase&id=58a5aac5331388a175a42b6ed2154f0559cefb21
> 
> This puts a nail on coffin for the ASM hack:
> https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/commit/?h=x86/fsgsbase&id=0dd0036f6e07f741a1356b424b84a3164b6e59cf

Ah right, that.

Thanks!

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.