Re: [PATCH 1/9] KVM: PPC: Book3S: Add infrastructure to implement kernel-side RTAS calls

Alexander Graf
2013-04-04T09:49:55Z

On 04.04.2013, at 07:37, Paul Mackerras wrote:

> On Thu, Mar 21, 2013 at 09:52:16AM +0100, Alexander Graf wrote:
>>> +/* Platform specific hcalls, used by KVM */
>>> +#define H_RTAS 0xf000
>>
>> How about you define a different hcall ID for this? Then QEMU would
>> create its "rtas entry blob" such that KVM-routed RTAS handling goes
>> to KVM directly.
>
> QEMU can still do that, and I don't see that it would change the
> kernel side if it did. We would still have to have agreement between
> the kernel and userspace as to what the hcall number for invoking the
> in-kernel RTAS calls was, and the kernel would still have to keep a
> list of token numbers and how they correspond to the functions it
> provides. The only thing different would be that the in-kernel RTAS
> hcall could return to the guest if it didn't recognize the token
> number, rather than pushing the problem up to userspace. However,
> that wouldn't make the code any simpler, and it isn't a situation
> where performance is an issue.
>
> Do you see some kernel-side improvements or simplifications from your
> suggestion that I'm missing? Remember, the guest gets the token
> numbers from the device tree (properties under the /rtas node), so
> they are under the control of userspace/QEMU.

The code flow with this patch:

<setup time>

foreach (override in overrides)
ioctl(OVERRIDE_RTAS, ...);

<runtime>

switch (hcall_id) {
case QEMU_RTAS_ID:
foreach (override in kvm_overrides) {
int rtas_id = ...;
if (override.rtas_id = rtas_id) {
handle_rtas();
handled = true;
}
}
if (!handled)
pass_to_qemu();
break;
default:
pass_to_qemu();
break
}

What I'm suggesting:

<setup time>

nothing from KVM's point of view

<runtime>

switch (hcall_id) {
case KVM_RTAS_ID:
handle_rtas();
break;
default:
pass_to_qemu();
break;
}

Which one looks easier and less error prone to you? :)

Speaking of which, how does user space know that the kernel actually supports a specific RTAS token?

Alex