Re: [PATCH linux 1/2] xen: delay xen_hvm_init_time_ops() if kdump is boot on vcpu>=32
Hi Boris,
On 10/12/21 10:17 AM, Boris Ostrovsky wrote:
>
> On 10/12/21 3:24 AM, Dongli Zhang wrote:
>> The sched_clock() can be used very early since upstream
>> commit 857baa87b642 ("sched/clock: Enable sched clock early"). In addition,
>> with upstream commit 38669ba205d1 ("x86/xen/time: Output xen sched_clock
>> time from 0"), kdump kernel in Xen HVM guest may panic at very early stage
>> when accessing &__this_cpu_read(xen_vcpu)->time as in below:
>
>
> Please drop "upstream". It's always upstream here.
>
>
>> +
>> + /*
>> + * Only MAX_VIRT_CPUS 'vcpu_info' are embedded inside 'shared_info'
>> + * and the VM would use them until xen_vcpu_setup() is used to
>> + * allocate/relocate them at arbitrary address.
>> + *
>> + * However, when Xen HVM guest panic on vcpu >= MAX_VIRT_CPUS,
>> + * per_cpu(xen_vcpu, cpu) is still NULL at this stage. To access
>> + * per_cpu(xen_vcpu, cpu) via xen_clocksource_read() would panic.
>> + *
>> + * Therefore we delay xen_hvm_init_time_ops() to
>> + * xen_hvm_smp_prepare_boot_cpu() when boot vcpu is >= MAX_VIRT_CPUS.
>> + */
>> + if (xen_vcpu_nr(0) >= MAX_VIRT_CPUS)
>
>
> What about always deferring this when panicing? Would that work?
>
>
> Deciding whether to defer based on cpu number feels a bit awkward.
>
>
> -boris
>
I did some tests and I do not think this works well. I prefer to delay the
initialization only for VCPU >= 32.
This is the syslog if we always delay xen_hvm_init_time_ops(), regardless
whether VCPU >= 32.
[0.032372] Booting paravirtualized kernel on Xen HVM
[0.032376] clocksource: refined-jiffies: mask: 0x max_cycles:
0x, max_idle_ns: 1910969940391419 ns
[0.037683] setup_percpu: NR_CPUS:64 nr_cpumask_bits:64 nr_cpu_ids:64
nr_node_ids:2
[0.041876] percpu: Embedded 49 pages/cpu s162968 r8192 d29544 u262144
--> There is a clock backwards from 0.041876 to 0.10.
[0.10] Built 2 zonelists, mobility grouping on. Total pages: 2015744
[0.12] Policy zone: Normal
[0.14] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-5.15.0-rc6xen+
root=UUID=2a5975ab-a059-4697-9aee-7a53ddfeea21 ro text console=ttyS0,115200n8
console=tty1 crashkernel=512M-:192M
This is because the initial pv_sched_clock is native_sched_clock(), and it
switches to xen_sched_clock() in xen_hvm_init_time_ops(). Is it fine to always
have a clock backward for non-kdump kernel?
To avoid the clock backward, we may register a dummy clocksource which always
returns 0, before xen_hvm_init_time_ops(). I do not think this is reasonable.
Thank you very much!
Dongli Zhang
Re: [PATCH linux 1/2] xen: delay xen_hvm_init_time_ops() if kdump is boot on vcpu>=32
On 10/12/21 3:24 AM, Dongli Zhang wrote:
The sched_clock() can be used very early since upstream
commit 857baa87b642 ("sched/clock: Enable sched clock early"). In addition,
with upstream commit 38669ba205d1 ("x86/xen/time: Output xen sched_clock
time from 0"), kdump kernel in Xen HVM guest may panic at very early stage
when accessing &__this_cpu_read(xen_vcpu)->time as in below:
Please drop "upstream". It's always upstream here.
+
+ /*
+* Only MAX_VIRT_CPUS 'vcpu_info' are embedded inside 'shared_info'
+* and the VM would use them until xen_vcpu_setup() is used to
+* allocate/relocate them at arbitrary address.
+*
+* However, when Xen HVM guest panic on vcpu >= MAX_VIRT_CPUS,
+* per_cpu(xen_vcpu, cpu) is still NULL at this stage. To access
+* per_cpu(xen_vcpu, cpu) via xen_clocksource_read() would panic.
+*
+* Therefore we delay xen_hvm_init_time_ops() to
+* xen_hvm_smp_prepare_boot_cpu() when boot vcpu is >= MAX_VIRT_CPUS.
+*/
+ if (xen_vcpu_nr(0) >= MAX_VIRT_CPUS)
What about always deferring this when panicing? Would that work?
Deciding whether to defer based on cpu number feels a bit awkward.
-boris
+ pr_info("Delay xen_hvm_init_time_ops() as kernel is running on
vcpu=%d\n",
+ xen_vcpu_nr(0));
+ else
+ xen_hvm_init_time_ops();
+
xen_hvm_init_mmu_ops();
Re: [PATCH linux 1/2] xen: delay xen_hvm_init_time_ops() if kdump is boot on vcpu>=32
On 12.10.21 09:24, Dongli Zhang wrote:
The sched_clock() can be used very early since upstream
commit 857baa87b642 ("sched/clock: Enable sched clock early"). In addition,
with upstream commit 38669ba205d1 ("x86/xen/time: Output xen sched_clock
time from 0"), kdump kernel in Xen HVM guest may panic at very early stage
when accessing &__this_cpu_read(xen_vcpu)->time as in below:
setup_arch()
-> init_hypervisor_platform()
-> x86_init.hyper.init_platform = xen_hvm_guest_init()
-> xen_hvm_init_time_ops()
-> xen_clocksource_read()
-> src = &__this_cpu_read(xen_vcpu)->time;
This is because Xen HVM supports at most MAX_VIRT_CPUS=32 'vcpu_info'
embedded inside 'shared_info' during early stage until xen_vcpu_setup() is
used to allocate/relocate 'vcpu_info' for boot cpu at arbitrary address.
However, when Xen HVM guest panic on vcpu >= 32, since
xen_vcpu_info_reset(0) would set per_cpu(xen_vcpu, cpu) = NULL when
vcpu >= 32, xen_clocksource_read() on vcpu >= 32 would panic.
This patch delays xen_hvm_init_time_ops() to later in
xen_hvm_smp_prepare_boot_cpu() after the 'vcpu_info' for boot vcpu is
registered when the boot vcpu is >= 32.
This issue can be reproduced on purpose via below command at the guest
side when kdump/kexec is enabled:
"taskset -c 33 echo c > /proc/sysrq-trigger"
Cc: Joe Jin
Signed-off-by: Dongli Zhang
---
arch/x86/xen/enlighten_hvm.c | 20 +++-
arch/x86/xen/smp_hvm.c | 3 +++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index e68ea5f4ad1c..152279416d9a 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -216,7 +216,25 @@ static void __init xen_hvm_guest_init(void)
WARN_ON(xen_cpuhp_setup(xen_cpu_up_prepare_hvm, xen_cpu_dead_hvm));
xen_unplug_emulated_devices();
x86_init.irqs.intr_init = xen_init_IRQ;
- xen_hvm_init_time_ops();
+
+ /*
+* Only MAX_VIRT_CPUS 'vcpu_info' are embedded inside 'shared_info'
+* and the VM would use them until xen_vcpu_setup() is used to
+* allocate/relocate them at arbitrary address.
+*
+* However, when Xen HVM guest panic on vcpu >= MAX_VIRT_CPUS,
+* per_cpu(xen_vcpu, cpu) is still NULL at this stage. To access
+* per_cpu(xen_vcpu, cpu) via xen_clocksource_read() would panic.
+*
+* Therefore we delay xen_hvm_init_time_ops() to
+* xen_hvm_smp_prepare_boot_cpu() when boot vcpu is >= MAX_VIRT_CPUS.
+*/
+ if (xen_vcpu_nr(0) >= MAX_VIRT_CPUS)
+ pr_info("Delay xen_hvm_init_time_ops() as kernel is running on
vcpu=%d\n",
+ xen_vcpu_nr(0));
+ else
+ xen_hvm_init_time_ops();
+
xen_hvm_init_mmu_ops();
#ifdef CONFIG_KEXEC_CORE
diff --git a/arch/x86/xen/smp_hvm.c b/arch/x86/xen/smp_hvm.c
index 6ff3c887e0b9..60cd4fafd188 100644
--- a/arch/x86/xen/smp_hvm.c
+++ b/arch/x86/xen/smp_hvm.c
@@ -19,6 +19,9 @@ static void __init xen_hvm_smp_prepare_boot_cpu(void)
*/
xen_vcpu_setup(0);
+ if (xen_vcpu_nr(0) >= MAX_VIRT_CPUS)
+ xen_hvm_init_time_ops();
+
Please add a comment referencing the related code in
xen_hvm_guest_init().
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
[PATCH linux 1/2] xen: delay xen_hvm_init_time_ops() if kdump is boot on vcpu>=32
The sched_clock() can be used very early since upstream
commit 857baa87b642 ("sched/clock: Enable sched clock early"). In addition,
with upstream commit 38669ba205d1 ("x86/xen/time: Output xen sched_clock
time from 0"), kdump kernel in Xen HVM guest may panic at very early stage
when accessing &__this_cpu_read(xen_vcpu)->time as in below:
setup_arch()
-> init_hypervisor_platform()
-> x86_init.hyper.init_platform = xen_hvm_guest_init()
-> xen_hvm_init_time_ops()
-> xen_clocksource_read()
-> src = &__this_cpu_read(xen_vcpu)->time;
This is because Xen HVM supports at most MAX_VIRT_CPUS=32 'vcpu_info'
embedded inside 'shared_info' during early stage until xen_vcpu_setup() is
used to allocate/relocate 'vcpu_info' for boot cpu at arbitrary address.
However, when Xen HVM guest panic on vcpu >= 32, since
xen_vcpu_info_reset(0) would set per_cpu(xen_vcpu, cpu) = NULL when
vcpu >= 32, xen_clocksource_read() on vcpu >= 32 would panic.
This patch delays xen_hvm_init_time_ops() to later in
xen_hvm_smp_prepare_boot_cpu() after the 'vcpu_info' for boot vcpu is
registered when the boot vcpu is >= 32.
This issue can be reproduced on purpose via below command at the guest
side when kdump/kexec is enabled:
"taskset -c 33 echo c > /proc/sysrq-trigger"
Cc: Joe Jin
Signed-off-by: Dongli Zhang
---
arch/x86/xen/enlighten_hvm.c | 20 +++-
arch/x86/xen/smp_hvm.c | 3 +++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
index e68ea5f4ad1c..152279416d9a 100644
--- a/arch/x86/xen/enlighten_hvm.c
+++ b/arch/x86/xen/enlighten_hvm.c
@@ -216,7 +216,25 @@ static void __init xen_hvm_guest_init(void)
WARN_ON(xen_cpuhp_setup(xen_cpu_up_prepare_hvm, xen_cpu_dead_hvm));
xen_unplug_emulated_devices();
x86_init.irqs.intr_init = xen_init_IRQ;
- xen_hvm_init_time_ops();
+
+ /*
+* Only MAX_VIRT_CPUS 'vcpu_info' are embedded inside 'shared_info'
+* and the VM would use them until xen_vcpu_setup() is used to
+* allocate/relocate them at arbitrary address.
+*
+* However, when Xen HVM guest panic on vcpu >= MAX_VIRT_CPUS,
+* per_cpu(xen_vcpu, cpu) is still NULL at this stage. To access
+* per_cpu(xen_vcpu, cpu) via xen_clocksource_read() would panic.
+*
+* Therefore we delay xen_hvm_init_time_ops() to
+* xen_hvm_smp_prepare_boot_cpu() when boot vcpu is >= MAX_VIRT_CPUS.
+*/
+ if (xen_vcpu_nr(0) >= MAX_VIRT_CPUS)
+ pr_info("Delay xen_hvm_init_time_ops() as kernel is running on
vcpu=%d\n",
+ xen_vcpu_nr(0));
+ else
+ xen_hvm_init_time_ops();
+
xen_hvm_init_mmu_ops();
#ifdef CONFIG_KEXEC_CORE
diff --git a/arch/x86/xen/smp_hvm.c b/arch/x86/xen/smp_hvm.c
index 6ff3c887e0b9..60cd4fafd188 100644
--- a/arch/x86/xen/smp_hvm.c
+++ b/arch/x86/xen/smp_hvm.c
@@ -19,6 +19,9 @@ static void __init xen_hvm_smp_prepare_boot_cpu(void)
*/
xen_vcpu_setup(0);
+ if (xen_vcpu_nr(0) >= MAX_VIRT_CPUS)
+ xen_hvm_init_time_ops();
+
/*
* The alternative logic (which patches the unlock/lock) runs before
* the smp bootup up code is activated. Hence we need to set this up
--
2.17.1
