* [PATCH v2] powerpc/perf: Use PVR rather than oprofile field to determine CPU version
From: Rashmica Gupta @ 2019-02-06 6:30 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Rashmica Gupta, maddy, npiggin
Currently the perf CPU backend drivers detect what CPU they're on using
cur_cpu_spec->oprofile_cpu_type.
Although that works, it's a bit crufty to be using oprofile related fields,
especially seeing as oprofile is more or less unused these days.
It also means perf is reliant on the fragile logic in setup_cpu_spec()
which detects when we're using a logical PVR and copies back the PMU
related fields from the raw CPU entry. So lets check the PVR directly.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
---
v2: fixed misspelling of PVR_VER_E500V2
arch/powerpc/perf/e500-pmu.c | 10 ++++++----
arch/powerpc/perf/e6500-pmu.c | 5 +++--
arch/powerpc/perf/hv-24x7.c | 6 +++---
arch/powerpc/perf/mpc7450-pmu.c | 5 +++--
arch/powerpc/perf/power5+-pmu.c | 6 +++---
arch/powerpc/perf/power5-pmu.c | 5 +++--
arch/powerpc/perf/power6-pmu.c | 5 +++--
arch/powerpc/perf/power7-pmu.c | 7 ++++---
arch/powerpc/perf/power8-pmu.c | 5 +++--
arch/powerpc/perf/power9-pmu.c | 4 +---
arch/powerpc/perf/ppc970-pmu.c | 8 +++++---
11 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/arch/powerpc/perf/e500-pmu.c b/arch/powerpc/perf/e500-pmu.c
index fb664929f5da..e1a185a30928 100644
--- a/arch/powerpc/perf/e500-pmu.c
+++ b/arch/powerpc/perf/e500-pmu.c
@@ -122,12 +122,14 @@ static struct fsl_emb_pmu e500_pmu = {
static int init_e500_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type)
- return -ENODEV;
+ unsigned int pvr = mfspr(SPRN_PVR);
- if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500mc"))
+ /* ec500mc */
+ if ((PVR_VER(pvr) == PVR_VER_E500MC) || (PVR_VER(pvr) == PVR_VER_E5500))
num_events = 256;
- else if (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500"))
+ /* e500 */
+ else if ((PVR_VER(pvr) != PVR_VER_E500V1) &&
+ (PVR_VER(pvr) != PVR_VER_E500V2))
return -ENODEV;
return register_fsl_emb_pmu(&e500_pmu);
diff --git a/arch/powerpc/perf/e6500-pmu.c b/arch/powerpc/perf/e6500-pmu.c
index 3d877aa777b5..47c93d13da1a 100644
--- a/arch/powerpc/perf/e6500-pmu.c
+++ b/arch/powerpc/perf/e6500-pmu.c
@@ -111,8 +111,9 @@ static struct fsl_emb_pmu e6500_pmu = {
static int init_e6500_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e6500"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_VER_E6500)
return -ENODEV;
return register_fsl_emb_pmu(&e6500_pmu);
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 72238eedc360..30dd379ddcd3 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -1583,16 +1583,16 @@ static int hv_24x7_init(void)
{
int r;
unsigned long hret;
+ unsigned int pvr = mfspr(SPRN_PVR);
struct hv_perf_caps caps;
if (!firmware_has_feature(FW_FEATURE_LPAR)) {
pr_debug("not a virtualized system, not enabling\n");
return -ENODEV;
- } else if (!cur_cpu_spec->oprofile_cpu_type)
- return -ENODEV;
+ }
/* POWER8 only supports v1, while POWER9 only supports v2. */
- if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
+ if (PVR_VER(pvr) == PVR_POWER8)
interface_version = 1;
else {
interface_version = 2;
diff --git a/arch/powerpc/perf/mpc7450-pmu.c b/arch/powerpc/perf/mpc7450-pmu.c
index d115c5635bf3..17e69cabbcac 100644
--- a/arch/powerpc/perf/mpc7450-pmu.c
+++ b/arch/powerpc/perf/mpc7450-pmu.c
@@ -413,8 +413,9 @@ struct power_pmu mpc7450_pmu = {
static int __init init_mpc7450_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_7450)
return -ENODEV;
return register_power_pmu(&mpc7450_pmu);
diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c
index 0526dac66007..17a32e7ef234 100644
--- a/arch/powerpc/perf/power5+-pmu.c
+++ b/arch/powerpc/perf/power5+-pmu.c
@@ -679,9 +679,9 @@ static struct power_pmu power5p_pmu = {
static int __init init_power5p_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5+")
- && strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5++")))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_POWER5p)
return -ENODEV;
return register_power_pmu(&power5p_pmu);
diff --git a/arch/powerpc/perf/power5-pmu.c b/arch/powerpc/perf/power5-pmu.c
index 4dc99f9f7962..844782e6d367 100644
--- a/arch/powerpc/perf/power5-pmu.c
+++ b/arch/powerpc/perf/power5-pmu.c
@@ -620,8 +620,9 @@ static struct power_pmu power5_pmu = {
static int __init init_power5_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_POWER5)
return -ENODEV;
return register_power_pmu(&power5_pmu);
diff --git a/arch/powerpc/perf/power6-pmu.c b/arch/powerpc/perf/power6-pmu.c
index 9c9d646b68a1..9659b781f588 100644
--- a/arch/powerpc/perf/power6-pmu.c
+++ b/arch/powerpc/perf/power6-pmu.c
@@ -542,8 +542,9 @@ static struct power_pmu power6_pmu = {
static int __init init_power6_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power6"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_POWER6)
return -ENODEV;
return register_power_pmu(&power6_pmu);
diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
index 6dbae9884ec4..79f05a7f28c6 100644
--- a/arch/powerpc/perf/power7-pmu.c
+++ b/arch/powerpc/perf/power7-pmu.c
@@ -447,11 +447,12 @@ static struct power_pmu power7_pmu = {
static int __init init_power7_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power7"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if ((PVR_VER(pvr) != PVR_POWER7) && (PVR_VER(pvr) != PVR_POWER7p))
return -ENODEV;
- if (pvr_version_is(PVR_POWER7p))
+ if (PVR_VER(pvr) == PVR_POWER7p)
power7_pmu.flags |= PPMU_SIAR_VALID;
return register_power_pmu(&power7_pmu);
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index d12a2db26353..81a5142efab0 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -382,9 +382,10 @@ static struct power_pmu power8_pmu = {
static int __init init_power8_pmu(void)
{
int rc;
+ unsigned int pvr = mfspr(SPRN_PVR);
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
+ if ((PVR_VER(pvr) != PVR_POWER8E) && (PVR_VER(pvr) != PVR_POWER8NVL)
+ && PVR_VER(pvr) != PVR_POWER8)
return -ENODEV;
rc = register_power_pmu(&power8_pmu);
diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
index 0ff9c43733e9..6b414b8bedfd 100644
--- a/arch/powerpc/perf/power9-pmu.c
+++ b/arch/powerpc/perf/power9-pmu.c
@@ -438,9 +438,7 @@ static int __init init_power9_pmu(void)
int rc = 0;
unsigned int pvr = mfspr(SPRN_PVR);
- /* Comes from cpu_specs[] */
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power9"))
+ if (PVR_VER(pvr) != PVR_POWER9)
return -ENODEV;
/* Blacklist events */
diff --git a/arch/powerpc/perf/ppc970-pmu.c b/arch/powerpc/perf/ppc970-pmu.c
index 8b6a8a36fa38..5832de10e073 100644
--- a/arch/powerpc/perf/ppc970-pmu.c
+++ b/arch/powerpc/perf/ppc970-pmu.c
@@ -492,9 +492,11 @@ static struct power_pmu ppc970_pmu = {
static int __init init_ppc970_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970")
- && strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970MP")))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if ((PVR_VER(pvr) != PVR_970) && (PVR_VER(pvr) != PVR_970MP)
+ && (PVR_VER(pvr) != PVR_970FX)
+ && (PVR_VER(pvr) != PVR_970GX))
return -ENODEV;
return register_power_pmu(&ppc970_pmu);
--
2.17.2
^ permalink raw reply related
* [PATCH] powerpc/perf: Use PVR rather than oprofile field to determine CPU version
From: Rashmica Gupta @ 2019-02-06 5:40 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Rashmica Gupta, maddy, npiggin
Currently the perf CPU backend drivers detect what CPU they're on using
cur_cpu_spec->oprofile_cpu_type.
Although that works, it's a bit crufty to be using oprofile related fields,
especially seeing as oprofile is more or less unused these days.
It also means perf is reliant on the fragile logic in setup_cpu_spec()
which detects when we're using a logical PVR and copies back the PMU
related fields from the raw CPU entry. So lets check the PVR directly.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
---
arch/powerpc/perf/e500-pmu.c | 10 ++++++----
arch/powerpc/perf/e6500-pmu.c | 5 +++--
arch/powerpc/perf/hv-24x7.c | 6 +++---
arch/powerpc/perf/mpc7450-pmu.c | 5 +++--
arch/powerpc/perf/power5+-pmu.c | 6 +++---
arch/powerpc/perf/power5-pmu.c | 5 +++--
arch/powerpc/perf/power6-pmu.c | 5 +++--
arch/powerpc/perf/power7-pmu.c | 7 ++++---
arch/powerpc/perf/power8-pmu.c | 5 +++--
arch/powerpc/perf/power9-pmu.c | 4 +---
arch/powerpc/perf/ppc970-pmu.c | 8 +++++---
11 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/arch/powerpc/perf/e500-pmu.c b/arch/powerpc/perf/e500-pmu.c
index fb664929f5da..f3a4179f46c6 100644
--- a/arch/powerpc/perf/e500-pmu.c
+++ b/arch/powerpc/perf/e500-pmu.c
@@ -122,12 +122,14 @@ static struct fsl_emb_pmu e500_pmu = {
static int init_e500_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type)
- return -ENODEV;
+ unsigned int pvr = mfspr(SPRN_PVR);
- if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500mc"))
+ /* ec500mc */
+ if ((PVR_VER(pvr) == PVR_VER_E500MC) || (PVR_VER(pvr) == PVR_VER_E5500))
num_events = 256;
- else if (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e500"))
+ /* e500 */
+ else if ((PVR_VER(pvr) != PVR_VER_E500V1) &&
+ (PVR_VER(pvr) != PVR_VER_E50V2))
return -ENODEV;
return register_fsl_emb_pmu(&e500_pmu);
diff --git a/arch/powerpc/perf/e6500-pmu.c b/arch/powerpc/perf/e6500-pmu.c
index 3d877aa777b5..47c93d13da1a 100644
--- a/arch/powerpc/perf/e6500-pmu.c
+++ b/arch/powerpc/perf/e6500-pmu.c
@@ -111,8 +111,9 @@ static struct fsl_emb_pmu e6500_pmu = {
static int init_e6500_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/e6500"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_VER_E6500)
return -ENODEV;
return register_fsl_emb_pmu(&e6500_pmu);
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 72238eedc360..30dd379ddcd3 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -1583,16 +1583,16 @@ static int hv_24x7_init(void)
{
int r;
unsigned long hret;
+ unsigned int pvr = mfspr(SPRN_PVR);
struct hv_perf_caps caps;
if (!firmware_has_feature(FW_FEATURE_LPAR)) {
pr_debug("not a virtualized system, not enabling\n");
return -ENODEV;
- } else if (!cur_cpu_spec->oprofile_cpu_type)
- return -ENODEV;
+ }
/* POWER8 only supports v1, while POWER9 only supports v2. */
- if (!strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
+ if (PVR_VER(pvr) == PVR_POWER8)
interface_version = 1;
else {
interface_version = 2;
diff --git a/arch/powerpc/perf/mpc7450-pmu.c b/arch/powerpc/perf/mpc7450-pmu.c
index d115c5635bf3..17e69cabbcac 100644
--- a/arch/powerpc/perf/mpc7450-pmu.c
+++ b/arch/powerpc/perf/mpc7450-pmu.c
@@ -413,8 +413,9 @@ struct power_pmu mpc7450_pmu = {
static int __init init_mpc7450_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc/7450"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_7450)
return -ENODEV;
return register_power_pmu(&mpc7450_pmu);
diff --git a/arch/powerpc/perf/power5+-pmu.c b/arch/powerpc/perf/power5+-pmu.c
index 0526dac66007..17a32e7ef234 100644
--- a/arch/powerpc/perf/power5+-pmu.c
+++ b/arch/powerpc/perf/power5+-pmu.c
@@ -679,9 +679,9 @@ static struct power_pmu power5p_pmu = {
static int __init init_power5p_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5+")
- && strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5++")))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_POWER5p)
return -ENODEV;
return register_power_pmu(&power5p_pmu);
diff --git a/arch/powerpc/perf/power5-pmu.c b/arch/powerpc/perf/power5-pmu.c
index 4dc99f9f7962..844782e6d367 100644
--- a/arch/powerpc/perf/power5-pmu.c
+++ b/arch/powerpc/perf/power5-pmu.c
@@ -620,8 +620,9 @@ static struct power_pmu power5_pmu = {
static int __init init_power5_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power5"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_POWER5)
return -ENODEV;
return register_power_pmu(&power5_pmu);
diff --git a/arch/powerpc/perf/power6-pmu.c b/arch/powerpc/perf/power6-pmu.c
index 9c9d646b68a1..9659b781f588 100644
--- a/arch/powerpc/perf/power6-pmu.c
+++ b/arch/powerpc/perf/power6-pmu.c
@@ -542,8 +542,9 @@ static struct power_pmu power6_pmu = {
static int __init init_power6_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power6"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if (PVR_VER(pvr) != PVR_POWER6)
return -ENODEV;
return register_power_pmu(&power6_pmu);
diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
index 6dbae9884ec4..79f05a7f28c6 100644
--- a/arch/powerpc/perf/power7-pmu.c
+++ b/arch/powerpc/perf/power7-pmu.c
@@ -447,11 +447,12 @@ static struct power_pmu power7_pmu = {
static int __init init_power7_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power7"))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if ((PVR_VER(pvr) != PVR_POWER7) && (PVR_VER(pvr) != PVR_POWER7p))
return -ENODEV;
- if (pvr_version_is(PVR_POWER7p))
+ if (PVR_VER(pvr) == PVR_POWER7p)
power7_pmu.flags |= PPMU_SIAR_VALID;
return register_power_pmu(&power7_pmu);
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index d12a2db26353..81a5142efab0 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -382,9 +382,10 @@ static struct power_pmu power8_pmu = {
static int __init init_power8_pmu(void)
{
int rc;
+ unsigned int pvr = mfspr(SPRN_PVR);
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
+ if ((PVR_VER(pvr) != PVR_POWER8E) && (PVR_VER(pvr) != PVR_POWER8NVL)
+ && PVR_VER(pvr) != PVR_POWER8)
return -ENODEV;
rc = register_power_pmu(&power8_pmu);
diff --git a/arch/powerpc/perf/power9-pmu.c b/arch/powerpc/perf/power9-pmu.c
index 0ff9c43733e9..6b414b8bedfd 100644
--- a/arch/powerpc/perf/power9-pmu.c
+++ b/arch/powerpc/perf/power9-pmu.c
@@ -438,9 +438,7 @@ static int __init init_power9_pmu(void)
int rc = 0;
unsigned int pvr = mfspr(SPRN_PVR);
- /* Comes from cpu_specs[] */
- if (!cur_cpu_spec->oprofile_cpu_type ||
- strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power9"))
+ if (PVR_VER(pvr) != PVR_POWER9)
return -ENODEV;
/* Blacklist events */
diff --git a/arch/powerpc/perf/ppc970-pmu.c b/arch/powerpc/perf/ppc970-pmu.c
index 8b6a8a36fa38..5832de10e073 100644
--- a/arch/powerpc/perf/ppc970-pmu.c
+++ b/arch/powerpc/perf/ppc970-pmu.c
@@ -492,9 +492,11 @@ static struct power_pmu ppc970_pmu = {
static int __init init_ppc970_pmu(void)
{
- if (!cur_cpu_spec->oprofile_cpu_type ||
- (strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970")
- && strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/970MP")))
+ unsigned int pvr = mfspr(SPRN_PVR);
+
+ if ((PVR_VER(pvr) != PVR_970) && (PVR_VER(pvr) != PVR_970MP)
+ && (PVR_VER(pvr) != PVR_970FX)
+ && (PVR_VER(pvr) != PVR_970GX))
return -ENODEV;
return register_power_pmu(&ppc970_pmu);
--
2.17.2
^ permalink raw reply related
* linux-next: manual merge of the akpm-current tree with the powerpc-fixes tree
From: Stephen Rothwell @ 2019-02-06 5:15 UTC (permalink / raw)
To: Andrew Morton, Michael Ellerman, Benjamin Herrenschmidt, PowerPC
Cc: Aneesh Kumar K.V, Linux Next Mailing List,
Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 2641 bytes --]
Hi all,
Today's linux-next merge of the akpm-current tree got a conflict in:
arch/powerpc/mm/pgtable-book3s64.c
between commit:
579b9239c1f3 ("powerpc/radix: Fix kernel crash with mremap()")
from the powerpc-fixes tree and commit:
41bde21e85a7 ("arch/powerpc/mm: Nest MMU workaround for mprotect RW upgrade")
from the akpm-current tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc arch/powerpc/mm/pgtable-book3s64.c
index ecd31569a120,9f154efed1ae..c11c60056669
--- a/arch/powerpc/mm/pgtable-book3s64.c
+++ b/arch/powerpc/mm/pgtable-book3s64.c
@@@ -401,24 -398,27 +398,49 @@@ void arch_report_meminfo(struct seq_fil
}
#endif /* CONFIG_PROC_FS */
+/*
+ * For hash translation mode, we use the deposited table to store hash slot
+ * information and they are stored at PTRS_PER_PMD offset from related pmd
+ * location. Hence a pmd move requires deposit and withdraw.
+ *
+ * For radix translation with split pmd ptl, we store the deposited table in the
+ * pmd page. Hence if we have different pmd page we need to withdraw during pmd
+ * move.
+ *
+ * With hash we use deposited table always irrespective of anon or not.
+ * With radix we use deposited table only for anonymous mapping.
+ */
+int pmd_move_must_withdraw(struct spinlock *new_pmd_ptl,
+ struct spinlock *old_pmd_ptl,
+ struct vm_area_struct *vma)
+{
+ if (radix_enabled())
+ return (new_pmd_ptl != old_pmd_ptl) && vma_is_anonymous(vma);
+
+ return true;
+}
++
+ pte_t ptep_modify_prot_start(struct vm_area_struct *vma, unsigned long addr,
+ pte_t *ptep)
+ {
+ unsigned long pte_val;
+
+ /*
+ * Clear the _PAGE_PRESENT so that no hardware parallel update is
+ * possible. Also keep the pte_present true so that we don't take
+ * wrong fault.
+ */
+ pte_val = pte_update(vma->vm_mm, addr, ptep, _PAGE_PRESENT, _PAGE_INVALID, 0);
+
+ return __pte(pte_val);
+
+ }
+
+ void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr,
+ pte_t *ptep, pte_t old_pte, pte_t pte)
+ {
+ if (radix_enabled())
+ return radix__ptep_modify_prot_commit(vma, addr,
+ ptep, old_pte, pte);
+ set_pte_at(vma->vm_mm, addr, ptep, pte);
+ }
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/64s: Clear on-stack exception marker upon exception return
From: Michael Ellerman @ 2019-02-06 4:44 UTC (permalink / raw)
To: Balbir Singh
Cc: Joe Lawrence, Nicolai Stange, Jiri Kosina,
linux-kernel@vger.kernel.org, Torsten Duwe, Josh Poimboeuf,
live-patching, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <CAKTCnznr9MoL=eZsNWyGkKYdx7UbLQr86VL=xmC8h2ig8Sw+RQ@mail.gmail.com>
Balbir Singh <bsingharora@gmail.com> writes:
> On Tue, Feb 5, 2019 at 10:24 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Balbir Singh <bsingharora@gmail.com> writes:
>> > On Sat, Feb 2, 2019 at 12:14 PM Balbir Singh <bsingharora@gmail.com> wrote:
>> >> On Tue, Jan 22, 2019 at 10:57:21AM -0500, Joe Lawrence wrote:
>> >> > From: Nicolai Stange <nstange@suse.de>
>> >> >
>> >> > The ppc64 specific implementation of the reliable stacktracer,
>> >> > save_stack_trace_tsk_reliable(), bails out and reports an "unreliable
>> >> > trace" whenever it finds an exception frame on the stack. Stack frames
>> >> > are classified as exception frames if the STACK_FRAME_REGS_MARKER magic,
>> >> > as written by exception prologues, is found at a particular location.
>> >> >
>> >> > However, as observed by Joe Lawrence, it is possible in practice that
>> >> > non-exception stack frames can alias with prior exception frames and thus,
>> >> > that the reliable stacktracer can find a stale STACK_FRAME_REGS_MARKER on
>> >> > the stack. It in turn falsely reports an unreliable stacktrace and blocks
>> >> > any live patching transition to finish. Said condition lasts until the
>> >> > stack frame is overwritten/initialized by function call or other means.
>> >> >
>> >> > In principle, we could mitigate this by making the exception frame
>> >> > classification condition in save_stack_trace_tsk_reliable() stronger:
>> >> > in addition to testing for STACK_FRAME_REGS_MARKER, we could also take into
>> >> > account that for all exceptions executing on the kernel stack
>> >> > - their stack frames's backlink pointers always match what is saved
>> >> > in their pt_regs instance's ->gpr[1] slot and that
>> >> > - their exception frame size equals STACK_INT_FRAME_SIZE, a value
>> >> > uncommonly large for non-exception frames.
>> >> >
>> >> > However, while these are currently true, relying on them would make the
>> >> > reliable stacktrace implementation more sensitive towards future changes in
>> >> > the exception entry code. Note that false negatives, i.e. not detecting
>> >> > exception frames, would silently break the live patching consistency model.
>> >> >
>> >> > Furthermore, certain other places (diagnostic stacktraces, perf, xmon)
>> >> > rely on STACK_FRAME_REGS_MARKER as well.
>> >> >
>> >> > Make the exception exit code clear the on-stack STACK_FRAME_REGS_MARKER
>> >> > for those exceptions running on the "normal" kernel stack and returning
>> >> > to kernelspace: because the topmost frame is ignored by the reliable stack
>> >> > tracer anyway, returns to userspace don't need to take care of clearing
>> >> > the marker.
>> >> >
>> >> > Furthermore, as I don't have the ability to test this on Book 3E or
>> >> > 32 bits, limit the change to Book 3S and 64 bits.
>> >> >
>> >> > Finally, make the HAVE_RELIABLE_STACKTRACE Kconfig option depend on
>> >> > PPC_BOOK3S_64 for documentation purposes. Before this patch, it depended
>> >> > on PPC64 && CPU_LITTLE_ENDIAN and because CPU_LITTLE_ENDIAN implies
>> >> > PPC_BOOK3S_64, there's no functional change here.
>> >> >
>> >> > Fixes: df78d3f61480 ("powerpc/livepatch: Implement reliable stack tracing for the consistency model")
>> >> > Reported-by: Joe Lawrence <joe.lawrence@redhat.com>
>> >> > Signed-off-by: Nicolai Stange <nstange@suse.de>
>> >> > Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
>> >> > ---
>> >> > arch/powerpc/Kconfig | 2 +-
>> >> > arch/powerpc/kernel/entry_64.S | 7 +++++++
>> >> > 2 files changed, 8 insertions(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> >> > index 2890d36eb531..73bf87b1d274 100644
>> >> > --- a/arch/powerpc/Kconfig
>> >> > +++ b/arch/powerpc/Kconfig
>> >> > @@ -220,7 +220,7 @@ config PPC
>> >> > select HAVE_PERF_USER_STACK_DUMP
>> >> > select HAVE_RCU_TABLE_FREE if SMP
>> >> > select HAVE_REGS_AND_STACK_ACCESS_API
>> >> > - select HAVE_RELIABLE_STACKTRACE if PPC64 && CPU_LITTLE_ENDIAN
>> >> > + select HAVE_RELIABLE_STACKTRACE if PPC_BOOK3S_64 && CPU_LITTLE_ENDIAN
>> >> > select HAVE_SYSCALL_TRACEPOINTS
>> >> > select HAVE_VIRT_CPU_ACCOUNTING
>> >> > select HAVE_IRQ_TIME_ACCOUNTING
>> >> > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
>> >> > index 435927f549c4..a2c168b395d2 100644
>> >> > --- a/arch/powerpc/kernel/entry_64.S
>> >> > +++ b/arch/powerpc/kernel/entry_64.S
>> >> > @@ -1002,6 +1002,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>> >> > ld r2,_NIP(r1)
>> >> > mtspr SPRN_SRR0,r2
>> >> >
>> >> > + /*
>> >> > + * Leaving a stale exception_marker on the stack can confuse
>> >> > + * the reliable stack unwinder later on. Clear it.
>> >> > + */
>> >> > + li r2,0
>> >> > + std r2,STACK_FRAME_OVERHEAD-16(r1)
>> >> > +
>> >>
>> >> Could you please double check, r4 is already 0 at this point
>> >> IIUC. So the change might be a simple
>> >>
>> >> std r4,STACK_FRAME_OVERHEAD-16(r1)
>> >>
>> >
>> > r4 is not 0, sorry for the noise
>>
>> Isn't it?
>
> It is, I seem to be reading the wrong bits and confused myself, had to
> re-read mtmsrd to ensure it does not modify RS, just MSR. So I guess
> we could reuse r4.
Yeah it's a bit hard to follow now that we have the split exit paths for
user vs kernel. r4 does get used on the return to userspace case, by
ACCOUNT_CPU_USER_EXIT(), but for the return to kernel it's still got
zero in it.
> Should I send a patch on top of this? I have limited testing
> infrastructure at the moment, I could use qemu
I'm not sure. It's a bit fragile relying on the r4 value being zero, it
would be easy to accidentally reuse r4. Though it actually wouldn't
matter as long as r4 never has "regshere" in it.
In fact we could store any random value there, it just needs to not be
the exception marker. eg. we could just stick the SRR0 value in there,
that should never alias with "regshere".
But I think maybe we're over thinking it, the cost of the li is pretty
minimal compared to everything else going on here, and this is only on
the return to kernel case, which is arguably not a super hot path.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/powernv/npu: Remove redundant change_pte() hook
From: Balbir Singh @ 2019-02-06 2:55 UTC (permalink / raw)
To: Alistair Popple
Cc: Andrea Arcangeli, Alexey Kardashevskiy, Jason Wang,
Mark Hairgrove, linux-kernel@vger.kernel.org, Peter Xu,
Jerome Glisse, Paul Mackerras,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), David Gibson
In-Reply-To: <3013350.qntrAZtlsQ@townsend>
On Tue, Feb 5, 2019 at 2:52 PM Alistair Popple <alistair@popple.id.au> wrote:
>
> On Thursday, 31 January 2019 12:11:06 PM AEDT Andrea Arcangeli wrote:
> > On Thu, Jan 31, 2019 at 06:30:22PM +0800, Peter Xu wrote:
> > > The change_pte() notifier was designed to use as a quick path to
> > > update secondary MMU PTEs on write permission changes or PFN changes.
> > > For KVM, it could reduce the vm-exits when vcpu faults on the pages
> > > that was touched up by KSM. It's not used to do cache invalidations,
> > > for example, if we see the notifier will be called before the real PTE
> > > update after all (please see set_pte_at_notify that set_pte_at was
> > > called later).
>
> Thanks for the fixup. I didn't realise that invalidate_range() always gets
> called but I now see that is the case so this change looks good to me as well.
>
> Reviewed-by: Alistair Popple <alistair@popple.id.au>
>
I checked the three callers of set_pte_at_notify and the assumption
seems correct
Reviewed-by: Balbir Singh <bsingharora@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/64s: Clear on-stack exception marker upon exception return
From: Balbir Singh @ 2019-02-06 2:48 UTC (permalink / raw)
To: Michael Ellerman
Cc: Joe Lawrence, Nicolai Stange, Jiri Kosina,
linux-kernel@vger.kernel.org, Torsten Duwe, Josh Poimboeuf,
live-patching, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <87bm3qmqak.fsf@concordia.ellerman.id.au>
On Tue, Feb 5, 2019 at 10:24 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Balbir Singh <bsingharora@gmail.com> writes:
> > On Sat, Feb 2, 2019 at 12:14 PM Balbir Singh <bsingharora@gmail.com> wrote:
> >>
> >> On Tue, Jan 22, 2019 at 10:57:21AM -0500, Joe Lawrence wrote:
> >> > From: Nicolai Stange <nstange@suse.de>
> >> >
> >> > The ppc64 specific implementation of the reliable stacktracer,
> >> > save_stack_trace_tsk_reliable(), bails out and reports an "unreliable
> >> > trace" whenever it finds an exception frame on the stack. Stack frames
> >> > are classified as exception frames if the STACK_FRAME_REGS_MARKER magic,
> >> > as written by exception prologues, is found at a particular location.
> >> >
> >> > However, as observed by Joe Lawrence, it is possible in practice that
> >> > non-exception stack frames can alias with prior exception frames and thus,
> >> > that the reliable stacktracer can find a stale STACK_FRAME_REGS_MARKER on
> >> > the stack. It in turn falsely reports an unreliable stacktrace and blocks
> >> > any live patching transition to finish. Said condition lasts until the
> >> > stack frame is overwritten/initialized by function call or other means.
> >> >
> >> > In principle, we could mitigate this by making the exception frame
> >> > classification condition in save_stack_trace_tsk_reliable() stronger:
> >> > in addition to testing for STACK_FRAME_REGS_MARKER, we could also take into
> >> > account that for all exceptions executing on the kernel stack
> >> > - their stack frames's backlink pointers always match what is saved
> >> > in their pt_regs instance's ->gpr[1] slot and that
> >> > - their exception frame size equals STACK_INT_FRAME_SIZE, a value
> >> > uncommonly large for non-exception frames.
> >> >
> >> > However, while these are currently true, relying on them would make the
> >> > reliable stacktrace implementation more sensitive towards future changes in
> >> > the exception entry code. Note that false negatives, i.e. not detecting
> >> > exception frames, would silently break the live patching consistency model.
> >> >
> >> > Furthermore, certain other places (diagnostic stacktraces, perf, xmon)
> >> > rely on STACK_FRAME_REGS_MARKER as well.
> >> >
> >> > Make the exception exit code clear the on-stack STACK_FRAME_REGS_MARKER
> >> > for those exceptions running on the "normal" kernel stack and returning
> >> > to kernelspace: because the topmost frame is ignored by the reliable stack
> >> > tracer anyway, returns to userspace don't need to take care of clearing
> >> > the marker.
> >> >
> >> > Furthermore, as I don't have the ability to test this on Book 3E or
> >> > 32 bits, limit the change to Book 3S and 64 bits.
> >> >
> >> > Finally, make the HAVE_RELIABLE_STACKTRACE Kconfig option depend on
> >> > PPC_BOOK3S_64 for documentation purposes. Before this patch, it depended
> >> > on PPC64 && CPU_LITTLE_ENDIAN and because CPU_LITTLE_ENDIAN implies
> >> > PPC_BOOK3S_64, there's no functional change here.
> >> >
> >> > Fixes: df78d3f61480 ("powerpc/livepatch: Implement reliable stack tracing for the consistency model")
> >> > Reported-by: Joe Lawrence <joe.lawrence@redhat.com>
> >> > Signed-off-by: Nicolai Stange <nstange@suse.de>
> >> > Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> >> > ---
> >> > arch/powerpc/Kconfig | 2 +-
> >> > arch/powerpc/kernel/entry_64.S | 7 +++++++
> >> > 2 files changed, 8 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> >> > index 2890d36eb531..73bf87b1d274 100644
> >> > --- a/arch/powerpc/Kconfig
> >> > +++ b/arch/powerpc/Kconfig
> >> > @@ -220,7 +220,7 @@ config PPC
> >> > select HAVE_PERF_USER_STACK_DUMP
> >> > select HAVE_RCU_TABLE_FREE if SMP
> >> > select HAVE_REGS_AND_STACK_ACCESS_API
> >> > - select HAVE_RELIABLE_STACKTRACE if PPC64 && CPU_LITTLE_ENDIAN
> >> > + select HAVE_RELIABLE_STACKTRACE if PPC_BOOK3S_64 && CPU_LITTLE_ENDIAN
> >> > select HAVE_SYSCALL_TRACEPOINTS
> >> > select HAVE_VIRT_CPU_ACCOUNTING
> >> > select HAVE_IRQ_TIME_ACCOUNTING
> >> > diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> >> > index 435927f549c4..a2c168b395d2 100644
> >> > --- a/arch/powerpc/kernel/entry_64.S
> >> > +++ b/arch/powerpc/kernel/entry_64.S
> >> > @@ -1002,6 +1002,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> >> > ld r2,_NIP(r1)
> >> > mtspr SPRN_SRR0,r2
> >> >
> >> > + /*
> >> > + * Leaving a stale exception_marker on the stack can confuse
> >> > + * the reliable stack unwinder later on. Clear it.
> >> > + */
> >> > + li r2,0
> >> > + std r2,STACK_FRAME_OVERHEAD-16(r1)
> >> > +
> >>
> >> Could you please double check, r4 is already 0 at this point
> >> IIUC. So the change might be a simple
> >>
> >> std r4,STACK_FRAME_OVERHEAD-16(r1)
> >>
> >
> > r4 is not 0, sorry for the noise
>
>
Isn't it?
It is, I seem to be reading the wrong bits and confused myself, had to
re-read mtmsrd to ensure it does not modify RS, just MSR. So I guess
we could reuse r4. Should I send a patch on top of this? I have
limited testing infrastructure at the moment, I could use qemu
Balbir Singh.
^ permalink raw reply
* Re: [PATCH 00/19] KVM: PPC: Book3S HV: add XIVE native exploitation mode
From: David Gibson @ 2019-02-06 1:18 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm, kvm-ppc, Cédric Le Goater, linuxppc-dev
In-Reply-To: <20190205221315.GB29038@blackberry>
[-- Attachment #1: Type: text/plain, Size: 1547 bytes --]
On Wed, Feb 06, 2019 at 09:13:15AM +1100, Paul Mackerras wrote:
> On Tue, Feb 05, 2019 at 12:31:28PM +0100, Cédric Le Goater wrote:
> > >>> As for nesting, I suggest for the foreseeable future we stick to XICS
> > >>> emulation in nested guests.
> > >>
> > >> ok. so no kernel_irqchip at all. hmm.
> >
> > I was confused with what Paul calls 'XICS emulation'. It's not the QEMU
> > XICS emulated device but the XICS-over-XIVE KVM device, the KVM XICS
> > device KVM uses when under a P9 processor.
>
> Actually there are two separate implementations of XICS emulation in
> KVM. The first (older) one is almost entirely a software emulation
> but does have some cases where it accesses an underlying XICS device
> in order to make some things faster (IPIs and pass-through of a device
> interrupt to a guest). The other, newer one is the XICS-on-XIVE
> emulation that Ben wrote, which uses the XIVE hardware pretty heavily.
> My patch was about making the the older code work when there is no
> XICS available to the host.
Ah, right. To clarify my earlier statements in light of this:
* We definitely want some sort of kernel-XICS available in a nested
guest. AIUI, this is now accomplished, so, Yay!
* Implementing the L2 XICS in terms of L1's PAPR-XIVE would be a
bonus, but it's a much lower priority.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 15/19] KVM: PPC: Book3S HV: add get/set accessors for the source configuration
From: David Gibson @ 2019-02-06 1:24 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190206012329.GQ22661@umbus.fritz.box>
[-- Attachment #1: Type: text/plain, Size: 4277 bytes --]
On Wed, Feb 06, 2019 at 12:23:29PM +1100, David Gibson wrote:
> On Tue, Feb 05, 2019 at 02:03:11PM +0100, Cédric Le Goater wrote:
> > On 2/5/19 6:32 AM, David Gibson wrote:
> > > On Mon, Feb 04, 2019 at 05:07:28PM +0100, Cédric Le Goater wrote:
> > >> On 2/4/19 6:21 AM, David Gibson wrote:
> > >>> On Mon, Jan 07, 2019 at 07:43:27PM +0100, Cédric Le Goater wrote:
> > >>>> Theses are use to capure the XIVE EAS table of the KVM device, the
> > >>>> configuration of the source targets.
> > >>>>
> > >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > >>>> ---
> > >>>> arch/powerpc/include/uapi/asm/kvm.h | 11 ++++
> > >>>> arch/powerpc/kvm/book3s_xive_native.c | 87 +++++++++++++++++++++++++++
> > >>>> 2 files changed, 98 insertions(+)
> > >>>>
> > >>>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> > >>>> index 1a8740629acf..faf024f39858 100644
> > >>>> --- a/arch/powerpc/include/uapi/asm/kvm.h
> > >>>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> > >>>> @@ -683,9 +683,20 @@ struct kvm_ppc_cpu_char {
> > >>>> #define KVM_DEV_XIVE_SAVE_EQ_PAGES 4
> > >>>> #define KVM_DEV_XIVE_GRP_SOURCES 2 /* 64-bit source attributes */
> > >>>> #define KVM_DEV_XIVE_GRP_SYNC 3 /* 64-bit source attributes */
> > >>>> +#define KVM_DEV_XIVE_GRP_EAS 4 /* 64-bit eas attributes */
> > >>>>
> > >>>> /* Layout of 64-bit XIVE source attribute values */
> > >>>> #define KVM_XIVE_LEVEL_SENSITIVE (1ULL << 0)
> > >>>> #define KVM_XIVE_LEVEL_ASSERTED (1ULL << 1)
> > >>>>
> > >>>> +/* Layout of 64-bit eas attribute values */
> > >>>> +#define KVM_XIVE_EAS_PRIORITY_SHIFT 0
> > >>>> +#define KVM_XIVE_EAS_PRIORITY_MASK 0x7
> > >>>> +#define KVM_XIVE_EAS_SERVER_SHIFT 3
> > >>>> +#define KVM_XIVE_EAS_SERVER_MASK 0xfffffff8ULL
> > >>>> +#define KVM_XIVE_EAS_MASK_SHIFT 32
> > >>>> +#define KVM_XIVE_EAS_MASK_MASK 0x100000000ULL
> > >>>> +#define KVM_XIVE_EAS_EISN_SHIFT 33
> > >>>> +#define KVM_XIVE_EAS_EISN_MASK 0xfffffffe00000000ULL
> > >>>> +
> > >>>> #endif /* __LINUX_KVM_POWERPC_H */
> > >>>> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> > >>>> index f2de1bcf3b35..0468b605baa7 100644
> > >>>> --- a/arch/powerpc/kvm/book3s_xive_native.c
> > >>>> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> > >>>> @@ -525,6 +525,88 @@ static int kvmppc_xive_native_sync(struct kvmppc_xive *xive, long irq, u64 addr)
> > >>>> return 0;
> > >>>> }
> > >>>>
> > >>>> +static int kvmppc_xive_native_set_eas(struct kvmppc_xive *xive, long irq,
> > >>>> + u64 addr)
> > >>>
> > >>> I'd prefer to avoid the name "EAS" here. IIUC these aren't "raw" EAS
> > >>> values, but rather essentially the "source config" in the terminology
> > >>> of the PAPR hcalls. Which, yes, is basically implemented by setting
> > >>> the EAS, but since it's the PAPR architected state that we need to
> > >>> preserve across migration, I'd prefer to stick as close as we can to
> > >>> the PAPR terminology.
> > >>
> > >> But we don't have an equivalent name in the PAPR specs for the tuple
> > >> (prio, server). We could use the generic 'target' name may be ? even
> > >> if this is usually referring to a CPU number.
> > >
> > > Um.. what? That's about terminology for one of the fields in this
> > > thing, not about the name for the thing itself.
> > >
> > >> Or, IVE (Interrupt Vector Entry) ? which makes some sense.
> > >> This is was the former name in HW. I think we recycle it for KVM.
> > >
> > > That's a terrible idea, which will make a confusing situation even
> > > more confusing.
> >
> > Let's use SOURCE_CONFIG and QUEUE_CONFIG. The KVM ioctls are very
> > similar to the hcalls anyhow.
>
> Yes, I think that's a good idea.
Actually... AIUI the SET_CONFIG hcalls shouldn't be a fast path. Can
we simplify things further by removing the hcall implementation from
the kernel entirely, and have qemu implement them by basically just
forwarding them to the appropriate SET_CONFIG ioctl()?
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 06/19] KVM: PPC: Book3S HV: add a GET_ESB_FD control to the XIVE native device
From: David Gibson @ 2019-02-06 1:23 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <4d565738-a99b-0333-8533-037677358faa@kaod.org>
[-- Attachment #1: Type: text/plain, Size: 2877 bytes --]
On Tue, Feb 05, 2019 at 01:55:40PM +0100, Cédric Le Goater wrote:
> On 2/5/19 6:28 AM, David Gibson wrote:
> > On Mon, Feb 04, 2019 at 12:30:39PM +0100, Cédric Le Goater wrote:
> >> On 2/4/19 5:45 AM, David Gibson wrote:
> >>> On Mon, Jan 07, 2019 at 07:43:18PM +0100, Cédric Le Goater wrote:
> >>>> This will let the guest create a memory mapping to expose the ESB MMIO
> >>>> regions used to control the interrupt sources, to trigger events, to
> >>>> EOI or to turn off the sources.
> >>>>
> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >>>> ---
> >>>> arch/powerpc/include/uapi/asm/kvm.h | 4 ++
> >>>> arch/powerpc/kvm/book3s_xive_native.c | 97 +++++++++++++++++++++++++++
> >>>> 2 files changed, 101 insertions(+)
> >>>>
> >>>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> >>>> index 8c876c166ef2..6bb61ba141c2 100644
> >>>> --- a/arch/powerpc/include/uapi/asm/kvm.h
> >>>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> >>>> @@ -675,4 +675,8 @@ struct kvm_ppc_cpu_char {
> >>>> #define KVM_XICS_PRESENTED (1ULL << 43)
> >>>> #define KVM_XICS_QUEUED (1ULL << 44)
> >>>>
> >>>> +/* POWER9 XIVE Native Interrupt Controller */
> >>>> +#define KVM_DEV_XIVE_GRP_CTRL 1
> >>>> +#define KVM_DEV_XIVE_GET_ESB_FD 1
> >>>
> >>> Introducing a new FD for ESB and TIMA seems overkill. Can't you get
> >>> to both with an mmap() directly on the xive device fd? Using the
> >>> offset to distinguish which one to map, obviously.
> >>
> >> The page offset would define some sort of user API. It seems feasible.
> >> But I am not sure this would be practical in the future if we need to
> >> tune the length.
> >
> > Um.. why not? I mean, yes the XIVE supports rather a lot of
> > interrupts, but we have 64-bits of offset we can play with - we can
> > leave room for billions of ESB slots and still have room for billions
> > of VPs.
>
> So the first 4 pages could be the TIMA pages and then would come
> the pages for the interrupt ESBs. I think that we can have different
> vm_fault handler for each mapping.
Um.. no, I'm saying you don't need to tightly pack them. So you could
have the ESB pages at 0, the TIMA at, say offset 2^60.
> I wonder how this will work out with pass-through. As Paul said in
> a previous email, it would be better to let QEMU request a new
> mapping to handle the ESB pages of the device being passed through.
> I guess this is not a special case, just another offset and length.
Right, if we need multiple "chunks" of ESB pages we can given them
each their own terabyte or several. No need to be stingy with address
space.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 15/19] KVM: PPC: Book3S HV: add get/set accessors for the source configuration
From: David Gibson @ 2019-02-06 1:23 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <c0faf4dd-7af1-d97a-f8f1-838fcf412ab8@kaod.org>
[-- Attachment #1: Type: text/plain, Size: 3798 bytes --]
On Tue, Feb 05, 2019 at 02:03:11PM +0100, Cédric Le Goater wrote:
> On 2/5/19 6:32 AM, David Gibson wrote:
> > On Mon, Feb 04, 2019 at 05:07:28PM +0100, Cédric Le Goater wrote:
> >> On 2/4/19 6:21 AM, David Gibson wrote:
> >>> On Mon, Jan 07, 2019 at 07:43:27PM +0100, Cédric Le Goater wrote:
> >>>> Theses are use to capure the XIVE EAS table of the KVM device, the
> >>>> configuration of the source targets.
> >>>>
> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >>>> ---
> >>>> arch/powerpc/include/uapi/asm/kvm.h | 11 ++++
> >>>> arch/powerpc/kvm/book3s_xive_native.c | 87 +++++++++++++++++++++++++++
> >>>> 2 files changed, 98 insertions(+)
> >>>>
> >>>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> >>>> index 1a8740629acf..faf024f39858 100644
> >>>> --- a/arch/powerpc/include/uapi/asm/kvm.h
> >>>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> >>>> @@ -683,9 +683,20 @@ struct kvm_ppc_cpu_char {
> >>>> #define KVM_DEV_XIVE_SAVE_EQ_PAGES 4
> >>>> #define KVM_DEV_XIVE_GRP_SOURCES 2 /* 64-bit source attributes */
> >>>> #define KVM_DEV_XIVE_GRP_SYNC 3 /* 64-bit source attributes */
> >>>> +#define KVM_DEV_XIVE_GRP_EAS 4 /* 64-bit eas attributes */
> >>>>
> >>>> /* Layout of 64-bit XIVE source attribute values */
> >>>> #define KVM_XIVE_LEVEL_SENSITIVE (1ULL << 0)
> >>>> #define KVM_XIVE_LEVEL_ASSERTED (1ULL << 1)
> >>>>
> >>>> +/* Layout of 64-bit eas attribute values */
> >>>> +#define KVM_XIVE_EAS_PRIORITY_SHIFT 0
> >>>> +#define KVM_XIVE_EAS_PRIORITY_MASK 0x7
> >>>> +#define KVM_XIVE_EAS_SERVER_SHIFT 3
> >>>> +#define KVM_XIVE_EAS_SERVER_MASK 0xfffffff8ULL
> >>>> +#define KVM_XIVE_EAS_MASK_SHIFT 32
> >>>> +#define KVM_XIVE_EAS_MASK_MASK 0x100000000ULL
> >>>> +#define KVM_XIVE_EAS_EISN_SHIFT 33
> >>>> +#define KVM_XIVE_EAS_EISN_MASK 0xfffffffe00000000ULL
> >>>> +
> >>>> #endif /* __LINUX_KVM_POWERPC_H */
> >>>> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> >>>> index f2de1bcf3b35..0468b605baa7 100644
> >>>> --- a/arch/powerpc/kvm/book3s_xive_native.c
> >>>> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> >>>> @@ -525,6 +525,88 @@ static int kvmppc_xive_native_sync(struct kvmppc_xive *xive, long irq, u64 addr)
> >>>> return 0;
> >>>> }
> >>>>
> >>>> +static int kvmppc_xive_native_set_eas(struct kvmppc_xive *xive, long irq,
> >>>> + u64 addr)
> >>>
> >>> I'd prefer to avoid the name "EAS" here. IIUC these aren't "raw" EAS
> >>> values, but rather essentially the "source config" in the terminology
> >>> of the PAPR hcalls. Which, yes, is basically implemented by setting
> >>> the EAS, but since it's the PAPR architected state that we need to
> >>> preserve across migration, I'd prefer to stick as close as we can to
> >>> the PAPR terminology.
> >>
> >> But we don't have an equivalent name in the PAPR specs for the tuple
> >> (prio, server). We could use the generic 'target' name may be ? even
> >> if this is usually referring to a CPU number.
> >
> > Um.. what? That's about terminology for one of the fields in this
> > thing, not about the name for the thing itself.
> >
> >> Or, IVE (Interrupt Vector Entry) ? which makes some sense.
> >> This is was the former name in HW. I think we recycle it for KVM.
> >
> > That's a terrible idea, which will make a confusing situation even
> > more confusing.
>
> Let's use SOURCE_CONFIG and QUEUE_CONFIG. The KVM ioctls are very
> similar to the hcalls anyhow.
Yes, I think that's a good idea.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 17/19] KVM: PPC: Book3S HV: add get/set accessors for the VP XIVE state
From: David Gibson @ 2019-02-06 1:19 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <5e18629d-8aaf-1e20-46a1-64095da18bfb@kaod.org>
[-- Attachment #1: Type: text/plain, Size: 4284 bytes --]
On Tue, Feb 05, 2019 at 12:58:54PM +0100, Cédric Le Goater wrote:
> On 2/5/19 6:33 AM, David Gibson wrote:
> > On Mon, Feb 04, 2019 at 07:57:26PM +0100, Cédric Le Goater wrote:
> >> On 2/4/19 6:26 AM, David Gibson wrote:
> >>> On Mon, Jan 07, 2019 at 08:10:04PM +0100, Cédric Le Goater wrote:
> >>>> At a VCPU level, the state of the thread context interrupt management
> >>>> registers needs to be collected. These registers are cached under the
> >>>> 'xive_saved_state.w01' field of the VCPU when the VPCU context is
> >>>> pulled from the HW thread. An OPAL call retrieves the backup of the
> >>>> IPB register in the NVT structure and merges it in the KVM state.
> >>>>
> >>>> The structures of the interface between QEMU and KVM provisions some
> >>>> extra room (two u64) for further extensions if more state needs to be
> >>>> transferred back to QEMU.
> >>>>
> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >>>> ---
> >>>> arch/powerpc/include/asm/kvm_ppc.h | 5 ++
> >>>> arch/powerpc/include/uapi/asm/kvm.h | 2 +
> >>>> arch/powerpc/kvm/book3s.c | 24 +++++++++
> >>>> arch/powerpc/kvm/book3s_xive_native.c | 78 +++++++++++++++++++++++++++
> >>>> 4 files changed, 109 insertions(+)
> >>>>
> >>>> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> >>>> index 4cc897039485..49c488af168c 100644
> >>>> --- a/arch/powerpc/include/asm/kvm_ppc.h
> >>>> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> >>>> @@ -270,6 +270,7 @@ union kvmppc_one_reg {
> >>>> u64 addr;
> >>>> u64 length;
> >>>> } vpaval;
> >>>> + u64 xive_timaval[4];
> >>>> };
> >>>>
> >>>> struct kvmppc_ops {
> >>>> @@ -603,6 +604,8 @@ extern void kvmppc_xive_native_cleanup_vcpu(struct kvm_vcpu *vcpu);
> >>>> extern void kvmppc_xive_native_init_module(void);
> >>>> extern void kvmppc_xive_native_exit_module(void);
> >>>> extern int kvmppc_xive_native_hcall(struct kvm_vcpu *vcpu, u32 cmd);
> >>>> +extern int kvmppc_xive_native_get_vp(struct kvm_vcpu *vcpu, union kvmppc_one_reg *val);
> >>>> +extern int kvmppc_xive_native_set_vp(struct kvm_vcpu *vcpu, union kvmppc_one_reg *val);
> >>>>
> >>>> #else
> >>>> static inline int kvmppc_xive_set_xive(struct kvm *kvm, u32 irq, u32 server,
> >>>> @@ -637,6 +640,8 @@ static inline void kvmppc_xive_native_init_module(void) { }
> >>>> static inline void kvmppc_xive_native_exit_module(void) { }
> >>>> static inline int kvmppc_xive_native_hcall(struct kvm_vcpu *vcpu, u32 cmd)
> >>>> { return 0; }
> >>>> +static inline int kvmppc_xive_native_get_vp(struct kvm_vcpu *vcpu, union kvmppc_one_reg *val) { return 0; }
> >>>> +static inline int kvmppc_xive_native_set_vp(struct kvm_vcpu *vcpu, union kvmppc_one_reg *val) { return -ENOENT; }
> >>>
> >>> IIRC "VP" is the old name for "TCTX". Since we're using tctx in the
> >>> rest of the XIVE code, can we use it here as well.
> >>
> >> OK. The state we are getting or setting is indeed related to the thread
> >> interrupt context registers.
> >>
> >> The name VP is related to an identifier to some interrupt context under
> >> OPAL (NVT in HW to be precise).
> >
> > Oh, sorry, "NVT" was the name I was looking for, not "TCTX". But in
> > any case, please lets standardize on one.
>
> There is some confusion in the naming for :
>
> - VP Virtual Processor (XIVE 1)
> - VPD Virtual Processor Descriptor (XIVE 1)
> - TCTX Thread interrupt context registers
> - NVT Notify Virtual Target. Former VP.
> - NVTS Notify Virtual Target Structure. Where the TCTX regs are cached.
>
>
> I am fine with using NVT because this is indeed the name of the XIVE
> structure where the HW caches the thread interrupt context registers.
>
> But the XIVE native layer and the XICS-over-XIVE KVM device use the
> name VP (the old one). I don't think we want to change these now.
Ah, right. It now occurs to me that the place I've already seen NVT
used is in the qemu code, whereas this is kernel. In that case
sticking to VP here makes sense.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] mtd: powernv: SPDX and comment fixups
From: Stewart Smith @ 2019-02-06 1:08 UTC (permalink / raw)
To: Joel Stanley, David Woodhouse, Brian Norris, Boris Brezillon,
Marek Vasut, Richard Weinberger
Cc: linuxppc-dev, linux-mtd
In-Reply-To: <20190206003659.27107-1-joel@jms.id.au>
Joel Stanley <joel@jms.id.au> writes:
> converts the powernv flash driver to use SPDX, and adds some
> clarifying comments that came out of a discussion on how the mtd driver
> works.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
We probably don't need to mention the dim dark corners of the FFS format
and I kind of wish it'd die rather than spread further.
Reviewed-by: Stewart Smith <stewart@linux.ibm.com>
--
Stewart Smith
OPAL Architect, IBM.
^ permalink raw reply
* [PATCH] mtd: powernv: SPDX and comment fixups
From: Joel Stanley @ 2019-02-06 0:36 UTC (permalink / raw)
To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut,
Richard Weinberger
Cc: linux-mtd, linuxppc-dev
This converts the powernv flash driver to use SPDX, and adds some
clarifying comments that came out of a discussion on how the mtd driver
works.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/mtd/devices/powernv_flash.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/devices/powernv_flash.c b/drivers/mtd/devices/powernv_flash.c
index 22f753e555ac..0bf43336c3f7 100644
--- a/drivers/mtd/devices/powernv_flash.c
+++ b/drivers/mtd/devices/powernv_flash.c
@@ -1,17 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0+
+
/*
* OPAL PNOR flash MTD abstraction
*
* Copyright IBM 2015
- *
- * 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 your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
*/
#include <linux/kernel.h>
@@ -261,6 +253,14 @@ static int powernv_flash_probe(struct platform_device *pdev)
* The current flash that skiboot exposes is one contiguous flash chip
* with an ffs partition at the start, it should prove easier for users
* to deal with partitions or not as they see fit
+ *
+ * When developing the skiboot MTD driver an experiment with FFS
+ * parsing in the kernel, and exposing a seperate /dev/mtdX for each
+ * partition (eg BOOTKERNEL, PAYLOAD, NVRAM, etc), was done.
+ *
+ * We didn't go with that as it meant users couldn't do a full flash
+ * re-write, as this can cause a partition to change size, and there
+ * wasn't a way to tell the MTD layer that a device has shrunk/grown.
*/
return mtd_device_register(&data->mtd, NULL, 0);
}
--
2.20.1
^ permalink raw reply related
* [PATCH net] net: fsl_ucc_hdlc: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-05 16:14 UTC (permalink / raw)
To: netdev, linuxppc-dev; +Cc: yang.wei9, albin_yang, davem, qiang.zhao
From: Yang Wei <yang.wei9@zte.com.cn>
dev_consume_skb_irq() should be called in hdlc_tx_done() when skb
xmit done. It makes drop profiles(dropwatch, perf) more friendly.
Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
drivers/net/wan/fsl_ucc_hdlc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 66d889d..a08f04c 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -482,7 +482,7 @@ static int hdlc_tx_done(struct ucc_hdlc_private *priv)
memset(priv->tx_buffer +
(be32_to_cpu(bd->buf) - priv->dma_tx_addr),
0, skb->len);
- dev_kfree_skb_irq(skb);
+ dev_consume_skb_irq(skb);
priv->tx_skbuff[priv->skb_dirtytx] = NULL;
priv->skb_dirtytx =
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 00/19] KVM: PPC: Book3S HV: add XIVE native exploitation mode
From: Paul Mackerras @ 2019-02-05 22:13 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <dc59257b-c780-7b81-f3d8-46269b834c2d@kaod.org>
On Tue, Feb 05, 2019 at 12:31:28PM +0100, Cédric Le Goater wrote:
> >>> As for nesting, I suggest for the foreseeable future we stick to XICS
> >>> emulation in nested guests.
> >>
> >> ok. so no kernel_irqchip at all. hmm.
>
> I was confused with what Paul calls 'XICS emulation'. It's not the QEMU
> XICS emulated device but the XICS-over-XIVE KVM device, the KVM XICS
> device KVM uses when under a P9 processor.
Actually there are two separate implementations of XICS emulation in
KVM. The first (older) one is almost entirely a software emulation
but does have some cases where it accesses an underlying XICS device
in order to make some things faster (IPIs and pass-through of a device
interrupt to a guest). The other, newer one is the XICS-on-XIVE
emulation that Ben wrote, which uses the XIVE hardware pretty heavily.
My patch was about making the the older code work when there is no
XICS available to the host.
Paul.
^ permalink raw reply
* [PATCH v2] powerpc/mm: move a KERN_WARNING message to pr_debug()
From: Laurent Vivier @ 2019-02-05 20:21 UTC (permalink / raw)
To: linux-kernel; +Cc: linuxppc-dev, David Gibson
resize_hpt_for_hotplug() reports a warning when it cannot
increase the hash page table ("Unable to resize hash page
table to target order") but this is not blocking and
can make user thinks something has not worked properly.
As we move the message to the debug area, report again the
ENODEV error.
If the operation cannot be done the real error message
will be reported by arch_add_memory() if create_section_mapping()
fails.
Fixes: 7339390d772dd
powerpc/pseries: Don't give a warning when HPT resizing isn't available
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
Notes:
v2:
- use pr_debug instead of printk(KERN_DEBUG
- remove check for ENODEV
arch/powerpc/mm/hash_utils_64.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index 0cc7fbc3bd1c..6a0cc4eb2c83 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -777,10 +777,9 @@ void resize_hpt_for_hotplug(unsigned long new_mem_size)
int rc;
rc = mmu_hash_ops.resize_hpt(target_hpt_shift);
- if (rc && (rc != -ENODEV))
- printk(KERN_WARNING
- "Unable to resize hash page table to target order %d: %d\n",
- target_hpt_shift, rc);
+ if (rc)
+ pr_debug("Unable to resize hash page table to target order %d: %d\n",
+ target_hpt_shift, rc);
}
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] powerpc/mm: move a KERN_WARNING message to KERN_DEBUG
From: Christophe Leroy @ 2019-02-05 18:11 UTC (permalink / raw)
To: Laurent Vivier, linux-kernel; +Cc: linuxppc-dev, David Gibson
In-Reply-To: <20190205180329.1452-1-lvivier@redhat.com>
Le 05/02/2019 à 19:03, Laurent Vivier a écrit :
> resize_hpt_for_hotplug() reports a warning when it cannot
> increase the hash page table ("Unable to resize hash page
> table to target order") but this is not blocking and
> can make user thinks something has not worked properly.
>
> If the operation cannot be done the real error message
> will be reported by arch_add_memory() if create_section_mapping()
> fails.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> arch/powerpc/mm/hash_utils_64.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
> index 0cc7fbc3bd1c..b762bdceb510 100644
> --- a/arch/powerpc/mm/hash_utils_64.c
> +++ b/arch/powerpc/mm/hash_utils_64.c
> @@ -778,7 +778,7 @@ void resize_hpt_for_hotplug(unsigned long new_mem_size)
>
> rc = mmu_hash_ops.resize_hpt(target_hpt_shift);
> if (rc && (rc != -ENODEV))
> - printk(KERN_WARNING
> + printk(KERN_DEBUG
You should use pr_debug() instead.
Christophe
> "Unable to resize hash page table to target order %d: %d\n",
> target_hpt_shift, rc);
> }
>
^ permalink raw reply
* Re: [PATCH 16/19] KVM: PPC: Book3S HV: add get/set accessors for the EQ configuration
From: Cédric Le Goater @ 2019-02-05 17:45 UTC (permalink / raw)
To: David Gibson; +Cc: kvm, kvm-ppc, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190204052436.GI1927@umbus.fritz.box>
On 2/4/19 6:24 AM, David Gibson wrote:
> On Mon, Jan 07, 2019 at 07:43:28PM +0100, Cédric Le Goater wrote:
>> These are used to capture the XIVE END table of the KVM device. It
>> relies on an OPAL call to retrieve from the XIVE IC the EQ toggle bit
>> and index which are updated by the HW when events are enqueued in the
>> guest RAM.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> arch/powerpc/include/uapi/asm/kvm.h | 21 ++++
>> arch/powerpc/kvm/book3s_xive_native.c | 166 ++++++++++++++++++++++++++
>> 2 files changed, 187 insertions(+)
>>
>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
>> index faf024f39858..95302558ce10 100644
>> --- a/arch/powerpc/include/uapi/asm/kvm.h
>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
>> @@ -684,6 +684,7 @@ struct kvm_ppc_cpu_char {
>> #define KVM_DEV_XIVE_GRP_SOURCES 2 /* 64-bit source attributes */
>> #define KVM_DEV_XIVE_GRP_SYNC 3 /* 64-bit source attributes */
>> #define KVM_DEV_XIVE_GRP_EAS 4 /* 64-bit eas attributes */
>> +#define KVM_DEV_XIVE_GRP_EQ 5 /* 64-bit eq attributes */
>>
>> /* Layout of 64-bit XIVE source attribute values */
>> #define KVM_XIVE_LEVEL_SENSITIVE (1ULL << 0)
>> @@ -699,4 +700,24 @@ struct kvm_ppc_cpu_char {
>> #define KVM_XIVE_EAS_EISN_SHIFT 33
>> #define KVM_XIVE_EAS_EISN_MASK 0xfffffffe00000000ULL
>>
>> +/* Layout of 64-bit eq attribute */
>> +#define KVM_XIVE_EQ_PRIORITY_SHIFT 0
>> +#define KVM_XIVE_EQ_PRIORITY_MASK 0x7
>> +#define KVM_XIVE_EQ_SERVER_SHIFT 3
>> +#define KVM_XIVE_EQ_SERVER_MASK 0xfffffff8ULL
>> +
>> +/* Layout of 64-bit eq attribute values */
>> +struct kvm_ppc_xive_eq {
>> + __u32 flags;
>> + __u32 qsize;
>> + __u64 qpage;
>> + __u32 qtoggle;
>> + __u32 qindex;
>
> Should we pad this in case a) we discover some fields in the EQ that
> we thought weren't relevant to the guest actually are or b) future
> XIVE extensions add something we need to migrate.
The underlying XIVE structure is composed of 32bytes. I will double the
size.
Thanks,
C.
>
>> +};
>> +
>> +#define KVM_XIVE_EQ_FLAG_ENABLED 0x00000001
>> +#define KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY 0x00000002
>> +#define KVM_XIVE_EQ_FLAG_ESCALATE 0x00000004
>> +
>> +
>> #endif /* __LINUX_KVM_POWERPC_H */
>> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
>> index 0468b605baa7..f4eb71eafc57 100644
>> --- a/arch/powerpc/kvm/book3s_xive_native.c
>> +++ b/arch/powerpc/kvm/book3s_xive_native.c
>> @@ -607,6 +607,164 @@ static int kvmppc_xive_native_get_eas(struct kvmppc_xive *xive, long irq,
>> return 0;
>> }
>>
>> +static int kvmppc_xive_native_set_queue(struct kvmppc_xive *xive, long eq_idx,
>> + u64 addr)
>> +{
>> + struct kvm *kvm = xive->kvm;
>> + struct kvm_vcpu *vcpu;
>> + struct kvmppc_xive_vcpu *xc;
>> + void __user *ubufp = (u64 __user *) addr;
>> + u32 server;
>> + u8 priority;
>> + struct kvm_ppc_xive_eq kvm_eq;
>> + int rc;
>> + __be32 *qaddr = 0;
>> + struct page *page;
>> + struct xive_q *q;
>> +
>> + /*
>> + * Demangle priority/server tuple from the EQ index
>> + */
>> + priority = (eq_idx & KVM_XIVE_EQ_PRIORITY_MASK) >>
>> + KVM_XIVE_EQ_PRIORITY_SHIFT;
>> + server = (eq_idx & KVM_XIVE_EQ_SERVER_MASK) >>
>> + KVM_XIVE_EQ_SERVER_SHIFT;
>> +
>> + if (copy_from_user(&kvm_eq, ubufp, sizeof(kvm_eq)))
>> + return -EFAULT;
>> +
>> + vcpu = kvmppc_xive_find_server(kvm, server);
>> + if (!vcpu) {
>> + pr_err("Can't find server %d\n", server);
>> + return -ENOENT;
>> + }
>> + xc = vcpu->arch.xive_vcpu;
>> +
>> + if (priority != xive_prio_from_guest(priority)) {
>> + pr_err("Trying to restore invalid queue %d for VCPU %d\n",
>> + priority, server);
>> + return -EINVAL;
>> + }
>> + q = &xc->queues[priority];
>> +
>> + pr_devel("%s VCPU %d priority %d fl:%x sz:%d addr:%llx g:%d idx:%d\n",
>> + __func__, server, priority, kvm_eq.flags,
>> + kvm_eq.qsize, kvm_eq.qpage, kvm_eq.qtoggle, kvm_eq.qindex);
>> +
>> + rc = xive_native_validate_queue_size(kvm_eq.qsize);
>> + if (rc || !kvm_eq.qsize) {
>> + pr_err("invalid queue size %d\n", kvm_eq.qsize);
>> + return rc;
>> + }
>> +
>> + page = gfn_to_page(kvm, gpa_to_gfn(kvm_eq.qpage));
>> + if (is_error_page(page)) {
>> + pr_warn("Couldn't get guest page for %llx!\n", kvm_eq.qpage);
>> + return -ENOMEM;
>> + }
>> + qaddr = page_to_virt(page) + (kvm_eq.qpage & ~PAGE_MASK);
>> +
>> + /* Backup queue page guest address for migration */
>> + q->guest_qpage = kvm_eq.qpage;
>> + q->guest_qsize = kvm_eq.qsize;
>> +
>> + rc = xive_native_configure_queue(xc->vp_id, q, priority,
>> + (__be32 *) qaddr, kvm_eq.qsize, true);
>> + if (rc) {
>> + pr_err("Failed to configure queue %d for VCPU %d: %d\n",
>> + priority, xc->server_num, rc);
>> + put_page(page);
>> + return rc;
>> + }
>> +
>> + rc = xive_native_set_queue_state(xc->vp_id, priority, kvm_eq.qtoggle,
>> + kvm_eq.qindex);
>> + if (rc)
>> + goto error;
>> +
>> + rc = kvmppc_xive_attach_escalation(vcpu, priority);
>> +error:
>> + if (rc)
>> + xive_native_cleanup_queue(vcpu, priority);
>> + return rc;
>> +}
>> +
>> +static int kvmppc_xive_native_get_queue(struct kvmppc_xive *xive, long eq_idx,
>> + u64 addr)
>> +{
>> + struct kvm *kvm = xive->kvm;
>> + struct kvm_vcpu *vcpu;
>> + struct kvmppc_xive_vcpu *xc;
>> + struct xive_q *q;
>> + void __user *ubufp = (u64 __user *) addr;
>> + u32 server;
>> + u8 priority;
>> + struct kvm_ppc_xive_eq kvm_eq;
>> + u64 qpage;
>> + u64 qsize;
>> + u64 qeoi_page;
>> + u32 escalate_irq;
>> + u64 qflags;
>> + int rc;
>> +
>> + /*
>> + * Demangle priority/server tuple from the EQ index
>> + */
>> + priority = (eq_idx & KVM_XIVE_EQ_PRIORITY_MASK) >>
>> + KVM_XIVE_EQ_PRIORITY_SHIFT;
>> + server = (eq_idx & KVM_XIVE_EQ_SERVER_MASK) >>
>> + KVM_XIVE_EQ_SERVER_SHIFT;
>> +
>> + vcpu = kvmppc_xive_find_server(kvm, server);
>> + if (!vcpu) {
>> + pr_err("Can't find server %d\n", server);
>> + return -ENOENT;
>> + }
>> + xc = vcpu->arch.xive_vcpu;
>> +
>> + if (priority != xive_prio_from_guest(priority)) {
>> + pr_err("invalid priority for queue %d for VCPU %d\n",
>> + priority, server);
>> + return -EINVAL;
>> + }
>> + q = &xc->queues[priority];
>> +
>> + memset(&kvm_eq, 0, sizeof(kvm_eq));
>> +
>> + if (!q->qpage)
>> + return 0;
>> +
>> + rc = xive_native_get_queue_info(xc->vp_id, priority, &qpage, &qsize,
>> + &qeoi_page, &escalate_irq, &qflags);
>> + if (rc)
>> + return rc;
>> +
>> + kvm_eq.flags = 0;
>> + if (qflags & OPAL_XIVE_EQ_ENABLED)
>> + kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ENABLED;
>> + if (qflags & OPAL_XIVE_EQ_ALWAYS_NOTIFY)
>> + kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ALWAYS_NOTIFY;
>> + if (qflags & OPAL_XIVE_EQ_ESCALATE)
>> + kvm_eq.flags |= KVM_XIVE_EQ_FLAG_ESCALATE;
>> +
>> + kvm_eq.qsize = q->guest_qsize;
>> + kvm_eq.qpage = q->guest_qpage;
>> +
>> + rc = xive_native_get_queue_state(xc->vp_id, priority, &kvm_eq.qtoggle,
>> + &kvm_eq.qindex);
>> + if (rc)
>> + return rc;
>> +
>> + pr_devel("%s VCPU %d priority %d fl:%x sz:%d addr:%llx g:%d idx:%d\n",
>> + __func__, server, priority, kvm_eq.flags,
>> + kvm_eq.qsize, kvm_eq.qpage, kvm_eq.qtoggle, kvm_eq.qindex);
>> +
>> + if (copy_to_user(ubufp, &kvm_eq, sizeof(kvm_eq)))
>> + return -EFAULT;
>> +
>> + return 0;
>> +}
>> +
>> static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>> struct kvm_device_attr *attr)
>> {
>> @@ -628,6 +786,9 @@ static int kvmppc_xive_native_set_attr(struct kvm_device *dev,
>> return kvmppc_xive_native_sync(xive, attr->attr, attr->addr);
>> case KVM_DEV_XIVE_GRP_EAS:
>> return kvmppc_xive_native_set_eas(xive, attr->attr, attr->addr);
>> + case KVM_DEV_XIVE_GRP_EQ:
>> + return kvmppc_xive_native_set_queue(xive, attr->attr,
>> + attr->addr);
>> }
>> return -ENXIO;
>> }
>> @@ -650,6 +811,9 @@ static int kvmppc_xive_native_get_attr(struct kvm_device *dev,
>> break;
>> case KVM_DEV_XIVE_GRP_EAS:
>> return kvmppc_xive_native_get_eas(xive, attr->attr, attr->addr);
>> + case KVM_DEV_XIVE_GRP_EQ:
>> + return kvmppc_xive_native_get_queue(xive, attr->attr,
>> + attr->addr);
>> }
>> return -ENXIO;
>> }
>> @@ -674,6 +838,8 @@ static int kvmppc_xive_native_has_attr(struct kvm_device *dev,
>> attr->attr < KVMPPC_XIVE_NR_IRQS)
>> return 0;
>> break;
>> + case KVM_DEV_XIVE_GRP_EQ:
>> + return 0;
>> }
>> return -ENXIO;
>> }
>
^ permalink raw reply
* [PATCH] powerpc/mm: move a KERN_WARNING message to KERN_DEBUG
From: Laurent Vivier @ 2019-02-05 18:03 UTC (permalink / raw)
To: linux-kernel; +Cc: linuxppc-dev, David Gibson
resize_hpt_for_hotplug() reports a warning when it cannot
increase the hash page table ("Unable to resize hash page
table to target order") but this is not blocking and
can make user thinks something has not worked properly.
If the operation cannot be done the real error message
will be reported by arch_add_memory() if create_section_mapping()
fails.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
arch/powerpc/mm/hash_utils_64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index 0cc7fbc3bd1c..b762bdceb510 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -778,7 +778,7 @@ void resize_hpt_for_hotplug(unsigned long new_mem_size)
rc = mmu_hash_ops.resize_hpt(target_hpt_shift);
if (rc && (rc != -ENODEV))
- printk(KERN_WARNING
+ printk(KERN_DEBUG
"Unable to resize hash page table to target order %d: %d\n",
target_hpt_shift, rc);
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCHv6 1/4] dt-bindings: add DT binding for the layerscape PCIe controller with EP mode
From: Lorenzo Pieralisi @ 2019-02-05 18:02 UTC (permalink / raw)
To: Xiaowei Bao
Cc: mark.rutland, roy.zang, arnd, devicetree, gregkh, kstewart,
linuxppc-dev, linux-pci, linux-kernel, kishon, minghuan.Lian,
robh+dt, cyrille.pitchen, linux-arm-kernel, pombredanne, bhelgaas,
leoyang.li, shawnguo, shawn.lin, mingkai.hu
In-Reply-To: <20190122063328.25228-1-xiaowei.bao@nxp.com>
On Tue, Jan 22, 2019 at 02:33:25PM +0800, Xiaowei Bao wrote:
> Add the documentation for the Device Tree binding for the layerscape PCIe
> controller with EP mode.
>
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> Reviewed-by: Minghuan Lian <minghuan.lian@nxp.com>
> Reviewed-by: Zhiqiang Hou <zhiqiang.hou@nxp.com>
> Reviewed-by: Rob Herring <robh+dt@kernel.org>
> ---
> v2:
> - Add the SoC specific compatibles.
> v3:
> - modify the commit message.
> v4:
> - no change.
> v5:
> - no change.
> v6:
> - no change.
>
> .../devicetree/bindings/pci/layerscape-pci.txt | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
Applied the series to pci/layerscape for v5.1, thanks.
Lorenzo
> diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> index 9b2b8d6..e20ceaa 100644
> --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> @@ -13,6 +13,7 @@ information.
>
> Required properties:
> - compatible: should contain the platform identifier such as:
> + RC mode:
> "fsl,ls1021a-pcie"
> "fsl,ls2080a-pcie", "fsl,ls2085a-pcie"
> "fsl,ls2088a-pcie"
> @@ -20,6 +21,8 @@ Required properties:
> "fsl,ls1046a-pcie"
> "fsl,ls1043a-pcie"
> "fsl,ls1012a-pcie"
> + EP mode:
> + "fsl,ls1046a-pcie-ep", "fsl,ls-pcie-ep"
> - reg: base addresses and lengths of the PCIe controller register blocks.
> - interrupts: A list of interrupt outputs of the controller. Must contain an
> entry for each entry in the interrupt-names property.
> --
> 1.7.1
>
^ permalink raw reply
* Re: [PATCH v3 1/2] mm: add probe_user_read()
From: Murilo Opsfelder Araujo @ 2019-02-05 17:42 UTC (permalink / raw)
To: Christophe Leroy
Cc: Kees Cook, linux-kernel, Mike Rapoport, linux-mm, Paul Mackerras,
Andrew Morton, linuxppc-dev
In-Reply-To: <39fb6c5a191025378676492e140dc012915ecaeb.1547652372.git.christophe.leroy@c-s.fr>
Hi, Christophe.
On Wed, Jan 16, 2019 at 04:59:27PM +0000, Christophe Leroy wrote:
> In powerpc code, there are several places implementing safe
> access to user data. This is sometimes implemented using
> probe_kernel_address() with additional access_ok() verification,
> sometimes with get_user() enclosed in a pagefault_disable()/enable()
> pair, etc. :
> show_user_instructions()
> bad_stack_expansion()
> p9_hmi_special_emu()
> fsl_pci_mcheck_exception()
> read_user_stack_64()
> read_user_stack_32() on PPC64
> read_user_stack_32() on PPC32
> power_pmu_bhrb_to()
>
> In the same spirit as probe_kernel_read(), this patch adds
> probe_user_read().
>
> probe_user_read() does the same as probe_kernel_read() but
> first checks that it is really a user address.
>
> The patch defines this function as a static inline so the "size"
> variable can be examined for const-ness by the check_object_size()
> in __copy_from_user_inatomic()
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> v3: Moved 'Returns:" comment after description.
> Explained in the commit log why the function is defined static inline
>
> v2: Added "Returns:" comment and removed probe_user_address()
>
> include/linux/uaccess.h | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index 37b226e8df13..ef99edd63da3 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -263,6 +263,40 @@ extern long strncpy_from_unsafe(char *dst, const void *unsafe_addr, long count);
> #define probe_kernel_address(addr, retval) \
> probe_kernel_read(&retval, addr, sizeof(retval))
>
> +/**
> + * probe_user_read(): safely attempt to read from a user location
> + * @dst: pointer to the buffer that shall take the data
> + * @src: address to read from
> + * @size: size of the data chunk
> + *
> + * Safely read from address @src to the buffer at @dst. If a kernel fault
> + * happens, handle that and return -EFAULT.
> + *
> + * We ensure that the copy_from_user is executed in atomic context so that
> + * do_page_fault() doesn't attempt to take mmap_sem. This makes
> + * probe_user_read() suitable for use within regions where the caller
> + * already holds mmap_sem, or other locks which nest inside mmap_sem.
> + *
> + * Returns: 0 on success, -EFAULT on error.
> + */
> +
> +#ifndef probe_user_read
> +static __always_inline long probe_user_read(void *dst, const void __user *src,
> + size_t size)
> +{
> + long ret;
> +
> + if (!access_ok(src, size))
> + return -EFAULT;
Hopefully, there is still time for a minor comment.
Do we need to differentiate the returned error here, e.g.: return
-EACCES?
I wonder if there will be situations where callers need to know why
probe_user_read() failed.
Besides that:
Acked-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com>
> +
> + pagefault_disable();
> + ret = __copy_from_user_inatomic(dst, src, size);
> + pagefault_enable();
> +
> + return ret ? -EFAULT : 0;
> +}
> +#endif
> +
> #ifndef user_access_begin
> #define user_access_begin(ptr,len) access_ok(ptr, len)
> #define user_access_end() do { } while (0)
> --
> 2.13.3
>
--
Murilo
^ permalink raw reply
* Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
From: Michael S. Tsirkin @ 2019-02-05 16:13 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jean-Philippe Brucker, Jason Wang, Alexey Kardashevskiy, Ram Pai,
linux-kernel, virtualization, iommu, linuxppc-dev,
Thiago Jung Bauermann, David Gibson
In-Reply-To: <20190205072407.GA4311@lst.de>
On Tue, Feb 05, 2019 at 08:24:07AM +0100, Christoph Hellwig wrote:
> On Mon, Feb 04, 2019 at 04:38:21PM -0500, Michael S. Tsirkin wrote:
> > It was designed to make, when set, as many guests as we can work
> > correctly, and it seems to be successful in doing exactly that.
> >
> > Unfortunately there could be legacy guests that do work correctly but
> > become slow. Whether trying to somehow work around that
> > can paint us into a corner where things again don't
> > work for some people is a question worth discussing.
>
> The other problem is that some qemu machines just throw passthrough
> devices and virtio devices on the same virtual PCI(e) bus, and have a
> common IOMMU setup for the whole bus / root port / domain. I think
> this is completely bogus, but unfortunately it is out in the field.
>
> Given that power is one of these examples I suspect that is what
> Thiago referes to. But in this case the answer can't be that we
> pile on hack ontop of another, but instead introduce a new qemu
> machine that separates these clearly, and make that mandatory for
> the secure guest support.
That could we one approach, assuming one exists that guests
already support.
--
MST
^ permalink raw reply
* Re: [RFC PATCH] powerpc: fix get_arch_dma_ops() for NTB devices
From: Christoph Hellwig @ 2019-02-05 16:13 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, Christoph Hellwig, Alexander Fomichev, linux
In-Reply-To: <87h8dimqgv.fsf@concordia.ellerman.id.au>
On Tue, Feb 05, 2019 at 10:20:32PM +1100, Michael Ellerman wrote:
> get_dma_ops() falls into arch-dependant get_arch_dma_ops(), which
> historically returns NULL on PowerPC. Therefore dma_set_mask() fails.
> This affects Switchtec (and probably other) NTB devices, that they fail
> to initialize.
> The proposed patch should fix the issue.
I'm not a fan of this. powerpc, just like arm64 for example has
required that we set a specific per-device dma ops, which keeps
the assignments clear. Where do the NTB devices come from?
Might be worth adding the NTB maintainers to the CC list and maybe
linux-iommu.
^ permalink raw reply
* [PATCH 2/8] perf mem/c2c: Fix perf_mem_events to support powerpc
From: Arnaldo Carvalho de Melo @ 2019-02-05 15:07 UTC (permalink / raw)
To: Ingo Molnar
Cc: Don Zickus, Ravi Bangoria, Madhavan Srinivasan,
Arnaldo Carvalho de Melo, Joe Mario, Clark Williams, Dick Fowles,
linux-kernel, linux-perf-users, Jiri Olsa, Namhyung Kim,
linuxppc-dev
In-Reply-To: <20190205150708.9012-1-acme@kernel.org>
From: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
PowerPC hardware does not have a builtin latency filter (--ldlat) for
the "mem-load" event and perf_mem_events by default includes
"/ldlat=30/" which is causing a failure on PowerPC. Refactor the code to
support "perf mem/c2c" on PowerPC.
This patch depends on kernel side changes done my Madhavan:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-December/182596.html
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Dick Fowles <fowles@inreach.com>
Cc: Don Zickus <dzickus@redhat.com>
Cc: Joe Mario <jmario@redhat.com>
Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lkml.kernel.org/r/20190129132412.771-1-ravi.bangoria@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/Documentation/perf-c2c.txt | 16 ++++++++++++----
tools/perf/Documentation/perf-mem.txt | 2 +-
tools/perf/arch/powerpc/util/Build | 1 +
tools/perf/arch/powerpc/util/mem-events.c | 11 +++++++++++
tools/perf/util/mem-events.c | 2 +-
5 files changed, 26 insertions(+), 6 deletions(-)
create mode 100644 tools/perf/arch/powerpc/util/mem-events.c
diff --git a/tools/perf/Documentation/perf-c2c.txt b/tools/perf/Documentation/perf-c2c.txt
index 095aebdc5bb7..e6150f21267d 100644
--- a/tools/perf/Documentation/perf-c2c.txt
+++ b/tools/perf/Documentation/perf-c2c.txt
@@ -19,8 +19,11 @@ C2C stands for Cache To Cache.
The perf c2c tool provides means for Shared Data C2C/HITM analysis. It allows
you to track down the cacheline contentions.
-The tool is based on x86's load latency and precise store facility events
-provided by Intel CPUs. These events provide:
+On x86, the tool is based on load latency and precise store facility events
+provided by Intel CPUs. On PowerPC, the tool uses random instruction sampling
+with thresholding feature.
+
+These events provide:
- memory address of the access
- type of the access (load and store details)
- latency (in cycles) of the load access
@@ -46,7 +49,7 @@ RECORD OPTIONS
-l::
--ldlat::
- Configure mem-loads latency.
+ Configure mem-loads latency. (x86 only)
-k::
--all-kernel::
@@ -119,11 +122,16 @@ Following perf record options are configured by default:
-W,-d,--phys-data,--sample-cpu
Unless specified otherwise with '-e' option, following events are monitored by
-default:
+default on x86:
cpu/mem-loads,ldlat=30/P
cpu/mem-stores/P
+and following on PowerPC:
+
+ cpu/mem-loads/
+ cpu/mem-stores/
+
User can pass any 'perf record' option behind '--' mark, like (to enable
callchains and system wide monitoring):
diff --git a/tools/perf/Documentation/perf-mem.txt b/tools/perf/Documentation/perf-mem.txt
index f8d2167cf3e7..199ea0f0a6c0 100644
--- a/tools/perf/Documentation/perf-mem.txt
+++ b/tools/perf/Documentation/perf-mem.txt
@@ -82,7 +82,7 @@ RECORD OPTIONS
Be more verbose (show counter open errors, etc)
--ldlat <n>::
- Specify desired latency for loads event.
+ Specify desired latency for loads event. (x86 only)
In addition, for report all perf report options are valid, and for record
all perf record options.
diff --git a/tools/perf/arch/powerpc/util/Build b/tools/perf/arch/powerpc/util/Build
index 2e6595310420..ba98bd006488 100644
--- a/tools/perf/arch/powerpc/util/Build
+++ b/tools/perf/arch/powerpc/util/Build
@@ -2,6 +2,7 @@ libperf-y += header.o
libperf-y += sym-handling.o
libperf-y += kvm-stat.o
libperf-y += perf_regs.o
+libperf-y += mem-events.o
libperf-$(CONFIG_DWARF) += dwarf-regs.o
libperf-$(CONFIG_DWARF) += skip-callchain-idx.o
diff --git a/tools/perf/arch/powerpc/util/mem-events.c b/tools/perf/arch/powerpc/util/mem-events.c
new file mode 100644
index 000000000000..d08311f04e95
--- /dev/null
+++ b/tools/perf/arch/powerpc/util/mem-events.c
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "mem-events.h"
+
+/* PowerPC does not support 'ldlat' parameter. */
+char *perf_mem_events__name(int i)
+{
+ if (i == PERF_MEM_EVENTS__LOAD)
+ return (char *) "cpu/mem-loads/";
+
+ return (char *) "cpu/mem-stores/";
+}
diff --git a/tools/perf/util/mem-events.c b/tools/perf/util/mem-events.c
index 93f74d8d3cdd..42c3e5a229d2 100644
--- a/tools/perf/util/mem-events.c
+++ b/tools/perf/util/mem-events.c
@@ -28,7 +28,7 @@ struct perf_mem_event perf_mem_events[PERF_MEM_EVENTS__MAX] = {
static char mem_loads_name[100];
static bool mem_loads_name__init;
-char *perf_mem_events__name(int i)
+char * __weak perf_mem_events__name(int i)
{
if (i == PERF_MEM_EVENTS__LOAD) {
if (!mem_loads_name__init) {
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] powerpc/prom_init: add __init markers to all functions
From: Masahiro Yamada @ 2019-02-05 13:57 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mathieu Malaterre, Linux Kernel Mailing List, Paul Mackerras,
Aneesh Kumar K.V, linuxppc-dev
In-Reply-To: <87k1iemsvz.fsf@concordia.ellerman.id.au>
On Tue, Feb 5, 2019 at 7:33 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
>
> > It is fragile to rely on the compiler's optimization to avoid the
> > section mismatch. Some functions may not be necessarily inlined
> > when the compiler's inlining heuristic changes.
> >
> > Add __init markers consistently.
> >
> > As for prom_getprop() and prom_getproplen(), they are marked as
> > 'inline', so inlining is guaranteed because PowerPC never enables
> > CONFIG_OPTIMIZE_INLINING. However, it would be better to leave the
> > inlining decision to the compiler. I replaced 'inline' with __init.
>
> I'm going to drop that part because it breaks the build in some
> configurations (as reported by the build robot).
If you drop this part, my motivation for this patch is lost.
My motivation is to allow all architectures to enable
CONFIG_OPTIMIZE_INLINING.
(Currently, only x86 can enable it, but I see nothing arch-dependent
in this feature.)
When I tested it in 0-day bot, it reported
section mismatches from prom_getprop() and prom_getproplen().
So, I want to fix the section mismatches without
relying on 'inline'.
My suggestion is this:
static int __init __maybe_unused prom_getproplen(phandle node,
const char *pname)
{
return call_prom("getproplen", 2, 1, node, ADDR(pname));
}
It is true you can use the side-effect of 'inline'
to hide the unused function warnings, but I prefer
as less inline markers as possible in *.c files.
> > diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> > index f33ff41..85b0719 100644
> > --- a/arch/powerpc/kernel/prom_init.c
> > +++ b/arch/powerpc/kernel/prom_init.c
> > @@ -501,19 +501,19 @@ static int __init prom_next_node(phandle *nodep)
> > }
> > }
> >
> > -static inline int prom_getprop(phandle node, const char *pname,
> > +static int __init prom_getprop(phandle node, const char *pname,
> > void *value, size_t valuelen)
> > {
> > return call_prom("getprop", 4, 1, node, ADDR(pname),
> > (u32)(unsigned long) value, (u32) valuelen);
> > }
> >
> > -static inline int prom_getproplen(phandle node, const char *pname)
> > +static int __init prom_getproplen(phandle node, const char *pname)
> > {
> > return call_prom("getproplen", 2, 1, node, ADDR(pname));
> > }
> >
> > -static void add_string(char **str, const char *q)
> > +static void __init add_string(char **str, const char *q)
> > {
> > char *p = *str;
> >
> > @@ -523,7 +523,7 @@ static void add_string(char **str, const char *q)
> > *str = p;
> > }
> >
> > -static char *tohex(unsigned int x)
> > +static char __init *tohex(unsigned int x)
> > {
> > static const char digits[] __initconst = "0123456789abcdef";
> > static char result[9] __prombss;
> > @@ -570,7 +570,7 @@ static int __init prom_setprop(phandle node, const char *nodename,
> > #define islower(c) ('a' <= (c) && (c) <= 'z')
> > #define toupper(c) (islower(c) ? ((c) - 'a' + 'A') : (c))
> >
> > -static unsigned long prom_strtoul(const char *cp, const char **endp)
> > +static unsigned long __init prom_strtoul(const char *cp, const char **endp)
> > {
> > unsigned long result = 0, base = 10, value;
> >
> > @@ -595,7 +595,7 @@ static unsigned long prom_strtoul(const char *cp, const char **endp)
> > return result;
> > }
> >
> > -static unsigned long prom_memparse(const char *ptr, const char **retptr)
> > +static unsigned long __init prom_memparse(const char *ptr, const char **retptr)
> > {
> > unsigned long ret = prom_strtoul(ptr, retptr);
> > int shift = 0;
> > @@ -2924,7 +2924,7 @@ static void __init fixup_device_tree_pasemi(void)
> > prom_setprop(iob, name, "device_type", "isa", sizeof("isa"));
> > }
> > #else /* !CONFIG_PPC_PASEMI_NEMO */
> > -static inline void fixup_device_tree_pasemi(void) { }
> > +static inline void __init fixup_device_tree_pasemi(void) { }
>
> I don't think we need __init for an empty static inline.
I prefer 'static __init' to 'static inline',
but I can drop this if you are uncomfortable with it.
My work will not be blocked by this.
> > #endif
> >
> > static void __init fixup_device_tree(void)
> > @@ -2986,15 +2986,15 @@ static void __init prom_check_initrd(unsigned long r3, unsigned long r4)
> >
> > #ifdef CONFIG_PPC64
> > #ifdef CONFIG_RELOCATABLE
> > -static void reloc_toc(void)
> > +static void __init reloc_toc(void)
> > {
> > }
> >
> > -static void unreloc_toc(void)
> > +static void __init unreloc_toc(void)
> > {
> > }
>
> Those should be empty static inlines, I'll fix them up.
As I said above, I believe 'static inline' is mostly useful in headers,
but this is up to you.
BTW, I have v2 in hand already.
Do you need it if it is convenient for you?
I added __init to enter_prom() as well,
but you may not be comfortable with
replacing inline with __init.
> > #else
> > -static void __reloc_toc(unsigned long offset, unsigned long nr_entries)
> > +static void __init __reloc_toc(unsigned long offset, unsigned long nr_entries)
> > {
> > unsigned long i;
> > unsigned long *toc_entry;
> > @@ -3008,7 +3008,7 @@ static void __reloc_toc(unsigned long offset, unsigned long nr_entries)
> > }
> > }
> >
> > -static void reloc_toc(void)
> > +static void __init reloc_toc(void)
> > {
> > unsigned long offset = reloc_offset();
> > unsigned long nr_entries =
> > @@ -3019,7 +3019,7 @@ static void reloc_toc(void)
> > mb();
> > }
> >
> > -static void unreloc_toc(void)
> > +static void __init unreloc_toc(void)
> > {
> > unsigned long offset = reloc_offset();
> > unsigned long nr_entries =
>
>
> cheers
--
Best Regards
Masahiro Yamada
^ 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