* Re: [PATCH v3 9/9] Documentation/powerpc: VAS API
From: Daniel Axtens @ 2020-03-20 12:24 UTC (permalink / raw)
To: Haren Myneni, herbert; +Cc: mikey, sukadev, linuxppc-dev, linux-crypto, npiggin
In-Reply-To: <1583541541.9256.50.camel@hbabu-laptop>
Hi Haren,
This is good documentation.
> Power9 introduced Virtual Accelerator Switchboard (VAS) which allows
> userspace to communicate with Nest Accelerator (NX) directly. But
> kernel has to establish channel to NX for userspace. This document
> describes user space API that application can use to establish
> communication channel.
>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
> Documentation/powerpc/index.rst | 1 +
> Documentation/powerpc/vas-api.rst | 246 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 247 insertions(+)
> create mode 100644 Documentation/powerpc/vas-api.rst
>
> diff --git a/Documentation/powerpc/index.rst b/Documentation/powerpc/index.rst
> index 0d45f0f..afe2d5e 100644
> --- a/Documentation/powerpc/index.rst
> +++ b/Documentation/powerpc/index.rst
> @@ -30,6 +30,7 @@ powerpc
> syscall64-abi
> transactional_memory
> ultravisor
> + vas-api
>
> .. only:: subproject and html
>
> diff --git a/Documentation/powerpc/vas-api.rst b/Documentation/powerpc/vas-api.rst
> new file mode 100644
> index 0000000..13ce4e7
> --- /dev/null
> +++ b/Documentation/powerpc/vas-api.rst
> @@ -0,0 +1,246 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +.. _VAS-API:
> +
> +===================================================
> +Virtual Accelerator Switchboard (VAS) userspace API
> +===================================================
> +
> +Introduction
> +============
> +
> +Power9 processor introduced Virtual Accelerator Switchboard (VAS) which
> +allows both userspace and kernel communicate to co-processor
> +(hardware accelerator) referred to as the Nest Accelerator (NX). The NX
> +unit comprises of one or more hardware engines or co-processor types
> +such as 842 compression, GZIP compression and encryption. On power9,
> +userspace applications will have access to only GZIP Compression engine
> +which supports ZLIB and GZIP compression algorithms in the hardware.
> +
> +To communicate with NX, kernel has to establish a channel or window and
> +then requests can be submitted directly without kernel involvement.
> +Requests to the GZIP engine must be formatted as a co-processor Request
> +Block (CRB) and these CRBs must be submitted to the NX using COPY/PASTE
> +instructions to paste the CRB to hardware address that is associated with
> +the engine's request queue.
> +
> +The GZIP engine provides two priority levels of requests: Normal and
> +High. Only Normal requests are supported from userspace right now.
> +
> +This document explains userspace API that is used to interact with
> +kernel to setup channel / window which can be used to send compression
> +requests directly to NX accelerator.
> +
> +
> +Overview
> +========
> +
> +Application access to the GZIP engine is provided through
> +/dev/crypto/nx-gzip device node implemented by the VAS/NX device driver.
> +An application must open the /dev/crypto/nx-gzip device to obtain a file
> +descriptor (fd). Then should issue VAS_TX_WIN_OPEN ioctl with this fd to
> +establish connection to the engine. It means send window is opened on GZIP
> +engine for this process. Once a connection is established, the application
> +should use the mmap() system call to map the hardware address of engine's
> +request queue into the application's virtual address space.
> +
> +The application can then submit one or more requests to the the engine by
> +using copy/paste instructions and pasting the CRBs to the virtual address
> +(aka paste_address) returned by mmap(). User space can close the
> +established connection or send window by closing the file descriptior
> +(close(fd)) or upon the process exit.
> +
> +Note that applications can send several requests with the same window or
> +can establish multiple windows, but one window for each file descriptor.
> +
> +Following sections provide additional details and references about the
> +individual steps.
> +
> +NX-GZIP Device Node
> +===================
> +
> +There is one /dev/crypto/nx-gzip node in the system and it provides
> +access to all GZIP engines in the system. The only valid operations on
> +/dev/crypto/nx-gzip are:
> +
> + * open() the device for read and write.
> + * issue VAS_TX_WIN_OPEN ioctl
> + * mmap() the engine's request queue into application's virtual
> + address space (i.e. get a paste_address for the co-processor
> + engine).
> + * close the device node.
> +
> +Other file operations on this device node are undefined.
> +
> +Note that the copy and paste operations go directly to the hardware and
> +do not go through this device. Refer COPY/PASTE document for more
> +details.
> +
> +Although a system may have several instances of the NX co-processor
> +engines (typically, one per P9 chip) there is just one
> +/dev/crypto/nx-gzip device node in the system. When the nx-gzip device
> +node is opened, Kernel opens send window on a suitable instance of NX
> +accelerator. It finds CPU on which the user process is executing and
> +determine the NX instance for the corresponding chip on which this CPU
> +belongs.
> +
> +Applications may chose a specific instance of the NX co-processor using
> +the vas_id field in the VAS_TX_WIN_OPEN ioctl as detailed below.
> +
> +A userspace library libnxz is available here but still in development:
> + https://github.com/abalib/power-gzip
> +
> +Applications that use inflate / deflate calls can link with libnxz
> +instead of libz and use NX GZIP compression without any modification.
> +
> +Open /dev/crypto/nx-gzip
> +========================
> +
> +The nx-gzip device should be opened for read and write. No special
> +privileges are needed to open the device. Each window coreesponds to one
s/coreesponds/corresponds/
> +file descriptor. So if the userspace process needs multiple windows,
> +several open calls have to be issued.
> +
> +See open(2) system call man pages for other details such as return values,
> +error codes and restrictions.
> +codes and restrictions.
You have 'codes and restrictions' twice here.
> +
> +VAS_TX_WIN_OPEN ioctl
> +=====================
> +
> +Applications should use the VAS_TX_WIN_OPEN ioctl as follows to establish
> +a connection with NX co-processor engine:
> +
> + ::
> + struct vas_tx_win_open_attr {
> + __u32 version;
> + __s16 vas_id; /* specific instance of vas or -1
> + for default */
> + __u16 reserved1;
> + __u64 flags; /* For future use */
> + __u64 reserved2[6];
> + };
> +
> + version: The version field must be currently set to 1.
> + vas_id: If '-1' is passed, kernel will make a best-effort attempt
> + to assign an optimal instance of NX for the process. To
> + select the specific VAS instance, refer
> + "Discovery of available VAS engines" section below.
> +
> + flags, reserved1 and reserved2[6] fields are for future extension
> + and must be set to 0.
> +
> + The attributes attr for the VAS_TX_WIN_OPEN ioctl are defined as
> + follows:
> + #define VAS_MAGIC 'v'
> + #define VAS_TX_WIN_OPEN _IOW(VAS_MAGIC, 1,
> + struct vas_tx_win_open_attr)
> +
> + struct vas_tx_win_open_attr attr;
> + rc = ioctl(fd, VAS_TX_WIN_OPEN, &attr);
> +
> + The VAS_TX_WIN_OPEN ioctl returns 0 on success. On errors, it
> + returns -1 and sets the errno variable to indicate the error.
> +
> + Error conditions:
> + EINVAL fd does not refer to a valid VAS device.
> + EINVAL Invalid vas ID
> + EINVAL version is not set with proper value
> + EEXIST Window is already opened for the given fd
> + ENOMEM Memory is not available to allocate window
> + ENOSPC System has too many active windows (connections)
> + opened
> + EINVAL reserved fields are not set to 0.
> +
> + See the ioctl(2) man page for more details, error codes and
> + restrictions.
> +
> +mmap() NX-GZIP device
> +=====================
> +
> +The mmap() system call for a NX-GZIP device fd returns a paste_address
> +that the application can use to copy/paste its CRB to the hardware engines.
> + ::
> +
> + paste_addr = mmap(addr, size, prot, flags, fd, offset);
> +
> + Only restrictions on mmap for a NX-GZIP device fd are:
> + * size should be 4K page size
Patch 3 seems to allow a 64k page if the system is compiled for 64k
pages... Should it restrict it to 4K?
> + * offset parameter should be 0ULL
> +
> + Refer to mmap(2) man page for additional details/restrictions.
> + In addition to the error conditions listed on the mmap(2) man
> + page, can also fail with one of the following error codes:
> +
> + EINVAL fd is not associated with an open window
> + (i.e mmap() does not follow a successful call
> + to the VAS_TX_WIN_OPEN ioctl).
> + EINVAL offset field is not 0ULL.
> +
> +Discovery of available VAS engines
> +==================================
> +
> +Each available VAS instance in the system will have a device tree node
> +like /proc/device-tree/vas@* or /proc/device-tree/xscom@*/vas@*.
> +Determine the chip or VAS instance and use the corresponding ibm,vas-id
> +property value in this node to select specific VAS instance.
> +
> +Copy/Paste operations
> +=====================
> +
> +Applications should use the copy and paste instructions defined in the RFC
> +to copy/paste the CRB.
In which RFC?
> +
> +CRB Specification and use NX
> +============================
> +
> +Applications should format requests to the co-processor using the
> +co-processor Request Block (CRBs). Refer NX workbook for the format of
> +CRB and use NX from userspace such as sending requests and checking
> +request status.
Where would someone find the NX workbook?
Regards,
Daniel
> +
> +Simple example
> +==============
> +
> + ::
> + int use_nx_gzip()
> + {
> + int rc, fd;
> + void *addr;
> + struct vas_setup_attr txattr;
> +
> + fd = open("/dev/crypto/nx-gzip", O_RDWR);
> + if (fd < 0) {
> + fprintf(stderr, "open nx-gzip failed\n");
> + return -1;
> + }
> + memset(&txattr, 0, sizeof(txattr));
> + txattr.version = 1;
> + txattr.vas_id = -1
> + rc = ioctl(fd, VAS_TX_WIN_OPEN,
> + (unsigned long)&txattr);
> + if (rc < 0) {
> + fprintf(stderr, "ioctl() n %d, error %d\n",
> + rc, errno);
> + return rc;
> + }
> + addr = mmap(NULL, 4096, PROT_READ|PROT_WRITE,
> + MAP_SHARED, fd, 0ULL);
> + if (addr == MAP_FAILED) {
> + fprintf(stderr, "mmap() failed, errno %d\n",
> + errno);
> + return -errno;
> + }
> + do {
> + //Format CRB request with compression or
> + //uncompression
> + // Refer tests for vas_copy/vas_paste
> + vas_copy((&crb, 0, 1);
> + vas_paste(addr, 0, 1);
> + // Poll on csb.flags with timeout
> + // csb address is listed in CRB
> + } while (true)
> + close(fd) or window can be closed upon process exit
> + }
> +
> + Refer https://github.com/abalib/power-gzip for tests or more
> + use cases.
> --
> 1.8.3.1
^ permalink raw reply
* Re: [PATCH v12 8/8] MAINTAINERS: perf: Add pattern that matches ppc perf to the perf entry.
From: Andy Shevchenko @ 2020-03-20 12:42 UTC (permalink / raw)
To: Michal Suchánek
Cc: Mark Rutland, Gustavo Luiz Duarte, Peter Zijlstra,
Sebastian Andrzej Siewior, linux-kernel, Paul Mackerras,
Jiri Olsa, Rob Herring, Michael Neuling, Mauro Carvalho Chehab,
Masahiro Yamada, Nayna Jain, Alexander Shishkin, Ingo Molnar,
Allison Randal, Jordan Niethe, Valentin Schneider, Arnd Bergmann,
Arnaldo Carvalho de Melo, Alexander Viro, Jonathan Cameron,
Namhyung Kim, Thomas Gleixner, Hari Bathini, Greg Kroah-Hartman,
Nicholas Piggin, Claudio Carvalho, Eric Richter,
Eric W. Biederman, linux-fsdevel, linuxppc-dev, David S. Miller,
Thiago Jung Bauermann
In-Reply-To: <20200320112338.GP25468@kitsune.suse.cz>
On Fri, Mar 20, 2020 at 12:23:38PM +0100, Michal Suchánek wrote:
> On Fri, Mar 20, 2020 at 12:33:50PM +0200, Andy Shevchenko wrote:
> > On Fri, Mar 20, 2020 at 11:20:19AM +0100, Michal Suchanek wrote:
> > > While at it also simplify the existing perf patterns.
> > And still missed fixes from parse-maintainers.pl.
>
> Oh, that script UX is truly ingenious.
You have at least two options, their combinations, etc:
- complain to the author :-)
- send a patch :-)
> It provides no output and quietly
> creates MAINTAINERS.new which is, of course, not included in the patch.
Yes. it also took me a while to understand how it works, luckily it has a
little help note.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v6 00/11] powerpc/perf: Add json file metric support for the hv_24x7 socket/chip level events
From: Kajol Jain @ 2020-03-20 12:53 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
Patchset fixes the inconsistent results we are getting when
we run multiple 24x7 events.
Patchset adds json file metric support for the hv_24x7 socket/chip level
events. "hv_24x7" pmu interface events needs system dependent parameter
like socket/chip/core. For example, hv_24x7 chip level events needs
specific chip-id to which the data is requested should be added as part
of pmu events.
So to enable JSON file support to "hv_24x7" interface, patchset expose
total number of sockets and chips per-socket details in sysfs
files (sockets, chips) under "/sys/devices/hv_24x7/interface/".
To get sockets and number of chips per sockets, patchset adds a rtas call
with token "PROCESSOR_MODULE_INFO" to get these details. Patchset also
handles partition migration case to re-init these system depended
parameters by adding proper calls in post_mobility_fixup() (mobility.c).
Second patch of the patchset adds expr_scanner_ctx object to hold user
data for the expr scanner, which can be used to hold runtime parameter.
Patch 9 & 11 of the patchset handles perf tool plumbing needed to replace
the "?" character in the metric expression to proper value and hv_24x7
json metric file for different Socket/chip resources.
Patch set also enable Hz/hz prinitg for --metric-only option to print
metric data for bus frequency.
Applied and tested all these patches cleanly on top of jiri's flex changes
with the changes done by Kan Liang for "Support metric group constraint"
patchset and made required changes.
Changelog:
v5 -> v6
- resolve compilation issue due to rearranging patch series.
- Rather then adding new function to take careof case for runtime param
in metricgroup__add_metric, using metricgroup__add_metric_param itself
for that work.
- Address some optimization suggested like using directly file path
rather then adding new macro in header.c
- Change commit message on patch where we are adding "?" support
by adding simple example.
v4 -> v5
- Using sysfs__read_int instead of sysfs__read_ull while reading
parameter value in powerpc/util/header.c file.
- Using asprintf rather then malloc and sprintf
Suggested by Arnaldo Carvalho de Melo
- Break patch 6 from previous version to two patch,
- One to add refactor current "metricgroup__add_metric" function
and another where actually "?" handling infra added.
- Add expr__runtimeparam as part of 'expr_scanner_ctx' struct
rather then making it global variable. Thanks Jiri for
adding this structure to hold user data for the expr scanner.
- Add runtime param as agrugement to function 'expr__find_other'
and 'expr__parse' and made changes on references accordingly.
v3 -> v4
- Apply these patch on top of Kan liang changes.
As suggested by Jiri.
v2 -> v3
- Remove setting event_count to 0 part in function 'h_24x7_event_read'
with comment rather then adding 0 to event_count value.
Suggested by: Sukadev Bhattiprolu
- Apply tool side changes require to replace "?" on Jiri's flex patch
series and made all require changes to make it compatible with added
flex change.
v1 -> v2
- Rename hv-24x7 metric json file as nest_metrics.json
Jiri Olsa (2):
perf expr: Add expr_ prefix for parse_ctx and parse_id
perf expr: Add expr_scanner_ctx object
Kajol Jain (9):
powerpc/perf/hv-24x7: Fix inconsistent output values incase multiple
hv-24x7 events run
powerpc/hv-24x7: Add rtas call in hv-24x7 driver to get processor
details
powerpc/hv-24x7: Add sysfs files inside hv-24x7 device to show
processor details
Documentation/ABI: Add ABI documentation for chips and sockets
powerpc/hv-24x7: Update post_mobility_fixup() to handle migration
perf/tools: Refactoring metricgroup__add_metric function
perf/tools: Enhance JSON/metric infrastructure to handle "?"
tools/perf: Enable Hz/hz prinitg for --metric-only option
perf/tools/pmu-events/powerpc: Add hv_24x7 socket/chip level metric
events
.../sysfs-bus-event_source-devices-hv_24x7 | 14 +++
arch/powerpc/perf/hv-24x7.c | 104 ++++++++++++++++--
arch/powerpc/platforms/pseries/mobility.c | 12 ++
arch/powerpc/platforms/pseries/pseries.h | 3 +
tools/perf/arch/powerpc/util/header.c | 8 ++
.../arch/powerpc/power9/nest_metrics.json | 19 ++++
tools/perf/tests/expr.c | 12 +-
tools/perf/util/expr.c | 25 +++--
tools/perf/util/expr.h | 19 ++--
tools/perf/util/expr.l | 37 +++++--
tools/perf/util/expr.y | 6 +-
tools/perf/util/metricgroup.c | 88 +++++++++++----
tools/perf/util/metricgroup.h | 1 +
tools/perf/util/stat-display.c | 2 -
tools/perf/util/stat-shadow.c | 14 ++-
15 files changed, 287 insertions(+), 77 deletions(-)
create mode 100644 tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
--
2.18.1
^ permalink raw reply
* [PATCH v6 01/11] perf expr: Add expr_ prefix for parse_ctx and parse_id
From: Kajol Jain @ 2020-03-20 12:53 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200320125406.30995-1-kjain@linux.ibm.com>
From: Jiri Olsa <jolsa@kernel.org>
Adding expr_ prefix for parse_ctx and parse_id,
to straighten out the expr* namespace.
There's no functional change.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/tests/expr.c | 4 ++--
tools/perf/util/expr.c | 10 +++++-----
tools/perf/util/expr.h | 12 ++++++------
tools/perf/util/expr.y | 6 +++---
tools/perf/util/stat-shadow.c | 2 +-
5 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index 28313e59d6f6..ea10fc4412c4 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -6,7 +6,7 @@
#include <string.h>
#include <linux/zalloc.h>
-static int test(struct parse_ctx *ctx, const char *e, double val2)
+static int test(struct expr_parse_ctx *ctx, const char *e, double val2)
{
double val;
@@ -22,7 +22,7 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
const char **other;
double val;
int i, ret;
- struct parse_ctx ctx;
+ struct expr_parse_ctx ctx;
int num_other;
expr__ctx_init(&ctx);
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index fd192ddf93c1..c8ccc548a585 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -11,7 +11,7 @@ extern int expr_debug;
#endif
/* Caller must make sure id is allocated */
-void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
+void expr__add_id(struct expr_parse_ctx *ctx, const char *name, double val)
{
int idx;
@@ -21,13 +21,13 @@ void expr__add_id(struct parse_ctx *ctx, const char *name, double val)
ctx->ids[idx].val = val;
}
-void expr__ctx_init(struct parse_ctx *ctx)
+void expr__ctx_init(struct expr_parse_ctx *ctx)
{
ctx->num_ids = 0;
}
static int
-__expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
+__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
int start)
{
YY_BUFFER_STATE buffer;
@@ -52,7 +52,7 @@ __expr__parse(double *val, struct parse_ctx *ctx, const char *expr,
return ret;
}
-int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr)
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr)
{
return __expr__parse(final_val, ctx, expr, EXPR_PARSE) ? -1 : 0;
}
@@ -75,7 +75,7 @@ int expr__find_other(const char *expr, const char *one, const char ***other,
int *num_other)
{
int err, i = 0, j = 0;
- struct parse_ctx ctx;
+ struct expr_parse_ctx ctx;
expr__ctx_init(&ctx);
err = __expr__parse(NULL, &ctx, expr, EXPR_OTHER);
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 9377538f4097..b9e53f2b5844 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -5,19 +5,19 @@
#define EXPR_MAX_OTHER 20
#define MAX_PARSE_ID EXPR_MAX_OTHER
-struct parse_id {
+struct expr_parse_id {
const char *name;
double val;
};
-struct parse_ctx {
+struct expr_parse_ctx {
int num_ids;
- struct parse_id ids[MAX_PARSE_ID];
+ struct expr_parse_id ids[MAX_PARSE_ID];
};
-void expr__ctx_init(struct parse_ctx *ctx);
-void expr__add_id(struct parse_ctx *ctx, const char *id, double val);
-int expr__parse(double *final_val, struct parse_ctx *ctx, const char *expr);
+void expr__ctx_init(struct expr_parse_ctx *ctx);
+void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
int expr__find_other(const char *expr, const char *one, const char ***other,
int *num_other);
diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y
index 4720cbe79357..cd17486c1c5d 100644
--- a/tools/perf/util/expr.y
+++ b/tools/perf/util/expr.y
@@ -15,7 +15,7 @@
%define api.pure full
%parse-param { double *final_val }
-%parse-param { struct parse_ctx *ctx }
+%parse-param { struct expr_parse_ctx *ctx }
%parse-param {void *scanner}
%lex-param {void* scanner}
@@ -39,14 +39,14 @@
%{
static void expr_error(double *final_val __maybe_unused,
- struct parse_ctx *ctx __maybe_unused,
+ struct expr_parse_ctx *ctx __maybe_unused,
void *scanner,
const char *s)
{
pr_debug("%s\n", s);
}
-static int lookup_id(struct parse_ctx *ctx, char *id, double *val)
+static int lookup_id(struct expr_parse_ctx *ctx, char *id, double *val)
{
int i;
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 0fd713d3674f..402af3e8d287 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -729,7 +729,7 @@ static void generic_metric(struct perf_stat_config *config,
struct runtime_stat *st)
{
print_metric_t print_metric = out->print_metric;
- struct parse_ctx pctx;
+ struct expr_parse_ctx pctx;
double ratio, scale;
int i;
void *ctxp = out->ctx;
--
2.18.1
^ permalink raw reply related
* [PATCH v6 02/11] perf expr: Add expr_scanner_ctx object
From: Kajol Jain @ 2020-03-20 12:53 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200320125406.30995-1-kjain@linux.ibm.com>
From: Jiri Olsa <jolsa@kernel.org>
Adding expr_scanner_ctx object to hold user data
for the expr scanner. Currently it holds only
start_token, Kajol Jain will use it to hold 24x7
runtime param.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/expr.c | 6 ++++--
tools/perf/util/expr.h | 4 ++++
tools/perf/util/expr.l | 10 +++++-----
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index c8ccc548a585..c3382d58cf40 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -3,7 +3,6 @@
#include <assert.h>
#include "expr.h"
#include "expr-bison.h"
-#define YY_EXTRA_TYPE int
#include "expr-flex.h"
#ifdef PARSER_DEBUG
@@ -30,11 +29,14 @@ static int
__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
int start)
{
+ struct expr_scanner_ctx scanner_ctx = {
+ .start_token = start,
+ };
YY_BUFFER_STATE buffer;
void *scanner;
int ret;
- ret = expr_lex_init_extra(start, &scanner);
+ ret = expr_lex_init_extra(&scanner_ctx, &scanner);
if (ret)
return ret;
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index b9e53f2b5844..0938ad166ece 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -15,6 +15,10 @@ struct expr_parse_ctx {
struct expr_parse_id ids[MAX_PARSE_ID];
};
+struct expr_scanner_ctx {
+ int start_token;
+};
+
void expr__ctx_init(struct expr_parse_ctx *ctx);
void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
diff --git a/tools/perf/util/expr.l b/tools/perf/util/expr.l
index eaad29243c23..2582c2464938 100644
--- a/tools/perf/util/expr.l
+++ b/tools/perf/util/expr.l
@@ -76,13 +76,13 @@ sym [0-9a-zA-Z_\.:@]+
symbol {spec}*{sym}*{spec}*{sym}*
%%
- {
- int start_token;
+ struct expr_scanner_ctx *sctx = expr_get_extra(yyscanner);
- start_token = expr_get_extra(yyscanner);
+ {
+ int start_token = sctx->start_token;
- if (start_token) {
- expr_set_extra(NULL, yyscanner);
+ if (sctx->start_token) {
+ sctx->start_token = 0;
return start_token;
}
}
--
2.18.1
^ permalink raw reply related
* [PATCH v6 03/11] powerpc/perf/hv-24x7: Fix inconsistent output values incase multiple hv-24x7 events run
From: Kajol Jain @ 2020-03-20 12:53 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200320125406.30995-1-kjain@linux.ibm.com>
Commit 2b206ee6b0df ("powerpc/perf/hv-24x7: Display change in counter
values")' added to print _change_ in the counter value rather then raw
value for 24x7 counters. Incase of transactions, the event count
is set to 0 at the beginning of the transaction. It also sets
the event's prev_count to the raw value at the time of initialization.
Because of setting event count to 0, we are seeing some weird behaviour,
whenever we run multiple 24x7 events at a time.
For example:
command#: ./perf stat -e "{hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/,
hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/}"
-C 0 -I 1000 sleep 100
1.000121704 120 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
1.000121704 5 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
2.000357733 8 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
2.000357733 10 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
3.000495215 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
3.000495215 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
4.000641884 56 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
4.000641884 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
5.000791887 18,446,744,073,709,551,616 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
Getting these large values in case we do -I.
As we are setting event_count to 0, for interval case, overall event_count is not
coming in incremental order. As we may can get new delta lesser then previous count.
Because of which when we print intervals, we are getting negative value which create
these large values.
This patch removes part where we set event_count to 0 in function
'h_24x7_event_read'. There won't be much impact as we do set event->hw.prev_count
to the raw value at the time of initialization to print change value.
With this patch
In power9 platform
command#: ./perf stat -e "{hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/,
hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/}"
-C 0 -I 1000 sleep 100
1.000117685 93 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
1.000117685 1 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
2.000349331 98 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
2.000349331 2 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
3.000495900 131 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
3.000495900 4 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
4.000645920 204 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
4.000645920 61 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=1/
4.284169997 22 hv_24x7/PM_MCS01_128B_RD_DISP_PORT01,chip=0/
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Suggested-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
arch/powerpc/perf/hv-24x7.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 573e0b309c0c..48e8f4b17b91 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -1400,16 +1400,6 @@ static void h_24x7_event_read(struct perf_event *event)
h24x7hw = &get_cpu_var(hv_24x7_hw);
h24x7hw->events[i] = event;
put_cpu_var(h24x7hw);
- /*
- * Clear the event count so we can compute the _change_
- * in the 24x7 raw counter value at the end of the txn.
- *
- * Note that we could alternatively read the 24x7 value
- * now and save its value in event->hw.prev_count. But
- * that would require issuing a hcall, which would then
- * defeat the purpose of using the txn interface.
- */
- local64_set(&event->count, 0);
}
put_cpu_var(hv_24x7_reqb);
--
2.18.1
^ permalink raw reply related
* [PATCH v6 04/11] powerpc/hv-24x7: Add rtas call in hv-24x7 driver to get processor details
From: Kajol Jain @ 2020-03-20 12:53 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200320125406.30995-1-kjain@linux.ibm.com>
For hv_24x7 socket/chip level events, specific chip-id to which
the data requested should be added as part of pmu events.
But number of chips/socket in the system details are not exposed.
Patch implements read_sys_info_pseries() to get system
parameter values like number of sockets and chips per socket.
Rtas_call with token "PROCESSOR_MODULE_INFO"
is used to get these values.
Sub-sequent patch exports these values via sysfs.
Patch also make these parameters default to 1.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/perf/hv-24x7.c | 72 ++++++++++++++++++++++++
arch/powerpc/platforms/pseries/pseries.h | 3 +
2 files changed, 75 insertions(+)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 48e8f4b17b91..9ae00f29bd21 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -20,6 +20,11 @@
#include <asm/io.h>
#include <linux/byteorder/generic.h>
+#ifdef CONFIG_PPC_RTAS
+#include <asm/rtas.h>
+#include <../../platforms/pseries/pseries.h>
+#endif
+
#include "hv-24x7.h"
#include "hv-24x7-catalog.h"
#include "hv-common.h"
@@ -57,6 +62,69 @@ static bool is_physical_domain(unsigned domain)
}
}
+#ifdef CONFIG_PPC_RTAS
+#define PROCESSOR_MODULE_INFO 43
+#define PROCESSOR_MAX_LENGTH (8 * 1024)
+
+static int strbe16toh(const char *buf, int offset)
+{
+ return (buf[offset] << 8) + buf[offset + 1];
+}
+
+static u32 physsockets; /* Physical sockets */
+static u32 physchips; /* Physical chips */
+
+/*
+ * Function read_sys_info_pseries() make a rtas_call which require
+ * data buffer of size 8K. As standard 'rtas_data_buf' is of size
+ * 4K, we are adding new local buffer 'rtas_local_data_buf'.
+ */
+char rtas_local_data_buf[PROCESSOR_MAX_LENGTH] __cacheline_aligned;
+
+/*
+ * read_sys_info_pseries()
+ * Retrieve the number of sockets and chips per socket details
+ * through the get-system-parameter rtas call.
+ */
+void read_sys_info_pseries(void)
+{
+ int call_status, len, ntypes;
+
+ /*
+ * Making system parameter: chips and sockets default to 1.
+ */
+ physsockets = 1;
+ physchips = 1;
+ memset(rtas_local_data_buf, 0, PROCESSOR_MAX_LENGTH);
+ spin_lock(&rtas_data_buf_lock);
+
+ call_status = rtas_call(rtas_token("ibm,get-system-parameter"), 3, 1,
+ NULL,
+ PROCESSOR_MODULE_INFO,
+ __pa(rtas_local_data_buf),
+ PROCESSOR_MAX_LENGTH);
+
+ spin_unlock(&rtas_data_buf_lock);
+
+ if (call_status != 0) {
+ pr_info("%s %s Error calling get-system-parameter (0x%x)\n",
+ __FILE__, __func__, call_status);
+ } else {
+ rtas_local_data_buf[PROCESSOR_MAX_LENGTH - 1] = '\0';
+ len = strbe16toh(rtas_local_data_buf, 0);
+ if (len < 6)
+ return;
+
+ ntypes = strbe16toh(rtas_local_data_buf, 2);
+
+ if (!ntypes)
+ return;
+ physsockets = strbe16toh(rtas_local_data_buf, 4);
+ physchips = strbe16toh(rtas_local_data_buf, 6);
+ }
+}
+#endif /* CONFIG_PPC_RTAS */
+
/* Domains for which more than one result element are returned for each event. */
static bool domain_needs_aggregation(unsigned int domain)
{
@@ -1605,6 +1673,10 @@ static int hv_24x7_init(void)
if (r)
return r;
+#ifdef CONFIG_PPC_RTAS
+ read_sys_info_pseries();
+#endif
+
return 0;
}
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 13fa370a87e4..1727559ce304 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -19,6 +19,9 @@ extern void request_event_sources_irqs(struct device_node *np,
struct pt_regs;
extern int pSeries_system_reset_exception(struct pt_regs *regs);
+#ifdef CONFIG_PPC_RTAS
+extern void read_sys_info_pseries(void);
+#endif
extern int pSeries_machine_check_exception(struct pt_regs *regs);
extern long pseries_machine_check_realmode(struct pt_regs *regs);
--
2.18.1
^ permalink raw reply related
* [PATCH v6 05/11] powerpc/hv-24x7: Add sysfs files inside hv-24x7 device to show processor details
From: Kajol Jain @ 2020-03-20 12:54 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200320125406.30995-1-kjain@linux.ibm.com>
To expose the system dependent parameter like total number of
sockets and numbers of chips per socket, patch adds two sysfs files.
"sockets" and "chips" are added to /sys/devices/hv_24x7/interface/
of the "hv_24x7" pmu.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/perf/hv-24x7.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 9ae00f29bd21..a31bd5b88f7a 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -454,6 +454,20 @@ static ssize_t device_show_string(struct device *dev,
return sprintf(buf, "%s\n", (char *)d->var);
}
+#ifdef CONFIG_PPC_RTAS
+static ssize_t sockets_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%d\n", physsockets);
+}
+
+static ssize_t chips_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%d\n", physchips);
+}
+#endif
+
static struct attribute *device_str_attr_create_(char *name, char *str)
{
struct dev_ext_attribute *attr = kzalloc(sizeof(*attr), GFP_KERNEL);
@@ -1100,6 +1114,10 @@ PAGE_0_ATTR(catalog_len, "%lld\n",
(unsigned long long)be32_to_cpu(page_0->length) * 4096);
static BIN_ATTR_RO(catalog, 0/* real length varies */);
static DEVICE_ATTR_RO(domains);
+#ifdef CONFIG_PPC_RTAS
+static DEVICE_ATTR_RO(sockets);
+static DEVICE_ATTR_RO(chips);
+#endif
static struct bin_attribute *if_bin_attrs[] = {
&bin_attr_catalog,
@@ -1110,6 +1128,10 @@ static struct attribute *if_attrs[] = {
&dev_attr_catalog_len.attr,
&dev_attr_catalog_version.attr,
&dev_attr_domains.attr,
+#ifdef CONFIG_PPC_RTAS
+ &dev_attr_sockets.attr,
+ &dev_attr_chips.attr,
+#endif
NULL,
};
--
2.18.1
^ permalink raw reply related
* [PATCH v6 06/11] Documentation/ABI: Add ABI documentation for chips and sockets
From: Kajol Jain @ 2020-03-20 12:54 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200320125406.30995-1-kjain@linux.ibm.com>
Add documentation for the following sysfs files:
/sys/devices/hv_24x7/interface/chips,
/sys/devices/hv_24x7/interface/sockets
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
.../testing/sysfs-bus-event_source-devices-hv_24x7 | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7 b/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
index ec27c6c9e737..e17e5b444a1c 100644
--- a/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
+++ b/Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7
@@ -22,6 +22,20 @@ Description:
Exposes the "version" field of the 24x7 catalog. This is also
extractable from the provided binary "catalog" sysfs entry.
+What: /sys/devices/hv_24x7/interface/sockets
+Date: March 2020
+Contact: Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
+Description: read only
+ This sysfs interface exposes the number of sockets present in the
+ system.
+
+What: /sys/devices/hv_24x7/interface/chips
+Date: March 2020
+Contact: Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
+Description: read only
+ This sysfs interface exposes the number of chips per socket
+ present in the system.
+
What: /sys/bus/event_source/devices/hv_24x7/event_descs/<event-name>
Date: February 2014
Contact: Linux on PowerPC Developer List <linuxppc-dev@lists.ozlabs.org>
--
2.18.1
^ permalink raw reply related
* [PATCH v6 07/11] powerpc/hv-24x7: Update post_mobility_fixup() to handle migration
From: Kajol Jain @ 2020-03-20 12:54 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200320125406.30995-1-kjain@linux.ibm.com>
Function 'read_sys_info_pseries()' is added to get system parameter
values like number of sockets and chips per socket.
and it gets these details via rtas_call with token
"PROCESSOR_MODULE_INFO".
Incase lpar migrate from one system to another, system
parameter details like chips per sockets or number of sockets might
change. So, it needs to be re-initialized otherwise, these values
corresponds to previous system values.
This patch adds a call to 'read_sys_info_pseries()' from
'post-mobility_fixup()' to re-init the physsockets and physchips values.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
arch/powerpc/platforms/pseries/mobility.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index b571285f6c14..226accd6218b 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -371,6 +371,18 @@ void post_mobility_fixup(void)
/* Possibly switch to a new RFI flush type */
pseries_setup_rfi_flush();
+ /*
+ * Incase lpar migrate from one system to another, system
+ * parameter details like chips per sockets and number of sockets
+ * might change. So, it needs to be re-initialized otherwise these
+ * values corresponds to previous system.
+ * Here, adding a call to read_sys_info_pseries() declared in
+ * platforms/pseries/pseries.h to re-init the physsockets and
+ * physchips value.
+ */
+ if (IS_ENABLED(CONFIG_HV_PERF_CTRS) && IS_ENABLED(CONFIG_PPC_RTAS))
+ read_sys_info_pseries();
+
return;
}
--
2.18.1
^ permalink raw reply related
* Re: [PATCH 18/15] kvm: Replace vcpu->swait with rcuwait
From: Peter Zijlstra @ 2020-03-20 12:54 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: rdunlap, linux-pci, bigeasy, linux-kernel, joel, will, mingo,
arnd, Davidlohr Bueso, torvalds, paulmck, linuxppc-dev, rostedt,
bhelgaas, kurt.schwemmer, kvalo, balbi, gregkh, linux-usb,
linux-wireless, oleg, tglx, netdev, Paolo Bonzini, logang, davem
In-Reply-To: <20200320085527.23861-3-dave@stgolabs.net>
On Fri, Mar 20, 2020 at 01:55:26AM -0700, Davidlohr Bueso wrote:
> - swait_event_interruptible_exclusive(*wq, ((!vcpu->arch.power_off) &&
> - (!vcpu->arch.pause)));
> + rcuwait_wait_event(*wait,
> + (!vcpu->arch.power_off) && (!vcpu->arch.pause),
> + TASK_INTERRUPTIBLE);
> - for (;;) {
> - prepare_to_swait_exclusive(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
> -
> - if (kvm_vcpu_check_block(vcpu) < 0)
> - break;
> -
> - waited = true;
> - schedule();
> - }
> -
> - finish_swait(&vcpu->wq, &wait);
> + rcuwait_wait_event(&vcpu->wait,
> + (block_check = kvm_vcpu_check_block(vcpu)) < 0,
> + TASK_INTERRUPTIBLE);
Are these yet more instances that really want to be TASK_IDLE ?
^ permalink raw reply
* [PATCH v6 10/11] tools/perf: Enable Hz/hz prinitg for --metric-only option
From: Kajol Jain @ 2020-03-20 12:54 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200320125406.30995-1-kjain@linux.ibm.com>
Commit 54b5091606c18 ("perf stat: Implement --metric-only mode")
added function 'valid_only_metric()' which drops "Hz" or "hz",
if it is part of "ScaleUnit". This patch enable it since hv_24x7
supports couple of frequency events.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
tools/perf/util/stat-display.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index 16efdba1973a..ecdebfcdd379 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -237,8 +237,6 @@ static bool valid_only_metric(const char *unit)
if (!unit)
return false;
if (strstr(unit, "/sec") ||
- strstr(unit, "hz") ||
- strstr(unit, "Hz") ||
strstr(unit, "CPUs utilized"))
return false;
return true;
--
2.18.1
^ permalink raw reply related
* [PATCH v6 08/11] perf/tools: Refactoring metricgroup__add_metric function
From: Kajol Jain @ 2020-03-20 12:54 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200320125406.30995-1-kjain@linux.ibm.com>
This patch refactor metricgroup__add_metric function where
some part of it move to function metricgroup__add_metric_param.
No logic change.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
tools/perf/util/metricgroup.c | 64 +++++++++++++++++++++--------------
1 file changed, 39 insertions(+), 25 deletions(-)
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index c3a8c701609a..52fb119d25c8 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -474,6 +474,42 @@ static bool metricgroup__has_constraint(struct pmu_event *pe)
return false;
}
+static int metricgroup__add_metric_param(struct strbuf *events,
+ struct list_head *group_list, struct pmu_event *pe)
+{
+
+ const char **ids;
+ int idnum;
+ struct egroup *eg;
+ int ret = -EINVAL;
+
+ if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum) < 0)
+ return ret;
+
+ if (events->len > 0)
+ strbuf_addf(events, ",");
+
+ if (metricgroup__has_constraint(pe))
+ metricgroup__add_metric_non_group(events, ids, idnum);
+ else
+ metricgroup__add_metric_weak_group(events, ids, idnum);
+
+ eg = malloc(sizeof(*eg));
+ if (!eg) {
+ ret = -ENOMEM;
+ return ret;
+ }
+
+ eg->ids = ids;
+ eg->idnum = idnum;
+ eg->metric_name = pe->metric_name;
+ eg->metric_expr = pe->metric_expr;
+ eg->metric_unit = pe->unit;
+ list_add_tail(&eg->nd, group_list);
+
+ return 0;
+}
+
static int metricgroup__add_metric(const char *metric, struct strbuf *events,
struct list_head *group_list)
{
@@ -493,35 +529,13 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
continue;
if (match_metric(pe->metric_group, metric) ||
match_metric(pe->metric_name, metric)) {
- const char **ids;
- int idnum;
- struct egroup *eg;
pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
- if (expr__find_other(pe->metric_expr,
- NULL, &ids, &idnum) < 0)
+ ret = metricgroup__add_metric_param(events,
+ group_list, pe);
+ if (ret == -EINVAL || !ret)
continue;
- if (events->len > 0)
- strbuf_addf(events, ",");
-
- if (metricgroup__has_constraint(pe))
- metricgroup__add_metric_non_group(events, ids, idnum);
- else
- metricgroup__add_metric_weak_group(events, ids, idnum);
-
- eg = malloc(sizeof(struct egroup));
- if (!eg) {
- ret = -ENOMEM;
- break;
- }
- eg->ids = ids;
- eg->idnum = idnum;
- eg->metric_name = pe->metric_name;
- eg->metric_expr = pe->metric_expr;
- eg->metric_unit = pe->unit;
- list_add_tail(&eg->nd, group_list);
- ret = 0;
}
}
return ret;
--
2.18.1
^ permalink raw reply related
* [PATCH v6 11/11] perf/tools/pmu-events/powerpc: Add hv_24x7 socket/chip level metric events
From: Kajol Jain @ 2020-03-20 12:54 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200320125406.30995-1-kjain@linux.ibm.com>
The hv_24×7 feature in IBM® POWER9™ processor-based servers provide the
facility to continuously collect large numbers of hardware performance
metrics efficiently and accurately.
This patch adds hv_24x7 metric file for different Socket/chip
resources.
Result:
power9 platform:
command:# ./perf stat --metric-only -M Memory_RD_BW_Chip -C 0 -I 1000
1.000096188 0.9 0.3
2.000285720 0.5 0.1
3.000424990 0.4 0.1
command:# ./perf stat --metric-only -M PowerBUS_Frequency -C 0 -I 1000
1.000097981 2.3 2.3
2.000291713 2.3 2.3
3.000421719 2.3 2.3
4.000550912 2.3 2.3
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
.../arch/powerpc/power9/nest_metrics.json | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
diff --git a/tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json b/tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
new file mode 100644
index 000000000000..ac38f5540ac6
--- /dev/null
+++ b/tools/perf/pmu-events/arch/powerpc/power9/nest_metrics.json
@@ -0,0 +1,19 @@
+[
+ {
+ "MetricExpr": "(hv_24x7@PM_MCS01_128B_RD_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS01_128B_RD_DISP_PORT23\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_RD_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_RD_DISP_PORT23\\,chip\\=?@)",
+ "MetricName": "Memory_RD_BW_Chip",
+ "MetricGroup": "Memory_BW",
+ "ScaleUnit": "1.6e-2MB"
+ },
+ {
+ "MetricExpr": "(hv_24x7@PM_MCS01_128B_WR_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS01_128B_WR_DISP_PORT23\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_WR_DISP_PORT01\\,chip\\=?@ + hv_24x7@PM_MCS23_128B_WR_DISP_PORT23\\,chip\\=?@ )",
+ "MetricName": "Memory_WR_BW_Chip",
+ "MetricGroup": "Memory_BW",
+ "ScaleUnit": "1.6e-2MB"
+ },
+ {
+ "MetricExpr": "(hv_24x7@PM_PB_CYC\\,chip\\=?@ )",
+ "MetricName": "PowerBUS_Frequency",
+ "ScaleUnit": "2.5e-7GHz"
+ }
+]
--
2.18.1
^ permalink raw reply related
* [PATCH v6 09/11] perf/tools: Enhance JSON/metric infrastructure to handle "?"
From: Kajol Jain @ 2020-03-20 12:54 UTC (permalink / raw)
To: acme, linuxppc-dev, mpe, sukadev
Cc: mark.rutland, maddy, peterz, yao.jin, mingo, kan.liang, ak,
alexander.shishkin, anju, mamatha4, ravi.bangoria, kjain, jmario,
namhyung, tglx, mpetlan, gregkh, linux-kernel, linux-perf-users,
jolsa
In-Reply-To: <20200320125406.30995-1-kjain@linux.ibm.com>
Patch enhances current metric infrastructure to handle "?" in the metric
expression. The "?" can be use for parameters whose value not known while
creating metric events and which can be replace later at runtime to
the proper value. It also add flexibility to create multiple events out
of single metric event added in json file.
Patch adds function 'arch_get_runtimeparam' which is a arch specific
function, returns the count of metric events need to be created.
By default it return 1.
This infrastructure needed for hv_24x7 socket/chip level events.
"hv_24x7" chip level events needs specific chip-id to which the
data is requested. Function 'arch_get_runtimeparam' implemented
in header.c which extract number of sockets from sysfs file
"sockets" under "/sys/devices/hv_24x7/interface/".
With this patch basically we are trying to create as many metric events
as define by runtime_param.
For that one loop is added in function 'metricgroup__add_metric',
which create multiple events at run time depend on return value of
'arch_get_runtimeparam' and merge that event in 'group_list'.
To achieve that we are actually passing this parameter value as part of
`expr__find_other` function and changing "?" present in metric expression
with this value.
As in our json file, there gonna be single metric event, and out of
which we are creating multiple events, I am also merging this value
to the original metric name to specify parameter value.
For example,
command:# ./perf stat -M PowerBUS_Frequency -C 0 -I 1000
# time counts unit events
1.000101867 9,356,933 hv_24x7/pm_pb_cyc,chip=0/ # 2.3 GHz PowerBUS_Frequency_0
1.000101867 9,366,134 hv_24x7/pm_pb_cyc,chip=1/ # 2.3 GHz PowerBUS_Frequency_1
2.000314878 9,365,868 hv_24x7/pm_pb_cyc,chip=0/ # 2.3 GHz PowerBUS_Frequency_0
2.000314878 9,366,092 hv_24x7/pm_pb_cyc,chip=1/ # 2.3 GHz PowerBUS_Frequency_1
So, here _0 and _1 after PowerBUS_Frequency specify parameter value.
As after adding this to group_list, again we call expr__parse
in 'generic_metric' function present in util/stat-display.c.
By this time again we need to pass this parameter value. So, now to get this value
actually I am trying to extract it from metric name itself. Because
otherwise it gonna point to last updated value present in runtime_param.
And gonna match for that value only.
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
---
| 8 ++++++
tools/perf/tests/expr.c | 8 +++---
tools/perf/util/expr.c | 11 ++++----
tools/perf/util/expr.h | 5 ++--
tools/perf/util/expr.l | 27 +++++++++++++-----
tools/perf/util/metricgroup.c | 40 +++++++++++++++++++++++----
tools/perf/util/metricgroup.h | 1 +
tools/perf/util/stat-shadow.c | 12 ++++++--
8 files changed, 86 insertions(+), 26 deletions(-)
--git a/tools/perf/arch/powerpc/util/header.c b/tools/perf/arch/powerpc/util/header.c
index 3b4cdfc5efd6..c0a86afe63fb 100644
--- a/tools/perf/arch/powerpc/util/header.c
+++ b/tools/perf/arch/powerpc/util/header.c
@@ -7,6 +7,8 @@
#include <string.h>
#include <linux/stringify.h>
#include "header.h"
+#include "metricgroup.h"
+#include <api/fs/fs.h>
#define mfspr(rn) ({unsigned long rval; \
asm volatile("mfspr %0," __stringify(rn) \
@@ -44,3 +46,9 @@ get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
return bufp;
}
+
+int arch_get_runtimeparam(void)
+{
+ int count;
+ return sysfs__read_int("/devices/hv_24x7/interface/sockets", &count) < 0 ? 3 : count;
+}
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index ea10fc4412c4..516504cf0ea5 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -10,7 +10,7 @@ static int test(struct expr_parse_ctx *ctx, const char *e, double val2)
{
double val;
- if (expr__parse(&val, ctx, e))
+ if (expr__parse(&val, ctx, e, 1))
TEST_ASSERT_VAL("parse test failed", 0);
TEST_ASSERT_VAL("unexpected value", val == val2);
return 0;
@@ -44,15 +44,15 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
return ret;
p = "FOO/0";
- ret = expr__parse(&val, &ctx, p);
+ ret = expr__parse(&val, &ctx, p, 1);
TEST_ASSERT_VAL("division by zero", ret == -1);
p = "BAR/";
- ret = expr__parse(&val, &ctx, p);
+ ret = expr__parse(&val, &ctx, p, 1);
TEST_ASSERT_VAL("missing operand", ret == -1);
TEST_ASSERT_VAL("find other",
- expr__find_other("FOO + BAR + BAZ + BOZO", "FOO", &other, &num_other) == 0);
+ expr__find_other("FOO + BAR + BAZ + BOZO", "FOO", &other, &num_other, 1) == 0);
TEST_ASSERT_VAL("find other", num_other == 3);
TEST_ASSERT_VAL("find other", !strcmp(other[0], "BAR"));
TEST_ASSERT_VAL("find other", !strcmp(other[1], "BAZ"));
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index c3382d58cf40..9d56e5fb3873 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -27,10 +27,11 @@ void expr__ctx_init(struct expr_parse_ctx *ctx)
static int
__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
- int start)
+ int start, int param)
{
struct expr_scanner_ctx scanner_ctx = {
.start_token = start,
+ .runtime_param = param,
};
YY_BUFFER_STATE buffer;
void *scanner;
@@ -54,9 +55,9 @@ __expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
return ret;
}
-int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr)
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr, int param)
{
- return __expr__parse(final_val, ctx, expr, EXPR_PARSE) ? -1 : 0;
+ return __expr__parse(final_val, ctx, expr, EXPR_PARSE, param) ? -1 : 0;
}
static bool
@@ -74,13 +75,13 @@ already_seen(const char *val, const char *one, const char **other,
}
int expr__find_other(const char *expr, const char *one, const char ***other,
- int *num_other)
+ int *num_other, int param)
{
int err, i = 0, j = 0;
struct expr_parse_ctx ctx;
expr__ctx_init(&ctx);
- err = __expr__parse(NULL, &ctx, expr, EXPR_OTHER);
+ err = __expr__parse(NULL, &ctx, expr, EXPR_OTHER, param);
if (err)
return -1;
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index 0938ad166ece..0b91597d6512 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -17,12 +17,13 @@ struct expr_parse_ctx {
struct expr_scanner_ctx {
int start_token;
+ int runtime_param;
};
void expr__ctx_init(struct expr_parse_ctx *ctx);
void expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val);
-int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr);
+int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr, int param);
int expr__find_other(const char *expr, const char *one, const char ***other,
- int *num_other);
+ int *num_other, int param);
#endif
diff --git a/tools/perf/util/expr.l b/tools/perf/util/expr.l
index 2582c2464938..86eb822c51cc 100644
--- a/tools/perf/util/expr.l
+++ b/tools/perf/util/expr.l
@@ -35,7 +35,7 @@ static int value(yyscan_t scanner, int base)
* Allow @ instead of / to be able to specify pmu/event/ without
* conflicts with normal division.
*/
-static char *normalize(char *str)
+static char *normalize(char *str, int runtime_param)
{
char *ret = str;
char *dst = str;
@@ -45,6 +45,19 @@ static char *normalize(char *str)
*dst++ = '/';
else if (*str == '\\')
*dst++ = *++str;
+ else if (*str == '?') {
+ char *paramval;
+ int i = 0;
+ int size = asprintf(¶mval, "%d", runtime_param);
+
+ if (size < 0)
+ *dst++ = '0';
+ else {
+ while (i < size)
+ *dst++ = paramval[i++];
+ free(paramval);
+ }
+ }
else
*dst++ = *str;
str++;
@@ -54,16 +67,16 @@ static char *normalize(char *str)
return ret;
}
-static int str(yyscan_t scanner, int token)
+static int str(yyscan_t scanner, int token, int runtime_param)
{
YYSTYPE *yylval = expr_get_lval(scanner);
char *text = expr_get_text(scanner);
- yylval->str = normalize(strdup(text));
+ yylval->str = normalize(strdup(text), runtime_param);
if (!yylval->str)
return EXPR_ERROR;
- yylval->str = normalize(yylval->str);
+ yylval->str = normalize(yylval->str, runtime_param);
return token;
}
%}
@@ -72,8 +85,8 @@ number [0-9]+
sch [-,=]
spec \\{sch}
-sym [0-9a-zA-Z_\.:@]+
-symbol {spec}*{sym}*{spec}*{sym}*
+sym [0-9a-zA-Z_\.:@?]+
+symbol {spec}*{sym}*{spec}*{sym}*{spec}*{sym}
%%
struct expr_scanner_ctx *sctx = expr_get_extra(yyscanner);
@@ -93,7 +106,7 @@ if { return IF; }
else { return ELSE; }
#smt_on { return SMT_ON; }
{number} { return value(yyscanner, 10); }
-{symbol} { return str(yyscanner, ID); }
+{symbol} { return str(yyscanner, ID, sctx->runtime_param); }
"|" { return '|'; }
"^" { return '^'; }
"&" { return '&'; }
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 52fb119d25c8..b4b91d8ad5be 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -474,8 +474,13 @@ static bool metricgroup__has_constraint(struct pmu_event *pe)
return false;
}
+int __weak arch_get_runtimeparam(void)
+{
+ return 1;
+}
+
static int metricgroup__add_metric_param(struct strbuf *events,
- struct list_head *group_list, struct pmu_event *pe)
+ struct list_head *group_list, struct pmu_event *pe, int param)
{
const char **ids;
@@ -483,7 +488,7 @@ static int metricgroup__add_metric_param(struct strbuf *events,
struct egroup *eg;
int ret = -EINVAL;
- if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum) < 0)
+ if (expr__find_other(pe->metric_expr, NULL, &ids, &idnum, param) < 0)
return ret;
if (events->len > 0)
@@ -502,11 +507,21 @@ static int metricgroup__add_metric_param(struct strbuf *events,
eg->ids = ids;
eg->idnum = idnum;
- eg->metric_name = pe->metric_name;
+ if (strstr(pe->metric_expr, "?")) {
+ char value[PATH_MAX];
+
+ sprintf(value, "%s%c%d", pe->metric_name, '_', param);
+ eg->metric_name = strdup(value);
+ if (!eg->metric_name) {
+ ret = -ENOMEM;
+ return ret;
+ }
+ }
+ else
+ eg->metric_name = pe->metric_name;
eg->metric_expr = pe->metric_expr;
eg->metric_unit = pe->unit;
list_add_tail(&eg->nd, group_list);
-
return 0;
}
@@ -532,8 +547,21 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
- ret = metricgroup__add_metric_param(events,
- group_list, pe);
+ if (!strstr(pe->metric_expr, "?")) {
+ ret = metricgroup__add_metric_param(events, group_list, pe, 1);
+ } else {
+ int j, count;
+
+ count = arch_get_runtimeparam();
+
+ /* This loop is added to create multiple
+ * events depend on count value and add
+ * those events to group_list.
+ */
+
+ for (j = 0; j < count; j++)
+ ret = metricgroup__add_metric_param(events, group_list, pe, j);
+ }
if (ret == -EINVAL || !ret)
continue;
}
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
index 475c7f912864..81224ba1270d 100644
--- a/tools/perf/util/metricgroup.h
+++ b/tools/perf/util/metricgroup.h
@@ -34,4 +34,5 @@ int metricgroup__parse_groups(const struct option *opt,
void metricgroup__print(bool metrics, bool groups, char *filter,
bool raw, bool details);
bool metricgroup__has_metric(const char *metric);
+int arch_get_runtimeparam(void);
#endif
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 402af3e8d287..85ac6d913782 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -336,7 +336,7 @@ void perf_stat__collect_metric_expr(struct evlist *evsel_list)
metric_events = counter->metric_events;
if (!metric_events) {
if (expr__find_other(counter->metric_expr, counter->name,
- &metric_names, &num_metric_names) < 0)
+ &metric_names, &num_metric_names, 1) < 0)
continue;
metric_events = calloc(sizeof(struct evsel *),
@@ -777,7 +777,15 @@ static void generic_metric(struct perf_stat_config *config,
}
if (!metric_events[i]) {
- if (expr__parse(&ratio, &pctx, metric_expr) == 0) {
+ int param = 1;
+ if (strstr(metric_expr, "?")) {
+ char *tmp = strrchr(metric_name, '_');
+
+ tmp++;
+ param = strtol(tmp, &tmp, 10);
+ }
+
+ if (expr__parse(&ratio, &pctx, metric_expr, param) == 0) {
char *unit;
char metric_bf[64];
--
2.18.1
^ permalink raw reply related
* Re: [PATCH 1/2] KVM: PPC: Book3S HV: check caller of H_SVM_* Hcalls
From: Greg Kurz @ 2020-03-20 12:22 UTC (permalink / raw)
To: Laurent Dufour; +Cc: linux-kernel, kvm-ppc, Bharata B Rao, linuxppc-dev
In-Reply-To: <20200320102643.15516-2-ldufour@linux.ibm.com>
On Fri, 20 Mar 2020 11:26:42 +0100
Laurent Dufour <ldufour@linux.ibm.com> wrote:
> The Hcall named H_SVM_* are reserved to the Ultravisor. However, nothing
> prevent a malicious VM or SVM to call them. This could lead to weird result
> and should be filtered out.
>
> Checking the Secure bit of the calling MSR ensure that the call is coming
> from either the Ultravisor or a SVM. But any system call made from a SVM
> are going through the Ultravisor, and the Ultravisor should filter out
> these malicious call. This way, only the Ultravisor is able to make such a
> Hcall.
"Ultravisor should filter" ? And what if it doesn't (eg. because of a bug) ?
Shouldn't we also check the HV bit of the calling MSR as well to
disambiguate SVM and UV ?
>
> Cc: Bharata B Rao <bharata@linux.ibm.com>
> Cc: Paul Mackerras <paulus@ozlabs.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv.c | 32 +++++++++++++++++++++-----------
> 1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 33be4d93248a..43773182a737 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1074,25 +1074,35 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
> kvmppc_get_gpr(vcpu, 6));
> break;
> case H_SVM_PAGE_IN:
> - ret = kvmppc_h_svm_page_in(vcpu->kvm,
> - kvmppc_get_gpr(vcpu, 4),
> - kvmppc_get_gpr(vcpu, 5),
> - kvmppc_get_gpr(vcpu, 6));
> + ret = H_UNSUPPORTED;
> + if (kvmppc_get_srr1(vcpu) & MSR_S)
> + ret = kvmppc_h_svm_page_in(vcpu->kvm,
> + kvmppc_get_gpr(vcpu, 4),
> + kvmppc_get_gpr(vcpu, 5),
> + kvmppc_get_gpr(vcpu, 6));
If calling kvmppc_h_svm_page_in() produces a "weird result" when
the MSR_S bit isn't set, then I think it should do the checking
itself, ie. pass vcpu.
This would also prevent adding that many lines in kvmppc_pseries_do_hcall()
which is a big enough function already. The checking could be done in a
helper in book3s_hv_uvmem.c and used by all UV specific hcalls.
> break;
> case H_SVM_PAGE_OUT:
> - ret = kvmppc_h_svm_page_out(vcpu->kvm,
> - kvmppc_get_gpr(vcpu, 4),
> - kvmppc_get_gpr(vcpu, 5),
> - kvmppc_get_gpr(vcpu, 6));
> + ret = H_UNSUPPORTED;
> + if (kvmppc_get_srr1(vcpu) & MSR_S)
> + ret = kvmppc_h_svm_page_out(vcpu->kvm,
> + kvmppc_get_gpr(vcpu, 4),
> + kvmppc_get_gpr(vcpu, 5),
> + kvmppc_get_gpr(vcpu, 6));
> break;
> case H_SVM_INIT_START:
> - ret = kvmppc_h_svm_init_start(vcpu->kvm);
> + ret = H_UNSUPPORTED;
> + if (kvmppc_get_srr1(vcpu) & MSR_S)
> + ret = kvmppc_h_svm_init_start(vcpu->kvm);
> break;
> case H_SVM_INIT_DONE:
> - ret = kvmppc_h_svm_init_done(vcpu->kvm);
> + ret = H_UNSUPPORTED;
> + if (kvmppc_get_srr1(vcpu) & MSR_S)
> + ret = kvmppc_h_svm_init_done(vcpu->kvm);
> break;
> case H_SVM_INIT_ABORT:
> - ret = kvmppc_h_svm_init_abort(vcpu->kvm);
> + ret = H_UNSUPPORTED;
> + if (kvmppc_get_srr1(vcpu) & MSR_S)
> + ret = kvmppc_h_svm_init_abort(vcpu->kvm);
> break;
>
> default:
^ permalink raw reply
* Re: [PATCH 1/2] mm, slub: prevent kmalloc_node crashes and memory leaks
From: Srikar Dronamraju @ 2020-03-20 13:10 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Sachin Sant, Nathan Lynch, PUVICHAKRAVARTHY RAMACHANDRAN,
linuxppc-dev, stable, Bharata B Rao, Pekka Enberg, linux-mm,
Kirill Tkhai, David Rientjes, Joonsoo Kim, Andrew Morton,
Michal Hocko, Mel Gorman, Christopher Lameter
In-Reply-To: <20200320115533.9604-1-vbabka@suse.cz>
* Vlastimil Babka <vbabka@suse.cz> [2020-03-20 12:55:32]:
> Sachin reports [1] a crash in SLUB __slab_alloc():
>
> BUG: Kernel NULL pointer dereference on read at 0x000073b0
> Faulting instruction address: 0xc0000000003d55f4
> Oops: Kernel access of bad area, sig: 11 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> Modules linked in:
> CPU: 19 PID: 1 Comm: systemd Not tainted 5.6.0-rc2-next-20200218-autotest #1
> NIP: c0000000003d55f4 LR: c0000000003d5b94 CTR: 0000000000000000
> REGS: c0000008b37836d0 TRAP: 0300 Not tainted (5.6.0-rc2-next-20200218-autotest)
> MSR: 8000000000009033 <SF,EE,ME,IR,DR,RI,LE> CR: 24004844 XER: 00000000
> CFAR: c00000000000dec4 DAR: 00000000000073b0 DSISR: 40000000 IRQMASK: 1
> GPR00: c0000000003d5b94 c0000008b3783960 c00000000155d400 c0000008b301f500
> GPR04: 0000000000000dc0 0000000000000002 c0000000003443d8 c0000008bb398620
> GPR08: 00000008ba2f0000 0000000000000001 0000000000000000 0000000000000000
> GPR12: 0000000024004844 c00000001ec52a00 0000000000000000 0000000000000000
> GPR16: c0000008a1b20048 c000000001595898 c000000001750c18 0000000000000002
> GPR20: c000000001750c28 c000000001624470 0000000fffffffe0 5deadbeef0000122
> GPR24: 0000000000000001 0000000000000dc0 0000000000000002 c0000000003443d8
> GPR28: c0000008b301f500 c0000008bb398620 0000000000000000 c00c000002287180
> NIP [c0000000003d55f4] ___slab_alloc+0x1f4/0x760
> LR [c0000000003d5b94] __slab_alloc+0x34/0x60
> Call Trace:
> [c0000008b3783960] [c0000000003d5734] ___slab_alloc+0x334/0x760 (unreliable)
> [c0000008b3783a40] [c0000000003d5b94] __slab_alloc+0x34/0x60
> [c0000008b3783a70] [c0000000003d6fa0] __kmalloc_node+0x110/0x490
> [c0000008b3783af0] [c0000000003443d8] kvmalloc_node+0x58/0x110
> [c0000008b3783b30] [c0000000003fee38] mem_cgroup_css_online+0x108/0x270
> [c0000008b3783b90] [c000000000235aa8] online_css+0x48/0xd0
> [c0000008b3783bc0] [c00000000023eaec] cgroup_apply_control_enable+0x2ec/0x4d0
> [c0000008b3783ca0] [c000000000242318] cgroup_mkdir+0x228/0x5f0
> [c0000008b3783d10] [c00000000051e170] kernfs_iop_mkdir+0x90/0xf0
> [c0000008b3783d50] [c00000000043dc00] vfs_mkdir+0x110/0x230
> [c0000008b3783da0] [c000000000441c90] do_mkdirat+0xb0/0x1a0
> [c0000008b3783e20] [c00000000000b278] system_call+0x5c/0x68
>
> This is a PowerPC platform with following NUMA topology:
>
> available: 2 nodes (0-1)
> node 0 cpus:
> node 0 size: 0 MB
> node 0 free: 0 MB
> node 1 cpus: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
> node 1 size: 35247 MB
> node 1 free: 30907 MB
> node distances:
> node 0 1
> 0: 10 40
> 1: 40 10
>
> possible numa nodes: 0-31
>
> This only happens with a mmotm patch "mm/memcontrol.c: allocate shrinker_map on
> appropriate NUMA node" [2] which effectively calls kmalloc_node for each
> possible node. SLUB however only allocates kmem_cache_node on online
> N_NORMAL_MEMORY nodes, and relies on node_to_mem_node to return such valid node
> for other nodes since commit a561ce00b09e ("slub: fall back to
> node_to_mem_node() node if allocating on memoryless node"). This is however not
> true in this configuration where the _node_numa_mem_ array is not initialized
> for nodes 0 and 2-31, thus it contains zeroes and get_partial() ends up
> accessing non-allocated kmem_cache_node.
>
> A related issue was reported by Bharata (originally by Ramachandran) [3] where
> a similar PowerPC configuration, but with mainline kernel without patch [2]
> ends up allocating large amounts of pages by kmalloc-1k kmalloc-512. This seems
> to have the same underlying issue with node_to_mem_node() not behaving as
> expected, and might probably also lead to an infinite loop with
> CONFIG_SLUB_CPU_PARTIAL [4].
>
> This patch should fix both issues by not relying on node_to_mem_node() anymore
> and instead simply falling back to NUMA_NO_NODE, when kmalloc_node(node) is
> attempted for a node that's not online, or has no usable memory. The "usable
> memory" condition is also changed from node_present_pages() to N_NORMAL_MEMORY
> node state, as that is exactly the condition that SLUB uses to allocate
> kmem_cache_node structures. The check in get_partial() is removed completely,
> as the checks in ___slab_alloc() are now sufficient to prevent get_partial()
> being reached with an invalid node.
>
> [1] https://lore.kernel.org/linux-next/3381CD91-AB3D-4773-BA04-E7A072A63968@linux.vnet.ibm.com/
> [2] https://lore.kernel.org/linux-mm/fff0e636-4c36-ed10-281c-8cdb0687c839@virtuozzo.com/
> [3] https://lore.kernel.org/linux-mm/20200317092624.GB22538@in.ibm.com/
> [4] https://lore.kernel.org/linux-mm/088b5996-faae-8a56-ef9c-5b567125ae54@suse.cz/
>
> Reported-and-tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
> Reported-by: PUVICHAKRAVARTHY RAMACHANDRAN <puvichakravarthy@in.ibm.com>
> Tested-by: Bharata B Rao <bharata@linux.ibm.com>
> Debugged-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> Fixes: a561ce00b09e ("slub: fall back to node_to_mem_node() node if allocating on memoryless node")
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
--
Thanks and Regards
Srikar Dronamraju
^ permalink raw reply
* [PATCH] powerpc/64s/hash: add torture_segments kernel boot option to increase SLB faults
From: Nicholas Piggin @ 2020-03-20 13:43 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
This option increases the number of SLB misses by limiting the number of
kernel SLB entries, and increased flushing of cached lookaside information.
This helps stress test difficult to hit paths in the kernel.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
.../admin-guide/kernel-parameters.txt | 4 +
arch/powerpc/include/asm/book3s/64/mmu-hash.h | 7 +
arch/powerpc/mm/book3s64/hash_utils.c | 13 ++
arch/powerpc/mm/book3s64/slb.c | 145 ++++++++++++------
4 files changed, 124 insertions(+), 45 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index dbc22d684627..cd3ea9f0c6b1 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -861,6 +861,10 @@
can be useful when debugging issues that require an SLB
miss to occur.
+ torture_segments [PPC]
+ Limits the number of SLB entries used, and flushes
+ them frequently to stress SLB faults.
+
disable= [IPV6]
See Documentation/networking/ipv6.txt.
diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
index 3fa1b962dc27..de34bf94f38c 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
@@ -317,6 +317,13 @@ extern unsigned long tce_alloc_start, tce_alloc_end;
*/
extern int mmu_ci_restrictions;
+extern bool torture_segments_enabled;
+DECLARE_STATIC_KEY_FALSE(torture_segments_key);
+static inline bool torture_segments(void)
+{
+ return static_branch_unlikely(&torture_segments_key);
+}
+
/*
* This computes the AVPN and B fields of the first dword of a HPTE,
* for use when we want to match an existing PTE. The bottom 7 bits
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index 523d4d39d11e..1e5028e22aae 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -354,6 +354,7 @@ int htab_remove_mapping(unsigned long vstart, unsigned long vend,
}
static bool disable_1tb_segments = false;
+bool torture_segments_enabled __read_mostly = false;
static int __init parse_disable_1tb_segments(char *p)
{
@@ -362,6 +363,13 @@ static int __init parse_disable_1tb_segments(char *p)
}
early_param("disable_1tb_segments", parse_disable_1tb_segments);
+static int __init parse_torture_segments(char *p)
+{
+ torture_segments_enabled = true;
+ return 0;
+}
+early_param("torture_segments", parse_torture_segments);
+
static int __init htab_dt_scan_seg_sizes(unsigned long node,
const char *uname, int depth,
void *data)
@@ -853,6 +861,8 @@ static void __init hash_init_partition_table(phys_addr_t hash_table,
pr_info("Partition table %p\n", partition_tb);
}
+DEFINE_STATIC_KEY_FALSE(torture_segments_key);
+
static void __init htab_initialize(void)
{
unsigned long table;
@@ -869,6 +879,9 @@ static void __init htab_initialize(void)
printk(KERN_INFO "Using 1TB segments\n");
}
+ if (torture_segments_enabled)
+ static_branch_enable(&torture_segments_key);
+
/*
* Calculate the required size of the htab. We want the number of
* PTEGs to equal one half the number of real pages.
diff --git a/arch/powerpc/mm/book3s64/slb.c b/arch/powerpc/mm/book3s64/slb.c
index 716204aee3da..d5efce53c54f 100644
--- a/arch/powerpc/mm/book3s64/slb.c
+++ b/arch/powerpc/mm/book3s64/slb.c
@@ -68,7 +68,7 @@ static void assert_slb_presence(bool present, unsigned long ea)
* slbfee. requires bit 24 (PPC bit 39) be clear in RB. Hardware
* ignores all other bits from 0-27, so just clear them all.
*/
- ea &= ~((1UL << 28) - 1);
+ ea &= ~((1UL << SID_SHIFT) - 1);
asm volatile(__PPC_SLBFEE_DOT(%0, %1) : "=r"(tmp) : "r"(ea) : "cr0");
WARN_ON(present == (tmp == 0));
@@ -153,14 +153,28 @@ void slb_flush_all_realmode(void)
asm volatile("slbmte %0,%0; slbia" : : "r" (0));
}
+static __always_inline void __slb_flush_and_restore_bolted(u32 ih)
+{
+ struct slb_shadow *p = get_slb_shadow();
+ unsigned long ksp_esid_data, ksp_vsid_data;
+
+ ksp_esid_data = be64_to_cpu(p->save_area[KSTACK_INDEX].esid);
+ ksp_vsid_data = be64_to_cpu(p->save_area[KSTACK_INDEX].vsid);
+
+ asm volatile(PPC_SLBIA(%0)" \n"
+ "slbmte %1, %2 \n"
+ :: "i" (ih),
+ "r" (ksp_vsid_data),
+ "r" (ksp_esid_data)
+ : "memory");
+}
+
/*
* This flushes non-bolted entries, it can be run in virtual mode. Must
* be called with interrupts disabled.
*/
void slb_flush_and_restore_bolted(void)
{
- struct slb_shadow *p = get_slb_shadow();
-
BUILD_BUG_ON(SLB_NUM_BOLTED != 2);
WARN_ON(!irqs_disabled());
@@ -171,13 +185,10 @@ void slb_flush_and_restore_bolted(void)
*/
hard_irq_disable();
- asm volatile("isync\n"
- "slbia\n"
- "slbmte %0, %1\n"
- "isync\n"
- :: "r" (be64_to_cpu(p->save_area[KSTACK_INDEX].vsid)),
- "r" (be64_to_cpu(p->save_area[KSTACK_INDEX].esid))
- : "memory");
+ isync();
+ __slb_flush_and_restore_bolted(0);
+ isync();
+
assert_slb_presence(true, get_paca()->kstack);
get_paca()->slb_cache_ptr = 0;
@@ -400,6 +411,30 @@ void preload_new_slb_context(unsigned long start, unsigned long sp)
local_irq_enable();
}
+static void slb_cache_slbie_kernel(unsigned int index)
+{
+ unsigned long slbie_data = get_paca()->slb_cache[index];
+ unsigned long ksp = get_paca()->kstack;
+
+ slbie_data <<= SID_SHIFT;
+ slbie_data |= 0xc000000000000000ULL;
+ if ((ksp & slb_esid_mask(mmu_kernel_ssize)) == slbie_data)
+ return;
+ slbie_data |= mmu_kernel_ssize << SLBIE_SSIZE_SHIFT;
+
+ asm volatile("slbie %0" : : "r" (slbie_data));
+}
+
+static void slb_cache_slbie(unsigned int index)
+{
+ unsigned long slbie_data = get_paca()->slb_cache[index];
+
+ slbie_data <<= SID_SHIFT;
+ slbie_data |= user_segment_size(slbie_data) << SLBIE_SSIZE_SHIFT;
+ slbie_data |= SLBIE_C; /* user slbs have C=1 */
+
+ asm volatile("slbie %0" : : "r" (slbie_data));
+}
/* Flush all user entries from the segment table of the current processor. */
void switch_slb(struct task_struct *tsk, struct mm_struct *mm)
@@ -414,8 +449,14 @@ void switch_slb(struct task_struct *tsk, struct mm_struct *mm)
* which would update the slb_cache/slb_cache_ptr fields in the PACA.
*/
hard_irq_disable();
- asm volatile("isync" : : : "memory");
- if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+ isync();
+ if (torture_segments()) {
+ __slb_flush_and_restore_bolted(0);
+ isync();
+ get_paca()->slb_cache_ptr = 0;
+ get_paca()->slb_kern_bitmap = (1U << SLB_NUM_BOLTED) - 1;
+
+ } else if (cpu_has_feature(CPU_FTR_ARCH_300)) {
/*
* SLBIA IH=3 invalidates all Class=1 SLBEs and their
* associated lookaside structures, which matches what
@@ -423,47 +464,36 @@ void switch_slb(struct task_struct *tsk, struct mm_struct *mm)
* cache.
*/
asm volatile(PPC_SLBIA(3));
+
} else {
unsigned long offset = get_paca()->slb_cache_ptr;
if (!mmu_has_feature(MMU_FTR_NO_SLBIE_B) &&
offset <= SLB_CACHE_ENTRIES) {
- unsigned long slbie_data = 0;
-
- for (i = 0; i < offset; i++) {
- unsigned long ea;
-
- ea = (unsigned long)
- get_paca()->slb_cache[i] << SID_SHIFT;
- /*
- * Could assert_slb_presence(true) here, but
- * hypervisor or machine check could have come
- * in and removed the entry at this point.
- */
-
- slbie_data = ea;
- slbie_data |= user_segment_size(slbie_data)
- << SLBIE_SSIZE_SHIFT;
- slbie_data |= SLBIE_C; /* user slbs have C=1 */
- asm volatile("slbie %0" : : "r" (slbie_data));
- }
+ /*
+ * Could assert_slb_presence(true) here, but
+ * hypervisor or machine check could have come
+ * in and removed the entry at this point.
+ */
+
+ for (i = 0; i < offset; i++)
+ slb_cache_slbie(i);
/* Workaround POWER5 < DD2.1 issue */
if (!cpu_has_feature(CPU_FTR_ARCH_207S) && offset == 1)
- asm volatile("slbie %0" : : "r" (slbie_data));
+ slb_cache_slbie(0);
} else {
- struct slb_shadow *p = get_slb_shadow();
- unsigned long ksp_esid_data =
- be64_to_cpu(p->save_area[KSTACK_INDEX].esid);
- unsigned long ksp_vsid_data =
- be64_to_cpu(p->save_area[KSTACK_INDEX].vsid);
-
- asm volatile(PPC_SLBIA(1) "\n"
- "slbmte %0,%1\n"
- "isync"
- :: "r"(ksp_vsid_data),
- "r"(ksp_esid_data));
+ /*
+ * SLBIA IH=1 on ISA v2.05 and newer will preserve
+ * lookaside information created with Class=0 (kernel)
+ * entires, though the SLB entries themselves are
+ * still invalidated.
+ *
+ * Older processors will ignore this optimisation.
+ */
+ __slb_flush_and_restore_bolted(1);
+ isync();
get_paca()->slb_kern_bitmap = (1U << SLB_NUM_BOLTED) - 1;
}
@@ -503,7 +533,7 @@ void switch_slb(struct task_struct *tsk, struct mm_struct *mm)
* address accesses by the kernel (user mode won't happen until
* rfid, which is safe).
*/
- asm volatile("isync" : : : "memory");
+ isync();
}
void slb_set_size(u16 size)
@@ -571,6 +601,9 @@ static void slb_cache_update(unsigned long esid_data)
if (cpu_has_feature(CPU_FTR_ARCH_300))
return; /* ISAv3.0B and later does not use slb_cache */
+ if (torture_segments())
+ return;
+
/*
* Now update slb cache entries
*/
@@ -580,7 +613,7 @@ static void slb_cache_update(unsigned long esid_data)
* We have space in slb cache for optimized switch_slb().
* Top 36 bits from esid_data as per ISA
*/
- local_paca->slb_cache[slb_cache_index++] = esid_data >> 28;
+ local_paca->slb_cache[slb_cache_index++] = esid_data >> SID_SHIFT;
local_paca->slb_cache_ptr++;
} else {
/*
@@ -671,6 +704,28 @@ static long slb_insert_entry(unsigned long ea, unsigned long context,
* accesses user memory before it returns to userspace with rfid.
*/
assert_slb_presence(false, ea);
+ if (torture_segments()) {
+ int slb_cache_index = local_paca->slb_cache_ptr;
+
+ /*
+ * torture_segments() does not use slb cache, repurpose as a
+ * cache of inserted (non-bolted) kernel SLB entries. All
+ * non-bolted kernel entries are flushed on any user fault,
+ * or if there are already 3 non-boled kernel entries.
+ */
+ BUILD_BUG_ON(SLB_CACHE_ENTRIES < 3);
+ if (!kernel || slb_cache_index == 3) {
+ int i;
+
+ for (i = 0; i < slb_cache_index; i++)
+ slb_cache_slbie_kernel(i);
+ slb_cache_index = 0;
+ }
+
+ if (kernel)
+ local_paca->slb_cache[slb_cache_index++] = esid_data >> SID_SHIFT;
+ local_paca->slb_cache_ptr = slb_cache_index;
+ }
asm volatile("slbmte %0, %1" : : "r" (vsid_data), "r" (esid_data));
barrier();
--
2.23.0
^ permalink raw reply related
* generic DMA bypass flag v2
From: Christoph Hellwig @ 2020-03-20 14:16 UTC (permalink / raw)
To: iommu, Alexey Kardashevskiy
Cc: Greg Kroah-Hartman, Joerg Roedel, Robin Murphy, linux-kernel,
linuxppc-dev, Lu Baolu
Hi all,
I've recently beeing chatting with Lu about using dma-iommu and
per-device DMA ops in the intel IOMMU driver, and one missing feature
in dma-iommu is a bypass mode where the direct mapping is used even
when an iommu is attached to improve performance. The powerpc
code already has a similar mode, so I'd like to move it to the core
DMA mapping code. As part of that I noticed that the current
powerpc code has a little bug in that it used the wrong check in the
dma_sync_* routines to see if the direct mapping code is used.
These two patches just add the generic code and move powerpc over,
the intel IOMMU bits will require a separate discussion.
The x86 AMD Gart code also has a bypass mode, but it is a lot
strange, so I'm not going to touch it for now.
Changes since v1:
- rebased to the current dma-mapping-for-next tree
^ permalink raw reply
* [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Christoph Hellwig @ 2020-03-20 14:16 UTC (permalink / raw)
To: iommu, Alexey Kardashevskiy
Cc: Greg Kroah-Hartman, Joerg Roedel, Robin Murphy, linux-kernel,
linuxppc-dev, Lu Baolu
In-Reply-To: <20200320141640.366360-1-hch@lst.de>
Several IOMMU drivers have a bypass mode where they can use a direct
mapping if the devices DMA mask is large enough. Add generic support
to the core dma-mapping code to do that to switch those drivers to
a common solution.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/device.h | 6 ++++++
include/linux/dma-mapping.h | 30 ++++++++++++++++++------------
kernel/dma/mapping.c | 36 +++++++++++++++++++++++++++---------
3 files changed, 51 insertions(+), 21 deletions(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index 0cd7c647c16c..09be8bb2c4a6 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -525,6 +525,11 @@ struct dev_links_info {
* sync_state() callback.
* @dma_coherent: this particular device is dma coherent, even if the
* architecture supports non-coherent devices.
+ * @dma_ops_bypass: If set to %true then the dma_ops are bypassed for the
+ * streaming DMA operations (->map_* / ->unmap_* / ->sync_*),
+ * and optionall (if the coherent mask is large enough) also
+ * for dma allocations. This flag is managed by the dma ops
+ * instance from ->dma_supported.
*
* At the lowest level, every device in a Linux system is represented by an
* instance of struct device. The device structure contains the information
@@ -625,6 +630,7 @@ struct device {
defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
bool dma_coherent:1;
#endif
+ bool dma_ops_bypass : 1;
};
static inline struct device *kobj_to_dev(struct kobject *kobj)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 330ad58fbf4d..c3af0cf5e435 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -188,9 +188,15 @@ static inline int dma_mmap_from_global_coherent(struct vm_area_struct *vma,
}
#endif /* CONFIG_DMA_DECLARE_COHERENT */
-static inline bool dma_is_direct(const struct dma_map_ops *ops)
+/*
+ * Check if the devices uses a direct mapping for streaming DMA operations.
+ * This allows IOMMU drivers to set a bypass mode if the DMA mask is large
+ * enough.
+ */
+static inline bool dma_map_direct(struct device *dev,
+ const struct dma_map_ops *ops)
{
- return likely(!ops);
+ return likely(!ops) || dev->dma_ops_bypass;
}
/*
@@ -279,7 +285,7 @@ static inline dma_addr_t dma_map_page_attrs(struct device *dev,
dma_addr_t addr;
BUG_ON(!valid_dma_direction(dir));
- if (dma_is_direct(ops))
+ if (dma_map_direct(dev, ops))
addr = dma_direct_map_page(dev, page, offset, size, dir, attrs);
else
addr = ops->map_page(dev, page, offset, size, dir, attrs);
@@ -294,7 +300,7 @@ static inline void dma_unmap_page_attrs(struct device *dev, dma_addr_t addr,
const struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!valid_dma_direction(dir));
- if (dma_is_direct(ops))
+ if (dma_map_direct(dev, ops))
dma_direct_unmap_page(dev, addr, size, dir, attrs);
else if (ops->unmap_page)
ops->unmap_page(dev, addr, size, dir, attrs);
@@ -313,7 +319,7 @@ static inline int dma_map_sg_attrs(struct device *dev, struct scatterlist *sg,
int ents;
BUG_ON(!valid_dma_direction(dir));
- if (dma_is_direct(ops))
+ if (dma_map_direct(dev, ops))
ents = dma_direct_map_sg(dev, sg, nents, dir, attrs);
else
ents = ops->map_sg(dev, sg, nents, dir, attrs);
@@ -331,7 +337,7 @@ static inline void dma_unmap_sg_attrs(struct device *dev, struct scatterlist *sg
BUG_ON(!valid_dma_direction(dir));
debug_dma_unmap_sg(dev, sg, nents, dir);
- if (dma_is_direct(ops))
+ if (dma_map_direct(dev, ops))
dma_direct_unmap_sg(dev, sg, nents, dir, attrs);
else if (ops->unmap_sg)
ops->unmap_sg(dev, sg, nents, dir, attrs);
@@ -352,7 +358,7 @@ static inline dma_addr_t dma_map_resource(struct device *dev,
if (WARN_ON_ONCE(pfn_valid(PHYS_PFN(phys_addr))))
return DMA_MAPPING_ERROR;
- if (dma_is_direct(ops))
+ if (dma_map_direct(dev, ops))
addr = dma_direct_map_resource(dev, phys_addr, size, dir, attrs);
else if (ops->map_resource)
addr = ops->map_resource(dev, phys_addr, size, dir, attrs);
@@ -368,7 +374,7 @@ static inline void dma_unmap_resource(struct device *dev, dma_addr_t addr,
const struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!valid_dma_direction(dir));
- if (!dma_is_direct(ops) && ops->unmap_resource)
+ if (!dma_map_direct(dev, ops) && ops->unmap_resource)
ops->unmap_resource(dev, addr, size, dir, attrs);
debug_dma_unmap_resource(dev, addr, size, dir);
}
@@ -380,7 +386,7 @@ static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
const struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!valid_dma_direction(dir));
- if (dma_is_direct(ops))
+ if (dma_map_direct(dev, ops))
dma_direct_sync_single_for_cpu(dev, addr, size, dir);
else if (ops->sync_single_for_cpu)
ops->sync_single_for_cpu(dev, addr, size, dir);
@@ -394,7 +400,7 @@ static inline void dma_sync_single_for_device(struct device *dev,
const struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!valid_dma_direction(dir));
- if (dma_is_direct(ops))
+ if (dma_map_direct(dev, ops))
dma_direct_sync_single_for_device(dev, addr, size, dir);
else if (ops->sync_single_for_device)
ops->sync_single_for_device(dev, addr, size, dir);
@@ -408,7 +414,7 @@ dma_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,
const struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!valid_dma_direction(dir));
- if (dma_is_direct(ops))
+ if (dma_map_direct(dev, ops))
dma_direct_sync_sg_for_cpu(dev, sg, nelems, dir);
else if (ops->sync_sg_for_cpu)
ops->sync_sg_for_cpu(dev, sg, nelems, dir);
@@ -422,7 +428,7 @@ dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg,
const struct dma_map_ops *ops = get_dma_ops(dev);
BUG_ON(!valid_dma_direction(dir));
- if (dma_is_direct(ops))
+ if (dma_map_direct(dev, ops))
dma_direct_sync_sg_for_device(dev, sg, nelems, dir);
else if (ops->sync_sg_for_device)
ops->sync_sg_for_device(dev, sg, nelems, dir);
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index 12ff766ec1fa..fdea45574345 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -105,6 +105,24 @@ void *dmam_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
}
EXPORT_SYMBOL(dmam_alloc_attrs);
+static bool dma_alloc_direct(struct device *dev, const struct dma_map_ops *ops)
+{
+ if (!ops)
+ return true;
+
+ /*
+ * Allows IOMMU drivers to bypass dynamic translations if the DMA mask
+ * is large enough.
+ */
+ if (dev->dma_ops_bypass) {
+ if (min_not_zero(dev->coherent_dma_mask, dev->bus_dma_limit) >=
+ dma_direct_get_required_mask(dev))
+ return true;
+ }
+
+ return false;
+}
+
/*
* Create scatter-list for the already allocated DMA buffer.
*/
@@ -138,7 +156,7 @@ int dma_get_sgtable_attrs(struct device *dev, struct sg_table *sgt,
{
const struct dma_map_ops *ops = get_dma_ops(dev);
- if (dma_is_direct(ops))
+ if (dma_alloc_direct(dev, ops))
return dma_direct_get_sgtable(dev, sgt, cpu_addr, dma_addr,
size, attrs);
if (!ops->get_sgtable)
@@ -206,7 +224,7 @@ bool dma_can_mmap(struct device *dev)
{
const struct dma_map_ops *ops = get_dma_ops(dev);
- if (dma_is_direct(ops))
+ if (dma_alloc_direct(dev, ops))
return dma_direct_can_mmap(dev);
return ops->mmap != NULL;
}
@@ -231,7 +249,7 @@ int dma_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
{
const struct dma_map_ops *ops = get_dma_ops(dev);
- if (dma_is_direct(ops))
+ if (dma_alloc_direct(dev, ops))
return dma_direct_mmap(dev, vma, cpu_addr, dma_addr, size,
attrs);
if (!ops->mmap)
@@ -244,7 +262,7 @@ u64 dma_get_required_mask(struct device *dev)
{
const struct dma_map_ops *ops = get_dma_ops(dev);
- if (dma_is_direct(ops))
+ if (dma_map_direct(dev, ops))
return dma_direct_get_required_mask(dev);
if (ops->get_required_mask)
return ops->get_required_mask(dev);
@@ -275,7 +293,7 @@ void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
/* let the implementation decide on the zone to allocate from: */
flag &= ~(__GFP_DMA | __GFP_DMA32 | __GFP_HIGHMEM);
- if (dma_is_direct(ops))
+ if (dma_alloc_direct(dev, ops))
cpu_addr = dma_direct_alloc(dev, size, dma_handle, flag, attrs);
else if (ops->alloc)
cpu_addr = ops->alloc(dev, size, dma_handle, flag, attrs);
@@ -307,7 +325,7 @@ void dma_free_attrs(struct device *dev, size_t size, void *cpu_addr,
return;
debug_dma_free_coherent(dev, size, cpu_addr, dma_handle);
- if (dma_is_direct(ops))
+ if (dma_alloc_direct(dev, ops))
dma_direct_free(dev, size, cpu_addr, dma_handle, attrs);
else if (ops->free)
ops->free(dev, size, cpu_addr, dma_handle, attrs);
@@ -318,7 +336,7 @@ int dma_supported(struct device *dev, u64 mask)
{
const struct dma_map_ops *ops = get_dma_ops(dev);
- if (dma_is_direct(ops))
+ if (!ops)
return dma_direct_supported(dev, mask);
if (!ops->dma_supported)
return 1;
@@ -374,7 +392,7 @@ void dma_cache_sync(struct device *dev, void *vaddr, size_t size,
BUG_ON(!valid_dma_direction(dir));
- if (dma_is_direct(ops))
+ if (dma_alloc_direct(dev, ops))
arch_dma_cache_sync(dev, vaddr, size, dir);
else if (ops->cache_sync)
ops->cache_sync(dev, vaddr, size, dir);
@@ -386,7 +404,7 @@ size_t dma_max_mapping_size(struct device *dev)
const struct dma_map_ops *ops = get_dma_ops(dev);
size_t size = SIZE_MAX;
- if (dma_is_direct(ops))
+ if (dma_map_direct(dev, ops))
size = dma_direct_max_mapping_size(dev);
else if (ops && ops->max_mapping_size)
size = ops->max_mapping_size(dev);
--
2.25.1
^ permalink raw reply related
* [PATCH 2/2] powerpc: use the generic dma_ops_bypass mode
From: Christoph Hellwig @ 2020-03-20 14:16 UTC (permalink / raw)
To: iommu, Alexey Kardashevskiy
Cc: Greg Kroah-Hartman, Joerg Roedel, Robin Murphy, linux-kernel,
linuxppc-dev, Lu Baolu
In-Reply-To: <20200320141640.366360-1-hch@lst.de>
Use the DMA API bypass mechanism for direct window mappings. This uses
common code and speed up the direct mapping case by avoiding indirect
calls just when not using dma ops at all. It also fixes a problem where
the sync_* methods were using the bypass check for DMA allocations, but
those are part of the streaming ops.
Note that this patch loses the DMA_ATTR_WEAK_ORDERING override, which
has never been well defined, as is only used by a few drivers, which
IIRC never showed up in the typical Cell blade setups that are affected
by the ordering workaround.
Fixes: efd176a04bef ("powerpc/pseries/dma: Allow SWIOTLB")
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/powerpc/include/asm/device.h | 5 --
arch/powerpc/kernel/dma-iommu.c | 90 ++++---------------------------
2 files changed, 9 insertions(+), 86 deletions(-)
diff --git a/arch/powerpc/include/asm/device.h b/arch/powerpc/include/asm/device.h
index 266542769e4b..452402215e12 100644
--- a/arch/powerpc/include/asm/device.h
+++ b/arch/powerpc/include/asm/device.h
@@ -18,11 +18,6 @@ struct iommu_table;
* drivers/macintosh/macio_asic.c
*/
struct dev_archdata {
- /*
- * Set to %true if the dma_iommu_ops are requested to use a direct
- * window instead of dynamically mapping memory.
- */
- bool iommu_bypass : 1;
/*
* These two used to be a union. However, with the hybrid ops we need
* both so here we store both a DMA offset for direct mappings and
diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c
index e486d1d78de2..569fecd7b5b2 100644
--- a/arch/powerpc/kernel/dma-iommu.c
+++ b/arch/powerpc/kernel/dma-iommu.c
@@ -14,23 +14,6 @@
* Generic iommu implementation
*/
-/*
- * The coherent mask may be smaller than the real mask, check if we can
- * really use a direct window.
- */
-static inline bool dma_iommu_alloc_bypass(struct device *dev)
-{
- return dev->archdata.iommu_bypass && !iommu_fixed_is_weak &&
- dma_direct_supported(dev, dev->coherent_dma_mask);
-}
-
-static inline bool dma_iommu_map_bypass(struct device *dev,
- unsigned long attrs)
-{
- return dev->archdata.iommu_bypass &&
- (!iommu_fixed_is_weak || (attrs & DMA_ATTR_WEAK_ORDERING));
-}
-
/* Allocates a contiguous real buffer and creates mappings over it.
* Returns the virtual address of the buffer and sets dma_handle
* to the dma address (mapping) of the first page.
@@ -39,8 +22,6 @@ static void *dma_iommu_alloc_coherent(struct device *dev, size_t size,
dma_addr_t *dma_handle, gfp_t flag,
unsigned long attrs)
{
- if (dma_iommu_alloc_bypass(dev))
- return dma_direct_alloc(dev, size, dma_handle, flag, attrs);
return iommu_alloc_coherent(dev, get_iommu_table_base(dev), size,
dma_handle, dev->coherent_dma_mask, flag,
dev_to_node(dev));
@@ -50,11 +31,7 @@ static void dma_iommu_free_coherent(struct device *dev, size_t size,
void *vaddr, dma_addr_t dma_handle,
unsigned long attrs)
{
- if (dma_iommu_alloc_bypass(dev))
- dma_direct_free(dev, size, vaddr, dma_handle, attrs);
- else
- iommu_free_coherent(get_iommu_table_base(dev), size, vaddr,
- dma_handle);
+ iommu_free_coherent(get_iommu_table_base(dev), size, vaddr, dma_handle);
}
/* Creates TCEs for a user provided buffer. The user buffer must be
@@ -67,9 +44,6 @@ static dma_addr_t dma_iommu_map_page(struct device *dev, struct page *page,
enum dma_data_direction direction,
unsigned long attrs)
{
- if (dma_iommu_map_bypass(dev, attrs))
- return dma_direct_map_page(dev, page, offset, size, direction,
- attrs);
return iommu_map_page(dev, get_iommu_table_base(dev), page, offset,
size, dma_get_mask(dev), direction, attrs);
}
@@ -79,11 +53,8 @@ static void dma_iommu_unmap_page(struct device *dev, dma_addr_t dma_handle,
size_t size, enum dma_data_direction direction,
unsigned long attrs)
{
- if (!dma_iommu_map_bypass(dev, attrs))
- iommu_unmap_page(get_iommu_table_base(dev), dma_handle, size,
- direction, attrs);
- else
- dma_direct_unmap_page(dev, dma_handle, size, direction, attrs);
+ iommu_unmap_page(get_iommu_table_base(dev), dma_handle, size, direction,
+ attrs);
}
@@ -91,8 +62,6 @@ static int dma_iommu_map_sg(struct device *dev, struct scatterlist *sglist,
int nelems, enum dma_data_direction direction,
unsigned long attrs)
{
- if (dma_iommu_map_bypass(dev, attrs))
- return dma_direct_map_sg(dev, sglist, nelems, direction, attrs);
return ppc_iommu_map_sg(dev, get_iommu_table_base(dev), sglist, nelems,
dma_get_mask(dev), direction, attrs);
}
@@ -101,11 +70,8 @@ static void dma_iommu_unmap_sg(struct device *dev, struct scatterlist *sglist,
int nelems, enum dma_data_direction direction,
unsigned long attrs)
{
- if (!dma_iommu_map_bypass(dev, attrs))
- ppc_iommu_unmap_sg(get_iommu_table_base(dev), sglist, nelems,
+ ppc_iommu_unmap_sg(get_iommu_table_base(dev), sglist, nelems,
direction, attrs);
- else
- dma_direct_unmap_sg(dev, sglist, nelems, direction, attrs);
}
static bool dma_iommu_bypass_supported(struct device *dev, u64 mask)
@@ -113,8 +79,9 @@ static bool dma_iommu_bypass_supported(struct device *dev, u64 mask)
struct pci_dev *pdev = to_pci_dev(dev);
struct pci_controller *phb = pci_bus_to_host(pdev->bus);
- return phb->controller_ops.iommu_bypass_supported &&
- phb->controller_ops.iommu_bypass_supported(pdev, mask);
+ if (iommu_fixed_is_weak || !phb->controller_ops.iommu_bypass_supported)
+ return false;
+ return phb->controller_ops.iommu_bypass_supported(pdev, mask);
}
/* We support DMA to/from any memory page via the iommu */
@@ -123,7 +90,7 @@ int dma_iommu_dma_supported(struct device *dev, u64 mask)
struct iommu_table *tbl = get_iommu_table_base(dev);
if (dev_is_pci(dev) && dma_iommu_bypass_supported(dev, mask)) {
- dev->archdata.iommu_bypass = true;
+ dev->dma_ops_bypass = true;
dev_dbg(dev, "iommu: 64-bit OK, using fixed ops\n");
return 1;
}
@@ -141,7 +108,7 @@ int dma_iommu_dma_supported(struct device *dev, u64 mask)
}
dev_dbg(dev, "iommu: not 64-bit, using default ops\n");
- dev->archdata.iommu_bypass = false;
+ dev->dma_ops_bypass = false;
return 1;
}
@@ -153,47 +120,12 @@ u64 dma_iommu_get_required_mask(struct device *dev)
if (!tbl)
return 0;
- if (dev_is_pci(dev)) {
- u64 bypass_mask = dma_direct_get_required_mask(dev);
-
- if (dma_iommu_bypass_supported(dev, bypass_mask))
- return bypass_mask;
- }
-
mask = 1ULL < (fls_long(tbl->it_offset + tbl->it_size) - 1);
mask += mask - 1;
return mask;
}
-static void dma_iommu_sync_for_cpu(struct device *dev, dma_addr_t addr,
- size_t size, enum dma_data_direction dir)
-{
- if (dma_iommu_alloc_bypass(dev))
- dma_direct_sync_single_for_cpu(dev, addr, size, dir);
-}
-
-static void dma_iommu_sync_for_device(struct device *dev, dma_addr_t addr,
- size_t sz, enum dma_data_direction dir)
-{
- if (dma_iommu_alloc_bypass(dev))
- dma_direct_sync_single_for_device(dev, addr, sz, dir);
-}
-
-extern void dma_iommu_sync_sg_for_cpu(struct device *dev,
- struct scatterlist *sgl, int nents, enum dma_data_direction dir)
-{
- if (dma_iommu_alloc_bypass(dev))
- dma_direct_sync_sg_for_cpu(dev, sgl, nents, dir);
-}
-
-extern void dma_iommu_sync_sg_for_device(struct device *dev,
- struct scatterlist *sgl, int nents, enum dma_data_direction dir)
-{
- if (dma_iommu_alloc_bypass(dev))
- dma_direct_sync_sg_for_device(dev, sgl, nents, dir);
-}
-
const struct dma_map_ops dma_iommu_ops = {
.alloc = dma_iommu_alloc_coherent,
.free = dma_iommu_free_coherent,
@@ -203,10 +135,6 @@ const struct dma_map_ops dma_iommu_ops = {
.map_page = dma_iommu_map_page,
.unmap_page = dma_iommu_unmap_page,
.get_required_mask = dma_iommu_get_required_mask,
- .sync_single_for_cpu = dma_iommu_sync_for_cpu,
- .sync_single_for_device = dma_iommu_sync_for_device,
- .sync_sg_for_cpu = dma_iommu_sync_sg_for_cpu,
- .sync_sg_for_device = dma_iommu_sync_sg_for_device,
.mmap = dma_common_mmap,
.get_sgtable = dma_common_get_sgtable,
};
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 2/2] KVM: PPC: Book3S HV: H_SVM_INIT_START must call UV_RETURN
From: Laurent Dufour @ 2020-03-20 14:36 UTC (permalink / raw)
To: bharata; +Cc: linux-kernel, kvm-ppc, linuxppc-dev
In-Reply-To: <20200320112403.GG26049@in.ibm.com>
Le 20/03/2020 à 12:24, Bharata B Rao a écrit :
> On Fri, Mar 20, 2020 at 11:26:43AM +0100, Laurent Dufour wrote:
>> When the call to UV_REGISTER_MEM_SLOT is failing, for instance because
>> there is not enough free secured memory, the Hypervisor (HV) has to call
>> UV_RETURN to report the error to the Ultravisor (UV). Then the UV will call
>> H_SVM_INIT_ABORT to abort the securing phase and go back to the calling VM.
>>
>> If the kvm->arch.secure_guest is not set, in the return path rfid is called
>> but there is no valid context to get back to the SVM since the Hcall has
>> been routed by the Ultravisor.
>>
>> Move the setting of kvm->arch.secure_guest earlier in
>> kvmppc_h_svm_init_start() so in the return path, UV_RETURN will be called
>> instead of rfid.
>>
>> Cc: Bharata B Rao <bharata@linux.ibm.com>
>> Cc: Paul Mackerras <paulus@ozlabs.org>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>> ---
>> arch/powerpc/kvm/book3s_hv_uvmem.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c
>> index 79b1202b1c62..68dff151315c 100644
>> --- a/arch/powerpc/kvm/book3s_hv_uvmem.c
>> +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
>> @@ -209,6 +209,8 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)
>> int ret = H_SUCCESS;
>> int srcu_idx;
>>
>> + kvm->arch.secure_guest = KVMPPC_SECURE_INIT_START;
>> +
>> if (!kvmppc_uvmem_bitmap)
>> return H_UNSUPPORTED;
>>
>> @@ -233,7 +235,6 @@ unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)
>> goto out;
>> }
>> }
>> - kvm->arch.secure_guest |= KVMPPC_SECURE_INIT_START;
>
> There is an assumption that memory slots would have been registered with UV
> if KVMPPC_SECURE_INIT_START has been done. KVM_PPC_SVM_OFF ioctl will skip
> unregistration and other steps during reboot if KVMPPC_SECURE_INIT_START
> hasn't been done.
>
> Have you checked if that path isn't affected by this change?
I checked that and didn't find any issue there.
My only concern was that block:
kvm_for_each_vcpu(i, vcpu, kvm) {
spin_lock(&vcpu->arch.vpa_update_lock);
unpin_vpa_reset(kvm, &vcpu->arch.dtl);
unpin_vpa_reset(kvm, &vcpu->arch.slb_shadow);
unpin_vpa_reset(kvm, &vcpu->arch.vpa);
spin_unlock(&vcpu->arch.vpa_update_lock);
}
But that seems to be safe.
However I'm not a familiar with the KVM's code, do you think an additional
KVMPPC_SECURE_INIT_* value needed here?
Thanks,
Laurent.
^ permalink raw reply
* Re: [PATCH v12 8/8] MAINTAINERS: perf: Add pattern that matches ppc perf to the perf entry.
From: Joe Perches @ 2020-03-20 14:42 UTC (permalink / raw)
To: Andy Shevchenko, Michal Suchánek
Cc: Mark Rutland, Gustavo Luiz Duarte, Peter Zijlstra,
Sebastian Andrzej Siewior, linux-kernel, Paul Mackerras,
Jiri Olsa, Rob Herring, Michael Neuling, Mauro Carvalho Chehab,
Masahiro Yamada, Nayna Jain, Alexander Shishkin, Ingo Molnar,
Allison Randal, Jordan Niethe, Valentin Schneider, Arnd Bergmann,
Arnaldo Carvalho de Melo, Alexander Viro, Jonathan Cameron,
Namhyung Kim, Thomas Gleixner, Hari Bathini, Greg Kroah-Hartman,
Nicholas Piggin, Claudio Carvalho, Eric Richter,
Eric W. Biederman, linux-fsdevel, linuxppc-dev, David S. Miller,
Thiago Jung Bauermann
In-Reply-To: <20200320124251.GW1922688@smile.fi.intel.com>
On Fri, 2020-03-20 at 14:42 +0200, Andy Shevchenko wrote:
> On Fri, Mar 20, 2020 at 12:23:38PM +0100, Michal Suchánek wrote:
> > On Fri, Mar 20, 2020 at 12:33:50PM +0200, Andy Shevchenko wrote:
> > > On Fri, Mar 20, 2020 at 11:20:19AM +0100, Michal Suchanek wrote:
> > > > While at it also simplify the existing perf patterns.
> > > And still missed fixes from parse-maintainers.pl.
> >
> > Oh, that script UX is truly ingenious.
>
> You have at least two options, their combinations, etc:
> - complain to the author :-)
> - send a patch :-)
Recently:
https://lore.kernel.org/lkml/4d5291fa3fb4962b1fa55e8fd9ef421ef0c1b1e5.camel@perches.com/
^ permalink raw reply
* Re: [PATCH 1/2] KVM: PPC: Book3S HV: check caller of H_SVM_* Hcalls
From: Laurent Dufour @ 2020-03-20 14:43 UTC (permalink / raw)
To: Greg Kurz; +Cc: linux-kernel, kvm-ppc, Bharata B Rao, linuxppc-dev
In-Reply-To: <20200320132248.44b81b3b@bahia.lan>
Le 20/03/2020 à 13:22, Greg Kurz a écrit :
> On Fri, 20 Mar 2020 11:26:42 +0100
> Laurent Dufour <ldufour@linux.ibm.com> wrote:
>
>> The Hcall named H_SVM_* are reserved to the Ultravisor. However, nothing
>> prevent a malicious VM or SVM to call them. This could lead to weird result
>> and should be filtered out.
>>
>> Checking the Secure bit of the calling MSR ensure that the call is coming
>> from either the Ultravisor or a SVM. But any system call made from a SVM
>> are going through the Ultravisor, and the Ultravisor should filter out
>> these malicious call. This way, only the Ultravisor is able to make such a
>> Hcall.
>
> "Ultravisor should filter" ? And what if it doesn't (eg. because of a bug) ?
If it doesn't, a malicious SVM would be able to call UV reserved Hcall like
H_SVM_INIT_ABORT, etc... which is not a good idea.
>
> Shouldn't we also check the HV bit of the calling MSR as well to
> disambiguate SVM and UV ?
That's another way to do so, but since the SVM Hcall are going through the UV,
it seems the right place (the UV) to do the filtering.
>>
>> Cc: Bharata B Rao <bharata@linux.ibm.com>
>> Cc: Paul Mackerras <paulus@ozlabs.org>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>> ---
>> arch/powerpc/kvm/book3s_hv.c | 32 +++++++++++++++++++++-----------
>> 1 file changed, 21 insertions(+), 11 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 33be4d93248a..43773182a737 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -1074,25 +1074,35 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
>> kvmppc_get_gpr(vcpu, 6));
>> break;
>> case H_SVM_PAGE_IN:
>> - ret = kvmppc_h_svm_page_in(vcpu->kvm,
>> - kvmppc_get_gpr(vcpu, 4),
>> - kvmppc_get_gpr(vcpu, 5),
>> - kvmppc_get_gpr(vcpu, 6));
>> + ret = H_UNSUPPORTED;
>> + if (kvmppc_get_srr1(vcpu) & MSR_S)
>> + ret = kvmppc_h_svm_page_in(vcpu->kvm,
>> + kvmppc_get_gpr(vcpu, 4),
>> + kvmppc_get_gpr(vcpu, 5),
>> + kvmppc_get_gpr(vcpu, 6));
>
> If calling kvmppc_h_svm_page_in() produces a "weird result" when
> the MSR_S bit isn't set, then I think it should do the checking
> itself, ie. pass vcpu.
>
> This would also prevent adding that many lines in kvmppc_pseries_do_hcall()
> which is a big enough function already. The checking could be done in a
> helper in book3s_hv_uvmem.c and used by all UV specific hcalls.
I'm not convinced that would be better, and I followed the way checks for other
Hcalls has been made (see H_TLB_INVALIDATE,..).
I agree kvmppc_pseries_do_hcall() is long but this is just a big switch(),
quite linear.
>
>> break;
>> case H_SVM_PAGE_OUT:
>> - ret = kvmppc_h_svm_page_out(vcpu->kvm,
>> - kvmppc_get_gpr(vcpu, 4),
>> - kvmppc_get_gpr(vcpu, 5),
>> - kvmppc_get_gpr(vcpu, 6));
>> + ret = H_UNSUPPORTED;
>> + if (kvmppc_get_srr1(vcpu) & MSR_S)
>> + ret = kvmppc_h_svm_page_out(vcpu->kvm,
>> + kvmppc_get_gpr(vcpu, 4),
>> + kvmppc_get_gpr(vcpu, 5),
>> + kvmppc_get_gpr(vcpu, 6));
>> break;
>> case H_SVM_INIT_START:
>> - ret = kvmppc_h_svm_init_start(vcpu->kvm);
>> + ret = H_UNSUPPORTED;
>> + if (kvmppc_get_srr1(vcpu) & MSR_S)
>> + ret = kvmppc_h_svm_init_start(vcpu->kvm);
>> break;
>> case H_SVM_INIT_DONE:
>> - ret = kvmppc_h_svm_init_done(vcpu->kvm);
>> + ret = H_UNSUPPORTED;
>> + if (kvmppc_get_srr1(vcpu) & MSR_S)
>> + ret = kvmppc_h_svm_init_done(vcpu->kvm);
>> break;
>> case H_SVM_INIT_ABORT:
>> - ret = kvmppc_h_svm_init_abort(vcpu->kvm);
>> + ret = H_UNSUPPORTED;
>> + if (kvmppc_get_srr1(vcpu) & MSR_S)
>> + ret = kvmppc_h_svm_init_abort(vcpu->kvm);
>> break;
>>
>> default:
>
^ permalink raw reply
* Re: [PATCH 1/2] dma-mapping: add a dma_ops_bypass flag to struct device
From: Greg Kroah-Hartman @ 2020-03-20 15:02 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Alexey Kardashevskiy, linuxppc-dev, Joerg Roedel, linux-kernel,
iommu, Robin Murphy, Lu Baolu
In-Reply-To: <20200320141640.366360-2-hch@lst.de>
On Fri, Mar 20, 2020 at 03:16:39PM +0100, Christoph Hellwig wrote:
> Several IOMMU drivers have a bypass mode where they can use a direct
> mapping if the devices DMA mask is large enough. Add generic support
> to the core dma-mapping code to do that to switch those drivers to
> a common solution.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> include/linux/device.h | 6 ++++++
> include/linux/dma-mapping.h | 30 ++++++++++++++++++------------
> kernel/dma/mapping.c | 36 +++++++++++++++++++++++++++---------
> 3 files changed, 51 insertions(+), 21 deletions(-)
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ 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