diff mbox series

[3/6] x86: Add nopv parameter to disable PV extensions

Message ID 1561294903-6166-3-git-send-email-zhenzhong.duan@oracle.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Zhenzhong Duan June 23, 2019, 1:01 p.m. UTC
In virtualization environment, PV extensions (drivers, interrupts,
timers, etc) are enabled in the majority of use cases which is the
best option.

However, in some cases (kexec not fully working, benchmarking)
we want to disable PV extensions. As such introduce the
'nopv' parameter that will do it.

There is already 'xen_nopv' parameter for XEN platform but not for
others. 'xen_nopv' can then be removed with this change.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Cc: xen-devel@lists.xenproject.org
---
 Documentation/admin-guide/kernel-parameters.txt |  4 ++++
 arch/x86/kernel/cpu/hypervisor.c                | 11 +++++++++++
 2 files changed, 15 insertions(+)

Comments

Jürgen Groß June 24, 2019, 1:07 p.m. UTC | #1
On 23.06.19 15:01, Zhenzhong Duan wrote:
> In virtualization environment, PV extensions (drivers, interrupts,
> timers, etc) are enabled in the majority of use cases which is the
> best option.
> 
> However, in some cases (kexec not fully working, benchmarking)
> we want to disable PV extensions. As such introduce the
> 'nopv' parameter that will do it.
> 
> There is already 'xen_nopv' parameter for XEN platform but not for
> others. 'xen_nopv' can then be removed with this change.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> Cc: xen-devel@lists.xenproject.org
> ---
>   Documentation/admin-guide/kernel-parameters.txt |  4 ++++
>   arch/x86/kernel/cpu/hypervisor.c                | 11 +++++++++++
>   2 files changed, 15 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 138f666..b352f36 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5268,6 +5268,10 @@
>   			improve timer resolution at the expense of processing
>   			more timer interrupts.
>   
> +	nopv=		[X86]
> +			Disables the PV optimizations forcing the guest to run
> +			as generic guest with no PV drivers.
> +
>   	xirc2ps_cs=	[NET,PCMCIA]
>   			Format:
>   			<irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
> diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
> index 479ca47..4f2c875 100644
> --- a/arch/x86/kernel/cpu/hypervisor.c
> +++ b/arch/x86/kernel/cpu/hypervisor.c
> @@ -85,10 +85,21 @@ static void __init copy_array(const void *src, void *target, unsigned int size)
>   			to[i] = from[i];
>   }
>   
> +static bool nopv;
> +static __init int xen_parse_nopv(char *arg)

You really don't want to use the "xen_" prefix here.


Juergen
Jürgen Groß June 24, 2019, 1:18 p.m. UTC | #2
On 23.06.19 15:01, Zhenzhong Duan wrote:
> In virtualization environment, PV extensions (drivers, interrupts,
> timers, etc) are enabled in the majority of use cases which is the
> best option.
> 
> However, in some cases (kexec not fully working, benchmarking)
> we want to disable PV extensions. As such introduce the
> 'nopv' parameter that will do it.
> 
> There is already 'xen_nopv' parameter for XEN platform but not for
> others. 'xen_nopv' can then be removed with this change.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> Cc: xen-devel@lists.xenproject.org
> ---
>   Documentation/admin-guide/kernel-parameters.txt |  4 ++++
>   arch/x86/kernel/cpu/hypervisor.c                | 11 +++++++++++
>   2 files changed, 15 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 138f666..b352f36 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5268,6 +5268,10 @@
>   			improve timer resolution at the expense of processing
>   			more timer interrupts.
>   
> +	nopv=		[X86]
> +			Disables the PV optimizations forcing the guest to run
> +			as generic guest with no PV drivers.
> +
>   	xirc2ps_cs=	[NET,PCMCIA]
>   			Format:
>   			<irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
> diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
> index 479ca47..4f2c875 100644
> --- a/arch/x86/kernel/cpu/hypervisor.c
> +++ b/arch/x86/kernel/cpu/hypervisor.c
> @@ -85,10 +85,21 @@ static void __init copy_array(const void *src, void *target, unsigned int size)
>   			to[i] = from[i];
>   }
>   
> +static bool nopv;
> +static __init int xen_parse_nopv(char *arg)
> +{
> +	nopv = true;
> +	return 0;
> +}
> +early_param("nopv", xen_parse_nopv);
> +
>   void __init init_hypervisor_platform(void)
>   {
>   	const struct hypervisor_x86 *h;
>   
> +	if (nopv)
> +		return;
> +

Oh, this is no good idea.

There are guest types which just won't work without pv interfaces, like
Xen PV and Xen PVH. Letting them fail due to just a wrong command line
parameter is not nice, especially as the failure might be very hard to
track down to the issue for the user.

I guess you could add a "ignore_nopv" member to struct hypervisor_x86
set to true for the mentioned guest types and call the detect functions
only if nopv is false or ignore_nopv is true.


Juergen
Zhenzhong Duan June 24, 2019, 1:50 p.m. UTC | #3
On 2019/6/24 21:18, Juergen Gross wrote:
> On 23.06.19 15:01, Zhenzhong Duan wrote:
>> In virtualization environment, PV extensions (drivers, interrupts,
>> timers, etc) are enabled in the majority of use cases which is the
>> best option.
>>
>> However, in some cases (kexec not fully working, benchmarking)
>> we want to disable PV extensions. As such introduce the
>> 'nopv' parameter that will do it.
>>
>> There is already 'xen_nopv' parameter for XEN platform but not for
>> others. 'xen_nopv' can then be removed with this change.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
>> Cc: xen-devel@lists.xenproject.org
>> ---
>>   Documentation/admin-guide/kernel-parameters.txt |  4 ++++
>>   arch/x86/kernel/cpu/hypervisor.c                | 11 +++++++++++
>>   2 files changed, 15 insertions(+)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt 
>> b/Documentation/admin-guide/kernel-parameters.txt
>> index 138f666..b352f36 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -5268,6 +5268,10 @@
>>               improve timer resolution at the expense of processing
>>               more timer interrupts.
>>   +    nopv=        [X86]
>> +            Disables the PV optimizations forcing the guest to run
>> +            as generic guest with no PV drivers.
>> +
>>       xirc2ps_cs=    [NET,PCMCIA]
>>               Format:
>> <irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
>> diff --git a/arch/x86/kernel/cpu/hypervisor.c 
>> b/arch/x86/kernel/cpu/hypervisor.c
>> index 479ca47..4f2c875 100644
>> --- a/arch/x86/kernel/cpu/hypervisor.c
>> +++ b/arch/x86/kernel/cpu/hypervisor.c
>> @@ -85,10 +85,21 @@ static void __init copy_array(const void *src, 
>> void *target, unsigned int size)
>>               to[i] = from[i];
>>   }
>>   +static bool nopv;
>> +static __init int xen_parse_nopv(char *arg)
>> +{
>> +    nopv = true;
>> +    return 0;
>> +}
>> +early_param("nopv", xen_parse_nopv);
>> +
>>   void __init init_hypervisor_platform(void)
>>   {
>>       const struct hypervisor_x86 *h;
>>   +    if (nopv)
>> +        return;
>> +
>
> Oh, this is no good idea.
>
> There are guest types which just won't work without pv interfaces, like
> Xen PV and Xen PVH. Letting them fail due to just a wrong command line
> parameter is not nice, especially as the failure might be very hard to
> track down to the issue for the user.
Yes, thanks for catching.
>
> I guess you could add a "ignore_nopv" member to struct hypervisor_x86
> set to true for the mentioned guest types and call the detect functions
> only if nopv is false or ignore_nopv is true.

I think your suggestion is good, I'll rework it based on your suggestion.

Thanks

Zhenzhong
diff mbox series

Patch

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 138f666..b352f36 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5268,6 +5268,10 @@ 
 			improve timer resolution at the expense of processing
 			more timer interrupts.
 
+	nopv=		[X86]
+			Disables the PV optimizations forcing the guest to run
+			as generic guest with no PV drivers.
+
 	xirc2ps_cs=	[NET,PCMCIA]
 			Format:
 			<irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
index 479ca47..4f2c875 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -85,10 +85,21 @@  static void __init copy_array(const void *src, void *target, unsigned int size)
 			to[i] = from[i];
 }
 
+static bool nopv;
+static __init int xen_parse_nopv(char *arg)
+{
+	nopv = true;
+	return 0;
+}
+early_param("nopv", xen_parse_nopv);
+
 void __init init_hypervisor_platform(void)
 {
 	const struct hypervisor_x86 *h;
 
+	if (nopv)
+		return;
+
 	h = detect_hypervisor_vendor();
 
 	if (!h)