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 <npiggin@xxxxxxxxx>
>
> Signed-off-by: Fabiano Rosas <farosas@xxxxxxxxxxxxx>
> Reviewed-by: Alexey Kardashevskiy <aik@xxxxxxxxx>
> ---
> 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
