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

[Aravinda Prasad]

On Thursday 03 September 2015 07:32 AM, Paul Mackerras wrote:
> On Sun, Aug 09, 2015 at 03:53:02PM +0200, Alexander Graf wrote:
>>
>>
>> On 07.08.15 05:37, Sam Bobroff wrote:
>>> The RTAS call being discussed in this thread actually has two vectors to 
>>> patch
>>> (System Reset and Machine Check), and the patches so far only address the
>>> Machine Check part. I've been looking at filling in the System Reset part 
>>> and
>>> that will mean basing my code on top of this set.  I would like to keep the
>>> same style of solution for both vectors, so I'd like to get the discussion
>>> started again :-)
>>>
>>> So (1) do we use a trampoline in guest memory, and if so (2) how is the
>>> trampoline code handled?
>>>
>>> (1) It does seem simpler to me to deliver directly to the handler, but I'm
>>> worried about a few things:
>>>
>>> If a guest were to call ibm,nmi-register and then kexec to a new kernel that
>>> does not call ibm,nmi-register, would the exception cause a jump to a stale
>>> address?
>>
>> Probably - how does that get handled today with pHyp? Does pHyp just
>> override the actual exception vector code and thus the kexec'ed code
>> path gets overwritten?
>>
>> I don't remember the original patch set fully, but if all we need is to
>> override 0x200, why can't we replace the code with
>>
>>   mtsprg scratch, r0
>>   li r0, HCALL_KVM_MC
>>   sc 1
>>
>> then there is no complexity in that code at all with dynamically patched
>> bits. Or am I missing the obvious?
> 
> Well, sc 1 will overwrite SRR0/1, and as far as I can see SRR0/1 have
> the only record of where the machine check occurred.  So we can't use
> sc 1 unless we first save SRR0/1 somewhere.  We could instead use some
> specific illegal instruction, which will cause a hypervisor emulation
> assist interrupt using HSRR0/1.

I now see that I am not saving SRR0/1 which contains information (nip,
msr) on machine check in 0x200 vector.

I am restoring SRR0/1 in the private hcall h_report_mc_err() which is wrong:

...
+    CPUPPCState *env = &cpu->env;
...
+    mc_log.srr0 = env->spr[SPR_SRR0];
+    mc_log.srr1 = env->spr[SPR_SRR1];
...

SRR0/1 above contains the values at the time when private hcall is
invoked, not the values at the time when machine check exception occurred.

Regards,
Aravinda

> 
> Paul.
> 

-- 
Regards,
Aravinda