On 8/28/2014 3:37 AM, 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 <aravi...@linux.vnet.ibm.com> >>>> --- >>>> 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 :). > >> >>> >>>> + 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: It is here: +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; ^ ^