* [PATCH v2 0/2] KVM/arm64: sys_reg_table_init() small improvements @ 2022-04-28 10:34 ` Alexandru Elisei 0 siblings, 0 replies; 8+ messages in thread From: Alexandru Elisei @ 2022-04-28 10:34 UTC (permalink / raw) To: maz, james.morse, suzuki.poulose, linux-arm-kernel, kvmarm These are two small improvements to how KVM handles an out-of-order sys_reg_desc table. These should only affect KVM developers, as the end user should never see an error caused by an unsorted sys_reg_desc table. Changes in v2: * Tweaked how the error is detected and propagated in kvm_arch_init(). * Use %pS to print the table name and entry offset (Marc). * Tweaked the error message to spell out that magic number refers to the offending entry. Alexandru Elisei (2): KVM/arm64: Don't BUG_ON() if emulated register table is unsorted KVM/arm64: Print emulated register table name when it is unsorted arch/arm64/include/asm/kvm_host.h | 2 +- arch/arm64/kvm/arm.c | 8 +++++-- arch/arm64/kvm/sys_regs.c | 35 ++++++++++++++++++------------- 3 files changed, 27 insertions(+), 18 deletions(-) -- 2.36.0 _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 0/2] KVM/arm64: sys_reg_table_init() small improvements @ 2022-04-28 10:34 ` Alexandru Elisei 0 siblings, 0 replies; 8+ messages in thread From: Alexandru Elisei @ 2022-04-28 10:34 UTC (permalink / raw) To: maz, james.morse, suzuki.poulose, linux-arm-kernel, kvmarm These are two small improvements to how KVM handles an out-of-order sys_reg_desc table. These should only affect KVM developers, as the end user should never see an error caused by an unsorted sys_reg_desc table. Changes in v2: * Tweaked how the error is detected and propagated in kvm_arch_init(). * Use %pS to print the table name and entry offset (Marc). * Tweaked the error message to spell out that magic number refers to the offending entry. Alexandru Elisei (2): KVM/arm64: Don't BUG_ON() if emulated register table is unsorted KVM/arm64: Print emulated register table name when it is unsorted arch/arm64/include/asm/kvm_host.h | 2 +- arch/arm64/kvm/arm.c | 8 +++++-- arch/arm64/kvm/sys_regs.c | 35 ++++++++++++++++++------------- 3 files changed, 27 insertions(+), 18 deletions(-) -- 2.36.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] KVM/arm64: Don't BUG_ON() if emulated register table is unsorted 2022-04-28 10:34 ` Alexandru Elisei @ 2022-04-28 10:34 ` Alexandru Elisei -1 siblings, 0 replies; 8+ messages in thread From: Alexandru Elisei @ 2022-04-28 10:34 UTC (permalink / raw) To: maz, james.morse, suzuki.poulose, linux-arm-kernel, kvmarm To emulate a register access, KVM uses a table of registers sorted by register encoding to speed up queries using binary search. When Linux boots, KVM checks that the table is sorted and uses a BUG_ON() statement to let the user know if it's not. The unfortunate side effect is that an unsorted sysreg table brings down the whole kernel, not just KVM, even though the rest of the kernel can function just fine without KVM. To make matters worse, on machines which lack a serial console, the user is left pondering why the machine is taking so long to boot. Improve this situation by returning an error from kvm_arch_init() if the sysreg tables are not in the correct order. The machine is still very much usable for the user, with the exception of virtualization, who can now easily determine what went wrong. A minor typo has also been corrected in the check_sysreg_table() function. Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- arch/arm64/include/asm/kvm_host.h | 2 +- arch/arm64/kvm/arm.c | 8 ++++++-- arch/arm64/kvm/sys_regs.c | 33 ++++++++++++++++++------------- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 94a27a7520f4..39ca91cc1f01 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -686,7 +686,7 @@ int kvm_handle_sys_reg(struct kvm_vcpu *vcpu); void kvm_reset_sys_regs(struct kvm_vcpu *vcpu); -void kvm_sys_reg_table_init(void); +int kvm_sys_reg_table_init(void); /* MMIO helpers */ void kvm_mmio_write_buf(void *buf, unsigned int len, unsigned long data); diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 523bc934fe2f..ab365358747a 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -1761,8 +1761,6 @@ static int init_subsystems(void) kvm_register_perf_callbacks(NULL); - kvm_sys_reg_table_init(); - out: if (err || !is_protected_kvm_enabled()) on_each_cpu(_kvm_arch_hardware_disable, NULL, 1); @@ -2089,6 +2087,12 @@ int kvm_arch_init(void *opaque) return -ENODEV; } + err = kvm_sys_reg_table_init(); + if (err) { + kvm_info("Error initializing system register tables"); + return err; + } + in_hyp_mode = is_kernel_in_hyp_mode(); if (cpus_have_final_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) || diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 7b45c040cc27..57302048afd0 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2187,25 +2187,24 @@ static const struct sys_reg_desc cp15_64_regs[] = { { SYS_DESC(SYS_AARCH32_CNTP_CVAL), access_arch_timer }, }; -static int check_sysreg_table(const struct sys_reg_desc *table, unsigned int n, - bool is_32) +static bool check_sysreg_table(const struct sys_reg_desc *table, unsigned int n, + bool is_32) { unsigned int i; for (i = 0; i < n; i++) { if (!is_32 && table[i].reg && !table[i].reset) { - kvm_err("sys_reg table %p entry %d has lacks reset\n", - table, i); - return 1; + kvm_err("sys_reg table %p entry %d lacks reset\n", table, i); + return false; } if (i && cmp_sys_reg(&table[i-1], &table[i]) >= 0) { kvm_err("sys_reg table %p out of order (%d)\n", table, i - 1); - return 1; + return false; } } - return 0; + return true; } int kvm_handle_cp14_load_store(struct kvm_vcpu *vcpu) @@ -2860,18 +2859,22 @@ int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices) return write_demux_regids(uindices); } -void kvm_sys_reg_table_init(void) +int kvm_sys_reg_table_init(void) { + bool valid = true; unsigned int i; struct sys_reg_desc clidr; /* Make sure tables are unique and in order. */ - BUG_ON(check_sysreg_table(sys_reg_descs, ARRAY_SIZE(sys_reg_descs), false)); - BUG_ON(check_sysreg_table(cp14_regs, ARRAY_SIZE(cp14_regs), true)); - BUG_ON(check_sysreg_table(cp14_64_regs, ARRAY_SIZE(cp14_64_regs), true)); - BUG_ON(check_sysreg_table(cp15_regs, ARRAY_SIZE(cp15_regs), true)); - BUG_ON(check_sysreg_table(cp15_64_regs, ARRAY_SIZE(cp15_64_regs), true)); - BUG_ON(check_sysreg_table(invariant_sys_regs, ARRAY_SIZE(invariant_sys_regs), false)); + valid &= check_sysreg_table(sys_reg_descs, ARRAY_SIZE(sys_reg_descs), false); + valid &= check_sysreg_table(cp14_regs, ARRAY_SIZE(cp14_regs), true); + valid &= check_sysreg_table(cp14_64_regs, ARRAY_SIZE(cp14_64_regs), true); + valid &= check_sysreg_table(cp15_regs, ARRAY_SIZE(cp15_regs), true); + valid &= check_sysreg_table(cp15_64_regs, ARRAY_SIZE(cp15_64_regs), true); + valid &= check_sysreg_table(invariant_sys_regs, ARRAY_SIZE(invariant_sys_regs), false); + + if (!valid) + return -EINVAL; /* We abuse the reset function to overwrite the table itself. */ for (i = 0; i < ARRAY_SIZE(invariant_sys_regs); i++) @@ -2894,4 +2897,6 @@ void kvm_sys_reg_table_init(void) break; /* Clear all higher bits. */ cache_levels &= (1 << (i*3))-1; + + return 0; } -- 2.36.0 _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] KVM/arm64: Don't BUG_ON() if emulated register table is unsorted @ 2022-04-28 10:34 ` Alexandru Elisei 0 siblings, 0 replies; 8+ messages in thread From: Alexandru Elisei @ 2022-04-28 10:34 UTC (permalink / raw) To: maz, james.morse, suzuki.poulose, linux-arm-kernel, kvmarm To emulate a register access, KVM uses a table of registers sorted by register encoding to speed up queries using binary search. When Linux boots, KVM checks that the table is sorted and uses a BUG_ON() statement to let the user know if it's not. The unfortunate side effect is that an unsorted sysreg table brings down the whole kernel, not just KVM, even though the rest of the kernel can function just fine without KVM. To make matters worse, on machines which lack a serial console, the user is left pondering why the machine is taking so long to boot. Improve this situation by returning an error from kvm_arch_init() if the sysreg tables are not in the correct order. The machine is still very much usable for the user, with the exception of virtualization, who can now easily determine what went wrong. A minor typo has also been corrected in the check_sysreg_table() function. Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- arch/arm64/include/asm/kvm_host.h | 2 +- arch/arm64/kvm/arm.c | 8 ++++++-- arch/arm64/kvm/sys_regs.c | 33 ++++++++++++++++++------------- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 94a27a7520f4..39ca91cc1f01 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -686,7 +686,7 @@ int kvm_handle_sys_reg(struct kvm_vcpu *vcpu); void kvm_reset_sys_regs(struct kvm_vcpu *vcpu); -void kvm_sys_reg_table_init(void); +int kvm_sys_reg_table_init(void); /* MMIO helpers */ void kvm_mmio_write_buf(void *buf, unsigned int len, unsigned long data); diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 523bc934fe2f..ab365358747a 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -1761,8 +1761,6 @@ static int init_subsystems(void) kvm_register_perf_callbacks(NULL); - kvm_sys_reg_table_init(); - out: if (err || !is_protected_kvm_enabled()) on_each_cpu(_kvm_arch_hardware_disable, NULL, 1); @@ -2089,6 +2087,12 @@ int kvm_arch_init(void *opaque) return -ENODEV; } + err = kvm_sys_reg_table_init(); + if (err) { + kvm_info("Error initializing system register tables"); + return err; + } + in_hyp_mode = is_kernel_in_hyp_mode(); if (cpus_have_final_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) || diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 7b45c040cc27..57302048afd0 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2187,25 +2187,24 @@ static const struct sys_reg_desc cp15_64_regs[] = { { SYS_DESC(SYS_AARCH32_CNTP_CVAL), access_arch_timer }, }; -static int check_sysreg_table(const struct sys_reg_desc *table, unsigned int n, - bool is_32) +static bool check_sysreg_table(const struct sys_reg_desc *table, unsigned int n, + bool is_32) { unsigned int i; for (i = 0; i < n; i++) { if (!is_32 && table[i].reg && !table[i].reset) { - kvm_err("sys_reg table %p entry %d has lacks reset\n", - table, i); - return 1; + kvm_err("sys_reg table %p entry %d lacks reset\n", table, i); + return false; } if (i && cmp_sys_reg(&table[i-1], &table[i]) >= 0) { kvm_err("sys_reg table %p out of order (%d)\n", table, i - 1); - return 1; + return false; } } - return 0; + return true; } int kvm_handle_cp14_load_store(struct kvm_vcpu *vcpu) @@ -2860,18 +2859,22 @@ int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices) return write_demux_regids(uindices); } -void kvm_sys_reg_table_init(void) +int kvm_sys_reg_table_init(void) { + bool valid = true; unsigned int i; struct sys_reg_desc clidr; /* Make sure tables are unique and in order. */ - BUG_ON(check_sysreg_table(sys_reg_descs, ARRAY_SIZE(sys_reg_descs), false)); - BUG_ON(check_sysreg_table(cp14_regs, ARRAY_SIZE(cp14_regs), true)); - BUG_ON(check_sysreg_table(cp14_64_regs, ARRAY_SIZE(cp14_64_regs), true)); - BUG_ON(check_sysreg_table(cp15_regs, ARRAY_SIZE(cp15_regs), true)); - BUG_ON(check_sysreg_table(cp15_64_regs, ARRAY_SIZE(cp15_64_regs), true)); - BUG_ON(check_sysreg_table(invariant_sys_regs, ARRAY_SIZE(invariant_sys_regs), false)); + valid &= check_sysreg_table(sys_reg_descs, ARRAY_SIZE(sys_reg_descs), false); + valid &= check_sysreg_table(cp14_regs, ARRAY_SIZE(cp14_regs), true); + valid &= check_sysreg_table(cp14_64_regs, ARRAY_SIZE(cp14_64_regs), true); + valid &= check_sysreg_table(cp15_regs, ARRAY_SIZE(cp15_regs), true); + valid &= check_sysreg_table(cp15_64_regs, ARRAY_SIZE(cp15_64_regs), true); + valid &= check_sysreg_table(invariant_sys_regs, ARRAY_SIZE(invariant_sys_regs), false); + + if (!valid) + return -EINVAL; /* We abuse the reset function to overwrite the table itself. */ for (i = 0; i < ARRAY_SIZE(invariant_sys_regs); i++) @@ -2894,4 +2897,6 @@ void kvm_sys_reg_table_init(void) break; /* Clear all higher bits. */ cache_levels &= (1 << (i*3))-1; + + return 0; } -- 2.36.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] KVM/arm64: Print emulated register table name when it is unsorted 2022-04-28 10:34 ` Alexandru Elisei @ 2022-04-28 10:34 ` Alexandru Elisei -1 siblings, 0 replies; 8+ messages in thread From: Alexandru Elisei @ 2022-04-28 10:34 UTC (permalink / raw) To: maz, james.morse, suzuki.poulose, linux-arm-kernel, kvmarm When a sysreg table entry is out-of-order, KVM attempts to print the address of the table: [ 0.143911] kvm [1]: sys_reg table (____ptrval____) out of order (1) Printing the name of the table instead of a pointer is more helpful in this case. The message has also been slightly tweaked to be point out the offending entry (and to match the missing reset error message): [ 0.143891] kvm [1]: sys_reg table sys_reg_descs+0x50/0x7490 entry 1 out of order Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- arch/arm64/kvm/sys_regs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 57302048afd0..a84a556eeb8f 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2194,12 +2194,12 @@ static bool check_sysreg_table(const struct sys_reg_desc *table, unsigned int n, for (i = 0; i < n; i++) { if (!is_32 && table[i].reg && !table[i].reset) { - kvm_err("sys_reg table %p entry %d lacks reset\n", table, i); + kvm_err("sys_reg table %pS entry %d lacks reset\n", &table[i], i); return false; } if (i && cmp_sys_reg(&table[i-1], &table[i]) >= 0) { - kvm_err("sys_reg table %p out of order (%d)\n", table, i - 1); + kvm_err("sys_reg table %pS entry %d out of order\n", &table[i - 1], i - 1); return false; } } -- 2.36.0 _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] KVM/arm64: Print emulated register table name when it is unsorted @ 2022-04-28 10:34 ` Alexandru Elisei 0 siblings, 0 replies; 8+ messages in thread From: Alexandru Elisei @ 2022-04-28 10:34 UTC (permalink / raw) To: maz, james.morse, suzuki.poulose, linux-arm-kernel, kvmarm When a sysreg table entry is out-of-order, KVM attempts to print the address of the table: [ 0.143911] kvm [1]: sys_reg table (____ptrval____) out of order (1) Printing the name of the table instead of a pointer is more helpful in this case. The message has also been slightly tweaked to be point out the offending entry (and to match the missing reset error message): [ 0.143891] kvm [1]: sys_reg table sys_reg_descs+0x50/0x7490 entry 1 out of order Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- arch/arm64/kvm/sys_regs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 57302048afd0..a84a556eeb8f 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -2194,12 +2194,12 @@ static bool check_sysreg_table(const struct sys_reg_desc *table, unsigned int n, for (i = 0; i < n; i++) { if (!is_32 && table[i].reg && !table[i].reset) { - kvm_err("sys_reg table %p entry %d lacks reset\n", table, i); + kvm_err("sys_reg table %pS entry %d lacks reset\n", &table[i], i); return false; } if (i && cmp_sys_reg(&table[i-1], &table[i]) >= 0) { - kvm_err("sys_reg table %p out of order (%d)\n", table, i - 1); + kvm_err("sys_reg table %pS entry %d out of order\n", &table[i - 1], i - 1); return false; } } -- 2.36.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/2] KVM/arm64: sys_reg_table_init() small improvements 2022-04-28 10:34 ` Alexandru Elisei @ 2022-05-04 18:55 ` Marc Zyngier -1 siblings, 0 replies; 8+ messages in thread From: Marc Zyngier @ 2022-05-04 18:55 UTC (permalink / raw) To: Alexandru Elisei, linux-arm-kernel, james.morse, kvmarm, suzuki.poulose On Thu, 28 Apr 2022 11:34:03 +0100, Alexandru Elisei wrote: > These are two small improvements to how KVM handles an out-of-order > sys_reg_desc table. These should only affect KVM developers, as the end > user should never see an error caused by an unsorted sys_reg_desc table. > > Changes in v2: > > * Tweaked how the error is detected and propagated in kvm_arch_init(). > * Use %pS to print the table name and entry offset (Marc). > * Tweaked the error message to spell out that magic number refers to the > offending entry. > > [...] Applied to next, thanks! [1/2] KVM/arm64: Don't BUG_ON() if emulated register table is unsorted commit: f1f0c0cfeaa7c10eb536d9919bf3902af0e17bce [2/2] KVM/arm64: Print emulated register table name when it is unsorted commit: 325031d4f39cfb1adf41708c1329e60b9abfbdac Cheers, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/2] KVM/arm64: sys_reg_table_init() small improvements @ 2022-05-04 18:55 ` Marc Zyngier 0 siblings, 0 replies; 8+ messages in thread From: Marc Zyngier @ 2022-05-04 18:55 UTC (permalink / raw) To: Alexandru Elisei, linux-arm-kernel, james.morse, kvmarm, suzuki.poulose On Thu, 28 Apr 2022 11:34:03 +0100, Alexandru Elisei wrote: > These are two small improvements to how KVM handles an out-of-order > sys_reg_desc table. These should only affect KVM developers, as the end > user should never see an error caused by an unsorted sys_reg_desc table. > > Changes in v2: > > * Tweaked how the error is detected and propagated in kvm_arch_init(). > * Use %pS to print the table name and entry offset (Marc). > * Tweaked the error message to spell out that magic number refers to the > offending entry. > > [...] Applied to next, thanks! [1/2] KVM/arm64: Don't BUG_ON() if emulated register table is unsorted commit: f1f0c0cfeaa7c10eb536d9919bf3902af0e17bce [2/2] KVM/arm64: Print emulated register table name when it is unsorted commit: 325031d4f39cfb1adf41708c1329e60b9abfbdac Cheers, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-05-04 18:56 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-28 10:34 [PATCH v2 0/2] KVM/arm64: sys_reg_table_init() small improvements Alexandru Elisei 2022-04-28 10:34 ` Alexandru Elisei 2022-04-28 10:34 ` [PATCH v2 1/2] KVM/arm64: Don't BUG_ON() if emulated register table is unsorted Alexandru Elisei 2022-04-28 10:34 ` Alexandru Elisei 2022-04-28 10:34 ` [PATCH v2 2/2] KVM/arm64: Print emulated register table name when it " Alexandru Elisei 2022-04-28 10:34 ` Alexandru Elisei 2022-05-04 18:55 ` [PATCH v2 0/2] KVM/arm64: sys_reg_table_init() small improvements Marc Zyngier 2022-05-04 18:55 ` Marc Zyngier
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.