Re: [Qemu-devel] [PATCH v5 00/12] KVM Support for MIPS32 Processors

[James Hogan <james.hogan@imgtec.com>]

Hi Peter,

On 10/07/14 13:17, Peter Maydell wrote:
> On 17 June 2014 23:10, James Hogan <james.hogan@imgtec.com> wrote:
>> The patchset depends on v4 of "target-mips: implement UserLocal
>> Register". I'm aiming for QEMU 2.1, hopefully it isn't too late to get
>> some final review.
>>
>> Thanks to everybody who has already taken part in review.
>>
>> This patchset implements KVM support for MIPS32 processors, using Trap &
>> Emulation.
> 
> I was looking at what happens for MMIO accesses to nonexistent
> memory when we're running under KVM, and interestingly this
> patchset means MIPS is now the only CPU which both (a) supports
> KVM and (b) has an implementation of the do_unassigned_access()
> CPU hook. Does the current code support a guest attempt to
> access unassigned physical addresses? I had a look at the code
> and it seems like mips_cpu_unassigned_access() will end up
> calling cpu_restore_state() and cpu_loop_exit(), which I think
> will probably crash if you call them when using KVM rather than
> TCG...

Yes, I have observed this in the past when experimentally writing to the
Malta reset region from the guest (see the patch "mips/malta: allow
volatile writes to reset flash" which didn't get applied but worked
around the specific issue). That was because read only memory regions
were treated as unassigned from the point of view of writes (which tbh
seems wrong), but it could happen with any unassigned access >
0x80000000 from the guest AFAICT.

So yeh, until mips_cpu_unassigned_access does something more portable
for KVM, conditionally setting do_unassigned_access only if
!kvm_enabled() looks sensible. I'll see if I can reproduce it and submit
a patch.

> More generally, there doesn't really seem to be provision in the
> KVM KVM_EXIT_MMIO API for returning "this access failed".
> I guess in theory userspace could do all the "figure out how
> to adjust CPU state to do exception entry and then run VCPU",
> but that seems like quite a lot of work which the kernel already
> knows how to do; is there some way to provide a simpler API
> that lets userspace just inform the kernel that it needs to
> fault the access?

Indeed. Paolo's idea would work well I think. A data bus error exception
would likely be the only sensible error response required other than
ignoring writes or returning a garbage value for a read (which the
current KVM MMIO API already allows).

Cheers
James