Re: [kvmarm] [PATCH v5 13/14] KVM: ARM: Handle I/O aborts

Christoffer Dall
2013-01-14T18:56:44Z

On Mon, Jan 14, 2013 at 1:53 PM, Alexander Graf <agraf@suse.de> wrote:
> On 01/14/2013 07:50 PM, Will Deacon wrote:
>>
>> On Mon, Jan 14, 2013 at 06:43:19PM +0000, Russell King - ARM Linux wrote:
>>>
>>> On Mon, Jan 14, 2013 at 01:25:39PM -0500, Christoffer Dall wrote:
>>>>
>>>> However, unifying all instruction decoding within arch/arm is quite
>>>> the heavy task, and requires agreeing on some canonical API that
>>>> people can live with and it will likely take a long time. I seem to
>>>> recall there were also arguments against unifying kprobe code with
>>>> other instruction decoding, as the kprobe code was also written to
>>>> work highly optimized under certain assumptions, if I understood
>>>> previous comments correctly.
>>>
>>> Yes, I know Rusty had a go.
>>>
>>> What I think may make sense is to unify this and the alignment code.
>>> They're really after the same things, which are:
>>>
>>> - Given an instruction, and register set, calculate the address of the
>>> access, size, number of accesses, and the source/destination
>>> registers.
>>> - Update the register set as though the instruction had been executed
>>> by the CPU.
>>>
>>> However, I've changed tack slightly from the above in the last 10 minutes
>>> or so. I'm thinking a little more that we might be able to take what we
>>> already have in alignment.c and provide it with a set of accessors
>>> according to size etc.
>>
>> FWIW, KVM only needs this code for handling complex MMIO instructions,
>> which
>> aren't even generated by recent guest kernels. I'm inclined to suggest
>> removing
>> this emulation code from KVM entirely given that it's likely to bitrot as
>> it is executed less and less often.
>
>
> That'd mean that you heavily limit what type of guests you're executing,
> which I don't think is a good idea.
>
It would limit legacy Linux kernels at least, but I think getting
KVM/ARM code in mainline is the highest priority, so if merging the
current code is unacceptable, I'm willing to drop the mmio emulation
for now and queue the task of unifying the code for later.

A bit of a shame (think about someone wanting to run some proprietary
custom OS in a VM), but this code has been out-of-tree for too long
already, and I'm afraid unifying the decoding pre-merge is going to
hold things up.

-Christoffer