linux-kernel - Re: [Xen-devel] [PATCH 0/9] x86/init: replace paravirt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56C71A03.7030105@citrix.com>
Date:	Fri, 19 Feb 2016 13:34:59 +0000
From:	David Vrabel <david.vrabel@...rix.com>
To:	"Luis R. Rodriguez" <mcgrof@...nel.org>, <bp@...en8.de>
CC:	<rusty@...tcorp.com.au>, <x86@...nel.org>,
	<linux-kernel@...r.kernel.org>, <luto@...capital.net>,
	<xen-devel@...ts.xensource.com>, <david.vrabel@...rix.com>,
	<boris.ostrovsky@...cle.com>
Subject: Re: [Xen-devel] [PATCH 0/9] x86/init: replace paravirt_enabled() were
 possible

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