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

[David Vrabel <david.vrabel@citrix.com>]

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