On Tuesday 26 August 2014 11:32 AM, David Gibson wrote: > On Mon, Aug 25, 2014 at 07:15:45PM +0530, 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 */ > > You should construct the instruction from the KVMPPC_H_REPORT_ERR > constant, otherwise it's an undocumented magic dependency between > parts of the code.
hmm.. yes. will do > >> + 0x6063f004, /* ori r3,r3,f004 */ >> + /* Issue H_CALL */ >> + 0x44000022, /* sc 1 */ >> + 0x7c9243a6, /* mtspr r4 sprg2 */ >> + 0xe8830000, /* ld r4, 0(r3) */ >> + 0x7c9a03a6, /* mtspr r4, srr0 */ >> + 0xe8830008, /* ld r4, 8(r3) */ >> + 0x7c9b03a6, /* mtspr r4, srr1 */ >> + 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; > > The guest has absolutely no influence over this, so this should be an > assert() rather than returning an rtas error. Or better yet, detect > the failure at compile time, since it doesn't depend on anything > runtime either. yes better to detect at compile time. > >> + } >> + >> + /* >> + * Update the branch instruction in trampoline with the absolute >> + * machine check address requested by OS >> + */ >> + branch_inst |= guest_machine_check_addr; > > Surely there should be some sanity checking of the address here, or it > can clobber any part of that branch instruction. Will include a check to make sure instruction opcode is not clobbered. > >> + memcpy(&trampoline[11], &branch_inst, sizeof(branch_inst)); >> + >> + /* Handle all Host/Guest LE/BE combinations */ >> + if ((*pcc->interrupts_big_endian)(cpu)) { >> + for (i = 0; i < total_inst; i++) { >> + trampoline[i] = cpu_to_be32(trampoline[i]); >> + } >> + } else { >> + for (i = 0; i < total_inst; i++) { >> + trampoline[i] = cpu_to_le32(trampoline[i]); >> + } >> + } >> + >> + /* Patch 0x200 NMI interrupt vector memory area of guest */ >> + cpu_physical_memory_write(MC_INTERRUPT_VECTOR, trampoline, >> + sizeof(trampoline)); >> + >> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); >> +} >> + >> +static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu, >> + sPAPREnvironment *spapr, >> + uint32_t token, uint32_t nargs, >> + target_ulong args, >> + uint32_t nret, target_ulong rets) >> +{ >> + rtas_st(rets, 0, RTAS_OUT_SUCCESS); >> +} >> + > > This nmi_interlock call isn't described in the commit message. Yes. will include Regards, Aravinda > -- Regards, Aravinda