* RE: [PATCH v2] memory: Freescale CoreNet Coherency Fabric error reporting driver
From: Bharat.Bhushan @ 2014-07-03 3:37 UTC (permalink / raw)
To: Scott Wood, linuxppc-dev@lists.ozlabs.org; +Cc: linux-kernel@vger.kernel.org
In-Reply-To: <20140702235211.GA3898@home.buserror.net>
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, July 03, 2014 5:22 AM
> To: linuxppc-dev@lists.ozlabs.org
> Cc: linux-kernel@vger.kernel.org; Bhushan Bharat-R65777
> Subject: [PATCH v2] memory: Freescale CoreNet Coherency Fabric error repo=
rting
> driver
>=20
> The CoreNet Coherency Fabric is part of the memory subsystem on
> some Freescale QorIQ chips. It can report coherency violations (e.g.
> due to misusing memory that is mapped noncoherent) as well as
> transactions that do not hit any local access window, or which hit a
> local access window with an invalid target ID.
>=20
> Signed-off-by: Scott Wood <scottwood@freescale.com>
Reviewed-by: Bharat Bhushan <bharat.bhushan@freescale.com>
Regards
-Bharat
> ---
> v2:
> - s/cecadr/cecaddr/ to consistently use ccf2 names
> - fix ERRDET_CTYPE_MASK value
> - use the ccf2 value for CECADDRH_ADDRH (harmless on ccf1)
> - add comment explaining why we disable detection on remove for
> ccf1 but not ccf2
> ---
> arch/powerpc/configs/corenet32_smp_defconfig | 1 +
> arch/powerpc/configs/corenet64_smp_defconfig | 1 +
> drivers/memory/Kconfig | 10 ++
> drivers/memory/Makefile | 1 +
> drivers/memory/fsl-corenet-cf.c | 251 +++++++++++++++++++++=
++++++
> 5 files changed, 264 insertions(+)
> create mode 100644 drivers/memory/fsl-corenet-cf.c
>=20
> diff --git a/arch/powerpc/configs/corenet32_smp_defconfig
> b/arch/powerpc/configs/corenet32_smp_defconfig
> index 7d0c837..6a3c58a 100644
> --- a/arch/powerpc/configs/corenet32_smp_defconfig
> +++ b/arch/powerpc/configs/corenet32_smp_defconfig
> @@ -180,3 +180,4 @@ CONFIG_CRYPTO_SHA512=3Dy
> CONFIG_CRYPTO_AES=3Dy
> # CONFIG_CRYPTO_ANSI_CPRNG is not set
> CONFIG_CRYPTO_DEV_FSL_CAAM=3Dy
> +CONFIG_FSL_CORENET_CF=3Dy
> diff --git a/arch/powerpc/configs/corenet64_smp_defconfig
> b/arch/powerpc/configs/corenet64_smp_defconfig
> index 6ae07e1..4b07bad 100644
> --- a/arch/powerpc/configs/corenet64_smp_defconfig
> +++ b/arch/powerpc/configs/corenet64_smp_defconfig
> @@ -179,3 +179,4 @@ CONFIG_CRYPTO_SHA256=3Dy
> CONFIG_CRYPTO_SHA512=3Dy
> # CONFIG_CRYPTO_ANSI_CPRNG is not set
> CONFIG_CRYPTO_DEV_FSL_CAAM=3Dy
> +CONFIG_FSL_CORENET_CF=3Dy
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index c59e9c9..fab81a1 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -61,6 +61,16 @@ config TEGRA30_MC
> analysis, especially for IOMMU/SMMU(System Memory Management
> Unit) module.
>=20
> +config FSL_CORENET_CF
> + tristate "Freescale CoreNet Error Reporting"
> + depends on FSL_SOC_BOOKE
> + help
> + Say Y for reporting of errors from the Freescale CoreNet
> + Coherency Fabric. Errors reported include accesses to
> + physical addresses that mapped by no local access window
> + (LAW) or an invalid LAW, as well as bad cache state that
> + represents a coherency violation.
> +
> config FSL_IFC
> bool
> depends on FSL_SOC
> diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
> index 71160a2..4055c47 100644
> --- a/drivers/memory/Makefile
> +++ b/drivers/memory/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_OF) +=3D of_memory.o
> endif
> obj-$(CONFIG_TI_AEMIF) +=3D ti-aemif.o
> obj-$(CONFIG_TI_EMIF) +=3D emif.o
> +obj-$(CONFIG_FSL_CORENET_CF) +=3D fsl-corenet-cf.o
> obj-$(CONFIG_FSL_IFC) +=3D fsl_ifc.o
> obj-$(CONFIG_MVEBU_DEVBUS) +=3D mvebu-devbus.o
> obj-$(CONFIG_TEGRA20_MC) +=3D tegra20-mc.o
> diff --git a/drivers/memory/fsl-corenet-cf.c b/drivers/memory/fsl-corenet=
-cf.c
> new file mode 100644
> index 0000000..c9443fc
> --- /dev/null
> +++ b/drivers/memory/fsl-corenet-cf.c
> @@ -0,0 +1,251 @@
> +/*
> + * CoreNet Coherency Fabric error reporting
> + *
> + * Copyright 2014 Freescale Semiconductor Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify=
it
> + * under the terms of the GNU General Public License as published by =
the
> + * Free Software Foundation; either version 2 of the License, or (at y=
our
> + * option) any later version.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +
> +enum ccf_version {
> + CCF1,
> + CCF2,
> +};
> +
> +struct ccf_info {
> + enum ccf_version version;
> + int err_reg_offs;
> +};
> +
> +static const struct ccf_info ccf1_info =3D {
> + .version =3D CCF1,
> + .err_reg_offs =3D 0xa00,
> +};
> +
> +static const struct ccf_info ccf2_info =3D {
> + .version =3D CCF2,
> + .err_reg_offs =3D 0xe40,
> +};
> +
> +static const struct of_device_id ccf_matches[] =3D {
> + {
> + .compatible =3D "fsl,corenet1-cf",
> + .data =3D &ccf1_info,
> + },
> + {
> + .compatible =3D "fsl,corenet2-cf",
> + .data =3D &ccf2_info,
> + },
> + {}
> +};
> +
> +struct ccf_err_regs {
> + u32 errdet; /* 0x00 Error Detect Register */
> + /* 0x04 Error Enable (ccf1)/Disable (ccf2) Register */
> + u32 errdis;
> + /* 0x08 Error Interrupt Enable Register (ccf2 only) */
> + u32 errinten;
> + u32 cecar; /* 0x0c Error Capture Attribute Register */
> + u32 cecaddrh; /* 0x10 Error Capture Address High */
> + u32 cecaddrl; /* 0x14 Error Capture Address Low */
> + u32 cecar2; /* 0x18 Error Capture Attribute Register 2 */
> +};
> +
> +/* LAE/CV also valid for errdis and errinten */
> +#define ERRDET_LAE (1 << 0) /* Local Access Error */
> +#define ERRDET_CV (1 << 1) /* Coherency Violation */
> +#define ERRDET_CTYPE_SHIFT 26 /* Capture Type (ccf2 only) */
> +#define ERRDET_CTYPE_MASK (0x1f << ERRDET_CTYPE_SHIFT)
> +#define ERRDET_CAP (1 << 31) /* Capture Valid (ccf2 only) */
> +
> +#define CECAR_VAL (1 << 0) /* Valid (ccf1 only) */
> +#define CECAR_UVT (1 << 15) /* Unavailable target ID (ccf1) */
> +#define CECAR_SRCID_SHIFT_CCF1 24
> +#define CECAR_SRCID_MASK_CCF1 (0xff << CECAR_SRCID_SHIFT_CCF1)
> +#define CECAR_SRCID_SHIFT_CCF2 18
> +#define CECAR_SRCID_MASK_CCF2 (0xff << CECAR_SRCID_SHIFT_CCF2)
> +
> +#define CECADDRH_ADDRH 0xff
> +
> +struct ccf_private {
> + const struct ccf_info *info;
> + struct device *dev;
> + void __iomem *regs;
> + struct ccf_err_regs __iomem *err_regs;
> +};
> +
> +static irqreturn_t ccf_irq(int irq, void *dev_id)
> +{
> + struct ccf_private *ccf =3D dev_id;
> + static DEFINE_RATELIMIT_STATE(ratelimit, DEFAULT_RATELIMIT_INTERVAL,
> + DEFAULT_RATELIMIT_BURST);
> + u32 errdet, cecar, cecar2;
> + u64 addr;
> + u32 src_id;
> + bool uvt =3D false;
> + bool cap_valid =3D false;
> +
> + errdet =3D ioread32be(&ccf->err_regs->errdet);
> + cecar =3D ioread32be(&ccf->err_regs->cecar);
> + cecar2 =3D ioread32be(&ccf->err_regs->cecar2);
> + addr =3D ioread32be(&ccf->err_regs->cecaddrl);
> + addr |=3D ((u64)(ioread32be(&ccf->err_regs->cecaddrh) &
> + CECADDRH_ADDRH)) << 32;
> +
> + if (!__ratelimit(&ratelimit))
> + goto out;
> +
> + switch (ccf->info->version) {
> + case CCF1:
> + if (cecar & CECAR_VAL) {
> + if (cecar & CECAR_UVT)
> + uvt =3D true;
> +
> + src_id =3D (cecar & CECAR_SRCID_MASK_CCF1) >>
> + CECAR_SRCID_SHIFT_CCF1;
> + cap_valid =3D true;
> + }
> +
> + break;
> + case CCF2:
> + if (errdet & ERRDET_CAP) {
> + src_id =3D (cecar & CECAR_SRCID_MASK_CCF2) >>
> + CECAR_SRCID_SHIFT_CCF2;
> + cap_valid =3D true;
> + }
> +
> + break;
> + }
> +
> + dev_crit(ccf->dev, "errdet 0x%08x cecar 0x%08x cecar2 0x%08x\n",
> + errdet, cecar, cecar2);
> +
> + if (errdet & ERRDET_LAE) {
> + if (uvt)
> + dev_crit(ccf->dev, "LAW Unavailable Target ID\n");
> + else
> + dev_crit(ccf->dev, "Local Access Window Error\n");
> + }
> +
> + if (errdet & ERRDET_CV)
> + dev_crit(ccf->dev, "Coherency Violation\n");
> +
> + if (cap_valid) {
> + dev_crit(ccf->dev, "address 0x%09llx, src id 0x%x\n",
> + addr, src_id);
> + }
> +
> +out:
> + iowrite32be(errdet, &ccf->err_regs->errdet);
> + return errdet ? IRQ_HANDLED : IRQ_NONE;
> +}
> +
> +static int ccf_probe(struct platform_device *pdev)
> +{
> + struct ccf_private *ccf;
> + struct resource *r;
> + const struct of_device_id *match;
> + int ret, irq;
> +
> + match =3D of_match_device(ccf_matches, &pdev->dev);
> + if (WARN_ON(!match))
> + return -ENODEV;
> +
> + ccf =3D devm_kzalloc(&pdev->dev, sizeof(*ccf), GFP_KERNEL);
> + if (!ccf)
> + return -ENOMEM;
> +
> + r =3D platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!r) {
> + dev_err(&pdev->dev, "%s: no mem resource\n", __func__);
> + return -ENXIO;
> + }
> +
> + ccf->regs =3D devm_ioremap_resource(&pdev->dev, r);
> + if (IS_ERR(ccf->regs)) {
> + dev_err(&pdev->dev, "%s: can't map mem resource\n", __func__);
> + return PTR_ERR(ccf->regs);
> + }
> +
> + ccf->dev =3D &pdev->dev;
> + ccf->info =3D match->data;
> + ccf->err_regs =3D ccf->regs + ccf->info->err_reg_offs;
> +
> + dev_set_drvdata(&pdev->dev, ccf);
> +
> + irq =3D platform_get_irq(pdev, 0);
> + if (!irq) {
> + dev_err(&pdev->dev, "%s: no irq\n", __func__);
> + return -ENXIO;
> + }
> +
> + ret =3D devm_request_irq(&pdev->dev, irq, ccf_irq, 0, pdev->name, ccf);
> + if (ret) {
> + dev_err(&pdev->dev, "%s: can't request irq\n", __func__);
> + return ret;
> + }
> +
> + switch (ccf->info->version) {
> + case CCF1:
> + /* On CCF1 this register enables rather than disables. */
> + iowrite32be(ERRDET_LAE | ERRDET_CV, &ccf->err_regs->errdis);
> + break;
> +
> + case CCF2:
> + iowrite32be(0, &ccf->err_regs->errdis);
> + iowrite32be(ERRDET_LAE | ERRDET_CV, &ccf->err_regs->errinten);
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int ccf_remove(struct platform_device *pdev)
> +{
> + struct ccf_private *ccf =3D dev_get_drvdata(&pdev->dev);
> +
> + switch (ccf->info->version) {
> + case CCF1:
> + iowrite32be(0, &ccf->err_regs->errdis);
> + break;
> +
> + case CCF2:
> + /*
> + * We clear errdis on ccf1 because that's the only way to
> + * disable interrupts, but on ccf2 there's no need to disable
> + * detection.
> + */
> + iowrite32be(0, &ccf->err_regs->errinten);
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static struct platform_driver ccf_driver =3D {
> + .driver =3D {
> + .name =3D KBUILD_MODNAME,
> + .owner =3D THIS_MODULE,
> + .of_match_table =3D ccf_matches,
> + },
> + .probe =3D ccf_probe,
> + .remove =3D ccf_remove,
> +};
> +
> +module_platform_driver(ccf_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Freescale Semiconductor");
> +MODULE_DESCRIPTION("Freescale CoreNet Coherency Fabric error reporting")=
;
> --
> 1.9.1
^ permalink raw reply
* [PATCH 1/2] powerpc/pseries: Use jump labels for hcall tracepoints
From: Anton Blanchard @ 2014-07-03 5:52 UTC (permalink / raw)
To: benh, paulus, mpe; +Cc: linuxppc-dev
hcall tracepoints add quite a few instructions to our hcall path:
plpar_hcall:
mr r2,r2
mfcr r0
stw r0,8(r1)
b 164 <---- start
ld r12,0(r2)
std r12,32(r1)
cmpdi r12,0
beq 164 <---- end
...
We have an unconditional branch that gets noped out during boot and
a load/compare/branch. We also store the tracepoint value to the
stack for the hcall_exit path to use.
By using jump labels we can simplify this to just a single nop that
gets replaced with a branch when the tracepoint is enabled:
plpar_hcall:
mr r2,r2
mfcr r0
stw r0,8(r1)
nop <----
...
If jump labels are not enabled, we fall back to the old method.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: b/arch/powerpc/include/asm/jump_label.h
===================================================================
--- a/arch/powerpc/include/asm/jump_label.h
+++ b/arch/powerpc/include/asm/jump_label.h
@@ -10,6 +10,7 @@
* 2 of the License, or (at your option) any later version.
*/
+#ifndef __ASSEMBLY__
#include <linux/types.h>
#include <asm/feature-fixups.h>
@@ -42,4 +43,12 @@ struct jump_entry {
jump_label_t key;
};
+#else
+#define ARCH_STATIC_BRANCH(LABEL, KEY) \
+1098: nop; \
+ .pushsection __jump_table, "aw"; \
+ FTR_ENTRY_LONG 1098b, LABEL, KEY; \
+ .popsection
+#endif
+
#endif /* _ASM_POWERPC_JUMP_LABEL_H */
Index: b/arch/powerpc/platforms/pseries/hvCall.S
===================================================================
--- a/arch/powerpc/platforms/pseries/hvCall.S
+++ b/arch/powerpc/platforms/pseries/hvCall.S
@@ -12,9 +12,13 @@
#include <asm/ppc_asm.h>
#include <asm/asm-offsets.h>
#include <asm/ptrace.h>
+#include <asm/jump_label.h>
+
+ .section ".text"
#ifdef CONFIG_TRACEPOINTS
+#ifndef CONFIG_JUMP_LABEL
.section ".toc","aw"
.globl hcall_tracepoint_refcount
@@ -22,21 +26,13 @@ hcall_tracepoint_refcount:
.llong 0
.section ".text"
+#endif
/*
* precall must preserve all registers. use unused STK_PARAM()
- * areas to save snapshots and opcode. We branch around this
- * in early init (eg when populating the MMU hashtable) by using an
- * unconditional cpu feature.
+ * areas to save snapshots and opcode.
*/
#define HCALL_INST_PRECALL(FIRST_REG) \
-BEGIN_FTR_SECTION; \
- b 1f; \
-END_FTR_SECTION(0, 1); \
- ld r12,hcall_tracepoint_refcount@toc(r2); \
- std r12,32(r1); \
- cmpdi r12,0; \
- beq+ 1f; \
mflr r0; \
std r3,STK_PARAM(R3)(r1); \
std r4,STK_PARAM(R4)(r1); \
@@ -60,22 +56,13 @@ END_FTR_SECTION(0, 1); \
ld r8,STK_PARAM(R8)(r1); \
ld r9,STK_PARAM(R9)(r1); \
ld r10,STK_PARAM(R10)(r1); \
- mtlr r0; \
-1:
+ mtlr r0
/*
* postcall is performed immediately before function return which
- * allows liberal use of volatile registers. We branch around this
- * in early init (eg when populating the MMU hashtable) by using an
- * unconditional cpu feature.
+ * allows liberal use of volatile registers.
*/
#define __HCALL_INST_POSTCALL \
-BEGIN_FTR_SECTION; \
- b 1f; \
-END_FTR_SECTION(0, 1); \
- ld r12,32(r1); \
- cmpdi r12,0; \
- beq+ 1f; \
mflr r0; \
ld r6,STK_PARAM(R3)(r1); \
std r3,STK_PARAM(R3)(r1); \
@@ -87,8 +74,7 @@ END_FTR_SECTION(0, 1); \
addi r1,r1,STACK_FRAME_OVERHEAD; \
ld r0,16(r1); \
ld r3,STK_PARAM(R3)(r1); \
- mtlr r0; \
-1:
+ mtlr r0
#define HCALL_INST_POSTCALL_NORETS \
li r5,0; \
@@ -98,37 +84,62 @@ END_FTR_SECTION(0, 1); \
mr r5,BUFREG; \
__HCALL_INST_POSTCALL
+#ifdef CONFIG_JUMP_LABEL
+#define HCALL_BRANCH(LABEL) \
+ ARCH_STATIC_BRANCH(LABEL, hcall_tracepoint_key)
+#else
+
+/*
+ * We branch around this in early init (eg when populating the MMU
+ * hashtable) by using an unconditional cpu feature.
+ */
+#define HCALL_BRANCH(LABEL) \
+BEGIN_FTR_SECTION; \
+ b 1f; \
+END_FTR_SECTION(0, 1); \
+ ld r12,hcall_tracepoint_refcount@toc(r2); \
+ std r12,32(r1); \
+ cmpdi r12,0; \
+ bne- LABEL; \
+1:
+#endif
+
#else
#define HCALL_INST_PRECALL(FIRST_ARG)
#define HCALL_INST_POSTCALL_NORETS
#define HCALL_INST_POSTCALL(BUFREG)
+#define HCALL_BRANCH(LABEL)
#endif
- .text
-
_GLOBAL_TOC(plpar_hcall_norets)
HMT_MEDIUM
mfcr r0
stw r0,8(r1)
-
- HCALL_INST_PRECALL(R4)
-
+ HCALL_BRANCH(plpar_hcall_norets_trace)
HVSC /* invoke the hypervisor */
- HCALL_INST_POSTCALL_NORETS
-
lwz r0,8(r1)
mtcrf 0xff,r0
blr /* return r3 = status */
+#ifdef CONFIG_TRACEPOINTS
+plpar_hcall_norets_trace:
+ HCALL_INST_PRECALL(R4)
+ HVSC
+ HCALL_INST_POSTCALL_NORETS
+ lwz r0,8(r1)
+ mtcrf 0xff,r0
+ blr
+#endif
+
_GLOBAL_TOC(plpar_hcall)
HMT_MEDIUM
mfcr r0
stw r0,8(r1)
- HCALL_INST_PRECALL(R5)
+ HCALL_BRANCH(plpar_hcall_trace)
std r4,STK_PARAM(R4)(r1) /* Save ret buffer */
@@ -147,12 +158,40 @@ _GLOBAL_TOC(plpar_hcall)
std r6, 16(r12)
std r7, 24(r12)
+ lwz r0,8(r1)
+ mtcrf 0xff,r0
+
+ blr /* return r3 = status */
+
+#ifdef CONFIG_TRACEPOINTS
+plpar_hcall_trace:
+ HCALL_INST_PRECALL(R5)
+
+ std r4,STK_PARAM(R4)(r1)
+ mr r0,r4
+
+ mr r4,r5
+ mr r5,r6
+ mr r6,r7
+ mr r7,r8
+ mr r8,r9
+ mr r9,r10
+
+ HVSC
+
+ ld r12,STK_PARAM(R4)(r1)
+ std r4,0(r12)
+ std r5,8(r12)
+ std r6,16(r12)
+ std r7,24(r12)
+
HCALL_INST_POSTCALL(r12)
lwz r0,8(r1)
mtcrf 0xff,r0
- blr /* return r3 = status */
+ blr
+#endif
/*
* plpar_hcall_raw can be called in real mode. kexec/kdump need some
@@ -194,7 +233,7 @@ _GLOBAL_TOC(plpar_hcall9)
mfcr r0
stw r0,8(r1)
- HCALL_INST_PRECALL(R5)
+ HCALL_BRANCH(plpar_hcall9_trace)
std r4,STK_PARAM(R4)(r1) /* Save ret buffer */
@@ -222,12 +261,49 @@ _GLOBAL_TOC(plpar_hcall9)
std r11,56(r12)
std r0, 64(r12)
+ lwz r0,8(r1)
+ mtcrf 0xff,r0
+
+ blr /* return r3 = status */
+
+#ifdef CONFIG_TRACEPOINTS
+plpar_hcall9_trace:
+ HCALL_INST_PRECALL(R5)
+
+ std r4,STK_PARAM(R4)(r1)
+ mr r0,r4
+
+ mr r4,r5
+ mr r5,r6
+ mr r6,r7
+ mr r7,r8
+ mr r8,r9
+ mr r9,r10
+ ld r10,STK_PARAM(R11)(r1)
+ ld r11,STK_PARAM(R12)(r1)
+ ld r12,STK_PARAM(R13)(r1)
+
+ HVSC
+
+ mr r0,r12
+ ld r12,STK_PARAM(R4)(r1)
+ std r4,0(r12)
+ std r5,8(r12)
+ std r6,16(r12)
+ std r7,24(r12)
+ std r8,32(r12)
+ std r9,40(r12)
+ std r10,48(r12)
+ std r11,56(r12)
+ std r0,64(r12)
+
HCALL_INST_POSTCALL(r12)
lwz r0,8(r1)
mtcrf 0xff,r0
- blr /* return r3 = status */
+ blr
+#endif
/* See plpar_hcall_raw to see why this is needed */
_GLOBAL(plpar_hcall9_raw)
Index: b/arch/powerpc/platforms/pseries/lpar.c
===================================================================
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -26,6 +26,7 @@
#include <linux/dma-mapping.h>
#include <linux/console.h>
#include <linux/export.h>
+#include <linux/static_key.h>
#include <asm/processor.h>
#include <asm/mmu.h>
#include <asm/page.h>
@@ -649,6 +650,19 @@ EXPORT_SYMBOL(arch_free_page);
#endif
#ifdef CONFIG_TRACEPOINTS
+#ifdef CONFIG_JUMP_LABEL
+struct static_key hcall_tracepoint_key = STATIC_KEY_INIT;
+
+void hcall_tracepoint_regfunc(void)
+{
+ static_key_slow_inc(&hcall_tracepoint_key);
+}
+
+void hcall_tracepoint_unregfunc(void)
+{
+ static_key_slow_dec(&hcall_tracepoint_key);
+}
+#else
/*
* We optimise our hcall path by placing hcall_tracepoint_refcount
* directly in the TOC so we can check if the hcall tracepoints are
@@ -658,13 +672,6 @@ EXPORT_SYMBOL(arch_free_page);
/* NB: reg/unreg are called while guarded with the tracepoints_mutex */
extern long hcall_tracepoint_refcount;
-/*
- * Since the tracing code might execute hcalls we need to guard against
- * recursion. One example of this are spinlocks calling H_YIELD on
- * shared processor partitions.
- */
-static DEFINE_PER_CPU(unsigned int, hcall_trace_depth);
-
void hcall_tracepoint_regfunc(void)
{
hcall_tracepoint_refcount++;
@@ -674,6 +681,15 @@ void hcall_tracepoint_unregfunc(void)
{
hcall_tracepoint_refcount--;
}
+#endif
+
+/*
+ * Since the tracing code might execute hcalls we need to guard against
+ * recursion. One example of this are spinlocks calling H_YIELD on
+ * shared processor partitions.
+ */
+static DEFINE_PER_CPU(unsigned int, hcall_trace_depth);
+
void __trace_hcall_entry(unsigned long opcode, unsigned long *args)
{
^ permalink raw reply
* [PATCH 2/2] powerpc/pseries: optimise hcall tracepoints
From: Anton Blanchard @ 2014-07-03 5:52 UTC (permalink / raw)
To: benh, paulus, mpe; +Cc: linuxppc-dev
In-Reply-To: <20140703155203.33af5387@kryten>
Now that we execute the hcall tracepoint entry and exit code out of
line, we can use the same stack across both functions.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: b/arch/powerpc/platforms/pseries/hvCall.S
===================================================================
--- a/arch/powerpc/platforms/pseries/hvCall.S
+++ b/arch/powerpc/platforms/pseries/hvCall.S
@@ -46,33 +46,27 @@ hcall_tracepoint_refcount:
addi r4,r1,STK_PARAM(FIRST_REG); \
stdu r1,-STACK_FRAME_OVERHEAD(r1); \
bl __trace_hcall_entry; \
- addi r1,r1,STACK_FRAME_OVERHEAD; \
- ld r0,16(r1); \
- ld r3,STK_PARAM(R3)(r1); \
- ld r4,STK_PARAM(R4)(r1); \
- ld r5,STK_PARAM(R5)(r1); \
- ld r6,STK_PARAM(R6)(r1); \
- ld r7,STK_PARAM(R7)(r1); \
- ld r8,STK_PARAM(R8)(r1); \
- ld r9,STK_PARAM(R9)(r1); \
- ld r10,STK_PARAM(R10)(r1); \
- mtlr r0
+ ld r3,STACK_FRAME_OVERHEAD+STK_PARAM(R3)(r1); \
+ ld r4,STACK_FRAME_OVERHEAD+STK_PARAM(R4)(r1); \
+ ld r5,STACK_FRAME_OVERHEAD+STK_PARAM(R5)(r1); \
+ ld r6,STACK_FRAME_OVERHEAD+STK_PARAM(R6)(r1); \
+ ld r7,STACK_FRAME_OVERHEAD+STK_PARAM(R7)(r1); \
+ ld r8,STACK_FRAME_OVERHEAD+STK_PARAM(R8)(r1); \
+ ld r9,STACK_FRAME_OVERHEAD+STK_PARAM(R9)(r1); \
+ ld r10,STACK_FRAME_OVERHEAD+STK_PARAM(R10)(r1)
/*
* postcall is performed immediately before function return which
* allows liberal use of volatile registers.
*/
#define __HCALL_INST_POSTCALL \
- mflr r0; \
- ld r6,STK_PARAM(R3)(r1); \
- std r3,STK_PARAM(R3)(r1); \
+ ld r0,STACK_FRAME_OVERHEAD+STK_PARAM(R3)(r1); \
+ std r3,STACK_FRAME_OVERHEAD+STK_PARAM(R3)(r1); \
mr r4,r3; \
- mr r3,r6; \
- std r0,16(r1); \
- stdu r1,-STACK_FRAME_OVERHEAD(r1); \
+ mr r3,r0; \
bl __trace_hcall_exit; \
+ ld r0,STACK_FRAME_OVERHEAD+16(r1); \
addi r1,r1,STACK_FRAME_OVERHEAD; \
- ld r0,16(r1); \
ld r3,STK_PARAM(R3)(r1); \
mtlr r0
@@ -279,14 +273,14 @@ plpar_hcall9_trace:
mr r7,r8
mr r8,r9
mr r9,r10
- ld r10,STK_PARAM(R11)(r1)
- ld r11,STK_PARAM(R12)(r1)
- ld r12,STK_PARAM(R13)(r1)
+ ld r10,STACK_FRAME_OVERHEAD+STK_PARAM(R11)(r1)
+ ld r11,STACK_FRAME_OVERHEAD+STK_PARAM(R12)(r1)
+ ld r12,STACK_FRAME_OVERHEAD+STK_PARAM(R13)(r1)
HVSC
mr r0,r12
- ld r12,STK_PARAM(R4)(r1)
+ ld r12,STACK_FRAME_OVERHEAD+STK_PARAM(R4)(r1)
std r4,0(r12)
std r5,8(r12)
std r6,16(r12)
^ permalink raw reply
* [PATCH 0/3] POWER8 PMU bugfixes
From: Joel Stanley @ 2014-07-03 6:12 UTC (permalink / raw)
To: paulus, benh, mpe; +Cc: linuxppc-dev
These three patches are required for correct operation of perf counters on
POWER8 boxes when running KVM guests. There were two bugs; not clearing MMCR2
and writing over the saved value when entering a guest. However the first
obscured the second.
Thanks to who mpe helped diagnose the issue and pointed out the fix.
Joel Stanley (3):
powerpc/kvm: Remove redundant save of SIER AND MMCR2
powerpc/perf: Add PPMU_ARCH_207S define
powerpc/perf: Clear MMCR2 when enabling PMU
arch/powerpc/include/asm/perf_event_server.h | 3 +--
arch/powerpc/kvm/book3s_hv_interrupts.S | 5 -----
arch/powerpc/perf/core-book3s.c | 9 ++++++---
arch/powerpc/perf/power8-pmu.c | 2 +-
4 files changed, 8 insertions(+), 11 deletions(-)
--
2.0.0
^ permalink raw reply
* [PATCH 1/3] powerpc/kvm: Remove redundant save of SIER AND MMCR2
From: Joel Stanley @ 2014-07-03 6:12 UTC (permalink / raw)
To: paulus, benh, mpe; +Cc: linuxppc-dev
In-Reply-To: <1404367956-19515-1-git-send-email-joel@jms.id.au>
These two registers are already saved in the block above. Aside from
being unnecessary, by the time we get down to the second save location
r8 no longer contains MMCR2, so we are clobbering the saved value with
PMC5.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
arch/powerpc/kvm/book3s_hv_interrupts.S | 5 -----
1 file changed, 5 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_interrupts.S b/arch/powerpc/kvm/book3s_hv_interrupts.S
index 8c86422..731be74 100644
--- a/arch/powerpc/kvm/book3s_hv_interrupts.S
+++ b/arch/powerpc/kvm/book3s_hv_interrupts.S
@@ -127,11 +127,6 @@ BEGIN_FTR_SECTION
stw r10, HSTATE_PMC + 24(r13)
stw r11, HSTATE_PMC + 28(r13)
END_FTR_SECTION_IFSET(CPU_FTR_ARCH_201)
-BEGIN_FTR_SECTION
- mfspr r9, SPRN_SIER
- std r8, HSTATE_MMCR + 40(r13)
- std r9, HSTATE_MMCR + 48(r13)
-END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
31:
/*
--
2.0.0
^ permalink raw reply related
* [PATCH 2/3] powerpc/perf: Add PPMU_ARCH_207S define
From: Joel Stanley @ 2014-07-03 6:12 UTC (permalink / raw)
To: paulus, benh, mpe; +Cc: linuxppc-dev
In-Reply-To: <1404367956-19515-1-git-send-email-joel@jms.id.au>
Instead of separate bits for every POWER8 PMU feature, have a single one
for v2.07 of the architecture.
This saves us adding a MMCR2 define for a future patch.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
arch/powerpc/include/asm/perf_event_server.h | 3 +--
arch/powerpc/perf/core-book3s.c | 6 +++---
arch/powerpc/perf/power8-pmu.c | 2 +-
3 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
index 9ed73714..b3e9360 100644
--- a/arch/powerpc/include/asm/perf_event_server.h
+++ b/arch/powerpc/include/asm/perf_event_server.h
@@ -61,8 +61,7 @@ struct power_pmu {
#define PPMU_SIAR_VALID 0x00000010 /* Processor has SIAR Valid bit */
#define PPMU_HAS_SSLOT 0x00000020 /* Has sampled slot in MMCRA */
#define PPMU_HAS_SIER 0x00000040 /* Has SIER */
-#define PPMU_BHRB 0x00000080 /* has BHRB feature enabled */
-#define PPMU_EBB 0x00000100 /* supports event based branch */
+#define PPMU_ARCH_207S 0x00000080 /* PMC is architecture v2.07S */
/*
* Values for flags to get_alternatives()
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 4520c93..a2ff1bd 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -485,7 +485,7 @@ static bool is_ebb_event(struct perf_event *event)
* check that the PMU supports EBB, meaning those that don't can still
* use bit 63 of the event code for something else if they wish.
*/
- return (ppmu->flags & PPMU_EBB) &&
+ return (ppmu->flags & PPMU_ARCH_207S) &&
((event->attr.config >> PERF_EVENT_CONFIG_EBB_SHIFT) & 1);
}
@@ -777,7 +777,7 @@ void perf_event_print_debug(void)
if (ppmu->flags & PPMU_HAS_SIER)
sier = mfspr(SPRN_SIER);
- if (ppmu->flags & PPMU_EBB) {
+ if (ppmu->flags & PPMU_ARCH_207S) {
pr_info("MMCR2: %016lx EBBHR: %016lx\n",
mfspr(SPRN_MMCR2), mfspr(SPRN_EBBHR));
pr_info("EBBRR: %016lx BESCR: %016lx\n",
@@ -1696,7 +1696,7 @@ static int power_pmu_event_init(struct perf_event *event)
if (has_branch_stack(event)) {
/* PMU has BHRB enabled */
- if (!(ppmu->flags & PPMU_BHRB))
+ if (!(ppmu->flags & PPMU_ARCH_207S))
return -EOPNOTSUPP;
}
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index fe2763b..639cd91 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -792,7 +792,7 @@ static struct power_pmu power8_pmu = {
.get_constraint = power8_get_constraint,
.get_alternatives = power8_get_alternatives,
.disable_pmc = power8_disable_pmc,
- .flags = PPMU_HAS_SSLOT | PPMU_HAS_SIER | PPMU_BHRB | PPMU_EBB,
+ .flags = PPMU_HAS_SSLOT | PPMU_HAS_SIER | PPMU_ARCH_207S,
.n_generic = ARRAY_SIZE(power8_generic_events),
.generic_events = power8_generic_events,
.cache_events = &power8_cache_events,
--
2.0.0
^ permalink raw reply related
* [PATCH 3/3] powerpc/perf: Clear MMCR2 when enabling PMU
From: Joel Stanley @ 2014-07-03 6:12 UTC (permalink / raw)
To: paulus, benh, mpe; +Cc: linuxppc-dev
In-Reply-To: <1404367956-19515-1-git-send-email-joel@jms.id.au>
On POWER8 when switching to a KVM guest we set bits in MMCR2 to freeze
the PMU counters. Aside from on boot they are then never reset,
resulting in stuck perf counters for any user in the guest or host.
We now set MMCR2 to 0 whenever enabling the PMU, which provides a sane
state for perf to use the PMU counters under either the guest or the
host.
This was manifesting as a bug with ppc64_cpu --frequency:
$ sudo ppc64_cpu --frequency
WARNING: couldn't run on cpu 0
WARNING: couldn't run on cpu 8
...
WARNING: couldn't run on cpu 144
WARNING: couldn't run on cpu 152
min: 18446744073.710 GHz (cpu -1)
max: 0.000 GHz (cpu -1)
avg: 0.000 GHz
The command uses a perf counter to measure CPU cycles over a fixed
amount of time, in order to approximate the frequency of the machine.
The counters were returning zero once a guest was started, regardless of
weather it was still running or had been shut down.
By dumping the value of MMCR2, it was observed that once a guest is
running MMCR2 is set to 1s - which stops counters from running:
$ sudo sh -c 'echo p > /proc/sysrq-trigger'
CPU: 0 PMU registers, ppmu = POWER8 n_counters = 6
PMC1: 5b635e38 PMC2: 00000000 PMC3: 00000000 PMC4: 00000000
PMC5: 1bf5a646 PMC6: 5793d378 PMC7: deadbeef PMC8: deadbeef
MMCR0: 0000000080000000 MMCR1: 000000001e000000 MMCRA: 0000040000000000
MMCR2: fffffffffffffc00 EBBHR: 0000000000000000
EBBRR: 0000000000000000 BESCR: 0000000000000000
SIAR: 00000000000a51cc SDAR: c00000000fc40000 SIER: 0000000001000000
This is done unconditionally in book3s_hv_interrupts.S upon entering the
guest, and the original value is only save/restored if the host has
indicated it was using the PMU. This is okay, however the user of the
PMU needs to ensure that it is in a defined state when it starts using
it.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
arch/powerpc/perf/core-book3s.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index a2ff1bd..bae697c 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1300,6 +1300,9 @@ static void power_pmu_enable(struct pmu *pmu)
write_mmcr0(cpuhw, mmcr0);
+ if (ppmu->flags & PPMU_ARCH_207S)
+ mtspr(SPRN_MMCR2, 0);
+
/*
* Enable instruction sampling if necessary
*/
--
2.0.0
^ permalink raw reply related
* [PATCH] powernv: Add OPAL tracepoints
From: Anton Blanchard @ 2014-07-03 7:20 UTC (permalink / raw)
To: benh, paulus, mpe, mikey, paulmck; +Cc: linuxppc-dev
Knowing how long we spend in firmware calls is an important part of
minimising OS jitter.
This patch adds tracepoints to each OPAL call. If tracepoints are
enabled we branch out to a common routine that calls an entry and exit
tracepoint.
This allows us to write tools that monitor the frequency and duration
of OPAL calls, eg:
name count total(ms) min(ms) max(ms) avg(ms) period(ms)
OPAL_HANDLE_INTERRUPT 5 0.199 0.037 0.042 0.040 12547.545
OPAL_POLL_EVENTS 204 2.590 0.012 0.036 0.013 2264.899
OPAL_PCI_MSI_EOI 2830 3.066 0.001 0.005 0.001 81.166
We use jump labels if configured, which means we only add a single
nop instruction to every OPAL call when the tracepoints are disabled.
Signed-off-by: Anton Blanchard <anton@samba.org>
---
Index: b/arch/powerpc/include/asm/trace.h
===================================================================
--- a/arch/powerpc/include/asm/trace.h
+++ b/arch/powerpc/include/asm/trace.h
@@ -99,6 +99,51 @@ TRACE_EVENT_FN(hcall_exit,
);
#endif
+#ifdef CONFIG_PPC_POWERNV
+extern void opal_tracepoint_regfunc(void);
+extern void opal_tracepoint_unregfunc(void);
+
+TRACE_EVENT_FN(opal_entry,
+
+ TP_PROTO(unsigned long opcode, unsigned long *args),
+
+ TP_ARGS(opcode, args),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, opcode)
+ ),
+
+ TP_fast_assign(
+ __entry->opcode = opcode;
+ ),
+
+ TP_printk("opcode=%lu", __entry->opcode),
+
+ opal_tracepoint_regfunc, opal_tracepoint_unregfunc
+);
+
+TRACE_EVENT_FN(opal_exit,
+
+ TP_PROTO(unsigned long opcode, unsigned long retval),
+
+ TP_ARGS(opcode, retval),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, opcode)
+ __field(unsigned long, retval)
+ ),
+
+ TP_fast_assign(
+ __entry->opcode = opcode;
+ __entry->retval = retval;
+ ),
+
+ TP_printk("opcode=%lu retval=%lu", __entry->opcode, __entry->retval),
+
+ opal_tracepoint_regfunc, opal_tracepoint_unregfunc
+);
+#endif
+
#endif /* _TRACE_POWERPC_H */
#undef TRACE_INCLUDE_PATH
Index: b/arch/powerpc/platforms/powernv/Makefile
===================================================================
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_PCI) += pci.o pci-p5ioc2.o
obj-$(CONFIG_EEH) += eeh-ioda.o eeh-powernv.o
obj-$(CONFIG_PPC_SCOM) += opal-xscom.o
obj-$(CONFIG_MEMORY_FAILURE) += opal-memory-errors.o
+obj-$(CONFIG_TRACEPOINTS) += opal-tracepoints.o
Index: b/arch/powerpc/platforms/powernv/opal-wrappers.S
===================================================================
--- a/arch/powerpc/platforms/powernv/opal-wrappers.S
+++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
@@ -13,30 +13,69 @@
#include <asm/hvcall.h>
#include <asm/asm-offsets.h>
#include <asm/opal.h>
+#include <asm/jump_label.h>
+
+ .section ".text"
+
+#ifdef CONFIG_TRACEPOINTS
+#ifdef CONFIG_JUMP_LABEL
+#define OPAL_BRANCH(LABEL) \
+ ARCH_STATIC_BRANCH(LABEL, opal_tracepoint_key)
+#else
+
+ .section ".toc","aw"
+
+ .globl opal_tracepoint_refcount
+opal_tracepoint_refcount:
+ .llong 0
+
+ .section ".text"
+
+/*
+ * We branch around this in early init by using an unconditional cpu
+ * feature.
+ */
+#define OPAL_BRANCH(LABEL) \
+BEGIN_FTR_SECTION; \
+ b 1f; \
+END_FTR_SECTION(0, 1); \
+ ld r12,opal_tracepoint_refcount@toc(r2); \
+ std r12,32(r1); \
+ cmpdi r12,0; \
+ bne- LABEL; \
+1:
+
+#endif
+
+#else
+#define OPAL_BRANCH(LABEL)
+#endif
/* TODO:
*
* - Trace irqs in/off (needs saving/restoring all args, argh...)
* - Get r11 feed up by Dave so I can have better register usage
*/
+
#define OPAL_CALL(name, token) \
_GLOBAL(name); \
mflr r0; \
- mfcr r12; \
std r0,16(r1); \
+ li r0,token; \
+ OPAL_BRANCH(opal_tracepoint_entry) \
+ mfcr r12; \
stw r12,8(r1); \
std r1,PACAR1(r13); \
- li r0,0; \
+ li r11,0; \
mfmsr r12; \
- ori r0,r0,MSR_EE; \
+ ori r11,r11,MSR_EE; \
std r12,PACASAVEDMSR(r13); \
- andc r12,r12,r0; \
+ andc r12,r12,r11; \
mtmsrd r12,1; \
- LOAD_REG_ADDR(r0,opal_return); \
- mtlr r0; \
- li r0,MSR_DR|MSR_IR|MSR_LE;\
- andc r12,r12,r0; \
- li r0,token; \
+ LOAD_REG_ADDR(r11,opal_return); \
+ mtlr r11; \
+ li r11,MSR_DR|MSR_IR|MSR_LE;\
+ andc r12,r12,r11; \
mtspr SPRN_HSRR1,r12; \
LOAD_REG_ADDR(r11,opal); \
ld r12,8(r11); \
@@ -61,6 +100,64 @@ opal_return:
mtcr r4;
rfid
+#ifdef CONFIG_TRACEPOINTS
+opal_tracepoint_entry:
+ stdu r1,-STACKFRAMESIZE(r1)
+ std r0,STK_REG(R23)(r1)
+ std r3,STK_REG(R24)(r1)
+ std r4,STK_REG(R25)(r1)
+ std r5,STK_REG(R26)(r1)
+ std r6,STK_REG(R27)(r1)
+ std r7,STK_REG(R28)(r1)
+ std r8,STK_REG(R29)(r1)
+ std r9,STK_REG(R30)(r1)
+ std r10,STK_REG(R31)(r1)
+ mr r3,r0
+ addi r4,r1,STK_REG(R24)
+ bl __trace_opal_entry
+ ld r0,STK_REG(R23)(r1)
+ ld r3,STK_REG(R24)(r1)
+ ld r4,STK_REG(R25)(r1)
+ ld r5,STK_REG(R26)(r1)
+ ld r6,STK_REG(R27)(r1)
+ ld r7,STK_REG(R28)(r1)
+ ld r8,STK_REG(R29)(r1)
+ ld r9,STK_REG(R30)(r1)
+ ld r10,STK_REG(R31)(r1)
+ LOAD_REG_ADDR(r11,opal_tracepoint_return)
+ mfcr r12
+ std r11,16(r1)
+ stw r12,8(r1)
+ std r1,PACAR1(r13)
+ li r11,0
+ mfmsr r12
+ ori r11,r11,MSR_EE
+ std r12,PACASAVEDMSR(r13)
+ andc r12,r12,r11
+ mtmsrd r12,1
+ LOAD_REG_ADDR(r11,opal_return)
+ mtlr r11
+ li r11,MSR_DR|MSR_IR|MSR_LE
+ andc r12,r12,r11
+ mtspr SPRN_HSRR1,r12
+ LOAD_REG_ADDR(r11,opal)
+ ld r12,8(r11)
+ ld r2,0(r11)
+ mtspr SPRN_HSRR0,r12
+ hrfid
+
+opal_tracepoint_return:
+ std r3,STK_REG(R31)(r1)
+ mr r4,r3
+ ld r0,STK_REG(R23)(r1)
+ bl __trace_opal_exit
+ ld r3,STK_REG(R31)(r1)
+ addi r1,r1,STACKFRAMESIZE
+ ld r0,16(r1)
+ mtlr r0
+ blr
+#endif
+
OPAL_CALL(opal_invalid_call, OPAL_INVALID_CALL);
OPAL_CALL(opal_console_write, OPAL_CONSOLE_WRITE);
OPAL_CALL(opal_console_read, OPAL_CONSOLE_READ);
Index: b/arch/powerpc/platforms/powernv/opal-tracepoints.c
===================================================================
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-tracepoints.c
@@ -0,0 +1,84 @@
+#include <linux/percpu.h>
+#include <linux/jump_label.h>
+#include <asm/trace.h>
+
+#ifdef CONFIG_JUMP_LABEL
+struct static_key opal_tracepoint_key = STATIC_KEY_INIT;
+
+void opal_tracepoint_regfunc(void)
+{
+ static_key_slow_inc(&opal_tracepoint_key);
+}
+
+void opal_tracepoint_unregfunc(void)
+{
+ static_key_slow_dec(&opal_tracepoint_key);
+}
+#else
+/*
+ * We optimise OPAL calls by placing opal_tracepoint_refcount
+ * directly in the TOC so we can check if the opal tracepoints are
+ * enabled via a single load.
+ */
+
+/* NB: reg/unreg are called while guarded with the tracepoints_mutex */
+extern long opal_tracepoint_refcount;
+
+void opal_tracepoint_regfunc(void)
+{
+ opal_tracepoint_refcount++;
+}
+
+void opal_tracepoint_unregfunc(void)
+{
+ opal_tracepoint_refcount--;
+}
+#endif
+
+/*
+ * Since the tracing code might execute OPAL calls we need to guard against
+ * recursion.
+ */
+static DEFINE_PER_CPU(unsigned int, opal_trace_depth);
+
+void __trace_opal_entry(unsigned long opcode, unsigned long *args)
+{
+ unsigned long flags;
+ unsigned int *depth;
+
+ local_irq_save(flags);
+
+ depth = &__get_cpu_var(opal_trace_depth);
+
+ if (*depth)
+ goto out;
+
+ (*depth)++;
+ preempt_disable();
+ trace_opal_entry(opcode, args);
+ (*depth)--;
+
+out:
+ local_irq_restore(flags);
+}
+
+void __trace_opal_exit(long opcode, unsigned long retval)
+{
+ unsigned long flags;
+ unsigned int *depth;
+
+ local_irq_save(flags);
+
+ depth = &__get_cpu_var(opal_trace_depth);
+
+ if (*depth)
+ goto out;
+
+ (*depth)++;
+ trace_opal_exit(opcode, retval);
+ preempt_enable();
+ (*depth)--;
+
+out:
+ local_irq_restore(flags);
+}
^ permalink raw reply
* Re: [PATCH v5] flexcan: add err_irq handler for flexcan
From: Marc Kleine-Budde @ 2014-07-03 9:14 UTC (permalink / raw)
To: Zhao Qiang, linuxppc-dev, wg, linux-can, B07421
In-Reply-To: <1404354180-46431-1-git-send-email-B45475@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 1081 bytes --]
On 07/03/2014 04:23 AM, Zhao Qiang wrote:
> when flexcan is not physically linked, command 'cantest' will
> trigger an err_irq, add err_irq handler for it.
>
> Signed-off-by: Zhao Qiang <B45475@freescale.com>
> ---
> Changes for v2:
> - use a space instead of tab
> - use flexcan_poll_state instead of print
> Changes for v3:
> - return IRQ_HANDLED if err is triggered
> - stop transmitted packets when there is an err_interrupt
> Changes for v4:
> - call flexcan_irq
> Changes for v5:
> - move err_int_handling code from flexcan_irq to flexcan_err_irq
> - call flexcan_err_irq from flexcan_irq
Why do you move the RX IRQ handling into the error IRQ function? If I
understand you correctly you only want to do error interrupt handling
the error IRQ function, right?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]
^ permalink raw reply
* RE: [PATCH 1/3] PCI/MSI: Add pci_enable_msi_partial()
From: David Laight @ 2014-07-03 9:20 UTC (permalink / raw)
To: 'Bjorn Helgaas', Alexander Gordeev
Cc: linux-mips@linux-mips.org, linux-s390@vger.kernel.org,
linux-doc@vger.kernel.org, linux-pci@vger.kernel.org,
x86@kernel.org, linux-kernel@vger.kernel.org,
linux-ide@vger.kernel.org, iommu@lists.linux-foundation.org,
xen-devel@lists.xenproject.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20140702202201.GA28852@google.com>
RnJvbTogQmpvcm4gSGVsZ2Fhcw0KPiBPbiBUdWUsIEp1biAxMCwgMjAxNCBhdCAwMzoxMDozMFBN
ICswMjAwLCBBbGV4YW5kZXIgR29yZGVldiB3cm90ZToNCj4gPiBUaGVyZSBhcmUgUENJIGRldmlj
ZXMgdGhhdCByZXF1aXJlIGEgcGFydGljdWxhciB2YWx1ZSB3cml0dGVuDQo+ID4gdG8gdGhlIE11
bHRpcGxlIE1lc3NhZ2UgRW5hYmxlIChNTUUpIHJlZ2lzdGVyIHdoaWxlIGFsaWduZWQgb24NCj4g
PiBwb3dlciBvZiAyIGJvdW5kYXJ5IHZhbHVlIG9mIGFjdHVhbGx5IHVzZWQgTVNJIHZlY3RvcnMg
J252ZWMnDQo+ID4gaXMgYSBsZXNzZXIgb2YgdGhhdCBNTUUgdmFsdWU6DQo+ID4NCj4gPiAJcm91
bmR1cF9wb3dfb2ZfdHdvKG52ZWMpIDwgJ011bHRpcGxlIE1lc3NhZ2UgRW5hYmxlJw0KPiA+DQo+
ID4gSG93ZXZlciB0aGUgZXhpc3RpbmcgcGNpX2VuYWJsZV9tc2lfYmxvY2soKSBpbnRlcmZhY2Ug
aXMgbm90DQo+ID4gYWJsZSB0byBjb25maWd1cmUgc3VjaCBkZXZpY2VzLCBzaW5jZSB0aGUgdmFs
dWUgd3JpdHRlbiB0byB0aGUNCj4gPiBNTUUgcmVnaXN0ZXIgaXMgY2FsY3VsYXRlZCBmcm9tIHRo
ZSBudW1iZXIgb2YgcmVxdWVzdGVkIE1TSXMNCj4gPiAnbnZlYyc6DQo+ID4NCj4gPiAJJ011bHRp
cGxlIE1lc3NhZ2UgRW5hYmxlJyA9IHJvdW5kdXBfcG93X29mX3R3byhudmVjKQ0KPiANCj4gRm9y
IE1TSSwgc29mdHdhcmUgbGVhcm5zIGhvdyBtYW55IHZlY3RvcnMgYSBkZXZpY2UgcmVxdWVzdHMg
YnkgcmVhZGluZw0KPiB0aGUgTXVsdGlwbGUgTWVzc2FnZSBDYXBhYmxlIChNTUMpIGZpZWxkLiAg
VGhpcyBmaWVsZCBpcyBlbmNvZGVkLCBzbyBhDQo+IGRldmljZSBjYW4gb25seSByZXF1ZXN0IDEs
IDIsIDQsIDgsIGV0Yy4sIHZlY3RvcnMuICBJdCdzIGltcG9zc2libGUNCj4gZm9yIGEgZGV2aWNl
IHRvIHJlcXVlc3QgMyB2ZWN0b3JzOyBpdCB3b3VsZCBoYXZlIHRvIHJvdW5kIHVwIHRoYXQgdXAN
Cj4gdG8gYSBwb3dlciBvZiB0d28gYW5kIHJlcXVlc3QgNCB2ZWN0b3JzLg0KPiANCj4gU29mdHdh
cmUgd3JpdGVzIHNpbWlsYXJseSBlbmNvZGVkIHZhbHVlcyB0byBNTUUgdG8gdGVsbCB0aGUgZGV2
aWNlIGhvdw0KPiBtYW55IHZlY3RvcnMgaGF2ZSBiZWVuIGFsbG9jYXRlZCBmb3IgaXRzIHVzZS4g
IEZvciBleGFtcGxlLCBpdCdzDQo+IGltcG9zc2libGUgdG8gdGVsbCB0aGUgZGV2aWNlIHRoYXQg
aXQgY2FuIHVzZSAzIHZlY3RvcnM7IHRoZSBPUyBoYXMgdG8NCj4gcm91bmQgdGhhdCB1cCBhbmQg
dGVsbCB0aGUgZGV2aWNlIGl0IGNhbiB1c2UgNCB2ZWN0b3JzLg0KPiANCj4gU28gaWYgSSB1bmRl
cnN0YW5kIGNvcnJlY3RseSwgdGhlIHBvaW50IG9mIHRoaXMgc2VyaWVzIGlzIHRvIHRha2UNCj4g
YWR2YW50YWdlIG9mIGRldmljZS1zcGVjaWZpYyBrbm93bGVkZ2UsIGUuZy4sIHRoZSBkZXZpY2Ug
cmVxdWVzdHMgNA0KPiB2ZWN0b3JzIHZpYSBNTUMsIGJ1dCB3ZSAia25vdyIgdGhlIGRldmljZSBp
cyBvbmx5IGNhcGFibGUgb2YgdXNpbmcgMy4NCj4gTW9yZW92ZXIsIHdlIHRlbGwgdGhlIGRldmlj
ZSB2aWEgTU1FIHRoYXQgNCB2ZWN0b3JzIGFyZSBhdmFpbGFibGUsIGJ1dA0KPiB3ZSd2ZSBvbmx5
IGFjdHVhbGx5IHNldCB1cCAzIG9mIHRoZW0uDQouLi4NCg0KRXZlbiBpZiB5b3UgZG8gdGhhdCwg
eW91IG91Z2h0IHRvIHdyaXRlIHZhbGlkIGludGVycnVwdCBpbmZvcm1hdGlvbg0KaW50byB0aGUg
NHRoIHNsb3QgKG1heWJlIHJlcGxpY2F0aW5nIG9uZSBvZiB0aGUgZWFybGllciBpbnRlcnJ1cHRz
KS4NClRoZW4sIGlmIHRoZSBkZXZpY2UgZG9lcyByYWlzZSB0aGUgJ3VuZXhwZWN0ZWQnIGludGVy
cnVwdCB5b3UgZG9uJ3QNCmdldCBhIHdyaXRlIHRvIGEgcmFuZG9tIGtlcm5lbCBsb2NhdGlvbi4N
Cg0KUGxhdXNpYmx5IHNvbWV0aGluZyBzaW1pbGFyIHNob3VsZCBiZSBkb25lIHdoZW4gYSBzbWFs
bGVyIG51bWJlciBvZg0KaW50ZXJydXB0cyBpcyBhc3NpZ25lZC4NCg0KCURhdmlkDQoNCg==
^ permalink raw reply
* [PATCH v6] flexcan: add err_irq handler for flexcan
From: Zhao Qiang @ 2014-07-03 9:22 UTC (permalink / raw)
To: linuxppc-dev, wg, mkl, linux-can, B07421; +Cc: Zhao Qiang
when flexcan is not physically linked, command 'cantest' will
trigger an err_irq, add err_irq handler for it.
Signed-off-by: Zhao Qiang <B45475@freescale.com>
---
Changes for v2:
- use a space instead of tab
- use flexcan_poll_state instead of print
Changes for v3:
- return IRQ_HANDLED if err is triggered
- stop transmitted packets when there is an err_interrupt
Changes for v4:
- call flexcan_irq
Changes for v5:
- move err_int_handling code from flexcan_irq to flexcan_err_irq
- call flexcan_err_irq from flexcan_irq
Changes for v6:
- move RX_IRQ handling back to flexcan_irq
drivers/net/can/flexcan.c | 52 +++++++++++++++++++++++++++++++++++++++++------
1 file changed, 46 insertions(+), 6 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index f425ec2..f6c92bc 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -208,6 +208,7 @@ struct flexcan_priv {
void __iomem *base;
u32 reg_esr;
u32 reg_ctrl_default;
+ int err_irq;
struct clk *clk_ipg;
struct clk *clk_per;
@@ -690,6 +691,28 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
return work_done;
}
+static irqreturn_t flexcan_err_irq(int irq, void *dev_id)
+{
+ struct net_device *dev = dev_id;
+ struct flexcan_priv *priv = netdev_priv(dev);
+ struct flexcan_regs __iomem *regs = priv->base;
+ u32 reg_ctrl, reg_esr;
+ irqreturn_t ret = IRQ_NONE;
+
+ reg_esr = flexcan_read(®s->esr);
+ reg_ctrl = flexcan_read(®s->ctrl);
+
+ if (reg_esr & FLEXCAN_ESR_ALL_INT) {
+ flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr);
+ ret = IRQ_HANDLED;
+ }
+
+ if (reg_esr & FLEXCAN_ESR_ERR_INT)
+ flexcan_write(reg_ctrl & ~FLEXCAN_CTRL_ERR_MSK, ®s->ctrl);
+
+ return ret;
+}
+
static irqreturn_t flexcan_irq(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
@@ -698,11 +721,10 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
struct flexcan_regs __iomem *regs = priv->base;
u32 reg_iflag1, reg_esr;
- reg_iflag1 = flexcan_read(®s->iflag1);
reg_esr = flexcan_read(®s->esr);
- /* ACK all bus error and state change IRQ sources */
- if (reg_esr & FLEXCAN_ESR_ALL_INT)
- flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr);
+ reg_iflag1 = flexcan_read(®s->iflag1);
+
+ flexcan_err_irq(irq, dev);
/*
* schedule NAPI in case of:
@@ -944,10 +966,17 @@ static int flexcan_open(struct net_device *dev)
if (err)
goto out_close;
+ if (priv->err_irq) {
+ err = request_irq(priv->err_irq, flexcan_err_irq, IRQF_SHARED,
+ dev->name, dev);
+ if (err)
+ goto out_free_irq;
+ }
+
/* start chip and queuing */
err = flexcan_chip_start(dev);
if (err)
- goto out_free_irq;
+ goto out_free_err_irq;
can_led_event(dev, CAN_LED_EVENT_OPEN);
@@ -956,6 +985,9 @@ static int flexcan_open(struct net_device *dev)
return 0;
+ out_free_err_irq:
+ if (priv->err_irq)
+ free_irq(priv->err_irq, dev);
out_free_irq:
free_irq(dev->irq, dev);
out_close:
@@ -977,6 +1009,9 @@ static int flexcan_close(struct net_device *dev)
flexcan_chip_stop(dev);
free_irq(dev->irq, dev);
+ if (priv->err_irq)
+ free_irq(priv->err_irq, dev);
+
clk_disable_unprepare(priv->clk_per);
clk_disable_unprepare(priv->clk_ipg);
@@ -1099,7 +1134,7 @@ static int flexcan_probe(struct platform_device *pdev)
struct resource *mem;
struct clk *clk_ipg = NULL, *clk_per = NULL;
void __iomem *base;
- int err, irq;
+ int err, irq, err_irq;
u32 clock_freq = 0;
if (pdev->dev.of_node)
@@ -1126,6 +1161,10 @@ static int flexcan_probe(struct platform_device *pdev)
if (irq <= 0)
return -ENODEV;
+ err_irq = platform_get_irq(pdev, 1);
+ if (err_irq <= 0)
+ err_irq = 0;
+
base = devm_ioremap_resource(&pdev->dev, mem);
if (IS_ERR(base))
return PTR_ERR(base);
@@ -1149,6 +1188,7 @@ static int flexcan_probe(struct platform_device *pdev)
dev->flags |= IFF_ECHO;
priv = netdev_priv(dev);
+ priv->err_irq = err_irq;
priv->can.clock.freq = clock_freq;
priv->can.bittiming_const = &flexcan_bittiming_const;
priv->can.do_set_mode = flexcan_set_mode;
--
1.8.5
^ permalink raw reply related
* [PATCH] devicetree/bindings: Add binding for micron n25q512a memory
From: Priyanka Jain @ 2014-07-03 10:12 UTC (permalink / raw)
To: devicetree, linuxppc-dev, scottwood; +Cc: Priyanka Jain
-Micron n25q512a memory is supported by m25p80 driver.
Add compatible field required to support n25q512a in m25p80.txt
-Add micron to the vendor-prefixes.txt file
Signed-off-by: Priyanka Jain <Priyanka.Jain@freescale.com>
---
Documentation/devicetree/bindings/mtd/m25p80.txt | 1 +
.../devicetree/bindings/vendor-prefixes.txt | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/m25p80.txt b/Documentation/devicetree/bindings/mtd/m25p80.txt
index 4611aa8..ce02e81 100644
--- a/Documentation/devicetree/bindings/mtd/m25p80.txt
+++ b/Documentation/devicetree/bindings/mtd/m25p80.txt
@@ -7,6 +7,7 @@ Required properties:
the DT binding is not Linux-only, but in case of Linux, see the
"spi_nor_ids" table in drivers/mtd/spi-nor/spi-nor.c for the list
of supported chips.
+ example: "micron,n25q512a"
- reg : Chip-Select number
- spi-max-frequency : Maximum frequency of the SPI bus the chip can operate at
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 4d7f375..a44cfee 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -79,6 +79,7 @@ marvell Marvell Technology Group Ltd.
maxim Maxim Integrated Products
micrel Micrel Inc.
microchip Microchip Technology Inc.
+micron Micron Technology Inc.
mosaixtech Mosaix Technologies, Inc.
moxa Moxa
mpl MPL AG
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH v6] flexcan: add err_irq handler for flexcan
From: Marc Kleine-Budde @ 2014-07-03 10:13 UTC (permalink / raw)
To: Zhao Qiang, linuxppc-dev, wg, linux-can, B07421
In-Reply-To: <1404379337-15351-1-git-send-email-B45475@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 2994 bytes --]
On 07/03/2014 11:22 AM, Zhao Qiang wrote:
> when flexcan is not physically linked, command 'cantest' will
> trigger an err_irq, add err_irq handler for it.
>
> Signed-off-by: Zhao Qiang <B45475@freescale.com>
> ---
> Changes for v2:
> - use a space instead of tab
> - use flexcan_poll_state instead of print
> Changes for v3:
> - return IRQ_HANDLED if err is triggered
> - stop transmitted packets when there is an err_interrupt
> Changes for v4:
> - call flexcan_irq
> Changes for v5:
> - move err_int_handling code from flexcan_irq to flexcan_err_irq
> - call flexcan_err_irq from flexcan_irq
> Changes for v6:
> - move RX_IRQ handling back to flexcan_irq
>
> drivers/net/can/flexcan.c | 52 +++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 46 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index f425ec2..f6c92bc 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -208,6 +208,7 @@ struct flexcan_priv {
> void __iomem *base;
> u32 reg_esr;
> u32 reg_ctrl_default;
> + int err_irq;
>
> struct clk *clk_ipg;
> struct clk *clk_per;
> @@ -690,6 +691,28 @@ static int flexcan_poll(struct napi_struct *napi, int quota)
> return work_done;
> }
>
> +static irqreturn_t flexcan_err_irq(int irq, void *dev_id)
> +{
> + struct net_device *dev = dev_id;
> + struct flexcan_priv *priv = netdev_priv(dev);
> + struct flexcan_regs __iomem *regs = priv->base;
> + u32 reg_ctrl, reg_esr;
> + irqreturn_t ret = IRQ_NONE;
> +
> + reg_esr = flexcan_read(®s->esr);
> + reg_ctrl = flexcan_read(®s->ctrl);
> +
> + if (reg_esr & FLEXCAN_ESR_ALL_INT) {
> + flexcan_write(reg_esr & FLEXCAN_ESR_ALL_INT, ®s->esr);
> + ret = IRQ_HANDLED;
> + }
> +
> + if (reg_esr & FLEXCAN_ESR_ERR_INT)
> + flexcan_write(reg_ctrl & ~FLEXCAN_CTRL_ERR_MSK, ®s->ctrl);
Now you just ACK the interrupts, but you're not triggering the NAPI,
where the actual error interrupts are handled.
In the current interrupt handler we are looking at two registers for
interrupts, that is "iflag1" and "esr". "iflag1" is triggered on RX,
RX-FIFO overflow and TX-complete. "esr" is triggered by errors in
general. In the interrupt handler we distinguish between bus errors
(FLEXCAN_ESR_ERR_BUS) and errors effecting a change of state
(FLEXCAN_ESR_ERR_STATE).
The next step is to figure out which bits in which register trigger the
error irq. Then move the code handling this register into the error irq
handler. As the seconds half of the error irq handling takes place in
the napi function, you have to duplicate the napi_schedule(&priv->napi).
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 242 bytes --]
^ permalink raw reply
* [PATCH] devicetree/binding/powerpc/fsl: Add binding for CPLD
From: Priyanka Jain @ 2014-07-03 10:09 UTC (permalink / raw)
To: devicetree, linuxppc-dev, scottwood; +Cc: Priyanka Jain
Some Freescale boards like T1040RDB have on board CPLD connected on
the IFC bus. Add binding for this in board.txt file
Signed-off-by: Priyanka Jain <Priyanka.Jain@freescale.com>
---
.../devicetree/bindings/powerpc/fsl/board.txt | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/board.txt b/Documentation/devicetree/bindings/powerpc/fsl/board.txt
index 700dec4..f35f295 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/board.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/board.txt
@@ -84,3 +84,22 @@ Example:
compatible = "fsl,bsc9132qds-fpga", "fsl,fpga-qixis-i2c";
reg = <0x66>;
};
+
+* Freescale on-board CPLD
+
+Some Freescale boards like T1040RDB have on board CPLD connected on
+the IFC bus.
+
+Required properties:
+- compatible: Should be a board-specific string like "fsl,<board>-cpld"
+ Example:
+ "fsl,T1040RDB-cpld", "fsl,T1042RDB-cpld", "fsl,T1042RDB_PI-cpld"
+- reg: Should contain the chip select, address offset and length of the CPLD
+
+Example:
+ cpld@3,0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ compatible = "fsl,T1040RDB-cpld";
+ reg = <3 0 0x300>;
+ };
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH v2] powerpc/kvm: support to handle sw breakpoint
From: Alexander Graf @ 2014-07-03 11:51 UTC (permalink / raw)
To: Madhavan Srinivasan, benh, paulus; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1404204062-19635-1-git-send-email-maddy@linux.vnet.ibm.com>
On 01.07.14 10:41, Madhavan Srinivasan wrote:
> This patch adds kernel side support for software breakpoint.
> Design is that, by using an illegal instruction, we trap to hypervisor
> via Emulation Assistance interrupt, where we check for the illegal instruction
> and accordingly we return to Host or Guest. Patch also adds support for
> software breakpoint in PR KVM.
>
> Patch mandates use of "abs" instruction as sw breakpoint instruction
> (primary opcode 31 and extended opcode 360). Based on PowerISA v2.01,
> ABS instruction has been dropped from the architecture and treated an
> illegal instruction.
>
> Changes v1->v2:
>
> Moved the debug instruction #def to kvm_book3s.h. This way PR_KVM can also share it.
> Added code to use KVM get one reg infrastructure to get debug opcode.
> Updated emulate.c to include emulation of debug instruction incase of PR_KVM.
> Made changes to commit message.
>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/kvm_book3s.h | 8 ++++++++
> arch/powerpc/include/asm/ppc-opcode.h | 5 +++++
> arch/powerpc/kvm/book3s.c | 3 ++-
> arch/powerpc/kvm/book3s_hv.c | 9 +++++++++
> arch/powerpc/kvm/book3s_pr.c | 3 +++
> arch/powerpc/kvm/emulate.c | 10 ++++++++++
> 6 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> index f52f656..180d549 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -24,6 +24,14 @@
> #include <linux/kvm_host.h>
> #include <asm/kvm_book3s_asm.h>
>
> +/*
> + * KVMPPC_INST_BOOK3S_DEBUG is debug Instruction for supporting Software Breakpoint.
> + * Instruction mnemonic is ABS, primary opcode is 31 and extended opcode is 360.
> + * Based on PowerISA v2.01, ABS instruction has been dropped from the architecture
> + * and treated an illegal instruction.
> + */
> +#define KVMPPC_INST_BOOK3S_DEBUG 0x7c0002d0
This will still break with LE guests.
> +
> struct kvmppc_bat {
> u64 raw;
> u32 bepi;
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index 3132bb9..3fbb4c1 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -111,6 +111,11 @@
> #define OP_31_XOP_LHBRX 790
> #define OP_31_XOP_STHBRX 918
>
> +/* KVMPPC_INST_BOOK3S_DEBUG -- Software breakpoint Instruction
> + * Instruction mnemonic is ABS, primary opcode is 31 and extended opcode is 360.
> + */
> +#define OP_31_XOP_ABS 360
> +
> #define OP_LWZ 32
> #define OP_LD 58
> #define OP_LWZU 33
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index c254c27..b40fe5d 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -789,7 +789,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> struct kvm_guest_debug *dbg)
> {
> - return -EINVAL;
> + vcpu->guest_debug = dbg->control;
> + return 0;
> }
>
> void kvmppc_decrementer_func(unsigned long data)
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 7a12edb..402c1ec 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -725,8 +725,14 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
> * we don't emulate any guest instructions at this stage.
> */
> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
> + if (kvmppc_get_last_inst(vcpu) == KVMPPC_INST_BOOK3S_DEBUG ) {
> + run->exit_reason = KVM_EXIT_DEBUG;
> + run->debug.arch.address = kvmppc_get_pc(vcpu);
> + r = RESUME_HOST;
Phew - why can't we just go into the normal instruction emulator for
EMUL_ASSIST?
Alex
^ permalink raw reply
* Re: [PATCH] KVM: PPC: e500: Emulate power management control SPR
From: Alexander Graf @ 2014-07-03 12:03 UTC (permalink / raw)
To: Scott Wood, Mihai Caraman; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1404152404.2435.132.camel@snotra.buserror.net>
On 30.06.14 20:20, Scott Wood wrote:
> On Mon, 2014-06-30 at 15:55 +0300, Mihai Caraman wrote:
>> For FSL e6500 core the kernel uses power management SPR register (PWRMGTCR0)
>> to enable idle power down for cores and devices by setting up the idle count
>> period at boot time. With the host already controlling the power management
>> configuration the guest could simply benefit from it, so emulate guest request
>> as nop.
>>
>> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>> ---
>> arch/powerpc/kvm/e500_emulate.c | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c
>> index 002d517..98a22e5 100644
>> --- a/arch/powerpc/kvm/e500_emulate.c
>> +++ b/arch/powerpc/kvm/e500_emulate.c
>> @@ -250,6 +250,10 @@ int kvmppc_core_emulate_mtspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong spr_va
>> spr_val);
>> break;
>>
>> + case SPRN_PWRMGTCR0:
>> + /* Guest relies on host power management configurations */
>> + break;
>> +
>> /* extra exceptions */
>> case SPRN_IVOR32:
>> vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL] = spr_val;
>> @@ -355,6 +359,10 @@ int kvmppc_core_emulate_mfspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong *spr_v
>> *spr_val = 0;
>> break;
>>
>> + case SPRN_PWRMGTCR0:
>> + *spr_val = 0;
>> + break;
>> +
>> case SPRN_MMUCFG:
>> *spr_val = vcpu->arch.mmucfg;
>> break;
> When reading, is it better to return zero, or the current host value, or
> the value last written by the guest (even though it wasn't written to
> hardware)?
I think it makes sense to treat it as general storage. I don't think
leaking the host value into the guest is useful. And while zero works,
the spec does say that the value gets retained, so I think we should do
the same.
Alex
^ permalink raw reply
* Re: [PATCH] KVM: PPC: e500: Fix default tlb for victim hint
From: Alexander Graf @ 2014-07-03 12:08 UTC (permalink / raw)
To: Scott Wood, Mihai Caraman; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1404152335.2435.130.camel@snotra.buserror.net>
On 30.06.14 20:18, Scott Wood wrote:
> On Mon, 2014-06-30 at 15:54 +0300, Mihai Caraman wrote:
>> Tlb search operation used for victim hint relies on the default tlb set by the
>> host. When hardware tablewalk support is enabled in the host, the default tlb is
>> TLB1 which leads KVM to evict the bolted entry. Set and restore the default tlb
>> when searching for victim hint.
>>
>> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
>> ---
>> arch/powerpc/include/asm/mmu-book3e.h | 5 ++++-
>> arch/powerpc/kvm/e500_mmu_host.c | 4 ++++
>> 2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/mmu-book3e.h b/arch/powerpc/include/asm/mmu-book3e.h
>> index 901dac6..5dad378 100644
>> --- a/arch/powerpc/include/asm/mmu-book3e.h
>> +++ b/arch/powerpc/include/asm/mmu-book3e.h
>> @@ -40,7 +40,9 @@
>>
>> /* MAS registers bit definitions */
>>
>> -#define MAS0_TLBSEL(x) (((x) << 28) & 0x30000000)
>> +#define MAS0_TLBSEL_MASK 0x30000000
>> +#define MAS0_TLBSEL_SHIFT 28
>> +#define MAS0_TLBSEL(x) (((x) << MAS0_TLBSEL_SHIFT) & MAS0_TLBSEL_MASK)
>> #define MAS0_ESEL_MASK 0x0FFF0000
>> #define MAS0_ESEL_SHIFT 16
>> #define MAS0_ESEL(x) (((x) << MAS0_ESEL_SHIFT) & MAS0_ESEL_MASK)
>> @@ -86,6 +88,7 @@
>> #define MAS3_SPSIZE 0x0000003e
>> #define MAS3_SPSIZE_SHIFT 1
>>
>> +#define MAS4_TLBSEL_MASK MAS0_TLBSEL_MASK
>> #define MAS4_TLBSELD(x) MAS0_TLBSEL(x)
>> #define MAS4_INDD 0x00008000 /* Default IND */
>> #define MAS4_TSIZED(x) MAS1_TSIZE(x)
>> diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
>> index dd2cc03..79677d7 100644
>> --- a/arch/powerpc/kvm/e500_mmu_host.c
>> +++ b/arch/powerpc/kvm/e500_mmu_host.c
>> @@ -107,11 +107,15 @@ static u32 get_host_mas0(unsigned long eaddr)
>> {
>> unsigned long flags;
>> u32 mas0;
>> + u32 mas4;
>>
>> local_irq_save(flags);
>> mtspr(SPRN_MAS6, 0);
>> + mas4 = mfspr(SPRN_MAS4);
>> + mtspr(SPRN_MAS4, mas4 & ~MAS4_TLBSEL_MASK);
>> asm volatile("tlbsx 0, %0" : : "b" (eaddr & ~CONFIG_PAGE_OFFSET));
>> mas0 = mfspr(SPRN_MAS0);
>> + mtspr(SPRN_MAS4, mas4);
>> local_irq_restore(flags);
>>
>> return mas0;
> Reviewed-by: Scott Wood <scottwood@freescale.com>
Thanks, applied to kvm-ppc-queue.
Alex
^ permalink raw reply
* Re: [PATCH 1/6 v2] KVM: PPC: Book3E: Use common defines for SPE/FP/AltiVec int numbers
From: Alexander Graf @ 2014-07-03 12:21 UTC (permalink / raw)
To: Mihai Caraman, kvm-ppc; +Cc: linuxppc-dev, kvm
In-Reply-To: <1404142497-6430-2-git-send-email-mihai.caraman@freescale.com>
On 30.06.14 17:34, Mihai Caraman wrote:
> Use common BOOKE_IRQPRIO and BOOKE_INTERRUPT defines for SPE/FP/AltiVec
> which share the same interrupt numbers.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> v2:
> - remove outdated definitions
>
> arch/powerpc/include/asm/kvm_asm.h | 8 --------
> arch/powerpc/kvm/booke.c | 17 +++++++++--------
> arch/powerpc/kvm/booke.h | 4 ++--
> arch/powerpc/kvm/booke_interrupts.S | 9 +++++----
> arch/powerpc/kvm/bookehv_interrupts.S | 4 ++--
> arch/powerpc/kvm/e500.c | 10 ++++++----
> arch/powerpc/kvm/e500_emulate.c | 10 ++++++----
> 7 files changed, 30 insertions(+), 32 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_asm.h b/arch/powerpc/include/asm/kvm_asm.h
> index 9601741..c94fd33 100644
> --- a/arch/powerpc/include/asm/kvm_asm.h
> +++ b/arch/powerpc/include/asm/kvm_asm.h
> @@ -56,14 +56,6 @@
> /* E500 */
> #define BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL 32
> #define BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST 33
> -/*
> - * TODO: Unify 32-bit and 64-bit kernel exception handlers to use same defines
> - */
> -#define BOOKE_INTERRUPT_SPE_UNAVAIL BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL
> -#define BOOKE_INTERRUPT_SPE_FP_DATA BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST
> -#define BOOKE_INTERRUPT_ALTIVEC_UNAVAIL BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL
> -#define BOOKE_INTERRUPT_ALTIVEC_ASSIST \
> - BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST
I think I'd prefer to keep them separate.
> #define BOOKE_INTERRUPT_SPE_FP_ROUND 34
> #define BOOKE_INTERRUPT_PERFORMANCE_MONITOR 35
> #define BOOKE_INTERRUPT_DOORBELL 36
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index ab62109..3c86d9b 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -388,8 +388,8 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu,
> case BOOKE_IRQPRIO_ITLB_MISS:
> case BOOKE_IRQPRIO_SYSCALL:
> case BOOKE_IRQPRIO_FP_UNAVAIL:
> - case BOOKE_IRQPRIO_SPE_UNAVAIL:
> - case BOOKE_IRQPRIO_SPE_FP_DATA:
> + case BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL:
> + case BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST:
#ifdef CONFIG_KVM_E500V2
case ...SPE:
#else
case ..ALTIVEC:
#endif
> case BOOKE_IRQPRIO_SPE_FP_ROUND:
> case BOOKE_IRQPRIO_AP_UNAVAIL:
> allowed = 1;
> @@ -977,18 +977,19 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> break;
>
> #ifdef CONFIG_SPE
> - case BOOKE_INTERRUPT_SPE_UNAVAIL: {
> + case BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL: {
> if (vcpu->arch.shared->msr & MSR_SPE)
> kvmppc_vcpu_enable_spe(vcpu);
> else
> kvmppc_booke_queue_irqprio(vcpu,
> - BOOKE_IRQPRIO_SPE_UNAVAIL);
> + BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL);
> r = RESUME_GUEST;
> break;
> }
>
> - case BOOKE_INTERRUPT_SPE_FP_DATA:
> - kvmppc_booke_queue_irqprio(vcpu, BOOKE_IRQPRIO_SPE_FP_DATA);
> + case BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST:
> + kvmppc_booke_queue_irqprio(vcpu,
> + BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST);
> r = RESUME_GUEST;
> break;
>
> @@ -997,7 +998,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> r = RESUME_GUEST;
> break;
> #else
> - case BOOKE_INTERRUPT_SPE_UNAVAIL:
> + case BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL:
> /*
> * Guest wants SPE, but host kernel doesn't support it. Send
> * an "unimplemented operation" program check to the guest.
> @@ -1010,7 +1011,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> * These really should never happen without CONFIG_SPE,
> * as we should never enable the real MSR[SPE] in the guest.
> */
> - case BOOKE_INTERRUPT_SPE_FP_DATA:
> + case BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST:
> case BOOKE_INTERRUPT_SPE_FP_ROUND:
> printk(KERN_CRIT "%s: unexpected SPE interrupt %u at %08lx\n",
> __func__, exit_nr, vcpu->arch.pc);
> diff --git a/arch/powerpc/kvm/booke.h b/arch/powerpc/kvm/booke.h
> index b632cd3..f182b32 100644
> --- a/arch/powerpc/kvm/booke.h
> +++ b/arch/powerpc/kvm/booke.h
> @@ -32,8 +32,8 @@
> #define BOOKE_IRQPRIO_ALIGNMENT 2
> #define BOOKE_IRQPRIO_PROGRAM 3
> #define BOOKE_IRQPRIO_FP_UNAVAIL 4
> -#define BOOKE_IRQPRIO_SPE_UNAVAIL 5
> -#define BOOKE_IRQPRIO_SPE_FP_DATA 6
> +#define BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL 5
> +#define BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST 6
#ifdef CONFIG_KVM_E500V2
#define ...SPE...
#else
#define ...ALTIVEC...
#endif
That way we can be 100% sure that no SPE cruft leaks into anything.
> #define BOOKE_IRQPRIO_SPE_FP_ROUND 7
> #define BOOKE_IRQPRIO_SYSCALL 8
> #define BOOKE_IRQPRIO_AP_UNAVAIL 9
> diff --git a/arch/powerpc/kvm/booke_interrupts.S b/arch/powerpc/kvm/booke_interrupts.S
> index 2c6deb5ef..a275dc5 100644
> --- a/arch/powerpc/kvm/booke_interrupts.S
> +++ b/arch/powerpc/kvm/booke_interrupts.S
> @@ -137,8 +137,9 @@ KVM_HANDLER BOOKE_INTERRUPT_WATCHDOG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0
> KVM_HANDLER BOOKE_INTERRUPT_DTLB_MISS SPRN_SPRG_RSCRATCH0 SPRN_SRR0
> KVM_HANDLER BOOKE_INTERRUPT_ITLB_MISS SPRN_SPRG_RSCRATCH0 SPRN_SRR0
> KVM_DBG_HANDLER BOOKE_INTERRUPT_DEBUG SPRN_SPRG_RSCRATCH_CRIT SPRN_CSRR0
> -KVM_HANDLER BOOKE_INTERRUPT_SPE_UNAVAIL SPRN_SPRG_RSCRATCH0 SPRN_SRR0
> -KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_DATA SPRN_SPRG_RSCRATCH0 SPRN_SRR0
> +KVM_HANDLER BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL SPRN_SPRG_RSCRATCH0 SPRN_SRR0
> +KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST SPRN_SPRG_RSCRATCH0 \
> + SPRN_SRR0
Same thing here - just only trap SPE when CONFIG_KVM_E500V2 is available
and trap altivec otherwise (to make sure we always have a handler).
Alex
> KVM_HANDLER BOOKE_INTERRUPT_SPE_FP_ROUND SPRN_SPRG_RSCRATCH0 SPRN_SRR0
> _GLOBAL(kvmppc_handlers_end)
>
> @@ -525,8 +526,8 @@ KVM_HANDLER_ADDR BOOKE_INTERRUPT_WATCHDOG
> KVM_HANDLER_ADDR BOOKE_INTERRUPT_DTLB_MISS
> KVM_HANDLER_ADDR BOOKE_INTERRUPT_ITLB_MISS
> KVM_HANDLER_ADDR BOOKE_INTERRUPT_DEBUG
> -KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_UNAVAIL
> -KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_FP_DATA
> +KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL
> +KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST
> KVM_HANDLER_ADDR BOOKE_INTERRUPT_SPE_FP_ROUND
> KVM_HANDLER_END /*Always keep this in end*/
>
> diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S
> index a1712b8..ff73143 100644
> --- a/arch/powerpc/kvm/bookehv_interrupts.S
> +++ b/arch/powerpc/kvm/bookehv_interrupts.S
> @@ -300,9 +300,9 @@ kvm_handler BOOKE_INTERRUPT_DTLB_MISS, EX_PARAMS_TLB, \
> SPRN_SRR0, SPRN_SRR1, (NEED_EMU | NEED_DEAR | NEED_ESR)
> kvm_handler BOOKE_INTERRUPT_ITLB_MISS, EX_PARAMS_TLB, \
> SPRN_SRR0, SPRN_SRR1, 0
> -kvm_handler BOOKE_INTERRUPT_SPE_UNAVAIL, EX_PARAMS(GEN), \
> +kvm_handler BOOKE_INTERRUPT_SPE_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \
> SPRN_SRR0, SPRN_SRR1, 0
> -kvm_handler BOOKE_INTERRUPT_SPE_FP_DATA, EX_PARAMS(GEN), \
> +kvm_handler BOOKE_INTERRUPT_SPE_FP_DATA_ALTIVEC_ASSIST, EX_PARAMS(GEN), \
> SPRN_SRR0, SPRN_SRR1, 0
> kvm_handler BOOKE_INTERRUPT_SPE_FP_ROUND, EX_PARAMS(GEN), \
> SPRN_SRR0, SPRN_SRR1, 0
> diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c
> index 2e02ed8..3c1a30d 100644
> --- a/arch/powerpc/kvm/e500.c
> +++ b/arch/powerpc/kvm/e500.c
> @@ -383,8 +383,10 @@ static int kvmppc_core_get_sregs_e500(struct kvm_vcpu *vcpu,
> sregs->u.e.impl.fsl.hid0 = vcpu_e500->hid0;
> sregs->u.e.impl.fsl.mcar = vcpu_e500->mcar;
>
> - sregs->u.e.ivor_high[0] = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL];
> - sregs->u.e.ivor_high[1] = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA];
> + sregs->u.e.ivor_high[0] =
> + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL];
> + sregs->u.e.ivor_high[1] =
> + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST];
> sregs->u.e.ivor_high[2] = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_ROUND];
> sregs->u.e.ivor_high[3] =
> vcpu->arch.ivor[BOOKE_IRQPRIO_PERFORMANCE_MONITOR];
> @@ -414,9 +416,9 @@ static int kvmppc_core_set_sregs_e500(struct kvm_vcpu *vcpu,
> return 0;
>
> if (sregs->u.e.features & KVM_SREGS_E_SPE) {
> - vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL] =
> + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL] =
> sregs->u.e.ivor_high[0];
> - vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA] =
> + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST] =
> sregs->u.e.ivor_high[1];
> vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_ROUND] =
> sregs->u.e.ivor_high[2];
> diff --git a/arch/powerpc/kvm/e500_emulate.c b/arch/powerpc/kvm/e500_emulate.c
> index 98a22e5..6a6833f 100644
> --- a/arch/powerpc/kvm/e500_emulate.c
> +++ b/arch/powerpc/kvm/e500_emulate.c
> @@ -256,10 +256,11 @@ int kvmppc_core_emulate_mtspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong spr_va
>
> /* extra exceptions */
> case SPRN_IVOR32:
> - vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL] = spr_val;
> + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL] = spr_val;
> break;
> case SPRN_IVOR33:
> - vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA] = spr_val;
> + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST] =
> + spr_val;
> break;
> case SPRN_IVOR34:
> vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_ROUND] = spr_val;
> @@ -378,10 +379,11 @@ int kvmppc_core_emulate_mfspr_e500(struct kvm_vcpu *vcpu, int sprn, ulong *spr_v
>
> /* extra exceptions */
> case SPRN_IVOR32:
> - *spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_UNAVAIL];
> + *spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_ALTIVEC_UNAVAIL];
> break;
> case SPRN_IVOR33:
> - *spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA];
> + *spr_val =
> + vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_DATA_ALTIVEC_ASSIST];
> break;
> case SPRN_IVOR34:
> *spr_val = vcpu->arch.ivor[BOOKE_IRQPRIO_SPE_FP_ROUND];
^ permalink raw reply
* Re: [PATCH 2/6 v2] KVM: PPC: Book3E: Refactor SPE/FP exit handling
From: Alexander Graf @ 2014-07-03 12:21 UTC (permalink / raw)
To: Mihai Caraman, kvm-ppc; +Cc: linuxppc-dev, kvm
In-Reply-To: <1404142497-6430-3-git-send-email-mihai.caraman@freescale.com>
On 30.06.14 17:34, Mihai Caraman wrote:
> SPE/FP/AltiVec interrupts share the same numbers. Refactor SPE/FP exit handling
> to accommodate AltiVec later on the same flow. Add kvmppc_supports_spe() to detect
> suport for the unit at runtime since it can be configured in the kernel but not
> featured on hardware and vice versa.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
Why not keep them 100% separate?
Alex
^ permalink raw reply
* Re: [PATCH 3/6 v2] KVM: PPC: Book3E: Increase FPU laziness
From: Alexander Graf @ 2014-07-03 12:28 UTC (permalink / raw)
To: Mihai Caraman, kvm-ppc; +Cc: linuxppc-dev, kvm
In-Reply-To: <1404142497-6430-4-git-send-email-mihai.caraman@freescale.com>
On 30.06.14 17:34, Mihai Caraman wrote:
> Increase FPU laziness by calling kvmppc_load_guest_fp() just before
> returning to guest instead of each sched in. Without this improvement
> an interrupt may also claim floting point corrupting guest state.
How do you handle context switching with this patch applied? During most
of the guest's lifetime we never exit kvmppc_vcpu_run(), so when the
guest gets switched out all FPU state gets lost?
Alex
^ permalink raw reply
* Re: [PATCH 4/6 v2] KVM: PPC: Book3E: Add AltiVec support
From: Alexander Graf @ 2014-07-03 12:32 UTC (permalink / raw)
To: Mihai Caraman, kvm-ppc; +Cc: linuxppc-dev, kvm
In-Reply-To: <1404142497-6430-5-git-send-email-mihai.caraman@freescale.com>
On 30.06.14 17:34, Mihai Caraman wrote:
> Add KVM Book3E AltiVec support. KVM Book3E FPU support gracefully reuse host
> infrastructure so follow the same approach for AltiVec.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
Same comment here - I fail to see how we refetch Altivec state after a
context switch.
Alex
^ permalink raw reply
* Re: [PATCH 5/6 v2] KVM: PPC: Book3E: Add ONE_REG AltiVec support
From: Alexander Graf @ 2014-07-03 12:33 UTC (permalink / raw)
To: Mihai Caraman, kvm-ppc; +Cc: linuxppc-dev, kvm
In-Reply-To: <1404142497-6430-6-git-send-email-mihai.caraman@freescale.com>
On 30.06.14 17:34, Mihai Caraman wrote:
> Add ONE_REG support for AltiVec on Book3E.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
Any chance we can handle these in generic code?
Alex
^ permalink raw reply
* Re: [PATCH 3/5 v4] KVM: PPC: Book3s: Remove kvmppc_read_inst() function
From: Alexander Graf @ 2014-07-03 13:37 UTC (permalink / raw)
To: Mihai Caraman, kvm-ppc; +Cc: linuxppc-dev, kvm
In-Reply-To: <1403909347-31622-4-git-send-email-mihai.caraman@freescale.com>
On 28.06.14 00:49, Mihai Caraman wrote:
> In the context of replacing kvmppc_ld() function calls with a version of
> kvmppc_get_last_inst() which allow to fail, Alex Graf suggested this:
>
> "If we get EMULATE_AGAIN, we just have to make sure we go back into the guest.
> No need to inject an ISI into the guest - it'll do that all by itself.
> With an error returning kvmppc_get_last_inst we can just use completely
> get rid of kvmppc_read_inst() and only use kvmppc_get_last_inst() instead."
>
> As a intermediate step get rid of kvmppc_read_inst() and only use kvmppc_ld()
> instead.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> v4:
> - new patch
>
> arch/powerpc/kvm/book3s_pr.c | 85 ++++++++++++++++++--------------------------
> 1 file changed, 35 insertions(+), 50 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> index 15fd6c2..d247d88 100644
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c
> @@ -665,42 +665,6 @@ static void kvmppc_giveup_fac(struct kvm_vcpu *vcpu, ulong fac)
> #endif
> }
>
> -static int kvmppc_read_inst(struct kvm_vcpu *vcpu)
> -{
> - ulong srr0 = kvmppc_get_pc(vcpu);
> - u32 last_inst = kvmppc_get_last_inst(vcpu);
> - int ret;
> -
> - ret = kvmppc_ld(vcpu, &srr0, sizeof(u32), &last_inst, false);
> - if (ret == -ENOENT) {
> - ulong msr = kvmppc_get_msr(vcpu);
> -
> - msr = kvmppc_set_field(msr, 33, 33, 1);
> - msr = kvmppc_set_field(msr, 34, 36, 0);
> - msr = kvmppc_set_field(msr, 42, 47, 0);
> - kvmppc_set_msr_fast(vcpu, msr);
> - kvmppc_book3s_queue_irqprio(vcpu, BOOK3S_INTERRUPT_INST_STORAGE);
> - return EMULATE_AGAIN;
> - }
> -
> - return EMULATE_DONE;
> -}
> -
> -static int kvmppc_check_ext(struct kvm_vcpu *vcpu, unsigned int exit_nr)
> -{
> -
> - /* Need to do paired single emulation? */
> - if (!(vcpu->arch.hflags & BOOK3S_HFLAG_PAIRED_SINGLE))
> - return EMULATE_DONE;
> -
> - /* Read out the instruction */
> - if (kvmppc_read_inst(vcpu) == EMULATE_DONE)
> - /* Need to emulate */
> - return EMULATE_FAIL;
> -
> - return EMULATE_AGAIN;
> -}
> -
> /* Handle external providers (FPU, Altivec, VSX) */
> static int kvmppc_handle_ext(struct kvm_vcpu *vcpu, unsigned int exit_nr,
> ulong msr)
> @@ -1101,31 +1065,51 @@ program_interrupt:
> case BOOK3S_INTERRUPT_VSX:
> {
> int ext_msr = 0;
> + int emul;
> + ulong pc;
> + u32 last_inst;
>
> - switch (exit_nr) {
> - case BOOK3S_INTERRUPT_FP_UNAVAIL: ext_msr = MSR_FP; break;
> - case BOOK3S_INTERRUPT_ALTIVEC: ext_msr = MSR_VEC; break;
> - case BOOK3S_INTERRUPT_VSX: ext_msr = MSR_VSX; break;
> - }
> + if (!(vcpu->arch.hflags & BOOK3S_HFLAG_PAIRED_SINGLE)) {
Please make paired single emulation the unusual, if()'ed case, not the
normal exit path :).
Alex
^ permalink raw reply
* Re: [PATCH 4/5 v4] KVM: PPC: Alow kvmppc_get_last_inst() to fail
From: Alexander Graf @ 2014-07-03 13:44 UTC (permalink / raw)
To: Mihai Caraman, kvm-ppc; +Cc: linuxppc-dev, kvm
In-Reply-To: <1403909347-31622-5-git-send-email-mihai.caraman@freescale.com>
On 28.06.14 00:49, Mihai Caraman wrote:
> On book3e, guest last instruction is read on the exit path using load
> external pid (lwepx) dedicated instruction. This load operation may fail
> due to TLB eviction and execute-but-not-read entries.
>
> This patch lay down the path for an alternative solution to read the guest
> last instruction, by allowing kvmppc_get_lat_inst() function to fail.
> Architecture specific implmentations of kvmppc_load_last_inst() may read
> last guest instruction and instruct the emulation layer to re-execute the
> guest in case of failure.
>
> Make kvmppc_get_last_inst() definition common between architectures.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> v4:
> - these changes compile on book3s, please validate the functionality and
> do the necessary adaptations!
> - common declaration and enum for kvmppc_load_last_inst()
> - remove kvmppc_read_inst() in a preceding patch
>
> v3:
> - rework patch description
> - add common definition for kvmppc_get_last_inst()
> - check return values in book3s code
>
> v2:
> - integrated kvmppc_get_last_inst() in book3s code and checked build
> - addressed cosmetic feedback
>
> arch/powerpc/include/asm/kvm_book3s.h | 26 ------------------
> arch/powerpc/include/asm/kvm_booke.h | 5 ----
> arch/powerpc/include/asm/kvm_ppc.h | 24 +++++++++++++++++
> arch/powerpc/kvm/book3s.c | 11 ++++++++
> arch/powerpc/kvm/book3s_64_mmu_hv.c | 17 ++++--------
> arch/powerpc/kvm/book3s_paired_singles.c | 38 +++++++++++++++++----------
> arch/powerpc/kvm/book3s_pr.c | 45 ++++++++++++++++++++++++--------
> arch/powerpc/kvm/booke.c | 3 +++
> arch/powerpc/kvm/e500_mmu_host.c | 6 +++++
> arch/powerpc/kvm/emulate.c | 18 ++++++++-----
> arch/powerpc/kvm/powerpc.c | 11 ++++++--
> 11 files changed, 128 insertions(+), 76 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> index ceb70aa..1300cd9 100644
> --- a/arch/powerpc/include/asm/kvm_book3s.h
> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> @@ -276,32 +276,6 @@ static inline bool kvmppc_need_byteswap(struct kvm_vcpu *vcpu)
> return (kvmppc_get_msr(vcpu) & MSR_LE) != (MSR_KERNEL & MSR_LE);
> }
>
> -static inline u32 kvmppc_get_last_inst_internal(struct kvm_vcpu *vcpu, ulong pc)
> -{
> - /* Load the instruction manually if it failed to do so in the
> - * exit path */
> - if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED)
> - kvmppc_ld(vcpu, &pc, sizeof(u32), &vcpu->arch.last_inst, false);
> -
> - return kvmppc_need_byteswap(vcpu) ? swab32(vcpu->arch.last_inst) :
> - vcpu->arch.last_inst;
> -}
> -
> -static inline u32 kvmppc_get_last_inst(struct kvm_vcpu *vcpu)
> -{
> - return kvmppc_get_last_inst_internal(vcpu, kvmppc_get_pc(vcpu));
> -}
> -
> -/*
> - * Like kvmppc_get_last_inst(), but for fetching a sc instruction.
> - * Because the sc instruction sets SRR0 to point to the following
> - * instruction, we have to fetch from pc - 4.
> - */
> -static inline u32 kvmppc_get_last_sc(struct kvm_vcpu *vcpu)
> -{
> - return kvmppc_get_last_inst_internal(vcpu, kvmppc_get_pc(vcpu) - 4);
> -}
> -
> static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu)
> {
> return vcpu->arch.fault_dar;
> diff --git a/arch/powerpc/include/asm/kvm_booke.h b/arch/powerpc/include/asm/kvm_booke.h
> index c7aed61..cbb1990 100644
> --- a/arch/powerpc/include/asm/kvm_booke.h
> +++ b/arch/powerpc/include/asm/kvm_booke.h
> @@ -69,11 +69,6 @@ static inline bool kvmppc_need_byteswap(struct kvm_vcpu *vcpu)
> return false;
> }
>
> -static inline u32 kvmppc_get_last_inst(struct kvm_vcpu *vcpu)
> -{
> - return vcpu->arch.last_inst;
> -}
> -
> static inline void kvmppc_set_ctr(struct kvm_vcpu *vcpu, ulong val)
> {
> vcpu->arch.ctr = val;
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index e2fd5a1..ec326c8 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -47,6 +47,11 @@ enum emulation_result {
> EMULATE_EXIT_USER, /* emulation requires exit to user-space */
> };
>
> +enum instruction_type {
> + INST_GENERIC,
> + INST_SC, /* system call */
> +};
> +
> extern int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu);
> extern int __kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu);
> extern void kvmppc_handler_highmem(void);
> @@ -62,6 +67,9 @@ extern int kvmppc_handle_store(struct kvm_run *run, struct kvm_vcpu *vcpu,
> u64 val, unsigned int bytes,
> int is_default_endian);
>
> +extern int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
> + enum instruction_type type, u32 *inst);
> +
> extern int kvmppc_emulate_instruction(struct kvm_run *run,
> struct kvm_vcpu *vcpu);
> extern int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu);
> @@ -234,6 +242,22 @@ struct kvmppc_ops {
> extern struct kvmppc_ops *kvmppc_hv_ops;
> extern struct kvmppc_ops *kvmppc_pr_ops;
>
> +static inline int kvmppc_get_last_inst(struct kvm_vcpu *vcpu,
> + enum instruction_type type, u32 *inst)
> +{
> + int ret = EMULATE_DONE;
> +
> + /* Load the instruction manually if it failed to do so in the
> + * exit path */
> + if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED)
> + ret = kvmppc_load_last_inst(vcpu, type, &vcpu->arch.last_inst);
> +
> + *inst = kvmppc_need_byteswap(vcpu) ? swab32(vcpu->arch.last_inst) :
> + vcpu->arch.last_inst;
We swap needlessly when the translation failed, but I don't think it
really hurts.
> +
> + return ret;
So here we just return the return value of kvmppc_load_last_inst() ...
> +}
> +
> static inline bool is_kvmppc_hv_enabled(struct kvm *kvm)
> {
> return kvm->arch.kvm_ops == kvmppc_hv_ops;
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index bd75902..c17f101 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -467,6 +467,17 @@ mmio:
> }
> EXPORT_SYMBOL_GPL(kvmppc_ld);
>
> +int kvmppc_load_last_inst(struct kvm_vcpu *vcpu, enum instruction_type type,
> + u32 *inst)
> +{
> + ulong pc = kvmppc_get_pc(vcpu);
> +
> + if (type == INST_SC)
> + pc -= 4;
> + return kvmppc_ld(vcpu, &pc, sizeof(u32), &vcpu->arch.last_inst, false);
... which simply returns the return value of kvmppc_ld(). However,
kvmppc_ld() returns -ENOENT when it doesn't find a translation. We need
to convert the return value space of kvmppc_ld() into an EMULATE_ name
space that makes sense for this function.
How about something like
r = kvmppc_ld();
if (r == EMULATE_DONE)
return r;
else
return EMULATE_AGAIN;
That way we make sure that we tell the caller only "yes, all is good" or
"please go back into the guest".
> +}
> +EXPORT_SYMBOL_GPL(kvmppc_load_last_inst);
> +
> int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
> {
> return 0;
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> index 8056107..2c2b7ad 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> @@ -530,21 +530,14 @@ static int instruction_is_store(unsigned int instr)
> static int kvmppc_hv_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu,
> unsigned long gpa, gva_t ea, int is_store)
> {
> - int ret;
> u32 last_inst;
> - unsigned long srr0 = kvmppc_get_pc(vcpu);
>
> - /* We try to load the last instruction. We don't let
> - * emulate_instruction do it as it doesn't check what
> - * kvmppc_ld returns.
> + /*
> * If we fail, we just return to the guest and try executing it again.
> */
> - if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED) {
> - ret = kvmppc_ld(vcpu, &srr0, sizeof(u32), &last_inst, false);
> - if (ret != EMULATE_DONE || last_inst == KVM_INST_FETCH_FAILED)
> - return RESUME_GUEST;
> - vcpu->arch.last_inst = last_inst;
> - }
> + if (kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst) !=
> + EMULATE_DONE)
> + return RESUME_GUEST;
This looks great.
>
> /*
> * WARNING: We do not know for sure whether the instruction we just
> @@ -558,7 +551,7 @@ static int kvmppc_hv_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu,
> * we just return and retry the instruction.
> */
>
> - if (instruction_is_store(kvmppc_get_last_inst(vcpu)) != !!is_store)
> + if (instruction_is_store(last_inst) != !!is_store)
> return RESUME_GUEST;
>
> /*
> diff --git a/arch/powerpc/kvm/book3s_paired_singles.c b/arch/powerpc/kvm/book3s_paired_singles.c
> index 6c8011f..bfb8035 100644
> --- a/arch/powerpc/kvm/book3s_paired_singles.c
> +++ b/arch/powerpc/kvm/book3s_paired_singles.c
> @@ -639,26 +639,36 @@ static int kvmppc_ps_one_in(struct kvm_vcpu *vcpu, bool rc,
>
> int kvmppc_emulate_paired_single(struct kvm_run *run, struct kvm_vcpu *vcpu)
> {
> - u32 inst = kvmppc_get_last_inst(vcpu);
> + u32 inst;
> enum emulation_result emulated = EMULATE_DONE;
> + int ax_rd, ax_ra, ax_rb, ax_rc;
> + short full_d;
> + u64 *fpr_d, *fpr_a, *fpr_b, *fpr_c;
>
> - int ax_rd = inst_get_field(inst, 6, 10);
> - int ax_ra = inst_get_field(inst, 11, 15);
> - int ax_rb = inst_get_field(inst, 16, 20);
> - int ax_rc = inst_get_field(inst, 21, 25);
> - short full_d = inst_get_field(inst, 16, 31);
> -
> - u64 *fpr_d = &VCPU_FPR(vcpu, ax_rd);
> - u64 *fpr_a = &VCPU_FPR(vcpu, ax_ra);
> - u64 *fpr_b = &VCPU_FPR(vcpu, ax_rb);
> - u64 *fpr_c = &VCPU_FPR(vcpu, ax_rc);
> -
> - bool rcomp = (inst & 1) ? true : false;
> - u32 cr = kvmppc_get_cr(vcpu);
> + bool rcomp;
> + u32 cr;
> #ifdef DEBUG
> int i;
> #endif
>
> + emulated = kvmppc_get_last_inst(vcpu, INST_GENERIC, &inst);
> + if (emulated != EMULATE_DONE)
> + return emulated;
Here we're already leaking the "emulated" return value into the caller.
The caller in this chain should know what EMULATE_AGAIN means, so with
the change above we're safe.
> +
> + ax_rd = inst_get_field(inst, 6, 10);
> + ax_ra = inst_get_field(inst, 11, 15);
> + ax_rb = inst_get_field(inst, 16, 20);
> + ax_rc = inst_get_field(inst, 21, 25);
> + full_d = inst_get_field(inst, 16, 31);
> +
> + fpr_d = &VCPU_FPR(vcpu, ax_rd);
> + fpr_a = &VCPU_FPR(vcpu, ax_ra);
> + fpr_b = &VCPU_FPR(vcpu, ax_rb);
> + fpr_c = &VCPU_FPR(vcpu, ax_rc);
> +
> + rcomp = (inst & 1) ? true : false;
> + cr = kvmppc_get_cr(vcpu);
> +
> if (!kvmppc_inst_is_paired_single(vcpu, inst))
> return EMULATE_FAIL;
>
> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> index d247d88..7bbaeec 100644
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c
> @@ -970,15 +970,24 @@ int kvmppc_handle_exit_pr(struct kvm_run *run, struct kvm_vcpu *vcpu,
> {
> enum emulation_result er;
> ulong flags;
> + u32 last_inst;
> + int emul;
>
> program_interrupt:
> flags = vcpu->arch.shadow_srr1 & 0x1f0000ull;
>
> + emul = kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst);
> + if (emul != EMULATE_DONE) {
> + r = RESUME_GUEST;
Yes, this looks good.
> + break;
> + }
> +
> if (kvmppc_get_msr(vcpu) & MSR_PR) {
> #ifdef EXIT_DEBUG
> - printk(KERN_INFO "Userspace triggered 0x700 exception at 0x%lx (0x%x)\n", kvmppc_get_pc(vcpu), kvmppc_get_last_inst(vcpu));
> + pr_info("Userspace triggered 0x700 exception at\n 0x%lx (0x%x)\n",
> + kvmppc_get_pc(vcpu), last_inst);
> #endif
> - if ((kvmppc_get_last_inst(vcpu) & 0xff0007ff) !=
> + if ((last_inst & 0xff0007ff) !=
> (INS_DCBZ & 0xfffffff7)) {
> kvmppc_core_queue_program(vcpu, flags);
> r = RESUME_GUEST;
> @@ -997,7 +1006,7 @@ program_interrupt:
> break;
> case EMULATE_FAIL:
> printk(KERN_CRIT "%s: emulation at %lx failed (%08x)\n",
> - __func__, kvmppc_get_pc(vcpu), kvmppc_get_last_inst(vcpu));
> + __func__, kvmppc_get_pc(vcpu), last_inst);
> kvmppc_core_queue_program(vcpu, flags);
> r = RESUME_GUEST;
> break;
> @@ -1014,8 +1023,25 @@ program_interrupt:
> break;
> }
> case BOOK3S_INTERRUPT_SYSCALL:
> + {
> + u32 last_sc;
> + int emul;
> +
> + /* Get last sc for papr */
> + if (vcpu->arch.papr_enabled) {
> + /*
> + * The sc instuction sets SRR0 to point to the next inst
> + */
> + emul = kvmppc_get_last_inst(vcpu, INST_SC, &last_sc);
> + if (emul != EMULATE_DONE) {
> + kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) - 4);
> + r = RESUME_GUEST;
This also looks good :).
> + break;
> + }
> + }
> +
> if (vcpu->arch.papr_enabled &&
> - (kvmppc_get_last_sc(vcpu) == 0x44000022) &&
> + (last_sc == 0x44000022) &&
> !(kvmppc_get_msr(vcpu) & MSR_PR)) {
> /* SC 1 papr hypercalls */
> ulong cmd = kvmppc_get_gpr(vcpu, 3);
> @@ -1060,13 +1086,13 @@ program_interrupt:
> r = RESUME_GUEST;
> }
> break;
> + }
> case BOOK3S_INTERRUPT_FP_UNAVAIL:
> case BOOK3S_INTERRUPT_ALTIVEC:
> case BOOK3S_INTERRUPT_VSX:
> {
> int ext_msr = 0;
> int emul;
> - ulong pc;
> u32 last_inst;
>
> if (!(vcpu->arch.hflags & BOOK3S_HFLAG_PAIRED_SINGLE)) {
> @@ -1090,9 +1116,7 @@ program_interrupt:
> break;
> }
>
> - pc = kvmppc_get_pc(vcpu);
> - last_inst = kvmppc_get_last_inst(vcpu);
> - emul = kvmppc_ld(vcpu, &pc, sizeof(u32), &last_inst, false);
> + emul = kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst);
> if (emul == EMULATE_DONE) {
> /* we need to emulate this instruction */
> goto program_interrupt;
> @@ -1105,9 +1129,8 @@ program_interrupt:
> }
> case BOOK3S_INTERRUPT_ALIGNMENT:
> {
> - ulong pc = kvmppc_get_pc(vcpu);
> - u32 last_inst = kvmppc_get_last_inst(vcpu);
> - int emul = kvmppc_ld(vcpu, &pc, sizeof(u32), &last_inst, false);
> + u32 last_inst;
> + int emul = kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst);
>
> if (emul == EMULATE_DONE) {
> u32 dsisr;
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index ab62109..34a42b9 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -752,6 +752,9 @@ static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu)
> * they were actually modified by emulation. */
> return RESUME_GUEST_NV;
>
> + case EMULATE_AGAIN:
> + return RESUME_GUEST;
Yup :).
> +
> case EMULATE_DO_DCR:
> run->exit_reason = KVM_EXIT_DCR;
> return RESUME_HOST;
> diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
> index dd2cc03..4b4e8d6 100644
> --- a/arch/powerpc/kvm/e500_mmu_host.c
> +++ b/arch/powerpc/kvm/e500_mmu_host.c
> @@ -606,6 +606,12 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 eaddr, gpa_t gpaddr,
> }
> }
>
> +int kvmppc_load_last_inst(struct kvm_vcpu *vcpu, enum instruction_type type,
> + u32 *instr)
> +{
> + return EMULATE_FAIL;
This should be EMULATE_AGAIN, no?
> +}
> +
> /************* MMU Notifiers *************/
>
> int kvm_unmap_hva(struct kvm *kvm, unsigned long hva)
> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
> index da86d9b..c5c64b6 100644
> --- a/arch/powerpc/kvm/emulate.c
> +++ b/arch/powerpc/kvm/emulate.c
> @@ -224,19 +224,25 @@ static int kvmppc_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt)
> * from opcode tables in the future. */
> int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
> {
> - u32 inst = kvmppc_get_last_inst(vcpu);
> - int ra = get_ra(inst);
> - int rs = get_rs(inst);
> - int rt = get_rt(inst);
> - int sprn = get_sprn(inst);
> - enum emulation_result emulated = EMULATE_DONE;
> + u32 inst;
> + int ra, rs, rt, sprn;
> + enum emulation_result emulated;
> int advance = 1;
>
> /* this default type might be overwritten by subcategories */
> kvmppc_set_exit_type(vcpu, EMULATED_INST_EXITS);
>
> + emulated = kvmppc_get_last_inst(vcpu, false, &inst);
> + if (emulated != EMULATE_DONE)
> + return emulated;
and here we're leaking again ;). It's perfectly fine to do so, but let's
make sure we don't leak -ENOENT :). So here too the fix above should
make things work.
Alex
> +
> pr_debug("Emulating opcode %d / %d\n", get_op(inst), get_xop(inst));
>
> + ra = get_ra(inst);
> + rs = get_rs(inst);
> + rt = get_rt(inst);
> + sprn = get_sprn(inst);
> +
> switch (get_op(inst)) {
> case OP_TRAP:
> #ifdef CONFIG_PPC_BOOK3S
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 7efc2b7..da54e4b 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -261,6 +261,9 @@ int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu)
> * actually modified. */
> r = RESUME_GUEST_NV;
> break;
> + case EMULATE_AGAIN:
> + r = RESUME_GUEST;
> + break;
> case EMULATE_DO_MMIO:
> run->exit_reason = KVM_EXIT_MMIO;
> /* We must reload nonvolatiles because "update" load/store
> @@ -270,11 +273,15 @@ int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu)
> r = RESUME_HOST_NV;
> break;
> case EMULATE_FAIL:
> + {
> + u32 last_inst;
> +
> + kvmppc_get_last_inst(vcpu, false, &last_inst);
> /* XXX Deliver Program interrupt to guest. */
> - printk(KERN_EMERG "%s: emulation failed (%08x)\n", __func__,
> - kvmppc_get_last_inst(vcpu));
> + pr_emerg("%s: emulation failed (%08x)\n", __func__, last_inst);
> r = RESUME_HOST;
> break;
> + }
> default:
> WARN_ON(1);
> r = RESUME_GUEST;
^ permalink raw reply
* Re: [PATCH 5/5 v4] KVM: PPC: Bookehv: Get vcpu's last instruction for emulation
From: Alexander Graf @ 2014-07-03 13:53 UTC (permalink / raw)
To: Mihai Caraman, kvm-ppc; +Cc: linuxppc-dev, kvm
In-Reply-To: <1403909347-31622-6-git-send-email-mihai.caraman@freescale.com>
On 28.06.14 00:49, Mihai Caraman wrote:
> On book3e, KVM uses load external pid (lwepx) dedicated instruction to read
> guest last instruction on the exit path. lwepx exceptions (DTLB_MISS, DSI
> and LRAT), generated by loading a guest address, needs to be handled by KVM.
> These exceptions are generated in a substituted guest translation context
> (EPLC[EGS] = 1) from host context (MSR[GS] = 0).
>
> Currently, KVM hooks only interrupts generated from guest context (MSR[GS] = 1),
> doing minimal checks on the fast path to avoid host performance degradation.
> lwepx exceptions originate from host state (MSR[GS] = 0) which implies
> additional checks in DO_KVM macro (beside the current MSR[GS] = 1) by looking
> at the Exception Syndrome Register (ESR[EPID]) and the External PID Load Context
> Register (EPLC[EGS]). Doing this on each Data TLB miss exception is obvious
> too intrusive for the host.
>
> Read guest last instruction from kvmppc_load_last_inst() by searching for the
> physical address and kmap it. This address the TODO for TLB eviction and
> execute-but-not-read entries, and allow us to get rid of lwepx until we are
> able to handle failures.
>
> A simple stress benchmark shows a 1% sys performance degradation compared with
> previous approach (lwepx without failure handling):
>
> time for i in `seq 1 10000`; do /bin/echo > /dev/null; done
>
> real 0m 8.85s
> user 0m 4.34s
> sys 0m 4.48s
>
> vs
>
> real 0m 8.84s
> user 0m 4.36s
> sys 0m 4.44s
>
> An alternative solution, to handle lwepx exceptions in KVM, is to temporary
> highjack the interrupt vector from host. Some cores share host IVOR registers
> between hardware threads, which is the case of FSL e6500, which impose additional
> synchronization logic for this solution to work. The optimization can be addressed
> later on top of this patch.
>
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> v4:
> - add switch and new function when getting last inst earlier
> - use enum instead of prev semnatic
> - get rid of mas0, optimize mas7_mas3
> - give more context in visible messages
> - check storage attributes mismatch on MMUv2
> - get rid of pfn_valid check
>
> v3:
> - reworked patch description
> - use unaltered kmap addr for kunmap
> - get last instruction before beeing preempted
>
> v2:
> - reworked patch description
> - used pr_* functions
> - addressed cosmetic feedback
>
> arch/powerpc/kvm/booke.c | 44 +++++++++++++++++
> arch/powerpc/kvm/bookehv_interrupts.S | 37 ++++----------
> arch/powerpc/kvm/e500_mmu_host.c | 91 +++++++++++++++++++++++++++++++++++
> 3 files changed, 144 insertions(+), 28 deletions(-)
>
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 34a42b9..843077b 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -869,6 +869,28 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu *vcpu,
> }
> }
>
> +static int kvmppc_resume_inst_load(struct kvm_run *run, struct kvm_vcpu *vcpu,
> + enum emulation_result emulated, u32 last_inst)
> +{
> + switch (emulated) {
> + case EMULATE_AGAIN:
> + return RESUME_GUEST;
> +
> + case EMULATE_FAIL:
> + pr_debug("%s: load instruction from guest address %lx failed\n",
> + __func__, vcpu->arch.pc);
> + /* For debugging, encode the failing instruction and
> + * report it to userspace. */
> + run->hw.hardware_exit_reason = ~0ULL << 32;
> + run->hw.hardware_exit_reason |= last_inst;
> + kvmppc_core_queue_program(vcpu, ESR_PIL);
> + return RESUME_HOST;
> +
> + default:
> + BUG();
> + }
> +}
> +
> /**
> * kvmppc_handle_exit
> *
> @@ -880,6 +902,8 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> int r = RESUME_HOST;
> int s;
> int idx;
> + u32 last_inst = KVM_INST_FETCH_FAILED;
> + enum emulation_result emulated = EMULATE_DONE;
>
> /* update before a new last_exit_type is rewritten */
> kvmppc_update_timing_stats(vcpu);
> @@ -887,6 +911,20 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> /* restart interrupts if they were meant for the host */
> kvmppc_restart_interrupt(vcpu, exit_nr);
>
> + /*
> + * get last instruction before beeing preempted
> + * TODO: for e6500 check also BOOKE_INTERRUPT_LRAT_ERROR & ESR_DATA
> + */
> + switch (exit_nr) {
> + case BOOKE_INTERRUPT_DATA_STORAGE:
> + case BOOKE_INTERRUPT_DTLB_MISS:
> + case BOOKE_INTERRUPT_HV_PRIV:
> + emulated = kvmppc_get_last_inst(vcpu, false, &last_inst);
> + break;
> + default:
> + break;
> + }
> +
> local_irq_enable();
>
> trace_kvm_exit(exit_nr, vcpu);
> @@ -895,6 +933,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> run->exit_reason = KVM_EXIT_UNKNOWN;
> run->ready_for_interrupt_injection = 1;
>
> + if (emulated != EMULATE_DONE) {
> + r = kvmppc_resume_inst_load(run, vcpu, emulated, last_inst);
> + goto out;
> + }
> +
> switch (exit_nr) {
> case BOOKE_INTERRUPT_MACHINE_CHECK:
> printk("MACHINE CHECK: %lx\n", mfspr(SPRN_MCSR));
> @@ -1184,6 +1227,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> BUG();
> }
>
> +out:
> /*
> * To avoid clobbering exit_reason, only check for signals if we
> * aren't already exiting to userspace for some other reason.
> diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S
> index 6ff4480..e000b39 100644
> --- a/arch/powerpc/kvm/bookehv_interrupts.S
> +++ b/arch/powerpc/kvm/bookehv_interrupts.S
> @@ -121,38 +121,14 @@
> 1:
>
> .if \flags & NEED_EMU
> - /*
> - * This assumes you have external PID support.
> - * To support a bookehv CPU without external PID, you'll
> - * need to look up the TLB entry and create a temporary mapping.
> - *
> - * FIXME: we don't currently handle if the lwepx faults. PR-mode
> - * booke doesn't handle it either. Since Linux doesn't use
> - * broadcast tlbivax anymore, the only way this should happen is
> - * if the guest maps its memory execute-but-not-read, or if we
> - * somehow take a TLB miss in the middle of this entry code and
> - * evict the relevant entry. On e500mc, all kernel lowmem is
> - * bolted into TLB1 large page mappings, and we don't use
> - * broadcast invalidates, so we should not take a TLB miss here.
> - *
> - * Later we'll need to deal with faults here. Disallowing guest
> - * mappings that are execute-but-not-read could be an option on
> - * e500mc, but not on chips with an LRAT if it is used.
> - */
> -
> - mfspr r3, SPRN_EPLC /* will already have correct ELPID and EGS */
> PPC_STL r15, VCPU_GPR(R15)(r4)
> PPC_STL r16, VCPU_GPR(R16)(r4)
> PPC_STL r17, VCPU_GPR(R17)(r4)
> PPC_STL r18, VCPU_GPR(R18)(r4)
> PPC_STL r19, VCPU_GPR(R19)(r4)
> - mr r8, r3
> PPC_STL r20, VCPU_GPR(R20)(r4)
> - rlwimi r8, r6, EPC_EAS_SHIFT - MSR_IR_LG, EPC_EAS
> PPC_STL r21, VCPU_GPR(R21)(r4)
> - rlwimi r8, r6, EPC_EPR_SHIFT - MSR_PR_LG, EPC_EPR
> PPC_STL r22, VCPU_GPR(R22)(r4)
> - rlwimi r8, r10, EPC_EPID_SHIFT, EPC_EPID
> PPC_STL r23, VCPU_GPR(R23)(r4)
> PPC_STL r24, VCPU_GPR(R24)(r4)
> PPC_STL r25, VCPU_GPR(R25)(r4)
> @@ -162,10 +138,15 @@
> PPC_STL r29, VCPU_GPR(R29)(r4)
> PPC_STL r30, VCPU_GPR(R30)(r4)
> PPC_STL r31, VCPU_GPR(R31)(r4)
> - mtspr SPRN_EPLC, r8
> - isync
> - lwepx r9, 0, r5
> - mtspr SPRN_EPLC, r3
> +
> + /*
> + * We don't use external PID support. lwepx faults would need to be
> + * handled by KVM and this implies aditional code in DO_KVM (for
> + * DTB_MISS, DSI and LRAT) to check ESR[EPID] and EPLC[EGS] which
> + * is too intrusive for the host. Get last instuction in
> + * kvmppc_get_last_inst().
> + */
> + li r9, KVM_INST_FETCH_FAILED
> stw r9, VCPU_LAST_INST(r4)
> .endif
>
> diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
> index 4b4e8d6..57463e5 100644
> --- a/arch/powerpc/kvm/e500_mmu_host.c
> +++ b/arch/powerpc/kvm/e500_mmu_host.c
> @@ -606,11 +606,102 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 eaddr, gpa_t gpaddr,
> }
> }
>
> +#ifdef CONFIG_KVM_BOOKE_HV
> int kvmppc_load_last_inst(struct kvm_vcpu *vcpu, enum instruction_type type,
> u32 *instr)
> {
> + gva_t geaddr;
> + hpa_t addr;
> + hfn_t pfn;
> + hva_t eaddr;
> + u32 mas1, mas2, mas3;
> + u64 mas7_mas3;
> + struct page *page;
> + unsigned int addr_space, psize_shift;
> + bool pr;
> + unsigned long flags;
> +
> + /* Search TLB for guest pc to get the real address */
> + geaddr = kvmppc_get_pc(vcpu);
> +
> + addr_space = (vcpu->arch.shared->msr & MSR_IS) >> MSR_IR_LG;
> +
> + local_irq_save(flags);
> + mtspr(SPRN_MAS6, (vcpu->arch.pid << MAS6_SPID_SHIFT) | addr_space);
> + mtspr(SPRN_MAS5, MAS5_SGS | vcpu->kvm->arch.lpid);
> + asm volatile("tlbsx 0, %[geaddr]\n" : :
> + [geaddr] "r" (geaddr));
> + mtspr(SPRN_MAS5, 0);
> + mtspr(SPRN_MAS8, 0);
> + mas1 = mfspr(SPRN_MAS1);
> + mas2 = mfspr(SPRN_MAS2);
> + mas3 = mfspr(SPRN_MAS3);
> +#ifdef CONFIG_64BIT
> + mas7_mas3 = mfspr(SPRN_MAS7_MAS3);
> +#else
> + mas7_mas3 = ((u64)mfspr(SPRN_MAS7) << 32) | mas3;
> +#endif
> + local_irq_restore(flags);
> +
> + /*
> + * If the TLB entry for guest pc was evicted, return to the guest.
> + * There are high chances to find a valid TLB entry next time.
> + */
> + if (!(mas1 & MAS1_VALID))
> + return EMULATE_AGAIN;
> +
> + /*
> + * Another thread may rewrite the TLB entry in parallel, don't
> + * execute from the address if the execute permission is not set
> + */
> + pr = vcpu->arch.shared->msr & MSR_PR;
> + if (unlikely((pr && !(mas3 & MAS3_UX)) ||
> + (!pr && !(mas3 & MAS3_SX)))) {
> + pr_debug("%s: Instuction emulation from guest addres %08lx without execute permission\n",
> + __func__, geaddr);
> + return EMULATE_FAIL;
In this case how did we ever get here? Why can't we just evict the entry
and return EMULATE_AGAIN?
> + }
> +
> + /*
> + * The real address will be mapped by a cacheable, memory coherent,
> + * write-back page. Check for mismatches when LRAT is used.
> + */
> + if (has_feature(vcpu, VCPU_FTR_MMU_V2) &&
> + unlikely((mas2 & MAS2_I) || (mas2 & MAS2_W) || !(mas2 & MAS2_M))) {
> + pr_debug("%s: Instuction emulation from guest addres %08lx mismatches storage attributes\n",
> + __func__, geaddr);
> + return EMULATE_FAIL;
Hrm - do we really want to deal with injecting faults here? I'd say it's
ok to just end up in an endless EMULATE_AGAIN loop.
> + }
> +
> + /* Get page size */
> + psize_shift = MAS1_GET_TSIZE(mas1) + 10;
> +
> + /* Map a page and get guest's instruction */
> + addr = (mas7_mas3 & (~0ULL << psize_shift)) |
> + (geaddr & ((1ULL << psize_shift) - 1ULL));
> + pfn = addr >> PAGE_SHIFT;
> +
> + /* Guard us against emulation from devices area */
> + if (unlikely(!page_is_ram(pfn))) {
> + pr_debug("%s: Instruction emulation from non-RAM host addres %08llx is not supported\n",
> + __func__, addr);
> + return EMULATE_FAIL;
Same here :).
> + }
> +
> + page = pfn_to_page(pfn);
> + eaddr = (unsigned long)kmap_atomic(page);
> + *instr = *(u32 *)(eaddr | (addr & ~PAGE_MASK));
> + kunmap_atomic((u32 *)eaddr);
Doesn't kmap_atomic() have to be guarded somehow?
Alex
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox