[PATCH v3 00/11] x86/init: replace paravirt_enabled() were possible

[PATCH v3 00/11] x86/init: replace paravirt_enabled() were possible

[Luis R. Rodriguez]

Boris,

this v3 series addresses a feedback from my last series in trying to
replace or remove paravirt_enabled() as we have been discussing. Some
patches are being spinned once, some others by now 3 times, hence the
v3. Changes per iteration are documented in the commit log on each patch.
I've documented progress on a wiki so far [0] on the crusade to remove
paravirt_enabled(). Seems we just have one more item to go.

Since we're using BIT() for the platform flags I've folded into this
series the patch that adds BIT() for early boot code.

This series has been tested with no issues by 0-day bot.

Any followup on the linker table work or its use through the proof of
concept ports and the new proposed x86 proposed use will depend on this
series, I'll follow through these series using the x86/init prefix unless
told otherwise.

Rusty, a few patches touch touch lguest, although they're trivial it'd still
be good to get your Acked-by or Reviewed-by.

In case anyone needs it these patches are also up on my linux-next
tree on the 20160222-remove-pv-enabled-test-02 branch. They're all
based on linux-next tag next-20160222.

[0] http://kernelnewbies.org/KernelProjects/remove-paravirt-enabled
[1] https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux-next.git/log/?h=20160222-remove-pv-enabled-test-02

Luis R. Rodriguez (11):
  x86/boot: enumerate documentation for the x86 hardware_subarch
  tools/lguest: make lguest launcher use X86_SUBARCH_LGUEST explicitly
  x86/xen: use X86_SUBARCH_XEN for PV guest boots
  x86/init: make ebda depend on PC subarch
  tools/lguest: force disable tboot and apm
  apm32: remove paravirt_enabled() use
  x86/tboot: remove paravirt_enabled()
  x86/cpu/intel: replace paravirt_enabled() for f00f work around
  x86/boot: add BIT() to boot/bitops.h
  x86/rtc: replace paravirt rtc check with x86 specific solution
  pnpbios: replace paravirt_enabled() check with subarch checks

 arch/x86/boot/bitops.h                  |  2 ++
 arch/x86/boot/boot.h                    |  2 +-
 arch/x86/include/asm/paravirt.h         |  6 ------
 arch/x86/include/asm/paravirt_types.h   |  5 -----
 arch/x86/include/asm/processor.h        |  1 -
 arch/x86/include/asm/x86_init.h         | 12 ++++++++++++
 arch/x86/include/uapi/asm/bootparam.h   | 31 ++++++++++++++++++++++++++++++-
 arch/x86/kernel/acpi/boot.c             |  4 ++++
 arch/x86/kernel/apm_32.c                |  2 +-
 arch/x86/kernel/cpu/intel.c             |  5 ++++-
 arch/x86/kernel/head.c                  |  2 +-
 arch/x86/kernel/rtc.c                   | 15 ++-------------
 arch/x86/kernel/tboot.c                 |  6 ------
 arch/x86/lguest/boot.c                  |  2 +-
 arch/x86/platform/intel-mid/intel-mid.c |  3 +++
 arch/x86/xen/enlighten.c                |  4 +---
 arch/x86/xen/time.c                     |  4 +++-
 drivers/pnp/pnpbios/core.c              |  4 +++-
 tools/lguest/lguest.c                   | 10 ++++++++--
 19 files changed, 76 insertions(+), 44 deletions(-)

-- 
2.7.0

[PATCH v3 01/11] 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.

v2: fix typos

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

diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index 329254373479..50d5009cf276 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -157,7 +157,36 @@ 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 supports 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 do not 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.
+ *
+ * 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.
+ *
+ * @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.0

[PATCH v3 02/11] 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.0

[PATCH v3 03/11] 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 d09e4c9d7cc5..5b3f1c763806 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1669,6 +1669,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.0

[PATCH v3 04/11] x86/init: make ebda depend on PC subarch

[Luis R. Rodriguez]

This lets us remove its use of paravirt_enabled(). The
other subarchs are not needed here given that on 32-bit
there is a switch already that negates access to this
code on X86_SUBARCH_INTEL_MID, and X86_SUBARCH_CE4100.
Both lguest and Xen had paravirt_enabled so that
excludes them.

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

diff --git a/arch/x86/kernel/head.c b/arch/x86/kernel/head.c
index 992f442ca155..4e3be58a1a77 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 (boot_params.hdr.hardware_subarch != X86_SUBARCH_PC)
 		return;
 
 	/* end of low (conventional) memory */
-- 
2.7.0

[PATCH v3 05/11] tools/lguest: force disable tboot and apm

[Luis R. Rodriguez]

The paravirt_enabled() check is going away, force disable
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.0

[PATCH v3 06/11] 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) do never set the apm_bios_info,
the paravirt_enabled() check is simply not needed.

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 052c9c3026cc..749f7a081257 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.0

[PATCH v3 07/11] x86/tboot: remove paravirt_enabled()

[Luis R. Rodriguez]

There is already a check for boot_params.tboot_addr prior
to paravirt_enabled() and both Xen and lguest never set this.
This check is not 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 91a4496db434..a2df1d3fab74 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.0

[PATCH v3 08/11] x86/cpu/intel: replace paravirt_enabled() for f00f work around

[Luis R. Rodriguez]

Use the harware subarch instead now that they are set. If we
want to test removing this work around on Xen we can do so
separatley as another independent change, for now we just want
to remove paravirt_enabled().

v3: fix 0-day-bot compile error on a randconfig, missing
    to include <asm/setup.h>

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

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 565648bc1a0a..48ab1b6e0889 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -13,6 +13,7 @@
 #include <asm/msr.h>
 #include <asm/bugs.h>
 #include <asm/cpu.h>
+#include <asm/setup.h>
 
 #ifdef CONFIG_X86_64
 #include <linux/topology.h>
@@ -220,7 +221,9 @@ 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 (boot_params.hdr.hardware_subarch != X86_SUBARCH_XEN &&
+	    boot_params.hdr.hardware_subarch != X86_SUBARCH_LGUEST &&
+	    c->x86 == 5 && c->x86_model < 9) {
 		static int f00f_workaround_enabled;
 
 		set_cpu_bug(c, X86_BUG_F00F);
-- 
2.7.0

[PATCH v3 09/11] x86/boot: add BIT() to boot/bitops.h

[Luis R. Rodriguez]

The boot/bitops.h has guards against including the
regular bitops (include/asm-generic/bitops.h), it only
implements what we need at early boot. We'll be making
use of BIT() later so add it.

Users of boot/boot.h must include it prior to asm/setup.h
otherwise the guard protection devised against the regular
linux/bitops.h will not take effect.

v2: spelling fixes, and language descriptipon enhancements
    by Konrad.
v3: Parenthesize (x), just match what is in include/linux/bitmap.h

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 arch/x86/boot/bitops.h | 2 ++
 arch/x86/boot/boot.h   | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/boot/bitops.h b/arch/x86/boot/bitops.h
index 878e4b9940d9..cfe154c0bd3d 100644
--- a/arch/x86/boot/bitops.h
+++ b/arch/x86/boot/bitops.h
@@ -40,4 +40,6 @@ static inline void set_bit(int nr, void *addr)
 	asm("btsl %1,%0" : "+m" (*(u32 *)addr) : "Ir" (nr));
 }
 
+#define BIT(nr)	(1UL << (nr))
+
 #endif /* BOOT_BITOPS_H */
diff --git a/arch/x86/boot/boot.h b/arch/x86/boot/boot.h
index 9011a88353de..4fb53da1f48a 100644
--- a/arch/x86/boot/boot.h
+++ b/arch/x86/boot/boot.h
@@ -23,8 +23,8 @@
 #include <stdarg.h>
 #include <linux/types.h>
 #include <linux/edd.h>
-#include <asm/setup.h>
 #include "bitops.h"
+#include <asm/setup.h>
 #include "ctype.h"
 #include "cpuflags.h"
 
-- 
2.7.0

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

[Luis R. Rodriguez]

We have 4 types of x86 platforms that disable RTC:

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

We can consolidate all of these into a platform specific solution.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 arch/x86/include/asm/paravirt.h         |  6 ------
 arch/x86/include/asm/paravirt_types.h   |  5 -----
 arch/x86/include/asm/processor.h        |  1 -
 arch/x86/include/asm/x86_init.h         | 12 ++++++++++++
 arch/x86/kernel/acpi/boot.c             |  4 ++++
 arch/x86/kernel/rtc.c                   | 15 ++-------------
 arch/x86/lguest/boot.c                  |  2 +-
 arch/x86/platform/intel-mid/intel-mid.c |  3 +++
 arch/x86/xen/enlighten.c                |  3 ---
 arch/x86/xen/time.c                     |  4 +++-
 10 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index f6192502149e..c261402340e3 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -19,12 +19,6 @@ static inline int paravirt_enabled(void)
 	return pv_info.paravirt_enabled;
 }
 
-static inline int paravirt_has_feature(unsigned int feature)
-{
-	WARN_ON_ONCE(!pv_info.paravirt_enabled);
-	return (pv_info.features & feature);
-}
-
 static inline void load_sp0(struct tss_struct *tss,
 			     struct thread_struct *thread)
 {
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 77db5616a473..2489d6a08e89 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -70,14 +70,9 @@ struct pv_info {
 #endif
 
 	int paravirt_enabled;
-	unsigned int features;	  /* valid only if paravirt_enabled is set */
 	const char *name;
 };
 
-#define paravirt_has(x) paravirt_has_feature(PV_SUPPORTED_##x)
-/* Supported features */
-#define PV_SUPPORTED_RTC        (1<<0)
-
 struct pv_init_ops {
 	/*
 	 * Patch may replace one of the defined code sequences with
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 20c11d1aa4cc..10f3614265c1 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -472,7 +472,6 @@ static inline unsigned long current_top_of_stack(void)
 #else
 #define __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..0ef697e46f6e 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -1,6 +1,7 @@
 #ifndef _ASM_X86_PLATFORM_H
 #define _ASM_X86_PLATFORM_H
 
+#include <linux/bitops.h>
 #include <asm/bootparam.h>
 
 struct mpc_bus;
@@ -116,6 +117,7 @@ struct x86_init_pci {
 /**
  * struct x86_init_ops - functions for platform specific setup
  *
+ * @platform_flags:		bitmask of enum x86_platform_flags.
  */
 struct x86_init_ops {
 	struct x86_init_resources	resources;
@@ -126,6 +128,7 @@ struct x86_init_ops {
 	struct x86_init_timers		timers;
 	struct x86_init_iommu		iommu;
 	struct x86_init_pci		pci;
+	u64 platform_flags;
 };
 
 /**
@@ -142,6 +145,15 @@ struct x86_cpuinit_ops {
 struct timespec;
 
 /**
+ * enum x86_platform_flags - x86 platform flags
+ *
+ * X86_PLATFORM_NO_RTC: set when platform has no CMOS real-time clock present
+ */
+enum x86_platform_flags {
+	X86_PLATFORM_NO_RTC	= BIT(0),
+};
+
+/**
  * struct x86_platform_ops - platform specific runtime functions
  * @calibrate_tsc:		calibrate TSC
  * @get_wallclock:		get time from HW clock like RTC etc.
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index e75907601a41..d156f6cd2be3 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -913,6 +913,10 @@ late_initcall(hpet_insert_resource);
 
 static int __init acpi_parse_fadt(struct acpi_table_header *table)
 {
+	if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) {
+		pr_debug("ACPI: not registering RTC platform device\n");
+		x86_init.platform_flags |= X86_PLATFORM_NO_RTC;
+	}
 
 #ifdef CONFIG_X86_PM_TIMER
 	/* detect the location of the ACPI PM Timer */
diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
index 4af8d063fb36..5f95e42ecc68 100644
--- a/arch/x86/kernel/rtc.c
+++ b/arch/x86/kernel/rtc.c
@@ -14,6 +14,7 @@
 #include <asm/time.h>
 #include <asm/intel-mid.h>
 #include <asm/rtc.h>
+#include <asm/setup.h>
 
 #ifdef CONFIG_X86_32
 /*
@@ -188,19 +189,7 @@ static __init int add_rtc_cmos(void)
 	if (of_have_populated_dt())
 		return 0;
 
-	/* Intel MID platforms don't have ioport rtc */
-	if (intel_mid_identify_cpu())
-		return -ENODEV;
-
-#ifdef CONFIG_ACPI
-	if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) {
-		/* This warning can likely go away again in a year or two. */
-		pr_info("ACPI: not registering RTC platform device\n");
-		return -ENODEV;
-	}
-#endif
-
-	if (paravirt_enabled() && !paravirt_has(RTC))
+	if (x86_init.platform_flags & X86_PLATFORM_NO_RTC)
 		return -ENODEV;
 
 	platform_device_register(&rtc_device);
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index 4ba229ac3f4f..a85add6ce776 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -1414,7 +1414,6 @@ __init void lguest_init(void)
 	pv_info.kernel_rpl = 1;
 	/* Everyone except Xen runs with this set. */
 	pv_info.shared_kernel_pmd = 1;
-	pv_info.features = 0;
 
 	/*
 	 * We set up all the lguest overrides for sensitive operations.  These
@@ -1482,6 +1481,7 @@ __init void lguest_init(void)
 	x86_init.resources.memory_setup = lguest_memory_setup;
 	x86_init.irqs.intr_init = lguest_init_IRQ;
 	x86_init.timers.timer_init = lguest_time_init;
+	x86_init.platform_flags |= X86_PLATFORM_NO_RTC;
 	x86_platform.calibrate_tsc = lguest_tsc_khz;
 	x86_platform.get_wallclock =  lguest_get_wallclock;
 
diff --git a/arch/x86/platform/intel-mid/intel-mid.c b/arch/x86/platform/intel-mid/intel-mid.c
index 90bb997ed0a2..17953abcea52 100644
--- a/arch/x86/platform/intel-mid/intel-mid.c
+++ b/arch/x86/platform/intel-mid/intel-mid.c
@@ -175,6 +175,9 @@ void __init x86_intel_mid_early_setup(void)
 	x86_init.timers.timer_init = intel_mid_time_init;
 	x86_init.timers.setup_percpu_clockev = x86_init_noop;
 
+	/* Intel MID platforms don't have ioport rtc */
+	x86_init.platform_flags |= X86_PLATFORM_NO_RTC;
+
 	x86_init.irqs.pre_vector_init = x86_init_noop;
 
 	x86_init.oem.arch_setup = intel_mid_arch_setup;
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 5b3f1c763806..5c06169bce27 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1192,7 +1192,6 @@ static const struct pv_info xen_info __initconst = {
 #ifdef CONFIG_X86_64
 	.extra_user_64bit_cs = FLAT_USER_CS64,
 #endif
-	.features = 0,
 	.name = "Xen",
 };
 
@@ -1526,8 +1525,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
 
 	/* Install Xen paravirt ops */
 	pv_info = xen_info;
-	if (xen_initial_domain())
-		pv_info.features |= PV_SUPPORTED_RTC;
 	pv_init_ops = xen_init_ops;
 	if (!xen_pvh_domain()) {
 		pv_cpu_ops = xen_cpu_ops;
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index a0a4e554c6f1..fbfcb01015f0 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -446,8 +446,10 @@ void __init xen_init_time_ops(void)
 	x86_platform.calibrate_tsc = xen_tsc_khz;
 	x86_platform.get_wallclock = xen_get_wallclock;
 	/* Dom0 uses the native method to set the hardware RTC. */
-	if (!xen_initial_domain())
+	if (!xen_initial_domain()) {
+		x86_init.platform_flags |= X86_PLATFORM_NO_RTC;
 		x86_platform.set_wallclock = xen_set_wallclock;
+	}
 }
 
 #ifdef CONFIG_XEN_PVHVM
-- 
2.7.0

[PATCH v3 11/11] pnpbios: replace paravirt_enabled() check with subarch checks

[Luis R. Rodriguez]

Since we are removing paravirt_enabled() replace it with a
logical equivalent.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/pnp/pnpbios/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pnp/pnpbios/core.c b/drivers/pnp/pnpbios/core.c
index facd43b8516c..96dc1f905dba 100644
--- a/drivers/pnp/pnpbios/core.c
+++ b/drivers/pnp/pnpbios/core.c
@@ -66,6 +66,7 @@
 #include <asm/page.h>
 #include <asm/desc.h>
 #include <asm/byteorder.h>
+#include <asm/setup.h>
 
 #include "../base.h"
 #include "pnpbios.h"
@@ -521,7 +522,8 @@ static int __init pnpbios_init(void)
 	int ret;
 
 	if (pnpbios_disabled || dmi_check_system(pnpbios_dmi_table) ||
-	    paravirt_enabled()) {
+	    boot_params.hdr.hardware_subarch == X86_SUBARCH_LGUEST ||
+	    boot_params.hdr.hardware_subarch == X86_SUBARCH_XEN) {
 		printk(KERN_INFO "PnPBIOS: Disabled\n");
 		return -ENODEV;
 	}
-- 
2.7.0

Re: [PATCH v3 01/11] x86/boot: enumerate documentation for the x86 hardware_subarch

[Ingo Molnar]

* Luis R. Rodriguez <mcgrof@kernel.org> 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.
> 
> v2: fix typos
> 
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
>  arch/x86/include/uapi/asm/bootparam.h | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
> index 329254373479..50d5009cf276 100644
> --- a/arch/x86/include/uapi/asm/bootparam.h
> +++ b/arch/x86/include/uapi/asm/bootparam.h
> @@ -157,7 +157,36 @@ 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 supports 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 do not 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.
> + *
> + * 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.
> + *
> + * @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,

No, this is really backwards.

While I agree that we want to get rid of paravirt_enabled(), we _dont_ want to 
spread the use of (arguably broken) boot flags like this! This is one of the cases 
where the cure is worse than the disease.

The 'modern' way to handle platform quirks is to have hardware drivers with 
auto-detection, and drivers know how to detect whether the hardware is present or 
not. For legacy cases where no auto-detection is possible, we have per hardware 
capability flags to turn it off.

The 'hardware subarch' flag of the bootloader can be used to install certain 
quirks, in a single early bootup function - but that should be all: ideally no 
quirks are needed. We don't want to spread 'subarch flags' into various unrelated 
code.

Let's go over your series and see whether and how that principle can be applied:

 - patch #1, #2: should be dropped

 - patch #3, #4: EBDA support.

   The EBDA BIOS signature is an ancient data structure, starting off at
   physical memory 0x40E - which is the very first physical memory page of the
   system.

   We should add an x86_ebda_bios flag that is set to 1 by default, but which
   paravirt bootup can set to 0. That would avoid the reservation of the BIOS 
   area and will save a bit of RAM.

 - patch #5, #6, #7: looks good, does not use a subarch flag

 - patch #8: f00f workaround. Subarch flag use is wrong. The complication with 
   this workaround is that it uses MM tricks to install an IDT. Could you check 
   whether Xen truly needs this quirk? If yes then there should be a new flag, 
   something like x86_idt_readonly, which is set to 0 but Xen can set it to 1. If 
   that flag is set then the F00F workaround does not have to be installed.

   Or something like that: the point is to use a specific flag.

 - Patch #9, #10: RTC support. The problem with RTC platform driver is that it's 
   not possible to detect the RTC reliably - so we sometimes have to quirk it off.

   Instead of using bitflags, add something like x86_platform.rtc_available, which 
   defaults to 1. Don't add negation to the name and don't use bitflags - use a 
   byte flag.

 - Patch #11: this patch wants to disable the PNP BIOS code.

   The complication with the PNP BIOS code is that the PNP BIOS is defined 
   in physical memory, in the 0xf0000-0xffff0 memory range - a 64kB large area. 
   Paravirt images don't want to waste 64kB of RAM just to tell the kernel that no
   PNP BIOS is available.

   But instead of using subarch flags, please define a pnpbios_disable() API call 
   that paravirt bootup can call into. That can disable the PNP BIOS code.

Also, feel free to define a single 'disable PC legacies' quirk function that 
disables all the usual PC legacies that paravirt does not care about or does not 
want to define via RAM. Specific paravirt initialization functions would only have 
to call this one (shared) function.

Thanks,

	Ingo

Re: [PATCH v3 01/11] x86/boot: enumerate documentation for the x86 hardware_subarch

[Luis R. Rodriguez]

On Tue, Feb 23, 2016 at 09:51:19AM +0100, Ingo Molnar wrote:
> 
> * Luis R. Rodriguez <mcgrof@kernel.org> 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.
> > 
> > v2: fix typos
> > 
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> > ---
> >  arch/x86/include/uapi/asm/bootparam.h | 31 ++++++++++++++++++++++++++++++-
> >  1 file changed, 30 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
> > index 329254373479..50d5009cf276 100644
> > --- a/arch/x86/include/uapi/asm/bootparam.h
> > +++ b/arch/x86/include/uapi/asm/bootparam.h
> > @@ -157,7 +157,36 @@ 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 supports 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 do not 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.
> > + *
> > + * 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.
> > + *
> > + * @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,
> 
> No, this is really backwards.
> 
> While I agree that we want to get rid of paravirt_enabled(), we _dont_ want to 
> spread the use of (arguably broken) boot flags like this!

I agree that we should not see the spread of boot flags around general x86
code, its not my goal to spread it though, the code that uses it here though is
*early boot code* (although in retrospect the pnpbios use was a fuckup), and I
have some special considerations for early boot code which I think does give
merit to it use. But also keep in mind my goal is to rather fold the boot flag
so its more just an architectural consideration eventually. More on this below.

> This is one of the cases 
> where the cure is worse than the disease.
> 
> The 'modern' way to handle platform quirks is to have hardware drivers with 
> auto-detection, and drivers know how to detect whether the hardware is present or 
> not. For legacy cases where no auto-detection is possible, we have per hardware 
> capability flags to turn it off.
> 
> The 'hardware subarch' flag of the bootloader can be used to install certain 
> quirks, in a single early bootup function - but that should be all: ideally no 
> quirks are needed. We don't want to spread 'subarch flags' into various unrelated 
> code.

There's another possible use for the subarch which I intend on using later for
early boot code to proactively help with disconnect on modifications of the
different x86 entry points. A summary of this is, at times x86 may get some
enhancement / feature which requires some small modifications on early boot
code and if only the bare metal entry point gets vetted on development then
chances are high the xen entry path will be faulty. At times some "feature"
or enhancement may be rather small, and all we need is a respective call
placed on the Xen entry path. Such was the case when the cr4 shadow was
added, since Xen didn't get it Xen crashed. Adding the call on the Xen
entry path fixed the issue later. Other times features may require more
work, such is the case with Kasan. Kasan currently only works on bare
metal and KVM, but not on Xen when PV guests are used. There was an oversight
on review. When I pointed the possible issue with Kasan it was thought that
adding support for Kasan for Xen would be trivial but upon review with the
Kasan maintainer we've realized it may take a bit more work. I've also
confirmed that enabling it on Xen crashes it. There's no fix in sight
and we can't disable Kasan at run time...

What I'm after is a proactive stop-gap that helps us ensure *early boot*
requirements are spelled out so that if some developer is adding a new
feature which mucks with *early boot code* they'll have to consider both entry
points. It will be fine if they only add support for bare metal -- but at least
it will be clearly annotated then. With this in place for instance I would
have expected Kasan would not have been merged unless Xen was also supportedddd
or at the very least Kasan got support to be disabled at run time in early
boot (and then Xen would disable it until it gets support for it). I won't bore
you with more details but if you want to read more about what I mean:

http://www.do-not-panic.com/2015/12/avoiding-dead-code-pvops-not-silver-bullet.html
http://www.do-not-panic.com/2015/12/xen-and-x86-linux-zero-page.html

Now just keep this type of proactive use case in mind for now, its one of
the things I was originally after with the linker tables and x86's use
for it.

> Let's go over your series and see whether and how that principle can be applied:
> 
>  - patch #1, #2: should be dropped
> 
>  - patch #3, #4: EBDA support.
> 
>    The EBDA BIOS signature is an ancient data structure, starting off at
>    physical memory 0x40E - which is the very first physical memory page of the
>    system.
> 
>    We should add an x86_ebda_bios flag that is set to 1 by default, but which
>    paravirt bootup can set to 0. That would avoid the reservation of the BIOS 
>    area and will save a bit of RAM.

Using a flag is certainly one way to go about it. With the subarch approach
I just wanted to point out that my goal was to eventually fold it underneath
the code, we'd just declare the ebda reservation as follows:

x86_init_early_pc_simple(reserve_ebda_region);

See:

http://lkml.kernel.org/r/1455891343-10016-4-git-send-email-mcgrof@kernel.org

Since linker tables are used the ebda call would just be one of the init
calls that x86_init_fn_early_init() would call. The relevant stub for that:

void __init x86_init_fn_early_init(void)                                         
{                                                                               
        struct x86_init_fn *init_fn;                                            
        unsigned int num_inits = LINKTABLE_SIZE(x86_init_fns);                  
                                                                                
        if (!num_inits)                                                         
                return;                                                         
                                                                                
        pr_debug("Number of init entries: %d\n", num_inits);                    
                                                                                
        LINKTABLE_FOR_EACH(init_fn, x86_init_fns) {                             
                if (!x86_init_fn_supports_subarch(init_fn))                     
                        continue;   
		...
		init_fn->early_init();
	}
}

And its helper:

static bool x86_init_fn_supports_subarch(struct x86_init_fn *fn)                
{                                                                               
        if (!fn->supp_hardware_subarch) {                                       
                pr_err("Init sequence fails to declares any supported subarchs: %pF\n", fn->early_init);
                WARN_ON(1);                                                     
        }                                                                       
        if (BIT(boot_params.hdr.hardware_subarch) & fn->supp_hardware_subarch)  
                return true;                                                    
        return false;                                                           
} 

struct x86_init_fn would have a __u32 supp_hardware_subarch of supported
subarchs as a bitmask.

What gains would we have to fold such a subarch check on early code?
Well for starters it can provide a proactive means stop-gap measure
for discrepancies between the different entry points to early boot
code. When developers write new early boot code they'd have to
think about which subarchs are supported.

So we can take either approach:

  a) use subarch with the plan to only use it only early x86
     boot code, in this case something like x86_init_early_pc_simple().
  b) add x86_ebda_bios flag and have PV disable it as you note

But I think if we wanted to remain compatible with a proactive strategy
using a) strategy we'd need a flag 'per feature'? I think approach a) scales
a bit more easily in this regard.

Mind you, if you say ebda should not use subarch I understand that it does not
mean other code which I'm adding should not use this linker table arrangement
and subarch, in particular if the subarch was already used (see use of linker
table on MID early boot code [0]), but some guidance if it should not
even be considered for something proactive as I'm trying to achieve as
with kasan (as an example) would be appreciated.

[0] http://lkml.kernel.org/r/1455891343-10016-7-git-send-email-mcgrof@kernel.org

>  - patch #5, #6, #7: looks good, does not use a subarch flag

OK thanks.

>  - patch #8: f00f workaround. Subarch flag use is wrong. The complication with 
>    this workaround is that it uses MM tricks to install an IDT. Could you check 
>    whether Xen truly needs this quirk?

I reviewed this at the Xen developer summit and have been through these
patches: the theory goes that xen and lguest do not need the f00f workaround
(so as per your language needs the quirk) as the hypervisor should be the one
doing the work around. Konrad also mentioned though that he actually thought
that perhaps the work around alone could be removed from the kernel all
together already -- but he was unable to provide a direct confirmation of that.

So yes we need it.

>    If yes then there should be a new flag, 
>    something like x86_idt_readonly, which is set to 0 but Xen can set it to 1. If 
>    that flag is set then the F00F workaround does not have to be installed.
> 
>    Or something like that: the point is to use a specific flag.

OK.

>  - Patch #9, #10: RTC support. The problem with RTC platform driver is that it's 
>    not possible to detect the RTC reliably - so we sometimes have to quirk it off.
> 
>    Instead of using bitflags, add something like x86_platform.rtc_available, which 
>    defaults to 1. Don't add negation to the name and don't use bitflags - use a 
>    byte flag.

OK.

>  - Patch #11: this patch wants to disable the PNP BIOS code.
> 
>    The complication with the PNP BIOS code is that the PNP BIOS is defined 
>    in physical memory, in the 0xf0000-0xffff0 memory range - a 64kB large area. 
>    Paravirt images don't want to waste 64kB of RAM just to tell the kernel that no
>    PNP BIOS is available.
> 
>    But instead of using subarch flags, please define a pnpbios_disable() API call 
>    that paravirt bootup can call into. That can disable the PNP BIOS code.

OK, sure, much better thanks.

> Also, feel free to define a single 'disable PC legacies' quirk function that 
> disables all the usual PC legacies that paravirt does not care about or does not 
> want to define via RAM. Specific paravirt initialization functions would only have 
> to call this one (shared) function.

Sure.

  Luis

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

[David Vrabel]

On 23/02/16 07:24, Luis R. Rodriguez wrote:
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
[...]
> @@ -1526,8 +1525,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
>  
>  	/* Install Xen paravirt ops */
>  	pv_info = xen_info;
> -	if (xen_initial_domain())
> -		pv_info.features |= PV_SUPPORTED_RTC;
>  	pv_init_ops = xen_init_ops;
>  	if (!xen_pvh_domain()) {
>  		pv_cpu_ops = xen_cpu_ops;
> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> index a0a4e554c6f1..fbfcb01015f0 100644
> --- a/arch/x86/xen/time.c
> +++ b/arch/x86/xen/time.c
> @@ -446,8 +446,10 @@ void __init xen_init_time_ops(void)
>  	x86_platform.calibrate_tsc = xen_tsc_khz;
>  	x86_platform.get_wallclock = xen_get_wallclock;
>  	/* Dom0 uses the native method to set the hardware RTC. */
> -	if (!xen_initial_domain())
> +	if (!xen_initial_domain()) {
> +		x86_init.platform_flags |= X86_PLATFORM_NO_RTC;
>  		x86_platform.set_wallclock = xen_set_wallclock;
> +	}
>  }

Is this an early enough point to set this flag?

David

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

[Luis R. Rodriguez]

On Tue, Feb 23, 2016 at 11:57:16AM +0000, David Vrabel wrote:
> On 23/02/16 07:24, Luis R. Rodriguez wrote:
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> [...]
> > @@ -1526,8 +1525,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
> >  
> >  	/* Install Xen paravirt ops */
> >  	pv_info = xen_info;
> > -	if (xen_initial_domain())
> > -		pv_info.features |= PV_SUPPORTED_RTC;
> >  	pv_init_ops = xen_init_ops;
> >  	if (!xen_pvh_domain()) {
> >  		pv_cpu_ops = xen_cpu_ops;
> > diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> > index a0a4e554c6f1..fbfcb01015f0 100644
> > --- a/arch/x86/xen/time.c
> > +++ b/arch/x86/xen/time.c
> > @@ -446,8 +446,10 @@ void __init xen_init_time_ops(void)
> >  	x86_platform.calibrate_tsc = xen_tsc_khz;
> >  	x86_platform.get_wallclock = xen_get_wallclock;
> >  	/* Dom0 uses the native method to set the hardware RTC. */
> > -	if (!xen_initial_domain())
> > +	if (!xen_initial_domain()) {
> > +		x86_init.platform_flags |= X86_PLATFORM_NO_RTC;
> >  		x86_platform.set_wallclock = xen_set_wallclock;
> > +	}
> >  }
> 
> Is this an early enough point to set this flag?

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

  Luis

Re: [PATCH v3 01/11] x86/boot: enumerate documentation for the x86 hardware_subarch

[Luis R. Rodriguez]

On Tue, Feb 23, 2016 at 11:34:09AM +0100, Luis R. Rodriguez wrote:
> On Tue, Feb 23, 2016 at 09:51:19AM +0100, Ingo Molnar wrote:
> > 
> > * Luis R. Rodriguez <mcgrof@kernel.org> 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.
> > > 
> > > v2: fix typos
> > > 
> > > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> > > ---
> > >  arch/x86/include/uapi/asm/bootparam.h | 31 ++++++++++++++++++++++++++++++-
> > >  1 file changed, 30 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
> > > index 329254373479..50d5009cf276 100644
> > > --- a/arch/x86/include/uapi/asm/bootparam.h
> > > +++ b/arch/x86/include/uapi/asm/bootparam.h
> > > @@ -157,7 +157,36 @@ 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 supports 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 do not 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.
> > > + *
> > > + * 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.
> > > + *
> > > + * @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,
> > 
> > No, this is really backwards.
> > 
> > While I agree that we want to get rid of paravirt_enabled(), we _dont_ want to 
> > spread the use of (arguably broken) boot flags like this!
> 
> I agree that we should not see the spread of boot flags around general x86
> code, its not my goal to spread it though, the code that uses it here though is
> *early boot code* (although in retrospect the pnpbios use was a fuckup), and I
> have some special considerations for early boot code which I think does give
> merit to it use. But also keep in mind my goal is to rather fold the boot flag
> so its more just an architectural consideration eventually. More on this below.

It seems I TL;DR suck; all this is a long winded way of asking, can we keep the
subarch just for EBDA and use the flags for the other things as you noted?

  Luis

Re: [PATCH v3 01/11] x86/boot: enumerate documentation for the x86 hardware_subarch

[Ingo Molnar]

* Luis R. Rodriguez <mcgrof@kernel.org> wrote:

> > > > +enum x86_hardware_subarch {
> > > >  	X86_SUBARCH_PC = 0,
> > > >  	X86_SUBARCH_LGUEST,
> > > >  	X86_SUBARCH_XEN,
> > > 
> > > No, this is really backwards.
> > > 
> > > While I agree that we want to get rid of paravirt_enabled(), we _dont_ want to 
> > > spread the use of (arguably broken) boot flags like this!
> > 
> > I agree that we should not see the spread of boot flags around general x86 
> > code, its not my goal to spread it though, the code that uses it here though 
> > is *early boot code* (although in retrospect the pnpbios use was a fuckup), 
> > and I have some special considerations for early boot code which I think does 
> > give merit to it use. But also keep in mind my goal is to rather fold the boot 
> > flag so its more just an architectural consideration eventually. More on this 
> > below.
> 
> It seems I TL;DR suck; all this is a long winded way of asking, can we keep the 
> subarch just for EBDA and use the flags for the other things as you noted?

No, even for EBDA we should make the flag EBDA specific, because that really tells 
us what it's about.

The EBDA code could not care less about what subarch it's running on - it only 
cares about whether it's safe to search for the EBDA signature in the hardware's 
low RAM.

So instead of this:

@@ -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 (boot_params.hdr.hardware_subarch != X86_SUBARCH_PC)
                return;

I'd suggest adding an EBDA search flag, something like this:

	if (!x86_platform.legacy.ebda_search)
		return;

Note that the 'legacy' intermediate structure can be used to group various 
legacies, while making it clear that this is not something modern hardware should 
care about.

The x86_platform.legacy.ebda_search flag can then be set up during (very) early 
bootup:

	if (boot_params.hdr.hardware_subarch == X86_SUBARCH_PC)
		x86_platform.legacy.ebda_search = 1;

This might look like an unnecessary indirection but it isn't: beyond the 
documentation value it also makes things a lot clearer should we introduce other 
legacy flags in x86_platform.legacy.

For hard coded platform quirks I'd suggest we add x86_platform.quirks flags. For 
example the F00F hack for Xen could be done via:

	x86_platform.quirks.idt_remap = 0;

and would be set like this during early init:

void early_init_platform_quirks(void)
{
	x86_platform.legacy.ebda_search = 0;
	x86_platform.quirks.idt_remap = 1;

	switch (boot_params.hdr.hardware_subarch) {
		case X86_SUBARCH_PC:
			x86_platform.legacy.ebda_search = 1;
			break;
		case X86_SUBARCH_XEN:
			x86_platform.quirks.idt_remap = 0;
			break;
		case X86_SUBARCH_LGUEST:
			x86_platform.quirks.idt_remap = 0;
			break;
	}
}

And if also add the legacy RTC flag, it becomes:

void early_init_hardcoded_platform_quirks(void)
{
	x86_platform.legacy.ebda_search = 0;
	x86_platform.quirks.idt_remap = 1;
	x86_platform.legacy.rtc = 1;

	switch (boot_params.hdr.hardware_subarch) {
		case X86_SUBARCH_PC:
			x86_platform.legacy.ebda_search = 1;
			break;
		case X86_SUBARCH_XEN:
			x86_platform.quirks.idt_remap = 0;
			x86_platform.legacy.rtc = 0;
			break;
		case X86_SUBARCH_LGUEST:
			x86_platform.quirks.idt_remap = 0;
			x86_platform.legacy.rtc = 0;
			break;
	}
}

Note that both opt-in and opt-out quirks/legacies are possible this way, and note 
how cleanly and consistently it's all organized: setup of all hard coded 
legacies/quirks is concentrated in a single function, and the actual usage sites 
don't know anything about subarchitectures.

Ok? Functionally this approach is equivalent to your series, but it's cleaner, and 
it's also easier to maintain in the long run: there's only a single place to look 
to check all hard coded legacies/quirks - instead of the code being spread out all 
around the x86 code.

Furthermore we should probably move a few other existing legacies to this flag 
space as well, to make all this more consistent.

Thanks,

	Ingo

Re: [PATCH v3 01/11] x86/boot: enumerate documentation for the x86 hardware_subarch

[Andy Lutomirski]

On Feb 24, 2016 12:33 AM, "Ingo Molnar" <mingo@kernel.org> wrote:
>
> For hard coded platform quirks I'd suggest we add x86_platform.quirks flags. For
> example the F00F hack for Xen could be done via:
>
>         x86_platform.quirks.idt_remap = 0;
>

Don't we unconditionally remap the IDT?  I think Kees did it for
general purpose hardening due to our complete inability to hide the
IDT address. I.e. I think we can remove the f00f condition entirely.

Other than that, I agree with you.

Re: [PATCH v3 01/11] x86/boot: enumerate documentation for the x86 hardware_subarch

[Andy Lutomirski]

On Wed, Feb 24, 2016 at 5:18 PM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
>
> On Feb 24, 2016 8:40 AM, "Andy Lutomirski" <luto@amacapital.net> wrote:
>>
>> On Feb 24, 2016 12:33 AM, "Ingo Molnar" <mingo@kernel.org> wrote:
>> >
>> > For hard coded platform quirks I'd suggest we add x86_platform.quirks
>> > flags. For
>> > example the F00F hack for Xen could be done via:
>> >
>> >         x86_platform.quirks.idt_remap = 0;
>> >
>>
>> Don't we unconditionally remap the IDT?  I think Kees did it for
>> general purpose hardening due to our complete inability to hide the
>> IDT address. I.e. I think we can remove the f00f condition entirely.
>>
>
> Kees can you confirm ?
>

No need.

        /*
         * Set the IDT descriptor to a fixed read-only location, so that the
         * "sidt" instruction will not leak the location of the kernel, and
         * to defend the IDT against arbitrary memory write vulnerabilities.
         * It will be reloaded in cpu_init() */
        __set_fixmap(FIX_RO_IDT, __pa_symbol(idt_table), PAGE_KERNEL_RO);
        idt_descr.address = fix_to_virt(FIX_RO_IDT);

IIUI this works around f00f as a side effect.  The only other thing
needed is the code that X86_BUG_F00F guards, which is responsible for
fixing up the error generated on attempted F00F exploitation from an
OOPS to a SIGILL.  I see no reason why that code couldn't be allowed
to run on even a PV guest on a F00F-affected CPU -- it would never
trigger anyway.

Re: [PATCH v3 01/11] x86/boot: enumerate documentation for the x86 hardware_subarch

[Ingo Molnar]

* Andy Lutomirski <luto@amacapital.net> wrote:

> On Feb 24, 2016 12:33 AM, "Ingo Molnar" <mingo@kernel.org> wrote:
> >
> > For hard coded platform quirks I'd suggest we add x86_platform.quirks flags. For
> > example the F00F hack for Xen could be done via:
> >
> >         x86_platform.quirks.idt_remap = 0;
> >
> 
> Don't we unconditionally remap the IDT?  I think Kees did it for
> general purpose hardening due to our complete inability to hide the
> IDT address. I.e. I think we can remove the f00f condition entirely.

Yeah, indeed - I only judged by the (limited) patch context and assumed the Xen 
problem was with IDT remapping.

But what the quirk really does is only to avoid printing the f00f workaround - 
i.e. a cosmetic change. I think we should just drop the paravirt_enabled() check.

Thanks,

	Ingo

Re: [PATCH v3 01/11] x86/boot: enumerate documentation for the x86 hardware_subarch

[Luis R. Rodriguez]

On Wed, Feb 24, 2016 at 09:32:59AM +0100, Ingo Molnar wrote:
> And if also add the legacy RTC flag, it becomes:
> 
> void early_init_hardcoded_platform_quirks(void)
> {
> 	x86_platform.legacy.ebda_search = 0;
> 	x86_platform.quirks.idt_remap = 1;
> 	x86_platform.legacy.rtc = 1;
> 
> 	switch (boot_params.hdr.hardware_subarch) {
> 		case X86_SUBARCH_PC:
> 			x86_platform.legacy.ebda_search = 1;
> 			break;
> 		case X86_SUBARCH_XEN:
> 			x86_platform.quirks.idt_remap = 0;
> 			x86_platform.legacy.rtc = 0;
> 			break;
> 		case X86_SUBARCH_LGUEST:
> 			x86_platform.quirks.idt_remap = 0;
> 			x86_platform.legacy.rtc = 0;
> 			break;
> 	}
> }
> 
> Note that both opt-in and opt-out quirks/legacies are possible this way, and note 
> how cleanly and consistently it's all organized: setup of all hard coded 
> legacies/quirks is concentrated in a single function, and the actual usage sites 
> don't know anything about subarchitectures.

I've tried this and so far so good, and I agree and I like how the quirks
are bundled in one place.

> Ok? Functionally this approach is equivalent to your series, but it's cleaner, and 
> it's also easier to maintain in the long run: there's only a single place to look 
> to check all hard coded legacies/quirks - instead of the code being spread out all 
> around the x86 code.

Sure, but I'll note I was not intending on spreading use of subarch further,
the use of subarch in pnpbios was certainly an overlooked mistake on my part.

There's only one problem with this strategy I can think so far which differs
from my original approach, which is partly why I actually started looking at
this stuff:

  it doesn't help us pro-actively vet each early boot sequence
  thrown at the x86 path well work on all required subarchs

The scope of addressing requirements I'm trying to address are things stuffed
into x86 init path or the kernel's init path from the first entry point down
to perhaps arch specific setup_arch() calls. Now granted we don't get
modifications in this area a lot, but when we do, if folks did not consider
our different requirements (supporting each subarch/entry path) chances are
high we'll crash or have a feature not fixed / not considered a subarch.

Case in point, kasan. Its still busted on Xen and no one seems to care.
Too late. How do we proactively prevent such things ? Granted peer review
should have caught this, but why not also do something proactively ?

The quirks stuff / proactive solution should perhaps be considered orthogonal.
It just so happened that I was able to address some quirks with what I was
doing, and obviously there's a better way for those. The use of
paravirt_enabled() for quirks also happened to reveal the lack of clarity
on semantics between paravirtualized environments / non-PV hypervisors and
late boot PV/hypervisor checks.

If you can think of another proactive strategy let me know.

> Furthermore we should probably move a few other existing legacies to this flag 
> space as well, to make all this more consistent.

Indeed.

  Luis

Re: [PATCH v3 01/11] x86/boot: enumerate documentation for the x86 hardware_subarch

[Luis R. Rodriguez]

On Wed, Mar 02, 2016 at 01:43:42AM +0100, Luis R. Rodriguez wrote:
> On Wed, Feb 24, 2016 at 09:32:59AM +0100, Ingo Molnar wrote:
> There's only one problem with this strategy I can think so far which differs
> from my original approach, which is partly why I actually started looking at
> this stuff:
> 
>   it doesn't help us pro-actively vet each early boot sequence
>   thrown at the x86 path well work on all required subarchs
> 
> The quirks stuff / proactive solution should perhaps be considered orthogonal.
> It just so happened that I was able to address some quirks with what I was

Since it is orthogonal I'll simply work off on the paravirt_enabled() removal
separately as its possible. Since the clarity on semantics will be needed
for other work I'm doing (proactive solution to avoid issues on early boot)
and since the proposed alternative still uses subarch for the quirks as
you recommended I'll at least still push for documentation update on subarch
use as well for now.

After this, and then after sort a simple link table upstream I can then start
focusing more on the proactive solution once again. That should help keep
things separate and make it clearer what I'm trying to achieve later.

  Luis

Re: [PATCH v3 01/11] x86/boot: enumerate documentation for the x86 hardware_subarch

[Luis R. Rodriguez]

David, please note below the highlighted code.

On Wed, Feb 24, 2016 at 09:32:59AM +0100, Ingo Molnar wrote:
> 
> * Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> For hard coded platform quirks I'd suggest we add x86_platform.quirks flags. For 
> example the F00F hack for Xen could be done via:
> 
> 	x86_platform.quirks.idt_remap = 0;
> 
> and would be set like this during early init:
> 
> void early_init_platform_quirks(void)
> {
> 	x86_platform.legacy.ebda_search = 0;
> 	x86_platform.quirks.idt_remap = 1;
> 
> 	switch (boot_params.hdr.hardware_subarch) {
> 		case X86_SUBARCH_PC:
> 			x86_platform.legacy.ebda_search = 1;
> 			break;
> 		case X86_SUBARCH_XEN:
> 			x86_platform.quirks.idt_remap = 0;
> 			break;
> 		case X86_SUBARCH_LGUEST:
> 			x86_platform.quirks.idt_remap = 0;
> 			break;
> 	}
> }
> 
> And if also add the legacy RTC flag, it becomes:
> 
> void early_init_hardcoded_platform_quirks(void)
> {
> 	x86_platform.legacy.ebda_search = 0;
> 	x86_platform.quirks.idt_remap = 1;
> 	x86_platform.legacy.rtc = 1;
> 
> 	switch (boot_params.hdr.hardware_subarch) {
> 		case X86_SUBARCH_PC:
> 			x86_platform.legacy.ebda_search = 1;
> 			break;
> 		case X86_SUBARCH_XEN:
> 			x86_platform.quirks.idt_remap = 0;
> 			x86_platform.legacy.rtc = 0;
> 			break;
> 		case X86_SUBARCH_LGUEST:
> 			x86_platform.quirks.idt_remap = 0;
> 			x86_platform.legacy.rtc = 0;
> 			break;
> 	}
> }
> 
> Note that both opt-in and opt-out quirks/legacies are possible this way, and note 
> how cleanly and consistently it's all organized: setup of all hard coded 
> legacies/quirks is concentrated in a single function, and the actual usage sites 
> don't know anything about subarchitectures.

<-- snip -- > 

So.. I went with Ingo's template.

> Furthermore we should probably move a few other existing legacies to this flag 
> space as well, to make all this more consistent.

And this suggestion should explain a bit of the effort I put into making
room for other legacy things, which I'll elaborate in the other thread.

  Luis