Re: [Qemu-devel] [Qemu-ppc] [PATCH 4/5] target-ppc: Handle ibm, nmi-register RTAS call

[Aravinda Prasad]

On Thursday 28 August 2014 02:07 PM, Alexander Graf wrote:
> 
> 
> On 28.08.14 08:38, Aravinda Prasad wrote:
>>
>>
>> On Wednesday 27 August 2014 04:07 PM, Alexander Graf wrote:
>>>
>>>
>>> On 25.08.14 15:45, Aravinda Prasad wrote:
>>>> This patch adds FWNMI support in qemu for powerKVM
>>>> guests by handling the ibm,nmi-register rtas call.
>>>> Whenever OS issues ibm,nmi-register RTAS call, the
>>>> machine check notification address is saved and the
>>>> machine check interrupt vector 0x200 is patched to
>>>> issue a private hcall.
>>>>
>>>> Signed-off-by: Aravinda Prasad <address@hidden>
>>>> ---
>>>>  hw/ppc/spapr_rtas.c    |   91 
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  include/hw/ppc/spapr.h |    8 ++++
>>>>  2 files changed, 98 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>>>> index 02ddbf9..1135d2b 100644
>>>> --- a/hw/ppc/spapr_rtas.c
>>>> +++ b/hw/ppc/spapr_rtas.c
>>>> @@ -277,6 +277,91 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU 
>>>> *cpu,
>>>>      rtas_st(rets, 0, ret);
>>>>  }
>>>>  
>>>> +static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>>>> +                                  sPAPREnvironment *spapr,
>>>> +                                  uint32_t token, uint32_t nargs,
>>>> +                                  target_ulong args,
>>>> +                                  uint32_t nret, target_ulong rets)
>>>> +{
>>>> +    int i;
>>>> +    uint32_t branch_inst = 0x48000002;
>>>> +    target_ulong guest_machine_check_addr;
>>>> +    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>>>> +    /*
>>>> +     * Trampoline saves r3 in sprg2 and issues private hcall
>>>> +     * to request qemu to build error log. QEMU builds the
>>>> +     * error log, copies to rtas-blob and returns the address.
>>>> +     * The initial 16 bytes in rtas-blob consists of saved srr0
>>>> +     * and srr1 which we restore and pass on the actual error
>>>> +     * log address to OS handled mcachine check notification
>>>> +     * routine
>>>> +     */
>>>> +    uint32_t trampoline[] = {
>>>> +        0x7c7243a6,    /* mtspr   SPRN_SPRG2,r3 */
>>>> +        0x38600000,    /* li      r3,0   */
>>>> +        /* 0xf004 is the KVMPPC_H_REPORT_ERR private HCALL */
>>>> +        0x6063f004,    /* ori     r3,r3,f004  */
>>>> +        /* Issue H_CALL */
>>>> +        0x44000022,    /*  sc      1     */
>>>
>>> So up to here we're saving r3 in SPRG2 (how do we know that we can
>>> clobber it?) and call our special hypercall.
>>>
>>> But what does all the cruft below here do?
>>
>> The saved r3 in SPRG2 is consumed in KVMPPC_H_REPORT_ERR hcall, hence we
>> can clobber SPRG2 after hcall returns. I have included a comment in
>> patch 3/5 while building error log. I think better I add one here as well.
>>
>>>
>>>> +        0x7c9243a6,    /* mtspr r4 sprg2 */
>>>
>>> Apart from th fact that your order is wrong, this destroys the value of
>>> r3 that we saved above again.
>>
>> SPRG2 is saved inside hcall and hence we don't need SPRG2 further after
>> KVMPPC_H_REPORT_ERR hcall returns.
>>
>>>
>>>> +        0xe8830000,    /* ld r4, 0(r3) */
>>>> +        0x7c9a03a6,    /* mtspr r4, srr0 */
>>>> +        0xe8830008,    /* ld r4, 8(r3) */
>>>> +        0x7c9b03a6,    /* mtspr r4, srr1 */
>>>
>>> Can't we just set srr0 and srr1 directly?
>>
>> I checked for instructions in ISA which set srr0/1 directly given an
>> address, but could not find any such instructions.
> 
> I mean from QEMU :).

srr0 and srr1, which are properly set when 0x200 is invoked, are
clobbered when we return from KVMPPC_H_REPORT_ERR hcall. I think they
are modified before issuing rfid (I can see them getting clobbered from
QEMU monitor). However when we jump to OS registered machine check
routine srr0 and srr1 should reflect the value they had when 0x200 was
invoked.

Hence srr0 and srr1 are saved in hcall and restored when we return from
hcall. Also we don't have enough scratch registers available to save
these before invoking hcall from 0x200.

Or am I missing other ways to do this from QEMU?

Regards,
Aravinda

> 
>>
>>>
>>>> +        0x38630010,    /* addi r3,r3,16 */
>>>> +        0x7c9242a6,    /* mfspr r4 sprg2 */
>>>
>>>
>>>> +        0x48000002,    /* Branch to address registered
>>>> +                        * by OS. The branch address is
>>>> +                        * patched below */
>>>> +        0x48000000,    /* b . */
>>>> +    };
>>>> +    int total_inst = sizeof(trampoline) / sizeof(uint32_t);
>>>> +
>>>> +    /* Store the system reset and machine check address */
>>>> +    guest_machine_check_addr = rtas_ld(args, 1);
>>>> +
>>>> +    /* Safety Check */
>>>> +    if (sizeof(trampoline) >= MC_INTERRUPT_VECTOR_SIZE) {
>>>> +        fprintf(stderr, "Unable to register ibm,nmi_register: "
>>>> +                "Trampoline size exceeded\n");
>>>> +        rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    /*
>>>> +     * Update the branch instruction in trampoline with the absolute
>>>> +     * machine check address requested by OS
>>>> +     */
>>>> +    branch_inst |= guest_machine_check_addr;
>>>
>>> Does this even work? You're creating a relative branch here.
>>
>> We do an absolute branch here. guest_machine_check_addr contains the
>> physical address of machine check handler. We update the branch
>> instruction operand to do an absolute branch to this physical address.
> 
> Where? I don't see the absolute bit anywhere.
> 
> 
> Alex
> 

-- 
Regards,
Aravinda