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(-)

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");
-- 
2.21.3

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