* [PATCH 8/9][v5] powerpc/perf: Export Power7 memory hierarchy info to user space.
From: Sukadev Bhattiprolu @ 2013-10-02 0:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Michael Ellerman, linux-kernel, Stephane Eranian, linuxppc-dev,
Paul Mackerras, Anshuman Khandual
In-Reply-To: <1380672911-12812-1-git-send-email-sukadev@linux.vnet.ibm.com>
On Power7, the DCACHE_SRC field in MMCRA register identifies the memory
hierarchy level (eg: L2, L3 etc) from which a data-cache miss for a
marked instruction was satisfied.
Use the 'perf_mem_data_src' object to export this hierarchy level to user
space. Some memory hierarchy levels in Power7 don't map into the arch-neutral
levels. However, since newer generation of the processor (i.e. Power8) uses
fewer levels than in Power7, we don't really need to define new hierarchy
levels just for Power7.
We instead, map as many levels as possible and approximate the rest. See
comments near dcache-src_map[] in the patch.
Usage:
perf record -d -e 'cpu/PM_MRK_GRP_CMPL/' <application>
perf report -n --mem-mode --sort=mem,sym,dso,symbol_daddr,dso_daddr"
For samples involving load/store instructions, the memory
hierarchy level is shown as "L1 hit", "Remote RAM hit" etc.
# or
perf record --data <application>
perf report -D
Sample records contain a 'data_src' field which encodes the
memory hierarchy level: Eg: data_src 0x442 indicates
MEM_OP_LOAD, MEM_LVL_HIT, MEM_LVL_L2 (i.e load hit L2).
Note that the PMU event PM_MRK_GRP_CMPL tracks all marked group completions
events. While some of these are loads and stores, others like 'add'
instructions may also be sampled.
As such, the precise semantics of 'perf mem -t load' or 'perf mem -t store'
(which require sampling only loads or only stores cannot be implemented on
Power. (Sampling on PM_MRK_GRP_CMPL and throwing away non-loads and non-store
samples could yield an inconsistent profile of the application).
Thanks to input from Stephane Eranian, Michael Ellerman and Michael Neuling.
Cc: Stephane Eranian <eranian@google.com>
Cc: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Reviewed-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
Changelog[v4]:
Drop support for 'perf mem' for Power (use perf-record and perf-report
directly)
Changelog[v3]:
[Michael Ellerman] If newer levels that we defined in [v2] are not
needed for Power8, ignore the new levels for Power7 also, and
approximate them.
Separate the TLB level mapping to a separate patchset.
Changelog[v2]:
[Stephane Eranian] Define new levels rather than ORing the L2 and L3
with REM_CCE1 and REM_CCE2.
[Stephane Eranian] allocate a bit PERF_MEM_XLVL_NA for architectures
that don't use the ->mem_xlvl field.
Insert the TLB patch ahead so the new TLB bits are contigous with
existing TLB bits.
arch/powerpc/perf/power7-pmu.c | 94 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 94 insertions(+)
diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
index 56c67bc..ddfa548 100644
--- a/arch/powerpc/perf/power7-pmu.c
+++ b/arch/powerpc/perf/power7-pmu.c
@@ -11,8 +11,10 @@
#include <linux/kernel.h>
#include <linux/perf_event.h>
#include <linux/string.h>
+#include <linux/uaccess.h>
#include <asm/reg.h>
#include <asm/cputable.h>
+#include <asm/code-patching.h>
/*
* Bits in event code for POWER7
@@ -317,6 +319,97 @@ static void power7_disable_pmc(unsigned int pmc, unsigned long mmcr[])
mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SH(pmc));
}
+#define POWER7_MMCRA_DCACHE_MISS (0x1LL << 55)
+#define POWER7_MMCRA_DCACHE_SRC_SHIFT 51
+#define POWER7_MMCRA_DCACHE_SRC_MASK (0xFLL << POWER7_MMCRA_DCACHE_SRC_SHIFT)
+
+#define P(a, b) PERF_MEM_S(a, b)
+#define PLH(a, b) (P(OP, LOAD) | P(LVL, HIT) | P(a, b))
+/*
+ * Map the Power7 DCACHE_SRC field (bits 9..12) in MMCRA register to the
+ * architecture-neutral memory hierarchy levels. For the levels in Power7
+ * that don't map to the arch-neutral levels, approximate to nearest
+ * level.
+ *
+ * 1-hop: indicates another core on the same chip (2.1 and 3.1 levels).
+ * 2-hops: indicates a different chip on same or different node (remote
+ * and distant levels).
+ *
+ * For consistency with this interpretation of the hops, we dont use
+ * the REM_RAM1 level below.
+ *
+ * The *SHR and *MOD states of the cache are ignored/not exported to user.
+ *
+ * ### Levels marked with ### in comments below are approximated
+ */
+static u64 dcache_src_map[] = {
+ PLH(LVL, L2), /* 00: FROM_L2 */
+ PLH(LVL, L3), /* 01: FROM_L3 */
+
+ P(LVL, NA), /* 02: Reserved */
+ P(LVL, NA), /* 03: Reserved */
+
+ PLH(LVL, REM_CCE1), /* 04: FROM_L2.1_SHR ### */
+ PLH(LVL, REM_CCE1), /* 05: FROM_L2.1_MOD ### */
+
+ PLH(LVL, REM_CCE1), /* 06: FROM_L3.1_SHR ### */
+ PLH(LVL, REM_CCE1), /* 07: FROM_L3.1_MOD ### */
+
+ PLH(LVL, REM_CCE2), /* 08: FROM_RL2L3_SHR ### */
+ PLH(LVL, REM_CCE2), /* 09: FROM_RL2L3_MOD ### */
+
+ PLH(LVL, REM_CCE2), /* 10: FROM_DL2L3_SHR ### */
+ PLH(LVL, REM_CCE2), /* 11: FROM_DL2L3_MOD ### */
+
+ PLH(LVL, LOC_RAM), /* 12: FROM_LMEM */
+ PLH(LVL, REM_RAM2), /* 13: FROM_RMEM ### */
+ PLH(LVL, REM_RAM2), /* 14: FROM_DMEM */
+
+ P(LVL, NA), /* 15: Reserved */
+};
+
+/*
+ * Determine the memory-hierarchy information (if applicable) for the
+ * instruction/address we are sampling. If we encountered a DCACHE_MISS,
+ * mmcra[DCACHE_SRC_MASK] specifies the memory level from which the operand
+ * was loaded.
+ *
+ * Otherwise, it is an L1-hit, provided the instruction was a load/store.
+ */
+static void power7_get_mem_data_src(union perf_mem_data_src *dsrc,
+ struct pt_regs *regs)
+{
+ u64 idx;
+ u64 mmcra = regs->dsisr;
+ u64 addr;
+ int ret;
+ unsigned int instr;
+
+ if (mmcra & POWER7_MMCRA_DCACHE_MISS) {
+ idx = mmcra & POWER7_MMCRA_DCACHE_SRC_MASK;
+ idx >>= POWER7_MMCRA_DCACHE_SRC_SHIFT;
+
+ dsrc->val |= dcache_src_map[idx];
+ return;
+ }
+
+ instr = 0;
+ addr = perf_instruction_pointer(regs);
+
+ if (is_kernel_addr(addr))
+ instr = *(unsigned int *)addr;
+ else {
+ pagefault_disable();
+ ret = __get_user_inatomic(instr, (unsigned int __user *)addr);
+ pagefault_enable();
+ if (ret)
+ instr = 0;
+ }
+ if (instr && instr_is_load_store(&instr))
+ dsrc->val |= PLH(LVL, L1);
+}
+
+
static int power7_generic_events[] = {
[PERF_COUNT_HW_CPU_CYCLES] = PME_PM_CYC,
[PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = PME_PM_GCT_NOSLOT_CYC,
@@ -437,6 +530,7 @@ static struct power_pmu power7_pmu = {
.get_constraint = power7_get_constraint,
.get_alternatives = power7_get_alternatives,
.disable_pmc = power7_disable_pmc,
+ .get_mem_data_src = power7_get_mem_data_src,
.flags = PPMU_ALT_SIPR,
.attr_groups = power7_pmu_attr_groups,
.n_generic = ARRAY_SIZE(power7_generic_events),
--
1.7.9.5
^ permalink raw reply related
* [PATCH 9/9][v5] powerpc/perf: Update perf-mem man page for Power
From: Sukadev Bhattiprolu @ 2013-10-02 0:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Michael Ellerman, linux-kernel, Stephane Eranian, linuxppc-dev,
Paul Mackerras, Anshuman Khandual
In-Reply-To: <1380672911-12812-1-git-send-email-sukadev@linux.vnet.ibm.com>
Add a few lines to the perf-mem man page to indicate:
- its dependence on the mem-loads and mem-stores events
- how to use the feature on Power architecture.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
tools/perf/Documentation/perf-mem.txt | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tools/perf/Documentation/perf-mem.txt b/tools/perf/Documentation/perf-mem.txt
index 888d511..f4881a0 100644
--- a/tools/perf/Documentation/perf-mem.txt
+++ b/tools/perf/Documentation/perf-mem.txt
@@ -18,6 +18,17 @@ from it, into perf.data. Perf record options are accepted and are passed through
"perf mem -t <TYPE> report" displays the result. It invokes perf report with the
right set of options to display a memory access profile.
+This command works on architectures that implement *mem-loads* and *mem-stores*
+perf events.
+
+The PowerPC architecture does not implement *mem-loads* and *mem-stores*
+events. To get the memory hierarchy information for samples involving
+memory loads and stores, use a marked event like PM_MRK_GRP_CMPL.
+
+ perf record -d -e 'cpu/PM_MRK_GRP_CMPL/' <application>
+
+ perf report -n --mem-mode
+
OPTIONS
-------
<command>...::
--
1.7.9.5
^ permalink raw reply related
* [PATCH 6/9][v5] powerpc/perf: Define big-endian version of perf_mem_data_src
From: Sukadev Bhattiprolu @ 2013-10-02 0:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Michael Ellerman, linux-kernel, Stephane Eranian, linuxppc-dev,
Paul Mackerras, Anshuman Khandual
In-Reply-To: <1380672911-12812-1-git-send-email-sukadev@linux.vnet.ibm.com>
perf_mem_data_src is an union that is initialized via the ->val field
and accessed via the bitmap fields. For this to work on big endian
platforms, we also need a big-endian represenation of perf_mem_data_src.
Cc: Stephane Eranian <eranian@google.com>
Cc: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Reviewed-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
Changelog [v5]:
- include <endian.h> in local byteorder.h
Changelog [v4]:
- perf_event.h includes <byteorder.h> which pulls in the local
byteorder.h when building the perf tool. This local byteorder.h
leaves __LITTLE_ENDIAN and __BIG_ENDIAN undefined.
Include <endian.h> explicitly in the local byteorder.h.
Changelog [v2]:
- [Vince Weaver, Michael Ellerman] No __KERNEL__ in uapi headers.
include/uapi/linux/perf_event.h | 58 +++++++++++++++++++++++++++++++
tools/perf/util/include/asm/byteorder.h | 1 +
2 files changed, 59 insertions(+)
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index ca1d90b..846f399 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -19,6 +19,50 @@
#include <asm/byteorder.h>
/*
+ * Kernel and userspace check for endianness in incompatible ways.
+ * In user space, <endian.h> defines both __BIG_ENDIAN and __LITTLE_ENDIAN
+ * but sets __BYTE_ORDER to one or the other. So user space uses checks are:
+ *
+ * #if __BYTE_ORDER == __LITTLE_ENDIAN
+ *
+ * In the kernel, __BYTE_ORDER is undefined, so using the above check doesn't
+ * work. Further, kernel code assumes that exactly one of __BIG_ENDIAN and
+ * __LITTLE_ENDIAN is defined. So the kernel checks are like:
+ *
+ * #if defined(__LITTLE_ENDIAN)
+ *
+ * But we can't use that check in user space since __LITTLE_ENDIAN (and
+ * __BIG_ENDIAN) are always defined.
+ *
+ * Since some perf data structures depend on endianness _and_ are shared
+ * between kernel and user, perf needs its own notion of endian macros (at
+ * least until user and kernel endian checks converge).
+ */
+#define __PERF_LE 1234
+#define __PERF_BE 4321
+
+#if defined(__BYTE_ORDER)
+
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+#define __PERF_BYTE_ORDER __PERF_LE
+#elif __BYTE_ORDER == __BIG_ENDIAN
+#define __PERF_BYTE_ORDER __PERF_BE
+#endif
+
+#else /* __BYTE_ORDER */
+
+#if defined(__LITTLE_ENDIAN) && defined(__BIG_ENDIAN)
+#error "Cannot determine endianness"
+#elif defined(__LITTLE_ENDIAN)
+#define __PERF_BYTE_ORDER __PERF_LE
+#elif defined(__BIG_ENDIAN)
+#define __PERF_BYTE_ORDER __PERF_BE
+#endif
+
+
+#endif /* __BYTE_ORDER */
+
+/*
* User-space ABI bits:
*/
@@ -695,6 +739,7 @@ enum perf_callchain_context {
#define PERF_FLAG_FD_OUTPUT (1U << 1)
#define PERF_FLAG_PID_CGROUP (1U << 2) /* pid=cgroup id, per-cpu mode only */
+#if __PERF_BYTE_ORDER == __PERF_LE
union perf_mem_data_src {
__u64 val;
struct {
@@ -706,6 +751,19 @@ union perf_mem_data_src {
mem_rsvd:31;
};
};
+#elif __PERF_BYTE_ORDER == __PERF_BE
+union perf_mem_data_src {
+ __u64 val;
+ struct {
+ __u64 mem_rsvd:31,
+ mem_dtlb:7, /* tlb access */
+ mem_lock:2, /* lock instr */
+ mem_snoop:5, /* snoop mode */
+ mem_lvl:14, /* memory hierarchy level */
+ mem_op:5; /* type of opcode */
+ };
+};
+#endif
/* type of opcode (load/store/prefetch,code) */
#define PERF_MEM_OP_NA 0x01 /* not available */
diff --git a/tools/perf/util/include/asm/byteorder.h b/tools/perf/util/include/asm/byteorder.h
index 2a9bdc0..7112913 100644
--- a/tools/perf/util/include/asm/byteorder.h
+++ b/tools/perf/util/include/asm/byteorder.h
@@ -1,2 +1,3 @@
#include <asm/types.h>
#include "../../../../include/uapi/linux/swab.h"
+#include <endian.h>
--
1.7.9.5
^ permalink raw reply related
* [PATCH 4/9][v5] powerpc: Rename branch_opcode() to instr_opcode()
From: Sukadev Bhattiprolu @ 2013-10-02 0:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Michael Ellerman, linux-kernel, Stephane Eranian, linuxppc-dev,
Paul Mackerras, Anshuman Khandual
In-Reply-To: <1380672911-12812-1-git-send-email-sukadev@linux.vnet.ibm.com>
The logic used in branch_opcode() to extract the opcode for an instruction
applies to non branch instructions also. So rename to instr_opcode().
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Reviewed-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
arch/powerpc/lib/code-patching.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 17e5b23..2bc9db3 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -72,19 +72,19 @@ unsigned int create_cond_branch(const unsigned int *addr,
return instruction;
}
-static unsigned int branch_opcode(unsigned int instr)
+static unsigned int instr_opcode(unsigned int instr)
{
return (instr >> 26) & 0x3F;
}
static int instr_is_branch_iform(unsigned int instr)
{
- return branch_opcode(instr) == 18;
+ return instr_opcode(instr) == 18;
}
static int instr_is_branch_bform(unsigned int instr)
{
- return branch_opcode(instr) == 16;
+ return instr_opcode(instr) == 16;
}
int instr_is_relative_branch(unsigned int instr)
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/9][v5] powerpc/perf: Rename Power8 macros to start with PME
From: Sukadev Bhattiprolu @ 2013-10-02 0:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Michael Ellerman, linux-kernel, Stephane Eranian, linuxppc-dev,
Paul Mackerras, Anshuman Khandual
In-Reply-To: <1380672911-12812-1-git-send-email-sukadev@linux.vnet.ibm.com>
We use helpers like GENERIC_EVENT_ATTR() to list the generic events in
sysfs. To avoid name collisions, GENERIC_EVENT_ATTR() requires the perf
event macros to start with PME.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Reviewed-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
arch/powerpc/perf/power8-pmu.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index 2ee4a70..976c203 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -18,12 +18,12 @@
/*
* Some power8 event codes.
*/
-#define PM_CYC 0x0001e
-#define PM_GCT_NOSLOT_CYC 0x100f8
-#define PM_CMPLU_STALL 0x4000a
-#define PM_INST_CMPL 0x00002
-#define PM_BRU_FIN 0x10068
-#define PM_BR_MPRED_CMPL 0x400f6
+#define PME_PM_CYC 0x0001e
+#define PME_PM_GCT_NOSLOT_CYC 0x100f8
+#define PME_PM_CMPLU_STALL 0x4000a
+#define PME_PM_INST_CMPL 0x00002
+#define PME_PM_BRU_FIN 0x10068
+#define PME_PM_BR_MPRED_CMPL 0x400f6
/*
@@ -550,12 +550,12 @@ static const struct attribute_group *power8_pmu_attr_groups[] = {
};
static int power8_generic_events[] = {
- [PERF_COUNT_HW_CPU_CYCLES] = PM_CYC,
- [PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = PM_GCT_NOSLOT_CYC,
- [PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = PM_CMPLU_STALL,
- [PERF_COUNT_HW_INSTRUCTIONS] = PM_INST_CMPL,
- [PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = PM_BRU_FIN,
- [PERF_COUNT_HW_BRANCH_MISSES] = PM_BR_MPRED_CMPL,
+ [PERF_COUNT_HW_CPU_CYCLES] = PME_PM_CYC,
+ [PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = PME_PM_GCT_NOSLOT_CYC,
+ [PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = PME_PM_CMPLU_STALL,
+ [PERF_COUNT_HW_INSTRUCTIONS] = PME_PM_INST_CMPL,
+ [PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = PME_PM_BRU_FIN,
+ [PERF_COUNT_HW_BRANCH_MISSES] = PME_PM_BR_MPRED_CMPL,
};
static u64 power8_bhrb_filter_map(u64 branch_sample_type)
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/9][v5] powerpc/perf: Export Power8 generic events in sysfs
From: Sukadev Bhattiprolu @ 2013-10-02 0:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Michael Ellerman, linux-kernel, Stephane Eranian, linuxppc-dev,
Paul Mackerras, Anshuman Khandual
In-Reply-To: <1380672911-12812-1-git-send-email-sukadev@linux.vnet.ibm.com>
Export generic perf events for Power8 in sysfs.
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Reviewed-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
arch/powerpc/perf/power8-pmu.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index 976c203..b991b2e 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -510,6 +510,28 @@ static void power8_disable_pmc(unsigned int pmc, unsigned long mmcr[])
mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SHIFT(pmc + 1));
}
+GENERIC_EVENT_ATTR(cpu-cyles, PM_CYC);
+GENERIC_EVENT_ATTR(stalled-cycles-frontend, PM_GCT_NOSLOT_CYC);
+GENERIC_EVENT_ATTR(stalled-cycles-backend, PM_CMPLU_STALL);
+GENERIC_EVENT_ATTR(instructions, PM_INST_CMPL);
+GENERIC_EVENT_ATTR(branch-instructions, PM_BRU_FIN);
+GENERIC_EVENT_ATTR(branch-misses, PM_BR_MPRED_CMPL);
+
+static struct attribute *power8_events_attr[] = {
+ GENERIC_EVENT_PTR(PM_CYC),
+ GENERIC_EVENT_PTR(PM_GCT_NOSLOT_CYC),
+ GENERIC_EVENT_PTR(PM_CMPLU_STALL),
+ GENERIC_EVENT_PTR(PM_INST_CMPL),
+ GENERIC_EVENT_PTR(PM_BRU_FIN),
+ GENERIC_EVENT_PTR(PM_BR_MPRED_CMPL),
+ NULL
+};
+
+static struct attribute_group power8_pmu_events_group = {
+ .name = "events",
+ .attrs = power8_events_attr,
+};
+
PMU_FORMAT_ATTR(event, "config:0-49");
PMU_FORMAT_ATTR(pmcxsel, "config:0-7");
PMU_FORMAT_ATTR(mark, "config:8");
@@ -546,6 +568,7 @@ struct attribute_group power8_pmu_format_group = {
static const struct attribute_group *power8_pmu_attr_groups[] = {
&power8_pmu_format_group,
+ &power8_pmu_events_group,
NULL,
};
--
1.7.9.5
^ permalink raw reply related
* (no subject)
From: Sukadev Bhattiprolu @ 2013-10-02 0:15 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Michael Ellerman, linux-kernel, Stephane Eranian, linuxppc-dev,
Paul Mackerras, Anshuman Khandual
Subject: [PATCH 0/9][v5] powerpc/perf: Export memory hierarchy level in Power7/8.
Power7 and Power8 processors save the memory hierarchy level (eg: L2, L3)
from which a load or store instruction was satisfied. Export this hierarchy
information to the user via the perf_mem_data_src object.
Thanks to input from Stephane Eranian, Michael Ellerman, Michael Neuling
and Anshuman Khandual.
Sukadev Bhattiprolu (9):
powerpc/perf: Rename Power8 macros to start with PME
powerpc/perf: Export Power8 generic events in sysfs
powerpc/perf: Add Power8 event PM_MRK_GRP_CMPL to sysfs.
powerpc: Rename branch_opcode() to instr_opcode()
powerpc: implement is_instr_load_store().
powerpc/perf: Define big-endian version of perf_mem_data_src
powerpc/perf: Export Power8 memory hierarchy info to user space.
powerpc/perf: Export Power7 memory hierarchy info to user space.
powerpc/perf: Update perf-mem man page for Power
arch/powerpc/include/asm/code-patching.h | 1 +
arch/powerpc/include/asm/perf_event_server.h | 2 +
arch/powerpc/lib/code-patching.c | 96 ++++++++++++++++++++++-
arch/powerpc/perf/core-book3s.c | 11 +++
arch/powerpc/perf/power7-pmu.c | 94 +++++++++++++++++++++++
arch/powerpc/perf/power8-pmu.c | 105 +++++++++++++++++++++++---
include/uapi/linux/perf_event.h | 58 ++++++++++++++
tools/perf/Documentation/perf-mem.txt | 11 +++
tools/perf/util/include/asm/byteorder.h | 1 +
9 files changed, 364 insertions(+), 15 deletions(-)
--
1.7.9.5
^ permalink raw reply
* Re: [PATCH 3/3] KVM: PPC: Book3S: Add support for hwrng found on some powernv systems
From: Benjamin Herrenschmidt @ 2013-10-01 21:44 UTC (permalink / raw)
To: Paolo Bonzini
Cc: tytso, kvm, Gleb Natapov, linuxppc-dev, linux-kernel, kvm-ppc,
agraf, herbert, Paul Mackerras, mpm
In-Reply-To: <524AAFAA.3010801@redhat.com>
On Tue, 2013-10-01 at 13:19 +0200, Paolo Bonzini wrote:
> Il 01/10/2013 11:38, Benjamin Herrenschmidt ha scritto:
> > So for the sake of that dogma you are going to make us do something that
> > is about 100 times slower ? (and possibly involves more lines of code)
>
> If it's 100 times slower there is something else that's wrong. It's
> most likely not 100 times slower, and this makes me wonder if you or
> Michael actually timed the code at all.
We haven't but it's pretty obvious:
- The KVM real mode implementation: guest issues the hcall, we remain
in real mode, within the MMU context of the guest, all secondary threads
on the core are still running in the guest, and we do an MMIO & return.
- The qemu variant: guest issues the hcall we need to exit the guest,
which means bring *all* threads on the core out of KVM, switch the full
MMU context back to host (which among others involves flushing the ERAT,
aka level 1 TLB), while sending the secondary threads into idle loops.
Then we return to qemu user context, which will then use /dev/random ->
back into the kernel and out, at which point we can return to the guest,
so back into the kernel, back into run which means IPI the secondary
threads on the core, switch the MMU context again until we can finally
go back to executing guest instructions.
So no we haven't measured. But it is going to be VERY VERY VERY much
slower. Our exit latencies are bad with our current MMU *and* any exit
is going to cause all secondary threads on the core to have to exit as
well (remember P7 is 4 threads, P8 is 8)
> > It's not just speed ... H_RANDOM is going to be called by the guest
> > kernel. A round trip to qemu is going to introduce a kernel jitter
> > (complete stop of operations of the kernel on that virtual processor) of
> > a full exit + round trip to qemu + back to the kernel to get to some
> > source of random number ... this is going to be in the dozens of ns at
> > least.
>
> I guess you mean dozens of *micro*seconds, which is somewhat exaggerated
> but not too much. On x86 some reasonable timings are:
Yes.
> 100 cycles bare metal rdrand
> 2000 cycles guest->hypervisor->guest
> 15000 cycles guest->userspace->guest
>
> (100 cycles = 40 ns = 200 MB/sec; 2000 cycles = ~1 microseconds; 15000
> cycles = ~7.5 microseconds). Even on 5 year old hardware, a userspace
> roundtrip is around a dozen microseconds.
So in your case going to qemu to "emulate" rdrand would indeed be 150
times slower, I don't see in what universe that would be considered a
good idea.
> Anyhow, I would like to know more about this hwrng and hypercall.
>
> Does the hwrng return random numbers (like rdrand) or real entropy (like
> rdseed that Intel will add in Broadwell)?
It's a random number obtained from sampling a set of oscillators. It's
slightly biased but we have very simple code (I believe shared with the
host kernel implementation) for whitening it as is required by PAPR.
> What about the hypercall?
> For example virtio-rng is specified to return actual entropy, it doesn't
> matter if it is from hardware or software.
>
> In either case, the patches have problems.
>
> 1) If the hwrng returns random numbers, the whitening you're doing is
> totally insufficient and patch 2 is forging entropy that doesn't exist.
I will let Paul to comment on the whitening, it passes all the tests
we've been running it through.
> 2) If the hwrng returns entropy, a read from the hwrng is going to even
> more expensive than an x86 rdrand (perhaps ~2000 cycles).
Depends how often you read, the HW I think is sampling asynchronously so
you only block on the MMIO if you already consumed the previous sample
but I'll let Paulus provide more details here.
> Hence, doing
> the emulation in the kernel is even less necessary. Also, if the hwrng
> returns entropy patch 1 is unnecessary: you do not need to waste
> precious entropy bits by passing them to arch_get_random_long; just run
> rngd in the host as that will put the entropy to much better use.
>
> 3) If the hypercall returns random numbers, then it is a pretty
> braindead interface since returning 8 bytes at a time limits the
> throughput to a handful of MB/s (compare to 200 MB/sec for x86 rdrand).
> But more important: in this case drivers/char/hw_random/pseries-rng.c
> is completely broken and insecure, just like patch 2 in case (1) above.
How so ?
> 4) If the hypercall returns entropy (same as virtio-rng), the same
> considerations on speed apply. If you can only produce entropy at say 1
> MB/s (so reading 8 bytes take 8 microseconds---which is actually very
> fast), it doesn't matter that much to spend 7 microseconds on a
> userspace roundtrip. It's going to be only half the speed of bare
> metal, not 100 times slower.
>
>
> Also, you will need _anyway_ extra code that is not present here to
> either disable the rng based on userspace command-line, or to emulate
> the rng from userspace. It is absolutely _not_ acceptable to have a
> hypercall disappear across migration. You're repeatedly ignoring these
> issues, but rest assured that they will come back and bite you
> spectacularly.
>
> Based on all this, I would simply ignore the part of the spec where they
> say "the hypercall should return numbers from a hardware source". All
> that matters in virtualization is to have a good source of _entropy_.
> Then you can run rngd without randomness checks, which will more than
> recover the cost of userspace roundtrips.
>
> In any case, deciding where to get that entropy from is definitely
> outside the scope of KVM, and in fact QEMU already has a configurable
> mechanism for that.
>
> Paolo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] powerpc/iommu: use GFP_KERNEL instead of GFP_ATOMIC in iommu_init_table()
From: Nishanth Aravamudan @ 2013-10-01 21:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Paul Mackerras, linuxppc-dev, Thadeu Lima de Souza Cascardo,
Anton Blanchard
Under heavy (DLPAR?) stress, we tripped this panic() in
arch/powerpc/kernel/iommu.c::iommu_init_table():
page = alloc_pages_node(nid, GFP_ATOMIC, get_order(sz));
if (!page)
panic("iommu_init_table: Can't allocate %ld bytes\n",
sz);
Before the panic() we got a page allocation failure for an order-2
allocation. There appears to be memory free, but perhaps not in the
ATOMIC context. I looked through all the call-sites of
iommu_init_table() and didn't see any obvious reason to need an ATOMIC
allocation. Most call-sites in fact have an explicit GFP_KERNEL
allocation shortly before the call to iommu_init_table(), indicating we
are not in an atomic context. There is some indirection for some paths,
but I didn't see any locks indicating that GFP_KERNEL is inappropriate.
With this change under the same conditions, we have not been able to
reproduce the panic.
Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 0adab06..572bb5b 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -661,7 +661,7 @@ struct iommu_table *iommu_init_table(struct iommu_table *tbl, int nid)
/* number of bytes needed for the bitmap */
sz = BITS_TO_LONGS(tbl->it_size) * sizeof(unsigned long);
- page = alloc_pages_node(nid, GFP_ATOMIC, get_order(sz));
+ page = alloc_pages_node(nid, GFP_KERNEL, get_order(sz));
if (!page)
panic("iommu_init_table: Can't allocate %ld bytes\n", sz);
tbl->it_map = page_address(page);
^ permalink raw reply related
* Re: [PATCH 1/2][v7] powerpc/mpc85xx:Add initial device tree support of T104x
From: Scott Wood @ 2013-10-01 19:56 UTC (permalink / raw)
To: Prabhakar Kushwaha
Cc: Varun Sethi, linuxppc-dev, Poonam Aggrwal, Priyanka Jain
In-Reply-To: <524A40F4.9040406@freescale.com>
On Tue, 2013-10-01 at 08:56 +0530, Prabhakar Kushwaha wrote:
> On 10/01/2013 01:17 AM, Scott Wood wrote:
> > On Mon, 2013-09-30 at 12:24 +0530, Prabhakar Kushwaha wrote:
> >> - Removed l2switch. It will be added later
> > Why?
>
> I am not aware of bindings required for l2switch as we are not working
> on the driver.
> Earlier I thought of putting a place holder. but as you suggested to put
> bindings in documentation.
> It will be good if it is put by actual driver owner.
Is there a reason to believe the binding will be complicated?
Does any such "driver owner" exist yet?
> >> +sata@220000 {
> >> + fsl,iommu-parent = <&pamu0>;
> >> + fsl,liodn-reg = <&guts 0x550>; /* SATA1LIODNR */
> >> +};
> >> +/include/ "qoriq-sata2-1.dtsi"
> >> +sata@221000 {
> >> + fsl,iommu-parent = <&pamu0>;
> >> + fsl,liodn-reg = <&guts 0x554>; /* SATA2LIODNR */
> >> +};
> > Whitespace
>
> do we have any scripts which check for whitespace as checkpatch never
> give any warning/error.
> it is a very silly mistake which I am doing continuously :(
checkpatch doesn't check dts files.
> >> +/include/ "t1040si-post.dtsi"
> > Should at least have a comment indicating that eventually this should
> > hold the l2 switch node.
>
> yes. Ideally it should be.
> but if I put a comment then I believe this patch will not be completed.
> it will think as a RFC.
> as I believe putting of TODO is generally for RFC patches.
As is, one would wonder why the separate file exists at all.
The TODO is there whether you have a comment acknowledging it or
not. :-)
-Scott
^ permalink raw reply
* Re: [PATCH] powerpc/85xx: DTS - re-organize the SPI partitions property
From: Scott Wood @ 2013-10-01 19:37 UTC (permalink / raw)
To: Hu Mingkai-B21284; +Cc: Wood Scott-B07421, linuxppc-dev@ozlabs.org
In-Reply-To: <CF6CBFBA8EBBB949B6E321556F6E15090A157023@039-SN2MPN1-012.039d.mgd.msft.net>
On Mon, 2013-09-30 at 03:31 -0500, Hu Mingkai-B21284 wrote:
>
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, September 25, 2013 3:37 AM
> > To: Hu Mingkai-B21284
> > Cc: Wood Scott-B07421; linuxppc-dev@ozlabs.org
> > Subject: Re: [PATCH] powerpc/85xx: DTS - re-organize the SPI partitions
> > property
> >
> > Fixing U-Boot would make the problem go away without any issues with
> > partition compatibility. Are you sure nobody's using these SPI
> > partitions without booting from SPI? Even if nobody's using this, it
> > seems a wasteful solution. These are pretty small flashes.
> >
> Scott,
>
> I will submit a patch in U-Boot to fix this issue. Some quick questions:
> 1. Should we set the SPI flash as MTDPARTS_DEFAULT?
> 2. Should we consider the partition for NAND/NOR in mtdparts?
> 3. We need to remove the partition table in device tree, right?
Fixing the U-Boot size problem is separate from changing how we do
partitioning, but yes, we should transition all flashes to using
mtdparts instead of device tree partitions.
As to when to remove them from the device tree, that's a bit tricky.
There's no guarantee when a user updates their U-Boot environment
relative to when they update their device tree. It's better to have
the partitions appear twice than to not appear at all. But we do want
to discourage the use of the device tree partitions, and it would be
bad if the descriptions don't match and a user ends up mixing the two.
-Scott
^ permalink raw reply
* [PATCH v3] powerpc/kernel/sysfs: Disable writing to PURR in guest mode
From: Madhavan Srinivasan @ 2013-10-01 19:04 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev, Madhavan Srinivasan
powerpc/kernel/sysfs.c exports PURR with write permission.
This may be valid for kernel in phyp mode. But writing to
the file in guest mode causes crash.
# echo 0 > purr
cpu 0x0: Vector: 700 (Program Check) at [c000000000d072b0]
pc: c00000000001770c: .write_purr+0x1c/0x40
lr: c000000000017708: .write_purr+0x18/0x40
sp: c000000000d07530
msr: 8000000000049032
current = 0xc000000000c53de0
paca = 0xc00000000ec70000 softe: 0 irq_happened: 0x01
pid = 0, comm = swapper/0
enter ? for help
[c000000000d075b0] c0000000000fba64
.generic_smp_call_function_single_interrupt+0x104/0x190
[c000000000d07650] c000000000037748 .smp_ipi_demux+0xa8/0xf0
[c000000000d076e0] c000000000035314 .doorbell_exception+0x74/0xb0
[c000000000d07760] c000000000002950 doorbell_super_common+0x150/0x180
--- Exception: a01 (Doorbell) at c000000000060904
.plpar_hcall_norets+0x84/0xd4
[link register ] c00000000006dbd4 .check_and_cede_processor+0x24/0x40
[c000000000d07a50] c000000001002558 (unreliable)
[c000000000d07ac0] c00000000006dd0c .shared_cede_loop+0x2c/0x70
[c000000000d07b40] c0000000006ae954 .cpuidle_enter_state+0x64/0x150
[c000000000d07c00] c0000000006aeb30 .cpuidle_idle_call+0xf0/0x300
[c000000000d07cb0] c000000000062fa0 .pseries_lpar_idle+0x10/0x50
[c000000000d07d20] c000000000016d14 .arch_cpu_idle+0x64/0x150
[c000000000d07da0] c0000000000e0060 .cpu_startup_entry+0x1a0/0x2c0
[c000000000d07e80] c00000000000bca4 .rest_init+0x94/0xb0
[c000000000d07ef0] c000000000b54530 .start_kernel+0x478/0x494
[c000000000d07f90] c000000000009be0 .start_here_common+0x20/0x40
0:mon>
v3 changes:
1) Changed the test for to firmware lpar feature support instead of hypervisor
2) Removed type cast
3) changed the commit to explain better.
v2 changes:
1) Changed the test for to hypervisor mode instead of platform
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
arch/powerpc/kernel/sysfs.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
index 27a90b9..b4e6676 100644
--- a/arch/powerpc/kernel/sysfs.c
+++ b/arch/powerpc/kernel/sysfs.c
@@ -17,6 +17,7 @@
#include <asm/machdep.h>
#include <asm/smp.h>
#include <asm/pmc.h>
+#include <asm/firmware.h>
#include "cacheinfo.h"
@@ -179,15 +180,25 @@ SYSFS_PMCSETUP(spurr, SPRN_SPURR);
SYSFS_PMCSETUP(dscr, SPRN_DSCR);
SYSFS_PMCSETUP(pir, SPRN_PIR);
+/*
+ Lets only enable read for phyp resources and
+ enable write when needed with a separate function.
+ Lets be conservative and default to pseries.
+*/
static DEVICE_ATTR(mmcra, 0600, show_mmcra, store_mmcra);
static DEVICE_ATTR(spurr, 0400, show_spurr, NULL);
static DEVICE_ATTR(dscr, 0600, show_dscr, store_dscr);
-static DEVICE_ATTR(purr, 0600, show_purr, store_purr);
+static DEVICE_ATTR(purr, 0400, show_purr, store_purr);
static DEVICE_ATTR(pir, 0400, show_pir, NULL);
unsigned long dscr_default = 0;
EXPORT_SYMBOL(dscr_default);
+static void add_write_permission_dev_attr(struct device_attribute *attr)
+{
+ attr->attr.mode |= 0200;
+}
+
static ssize_t show_dscr_default(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -394,8 +405,11 @@ static void register_cpu_online(unsigned int cpu)
if (cpu_has_feature(CPU_FTR_MMCRA))
device_create_file(s, &dev_attr_mmcra);
- if (cpu_has_feature(CPU_FTR_PURR))
+ if (cpu_has_feature(CPU_FTR_PURR)) {
+ if (!firmware_has_feature(FW_FEATURE_LPAR))
+ add_write_permission_dev_attr(&dev_attr_purr);
device_create_file(s, &dev_attr_purr);
+ }
if (cpu_has_feature(CPU_FTR_SPURR))
device_create_file(s, &dev_attr_spurr);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] Revert "powerpc: 52xx: provide a default in mpc52xx_irqhost_map()"
From: Wolfram Sang @ 2013-10-01 19:03 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: linuxppc-dev, Anatolij Gustschin, linux-rt-users
In-Reply-To: <524AF913.6020007@linutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1878 bytes --]
> > So people can compile with -Werror (RT patchset).
>
> Why do you mention the RT patch set here? Doesn't the vanila tree gets
> compiled with -Werror as well?
Not for me.
> > irq_chip *irqchip = NULL; /* pet old compilers */
>
> That would probably work, too. I would drop that comment but then
> someone might clean that up :P
Yup. But I just remembered a better solution:
From: Wolfram Sang <wsa@the-dreams.de>
Subject: [PATCH] ppc: mpc52xx: silence false positive from old GCC
So people can compile with -Werror.
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
arch/powerpc/platforms/52xx/mpc52xx_pic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_pic.c b/arch/powerpc/platforms/52xx/mpc52xx_pic.c
index b89ef65..2898b73 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_pic.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_pic.c
@@ -340,7 +340,7 @@ static int mpc52xx_irqhost_map(struct irq_domain *h, unsigned int virq,
{
int l1irq;
int l2irq;
- struct irq_chip *irqchip;
+ struct irq_chip *uninitialized_var(irqchip);
void *hndlr;
int type;
u32 reg;
uninitialized_var was created for exactly that purpose IIRC.
> > People not realizing 'default' is a no-op might wonder why unknown
> > levels are mapped to critical.
>
> I see. And what would you suggest as default in case we would have an
> additional bit?
-Esome or a different error message. But let's postpone that until that
case happens ;)
> Hmmm. I assumed that critical / SDMA / … are interrupt numbers but they
> are seem not be. In that case I guess l2 is more important. l1 kinda
> looks important since it is the value in the switch case which failed
> but since it can only hold one possible value, I guess your info is
> better :)
Thanks,
Wolfram
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply related
* Re: [PATCH] Revert "powerpc: 52xx: provide a default in mpc52xx_irqhost_map()"
From: Sebastian Andrzej Siewior @ 2013-10-01 16:32 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linuxppc-dev, Anatolij Gustschin, linux-rt-users
In-Reply-To: <20131001091115.GB2993@katana>
On 10/01/2013 11:11 AM, Wolfram Sang wrote:
> Hi,
Hi Wolfram,
> Well, if you insist, I'd prefer the following patch.
>
> From: Wolfram Sang <wsa@the-dreams.de> Subject: [PATCH] ppc:
> mpc52xx: silence false positive from old GCC
>
> So people can compile with -Werror (RT patchset).
Why do you mention the RT patch set here? Doesn't the vanila tree gets
compiled with -Werror as well?
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de> ---
> arch/powerpc/platforms/52xx/mpc52xx_pic.c | 2 +- 1 file changed,
> 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_pic.c
> b/arch/powerpc/platforms/52xx/mpc52xx_pic.c index b89ef65..ad3c9b0
> 100644 --- a/arch/powerpc/platforms/52xx/mpc52xx_pic.c +++
> b/arch/powerpc/platforms/52xx/mpc52xx_pic.c @@ -340,7 +340,7 @@
> static int mpc52xx_irqhost_map(struct irq_domain *h, unsigned int
> virq, { int l1irq; int l2irq; - struct irq_chip *irqchip; + struct
> irq_chip *irqchip = NULL; /* pet old compilers */
That would probably work, too. I would drop that comment but then
someone might clean that up :P
> void *hndlr; int type; u32 reg;
>
>> Why miss leading code? Default here does the same as unhandled
>> and crit where it does nothing.
>
> People not realizing 'default' is a no-op might wonder why unknown
> levels are mapped to critical.
I see. And what would you suggest as default in case we would have an
additional bit?
>
>> Any why do you want to see l2irq since it was not in the case
>> statement? l2 holds the number, l1 the level.
>
> We know which level it was, since the printout is only for that
> level. We probably want to know which requested IRQ was causing
> this, so we can fix the assorted driver. Otherwise we only know
> that some critical IRQ was requested somewhere.
Hmmm. I assumed that critical / SDMA / … are interrupt numbers but they
are seem not be. In that case I guess l2 is more important. l1 kinda
looks important since it is the value in the switch case which failed
but since it can only hold one possible value, I guess your info is
better :)
>
> Thanks,
>
> Wolfram
>
Sebastian
^ permalink raw reply
* Re: Avoiding the dentry d_lock on final dput(), part deux: transactional memory
From: Paul E. McKenney @ 2013-10-01 13:42 UTC (permalink / raw)
To: Michael Neuling
Cc: Waiman Long, ppc-dev, Peter Zijlstra, George Spelvin,
Linux Kernel Mailing List, Chandramouleeswaran, Aswin,
Norton, Scott J, linux-fsdevel, Linus Torvalds, Ingo Molnar
In-Reply-To: <20131001121654.GX19582@linux.vnet.ibm.com>
On Tue, Oct 01, 2013 at 05:16:54AM -0700, Paul E. McKenney wrote:
> On Tue, Oct 01, 2013 at 02:52:28PM +1000, Michael Neuling wrote:
> > >> Well we don't have to, I think Mikey wasn't totally clear about that
> > >> "making all registers volatile" business :-) This is just something we
> > >> need to handle in assembly if we are going to reclaim the suspended
> > >> transaction.
> >
> > Yeah, sorry. The slow path with all registers as volatile is only
> > needed if we get pre-empted during the transaction.
> >
> > >>
> > >> So basically, what we need is something along the lines of
> > >> enable_kernel_tm() which checks if there's a suspended user transaction
> > >> and if yes, kills/reclaims it.
> > >>
> > >> Then we also need to handle in our interrupt handlers that we have an
> > >> active/suspended transaction from a kernel state, which we don't deal
> > >> with at this point, and do whatever has to be done to kill it... we
> > >> might get away with something simple if we can state that we only allow
> > >> kernel transactions at task level and not from interrupt/softirq
> > >> contexts, at least initially.
> > >
> > > Call me a coward, but this is starting to sound a bit scary. ;-)
> >
> > We are just wanting to prototype it for now to see if we could make it
> > go faster. If it's worth it, then we'd consider the additional
> > complexity this would bring.
> >
> > I don't think it'll be that bad, but I'd certainly want to make sure
> > it's worth it before trying :-)
>
> OK, fair point. ;-)
That is, a fair point -assuming- that we also try the memory-barrier-free
cmpxchg that Linus suggested...
Thanx, Paul
^ permalink raw reply
* Re: Avoiding the dentry d_lock on final dput(), part deux: transactional memory
From: Paul E. McKenney @ 2013-10-01 12:16 UTC (permalink / raw)
To: Michael Neuling
Cc: Waiman Long, ppc-dev, Peter Zijlstra, George Spelvin,
Linux Kernel Mailing List, Chandramouleeswaran, Aswin,
Norton, Scott J, linux-fsdevel, Linus Torvalds, Ingo Molnar
In-Reply-To: <CAEjGV6xuscOh=t6BhbdRKAPcZ1DOJkD6ZcB1wgTf9-AedKSaxA@mail.gmail.com>
On Tue, Oct 01, 2013 at 02:52:28PM +1000, Michael Neuling wrote:
> >> Well we don't have to, I think Mikey wasn't totally clear about that
> >> "making all registers volatile" business :-) This is just something we
> >> need to handle in assembly if we are going to reclaim the suspended
> >> transaction.
>
> Yeah, sorry. The slow path with all registers as volatile is only
> needed if we get pre-empted during the transaction.
>
> >>
> >> So basically, what we need is something along the lines of
> >> enable_kernel_tm() which checks if there's a suspended user transaction
> >> and if yes, kills/reclaims it.
> >>
> >> Then we also need to handle in our interrupt handlers that we have an
> >> active/suspended transaction from a kernel state, which we don't deal
> >> with at this point, and do whatever has to be done to kill it... we
> >> might get away with something simple if we can state that we only allow
> >> kernel transactions at task level and not from interrupt/softirq
> >> contexts, at least initially.
> >
> > Call me a coward, but this is starting to sound a bit scary. ;-)
>
> We are just wanting to prototype it for now to see if we could make it
> go faster. If it's worth it, then we'd consider the additional
> complexity this would bring.
>
> I don't think it'll be that bad, but I'd certainly want to make sure
> it's worth it before trying :-)
OK, fair point. ;-)
Thanx, Paul
^ permalink raw reply
* Re: [PATCH v2 2/6] PCI/MSI: Factor out pci_get_msi_cap() interface
From: Tejun Heo @ 2013-10-01 11:55 UTC (permalink / raw)
To: Michael Ellerman
Cc: Joerg Roedel, x86@kernel.org, linux-kernel@vger.kernel.org,
linux-ide@vger.kernel.org, Alexander Gordeev, Jan Beulich,
linux-pci@vger.kernel.org, Bjorn Helgaas, linuxppc-dev,
Ingo Molnar
In-Reply-To: <20131001073548.GI17966@concordia>
Hello,
On Tue, Oct 01, 2013 at 05:35:48PM +1000, Michael Ellerman wrote:
> > > Roughly third of the drivers just do not care and bail out once
> > > pci_enable_msix() has not succeeded. Not sure how many of these are
> > > mandated by the hardware.
> >
> > Yeah, I mean, this type of interface is a trap. People have to
> > actively resist to avoid doing silly stuff which is a lot to ask.
>
> I really think you're overstating the complexity here.
>
> Functions typically return a boolean -> nothing to see here
> This function returns a tristate value -> brain explosion!
It is an interface which forces the driver writers to write
complicated fallback code which won't usually be excercised. The same
goes for the hardware. In isolation, it doesn't look like much but
things like this are bound to lead to subtle bugs which are diffiuclt
to trigger.
> > * Determine the number of MSIs the controller wants. Don't worry
> > about quotas or limits or anything. Just determine the number
> > necessary to enable enhanced interrupt handling.
> >
> > * Try allocating that number of MSIs. If it fails, then just revert
> > to single interrupt mode. It's not the end of the world and mostly
> > guaranteed to work. Let's please not even try to do partial
> > multiple interrupts. I really don't think it's worth the risk or
> > complexity.
>
> It will potentially break existing setups on our hardware.
I think it'd be much better to have a separate interface for the
drivers which actually require it *in practice* rather than forcing
everyone to go "oh this interface supports that, I don't know if I
need it but let's implement fallback logic which I won't and have no
means of testing". Are we talking about some limited number of device
drivers here? Also, is the quota still necessary for machines in
production today?
Thanks.
--
tejun
^ permalink raw reply
* Re: [RFC PATCH 00/11 Allow PR and HV KVM to coexist in one kernel
From: Alexander Graf @ 2013-10-01 11:43 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: <kvm@vger.kernel.org> list, Gleb Natapov, kvm-ppc,
Paul Mackerras, Paolo Bonzini, linuxppc-dev, Andreas Färber
In-Reply-To: <524AB3AE.5000503@suse.de>
On 10/01/2013 01:36 PM, Alexander Graf wrote:
> On 10/01/2013 01:26 PM, Aneesh Kumar K.V wrote:
>> Alexander Graf<agraf@suse.de> writes:
>>
>>> On 09/30/2013 03:09 PM, Aneesh Kumar K.V wrote:
>>>> Alexander Graf<agraf@suse.de> writes:
>>>>
>>>>> On 27.09.2013, at 12:52, Aneesh Kumar K.V wrote:
>>>>>
>>>>>> "Aneesh Kumar K.V"<aneesh.kumar@linux.vnet.ibm.com> writes:
>>>>>>
>>>>>>> Hi All,
>>>>>>>
>>>>>>> This patch series support enabling HV and PR KVM together in the
>>>>>>> same kernel. We
>>>>>>> extend machine property with new property "kvm_type". A value of
>>>>>>> 1 will force HV
>>>>>>> KVM and 2 PR KVM. The default value is 0 which will select the
>>>>>>> fastest KVM mode.
>>>>>>> ie, HV if that is supported otherwise PR.
>>>>>>>
>>>>>>> With Qemu command line having
>>>>>>>
>>>>>>> -machine pseries,accel=kvm,kvm_type=1
>>>>>>>
>>>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>>>> failed to initialize KVM: Invalid argument
>>>>>>> [root@llmp24l02 qemu]# modprobe kvm-pr
>>>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>>>> failed to initialize KVM: Invalid argument
>>>>>>> [root@llmp24l02 qemu]# modprobe kvm-hv
>>>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>>>>
>>>>>>> now with
>>>>>>>
>>>>>>> -machine pseries,accel=kvm,kvm_type=2
>>>>>>>
>>>>>>> [root@llmp24l02 qemu]# rmmod kvm-pr
>>>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>>>> failed to initialize KVM: Invalid argument
>>>>>>> [root@llmp24l02 qemu]#
>>>>>>> [root@llmp24l02 qemu]# modprobe kvm-pr
>>>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>>>>
>>>>>>> if don't specify kvm_type machine property, it will take a
>>>>>>> default value 0,
>>>>>>> which means fastest supported.
>>>>>> Related qemu patch
>>>>>>
>>>>>> commit 8d139053177d48a70cb710b211ea4c2843eccdfb
>>>>>> Author: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
>>>>>> Date: Mon Sep 23 12:28:37 2013 +0530
>>>>>>
>>>>>> kvm: Add a new machine property kvm_type
>>>>>>
>>>>>> Targets like ppc64 support different type of KVM, one which use
>>>>>> hypervisor mode and the other which doesn't. Add a new machine
>>>>>> property kvm_type that helps in selecting the respective ones
>>>>>>
>>>>>> Signed-off-by: Aneesh Kumar
>>>>>> K.V<aneesh.kumar@linux.vnet.ibm.com>
>>>>> This really is too early, as we can't possibly run in HV mode for
>>>>> non-pseries machines, so the interpretation (or at least sanity
>>>>> checking) of what values are reasonable should occur in the
>>>>> machine. That's why it's a variable in the "machine opts".
>>>> With the current code CREATE_VM will fail, because we won't have
>>>> kvm-hv.ko loaded and trying to create a vm with type 1 will fail.
>>>> Now the challenge related to moving that to machine_init or later
>>>> is, we
>>>> depend on HV or PR callback early in CREATE_VM. With the changes we
>>>> have
>>>>
>>>> int kvmppc_core_init_vm(struct kvm *kvm)
>>>> {
>>>>
>>>> #ifdef CONFIG_PPC64
>>>> INIT_LIST_HEAD(&kvm->arch.spapr_tce_tables);
>>>> INIT_LIST_HEAD(&kvm->arch.rtas_tokens);
>>>> #endif
>>>>
>>>> return kvm->arch.kvm_ops->init_vm(kvm);
>>>> }
>>>>
>>>> Also the mmu notifier callback do end up calling kvm_unmap_hva etc
>>>> which
>>>> are all HV/PR dependent.
>>> Yes, so we should verify in the machine models that we're runnable with
>>> the currently selected type at least, to give the user a sensible error
>>> message.
>> Something like the below
>
> I like that one a lot. Andreas, Paolo, what do you think?
To clarify this a bit more, this patch is missing a few critical pieces:
1) The default implementation for the kvm_type callback needs to
error out when kvm_type is set to anything
2) All non-papr-non-e500-PPC machines should force "PR only" mode and
bail out on anything else
3) We're missing sensible error messages for the user still when the
VM could not get created
but I like the overall concept :).
Alex
^ permalink raw reply
* Re: [RFC PATCH 00/11 Allow PR and HV KVM to coexist in one kernel
From: Paolo Bonzini @ 2013-10-01 11:41 UTC (permalink / raw)
To: Alexander Graf
Cc: Gleb Natapov, <kvm@vger.kernel.org> list, kvm-ppc,
Paul Mackerras, Aneesh Kumar K.V, linuxppc-dev,
Andreas Färber
In-Reply-To: <524AB3AE.5000503@suse.de>
Il 01/10/2013 13:36, Alexander Graf ha scritto:
>>>>
>>> Yes, so we should verify in the machine models that we're runnable with
>>> the currently selected type at least, to give the user a sensible error
>>> message.
>> Something like the below
>
> I like that one a lot. Andreas, Paolo, what do you think?
Yes, it's fine.
Paolo
^ permalink raw reply
* Re: [RFC PATCH 00/11 Allow PR and HV KVM to coexist in one kernel
From: Alexander Graf @ 2013-10-01 11:36 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: <kvm@vger.kernel.org> list, Gleb Natapov, kvm-ppc,
Paul Mackerras, Paolo Bonzini, linuxppc-dev, Andreas Färber
In-Reply-To: <87ob79cjvm.fsf@linux.vnet.ibm.com>
On 10/01/2013 01:26 PM, Aneesh Kumar K.V wrote:
> Alexander Graf<agraf@suse.de> writes:
>
>> On 09/30/2013 03:09 PM, Aneesh Kumar K.V wrote:
>>> Alexander Graf<agraf@suse.de> writes:
>>>
>>>> On 27.09.2013, at 12:52, Aneesh Kumar K.V wrote:
>>>>
>>>>> "Aneesh Kumar K.V"<aneesh.kumar@linux.vnet.ibm.com> writes:
>>>>>
>>>>>> Hi All,
>>>>>>
>>>>>> This patch series support enabling HV and PR KVM together in the same kernel. We
>>>>>> extend machine property with new property "kvm_type". A value of 1 will force HV
>>>>>> KVM and 2 PR KVM. The default value is 0 which will select the fastest KVM mode.
>>>>>> ie, HV if that is supported otherwise PR.
>>>>>>
>>>>>> With Qemu command line having
>>>>>>
>>>>>> -machine pseries,accel=kvm,kvm_type=1
>>>>>>
>>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>>> failed to initialize KVM: Invalid argument
>>>>>> [root@llmp24l02 qemu]# modprobe kvm-pr
>>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>>> failed to initialize KVM: Invalid argument
>>>>>> [root@llmp24l02 qemu]# modprobe kvm-hv
>>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>>>
>>>>>> now with
>>>>>>
>>>>>> -machine pseries,accel=kvm,kvm_type=2
>>>>>>
>>>>>> [root@llmp24l02 qemu]# rmmod kvm-pr
>>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>>> failed to initialize KVM: Invalid argument
>>>>>> [root@llmp24l02 qemu]#
>>>>>> [root@llmp24l02 qemu]# modprobe kvm-pr
>>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>>>
>>>>>> if don't specify kvm_type machine property, it will take a default value 0,
>>>>>> which means fastest supported.
>>>>> Related qemu patch
>>>>>
>>>>> commit 8d139053177d48a70cb710b211ea4c2843eccdfb
>>>>> Author: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
>>>>> Date: Mon Sep 23 12:28:37 2013 +0530
>>>>>
>>>>> kvm: Add a new machine property kvm_type
>>>>>
>>>>> Targets like ppc64 support different type of KVM, one which use
>>>>> hypervisor mode and the other which doesn't. Add a new machine
>>>>> property kvm_type that helps in selecting the respective ones
>>>>>
>>>>> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
>>>> This really is too early, as we can't possibly run in HV mode for
>>>> non-pseries machines, so the interpretation (or at least sanity
>>>> checking) of what values are reasonable should occur in the
>>>> machine. That's why it's a variable in the "machine opts".
>>> With the current code CREATE_VM will fail, because we won't have
>>> kvm-hv.ko loaded and trying to create a vm with type 1 will fail.
>>> Now the challenge related to moving that to machine_init or later is, we
>>> depend on HV or PR callback early in CREATE_VM. With the changes we have
>>>
>>> int kvmppc_core_init_vm(struct kvm *kvm)
>>> {
>>>
>>> #ifdef CONFIG_PPC64
>>> INIT_LIST_HEAD(&kvm->arch.spapr_tce_tables);
>>> INIT_LIST_HEAD(&kvm->arch.rtas_tokens);
>>> #endif
>>>
>>> return kvm->arch.kvm_ops->init_vm(kvm);
>>> }
>>>
>>> Also the mmu notifier callback do end up calling kvm_unmap_hva etc which
>>> are all HV/PR dependent.
>> Yes, so we should verify in the machine models that we're runnable with
>> the currently selected type at least, to give the user a sensible error
>> message.
> Something like the below
I like that one a lot. Andreas, Paolo, what do you think?
Alex
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 004184d..7d59ac1 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1337,6 +1337,21 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> assert(spapr->fdt_skel != NULL);
> }
>
> +static int spapr_get_vm_type(const char *vm_type)
> +{
> + if (!vm_type)
> + return 0;
> +
> + if (!strcmp(vm_type, "HV"))
> + return 1;
> +
> + if (!strcmp(vm_type, "PR"))
> + return 2;
> +
> + hw_error("qemu: unknown kvm_type specified '%s'", vm_type);
> + exit(1);
> +}
> +
> static QEMUMachine spapr_machine = {
> .name = "pseries",
> .desc = "pSeries Logical Partition (PAPR compliant)",
> @@ -1347,6 +1362,7 @@ static QEMUMachine spapr_machine = {
> .max_cpus = MAX_CPUS,
> .no_parallel = 1,
> .default_boot_order = NULL,
> + .get_vm_type = spapr_get_vm_type,
> };
>
> static void spapr_machine_init(void)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 5a7ae9f..2130488 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -21,6 +21,8 @@ typedef void QEMUMachineResetFunc(void);
>
> typedef void QEMUMachineHotAddCPUFunc(const int64_t id, Error **errp);
>
> +typedef int QEMUMachineGetVmTypeFunc(const char *arg);
> +
> typedef struct QEMUMachine {
> const char *name;
> const char *alias;
> @@ -28,6 +30,7 @@ typedef struct QEMUMachine {
> QEMUMachineInitFunc *init;
> QEMUMachineResetFunc *reset;
> QEMUMachineHotAddCPUFunc *hot_add_cpu;
> + QEMUMachineGetVmTypeFunc *get_vm_type;
> BlockInterfaceType block_default_type;
> int max_cpus;
> unsigned int no_serial:1,
> diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
> index e1f88bf..acc3d74 100644
> --- a/include/hw/xen/xen.h
> +++ b/include/hw/xen/xen.h
> @@ -36,7 +36,8 @@ void xen_cmos_set_s3_resume(void *opaque, int irq, int level);
>
> qemu_irq *xen_interrupt_controller_init(void);
>
> -int xen_init(void);
> +typedef struct QEMUMachine QEMUMachine;
> +int xen_init(QEMUMachine *machine);
> int xen_hvm_init(MemoryRegion **ram_memory);
> void xenstore_store_pv_console_info(int i, struct CharDriverState *chr);
>
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 9bbe3db..f25caec 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -142,8 +142,8 @@ typedef struct KVMState KVMState;
> extern KVMState *kvm_state;
>
> /* external API */
> -
> -int kvm_init(void);
> +typedef struct QEMUMachine QEMUMachine;
> +int kvm_init(QEMUMachine *machine);
>
> int kvm_has_sync_mmu(void);
> int kvm_has_vcpu_events(void);
> diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h
> index 9a0c6b3..d71343d 100644
> --- a/include/sysemu/qtest.h
> +++ b/include/sysemu/qtest.h
> @@ -31,7 +31,8 @@ static inline int qtest_available(void)
> return 1;
> }
>
> -int qtest_init(void);
> +typedef struct QEMUMachine QEMUMachine;
> +int qtest_init(QEMUMachine *machine);
> #else
> static inline bool qtest_enabled(void)
> {
> @@ -43,7 +44,7 @@ static inline int qtest_available(void)
> return 0;
> }
>
> -static inline int qtest_init(void)
> +static inline int qtest_init(QEMUMachine *machine)
> {
> return 0;
> }
> diff --git a/kvm-all.c b/kvm-all.c
> index b87215c..3863abd 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -35,6 +35,8 @@
> #include "qemu/event_notifier.h"
> #include "trace.h"
>
> +#include "hw/boards.h"
> +
> /* This check must be after config-host.h is included */
> #ifdef CONFIG_EVENTFD
> #include<sys/eventfd.h>
> @@ -1342,7 +1344,7 @@ static int kvm_max_vcpus(KVMState *s)
> return 4;
> }
>
> -int kvm_init(void)
> +int kvm_init(QEMUMachine *machine)
> {
> static const char upgrade_note[] =
> "Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n"
> @@ -1350,7 +1352,7 @@ int kvm_init(void)
> KVMState *s;
> const KVMCapabilityInfo *missing_cap;
> int ret;
> - int i;
> + int i, kvm_type = 0;
> int max_vcpus;
>
> s = g_malloc0(sizeof(KVMState));
> @@ -1407,7 +1409,11 @@ int kvm_init(void)
> goto err;
> }
>
> - s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
> + if (machine->get_vm_type) {
> + kvm_type = machine->get_vm_type(qemu_opt_get(qemu_get_machine_opts(),
> + "kvm_type"));
> + }
> + s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, kvm_type);
> if (s->vmfd< 0) {
> #ifdef TARGET_S390X
> fprintf(stderr, "Please add the 'switch_amode' kernel parameter to "
> diff --git a/kvm-stub.c b/kvm-stub.c
> index 548f471..ccb7b8c 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -19,6 +19,8 @@
> #include "hw/pci/msi.h"
> #endif
>
> +#include "hw/boards.h"
> +
> KVMState *kvm_state;
> bool kvm_kernel_irqchip;
> bool kvm_async_interrupts_allowed;
> @@ -33,7 +35,7 @@ int kvm_init_vcpu(CPUState *cpu)
> return -ENOSYS;
> }
>
> -int kvm_init(void)
> +int kvm_init(QEMUMachine *machine)
> {
> return -ENOSYS;
> }
> diff --git a/qtest.c b/qtest.c
> index 584c707..ef3c473 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -502,7 +502,7 @@ static void qtest_event(void *opaque, int event)
> }
> }
>
> -int qtest_init(void)
> +int qtest_init(QEMUMachine *machine)
> {
> CharDriverState *chr;
>
> diff --git a/vl.c b/vl.c
> index 4e709d5..7ecc581 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -427,7 +427,12 @@ static QemuOptsList qemu_machine_opts = {
> .name = "usb",
> .type = QEMU_OPT_BOOL,
> .help = "Set on/off to enable/disable usb",
> + },{
> + .name = "kvm_type",
> + .type = QEMU_OPT_STRING,
> + .help = "Set to kvm type to be used in create vm ioctl",
> },
> +
> { /* End of list */ }
> },
> };
> @@ -2608,7 +2613,7 @@ static QEMUMachine *machine_parse(const char *name)
> exit(!name || !is_help_option(name));
> }
>
> -static int tcg_init(void)
> +static int tcg_init(QEMUMachine *machine)
> {
> tcg_exec_init(tcg_tb_size * 1024 * 1024);
> return 0;
> @@ -2618,7 +2623,7 @@ static struct {
> const char *opt_name;
> const char *name;
> int (*available)(void);
> - int (*init)(void);
> + int (*init)(QEMUMachine *);
> bool *allowed;
> } accel_list[] = {
> { "tcg", "tcg", tcg_available, tcg_init,&tcg_allowed },
> @@ -2627,7 +2632,7 @@ static struct {
> { "qtest", "QTest", qtest_available, qtest_init,&qtest_allowed },
> };
>
> -static int configure_accelerator(void)
> +static int configure_accelerator(QEMUMachine *machine)
> {
> const char *p;
> char buf[10];
> @@ -2654,7 +2659,7 @@ static int configure_accelerator(void)
> continue;
> }
> *(accel_list[i].allowed) = true;
> - ret = accel_list[i].init();
> + ret = accel_list[i].init(machine);
> if (ret< 0) {
> init_failed = true;
> fprintf(stderr, "failed to initialize %s: %s\n",
> @@ -4037,10 +4042,10 @@ int main(int argc, char **argv, char **envp)
> exit(0);
> }
>
> - configure_accelerator();
> + configure_accelerator(machine);
>
> if (!qtest_enabled()&& qtest_chrdev) {
> - qtest_init();
> + qtest_init(machine);
> }
>
> machine_opts = qemu_get_machine_opts();
> diff --git a/xen-all.c b/xen-all.c
> index 839f14f..ac3654b 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -1000,7 +1000,7 @@ static void xen_exit_notifier(Notifier *n, void *data)
> xs_daemon_close(state->xenstore);
> }
>
> -int xen_init(void)
> +int xen_init(QEMUMachine *machine)
> {
> xen_xc = xen_xc_interface_open(0, 0, 0);
> if (xen_xc == XC_HANDLER_INITIAL_VALUE) {
> diff --git a/xen-stub.c b/xen-stub.c
> index ad189a6..59927cb 100644
> --- a/xen-stub.c
> +++ b/xen-stub.c
> @@ -47,7 +47,7 @@ qemu_irq *xen_interrupt_controller_init(void)
> return NULL;
> }
>
> -int xen_init(void)
> +int xen_init(QEMUMachine *machine)
> {
> return -ENOSYS;
> }
>
>>>
>>>
>>>> Also, users don't want to say type=0. They want to say type=PR or
>>>> type=HV or type=HV,PR. In fact, can't you make this a property of
>>>> -accel? Then it's truly accel specific and everything should be well.
>>> If we are doing this as machine property, we can't specify string,
>>> because "HV"/"PR" are all powerpc dependent, so parsing that is not
>>> possible in kvm_init in qemu. But, yes ideally it would be nice to be
>> Well, we could do the "name to integer" conversion in an arch specific
>> function, no?
>>
>>> able to speicy the type using string. I thought accel is a machine
>>> property, hence was not sure whether I can have additional properties
>>> against that. I was using it as below.
>>>
>>> -machine pseries,accel=kvm,kvm_type=1
> Can we really specific -accel ? I check and I am finding that as machine
> property.
>
> -aneesh
>
^ permalink raw reply
* Re: [RFC PATCH 00/11 Allow PR and HV KVM to coexist in one kernel
From: Aneesh Kumar K.V @ 2013-10-01 11:26 UTC (permalink / raw)
To: Alexander Graf
Cc: <kvm@vger.kernel.org> list, Gleb Natapov, kvm-ppc,
Paul Mackerras, Paolo Bonzini, linuxppc-dev
In-Reply-To: <524990A9.60704@suse.de>
Alexander Graf <agraf@suse.de> writes:
> On 09/30/2013 03:09 PM, Aneesh Kumar K.V wrote:
>> Alexander Graf<agraf@suse.de> writes:
>>
>>> On 27.09.2013, at 12:52, Aneesh Kumar K.V wrote:
>>>
>>>> "Aneesh Kumar K.V"<aneesh.kumar@linux.vnet.ibm.com> writes:
>>>>
>>>>> Hi All,
>>>>>
>>>>> This patch series support enabling HV and PR KVM together in the same kernel. We
>>>>> extend machine property with new property "kvm_type". A value of 1 will force HV
>>>>> KVM and 2 PR KVM. The default value is 0 which will select the fastest KVM mode.
>>>>> ie, HV if that is supported otherwise PR.
>>>>>
>>>>> With Qemu command line having
>>>>>
>>>>> -machine pseries,accel=kvm,kvm_type=1
>>>>>
>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>> failed to initialize KVM: Invalid argument
>>>>> [root@llmp24l02 qemu]# modprobe kvm-pr
>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>> failed to initialize KVM: Invalid argument
>>>>> [root@llmp24l02 qemu]# modprobe kvm-hv
>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>>
>>>>> now with
>>>>>
>>>>> -machine pseries,accel=kvm,kvm_type=2
>>>>>
>>>>> [root@llmp24l02 qemu]# rmmod kvm-pr
>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>> failed to initialize KVM: Invalid argument
>>>>> [root@llmp24l02 qemu]#
>>>>> [root@llmp24l02 qemu]# modprobe kvm-pr
>>>>> [root@llmp24l02 qemu]# bash ../qemu
>>>>>
>>>>> if don't specify kvm_type machine property, it will take a default value 0,
>>>>> which means fastest supported.
>>>> Related qemu patch
>>>>
>>>> commit 8d139053177d48a70cb710b211ea4c2843eccdfb
>>>> Author: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
>>>> Date: Mon Sep 23 12:28:37 2013 +0530
>>>>
>>>> kvm: Add a new machine property kvm_type
>>>>
>>>> Targets like ppc64 support different type of KVM, one which use
>>>> hypervisor mode and the other which doesn't. Add a new machine
>>>> property kvm_type that helps in selecting the respective ones
>>>>
>>>> Signed-off-by: Aneesh Kumar K.V<aneesh.kumar@linux.vnet.ibm.com>
>>> This really is too early, as we can't possibly run in HV mode for
>>> non-pseries machines, so the interpretation (or at least sanity
>>> checking) of what values are reasonable should occur in the
>>> machine. That's why it's a variable in the "machine opts".
>> With the current code CREATE_VM will fail, because we won't have
>> kvm-hv.ko loaded and trying to create a vm with type 1 will fail.
>> Now the challenge related to moving that to machine_init or later is, we
>> depend on HV or PR callback early in CREATE_VM. With the changes we have
>>
>> int kvmppc_core_init_vm(struct kvm *kvm)
>> {
>>
>> #ifdef CONFIG_PPC64
>> INIT_LIST_HEAD(&kvm->arch.spapr_tce_tables);
>> INIT_LIST_HEAD(&kvm->arch.rtas_tokens);
>> #endif
>>
>> return kvm->arch.kvm_ops->init_vm(kvm);
>> }
>>
>> Also the mmu notifier callback do end up calling kvm_unmap_hva etc which
>> are all HV/PR dependent.
>
> Yes, so we should verify in the machine models that we're runnable with
> the currently selected type at least, to give the user a sensible error
> message.
Something like the below
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 004184d..7d59ac1 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1337,6 +1337,21 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
assert(spapr->fdt_skel != NULL);
}
+static int spapr_get_vm_type(const char *vm_type)
+{
+ if (!vm_type)
+ return 0;
+
+ if (!strcmp(vm_type, "HV"))
+ return 1;
+
+ if (!strcmp(vm_type, "PR"))
+ return 2;
+
+ hw_error("qemu: unknown kvm_type specified '%s'", vm_type);
+ exit(1);
+}
+
static QEMUMachine spapr_machine = {
.name = "pseries",
.desc = "pSeries Logical Partition (PAPR compliant)",
@@ -1347,6 +1362,7 @@ static QEMUMachine spapr_machine = {
.max_cpus = MAX_CPUS,
.no_parallel = 1,
.default_boot_order = NULL,
+ .get_vm_type = spapr_get_vm_type,
};
static void spapr_machine_init(void)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 5a7ae9f..2130488 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -21,6 +21,8 @@ typedef void QEMUMachineResetFunc(void);
typedef void QEMUMachineHotAddCPUFunc(const int64_t id, Error **errp);
+typedef int QEMUMachineGetVmTypeFunc(const char *arg);
+
typedef struct QEMUMachine {
const char *name;
const char *alias;
@@ -28,6 +30,7 @@ typedef struct QEMUMachine {
QEMUMachineInitFunc *init;
QEMUMachineResetFunc *reset;
QEMUMachineHotAddCPUFunc *hot_add_cpu;
+ QEMUMachineGetVmTypeFunc *get_vm_type;
BlockInterfaceType block_default_type;
int max_cpus;
unsigned int no_serial:1,
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index e1f88bf..acc3d74 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -36,7 +36,8 @@ void xen_cmos_set_s3_resume(void *opaque, int irq, int level);
qemu_irq *xen_interrupt_controller_init(void);
-int xen_init(void);
+typedef struct QEMUMachine QEMUMachine;
+int xen_init(QEMUMachine *machine);
int xen_hvm_init(MemoryRegion **ram_memory);
void xenstore_store_pv_console_info(int i, struct CharDriverState *chr);
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 9bbe3db..f25caec 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -142,8 +142,8 @@ typedef struct KVMState KVMState;
extern KVMState *kvm_state;
/* external API */
-
-int kvm_init(void);
+typedef struct QEMUMachine QEMUMachine;
+int kvm_init(QEMUMachine *machine);
int kvm_has_sync_mmu(void);
int kvm_has_vcpu_events(void);
diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h
index 9a0c6b3..d71343d 100644
--- a/include/sysemu/qtest.h
+++ b/include/sysemu/qtest.h
@@ -31,7 +31,8 @@ static inline int qtest_available(void)
return 1;
}
-int qtest_init(void);
+typedef struct QEMUMachine QEMUMachine;
+int qtest_init(QEMUMachine *machine);
#else
static inline bool qtest_enabled(void)
{
@@ -43,7 +44,7 @@ static inline int qtest_available(void)
return 0;
}
-static inline int qtest_init(void)
+static inline int qtest_init(QEMUMachine *machine)
{
return 0;
}
diff --git a/kvm-all.c b/kvm-all.c
index b87215c..3863abd 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -35,6 +35,8 @@
#include "qemu/event_notifier.h"
#include "trace.h"
+#include "hw/boards.h"
+
/* This check must be after config-host.h is included */
#ifdef CONFIG_EVENTFD
#include <sys/eventfd.h>
@@ -1342,7 +1344,7 @@ static int kvm_max_vcpus(KVMState *s)
return 4;
}
-int kvm_init(void)
+int kvm_init(QEMUMachine *machine)
{
static const char upgrade_note[] =
"Please upgrade to at least kernel 2.6.29 or recent kvm-kmod\n"
@@ -1350,7 +1352,7 @@ int kvm_init(void)
KVMState *s;
const KVMCapabilityInfo *missing_cap;
int ret;
- int i;
+ int i, kvm_type = 0;
int max_vcpus;
s = g_malloc0(sizeof(KVMState));
@@ -1407,7 +1409,11 @@ int kvm_init(void)
goto err;
}
- s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, 0);
+ if (machine->get_vm_type) {
+ kvm_type = machine->get_vm_type(qemu_opt_get(qemu_get_machine_opts(),
+ "kvm_type"));
+ }
+ s->vmfd = kvm_ioctl(s, KVM_CREATE_VM, kvm_type);
if (s->vmfd < 0) {
#ifdef TARGET_S390X
fprintf(stderr, "Please add the 'switch_amode' kernel parameter to "
diff --git a/kvm-stub.c b/kvm-stub.c
index 548f471..ccb7b8c 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -19,6 +19,8 @@
#include "hw/pci/msi.h"
#endif
+#include "hw/boards.h"
+
KVMState *kvm_state;
bool kvm_kernel_irqchip;
bool kvm_async_interrupts_allowed;
@@ -33,7 +35,7 @@ int kvm_init_vcpu(CPUState *cpu)
return -ENOSYS;
}
-int kvm_init(void)
+int kvm_init(QEMUMachine *machine)
{
return -ENOSYS;
}
diff --git a/qtest.c b/qtest.c
index 584c707..ef3c473 100644
--- a/qtest.c
+++ b/qtest.c
@@ -502,7 +502,7 @@ static void qtest_event(void *opaque, int event)
}
}
-int qtest_init(void)
+int qtest_init(QEMUMachine *machine)
{
CharDriverState *chr;
diff --git a/vl.c b/vl.c
index 4e709d5..7ecc581 100644
--- a/vl.c
+++ b/vl.c
@@ -427,7 +427,12 @@ static QemuOptsList qemu_machine_opts = {
.name = "usb",
.type = QEMU_OPT_BOOL,
.help = "Set on/off to enable/disable usb",
+ },{
+ .name = "kvm_type",
+ .type = QEMU_OPT_STRING,
+ .help = "Set to kvm type to be used in create vm ioctl",
},
+
{ /* End of list */ }
},
};
@@ -2608,7 +2613,7 @@ static QEMUMachine *machine_parse(const char *name)
exit(!name || !is_help_option(name));
}
-static int tcg_init(void)
+static int tcg_init(QEMUMachine *machine)
{
tcg_exec_init(tcg_tb_size * 1024 * 1024);
return 0;
@@ -2618,7 +2623,7 @@ static struct {
const char *opt_name;
const char *name;
int (*available)(void);
- int (*init)(void);
+ int (*init)(QEMUMachine *);
bool *allowed;
} accel_list[] = {
{ "tcg", "tcg", tcg_available, tcg_init, &tcg_allowed },
@@ -2627,7 +2632,7 @@ static struct {
{ "qtest", "QTest", qtest_available, qtest_init, &qtest_allowed },
};
-static int configure_accelerator(void)
+static int configure_accelerator(QEMUMachine *machine)
{
const char *p;
char buf[10];
@@ -2654,7 +2659,7 @@ static int configure_accelerator(void)
continue;
}
*(accel_list[i].allowed) = true;
- ret = accel_list[i].init();
+ ret = accel_list[i].init(machine);
if (ret < 0) {
init_failed = true;
fprintf(stderr, "failed to initialize %s: %s\n",
@@ -4037,10 +4042,10 @@ int main(int argc, char **argv, char **envp)
exit(0);
}
- configure_accelerator();
+ configure_accelerator(machine);
if (!qtest_enabled() && qtest_chrdev) {
- qtest_init();
+ qtest_init(machine);
}
machine_opts = qemu_get_machine_opts();
diff --git a/xen-all.c b/xen-all.c
index 839f14f..ac3654b 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -1000,7 +1000,7 @@ static void xen_exit_notifier(Notifier *n, void *data)
xs_daemon_close(state->xenstore);
}
-int xen_init(void)
+int xen_init(QEMUMachine *machine)
{
xen_xc = xen_xc_interface_open(0, 0, 0);
if (xen_xc == XC_HANDLER_INITIAL_VALUE) {
diff --git a/xen-stub.c b/xen-stub.c
index ad189a6..59927cb 100644
--- a/xen-stub.c
+++ b/xen-stub.c
@@ -47,7 +47,7 @@ qemu_irq *xen_interrupt_controller_init(void)
return NULL;
}
-int xen_init(void)
+int xen_init(QEMUMachine *machine)
{
return -ENOSYS;
}
>
>>
>>
>>
>>> Also, users don't want to say type=0. They want to say type=PR or
>>> type=HV or type=HV,PR. In fact, can't you make this a property of
>>> -accel? Then it's truly accel specific and everything should be well.
>> If we are doing this as machine property, we can't specify string,
>> because "HV"/"PR" are all powerpc dependent, so parsing that is not
>> possible in kvm_init in qemu. But, yes ideally it would be nice to be
>
> Well, we could do the "name to integer" conversion in an arch specific
> function, no?
>
>> able to speicy the type using string. I thought accel is a machine
>> property, hence was not sure whether I can have additional properties
>> against that. I was using it as below.
>>
>> -machine pseries,accel=kvm,kvm_type=1
Can we really specific -accel ? I check and I am finding that as machine
property.
-aneesh
^ permalink raw reply related
* Re: [PATCH 3/3] KVM: PPC: Book3S: Add support for hwrng found on some powernv systems
From: Paolo Bonzini @ 2013-10-01 11:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: tytso, kvm, Gleb Natapov, linuxppc-dev, linux-kernel, kvm-ppc,
agraf, herbert, Paul Mackerras, mpm
In-Reply-To: <1380620338.645.22.camel@pasglop>
Il 01/10/2013 11:38, Benjamin Herrenschmidt ha scritto:
> So for the sake of that dogma you are going to make us do something that
> is about 100 times slower ? (and possibly involves more lines of code)
If it's 100 times slower there is something else that's wrong. It's
most likely not 100 times slower, and this makes me wonder if you or
Michael actually timed the code at all.
> It's not just speed ... H_RANDOM is going to be called by the guest
> kernel. A round trip to qemu is going to introduce a kernel jitter
> (complete stop of operations of the kernel on that virtual processor) of
> a full exit + round trip to qemu + back to the kernel to get to some
> source of random number ... this is going to be in the dozens of ns at
> least.
I guess you mean dozens of *micro*seconds, which is somewhat exaggerated
but not too much. On x86 some reasonable timings are:
100 cycles bare metal rdrand
2000 cycles guest->hypervisor->guest
15000 cycles guest->userspace->guest
(100 cycles = 40 ns = 200 MB/sec; 2000 cycles = ~1 microseconds; 15000
cycles = ~7.5 microseconds). Even on 5 year old hardware, a userspace
roundtrip is around a dozen microseconds.
Anyhow, I would like to know more about this hwrng and hypercall.
Does the hwrng return random numbers (like rdrand) or real entropy (like
rdseed that Intel will add in Broadwell)? What about the hypercall?
For example virtio-rng is specified to return actual entropy, it doesn't
matter if it is from hardware or software.
In either case, the patches have problems.
1) If the hwrng returns random numbers, the whitening you're doing is
totally insufficient and patch 2 is forging entropy that doesn't exist.
2) If the hwrng returns entropy, a read from the hwrng is going to even
more expensive than an x86 rdrand (perhaps ~2000 cycles). Hence, doing
the emulation in the kernel is even less necessary. Also, if the hwrng
returns entropy patch 1 is unnecessary: you do not need to waste
precious entropy bits by passing them to arch_get_random_long; just run
rngd in the host as that will put the entropy to much better use.
3) If the hypercall returns random numbers, then it is a pretty
braindead interface since returning 8 bytes at a time limits the
throughput to a handful of MB/s (compare to 200 MB/sec for x86 rdrand).
But more important: in this case drivers/char/hw_random/pseries-rng.c
is completely broken and insecure, just like patch 2 in case (1) above.
4) If the hypercall returns entropy (same as virtio-rng), the same
considerations on speed apply. If you can only produce entropy at say 1
MB/s (so reading 8 bytes take 8 microseconds---which is actually very
fast), it doesn't matter that much to spend 7 microseconds on a
userspace roundtrip. It's going to be only half the speed of bare
metal, not 100 times slower.
Also, you will need _anyway_ extra code that is not present here to
either disable the rng based on userspace command-line, or to emulate
the rng from userspace. It is absolutely _not_ acceptable to have a
hypercall disappear across migration. You're repeatedly ignoring these
issues, but rest assured that they will come back and bite you
spectacularly.
Based on all this, I would simply ignore the part of the spec where they
say "the hypercall should return numbers from a hardware source". All
that matters in virtualization is to have a good source of _entropy_.
Then you can run rngd without randomness checks, which will more than
recover the cost of userspace roundtrips.
In any case, deciding where to get that entropy from is definitely
outside the scope of KVM, and in fact QEMU already has a configurable
mechanism for that.
Paolo
^ permalink raw reply
* Re: [PATCH v2 2/6] PCI/MSI: Factor out pci_get_msi_cap() interface
From: Alexander Gordeev @ 2013-10-01 10:35 UTC (permalink / raw)
To: Michael Ellerman
Cc: linuxppc-dev, Joerg Roedel, x86@kernel.org,
linux-kernel@vger.kernel.org, linux-ide@vger.kernel.org,
Jan Beulich, linux-pci@vger.kernel.org, Tejun Heo, Bjorn Helgaas,
Ingo Molnar
In-Reply-To: <20131001075133.GJ17966@concordia>
On Tue, Oct 01, 2013 at 05:51:33PM +1000, Michael Ellerman wrote:
> The disadvantage is that any restriction imposed on us above the quota
> can only be reported as an error from pci_enable_msix().
>
> The quota code, called from pci_get_msix_limit(), can only do so much to
> interogate firmware about the limitations. The ultimate way to check if
> firmware will give us enough MSIs is to try and allocate them. But we
> can't do that from pci_get_msix_limit() because the driver is not asking
> us to enable MSIs, just query them.
If things are this way then pci_enable_msix() already exposed to this
problem internally on pSeries.
I see that even successful quota checks in rtas_msi_check_device() and
rtas_setup_msi_irqs() do not guarantee (as you say) that firmware will
give enough MSIs. Hence, pci_enable_msix() might fail even though the
its quota checks succeeded.
Therefore, nothing will really change if we make pci_get_msix_limit() check
quota and hope the follow-up call to pci_enable_msix() succeeded.
(Of course, we could allocate-deallocate MSIs at check time, but I think it
is an overkill).
> You'll also need to add another arch hook, for the quota check, and
> we'll have to add it to our per-platform indirection as well.
Already, in a branch, hidden from Bjorn & Tejun eyes ;)
> All a lot of bother for no real gain IMHO.
Well, I do not have a strong opinion here. I leave it to the ones who have :)
But few drivers have became clearer as result of this change (and messy ones
are still messy).
> cheers
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply
* Re: [PATCH 3/3] KVM: PPC: Book3S: Add support for hwrng found on some powernv systems
From: Alexander Graf @ 2013-10-01 10:00 UTC (permalink / raw)
To: Paul Mackerras
Cc: tytso, kvm, Gleb Natapov, linuxppc-dev, linux-kernel, kvm-ppc,
herbert, mpm, Paolo Bonzini
In-Reply-To: <20131001092320.GA2000@iris.ozlabs.ibm.com>
On 10/01/2013 11:23 AM, Paul Mackerras wrote:
> On Tue, Oct 01, 2013 at 11:39:08AM +0300, Gleb Natapov wrote:
>> On Tue, Oct 01, 2013 at 06:34:26PM +1000, Michael Ellerman wrote:
>>> On Thu, Sep 26, 2013 at 11:06:59AM +0200, Paolo Bonzini wrote:
>>>> Il 26/09/2013 08:31, Michael Ellerman ha scritto:
>>>>> Some powernv systems include a hwrng. Guests can access it via the
>>>>> H_RANDOM hcall.
>>>> Is there any reason to do this in the kernel?
>>> It's less code, and it's faster :)
>>>
>>>> It does not have to be a particularly fast path;
>>> Sure, but do we have to make it slow on purpose?
>>>
>> We do not put non performance critical devices into the kernel.
> It's not a device, it's a single hypercall, specified by PAPR, which
> is the moral equivalent of x86's RDRAND.
Yes, and hypercalls should be handled in user space unless impossible
otherwise (like MMU hypercalls which modify state that user space has no
priviledge to access).
I think the most reasonable way forward would be to implement the path
that jumps through hoops and goes through user space, then add a new
device in kvm that registers on this hcall inside of kvm.
That way we ensure consistency (user space knows what to put into device
tree, can disable it if it wants to, can run with TCG, etc) and you can
prove that your user space interface works along the way.
Alex
^ permalink raw reply
* Re: [PATCH 3/3] KVM: PPC: Book3S: Add support for hwrng found on some powernv systems
From: Gleb Natapov @ 2013-10-01 9:57 UTC (permalink / raw)
To: Paul Mackerras
Cc: tytso, kvm, linuxppc-dev, agraf, kvm-ppc, linux-kernel, herbert,
mpm, Paolo Bonzini
In-Reply-To: <20131001092320.GA2000@iris.ozlabs.ibm.com>
On Tue, Oct 01, 2013 at 07:23:20PM +1000, Paul Mackerras wrote:
> On Tue, Oct 01, 2013 at 11:39:08AM +0300, Gleb Natapov wrote:
> > On Tue, Oct 01, 2013 at 06:34:26PM +1000, Michael Ellerman wrote:
> > > On Thu, Sep 26, 2013 at 11:06:59AM +0200, Paolo Bonzini wrote:
> > > > Il 26/09/2013 08:31, Michael Ellerman ha scritto:
> > > > > Some powernv systems include a hwrng. Guests can access it via the
> > > > > H_RANDOM hcall.
> > > >
> > > > Is there any reason to do this in the kernel?
> > >
> > > It's less code, and it's faster :)
> > >
> > > > It does not have to be a particularly fast path;
> > >
> > > Sure, but do we have to make it slow on purpose?
> > >
> > We do not put non performance critical devices into the kernel.
>
> It's not a device, it's a single hypercall, specified by PAPR, which
> is the moral equivalent of x86's RDRAND.
>
OK. A couple of general questions. How guest knows when this hypercall
is available? Is QEMU enable it when KVM_CAP_PPC_HWRNG is available (I
haven't seen userspace patch that uses KVM_CAP_PPC_HWRNG)? What about
QEMU TCG does it implement this hypercall or it emulates hwrng directly?
--
Gleb.
^ 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