Re: [Qemu-ppc] [Qemu-devel] [PATCH v3 0/4] target-ppc: Add FWNMI support in qemu for powerKVM guests

[Aravinda Prasad]

On Wednesday 02 September 2015 12:04 PM, Sam Bobroff wrote:
> On Tue, Sep 01, 2015 at 04:37:51PM +0530, Aravinda Prasad wrote:

[...]

>>>
>>> So, in summary, it sounds like a decent approach would be:
>>> * store the guest's handlers in QEMU's spapr structure,
>>> * simplify the trampolines down to a single, non-returning, hcall,
>>
>> However, other instructions such as saving r3 and re-trying hcall are
>> still required.
> 
> Ah yes, that's true. I was thinking that the retrying could happen inside the
> hcall but it can't.
> 
>>> * implement the new hcall in QEMU, where it has the handler addresses,
>>> * include the (now tiny) trampoline directly in the code,
>>
>> This was my first approach...
>>
>> If we simplify the trampoline, we can directly include the trampoline in
>> the code. With the assumption on the new hcall, trampoline should be
>> reduced by half. But it will still have 7 to 8 instructions.
> 
> Hmm. That's not as small as I'd hoped.
> 
>>> * no new blob or blob code,
>>
>> Yes. no new blob or blob code. However, we still need to put error log
>> in RTAS blob as per PAPR.
>>
>>> * no changes to KVM,
>>> * no problems with kexec.
>>
>> Not sure if this solves kexec. What if the kexec kernel does not call
>> nmi-register? Upon system-reset/machine-check we still jump to the old
>> stale address.
> 
> I thought that the kexec would overwrite the trampoline code with the new
> kernel's default handler, making it safe to use. Is that not the case?

I am not sure. I need to check.

It should be fine if kexec kernel is overwriting trampoline code and
calling ibm,nmi-register.

> 
>>> Aravinda: what do you think?
>>
>> Also, we need different trampolines for system reset and machine check
>> as retrying is not required for system reset.
> 
> Yes.

A second thought. A common trampoline can work for both, but for system
reset "retrying" is never exercised.

> 
>> Apart from that does system reset requires any error log to be passed on
>> to the guest?
> 
> No it doesn't. It just needs to jump to the handler :-)

ok.

> 
>> This approach sounds good. I will modify the code and post it soon.
> 
> Since we can't get the trampoline code as small as I'd hoped, I wonder if we
> could go the other way and implement all of h_report_mc_err() directly in the
> vector? (The RTAS blob address could be patched in like the handler address.)

We can't do that for two reasons at least for machine-check. One is that
we have limited space (0x100 bytes, i.e., 64 instructions) in these
vectors. The second reason is we still need a private hcall. Details below.

> 
> We would obviously need to generate this code from the assembler and probably
> store it in a new blob, but we could remove the private hcall. Perhaps this
> would be better overall. Is there some reason this won't work, and we need it
> to be an hcall?

Yes. We still need a private hcall. As per PAPR, QEMU should build error
log and report the error log to guest via RTAS blob and guests are not
supposed to directly access RTAS blob. This requires h_report_mc_err()
to be part of QEMU.

As David mentioned earlier, there is a simpler way which can eliminate
private hcall and tramploine code: KVM instead of delivering
machine-check or system-reset to 0x200 or 0x100 of guest, performs a
special exit to QEMU. Upon such exit QEMU builds error log (for
machine-check) and directly invokes guest handler.

Regards,
Aravinda

> 
>> Regards,
>> Aravinda
>>
>>>
>>>> Alex
>>>
>>> Thanks Alex,
>>> Sam.
>>>
>>
>> -- 
>> Regards,
>> Aravinda
> 
> 

-- 
Regards,
Aravinda