xen/arch/arm/traps.c | 26 +++++++++++++++++++++++++- xen/common/schedule.c | 2 +- xen/include/xen/sched.h | 1 + 3 files changed, 27 insertions(+), 2 deletions(-)
Introduce two global parameters to disable debug registers trapping and
perf counters trapping. They are only safe to use in static partitiong
scenarios where sched=null is used -- vcpu cannot be migrated from one
pcpu to the next.
Introduce a new simple umbrella command line option
"static_partitioning" that enables vwfi=native, sched=null, and also
sets debug_registers_trap and perf_counters_trap to false.
Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
CC: Julien Grall <julien.grall@arm.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Tim Deegan <tim@xen.org>
CC: Wei Liu <wl@xen.org>
CC: George Dunlap <george.dunlap@eu.citrix.com>
CC: Dario Faggioli <dfaggioli@suse.com>
---
This is not ideal. The best course of action would be to implement
proper context switching of all the necessary debug and perf counters
registers. This is an imperfect shortcut, which could reasonably be left
out of the upstream tree but I shared it with others for their
convenience.
---
xen/arch/arm/traps.c | 26 +++++++++++++++++++++++++-
xen/common/schedule.c | 2 +-
xen/include/xen/sched.h | 1 +
3 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 5c18e918b0..d6eaffde23 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -118,6 +118,28 @@ static int __init parse_vwfi(const char *s)
}
custom_param("vwfi", parse_vwfi);
+static bool debug_registers_trap = true;
+static bool perf_counters_trap = true;
+
+static int __init opt_static_partitioning(const char *s)
+{
+ if ( strcmp(s, "true") &&
+ strcmp(s, "True") &&
+ strcmp(s, "1") )
+ return 0;
+
+ vwfi = NATIVE;
+ debug_registers_trap = false;
+ perf_counters_trap = false;
+ memcpy(opt_sched, "null", 5);
+
+ /* Disable Trap Debug and Performance Monitor now for CPU0 */
+ WRITE_SYSREG(HDCR_TDRA, MDCR_EL2);
+
+ return 0;
+}
+custom_param("static_partitioning", opt_static_partitioning);
+
register_t get_default_hcr_flags(void)
{
return (HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
@@ -165,7 +187,9 @@ void init_traps(void)
WRITE_SYSREG((vaddr_t)hyp_traps_vector, VBAR_EL2);
/* Trap Debug and Performance Monitor accesses */
- WRITE_SYSREG(HDCR_TDRA|HDCR_TDOSA|HDCR_TDA|HDCR_TPM|HDCR_TPMCR,
+ WRITE_SYSREG(HDCR_TDRA |
+ (debug_registers_trap ? HDCR_TDOSA|HDCR_TDA : 0) |
+ (perf_counters_trap ? HDCR_TPM|HDCR_TPMCR : 0),
MDCR_EL2);
/* Trap CP15 c15 used for implementation defined registers */
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 049f93f7aa..51eb3d770b 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -39,7 +39,7 @@
#include <xen/err.h>
/* opt_sched: scheduler - default to configured value */
-static char __initdata opt_sched[10] = CONFIG_SCHED_DEFAULT;
+char __initdata opt_sched[10] = CONFIG_SCHED_DEFAULT;
string_param("sched", opt_sched);
/* if sched_smt_power_savings is set,
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index b73ccbdf3a..c40a1b5dbc 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -906,6 +906,7 @@ static inline bool is_vcpu_online(const struct vcpu *v)
}
extern bool sched_smt_power_savings;
+extern char opt_sched[10];
extern enum cpufreq_controller {
FREQCTL_none, FREQCTL_dom0_kernel, FREQCTL_xen
--
2.17.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-develOn 31/05/2019 16:00, Stefano Stabellini wrote:
> Introduce two global parameters to disable debug registers trapping and
> perf counters trapping. They are only safe to use in static partitiong
> scenarios where sched=null is used -- vcpu cannot be migrated from one
> pcpu to the next.
>
> Introduce a new simple umbrella command line option
> "static_partitioning" that enables vwfi=native, sched=null, and also
> sets debug_registers_trap and perf_counters_trap to false.
>
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> CC: Julien Grall <julien.grall@arm.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: George Dunlap <George.Dunlap@eu.citrix.com>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: Tim Deegan <tim@xen.org>
> CC: Wei Liu <wl@xen.org>
> CC: George Dunlap <george.dunlap@eu.citrix.com>
> CC: Dario Faggioli <dfaggioli@suse.com>
> ---
> This is not ideal. The best course of action would be to implement
> proper context switching of all the necessary debug and perf counters
> registers. This is an imperfect shortcut, which could reasonably be left
> out of the upstream tree but I shared it with others for their
> convenience.
> ---
> xen/arch/arm/traps.c | 26 +++++++++++++++++++++++++-
> xen/common/schedule.c | 2 +-
> xen/include/xen/sched.h | 1 +
I see no hunk in docs/ ...
> 3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 5c18e918b0..d6eaffde23 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -118,6 +118,28 @@ static int __init parse_vwfi(const char *s)
> }
> custom_param("vwfi", parse_vwfi);
>
> +static bool debug_registers_trap = true;
> +static bool perf_counters_trap = true;
> +
> +static int __init opt_static_partitioning(const char *s)
> +{
> + if ( strcmp(s, "true") &&
> + strcmp(s, "True") &&
> + strcmp(s, "1") )
> + return 0;
Please please please don't opencode boolean checking. I think I've
purged all of it (certainly all that I'm aware of), and this example
isn't remotely compatible with existing boolean options.
Furthermore, you can't make something which looks like a single boolean
option with custom_param(), because you can't make the no- prefix work.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Hi Stefano,
On 6/1/19 12:00 AM, Stefano Stabellini wrote:
> Introduce two global parameters to disable debug registers trapping and
> perf counters trapping. They are only safe to use in static partitiong
> scenarios where sched=null is used -- vcpu cannot be migrated from one
> pcpu to the next.
sched=null only indicates that Xen will use NULL scheduler by default.
But you can still use a different scheduler by creating cpupool after
boot. So your vCPU may be able to migrate between CPU if the domain is
assigned to a different cpupool.
>
> Introduce a new simple umbrella command line option
> "static_partitioning" that enables vwfi=native, sched=null, and also
> sets debug_registers_trap and perf_counters_trap to false.
>
> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com>
> CC: Julien Grall <julien.grall@arm.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: George Dunlap <George.Dunlap@eu.citrix.com>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: Tim Deegan <tim@xen.org>
> CC: Wei Liu <wl@xen.org>
> CC: George Dunlap <george.dunlap@eu.citrix.com>
> CC: Dario Faggioli <dfaggioli@suse.com>
> ---
> This is not ideal. The best course of action would be to implement
> proper context switching of all the necessary debug and perf counters
> registers. This is an imperfect shortcut, which could reasonably be left
> out of the upstream tree but I shared it with others for their
> convenience.
I am happy to consider this option in upstream but, I think, there are a
bit more work to get it fully working/safe:
1) sched=null does not prevent using a different scheduler later on
(see above)
2) AFAIK the configuration you suggest is buggy because [1] has not
been resolved yet. This means that destroying VM may not work with your
option.
3) The perf/debug registers are not reset when a VM is destroyed so
they will leak to the next vCPU created.
4) A guest can single step an instruction, how does this works when
the instruction is skipped by Xen? More importantly how does that fit if
the hypercall is preempted (we would re-execute hvc)?
It would also be good to explain how this is it safe to give full access
to the debug registers. So far, I only skimmed the Arm Arm to see if I
can find some ground here.
For Xen Arm64, from my understanding, we would be safe if SPSR_EL2.D is
set to 1 (i.e Debug exception masked at EL2). We disable all the
interrupts at the boot (see arm64/head.S), so assuming we don't touch it
afterwards (someone need to check that) this should be OK.
For Xen Arm32, it is not clear what actually prevent it.
Similar exercise should be done with the PMU registers.
> ---
> xen/arch/arm/traps.c | 26 +++++++++++++++++++++++++-
> xen/common/schedule.c | 2 +-
> xen/include/xen/sched.h | 1 +
> 3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 5c18e918b0..d6eaffde23 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -118,6 +118,28 @@ static int __init parse_vwfi(const char *s)
> }
> custom_param("vwfi", parse_vwfi);
>
> +static bool debug_registers_trap = true;
> +static bool perf_counters_trap = true;
> +
> +static int __init opt_static_partitioning(const char *s)
> +{
> + if ( strcmp(s, "true") &&
> + strcmp(s, "True") &&
> + strcmp(s, "1") )
> + return 0;
> +
> + vwfi = NATIVE;
> + debug_registers_trap = false;
> + perf_counters_trap = false;
> + memcpy(opt_sched, "null", 5);
> +
> + /* Disable Trap Debug and Performance Monitor now for CPU0 */
> + WRITE_SYSREG(HDCR_TDRA, MDCR_EL2);
I don't particularly like this approach because it makes difficult to
understand whether this WRITE_SYSREG or the one below will the last one.
It would be best of we rework the code to initialize MDCR_EL2 much later
in the boot.
> +
> + return 0;
> +}
> +custom_param("static_partitioning", opt_static_partitioning);
> +
> register_t get_default_hcr_flags(void)
> {
> return (HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
> @@ -165,7 +187,9 @@ void init_traps(void)
> WRITE_SYSREG((vaddr_t)hyp_traps_vector, VBAR_EL2);
>
> /* Trap Debug and Performance Monitor accesses */
> - WRITE_SYSREG(HDCR_TDRA|HDCR_TDOSA|HDCR_TDA|HDCR_TPM|HDCR_TPMCR,
> + WRITE_SYSREG(HDCR_TDRA |
> + (debug_registers_trap ? HDCR_TDOSA|HDCR_TDA : 0) |
> + (perf_counters_trap ? HDCR_TPM|HDCR_TPMCR : 0),
> MDCR_EL2);
>
> /* Trap CP15 c15 used for implementation defined registers */
> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
> index 049f93f7aa..51eb3d770b 100644
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -39,7 +39,7 @@
> #include <xen/err.h>
>
> /* opt_sched: scheduler - default to configured value */
> -static char __initdata opt_sched[10] = CONFIG_SCHED_DEFAULT;
> +char __initdata opt_sched[10] = CONFIG_SCHED_DEFAULT;
> string_param("sched", opt_sched);
>
> /* if sched_smt_power_savings is set,
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index b73ccbdf3a..c40a1b5dbc 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -906,6 +906,7 @@ static inline bool is_vcpu_online(const struct vcpu *v)
> }
>
> extern bool sched_smt_power_savings;
> +extern char opt_sched[10];
>
> extern enum cpufreq_controller {
> FREQCTL_none, FREQCTL_dom0_kernel, FREQCTL_xen
>
Cheers,
[1] <alpine.DEB.2.10.1809121559330.4255@sstabellini-ThinkPad-X260> "Null
scheduler bug"
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
>>> On 01.06.19 at 01:00, <sstabellini@kernel.org> wrote:
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -118,6 +118,28 @@ static int __init parse_vwfi(const char *s)
> }
> custom_param("vwfi", parse_vwfi);
>
> +static bool debug_registers_trap = true;
> +static bool perf_counters_trap = true;
> +
> +static int __init opt_static_partitioning(const char *s)
> +{
> + if ( strcmp(s, "true") &&
> + strcmp(s, "True") &&
> + strcmp(s, "1") )
> + return 0;
> +
> + vwfi = NATIVE;
> + debug_registers_trap = false;
> + perf_counters_trap = false;
> + memcpy(opt_sched, "null", 5);
> +
> + /* Disable Trap Debug and Performance Monitor now for CPU0 */
> + WRITE_SYSREG(HDCR_TDRA, MDCR_EL2);
> +
> + return 0;
> +}
> +custom_param("static_partitioning", opt_static_partitioning);
Please no underscores anymore in new option names, at least until
we perhaps implement string comparison to treat underscore and
dash as matching up.
> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -39,7 +39,7 @@
> #include <xen/err.h>
>
> /* opt_sched: scheduler - default to configured value */
> -static char __initdata opt_sched[10] = CONFIG_SCHED_DEFAULT;
> +char __initdata opt_sched[10] = CONFIG_SCHED_DEFAULT;
> string_param("sched", opt_sched);
>
> /* if sched_smt_power_savings is set,
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -906,6 +906,7 @@ static inline bool is_vcpu_online(const struct vcpu *v)
> }
>
> extern bool sched_smt_power_savings;
> +extern char opt_sched[10];
This, imo, is a layering violation, and hence should be solved in a
different way.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2024 Red Hat, Inc.