Re: [PATCH v3 6/6] KVM: PPC: mmio: Reject instructions that access more than mmio.data size
Excerpts from Fabiano Rosas's message of January 8, 2022 7:00 am:
> The MMIO interface between the kernel and userspace uses a structure
> that supports a maximum of 8-bytes of data. Instructions that access
> more than that need to be emulated in parts.
>
> We currently don't have generic support for splitting the emulation in
> parts and each set of instructions needs to be explicitly included.
>
> There's already an error message being printed when a load or store
> exceeds the mmio.data buffer but we don't fail the emulation until
> later at kvmppc_complete_mmio_load and even then we allow userspace to
> make a partial copy of the data, which ends up overwriting some fields
> of the mmio structure.
>
> This patch makes the emulation fail earlier at kvmppc_handle_load|store,
> which will send a Program interrupt to the guest. This is better than
> allowing the guest to proceed with partial data.
>
> Note that this was caught in a somewhat artificial scenario using
> quadword instructions (lq/stq), there's no account of an actual guest
> in the wild running instructions that are not properly emulated.
>
> (While here, fix the error message to check against 'bytes' and not
> 'run->mmio.len' which at this point has an old value.)
This looks good to me
Reviewed-by: Nicholas Piggin
>
> Signed-off-by: Fabiano Rosas
> Reviewed-by: Alexey Kardashevskiy
> ---
> arch/powerpc/kvm/powerpc.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 56b0faab7a5f..a1643ca988e0 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -1246,7 +1246,8 @@ static int __kvmppc_handle_load(struct kvm_vcpu *vcpu,
>
> if (bytes > sizeof(run->mmio.data)) {
> printk(KERN_ERR "%s: bad MMIO length: %d\n", __func__,
> -run->mmio.len);
> +bytes);
I wonder though this should probably be ratelimited, informational (or
at least warning because it's a host message), and perhaps a bit more
explanatory that it's a guest problem (or at least lack of host support
for particular guest MMIO sizes).
Thanks,
Nick
Re: [PATCH v3 5/6] KVM: PPC: mmio: Return to guest after emulation failure
Excerpts from Fabiano Rosas's message of January 8, 2022 7:00 am:
> If MMIO emulation fails we don't want to crash the whole guest by
> returning to userspace.
>
> The original commit bbf45ba57eae ("KVM: ppc: PowerPC 440 KVM
> implementation") added a todo:
>
> /* XXX Deliver Program interrupt to guest. */
>
> and later the commit d69614a295ae ("KVM: PPC: Separate loadstore
> emulation from priv emulation") added the Program interrupt injection
> but in another file, so I'm assuming it was missed that this block
> needed to be altered.
>
> Signed-off-by: Fabiano Rosas
> Reviewed-by: Alexey Kardashevskiy
> ---
> arch/powerpc/kvm/powerpc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 6daeea4a7de1..56b0faab7a5f 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -309,7 +309,7 @@ int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu)
> kvmppc_get_last_inst(vcpu, INST_GENERIC, _inst);
> kvmppc_core_queue_program(vcpu, 0);
> pr_info("%s: emulation failed (%08x)\n", __func__, last_inst);
> - r = RESUME_HOST;
> + r = RESUME_GUEST;
So at this point can the pr_info just go away?
I wonder if this shouldn't be a DSI rather than a program check.
DSI with DSISR[37] looks a bit more expected. Not that Linux
probably does much with it but at least it would give a SIGBUS
rather than SIGILL.
Thanks,
Nick
Re: [PATCH v3 4/6] KVM: PPC: mmio: Queue interrupt at kvmppc_emulate_mmio
Excerpts from Fabiano Rosas's message of January 8, 2022 7:00 am:
> If MMIO emulation fails, we queue a Program interrupt to the
> guest. Move that line up into kvmppc_emulate_mmio, which is where we
> set RESUME_GUEST/HOST. This allows the removal of the 'advance'
> variable.
>
> No functional change, just separation of responsibilities.
Looks cleaner.
Reviewed-by: Nicholas Piggin
>
> Signed-off-by: Fabiano Rosas
> ---
> arch/powerpc/kvm/emulate_loadstore.c | 8 +---
> arch/powerpc/kvm/powerpc.c | 2 +-
> 2 files changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/kvm/emulate_loadstore.c
> b/arch/powerpc/kvm/emulate_loadstore.c
> index 48272a9b9c30..4dec920fe4c9 100644
> --- a/arch/powerpc/kvm/emulate_loadstore.c
> +++ b/arch/powerpc/kvm/emulate_loadstore.c
> @@ -73,7 +73,6 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
> {
> u32 inst;
> enum emulation_result emulated = EMULATE_FAIL;
> - int advance = 1;
> struct instruction_op op;
>
> /* this default type might be overwritten by subcategories */
> @@ -355,15 +354,10 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
> }
> }
>
> - if (emulated == EMULATE_FAIL) {
> - advance = 0;
> - kvmppc_core_queue_program(vcpu, 0);
> - }
> -
> trace_kvm_ppc_instr(inst, kvmppc_get_pc(vcpu), emulated);
>
> /* Advance past emulated instruction. */
> - if (advance)
> + if (emulated != EMULATE_FAIL)
> kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
>
> return emulated;
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 4d7d0d080232..6daeea4a7de1 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -307,7 +307,7 @@ int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu)
> u32 last_inst;
>
> kvmppc_get_last_inst(vcpu, INST_GENERIC, _inst);
> - /* XXX Deliver Program interrupt to guest. */
> + kvmppc_core_queue_program(vcpu, 0);
> pr_info("%s: emulation failed (%08x)\n", __func__, last_inst);
> r = RESUME_HOST;
> break;
> --
> 2.33.1
>
>
Re: [PATCH v3 3/6] KVM: PPC: Don't use pr_emerg when mmio emulation fails
Excerpts from Fabiano Rosas's message of January 8, 2022 7:00 am:
> If MMIO emulation fails we deliver a Program interrupt to the
> guest. This is a normal event for the host, so use pr_info.
>
> Signed-off-by: Fabiano Rosas
> ---
Should it be rate limited to prevent guest spamming host log? In the
case of informational messages it's always good if it gives the
administrator some idea of what to do with it. If it's normal
for the host does it even need a message? If yes, then identifying which
guest and adding something like "(this might becaused by a bug in guest
driver)" would set the poor admin's mind at rest.
Thanks,
Nick
> arch/powerpc/kvm/powerpc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 92e552ab5a77..4d7d0d080232 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -308,7 +308,7 @@ int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu)
>
> kvmppc_get_last_inst(vcpu, INST_GENERIC, _inst);
> /* XXX Deliver Program interrupt to guest. */
> - pr_emerg("%s: emulation failed (%08x)\n", __func__, last_inst);
> + pr_info("%s: emulation failed (%08x)\n", __func__, last_inst);
> r = RESUME_HOST;
> break;
> }
> --
> 2.33.1
>
>
Re: [PATCH 00/13] powerpc/bpf: Some fixes and updates
Daniel Borkmann writes: > On 1/7/22 8:36 AM, Naveen N. Rao wrote: >> Daniel Borkmann wrote: >>> On 1/6/22 12:45 PM, Naveen N. Rao wrote: A set of fixes and updates to powerpc BPF JIT: - Patches 1-3 fix issues with the existing powerpc JIT and are tagged for -stable. - Patch 4 fixes a build issue with bpf selftests on powerpc. - Patches 5-9 handle some corner cases and make some small improvements. - Patches 10-13 optimize how function calls are handled in ppc64. Patches 7 and 8 were previously posted, and while patch 7 has no changes, patch 8 has been reworked to handle BPF_EXIT differently. >>> >>> Is the plan to route these via ppc trees? Fwiw, patch 1 and 4 look generic >>> and in general good to me, we could also take these two via bpf-next tree >>> given outside of arch/powerpc/? Whichever works best. >> >> Yes, I would like to route this through the powerpc tree. Though patches 1 >> and 4 are generic, they primarily affect powerpc and I do not see >> conflicting changes in bpf-next. Request you to please ack those patches so >> that Michael can take it through the powerpc tree. > > Ok, works for me. I presume this will end up in the upcoming merge window > anyway, so not too long time until we can sync these back to bpf/bpf-next > trees then. Hmm. This series landed a little late for me to get it into linux-next before the merge window opened. It's mostly small and includes some bug fixes, so I'm not saying it needs to wait for the next merge window, but I would like it to get some testing in linux-next before I ask Linus to pull it. When would you need it all merged into Linus' tree in order to sync up with the bpf tree for the next cycle? I assume as long as it's merged before rc1 that would be sufficient? cheers
Re: [PATCH v3 4/6] KVM: PPC: mmio: Queue interrupt at kvmppc_emulate_mmio
On 08/01/2022 08:00, Fabiano Rosas wrote:
If MMIO emulation fails, we queue a Program interrupt to the
guest. Move that line up into kvmppc_emulate_mmio, which is where we
set RESUME_GUEST/HOST. This allows the removal of the 'advance'
variable.
No functional change, just separation of responsibilities.
Signed-off-by: Fabiano Rosas
Reviewed-by: Alexey Kardashevskiy
---
arch/powerpc/kvm/emulate_loadstore.c | 8 +---
arch/powerpc/kvm/powerpc.c | 2 +-
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kvm/emulate_loadstore.c
b/arch/powerpc/kvm/emulate_loadstore.c
index 48272a9b9c30..4dec920fe4c9 100644
--- a/arch/powerpc/kvm/emulate_loadstore.c
+++ b/arch/powerpc/kvm/emulate_loadstore.c
@@ -73,7 +73,6 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
{
u32 inst;
enum emulation_result emulated = EMULATE_FAIL;
- int advance = 1;
struct instruction_op op;
/* this default type might be overwritten by subcategories */
@@ -355,15 +354,10 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
}
}
- if (emulated == EMULATE_FAIL) {
- advance = 0;
- kvmppc_core_queue_program(vcpu, 0);
- }
-
trace_kvm_ppc_instr(inst, kvmppc_get_pc(vcpu), emulated);
/* Advance past emulated instruction. */
- if (advance)
+ if (emulated != EMULATE_FAIL)
kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) + 4);
return emulated;
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 4d7d0d080232..6daeea4a7de1 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -307,7 +307,7 @@ int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu)
u32 last_inst;
kvmppc_get_last_inst(vcpu, INST_GENERIC, _inst);
- /* XXX Deliver Program interrupt to guest. */
+ kvmppc_core_queue_program(vcpu, 0);
pr_info("%s: emulation failed (%08x)\n", __func__, last_inst);
r = RESUME_HOST;
break;
Re: [PATCH]powerpc/xmon: Dump XIVE information for online-only processors.
On Wed, 05 Jan 2022 19:47:48 +0530, Sachin Sant wrote: > dxa command in XMON debugger iterates through all possible processors. > As a result, empty lines are printed even for processors which are not > online. > > CPU 47:pp=00 CPPR=ff IPI=0x0040002f PQ=-- EQ idx=699 T=0 > CPU 48: > CPU 49: > > [...] Applied to powerpc/next. [1/1] powerpc/xmon: Dump XIVE information for online-only processors. https://git.kernel.org/powerpc/c/f1aa0e47c29268776205698f2453dc07fab49855 cheers
Re: (subset) [PATCH V3 0/8] sched: Remove unused TASK_SIZE_OF for all archs
On Tue, 28 Dec 2021 14:47:21 +0800, guo...@kernel.org wrote: > From: Guo Ren > > This macro isn't used in Linux, now. Delete in include/linux/sched.h > and arch's include/asm. This would confuse people who are > implementing the COMPAT feature for architecture. > > Changes in v3: > - Fixup Documentation/process/submitting-patches.rst, add sender >Signed-off-by. > > [...] Applied to powerpc/next. [4/8] sched: powerpc: Remove unused TASK_SIZE_OF https://git.kernel.org/powerpc/c/08035a67f35a8765cac39bb12e56c61ee880227a cheers
Re: [PATCH] powerpc/opal: use default_groups in kobj_type
On Tue, 4 Jan 2022 17:13:18 +0100, Greg Kroah-Hartman wrote:
> There are currently 2 ways to create a set of sysfs files for a
> kobj_type, through the default_attrs field, and the default_groups
> field. Move the powerpc opal dump and elog sysfs code to use
> default_groups field which has been the preferred way since aa30f47cf666
> ("kobject: Add support for default attribute groups to kobj_type") so
> that we can soon get rid of the obsolete default_attrs field.
>
> [...]
Applied to powerpc/next.
[1/1] powerpc/opal: use default_groups in kobj_type
https://git.kernel.org/powerpc/c/32a1bda4b12a3d324bd585e1aa20dac824ab719c
cheers
Re: [PATCH] powerpc/cacheinfo: use default_groups in kobj_type
On Tue, 4 Jan 2022 16:54:50 +0100, Greg Kroah-Hartman wrote:
> There are currently 2 ways to create a set of sysfs files for a
> kobj_type, through the default_attrs field, and the default_groups
> field. Move the powerpc cacheinfo sysfs code to use default_groups
> field which has been the preferred way since aa30f47cf666 ("kobject: Add
> support for default attribute groups to kobj_type") so that we can soon
> get rid of the obsolete default_attrs field.
>
> [...]
Applied to powerpc/next.
[1/1] powerpc/cacheinfo: use default_groups in kobj_type
https://git.kernel.org/powerpc/c/2bdf3f9e9df0a4ce7709fc916b9997ca2dc30d25
cheers
Re: [PATCH] floppy: Remove usage of the deprecated "pci-dma-compat.h" API
On Sun, 2 Jan 2022 11:29:54 +0100, Christophe JAILLET wrote: > In [1], Christoph Hellwig has proposed to remove the wrappers in > include/linux/pci-dma-compat.h. > > Some reasons why this API should be removed have been given by Julia > Lawall in [2]. > > A coccinelle script has been used to perform the needed transformation > Only relevant parts are given below. > > [...] Applied to powerpc/next. [1/1] floppy: Remove usage of the deprecated "pci-dma-compat.h" API https://git.kernel.org/powerpc/c/e57c2fd6cdf8db581ac93b909b2664751e7cf30c cheers
Re: [PATCH] powerpc/xive: Add missing null check after calling kmalloc
On Sun, 26 Dec 2021 20:54:02 +0700, Ammar Faizi wrote:
> Commit 930914b7d528fc ("powerpc/xive: Add a debugfs file to dump
> internal XIVE state") forgot to add a null check.
>
> Add it.
>
>
Applied to powerpc/next.
[1/1] powerpc/xive: Add missing null check after calling kmalloc
https://git.kernel.org/powerpc/c/18dbfcdedc802f9500b2c29794f22a31d27639c0
cheers
Re: Linux kernel: powerpc: KVM guest can trigger host crash on Power8
Hi Michael! On 1/7/22 12:20, John Paul Adrian Glaubitz wrote: >> Can you separately test with (on the host): >> >> # echo 0 > /sys/module/kvm_hv/parameters/dynamic_mt_modes > > I'm trying to turn off "dynamic_mt_modes" first and see if that makes any > difference. > > I will report back. So far the machine is running stable now and the VM built gcc-9 without crashing the host. I will continue to monitor the machine and report back if it crashes, but it looks like this could be it. Adrian -- .''`. John Paul Adrian Glaubitz : :' : Debian Developer - glaub...@debian.org `. `' Freie Universitaet Berlin - glaub...@physik.fu-berlin.de `-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
[Bug 215470] New: Unable to boot NXP P2020 processor (freeze before any print to stdout)
https://bugzilla.kernel.org/show_bug.cgi?id=215470 Bug ID: 215470 Summary: Unable to boot NXP P2020 processor (freeze before any print to stdout) Product: Platform Specific/Hardware Version: 2.5 Kernel Version: >=v5.15-rc1 Hardware: PPC-32 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: PPC-32 Assignee: platform_ppc...@kernel-bugs.osdl.org Reporter: paweldembi...@gmail.com Regression: No Created attachment 300248 --> https://bugzilla.kernel.org/attachment.cgi?id=300248=edit my .config file Hello, I'm unable to boot kernel on NXP P2020 powerpc processor, image built from mainline tree after commit: https://lore.kernel.org/all/1fc81f07cabebb875b963e295408cc3dd38c8d85.1614674882.git.christophe.le...@csgroup.eu/ Workaround: After revert 1fc81f07cabe, it's start work again. My compiler: powerpc-openwrt-linux-gcc (OpenWrt GCC 11.2.0 r18180-95697901c8) 11.2.0, GNU ld (GNU Binutils) 2.37 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
