[PATCH 0/9] x86/init: replace paravirt_enabled() were possible

[PATCH 0/9] x86/init: replace paravirt_enabled() were possible

[Luis R. Rodriguez]

I originally set out to rename paravirt_enabled() to paravirt_legacy()
but we instead decided to remove paravirt_enabled() completely. Although
I have some linker table work which will help make this cleaner, instead
of waiting for that to go in, just remove the known cases that should
be safe for paravirt_enabled() and also replace it for the subarch
use.

While at it, document the subarch to avoid any possible misuses.

Luis R. Rodriguez (9):
  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
  apm32: remove paravirt_enabled() use
  x86/tboot: remove paravirt_enabled()
  x86/cpu/intel: replace paravirt_enabled() for f00f work around
  x86/rtc: replace paravirt_enabled() check with subarch check
  pnpbios: replace paravirt_enabled() check with subarch checks

 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/uapi/asm/bootparam.h | 32 +++++++++++++++++++++++++++++++-
 arch/x86/kernel/apm_32.c              |  2 +-
 arch/x86/kernel/cpu/intel.c           |  4 +++-
 arch/x86/kernel/head.c                |  2 +-
 arch/x86/kernel/rtc.c                 |  3 ++-
 arch/x86/kernel/tboot.c               |  6 ------
 arch/x86/xen/enlighten.c              |  4 +---
 drivers/pnp/pnpbios/core.c            |  4 +++-
 tools/lguest/lguest.c                 |  4 ++--
 12 files changed, 44 insertions(+), 29 deletions(-)

-- 
2.7.0

[PATCH 1/9] 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.

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

diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index 329254373479..dbfb9406436b 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -157,7 +157,37 @@ 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_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 nternet 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

Re: [Xen-devel] [PATCH 1/9] x86/boot: enumerate documentation for the x86 hardware_subarch

[Juergen Gross]

On 19/02/16 14:08, 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.
> 
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
>  arch/x86/include/uapi/asm/bootparam.h | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
> index 329254373479..dbfb9406436b 100644
> --- a/arch/x86/include/uapi/asm/bootparam.h
> +++ b/arch/x86/include/uapi/asm/bootparam.h
> @@ -157,7 +157,37 @@ 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_LGUEST: Used for x86 hypervisor demo, lguest

Double line

> + * @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 nternet Device) platform

s/nternet/Internet/

> + *	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,
> 

Juergen

Re: [Xen-devel] [PATCH 1/9] x86/boot: enumerate documentation for the x86 hardware_subarch

[David Vrabel]

On 19/02/16 13:08, 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.
> 
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
>  arch/x86/include/uapi/asm/bootparam.h | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
> index 329254373479..dbfb9406436b 100644
> --- a/arch/x86/include/uapi/asm/bootparam.h
> +++ b/arch/x86/include/uapi/asm/bootparam.h
> @@ -157,7 +157,37 @@ 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.

This documentation reads like a plan for future implementation.  Is this
the level of documentation that is needed here?

Also, "revector to a subarchitecture stub" is a rather odd way of saying
"call a subarch-specific stub".

David

Re: [Xen-devel] [PATCH 1/9] x86/boot: enumerate documentation for the x86 hardware_subarch

[Luis R. Rodriguez]

On Fri, Feb 19, 2016 at 01:40:33PM +0000, David Vrabel wrote:
> On 19/02/16 13:08, 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.
> > 
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> > ---
> >  arch/x86/include/uapi/asm/bootparam.h | 32 +++++++++++++++++++++++++++++++-
> >  1 file changed, 31 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
> > index 329254373479..dbfb9406436b 100644
> > --- a/arch/x86/include/uapi/asm/bootparam.h
> > +++ b/arch/x86/include/uapi/asm/bootparam.h
> > @@ -157,7 +157,37 @@ 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.
> 
> This documentation reads like a plan for future implementation.  Is this
> the level of documentation that is needed here?
> 
> Also, "revector to a subarchitecture stub" is a rather odd way of saying
> "call a subarch-specific stub".

I took feedback directly from hpa's replies about what the subarch is from
my original linker table series. So its not me concocting this. It also
shows how your own lack of clarify on this isn't doing us any good either,
and its precisely why I am documenting this now.

The lack of semantics here has gotten us intro trouble as it is not allowing
us to tool in proactive solutions to prevent bugs. My goal is first to
clarify some definitions, and then use them and tool them to proactiveley
avoid bugs.

  Luis

[PATCH 2/9] 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 3/9] 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 4/9] 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 5/9] 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

Re: [PATCH 5/9] apm32: remove paravirt_enabled() use

[Boris Ostrovsky]

On 02/19/2016 08:08 AM, 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) do never set the apm_bios_info,
> the paravirt_enabled() check is simply not needed.

We need to guarantee that boot_params is filled with zeroes. On 
baremetal path we clear .bss (which is where boot_params live) before 
copying data from zero page.

So we need to at least memset(&boot_params, 0, sz) in 
xen_start_kernel(). Better yet, clear whole .bss.

(This applies to the next patch as well).

-boris

Re: [PATCH 5/9] apm32: remove paravirt_enabled() use

[Luis R. Rodriguez]

On Fri, Feb 19, 2016 at 10:08:43AM -0500, Boris Ostrovsky wrote:
> 
> 
> On 02/19/2016 08:08 AM, 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) do never set the apm_bios_info,
> >the paravirt_enabled() check is simply not needed.
> 
> We need to guarantee that boot_params is filled with zeroes. On
> baremetal path we clear .bss (which is where boot_params live)
> before copying data from zero page.

To be clear Xen is the only user of this that just sets it
outright instead of poking at the file and using that as
reference, and setting what it needs. Its a good point
though that things like this which are 0 still should
probably be set.

> So we need to at least memset(&boot_params, 0, sz)

This use case would be just Xen specific, for lguest
we can just set the things we need clearly on the
launcher. So like this (I can fold in a separate
patch):


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);
 

> in xen_start_kernel(). Better yet, clear whole .bss.
> 
> (This applies to the next patch as well).

So clear_bss() -- oh look, another call that xen_start_kernel() could have made
good use of. :)  Can you send a respective patch I can old into this series?
I'm afraid it is by no means obvious to me where it would be safe to do this on
xen_start_kernel().

  Luis

Re: [PATCH 5/9] apm32: remove paravirt_enabled() use

[Boris Ostrovsky]

On 02/19/2016 03:58 PM, Luis R. Rodriguez wrote:
> On Fri, Feb 19, 2016 at 10:08:43AM -0500, Boris Ostrovsky wrote:
>
>> in xen_start_kernel(). Better yet, clear whole .bss.
>>
>> (This applies to the next patch as well).
> So clear_bss() -- oh look, another call that xen_start_kernel() could have made
> good use of. :)  Can you send a respective patch I can old into this series?
> I'm afraid it is by no means obvious to me where it would be safe to do this on
> xen_start_kernel().

OK, I'll test it next week, it really should be done irrespective of 
what you are doing here.

-boris

Re: [PATCH 5/9] apm32: remove paravirt_enabled() use

[Luis R. Rodriguez]

On Fri, Feb 19, 2016 at 05:17:27PM -0500, Boris Ostrovsky wrote:
> 
> 
> On 02/19/2016 03:58 PM, Luis R. Rodriguez wrote:
> >On Fri, Feb 19, 2016 at 10:08:43AM -0500, Boris Ostrovsky wrote:
> >
> >>in xen_start_kernel(). Better yet, clear whole .bss.
> >>
> >>(This applies to the next patch as well).
> >So clear_bss() -- oh look, another call that xen_start_kernel() could have made
> >good use of. :)  Can you send a respective patch I can old into this series?
> >I'm afraid it is by no means obvious to me where it would be safe to do this on
> >xen_start_kernel().
> 
> OK, I'll test it next week, it really should be done irrespective of
> what you are doing here.

OK so it can go in separately?

  Luis

Re: [PATCH 5/9] apm32: remove paravirt_enabled() use

[Boris Ostrovsky]

On 02/19/2016 07:42 PM, Luis R. Rodriguez wrote:
> On Fri, Feb 19, 2016 at 05:17:27PM -0500, Boris Ostrovsky wrote:
>>
>> On 02/19/2016 03:58 PM, Luis R. Rodriguez wrote:
>>> On Fri, Feb 19, 2016 at 10:08:43AM -0500, Boris Ostrovsky wrote:
>>>
>>>> in xen_start_kernel(). Better yet, clear whole .bss.
>>>>
>>>> (This applies to the next patch as well).
>>> So clear_bss() -- oh look, another call that xen_start_kernel() could have made
>>> good use of. :)  Can you send a respective patch I can old into this series?
>>> I'm afraid it is by no means obvious to me where it would be safe to do this on
>>> xen_start_kernel().
>> OK, I'll test it next week, it really should be done irrespective of
>> what you are doing here.
> OK so it can go in separately?


Yes, I'll send it as a separate patch.

-boris

[PATCH 6/9] 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 7/9] 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().

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

diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 565648bc1a0a..26d8c496c909 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -220,7 +220,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 8/9] x86/rtc: replace paravirt_enabled() check with subarch check

[Luis R. Rodriguez]

The current check is a super long winded way of asking if this
is on lguest. The flags is used for legacy features, this is
likely inspired by the ACPI IA-PC boot architecture flags, where
as for RTC it annotates No CMOS real-time clock present. I don't
expect we will be implementing more legacy features, its simply
pointless so just remove this legacy flag feature thing as well.

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/kernel/rtc.c                 | 3 ++-
 arch/x86/xen/enlighten.c              | 3 ---
 5 files changed, 2 insertions(+), 16 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/kernel/rtc.c b/arch/x86/kernel/rtc.c
index 4af8d063fb36..8471523ee127 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
 /*
@@ -200,7 +201,7 @@ static __init int add_rtc_cmos(void)
 	}
 #endif
 
-	if (paravirt_enabled() && !paravirt_has(RTC))
+	if (boot_params.hdr.hardware_subarch == X86_SUBARCH_LGUEST)
 		return -ENODEV;
 
 	platform_device_register(&rtc_device);
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;
-- 
2.7.0

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

[Juergen Gross]

On 19/02/16 14:08, Luis R. Rodriguez wrote:
> The current check is a super long winded way of asking if this
> is on lguest. The flags is used for legacy features, this is

What about Xen pv-domU? I wouldn't expect those to have PV_SUPPORTED_RTC
set.


Juergen

> likely inspired by the ACPI IA-PC boot architecture flags, where
> as for RTC it annotates No CMOS real-time clock present. I don't
> expect we will be implementing more legacy features, its simply
> pointless so just remove this legacy flag feature thing as well.
> 
> 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/kernel/rtc.c                 | 3 ++-
>  arch/x86/xen/enlighten.c              | 3 ---
>  5 files changed, 2 insertions(+), 16 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/kernel/rtc.c b/arch/x86/kernel/rtc.c
> index 4af8d063fb36..8471523ee127 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
>  /*
> @@ -200,7 +201,7 @@ static __init int add_rtc_cmos(void)
>  	}
>  #endif
>  
> -	if (paravirt_enabled() && !paravirt_has(RTC))
> +	if (boot_params.hdr.hardware_subarch == X86_SUBARCH_LGUEST)
>  		return -ENODEV;
>  
>  	platform_device_register(&rtc_device);
> 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;
> 

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

[Luis R. Rodriguez]

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

Hrm, I see -- how do we check for that in a standard more clean way?

  Luis

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

[Luis R. Rodriguez]

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

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

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

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

So how about this:

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index f6192502149e..c261402340e3 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -19,12 +19,6 @@ static inline int paravirt_enabled(void)
 	return pv_info.paravirt_enabled;
 }
 
-static inline int paravirt_has_feature(unsigned int feature)
-{
-	WARN_ON_ONCE(!pv_info.paravirt_enabled);
-	return (pv_info.features & feature);
-}
-
 static inline void load_sp0(struct tss_struct *tss,
 			     struct thread_struct *thread)
 {
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 77db5616a473..2489d6a08e89 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -70,14 +70,9 @@ struct pv_info {
 #endif
 
 	int paravirt_enabled;
-	unsigned int features;	  /* valid only if paravirt_enabled is set */
 	const char *name;
 };
 
-#define paravirt_has(x) paravirt_has_feature(PV_SUPPORTED_##x)
-/* Supported features */
-#define PV_SUPPORTED_RTC        (1<<0)
-
 struct pv_init_ops {
 	/*
 	 * Patch may replace one of the defined code sequences with
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index 20c11d1aa4cc..10f3614265c1 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -472,7 +472,6 @@ static inline unsigned long current_top_of_stack(void)
 #else
 #define __cpuid			native_cpuid
 #define paravirt_enabled()	0
-#define paravirt_has(x) 	0
 
 static inline void load_sp0(struct tss_struct *tss,
 			    struct thread_struct *thread)
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 1ae89a2721d6..fe0d579b63e3 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -84,11 +84,14 @@ struct x86_init_paging {
  *				boot cpu
  * @timer_init:			initialize the platform timer (default PIT/HPET)
  * @wallclock_init:		init the wallclock device
+ * @no_cmos_rtc:		set when platform has no CMOS real-time clock
+ * 				present
  */
 struct x86_init_timers {
 	void (*setup_percpu_clockev)(void);
 	void (*timer_init)(void);
 	void (*wallclock_init)(void);
+	bool no_cmos_rtc;
 };
 
 /**
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index e75907601a41..6b2cac0f276b 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -913,6 +913,10 @@ late_initcall(hpet_insert_resource);
 
 static int __init acpi_parse_fadt(struct acpi_table_header *table)
 {
+	if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) {
+		pr_debug("ACPI: not registering RTC platform device\n");
+		x86_init.timers.no_cmos_rtc = true;
+	}
 
 #ifdef CONFIG_X86_PM_TIMER
 	/* detect the location of the ACPI PM Timer */
diff --git a/arch/x86/kernel/rtc.c b/arch/x86/kernel/rtc.c
index 4af8d063fb36..ef92aa84c2e3 100644
--- a/arch/x86/kernel/rtc.c
+++ b/arch/x86/kernel/rtc.c
@@ -14,6 +14,7 @@
 #include <asm/time.h>
 #include <asm/intel-mid.h>
 #include <asm/rtc.h>
+#include <asm/setup.h>
 
 #ifdef CONFIG_X86_32
 /*
@@ -188,19 +189,7 @@ static __init int add_rtc_cmos(void)
 	if (of_have_populated_dt())
 		return 0;
 
-	/* Intel MID platforms don't have ioport rtc */
-	if (intel_mid_identify_cpu())
-		return -ENODEV;
-
-#ifdef CONFIG_ACPI
-	if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) {
-		/* This warning can likely go away again in a year or two. */
-		pr_info("ACPI: not registering RTC platform device\n");
-		return -ENODEV;
-	}
-#endif
-
-	if (paravirt_enabled() && !paravirt_has(RTC))
+	if (x86_init.timers.no_cmos_rtc)
 		return -ENODEV;
 
 	platform_device_register(&rtc_device);
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index 4ba229ac3f4f..62535a869e06 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -1414,7 +1414,6 @@ __init void lguest_init(void)
 	pv_info.kernel_rpl = 1;
 	/* Everyone except Xen runs with this set. */
 	pv_info.shared_kernel_pmd = 1;
-	pv_info.features = 0;
 
 	/*
 	 * We set up all the lguest overrides for sensitive operations.  These
@@ -1482,6 +1481,7 @@ __init void lguest_init(void)
 	x86_init.resources.memory_setup = lguest_memory_setup;
 	x86_init.irqs.intr_init = lguest_init_IRQ;
 	x86_init.timers.timer_init = lguest_time_init;
+	x86_init.timers.no_cmos_rtc = true;
 	x86_platform.calibrate_tsc = lguest_tsc_khz;
 	x86_platform.get_wallclock =  lguest_get_wallclock;
 
diff --git a/arch/x86/platform/intel-mid/intel-mid.c b/arch/x86/platform/intel-mid/intel-mid.c
index 90bb997ed0a2..083369a6b215 100644
--- a/arch/x86/platform/intel-mid/intel-mid.c
+++ b/arch/x86/platform/intel-mid/intel-mid.c
@@ -175,6 +175,9 @@ void __init x86_intel_mid_early_setup(void)
 	x86_init.timers.timer_init = intel_mid_time_init;
 	x86_init.timers.setup_percpu_clockev = x86_init_noop;
 
+	/* Intel MID platforms don't have ioport rtc */
+	x86_init.timers.no_cmos_rtc = true;
+
 	x86_init.irqs.pre_vector_init = x86_init_noop;
 
 	x86_init.oem.arch_setup = intel_mid_arch_setup;
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 5b3f1c763806..5c06169bce27 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1192,7 +1192,6 @@ static const struct pv_info xen_info __initconst = {
 #ifdef CONFIG_X86_64
 	.extra_user_64bit_cs = FLAT_USER_CS64,
 #endif
-	.features = 0,
 	.name = "Xen",
 };
 
@@ -1526,8 +1525,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
 
 	/* Install Xen paravirt ops */
 	pv_info = xen_info;
-	if (xen_initial_domain())
-		pv_info.features |= PV_SUPPORTED_RTC;
 	pv_init_ops = xen_init_ops;
 	if (!xen_pvh_domain()) {
 		pv_cpu_ops = xen_cpu_ops;
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index a0a4e554c6f1..963cdaf63496 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -446,8 +446,10 @@ void __init xen_init_time_ops(void)
 	x86_platform.calibrate_tsc = xen_tsc_khz;
 	x86_platform.get_wallclock = xen_get_wallclock;
 	/* Dom0 uses the native method to set the hardware RTC. */
-	if (!xen_initial_domain())
+	if (!xen_initial_domain()) {
+		x86_init.timers.no_cmos_rtc = true;
 		x86_platform.set_wallclock = xen_set_wallclock;
+	}
 }
 
 #ifdef CONFIG_XEN_PVHVM

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

[Borislav Petkov]

On Mon, Feb 22, 2016 at 07:07:56AM +0100, Luis R. Rodriguez wrote:
> diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
> index 1ae89a2721d6..fe0d579b63e3 100644
> --- a/arch/x86/include/asm/x86_init.h
> +++ b/arch/x86/include/asm/x86_init.h
> @@ -84,11 +84,14 @@ struct x86_init_paging {
>   *				boot cpu
>   * @timer_init:			initialize the platform timer (default PIT/HPET)
>   * @wallclock_init:		init the wallclock device
> + * @no_cmos_rtc:		set when platform has no CMOS real-time clock
> + * 				present
>   */
>  struct x86_init_timers {
>  	void (*setup_percpu_clockev)(void);
>  	void (*timer_init)(void);
>  	void (*wallclock_init)(void);
> +	bool no_cmos_rtc;

I'd add

	u64 flags;

to x86_init_ops and then set X86_PLATFORM_NO_RTC or so in there. The
reason being, others could use that flags field too, for other stuff and
define more bits.

-- 
Regards/Gruss,
    Boris.

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

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

[Boris Ostrovsky]

On 02/22/2016 05:27 AM, Borislav Petkov wrote:
> On Mon, Feb 22, 2016 at 07:07:56AM +0100, Luis R. Rodriguez wrote:
>> diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
>> index 1ae89a2721d6..fe0d579b63e3 100644
>> --- a/arch/x86/include/asm/x86_init.h
>> +++ b/arch/x86/include/asm/x86_init.h
>> @@ -84,11 +84,14 @@ struct x86_init_paging {
>>    *				boot cpu
>>    * @timer_init:			initialize the platform timer (default PIT/HPET)
>>    * @wallclock_init:		init the wallclock device
>> + * @no_cmos_rtc:		set when platform has no CMOS real-time clock
>> + * 				present
>>    */
>>   struct x86_init_timers {
>>   	void (*setup_percpu_clockev)(void);
>>   	void (*timer_init)(void);
>>   	void (*wallclock_init)(void);
>> +	bool no_cmos_rtc;
> I'd add
>
> 	u64 flags;
>
> to x86_init_ops and then set X86_PLATFORM_NO_RTC or so in there. The
> reason being, others could use that flags field too, for other stuff and
> define more bits.

Maybe timer_flags or platform_flags (or something else) to be a little 
more cscope-friendly?

-boris

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

[Luis R. Rodriguez]

On Mon, Feb 22, 2016 at 09:34:26AM -0500, Boris Ostrovsky wrote:
> On 02/22/2016 05:27 AM, Borislav Petkov wrote:
> >On Mon, Feb 22, 2016 at 07:07:56AM +0100, Luis R. Rodriguez wrote:
> >>diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
> >>index 1ae89a2721d6..fe0d579b63e3 100644
> >>--- a/arch/x86/include/asm/x86_init.h
> >>+++ b/arch/x86/include/asm/x86_init.h
> >>@@ -84,11 +84,14 @@ struct x86_init_paging {
> >>   *				boot cpu
> >>   * @timer_init:			initialize the platform timer (default PIT/HPET)
> >>   * @wallclock_init:		init the wallclock device
> >>+ * @no_cmos_rtc:		set when platform has no CMOS real-time clock
> >>+ * 				present
> >>   */
> >>  struct x86_init_timers {
> >>  	void (*setup_percpu_clockev)(void);
> >>  	void (*timer_init)(void);
> >>  	void (*wallclock_init)(void);
> >>+	bool no_cmos_rtc;
> >I'd add
> >
> >	u64 flags;
> >
> >to x86_init_ops and then set X86_PLATFORM_NO_RTC or so in there. The
> >reason being, others could use that flags field too, for other stuff and
> >define more bits.
> 
> Maybe timer_flags or platform_flags (or something else) to be a
> little more cscope-friendly?

Sure, I'll go with platform_flags on x86_init_ops. Will repost a new
series after 0-day testing.

  Luis

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

[David Vrabel]

On 19/02/16 13:08, Luis R. Rodriguez wrote:
> The current check is a super long winded way of asking if this
> is on lguest. The flags is used for legacy features, this is
> likely inspired by the ACPI IA-PC boot architecture flags, where
> as for RTC it annotates No CMOS real-time clock present. I don't
> expect we will be implementing more legacy features, its simply
> pointless so just remove this legacy flag feature thing as well.

No.  This check is needed for Xen PV domU guests and without it PV
guests don't get a console (as I think the history of the change that
introduced this makes clear).

I think this trend towards a coarse-grained subarch instead of a set of
feature bits is the wrong direction.

David

[PATCH 9/9] 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: [Xen-devel] [PATCH 0/9] x86/init: replace paravirt_enabled() were possible

[David Vrabel]

On 19/02/16 13:08, Luis R. Rodriguez wrote:
> I originally set out to rename paravirt_enabled() to paravirt_legacy()
> but we instead decided to remove paravirt_enabled() completely. Although
> I have some linker table work which will help make this cleaner, instead
> of waiting for that to go in, just remove the known cases that should
> be safe for paravirt_enabled() and also replace it for the subarch
> use.

I don't understand the purpose of this series.  How is replacing "if
(A)" with "if (B)" an improvement?

Using the hardware subarch in this way looks like paravirt_enabled()
with a slightly different flavour and I don't think this is that useful
to determine if certain hardware features should be used.

I think you should consider a finer-grained set of feature bits.  e.g.,
platform_has_pc_rtc() and platform_has_tboot() etc.

David

> 
> While at it, document the subarch to avoid any possible misuses.
> 
> Luis R. Rodriguez (9):
>   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
>   apm32: remove paravirt_enabled() use
>   x86/tboot: remove paravirt_enabled()
>   x86/cpu/intel: replace paravirt_enabled() for f00f work around
>   x86/rtc: replace paravirt_enabled() check with subarch check
>   pnpbios: replace paravirt_enabled() check with subarch checks
> 
>  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/uapi/asm/bootparam.h | 32 +++++++++++++++++++++++++++++++-
>  arch/x86/kernel/apm_32.c              |  2 +-
>  arch/x86/kernel/cpu/intel.c           |  4 +++-
>  arch/x86/kernel/head.c                |  2 +-
>  arch/x86/kernel/rtc.c                 |  3 ++-
>  arch/x86/kernel/tboot.c               |  6 ------
>  arch/x86/xen/enlighten.c              |  4 +---
>  drivers/pnp/pnpbios/core.c            |  4 +++-
>  tools/lguest/lguest.c                 |  4 ++--
>  12 files changed, 44 insertions(+), 29 deletions(-)
> 

Re: [Xen-devel] [PATCH 0/9] x86/init: replace paravirt_enabled() were possible

[Luis R. Rodriguez]

On Fri, Feb 19, 2016 at 01:34:59PM +0000, David Vrabel wrote:
> On 19/02/16 13:08, Luis R. Rodriguez wrote:
> > I originally set out to rename paravirt_enabled() to paravirt_legacy()
> > but we instead decided to remove paravirt_enabled() completely. Although
> > I have some linker table work which will help make this cleaner, instead
> > of waiting for that to go in, just remove the known cases that should
> > be safe for paravirt_enabled() and also replace it for the subarch
> > use.
> 
> I don't understand the purpose of this series.  How is replacing "if
> (A)" with "if (B)" an improvement?

Its about semantics. Let's recall I first set out to simply document
paravirt_enabled() as per Konrad's recommendation I also renamed it
to avoid confusion. What we've learned is that paravirt_enabled()'s
semantics were a mishap due to two main factors:

  1) Industry moving way from legacy:
    a) Exhibit A: PC system design guide  [0] - Move by Intel and
       Microsoft to help build legacy free systems.
    b) EXhibit B: ACPI IA-PC boot architecture flags (all for legacy)
  2) The array of different guest types during this time.

The best thing we had was subarch added via boot protocol 2.07
but as I've learned only lguest and a few hacks picked it up,
even though Xen clearly had a subarch added for it. The current
"grey" area of different guest types does not help, but this
just means we lack way to further describe things.

[0] http://tech-insider.org/windows/research/2000/1102.html

> Using the hardware subarch in this way looks like paravirt_enabled()
> with a slightly different flavour and I don't think this is that useful
> to determine if certain hardware features should be used.

I agree, and it was not my goal to just leave it like that.
The *real* way I am targeting use of the subarch is reflected
in how I am proposing we use linker tables and dependency
relationships on x86. Please refer to that series where
such annotations added here are removed. I posted this
series separately as otherwise the clutch to removal of
paravirt_enabled() is the progress on linker tables. That
may take time.

> I think you should consider a finer-grained set of feature bits.  e.g.,
> platform_has_pc_rtc() and platform_has_tboot() etc.

Are you suggesting as further x86 boot protocol changes?

  Luis