[Xen-devel] [PATCH v5] xen: get rid of paravirt op adjust_exception_frame

[Juergen Gross]

From: Juergen Gross <jgr...@suse.com>

When running as Xen pv-guest the exception frame on the stack contains
%r11 and %rcx additional to the other data pushed by the processor.

Instead of having a paravirt op being called for each exception type
prepend the Xen specific code to each exception entry. When running as
Xen pv-guest just use the exception entry with prepended instructions,
otherwise use the entry without the Xen specific code.

Signed-off-by: Juergen Gross <jgr...@suse.com>
---
 arch/x86/entry/entry_64.S | 23 ++--
 arch/x86/entry/entry_64_compat.S  |  1 -
 arch/x86/include/asm/paravirt.h   |  5 --
 arch/x86/include/asm/paravirt_types.h |  3 --
 arch/x86/include/asm/proto.h  |  3 ++
 arch/x86/include/asm/traps.h  | 28 --
 arch/x86/kernel/asm-offsets_64.c  |  1 -
 arch/x86/kernel/paravirt.c|  3 --
 arch/x86/xen/enlighten_pv.c   | 98 +++
 arch/x86/xen/irq.c|  3 --
 arch/x86/xen/xen-asm_64.S | 41 +--
 arch/x86/xen/xen-ops.h|  1 -
 12 files changed, 133 insertions(+), 77 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 7a1d383c2192..bdd024a9afc9 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -821,7 +821,6 @@ ENTRY(\sym)
.endif
 
ASM_CLAC
-   PARAVIRT_ADJUST_EXCEPTION_FRAME
 
.ifeq \has_error_code
pushq   $-1 /* ORIG_RAX: no syscall to 
restart */
@@ -967,7 +966,7 @@ ENTRY(do_softirq_own_stack)
 ENDPROC(do_softirq_own_stack)
 
 #ifdef CONFIG_XEN
-idtentry xen_hypervisor_callback xen_do_hypervisor_callback has_error_code=0
+idtentry hypervisor_callback xen_do_hypervisor_callback has_error_code=0
 
 /*
  * A note on the "critical region" in our callback handler.
@@ -1034,8 +1033,6 @@ ENTRY(xen_failsafe_callback)
movq8(%rsp), %r11
addq$0x30, %rsp
pushq   $0  /* RIP */
-   pushq   %r11
-   pushq   %rcx
UNWIND_HINT_IRET_REGS offset=8
jmp general_protection
 1: /* Segment mismatch => Category 1 (Bad segment). Retry the IRET. */
@@ -1066,9 +1063,8 @@ idtentry int3 do_int3 
has_error_code=0paranoid=1 shift_ist=DEBUG_STACK
 idtentry stack_segment do_stack_segmenthas_error_code=1
 
 #ifdef CONFIG_XEN
-idtentry xen_debug do_debughas_error_code=0
-idtentry xen_int3  do_int3 has_error_code=0
-idtentry xen_stack_segment do_stack_segmenthas_error_code=1
+idtentry xendebug  do_debughas_error_code=0
+idtentry xenint3   do_int3 has_error_code=0
 #endif
 
 idtentry general_protectiondo_general_protection   has_error_code=1
@@ -1232,21 +1228,10 @@ ENTRY(error_exit)
 END(error_exit)
 
 /* Runs on exception stack */
+/* XXX: broken on Xen PV */
 ENTRY(nmi)
UNWIND_HINT_IRET_REGS
/*
-* Fix up the exception frame if we're on Xen.
-* PARAVIRT_ADJUST_EXCEPTION_FRAME is guaranteed to push at most
-* one value to the stack on native, so it may clobber the rdx
-* scratch slot, but it won't clobber any of the important
-* slots past it.
-*
-* Xen is a different story, because the Xen frame itself overlaps
-* the "NMI executing" variable.
-*/
-   PARAVIRT_ADJUST_EXCEPTION_FRAME
-
-   /*
 * We allow breakpoints in NMIs. If a breakpoint occurs, then
 * the iretq it performs will take us out of NMI context.
 * This means that we can have nested NMIs where the next
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 5314d7b8e5ad..d8468ba24be0 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -293,7 +293,6 @@ ENTRY(entry_INT80_compat)
/*
 * Interrupts are off on entry.
 */
-   PARAVIRT_ADJUST_EXCEPTION_FRAME
ASM_CLAC/* Do this early to minimize exposure */
SWAPGS
 
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 9ccac1926587..c25dd22f7c70 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -960,11 +960,6 @@ extern void default_banner(void);
 #define GET_CR2_INTO_RAX   \
call PARA_INDIRECT(pv_mmu_ops+PV_MMU_read_cr2)
 
-#define PARAVIRT_ADJUST_EXCEPTION_FRAME
\
-   PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_adjust_exception_frame), \
- CLBR_NONE,\
- call PARA_INDIRECT(pv_irq_ops+PV_IRQ_adjust_exception

Re: [Xen-devel] linux-next: manual merge of the xen-tip tree with the tip tree

[Juergen Gross]

On 31/08/17 11:00, Thomas Gleixner wrote:
> On Thu, 31 Aug 2017, Thomas Gleixner wrote:
>> Hrm. For some reason I missed to remove these defines after getting rid of
>> the tracing idt.
>>
>> I'll remove that now in tip and pull in the XEN stuff to see what needs to
>> be done.
> 
> I pushed out the removal of the trace leftovers. Talked to Juergen on IRC
> and he suggested to revert the XEN patch in the xen tree and merge it
> through tip.
> 
> I've applied it on top of tip:x86/apic and fixed up the merge conflicts
> mindlessly. Patch below.
> 
> Juergen, can you please check the result?
> 
> Thanks,
> 
>   tglx
> 
> 8<---------
> Subject: xen: Get rid of paravirt op adjust_exception_frame
> From: Juergen Gross <jgr...@suse.com>
> Date: Fri, 11 Aug 2017 16:54:48 +0200
> 
> When running as Xen pv-guest the exception frame on the stack contains
> %r11 and %rcx additional to the other data pushed by the processor.
> 
> Instead of having a paravirt op being called for each exception type
> prepend the Xen specific code to each exception entry. When running as
> Xen pv-guest just use the exception entry with prepended instructions,
> otherwise use the entry without the Xen specific code.
> 
> Signed-off-by: Juergen Gross <jgr...@suse.com>
> Cc: xen-de...@lists.xenproject.org
> Cc: boris.ostrov...@oracle.com
> Cc: l...@amacapital.net
> Link: http://lkml.kernel.org/r/20170811145448.5679-1-jgr...@suse.com
> 
> ---
>  arch/x86/entry/entry_64.S |   11 +--
>  arch/x86/entry/entry_64_compat.S  |1 
>  arch/x86/include/asm/paravirt.h   |5 -
>  arch/x86/include/asm/paravirt_types.h |4 -
>  arch/x86/include/asm/proto.h  |3 +
>  arch/x86/include/asm/traps.h  |3 -
>  arch/x86/kernel/asm-offsets_64.c  |1 
>  arch/x86/kernel/paravirt.c|3 -
>  arch/x86/xen/enlighten_pv.c   |   96 
> ++
>  arch/x86/xen/irq.c|3 -
>  arch/x86/xen/xen-ops.h|1 
>  11 files changed, 70 insertions(+), 61 deletions(-)
> 
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -816,7 +816,6 @@ ENTRY(\sym)
>   .endif
>  
>   ASM_CLAC
> - PARAVIRT_ADJUST_EXCEPTION_FRAME
>  
>   .ifeq \has_error_code
>   pushq   $-1 /* ORIG_RAX: no syscall to 
> restart */
> @@ -962,7 +961,7 @@ ENTRY(do_softirq_own_stack)
>  ENDPROC(do_softirq_own_stack)
>  
>  #ifdef CONFIG_XEN
> -idtentry xen_hypervisor_callback xen_do_hypervisor_callback has_error_code=0
> +idtentry hypervisor_callback xen_do_hypervisor_callback has_error_code=0
>  
>  /*
>   * A note on the "critical region" in our callback handler.
> @@ -1029,8 +1028,6 @@ ENTRY(xen_failsafe_callback)
>   movq8(%rsp), %r11
>   addq$0x30, %rsp
>   pushq   $0  /* RIP */
> - pushq   %r11
> - pushq   %rcx
>   UNWIND_HINT_IRET_REGS offset=8
>   jmp general_protection
>  1:   /* Segment mismatch => Category 1 (Bad segment). Retry the IRET. */
> @@ -1061,9 +1058,8 @@ idtentry int3   do_int3 
> has_error_code
>  idtentry stack_segment   do_stack_segmenthas_error_code=1
>  
>  #ifdef CONFIG_XEN
> -idtentry xen_debug   do_debughas_error_code=0
> -idtentry xen_int3do_int3 has_error_code=0
> -idtentry xen_stack_segment   do_stack_segmenthas_error_code=1
> +idtentry xendebugdo_debughas_error_code=0
> +idtentry xenint3 do_int3 has_error_code=0
>  #endif
>  
>  idtentry general_protection  do_general_protection   has_error_code=1
> @@ -1227,6 +1223,7 @@ ENTRY(error_exit)
>  END(error_exit)
>  
>  /* Runs on exception stack */
> +/* XXX: broken on Xen PV */
>  ENTRY(nmi)
>   UNWIND_HINT_IRET_REGS
>   /*
> --- a/arch/x86/entry/entry_64_compat.S
> +++ b/arch/x86/entry/entry_64_compat.S
> @@ -293,7 +293,6 @@ ENTRY(entry_INT80_compat)
>   /*
>* Interrupts are off on entry.
>*/
> - PARAVIRT_ADJUST_EXCEPTION_FRAME
>   ASM_CLAC/* Do this early to minimize exposure */
>   SWAPGS
>  
> --- a/arch/x86/include/asm/paravirt.h
> +++ b/arch/x86/include/asm/paravirt.h
> @@ -960,11 +960,6 @@ extern void default_banner(void);
>  #define GET_CR2_INTO_RAX \
>   call PARA_INDIRECT(pv_mmu_ops+PV_MMU_read_cr2)
>  
> -#define PARAVIRT_ADJUST_EXCEPTION_FRAME  

Re: [Xen-devel] [PATCH] libxc: increase maximum migration stream record length

[Juergen Gross]

On 10/08/17 14:26, Andrew Cooper wrote:
> On 10/08/17 12:24, Juergen Gross wrote:
>> Today the maximum record lenth in a migration stream is 8MB. This
>> limits the size of a PV domain to a little bit less than 1TB in the
>> migration case, as the P2M frame list will exceed 8MB in this case.
>>
>> Raising the record size limit by a factor of 16 allows for domain
>> sizes of nearly 16TB to be migrated. This ought to be enough.
>>
>> Signed-off-by: Juergen Gross <jgr...@suse.com>
> 
> Hmm - Changing this isn't something I've considered when it comes to ABI
> compatibility.  I also see that there is no mention of the maximum
> record length in the stream spec, which is an oversight.
> 
> Worse still, there is no record length check in the python utilities,
> but both sides of the C code perform the check.
> 
> Let me ponder the implications.

Any result yet?


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [RFC PATCH v2 1/7] x86/paravirt: Add pv_idle_ops to paravirt ops

[Juergen Gross]

On 29/08/17 15:55, Konrad Rzeszutek Wilk wrote:
> On Tue, Aug 29, 2017 at 11:46:35AM +, Yang Zhang wrote:
>> So far, pv_idle_ops.poll is the only ops for pv_idle. .poll is called in
>> idle path which will polling for a while before we enter the real idle
>> state.
>>
>> In virtualization, idle path includes several heavy operations
>> includes timer access(LAPIC timer or TSC deadline timer) which will hurt
>> performance especially for latency intensive workload like message
>> passing task. The cost is mainly come from the vmexit which is a
>> hardware context switch between VM and hypervisor. Our solution is to
>> poll for a while and do not enter real idle path if we can get the
>> schedule event during polling.
>>
>> Poll may cause the CPU waste so we adopt a smart polling mechanism to
>> reduce the useless poll.
>>
>> Signed-off-by: Yang Zhang 
>> Signed-off-by: Quan Xu 
>> Cc: Jeremy Fitzhardinge 
>> Cc: Chris Wright 
>> Cc: Alok Kataria 
>> Cc: Rusty Russell 
>> Cc: Thomas Gleixner 
>> Cc: Ingo Molnar 
>> Cc: "H. Peter Anvin" 
>> Cc: x...@kernel.org
>> Cc: Peter Zijlstra 
>> Cc: Andy Lutomirski 
>> Cc: "Kirill A. Shutemov" 
>> Cc: Pan Xinhui 
>> Cc: Kees Cook 
>> Cc: virtualizat...@lists.linux-foundation.org
>> Cc: linux-ker...@vger.kernel.org
> 
> Adding xen-devel.
> 
> Juergen, we really should replace Jeremy's name with xen-devel or
> your name..

I wouldn't mind being added. What does Jeremy think of being removed?

> Wasn't there an patch by you that took some of the 
> mainternship over it?

I added include/linux/hypervisor.h to the PARAVIRT section and offered
to maintain it in case the PARAVIRT maintainers didn't want to.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH] xen: fix boolean parameter handling

[Juergen Gross]

Commit 63e8a1e5ffa7a7fdbde887805f673fea7e8d2e94 ("xen: check parameter
validity when parsing command line") introduced a bug for the case
when a boolean parameter was specified by its keyword only (no value).
It would set just the wrong boolean value for that parameter.

Signed-off-by: Juergen Gross <jgr...@suse.com>
---
 xen/common/kernel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index f96e402515..94fdf5c60a 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -136,7 +136,7 @@ static int parse_params(const char *cmdline, const struct 
kernel_param *start,
 rctmp = -EINVAL;
 break;
 case OPT_BOOL:
-rctmp = *optval ? parse_bool(optval, NULL) : 0;
+rctmp = *optval ? parse_bool(optval, NULL) : 1;
 if ( rctmp < 0 )
 break;
 if ( !rctmp )
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v5 08/15] xen/common/sched_credit2.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by parse_credit2_runqueue() can be removed.

Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Dario Faggioli <dario.faggi...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Dario Faggioli <dario.faggi...@citrix.com>
---
 xen/common/sched_credit2.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 9b1db1351f..2da9cc2ffb 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -352,8 +352,6 @@ static int parse_credit2_runqueue(const char *s)
 }
 }
 
-printk("WARNING, unrecognized value of credit2_runqueue option!\n");
-
 return -EINVAL;
 }
 custom_param("credit2_runqueue", parse_credit2_runqueue);
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v5 15/15] xen: make some console related parameters settable at runtime

[Juergen Gross]

Support modifying conswitch, console_timestamps, loglvl and
guest_loglvl at runtime.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
---
V3:
- *_runtime_param() now includes boot parameter setting (Jan Beulich)
---
 docs/misc/xen-command-line.markdown |  8 
 xen/drivers/char/console.c  | 18 +-
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index 4002eab08b..9797c8db2d 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -391,6 +391,8 @@ makes sense on its own.
 
 > Default: `none`
 
+> Can be modified at runtime
+
 Specify which timestamp format Xen should use for each console line.
 
 * `none`: No timestamps
@@ -417,6 +419,8 @@ into the console ring buffer.
 
 > Default: `conswitch=a`
 
+> Can be modified at runtime
+
 Specify which character should be used to switch serial input between
 Xen and dom0.  The required sequence is CTRL-switch char three
 times.
@@ -898,6 +902,8 @@ maximum number of maptrack frames domain.
 
 > Default: `guest_loglvl=none/warning`
 
+> Can be modified at runtime
+
 Set the logging level for Xen guests.  Any log message with equal more
 more importance will be printed.
 
@@ -1164,6 +1170,8 @@ if left disabled by the BIOS.
 
 > Default: `loglvl=warning`
 
+> Can be modified at runtime
+
 Set the logging level for Xen.  Any log message with equal more more
 importance will be printed.
 
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 36f94e6b94..19d0e74f17 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -40,7 +40,7 @@ string_param("console", opt_console);
 /* Char 2: If this character is 'x', then do not auto-switch to DOM0 when it */
 /* boots. Any other value, or omitting the char, enables auto-switch */
 static unsigned char __read_mostly opt_conswitch[3] = "a";
-string_param("conswitch", opt_conswitch);
+string_runtime_param("conswitch", opt_conswitch);
 
 /* sync_console: force synchronous console output (useful for debugging). */
 static bool_t __initdata opt_sync_console;
@@ -68,7 +68,7 @@ enum con_timestamp_mode
 static enum con_timestamp_mode __read_mostly opt_con_timestamp_mode = TSM_NONE;
 
 static int parse_console_timestamps(const char *s);
-custom_param("console_timestamps", parse_console_timestamps);
+custom_runtime_param("console_timestamps", parse_console_timestamps);
 
 /* conring_size: allows a large console ring than default (16kB). */
 static uint32_t __initdata opt_conring_size;
@@ -134,8 +134,8 @@ static int parse_guest_loglvl(const char *s);
  * Similar definitions for guest_loglvl, but applies to guest tracing.
  * Defaults: loglvl=warning ; guest_loglvl=none/warning
  */
-custom_param("loglvl", parse_loglvl);
-custom_param("guest_loglvl", parse_guest_loglvl);
+custom_runtime_param("loglvl", parse_loglvl);
+custom_runtime_param("guest_loglvl", parse_guest_loglvl);
 
 static atomic_t print_everything = ATOMIC_INIT(0);
 
@@ -145,7 +145,7 @@ static atomic_t print_everything = ATOMIC_INIT(0);
 return (lvlnum);\
 }
 
-static int __init __parse_loglvl(const char *s, const char **ps)
+static int __parse_loglvl(const char *s, const char **ps)
 {
 ___parse_loglvl(s, ps, "none",0);
 ___parse_loglvl(s, ps, "error",   1);
@@ -156,7 +156,7 @@ static int __init __parse_loglvl(const char *s, const char 
**ps)
 return 2; /* sane fallback */
 }
 
-static int __init _parse_loglvl(const char *s, int *lower, int *upper)
+static int _parse_loglvl(const char *s, int *lower, int *upper)
 {
 *lower = *upper = __parse_loglvl(s, );
 if ( *s == '/' )
@@ -167,12 +167,12 @@ static int __init _parse_loglvl(const char *s, int 
*lower, int *upper)
 return *s ? -EINVAL : 0;
 }
 
-static int __init parse_loglvl(const char *s)
+static int parse_loglvl(const char *s)
 {
 return _parse_loglvl(s, _lower_thresh, _upper_thresh);
 }
 
-static int __init parse_guest_loglvl(const char *s)
+static int parse_guest_loglvl(const char *s)
 {
 return _parse_loglvl(s, _guest_lower_thresh,
  _guest_upper_thresh);
@@ -606,7 +606,7 @@ static int printk_prefix_check(char *p, char **pp)
 ((loglvl <

[Xen-devel] [PATCH v5 07/15] xen/common/kexec.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
some messages issued by parse_low_crashinfo() and
parse_crashinfo_maxaddr() can be removed.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
 xen/common/kexec.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index fcc68bd4d8..e0f1e3eafa 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -213,7 +213,6 @@ static int __init parse_low_crashinfo(const char *str)
 low_crashinfo_mode = LOW_CRASHINFO_ALL;
 else
 {
-printk("Unknown low_crashinfo parameter '%s'.  Defaulting to min.\n", 
str);
 low_crashinfo_mode = LOW_CRASHINFO_MIN;
 return -EINVAL;
 }
@@ -240,11 +239,7 @@ static int __init parse_crashinfo_maxaddr(const char *str)
 if ( (addr = parse_size_and_unit(str, )) )
 crashinfo_maxaddr = addr;
 else
-{
-printk("Unable to parse crashinfo_maxaddr. Defaulting to 
%"PRIpaddr"\n",
-   crashinfo_maxaddr);
 return -EINVAL;
-}
 
 return *q ? -EINVAL : 0;
 }
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v5 03/15] xen/arch/x86/apic.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by apic_set_verbosity() can be removed.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/apic.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 325b48074a..2638414e08 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -800,11 +800,8 @@ static int __init apic_set_verbosity(const char *str)
 apic_verbosity = APIC_DEBUG;
 else if (strcmp("verbose", str) == 0)
 apic_verbosity = APIC_VERBOSE;
-else {
-printk(KERN_WARNING "APIC Verbosity level %s not recognised"
-   " use apic_verbosity=verbose or apic_verbosity=debug", str);
+else
 return -EINVAL;
-}
 
 return 0;
 }
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v5 00/15] Support for modifying parameters at runtime

[Juergen Gross]

rad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Paul Durrant <paul.durr...@citrix.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>

Juergen Gross (15):
  xen/arch/x86/psr.c: let custom parameter parsing routines return errno
  xen: check parameter validity when parsing command line
  xen/arch/x86/apic.c: remove custom_param() error messages
  xen/arch/x86/cpu/mcheck/mce.c: remove custom_param() error messages
  xen/arch/x86/hvm/viridian.c: remove custom_param() error messages
  xen/arch/x86/io_apic.c: remove custom_param() error messages
  xen/common/kexec.c: remove custom_param() error messages
  xen/common/sched_credit2.c: remove custom_param() error messages
  xen: carve out a generic parsing function from _cmdline_parse()
  xen: add basic support for runtime parameter changing
  xen: add hypercall for setting parameters at runtime
  libxc: add function to set hypervisor parameters
  libxl: add libxl_set_parameters() function
  xl: add new xl command set-parameters
  xen: make some console related parameters settable at runtime

 docs/man/xl.pod.1.in|  5 ++
 docs/misc/xen-command-line.markdown |  8 
 tools/flask/policy/modules/dom0.te  |  2 +-
 tools/libxc/include/xenctrl.h   |  1 +
 tools/libxc/xc_misc.c   | 21 +
 tools/libxl/libxl.c | 15 ++
 tools/libxl/libxl.h |  8 
 tools/xl/xl.h   |  1 +
 tools/xl/xl_cmdtable.c  |  5 ++
 tools/xl/xl_misc.c  | 20 
 xen/arch/arm/xen.lds.S  |  4 ++
 xen/arch/x86/apic.c |  5 +-
 xen/arch/x86/cpu/mcheck/mce.c   |  4 --
 xen/arch/x86/hvm/viridian.c | 10 ++--
 xen/arch/x86/io_apic.c  |  3 --
 xen/arch/x86/psr.c  | 58 ++-
 xen/arch/x86/xen.lds.S  |  4 ++
 xen/common/kernel.c | 94 +
 xen/common/kexec.c  |  5 --
 xen/common/sched_credit2.c  |  2 -
 xen/common/sysctl.c | 36 ++
 xen/drivers/char/console.c  | 18 +++
 xen/include/public/sysctl.h | 20 
 xen/include/xen/init.h  | 87 ++
 xen/include/xen/lib.h   |  1 +
 xen/include/xen/types.h |  3 ++
 xen/xsm/flask/hooks.c   |  3 ++
 xen/xsm/flask/policy/access_vectors |  2 +
 28 files changed, 362 insertions(+), 83 deletions(-)

-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v5 04/15] xen/arch/x86/cpu/mcheck/mce.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by mce_set_verbosity() can be removed.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/cpu/mcheck/mce.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 8015dff8c4..7affe2591e 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -67,11 +67,7 @@ static int __init mce_set_verbosity(const char *str)
 if (strcmp("verbose", str) == 0)
 mce_verbosity = MCE_VERBOSE;
 else
-{
-printk(KERN_DEBUG "Machine Check verbosity level %s not recognised"
-   "use mce_verbosity=verbose", str);
 return -EINVAL;
-}
 
 return 0;
 }
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v5 12/15] libxc: add function to set hypervisor parameters

[Juergen Gross]

Add a new libxc function to set hypervisor parameters at runtime
similar to boot time parameters via command line.

Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
---
V3:
- zero padding fields in sysctl.u.set_parameter
---
 tools/libxc/include/xenctrl.h |  1 +
 tools/libxc/xc_misc.c | 21 +
 2 files changed, 22 insertions(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index c7710b8f36..ad5e6b3d77 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1210,6 +1210,7 @@ int xc_readconsolering(xc_interface *xch,
int clear, int incremental, uint32_t *pindex);
 
 int xc_send_debug_keys(xc_interface *xch, char *keys);
+int xc_set_parameters(xc_interface *xch, char *params);
 
 typedef xen_sysctl_physinfo_t xc_physinfo_t;
 typedef xen_sysctl_cputopo_t xc_cputopo_t;
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index 2303293c6c..7e15e904e3 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -187,6 +187,27 @@ int xc_send_debug_keys(xc_interface *xch, char *keys)
 return ret;
 }
 
+int xc_set_parameters(xc_interface *xch, char *params)
+{
+int ret, len = strlen(params);
+DECLARE_SYSCTL;
+DECLARE_HYPERCALL_BOUNCE(params, len, XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+if ( xc_hypercall_bounce_pre(xch, params) )
+return -1;
+
+sysctl.cmd = XEN_SYSCTL_set_parameter;
+set_xen_guest_handle(sysctl.u.set_parameter.params, params);
+sysctl.u.set_parameter.size = len;
+memset(sysctl.u.set_parameter.pad, 0, sizeof(sysctl.u.set_parameter.pad));
+
+ret = do_sysctl(xch, );
+
+xc_hypercall_bounce_post(xch, params);
+
+return ret;
+}
+
 int xc_physinfo(xc_interface *xch,
 xc_physinfo_t *put_info)
 {
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v5 01/15] xen/arch/x86/psr.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/psr.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
V5:
- rename variable (value -> delim, val_str -> val_delim) (Jan Beulich)
---
 xen/arch/x86/psr.c | 58 ++
 1 file changed, 37 insertions(+), 21 deletions(-)

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 25a85b65b2..4515100785 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -418,50 +418,66 @@ static const struct feat_props l2_cat_props = {
 .write_msr = l2_cat_write_msr,
 };
 
-static void __init parse_psr_bool(char *s, char *value, char *feature,
+static bool __init parse_psr_bool(const char *s, const char *delim,
+  const char *ss, const char *feature,
   unsigned int mask)
 {
-if ( !strcmp(s, feature) )
+if ( !strncmp(s, feature, delim - s) )
 {
-if ( !value )
+if ( !*delim )
 opt_psr |= mask;
 else
 {
-int val_int = parse_bool(value, NULL);
+int val_int = parse_bool(delim + 1, ss);
 
 if ( val_int == 0 )
 opt_psr &= ~mask;
 else if ( val_int == 1 )
 opt_psr |= mask;
+else
+return false;
 }
+return true;
 }
+return false;
 }
 
-static void __init parse_psr_param(char *s)
+static int __init parse_psr_param(const char *s)
 {
-char *ss, *val_str;
+const char *ss, *val_delim;
+const char *q;
+int rc = 0;
 
 do {
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
-
-val_str = strchr(s, ':');
-if ( val_str )
-*val_str++ = '\0';
+if ( !ss )
+ss = strchr(s, '\0');
 
-parse_psr_bool(s, val_str, "cmt", PSR_CMT);
-parse_psr_bool(s, val_str, "cat", PSR_CAT);
-parse_psr_bool(s, val_str, "cdp", PSR_CDP);
+val_delim = strchr(s, ':');
+if ( !val_delim )
+val_delim = strchr(s, '\0');
 
-if ( val_str && !strcmp(s, "rmid_max") )
-opt_rmid_max = simple_strtoul(val_str, NULL, 0);
-
-if ( val_str && !strcmp(s, "cos_max") )
-opt_cos_max = simple_strtoul(val_str, NULL, 0);
+if ( *val_delim && !strncmp(s, "rmid_max", val_delim - s) )
+{
+opt_rmid_max = simple_strtoul(val_delim + 1, , 0);
+if ( *q && *q != ',' )
+rc = -EINVAL;
+}
+else if ( *val_delim && !strncmp(s, "cos_max", val_delim - s) )
+{
+opt_cos_max = simple_strtoul(val_delim + 1, , 0);
+if ( *q && *q != ',' )
+rc = -EINVAL;
+}
+else if ( !parse_psr_bool(s, val_delim, ss, "cmt", PSR_CMT) &&
+  !parse_psr_bool(s, val_delim, ss, "cat", PSR_CAT) &&
+  !parse_psr_bool(s, val_delim, ss, "cdp", PSR_CDP) )
+rc = -EINVAL;
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 custom_param("psr", parse_psr_param);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v5 09/15] xen: carve out a generic parsing function from _cmdline_parse()

[Juergen Gross]

In order to support generic parameter parsing carve out the parser from
_cmdline_parse(). As this generic function might be called after boot
remove the __init annotations from all called sub-functions.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/common/kernel.c | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index a0611d6c7c..6e2f5ed9da 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -23,8 +23,7 @@ enum system_state system_state = SYS_STATE_early_boot;
 xen_commandline_t saved_cmdline;
 static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
 
-static int __init assign_integer_param(
-const struct kernel_param *param, uint64_t val)
+static int assign_integer_param(const struct kernel_param *param, uint64_t val)
 {
 switch ( param->len )
 {
@@ -53,12 +52,13 @@ static int __init assign_integer_param(
 return 0;
 }
 
-static void __init _cmdline_parse(const char *cmdline)
+static int parse_params(const char *cmdline, const struct kernel_param *start,
+const struct kernel_param *end)
 {
 char opt[128], *optval, *optkey, *q;
 const char *p = cmdline, *key;
 const struct kernel_param *param;
-int rc;
+int rc, final_rc = 0;
 bool bool_assert, found;
 
 for ( ; ; )
@@ -100,7 +100,7 @@ static void __init _cmdline_parse(const char *cmdline)
 
 rc = 0;
 found = false;
-for ( param = __setup_start; param < __setup_end; param++ )
+for ( param = start; param < end; param++ )
 {
 int rctmp;
 const char *s;
@@ -172,11 +172,24 @@ static void __init _cmdline_parse(const char *cmdline)
 }
 
 if ( rc )
+{
 printk("parameter \"%s\" has invalid value \"%s\", rc=%d!\n",
 key, optval, rc);
+final_rc = rc;
+}
 if ( !found )
+{
 printk("parameter \"%s\" unknown!\n", key);
+final_rc = -EINVAL;
+}
 }
+
+return final_rc;
+}
+
+static void __init _cmdline_parse(const char *cmdline)
+{
+parse_params(cmdline, __setup_start, __setup_end);
 }
 
 /**
@@ -201,7 +214,7 @@ void __init cmdline_parse(const char *cmdline)
 #endif
 }
 
-int __init parse_bool(const char *s, const char *e)
+int parse_bool(const char *s, const char *e)
 {
 unsigned int len;
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v5 05/15] xen/arch/x86/hvm/viridian.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by parse_viridian_version() can be removed.

Cc: Paul Durrant <paul.durr...@citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Reviewed-by: Paul Durrant <paul.durr...@citrix.com>
---
 xen/arch/x86/hvm/viridian.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index 2edf9d0b23..e0546f32fc 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -1104,7 +1104,7 @@ static int __init parse_viridian_version(const char *arg)
 {
 n[i] = simple_strtoul(arg, , 0);
 if ( e != t )
-goto fail;
+break;
 }
 
 i++;
@@ -1112,12 +1112,12 @@ static int __init parse_viridian_version(const char 
*arg)
 } while ( *t );
 
 if ( i != 3 )
-goto fail;
+return -EINVAL;
 
 if ( ((typeof(viridian_major))n[0] != n[0]) ||
  ((typeof(viridian_minor))n[1] != n[1]) ||
  ((typeof(viridian_build))n[2] != n[2]) )
-goto fail;
+return -EINVAL;
 
 viridian_major = n[0];
 viridian_minor = n[1];
@@ -1126,10 +1126,6 @@ static int __init parse_viridian_version(const char *arg)
 printk("viridian-version = %#x,%#x,%#x\n",
viridian_major, viridian_minor, viridian_build);
 return 0;
-
- fail:
-printk(XENLOG_WARNING "Invalid viridian-version, using default\n");
-return -EINVAL;
 }
 custom_param("viridian-version", parse_viridian_version);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v5 11/15] xen: add hypercall for setting parameters at runtime

[Juergen Gross]

Add a sysctl hypercall to support setting parameters similar to
command line parameters, but at runtime. The parameters to set are
specified as a string, just like the boot parameters.

Cc: Daniel De Graaf <dgde...@tycho.nsa.gov>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Wei Liu <wei.l...@citrix.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
---
V4:
- use copy_from_guest() (Jan Beulich)
- add more descriptive comment to sysctl.h (Jan Beulich)

V3:
- check pad[] to be zero (Jan Beulich)
- return E2BIG in case of parameters too long (Jan Beulich)
- move max. parameter size define to sysctl.c (Jan Beulich)

V2:
- corrected XSM test (Daniel De Graaf)
---
 tools/flask/policy/modules/dom0.te  |  2 +-
 xen/common/sysctl.c | 36 
 xen/include/public/sysctl.h | 20 
 xen/xsm/flask/hooks.c   |  3 +++
 xen/xsm/flask/policy/access_vectors |  2 ++
 5 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/tools/flask/policy/modules/dom0.te 
b/tools/flask/policy/modules/dom0.te
index d0a4d91ac0..338caaf41e 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -16,7 +16,7 @@ allow dom0_t xen_t:xen {
 allow dom0_t xen_t:xen2 {
resource_op psr_cmt_op psr_cat_op pmu_ctrl get_symbol
get_cpu_levelling_caps get_cpu_featureset livepatch_op
-   gcov_op
+   gcov_op set_parameter
 };
 
 # Allow dom0 to use all XENVER_ subops that have checks.
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index ae58a0f650..a6882d1c9d 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -467,6 +467,42 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) 
u_sysctl)
 copyback = 1;
 break;
 
+case XEN_SYSCTL_set_parameter:
+{
+#define XEN_SET_PARAMETER_MAX_SIZE 1023
+char *params;
+
+if ( op->u.set_parameter.pad[0] || op->u.set_parameter.pad[1] ||
+ op->u.set_parameter.pad[2] )
+{
+ret = -EINVAL;
+break;
+}
+if ( op->u.set_parameter.size > XEN_SET_PARAMETER_MAX_SIZE )
+{
+ret = -E2BIG;
+break;
+}
+params = xmalloc_bytes(op->u.set_parameter.size + 1);
+if ( !params )
+{
+ret = -ENOMEM;
+break;
+}
+if ( copy_from_guest(params, op->u.set_parameter.params,
+ op->u.set_parameter.size) )
+ret = -EFAULT;
+else
+{
+params[op->u.set_parameter.size] = 0;
+ret = runtime_parse(params);
+}
+
+xfree(params);
+
+break;
+}
+
 default:
 ret = arch_do_sysctl(op, u_sysctl);
 copyback = 0;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 9e51af61e1..7830b987da 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1096,6 +1096,24 @@ struct xen_sysctl_livepatch_op {
 typedef struct xen_sysctl_livepatch_op xen_sysctl_livepatch_op_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_livepatch_op_t);
 
+/*
+ * XEN_SYSCTL_set_parameter
+ *
+ * Change hypervisor parameters at runtime.
+ * The input string is parsed similar to the boot parameters.
+ * Parameters are a single string terminated by a NUL byte of max. size
+ * characters. Multiple settings can be specified by separating them
+ * with blanks.
+ */
+
+struct xen_sysctl_set_parameter {
+XEN_GUEST_HANDLE_64(char) params;   /* IN: pointer to parameters. */
+uint16_t size;  /* IN: size of parameters. */
+uint16_t pad[3];/* IN: MUST be zero. */
+};
+typedef struct xen_sysctl_set_parameter xen_sysctl_set_parameter_t;
+DEFINE_XEN_GUEST_HANDLE(xen_sysctl_set_parameter_t);
+
 struct xen_sysctl {
 uint32_t cmd;
 #define XEN_SYSCTL_readconsole1
@@ -1124,6 +1142,7 @@ struct xen_sysctl {
 #define XEN_SYSCTL_get_cpu_levelling_caps25
 #define XEN_SYSCTL_get_cpu_featureset26
 #define XEN_SYSCTL_livepatch_op  27
+#define XEN_SYSCTL_set_parameter 28
 uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
 union {
 struct xen_sysctl_readconsole   readconsole;
@@ -1152,6 +1171,7 @@ struct xen_sysctl {
 struct xen_sysctl_cpu_levelling_caps cpu_levelling_caps;
 struct xen_sysctl_cpu_featuresetcpu_featureset;
 struct xen_sysctl_livepa

[Xen-devel] [PATCH v5 14/15] xl: add new xl command set-parameters

[Juergen Gross]

Add a new xl command "set-parameters" to set hypervisor parameters at
runtime similar to boot time parameters via command line.

Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
---
 docs/man/xl.pod.1.in   |  5 +
 tools/xl/xl.h  |  1 +
 tools/xl/xl_cmdtable.c |  5 +
 tools/xl/xl_misc.c | 20 
 4 files changed, 31 insertions(+)

diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
index 16c83066fe..3d5f2f7359 100644
--- a/docs/man/xl.pod.1.in
+++ b/docs/man/xl.pod.1.in
@@ -822,6 +822,11 @@ Pass the VNC password to vncviewer via stdin.
 Send debug I to Xen. It is the same as pressing the Xen
 "conswitch" (Ctrl-A by default) three times and then pressing "keys".
 
+=item B I
+
+Set hypervisor parameters as specified in I. This allows for some
+boot parameters of the hypervisor to be modified in the running systems.
+
 =item B [I]
 
 Reads the Xen message buffer, similar to dmesg on a Linux system.  The
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index aa95b77146..5d3d2a4835 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -154,6 +154,7 @@ int main_rename(int argc, char **argv);
 int main_trigger(int argc, char **argv);
 int main_sysrq(int argc, char **argv);
 int main_debug_keys(int argc, char **argv);
+int main_set_parameters(int argc, char **argv);
 int main_dmesg(int argc, char **argv);
 int main_top(int argc, char **argv);
 int main_networkattach(int argc, char **argv);
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 2c71a9f776..ba0159df67 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -309,6 +309,11 @@ struct cmd_spec cmd_table[] = {
   "Send debug keys to Xen",
   "",
 },
+{ "set-parameters",
+  _set_parameters, 0, 1,
+  "Set hypervisor parameters",
+  "",
+},
 { "dmesg",
   _dmesg, 0, 0,
   "Read and/or clear dmesg buffer",
diff --git a/tools/xl/xl_misc.c b/tools/xl/xl_misc.c
index 9c6227af23..dcf940a6d4 100644
--- a/tools/xl/xl_misc.c
+++ b/tools/xl/xl_misc.c
@@ -155,6 +155,26 @@ int main_debug_keys(int argc, char **argv)
 return EXIT_SUCCESS;
 }
 
+int main_set_parameters(int argc, char **argv)
+{
+int opt;
+char *params;
+
+SWITCH_FOREACH_OPT(opt, "", NULL, "set-parameters", 1) {
+/* No options */
+}
+
+params = argv[optind];
+
+if (libxl_set_parameters(ctx, params)) {
+fprintf(stderr, "cannot set parameters: %s\n", params);
+fprintf(stderr, "Use \"xl dmesg\" to look for possible reason.\n");
+return EXIT_FAILURE;
+}
+
+return EXIT_SUCCESS;
+}
+
 int main_devd(int argc, char **argv)
 {
 int ret = 0, opt = 0, daemonize = 1;
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v5 13/15] libxl: add libxl_set_parameters() function

[Juergen Gross]

Add a new libxl function to set hypervisor parameters at runtime
similar to boot time parameters via command line.

Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
---
V2:
- corrected coding style (Wei Liu)
- removed superfluous #ifdef (Wei Liu)

V3:
- use LOGEV() for error message
---
 tools/libxl/libxl.c | 15 +++
 tools/libxl/libxl.h |  8 
 2 files changed, 23 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0ef874406f..247c56cf83 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -652,6 +652,21 @@ int libxl_send_debug_keys(libxl_ctx *ctx, char *keys)
 return 0;
 }
 
+int libxl_set_parameters(libxl_ctx *ctx, char *params)
+{
+int ret;
+GC_INIT(ctx);
+
+ret = xc_set_parameters(ctx->xch, params);
+if (ret < 0) {
+LOGEV(ERROR, ret, "setting parameters");
+GC_FREE;
+return ERROR_FAIL;
+}
+GC_FREE;
+return 0;
+}
+
 static int fd_set_flags(libxl_ctx *ctx, int fd,
 int fcntlgetop, int fcntlsetop, const char *fl,
 int flagmask, int set_p)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 229e289750..17045253ab 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1051,6 +1051,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, 
const libxl_mac *src);
  */
 #define LIBXL_HAVE_QED 1
 
+/*
+ * LIBXL_HAVE_SET_PARAMETERS
+ *
+ * If this is defined setting hypervisor parameters is supported.
+ */
+#define LIBXL_HAVE_SET_PARAMETERS 1
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
@@ -2105,6 +2112,7 @@ int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
libxl_trigger trigger, uint32_t vcpuid);
 int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq);
 int libxl_send_debug_keys(libxl_ctx *ctx, char *keys);
+int libxl_set_parameters(libxl_ctx *ctx, char *params);
 
 typedef struct libxl__xen_console_reader libxl_xen_console_reader;
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v5 02/15] xen: check parameter validity when parsing command line

[Juergen Gross]

Where possible check validity of parameters in _cmdline_parse() and
issue a warning message in case of an error detected.

In order to make sure a custom parameter parsing function really
returns a value (error or success), don't use a void pointer for
storing the function address, but a proper typed function pointer.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
---
V4:
- removed test for empty string from parse_bool() again, avoid calling
  parse_bool() in that case (Jan Beulich)
- print error number in failure case (Jan Beulich)
- limit scope of some variables (Jan Beulich)
- use type bool for bool_assert (Jan Beulich)

V3:
- use function pointer in struct kernel_param (Jan Beulich)
- better range check in assign_integer_param() (Jan Beulich)
- dont assign int values in case of overflow (Jan Beulich)
- allow multiple handlers for a parameter (Jan Beulich)

V2:
- replaced literal 8 by BITS_PER_BYTE (Wei Liu)
- added test for empty string to parse_bool()
---
 xen/common/kernel.c | 68 ++---
 xen/include/xen/init.h  | 30 +-
 xen/include/xen/types.h |  3 +++
 3 files changed, 80 insertions(+), 21 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 71bc547d17..a0611d6c7c 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -23,34 +23,43 @@ enum system_state system_state = SYS_STATE_early_boot;
 xen_commandline_t saved_cmdline;
 static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
 
-static void __init assign_integer_param(
+static int __init assign_integer_param(
 const struct kernel_param *param, uint64_t val)
 {
 switch ( param->len )
 {
 case sizeof(uint8_t):
-*(uint8_t *)param->var = val;
+if ( val > UINT8_MAX && val < (uint64_t)INT8_MIN )
+return -EOVERFLOW;
+*(uint8_t *)param->par.var = val;
 break;
 case sizeof(uint16_t):
-*(uint16_t *)param->var = val;
+if ( val > UINT16_MAX && val < (uint64_t)INT16_MIN )
+return -EOVERFLOW;
+*(uint16_t *)param->par.var = val;
 break;
 case sizeof(uint32_t):
-*(uint32_t *)param->var = val;
+if ( val > UINT32_MAX && val < (uint64_t)INT32_MIN )
+return -EOVERFLOW;
+*(uint32_t *)param->par.var = val;
 break;
 case sizeof(uint64_t):
-*(uint64_t *)param->var = val;
+*(uint64_t *)param->par.var = val;
 break;
 default:
 BUG();
 }
+
+return 0;
 }
 
 static void __init _cmdline_parse(const char *cmdline)
 {
 char opt[128], *optval, *optkey, *q;
-const char *p = cmdline;
+const char *p = cmdline, *key;
 const struct kernel_param *param;
-int bool_assert;
+int rc;
+bool bool_assert, found;
 
 for ( ; ; )
 {
@@ -84,46 +93,66 @@ static void __init _cmdline_parse(const char *cmdline)
 }
 
 /* Boolean parameters can be inverted with 'no-' prefix. */
+key = optkey;
 bool_assert = !!strncmp("no-", optkey, 3);
 if ( !bool_assert )
 optkey += 3;
 
+rc = 0;
+found = false;
 for ( param = __setup_start; param < __setup_end; param++ )
 {
+int rctmp;
+const char *s;
+
 if ( strcmp(param->name, optkey) )
 {
 if ( param->type == OPT_CUSTOM && q &&
  strlen(param->name) == q + 1 - opt &&
  !strncmp(param->name, opt, q + 1 - opt) )
 {
+found = true;
 optval[-1] = '=';
-((void (*)(const char *))param->var)(q);
+rctmp = param->par.func(q);
 optval[-1] = '\0';
+if ( !rc )
+rc = rctmp;
 }
 continue;
 }
 
+rctmp = 0;
+found = true;
 switch ( param->type )
 {
 case OPT_STR:
-strlcpy(param->var, optval, param->len);
+strlcpy(param->par.var, optval, param->len);
 break;
 case OPT_UINT:
-assign_integer_param(
+rctmp = assign_integer_param(
 param,
-simple_strtoll(optval, NULL, 0));
+simpl

[Xen-devel] [PATCH v5 06/15] xen/arch/x86/io_apic.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by setup_ioapic_ack() can be removed.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/io_apic.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index f767c4560c..f959090ca0 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -1594,10 +1594,7 @@ static int __init setup_ioapic_ack(const char *s)
 ioapic_ack_forced = true;
 }
 else
-{
-printk("Unknown ioapic_ack value specified: '%s'\n", s);
 return -EINVAL;
-}
 
 return 0;
 }
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v5 10/15] xen: add basic support for runtime parameter changing

[Juergen Gross]

Add the needed infrastructure for runtime parameter changing similar
to that used at boot time via cmdline. We are using the same parsing
functions as for cmdline parsing, but with a different array of
parameter definitions.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
---
V2:
- added modification of ARM linker script (Wei Liu)

V3:
- moved runtime parameter array in linker scripts (Jan Beulich)
- renamed macros to *_runtime_param() (Jan Beulich)
- added *runtime_only_param() macros (Jan Beulich)
- let *_runtime_param() macros include boot param functionality
  (Jan Beulich)
---
 xen/arch/arm/xen.lds.S |  4 
 xen/arch/x86/xen.lds.S |  4 
 xen/common/kernel.c|  5 +
 xen/include/xen/init.h | 57 --
 xen/include/xen/lib.h  |  1 +
 5 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 2d54f224ec..c9b9546435 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -61,6 +61,10 @@ SECTIONS
*(.lockprofile.data)
__lock_profile_end = .;
 #endif
+   . = ALIGN(POINTER_ALIGN);
+   __param_start = .;
+   *(.data.param)
+   __param_end = .;
   } :text
 
 #if defined(BUILD_ID)
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index ff08bbe42a..6a7bbb8ca1 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -120,6 +120,10 @@ SECTIONS
*(.lockprofile.data)
__lock_profile_end = .;
 #endif
+   . = ALIGN(POINTER_ALIGN);
+   __param_start = .;
+   *(.data.param)
+   __param_end = .;
   } :text
 
 #if defined(BUILD_ID)
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 6e2f5ed9da..f96e402515 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -192,6 +192,11 @@ static void __init _cmdline_parse(const char *cmdline)
 parse_params(cmdline, __setup_start, __setup_end);
 }
 
+int runtime_parse(const char *line)
+{
+return parse_params(line, __param_start, __param_end);
+}
+
 /**
  *cmdline_parse -- parses the xen command line.
  * If CONFIG_CMDLINE is set, it would be parsed prior to @cmdline.
diff --git a/xen/include/xen/init.h b/xen/include/xen/init.h
index 234ec25aae..db06c76fdf 100644
--- a/xen/include/xen/init.h
+++ b/xen/include/xen/init.h
@@ -90,11 +90,16 @@ struct kernel_param {
 };
 
 extern const struct kernel_param __setup_start[], __setup_end[];
+extern const struct kernel_param __param_start[], __param_end[];
+
+#define __dataparam   __used_section(".data.param")
+
+#define __param(att)  static const att \
+__attribute__((__aligned__(sizeof(void * struct kernel_param
 
 #define __setup_str static const __initconst \
 __attribute__((__aligned__(1))) char
-#define __kparam static const __initsetup \
-__attribute__((__aligned__(sizeof(void * struct kernel_param
+#define __kparam  __param(__initsetup)
 
 #define custom_param(_name, _var) \
 __setup_str __setup_str_##_var[] = _name; \
@@ -131,6 +136,54 @@ extern const struct kernel_param __setup_start[], 
__setup_end[];
   .len = sizeof(_var), \
   .par.var = &_var }
 
+#define __rtparam __param(__dataparam)
+
+#define custom_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+  { .name = _name, \
+  .type = OPT_CUSTOM, \
+  .par.func = _var }
+#define boolean_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+{ .name = _name, \
+  .type = OPT_BOOL, \
+  .len = sizeof(_var), \
+  .par.var = &_var }
+#define integer_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+{ .name = _name, \
+  .type = OPT_UINT, \
+  .len = sizeof(_var), \
+  .par.var = &_var }
+#define size_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+{ .name = _name, \
+  .type = OPT_SIZE, \
+  .len = sizeof(_var), \
+  .par.var = &_var }
+#define string_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+{ .name = _name, \
+  .type = OPT_STR, \
+  .len = sizeof(_var), \
+  .par.var = &_var }
+
+#define custom_runtime_param(_name, _var) \
+custom_param(_name, _var); \
+custom_runtime_only_param(_name, _var)
+#define boolean_runtime_param(_name, _var) \
+boolean_param(_name, _var); \
+boolean_runtime_only_param(_name, _var)
+#define integer_runtime_param(_name, _var) 

Re: [Xen-devel] [PATCH v8 10/13] x86/xen: Bypass intr mode setup in enlighten_pv system

[Juergen Gross]

On 28/08/17 06:25, Juergen Gross wrote:
> On 28/08/17 05:20, Dou Liyang wrote:
>> XEN PV overrides smp_prepare_cpus(). xen_pv_smp_prepare_cpus()
>> initializes interrupts in the XEN PV specific way and does not invoke
>> native_smp_prepare_cpus(). As a consequence, x86_init.intr_mode_init() is
>> not invoked either.
>>
>> The invocation of x86_init.intr_mode_init() will be moved from
>> native_smp_prepare_cpus() in a follow up patch to solve > REASON/PROBLEM>.
> 
> Can you be a little bit more precise here, please? :-)
> 
>> That move would cause the invocation of x86_init.intr_mode_init() for XEN
>> PV platforms. To prevent that, override the default x86_init.
>> intr_mode_init() callback with a noop().
>>
>> [Rewrited by Thomas Gleixner <t...@linutronix.de>]
>>
>> Signed-off-by: Dou Liyang <douly.f...@cn.fujitsu.com>
>> Cc: xen-de...@lists.xenproject.org
>> Cc: boris.ostrov...@oracle.com
> 
> On which tree does this apply? Would be nice to get a hint against which
> source this can be reviewed.

Aah, just found the rest of the series. In case a single patch of a
series isn't stand alone it would be nice to receive at least the cover
letter of the series in order to know what its all about.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v8 10/13] x86/xen: Bypass intr mode setup in enlighten_pv system

[Juergen Gross]

On 28/08/17 05:20, Dou Liyang wrote:
> XEN PV overrides smp_prepare_cpus(). xen_pv_smp_prepare_cpus()
> initializes interrupts in the XEN PV specific way and does not invoke
> native_smp_prepare_cpus(). As a consequence, x86_init.intr_mode_init() is
> not invoked either.
> 
> The invocation of x86_init.intr_mode_init() will be moved from
> native_smp_prepare_cpus() in a follow up patch to solve  REASON/PROBLEM>.

Can you be a little bit more precise here, please? :-)

> That move would cause the invocation of x86_init.intr_mode_init() for XEN
> PV platforms. To prevent that, override the default x86_init.
> intr_mode_init() callback with a noop().
> 
> [Rewrited by Thomas Gleixner ]
> 
> Signed-off-by: Dou Liyang 
> Cc: xen-de...@lists.xenproject.org
> Cc: boris.ostrov...@oracle.com

On which tree does this apply? Would be nice to get a hint against which
source this can be reviewed.


Juergen

> ---
>  arch/x86/xen/enlighten_pv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index 811e4dd..07147dd 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -1250,6 +1250,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
>   x86_platform.get_nmi_reason = xen_get_nmi_reason;
>  
>   x86_init.resources.memory_setup = xen_memory_setup;
> + x86_init.irqs.intr_mode_init= x86_init_noop;
>   x86_init.oem.arch_setup = xen_arch_setup;
>   x86_init.oem.banner = xen_banner;
>  
> 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] common/gnttab: Introduce command line feature controls

[Juergen Gross]

On 25/08/17 18:21, George Dunlap wrote:
> On 08/25/2017 01:31 PM, Jan Beulich wrote:
>>>>> On 25.08.17 at 14:10, <andrew.coop...@citrix.com> wrote:
>>> On 25/08/17 10:57, Jan Beulich wrote:
>>>>>>> On 24.08.17 at 17:16, <andrew.coop...@citrix.com> wrote:
>>>>> On 24/08/17 16:01, Juergen Gross wrote:
>>>>>> On 24/08/17 16:50, Andrew Cooper wrote:
>>>>>>> --- a/docs/misc/xen-command-line.markdown
>>>>>>> +++ b/docs/misc/xen-command-line.markdown
>>>>>>> @@ -868,6 +868,19 @@ Controls EPT related features.
>>>>>>>  
>>>>>>>  Specify which console gdbstub should use. See **console**.
>>>>>>>  
>>>>>>> +### gnttab
>>>>>>> +> `= List of [ max_ver:, transitive ]`
>>>>>>> +
>>>>>>> +> Default: `gnttab=max_ver:2,transitive`
>>>>>>> +
>>>>>>> +Control various aspects of the grant table behaviour available to 
>>>>>>> guests.
>>>>>>> +
>>>>>>> +* `max_ver` Select the maximum grant table version to offer to guests. 
>>>>>>>  Valid
>>>>>>> +version are 1 and 2.
>>>>>>> +* `transitive` Permit or disallow the use of transitive grants.  Note 
>>>>>>> that the
>>>>>>> +use of grant table v2 without transitive grants is an ABI breakage 
>>>>>>> from the
>>>>>>> +guests point of view.
>>>>>> So shouldn't there be a way for the guest to query the support of
>>>>>> transient grants?
>>>>> Ideally yes, but how do you suggest doing this in a compatible way?
>>>>>
>>>>> All Xen downstreams which haven't backported the eventual transitive
>>>>> fixes will have this clobber in place, without any query-ability.
>>>> That workaround should not be used as an argument to not
>>>> provide a way to query the capability. It was put in place knowing
>>>> that it would cause problems for (hypothetical) guests using
>>>> transitive grants.
>>>
>>> I am not objecting to introducing a mechanism if a suitable one can be
>>> found.
>>>
>>> However, the heritage of XSA-226 is a valid reason to not block this
>>> patch because a mechanism isn't present.
>>
>> Code submission deadline for 4.10 isn't very far away; we shouldn't
>> ship a major version with a partial workaround.
> 
> I'd say we shouldn't ship a major version with a risky, unused feature
> on by default.

You are aware that this "unused feature" is part of the public interface
since about 8 years or so?


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH] xen: fix parse_bool() with empty string

[Juergen Gross]

parse_bool() should return -1 in case it is called with an empty
string. In order to allow boolean parameters in the cmdline without
specifying a value this case must be handled in _cmdline_parse() by
always passing a value string.

This fixes commit 532dec8e31174ed450adfd36a4b0b41dec27010d ("xen:
add an optional string end parameter to parse_bool()")

Signed-off-by: Juergen Gross <jgr...@suse.com>
---
 xen/common/kernel.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index ec7714961a..71bc547d17 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -114,7 +114,7 @@ static void __init _cmdline_parse(const char *cmdline)
 simple_strtoll(optval, NULL, 0));
 break;
 case OPT_BOOL:
-if ( !parse_bool(optval, NULL) )
+if ( !parse_bool(*optval ? optval : "yes", NULL) )
 bool_assert = !bool_assert;
 assign_integer_param(param, bool_assert);
 break;
@@ -168,6 +168,8 @@ int __init parse_bool(const char *s, const char *e)
 unsigned int len;
 
 len = e ? ({ ASSERT(e >= s); e - s; }) : strlen(s);
+if ( !len )
+return -1;
 
 if ( !strncmp("no", s, len) ||
  !strncmp("off", s, len) ||
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] common/gnttab: Introduce command line feature controls

[Juergen Gross]

On 25/08/17 14:29, Jan Beulich wrote:
 On 25.08.17 at 14:05,  wrote:
>> On 25/08/17 10:49, Jan Beulich wrote:
>> On 24.08.17 at 16:50,  wrote:
 --- a/docs/misc/xen-command-line.markdown
 +++ b/docs/misc/xen-command-line.markdown
 @@ -868,6 +868,19 @@ Controls EPT related features.
  
  Specify which console gdbstub should use. See **console**.
  
 +### gnttab
 +> `= List of [ max_ver:, transitive ]`
 +
 +> Default: `gnttab=max_ver:2,transitive`
 +
 +Control various aspects of the grant table behaviour available to guests.
 +
 +* `max_ver` Select the maximum grant table version to offer to guests.  
 Valid
 +version are 1 and 2.
 +* `transitive` Permit or disallow the use of transitive grants.  Note 
 that the
 +use of grant table v2 without transitive grants is an ABI breakage from 
 the
 +guests point of view.
>>> Btw, with the need to use v2 on huge systems I'm no longer
>>> convinced providing an option to disable it is a good idea.
>>
>> "necessary to support larger systems" is not a valid reason to prevent
>> smaller systems having the option to reduce their hypervisor attack surface.
> 
> Well, yes, one can view it that way. Two questions then, though:
> 1) If at some point someone comes up with a much better quality
> v3, how will your option syntax fit that (i.e. to exclude just v2)?
> 2) Switching between versions (post-migration) requires extra code
> in guests, albeit perhaps not very much. Is it wise to require OSes
> to be capable of switching back and forth?

BTW: the documentation of "transitive" could be better: does specifying
"transitive" permit or disallow the use of transitive grants?


Juergen


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] common/gnttab: Introduce command line feature controls

[Juergen Gross]

On 25/08/17 14:10, Andrew Cooper wrote:
> On 25/08/17 10:57, Jan Beulich wrote:
>>>>> On 24.08.17 at 17:16, <andrew.coop...@citrix.com> wrote:
>>> On 24/08/17 16:01, Juergen Gross wrote:
>>>> On 24/08/17 16:50, Andrew Cooper wrote:
>>>>> --- a/docs/misc/xen-command-line.markdown
>>>>> +++ b/docs/misc/xen-command-line.markdown
>>>>> @@ -868,6 +868,19 @@ Controls EPT related features.
>>>>>  
>>>>>  Specify which console gdbstub should use. See **console**.
>>>>>  
>>>>> +### gnttab
>>>>> +> `= List of [ max_ver:, transitive ]`
>>>>> +
>>>>> +> Default: `gnttab=max_ver:2,transitive`
>>>>> +
>>>>> +Control various aspects of the grant table behaviour available to guests.
>>>>> +
>>>>> +* `max_ver` Select the maximum grant table version to offer to guests.  
>>>>> Valid
>>>>> +version are 1 and 2.
>>>>> +* `transitive` Permit or disallow the use of transitive grants.  Note 
>>>>> that the
>>>>> +use of grant table v2 without transitive grants is an ABI breakage from 
>>>>> the
>>>>> +guests point of view.
>>>> So shouldn't there be a way for the guest to query the support of
>>>> transient grants?
>>> Ideally yes, but how do you suggest doing this in a compatible way?
>>>
>>> All Xen downstreams which haven't backported the eventual transitive
>>> fixes will have this clobber in place, without any query-ability.
>> That workaround should not be used as an argument to not
>> provide a way to query the capability. It was put in place knowing
>> that it would cause problems for (hypothetical) guests using
>> transitive grants.
> 
> I am not objecting to introducing a mechanism if a suitable one can be
> found.
> 
> However, the heritage of XSA-226 is a valid reason to not block this
> patch because a mechanism isn't present.
> 
>>
>> I'm not sure Jürgen's ELF note suggestion would be very useful
>> though: I don't see how Xen knowing a guest kernel can deal with
>> the situation would change anything - I don't think we should
>> fail the loading of a kernel without such a note when transitive
>> grants are disabled, not the least because we know of no kernels
>> using them, and hence we'd pointlessly prevent the use of older
>> kernels in such a case.
>>
>> What about a negative XENFEAT_*? New code could query it,
>> and existing code is hosed anyway if run on such a system.
> 
> Better yet, how about combining it with Juergens "xen: add new hypercall
> to get grant table limits"?

I suspect this new hypercall has just been killed.

> We could have a features_available bitmap along with other gnttab
> related maxima.

Feel free to recycle my patch then. :-)

OTOH XENFEAT_* might be just the place where such information should
be made available.


Juergen


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 5/5] xen: add new hypercall to get grant table limits

[Juergen Gross]

On 25/08/17 13:58, Jan Beulich wrote:
 On 25.08.17 at 13:40,  wrote:
>> So what about the following idea:
>>
>> - Set the default max number of grant frames to 32 on "small" hosts
>>   (max. physical memory address below 16TB) and to 64 on "large"
>>   hosts
> 
> This looks reasonable to me as long as it's properly documented.
> 
>> - Add a per-domain parameter for the max number of grant frames
>>
>> In the Linux kernel I would then:
>>
>> - Re-add grant v2 support
>> - Add boot parameter for selecting grant v1 or v2
>> - Use grant v2 if selected via boot parameter, or (pv only:) max. host
>>   address is above 16TB
> 
> Can a PV DomU know?

XENMEM_maximum_ram_page

> Plus can you easily switch between
> versions after having been migrated?

Other than console and xenbus there should be no active grant after
migration. So yes, I'm planning to support both, v1->v2 and v2->v1.

> And wouldn't HVM or ARM
> guests need to judge by max guest address?

Yes.


Juergen


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 5/5] xen: add new hypercall to get grant table limits

[Juergen Gross]

On 24/08/17 17:20, Jan Beulich wrote:
 On 24.08.17 at 17:13,  wrote:
>> On 24/08/17 17:04, Jan Beulich wrote:
>> On 24.08.17 at 16:48,  wrote:
 How would the guest know whether using v2 grants is no disadvantage?
>>>
>>> As said - it's always going to be a disadvantage. Even if controlling
>>> the number of frames per-domain, that same number of frames
>>> will always fit more v1 than v2 entries.
>>
>> And my patches break the assumption "same number of frames".
>>
>>> I don't think the config
>>> setting should directly control the number of active grants.
>>
>> Why not? In the end that number is the one the guest is interested in.
> 
> And the resource use is what the admin is interested in.
> 
>> BTW: the number of needed maptrack frames is depending on the number
>> of grants only, not on v1 or v2. And the default for the max. number
>> of maptrack frames is much higher than the one of the grant frames
>> (1024 vs 32).
> 
> Of course.

So what about the following idea:

- Set the default max number of grant frames to 32 on "small" hosts
  (max. physical memory address below 16TB) and to 64 on "large"
  hosts
- Add a per-domain parameter for the max number of grant frames

In the Linux kernel I would then:

- Re-add grant v2 support
- Add boot parameter for selecting grant v1 or v2
- Use grant v2 if selected via boot parameter, or (pv only:) max. host
  address is above 16TB


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 19/53] xen/arch/x86/psr.c: let custom parameter parsing routines return errno

[Juergen Gross]

On 25/08/17 10:17, Jan Beulich wrote:
 On 24.08.17 at 18:27,  wrote:
>> On 24/08/17 17:35, Jan Beulich wrote:
>> On 23.08.17 at 19:34,  wrote:
 --- a/xen/arch/x86/psr.c
 +++ b/xen/arch/x86/psr.c
 @@ -418,50 +418,66 @@ static const struct feat_props l2_cat_props = {
  .write_msr = l2_cat_write_msr,
  };
  
 -static void __init parse_psr_bool(char *s, char *value, char *feature,
 +static bool __init parse_psr_bool(const char *s, const char *value,
 +  const char *ss, const char *feature,
unsigned int mask)
  {
 -if ( !strcmp(s, feature) )
 +if ( !strncmp(s, feature, value - s) )
  {
 -if ( !value )
 +if ( !*value )
  opt_psr |= mask;
  else
  {
 -int val_int = parse_bool(value, NULL);
 +int val_int = parse_bool(value + 1, ss);
>>>
>>> Why "+ 1" here?
>>
>> value points to the delimiter ('\0' or ',') now.
> 
> That's pretty counterintuitive and error prone, don't you think?
> At the very least the parameter then shouldn't be called "value".

Okay, I'll rename it to "delim".


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 01/53] xen: add an optional string end parameter to parse_bool()

[Juergen Gross]

On 24/08/17 17:43, Andrew Cooper wrote:
> On 23/08/17 18:33, Juergen Gross wrote:
>> Add a parameter to parse_bool() to specify the end of the to be
>> parsed string. Specifying it as NULL will preserve the current
>> behavior to parse until the end of the input string, while passing
>> a non-NULL pointer will specify the first character after the input
>> string.
>>
>> This will allow to parse boolean sub-strings without having to
>> write a NUL byte into the input string.
>>
>> Modify all users of parse_bool() to pass NULL for the new parameter.
> 
> So I already had plans for parse_bool() during the XSA-226 embaro
> period, but couldn't discuss any of them, and this series appeared in
> the meantime.
> 
> One rather confusing problem we have is that top level booleans behave
> differently to sub-booleans.
> 
> Top-level booleans support all kinds of ={0,true,yes, ...} qualifiers,
> as well as no- prefixes.  sub-booleans may or may not support the
> qualifiers, and where they do support the no- prefixes, the same prefix
> gets silently eaten for non-boolean suboptions.

This is the "iommu=" case, right? And in fact the same statement is true
for all top-level parameters: you can easily specify "no-dma_bits=..."
which will be accepted. Only top-level custom parameters are handled in
a sane way with the "no-" prefix (they are accepted only with no option
value), and boolean parameters, of course.

> I had planned to modify parse_bool() to be
> 
> int parse_bool(const char *field, const char *s)
> {
> ...
> }
> 
> which cases care of considering the "no-" prefix, optionally skips the
> field name if it matches exactly, and then performs the current logic on
> the remainder of the string.  This way, boolean options should work
> consistently wherever they are.
> 
> It also means that a lot of custom_params() need simplifying to always
> pass intended boolean options to parse_bool().

I believe they do so in most cases.

> Could we see about merging this work together, rather than having two
> series going and modifying how the parsing works?

Hmm, I'm not sure it is worth the effort. Doing a quick search I found
only the iommu case where this would be relevant. All other cases are
handled correctly by _cmdline_parse(): a custom parameter prefixed with
"no-" is accepted only without a value specification.

I'd rather add one further patch to my series to correct the iommu case
and another one to fix _cmdline_parse() for the other cases where the
"no-" prefix is accepted for non-boolean parameters.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/5] xen: move XENMAPSPACE_grant_table code into grant_table.c

[Juergen Gross]

On 24/08/17 18:12, Julien Grall wrote:
> Hello,
> 
> I was expecting to be CCed on this patch.

Oops, sorry for that.

> 
> On 21/08/17 19:05, Juergen Gross wrote:
>> The x86 and arm versions of XENMAPSPACE_grant_table handling are nearly
>> identical. Move the code into a function in grant_table.c and add an
>> architecture dependant hook to handle the differences.
>>
>> This at once fixes a bug in the arm code which didn't unlock the grant
>> table in error case.
>>
>> Signed-off-by: Juergen Gross <jgr...@suse.com>
>> ---
>>  xen/arch/arm/mm.c | 34 
>>  xen/arch/x86/mm.c | 41
>> ++-
>>  xen/common/grant_table.c  | 38
>> 
>>  xen/include/asm-arm/grant_table.h |  6 ++
>>  xen/include/asm-x86/grant_table.h |  5 +
>>  xen/include/xen/grant_table.h |  3 +++
>>  6 files changed, 66 insertions(+), 61 deletions(-)
>>
>> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
>> index a810a056d7..6dad167a8e 100644
>> --- a/xen/arch/arm/mm.c
>> +++ b/xen/arch/arm/mm.c
>> @@ -24,6 +24,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
> 
> Why sched.h has been moved earlier? Likely it means one of the header
> doesn't include all its dependency.

I'll have a look.


Juergen


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] common/gnttab: Introduce command line feature controls

[Juergen Gross]

On 24/08/17 17:16, Andrew Cooper wrote:
> On 24/08/17 16:01, Juergen Gross wrote:
>> On 24/08/17 16:50, Andrew Cooper wrote:
>>> This patch was originally a workaround for XSA-226.  Since then, transitive
>>> grants are believed to be functioning properly, and the defaults have 
>>> changed
>>> appropriately.
>>>
>>> However, for those people who chose to use the workaround (especially from 
>>> an
>>> attack surface mitigation point of view), retain the ability for the host
>>> administrator to choose.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
>>> ---
>>> CC: Jan Beulich <jbeul...@suse.com>
>>> CC: George Dunlap <george.dun...@eu.citrix.com>
>>> CC: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
>>> CC: Stefano Stabellini <sstabell...@kernel.org>
>>> CC: Tim Deegan <t...@xen.org>
>>> CC: Wei Liu <wei.l...@citrix.com>
>>> ---
>>>  docs/misc/xen-command-line.markdown | 13 +++
>>>  xen/common/grant_table.c| 44 
>>> +++--
>>>  2 files changed, 55 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/docs/misc/xen-command-line.markdown 
>>> b/docs/misc/xen-command-line.markdown
>>> index 4002eab..78c7b51 100644
>>> --- a/docs/misc/xen-command-line.markdown
>>> +++ b/docs/misc/xen-command-line.markdown
>>> @@ -868,6 +868,19 @@ Controls EPT related features.
>>>  
>>>  Specify which console gdbstub should use. See **console**.
>>>  
>>> +### gnttab
>>> +> `= List of [ max_ver:, transitive ]`
>>> +
>>> +> Default: `gnttab=max_ver:2,transitive`
>>> +
>>> +Control various aspects of the grant table behaviour available to guests.
>>> +
>>> +* `max_ver` Select the maximum grant table version to offer to guests.  
>>> Valid
>>> +version are 1 and 2.
>>> +* `transitive` Permit or disallow the use of transitive grants.  Note that 
>>> the
>>> +use of grant table v2 without transitive grants is an ABI breakage from the
>>> +guests point of view.
>> So shouldn't there be a way for the guest to query the support of
>> transient grants?
> 
> Ideally yes, but how do you suggest doing this in a compatible way?

An ELF note in the guest kernel indicating it is aware of that
possibility in order to allow v2 grants for that guest without transient
grants?

> All Xen downstreams which haven't backported the eventual transitive
> fixes will have this clobber in place, without any query-ability.

So the only really compatible way would be to disallow v2 grants. OTOH
I guess there aren't so many guests using v2 right now...

> The reason this workaround was so effective, and why several large users
> still wish to clobber them, is because nothing uses transitive grants.

Right. And only very few guests use v2 grants.

> 
>>
>>> +
>>>  ### gnttab\_max\_frames
>>>  > `= `
>>>  
>>> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
>>> index 36895aa..f9c313d 100644
>>> --- a/xen/common/grant_table.c
>>> +++ b/xen/common/grant_table.c
>>> @@ -50,6 +50,42 @@ integer_param("gnttab_max_nr_frames", 
>>> max_nr_grant_frames);
>>>  unsigned int __read_mostly max_grant_frames;
>>>  integer_param("gnttab_max_frames", max_grant_frames);
>>>  
>>> +static unsigned int __read_mostly opt_gnttab_max_version = 2;
>>> +static bool __read_mostly opt_transitive_grants = true;
>>> +
>>> +static void __init parse_gnttab(char *s)
>> Do you mind using:
>>
>> static int __init parse_gnttab(const char *s)
>>
>> in order to comply with my runtime parameter series?
> 
> If the result will still compile.  What should the semantics be?  (I've
> had a quick look through your series, but I can't see the semantics
> stated anywhere obvious)

The parsing routine must not change the parameter string and should
return an error (e.g. -EINVAL) in case of an illegal parameter.


Juergen


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 01/53] xen: add an optional string end parameter to parse_bool()

[Juergen Gross]

On 24/08/17 17:33, Jan Beulich wrote:
 On 23.08.17 at 19:33,  wrote:
>> @@ -163,20 +163,24 @@ void __init cmdline_parse(const char *cmdline)
>>  #endif
>>  }
>>  
>> -int __init parse_bool(const char *s)
>> +int __init parse_bool(const char *s, const char *e)
>>  {
>> -if ( !strcmp("no", s) ||
>> - !strcmp("off", s) ||
>> - !strcmp("false", s) ||
>> - !strcmp("disable", s) ||
>> - !strcmp("0", s) )
>> +unsigned int len;
>> +
>> +len = e ? e - s : strlen(s);
> 
> If you don't mind, I'd like to see ASSERT(e >= s) added to the middle
> part here; should be easily doable while committing.

Sure, NP.


Juergen


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 19/53] xen/arch/x86/psr.c: let custom parameter parsing routines return errno

[Juergen Gross]

On 24/08/17 17:35, Jan Beulich wrote:
 On 23.08.17 at 19:34,  wrote:
>> --- a/xen/arch/x86/psr.c
>> +++ b/xen/arch/x86/psr.c
>> @@ -418,50 +418,66 @@ static const struct feat_props l2_cat_props = {
>>  .write_msr = l2_cat_write_msr,
>>  };
>>  
>> -static void __init parse_psr_bool(char *s, char *value, char *feature,
>> +static bool __init parse_psr_bool(const char *s, const char *value,
>> +  const char *ss, const char *feature,
>>unsigned int mask)
>>  {
>> -if ( !strcmp(s, feature) )
>> +if ( !strncmp(s, feature, value - s) )
>>  {
>> -if ( !value )
>> +if ( !*value )
>>  opt_psr |= mask;
>>  else
>>  {
>> -int val_int = parse_bool(value, NULL);
>> +int val_int = parse_bool(value + 1, ss);
> 
> Why "+ 1" here?

value points to the delimiter ('\0' or ',') now.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 4/5] xen: support different gnttab_max_frames for grant v1 and v2

[Juergen Gross]

On 24/08/17 17:01, Jan Beulich wrote:
 On 24.08.17 at 16:54,  wrote:
>> OTOH I think there should be a default especially on huge hosts
>> allowing to use v2 grants without reducing the number of allowed
>> grants, which doesn't need adding another parameter to the domain
>> config. Or do you want the tools to always set the per-domain value
>> possibly based on a xl.conf value? Then we could remove the
>> gnttab_max_frames command line parameter completely.
> 
> Yes, obsoleting that command line option would seem quite
> desirable to me.

Okay, I can modify my patches in that regard.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 5/5] xen: add new hypercall to get grant table limits

[Juergen Gross]

On 24/08/17 17:04, Jan Beulich wrote:
 On 24.08.17 at 16:48,  wrote:
>> On 24/08/17 16:28, Jan Beulich wrote:
>> On 21.08.17 at 20:05,  wrote:
 Today a guest can get the maximum grant table frame number for the
 currently selected grant table interface version (1 or 2) only. Add
 a new grant table operation to get the limits of both variants in
 order to give the guest an opportunity to select the version serving
 its needs best.

 Background for the need for this new hypercall is that e.g. the Linux
 kernel won't use v2 as long as this will allow less active grants as
 v1. As soon as the kernel can detect it can create as many v2 entries
 as for v1, it will have no reason not to use v2. And this will allow
 Xen placing a pv-domain with such a kernel above the current 16TB
 RAM limit.

 For setting up the grant table a kernel needs the following
 information:
 - current version (kexec case)
 - current size (kexec case)
 - max size v1
 - max size v2
 In order not to have to issue 3 hypercalls (GNTTABOP_query_size,
 GNTTABOP_get_version, GNTTABOP_get_v1_and_v2_max), let the new
 hypercall return all the needed information.
>>>
>>> I'm not sure I follow: v2 is always going to allow less active grants
>>> than v1, as the limit is always derived from the number of frames
>>> allowed for a domain.
>>
>> Right. Patch 3 adds support for different number of allowed frames for
>> v1 and v2. So the system can be configured to allow the same max.
>> number of grants for v1 and v2, or even more v2 grants than v1.
>>
>>> I also don't see a problem with issuing multiple
>>> calls - none of this ought to be performance critical. I would,
>>> however, agree that rather than adding a new hypercall to just
>>> return the max sizes adding one like you suggest would be
>>> preferable. I'm simply not convinced yet that returning the max
>>> sizes is actually necessary.
>>
>> How would the guest know whether using v2 grants is no disadvantage?
> 
> As said - it's always going to be a disadvantage. Even if controlling
> the number of frames per-domain, that same number of frames
> will always fit more v1 than v2 entries.

And my patches break the assumption "same number of frames".

> I don't think the config
> setting should directly control the number of active grants.

Why not? In the end that number is the one the guest is interested in.
BTW: the number of needed maptrack frames is depending on the number
of grants only, not on v1 or v2. And the default for the max. number
of maptrack frames is much higher than the one of the grant frames
(1024 vs 32).

> Or if we did it that way, then the answer to your question would be
> "based on hypervisor version".

Yeah, or based on the answer from the hypervisor regarding my new
info call (if the answer is "-ENOSYS" the guest probably would choose
v1 as today).


Juergen


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 5/5] xen: add new hypercall to get grant table limits

[Juergen Gross]

On 24/08/17 16:48, Juergen Gross wrote:
> On 24/08/17 16:28, Jan Beulich wrote:
>>>>> On 21.08.17 at 20:05, <jgr...@suse.com> wrote:
>>> Today a guest can get the maximum grant table frame number for the
>>> currently selected grant table interface version (1 or 2) only. Add
>>> a new grant table operation to get the limits of both variants in
>>> order to give the guest an opportunity to select the version serving
>>> its needs best.
>>>
>>> Background for the need for this new hypercall is that e.g. the Linux
>>> kernel won't use v2 as long as this will allow less active grants as
>>> v1. As soon as the kernel can detect it can create as many v2 entries
>>> as for v1, it will have no reason not to use v2. And this will allow
>>> Xen placing a pv-domain with such a kernel above the current 16TB
>>> RAM limit.
>>>
>>> For setting up the grant table a kernel needs the following
>>> information:
>>> - current version (kexec case)
>>> - current size (kexec case)
>>> - max size v1
>>> - max size v2
>>> In order not to have to issue 3 hypercalls (GNTTABOP_query_size,
>>> GNTTABOP_get_version, GNTTABOP_get_v1_and_v2_max), let the new
>>> hypercall return all the needed information.
>>
>> I'm not sure I follow: v2 is always going to allow less active grants
>> than v1, as the limit is always derived from the number of frames
>> allowed for a domain.
> 
> Right. Patch 3 adds support for different number of allowed frames for

This should read "Patch 4", of course

> v1 and v2. So the system can be configured to allow the same max.
> number of grants for v1 and v2, or even more v2 grants than v1.
> 
>> I also don't see a problem with issuing multiple
>> calls - none of this ought to be performance critical. I would,
>> however, agree that rather than adding a new hypercall to just
>> return the max sizes adding one like you suggest would be
>> preferable. I'm simply not convinced yet that returning the max
>> sizes is actually necessary.
> 
> How would the guest know whether using v2 grants is no disadvantage?
> 
> 
> Juergen
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel
> 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] common/gnttab: Introduce command line feature controls

[Juergen Gross]

On 24/08/17 16:50, Andrew Cooper wrote:
> This patch was originally a workaround for XSA-226.  Since then, transitive
> grants are believed to be functioning properly, and the defaults have changed
> appropriately.
> 
> However, for those people who chose to use the workaround (especially from an
> attack surface mitigation point of view), retain the ability for the host
> administrator to choose.
> 
> Signed-off-by: Andrew Cooper 
> ---
> CC: Jan Beulich 
> CC: George Dunlap 
> CC: Konrad Rzeszutek Wilk 
> CC: Stefano Stabellini 
> CC: Tim Deegan 
> CC: Wei Liu 
> ---
>  docs/misc/xen-command-line.markdown | 13 +++
>  xen/common/grant_table.c| 44 
> +++--
>  2 files changed, 55 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.markdown 
> b/docs/misc/xen-command-line.markdown
> index 4002eab..78c7b51 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -868,6 +868,19 @@ Controls EPT related features.
>  
>  Specify which console gdbstub should use. See **console**.
>  
> +### gnttab
> +> `= List of [ max_ver:, transitive ]`
> +
> +> Default: `gnttab=max_ver:2,transitive`
> +
> +Control various aspects of the grant table behaviour available to guests.
> +
> +* `max_ver` Select the maximum grant table version to offer to guests.  Valid
> +version are 1 and 2.
> +* `transitive` Permit or disallow the use of transitive grants.  Note that 
> the
> +use of grant table v2 without transitive grants is an ABI breakage from the
> +guests point of view.

So shouldn't there be a way for the guest to query the support of
transient grants?

> +
>  ### gnttab\_max\_frames
>  > `= `
>  
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 36895aa..f9c313d 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -50,6 +50,42 @@ integer_param("gnttab_max_nr_frames", max_nr_grant_frames);
>  unsigned int __read_mostly max_grant_frames;
>  integer_param("gnttab_max_frames", max_grant_frames);
>  
> +static unsigned int __read_mostly opt_gnttab_max_version = 2;
> +static bool __read_mostly opt_transitive_grants = true;
> +
> +static void __init parse_gnttab(char *s)

Do you mind using:

static int __init parse_gnttab(const char *s)

in order to comply with my runtime parameter series?


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 4/5] xen: support different gnttab_max_frames for grant v1 and v2

[Juergen Gross]

On 24/08/17 16:21, Jan Beulich wrote:
>>>> On 21.08.17 at 20:05, <jgr...@suse.com> wrote:
>> The number of grants a domain can setup is limited by the maximum
>> number of grant frames it is allowed to use. Today the limit is the
>> same regardless whether the domain uses grant v1 or v2. Using v2
>> will therefor be a disadvantage for the domain as only half the
>> number of grants compared to v1 can be used, because a grant v2 entry
>> is twice as large as the v1 entry. This is the reason for the lack of
>> grant v2 support in the Linux kernel (in fact grant v2 support has
>> been removed from Linux for this reason).
>>
>> OTOH using only grant v1 will limit a pv domain to the low 16TB of
>> memory of the host, as grant v1 entries only use a 32 bit mfn. So
>> if we want to support more than 16TB of memory and be able to use
>> that memory in pv domains, we have to remove the disadvantage of
>> using grant v2 by being able to setup the same number of grants as
>> with v1.
>>
>> In order to achieve this add support for limiting the number of grant
>> frames for v1 and v2 independently from each other. Per default let
>> the v2 number be twice the value of the v1 number. Modify the boot
>> parameter gnttab_max_frames to accept either a single value which
>> will set the v1 limit to that value and the v2 limit to 2*value, or
>> two values separated by a comma to set both limits to dedicated
>> values.
>>
>> Add some sanity checks to make sure the maximum number of frames isn't
>> lower than the initial number, as this leads to rather strange crashes.
>>
>> Signed-off-by: Juergen Gross <jgr...@suse.com>
> 
> As discussed elsewhere, this probably rather wants to become a
> per-domain setting then. Looking also at what patch 5 adds, I
> additionally wonder whether we shouldn't allow Dom0 to know
> whether it needs to use v2 at all (or maybe it can derive that
> already).

I'm absolutely in favor of adding a way to make this a per-domain
setting.

OTOH I think there should be a default especially on huge hosts
allowing to use v2 grants without reducing the number of allowed
grants, which doesn't need adding another parameter to the domain
config. Or do you want the tools to always set the per-domain value
possibly based on a xl.conf value? Then we could remove the
gnttab_max_frames command line parameter completely.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 5/5] xen: add new hypercall to get grant table limits

[Juergen Gross]

On 24/08/17 16:28, Jan Beulich wrote:
 On 21.08.17 at 20:05,  wrote:
>> Today a guest can get the maximum grant table frame number for the
>> currently selected grant table interface version (1 or 2) only. Add
>> a new grant table operation to get the limits of both variants in
>> order to give the guest an opportunity to select the version serving
>> its needs best.
>>
>> Background for the need for this new hypercall is that e.g. the Linux
>> kernel won't use v2 as long as this will allow less active grants as
>> v1. As soon as the kernel can detect it can create as many v2 entries
>> as for v1, it will have no reason not to use v2. And this will allow
>> Xen placing a pv-domain with such a kernel above the current 16TB
>> RAM limit.
>>
>> For setting up the grant table a kernel needs the following
>> information:
>> - current version (kexec case)
>> - current size (kexec case)
>> - max size v1
>> - max size v2
>> In order not to have to issue 3 hypercalls (GNTTABOP_query_size,
>> GNTTABOP_get_version, GNTTABOP_get_v1_and_v2_max), let the new
>> hypercall return all the needed information.
> 
> I'm not sure I follow: v2 is always going to allow less active grants
> than v1, as the limit is always derived from the number of frames
> allowed for a domain.

Right. Patch 3 adds support for different number of allowed frames for
v1 and v2. So the system can be configured to allow the same max.
number of grants for v1 and v2, or even more v2 grants than v1.

> I also don't see a problem with issuing multiple
> calls - none of this ought to be performance critical. I would,
> however, agree that rather than adding a new hypercall to just
> return the max sizes adding one like you suggest would be
> preferable. I'm simply not convinced yet that returning the max
> sizes is actually necessary.

How would the guest know whether using v2 grants is no disadvantage?


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 3/5] xen: clean up grant_table.h

[Juergen Gross]

On 24/08/17 16:17, Jan Beulich wrote:
 On 21.08.17 at 20:05,  wrote:
>> @@ -118,6 +154,18 @@ struct grant_mapping {
>>  uint32_t pad;   /* round size to a power of 2 */
>>  };
>>  
>> +/* Number of grant table frames. Caller must hold d's grant table lock. */
>> +static inline unsigned int nr_grant_frames(struct grant_table *gt)
>> +{
>> +return gt->nr_grant_frames;
>> +}
>> +
>> +/* Number of status grant table frames. Caller must hold d's gr. table 
>> lock.*/
>> +static inline unsigned int nr_status_frames(struct grant_table *gt)
>> +{
>> +return gt->nr_status_frames;
>> +}
> 
> For both the parameters want to be constified.

Okay.

> 
>> @@ -250,6 +318,15 @@ static inline void active_entry_release(struct 
>> active_grant_entry *act)
>>  spin_unlock(>lock);
>>  }
>>  
>> +#define GRANT_STATUS_PER_PAGE (PAGE_SIZE / sizeof(grant_status_t))
>> +#define GRANT_PER_PAGE (PAGE_SIZE / sizeof(grant_entry_v2_t))
>> +/* Number of grant table status entries. Caller must hold d's gr. table 
>> lock.*/
>> +static inline unsigned int grant_to_status_frames(int grant_frames)
> 
> unsigned int also for the parameter.

Okay.

> 
>> --- a/xen/include/xen/grant_table.h
>> +++ b/xen/include/xen/grant_table.h
>> @@ -29,65 +29,8 @@
>>  #include 
>>  #include 
>>  
>> -#ifndef DEFAULT_MAX_NR_GRANT_FRAMES /* to allow arch to override */
>> -/* Default maximum size of a grant table. [POLICY] */
>> -#define DEFAULT_MAX_NR_GRANT_FRAMES   32
>> -#endif
>>  /* The maximum size of a grant table. */
>> -extern unsigned int max_grant_frames;
>> -
>> -DECLARE_PERCPU_RWLOCK_GLOBAL(grant_rwlock);
>> -
>> -/* Per-domain grant information. */
>> -struct grant_table {
>> -/*
>> - * Lock protecting updates to grant table state (version, active
>> - * entry list, etc.)
>> - */
>> -percpu_rwlock_t   lock;
>> -/* Table size. Number of frames shared with guest */
>> -unsigned int  nr_grant_frames;
>> -/* Shared grant table (see include/public/grant_table.h). */
>> -union {
>> -void **shared_raw;
>> -struct grant_entry_v1 **shared_v1;
>> -union grant_entry_v2 **shared_v2;
>> -};
>> -/* Number of grant status frames shared with guest (for version 2) */
>> -unsigned int  nr_status_frames;
>> -/* State grant table (see include/public/grant_table.h). */
>> -grant_status_t   **status;
>> -/* Active grant table. */
>> -struct active_grant_entry **active;
>> -/* Mapping tracking table per vcpu. */
>> -struct grant_mapping **maptrack;
>> -unsigned int  maptrack_limit;
>> -/* Lock protecting the maptrack limit */
>> -spinlock_tmaptrack_lock;
>> -/* The defined versions are 1 and 2.  Set to 0 if we don't know
>> -   what version to use yet. */
>> -unsigned  gt_version;
>> -};
>> -
>> -static inline void grant_read_lock(struct grant_table *gt)
>> -{
>> -percpu_read_lock(grant_rwlock, >lock);
>> -}
>> -
>> -static inline void grant_read_unlock(struct grant_table *gt)
>> -{
>> -percpu_read_unlock(grant_rwlock, >lock);
>> -}
>> -
>> -static inline void grant_write_lock(struct grant_table *gt)
>> -{
>> -percpu_write_lock(grant_rwlock, >lock);
>> -}
>> -
>> -static inline void grant_write_unlock(struct grant_table *gt)
>> -{
>> -percpu_write_unlock(grant_rwlock, >lock);
>> -}
>> +extern unsigned int __read_mostly max_grant_frames;
> 
> Why are you adding __read_mostly here? Like all section placement
> annotations it belongs on definitions only, not declarations.

Oops, sorry.

> 
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -307,6 +307,7 @@ struct vm_event_per_domain
>>  };
>>  
>>  struct evtchn_port_ops;
>> +struct grant_table;
> 
> Why is this needed? There's no function with a respective parameter
> in the header here.

You are right, of course. Will remove it again.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen: Don't try to call xen_alloc_p2m_entry() on autotranslating guests

[Juergen Gross]

On 24/08/17 00:27, Boris Ostrovsky wrote:
> Commit aba831a69632 ("xen: remove tests for pvh mode in pure pv paths")
> removed XENFEAT_auto_translated_physmap test in xen_alloc_p2m_entry()
> since it is assumed that the routine is never called by non-PV guests.
> 
> However, alloc_xenballooned_pages() may make this call on a PVH guest.
> Prevent this from happening by adding XENFEAT_auto_translated_physmap
> check there.
> 
> Signed-off-by: Boris Ostrovsky <boris.ostrov...@oracle.com>
> Fixes: aba831a69632 ("xen: remove tests for pvh mode in pure pv paths")

Reviewed-by: Juergen Gross <jgr...@suse.com>


Thanks,

Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 10/53] xen/arch/x86/hvm/viridian.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/hvm/viridian.c

to indicate whether the parameter value was parsed successfully.

Fix an error in the parsing function: up to now it would overwrite the
stack in case more than 3 values are specified.

Cc: Paul Durrant <paul.durr...@citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Paul Durrant <paul.durr...@citrix.com>
---
V3:
- dont modify option value in parsing function
- fix error in parsing routine
---
 xen/arch/x86/hvm/viridian.c | 28 ++--
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index aa9b87c0ab..2edf9d0b23 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -1083,7 +1083,7 @@ static int viridian_load_vcpu_ctxt(struct domain *d, 
hvm_domain_context_t *h)
 HVM_REGISTER_SAVE_RESTORE(VIRIDIAN_VCPU, viridian_save_vcpu_ctxt,
   viridian_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
 
-static void __init parse_viridian_version(char *arg)
+static int __init parse_viridian_version(const char *arg)
 {
 const char *t;
 unsigned int n[3];
@@ -1093,17 +1093,24 @@ static void __init parse_viridian_version(char *arg)
 n[1] = viridian_minor;
 n[2] = viridian_build;
 
-while ( (t = strsep(, ",")) != NULL )
-{
+do {
 const char *e;
 
-if ( *t == '\0' )
-continue;
+t = strchr(arg, ',');
+if ( !t )
+t = strchr(arg, '\0');
+
+if ( *arg && *arg != ',' && i < 3 )
+{
+n[i] = simple_strtoul(arg, , 0);
+if ( e != t )
+goto fail;
+}
+
+i++;
+arg = t + 1;
+} while ( *t );
 
-n[i++] = simple_strtoul(t, , 0);
-if ( *e != '\0' )
-goto fail;
-}
 if ( i != 3 )
 goto fail;
 
@@ -1118,10 +1125,11 @@ static void __init parse_viridian_version(char *arg)
 
 printk("viridian-version = %#x,%#x,%#x\n",
viridian_major, viridian_minor, viridian_build);
-return;
+return 0;
 
  fail:
 printk(XENLOG_WARNING "Invalid viridian-version, using default\n");
+return -EINVAL;
 }
 custom_param("viridian-version", parse_viridian_version);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 51/53] libxl: add libxl_set_parameters() function

[Juergen Gross]

Add a new libxl function to set hypervisor parameters at runtime
similar to boot time parameters via command line.

Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
---
V2:
- corrected coding style (Wei Liu)
- removed superfluous #ifdef (Wei Liu)

V3:
- use LOGEV() for error message
---
 tools/libxl/libxl.c | 15 +++
 tools/libxl/libxl.h |  8 
 2 files changed, 23 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0ef874406f..247c56cf83 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -652,6 +652,21 @@ int libxl_send_debug_keys(libxl_ctx *ctx, char *keys)
 return 0;
 }
 
+int libxl_set_parameters(libxl_ctx *ctx, char *params)
+{
+int ret;
+GC_INIT(ctx);
+
+ret = xc_set_parameters(ctx->xch, params);
+if (ret < 0) {
+LOGEV(ERROR, ret, "setting parameters");
+GC_FREE;
+return ERROR_FAIL;
+}
+GC_FREE;
+return 0;
+}
+
 static int fd_set_flags(libxl_ctx *ctx, int fd,
 int fcntlgetop, int fcntlsetop, const char *fl,
 int flagmask, int set_p)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 229e289750..17045253ab 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1051,6 +1051,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, 
const libxl_mac *src);
  */
 #define LIBXL_HAVE_QED 1
 
+/*
+ * LIBXL_HAVE_SET_PARAMETERS
+ *
+ * If this is defined setting hypervisor parameters is supported.
+ */
+#define LIBXL_HAVE_SET_PARAMETERS 1
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
@@ -2105,6 +2112,7 @@ int libxl_send_trigger(libxl_ctx *ctx, uint32_t domid,
libxl_trigger trigger, uint32_t vcpuid);
 int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq);
 int libxl_send_debug_keys(libxl_ctx *ctx, char *keys);
+int libxl_set_parameters(libxl_ctx *ctx, char *params);
 
 typedef struct libxl__xen_console_reader libxl_xen_console_reader;
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 31/53] xen/drivers/char/console.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/char/console.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/drivers/char/console.c | 31 +++
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 8f2a24496a..36f94e6b94 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -67,7 +67,7 @@ enum con_timestamp_mode
 
 static enum con_timestamp_mode __read_mostly opt_con_timestamp_mode = TSM_NONE;
 
-static void parse_console_timestamps(char *s);
+static int parse_console_timestamps(const char *s);
 custom_param("console_timestamps", parse_console_timestamps);
 
 /* conring_size: allows a large console ring than default (16kB). */
@@ -123,8 +123,8 @@ static int __read_mostly xenlog_guest_upper_thresh =
 static int __read_mostly xenlog_guest_lower_thresh =
 XENLOG_GUEST_LOWER_THRESHOLD;
 
-static void parse_loglvl(char *s);
-static void parse_guest_loglvl(char *s);
+static int parse_loglvl(const char *s);
+static int parse_guest_loglvl(const char *s);
 
 /*
  *  := none|error|warning|info|debug|all
@@ -145,7 +145,7 @@ static atomic_t print_everything = ATOMIC_INIT(0);
 return (lvlnum);\
 }
 
-static int __init __parse_loglvl(char *s, char **ps)
+static int __init __parse_loglvl(const char *s, const char **ps)
 {
 ___parse_loglvl(s, ps, "none",0);
 ___parse_loglvl(s, ps, "error",   1);
@@ -156,23 +156,26 @@ static int __init __parse_loglvl(char *s, char **ps)
 return 2; /* sane fallback */
 }
 
-static void __init _parse_loglvl(char *s, int *lower, int *upper)
+static int __init _parse_loglvl(const char *s, int *lower, int *upper)
 {
 *lower = *upper = __parse_loglvl(s, );
 if ( *s == '/' )
 *upper = __parse_loglvl(s+1, );
 if ( *upper < *lower )
 *upper = *lower;
+
+return *s ? -EINVAL : 0;
 }
 
-static void __init parse_loglvl(char *s)
+static int __init parse_loglvl(const char *s)
 {
-_parse_loglvl(s, _lower_thresh, _upper_thresh);
+return _parse_loglvl(s, _lower_thresh, _upper_thresh);
 }
 
-static void __init parse_guest_loglvl(char *s)
+static int __init parse_guest_loglvl(const char *s)
 {
-_parse_loglvl(s, _guest_lower_thresh, _guest_upper_thresh);
+return _parse_loglvl(s, _guest_lower_thresh,
+ _guest_upper_thresh);
 }
 
 static char *loglvl_str(int lvl)
@@ -603,16 +606,16 @@ static int printk_prefix_check(char *p, char **pp)
 ((loglvl < upper_thresh) && printk_ratelimit()));
 } 
 
-static void __init parse_console_timestamps(char *s)
+static int __init parse_console_timestamps(const char *s)
 {
 switch ( parse_bool(s, NULL) )
 {
 case 0:
 opt_con_timestamp_mode = TSM_NONE;
-return;
+return 0;
 case 1:
 opt_con_timestamp_mode = TSM_DATE;
-return;
+return 0;
 }
 if ( *s == '\0' || /* Compat for old booleanparam() */
  !strcmp(s, "date") )
@@ -623,6 +626,10 @@ static void __init parse_console_timestamps(char *s)
 opt_con_timestamp_mode = TSM_BOOT;
 else if ( !strcmp(s, "none") )
 opt_con_timestamp_mode = TSM_NONE;
+else
+return -EINVAL;
+
+return 0;
 }
 
 static void printk_start_of_line(const char *prefix)
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 35/53] xen/drivers/passthrough/pci.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/passthrough/pci.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
V4:
- differentiate error return values (Jan Beulich)
- drop pointless test for input parameter being NULL (Jan Beulich)

V3:
- cosmetic changes (Jan Beulich)
- dont modify option value in parsing funtion
---
 xen/drivers/passthrough/pci.c | 38 --
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 27bdb7163c..ce22aa41a4 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -149,17 +149,20 @@ static struct phantom_dev {
 } phantom_devs[8];
 static unsigned int nr_phantom_devs;
 
-static void __init parse_phantom_dev(char *str) {
-const char *s = str;
+static int __init parse_phantom_dev(const char *str)
+{
+const char *s;
 unsigned int seg, bus, slot;
 struct phantom_dev phantom;
 
-if ( !s || !*s || nr_phantom_devs >= ARRAY_SIZE(phantom_devs) )
-return;
+if ( !*str )
+return -EINVAL;
+if ( nr_phantom_devs >= ARRAY_SIZE(phantom_devs) )
+return -E2BIG;
 
-s = parse_pci(s, , , , NULL);
+s = parse_pci(str, , , , NULL);
 if ( !s || *s != ',' )
-return;
+return -EINVAL;
 
 phantom.seg = seg;
 phantom.bus = bus;
@@ -170,10 +173,12 @@ static void __init parse_phantom_dev(char *str) {
 case 1: case 2: case 4:
 if ( *s )
 default:
-return;
+return -EINVAL;
 }
 
 phantom_devs[nr_phantom_devs++] = phantom;
+
+return 0;
 }
 custom_param("pci-phantom", parse_phantom_dev);
 
@@ -189,9 +194,10 @@ static u16 __read_mostly bridge_ctl_mask;
  *   perr   don't suppress parity errors (default)
  *   no-perrsuppress parity errors
  */
-static void __init parse_pci_param(char *s)
+static int __init parse_pci_param(const char *s)
 {
-char *ss;
+const char *ss;
+int rc = 0;
 
 do {
 bool_t on = !!strncmp(s, "no-", 3);
@@ -201,19 +207,21 @@ static void __init parse_pci_param(char *s)
 s += 3;
 
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
+if ( !ss )
+ss = strchr(s, '\0');
 
-if ( !strcmp(s, "serr") )
+if ( !strncmp(s, "serr", ss - s) )
 {
 cmd_mask = PCI_COMMAND_SERR;
 brctl_mask = PCI_BRIDGE_CTL_SERR | PCI_BRIDGE_CTL_DTMR_SERR;
 }
-else if ( !strcmp(s, "perr") )
+else if ( !strncmp(s, "perr", ss - s) )
 {
 cmd_mask = PCI_COMMAND_PARITY;
 brctl_mask = PCI_BRIDGE_CTL_PARITY;
 }
+else
+rc = -EINVAL;
 
 if ( on )
 {
@@ -227,7 +235,9 @@ static void __init parse_pci_param(char *s)
 }
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 custom_param("pci", parse_pci_param);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 07/53] xen/arch/x86/cpu/vpmu.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/cpu/vpmu.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/cpu/vpmu.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 1c0ea10777..fd2fcacc26 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -53,7 +53,7 @@ CHECK_pmu_params;
 static unsigned int __read_mostly opt_vpmu_enabled;
 unsigned int __read_mostly vpmu_mode = XENPMU_MODE_OFF;
 unsigned int __read_mostly vpmu_features = 0;
-static void parse_vpmu_params(char *s);
+static int parse_vpmu_params(const char *s);
 custom_param("vpmu", parse_vpmu_params);
 
 static DEFINE_SPINLOCK(vpmu_lock);
@@ -61,7 +61,7 @@ static unsigned vpmu_count;
 
 static DEFINE_PER_CPU(struct vcpu *, last_vcpu);
 
-static int parse_vpmu_param(char *s, unsigned int len)
+static int parse_vpmu_param(const char *s, unsigned int len)
 {
 if ( !*s || !len )
 return 0;
@@ -76,9 +76,9 @@ static int parse_vpmu_param(char *s, unsigned int len)
 return 0;
 }
 
-static void __init parse_vpmu_params(char *s)
+static int __init parse_vpmu_params(const char *s)
 {
-char *sep, *p = s;
+const char *sep, *p = s;
 
 switch ( parse_bool(s, NULL) )
 {
@@ -104,10 +104,11 @@ static void __init parse_vpmu_params(char *s)
 opt_vpmu_enabled = 1;
 break;
 }
-return;
+return 0;
 
  error:
 printk("VPMU: unknown flags: %s - vpmu disabled!\n", s);
+return -EINVAL;
 }
 
 void vpmu_lvtpc_update(uint32_t val)
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 11/53] xen/arch/x86/hvm/vmx/vmcs.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/hvm/vmx/vmcs.c

to indicate whether the parameter value was parsed successfully.

Cc: Jun Nakajima <jun.nakaj...@intel.com>
Cc: Kevin Tian <kevin.t...@intel.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
V3:
- dont modify option value in parsing function
---
 xen/arch/x86/hvm/vmx/vmcs.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 7854802cba..02f54e7f84 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -74,9 +74,10 @@ static s8 __read_mostly opt_ept_ad = -1;
  *  pml Enable PML
  *  ad  Use A/D bits
  */
-static void __init parse_ept_param(char *s)
+static int __init parse_ept_param(const char *s)
 {
-char *ss;
+const char *ss;
+int rc = 0;
 
 do {
 bool_t val = !!strncmp(s, "no-", 3);
@@ -85,16 +86,20 @@ static void __init parse_ept_param(char *s)
 s += 3;
 
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
+if ( !ss )
+ss = strchr(s, '\0');
 
-if ( !strcmp(s, "pml") )
+if ( !strncmp(s, "pml", ss - s) )
 opt_pml_enabled = val;
-else if ( !strcmp(s, "ad") )
+else if ( !strncmp(s, "ad", ss - s) )
 opt_ept_ad = val;
+else
+rc = -EINVAL;
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 custom_param("ept", parse_ept_param);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 40/53] xen: check parameter validity when parsing command line

[Juergen Gross]

Where possible check validity of parameters in _cmdline_parse() and
issue a warning message in case of an error detected.

In order to make sure a custom parameter parsing function really
returns a value (error or success), don't use a void pointer for
storing the function address, but a proper typed function pointer.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
V4:
- removed test for empty string from parse_bool() again, avoid calling
  parse_bool() in that case (Jan Beulich)
- print error number in failure case (Jan Beulich)
- limit scope of some variables (Jan Beulich)
- use type bool for bool_assert (Jan Beulich)

V3:
- use function pointer in struct kernel_param (Jan Beulich)
- better range check in assign_integer_param() (Jan Beulich)
- dont assign int values in case of overflow (Jan Beulich)
- allow multiple handlers for a parameter (Jan Beulich)

V2:
- replaced literal 8 by BITS_PER_BYTE (Wei Liu)
- added test for empty string to parse_bool()
---
 xen/common/kernel.c | 68 ++---
 xen/include/xen/init.h  | 30 +-
 xen/include/xen/types.h |  3 +++
 3 files changed, 80 insertions(+), 21 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 4979e1c49b..063ac99bff 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -23,34 +23,43 @@ enum system_state system_state = SYS_STATE_early_boot;
 xen_commandline_t saved_cmdline;
 static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
 
-static void __init assign_integer_param(
+static int __init assign_integer_param(
 const struct kernel_param *param, uint64_t val)
 {
 switch ( param->len )
 {
 case sizeof(uint8_t):
-*(uint8_t *)param->var = val;
+if ( val > UINT8_MAX && val < (uint64_t)INT8_MIN )
+return -EOVERFLOW;
+*(uint8_t *)param->par.var = val;
 break;
 case sizeof(uint16_t):
-*(uint16_t *)param->var = val;
+if ( val > UINT16_MAX && val < (uint64_t)INT16_MIN )
+return -EOVERFLOW;
+*(uint16_t *)param->par.var = val;
 break;
 case sizeof(uint32_t):
-*(uint32_t *)param->var = val;
+if ( val > UINT32_MAX && val < (uint64_t)INT32_MIN )
+return -EOVERFLOW;
+*(uint32_t *)param->par.var = val;
 break;
 case sizeof(uint64_t):
-*(uint64_t *)param->var = val;
+*(uint64_t *)param->par.var = val;
 break;
 default:
 BUG();
 }
+
+return 0;
 }
 
 static void __init _cmdline_parse(const char *cmdline)
 {
 char opt[128], *optval, *optkey, *q;
-const char *p = cmdline;
+const char *p = cmdline, *key;
 const struct kernel_param *param;
-int bool_assert;
+int rc;
+bool bool_assert, found;
 
 for ( ; ; )
 {
@@ -84,46 +93,66 @@ static void __init _cmdline_parse(const char *cmdline)
 }
 
 /* Boolean parameters can be inverted with 'no-' prefix. */
+key = optkey;
 bool_assert = !!strncmp("no-", optkey, 3);
 if ( !bool_assert )
 optkey += 3;
 
+rc = 0;
+found = false;
 for ( param = __setup_start; param < __setup_end; param++ )
 {
+int rctmp;
+const char *s;
+
 if ( strcmp(param->name, optkey) )
 {
 if ( param->type == OPT_CUSTOM && q &&
  strlen(param->name) == q + 1 - opt &&
  !strncmp(param->name, opt, q + 1 - opt) )
 {
+found = true;
 optval[-1] = '=';
-((void (*)(const char *))param->var)(q);
+rctmp = param->par.func(q);
 optval[-1] = '\0';
+if ( !rc )
+rc = rctmp;
 }
 continue;
 }
 
+rctmp = 0;
+found = true;
 switch ( param->type )
 {
 case OPT_STR:
-strlcpy(param->var, optval, param->len);
+strlcpy(param->par.var, optval, param->len);
 break;
 case OPT_UINT:
-assign_integer_param(
+rctmp = assign_integer_param(
 param,
-simple_strtoll(optval, NULL, 0));
+simple_strtoll(optval, , 0));
+if

[Xen-devel] [PATCH v4 19/53] xen/arch/x86/psr.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/psr.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
V4:
- pass end of parameter value to parse_bool()

V3:
- let parse_psr_bool() return bool value (Jan Beulich)
- return error in case no string matches (Jan Beulich)
- dont modify option value in parsing function
---
 xen/arch/x86/psr.c | 56 +++---
 1 file changed, 36 insertions(+), 20 deletions(-)

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 25a85b65b2..4594461de5 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -418,50 +418,66 @@ static const struct feat_props l2_cat_props = {
 .write_msr = l2_cat_write_msr,
 };
 
-static void __init parse_psr_bool(char *s, char *value, char *feature,
+static bool __init parse_psr_bool(const char *s, const char *value,
+  const char *ss, const char *feature,
   unsigned int mask)
 {
-if ( !strcmp(s, feature) )
+if ( !strncmp(s, feature, value - s) )
 {
-if ( !value )
+if ( !*value )
 opt_psr |= mask;
 else
 {
-int val_int = parse_bool(value, NULL);
+int val_int = parse_bool(value + 1, ss);
 
 if ( val_int == 0 )
 opt_psr &= ~mask;
 else if ( val_int == 1 )
 opt_psr |= mask;
+else
+return false;
 }
+return true;
 }
+return false;
 }
 
-static void __init parse_psr_param(char *s)
+static int __init parse_psr_param(const char *s)
 {
-char *ss, *val_str;
+const char *ss, *val_str;
+const char *q;
+int rc = 0;
 
 do {
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
+if ( !ss )
+ss = strchr(s, '\0');
 
 val_str = strchr(s, ':');
-if ( val_str )
-*val_str++ = '\0';
-
-parse_psr_bool(s, val_str, "cmt", PSR_CMT);
-parse_psr_bool(s, val_str, "cat", PSR_CAT);
-parse_psr_bool(s, val_str, "cdp", PSR_CDP);
+if ( !val_str )
+val_str = strchr(s, '\0');
 
-if ( val_str && !strcmp(s, "rmid_max") )
-opt_rmid_max = simple_strtoul(val_str, NULL, 0);
-
-if ( val_str && !strcmp(s, "cos_max") )
-opt_cos_max = simple_strtoul(val_str, NULL, 0);
+if ( *val_str && !strncmp(s, "rmid_max", val_str - s) )
+{
+opt_rmid_max = simple_strtoul(val_str + 1, , 0);
+if ( *q && *q != ',' )
+rc = -EINVAL;
+}
+else if ( *val_str && !strncmp(s, "cos_max", val_str - s) )
+{
+opt_cos_max = simple_strtoul(val_str + 1, , 0);
+if ( *q && *q != ',' )
+rc = -EINVAL;
+}
+else if ( !parse_psr_bool(s, val_str, ss, "cmt", PSR_CMT) &&
+  !parse_psr_bool(s, val_str, ss, "cat", PSR_CAT) &&
+  !parse_psr_bool(s, val_str, ss, "cdp", PSR_CDP) )
+rc = -EINVAL;
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 custom_param("psr", parse_psr_param);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 17/53] xen/arch/x86/numa.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/numa.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/numa.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
index ffeba6e180..32914bce27 100644
--- a/xen/arch/x86/numa.c
+++ b/xen/arch/x86/numa.c
@@ -18,7 +18,7 @@
 #include 
 #include 
 
-static int numa_setup(char *s);
+static int numa_setup(const char *s);
 custom_param("numa", numa_setup);
 
 #ifndef Dprintk
@@ -291,14 +291,14 @@ void numa_set_node(int cpu, nodeid_t node)
 }
 
 /* [numa=off] */
-static __init int numa_setup(char *opt) 
-{ 
+static __init int numa_setup(const char *opt)
+{
 if ( !strncmp(opt,"off",3) )
 numa_off = true;
-if ( !strncmp(opt,"on",2) )
+else if ( !strncmp(opt,"on",2) )
 numa_off = false;
 #ifdef CONFIG_NUMA_EMU
-if ( !strncmp(opt, "fake=", 5) )
+else if ( !strncmp(opt, "fake=", 5) )
 {
 numa_off = false;
 numa_fake = simple_strtoul(opt+5,NULL,0);
@@ -307,14 +307,16 @@ static __init int numa_setup(char *opt)
 }
 #endif
 #ifdef CONFIG_ACPI_NUMA
-if ( !strncmp(opt,"noacpi",6) )
+else if ( !strncmp(opt,"noacpi",6) )
 {
 numa_off = false;
 acpi_numa = -1;
 }
 #endif
+else
+return -EINVAL;
 
-return 1;
+return 0;
 } 
 
 /*
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 37/53] xen/drivers/passthrough/vtd/quirks.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/passthrough/vtd/quirks.c

to indicate whether the parameter value was parsed successfully.

Cc: Kevin Tian <kevin.t...@intel.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
---
 xen/drivers/passthrough/vtd/quirks.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/quirks.c 
b/xen/drivers/passthrough/vtd/quirks.c
index d6dd671dbf..d6db862678 100644
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -247,9 +247,10 @@ void vtd_ops_postamble_quirk(struct iommu* iommu)
 }
 }
 
-static void __init parse_snb_timeout(const char *s)
+static int __init parse_snb_timeout(const char *s)
 {
 int t;
+const char *q = NULL;
 
 t = parse_bool(s, NULL);
 if ( t < 0 )
@@ -259,13 +260,13 @@ static void __init parse_snb_timeout(const char *s)
 else if ( strcmp(s, "cap") == 0 )
 t = SNB_IGD_TIMEOUT;
 else
-t = strtoul(s, NULL, 0);
+t = strtoul(s, , 0);
 }
 else
 t = t ? SNB_IGD_TIMEOUT_LEGACY : 0;
 snb_igd_timeout = MILLISECS(t);
 
-return;
+return (q && *q) ? -EINVAL : 0;
 }
 custom_param("snb_igd_quirk", parse_snb_timeout);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 32/53] xen/drivers/cpufreq/cpufreq.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/cpufreq/cpufreq.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
V4:
- add __initdata (Jan Beulich)
- adapt parse_bool() call

V3:
- dont modify option value in handling function
- remove prototype of cpufreq_cmdline_parse() from cpufreq.h and make
  it static
---
 xen/drivers/cpufreq/cpufreq.c  | 39 +-
 xen/include/acpi/cpufreq/cpufreq.h |  2 --
 2 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c
index 5580bd370d..212f48f9f4 100644
--- a/xen/drivers/cpufreq/cpufreq.c
+++ b/xen/drivers/cpufreq/cpufreq.c
@@ -62,37 +62,41 @@ LIST_HEAD_READ_MOSTLY(cpufreq_governor_list);
 /* set xen as default cpufreq */
 enum cpufreq_controller cpufreq_controller = FREQCTL_xen;
 
-static void __init setup_cpufreq_option(char *str)
+static int __init cpufreq_cmdline_parse(const char *s);
+
+static int __init setup_cpufreq_option(const char *str)
 {
-char *arg = strpbrk(str, ",:");
+const char *arg = strpbrk(str, ",:");
 int choice;
 
-if ( arg )
-*arg++ = '\0';
-choice = parse_bool(str, NULL);
+if ( !arg )
+arg = strchr(str, '\0');
+choice = parse_bool(str, arg);
 
-if ( choice < 0 && !strcmp(str, "dom0-kernel") )
+if ( choice < 0 && !strncmp(str, "dom0-kernel", arg - str) )
 {
 xen_processor_pmbits &= ~XEN_PROCESSOR_PM_PX;
 cpufreq_controller = FREQCTL_dom0_kernel;
 opt_dom0_vcpus_pin = 1;
-return;
+return 0;
 }
 
-if ( choice == 0 || !strcmp(str, "none") )
+if ( choice == 0 || !strncmp(str, "none", arg - str) )
 {
 xen_processor_pmbits &= ~XEN_PROCESSOR_PM_PX;
 cpufreq_controller = FREQCTL_none;
-return;
+return 0;
 }
 
-if ( choice > 0 || !strcmp(str, "xen") )
+if ( choice > 0 || !strncmp(str, "xen", arg - str) )
 {
 xen_processor_pmbits |= XEN_PROCESSOR_PM_PX;
 cpufreq_controller = FREQCTL_xen;
-if ( arg && *arg )
-cpufreq_cmdline_parse(arg);
+if ( *arg && *(arg + 1) )
+return cpufreq_cmdline_parse(arg + 1);
 }
+
+return (choice < 0) ? -EINVAL : 0;
 }
 custom_param("cpufreq", setup_cpufreq_option);
 
@@ -571,7 +575,7 @@ static int __init cpufreq_handle_common_option(const char 
*name, const char *val
 return 0;
 }
 
-void __init cpufreq_cmdline_parse(char *str)
+static int __init cpufreq_cmdline_parse(const char *s)
 {
 static struct cpufreq_governor *__initdata cpufreq_governors[] =
 {
@@ -581,8 +585,12 @@ void __init cpufreq_cmdline_parse(char *str)
 _gov_performance,
 _gov_powersave
 };
+static char __initdata buf[128];
+char *str = buf;
 unsigned int gov_index = 0;
+int rc = 0;
 
+strlcpy(buf, s, sizeof(buf));
 do {
 char *val, *end = strchr(str, ',');
 unsigned int i;
@@ -611,11 +619,16 @@ void __init cpufreq_cmdline_parse(char *str)
 if (str && !cpufreq_handle_common_option(str, val) &&
 (!cpufreq_governors[gov_index]->handle_option ||
  !cpufreq_governors[gov_index]->handle_option(str, val)))
+{
 printk(XENLOG_WARNING "cpufreq/%s: option '%s' not recognized\n",
cpufreq_governors[gov_index]->name, str);
+rc = -EINVAL;
+}
 
 str = end;
 } while (str);
+
+return rc;
 }
 
 static int cpu_callback(
diff --git a/xen/include/acpi/cpufreq/cpufreq.h 
b/xen/include/acpi/cpufreq/cpufreq.h
index 48ad1d0004..a5cd7d08a1 100644
--- a/xen/include/acpi/cpufreq/cpufreq.h
+++ b/xen/include/acpi/cpufreq/cpufreq.h
@@ -79,8 +79,6 @@ DECLARE_PER_CPU(struct cpufreq_policy *, cpufreq_cpu_policy);
 extern int __cpufreq_set_policy(struct cpufreq_policy *data,
 struct cpufreq_policy *policy);
 
-void cpufreq_cmdline_parse(char *);
-
 #define CPUFREQ_SHARED_TYPE_NONE (0) /* None */
 #define CPUFREQ_SHARED_TYPE_HW   (1) /* HW does needed coordination */
 #define CPUFREQ_SHARED_TYPE_ALL  (2) /* All dependent CPUs should set freq */
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 53/53] xen: make some console related parameters settable at runtime

[Juergen Gross]

Support modifying conswitch, console_timestamps, loglvl and
guest_loglvl at runtime.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
---
V3:
- *_runtime_param() now includes boot parameter setting (Jan Beulich)
---
 docs/misc/xen-command-line.markdown |  8 
 xen/drivers/char/console.c  | 18 +-
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index 4002eab08b..9797c8db2d 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -391,6 +391,8 @@ makes sense on its own.
 
 > Default: `none`
 
+> Can be modified at runtime
+
 Specify which timestamp format Xen should use for each console line.
 
 * `none`: No timestamps
@@ -417,6 +419,8 @@ into the console ring buffer.
 
 > Default: `conswitch=a`
 
+> Can be modified at runtime
+
 Specify which character should be used to switch serial input between
 Xen and dom0.  The required sequence is CTRL-switch char three
 times.
@@ -898,6 +902,8 @@ maximum number of maptrack frames domain.
 
 > Default: `guest_loglvl=none/warning`
 
+> Can be modified at runtime
+
 Set the logging level for Xen guests.  Any log message with equal more
 more importance will be printed.
 
@@ -1164,6 +1170,8 @@ if left disabled by the BIOS.
 
 > Default: `loglvl=warning`
 
+> Can be modified at runtime
+
 Set the logging level for Xen.  Any log message with equal more more
 importance will be printed.
 
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 36f94e6b94..19d0e74f17 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -40,7 +40,7 @@ string_param("console", opt_console);
 /* Char 2: If this character is 'x', then do not auto-switch to DOM0 when it */
 /* boots. Any other value, or omitting the char, enables auto-switch */
 static unsigned char __read_mostly opt_conswitch[3] = "a";
-string_param("conswitch", opt_conswitch);
+string_runtime_param("conswitch", opt_conswitch);
 
 /* sync_console: force synchronous console output (useful for debugging). */
 static bool_t __initdata opt_sync_console;
@@ -68,7 +68,7 @@ enum con_timestamp_mode
 static enum con_timestamp_mode __read_mostly opt_con_timestamp_mode = TSM_NONE;
 
 static int parse_console_timestamps(const char *s);
-custom_param("console_timestamps", parse_console_timestamps);
+custom_runtime_param("console_timestamps", parse_console_timestamps);
 
 /* conring_size: allows a large console ring than default (16kB). */
 static uint32_t __initdata opt_conring_size;
@@ -134,8 +134,8 @@ static int parse_guest_loglvl(const char *s);
  * Similar definitions for guest_loglvl, but applies to guest tracing.
  * Defaults: loglvl=warning ; guest_loglvl=none/warning
  */
-custom_param("loglvl", parse_loglvl);
-custom_param("guest_loglvl", parse_guest_loglvl);
+custom_runtime_param("loglvl", parse_loglvl);
+custom_runtime_param("guest_loglvl", parse_guest_loglvl);
 
 static atomic_t print_everything = ATOMIC_INIT(0);
 
@@ -145,7 +145,7 @@ static atomic_t print_everything = ATOMIC_INIT(0);
 return (lvlnum);\
 }
 
-static int __init __parse_loglvl(const char *s, const char **ps)
+static int __parse_loglvl(const char *s, const char **ps)
 {
 ___parse_loglvl(s, ps, "none",0);
 ___parse_loglvl(s, ps, "error",   1);
@@ -156,7 +156,7 @@ static int __init __parse_loglvl(const char *s, const char 
**ps)
 return 2; /* sane fallback */
 }
 
-static int __init _parse_loglvl(const char *s, int *lower, int *upper)
+static int _parse_loglvl(const char *s, int *lower, int *upper)
 {
 *lower = *upper = __parse_loglvl(s, );
 if ( *s == '/' )
@@ -167,12 +167,12 @@ static int __init _parse_loglvl(const char *s, int 
*lower, int *upper)
 return *s ? -EINVAL : 0;
 }
 
-static int __init parse_loglvl(const char *s)
+static int parse_loglvl(const char *s)
 {
 return _parse_loglvl(s, _lower_thresh, _upper_thresh);
 }
 
-static int __init parse_guest_loglvl(const char *s)
+static int parse_guest_loglvl(const char *s)
 {
 return _parse_loglvl(s, _guest_lower_thresh,
  _guest_upper_thresh);
@@ -606,7 +606,7 @@ static int printk_prefix_check(char *p, char **pp)
 ((loglvl <

[Xen-devel] [PATCH v4 47/53] xen: carve out a generic parsing function from _cmdline_parse()

[Juergen Gross]

In order to support generic parameter parsing carve out the parser from
_cmdline_parse(). As this generic function might be called after boot
remove the __init annotations from all called sub-functions.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Reviewed-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/common/kernel.c | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 063ac99bff..65cd3a92ba 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -23,8 +23,7 @@ enum system_state system_state = SYS_STATE_early_boot;
 xen_commandline_t saved_cmdline;
 static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
 
-static int __init assign_integer_param(
-const struct kernel_param *param, uint64_t val)
+static int assign_integer_param(const struct kernel_param *param, uint64_t val)
 {
 switch ( param->len )
 {
@@ -53,12 +52,13 @@ static int __init assign_integer_param(
 return 0;
 }
 
-static void __init _cmdline_parse(const char *cmdline)
+static int parse_params(const char *cmdline, const struct kernel_param *start,
+const struct kernel_param *end)
 {
 char opt[128], *optval, *optkey, *q;
 const char *p = cmdline, *key;
 const struct kernel_param *param;
-int rc;
+int rc, final_rc = 0;
 bool bool_assert, found;
 
 for ( ; ; )
@@ -100,7 +100,7 @@ static void __init _cmdline_parse(const char *cmdline)
 
 rc = 0;
 found = false;
-for ( param = __setup_start; param < __setup_end; param++ )
+for ( param = start; param < end; param++ )
 {
 int rctmp;
 const char *s;
@@ -172,11 +172,24 @@ static void __init _cmdline_parse(const char *cmdline)
 }
 
 if ( rc )
+{
 printk("parameter \"%s\" has invalid value \"%s\", rc=%d!\n",
 key, optval, rc);
+final_rc = rc;
+}
 if ( !found )
+{
 printk("parameter \"%s\" unknown!\n", key);
+final_rc = -EINVAL;
+}
 }
+
+return final_rc;
+}
+
+static void __init _cmdline_parse(const char *cmdline)
+{
+parse_params(cmdline, __setup_start, __setup_end);
 }
 
 /**
@@ -201,7 +214,7 @@ void __init cmdline_parse(const char *cmdline)
 #endif
 }
 
-int __init parse_bool(const char *s, const char *e)
+int parse_bool(const char *s, const char *e)
 {
 unsigned int len;
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 41/53] xen/arch/x86/apic.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by apic_set_verbosity() can be removed.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/apic.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 325b48074a..2638414e08 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -800,11 +800,8 @@ static int __init apic_set_verbosity(const char *str)
 apic_verbosity = APIC_DEBUG;
 else if (strcmp("verbose", str) == 0)
 apic_verbosity = APIC_VERBOSE;
-else {
-printk(KERN_WARNING "APIC Verbosity level %s not recognised"
-   " use apic_verbosity=verbose or apic_verbosity=debug", str);
+else
 return -EINVAL;
-}
 
 return 0;
 }
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 02/53] xen/arch/arm/acpi/boot.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/arm/acpi/boot.c

to indicate whether the parameter value was parsed successfully.

Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Julien Grall <julien.gr...@arm.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Julien Grall <julien.gr...@arm.com>
---
 xen/arch/arm/acpi/boot.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
index a5a6f55f0e..6101bf39c9 100644
--- a/xen/arch/arm/acpi/boot.c
+++ b/xen/arch/arm/acpi/boot.c
@@ -193,16 +193,20 @@ static int __init acpi_parse_fadt(struct 
acpi_table_header *table)
 static bool_t __initdata param_acpi_off;
 static bool_t __initdata param_acpi_force;
 
-static void __init parse_acpi_param(char *arg)
+static int __init parse_acpi_param(const char *arg)
 {
 if ( !arg )
-return;
+return -EINVAL;
 
 /* Interpret the parameter for use within Xen. */
 if ( !parse_bool(arg, NULL) )
 param_acpi_off = true;
 else if ( !strcmp(arg, "force") ) /* force ACPI to be enabled */
 param_acpi_force = true;
+else
+return -EINVAL;
+
+return 0;
 }
 custom_param("acpi", parse_acpi_param);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 03/53] xen/arch/arm/domain_build.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/arm/domain_build.c

to indicate whether the parameter value was parsed successfully.

Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Julien Grall <julien.gr...@arm.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Julien Grall <julien.gr...@arm.com>
---
 xen/arch/arm/domain_build.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 1bec4fa23d..d6f9585503 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -33,9 +33,11 @@ int dom0_11_mapping = 1;
 
 static u64 __initdata dom0_mem;
 
-static void __init parse_dom0_mem(const char *s)
+static int __init parse_dom0_mem(const char *s)
 {
 dom0_mem = parse_size_and_unit(s, );
+
+return *s ? -EINVAL : 0;
 }
 custom_param("dom0_mem", parse_dom0_mem);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 00/53] Support for modifying parameters at runtime

[Juergen Gross]

Currently parameters of the hypervisor (e.g. console log level) can be
set via boot command line. Instead of having to reboot the system in
case another setting is desired, being able to modify many of those
parameters at runtime would be the better option.

This patch series addresses this by adding a new xl command
"xl set-parameters" which takes a string similar to the boot command
line as parameter and passes this string to the hypervisor which will
then use the same parsing infrastructure as for the command line in
order to apply the parameter settings.

As error checks for invalid parameters or parameter values have been
very sparse if present at all in the hypervisor, a major part of this
patch series addresses this problem first: all custom parameter parsing
functions are being changed to return success or an error. The main
parsing function tests for generic parameter value errors (like e.g.
overflow) or invalid parameters and issues a message in case an error
has been detected. Most error messages in the custom parsing functions
are removed then.

While not strictly required for runtime parameter modification I
believe an improved parameter validation is a win with or without the
runtime parameter modification support.

* Patch 1 prepares the following patches by adding an end pointer to
  parse_bool()
* Patches 2-39 are modifying the custom parameter parsing functions to
  return success or error
* Patch 40 adds generic parameter error tests in the core parameter
  parsing functions
* Patches 41-46 remove custom parsing function error messages
* Patches 47-52 add the runtime parameter modification support
* Patch 53 adds support for runtime modification of some console related
  parameters 

Changes in V4:
- added new patch 01
- patches 19, 23, 32, 34: pass end of parameter value to parse_bool()
(Jan Beulich)
- patch 21: issue a message in case of reboot=efi and no EFI runtime services
(Jan Beulich)
- patch 23: style correction (Jan Beulich)
- patch 32: add __initdata (Jan Beulich)
- patch 35: differentiate error return values (Jan Beulich)
- patch 35: drop pointless test for input parameter being NULL (Jan Beulich)
- patch 40: removed test for empty string from parse_bool() again, avoid
calling parse_bool() in that case (Jan Beulich)
- patch 40: print error number in failure case (Jan Beulich)
- patch 40: limit scope of some variables (Jan Beulich)
- patch 40: use type bool for bool_assert (Jan Beulich)
- patch 49: use copy_from_guest() (Jan Beulich)
- patch 49: add more descriptive comment to sysctl.h (Jan Beulich)

Changes in V3:
- patches 7, 14, 15, 24, 27, 34: cosmetic changes (Jan Beulich)
- patches 7, 9, 10, 18, 22, 25, 31, 33, 34: dont modify option value in
parsing function
- patches 8, 22, 25: dont return out of loop (Jan Beulich)
- patch 9: fix error in parsing routine
- patch 18: let parse_psr_bool() return bool value (Jan Beulich)
- patch 18: return error in case no string matches (Jan Beulich)
- patch 20: dont stop loop at first invalid character (Jan Beulich)
- patch 31: remove prototype of cpufreq_cmdline_parse() from cpufreq.h
and make it static
- patch 39: use function pointer in struct kernel_param (Jan Beulich)
- patch 39: better range check in assign_integer_param() (Jan Beulich)
- patch 39: dont assign int values in case of overflow (Jan Beulich)
- patch 39: allow multiple handlers for a parameter (Jan Beulich)
- patch 47: moved runtime parameter array in linker scripts (Jan Beulich)
- patch 47: renamed macros to *_runtime_param() (Jan Beulich)
- patch 47: added *runtime_only_param() macros (Jan Beulich)
- patch 47: let *_runtime_param() macros include boot param functionality
(Jan Beulich)
- patch 48: check pad[] to be zero (Jan Beulich)
- patch 48: return E2BIG in case of parameters too long (Jan Beulich)
- patch 48: move max. parameter size define to sysctl.c (Jan Beulich)
- patch 49: zero padding fields in sysctl.u.set_parameter
- patch 50: use LOGEV() for error message
- patch 52: *_runtime_param() now includes boot parameter setting
(Jan Beulich)

Changes in V2:
- patch 39 (xen: check parameter validity when parsing command line):
  replaced literal 8 by BITS_PER_BYTE (Wei Liu)
  added test for empty string to parse_bool()
- patch 47 (xen: add basic support for runtime parameter changing):
  added modification of ARM linker script (Wei Liu)
- patch 48 (xen: add hypercall for setting parameters at runtime):
  corrected XSM test (Daniel De Graaf)
- patch 50 (libxl: add libxl_set_parameters() function):
  corrected coding style (Wei Liu)


Juergen Gross (53):
  xen: add an optional string end parameter to parse_bool()
  xen/arch/arm/acpi/boot.c: let custom parameter parsing routines return
errno
  xen/arch/arm/domain_build.c: let custom parameter parsing routines
return errno
  xen/arch/arm/traps.c: let custom parameter parsing routines return
errno
  xen/arch/x86/apic.c: let custom parameter parsing routines return

[Xen-devel] [PATCH v4 15/53] xen/arch/x86/mm.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/mm.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
V3:
- cosmetic changes (Jan Beulich)
---
 xen/arch/x86/mm.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 5b0e55d7d9..3052cb0788 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -179,14 +179,20 @@ static uint32_t base_disallow_mask;
  L1_DISALLOW_MASK : (L1_DISALLOW_MASK & ~PAGE_CACHE_ATTRS))
 
 static s8 __read_mostly opt_mmio_relax;
-static void __init parse_mmio_relax(const char *s)
+
+static int __init parse_mmio_relax(const char *s)
 {
 if ( !*s )
 opt_mmio_relax = 1;
 else
 opt_mmio_relax = parse_bool(s, NULL);
 if ( opt_mmio_relax < 0 && strcmp(s, "all") )
+{
 opt_mmio_relax = 0;
+return -EINVAL;
+}
+
+return 0;
 }
 custom_param("mmio-relax", parse_mmio_relax);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 22/53] xen/arch/x86/time.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/time.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/time.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index b988b94d2e..eba7aed72d 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1870,7 +1870,7 @@ int hwdom_pit_access(struct ioreq *ioreq)
  * tsc=skewed: Assume TSCs are individually reliable, but skewed across CPUs.
  * tsc=stable:socket: Assume TSCs are reliable across sockets.
  */
-static void __init tsc_parse(const char *s)
+static int __init tsc_parse(const char *s)
 {
 if ( !strcmp(s, "unstable") )
 {
@@ -1882,6 +1882,10 @@ static void __init tsc_parse(const char *s)
 setup_clear_cpu_cap(X86_FEATURE_TSC_RELIABLE);
 else if ( !strcmp(s, "stable:socket") )
 tsc_flags |= TSC_RELIABLE_SOCKET;
+else
+return -EINVAL;
+
+return 0;
 }
 custom_param("tsc", tsc_parse);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 44/53] xen/arch/x86/io_apic.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by setup_ioapic_ack() can be removed.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/io_apic.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index f767c4560c..f959090ca0 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -1594,10 +1594,7 @@ static int __init setup_ioapic_ack(const char *s)
 ioapic_ack_forced = true;
 }
 else
-{
-printk("Unknown ioapic_ack value specified: '%s'\n", s);
 return -EINVAL;
-}
 
 return 0;
 }
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 52/53] xl: add new xl command set-parameters

[Juergen Gross]

Add a new xl command "set-parameters" to set hypervisor parameters at
runtime similar to boot time parameters via command line.

Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
---
 docs/man/xl.pod.1.in   |  5 +
 tools/xl/xl.h  |  1 +
 tools/xl/xl_cmdtable.c |  5 +
 tools/xl/xl_misc.c | 20 
 4 files changed, 31 insertions(+)

diff --git a/docs/man/xl.pod.1.in b/docs/man/xl.pod.1.in
index 16c83066fe..3d5f2f7359 100644
--- a/docs/man/xl.pod.1.in
+++ b/docs/man/xl.pod.1.in
@@ -822,6 +822,11 @@ Pass the VNC password to vncviewer via stdin.
 Send debug I to Xen. It is the same as pressing the Xen
 "conswitch" (Ctrl-A by default) three times and then pressing "keys".
 
+=item B I
+
+Set hypervisor parameters as specified in I. This allows for some
+boot parameters of the hypervisor to be modified in the running systems.
+
 =item B [I]
 
 Reads the Xen message buffer, similar to dmesg on a Linux system.  The
diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index aa95b77146..5d3d2a4835 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -154,6 +154,7 @@ int main_rename(int argc, char **argv);
 int main_trigger(int argc, char **argv);
 int main_sysrq(int argc, char **argv);
 int main_debug_keys(int argc, char **argv);
+int main_set_parameters(int argc, char **argv);
 int main_dmesg(int argc, char **argv);
 int main_top(int argc, char **argv);
 int main_networkattach(int argc, char **argv);
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 2c71a9f776..ba0159df67 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -309,6 +309,11 @@ struct cmd_spec cmd_table[] = {
   "Send debug keys to Xen",
   "",
 },
+{ "set-parameters",
+  _set_parameters, 0, 1,
+  "Set hypervisor parameters",
+  "",
+},
 { "dmesg",
   _dmesg, 0, 0,
   "Read and/or clear dmesg buffer",
diff --git a/tools/xl/xl_misc.c b/tools/xl/xl_misc.c
index 9c6227af23..dcf940a6d4 100644
--- a/tools/xl/xl_misc.c
+++ b/tools/xl/xl_misc.c
@@ -155,6 +155,26 @@ int main_debug_keys(int argc, char **argv)
 return EXIT_SUCCESS;
 }
 
+int main_set_parameters(int argc, char **argv)
+{
+int opt;
+char *params;
+
+SWITCH_FOREACH_OPT(opt, "", NULL, "set-parameters", 1) {
+/* No options */
+}
+
+params = argv[optind];
+
+if (libxl_set_parameters(ctx, params)) {
+fprintf(stderr, "cannot set parameters: %s\n", params);
+fprintf(stderr, "Use \"xl dmesg\" to look for possible reason.\n");
+return EXIT_FAILURE;
+}
+
+return EXIT_SUCCESS;
+}
+
 int main_devd(int argc, char **argv)
 {
 int ret = 0, opt = 0, daemonize = 1;
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 25/53] xen/common/domain.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/domain.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
V3:
- cosmetic changes (Jan Beulich)
---
 xen/common/domain.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index b22aacc57e..acd420a1a5 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -245,12 +245,15 @@ static int late_hwdom_init(struct domain *d)
 
 static unsigned int __read_mostly extra_hwdom_irqs;
 static unsigned int __read_mostly extra_domU_irqs = 32;
-static void __init parse_extra_guest_irqs(const char *s)
+
+static int __init parse_extra_guest_irqs(const char *s)
 {
 if ( isdigit(*s) )
 extra_domU_irqs = simple_strtoul(s, , 0);
 if ( *s == ',' && isdigit(*++s) )
 extra_hwdom_irqs = simple_strtoul(s, , 0);
+
+return *s ? -EINVAL : 0;
 }
 custom_param("extra_guest_irqs", parse_extra_guest_irqs);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 04/53] xen/arch/arm/traps.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/arm/traps.c

to indicate whether the parameter value was parsed successfully.

Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Julien Grall <julien.gr...@arm.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Julien Grall <julien.gr...@arm.com>
---
 xen/arch/arm/traps.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 2e92223c2f..aa838e8e77 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -108,12 +108,14 @@ static enum {
NATIVE,
 } vwfi;
 
-static void __init parse_vwfi(const char *s)
+static int __init parse_vwfi(const char *s)
 {
if ( !strcmp(s, "native") )
vwfi = NATIVE;
else
vwfi = TRAP;
+
+   return 0;
 }
 custom_param("vwfi", parse_vwfi);
 
@@ -130,7 +132,7 @@ static enum {
 SERRORS_PANIC,
 } serrors_op;
 
-static void __init parse_serrors_behavior(const char *str)
+static int __init parse_serrors_behavior(const char *str)
 {
 if ( !strcmp(str, "forward") )
 serrors_op = SERRORS_FORWARD;
@@ -139,7 +141,7 @@ static void __init parse_serrors_behavior(const char *str)
 else
 serrors_op = SERRORS_DIVERSE;
 
-return;
+return 0;
 }
 custom_param("serrors", parse_serrors_behavior);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 18/53] xen/arch/x86/oprofile/nmi_int.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/oprofile/nmi_int.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/oprofile/nmi_int.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/xen/arch/x86/oprofile/nmi_int.c b/xen/arch/x86/oprofile/nmi_int.c
index 126f7a8d9f..84ba108fdb 100644
--- a/xen/arch/x86/oprofile/nmi_int.c
+++ b/xen/arch/x86/oprofile/nmi_int.c
@@ -323,12 +323,15 @@ static int __init p4_init(char ** cpu_type)
 
 
 static int force_arch_perfmon;
+
 static int force_cpu_type(const char *str)
 {
if (!strcmp(str, "arch_perfmon")) {
force_arch_perfmon = 1;
printk(KERN_INFO "oprofile: forcing architectural perfmon\n");
}
+   else
+   return -EINVAL;
 
return 0;
 }
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 12/53] xen/arch/x86/io_apic.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/io_apic.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/io_apic.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index 2838f6bd99..f767c4560c 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -1581,7 +1581,7 @@ static unsigned int startup_level_ioapic_irq(struct 
irq_desc *desc)
 return 0; /* don't check for pending */
 }
 
-static void __init setup_ioapic_ack(char *s)
+static int __init setup_ioapic_ack(const char *s)
 {
 if ( !strcmp(s, "old") )
 {
@@ -1594,7 +1594,12 @@ static void __init setup_ioapic_ack(char *s)
 ioapic_ack_forced = true;
 }
 else
+{
 printk("Unknown ioapic_ack value specified: '%s'\n", s);
+return -EINVAL;
+}
+
+return 0;
 }
 custom_param("ioapic_ack", setup_ioapic_ack);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 46/53] xen/common/sched_credit2.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by parse_credit2_runqueue() can be removed.

Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Dario Faggioli <dario.faggi...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Dario Faggioli <dario.faggi...@citrix.com>
---
 xen/common/sched_credit2.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 9b1db1351f..2da9cc2ffb 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -352,8 +352,6 @@ static int parse_credit2_runqueue(const char *s)
 }
 }
 
-printk("WARNING, unrecognized value of credit2_runqueue option!\n");
-
 return -EINVAL;
 }
 custom_param("credit2_runqueue", parse_credit2_runqueue);
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 48/53] xen: add basic support for runtime parameter changing

[Juergen Gross]

Add the needed infrastructure for runtime parameter changing similar
to that used at boot time via cmdline. We are using the same parsing
functions as for cmdline parsing, but with a different array of
parameter definitions.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
---
V2:
- added modification of ARM linker script (Wei Liu)

V3:
- moved runtime parameter array in linker scripts (Jan Beulich)
- renamed macros to *_runtime_param() (Jan Beulich)
- added *runtime_only_param() macros (Jan Beulich)
- let *_runtime_param() macros include boot param functionality
  (Jan Beulich)
---
 xen/arch/arm/xen.lds.S |  4 
 xen/arch/x86/xen.lds.S |  4 
 xen/common/kernel.c|  5 +
 xen/include/xen/init.h | 57 --
 xen/include/xen/lib.h  |  1 +
 5 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 2d54f224ec..c9b9546435 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -61,6 +61,10 @@ SECTIONS
*(.lockprofile.data)
__lock_profile_end = .;
 #endif
+   . = ALIGN(POINTER_ALIGN);
+   __param_start = .;
+   *(.data.param)
+   __param_end = .;
   } :text
 
 #if defined(BUILD_ID)
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index ff08bbe42a..6a7bbb8ca1 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -120,6 +120,10 @@ SECTIONS
*(.lockprofile.data)
__lock_profile_end = .;
 #endif
+   . = ALIGN(POINTER_ALIGN);
+   __param_start = .;
+   *(.data.param)
+   __param_end = .;
   } :text
 
 #if defined(BUILD_ID)
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 65cd3a92ba..da1a709146 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -192,6 +192,11 @@ static void __init _cmdline_parse(const char *cmdline)
 parse_params(cmdline, __setup_start, __setup_end);
 }
 
+int runtime_parse(const char *line)
+{
+return parse_params(line, __param_start, __param_end);
+}
+
 /**
  *cmdline_parse -- parses the xen command line.
  * If CONFIG_CMDLINE is set, it would be parsed prior to @cmdline.
diff --git a/xen/include/xen/init.h b/xen/include/xen/init.h
index 234ec25aae..db06c76fdf 100644
--- a/xen/include/xen/init.h
+++ b/xen/include/xen/init.h
@@ -90,11 +90,16 @@ struct kernel_param {
 };
 
 extern const struct kernel_param __setup_start[], __setup_end[];
+extern const struct kernel_param __param_start[], __param_end[];
+
+#define __dataparam   __used_section(".data.param")
+
+#define __param(att)  static const att \
+__attribute__((__aligned__(sizeof(void * struct kernel_param
 
 #define __setup_str static const __initconst \
 __attribute__((__aligned__(1))) char
-#define __kparam static const __initsetup \
-__attribute__((__aligned__(sizeof(void * struct kernel_param
+#define __kparam  __param(__initsetup)
 
 #define custom_param(_name, _var) \
 __setup_str __setup_str_##_var[] = _name; \
@@ -131,6 +136,54 @@ extern const struct kernel_param __setup_start[], 
__setup_end[];
   .len = sizeof(_var), \
   .par.var = &_var }
 
+#define __rtparam __param(__dataparam)
+
+#define custom_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+  { .name = _name, \
+  .type = OPT_CUSTOM, \
+  .par.func = _var }
+#define boolean_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+{ .name = _name, \
+  .type = OPT_BOOL, \
+  .len = sizeof(_var), \
+  .par.var = &_var }
+#define integer_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+{ .name = _name, \
+  .type = OPT_UINT, \
+  .len = sizeof(_var), \
+  .par.var = &_var }
+#define size_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+{ .name = _name, \
+  .type = OPT_SIZE, \
+  .len = sizeof(_var), \
+  .par.var = &_var }
+#define string_runtime_only_param(_name, _var) \
+__rtparam __rtpar_##_var = \
+{ .name = _name, \
+  .type = OPT_STR, \
+  .len = sizeof(_var), \
+  .par.var = &_var }
+
+#define custom_runtime_param(_name, _var) \
+custom_param(_name, _var); \
+custom_runtime_only_param(_name, _var)
+#define boolean_runtime_param(_name, _var) \
+boolean_param(_name, _var); \
+boolean_runtime_only_param(_name, _var)
+#define integer_runtime_param(_name, _var) 

[Xen-devel] [PATCH v4 16/53] xen/arch/x86/nmi.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/nmi.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
V3:
- cosmetic changes (Jan Beulich)
---
 xen/arch/x86/nmi.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
index e44f88045e..d7fce28805 100644
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -46,35 +46,44 @@ bool __initdata opt_watchdog;
 /* watchdog_force: If true, process unknown NMIs when running the watchdog. */
 bool watchdog_force;
 
-static void __init parse_watchdog(char *s)
+static int __init parse_watchdog(const char *s)
 {
 if ( !*s )
 {
 opt_watchdog = true;
-return;
+return 0;
 }
 
 switch ( parse_bool(s, NULL) )
 {
 case 0:
 opt_watchdog = false;
-return;
+return 0;
 case 1:
 opt_watchdog = true;
-return;
+return 0;
 }
 
 if ( !strcmp(s, "force") )
 watchdog_force = opt_watchdog = true;
+else
+return -EINVAL;
+
+return 0;
 }
 custom_param("watchdog", parse_watchdog);
 
 /* opt_watchdog_timeout: Number of seconds to wait before panic. */
 static unsigned int opt_watchdog_timeout = 5;
-static void parse_watchdog_timeout(char * s)
+
+static int parse_watchdog_timeout(const char *s)
 {
-opt_watchdog_timeout = simple_strtoull(s, NULL, 0);
+const char *q;
+
+opt_watchdog_timeout = simple_strtoull(s, , 0);
 opt_watchdog = !!opt_watchdog_timeout;
+
+return *q ? -EINVAL : 0;
 }
 custom_param("watchdog_timeout", parse_watchdog_timeout);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 50/53] libxc: add function to set hypervisor parameters

[Juergen Gross]

Add a new libxc function to set hypervisor parameters at runtime
similar to boot time parameters via command line.

Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
---
V3:
- zero padding fields in sysctl.u.set_parameter
---
 tools/libxc/include/xenctrl.h |  1 +
 tools/libxc/xc_misc.c | 21 +
 2 files changed, 22 insertions(+)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index c7710b8f36..ad5e6b3d77 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1210,6 +1210,7 @@ int xc_readconsolering(xc_interface *xch,
int clear, int incremental, uint32_t *pindex);
 
 int xc_send_debug_keys(xc_interface *xch, char *keys);
+int xc_set_parameters(xc_interface *xch, char *params);
 
 typedef xen_sysctl_physinfo_t xc_physinfo_t;
 typedef xen_sysctl_cputopo_t xc_cputopo_t;
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index 2303293c6c..7e15e904e3 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -187,6 +187,27 @@ int xc_send_debug_keys(xc_interface *xch, char *keys)
 return ret;
 }
 
+int xc_set_parameters(xc_interface *xch, char *params)
+{
+int ret, len = strlen(params);
+DECLARE_SYSCTL;
+DECLARE_HYPERCALL_BOUNCE(params, len, XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+if ( xc_hypercall_bounce_pre(xch, params) )
+return -1;
+
+sysctl.cmd = XEN_SYSCTL_set_parameter;
+set_xen_guest_handle(sysctl.u.set_parameter.params, params);
+sysctl.u.set_parameter.size = len;
+memset(sysctl.u.set_parameter.pad, 0, sizeof(sysctl.u.set_parameter.pad));
+
+ret = do_sysctl(xch, );
+
+xc_hypercall_bounce_post(xch, params);
+
+return ret;
+}
+
 int xc_physinfo(xc_interface *xch,
 xc_physinfo_t *put_info)
 {
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 06/53] xen/arch/x86/cpu/mcheck/mce.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/cpu/mcheck/mce.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/cpu/mcheck/mce.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 30525dd78b..8015dff8c4 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -62,13 +62,18 @@ struct mca_banks *mca_allbanks;
 #endif
 
 int mce_verbosity;
-static void __init mce_set_verbosity(char *str)
+static int __init mce_set_verbosity(const char *str)
 {
 if (strcmp("verbose", str) == 0)
 mce_verbosity = MCE_VERBOSE;
 else
+{
 printk(KERN_DEBUG "Machine Check verbosity level %s not recognised"
"use mce_verbosity=verbose", str);
+return -EINVAL;
+}
+
+return 0;
 }
 custom_param("mce_verbosity", mce_set_verbosity);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 43/53] xen/arch/x86/hvm/viridian.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by parse_viridian_version() can be removed.

Cc: Paul Durrant <paul.durr...@citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Reviewed-by: Paul Durrant <paul.durr...@citrix.com>
---
 xen/arch/x86/hvm/viridian.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c
index 2edf9d0b23..e0546f32fc 100644
--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -1104,7 +1104,7 @@ static int __init parse_viridian_version(const char *arg)
 {
 n[i] = simple_strtoul(arg, , 0);
 if ( e != t )
-goto fail;
+break;
 }
 
 i++;
@@ -1112,12 +1112,12 @@ static int __init parse_viridian_version(const char 
*arg)
 } while ( *t );
 
 if ( i != 3 )
-goto fail;
+return -EINVAL;
 
 if ( ((typeof(viridian_major))n[0] != n[0]) ||
  ((typeof(viridian_minor))n[1] != n[1]) ||
  ((typeof(viridian_build))n[2] != n[2]) )
-goto fail;
+return -EINVAL;
 
 viridian_major = n[0];
 viridian_minor = n[1];
@@ -1126,10 +1126,6 @@ static int __init parse_viridian_version(const char *arg)
 printk("viridian-version = %#x,%#x,%#x\n",
viridian_major, viridian_minor, viridian_build);
 return 0;
-
- fail:
-printk(XENLOG_WARNING "Invalid viridian-version, using default\n");
-return -EINVAL;
 }
 custom_param("viridian-version", parse_viridian_version);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 30/53] xen/drivers/acpi/tables.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/acpi/tables.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/drivers/acpi/tables.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/acpi/tables.c b/xen/drivers/acpi/tables.c
index dd2031f36a..b890b73901 100644
--- a/xen/drivers/acpi/tables.c
+++ b/xen/drivers/acpi/tables.c
@@ -456,14 +456,15 @@ int __init acpi_table_init(void)
return 0;
 }
 
-static int __init acpi_parse_apic_instance(char *str)
+static int __init acpi_parse_apic_instance(const char *str)
 {
+   const char *q;
 
-   acpi_apic_instance = simple_strtoul(str, NULL, 0);
+   acpi_apic_instance = simple_strtoul(str, , 0);
 
printk(KERN_NOTICE PREFIX "Shall use APIC/MADT table %d\n",
   acpi_apic_instance);
 
-   return 0;
+   return *q ? -EINVAL : 0;
 }
 custom_param("acpi_apic_instance", acpi_parse_apic_instance);
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 28/53] xen/common/memory.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/memory.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
V3:
- cosmetic changes (Jan Beulich)
---
 xen/common/memory.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/common/memory.c b/xen/common/memory.c
index b2066db07e..26da6050f6 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -55,7 +55,8 @@ static unsigned int __read_mostly hwdom_max_order = 
CONFIG_HWDOM_MAX_ORDER;
 #ifdef HAS_PASSTHROUGH
 static unsigned int __read_mostly ptdom_max_order = CONFIG_PTDOM_MAX_ORDER;
 #endif
-static void __init parse_max_order(const char *s)
+
+static int __init parse_max_order(const char *s)
 {
 if ( *s != ',' )
 domu_max_order = simple_strtoul(s, , 0);
@@ -67,6 +68,8 @@ static void __init parse_max_order(const char *s)
 if ( *s == ',' && *++s != ',' )
 ptdom_max_order = simple_strtoul(s, , 0);
 #endif
+
+return *s ? -EINVAL : 0;
 }
 custom_param("memop-max-order", parse_max_order);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 29/53] xen/common/sched_credit2.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/sched_credit2.c

to indicate whether the parameter value was parsed successfully.

Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Dario Faggioli <dario.faggi...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Dario Faggioli <dario.faggi...@citrix.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
---
 xen/common/sched_credit2.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 29c002a63e..9b1db1351f 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -339,7 +339,7 @@ static const char *const opt_runqueue_str[] = {
 };
 static int __read_mostly opt_runqueue = OPT_RUNQUEUE_SOCKET;
 
-static void parse_credit2_runqueue(const char *s)
+static int parse_credit2_runqueue(const char *s)
 {
 unsigned int i;
 
@@ -348,11 +348,13 @@ static void parse_credit2_runqueue(const char *s)
 if ( !strcmp(s, opt_runqueue_str[i]) )
 {
 opt_runqueue = i;
-return;
+return 0;
 }
 }
 
 printk("WARNING, unrecognized value of credit2_runqueue option!\n");
+
+return -EINVAL;
 }
 custom_param("credit2_runqueue", parse_credit2_runqueue);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 34/53] xen/drivers/passthrough/iommu.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/passthrough/iommu.c

to indicate whether the parameter value was parsed successfully.

This at once fixes a potential problem with multiple iommu=...
parameters: before the change iommu=no iommu=yes would switch iommu
off.

Cc: Jan Beulich <jbeul...@suse.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
V4:
- adapt parse_bool() call (Jan Beulich)

V3:
- dont modify option value in parsing function
---
 xen/drivers/passthrough/iommu.c | 52 +++--
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index f1aefc47ce..1aecf7cf34 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -21,7 +21,7 @@
 #include 
 #include 
 
-static void parse_iommu_param(char *s);
+static int parse_iommu_param(const char *s);
 static void iommu_dump_p2m_table(unsigned char key);
 
 unsigned int __read_mostly iommu_dev_iotlb_timeout = 1000;
@@ -78,10 +78,10 @@ DEFINE_SPINLOCK(iommu_pt_cleanup_lock);
 PAGE_LIST_HEAD(iommu_pt_cleanup_list);
 static struct tasklet iommu_pt_cleanup_tasklet;
 
-static void __init parse_iommu_param(char *s)
+static int __init parse_iommu_param(const char *s)
 {
-char *ss;
-int val;
+const char *ss;
+int val, b, rc = 0;
 
 do {
 val = !!strncmp(s, "no-", 3);
@@ -89,44 +89,50 @@ static void __init parse_iommu_param(char *s)
 s += 3;
 
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
-
-if ( !parse_bool(s, NULL) )
-iommu_enable = 0;
-else if ( !strcmp(s, "force") || !strcmp(s, "required") )
+if ( !ss )
+ss = strchr(s, '\0');
+
+b = parse_bool(s, ss);
+if ( b >= 0 )
+iommu_enable = b;
+else if ( !strncmp(s, "force", ss - s) ||
+  !strncmp(s, "required", ss - s) )
 force_iommu = val;
-else if ( !strcmp(s, "workaround_bios_bug") )
+else if ( !strncmp(s, "workaround_bios_bug", ss - s) )
 iommu_workaround_bios_bug = val;
-else if ( !strcmp(s, "igfx") )
+else if ( !strncmp(s, "igfx", ss - s) )
 iommu_igfx = val;
-else if ( !strcmp(s, "verbose") )
+else if ( !strncmp(s, "verbose", ss - s) )
 iommu_verbose = val;
-else if ( !strcmp(s, "snoop") )
+else if ( !strncmp(s, "snoop", ss - s) )
 iommu_snoop = val;
-else if ( !strcmp(s, "qinval") )
+else if ( !strncmp(s, "qinval", ss - s) )
 iommu_qinval = val;
-else if ( !strcmp(s, "intremap") )
+else if ( !strncmp(s, "intremap", ss - s) )
 iommu_intremap = val;
-else if ( !strcmp(s, "intpost") )
+else if ( !strncmp(s, "intpost", ss - s) )
 iommu_intpost = val;
-else if ( !strcmp(s, "debug") )
+else if ( !strncmp(s, "debug", ss - s) )
 {
 iommu_debug = val;
 if ( val )
 iommu_verbose = 1;
 }
-else if ( !strcmp(s, "amd-iommu-perdev-intremap") )
+else if ( !strncmp(s, "amd-iommu-perdev-intremap", ss - s) )
 amd_iommu_perdev_intremap = val;
-else if ( !strcmp(s, "dom0-passthrough") )
+else if ( !strncmp(s, "dom0-passthrough", ss - s) )
 iommu_passthrough = val;
-else if ( !strcmp(s, "dom0-strict") )
+else if ( !strncmp(s, "dom0-strict", ss - s) )
 iommu_dom0_strict = val;
-else if ( !strcmp(s, "sharept") )
+else if ( !strncmp(s, "sharept", ss - s) )
 iommu_hap_pt_share = val;
+else
+rc = -EINVAL;
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 
 int iommu_domain_init(struct domain *d)
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 08/53] xen/arch/x86/dom0_build.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/dom0_build.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
---
V3:
- cosmetic changes (Jan Beulich)
- dont modify option value in parsing function
---
 xen/arch/x86/dom0_build.c | 32 ++--
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 0c125e61eb..f616b99ddc 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -47,7 +47,8 @@ static long __init parse_amt(const char *s, const char **ps)
 long pages = parse_size_and_unit((*s == '-') ? s+1 : s, ps) >> PAGE_SHIFT;
 return (*s == '-') ? -pages : pages;
 }
-static void __init parse_dom0_mem(const char *s)
+
+static int __init parse_dom0_mem(const char *s)
 {
 do {
 if ( !strncmp(s, "min:", 4) )
@@ -57,13 +58,15 @@ static void __init parse_dom0_mem(const char *s)
 else
 dom0_nrpages = parse_amt(s, );
 } while ( *s++ == ',' );
+
+return *s ? -EINVAL : 0;
 }
 custom_param("dom0_mem", parse_dom0_mem);
 
 static unsigned int __initdata opt_dom0_max_vcpus_min = 1;
 static unsigned int __initdata opt_dom0_max_vcpus_max = UINT_MAX;
 
-static void __init parse_dom0_max_vcpus(const char *s)
+static int __init parse_dom0_max_vcpus(const char *s)
 {
 if ( *s == '-' )   /* -M */
 opt_dom0_max_vcpus_max = simple_strtoul(s + 1, , 0);
@@ -77,6 +80,8 @@ static void __init parse_dom0_max_vcpus(const char *s)
 else if ( *s++ == '-' && *s ) /* N-M */
 opt_dom0_max_vcpus_max = simple_strtoul(s, , 0);
 }
+
+return *s ? -EINVAL : 0;
 }
 custom_param("dom0_max_vcpus", parse_dom0_max_vcpus);
 
@@ -85,7 +90,7 @@ static __initdata unsigned int dom0_pxms[MAX_NUMNODES] =
 { [0 ... MAX_NUMNODES - 1] = ~0 };
 static __initdata bool dom0_affinity_relaxed;
 
-static void __init parse_dom0_nodes(const char *s)
+static int __init parse_dom0_nodes(const char *s)
 {
 do {
 if ( isdigit(*s) )
@@ -103,6 +108,8 @@ static void __init parse_dom0_nodes(const char *s)
 else
 break;
 } while ( ++dom0_nr_pxms < ARRAY_SIZE(dom0_pxms) && *s++ == ',' );
+
+return *s ? -EINVAL : 0;
 }
 custom_param("dom0_nodes", parse_dom0_nodes);
 
@@ -183,25 +190,30 @@ bool __initdata dom0_pvh;
  *  - pvh   Create a PVHv2 Dom0.
  *  - shadowUse shadow paging for Dom0.
  */
-static void __init parse_dom0_param(char *s)
+static int __init parse_dom0_param(const char *s)
 {
-char *ss;
+const char *ss;
+int rc = 0;
 
 do {
 
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
+if ( !ss )
+ss = strchr(s, '\0');
 
-if ( !strcmp(s, "pvh") )
+if ( !strncmp(s, "pvh", ss - s) )
 dom0_pvh = true;
 #ifdef CONFIG_SHADOW_PAGING
-else if ( !strcmp(s, "shadow") )
+else if ( !strncmp(s, "shadow", ss - s) )
 opt_dom0_shadow = true;
 #endif
+else
+rc = -EINVAL;
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 custom_param("dom0", parse_dom0_param);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 14/53] xen/arch/x86/microcode.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/microcode.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/microcode.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 7558202efa..77c1efc97f 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -73,15 +73,19 @@ void __init microcode_set_module(unsigned int idx)
  * If the EFI has forced which of the multiboot payloads is to be used,
  * no parsing will be attempted.
  */
-static void __init parse_ucode(char *s)
+static int __init parse_ucode(const char *s)
 {
+const char *q = NULL;
+
 if ( ucode_mod_forced ) /* Forced by EFI */
-   return;
+   return 0;
 
 if ( !strncmp(s, "scan", 4) )
 ucode_scan = 1;
 else
-ucode_mod_idx = simple_strtol(s, NULL, 0);
+ucode_mod_idx = simple_strtol(s, , 0);
+
+return (q && *q) ? -EINVAL : 0;
 }
 custom_param("ucode", parse_ucode);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 45/53] xen/common/kexec.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
some messages issued by parse_low_crashinfo() and
parse_crashinfo_maxaddr() can be removed.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
 xen/common/kexec.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index fcc68bd4d8..e0f1e3eafa 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -213,7 +213,6 @@ static int __init parse_low_crashinfo(const char *str)
 low_crashinfo_mode = LOW_CRASHINFO_ALL;
 else
 {
-printk("Unknown low_crashinfo parameter '%s'.  Defaulting to min.\n", 
str);
 low_crashinfo_mode = LOW_CRASHINFO_MIN;
 return -EINVAL;
 }
@@ -240,11 +239,7 @@ static int __init parse_crashinfo_maxaddr(const char *str)
 if ( (addr = parse_size_and_unit(str, )) )
 crashinfo_maxaddr = addr;
 else
-{
-printk("Unable to parse crashinfo_maxaddr. Defaulting to 
%"PRIpaddr"\n",
-   crashinfo_maxaddr);
 return -EINVAL;
-}
 
 return *q ? -EINVAL : 0;
 }
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 42/53] xen/arch/x86/cpu/mcheck/mce.c: remove custom_param() error messages

[Juergen Gross]

With _cmdline_parse() now issuing error messages in case of illegal
parameters signalled by parsing functions specified in custom_param()
the message issued by mce_set_verbosity() can be removed.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/cpu/mcheck/mce.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index 8015dff8c4..7affe2591e 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -67,11 +67,7 @@ static int __init mce_set_verbosity(const char *str)
 if (strcmp("verbose", str) == 0)
 mce_verbosity = MCE_VERBOSE;
 else
-{
-printk(KERN_DEBUG "Machine Check verbosity level %s not recognised"
-   "use mce_verbosity=verbose", str);
 return -EINVAL;
-}
 
 return 0;
 }
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 20/53] xen/arch/x86/setup.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/setup.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/setup.c | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 414681d5a1..ec96287966 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -67,7 +67,7 @@ unsigned long __read_mostly cr4_pv32_mask;
 /* "acpi=force":  Override the disable blacklist.   */
 /* "acpi=ht": Limit ACPI just to boot-time to enable HT.*/
 /* "acpi=noirq":  Disables ACPI interrupt routing.  */
-static void parse_acpi_param(char *s);
+static int parse_acpi_param(const char *s);
 custom_param("acpi", parse_acpi_param);
 
 /*  Linux config option: propagated to domain0. */
@@ -102,59 +102,70 @@ unsigned long __read_mostly mmu_cr4_features = 
XEN_MINIMAL_CR4;
 /* smep: Enable/disable Supervisor Mode Execution Protection (default on). */
 #define SMEP_HVM_ONLY (-1)
 static s8 __initdata opt_smep = 1;
-static void __init parse_smep_param(char *s)
+
+static int __init parse_smep_param(const char *s)
 {
 if ( !*s )
 {
 opt_smep = 1;
-return;
+return 0;
 }
 
 switch ( parse_bool(s, NULL) )
 {
 case 0:
 opt_smep = 0;
-return;
+return 0;
 case 1:
 opt_smep = 1;
-return;
+return 0;
 }
 
 if ( !strcmp(s, "hvm") )
 opt_smep = SMEP_HVM_ONLY;
+else
+return -EINVAL;
+
+return 0;
 }
 custom_param("smep", parse_smep_param);
 
 /* smap: Enable/disable Supervisor Mode Access Prevention (default on). */
 #define SMAP_HVM_ONLY (-1)
 static s8 __initdata opt_smap = 1;
-static void __init parse_smap_param(char *s)
+
+static int __init parse_smap_param(const char *s)
 {
 if ( !*s )
 {
 opt_smap = 1;
-return;
+return 0;
 }
 
 switch ( parse_bool(s, NULL) )
 {
 case 0:
 opt_smap = 0;
-return;
+return 0;
 case 1:
 opt_smap = 1;
-return;
+return 0;
 }
 
 if ( !strcmp(s, "hvm") )
 opt_smap = SMAP_HVM_ONLY;
+else
+return -EINVAL;
+
+return 0;
 }
 custom_param("smap", parse_smap_param);
 
 bool __read_mostly acpi_disabled;
 bool __initdata acpi_force;
 static char __initdata acpi_param[10] = "";
-static void __init parse_acpi_param(char *s)
+
+static int __init parse_acpi_param(const char *s)
 {
 /* Save the parameter so it can be propagated to domain0. */
 safe_strcpy(acpi_param, s);
@@ -180,6 +191,10 @@ static void __init parse_acpi_param(char *s)
 {
 acpi_noirq_set();
 }
+else
+return -EINVAL;
+
+return 0;
 }
 
 static const module_t *__initdata initial_images;
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 21/53] xen/arch/x86/shutdown.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/shutdown.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
V4:
- issue a message in case of reboot=efi and no EFI runtime services
  (Jan Beulich)

V3:
- dont stop loop at first invalid character (Jan Beulich)
---
 xen/arch/x86/shutdown.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
index f63b8a668f..a87aa60add 100644
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -51,8 +51,11 @@ static int reboot_mode;
  * efiUse the EFI reboot (if running under EFI)
  */
 static enum reboot_type reboot_type = BOOT_INVALID;
-static void __init set_reboot_type(char *str)
+
+static int __init set_reboot_type(const char *str)
 {
+int rc = 0;
+
 for ( ; ; )
 {
 switch ( *str )
@@ -74,6 +77,9 @@ static void __init set_reboot_type(char *str)
 case 't':
 reboot_type = *str;
 break;
+default:
+rc = -EINVAL;
+break;
 }
 if ( (str = strchr(str, ',')) == NULL )
 break;
@@ -81,7 +87,13 @@ static void __init set_reboot_type(char *str)
 }
 
 if ( reboot_type == BOOT_EFI && !efi_enabled(EFI_RS) )
+{
+printk("EFI reboot selected, but no EFI runtime services available.\n"
+   "Falling back to default reboot type.\n");
 reboot_type = BOOT_INVALID;
+}
+
+return rc;
 }
 custom_param("reboot", set_reboot_type);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 38/53] xen/drivers/video/vesa.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/video/vesa.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/drivers/video/vesa.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/video/vesa.c b/xen/drivers/video/vesa.c
index 09d344c021..c92497e0bc 100644
--- a/xen/drivers/video/vesa.c
+++ b/xen/drivers/video/vesa.c
@@ -29,12 +29,14 @@ static unsigned int vram_remap;
 integer_param("vesa-map", vram_remap);
 
 static int font_height;
-static void __init parse_font_height(const char *s)
+static int __init parse_font_height(const char *s)
 {
 if ( simple_strtoul(s, , 10) == 8 && (*s++ == 'x') )
 font_height = simple_strtoul(s, , 10);
 if ( *s != '\0' )
 font_height = 0;
+
+return 0;
 }
 custom_param("font", parse_font_height);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 24/53] xen/common/core_parking.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/core_parking.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/common/core_parking.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/common/core_parking.c b/xen/common/core_parking.c
index de269e06c2..803c6c7911 100644
--- a/xen/common/core_parking.c
+++ b/xen/common/core_parking.c
@@ -41,14 +41,16 @@ static enum core_parking_controller {
 PERFORMANCE_FIRST
 } core_parking_controller = POWER_FIRST;
 
-static void __init setup_core_parking_option(char *str)
+static int __init setup_core_parking_option(const char *str)
 {
 if ( !strcmp(str, "power") )
 core_parking_controller = POWER_FIRST;
 else if ( !strcmp(str, "performance") )
 core_parking_controller = PERFORMANCE_FIRST;
 else
-return;
+return -EINVAL;
+
+return 0;
 }
 custom_param("core_parking", setup_core_parking_option);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 01/53] xen: add an optional string end parameter to parse_bool()

[Juergen Gross]

Add a parameter to parse_bool() to specify the end of the to be
parsed string. Specifying it as NULL will preserve the current
behavior to parse until the end of the input string, while passing
a non-NULL pointer will specify the first character after the input
string.

This will allow to parse boolean sub-strings without having to
write a NUL byte into the input string.

Modify all users of parse_bool() to pass NULL for the new parameter.

Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Julien Grall <julien.gr...@arm.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Tim Deegan <t...@xen.org>
Cc: Wei Liu <wei.l...@citrix.com>
Cc: Kevin Tian <kevin.t...@intel.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
 xen/arch/arm/acpi/boot.c  |  2 +-
 xen/arch/x86/cpu/vpmu.c   |  2 +-
 xen/arch/x86/mm.c |  2 +-
 xen/arch/x86/nmi.c|  2 +-
 xen/arch/x86/psr.c|  2 +-
 xen/arch/x86/setup.c  |  6 +++---
 xen/arch/x86/x86_64/mmconfig-shared.c |  2 +-
 xen/common/kernel.c   | 28 
 xen/drivers/char/console.c|  2 +-
 xen/drivers/cpufreq/cpufreq.c |  2 +-
 xen/drivers/passthrough/iommu.c   |  2 +-
 xen/drivers/passthrough/vtd/quirks.c  |  2 +-
 xen/include/xen/lib.h |  2 +-
 13 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
index 889208a0ea..a5a6f55f0e 100644
--- a/xen/arch/arm/acpi/boot.c
+++ b/xen/arch/arm/acpi/boot.c
@@ -199,7 +199,7 @@ static void __init parse_acpi_param(char *arg)
 return;
 
 /* Interpret the parameter for use within Xen. */
-if ( !parse_bool(arg) )
+if ( !parse_bool(arg, NULL) )
 param_acpi_off = true;
 else if ( !strcmp(arg, "force") ) /* force ACPI to be enabled */
 param_acpi_force = true;
diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 90954ca884..1c0ea10777 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -80,7 +80,7 @@ static void __init parse_vpmu_params(char *s)
 {
 char *sep, *p = s;
 
-switch ( parse_bool(s) )
+switch ( parse_bool(s, NULL) )
 {
 case 0:
 break;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index ed77270586..5b0e55d7d9 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -184,7 +184,7 @@ static void __init parse_mmio_relax(const char *s)
 if ( !*s )
 opt_mmio_relax = 1;
 else
-opt_mmio_relax = parse_bool(s);
+opt_mmio_relax = parse_bool(s, NULL);
 if ( opt_mmio_relax < 0 && strcmp(s, "all") )
 opt_mmio_relax = 0;
 }
diff --git a/xen/arch/x86/nmi.c b/xen/arch/x86/nmi.c
index 8914581f66..e44f88045e 100644
--- a/xen/arch/x86/nmi.c
+++ b/xen/arch/x86/nmi.c
@@ -54,7 +54,7 @@ static void __init parse_watchdog(char *s)
 return;
 }
 
-switch ( parse_bool(s) )
+switch ( parse_bool(s, NULL) )
 {
 case 0:
 opt_watchdog = false;
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index c2036cbed4..25a85b65b2 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -427,7 +427,7 @@ static void __init parse_psr_bool(char *s, char *value, 
char *feature,
 opt_psr |= mask;
 else
 {
-int val_int = parse_bool(value);
+int val_int = parse_bool(value, NULL);
 
 if ( val_int == 0 )
 opt_psr &= ~mask;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index db5df6956d..414681d5a1 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -110,7 +110,7 @@ static void __init parse_smep_param(char *s)
 return;
 }
 
-switch ( parse_bool(s) )
+switch ( parse_bool(s, NULL) )
 {
 case 0:
 opt_smep = 0;
@@ -136,7 +136,7 @@ static void __init parse_smap_param(char *s)
 return;
 }
 
-switch ( parse_bool(s) )
+switch ( parse_bool(s, NULL) )
 {
 case 0:
 opt_smap = 0;
@@ -160,7 +160,7 @@ static void __init parse_acpi_param(char *s)
 safe_strcpy(acpi_param, s);
 
 /* Interpret the parameter for use within Xen. */
-if ( !parse_bool(s) )
+if ( !parse_bool(s, NULL) )
 {
 disable_acpi();
 }
diff --git a/xen/arch/x86/x86_64/mmconfig-shared.c 
b/xen/arch/x86/x86_64/mmconfig-shared.c
index 488470bfeb..dbf9ff07fa 100644
--- a/xen/arch/x86/x86_64/mmconfig-shared.c
+++ b/xen/arch/x86/x86_64/mmconfig-shared.c
@@ -37,7 +37,7 @@ static void __init parse_mmcfg(char *s)
 if ( ss )
 *ss = '\0';
 
-if ( !parse_bool(s) )
+if ( !parse_bool(s, NULL) )
   

[Xen-devel] [PATCH v4 49/53] xen: add hypercall for setting parameters at runtime

[Juergen Gross]

Add a sysctl hypercall to support setting parameters similar to
command line parameters, but at runtime. The parameters to set are
specified as a string, just like the boot parameters.

Cc: Daniel De Graaf <dgde...@tycho.nsa.gov>
Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Wei Liu <wei.l...@citrix.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Cc: George Dunlap <george.dun...@eu.citrix.com>
Cc: Jan Beulich <jbeul...@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
Cc: Stefano Stabellini <sstabell...@kernel.org>
Cc: Tim Deegan <t...@xen.org>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
---
V4:
- use copy_from_guest() (Jan Beulich)
- add more descriptive comment to sysctl.h (Jan Beulich)

V3:
- check pad[] to be zero (Jan Beulich)
- return E2BIG in case of parameters too long (Jan Beulich)
- move max. parameter size define to sysctl.c (Jan Beulich)

V2:
- corrected XSM test (Daniel De Graaf)
---
 tools/flask/policy/modules/dom0.te  |  2 +-
 xen/common/sysctl.c | 36 
 xen/include/public/sysctl.h | 20 
 xen/xsm/flask/hooks.c   |  3 +++
 xen/xsm/flask/policy/access_vectors |  2 ++
 5 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/tools/flask/policy/modules/dom0.te 
b/tools/flask/policy/modules/dom0.te
index d0a4d91ac0..338caaf41e 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -16,7 +16,7 @@ allow dom0_t xen_t:xen {
 allow dom0_t xen_t:xen2 {
resource_op psr_cmt_op psr_cat_op pmu_ctrl get_symbol
get_cpu_levelling_caps get_cpu_featureset livepatch_op
-   gcov_op
+   gcov_op set_parameter
 };
 
 # Allow dom0 to use all XENVER_ subops that have checks.
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index ae58a0f650..a6882d1c9d 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -467,6 +467,42 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) 
u_sysctl)
 copyback = 1;
 break;
 
+case XEN_SYSCTL_set_parameter:
+{
+#define XEN_SET_PARAMETER_MAX_SIZE 1023
+char *params;
+
+if ( op->u.set_parameter.pad[0] || op->u.set_parameter.pad[1] ||
+ op->u.set_parameter.pad[2] )
+{
+ret = -EINVAL;
+break;
+}
+if ( op->u.set_parameter.size > XEN_SET_PARAMETER_MAX_SIZE )
+{
+ret = -E2BIG;
+break;
+}
+params = xmalloc_bytes(op->u.set_parameter.size + 1);
+if ( !params )
+{
+ret = -ENOMEM;
+break;
+}
+if ( copy_from_guest(params, op->u.set_parameter.params,
+ op->u.set_parameter.size) )
+ret = -EFAULT;
+else
+{
+params[op->u.set_parameter.size] = 0;
+ret = runtime_parse(params);
+}
+
+xfree(params);
+
+break;
+}
+
 default:
 ret = arch_do_sysctl(op, u_sysctl);
 copyback = 0;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 9e51af61e1..7830b987da 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -1096,6 +1096,24 @@ struct xen_sysctl_livepatch_op {
 typedef struct xen_sysctl_livepatch_op xen_sysctl_livepatch_op_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_livepatch_op_t);
 
+/*
+ * XEN_SYSCTL_set_parameter
+ *
+ * Change hypervisor parameters at runtime.
+ * The input string is parsed similar to the boot parameters.
+ * Parameters are a single string terminated by a NUL byte of max. size
+ * characters. Multiple settings can be specified by separating them
+ * with blanks.
+ */
+
+struct xen_sysctl_set_parameter {
+XEN_GUEST_HANDLE_64(char) params;   /* IN: pointer to parameters. */
+uint16_t size;  /* IN: size of parameters. */
+uint16_t pad[3];/* IN: MUST be zero. */
+};
+typedef struct xen_sysctl_set_parameter xen_sysctl_set_parameter_t;
+DEFINE_XEN_GUEST_HANDLE(xen_sysctl_set_parameter_t);
+
 struct xen_sysctl {
 uint32_t cmd;
 #define XEN_SYSCTL_readconsole1
@@ -1124,6 +1142,7 @@ struct xen_sysctl {
 #define XEN_SYSCTL_get_cpu_levelling_caps25
 #define XEN_SYSCTL_get_cpu_featureset26
 #define XEN_SYSCTL_livepatch_op  27
+#define XEN_SYSCTL_set_parameter 28
 uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
 union {
 struct xen_sysctl_readconsole   readconsole;
@@ -1152,6 +1171,7 @@ struct xen_sysctl {
 struct xen_sysctl_cpu_levelling_caps cpu_levelling_caps;
 struct xen_sysctl_cpu_featuresetcpu_featureset;
 struct xen_sysctl_livepatch_op  livepatch;
+struct xen_sysctl_set_parameter s

[Xen-devel] [PATCH v4 27/53] xen/common/kexec.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/kexec.c

to indicate whether the parameter value was parsed successfully.

Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
---
 xen/common/kexec.c | 30 ++
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index a52c30ba1e..fcc68bd4d8 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -102,9 +102,10 @@ static void *crash_heap_current = NULL, *crash_heap_end = 
NULL;
  * < and below are synonyomous, the latter being useful for grub2 systems
  * which would otherwise require escaping of the < option
  */
-static void __init parse_crashkernel(const char *str)
+static int __init parse_crashkernel(const char *str)
 {
 const char *cur;
+int rc = 0;
 
 if ( strchr(str, ':' ) )
 {
@@ -116,6 +117,7 @@ static void __init parse_crashkernel(const char *str)
 printk(XENLOG_WARNING "crashkernel: too many ranges\n");
 cur = NULL;
 str = strpbrk(str, "@,<");
+rc = -EINVAL;
 break;
 }
 
@@ -126,6 +128,7 @@ static void __init parse_crashkernel(const char *str)
 if ( *str != '-' )
 {
 printk(XENLOG_WARNING "crashkernel: '-' expected\n");
+rc = -EINVAL;
 break;
 }
 
@@ -137,6 +140,7 @@ static void __init parse_crashkernel(const char *str)
 if ( ranges[idx].end <= ranges[idx].start )
 {
 printk(XENLOG_WARNING "crashkernel: end <= start\n");
+rc = -EINVAL;
 break;
 }
 }
@@ -146,6 +150,7 @@ static void __init parse_crashkernel(const char *str)
 if ( *str != ':' )
 {
 printk(XENLOG_WARNING "crashkernel: ':' expected\n");
+rc = -EINVAL;
 break;
 }
 
@@ -169,10 +174,18 @@ static void __init parse_crashkernel(const char *str)
 else if ( !strncmp(str, ",below=", 7) )
 kexec_crash_area_limit = parse_size_and_unit(cur = str + 7, );
 else
+{
 printk(XENLOG_WARNING "crashkernel: '%s' ignored\n", str);
+rc = -EINVAL;
+}
 }
 if ( cur && cur == str )
+{
 printk(XENLOG_WARNING "crashkernel: memory value expected\n");
+rc = -EINVAL;
+}
+
+return rc;
 }
 custom_param("crashkernel", parse_crashkernel);
 
@@ -186,7 +199,7 @@ custom_param("crashkernel", parse_crashkernel);
  * - all will allocate additional structures such as domain and vcpu structs
  *   low so the crash kernel can perform an extended analysis of state.
  */
-static void __init parse_low_crashinfo(const char * str)
+static int __init parse_low_crashinfo(const char *str)
 {
 
 if ( !strlen(str) )
@@ -202,7 +215,10 @@ static void __init parse_low_crashinfo(const char * str)
 {
 printk("Unknown low_crashinfo parameter '%s'.  Defaulting to min.\n", 
str);
 low_crashinfo_mode = LOW_CRASHINFO_MIN;
+return -EINVAL;
 }
+
+return 0;
 }
 custom_param("low_crashinfo", parse_low_crashinfo);
 
@@ -212,19 +228,25 @@ custom_param("low_crashinfo", parse_low_crashinfo);
  *
  *  will be rounded down to the nearest power of two.  Defaults to 64G
  */
-static void __init parse_crashinfo_maxaddr(const char * str)
+static int __init parse_crashinfo_maxaddr(const char *str)
 {
 u64 addr;
+const char *q;
 
 /* if low_crashinfo_mode is unset, default to min. */
 if ( low_crashinfo_mode == LOW_CRASHINFO_INVALID )
 low_crashinfo_mode = LOW_CRASHINFO_MIN;
 
-if ( (addr = parse_size_and_unit(str, NULL)) )
+if ( (addr = parse_size_and_unit(str, )) )
 crashinfo_maxaddr = addr;
 else
+{
 printk("Unable to parse crashinfo_maxaddr. Defaulting to 
%"PRIpaddr"\n",
crashinfo_maxaddr);
+return -EINVAL;
+}
+
+return *q ? -EINVAL : 0;
 }
 custom_param("crashinfo_maxaddr", parse_crashinfo_maxaddr);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 39/53] xen/xsm/flask/flask_op.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/xsm/flask/flask_op.c

to indicate whether the parameter value was parsed successfully.

Cc: Daniel De Graaf <dgde...@tycho.nsa.gov>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Daniel De Graaf <dgde...@tycho.nsa.gov>
Acked-by: Wei Liu <wei.l...@citrix.com>
---
 xen/xsm/flask/flask_op.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index 15aa96b09f..03baa28b68 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -26,7 +26,7 @@
 #define _copy_from_guest copy_from_guest
 
 enum flask_bootparam_t __read_mostly flask_bootparam = 
FLASK_BOOTPARAM_ENFORCING;
-static void parse_flask_param(char *s);
+static int parse_flask_param(const char *s);
 custom_param("flask", parse_flask_param);
 
 bool __read_mostly flask_enforcing = true;
@@ -58,7 +58,7 @@ static int flask_security_make_bools(void);
 
 extern int ss_initialized;
 
-static void __init parse_flask_param(char *s)
+static int __init parse_flask_param(const char *s)
 {
 if ( !strcmp(s, "enforcing") )
 flask_bootparam = FLASK_BOOTPARAM_ENFORCING;
@@ -70,6 +70,8 @@ static void __init parse_flask_param(char *s)
 flask_bootparam = FLASK_BOOTPARAM_PERMISSIVE;
 else
 flask_bootparam = FLASK_BOOTPARAM_INVALID;
+
+return (flask_bootparam == FLASK_BOOTPARAM_INVALID) ? -EINVAL : 0;
 }
 
 static int domain_has_security(struct domain *d, u32 perms)
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 26/53] xen/common/efi/boot.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/common/efi/boot.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
---
V3:
- dont return out of loops (Jan Beulich)
- dont modify option value in parsing function
---
 xen/common/efi/boot.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 11bdc7a2a4..01d33004e0 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1306,9 +1306,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
*SystemTable)
 
 static bool __initdata efi_map_uc;
 
-static void __init parse_efi_param(char *s)
+static int __init parse_efi_param(const char *s)
 {
-char *ss;
+const char *ss;
+int rc = 0;
 
 do {
 bool val = strncmp(s, "no-", 3);
@@ -1317,21 +1318,25 @@ static void __init parse_efi_param(char *s)
 s += 3;
 
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
+if ( !ss )
+ss = strchr(s, '\0');
 
-if ( !strcmp(s, "rs") )
+if ( !strncmp(s, "rs", ss - s) )
 {
 if ( val )
 __set_bit(EFI_RS, _flags);
 else
 __clear_bit(EFI_RS, _flags);
 }
-else if ( !strcmp(s, "attr=uc") )
+else if ( !strncmp(s, "attr=uc", ss - s) )
 efi_map_uc = val;
+else
+rc = -EINVAL;
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 custom_param("efi", parse_efi_param);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 05/53] xen/arch/x86/apic.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/apic.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/apic.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 851a6cc6cb..325b48074a 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -785,23 +785,28 @@ int lapic_resume(void)
  * Original code written by Keir Fraser.
  */
 
-static void __init lapic_disable(char *str)
+static int __init lapic_disable(const char *str)
 {
 enable_local_apic = -1;
 setup_clear_cpu_cap(X86_FEATURE_APIC);
+return 0;
 }
 custom_param("nolapic", lapic_disable);
 boolean_param("lapic", enable_local_apic);
 
-static void __init apic_set_verbosity(char *str)
+static int __init apic_set_verbosity(const char *str)
 {
 if (strcmp("debug", str) == 0)
 apic_verbosity = APIC_DEBUG;
 else if (strcmp("verbose", str) == 0)
 apic_verbosity = APIC_VERBOSE;
-else
+else {
 printk(KERN_WARNING "APIC Verbosity level %s not recognised"
" use apic_verbosity=verbose or apic_verbosity=debug", str);
+return -EINVAL;
+}
+
+return 0;
 }
 custom_param("apic_verbosity", apic_set_verbosity);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 09/53] xen/arch/x86/genapic/probe.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/genapic/probe.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
---
V3:
- dont return out of loop (Jan Beulich)
---
 xen/arch/x86/genapic/probe.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/genapic/probe.c b/xen/arch/x86/genapic/probe.c
index 9a147ff64a..af3745aa21 100644
--- a/xen/arch/x86/genapic/probe.c
+++ b/xen/arch/x86/genapic/probe.c
@@ -44,12 +44,17 @@ void __init generic_bigsmp_probe(void)
}
 }
 
-static void __init genapic_apic_force(char *str)
+static int __init genapic_apic_force(const char *str)
 {
-   int i;
+   int i, rc = -EINVAL;
+
for (i = 0; apic_probe[i]; i++)
-   if (!strcmp(apic_probe[i]->name, str))
+   if (!strcmp(apic_probe[i]->name, str)) {
genapic = apic_probe[i];
+   rc = 0;
+   }
+
+   return rc;
 }
 custom_param("apic", genapic_apic_force);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 23/53] xen/arch/x86/x86_64/mmconfig-shared.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/x86_64/mmconfig-shared.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
V4:
- correct parse_bool() call
- style correction (Jan Beulich)

V3:
- dont return out of loop (Jan Beulich)
- dont modify option value in parsing function
---
 xen/arch/x86/x86_64/mmconfig-shared.c | 30 ++
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/x86_64/mmconfig-shared.c 
b/xen/arch/x86/x86_64/mmconfig-shared.c
index dbf9ff07fa..7c3b7fd30b 100644
--- a/xen/arch/x86/x86_64/mmconfig-shared.c
+++ b/xen/arch/x86/x86_64/mmconfig-shared.c
@@ -28,22 +28,36 @@
 
 unsigned int pci_probe = PCI_PROBE_CONF1 | PCI_PROBE_MMCONF;
 
-static void __init parse_mmcfg(char *s)
+static int __init parse_mmcfg(const char *s)
 {
-char *ss;
+const char *ss;
+int rc = 0;
 
 do {
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
+if ( !ss )
+ss = strchr(s, '\0');
 
-if ( !parse_bool(s, NULL) )
+switch ( parse_bool(s, ss) )
+{
+case 0:
 pci_probe &= ~PCI_PROBE_MMCONF;
-else if ( !strcmp(s, "amd_fam10") || !strcmp(s, "amd-fam10") )
-pci_probe |= PCI_CHECK_ENABLE_AMD_MMCONF;
+break;
+case 1:
+break;
+default:
+if ( !strncmp(s, "amd_fam10", ss - s) ||
+ !strncmp(s, "amd-fam10", ss - s) )
+pci_probe |= PCI_CHECK_ENABLE_AMD_MMCONF;
+else
+rc = -EINVAL;
+break;
+}
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 custom_param("mmcfg", parse_mmcfg);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 13/53] xen/arch/x86/irq.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/arch/x86/irq.c

to indicate whether the parameter value was parsed successfully.

Cc: Jan Beulich <jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
 xen/arch/x86/irq.c | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 57e6c18970..77ca181985 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -26,7 +26,7 @@
 #include 
 #include 
 
-static void parse_irq_vector_map_param(char *s);
+static int parse_irq_vector_map_param(const char *s);
 
 /* opt_noirqbalance: If true, software IRQ balancing/affinity is disabled. */
 bool __read_mostly opt_noirqbalance;
@@ -60,24 +60,29 @@ static struct timer irq_ratelimit_timer;
 static unsigned int __read_mostly irq_ratelimit_threshold = 1;
 integer_param("irq_ratelimit", irq_ratelimit_threshold);
 
-static void __init parse_irq_vector_map_param(char *s)
+static int __init parse_irq_vector_map_param(const char *s)
 {
-char *ss;
+const char *ss;
+int rc = 0;
 
 do {
 ss = strchr(s, ',');
-if ( ss )
-*ss = '\0';
+if ( !ss )
+ss = strchr(s, '\0');
 
-if ( !strcmp(s, "none"))
+if ( !strncmp(s, "none", ss - s))
 opt_irq_vector_map=OPT_IRQ_VECTOR_MAP_NONE;
-else if ( !strcmp(s, "global"))
+else if ( !strncmp(s, "global", ss - s))
 opt_irq_vector_map=OPT_IRQ_VECTOR_MAP_GLOBAL;
-else if ( !strcmp(s, "per-device"))
+else if ( !strncmp(s, "per-device", ss - s))
 opt_irq_vector_map=OPT_IRQ_VECTOR_MAP_PERDEV;
+else
+rc = -EINVAL;
 
 s = ss + 1;
-} while ( ss );
+} while ( *ss );
+
+return rc;
 }
 
 /* Must be called when irq disabled */
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 36/53] xen/drivers/passthrough/vtd/dmar.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/passthrough/vtd/dmar.c

to indicate whether the parameter value was parsed successfully.

Cc: Kevin Tian <kevin.t...@intel.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
---
 xen/drivers/passthrough/vtd/dmar.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/vtd/dmar.c 
b/xen/drivers/passthrough/vtd/dmar.c
index 82040ddc05..dd122da730 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -1090,7 +1090,7 @@ int intel_iommu_get_reserved_device_memory(iommu_grdm_t 
*func, void *ctxt)
  * If a segment is specified for other than the first device, and it does not
  * match the one specified for the first one, an error will be reported.
  */
-static void __init parse_rmrr_param(const char *str)
+static int __init parse_rmrr_param(const char *str)
 {
 const char *s = str, *cur, *stmp;
 unsigned int seg, bus, dev, func, dev_count;
@@ -1143,5 +1143,7 @@ static void __init parse_rmrr_param(const char *str)
 nr_rmrr++;
 
 } while ( *s++ == ';' && nr_rmrr < MAX_USER_RMRR );
+
+return *s ? -EINVAL : 0;
 }
 custom_param("rmrr", parse_rmrr_param);
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v4 33/53] xen/drivers/passthrough/amd/iommu_acpi.c: let custom parameter parsing routines return errno

[Juergen Gross]

Modify the custom parameter parsing routines in:

xen/drivers/passthrough/amd/iommu_acpi.c

to indicate whether the parameter value was parsed successfully.

Cc: Suravee Suthikulpanit <suravee.suthikulpa...@amd.com>
Signed-off-by: Juergen Gross <jgr...@suse.com>
Acked-by: Wei Liu <wei.l...@citrix.com>
---
 xen/drivers/passthrough/amd/iommu_acpi.c | 26 +-
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c 
b/xen/drivers/passthrough/amd/iommu_acpi.c
index f4c7206c2a..64d10481d7 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -632,21 +632,23 @@ static u16 __init parse_ivhd_device_extended_range(
 return dev_length;
 }
 
-static void __init parse_ivrs_ioapic(char *str)
+static int __init parse_ivrs_ioapic(const char *str)
 {
 const char *s = str;
 unsigned long id;
 unsigned int seg, bus, dev, func;
 unsigned int idx;
 
-ASSERT(*s == '[');
+if ( *s != '[' )
+return -EINVAL;
+
 id = simple_strtoul(s + 1, , 0);
 if ( *s != ']' || *++s != '=' )
-return;
+return -EINVAL;
 
 s = parse_pci(s + 1, , , , );
 if ( !s || *s )
-return;
+return -EINVAL;
 
 idx = ioapic_id_to_index(id);
 if ( idx == MAX_IO_APICS )
@@ -655,7 +657,7 @@ static void __init parse_ivrs_ioapic(char *str)
 if ( idx == MAX_IO_APICS )
 {
 printk(XENLOG_ERR "Error: %s: Too many IO APICs.\n", __func__);
-return;
+return -EINVAL;
 }
 }
 
@@ -663,28 +665,34 @@ static void __init parse_ivrs_ioapic(char *str)
 ioapic_sbdf[idx].seg = seg;
 ioapic_sbdf[idx].id = id;
 ioapic_sbdf[idx].cmdline = true;
+
+return 0;
 }
 custom_param("ivrs_ioapic[", parse_ivrs_ioapic);
 
-static void __init parse_ivrs_hpet(char *str)
+static int __init parse_ivrs_hpet(const char *str)
 {
 const char *s = str;
 unsigned long id;
 unsigned int seg, bus, dev, func;
 
-ASSERT(*s == '[');
+if ( *s != '[' )
+return -EINVAL;
+
 id = simple_strtoul(s + 1, , 0);
 if ( id != (typeof(hpet_sbdf.id))id || *s != ']' || *++s != '=' )
-return;
+return -EINVAL;
 
 s = parse_pci(s + 1, , , , );
 if ( !s || *s )
-return;
+return -EINVAL;
 
 hpet_sbdf.id = id;
 hpet_sbdf.bdf = PCI_BDF(bus, dev, func);
 hpet_sbdf.seg = seg;
 hpet_sbdf.init = HPET_CMDL;
+
+return 0;
 }
 custom_param("ivrs_hpet[", parse_ivrs_hpet);
 
-- 
2.12.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 39/52] xen: check parameter validity when parsing command line

[Juergen Gross]

On 23/08/17 15:18, Jan Beulich wrote:
 On 23.08.17 at 14:42,  wrote:
>> On 23/08/17 11:38, Jan Beulich wrote:
>> On 23.08.17 at 11:30,  wrote:
 On 22/08/17 13:24, Jan Beulich wrote:
 On 16.08.17 at 14:52,  wrote:
>> @@ -176,7 +210,8 @@ int __init parse_bool(const char *s)
>>   !strcmp("on", s) ||
>>   !strcmp("true", s) ||
>>   !strcmp("enable", s) ||
>> - !strcmp("1", s) )
>> + !strcmp("1", s) ||
>> + !*s )
>>  return 1;
>
> Careful with this: Taking the "iommu=" example that I've commented
> on in the other patch already, much depends on what you mean to
> do about the problem there: "iommu=,..." should not end up
> meaning "iommu=on,...".

 It won't. *s will be ',' in this case.
>>>
>>> Right, but as said - much depends on what you mean to do about
>>> the problem in the earlier patch.
>>
>> So I just hit this. And looking more thoroughly into it: today it in
>> fact has exactly this meaning. iommu_enable is "1" per default. So
>> specifying "iommu=,..." won't change this and has the same semantics
>> as "iommu=on,...".
> 
> But that's not the interesting case. Are you saying that
> "iommu=off iommu=,..." enables the IOMMU today? It doesn't
> look to me as if it would.

I guess this is a weird case after all: trying to be compatible to
"iommu=off iommu=,..." while "iommu=off iommu=on,..." doesn't work
correctly today is questionable.

OTOH the easiest way to avoid all discussions might be to drop the
parse_bool() modification to return "true" in case of an empty string
and handle this case by not calling parse_bool() at all from
_cmdline_parse() if no argument was given.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 39/52] xen: check parameter validity when parsing command line

[Juergen Gross]

On 23/08/17 11:38, Jan Beulich wrote:
 On 23.08.17 at 11:30,  wrote:
>> On 22/08/17 13:24, Jan Beulich wrote:
>> On 16.08.17 at 14:52,  wrote:
 @@ -176,7 +210,8 @@ int __init parse_bool(const char *s)
   !strcmp("on", s) ||
   !strcmp("true", s) ||
   !strcmp("enable", s) ||
 - !strcmp("1", s) )
 + !strcmp("1", s) ||
 + !*s )
  return 1;
>>>
>>> Careful with this: Taking the "iommu=" example that I've commented
>>> on in the other patch already, much depends on what you mean to
>>> do about the problem there: "iommu=,..." should not end up
>>> meaning "iommu=on,...".
>>
>> It won't. *s will be ',' in this case.
> 
> Right, but as said - much depends on what you mean to do about
> the problem in the earlier patch.

So I just hit this. And looking more thoroughly into it: today it in
fact has exactly this meaning. iommu_enable is "1" per default. So
specifying "iommu=,..." won't change this and has the same semantics
as "iommu=on,...".

So should I change this or not? With the new parse_bool() parameter
(end of the string pointer or NULL) I can easily tell the difference
between a boolean with or without following options.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 22/52] xen/arch/x86/x86_64/mmconfig-shared.c: let custom parameter parsing routines return errno

[Juergen Gross]

On 22/08/17 11:55, Jan Beulich wrote:
 On 16.08.17 at 14:51,  wrote:
>> --- a/xen/arch/x86/x86_64/mmconfig-shared.c
>> +++ b/xen/arch/x86/x86_64/mmconfig-shared.c
>> @@ -28,22 +28,35 @@
>>  
>>  unsigned int pci_probe = PCI_PROBE_CONF1 | PCI_PROBE_MMCONF;
>>  
>> -static void __init parse_mmcfg(char *s)
>> +static int __init parse_mmcfg(const char *s)
>>  {
>> -char *ss;
>> +const char *ss;
>> +int rc = 0;
>>  
>>  do {
>>  ss = strchr(s, ',');
>> -if ( ss )
>> -*ss = '\0';
>> +if ( !ss )
>> +ss = strchr(s, '\0');
>>  
>> -if ( !parse_bool(s) )
>> +switch ( parse_bool(s) ) {
> 
> Style. With this corrected

Another parse_bool() victim.


Juergen

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel