* [Qemu-devel] [PATCH v2 0/4] target-ppc: Add FWNMI support in qemu for powerKVM guests @ 2014-09-04 11:13 Aravinda Prasad 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 1/4] target-ppc: Extend rtas-blob Aravinda Prasad ` (3 more replies) 0 siblings, 4 replies; 13+ messages in thread From: Aravinda Prasad @ 2014-09-04 11:13 UTC (permalink / raw) To: aik, qemu-ppc, qemu-devel; +Cc: benh, paulus This series of patches add support for fwnmi in powerKVM guests. Currently upon machine check exception, if the address in error belongs to guest then KVM invokes guest's NMI interrupt vector 0x200. This patch series adds functionality where the guest's 0x200 interrupt vector is patched such that QEMU gets control. QEMU then builds error log and reports the error to OS registered machine check handlers through RTAS space. Apart from this, the patch series also takes care of synchronization when multiple processors encounter machine check at or about the same time. The patch set was tested by simulating a machine check error in the guest. Changes in v2: - Re-based to github.com/agraf/qemu.git branch: ppc-next - Merged patches 4 and 5. - Incorporated other review comments --- Aravinda Prasad (4): target-ppc: Extend rtas-blob target-ppc: Register and handle HCALL to receive updated RTAS region target-ppc: Build error log target-ppc: Handle ibm,nmi-register RTAS call hw/ppc/spapr.c | 4 + hw/ppc/spapr_hcall.c | 178 ++++++++++++++++++++++++++++++++++++++++++++++++ hw/ppc/spapr_rtas.c | 116 +++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 13 +++- 4 files changed, 309 insertions(+), 2 deletions(-) -- Aravinda Prasad ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 1/4] target-ppc: Extend rtas-blob 2014-09-04 11:13 [Qemu-devel] [PATCH v2 0/4] target-ppc: Add FWNMI support in qemu for powerKVM guests Aravinda Prasad @ 2014-09-04 11:13 ` Aravinda Prasad 2014-09-05 7:42 ` [Qemu-devel] [Qemu-ppc] " Alexander Graf 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 2/4] target-ppc: Register and handle HCALL to receive updated RTAS region Aravinda Prasad ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: Aravinda Prasad @ 2014-09-04 11:13 UTC (permalink / raw) To: aik, qemu-ppc, qemu-devel; +Cc: benh, paulus Extend rtas-blob to accommodate error log. Error log structure is saved in rtas space upon a machine check exception. Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> --- hw/ppc/spapr.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 4b20e36..4a7c0ae 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1409,6 +1409,10 @@ static void ppc_spapr_init(MachineState *machine) filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin"); spapr->rtas_size = get_image_size(filename); + + /* Resize blob to accommodate error log at a page aligned address */ + spapr->rtas_size = TARGET_PAGE_ALIGN(spapr->rtas_size) + TARGET_PAGE_SIZE; + spapr->rtas_blob = g_malloc(spapr->rtas_size); if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 0) { hw_error("qemu: could not load LPAR rtas '%s'\n", filename); ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 1/4] target-ppc: Extend rtas-blob 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 1/4] target-ppc: Extend rtas-blob Aravinda Prasad @ 2014-09-05 7:42 ` Alexander Graf 2014-09-05 7:56 ` Aravinda Prasad 0 siblings, 1 reply; 13+ messages in thread From: Alexander Graf @ 2014-09-05 7:42 UTC (permalink / raw) To: Aravinda Prasad, aik, qemu-ppc, qemu-devel; +Cc: benh, paulus On 04.09.14 13:13, Aravinda Prasad wrote: > Extend rtas-blob to accommodate error log. Error log > structure is saved in rtas space upon a machine check > exception. > > Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> > --- > hw/ppc/spapr.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 4b20e36..4a7c0ae 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1409,6 +1409,10 @@ static void ppc_spapr_init(MachineState *machine) > > filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin"); > spapr->rtas_size = get_image_size(filename); > + > + /* Resize blob to accommodate error log at a page aligned address */ > + spapr->rtas_size = TARGET_PAGE_ALIGN(spapr->rtas_size) + TARGET_PAGE_SIZE; I think it's clearer if you do #define RTAS_ERROR_LOG_SIZE 4096 and add this to the aligned size instead. Alex ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 1/4] target-ppc: Extend rtas-blob 2014-09-05 7:42 ` [Qemu-devel] [Qemu-ppc] " Alexander Graf @ 2014-09-05 7:56 ` Aravinda Prasad 0 siblings, 0 replies; 13+ messages in thread From: Aravinda Prasad @ 2014-09-05 7:56 UTC (permalink / raw) To: Alexander Graf; +Cc: benh, aik, qemu-devel, qemu-ppc, paulus On Friday 05 September 2014 01:12 PM, Alexander Graf wrote: > > > On 04.09.14 13:13, Aravinda Prasad wrote: >> Extend rtas-blob to accommodate error log. Error log >> structure is saved in rtas space upon a machine check >> exception. >> >> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> >> --- >> hw/ppc/spapr.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 4b20e36..4a7c0ae 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -1409,6 +1409,10 @@ static void ppc_spapr_init(MachineState *machine) >> >> filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, "spapr-rtas.bin"); >> spapr->rtas_size = get_image_size(filename); >> + >> + /* Resize blob to accommodate error log at a page aligned address */ >> + spapr->rtas_size = TARGET_PAGE_ALIGN(spapr->rtas_size) + TARGET_PAGE_SIZE; > > I think it's clearer if you do > > #define RTAS_ERROR_LOG_SIZE 4096 > > and add this to the aligned size instead. sure > > > Alex > -- Regards, Aravinda ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 2/4] target-ppc: Register and handle HCALL to receive updated RTAS region 2014-09-04 11:13 [Qemu-devel] [PATCH v2 0/4] target-ppc: Add FWNMI support in qemu for powerKVM guests Aravinda Prasad 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 1/4] target-ppc: Extend rtas-blob Aravinda Prasad @ 2014-09-04 11:13 ` Aravinda Prasad 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 3/4] target-ppc: Build error log Aravinda Prasad 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 4/4] target-ppc: Handle ibm, nmi-register RTAS call Aravinda Prasad 3 siblings, 0 replies; 13+ messages in thread From: Aravinda Prasad @ 2014-09-04 11:13 UTC (permalink / raw) To: aik, qemu-ppc, qemu-devel; +Cc: benh, paulus Receive updates from SLOF about the updated rtas-base. A separate patch for SLOF [1] adds functionality to invoke a a private HCALL whenever OS issues instantiate-rtas with a new rtas-base. This is required as qemu needs to know the updated rtas-base as it allocates error reporting structure in RTAS space upon a machine check exception. [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2014-August/120386.html Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> --- hw/ppc/spapr_hcall.c | 8 ++++++++ include/hw/ppc/spapr.h | 3 ++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index 8651447..01650ba 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -579,6 +579,13 @@ static target_ulong h_rtas(PowerPCCPU *cpu, sPAPREnvironment *spapr, nret, rtas_r3 + 12 + 4*nargs); } +static target_ulong h_rtas_update(PowerPCCPU *cpu, sPAPREnvironment *spapr, + target_ulong opcode, target_ulong *args) +{ + spapr->rtas_addr = args[0]; + return 0; +} + static target_ulong h_logical_load(PowerPCCPU *cpu, sPAPREnvironment *spapr, target_ulong opcode, target_ulong *args) { @@ -1003,6 +1010,7 @@ static void hypercall_register_types(void) /* qemu/KVM-PPC specific hcalls */ spapr_register_hypercall(KVMPPC_H_RTAS, h_rtas); + spapr_register_hypercall(KVMPPC_H_RTAS_UPDATE, h_rtas_update); spapr_register_hypercall(H_SET_MODE, h_set_mode); diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 749daf4..908db26 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -308,7 +308,8 @@ typedef struct sPAPREnvironment { #define KVMPPC_H_LOGICAL_MEMOP (KVMPPC_HCALL_BASE + 0x1) /* Client Architecture support */ #define KVMPPC_H_CAS (KVMPPC_HCALL_BASE + 0x2) -#define KVMPPC_HCALL_MAX KVMPPC_H_CAS +#define KVMPPC_H_RTAS_UPDATE (KVMPPC_HCALL_BASE + 0x3) +#define KVMPPC_HCALL_MAX KVMPPC_H_RTAS_UPDATE extern sPAPREnvironment *spapr; ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 3/4] target-ppc: Build error log 2014-09-04 11:13 [Qemu-devel] [PATCH v2 0/4] target-ppc: Add FWNMI support in qemu for powerKVM guests Aravinda Prasad 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 1/4] target-ppc: Extend rtas-blob Aravinda Prasad 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 2/4] target-ppc: Register and handle HCALL to receive updated RTAS region Aravinda Prasad @ 2014-09-04 11:13 ` Aravinda Prasad 2014-09-05 2:14 ` Alexey Kardashevskiy 2014-09-05 8:04 ` [Qemu-devel] [Qemu-ppc] " Alexander Graf 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 4/4] target-ppc: Handle ibm, nmi-register RTAS call Aravinda Prasad 3 siblings, 2 replies; 13+ messages in thread From: Aravinda Prasad @ 2014-09-04 11:13 UTC (permalink / raw) To: aik, qemu-ppc, qemu-devel; +Cc: benh, paulus Whenever there is a physical memory error due to bit flips, which cannot be corrected by hardware, the error is passed on to the kernel. If the memory address in error belongs to guest address space then guest kernel is responsible to take action. Hence the error is passed on to guest via KVM by invoking 0x200 NMI vector. However, guest OS, as per PAPR, expects an error log upon such error. This patch registers a new hcall which is issued from 0x200 interrupt vector and builds the error log, copies the error log to rtas space and passes the address of the error log to guest Enhancement to KVM to perform above functionality is already in upstream kernel. Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> --- hw/ppc/spapr_hcall.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 4 + 2 files changed, 157 insertions(+), 1 deletion(-) diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index 01650ba..c3aa448 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -14,6 +14,88 @@ struct SPRSyncState { target_ulong mask; }; +/* Offset from rtas-base where error log is placed */ +#define RTAS_ERROR_OFFSET (TARGET_PAGE_SIZE) + +#define RTAS_ELOG_SEVERITY_SHIFT 0x5 +#define RTAS_ELOG_DISPOSITION_SHIFT 0x3 +#define RTAS_ELOG_INITIATOR_SHIFT 0x4 + +/* + * Only required RTAS event severity, disposition, initiator + * target and type are copied from arch/powerpc/include/asm/rtas.h + */ + +/* RTAS event severity */ +#define RTAS_SEVERITY_ERROR_SYNC 0x3 + +/* RTAS event disposition */ +#define RTAS_DISP_NOT_RECOVERED 0x2 + +/* RTAS event initiator */ +#define RTAS_INITIATOR_MEMORY 0x4 + +/* RTAS event target */ +#define RTAS_TARGET_MEMORY 0x4 + +/* RTAS event type */ +#define RTAS_TYPE_ECC_UNCORR 0x09 + +/* + * Currently KVM only passes on the uncorrected machine + * check memory error to guest. Other machine check errors + * such as SLB multi-hit and TLB multi-hit are recovered + * in KVM and are not passed on to guest. + * + * DSISR Bit for uncorrected machine check error. Based + * on arch/powerpc/include/asm/mce.h + */ +#define PPC_BIT(bit) (0x8000000000000000ULL >> bit) +#define P7_DSISR_MC_UE (PPC_BIT(48)) /* P8 too */ + +/* Adopted from kernel source arch/powerpc/include/asm/rtas.h */ +struct rtas_error_log { + /* Byte 0 */ + uint8_t byte0; /* Architectural version */ + + /* Byte 1 */ + uint8_t byte1; + /* XXXXXXXX + * XXX 3: Severity level of error + * XX 2: Degree of recovery + * X 1: Extended log present? + * XX 2: Reserved + */ + + /* Byte 2 */ + uint8_t byte2; + /* XXXXXXXX + * XXXX 4: Initiator of event + * XXXX 4: Target of failed operation + */ + uint8_t byte3; /* General event or error*/ +}; + +/* + * Data format in RTAS-Blob + * + * This structure contains error information related to Machine + * Check exception. This is filled up and copied to rtas-blob + * upon machine check exception. + */ +struct rtas_mc_log { + target_ulong srr0; + target_ulong srr1; + /* + * Beginning of error log address. This is properly + * populated and passed on to OS registered machine + * check notification routine upon machine check + * exception + */ + target_ulong r3; + struct rtas_error_log err_log; +}; + static void do_spr_sync(void *arg) { struct SPRSyncState *s = arg; @@ -586,6 +668,77 @@ static target_ulong h_rtas_update(PowerPCCPU *cpu, sPAPREnvironment *spapr, return 0; } +static target_ulong h_report_mc_err(PowerPCCPU *cpu, sPAPREnvironment *spapr, + target_ulong opcode, target_ulong *args) +{ + struct rtas_mc_log mc_log; + CPUPPCState *env = &cpu->env; + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); + + /* + * We save the original r3 register in SPRG2 in 0x200 vector, + * which is patched during call to ibm.nmi-register. Original + * r3 is required to be included in error log + */ + mc_log.r3 = env->spr[SPR_SPRG2]; + + /* + * SRR0 and SRR1, containing nip and msr at the time of exception, + * are clobbered when we return from this hcall. Hence they + * need to be properly saved and restored. We save srr0 + * and srr1 in rtas blob and restore it in 0x200 vector + * before branching to OS registered machine check handler + */ + mc_log.srr0 = env->spr[SPR_SRR0]; + mc_log.srr1 = env->spr[SPR_SRR1]; + + /* Set error log fields */ + mc_log.err_log.byte0 = 0x00; + mc_log.err_log.byte1 = + (RTAS_SEVERITY_ERROR_SYNC << RTAS_ELOG_SEVERITY_SHIFT); + mc_log.err_log.byte1 |= + (RTAS_DISP_NOT_RECOVERED << RTAS_ELOG_DISPOSITION_SHIFT); + mc_log.err_log.byte2 = + (RTAS_INITIATOR_MEMORY << RTAS_ELOG_INITIATOR_SHIFT); + mc_log.err_log.byte2 |= RTAS_TARGET_MEMORY; + + if (env->spr[SPR_DSISR] & P7_DSISR_MC_UE) { + mc_log.err_log.byte3 = RTAS_TYPE_ECC_UNCORR; + } else { + mc_log.err_log.byte3 = 0x0; + } + + /* Handle all Host/Guest LE/BE combinations */ + if ((*pcc->interrupts_big_endian)(cpu)) { + mc_log.srr0 = cpu_to_be64(mc_log.srr0); + mc_log.srr1 = cpu_to_be64(mc_log.srr1); + mc_log.r3 = cpu_to_be64(mc_log.r3); + } else { + mc_log.srr0 = cpu_to_le64(mc_log.srr0); + mc_log.srr1 = cpu_to_le64(mc_log.srr1); + mc_log.r3 = cpu_to_le64(mc_log.r3); + } + + cpu_physical_memory_write(spapr->rtas_addr + RTAS_ERROR_OFFSET, + &mc_log, sizeof(mc_log)); + + /* + * spapr->rtas_addr + RTAS_ERROR_OFFSET now contains srr0, srr1, + * original r3, followed by the error log structure. The address + * of the error log should be passed on to guest's machine check + * notification routine. As this hcall is directly called from + * 0x200 interrupt vector and returns to assembly routine, we + * return (spapr->rtas_addr + RTAS_ERROR_OFFSET) instead of + * H_SUCCESS. Upon return, We restore srr0 and srr1, increment + * r3 to point to the error log and branch to machine check + * notification routine in 0x200. r3 containing the error address + * is now argument to OS registered machine check notification + * routine. This way we also avoids clobbering additional + * registers in 0x200 vector. + */ + return spapr->rtas_addr + RTAS_ERROR_OFFSET; +} + static target_ulong h_logical_load(PowerPCCPU *cpu, sPAPREnvironment *spapr, target_ulong opcode, target_ulong *args) { @@ -1011,6 +1164,7 @@ static void hypercall_register_types(void) /* qemu/KVM-PPC specific hcalls */ spapr_register_hypercall(KVMPPC_H_RTAS, h_rtas); spapr_register_hypercall(KVMPPC_H_RTAS_UPDATE, h_rtas_update); + spapr_register_hypercall(KVMPPC_H_REPORT_MC_ERR, h_report_mc_err); spapr_register_hypercall(H_SET_MODE, h_set_mode); diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 908db26..74cf881 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -309,7 +309,9 @@ typedef struct sPAPREnvironment { /* Client Architecture support */ #define KVMPPC_H_CAS (KVMPPC_HCALL_BASE + 0x2) #define KVMPPC_H_RTAS_UPDATE (KVMPPC_HCALL_BASE + 0x3) -#define KVMPPC_HCALL_MAX KVMPPC_H_RTAS_UPDATE +/* Report Machine Check error */ +#define KVMPPC_H_REPORT_MC_ERR (KVMPPC_HCALL_BASE + 0x4) +#define KVMPPC_HCALL_MAX KVMPPC_H_REPORT_MC_ERR extern sPAPREnvironment *spapr; ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] target-ppc: Build error log 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 3/4] target-ppc: Build error log Aravinda Prasad @ 2014-09-05 2:14 ` Alexey Kardashevskiy 2014-09-05 8:07 ` Aravinda Prasad 2014-09-05 8:04 ` [Qemu-devel] [Qemu-ppc] " Alexander Graf 1 sibling, 1 reply; 13+ messages in thread From: Alexey Kardashevskiy @ 2014-09-05 2:14 UTC (permalink / raw) To: Aravinda Prasad, qemu-ppc, qemu-devel; +Cc: benh, paulus On 09/04/2014 09:13 PM, Aravinda Prasad wrote: > Whenever there is a physical memory error due to bit > flips, which cannot be corrected by hardware, the error > is passed on to the kernel. If the memory address in > error belongs to guest address space then guest kernel > is responsible to take action. Hence the error is passed > on to guest via KVM by invoking 0x200 NMI vector. > > However, guest OS, as per PAPR, expects an error log > upon such error. This patch registers a new hcall > which is issued from 0x200 interrupt vector and builds > the error log, copies the error log to rtas space and > passes the address of the error log to guest > > Enhancement to KVM to perform above functionality is > already in upstream kernel. > > Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> > --- > hw/ppc/spapr_hcall.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++ > include/hw/ppc/spapr.h | 4 + > 2 files changed, 157 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index 01650ba..c3aa448 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -14,6 +14,88 @@ struct SPRSyncState { > target_ulong mask; > }; > > +/* Offset from rtas-base where error log is placed */ > +#define RTAS_ERROR_OFFSET (TARGET_PAGE_SIZE) > + > +#define RTAS_ELOG_SEVERITY_SHIFT 0x5 > +#define RTAS_ELOG_DISPOSITION_SHIFT 0x3 > +#define RTAS_ELOG_INITIATOR_SHIFT 0x4 > + > +/* > + * Only required RTAS event severity, disposition, initiator > + * target and type are copied from arch/powerpc/include/asm/rtas.h > + */ > + > +/* RTAS event severity */ > +#define RTAS_SEVERITY_ERROR_SYNC 0x3 > + > +/* RTAS event disposition */ > +#define RTAS_DISP_NOT_RECOVERED 0x2 > + > +/* RTAS event initiator */ > +#define RTAS_INITIATOR_MEMORY 0x4 > + > +/* RTAS event target */ > +#define RTAS_TARGET_MEMORY 0x4 > + > +/* RTAS event type */ > +#define RTAS_TYPE_ECC_UNCORR 0x09 > + > +/* > + * Currently KVM only passes on the uncorrected machine > + * check memory error to guest. Other machine check errors > + * such as SLB multi-hit and TLB multi-hit are recovered > + * in KVM and are not passed on to guest. > + * > + * DSISR Bit for uncorrected machine check error. Based > + * on arch/powerpc/include/asm/mce.h > + */ > +#define PPC_BIT(bit) (0x8000000000000000ULL >> bit) > +#define P7_DSISR_MC_UE (PPC_BIT(48)) /* P8 too */ > + > +/* Adopted from kernel source arch/powerpc/include/asm/rtas.h */ > +struct rtas_error_log { > + /* Byte 0 */ > + uint8_t byte0; /* Architectural version */ > + > + /* Byte 1 */ > + uint8_t byte1; > + /* XXXXXXXX > + * XXX 3: Severity level of error > + * XX 2: Degree of recovery > + * X 1: Extended log present? > + * XX 2: Reserved > + */ > + > + /* Byte 2 */ > + uint8_t byte2; > + /* XXXXXXXX > + * XXXX 4: Initiator of event > + * XXXX 4: Target of failed operation > + */ > + uint8_t byte3; /* General event or error*/ > +}; Any particular reason not to copy rtas_error_log as is? -- Alexey Kardashevskiy IBM OzLabs, LTC Team e-mail: aik@au1.ibm.com notes: Alexey Kardashevskiy/Australia/IBM ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/4] target-ppc: Build error log 2014-09-05 2:14 ` Alexey Kardashevskiy @ 2014-09-05 8:07 ` Aravinda Prasad 0 siblings, 0 replies; 13+ messages in thread From: Aravinda Prasad @ 2014-09-05 8:07 UTC (permalink / raw) To: Alexey Kardashevskiy; +Cc: benh, qemu-ppc, qemu-devel, paulus On Friday 05 September 2014 07:44 AM, Alexey Kardashevskiy wrote: > On 09/04/2014 09:13 PM, Aravinda Prasad wrote: >> Whenever there is a physical memory error due to bit >> flips, which cannot be corrected by hardware, the error >> is passed on to the kernel. If the memory address in >> error belongs to guest address space then guest kernel >> is responsible to take action. Hence the error is passed >> on to guest via KVM by invoking 0x200 NMI vector. >> >> However, guest OS, as per PAPR, expects an error log >> upon such error. This patch registers a new hcall >> which is issued from 0x200 interrupt vector and builds >> the error log, copies the error log to rtas space and >> passes the address of the error log to guest >> >> Enhancement to KVM to perform above functionality is >> already in upstream kernel. >> >> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> >> --- >> hw/ppc/spapr_hcall.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/ppc/spapr.h | 4 + >> 2 files changed, 157 insertions(+), 1 deletion(-) >> >> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c >> index 01650ba..c3aa448 100644 >> --- a/hw/ppc/spapr_hcall.c >> +++ b/hw/ppc/spapr_hcall.c >> @@ -14,6 +14,88 @@ struct SPRSyncState { >> target_ulong mask; >> }; >> >> +/* Offset from rtas-base where error log is placed */ >> +#define RTAS_ERROR_OFFSET (TARGET_PAGE_SIZE) >> + >> +#define RTAS_ELOG_SEVERITY_SHIFT 0x5 >> +#define RTAS_ELOG_DISPOSITION_SHIFT 0x3 >> +#define RTAS_ELOG_INITIATOR_SHIFT 0x4 >> + >> +/* >> + * Only required RTAS event severity, disposition, initiator >> + * target and type are copied from arch/powerpc/include/asm/rtas.h >> + */ >> + >> +/* RTAS event severity */ >> +#define RTAS_SEVERITY_ERROR_SYNC 0x3 >> + >> +/* RTAS event disposition */ >> +#define RTAS_DISP_NOT_RECOVERED 0x2 >> + >> +/* RTAS event initiator */ >> +#define RTAS_INITIATOR_MEMORY 0x4 >> + >> +/* RTAS event target */ >> +#define RTAS_TARGET_MEMORY 0x4 >> + >> +/* RTAS event type */ >> +#define RTAS_TYPE_ECC_UNCORR 0x09 >> + >> +/* >> + * Currently KVM only passes on the uncorrected machine >> + * check memory error to guest. Other machine check errors >> + * such as SLB multi-hit and TLB multi-hit are recovered >> + * in KVM and are not passed on to guest. >> + * >> + * DSISR Bit for uncorrected machine check error. Based >> + * on arch/powerpc/include/asm/mce.h >> + */ >> +#define PPC_BIT(bit) (0x8000000000000000ULL >> bit) >> +#define P7_DSISR_MC_UE (PPC_BIT(48)) /* P8 too */ >> + >> +/* Adopted from kernel source arch/powerpc/include/asm/rtas.h */ >> +struct rtas_error_log { >> + /* Byte 0 */ >> + uint8_t byte0; /* Architectural version */ >> + >> + /* Byte 1 */ >> + uint8_t byte1; >> + /* XXXXXXXX >> + * XXX 3: Severity level of error >> + * XX 2: Degree of recovery >> + * X 1: Extended log present? >> + * XX 2: Reserved >> + */ >> + >> + /* Byte 2 */ >> + uint8_t byte2; >> + /* XXXXXXXX >> + * XXXX 4: Initiator of event >> + * XXXX 4: Target of failed operation >> + */ >> + uint8_t byte3; /* General event or error*/ >> +}; > > > Any particular reason not to copy rtas_error_log as is? No specific reason. I overlooked it when I moved my code form early prototype version. Will include rest all members. Regards, Aravinda > > > > -- Regards, Aravinda ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 3/4] target-ppc: Build error log 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 3/4] target-ppc: Build error log Aravinda Prasad 2014-09-05 2:14 ` Alexey Kardashevskiy @ 2014-09-05 8:04 ` Alexander Graf 2014-09-05 8:28 ` Aravinda Prasad 1 sibling, 1 reply; 13+ messages in thread From: Alexander Graf @ 2014-09-05 8:04 UTC (permalink / raw) To: Aravinda Prasad, aik, qemu-ppc, qemu-devel; +Cc: benh, paulus On 04.09.14 13:13, Aravinda Prasad wrote: > Whenever there is a physical memory error due to bit > flips, which cannot be corrected by hardware, the error > is passed on to the kernel. If the memory address in > error belongs to guest address space then guest kernel > is responsible to take action. Hence the error is passed > on to guest via KVM by invoking 0x200 NMI vector. > > However, guest OS, as per PAPR, expects an error log > upon such error. This patch registers a new hcall > which is issued from 0x200 interrupt vector and builds > the error log, copies the error log to rtas space and > passes the address of the error log to guest > > Enhancement to KVM to perform above functionality is > already in upstream kernel. > > Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> > --- > hw/ppc/spapr_hcall.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++ > include/hw/ppc/spapr.h | 4 + > 2 files changed, 157 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index 01650ba..c3aa448 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -14,6 +14,88 @@ struct SPRSyncState { > target_ulong mask; > }; > > +/* Offset from rtas-base where error log is placed */ > +#define RTAS_ERROR_OFFSET (TARGET_PAGE_SIZE) You can't assume this. Please compute the value at the same place you compute the rtas-size. > + > +#define RTAS_ELOG_SEVERITY_SHIFT 0x5 > +#define RTAS_ELOG_DISPOSITION_SHIFT 0x3 > +#define RTAS_ELOG_INITIATOR_SHIFT 0x4 > + > +/* > + * Only required RTAS event severity, disposition, initiator > + * target and type are copied from arch/powerpc/include/asm/rtas.h > + */ > + > +/* RTAS event severity */ > +#define RTAS_SEVERITY_ERROR_SYNC 0x3 > + > +/* RTAS event disposition */ > +#define RTAS_DISP_NOT_RECOVERED 0x2 > + > +/* RTAS event initiator */ > +#define RTAS_INITIATOR_MEMORY 0x4 > + > +/* RTAS event target */ > +#define RTAS_TARGET_MEMORY 0x4 > + > +/* RTAS event type */ > +#define RTAS_TYPE_ECC_UNCORR 0x09 > + > +/* > + * Currently KVM only passes on the uncorrected machine > + * check memory error to guest. Other machine check errors > + * such as SLB multi-hit and TLB multi-hit are recovered > + * in KVM and are not passed on to guest. > + * > + * DSISR Bit for uncorrected machine check error. Based > + * on arch/powerpc/include/asm/mce.h Please don't include Linux code. This file is GPLv2+ licensed and I don't want to taint it as GPLv2 only just for the sake of mce. > + */ > +#define PPC_BIT(bit) (0x8000000000000000ULL >> bit) > +#define P7_DSISR_MC_UE (PPC_BIT(48)) /* P8 too */ > + > +/* Adopted from kernel source arch/powerpc/include/asm/rtas.h */ > +struct rtas_error_log { > + /* Byte 0 */ > + uint8_t byte0; /* Architectural version */ > + > + /* Byte 1 */ > + uint8_t byte1; > + /* XXXXXXXX > + * XXX 3: Severity level of error > + * XX 2: Degree of recovery > + * X 1: Extended log present? > + * XX 2: Reserved > + */ > + > + /* Byte 2 */ > + uint8_t byte2; > + /* XXXXXXXX > + * XXXX 4: Initiator of event > + * XXXX 4: Target of failed operation > + */ > + uint8_t byte3; /* General event or error*/ > +}; > + > +/* > + * Data format in RTAS-Blob > + * > + * This structure contains error information related to Machine > + * Check exception. This is filled up and copied to rtas-blob > + * upon machine check exception. > + */ > +struct rtas_mc_log { > + target_ulong srr0; > + target_ulong srr1; > + /* > + * Beginning of error log address. This is properly > + * populated and passed on to OS registered machine > + * check notification routine upon machine check > + * exception > + */ > + target_ulong r3; > + struct rtas_error_log err_log; > +}; > + > static void do_spr_sync(void *arg) > { > struct SPRSyncState *s = arg; > @@ -586,6 +668,77 @@ static target_ulong h_rtas_update(PowerPCCPU *cpu, sPAPREnvironment *spapr, > return 0; > } > > +static target_ulong h_report_mc_err(PowerPCCPU *cpu, sPAPREnvironment *spapr, > + target_ulong opcode, target_ulong *args) > +{ > + struct rtas_mc_log mc_log; > + CPUPPCState *env = &cpu->env; > + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); > + > + /* > + * We save the original r3 register in SPRG2 in 0x200 vector, > + * which is patched during call to ibm.nmi-register. Original > + * r3 is required to be included in error log > + */ > + mc_log.r3 = env->spr[SPR_SPRG2]; Don't you have to call cpu_synchronize_registers() before you access SPRG2? Otherwise the value may be stale. > + > + /* > + * SRR0 and SRR1, containing nip and msr at the time of exception, > + * are clobbered when we return from this hcall. Hence they > + * need to be properly saved and restored. We save srr0 > + * and srr1 in rtas blob and restore it in 0x200 vector > + * before branching to OS registered machine check handler > + */ > + mc_log.srr0 = env->spr[SPR_SRR0]; > + mc_log.srr1 = env->spr[SPR_SRR1]; > + > + /* Set error log fields */ > + mc_log.err_log.byte0 = 0x00; > + mc_log.err_log.byte1 = > + (RTAS_SEVERITY_ERROR_SYNC << RTAS_ELOG_SEVERITY_SHIFT); > + mc_log.err_log.byte1 |= > + (RTAS_DISP_NOT_RECOVERED << RTAS_ELOG_DISPOSITION_SHIFT); > + mc_log.err_log.byte2 = > + (RTAS_INITIATOR_MEMORY << RTAS_ELOG_INITIATOR_SHIFT); > + mc_log.err_log.byte2 |= RTAS_TARGET_MEMORY; > + > + if (env->spr[SPR_DSISR] & P7_DSISR_MC_UE) { > + mc_log.err_log.byte3 = RTAS_TYPE_ECC_UNCORR; > + } else { > + mc_log.err_log.byte3 = 0x0; > + } > + > + /* Handle all Host/Guest LE/BE combinations */ > + if ((*pcc->interrupts_big_endian)(cpu)) { This should check MSR.LE, not ILE. Alex ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 3/4] target-ppc: Build error log 2014-09-05 8:04 ` [Qemu-devel] [Qemu-ppc] " Alexander Graf @ 2014-09-05 8:28 ` Aravinda Prasad 2014-09-05 8:39 ` Alexander Graf 2014-09-11 1:23 ` Alexey Kardashevskiy 0 siblings, 2 replies; 13+ messages in thread From: Aravinda Prasad @ 2014-09-05 8:28 UTC (permalink / raw) To: Alexander Graf; +Cc: benh, aik, qemu-devel, qemu-ppc, paulus On Friday 05 September 2014 01:34 PM, Alexander Graf wrote: > > > On 04.09.14 13:13, Aravinda Prasad wrote: >> Whenever there is a physical memory error due to bit >> flips, which cannot be corrected by hardware, the error >> is passed on to the kernel. If the memory address in >> error belongs to guest address space then guest kernel >> is responsible to take action. Hence the error is passed >> on to guest via KVM by invoking 0x200 NMI vector. >> >> However, guest OS, as per PAPR, expects an error log >> upon such error. This patch registers a new hcall >> which is issued from 0x200 interrupt vector and builds >> the error log, copies the error log to rtas space and >> passes the address of the error log to guest >> >> Enhancement to KVM to perform above functionality is >> already in upstream kernel. >> >> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> >> --- >> hw/ppc/spapr_hcall.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/ppc/spapr.h | 4 + >> 2 files changed, 157 insertions(+), 1 deletion(-) >> >> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c >> index 01650ba..c3aa448 100644 >> --- a/hw/ppc/spapr_hcall.c >> +++ b/hw/ppc/spapr_hcall.c >> @@ -14,6 +14,88 @@ struct SPRSyncState { >> target_ulong mask; >> }; >> >> +/* Offset from rtas-base where error log is placed */ >> +#define RTAS_ERROR_OFFSET (TARGET_PAGE_SIZE) > > You can't assume this. Please compute the value at the same place you > compute the rtas-size. sure > >> + >> +#define RTAS_ELOG_SEVERITY_SHIFT 0x5 >> +#define RTAS_ELOG_DISPOSITION_SHIFT 0x3 >> +#define RTAS_ELOG_INITIATOR_SHIFT 0x4 >> + >> +/* >> + * Only required RTAS event severity, disposition, initiator >> + * target and type are copied from arch/powerpc/include/asm/rtas.h >> + */ >> + >> +/* RTAS event severity */ >> +#define RTAS_SEVERITY_ERROR_SYNC 0x3 >> + >> +/* RTAS event disposition */ >> +#define RTAS_DISP_NOT_RECOVERED 0x2 >> + >> +/* RTAS event initiator */ >> +#define RTAS_INITIATOR_MEMORY 0x4 >> + >> +/* RTAS event target */ >> +#define RTAS_TARGET_MEMORY 0x4 >> + >> +/* RTAS event type */ >> +#define RTAS_TYPE_ECC_UNCORR 0x09 >> + >> +/* >> + * Currently KVM only passes on the uncorrected machine >> + * check memory error to guest. Other machine check errors >> + * such as SLB multi-hit and TLB multi-hit are recovered >> + * in KVM and are not passed on to guest. >> + * >> + * DSISR Bit for uncorrected machine check error. Based >> + * on arch/powerpc/include/asm/mce.h > > Please don't include Linux code. This file is GPLv2+ licensed and I > don't want to taint it as GPLv2 only just for the sake of mce. Hmm.. ok. In that case I should not copy rtas_error_log structure below from kernel source as well. > >> + */ >> +#define PPC_BIT(bit) (0x8000000000000000ULL >> bit) >> +#define P7_DSISR_MC_UE (PPC_BIT(48)) /* P8 too */ >> + >> +/* Adopted from kernel source arch/powerpc/include/asm/rtas.h */ >> +struct rtas_error_log { >> + /* Byte 0 */ >> + uint8_t byte0; /* Architectural version */ >> + >> + /* Byte 1 */ >> + uint8_t byte1; >> + /* XXXXXXXX >> + * XXX 3: Severity level of error >> + * XX 2: Degree of recovery >> + * X 1: Extended log present? >> + * XX 2: Reserved >> + */ >> + >> + /* Byte 2 */ >> + uint8_t byte2; >> + /* XXXXXXXX >> + * XXXX 4: Initiator of event >> + * XXXX 4: Target of failed operation >> + */ >> + uint8_t byte3; /* General event or error*/ >> +}; >> + >> +/* >> + * Data format in RTAS-Blob >> + * >> + * This structure contains error information related to Machine >> + * Check exception. This is filled up and copied to rtas-blob >> + * upon machine check exception. >> + */ >> +struct rtas_mc_log { >> + target_ulong srr0; >> + target_ulong srr1; >> + /* >> + * Beginning of error log address. This is properly >> + * populated and passed on to OS registered machine >> + * check notification routine upon machine check >> + * exception >> + */ >> + target_ulong r3; >> + struct rtas_error_log err_log; >> +}; >> + >> static void do_spr_sync(void *arg) >> { >> struct SPRSyncState *s = arg; >> @@ -586,6 +668,77 @@ static target_ulong h_rtas_update(PowerPCCPU *cpu, sPAPREnvironment *spapr, >> return 0; >> } >> >> +static target_ulong h_report_mc_err(PowerPCCPU *cpu, sPAPREnvironment *spapr, >> + target_ulong opcode, target_ulong *args) >> +{ >> + struct rtas_mc_log mc_log; >> + CPUPPCState *env = &cpu->env; >> + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); >> + >> + /* >> + * We save the original r3 register in SPRG2 in 0x200 vector, >> + * which is patched during call to ibm.nmi-register. Original >> + * r3 is required to be included in error log >> + */ >> + mc_log.r3 = env->spr[SPR_SPRG2]; > > Don't you have to call cpu_synchronize_registers() before you access > SPRG2? Otherwise the value may be stale. Yes true. Will include. > >> + >> + /* >> + * SRR0 and SRR1, containing nip and msr at the time of exception, >> + * are clobbered when we return from this hcall. Hence they >> + * need to be properly saved and restored. We save srr0 >> + * and srr1 in rtas blob and restore it in 0x200 vector >> + * before branching to OS registered machine check handler >> + */ >> + mc_log.srr0 = env->spr[SPR_SRR0]; >> + mc_log.srr1 = env->spr[SPR_SRR1]; >> + >> + /* Set error log fields */ >> + mc_log.err_log.byte0 = 0x00; >> + mc_log.err_log.byte1 = >> + (RTAS_SEVERITY_ERROR_SYNC << RTAS_ELOG_SEVERITY_SHIFT); >> + mc_log.err_log.byte1 |= >> + (RTAS_DISP_NOT_RECOVERED << RTAS_ELOG_DISPOSITION_SHIFT); >> + mc_log.err_log.byte2 = >> + (RTAS_INITIATOR_MEMORY << RTAS_ELOG_INITIATOR_SHIFT); >> + mc_log.err_log.byte2 |= RTAS_TARGET_MEMORY; >> + >> + if (env->spr[SPR_DSISR] & P7_DSISR_MC_UE) { >> + mc_log.err_log.byte3 = RTAS_TYPE_ECC_UNCORR; >> + } else { >> + mc_log.err_log.byte3 = 0x0; >> + } >> + >> + /* Handle all Host/Guest LE/BE combinations */ >> + if ((*pcc->interrupts_big_endian)(cpu)) { > > This should check MSR.LE, not ILE. I did not quite get you. Did you mean to say I should check LE bit in MSR instead of invoking pcc->interrupts_big_endian? If so I should also change that check in 4/4 where I patch 0x200. Regards, Aravinda > > > Alex > -- Regards, Aravinda ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 3/4] target-ppc: Build error log 2014-09-05 8:28 ` Aravinda Prasad @ 2014-09-05 8:39 ` Alexander Graf 2014-09-11 1:23 ` Alexey Kardashevskiy 1 sibling, 0 replies; 13+ messages in thread From: Alexander Graf @ 2014-09-05 8:39 UTC (permalink / raw) To: Aravinda Prasad; +Cc: benh, aik, qemu-devel, qemu-ppc, paulus On 05.09.14 10:28, Aravinda Prasad wrote: > > > On Friday 05 September 2014 01:34 PM, Alexander Graf wrote: >> >> >> On 04.09.14 13:13, Aravinda Prasad wrote: >>> Whenever there is a physical memory error due to bit >>> flips, which cannot be corrected by hardware, the error >>> is passed on to the kernel. If the memory address in >>> error belongs to guest address space then guest kernel >>> is responsible to take action. Hence the error is passed >>> on to guest via KVM by invoking 0x200 NMI vector. >>> >>> However, guest OS, as per PAPR, expects an error log >>> upon such error. This patch registers a new hcall >>> which is issued from 0x200 interrupt vector and builds >>> the error log, copies the error log to rtas space and >>> passes the address of the error log to guest >>> >>> Enhancement to KVM to perform above functionality is >>> already in upstream kernel. >>> >>> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> >>> --- >>> hw/ppc/spapr_hcall.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++ >>> include/hw/ppc/spapr.h | 4 + >>> 2 files changed, 157 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c >>> index 01650ba..c3aa448 100644 >>> --- a/hw/ppc/spapr_hcall.c >>> +++ b/hw/ppc/spapr_hcall.c >>> @@ -14,6 +14,88 @@ struct SPRSyncState { >>> target_ulong mask; >>> }; >>> >>> +/* Offset from rtas-base where error log is placed */ >>> +#define RTAS_ERROR_OFFSET (TARGET_PAGE_SIZE) >> >> You can't assume this. Please compute the value at the same place you >> compute the rtas-size. > > sure > > >> >>> + >>> +#define RTAS_ELOG_SEVERITY_SHIFT 0x5 >>> +#define RTAS_ELOG_DISPOSITION_SHIFT 0x3 >>> +#define RTAS_ELOG_INITIATOR_SHIFT 0x4 >>> + >>> +/* >>> + * Only required RTAS event severity, disposition, initiator >>> + * target and type are copied from arch/powerpc/include/asm/rtas.h >>> + */ >>> + >>> +/* RTAS event severity */ >>> +#define RTAS_SEVERITY_ERROR_SYNC 0x3 >>> + >>> +/* RTAS event disposition */ >>> +#define RTAS_DISP_NOT_RECOVERED 0x2 >>> + >>> +/* RTAS event initiator */ >>> +#define RTAS_INITIATOR_MEMORY 0x4 >>> + >>> +/* RTAS event target */ >>> +#define RTAS_TARGET_MEMORY 0x4 >>> + >>> +/* RTAS event type */ >>> +#define RTAS_TYPE_ECC_UNCORR 0x09 >>> + >>> +/* >>> + * Currently KVM only passes on the uncorrected machine >>> + * check memory error to guest. Other machine check errors >>> + * such as SLB multi-hit and TLB multi-hit are recovered >>> + * in KVM and are not passed on to guest. >>> + * >>> + * DSISR Bit for uncorrected machine check error. Based >>> + * on arch/powerpc/include/asm/mce.h >> >> Please don't include Linux code. This file is GPLv2+ licensed and I >> don't want to taint it as GPLv2 only just for the sake of mce. > > Hmm.. ok. In that case I should not copy rtas_error_log structure below > from kernel source as well. > >> >>> + */ >>> +#define PPC_BIT(bit) (0x8000000000000000ULL >> bit) >>> +#define P7_DSISR_MC_UE (PPC_BIT(48)) /* P8 too */ >>> + >>> +/* Adopted from kernel source arch/powerpc/include/asm/rtas.h */ >>> +struct rtas_error_log { >>> + /* Byte 0 */ >>> + uint8_t byte0; /* Architectural version */ >>> + >>> + /* Byte 1 */ >>> + uint8_t byte1; >>> + /* XXXXXXXX >>> + * XXX 3: Severity level of error >>> + * XX 2: Degree of recovery >>> + * X 1: Extended log present? >>> + * XX 2: Reserved >>> + */ >>> + >>> + /* Byte 2 */ >>> + uint8_t byte2; >>> + /* XXXXXXXX >>> + * XXXX 4: Initiator of event >>> + * XXXX 4: Target of failed operation >>> + */ >>> + uint8_t byte3; /* General event or error*/ >>> +}; >>> + >>> +/* >>> + * Data format in RTAS-Blob >>> + * >>> + * This structure contains error information related to Machine >>> + * Check exception. This is filled up and copied to rtas-blob >>> + * upon machine check exception. >>> + */ >>> +struct rtas_mc_log { >>> + target_ulong srr0; >>> + target_ulong srr1; >>> + /* >>> + * Beginning of error log address. This is properly >>> + * populated and passed on to OS registered machine >>> + * check notification routine upon machine check >>> + * exception >>> + */ >>> + target_ulong r3; >>> + struct rtas_error_log err_log; >>> +}; >>> + >>> static void do_spr_sync(void *arg) >>> { >>> struct SPRSyncState *s = arg; >>> @@ -586,6 +668,77 @@ static target_ulong h_rtas_update(PowerPCCPU *cpu, sPAPREnvironment *spapr, >>> return 0; >>> } >>> >>> +static target_ulong h_report_mc_err(PowerPCCPU *cpu, sPAPREnvironment *spapr, >>> + target_ulong opcode, target_ulong *args) >>> +{ >>> + struct rtas_mc_log mc_log; >>> + CPUPPCState *env = &cpu->env; >>> + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); >>> + >>> + /* >>> + * We save the original r3 register in SPRG2 in 0x200 vector, >>> + * which is patched during call to ibm.nmi-register. Original >>> + * r3 is required to be included in error log >>> + */ >>> + mc_log.r3 = env->spr[SPR_SPRG2]; >> >> Don't you have to call cpu_synchronize_registers() before you access >> SPRG2? Otherwise the value may be stale. > > Yes true. Will include. > >> >>> + >>> + /* >>> + * SRR0 and SRR1, containing nip and msr at the time of exception, >>> + * are clobbered when we return from this hcall. Hence they >>> + * need to be properly saved and restored. We save srr0 >>> + * and srr1 in rtas blob and restore it in 0x200 vector >>> + * before branching to OS registered machine check handler >>> + */ >>> + mc_log.srr0 = env->spr[SPR_SRR0]; >>> + mc_log.srr1 = env->spr[SPR_SRR1]; >>> + >>> + /* Set error log fields */ >>> + mc_log.err_log.byte0 = 0x00; >>> + mc_log.err_log.byte1 = >>> + (RTAS_SEVERITY_ERROR_SYNC << RTAS_ELOG_SEVERITY_SHIFT); >>> + mc_log.err_log.byte1 |= >>> + (RTAS_DISP_NOT_RECOVERED << RTAS_ELOG_DISPOSITION_SHIFT); >>> + mc_log.err_log.byte2 = >>> + (RTAS_INITIATOR_MEMORY << RTAS_ELOG_INITIATOR_SHIFT); >>> + mc_log.err_log.byte2 |= RTAS_TARGET_MEMORY; >>> + >>> + if (env->spr[SPR_DSISR] & P7_DSISR_MC_UE) { >>> + mc_log.err_log.byte3 = RTAS_TYPE_ECC_UNCORR; >>> + } else { >>> + mc_log.err_log.byte3 = 0x0; >>> + } >>> + >>> + /* Handle all Host/Guest LE/BE combinations */ >>> + if ((*pcc->interrupts_big_endian)(cpu)) { >> >> This should check MSR.LE, not ILE. > > I did not quite get you. Did you mean to say I should check LE bit in > MSR instead of invoking pcc->interrupts_big_endian? > > If so I should also change that check in 4/4 where I patch 0x200. Not quite - the 0x200 patch should look at ILE, because it gets executed with ILE endianness. But at the point in time when this hypercall gets issued, ILE doesn't contain useful information for us - what we care about is whether the hypercall was triggered from little endian context. Alex ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 3/4] target-ppc: Build error log 2014-09-05 8:28 ` Aravinda Prasad 2014-09-05 8:39 ` Alexander Graf @ 2014-09-11 1:23 ` Alexey Kardashevskiy 1 sibling, 0 replies; 13+ messages in thread From: Alexey Kardashevskiy @ 2014-09-11 1:23 UTC (permalink / raw) To: Aravinda Prasad, Alexander Graf; +Cc: benh, qemu-ppc, qemu-devel, paulus On 09/05/2014 06:28 PM, Aravinda Prasad wrote: > > > On Friday 05 September 2014 01:34 PM, Alexander Graf wrote: >> >> >> On 04.09.14 13:13, Aravinda Prasad wrote: >>> Whenever there is a physical memory error due to bit >>> flips, which cannot be corrected by hardware, the error >>> is passed on to the kernel. If the memory address in >>> error belongs to guest address space then guest kernel >>> is responsible to take action. Hence the error is passed >>> on to guest via KVM by invoking 0x200 NMI vector. >>> >>> However, guest OS, as per PAPR, expects an error log >>> upon such error. This patch registers a new hcall >>> which is issued from 0x200 interrupt vector and builds >>> the error log, copies the error log to rtas space and >>> passes the address of the error log to guest >>> >>> Enhancement to KVM to perform above functionality is >>> already in upstream kernel. >>> >>> Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> >>> --- >>> hw/ppc/spapr_hcall.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++ >>> include/hw/ppc/spapr.h | 4 + >>> 2 files changed, 157 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c >>> index 01650ba..c3aa448 100644 >>> --- a/hw/ppc/spapr_hcall.c >>> +++ b/hw/ppc/spapr_hcall.c >>> @@ -14,6 +14,88 @@ struct SPRSyncState { >>> target_ulong mask; >>> }; >>> >>> +/* Offset from rtas-base where error log is placed */ >>> +#define RTAS_ERROR_OFFSET (TARGET_PAGE_SIZE) >> >> You can't assume this. Please compute the value at the same place you >> compute the rtas-size. > > sure > > >> >>> + >>> +#define RTAS_ELOG_SEVERITY_SHIFT 0x5 >>> +#define RTAS_ELOG_DISPOSITION_SHIFT 0x3 >>> +#define RTAS_ELOG_INITIATOR_SHIFT 0x4 >>> + >>> +/* >>> + * Only required RTAS event severity, disposition, initiator >>> + * target and type are copied from arch/powerpc/include/asm/rtas.h >>> + */ >>> + >>> +/* RTAS event severity */ >>> +#define RTAS_SEVERITY_ERROR_SYNC 0x3 >>> + >>> +/* RTAS event disposition */ >>> +#define RTAS_DISP_NOT_RECOVERED 0x2 >>> + >>> +/* RTAS event initiator */ >>> +#define RTAS_INITIATOR_MEMORY 0x4 >>> + >>> +/* RTAS event target */ >>> +#define RTAS_TARGET_MEMORY 0x4 >>> + >>> +/* RTAS event type */ >>> +#define RTAS_TYPE_ECC_UNCORR 0x09 >>> + >>> +/* >>> + * Currently KVM only passes on the uncorrected machine >>> + * check memory error to guest. Other machine check errors >>> + * such as SLB multi-hit and TLB multi-hit are recovered >>> + * in KVM and are not passed on to guest. >>> + * >>> + * DSISR Bit for uncorrected machine check error. Based >>> + * on arch/powerpc/include/asm/mce.h >> >> Please don't include Linux code. This file is GPLv2+ licensed and I >> don't want to taint it as GPLv2 only just for the sake of mce. > > Hmm.. ok. In that case I should not copy rtas_error_log structure below > from kernel source as well. I double checked with Alex and since arch/powerpc/include/asm/mce.h is GPL2+ and spapr_hcall.c does not have a license so it is what QEMU's license is (which is also GPL2+), copying is fine here. -- Alexey Kardashevskiy IBM OzLabs, LTC Team e-mail: aik@au1.ibm.com notes: Alexey Kardashevskiy/Australia/IBM ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v2 4/4] target-ppc: Handle ibm, nmi-register RTAS call 2014-09-04 11:13 [Qemu-devel] [PATCH v2 0/4] target-ppc: Add FWNMI support in qemu for powerKVM guests Aravinda Prasad ` (2 preceding siblings ...) 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 3/4] target-ppc: Build error log Aravinda Prasad @ 2014-09-04 11:13 ` Aravinda Prasad 3 siblings, 0 replies; 13+ messages in thread From: Aravinda Prasad @ 2014-09-04 11:13 UTC (permalink / raw) To: aik, qemu-ppc, qemu-devel; +Cc: benh, paulus 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. This patch also handles the cases when multi-processors experience machine check at or about the same time. As per PAPR, subsequent processors serialize waiting for the first processor to issue the ibm,nmi-interlock call. The second processor retries if the first processor which received a machine check is still reading the error log and is yet to issue ibm,nmi-interlock call. Signed-off-by: Aravinda Prasad <aravinda@linux.vnet.ibm.com> --- hw/ppc/spapr_hcall.c | 16 +++++++ hw/ppc/spapr_rtas.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 8 +++ 3 files changed, 139 insertions(+), 1 deletion(-) diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index c3aa448..380e75c 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -96,6 +96,9 @@ struct rtas_mc_log { struct rtas_error_log err_log; }; +/* Whether machine check handling is in progress by any CPU */ +bool mc_in_progress; + static void do_spr_sync(void *arg) { struct SPRSyncState *s = arg; @@ -676,6 +679,19 @@ static target_ulong h_report_mc_err(PowerPCCPU *cpu, sPAPREnvironment *spapr, PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); /* + * Only one VCPU can process machine check NMI at a time. Hence + * set the lock mc_in_progress. Once the VCPU finishes processing + * NMI, it executes ibm,nmi-interlock and mc_in_progress is unset + * in ibm,nmi-interlock handler. Meanwhile if other VCPUs encounter + * NMI we return 0 asking the VCPU to retry h_report_mc_err + */ + if (mc_in_progress == 1) { + return 0; + } + + mc_in_progress = 1; + + /* * We save the original r3 register in SPRG2 in 0x200 vector, * which is patched during call to ibm.nmi-register. Original * r3 is required to be included in error log diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c index 2ec2a8e..dfb8ee1 100644 --- a/hw/ppc/spapr_rtas.c +++ b/hw/ppc/spapr_rtas.c @@ -36,6 +36,9 @@ #include <libfdt.h> +#define BRANCH_INST_MASK 0xFC000000 +extern bool mc_in_progress; + static void rtas_display_character(PowerPCCPU *cpu, sPAPREnvironment *spapr, uint32_t token, uint32_t nargs, target_ulong args, @@ -290,6 +293,113 @@ static void rtas_ibm_os_term(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 ori_inst = 0x60630000; + 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 */ + 0x60630000, /* ori r3,r3,KVMPPC_H_REPORT_MC_ERR + * The KVMPPC_H_REPORT_MC_ERR value is + * patched below */ + /* Issue H_CALL */ + 0x44000022, /* sc 1 */ + 0x2fa30000, /* cmplwi r3,0 */ + 0x409e0008, /* bne continue */ + 0x4800020a, /* retry KVMPPC_H_REPORT_MC_ERR */ + /* KVMPPC_H_REPORT_MC_ERR restores the SPRG2, + * hence we are free to clobber it */ + 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 */ + QEMU_BUILD_BUG_ON(sizeof(trampoline) > MC_INTERRUPT_VECTOR_SIZE); + + /* Update the KVMPPC_H_REPORT_MC_ERR value in trampoline */ + ori_inst |= KVMPPC_H_REPORT_MC_ERR; + memcpy(&trampoline[2], &ori_inst, sizeof(ori_inst)); + + /* + * Sanity check guest_machine_check_addr to prevent clobbering + * operator value in branch instruction + */ + if (guest_machine_check_addr & BRANCH_INST_MASK) { + fprintf(stderr, "Unable to register ibm,nmi_register: " + "Invalid machine check handler address\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; + memcpy(&trampoline[14], &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) +{ + /* + * VCPU issuing ibm,nmi-interlock is done with NMI handling, + * hence unset mc_in_progress. + */ + mc_in_progress = 0; + rtas_st(rets, 0, RTAS_OUT_SUCCESS); +} + static struct rtas_call { const char *name; spapr_rtas_fn fn; @@ -419,6 +529,12 @@ static void core_rtas_register_types(void) rtas_ibm_set_system_parameter); spapr_rtas_register(RTAS_IBM_OS_TERM, "ibm,os-term", rtas_ibm_os_term); + spapr_rtas_register(RTAS_IBM_NMI_REGISTER, + "ibm,nmi-register", + rtas_ibm_nmi_register); + spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, + "ibm,nmi-interlock", + rtas_ibm_nmi_interlock); } type_init(core_rtas_register_types) diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 74cf881..bbb2747 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -384,8 +384,10 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi); #define RTAS_GET_SENSOR_STATE (RTAS_TOKEN_BASE + 0x1D) #define RTAS_IBM_CONFIGURE_CONNECTOR (RTAS_TOKEN_BASE + 0x1E) #define RTAS_IBM_OS_TERM (RTAS_TOKEN_BASE + 0x1F) +#define RTAS_IBM_NMI_REGISTER (RTAS_TOKEN_BASE + 0x20) +#define RTAS_IBM_NMI_INTERLOCK (RTAS_TOKEN_BASE + 0x21) -#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x20) +#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x22) /* RTAS ibm,get-system-parameter token values */ #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20 @@ -483,4 +485,8 @@ int spapr_dma_dt(void *fdt, int node_off, const char *propname, int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname, sPAPRTCETable *tcet); +/* Machine Check Interrupt related macros */ +#define MC_INTERRUPT_VECTOR 0x200 +#define MC_INTERRUPT_VECTOR_SIZE 0x100 + #endif /* !defined (__HW_SPAPR_H__) */ ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-09-11 1:24 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-04 11:13 [Qemu-devel] [PATCH v2 0/4] target-ppc: Add FWNMI support in qemu for powerKVM guests Aravinda Prasad 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 1/4] target-ppc: Extend rtas-blob Aravinda Prasad 2014-09-05 7:42 ` [Qemu-devel] [Qemu-ppc] " Alexander Graf 2014-09-05 7:56 ` Aravinda Prasad 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 2/4] target-ppc: Register and handle HCALL to receive updated RTAS region Aravinda Prasad 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 3/4] target-ppc: Build error log Aravinda Prasad 2014-09-05 2:14 ` Alexey Kardashevskiy 2014-09-05 8:07 ` Aravinda Prasad 2014-09-05 8:04 ` [Qemu-devel] [Qemu-ppc] " Alexander Graf 2014-09-05 8:28 ` Aravinda Prasad 2014-09-05 8:39 ` Alexander Graf 2014-09-11 1:23 ` Alexey Kardashevskiy 2014-09-04 11:13 ` [Qemu-devel] [PATCH v2 4/4] target-ppc: Handle ibm, nmi-register RTAS call Aravinda Prasad
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.