| Message ID | 20200518155308.15851-7-f4bug@amsat.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | exec/memory: Enforce checking MemTxResult values | expand |
On Mon, 18 May 2020 at 16:53, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > Give the hypervisor a possibility to catch any error > occuring during KVM_EXIT_MMIO. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > RFC because maybe we simply want to ignore this error instead The "right" answer is that the kernel should enhance the KVM_EXIT_MMIO API to allow userspace to say "sorry, you got a bus error on that memory access the guest just tried" (which the kernel then has to turn into an appropriate guest exception, or ignore, depending on what the architecture requires.) You don't want to set ret to non-zero here, because that will cause us to VM_STOP, and I suspect that x86 at least is relying on the implict RAZ/WI behaviour it currently gets. thanks -- PMM
On 5/18/20 6:01 PM, Peter Maydell wrote: > On Mon, 18 May 2020 at 16:53, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> >> Give the hypervisor a possibility to catch any error >> occuring during KVM_EXIT_MMIO. >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> RFC because maybe we simply want to ignore this error instead > > The "right" answer is that the kernel should enhance the KVM_EXIT_MMIO > API to allow userspace to say "sorry, you got a bus error on that > memory access the guest just tried" (which the kernel then has to > turn into an appropriate guest exception, or ignore, depending on > what the architecture requires.) You don't want to set ret to > non-zero here, because that will cause us to VM_STOP, and I > suspect that x86 at least is relying on the implict RAZ/WI > behaviour it currently gets. OK, similar to the worst case I expected here. Thank you for the clear explanation :) > > thanks > -- PMM >
On 18/05/20 18:01, Peter Maydell wrote: > The "right" answer is that the kernel should enhance the KVM_EXIT_MMIO > API to allow userspace to say "sorry, you got a bus error on that > memory access the guest just tried" (which the kernel then has to > turn into an appropriate guest exception, or ignore, depending on > what the architecture requires.) You don't want to set ret to > non-zero here, because that will cause us to VM_STOP, and I > suspect that x86 at least is relying on the implict RAZ/WI > behaviour it currently gets. Yes, it is. It may even be already possible to inject the right exception (on ARM) through KVM_SET_VCPU_EVENTS or something like that, too. Paolo
On Thu, 21 May 2020 at 16:39, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 18/05/20 18:01, Peter Maydell wrote: > > The "right" answer is that the kernel should enhance the KVM_EXIT_MMIO > > API to allow userspace to say "sorry, you got a bus error on that > > memory access the guest just tried" (which the kernel then has to > > turn into an appropriate guest exception, or ignore, depending on > > what the architecture requires.) You don't want to set ret to > > non-zero here, because that will cause us to VM_STOP, and I > > suspect that x86 at least is relying on the implict RAZ/WI > > behaviour it currently gets. > > Yes, it is. It may even be already possible to inject the right > exception (on ARM) through KVM_SET_VCPU_EVENTS or something like that, too. Yeah, in theory we could deliver an exception from userspace by updating all the register state, but I think the kernel really ought to do it both (a) because it's just a neater API to do it that way round and (b) because the kernel is the one that has the info about the faulting insn that it might need for things like setting up a syndrome register value. thanks -- PMM
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index d06cc04079..8dbcb8fda3 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -2357,6 +2357,7 @@ int kvm_cpu_exec(CPUState *cpu) do { MemTxAttrs attrs; + MemTxResult res; if (cpu->vcpu_dirty) { kvm_arch_put_registers(cpu, KVM_PUT_RUNTIME_STATE); @@ -2429,12 +2430,12 @@ int kvm_cpu_exec(CPUState *cpu) case KVM_EXIT_MMIO: DPRINTF("handle_mmio\n"); /* Called outside BQL */ - address_space_rw(&address_space_memory, - run->mmio.phys_addr, attrs, - run->mmio.data, - run->mmio.len, - run->mmio.is_write); - ret = 0; + res = address_space_rw(&address_space_memory, + run->mmio.phys_addr, attrs, + run->mmio.data, + run->mmio.len, + run->mmio.is_write); + ret = res == MEMTX_OK ? 0 : -1; break; case KVM_EXIT_IRQ_WINDOW_OPEN: DPRINTF("irq_window_open\n");
Give the hypervisor a possibility to catch any error occuring during KVM_EXIT_MMIO. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- RFC because maybe we simply want to ignore this error instead --- accel/kvm/kvm-all.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)