* [Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
From: bugzilla-daemon @ 2020-04-03 19:23 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206203-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206203
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #286805|0 |1
is obsolete| |
--- Comment #9 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 288187
--> https://bugzilla.kernel.org/attachment.cgi?id=288187&action=edit
kernel .config (kernel 5.6.2, Talos II)
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 206203] kmemleak reports various leaks in drivers/of/unittest.c
From: bugzilla-daemon @ 2020-04-03 19:22 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-206203-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=206203
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #286803|0 |1
is obsolete| |
--- Comment #8 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 288185
--> https://bugzilla.kernel.org/attachment.cgi?id=288185&action=edit
dmesg (kernel 5.6.2, Talos II)
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH v5 4/4] powerpc/papr_scm: Implement support for DSM_PAPR_SCM_HEALTH
From: Dan Williams @ 2020-04-03 18:41 UTC (permalink / raw)
To: Vaibhav Jain
Cc: Alastair D'Silva, linux-nvdimm, Aneesh Kumar K . V,
Jeff Moyer, Oliver O'Halloran, Vishal Verma, Michael Ellerman,
linuxppc-dev
In-Reply-To: <20200331143229.306718-5-vaibhav@linux.ibm.com>
On Tue, Mar 31, 2020 at 7:33 AM Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>
> This patch implements support for papr_scm command
> 'DSM_PAPR_SCM_HEALTH' that returns a newly introduced 'struct
> nd_papr_scm_dimm_health_stat' instance containing dimm health
> information back to user space in response to ND_CMD_CALL. This
> functionality is implemented in newly introduced papr_scm_get_health()
> that queries the scm-dimm health information and then copies these bitmaps
> to the package payload whose layout is defined by 'struct
> papr_scm_ndctl_health'.
>
> The patch also introduces a new member a new member 'struct
> papr_scm_priv.health' thats an instance of 'struct
> nd_papr_scm_dimm_health_stat' to cache the health information of a
> scm-dimm. As a result functions drc_pmem_query_health() and
> papr_flags_show() are updated to populate and use this new struct
> instead of two be64 integers that we earlier used.
Link to HCALL specification?
>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> Changelog:
>
> v4..v5: None
>
> v3..v4: Call the DSM_PAPR_SCM_HEALTH service function from
> papr_scm_service_dsm() instead of papr_scm_ndctl(). [Aneesh]
>
> v2..v3: Updated struct nd_papr_scm_dimm_health_stat_v1 to use '__xx'
> types as its exported to the userspace [Aneesh]
> Changed the constants DSM_PAPR_SCM_DIMM_XX indicating dimm
> health from enum to #defines [Aneesh]
>
> v1..v2: New patch in the series
> ---
> arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 40 +++++++
> arch/powerpc/platforms/pseries/papr_scm.c | 109 ++++++++++++++++---
> 2 files changed, 132 insertions(+), 17 deletions(-)
>
> diff --git a/arch/powerpc/include/uapi/asm/papr_scm_dsm.h b/arch/powerpc/include/uapi/asm/papr_scm_dsm.h
> index c039a49b41b4..8265125304ca 100644
> --- a/arch/powerpc/include/uapi/asm/papr_scm_dsm.h
> +++ b/arch/powerpc/include/uapi/asm/papr_scm_dsm.h
> @@ -132,6 +132,7 @@ struct nd_papr_scm_cmd_pkg {
> */
> enum dsm_papr_scm {
> DSM_PAPR_SCM_MIN = 0x10000,
> + DSM_PAPR_SCM_HEALTH,
> DSM_PAPR_SCM_MAX,
> };
>
> @@ -158,4 +159,43 @@ static void *papr_scm_pcmd_to_payload(struct nd_papr_scm_cmd_pkg *pcmd)
> else
> return (void *)((__u8 *) pcmd + pcmd->payload_offset);
> }
> +
> +/* Various scm-dimm health indicators */
> +#define DSM_PAPR_SCM_DIMM_HEALTHY 0
> +#define DSM_PAPR_SCM_DIMM_UNHEALTHY 1
> +#define DSM_PAPR_SCM_DIMM_CRITICAL 2
> +#define DSM_PAPR_SCM_DIMM_FATAL 3
> +
> +/*
> + * Struct exchanged between kernel & ndctl in for PAPR_DSM_PAPR_SMART_HEALTH
> + * Various bitflags indicate the health status of the dimm.
> + *
> + * dimm_unarmed : Dimm not armed. So contents wont persist.
> + * dimm_bad_shutdown : Previous shutdown did not persist contents.
> + * dimm_bad_restore : Contents from previous shutdown werent restored.
> + * dimm_scrubbed : Contents of the dimm have been scrubbed.
> + * dimm_locked : Contents of the dimm cant be modified until CEC reboot
> + * dimm_encrypted : Contents of dimm are encrypted.
> + * dimm_health : Dimm health indicator.
> + */
> +struct nd_papr_scm_dimm_health_stat_v1 {
> + __u8 dimm_unarmed;
> + __u8 dimm_bad_shutdown;
> + __u8 dimm_bad_restore;
> + __u8 dimm_scrubbed;
> + __u8 dimm_locked;
> + __u8 dimm_encrypted;
> + __u16 dimm_health;
> +};
Does the structure pack the same across different compilers and configurations?
> +
> +/*
> + * Typedef the current struct for dimm_health so that any application
> + * or kernel recompiled after introducing a new version automatically
> + * supports the new version.
> + */
> +#define nd_papr_scm_dimm_health_stat nd_papr_scm_dimm_health_stat_v1
> +
> +/* Current version number for the dimm health struct */
> +#define ND_PAPR_SCM_DIMM_HEALTH_VERSION 1
> +
> #endif /* _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_ */
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index e8ce96d2249e..ce94762954e0 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -47,8 +47,7 @@ struct papr_scm_priv {
> struct mutex dimm_mutex;
>
> /* Health information for the dimm */
> - __be64 health_bitmap;
> - __be64 health_bitmap_valid;
> + struct nd_papr_scm_dimm_health_stat health;
> };
>
> static int drc_pmem_bind(struct papr_scm_priv *p)
> @@ -158,6 +157,7 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)
> {
> unsigned long ret[PLPAR_HCALL_BUFSIZE];
> int64_t rc;
> + __be64 health;
>
> rc = plpar_hcall(H_SCM_HEALTH, ret, p->drc_index);
> if (rc != H_SUCCESS) {
> @@ -172,13 +172,41 @@ static int drc_pmem_query_health(struct papr_scm_priv *p)
> return rc;
>
> /* Store the retrieved health information in dimm platform data */
> - p->health_bitmap = ret[0];
> - p->health_bitmap_valid = ret[1];
> + health = ret[0] & ret[1];
>
> dev_dbg(&p->pdev->dev,
> "Queried dimm health info. Bitmap:0x%016llx Mask:0x%016llx\n",
> - be64_to_cpu(p->health_bitmap),
> - be64_to_cpu(p->health_bitmap_valid));
> + be64_to_cpu(ret[0]),
> + be64_to_cpu(ret[1]));
> +
> + memset(&p->health, 0, sizeof(p->health));
> +
> + /* Check for various masks in bitmap and set the buffer */
> + if (health & PAPR_SCM_DIMM_UNARMED_MASK)
> + p->health.dimm_unarmed = true;
> +
> + if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
> + p->health.dimm_bad_shutdown = true;
> +
> + if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
> + p->health.dimm_bad_restore = true;
> +
> + if (health & PAPR_SCM_DIMM_ENCRYPTED)
> + p->health.dimm_encrypted = true;
> +
> + if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED) {
> + p->health.dimm_locked = true;
> + p->health.dimm_scrubbed = true;
> + }
I don't think bool is suitable for ioctl ABI. For example the true
value may be positive, or negative depending on the arch. This should
be using explicit integer values.
I assume the reason you are translating this rather than transmitting
that raw 64-bit health value is for future compatibility if the HCALL
format changes / is extended?
> +
> + if (health & PAPR_SCM_DIMM_HEALTH_UNHEALTHY)
> + p->health.dimm_health = DSM_PAPR_SCM_DIMM_UNHEALTHY;
> +
> + if (health & PAPR_SCM_DIMM_HEALTH_CRITICAL)
> + p->health.dimm_health = DSM_PAPR_SCM_DIMM_CRITICAL;
> +
> + if (health & PAPR_SCM_DIMM_HEALTH_FATAL)
> + p->health.dimm_health = DSM_PAPR_SCM_DIMM_FATAL;
>
> mutex_unlock(&p->dimm_mutex);
> return 0;
> @@ -331,6 +359,51 @@ static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
> return 0;
> }
>
> +/* Fetch the DIMM health info and populate it in provided package. */
> +static int papr_scm_get_health(struct papr_scm_priv *p,
> + struct nd_papr_scm_cmd_pkg *pkg)
> +{
> + int rc;
> + size_t copysize = sizeof(p->health);
> +
> + rc = drc_pmem_query_health(p);
> + if (rc)
> + goto out;
> + /*
> + * If the requested payload version is greater than one we know
> + * about, return the payload version we know about and let
> + * caller/userspace handle.
> + */
> + if (pkg->payload_version > ND_PAPR_SCM_DIMM_HEALTH_VERSION)
> + pkg->payload_version = ND_PAPR_SCM_DIMM_HEALTH_VERSION;
> +
> + if (pkg->hdr.nd_size_out < copysize) {
> + dev_dbg(&p->pdev->dev, "%s Payload not large enough\n",
> + __func__);
> + dev_dbg(&p->pdev->dev, "%s Expected %lu, available %u\n",
> + __func__, copysize, pkg->hdr.nd_size_out);
> + rc = -ENOSPC;
> + goto out;
> + }
> +
> + dev_dbg(&p->pdev->dev, "%s Copying payload size=%lu version=0x%x\n",
> + __func__, copysize, pkg->payload_version);
> +
> + /* Copy a subset of health struct based on copysize */
> + memcpy(papr_scm_pcmd_to_payload(pkg), &p->health, copysize);
> + pkg->hdr.nd_fw_size = copysize;
> +
> +out:
> + /*
> + * Put the error in out package and return success from function
> + * so that errors if any are propogated back to userspace.
> + */
> + pkg->cmd_status = rc;
> + dev_dbg(&p->pdev->dev, "%s completion code = %d\n", __func__, rc);
> +
> + return 0;
> +}
> +
> static int papr_scm_service_dsm(struct papr_scm_priv *p,
> struct nd_papr_scm_cmd_pkg *call_pkg)
> {
> @@ -345,6 +418,9 @@ static int papr_scm_service_dsm(struct papr_scm_priv *p,
>
> /* Depending on the DSM command call appropriate service routine */
> switch (call_pkg->hdr.nd_command) {
> + case DSM_PAPR_SCM_HEALTH:
> + return papr_scm_get_health(p, call_pkg);
> +
> default:
> pr_debug("Unsupported DSM command 0x%llx\n",
> call_pkg->hdr.nd_command);
> @@ -431,7 +507,6 @@ static ssize_t papr_flags_show(struct device *dev,
> {
> struct nvdimm *dimm = to_nvdimm(dev);
> struct papr_scm_priv *p = nvdimm_provider_data(dimm);
> - __be64 health;
> int rc;
>
> rc = drc_pmem_query_health(p);
> @@ -443,26 +518,26 @@ static ssize_t papr_flags_show(struct device *dev,
> if (rc)
> return rc;
>
> - health = p->health_bitmap & p->health_bitmap_valid;
> -
> - /* Check for various masks in bitmap and set the buffer */
> - if (health & PAPR_SCM_DIMM_UNARMED_MASK)
> + if (p->health.dimm_unarmed)
> rc += sprintf(buf, "not_armed ");
>
> - if (health & PAPR_SCM_DIMM_BAD_SHUTDOWN_MASK)
> + if (p->health.dimm_bad_shutdown)
> rc += sprintf(buf + rc, "save_fail ");
Per my patch1 comment is this "save_fail" or "flush_fail"?
>
> - if (health & PAPR_SCM_DIMM_BAD_RESTORE_MASK)
> + if (p->health.dimm_bad_restore)
> rc += sprintf(buf + rc, "restore_fail ");
>
> - if (health & PAPR_SCM_DIMM_ENCRYPTED)
> + if (p->health.dimm_encrypted)
> rc += sprintf(buf + rc, "encrypted ");
>
> - if (health & PAPR_SCM_DIMM_SMART_EVENT_MASK)
> + if (p->health.dimm_health)
> rc += sprintf(buf + rc, "smart_notify ");
>
> - if (health & PAPR_SCM_DIMM_SCRUBBED_AND_LOCKED)
> - rc += sprintf(buf + rc, "scrubbed locked ");
> + if (p->health.dimm_scrubbed)
> + rc += sprintf(buf + rc, "scrubbed ");
> +
> + if (p->health.dimm_locked)
> + rc += sprintf(buf + rc, "locked ");
>
> if (rc > 0)
> rc += sprintf(buf + rc, "\n");
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH v2] powerpc/XIVE: SVM: share the event-queue page with the Hypervisor.
From: Thiago Jung Bauermann @ 2020-04-03 18:24 UTC (permalink / raw)
To: Ram Pai; +Cc: aik, andmike, groug, kvm-ppc, clg, sukadev, linuxppc-dev, david
In-Reply-To: <20200401053200.GE5903@oc0525413822.ibm.com>
Ram Pai <linuxram@us.ibm.com> writes:
> On Tue, Mar 31, 2020 at 08:53:07PM -0300, Thiago Jung Bauermann wrote:
>>
>> Hi Ram,
>>
>> Ram Pai <linuxram@us.ibm.com> writes:
>>
>> > diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
>> > index 55dc61c..608b52f 100644
>> > --- a/arch/powerpc/sysdev/xive/spapr.c
>> > +++ b/arch/powerpc/sysdev/xive/spapr.c
>> > @@ -26,6 +26,8 @@
>> > #include <asm/xive.h>
>> > #include <asm/xive-regs.h>
>> > #include <asm/hvcall.h>
>> > +#include <asm/svm.h>
>> > +#include <asm/ultravisor.h>
>> >
>> > #include "xive-internal.h"
>> >
>> > @@ -501,6 +503,9 @@ static int xive_spapr_configure_queue(u32 target, struct xive_q *q, u8 prio,
>> > rc = -EIO;
>> > } else {
>> > q->qpage = qpage;
>> > + if (is_secure_guest())
>> > + uv_share_page(PHYS_PFN(qpage_phys),
>> > + 1 << xive_alloc_order(order));
>>
>> If I understand this correctly, you're passing the number of bytes of
>> the queue to uv_share_page(), but that ultracall expects the number of
>> pages to be shared.
>
>
> static inline u32 xive_alloc_order(u32 queue_shift)
> {
> return (queue_shift > PAGE_SHIFT) ? (queue_shift - PAGE_SHIFT) : 0;
> }
>
> xive_alloc_order(order) returns the order of PAGE_SIZE pages.
> Hence the value passed to uv_shared_pages is the number of pages,
> and not the number of bytes.
>
> BTW: I did verify through testing that it was indeed passing 1 page to the
> uv_share_page().
Ah, my mistake. I misunderstood the code. Sorry for the noise and thanks
for clarifying.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v4 6/6] pseries/sysfs: Minimise IPI noise while reading [idle_][s]purr
From: Nathan Lynch @ 2020-04-03 18:10 UTC (permalink / raw)
To: Gautham R Shenoy
Cc: Tyrel Datwyler, linux-kernel, Kamalesh Babulal, Naveen N. Rao,
Vaidyanathan Srinivasan, linuxppc-dev
In-Reply-To: <20200403062818.GB9066@in.ibm.com>
Gautham R Shenoy <ego@linux.vnet.ibm.com> writes:
> On Thu, Apr 02, 2020 at 01:04:34PM +0530, Naveen N. Rao wrote:
>> >>
>> >>I wonder if we should introduce a sysctl interface to control thresholding.
>> >>It can default to 0, which disables thresholding so that the existing
>> >>behavior continues. Applications (lparstat) can optionally set it to suit
>> >>their use.
>> >
>> >We would be introducing 3 new sysfs interfaces that way instead of
>> >two.
>> >
>> >/sys/devices/system/cpu/purr_spurr_staleness
>> >/sys/devices/system/cpu/cpuX/idle_purr
>> >/sys/devices/system/cpu/cpuX/idle_spurr
>> >
>> >I don't have a problem with this. Nathan, Michael, thoughts on this?
No, I don't think this warrants a tunable when the issue it's intended
to address is still a bit speculative at this point. (Also, note that
this would be a system-wide value, but you could have multiple
concurrent users of the interface with different needs.)
>> >The alternative is to have a procfs interface, something like
>> >/proc/powerpc/resource_util_stats
>> >
>> >which gives a listing similar to /proc/stat, i.e
>> >
>> > CPUX <purr> <idle_purr> <spurr> <idle_spurr>
>> >
>> >Even in this case, the values can be obtained in one-shot with a
>> >single IPI and be printed in the row corresponding to the CPU.
>>
>> Right -- and that would be optimal requiring a single system call, at the
>> cost of using a legacy interface.
>>
>> The other option would be to drop this patch and to just go with patches 1-5
>> introducing the new sysfs interfaces for idle_[s]purr. It isn't entirely
>> clear how often this would be used, or its actual impact. We can perhaps
>> consider this optimization if and when this causes problems...
>
> I am ok with that. We can revisit the problem if IPI noise becomes
> noticable. However, if Nathan or Michael feel that this problem is
> better solved now, than leaving it for the future, we will have to
> take a call on what the interface is going to be.
While I maintain some concern about the overhead on larger LPARs (150us
per CPU works out to ~0.15s total to serially sample 1024 CPUs, ~0.3s
for 2048 and so on), I am OK with the straightforward addition of the
attributes without any batching or sampling thresholds behind the scenes
for now. I appreciate your consideration of the issue.
If this turns out to be too inefficient then I think we should consider
a non-sysfs mechanism such as chardev+ioctl.
^ permalink raw reply
* Re: [PATCH v2 5/5] uaccess: Rename user_access_begin/end() to user_full_access_begin/end()
From: Linus Torvalds @ 2020-04-03 18:01 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, Kees Cook, Dave Airlie, intel-gfx, Peter Anvin,
Linux Kernel Mailing List, Linux-MM, Paul Mackerras, Al Viro,
Daniel Vetter, Andrew Morton, linuxppc-dev
In-Reply-To: <42da416106d5c1cf92bda1e058434fe240b35f44.1585898438.git.christophe.leroy@c-s.fr>
On Fri, Apr 3, 2020 at 12:21 AM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
> Now we have user_read_access_begin() and user_write_access_begin()
> in addition to user_access_begin().
I realize Al asked for this, but I don't think it really adds anything
to the series.
The "full" makes the names longer, but not really any more legible.
So I like 1-4, but am unconvinced about 5 and would prefer that to be
dropped. Sorry for the bikeshedding.
And I like this series much better without the cookie that was
discussed, and just making the hard rule be that they can't nest.
Some architecture may obviously use a cookie internally if they have
some nesting behavior of their own, but it doesn't look like we have
any major reason to expose that as the actual interface.
The only other question is how to synchronize this? I'm ok with it
going through the ppc tree, for example, and just let others build on
that. Maybe using a shared immutable branch with 5.6 as a base?
Linus
^ permalink raw reply
* Re: [PATCH v5 3/4] powerpc/papr_scm,uapi: Add support for handling PAPR DSM commands
From: Dan Williams @ 2020-04-03 17:40 UTC (permalink / raw)
To: Vaibhav Jain
Cc: Alastair D'Silva, linux-nvdimm, Aneesh Kumar K . V,
Jeff Moyer, Oliver O'Halloran, Vishal Verma, Michael Ellerman,
linuxppc-dev
In-Reply-To: <20200331143229.306718-4-vaibhav@linux.ibm.com>
On Tue, Mar 31, 2020 at 7:33 AM Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>
> Implement support for handling PAPR DSM commands in papr_scm
> module. We advertise support for ND_CMD_CALL for the dimm command mask
> and implement necessary scaffolding in the module to handle ND_CMD_CALL
> ioctl and DSM commands that we receive.
They aren't ACPI Device Specific Methods in the papr_scm case, right?
I'd call them what the papr_scm specification calls them and replace
"DSM" throughout.
> The layout of the DSM commands as we expect from libnvdimm/libndctl is
> described in newly introduced uapi header 'papr_scm_dsm.h' which
> defines a new 'struct nd_papr_scm_cmd_pkg' header. This header is used
> to communicate the DSM command via 'nd_pkg_papr_scm->nd_command' and
> size of payload that need to be sent/received for servicing the DSM.
>
> The PAPR DSM commands are assigned indexes started from 0x10000 to
> prevent them from overlapping ND_CMD_* values and also makes handling
> dimm commands in papr_scm_ndctl().
You don't necessarily need to have command number separation like
that. The function number spaces are unique per family.
> A new function cmd_to_func() is
> implemented that reads the args to papr_scm_ndctl() and performs
> sanity tests on them. In case of a DSM command being sent via
> ND_CMD_CALL a newly introduced function papr_scm_service_dsm() is
> called to handle the request.
>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>
> ---
> Changelog:
>
> v4..v5: Fixed a bug in new implementation of papr_scm_ndctl().
>
> v3..v4: Updated papr_scm_ndctl() to delegate DSM command handling to a
> different function papr_scm_service_dsm(). [Aneesh]
>
> v2..v3: Updated the nd_papr_scm_cmd_pkg to use __xx types as its
> exported to the userspace [Aneesh]
>
> v1..v2: New patch in the series.
> ---
> arch/powerpc/include/uapi/asm/papr_scm_dsm.h | 161 +++++++++++++++++++
> arch/powerpc/platforms/pseries/papr_scm.c | 97 ++++++++++-
> 2 files changed, 252 insertions(+), 6 deletions(-)
> create mode 100644 arch/powerpc/include/uapi/asm/papr_scm_dsm.h
>
> diff --git a/arch/powerpc/include/uapi/asm/papr_scm_dsm.h b/arch/powerpc/include/uapi/asm/papr_scm_dsm.h
> new file mode 100644
> index 000000000000..c039a49b41b4
> --- /dev/null
> +++ b/arch/powerpc/include/uapi/asm/papr_scm_dsm.h
> @@ -0,0 +1,161 @@
> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> +/*
> + * PAPR SCM Device specific methods and struct for libndctl and ndctl
> + *
> + * (C) Copyright IBM 2020
> + *
> + * Author: Vaibhav Jain <vaibhav at linux.ibm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2, or (at your option)
> + * any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
These 2 paragraphs of redundant license text can be dropped. The SPDX
line is sufficient.
> + */
> +
> +#ifndef _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_
> +#define _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_
> +
> +#include <linux/types.h>
> +
> +#ifdef __KERNEL__
> +#include <linux/ndctl.h>
> +#else
> +#include <ndctl.h>
> +#endif
> +
> +/*
> + * DSM Envelope:
> + *
> + * The ioctl ND_CMD_CALL transfers data between user-space and kernel via
> + * 'envelopes' which consists of a header and user-defined payload sections.
> + * The header is described by 'struct nd_papr_scm_cmd_pkg' which expects a
> + * payload following it and offset of which relative to the struct is provided
> + * by 'nd_papr_scm_cmd_pkg.payload_offset'. *
> + *
> + * +-------------+---------------------+---------------------------+
> + * | 64-Bytes | 8-Bytes | Max 184-Bytes |
> + * +-------------+---------------------+---------------------------+
> + * | nd_papr_scm_cmd_pkg | |
> + * |-------------+ | |
> + * | nd_cmd_pkg | | |
> + * +-------------+---------------------+---------------------------+
> + * | nd_family | | |
> + * | nd_size_out | cmd_status | |
> + * | nd_size_in | payload_version | PAYLOAD |
> + * | nd_command | payload_offset -----> |
> + * | nd_fw_size | | |
> + * +-------------+---------------------+---------------------------+
> + *
> + * DSM Header:
> + *
> + * The header is defined as 'struct nd_papr_scm_cmd_pkg' which embeds a
> + * 'struct nd_cmd_pkg' instance. The DSM command is assigned to member
> + * 'nd_cmd_pkg.nd_command'. Apart from size information of the envelop which is
s/envelop/envelope/
There's a difference between commands and functions in libnvdimm.
Commands are the top-level ioctl methods where ND_CMD_CALL is the
generic payload command and functions are communicated inside of that
wrapper. Otherwise if the command does not wrap other commands like
ND_CMD_{GET,SET}_CONFIG_DATA then the individual bus providers
translate them to the bus-local functionality. For example
ND_CMD_{GET,SET}_CONFIG_DATA correspond to either the _LSx ACPI
methods, or the Intel DSM label methods. The details of those
implementations are hidden from the caller, the caller just sees the
kernel's ND_CMD_{GET,SET}_CONFIG_DATA interface.
> + * contained in 'struct nd_cmd_pkg', the header also has members following
> + * members:
> + *
> + * 'cmd_status' : (Out) Errors if any encountered while servicing DSM.
> + * 'payload_version' : (In/Out) Version number associated with the payload.
> + * 'payload_offset' : (In)Relative offset of payload from start of envelope.
> + *
> + * DSM Payload:
> + *
> + * The layout of the DSM Payload is defined by various structs shared between
> + * papr_scm and libndctl so that contents of payload can be interpreted. During
> + * servicing of a DSM the papr_scm module will read input args from the payload
> + * field by casting its contents to an appropriate struct pointer based on the
> + * DSM command. Similarly the output of servicing the DSM command will be copied
> + * to the payload field using the same struct.
> + *
> + * 'libnvdimm' enforces a hard limit of 256 bytes on the envelope size, which
> + * leaves around 184 bytes for the envelope payload (ignoring any padding that
> + * the compiler may silently introduce).
> + *
> + * Payload Version:
> + *
> + * A 'payload_version' field is present in DSM header that indicates a specific
> + * version of the structure present in DSM Payload for a given DSM command. This
> + * provides backward compatibility in case the DSM Payload structure evolves
> + * and different structures are supported by 'papr_scm' and 'libndctl'.
> + *
> + * When sending a DSM Payload to 'papr_scm', 'libndctl' should send the version
> + * of the payload struct it supports via 'payload_version' field. The 'papr_scm'
> + * module when servicing the DSM envelop checks the 'payload_version' and then
s/envelop/envelope/
> + * uses 'payload struct version' == MIN('payload_version field',
> + * 'max payload-struct-version supported by papr_scm') to service the DSM. After
> + * servicing the DSM, 'papr_scm' put the negotiated version of payload struct in
> + * returned 'payload_version' field.
> + *
> + * Libndctl on receiving the envelop back from papr_scm again checks the
s/envelop/envelope/
> + * 'payload_version' field and based on it use the appropriate version dsm
> + * struct to parse the results.
> + *
> + * Backward Compatibility:
> + *
> + * Above scheme of exchanging different versioned DSM struct between libndctl
> + * and papr_scm should provide backward compatibility until following two
> + * assumptions/conditions when defining new DSM structs hold:
> + *
> + * Let T(X) = { set of attributes in DSM struct 'T' versioned X }
> + *
> + * 1. T(X) is a proper subset of T(Y) if X > Y.
> + * i.e Each new version of DSM struct should retain existing struct
> + * attributes from previous version
> + *
> + * 2. If an entity (libndctl or papr_scm) supports a DSM struct T(X) then
> + * it should also support T(1), T(2)...T(X - 1).
> + * i.e When adding support for new version of a DSM struct, libndctl
> + * and papr_scm should retain support of the existing DSM struct
> + * version they support.
> + */
> +
> +/* Papr-scm-header + payload expected with ND_CMD_CALL ioctl from libnvdimm */
> +struct nd_papr_scm_cmd_pkg {
> + struct nd_cmd_pkg hdr; /* Package header containing sub-cmd */
> + __s32 cmd_status; /* Out: Sub-cmd status returned back */
> + __u16 payload_offset; /* In: offset from start of struct */
> + __u16 payload_version; /* In/Out: version of the payload */
> + __u8 payload[]; /* In/Out: Sub-cmd data buffer */
Does this structure pack the same on all compilers?
> +};
> +
> +/*
> + * Sub commands for ND_CMD_CALL. To prevent overlap from ND_CMD_*, values for
> + * these enums start at 0x10000. These values are then returned from
> + * cmd_to_func() making it easy to implement the switch-case block in
> + * papr_scm_ndctl(). These commands are sent to the kernel via
> + * 'nd_papr_scm_cmd_pkg.hdr.nd_command'
> + */
With nd_family set to NVDIMM_FAMILY_PAPR_SCM the function number space
can overlap the function numbers from other families.
> +enum dsm_papr_scm {
> + DSM_PAPR_SCM_MIN = 0x10000,
> + DSM_PAPR_SCM_MAX,
> +};
> +
> +/* Helpers to evaluate the size of PAPR_SCM envelope */
> +/* Calculate the papr_scm-header size */
> +#define ND_PAPR_SCM_ENVELOPE_CONTENT_HDR_SIZE \
> + (sizeof(struct nd_papr_scm_cmd_pkg) - sizeof(struct nd_cmd_pkg))
> +
> +/* Given a type calculate envelope-content size (papr_scm-header + payload) */
> +#define ND_PAPR_SCM_ENVELOPE_CONTENT_SIZE(_type_) \
> + (sizeof(_type_) + ND_PAPR_SCM_ENVELOPE_CONTENT_HDR_SIZE)
> +
> +/* Convert a libnvdimm nd_cmd_pkg to papr_scm specific pkg */
> +static struct nd_papr_scm_cmd_pkg *nd_to_papr_cmd_pkg(struct nd_cmd_pkg *cmd)
> +{
> + return (struct nd_papr_scm_cmd_pkg *) cmd;
> +}
> +
> +/* Return the payload pointer for a given pcmd */
> +static void *papr_scm_pcmd_to_payload(struct nd_papr_scm_cmd_pkg *pcmd)
> +{
> + if (pcmd->hdr.nd_size_in == 0 && pcmd->hdr.nd_size_out == 0)
> + return NULL;
> + else
> + return (void *)((__u8 *) pcmd + pcmd->payload_offset);
> +}
> +#endif /* _UAPI_ASM_POWERPC_PAPR_SCM_DSM_H_ */
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
> index aaf2e4ab1f75..e8ce96d2249e 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -15,13 +15,15 @@
>
> #include <asm/plpar_wrappers.h>
> #include <asm/papr_scm.h>
> +#include <asm/papr_scm_dsm.h>
>
> #define BIND_ANY_ADDR (~0ul)
>
> #define PAPR_SCM_DIMM_CMD_MASK \
> ((1ul << ND_CMD_GET_CONFIG_SIZE) | \
> (1ul << ND_CMD_GET_CONFIG_DATA) | \
> - (1ul << ND_CMD_SET_CONFIG_DATA))
> + (1ul << ND_CMD_SET_CONFIG_DATA) | \
> + (1ul << ND_CMD_CALL))
>
> struct papr_scm_priv {
> struct platform_device *pdev;
> @@ -283,15 +285,92 @@ static int papr_scm_meta_set(struct papr_scm_priv *p,
> return 0;
> }
>
> +/*
> + * Validate the inputs args to dimm-control function and return '0' if valid.
> + * This also does initial sanity validation to ND_CMD_CALL sub-command packages.
> + */
> +static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
> + unsigned int buf_len)
> +{
> + unsigned long cmd_mask = PAPR_SCM_DIMM_CMD_MASK;
> + struct nd_papr_scm_cmd_pkg *pkg = nd_to_papr_cmd_pkg(buf);
> +
> + /* Only dimm-specific calls are supported atm */
> + if (!nvdimm)
> + return -EINVAL;
> +
> + if (!test_bit(cmd, &cmd_mask)) {
> + pr_debug("%s: Unsupported cmd=%u\n", __func__, cmd);
Please convert all the pr_debug() statements in this file to
dev_dbg(). Also you don't need to put the __func__ in the print
statement because you can enable that via dynamic_debug_control see
the "f" flag in addition to the "p" flag for dynamic debug enabling.
> + return -EINVAL;
> + } else if (cmd == ND_CMD_CALL) {
> +
> + /* Verify the envelop package */
s/envelop/envelope/
> + if (!buf || buf_len < sizeof(struct nd_papr_scm_cmd_pkg)) {
> + pr_debug("%s: Invalid pkg size=%u\n", __func__,
> + buf_len);
> + return -EINVAL;
> + }
> +
> + /* Verify that the DSM command family is valid */
> + if (pkg->hdr.nd_family != NVDIMM_FAMILY_PAPR_SCM) {
> + pr_debug("%s: Invalid pkg family=0x%llx\n", __func__,
> + pkg->hdr.nd_family);
> + return -EINVAL;
> +
> + }
> +
> + /* We except a payload with all DSM commands */
> + if (papr_scm_pcmd_to_payload(pkg) == NULL) {
> + pr_debug("%s: Empty payload for sub-command=0x%llx\n",
> + __func__, pkg->hdr.nd_command);
> + return -EINVAL;
> + }
> + }
> +
> + /* Command looks valid */
> + return 0;
> +}
> +
> +static int papr_scm_service_dsm(struct papr_scm_priv *p,
> + struct nd_papr_scm_cmd_pkg *call_pkg)
> +{
> + /* unknown subcommands return error in packages */
> + if (call_pkg->hdr.nd_command <= DSM_PAPR_SCM_MIN ||
> + call_pkg->hdr.nd_command >= DSM_PAPR_SCM_MAX) {
> + pr_debug("Invalid DSM command 0x%llx\n",
> + call_pkg->hdr.nd_command);
> + call_pkg->cmd_status = -EINVAL;
> + return 0;
> + }
> +
> + /* Depending on the DSM command call appropriate service routine */
> + switch (call_pkg->hdr.nd_command) {
> + default:
> + pr_debug("Unsupported DSM command 0x%llx\n",
> + call_pkg->hdr.nd_command);
> + call_pkg->cmd_status = -ENOENT;
> + return 0;
> + }
> +}
> +
> int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
> unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
> {
> struct nd_cmd_get_config_size *get_size_hdr;
> struct papr_scm_priv *p;
> + struct nd_papr_scm_cmd_pkg *call_pkg = NULL;
> + int rc;
>
> - /* Only dimm-specific calls are supported atm */
> - if (!nvdimm)
> - return -EINVAL;
> + /* Use a local variable in case cmd_rc pointer is NULL */
> + if (cmd_rc == NULL)
> + cmd_rc = &rc;
> +
> + *cmd_rc = is_cmd_valid(nvdimm, cmd, buf, buf_len);
> + if (*cmd_rc) {
> + pr_debug("%s: Invalid cmd=0x%x. Err=%d\n", __func__,
> + cmd, *cmd_rc);
> + return *cmd_rc;
> + }
>
> p = nvdimm_provider_data(nvdimm);
>
> @@ -313,13 +392,19 @@ int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
> *cmd_rc = papr_scm_meta_set(p, buf);
> break;
>
> + case ND_CMD_CALL:
> + call_pkg = nd_to_papr_cmd_pkg(buf);
> + *cmd_rc = papr_scm_service_dsm(p, call_pkg);
> + break;
> +
> default:
> - return -EINVAL;
> + dev_dbg(&p->pdev->dev, "Unknown command = %d\n", cmd);
> + *cmd_rc = -EINVAL;
> }
>
> dev_dbg(&p->pdev->dev, "returned with cmd_rc = %d\n", *cmd_rc);
>
> - return 0;
> + return *cmd_rc;
> }
>
> static inline int papr_scm_node(int node)
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH RESEND 1/4] uaccess: Add user_read_access_begin/end and user_write_access_begin/end
From: Al Viro @ 2020-04-03 17:26 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: linux-arch, linuxppc-dev, Kees Cook, Christian Borntraeger,
airlied, Catalin Marinas, hpa, linux-kernel, linux-mm,
Paul Mackerras, daniel, akpm, torvalds
In-Reply-To: <20200403133719.GC25745@shell.armlinux.org.uk>
On Fri, Apr 03, 2020 at 02:37:19PM +0100, Russell King - ARM Linux admin wrote:
> > I think uaccess_enable() could indeed switch the kernel domain if
> > KERNEL_DS is set and move this out of set_fs(). It would reduce the
> > window the kernel domain permissions are overridden. Anyway,
> > uaccess_enable() appeared much later on arm when Russell introduced PAN
> > (SMAP) like support by switching the user domain.
>
> Yes, that would be a possibility. Another possibility would be to
> eliminate as much usage of KERNEL_DS as possible
That's definitely worth doing, but that's another long-term project ;-/
> - I've just found
> one instance in sys_oabi-compat.c that can be eliminated (epoll_ctl)
> but there's several there that can't with the current code structure,
> and re-coding the contents of some fs/* functions to work around that
> is a very bad idea. If there's some scope for rejigging some of the
> fs/* code, it may be possible to elimate some other cases in there.
Well, your do_locks() definitely can be converted. epoll_wait()...
not sure, need to look into that. Is that about the layout mismatch
between struct oabi_epoll_event and struct epoll_event? In case of
semtimedop... Hell knows, I would probably consider moving that thing
into ipc/sem.c under ifdef...
^ permalink raw reply
* Re: [PATCH v5 2/4] ndctl/uapi: Introduce NVDIMM_FAMILY_PAPR_SCM as a new NVDIMM DSM family
From: Dan Williams @ 2020-04-03 16:50 UTC (permalink / raw)
To: Vaibhav Jain
Cc: Alastair D'Silva, linux-nvdimm, Aneesh Kumar K . V,
Jeff Moyer, Oliver O'Halloran, Vishal Verma, Michael Ellerman,
linuxppc-dev
In-Reply-To: <20200331143229.306718-3-vaibhav@linux.ibm.com>
On Tue, Mar 31, 2020 at 7:33 AM Vaibhav Jain <vaibhav@linux.ibm.com> wrote:
>
> Add PAPR-scm family of DSM command-set to the white list of NVDIMM
> command sets.
>
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> Changelog:
>
> v4..v5 : None
>
> v3..v4 : None
>
> v2..v3 : Updated the patch prefix to 'ndctl/uapi' [Aneesh]
>
> v1..v2 : None
> ---
> include/uapi/linux/ndctl.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
> index de5d90212409..99fb60600ef8 100644
> --- a/include/uapi/linux/ndctl.h
> +++ b/include/uapi/linux/ndctl.h
> @@ -244,6 +244,7 @@ struct nd_cmd_pkg {
> #define NVDIMM_FAMILY_HPE2 2
> #define NVDIMM_FAMILY_MSFT 3
> #define NVDIMM_FAMILY_HYPERV 4
> +#define NVDIMM_FAMILY_PAPR_SCM 5
Looks good, but please squash it with patch 3.
^ permalink raw reply
* Re: [PATCH v3 0/5] mm: Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA
From: Hoan Tran @ 2020-04-03 16:36 UTC (permalink / raw)
To: Baoquan He
Cc: mmorana, Catalin Marinas, Heiko Carstens, Michal Hocko,
open list:MEMORY MANAGEMENT, Paul Mackerras, H. Peter Anvin,
sparclinux, Alexander Duyck, linux-s390, x86, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, Vlastimil Babka,
Pavel Tatashin, lho, Vasily Gorbik, Will Deacon, Borislav Petkov,
Thomas Gleixner, linux-arm-kernel, Oscar Salvador, linux-kernel,
Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200403070904.GO2402@MiWiFi-R3L-srv>
Hi,
On 4/3/20 12:09 AM, Baoquan He wrote:
> On 04/02/20 at 09:46pm, Hoan Tran wrote:
>> Hi All,
>>
>> On 3/31/20 7:31 AM, Baoquan He wrote:
>>> On 03/31/20 at 04:21pm, Michal Hocko wrote:
>>>> On Tue 31-03-20 22:03:32, Baoquan He wrote:
>>>>> Hi Michal,
>>>>>
>>>>> On 03/31/20 at 10:55am, Michal Hocko wrote:
>>>>>> On Tue 31-03-20 11:14:23, Mike Rapoport wrote:
>>>>>>> Maybe I mis-read the code, but I don't see how this could happen. In the
>>>>>>> HAVE_MEMBLOCK_NODE_MAP=y case, free_area_init_node() calls
>>>>>>> calculate_node_totalpages() that ensures that node->node_zones are entirely
>>>>>>> within the node because this is checked in zone_spanned_pages_in_node().
>>>>>>
>>>>>> zone_spanned_pages_in_node does chech the zone boundaries are within the
>>>>>> node boundaries. But that doesn't really tell anything about other
>>>>>> potential zones interleaving with the physical memory range.
>>>>>> zone->spanned_pages simply gives the physical range for the zone
>>>>>> including holes. Interleaving nodes are essentially a hole
>>>>>> (__absent_pages_in_range is going to skip those).
>>>>>>
>>>>>> That means that when free_area_init_core simply goes over the whole
>>>>>> physical zone range including holes and that is why we need to check
>>>>>> both for physical and logical holes (aka other nodes).
>>>>>>
>>>>>> The life would be so much easier if the whole thing would simply iterate
>>>>>> over memblocks...
>>>>>
>>>>> The memblock iterating sounds a great idea. I tried with putting the
>>>>> memblock iterating in the upper layer, memmap_init(), which is used for
>>>>> boot mem only anyway. Do you think it's doable and OK? It yes, I can
>>>>> work out a formal patch to make this simpler as you said. The draft code
>>>>> is as below. Like this it uses the existing code and involves little change.
>>>>
>>>> Doing this would be a step in the right direction! I haven't checked the
>>>> code very closely though. The below sounds way too simple to be truth I
>>>> am afraid. First for_each_mem_pfn_range is available only for
>>>> CONFIG_HAVE_MEMBLOCK_NODE_MAP (which is one of the reasons why I keep
>>>> saying that I really hate that being conditional). Also I haven't really
>>>> checked the deferred initialization path - I have a very vague
>>>> recollection that it has been converted to the memblock api but I have
>>>> happilly dropped all that memory.
>>>
>>> Thanks for your quick response and pointing out the rest suspect aspects,
>>> I will investigate what you mentioned, see if they impact.
>>
>> I would like to check if we still move on with my patch to remove
>> CONFIG_NODES_SPAN_OTHER_NODES and have another patch on top it?
>
> I think we would like to replace CONFIG_NODES_SPAN_OTHER_NODES with
> CONFIG_NUMA, and just let UMA return 0 as node id, as Michal replied in
> another mail. Anyway, your patch 2~5 are still needed to sit on top of
> the change of this new plan.
Got it. Thanks for quick response.
Regards
Hoan
>
^ permalink raw reply
* Re: [PATCH v2 4/4] ocxl: Remove custom service to allocate interrupts
From: Andrew Donnellan @ 2020-04-03 16:09 UTC (permalink / raw)
To: Frederic Barrat, linuxppc-dev, clg, christophe_lombard, ukrishn,
mrochs
Cc: haren, groug
In-Reply-To: <20200403153838.29224-5-fbarrat@linux.ibm.com>
On 4/4/20 2:38 am, Frederic Barrat wrote:
> We now allocate interrupts through xive directly.
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---
No obvious issues on a cursory review
Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH v2 3/4] ocxl: Don't return trigger page when allocating an interrupt
From: Andrew Donnellan @ 2020-04-03 16:08 UTC (permalink / raw)
To: Frederic Barrat, linuxppc-dev, clg, christophe_lombard, ukrishn,
mrochs
Cc: haren, groug
In-Reply-To: <20200403153838.29224-4-fbarrat@linux.ibm.com>
On 4/4/20 2:38 am, Frederic Barrat wrote:
> Existing users of ocxl_link_irq_alloc() have been converted to obtain
> the trigger page of an interrupt through xive directly, we therefore
> have no need to return the trigger page when allocating an interrupt.
>
> It also allows ocxl to use the xive native interface to allocate
> interrupts, instead of its custom service.
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
No obvious issues on a cursory review
Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH v2 1/4] scsi: cxlflash: Access interrupt trigger page from xive directly
From: Cédric Le Goater @ 2020-04-03 15:48 UTC (permalink / raw)
To: Frederic Barrat, linuxppc-dev, christophe_lombard, ajd, ukrishn,
mrochs
Cc: haren, groug
In-Reply-To: <20200403153838.29224-2-fbarrat@linux.ibm.com>
On 4/3/20 5:38 PM, Frederic Barrat wrote:
> xive is already mapping the trigger page in kernel space and it can be
> accessed through standard APIs, so let's reuse it and simplify the code.
>
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> Changelog:
> v2: Define rc = -ENXIO on the error path (Matt)
>
>
> drivers/scsi/cxlflash/ocxl_hw.c | 18 ++++++++----------
> drivers/scsi/cxlflash/ocxl_hw.h | 1 -
> 2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
> index 7018cd802569..d6eec434a607 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.c
> +++ b/drivers/scsi/cxlflash/ocxl_hw.c
> @@ -15,7 +15,8 @@
> #include <linux/pseudo_fs.h>
> #include <linux/poll.h>
> #include <linux/sched/signal.h>
> -
> +#include <linux/interrupt.h>
> +#include <asm/xive.h>
> #include <misc/ocxl.h>
>
> #include <uapi/misc/cxl.h>
> @@ -180,7 +181,7 @@ static int afu_map_irq(u64 flags, struct ocxlflash_context *ctx, int num,
> struct ocxl_hw_afu *afu = ctx->hw_afu;
> struct device *dev = afu->dev;
> struct ocxlflash_irqs *irq;
> - void __iomem *vtrig;
> + struct xive_irq_data *xd;
> u32 virq;
> int rc = 0;
>
> @@ -204,15 +205,15 @@ static int afu_map_irq(u64 flags, struct ocxlflash_context *ctx, int num,
> goto err1;
> }
>
> - vtrig = ioremap(irq->ptrig, PAGE_SIZE);
> - if (unlikely(!vtrig)) {
> - dev_err(dev, "%s: Trigger page mapping failed\n", __func__);
> - rc = -ENOMEM;
> + xd = irq_get_handler_data(virq);
> + if (unlikely(!xd)) {
> + dev_err(dev, "%s: Can't get interrupt data\n", __func__);
> + rc = -ENXIO;
> goto err2;
> }
>
> irq->virq = virq;
> - irq->vtrig = vtrig;
> + irq->vtrig = xd->trig_mmio;
> out:
> return rc;
> err2:
> @@ -259,8 +260,6 @@ static void afu_unmap_irq(u64 flags, struct ocxlflash_context *ctx, int num,
> }
>
> irq = &ctx->irqs[num];
> - if (irq->vtrig)
> - iounmap(irq->vtrig);
>
> if (irq_find_mapping(NULL, irq->hwirq)) {
> free_irq(irq->virq, cookie);
> @@ -648,7 +647,6 @@ static int alloc_afu_irqs(struct ocxlflash_context *ctx, int num)
> }
>
> irqs[i].hwirq = hwirq;
> - irqs[i].ptrig = addr;
> }
>
> ctx->irqs = irqs;
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.h b/drivers/scsi/cxlflash/ocxl_hw.h
> index fc6ad4f985de..f2fe88816bea 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.h
> +++ b/drivers/scsi/cxlflash/ocxl_hw.h
> @@ -13,7 +13,6 @@
> struct ocxlflash_irqs {
> int hwirq;
> u32 virq;
> - u64 ptrig;
> void __iomem *vtrig;
> };
>
>
^ permalink raw reply
* Re: [PATCH v2 1/4] scsi: cxlflash: Access interrupt trigger page from xive directly
From: Matthew R. Ochs @ 2020-04-03 15:51 UTC (permalink / raw)
To: Frederic Barrat
Cc: ukrishn, ajd, haren, groug, clg, linuxppc-dev, christophe_lombard
In-Reply-To: <20200403153838.29224-2-fbarrat@linux.ibm.com>
On Fri, Apr 03, 2020 at 05:38:35PM +0200, Frederic Barrat wrote:
> xive is already mapping the trigger page in kernel space and it can be
> accessed through standard APIs, so let's reuse it and simplify the code.
>
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
Looks good!
Acked-by: Matthew R. Ochs <mrochs@linux.ibm.com>
> ---
> Changelog:
> v2: Define rc = -ENXIO on the error path (Matt)
>
>
> drivers/scsi/cxlflash/ocxl_hw.c | 18 ++++++++----------
> drivers/scsi/cxlflash/ocxl_hw.h | 1 -
> 2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
> index 7018cd802569..d6eec434a607 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.c
> +++ b/drivers/scsi/cxlflash/ocxl_hw.c
> @@ -15,7 +15,8 @@
> #include <linux/pseudo_fs.h>
> #include <linux/poll.h>
> #include <linux/sched/signal.h>
> -
> +#include <linux/interrupt.h>
> +#include <asm/xive.h>
> #include <misc/ocxl.h>
>
> #include <uapi/misc/cxl.h>
> @@ -180,7 +181,7 @@ static int afu_map_irq(u64 flags, struct ocxlflash_context *ctx, int num,
> struct ocxl_hw_afu *afu = ctx->hw_afu;
> struct device *dev = afu->dev;
> struct ocxlflash_irqs *irq;
> - void __iomem *vtrig;
> + struct xive_irq_data *xd;
> u32 virq;
> int rc = 0;
>
> @@ -204,15 +205,15 @@ static int afu_map_irq(u64 flags, struct ocxlflash_context *ctx, int num,
> goto err1;
> }
>
> - vtrig = ioremap(irq->ptrig, PAGE_SIZE);
> - if (unlikely(!vtrig)) {
> - dev_err(dev, "%s: Trigger page mapping failed\n", __func__);
> - rc = -ENOMEM;
> + xd = irq_get_handler_data(virq);
> + if (unlikely(!xd)) {
> + dev_err(dev, "%s: Can't get interrupt data\n", __func__);
> + rc = -ENXIO;
> goto err2;
> }
>
> irq->virq = virq;
> - irq->vtrig = vtrig;
> + irq->vtrig = xd->trig_mmio;
> out:
> return rc;
> err2:
> @@ -259,8 +260,6 @@ static void afu_unmap_irq(u64 flags, struct ocxlflash_context *ctx, int num,
> }
>
> irq = &ctx->irqs[num];
> - if (irq->vtrig)
> - iounmap(irq->vtrig);
>
> if (irq_find_mapping(NULL, irq->hwirq)) {
> free_irq(irq->virq, cookie);
> @@ -648,7 +647,6 @@ static int alloc_afu_irqs(struct ocxlflash_context *ctx, int num)
> }
>
> irqs[i].hwirq = hwirq;
> - irqs[i].ptrig = addr;
> }
>
> ctx->irqs = irqs;
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.h b/drivers/scsi/cxlflash/ocxl_hw.h
> index fc6ad4f985de..f2fe88816bea 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.h
> +++ b/drivers/scsi/cxlflash/ocxl_hw.h
> @@ -13,7 +13,6 @@
> struct ocxlflash_irqs {
> int hwirq;
> u32 virq;
> - u64 ptrig;
> void __iomem *vtrig;
> };
>
> --
> 2.25.1
>
^ permalink raw reply
* [PATCH v2 0/4] ocxl: Cleanup AFU interrupt allocation
From: Frederic Barrat @ 2020-04-03 15:38 UTC (permalink / raw)
To: linuxppc-dev, clg, christophe_lombard, ajd, ukrishn, mrochs; +Cc: haren, groug
Short series to cleanup AFU interrupt allocation for opencapi.
Current code was using its own allocation service, calling opal
directly to get the trigger page. This is not needed and we can use
xive to achieve the same thing. The only caveat is that the trigger
page address is only valid after the interrupt has been mapped, but
that is not a problem with the way the code is using it.
No functional change.
Changelog:
v2:
Fix error path in patch 1
All other patches unchanged
Frederic Barrat (4):
scsi: cxlflash: Access interrupt trigger page from xive directly
ocxl: Access interrupt trigger page from xive directly
ocxl: Don't return trigger page when allocating an interrupt
ocxl: Remove custom service to allocate interrupts
arch/powerpc/include/asm/pnv-ocxl.h | 3 ---
arch/powerpc/platforms/powernv/ocxl.c | 30 ---------------------------
drivers/misc/ocxl/Kconfig | 2 +-
drivers/misc/ocxl/afu_irq.c | 12 ++++++-----
drivers/misc/ocxl/link.c | 15 +++++++-------
drivers/scsi/cxlflash/ocxl_hw.c | 21 ++++++++-----------
drivers/scsi/cxlflash/ocxl_hw.h | 1 -
include/misc/ocxl.h | 10 ++-------
8 files changed, 26 insertions(+), 68 deletions(-)
--
2.25.1
^ permalink raw reply
* [PATCH v2 1/4] scsi: cxlflash: Access interrupt trigger page from xive directly
From: Frederic Barrat @ 2020-04-03 15:38 UTC (permalink / raw)
To: linuxppc-dev, clg, christophe_lombard, ajd, ukrishn, mrochs; +Cc: haren, groug
In-Reply-To: <20200403153838.29224-1-fbarrat@linux.ibm.com>
xive is already mapping the trigger page in kernel space and it can be
accessed through standard APIs, so let's reuse it and simplify the code.
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
Changelog:
v2: Define rc = -ENXIO on the error path (Matt)
drivers/scsi/cxlflash/ocxl_hw.c | 18 ++++++++----------
drivers/scsi/cxlflash/ocxl_hw.h | 1 -
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
index 7018cd802569..d6eec434a607 100644
--- a/drivers/scsi/cxlflash/ocxl_hw.c
+++ b/drivers/scsi/cxlflash/ocxl_hw.c
@@ -15,7 +15,8 @@
#include <linux/pseudo_fs.h>
#include <linux/poll.h>
#include <linux/sched/signal.h>
-
+#include <linux/interrupt.h>
+#include <asm/xive.h>
#include <misc/ocxl.h>
#include <uapi/misc/cxl.h>
@@ -180,7 +181,7 @@ static int afu_map_irq(u64 flags, struct ocxlflash_context *ctx, int num,
struct ocxl_hw_afu *afu = ctx->hw_afu;
struct device *dev = afu->dev;
struct ocxlflash_irqs *irq;
- void __iomem *vtrig;
+ struct xive_irq_data *xd;
u32 virq;
int rc = 0;
@@ -204,15 +205,15 @@ static int afu_map_irq(u64 flags, struct ocxlflash_context *ctx, int num,
goto err1;
}
- vtrig = ioremap(irq->ptrig, PAGE_SIZE);
- if (unlikely(!vtrig)) {
- dev_err(dev, "%s: Trigger page mapping failed\n", __func__);
- rc = -ENOMEM;
+ xd = irq_get_handler_data(virq);
+ if (unlikely(!xd)) {
+ dev_err(dev, "%s: Can't get interrupt data\n", __func__);
+ rc = -ENXIO;
goto err2;
}
irq->virq = virq;
- irq->vtrig = vtrig;
+ irq->vtrig = xd->trig_mmio;
out:
return rc;
err2:
@@ -259,8 +260,6 @@ static void afu_unmap_irq(u64 flags, struct ocxlflash_context *ctx, int num,
}
irq = &ctx->irqs[num];
- if (irq->vtrig)
- iounmap(irq->vtrig);
if (irq_find_mapping(NULL, irq->hwirq)) {
free_irq(irq->virq, cookie);
@@ -648,7 +647,6 @@ static int alloc_afu_irqs(struct ocxlflash_context *ctx, int num)
}
irqs[i].hwirq = hwirq;
- irqs[i].ptrig = addr;
}
ctx->irqs = irqs;
diff --git a/drivers/scsi/cxlflash/ocxl_hw.h b/drivers/scsi/cxlflash/ocxl_hw.h
index fc6ad4f985de..f2fe88816bea 100644
--- a/drivers/scsi/cxlflash/ocxl_hw.h
+++ b/drivers/scsi/cxlflash/ocxl_hw.h
@@ -13,7 +13,6 @@
struct ocxlflash_irqs {
int hwirq;
u32 virq;
- u64 ptrig;
void __iomem *vtrig;
};
--
2.25.1
^ permalink raw reply related
* [PATCH v2 2/4] ocxl: Access interrupt trigger page from xive directly
From: Frederic Barrat @ 2020-04-03 15:38 UTC (permalink / raw)
To: linuxppc-dev, clg, christophe_lombard, ajd, ukrishn, mrochs; +Cc: haren, groug
In-Reply-To: <20200403153838.29224-1-fbarrat@linux.ibm.com>
We can access the trigger page through standard APIs so let's use it
and avoid saving it when allocating the interrupt. It will also allow
to simplify allocation in a later patch.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Acked-by: Andrew Donnellan <ajd@linux.ibm.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
drivers/misc/ocxl/afu_irq.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/ocxl/afu_irq.c b/drivers/misc/ocxl/afu_irq.c
index 70f8f1c3929d..b30ec0ef7be7 100644
--- a/drivers/misc/ocxl/afu_irq.c
+++ b/drivers/misc/ocxl/afu_irq.c
@@ -2,6 +2,7 @@
// Copyright 2017 IBM Corp.
#include <linux/interrupt.h>
#include <asm/pnv-ocxl.h>
+#include <asm/xive.h>
#include "ocxl_internal.h"
#include "trace.h"
@@ -196,13 +197,16 @@ void ocxl_afu_irq_free_all(struct ocxl_context *ctx)
u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, int irq_id)
{
+ struct xive_irq_data *xd;
struct afu_irq *irq;
u64 addr = 0;
mutex_lock(&ctx->irq_lock);
irq = idr_find(&ctx->irq_idr, irq_id);
- if (irq)
- addr = irq->trigger_page;
+ if (irq) {
+ xd = irq_get_handler_data(irq->virq);
+ addr = xd ? xd->trig_page : 0;
+ }
mutex_unlock(&ctx->irq_lock);
return addr;
}
--
2.25.1
^ permalink raw reply related
* [PATCH v2 3/4] ocxl: Don't return trigger page when allocating an interrupt
From: Frederic Barrat @ 2020-04-03 15:38 UTC (permalink / raw)
To: linuxppc-dev, clg, christophe_lombard, ajd, ukrishn, mrochs; +Cc: haren, groug
In-Reply-To: <20200403153838.29224-1-fbarrat@linux.ibm.com>
Existing users of ocxl_link_irq_alloc() have been converted to obtain
the trigger page of an interrupt through xive directly, we therefore
have no need to return the trigger page when allocating an interrupt.
It also allows ocxl to use the xive native interface to allocate
interrupts, instead of its custom service.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
drivers/misc/ocxl/Kconfig | 2 +-
drivers/misc/ocxl/afu_irq.c | 4 +---
drivers/misc/ocxl/link.c | 15 +++++++--------
drivers/scsi/cxlflash/ocxl_hw.c | 3 +--
include/misc/ocxl.h | 10 ++--------
5 files changed, 12 insertions(+), 22 deletions(-)
diff --git a/drivers/misc/ocxl/Kconfig b/drivers/misc/ocxl/Kconfig
index 2d2266c1439e..e65773f5cf59 100644
--- a/drivers/misc/ocxl/Kconfig
+++ b/drivers/misc/ocxl/Kconfig
@@ -9,7 +9,7 @@ config OCXL_BASE
config OCXL
tristate "OpenCAPI coherent accelerator support"
- depends on PPC_POWERNV && PCI && EEH
+ depends on PPC_POWERNV && PCI && EEH && PPC_XIVE_NATIVE
select OCXL_BASE
select HOTPLUG_PCI_POWERNV
default m
diff --git a/drivers/misc/ocxl/afu_irq.c b/drivers/misc/ocxl/afu_irq.c
index b30ec0ef7be7..ecdcfae025b7 100644
--- a/drivers/misc/ocxl/afu_irq.c
+++ b/drivers/misc/ocxl/afu_irq.c
@@ -11,7 +11,6 @@ struct afu_irq {
int hw_irq;
unsigned int virq;
char *name;
- u64 trigger_page;
irqreturn_t (*handler)(void *private);
void (*free_private)(void *private);
void *private;
@@ -125,8 +124,7 @@ int ocxl_afu_irq_alloc(struct ocxl_context *ctx, int *irq_id)
goto err_unlock;
}
- rc = ocxl_link_irq_alloc(ctx->afu->fn->link, &irq->hw_irq,
- &irq->trigger_page);
+ rc = ocxl_link_irq_alloc(ctx->afu->fn->link, &irq->hw_irq);
if (rc)
goto err_idr;
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index 58d111afd9f6..fd73d3bc0eb6 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -6,6 +6,7 @@
#include <linux/mmu_context.h>
#include <asm/copro.h>
#include <asm/pnv-ocxl.h>
+#include <asm/xive.h>
#include <misc/ocxl.h>
#include "ocxl_internal.h"
#include "trace.h"
@@ -682,23 +683,21 @@ int ocxl_link_remove_pe(void *link_handle, int pasid)
}
EXPORT_SYMBOL_GPL(ocxl_link_remove_pe);
-int ocxl_link_irq_alloc(void *link_handle, int *hw_irq, u64 *trigger_addr)
+int ocxl_link_irq_alloc(void *link_handle, int *hw_irq)
{
struct ocxl_link *link = (struct ocxl_link *) link_handle;
- int rc, irq;
- u64 addr;
+ int irq;
if (atomic_dec_if_positive(&link->irq_available) < 0)
return -ENOSPC;
- rc = pnv_ocxl_alloc_xive_irq(&irq, &addr);
- if (rc) {
+ irq = xive_native_alloc_irq();
+ if (!irq) {
atomic_inc(&link->irq_available);
- return rc;
+ return -ENXIO;
}
*hw_irq = irq;
- *trigger_addr = addr;
return 0;
}
EXPORT_SYMBOL_GPL(ocxl_link_irq_alloc);
@@ -707,7 +706,7 @@ void ocxl_link_free_irq(void *link_handle, int hw_irq)
{
struct ocxl_link *link = (struct ocxl_link *) link_handle;
- pnv_ocxl_free_xive_irq(hw_irq);
+ xive_native_free_irq(hw_irq);
atomic_inc(&link->irq_available);
}
EXPORT_SYMBOL_GPL(ocxl_link_free_irq);
diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
index d6eec434a607..e4e0d767b98e 100644
--- a/drivers/scsi/cxlflash/ocxl_hw.c
+++ b/drivers/scsi/cxlflash/ocxl_hw.c
@@ -614,7 +614,6 @@ static int alloc_afu_irqs(struct ocxlflash_context *ctx, int num)
struct ocxl_hw_afu *afu = ctx->hw_afu;
struct device *dev = afu->dev;
struct ocxlflash_irqs *irqs;
- u64 addr;
int rc = 0;
int hwirq;
int i;
@@ -639,7 +638,7 @@ static int alloc_afu_irqs(struct ocxlflash_context *ctx, int num)
}
for (i = 0; i < num; i++) {
- rc = ocxl_link_irq_alloc(afu->link_token, &hwirq, &addr);
+ rc = ocxl_link_irq_alloc(afu->link_token, &hwirq);
if (unlikely(rc)) {
dev_err(dev, "%s: ocxl_link_irq_alloc failed rc=%d\n",
__func__, rc);
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index 06dd5839e438..a2868adec22f 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -480,14 +480,8 @@ int ocxl_link_remove_pe(void *link_handle, int pasid);
* Allocate an AFU interrupt associated to the link.
*
* 'hw_irq' is the hardware interrupt number
- * 'obj_handle' is the 64-bit object handle to be passed to the AFU to
- * trigger the interrupt.
- * On P9, 'obj_handle' is an address, which, if written, triggers the
- * interrupt. It is an MMIO address which needs to be remapped (one
- * page).
- */
-int ocxl_link_irq_alloc(void *link_handle, int *hw_irq,
- u64 *obj_handle);
+ */
+int ocxl_link_irq_alloc(void *link_handle, int *hw_irq);
/*
* Free a previously allocated AFU interrupt
--
2.25.1
^ permalink raw reply related
* [PATCH v2 4/4] ocxl: Remove custom service to allocate interrupts
From: Frederic Barrat @ 2020-04-03 15:38 UTC (permalink / raw)
To: linuxppc-dev, clg, christophe_lombard, ajd, ukrishn, mrochs; +Cc: haren, groug
In-Reply-To: <20200403153838.29224-1-fbarrat@linux.ibm.com>
We now allocate interrupts through xive directly.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
arch/powerpc/include/asm/pnv-ocxl.h | 3 ---
arch/powerpc/platforms/powernv/ocxl.c | 30 ---------------------------
2 files changed, 33 deletions(-)
diff --git a/arch/powerpc/include/asm/pnv-ocxl.h b/arch/powerpc/include/asm/pnv-ocxl.h
index 7de82647e761..e90650328c9c 100644
--- a/arch/powerpc/include/asm/pnv-ocxl.h
+++ b/arch/powerpc/include/asm/pnv-ocxl.h
@@ -30,7 +30,4 @@ extern int pnv_ocxl_spa_setup(struct pci_dev *dev, void *spa_mem, int PE_mask,
extern void pnv_ocxl_spa_release(void *platform_data);
extern int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle);
-extern int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr);
-extern void pnv_ocxl_free_xive_irq(u32 irq);
-
#endif /* _ASM_PNV_OCXL_H */
diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
index 8c65aacda9c8..ecdad219d704 100644
--- a/arch/powerpc/platforms/powernv/ocxl.c
+++ b/arch/powerpc/platforms/powernv/ocxl.c
@@ -2,7 +2,6 @@
// Copyright 2017 IBM Corp.
#include <asm/pnv-ocxl.h>
#include <asm/opal.h>
-#include <asm/xive.h>
#include <misc/ocxl-config.h>
#include "pci.h"
@@ -484,32 +483,3 @@ int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle)
return rc;
}
EXPORT_SYMBOL_GPL(pnv_ocxl_spa_remove_pe_from_cache);
-
-int pnv_ocxl_alloc_xive_irq(u32 *irq, u64 *trigger_addr)
-{
- __be64 flags, trigger_page;
- s64 rc;
- u32 hwirq;
-
- hwirq = xive_native_alloc_irq();
- if (!hwirq)
- return -ENOENT;
-
- rc = opal_xive_get_irq_info(hwirq, &flags, NULL, &trigger_page, NULL,
- NULL);
- if (rc || !trigger_page) {
- xive_native_free_irq(hwirq);
- return -ENOENT;
- }
- *irq = hwirq;
- *trigger_addr = be64_to_cpu(trigger_page);
- return 0;
-
-}
-EXPORT_SYMBOL_GPL(pnv_ocxl_alloc_xive_irq);
-
-void pnv_ocxl_free_xive_irq(u32 irq)
-{
- xive_native_free_irq(irq);
-}
-EXPORT_SYMBOL_GPL(pnv_ocxl_free_xive_irq);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 1/2] powerpc: Add base support for ISA v3.1
From: Segher Boessenkool @ 2020-04-03 15:32 UTC (permalink / raw)
To: Alistair Popple; +Cc: mikey, linuxppc-dev, npiggin
In-Reply-To: <20200403041055.27535-1-alistair@popple.id.au>
Hi!
On Fri, Apr 03, 2020 at 03:10:54PM +1100, Alistair Popple wrote:
> +#define PCR_ARCH_300 0x10 /* Architecture 3.00 */
It's called 3.0, not 3.00?
Segher
^ permalink raw reply
* Re: [PATCH v2 13/14] powerpc/64s: system reset do not trace
From: Christophe Leroy @ 2020-04-03 14:45 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: Mahesh Salgaonkar, Naveen N . Rao, Ganesh Goudar
In-Reply-To: <20200403132622.130394-14-npiggin@gmail.com>
Le 03/04/2020 à 15:26, Nicholas Piggin a écrit :
> Similarly to the previous patch, do not trace system reset. This code
> is used when there is a crash or hang, and tracing disturbs the system
> more and has been known to crash in the crash handling path.
>
> Acked-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/kernel/traps.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 1845fd7e161a..ed7b7a6e2dc0 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -443,6 +443,9 @@ void system_reset_exception(struct pt_regs *regs)
> unsigned long hsrr0, hsrr1;
> bool nested = in_nmi();
> bool saved_hsrrs = false;
> + u8 ftrace_enabled = local_paca->ftrace_enabled;
> +
> + local_paca->ftrace_enabled = 0;
I predict a build failure here in the near future ...
See
https://elixir.bootlin.com/linux/v5.6/source/arch/powerpc/kernel/head_8xx.S#L125
>
> /*
> * Avoid crashes in case of nested NMI exceptions. Recoverability
> @@ -524,6 +527,8 @@ void system_reset_exception(struct pt_regs *regs)
> if (!nested)
> nmi_exit();
>
> + local_paca->ftrace_enabled = ftrace_enabled;
> +
> /* What should we do here? We could issue a shutdown or hard reset. */
> }
>
>
Christophe
^ permalink raw reply
* Re: [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests
From: Bharata B Rao @ 2020-04-03 14:38 UTC (permalink / raw)
To: Leonardo Bras
Cc: Greg Kroah-Hartman, Claudio Carvalho, linux-kernel,
Paul Mackerras, Hari Bathini, Nathan Fontenot, Thomas Gleixner,
linuxppc-dev, Allison Randal
In-Reply-To: <20200402195156.626430-1-leonardo@linux.ibm.com>
On Thu, Apr 02, 2020 at 04:51:57PM -0300, Leonardo Bras wrote:
> While providing guests, it's desirable to resize it's memory on demand.
>
> By now, it's possible to do so by creating a guest with a small base
> memory, hot-plugging all the rest, and using 'movable_node' kernel
> command-line parameter, which puts all hot-plugged memory in
> ZONE_MOVABLE, allowing it to be removed whenever needed.
>
> But there is an issue regarding guest reboot:
> If memory is hot-plugged, and then the guest is rebooted, all hot-plugged
> memory goes to ZONE_NORMAL, which offers no guaranteed hot-removal.
> It usually prevents this memory to be hot-removed from the guest.
>
> It's possible to use device-tree information to fix that behavior, as
> it stores flags for LMB ranges on ibm,dynamic-memory-vN.
> It involves marking each memblock with the correct flags as hotpluggable
> memory, which mm/memblock.c puts in ZONE_MOVABLE during boot if
> 'movable_node' is passed.
>
> For carrying such information, the new flag DRCONF_MEM_HOTREMOVABLE was
> proposed and accepted into Power Architecture documentation.
> This flag should be:
> - true (b=1) if the hypervisor may want to hot-remove it later, and
> - false (b=0) if it does not care.
>
> During boot, guest kernel reads the device-tree, early_init_drmem_lmb()
> is called for every added LMBs. Here, checking for this new flag and
> marking memblocks as hotplugable memory is enough to get the desirable
> behavior.
>
> This should cause no change if 'movable_node' parameter is not passed
> in kernel command-line.
>
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> Reviewed-by: Bharata B Rao <bharata@linux.ibm.com>
>
> ---
>
> Changes since v2:
> - New flag name changed from DRCONF_MEM_HOTPLUGGED to
> DRCONF_MEM_HOTREMOVABLE
The patch would be more complete with the following change that ensures
that DRCONF_MEM_HOTREMOVABLE flag is set for non-boot-time hotplugged
memory too. This will ensure that ibm,dynamic-memory-vN property
reflects the right flags value for memory that gets hotplugged
post boot.
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index a4d40a3ceea3..6d75f6e182ae 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -395,7 +395,8 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
invalidate_lmb_associativity_index(lmb);
lmb_clear_nid(lmb);
- lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+ lmb->flags &= ~(DRCONF_MEM_ASSIGNED |
+ DRCONF_MEM_HOTREMOVABLE);
return 0;
}
@@ -678,7 +679,8 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
invalidate_lmb_associativity_index(lmb);
lmb_clear_nid(lmb);
} else {
- lmb->flags |= DRCONF_MEM_ASSIGNED;
+ lmb->flags |= (DRCONF_MEM_ASSIGNED |
+ DRCONF_MEM_HOTREMOVABLE);
}
return rc;
Regards,
Bharata.
^ permalink raw reply related
* Re: [PATCH v2 08/14] powerpc/pseries/ras: fwnmi sreset should not interlock
From: Christophe Leroy @ 2020-04-03 14:35 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Mahesh Salgaonkar, Ganesh Goudar
In-Reply-To: <20200403132622.130394-9-npiggin@gmail.com>
Le 03/04/2020 à 15:26, Nicholas Piggin a écrit :
> PAPR does not specify that fwnmi sreset should be interlocked, and
> PowerVM (and therefore now QEMU) do not require it.
>
> These "ibm,nmi-interlock" calls are ignored by firmware, but there
> is a possibility that the sreset could have interrupted a machine
> check and release the machine check's interlock too early, corrupting
> it if another machine check came in.
>
> This is an extremely rare case, but it should be fixed for clarity
> and reducing the code executed in the sreset path. Firmware also
> does not provide error information for the sreset case to look at, so
> remove that comment.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/platforms/pseries/ras.c | 48 ++++++++++++++++++++--------
> 1 file changed, 34 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
> index a40598e6e525..833ae34b7fec 100644
> --- a/arch/powerpc/platforms/pseries/ras.c
> +++ b/arch/powerpc/platforms/pseries/ras.c
> @@ -406,6 +406,20 @@ static inline struct rtas_error_log *fwnmi_get_errlog(void)
> return (struct rtas_error_log *)local_paca->mce_data_buf;
> }
>
> +static unsigned long *fwnmi_get_savep(struct pt_regs *regs)
> +{
> + unsigned long savep_ra;
> +
> + /* Mask top two bits */
> + savep_ra = regs->gpr[3] & ~(0x3UL << 62);
> + if (!VALID_FWNMI_BUFFER(savep_ra)) {
> + printk(KERN_ERR "FWNMI: corrupt r3 0x%016lx\n", regs->gpr[3]);
Can't you use pr_err() instead ?
> + return NULL;
> + }
> +
> + return __va(savep_ra);
> +}
> +
> /*
> * Get the error information for errors coming through the
> * FWNMI vectors. The pt_regs' r3 will be updated to reflect
> @@ -423,20 +437,15 @@ static inline struct rtas_error_log *fwnmi_get_errlog(void)
> */
> static struct rtas_error_log *fwnmi_get_errinfo(struct pt_regs *regs)
> {
> - unsigned long savep_ra;
> unsigned long *savep;
> struct rtas_error_log *h;
>
> - /* Mask top two bits */
> - savep_ra = regs->gpr[3] & ~(0x3UL << 62);
> -
> - if (!VALID_FWNMI_BUFFER(savep_ra)) {
> - printk(KERN_ERR "FWNMI: corrupt r3 0x%016lx\n", regs->gpr[3]);
> + savep = fwnmi_get_savep(regs);
> + if (!savep)
> return NULL;
> - }
>
> - savep = __va(savep_ra);
> - regs->gpr[3] = be64_to_cpu(savep[0]); /* restore original r3 */
> + /* restore original r3 */
> + regs->gpr[3] = be64_to_cpu(savep[0]);
Is it needed to change the location of the comment ?
>
> h = (struct rtas_error_log *)&savep[1];
> /* Use the per cpu buffer from paca to store rtas error log */
> @@ -483,11 +492,22 @@ int pSeries_system_reset_exception(struct pt_regs *regs)
> #endif
>
> if (fwnmi_active) {
> - struct rtas_error_log *errhdr = fwnmi_get_errinfo(regs);
> - if (errhdr) {
> - /* XXX Should look at FWNMI information */
> - }
> - fwnmi_release_errinfo();
> + unsigned long *savep;
> +
> + /*
> + * Firmware (PowerVM and KVM) saves r3 to a save area like
> + * machine check, which is not exactly what PAPR (2.9)
> + * suggests but there is no way to detect otherwise, so this
> + * is the interface now.
> + *
> + * System resets do not save any error log or require an
> + * "ibm,nmi-interlock" rtas call to release.
> + */
> +
> + savep = fwnmi_get_savep(regs);
> + /* restore original r3 */
> + if (savep)
> + regs->gpr[3] = be64_to_cpu(savep[0]);
> }
>
> if (smp_handle_nmi_ipi(regs))
>
Christophe
^ permalink raw reply
* Re: [PATCH v2 06/14] powerpc/pseries/ras: FWNMI_VALID off by one
From: Christophe Leroy @ 2020-04-03 14:33 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: Mahesh Salgaonkar, Ganesh Goudar, Mahesh Salgaonkar
In-Reply-To: <20200403132622.130394-7-npiggin@gmail.com>
Le 03/04/2020 à 15:26, Nicholas Piggin a écrit :
> This was discovered developing qemu fwnmi sreset support. This
> off-by-one bug means the last 16 bytes of the rtas area can not
> be used for a 16 byte save area.
>
> It's not a serious bug, and QEMU implementation has to retain a
> workaround for old kernels, but it's good to tighten it.
>
> Acked-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/platforms/pseries/ras.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
> index c74d5e740922..9a37bda47468 100644
> --- a/arch/powerpc/platforms/pseries/ras.c
> +++ b/arch/powerpc/platforms/pseries/ras.c
> @@ -395,10 +395,11 @@ static irqreturn_t ras_error_interrupt(int irq, void *dev_id)
> /*
> * Some versions of FWNMI place the buffer inside the 4kB page starting at
> * 0x7000. Other versions place it inside the rtas buffer. We check both.
> + * Minimum size of the buffer is 16 bytes.
> */
> #define VALID_FWNMI_BUFFER(A) \
> - ((((A) >= 0x7000) && ((A) < 0x7ff0)) || \
> - (((A) >= rtas.base) && ((A) < (rtas.base + rtas.size - 16))))
> + ((((A) >= 0x7000) && ((A) <= 0x8000 - 16)) || \
> + (((A) >= rtas.base) && ((A) <= (rtas.base + rtas.size - 16))))
Why not just change < to <= and leave the 0x7ff0 ?
>
> static inline struct rtas_error_log *fwnmi_get_errlog(void)
> {
>
Christophe
^ permalink raw reply
* Re: [PATCH v2 05/14] powerpc/pseries/ras: avoid calling rtas_token in NMI paths
From: Christophe Leroy @ 2020-04-03 14:30 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: Mahesh Salgaonkar, Ganesh Goudar, Mahesh Salgaonkar
In-Reply-To: <20200403132622.130394-6-npiggin@gmail.com>
Le 03/04/2020 à 15:26, Nicholas Piggin a écrit :
> In the interest of reducing code and possible failures in the
> machine check and system reset paths, grab the "ibm,nmi-interlock"
> token at init time.
>
> Reviewed-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/firmware.h | 1 +
> arch/powerpc/platforms/pseries/ras.c | 2 +-
> arch/powerpc/platforms/pseries/setup.c | 13 ++++++++++---
> 3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
> index ca33f4ef6cb4..6003c2e533a0 100644
> --- a/arch/powerpc/include/asm/firmware.h
> +++ b/arch/powerpc/include/asm/firmware.h
> @@ -128,6 +128,7 @@ extern void machine_check_fwnmi(void);
>
> /* This is true if we are using the firmware NMI handler (typically LPAR) */
> extern int fwnmi_active;
> +extern int ibm_nmi_interlock_token;
>
> extern unsigned int __start___fw_ftr_fixup, __stop___fw_ftr_fixup;
>
> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
> index 1d7f973c647b..c74d5e740922 100644
> --- a/arch/powerpc/platforms/pseries/ras.c
> +++ b/arch/powerpc/platforms/pseries/ras.c
> @@ -458,7 +458,7 @@ static struct rtas_error_log *fwnmi_get_errinfo(struct pt_regs *regs)
> */
> static void fwnmi_release_errinfo(void)
> {
> - int ret = rtas_call(rtas_token("ibm,nmi-interlock"), 0, 1, NULL);
> + int ret = rtas_call(ibm_nmi_interlock_token, 0, 1, NULL);
> if (ret != 0)
> printk(KERN_ERR "FWNMI: nmi-interlock failed: %d\n", ret);
> }
> diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
> index 0c8421dd01ab..b582198be284 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -83,6 +83,7 @@ unsigned long CMO_PageSize = (ASM_CONST(1) << IOMMU_PAGE_SHIFT_4K);
> EXPORT_SYMBOL(CMO_PageSize);
>
> int fwnmi_active; /* TRUE if an FWNMI handler is present */
> +int ibm_nmi_interlock_token;
>
> static void pSeries_show_cpuinfo(struct seq_file *m)
> {
> @@ -113,9 +114,14 @@ static void __init fwnmi_init(void)
> struct slb_entry *slb_ptr;
> size_t size;
> #endif
> + int ibm_nmi_register_token;
>
> - int ibm_nmi_register = rtas_token("ibm,nmi-register");
> - if (ibm_nmi_register == RTAS_UNKNOWN_SERVICE)
> + ibm_nmi_register_token = rtas_token("ibm,nmi-register");
> + if (ibm_nmi_register_token == RTAS_UNKNOWN_SERVICE)
> + return;
> +
> + ibm_nmi_interlock_token = rtas_token("ibm,nmi-interlock");
> + if (WARN_ON(ibm_nmi_interlock_token == RTAS_UNKNOWN_SERVICE))
> return;
>
> /* If the kernel's not linked at zero we point the firmware at low
> @@ -123,7 +129,8 @@ static void __init fwnmi_init(void)
> system_reset_addr = __pa(system_reset_fwnmi) - PHYSICAL_START;
> machine_check_addr = __pa(machine_check_fwnmi) - PHYSICAL_START;
>
> - if (0 == rtas_call(ibm_nmi_register, 2, 1, NULL, system_reset_addr,
> + if (0 == rtas_call(ibm_nmi_register_token, 2, 1, NULL,
> + system_reset_addr,
> machine_check_addr))
Alignment is wrong. And you could put system_reset_addr and
machine_check_addr on the same line to limit the number of lines of the if.
> fwnmi_active = 1;
>
>
Christophe
^ 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