Re: [RFC PATCH v1 2/2] xen_nopv: Combine a bunch of the PV features that can be disabled

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 10/07/2015 04:21 PM, Konrad Rzeszutek Wilk wrote:
> under one parameter. Removes the xen_nopvspin parameter and
> makes it part of the xen_nopv.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>   Documentation/kernel-parameters.txt | 15 +++++------
>   arch/x86/xen/enlighten.c            | 50 +++++++++++++++++++++++++++++++++----
>   arch/x86/xen/spinlock.c             | 18 ++++---------
>   arch/x86/xen/xen-ops.h              |  4 +++
>   4 files changed, 62 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 22a4b68..73cd745 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -4125,13 +4125,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>   				the unplug protocol
>   			never -- do not unplug even if version check succeeds
>   
> -	xen_nopvspin	[X86,XEN]
> -			Disables the ticketlock slowpath using Xen PV
> -			optimizations.
> -
> -	xen_nopv	[X86]
> -			Disables the PV optimizations forcing the HVM guest to
> -			run as generic HVM guest with no PV drivers.
> +	xen_nopv=	[X86,XEN]
> +			Disables various (or all) PV optimizations forcing the
> +			HVM (or PV) guest to run without them.
> +			Format: [off0,][off]

{ [spin][,ipi] | all }


> +			all -- every PV feature on HVM.
> +			spin -- Disables the ticketlock slowpath using Xen PV
> +                                optimizations (PV and HVM).
> +			ipi - Disable PV event channel IPI (on HVM).
>   
>   	xirc2ps_cs=	[NET,PCMCIA]
>   			Format:
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 30d12af..c644d2c 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1829,17 +1829,57 @@ static void __init xen_hvm_guest_init(void)
>   }
>   #endif
>   
> -static bool xen_nopv = false;
> +static unsigned int xen_nopv_feat;
> +#define XEN_NOPV_SPIN	1<<1
> +#define XEN_NOPV_IPI	1<<2
> +#define XEN_NOPV_ALL	(XEN_NOPV_SPIN | XEN_NOPV_IPI)
> +
> +bool xen_no_pvspin(void)
> +{
> +	return xen_nopv_feat & XEN_NOPV_SPIN;
> +}
> +
> +bool xen_no_pv(void)
> +{
> +	return xen_nopv_feat & XEN_NOPV_ALL;
> +}

Should XEN_NOPV_ALL be the 'OR' of the other two? I think it implies 
them but is broader than that. We may decide to have NOPV_IPI and 
NOPV_SPIN but still run a Xen-aware guest (i.e. we want to return 'true' 
from xen_platform()).

(Besides, this will return true if only one of the two bits is set.)

> +
> +bool xen_no_pvipi(void)
> +{
> +	return xen_nopv_feat & XEN_NOPV_IPI;
> +}

I don't see anyone using this.

Also, I think these should be inlines.


-boris