* Re: [PATCH v3 07/12] asm-generic: Define 'func_desc_t' to commonly describe function descriptors
From: Nicholas Piggin @ 2021-10-18 6:29 UTC (permalink / raw)
To: Andrew Morton, Arnd Bergmann, Benjamin Herrenschmidt,
Christophe Leroy, Helge Deller, Greg Kroah-Hartman,
James E.J. Bottomley, Kees Cook, Michael Ellerman, Paul Mackerras
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <a33107c5b82580862510cc20af0d61e33a2b841d.1634457599.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of October 17, 2021 10:38 pm:
> We have three architectures using function descriptors, each with its
> own type and name.
>
> Add a common typedef that can be used in generic code.
>
> Also add a stub typedef for architecture without function descriptors,
> to avoid a forest of #ifdefs.
>
> It replaces the similar 'func_desc_t' previously defined in
> arch/powerpc/kernel/module_64.c
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
[...]
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index a918388d9bf6..33b51efe3a24 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -63,6 +63,9 @@ extern __visible const void __nosave_begin, __nosave_end;
> #else
> #define dereference_function_descriptor(p) ((void *)(p))
> #define dereference_kernel_function_descriptor(p) ((void *)(p))
> +typedef struct {
> + unsigned long addr;
> +} func_desc_t;
> #endif
>
I think that deserves a comment. If it's just to allow ifdef to be
avoided, I guess that's okay with a comment. Would be nice if you could
cause it to generate a link time error if it was ever used like
undefined functions, but I guess you can't. It's not a necessity though.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v3 07/12] asm-generic: Define 'func_desc_t' to commonly describe function descriptors
From: Christophe Leroy @ 2021-10-18 7:07 UTC (permalink / raw)
To: Nicholas Piggin, Andrew Morton, Arnd Bergmann,
Benjamin Herrenschmidt, Helge Deller, Greg Kroah-Hartman,
James E.J. Bottomley, Kees Cook, Michael Ellerman, Paul Mackerras
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <1634538449.eah9b31bbz.astroid@bobo.none>
Le 18/10/2021 à 08:29, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of October 17, 2021 10:38 pm:
>> We have three architectures using function descriptors, each with its
>> own type and name.
>>
>> Add a common typedef that can be used in generic code.
>>
>> Also add a stub typedef for architecture without function descriptors,
>> to avoid a forest of #ifdefs.
>>
>> It replaces the similar 'func_desc_t' previously defined in
>> arch/powerpc/kernel/module_64.c
>>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>
> [...]
>
>> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
>> index a918388d9bf6..33b51efe3a24 100644
>> --- a/include/asm-generic/sections.h
>> +++ b/include/asm-generic/sections.h
>> @@ -63,6 +63,9 @@ extern __visible const void __nosave_begin, __nosave_end;
>> #else
>> #define dereference_function_descriptor(p) ((void *)(p))
>> #define dereference_kernel_function_descriptor(p) ((void *)(p))
>> +typedef struct {
>> + unsigned long addr;
>> +} func_desc_t;
>> #endif
>>
>
> I think that deserves a comment. If it's just to allow ifdef to be
> avoided, I guess that's okay with a comment. Would be nice if you could
> cause it to generate a link time error if it was ever used like
> undefined functions, but I guess you can't. It's not a necessity though.
>
I tried to explain it in the commit message, but I can add a comment
here in addition for sure.
By the way, it IS used in powerpc's module_64.c:
static func_desc_t func_desc(unsigned long addr)
{
return (func_desc_t){addr};
}
static unsigned long func_addr(unsigned long addr)
{
return func_desc(addr).addr;
}
^ permalink raw reply
* Re: [PATCH] powerpc/kexec_file: Add of_node_put() before goto
From: Hari Bathini @ 2021-10-18 7:07 UTC (permalink / raw)
To: Wan Jiabing, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Rob Herring, Thiago Jung Bauermann,
Lakshmi Ramasubramanian, Sourabh Jain, linuxppc-dev, linux-kernel
Cc: kael_w
In-Reply-To: <20211018015418.10182-1-wanjiabing@vivo.com>
On 18/10/21 7:24 am, Wan Jiabing wrote:
> Fix following coccicheck warning:
> ./arch/powerpc/kexec/file_load_64.c:698:1-22: WARNING: Function
> for_each_node_by_type should have of_node_put() before goto
>
> Early exits from for_each_node_by_type should decrement the
> node reference counter.
>
> Signed-off-by: Wan Jiabing <wanjiabing@vivo.com>
Thanks for fixing this!
Acked-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
> arch/powerpc/kexec/file_load_64.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
> index 5056e175ca2c..b4981b651d9a 100644
> --- a/arch/powerpc/kexec/file_load_64.c
> +++ b/arch/powerpc/kexec/file_load_64.c
> @@ -700,6 +700,7 @@ static int update_usable_mem_fdt(void *fdt, struct crash_mem *usable_mem)
> if (ret) {
> pr_err("Failed to set linux,usable-memory property for %s node",
> dn->full_name);
> + of_node_put(dn);
> goto out;
> }
> }
>
^ permalink raw reply
* Re: [PATCH v3 04/12] powerpc: Prepare func_desc_t for refactorisation
From: Christophe Leroy @ 2021-10-18 7:08 UTC (permalink / raw)
To: Nicholas Piggin, Andrew Morton, Arnd Bergmann,
Benjamin Herrenschmidt, Helge Deller, Greg Kroah-Hartman,
James E.J. Bottomley, Kees Cook, Michael Ellerman, Paul Mackerras
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <1634536863.oq0s171f8c.astroid@bobo.none>
Le 18/10/2021 à 08:27, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of October 17, 2021 10:38 pm:
>> In preparation of making func_desc_t generic, change the ELFv2
>> version to a struct containing 'addr' element.
>>
>> This allows using single helpers common to ELFv1 and ELFv2.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>
>> ---
>> arch/powerpc/kernel/module_64.c | 32 ++++++++++++++------------------
>> 1 file changed, 14 insertions(+), 18 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
>> index a89da0ee25e2..b687ef88c4c4 100644
>> --- a/arch/powerpc/kernel/module_64.c
>> +++ b/arch/powerpc/kernel/module_64.c
>> @@ -33,19 +33,13 @@
>> #ifdef PPC64_ELF_ABI_v2
>>
>> /* An address is simply the address of the function. */
>> -typedef unsigned long func_desc_t;
>> +typedef struct {
>> + unsigned long addr;
>> +} func_desc_t;
>
> I'm not quite following why this change is done. I guess it is so you
> can move this func_desc_t type into core code, but why do that? Is it
> just to avoid using the preprocessor?
I explained it in patch 7 but yes it probably also deserves some more
explanation here as well.
That's right, it's to avoid having to spread #ifdefs everywhere.
>
> On its own this patch looks okay.
>
> Acked-by: Nicholas Piggin <npiggin@gmail.com>
>
^ permalink raw reply
* Re: [PATCH 02/13] nvme-multipath: add error handling support for add_disk()
From: Chaitanya Kulkarni @ 2021-10-18 5:34 UTC (permalink / raw)
To: Luis Chamberlain, axboe@kernel.dk, geoff@infradead.org,
mpe@ellerman.id.au, benh@kernel.crashing.org, paulus@samba.org,
jim@jtan.com, minchan@kernel.org, ngupta@vflare.org,
senozhatsky@chromium.org, richard@nod.at,
miquel.raynal@bootlin.com, vigneshr@ti.com,
dan.j.williams@intel.com, vishal.l.verma@intel.com,
dave.jiang@intel.com, ira.weiny@intel.com, kbusch@kernel.org,
hch@lst.de, sagi@grimberg.me
Cc: nvdimm@lists.linux.dev, linux-kernel@vger.kernel.org,
linux-nvme@lists.infradead.org, linux-block@vger.kernel.org,
linux-mtd@lists.infradead.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20211015235219.2191207-3-mcgrof@kernel.org>
On 10/15/21 16:52, Luis Chamberlain wrote:
> We never checked for errors on add_disk() as this function
> returned void. Now that this is fixed, use the shiny new
> error handling.
>
> Since we now can tell for sure when a disk was added, move
> setting the bit NVME_NSHEAD_DISK_LIVE only when we did
> add the disk successfully.
>
> Nothing to do here as the cleanup is done elsewhere. We take
> care and use test_and_set_bit() because it is protects against
> two nvme paths simultaneously calling device_add_disk() on the
> same namespace head.
>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Looks good.
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
^ permalink raw reply
* [REPORT PATCH v2] powerpc/papr_scm: Implement initial support for injecting smart errors
From: Shivaprasad G Bhat @ 2021-10-18 8:03 UTC (permalink / raw)
To: nvdimm
Cc: sbhat, aneesh.kumar, vaibhav, dan.j.williams, linuxppc-dev,
ira.weiny
From: Vaibhav Jain <vaibhav@linux.ibm.com>
Presently PAPR doesn't support injecting smart errors on an
NVDIMM. This makes testing the NVDIMM health reporting functionality
difficult as simulating NVDIMM health related events need a hacked up
qemu version.
To solve this problem this patch proposes simulating certain set of
NVDIMM health related events in papr_scm. Specifically 'fatal' health
state and 'dirty' shutdown state. These error can be injected via the
user-space 'ndctl-inject-smart(1)' command. With the proposed patch and
corresponding ndctl patches following command flow is expected:
$ sudo ndctl list -DH -d nmem0
...
"health_state":"ok",
"shutdown_state":"clean",
...
# inject unsafe shutdown and fatal health error
$ sudo ndctl inject-smart nmem0 -Uf
...
"health_state":"fatal",
"shutdown_state":"dirty",
...
# uninject all errors
$ sudo ndctl inject-smart nmem0 -N
...
"health_state":"ok",
"shutdown_state":"clean",
...
The patch adds two members 'health_bitmap_mask' and
'health_bitmap_override' inside struct papr_scm_priv which are then
bit blt'ed to the health bitmaps fetched from the hypervisor. In case
we are not able to fetch health information from the hypervisor we
service the health bitmap from these two members. These members are
accessible from sysfs at nmemX/papr/health_bitmap_override
A new PDSM named 'SMART_INJECT' is proposed that accepts newly
introduced 'struct nd_papr_pdsm_smart_inject' as payload thats
exchanged between libndctl and papr_scm to indicate the requested
smart-error states.
When the processing the PDSM 'SMART_INJECT', papr_pdsm_smart_inject()
constructs a pair or 'mask' and 'override' bitmaps from the payload
and bit-blt it to the 'health_bitmap_{mask, override}' members. This
ensures the after being fetched from the hypervisor, the health_bitmap
reflects requested smart-error states.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
---
Changelog:
Since v1:
Link: https://patchwork.kernel.org/project/linux-nvdimm/list/?series=513881
* Updated the patch description.
* Removed dependency of a header movement patch.
* Removed '__packed' attribute for 'struct nd_papr_pdsm_smart_inject' [Aneesh]
arch/powerpc/include/uapi/asm/papr_pdsm.h | 18 ++++++
arch/powerpc/platforms/pseries/papr_scm.c | 94 ++++++++++++++++++++++++++++-
2 files changed, 109 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/uapi/asm/papr_pdsm.h b/arch/powerpc/include/uapi/asm/papr_pdsm.h
index 82488b1e7276e..17439925045cf 100644
--- a/arch/powerpc/include/uapi/asm/papr_pdsm.h
+++ b/arch/powerpc/include/uapi/asm/papr_pdsm.h
@@ -116,6 +116,22 @@ struct nd_papr_pdsm_health {
};
};
+/* Flags for injecting specific smart errors */
+#define PDSM_SMART_INJECT_HEALTH_FATAL (1 << 0)
+#define PDSM_SMART_INJECT_BAD_SHUTDOWN (1 << 1)
+
+struct nd_papr_pdsm_smart_inject {
+ union {
+ struct {
+ /* One or more of PDSM_SMART_INJECT_ */
+ __u32 flags;
+ __u8 fatal_enable;
+ __u8 unsafe_shutdown_enable;
+ };
+ __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
+ };
+};
+
/*
* Methods to be embedded in ND_CMD_CALL request. These are sent to the kernel
* via 'nd_cmd_pkg.nd_command' member of the ioctl struct
@@ -123,12 +139,14 @@ struct nd_papr_pdsm_health {
enum papr_pdsm {
PAPR_PDSM_MIN = 0x0,
PAPR_PDSM_HEALTH,
+ PAPR_PDSM_SMART_INJECT,
PAPR_PDSM_MAX,
};
/* Maximal union that can hold all possible payload types */
union nd_pdsm_payload {
struct nd_papr_pdsm_health health;
+ struct nd_papr_pdsm_smart_inject smart_inject;
__u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
} __packed;
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index f48e87ac89c9b..de4cf329cfb3b 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -68,6 +68,10 @@
#define PAPR_SCM_PERF_STATS_EYECATCHER __stringify(SCMSTATS)
#define PAPR_SCM_PERF_STATS_VERSION 0x1
+/* Use bitblt method to override specific bits in the '_bitmap_' */
+#define BITBLT_BITMAP(_bitmap_, _mask_, _override_) \
+ (((_bitmap_) & ~(_mask_)) | ((_mask_) & (_override_)))
+
/* Struct holding a single performance metric */
struct papr_scm_perf_stat {
u8 stat_id[8];
@@ -120,6 +124,12 @@ struct papr_scm_priv {
/* length of the stat buffer as expected by phyp */
size_t stat_buffer_len;
+
+ /* The bits which needs to be overridden */
+ u64 health_bitmap_mask;
+
+ /* The overridden values for the bits having the masks set */
+ u64 health_bitmap_override;
};
static int papr_scm_pmem_flush(struct nd_region *nd_region,
@@ -347,19 +357,28 @@ static ssize_t drc_pmem_query_stats(struct papr_scm_priv *p,
static int __drc_pmem_query_health(struct papr_scm_priv *p)
{
unsigned long ret[PLPAR_HCALL_BUFSIZE];
+ u64 bitmap = 0;
long rc;
/* issue the hcall */
rc = plpar_hcall(H_SCM_HEALTH, ret, p->drc_index);
- if (rc != H_SUCCESS) {
+ if (rc == H_SUCCESS)
+ bitmap = ret[0] & ret[1];
+ else if (rc == H_FUNCTION)
+ dev_info_once(&p->pdev->dev,
+ "Hcall H_SCM_HEALTH not implemented, assuming empty health bitmap");
+ else {
+
dev_err(&p->pdev->dev,
"Failed to query health information, Err:%ld\n", rc);
return -ENXIO;
}
p->lasthealth_jiffies = jiffies;
- p->health_bitmap = ret[0] & ret[1];
-
+ /* Allow overriding specific health bits via bit blt. */
+ bitmap = BITBLT_BITMAP(bitmap, p->health_bitmap_mask,
+ p->health_bitmap_override);
+ WRITE_ONCE(p->health_bitmap, bitmap);
dev_dbg(&p->pdev->dev,
"Queried dimm health info. Bitmap:0x%016lx Mask:0x%016lx\n",
ret[0], ret[1]);
@@ -669,6 +688,54 @@ static int papr_pdsm_health(struct papr_scm_priv *p,
return rc;
}
+/* Inject a smart error Add the dirty-shutdown-counter value to the pdsm */
+static int papr_pdsm_smart_inject(struct papr_scm_priv *p,
+ union nd_pdsm_payload *payload)
+{
+ int rc;
+ u32 supported_flags = 0;
+ u64 mask = 0, override = 0;
+
+ /* Check for individual smart error flags and update mask and override */
+ if (payload->smart_inject.flags & PDSM_SMART_INJECT_HEALTH_FATAL) {
+ supported_flags |= PDSM_SMART_INJECT_HEALTH_FATAL;
+ mask |= PAPR_PMEM_HEALTH_FATAL;
+ override |= payload->smart_inject.fatal_enable ?
+ PAPR_PMEM_HEALTH_FATAL : 0;
+ }
+
+ if (payload->smart_inject.flags & PDSM_SMART_INJECT_BAD_SHUTDOWN) {
+ supported_flags |= PDSM_SMART_INJECT_BAD_SHUTDOWN;
+ mask |= PAPR_PMEM_SHUTDOWN_DIRTY;
+ override |= payload->smart_inject.unsafe_shutdown_enable ?
+ PAPR_PMEM_SHUTDOWN_DIRTY : 0;
+ }
+
+ dev_dbg(&p->pdev->dev, "[Smart-inject] Mask=%#llx override=%#llx\n",
+ mask, override);
+
+ /* Prevent concurrent access to dimm health bitmap related members */
+ rc = mutex_lock_interruptible(&p->health_mutex);
+ if (rc)
+ return rc;
+
+ /* Bitblt mask/override to corrosponding health_bitmap couterparts */
+ p->health_bitmap_mask = BITBLT_BITMAP(p->health_bitmap_mask,
+ mask, override);
+ p->health_bitmap_override = BITBLT_BITMAP(p->health_bitmap_override,
+ mask, override);
+
+ /* Invalidate cached health bitmap */
+ p->lasthealth_jiffies = 0;
+
+ mutex_unlock(&p->health_mutex);
+
+ /* Return the supported flags back to userspace */
+ payload->smart_inject.flags = supported_flags;
+
+ return sizeof(struct nd_papr_pdsm_health);
+}
+
/*
* 'struct pdsm_cmd_desc'
* Identifies supported PDSMs' expected length of in/out payloads
@@ -702,6 +769,12 @@ static const struct pdsm_cmd_desc __pdsm_cmd_descriptors[] = {
.size_out = sizeof(struct nd_papr_pdsm_health),
.service = papr_pdsm_health,
},
+
+ [PAPR_PDSM_SMART_INJECT] = {
+ .size_in = sizeof(struct nd_papr_pdsm_smart_inject),
+ .size_out = sizeof(struct nd_papr_pdsm_smart_inject),
+ .service = papr_pdsm_smart_inject,
+ },
/* Empty */
[PAPR_PDSM_MAX] = {
.size_in = 0,
@@ -838,6 +911,20 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
return 0;
}
+static ssize_t health_bitmap_override_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct nvdimm *dimm = to_nvdimm(dev);
+ struct papr_scm_priv *p = nvdimm_provider_data(dimm);
+
+ return sprintf(buf, "mask=%#llx override=%#llx\n",
+ READ_ONCE(p->health_bitmap_mask),
+ READ_ONCE(p->health_bitmap_override));
+}
+
+static DEVICE_ATTR_ADMIN_RO(health_bitmap_override);
+
static ssize_t perf_stats_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -952,6 +1039,7 @@ static struct attribute *papr_nd_attributes[] = {
&dev_attr_flags.attr,
&dev_attr_perf_stats.attr,
&dev_attr_dirty_shutdown.attr,
+ &dev_attr_health_bitmap_override.attr,
NULL,
};
^ permalink raw reply related
* [REPOST PATCH v2] tests/nvdimm/ndtest: Simulate nvdimm health, DSC and smart-inject
From: Shivaprasad G Bhat @ 2021-10-18 8:06 UTC (permalink / raw)
To: nvdimm
Cc: sbhat, aneesh.kumar, vaibhav, dan.j.williams, linuxppc-dev,
ira.weiny
The 'papr_scm' module and 'papr' implementation in libndctl supports
PDSMs for reporting PAPR NVDIMM health, dirty-shutdown-count and
injecting smart-errors. This patch adds support for those PDSMs in
ndtest module so that PDSM specific paths in libndctl can be exercised.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:
Since v1:
Link: https://patchwork.kernel.org/project/linux-nvdimm/list/?series=521767
* Removed the dependency on a header movement patch
tools/testing/nvdimm/test/ndtest.c | 148 ++++++++++++++++++++++++++++++++++++
tools/testing/nvdimm/test/ndtest.h | 96 +++++++++++++++++++++++
2 files changed, 244 insertions(+)
diff --git a/tools/testing/nvdimm/test/ndtest.c b/tools/testing/nvdimm/test/ndtest.c
index 6862915f1fb0c..45d42cd25e82f 100644
--- a/tools/testing/nvdimm/test/ndtest.c
+++ b/tools/testing/nvdimm/test/ndtest.c
@@ -48,6 +48,10 @@ static struct ndtest_dimm dimm_group1[] = {
.uuid_str = "1e5c75d2-b618-11ea-9aa3-507b9ddc0f72",
.physical_id = 0,
.num_formats = 2,
+ .flags = PAPR_PMEM_HEALTH_NON_CRITICAL,
+ .extension_flags = PDSM_DIMM_DSC_VALID | PDSM_DIMM_HEALTH_RUN_GAUGE_VALID,
+ .dimm_fuel_gauge = 95,
+ .dimm_dsc = 42,
},
{
.size = DIMM_SIZE,
@@ -55,6 +59,10 @@ static struct ndtest_dimm dimm_group1[] = {
.uuid_str = "1c4d43ac-b618-11ea-be80-507b9ddc0f72",
.physical_id = 1,
.num_formats = 2,
+ .flags = PAPR_PMEM_HEALTH_NON_CRITICAL,
+ .extension_flags = PDSM_DIMM_DSC_VALID | PDSM_DIMM_HEALTH_RUN_GAUGE_VALID,
+ .dimm_fuel_gauge = 95,
+ .dimm_dsc = 42,
},
{
.size = DIMM_SIZE,
@@ -62,6 +70,10 @@ static struct ndtest_dimm dimm_group1[] = {
.uuid_str = "a9f17ffc-b618-11ea-b36d-507b9ddc0f72",
.physical_id = 2,
.num_formats = 2,
+ .flags = PAPR_PMEM_HEALTH_NON_CRITICAL,
+ .extension_flags = PDSM_DIMM_DSC_VALID | PDSM_DIMM_HEALTH_RUN_GAUGE_VALID,
+ .dimm_fuel_gauge = 95,
+ .dimm_dsc = 42,
},
{
.size = DIMM_SIZE,
@@ -69,6 +81,10 @@ static struct ndtest_dimm dimm_group1[] = {
.uuid_str = "b6b83b22-b618-11ea-8aae-507b9ddc0f72",
.physical_id = 3,
.num_formats = 2,
+ .flags = PAPR_PMEM_HEALTH_NON_CRITICAL,
+ .extension_flags = PDSM_DIMM_DSC_VALID | PDSM_DIMM_HEALTH_RUN_GAUGE_VALID,
+ .dimm_fuel_gauge = 95,
+ .dimm_dsc = 42,
},
{
.size = DIMM_SIZE,
@@ -296,6 +312,103 @@ static int ndtest_get_config_size(struct ndtest_dimm *dimm, unsigned int buf_len
return 0;
}
+static int ndtest_pdsm_health(struct ndtest_dimm *dimm,
+ union nd_pdsm_payload *payload,
+ unsigned int buf_len)
+{
+ struct nd_papr_pdsm_health *health = &payload->health;
+
+ if (buf_len < sizeof(health))
+ return -EINVAL;
+
+ health->extension_flags = 0;
+ health->dimm_unarmed = !!(dimm->flags & PAPR_PMEM_UNARMED_MASK);
+ health->dimm_bad_shutdown = !!(dimm->flags & PAPR_PMEM_BAD_SHUTDOWN_MASK);
+ health->dimm_bad_restore = !!(dimm->flags & PAPR_PMEM_BAD_RESTORE_MASK);
+ health->dimm_health = PAPR_PDSM_DIMM_HEALTHY;
+
+ if (dimm->flags & PAPR_PMEM_HEALTH_FATAL)
+ health->dimm_health = PAPR_PDSM_DIMM_FATAL;
+ else if (dimm->flags & PAPR_PMEM_HEALTH_CRITICAL)
+ health->dimm_health = PAPR_PDSM_DIMM_CRITICAL;
+ else if (dimm->flags & PAPR_PMEM_HEALTH_UNHEALTHY ||
+ dimm->flags & PAPR_PMEM_HEALTH_NON_CRITICAL)
+ health->dimm_health = PAPR_PDSM_DIMM_UNHEALTHY;
+
+ health->extension_flags = 0;
+ if (dimm->extension_flags & PDSM_DIMM_HEALTH_RUN_GAUGE_VALID) {
+ health->dimm_fuel_gauge = dimm->dimm_fuel_gauge;
+ health->extension_flags |= PDSM_DIMM_HEALTH_RUN_GAUGE_VALID;
+ }
+ if (dimm->extension_flags & PDSM_DIMM_DSC_VALID) {
+ health->dimm_dsc = dimm->dimm_dsc;
+ health->extension_flags |= PDSM_DIMM_DSC_VALID;
+ }
+
+ return 0;
+}
+
+static void smart_notify(struct ndtest_dimm *dimm)
+{
+ struct device *bus = dimm->dev->parent;
+
+ if (!(dimm->flags & PAPR_PMEM_HEALTH_NON_CRITICAL) ||
+ (dimm->flags & PAPR_PMEM_BAD_SHUTDOWN_MASK)) {
+ device_lock(bus);
+ /* send smart notification */
+ if (dimm->notify_handle)
+ sysfs_notify_dirent(dimm->notify_handle);
+ device_unlock(bus);
+ }
+}
+
+static int ndtest_pdsm_smart_inject(struct ndtest_dimm *dimm,
+ union nd_pdsm_payload *payload,
+ unsigned int buf_len)
+{
+ struct nd_papr_pdsm_smart_inject *inj = &payload->smart_inject;
+
+ if (buf_len < sizeof(inj))
+ return -EINVAL;
+
+ if (inj->flags & PDSM_SMART_INJECT_HEALTH_FATAL) {
+ if (inj->fatal_enable)
+ dimm->flags |= PAPR_PMEM_HEALTH_FATAL;
+ else
+ dimm->flags &= ~PAPR_PMEM_HEALTH_FATAL;
+ }
+ if (inj->flags & PDSM_SMART_INJECT_BAD_SHUTDOWN) {
+ if (inj->unsafe_shutdown_enable)
+ dimm->flags |= PAPR_PMEM_SHUTDOWN_DIRTY;
+ else
+ dimm->flags &= ~PAPR_PMEM_SHUTDOWN_DIRTY;
+ }
+ smart_notify(dimm);
+
+ return 0;
+}
+
+static int ndtest_dimm_cmd_call(struct ndtest_dimm *dimm, unsigned int buf_len,
+ void *buf)
+{
+ struct nd_cmd_pkg *call_pkg = buf;
+ unsigned int len = call_pkg->nd_size_in + call_pkg->nd_size_out;
+ struct nd_pkg_pdsm *pdsm = (struct nd_pkg_pdsm *) call_pkg->nd_payload;
+ union nd_pdsm_payload *payload = &(pdsm->payload);
+ unsigned int func = call_pkg->nd_command;
+
+ switch (func) {
+ case PAPR_PDSM_HEALTH:
+ return ndtest_pdsm_health(dimm, payload, len);
+ case PAPR_PDSM_SMART_INJECT:
+ return ndtest_pdsm_smart_inject(dimm, payload, len);
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int ndtest_ctl(struct nvdimm_bus_descriptor *nd_desc,
struct nvdimm *nvdimm, unsigned int cmd, void *buf,
unsigned int buf_len, int *cmd_rc)
@@ -325,6 +438,9 @@ static int ndtest_ctl(struct nvdimm_bus_descriptor *nd_desc,
case ND_CMD_SET_CONFIG_DATA:
*cmd_rc = ndtest_config_set(dimm, buf_len, buf);
break;
+ case ND_CMD_CALL:
+ *cmd_rc = ndtest_dimm_cmd_call(dimm, buf_len, buf);
+ break;
default:
return -EINVAL;
}
@@ -614,6 +730,8 @@ static void put_dimms(void *data)
for (i = 0; i < p->config->dimm_count; i++)
if (p->config->dimms[i].dev) {
+ if (p->config->dimms[i].notify_handle)
+ sysfs_put(p->config->dimms[i].notify_handle);
device_unregister(p->config->dimms[i].dev);
p->config->dimms[i].dev = NULL;
}
@@ -826,6 +944,18 @@ static ssize_t flags_show(struct device *dev,
}
static DEVICE_ATTR_RO(flags);
+#define PAPR_PMEM_DIMM_CMD_MASK \
+ ((1U << PAPR_PDSM_HEALTH) \
+ | (1U << PAPR_PDSM_SMART_INJECT))
+
+static ssize_t dsm_mask_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sprintf(buf, "%#x\n", PAPR_PMEM_DIMM_CMD_MASK);
+}
+
+static DEVICE_ATTR_RO(dsm_mask);
+
static struct attribute *ndtest_nvdimm_attributes[] = {
&dev_attr_nvdimm_show_handle.attr,
&dev_attr_vendor.attr,
@@ -837,6 +967,7 @@ static struct attribute *ndtest_nvdimm_attributes[] = {
&dev_attr_format.attr,
&dev_attr_format1.attr,
&dev_attr_flags.attr,
+ &dev_attr_dsm_mask.attr,
NULL,
};
@@ -856,6 +987,7 @@ static int ndtest_dimm_register(struct ndtest_priv *priv,
{
struct device *dev = &priv->pdev.dev;
unsigned long dimm_flags = dimm->flags;
+ struct kernfs_node *papr_kernfs;
if (dimm->num_formats > 1) {
set_bit(NDD_ALIASING, &dimm_flags);
@@ -882,6 +1014,20 @@ static int ndtest_dimm_register(struct ndtest_priv *priv,
return -ENOMEM;
}
+ nd_synchronize();
+
+ papr_kernfs = sysfs_get_dirent(nvdimm_kobj(dimm->nvdimm)->sd, "papr");
+ if (!papr_kernfs) {
+ pr_err("Could not initialize the notifier handle\n");
+ return 0;
+ }
+
+ dimm->notify_handle = sysfs_get_dirent(papr_kernfs, "flags");
+ sysfs_put(papr_kernfs);
+ if (!dimm->notify_handle) {
+ pr_err("Could not initialize the notifier handle\n");
+ return 0;
+ }
return 0;
}
@@ -953,6 +1099,8 @@ static int ndtest_bus_register(struct ndtest_priv *p)
p->bus_desc.provider_name = NULL;
p->bus_desc.attr_groups = ndtest_attribute_groups;
+ set_bit(NVDIMM_FAMILY_PAPR, &p->bus_desc.dimm_family_mask);
+
p->bus = nvdimm_bus_register(&p->pdev.dev, &p->bus_desc);
if (!p->bus) {
dev_err(&p->pdev.dev, "Error creating nvdimm bus %pOF\n", p->dn);
diff --git a/tools/testing/nvdimm/test/ndtest.h b/tools/testing/nvdimm/test/ndtest.h
index 2c54c9cbb90c7..b9b3810213132 100644
--- a/tools/testing/nvdimm/test/ndtest.h
+++ b/tools/testing/nvdimm/test/ndtest.h
@@ -16,6 +16,8 @@
#define PAPR_PMEM_HEALTH_FATAL (1ULL << (63 - 5))
/* SCM contents cannot persist due to current platform health status */
#define PAPR_PMEM_HEALTH_UNHEALTHY (1ULL << (63 - 6))
+/* SCM device is unable to persist memory contents in certain conditions */
+#define PAPR_PMEM_HEALTH_NON_CRITICAL (1ULL << (63 - 7))
/* Bits status indicators for health bitmap indicating unarmed dimm */
#define PAPR_PMEM_UNARMED_MASK (PAPR_PMEM_UNARMED | \
@@ -80,6 +82,13 @@ struct ndtest_dimm {
int id;
int fail_cmd_code;
u8 no_alias;
+
+ struct kernfs_node *notify_handle;
+
+ /* SMART Health information */
+ u32 extension_flags;
+ u16 dimm_fuel_gauge;
+ u64 dimm_dsc;
};
struct ndtest_mapping {
@@ -98,6 +107,93 @@ struct ndtest_region {
u8 range_index;
};
+#define ND_PDSM_PAYLOAD_MAX_SIZE 184
+/*
+ * Methods to be embedded in ND_CMD_CALL request. These are sent to the kernel
+ * via 'nd_cmd_pkg.nd_command' member of the ioctl struct
+ */
+enum papr_pdsm {
+ PAPR_PDSM_MIN = 0x0,
+ PAPR_PDSM_HEALTH,
+ PAPR_PDSM_SMART_INJECT,
+ PAPR_PDSM_MAX,
+};
+
+/* Various nvdimm health indicators */
+#define PAPR_PDSM_DIMM_HEALTHY 0
+#define PAPR_PDSM_DIMM_UNHEALTHY 1
+#define PAPR_PDSM_DIMM_CRITICAL 2
+#define PAPR_PDSM_DIMM_FATAL 3
+
+/* struct nd_papr_pdsm_health.extension_flags field flags */
+
+/* Indicate that the 'dimm_fuel_gauge' field is valid */
+#define PDSM_DIMM_HEALTH_RUN_GAUGE_VALID 1
+
+/* Indicate that the 'dimm_dsc' field is valid */
+#define PDSM_DIMM_DSC_VALID 2
+
+/*
+ * Struct exchanged between kernel & ndctl in for PAPR_PDSM_HEALTH
+ * Various flags indicate the health status of the dimm.
+ */
+struct nd_papr_pdsm_health {
+ union {
+ struct {
+ __u32 extension_flags;
+ __u8 dimm_unarmed;
+ __u8 dimm_bad_shutdown;
+ __u8 dimm_bad_restore;
+ __u8 dimm_scrubbed;
+ __u8 dimm_locked;
+ __u8 dimm_encrypted;
+ __u16 dimm_health;
+
+ /* Extension flag PDSM_DIMM_HEALTH_RUN_GAUGE_VALID */
+ __u16 dimm_fuel_gauge;
+
+ /* Extension flag PDSM_DIMM_DSC_VALID */
+ __u64 dimm_dsc;
+ };
+ __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
+ };
+};
+
+/* Flags for injecting specific smart errors */
+#define PDSM_SMART_INJECT_HEALTH_FATAL (1 << 0)
+#define PDSM_SMART_INJECT_BAD_SHUTDOWN (1 << 1)
+
+struct nd_papr_pdsm_smart_inject {
+ union {
+ struct {
+ /* One or more of PDSM_SMART_INJECT_ */
+ __u32 flags;
+ __u8 fatal_enable;
+ __u8 unsafe_shutdown_enable;
+ };
+ __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
+ };
+};
+
+/* Maximal union that can hold all possible payload types */
+union nd_pdsm_payload {
+ struct nd_papr_pdsm_health health;
+ struct nd_papr_pdsm_smart_inject smart_inject;
+ __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
+} __packed;
+
+/*
+ * PDSM-header + payload expected with ND_CMD_CALL ioctl from libnvdimm
+ * Valid member of union 'payload' is identified via 'nd_cmd_pkg.nd_command'
+ * that should always precede this struct when sent to papr_scm via CMD_CALL
+ * interface.
+ */
+struct nd_pkg_pdsm {
+ __s32 cmd_status; /* Out: Sub-cmd status returned back */
+ __u16 reserved[2]; /* Ignored and to be set as '0' */
+ union nd_pdsm_payload payload;
+} __packed;
+
struct ndtest_config {
struct ndtest_dimm *dimms;
struct ndtest_region *regions;
^ permalink raw reply related
* [REPOST RFC PATCH v2] powerpc/papr_scm: Move duplicate definitions to common header files
From: Shivaprasad G Bhat @ 2021-10-18 8:07 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel, mpe
Cc: nvdimm, dan.j.williams, vaibhav, sbhat, aneesh.kumar
papr_scm and ndtest share common PDSM payload structs like
nd_papr_pdsm_health. Presently these structs are duplicated across
papr_pdsm.h and ndtest.h header files. Since 'ndtest' is essentially
arch independent and can run on platforms other than PPC64, a way
needs to be deviced to avoid redundancy and duplication of PDSM
structs in future.
So the patch proposes moving the PDSM header from arch/powerpc/include-
-/uapi/ to the generic include/uapi/linux directory. Also, there are
some #defines common between papr_scm and ndtest which are not exported
to the user space. So, move them to a header file which can be shared
across ndtest and papr_scm via newly introduced include/linux/papr_scm.h.
Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
Suggested-by: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
---
Changelog:
Since v1:
Link: https://patchwork.kernel.org/project/linux-nvdimm/patch/162505488483.72147.12741153746322191381.stgit@56e104a48989/
* Removed dependency on this patch for the other patches
MAINTAINERS | 2
arch/powerpc/include/uapi/asm/papr_pdsm.h | 165 -----------------------------
arch/powerpc/platforms/pseries/papr_scm.c | 43 --------
include/linux/papr_scm.h | 48 ++++++++
include/uapi/linux/papr_pdsm.h | 165 +++++++++++++++++++++++++++++
tools/testing/nvdimm/test/ndtest.c | 2
tools/testing/nvdimm/test/ndtest.h | 120 ---------------------
7 files changed, 219 insertions(+), 326 deletions(-)
delete mode 100644 arch/powerpc/include/uapi/asm/papr_pdsm.h
create mode 100644 include/linux/papr_scm.h
create mode 100644 include/uapi/linux/papr_pdsm.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 5b33791bb8e97..3e78f595f3d91 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10746,6 +10746,8 @@ F: drivers/rtc/rtc-opal.c
F: drivers/scsi/ibmvscsi/
F: drivers/tty/hvc/hvc_opal.c
F: drivers/watchdog/wdrtas.c
+F: include/linux/papr_scm.h
+F: include/uapi/linux/papr_pdsm.h
F: tools/testing/selftests/powerpc
N: /pmac
N: powermac
diff --git a/arch/powerpc/include/uapi/asm/papr_pdsm.h b/arch/powerpc/include/uapi/asm/papr_pdsm.h
deleted file mode 100644
index 17439925045cf..0000000000000
--- a/arch/powerpc/include/uapi/asm/papr_pdsm.h
+++ /dev/null
@@ -1,165 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-/*
- * PAPR nvDimm Specific Methods (PDSM) and structs for libndctl
- *
- * (C) Copyright IBM 2020
- *
- * Author: Vaibhav Jain <vaibhav at linux.ibm.com>
- */
-
-#ifndef _UAPI_ASM_POWERPC_PAPR_PDSM_H_
-#define _UAPI_ASM_POWERPC_PAPR_PDSM_H_
-
-#include <linux/types.h>
-#include <linux/ndctl.h>
-
-/*
- * PDSM Envelope:
- *
- * The ioctl ND_CMD_CALL exchange data between user-space and kernel via
- * envelope which consists of 2 headers sections and payload sections as
- * illustrated below:
- * +-----------------+---------------+---------------------------+
- * | 64-Bytes | 8-Bytes | Max 184-Bytes |
- * +-----------------+---------------+---------------------------+
- * | ND-HEADER | PDSM-HEADER | PDSM-PAYLOAD |
- * +-----------------+---------------+---------------------------+
- * | nd_family | | |
- * | nd_size_out | cmd_status | |
- * | nd_size_in | reserved | nd_pdsm_payload |
- * | nd_command | payload --> | |
- * | nd_fw_size | | |
- * | nd_payload ---> | | |
- * +---------------+-----------------+---------------------------+
- *
- * ND Header:
- * This is the generic libnvdimm header described as 'struct nd_cmd_pkg'
- * which is interpreted by libnvdimm before passed on to papr_scm. Important
- * member fields used are:
- * 'nd_family' : (In) NVDIMM_FAMILY_PAPR_SCM
- * 'nd_size_in' : (In) PDSM-HEADER + PDSM-IN-PAYLOAD (usually 0)
- * 'nd_size_out' : (In) PDSM-HEADER + PDSM-RETURN-PAYLOAD
- * 'nd_command' : (In) One of PAPR_PDSM_XXX
- * 'nd_fw_size' : (Out) PDSM-HEADER + size of actual payload returned
- *
- * PDSM Header:
- * This is papr-scm specific header that precedes the payload. This is defined
- * as nd_cmd_pdsm_pkg. Following fields aare available in this header:
- *
- * 'cmd_status' : (Out) Errors if any encountered while servicing PDSM.
- * 'reserved' : Not used, reserved for future and should be set to 0.
- * 'payload' : A union of all the possible payload structs
- *
- * PDSM Payload:
- *
- * The layout of the PDSM Payload is defined by various structs shared between
- * papr_scm and libndctl so that contents of payload can be interpreted. As such
- * its defined as a union of all possible payload structs as
- * 'union nd_pdsm_payload'. Based on the value of 'nd_cmd_pkg.nd_command'
- * appropriate member of the union is accessed.
- */
-
-/* Max payload size that we can handle */
-#define ND_PDSM_PAYLOAD_MAX_SIZE 184
-
-/* Max payload size that we can handle */
-#define ND_PDSM_HDR_SIZE \
- (sizeof(struct nd_pkg_pdsm) - ND_PDSM_PAYLOAD_MAX_SIZE)
-
-/* Various nvdimm health indicators */
-#define PAPR_PDSM_DIMM_HEALTHY 0
-#define PAPR_PDSM_DIMM_UNHEALTHY 1
-#define PAPR_PDSM_DIMM_CRITICAL 2
-#define PAPR_PDSM_DIMM_FATAL 3
-
-/* struct nd_papr_pdsm_health.extension_flags field flags */
-
-/* Indicate that the 'dimm_fuel_gauge' field is valid */
-#define PDSM_DIMM_HEALTH_RUN_GAUGE_VALID 1
-
-/* Indicate that the 'dimm_dsc' field is valid */
-#define PDSM_DIMM_DSC_VALID 2
-
-/*
- * Struct exchanged between kernel & ndctl in for PAPR_PDSM_HEALTH
- * Various flags indicate the health status of the dimm.
- *
- * extension_flags : Any extension fields present in the struct.
- * 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. One of PAPR_PDSM_DIMM_XXXX
- * dimm_fuel_gauge : Life remaining of DIMM as a percentage from 0-100
- */
-struct nd_papr_pdsm_health {
- union {
- struct {
- __u32 extension_flags;
- __u8 dimm_unarmed;
- __u8 dimm_bad_shutdown;
- __u8 dimm_bad_restore;
- __u8 dimm_scrubbed;
- __u8 dimm_locked;
- __u8 dimm_encrypted;
- __u16 dimm_health;
-
- /* Extension flag PDSM_DIMM_HEALTH_RUN_GAUGE_VALID */
- __u16 dimm_fuel_gauge;
-
- /* Extension flag PDSM_DIMM_DSC_VALID */
- __u64 dimm_dsc;
- };
- __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
- };
-};
-
-/* Flags for injecting specific smart errors */
-#define PDSM_SMART_INJECT_HEALTH_FATAL (1 << 0)
-#define PDSM_SMART_INJECT_BAD_SHUTDOWN (1 << 1)
-
-struct nd_papr_pdsm_smart_inject {
- union {
- struct {
- /* One or more of PDSM_SMART_INJECT_ */
- __u32 flags;
- __u8 fatal_enable;
- __u8 unsafe_shutdown_enable;
- };
- __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
- };
-};
-
-/*
- * Methods to be embedded in ND_CMD_CALL request. These are sent to the kernel
- * via 'nd_cmd_pkg.nd_command' member of the ioctl struct
- */
-enum papr_pdsm {
- PAPR_PDSM_MIN = 0x0,
- PAPR_PDSM_HEALTH,
- PAPR_PDSM_SMART_INJECT,
- PAPR_PDSM_MAX,
-};
-
-/* Maximal union that can hold all possible payload types */
-union nd_pdsm_payload {
- struct nd_papr_pdsm_health health;
- struct nd_papr_pdsm_smart_inject smart_inject;
- __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
-} __packed;
-
-/*
- * PDSM-header + payload expected with ND_CMD_CALL ioctl from libnvdimm
- * Valid member of union 'payload' is identified via 'nd_cmd_pkg.nd_command'
- * that should always precede this struct when sent to papr_scm via CMD_CALL
- * interface.
- */
-struct nd_pkg_pdsm {
- __s32 cmd_status; /* Out: Sub-cmd status returned back */
- __u16 reserved[2]; /* Ignored and to be set as '0' */
- union nd_pdsm_payload payload;
-} __packed;
-
-#endif /* _UAPI_ASM_POWERPC_PAPR_PDSM_H_ */
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index de4cf329cfb3b..b7437c61a2704 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -16,7 +16,8 @@
#include <linux/nd.h>
#include <asm/plpar_wrappers.h>
-#include <asm/papr_pdsm.h>
+#include <uapi/linux/papr_pdsm.h>
+#include <linux/papr_scm.h>
#include <asm/mce.h>
#include <asm/unaligned.h>
@@ -28,46 +29,6 @@
(1ul << ND_CMD_SET_CONFIG_DATA) | \
(1ul << ND_CMD_CALL))
-/* DIMM health bitmap bitmap indicators */
-/* SCM device is unable to persist memory contents */
-#define PAPR_PMEM_UNARMED (1ULL << (63 - 0))
-/* SCM device failed to persist memory contents */
-#define PAPR_PMEM_SHUTDOWN_DIRTY (1ULL << (63 - 1))
-/* SCM device contents are persisted from previous IPL */
-#define PAPR_PMEM_SHUTDOWN_CLEAN (1ULL << (63 - 2))
-/* SCM device contents are not persisted from previous IPL */
-#define PAPR_PMEM_EMPTY (1ULL << (63 - 3))
-/* SCM device memory life remaining is critically low */
-#define PAPR_PMEM_HEALTH_CRITICAL (1ULL << (63 - 4))
-/* SCM device will be garded off next IPL due to failure */
-#define PAPR_PMEM_HEALTH_FATAL (1ULL << (63 - 5))
-/* SCM contents cannot persist due to current platform health status */
-#define PAPR_PMEM_HEALTH_UNHEALTHY (1ULL << (63 - 6))
-/* SCM device is unable to persist memory contents in certain conditions */
-#define PAPR_PMEM_HEALTH_NON_CRITICAL (1ULL << (63 - 7))
-/* SCM device is encrypted */
-#define PAPR_PMEM_ENCRYPTED (1ULL << (63 - 8))
-/* SCM device has been scrubbed and locked */
-#define PAPR_PMEM_SCRUBBED_AND_LOCKED (1ULL << (63 - 9))
-
-/* Bits status indicators for health bitmap indicating unarmed dimm */
-#define PAPR_PMEM_UNARMED_MASK (PAPR_PMEM_UNARMED | \
- PAPR_PMEM_HEALTH_UNHEALTHY)
-
-/* Bits status indicators for health bitmap indicating unflushed dimm */
-#define PAPR_PMEM_BAD_SHUTDOWN_MASK (PAPR_PMEM_SHUTDOWN_DIRTY)
-
-/* Bits status indicators for health bitmap indicating unrestored dimm */
-#define PAPR_PMEM_BAD_RESTORE_MASK (PAPR_PMEM_EMPTY)
-
-/* Bit status indicators for smart event notification */
-#define PAPR_PMEM_SMART_EVENT_MASK (PAPR_PMEM_HEALTH_CRITICAL | \
- PAPR_PMEM_HEALTH_FATAL | \
- PAPR_PMEM_HEALTH_UNHEALTHY)
-
-#define PAPR_SCM_PERF_STATS_EYECATCHER __stringify(SCMSTATS)
-#define PAPR_SCM_PERF_STATS_VERSION 0x1
-
/* Use bitblt method to override specific bits in the '_bitmap_' */
#define BITBLT_BITMAP(_bitmap_, _mask_, _override_) \
(((_bitmap_) & ~(_mask_)) | ((_mask_) & (_override_)))
diff --git a/include/linux/papr_scm.h b/include/linux/papr_scm.h
new file mode 100644
index 0000000000000..f116e5ffef369
--- /dev/null
+++ b/include/linux/papr_scm.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __LINUX_PAPR_SCM_H
+#define __LINUX_PAPR_SCM_H
+
+/* DIMM health bitmap bitmap indicators */
+/* SCM device is unable to persist memory contents */
+#define PAPR_PMEM_UNARMED (1ULL << (63 - 0))
+/* SCM device failed to persist memory contents */
+#define PAPR_PMEM_SHUTDOWN_DIRTY (1ULL << (63 - 1))
+/* SCM device contents are persisted from previous IPL */
+#define PAPR_PMEM_SHUTDOWN_CLEAN (1ULL << (63 - 2))
+/* SCM device contents are not persisted from previous IPL */
+#define PAPR_PMEM_EMPTY (1ULL << (63 - 3))
+/* SCM device memory life remaining is critically low */
+#define PAPR_PMEM_HEALTH_CRITICAL (1ULL << (63 - 4))
+/* SCM device will be garded off next IPL due to failure */
+#define PAPR_PMEM_HEALTH_FATAL (1ULL << (63 - 5))
+/* SCM contents cannot persist due to current platform health status */
+#define PAPR_PMEM_HEALTH_UNHEALTHY (1ULL << (63 - 6))
+/* SCM device is unable to persist memory contents in certain conditions */
+#define PAPR_PMEM_HEALTH_NON_CRITICAL (1ULL << (63 - 7))
+/* SCM device is encrypted */
+#define PAPR_PMEM_ENCRYPTED (1ULL << (63 - 8))
+/* SCM device has been scrubbed and locked */
+#define PAPR_PMEM_SCRUBBED_AND_LOCKED (1ULL << (63 - 9))
+
+#define PAPR_PMEM_SAVE_FAILED (1ULL << (63 - 10))
+
+/* Bits status indicators for health bitmap indicating unarmed dimm */
+#define PAPR_PMEM_UNARMED_MASK (PAPR_PMEM_UNARMED | PAPR_PMEM_HEALTH_UNHEALTHY)
+
+/* Bits status indicators for health bitmap indicating unflushed dimm */
+#define PAPR_PMEM_BAD_SHUTDOWN_MASK (PAPR_PMEM_SHUTDOWN_DIRTY)
+
+/* Bits status indicators for health bitmap indicating unrestored dimm */
+#define PAPR_PMEM_BAD_RESTORE_MASK (PAPR_PMEM_EMPTY)
+
+/* Bit status indicators for smart event notification */
+#define PAPR_PMEM_SMART_EVENT_MASK (PAPR_PMEM_HEALTH_CRITICAL | \
+ PAPR_PMEM_HEALTH_FATAL | \
+ PAPR_PMEM_HEALTH_UNHEALTHY)
+
+#define PAPR_PMEM_SAVE_MASK (PAPR_PMEM_SAVE_FAILED)
+
+#define PAPR_SCM_PERF_STATS_EYECATCHER __stringify(SCMSTATS)
+#define PAPR_SCM_PERF_STATS_VERSION 0x1
+
+#endif /* __LINUX_PAPR_SCM_H */
diff --git a/include/uapi/linux/papr_pdsm.h b/include/uapi/linux/papr_pdsm.h
new file mode 100644
index 0000000000000..1be9906f45400
--- /dev/null
+++ b/include/uapi/linux/papr_pdsm.h
@@ -0,0 +1,165 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * PAPR nvDimm Specific Methods (PDSM) and structs for libndctl
+ *
+ * (C) Copyright IBM 2020-2021
+ *
+ * Author: Vaibhav Jain <vaibhav at linux.ibm.com>
+ */
+
+#ifndef _UAPI_LINUX_PAPR_PDSM_H_
+#define _UAPI_LINUX_PAPR_PDSM_H_
+
+#include <linux/types.h>
+#include <linux/ndctl.h>
+
+/*
+ * PDSM Envelope:
+ *
+ * The ioctl ND_CMD_CALL exchange data between user-space and kernel via
+ * envelope which consists of 2 headers sections and payload sections as
+ * illustrated below:
+ * +-----------------+---------------+---------------------------+
+ * | 64-Bytes | 8-Bytes | Max 184-Bytes |
+ * +-----------------+---------------+---------------------------+
+ * | ND-HEADER | PDSM-HEADER | PDSM-PAYLOAD |
+ * +-----------------+---------------+---------------------------+
+ * | nd_family | | |
+ * | nd_size_out | cmd_status | |
+ * | nd_size_in | reserved | nd_pdsm_payload |
+ * | nd_command | payload --> | |
+ * | nd_fw_size | | |
+ * | nd_payload ---> | | |
+ * +---------------+-----------------+---------------------------+
+ *
+ * ND Header:
+ * This is the generic libnvdimm header described as 'struct nd_cmd_pkg'
+ * which is interpreted by libnvdimm before passed on to papr_scm. Important
+ * member fields used are:
+ * 'nd_family' : (In) NVDIMM_FAMILY_PAPR_SCM
+ * 'nd_size_in' : (In) PDSM-HEADER + PDSM-IN-PAYLOAD (usually 0)
+ * 'nd_size_out' : (In) PDSM-HEADER + PDSM-RETURN-PAYLOAD
+ * 'nd_command' : (In) One of PAPR_PDSM_XXX
+ * 'nd_fw_size' : (Out) PDSM-HEADER + size of actual payload returned
+ *
+ * PDSM Header:
+ * This is papr-scm specific header that precedes the payload. This is defined
+ * as nd_cmd_pdsm_pkg. Following fields aare available in this header:
+ *
+ * 'cmd_status' : (Out) Errors if any encountered while servicing PDSM.
+ * 'reserved' : Not used, reserved for future and should be set to 0.
+ * 'payload' : A union of all the possible payload structs
+ *
+ * PDSM Payload:
+ *
+ * The layout of the PDSM Payload is defined by various structs shared between
+ * papr_scm and libndctl so that contents of payload can be interpreted. As such
+ * its defined as a union of all possible payload structs as
+ * 'union nd_pdsm_payload'. Based on the value of 'nd_cmd_pkg.nd_command'
+ * appropriate member of the union is accessed.
+ */
+
+/* Max payload size that we can handle */
+#define ND_PDSM_PAYLOAD_MAX_SIZE 184
+
+/* Max payload size that we can handle */
+#define ND_PDSM_HDR_SIZE \
+ (sizeof(struct nd_pkg_pdsm) - ND_PDSM_PAYLOAD_MAX_SIZE)
+
+/* Various nvdimm health indicators */
+#define PAPR_PDSM_DIMM_HEALTHY 0
+#define PAPR_PDSM_DIMM_UNHEALTHY 1
+#define PAPR_PDSM_DIMM_CRITICAL 2
+#define PAPR_PDSM_DIMM_FATAL 3
+
+/* struct nd_papr_pdsm_health.extension_flags field flags */
+
+/* Indicate that the 'dimm_fuel_gauge' field is valid */
+#define PDSM_DIMM_HEALTH_RUN_GAUGE_VALID 1
+
+/* Indicate that the 'dimm_dsc' field is valid */
+#define PDSM_DIMM_DSC_VALID 2
+
+/*
+ * Struct exchanged between kernel & ndctl in for PAPR_PDSM_HEALTH
+ * Various flags indicate the health status of the dimm.
+ *
+ * extension_flags : Any extension fields present in the struct.
+ * 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. One of PAPR_PDSM_DIMM_XXXX
+ * dimm_fuel_gauge : Life remaining of DIMM as a percentage from 0-100
+ */
+struct nd_papr_pdsm_health {
+ union {
+ struct {
+ __u32 extension_flags;
+ __u8 dimm_unarmed;
+ __u8 dimm_bad_shutdown;
+ __u8 dimm_bad_restore;
+ __u8 dimm_scrubbed;
+ __u8 dimm_locked;
+ __u8 dimm_encrypted;
+ __u16 dimm_health;
+
+ /* Extension flag PDSM_DIMM_HEALTH_RUN_GAUGE_VALID */
+ __u16 dimm_fuel_gauge;
+
+ /* Extension flag PDSM_DIMM_DSC_VALID */
+ __u64 dimm_dsc;
+ };
+ __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
+ };
+};
+
+/* Flags for injecting specific smart errors */
+#define PDSM_SMART_INJECT_HEALTH_FATAL (1 << 0)
+#define PDSM_SMART_INJECT_BAD_SHUTDOWN (1 << 1)
+
+struct nd_papr_pdsm_smart_inject {
+ union {
+ struct {
+ /* One or more of PDSM_SMART_INJECT_ */
+ __u32 flags;
+ __u8 fatal_enable;
+ __u8 unsafe_shutdown_enable;
+ };
+ __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
+ };
+};
+
+/*
+ * Methods to be embedded in ND_CMD_CALL request. These are sent to the kernel
+ * via 'nd_cmd_pkg.nd_command' member of the ioctl struct
+ */
+enum papr_pdsm {
+ PAPR_PDSM_MIN = 0x0,
+ PAPR_PDSM_HEALTH,
+ PAPR_PDSM_SMART_INJECT,
+ PAPR_PDSM_MAX,
+};
+
+/* Maximal union that can hold all possible payload types */
+union nd_pdsm_payload {
+ struct nd_papr_pdsm_health health;
+ struct nd_papr_pdsm_smart_inject smart_inject;
+ __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
+} __packed;
+
+/*
+ * PDSM-header + payload expected with ND_CMD_CALL ioctl from libnvdimm
+ * Valid member of union 'payload' is identified via 'nd_cmd_pkg.nd_command'
+ * that should always precede this struct when sent to papr_scm via CMD_CALL
+ * interface.
+ */
+struct nd_pkg_pdsm {
+ __s32 cmd_status; /* Out: Sub-cmd status returned back */
+ __u16 reserved[2]; /* Ignored and to be set as '0' */
+ union nd_pdsm_payload payload;
+} __packed;
+
+#endif /* _UAPI_LINUX_PAPR_PDSM_H_ */
diff --git a/tools/testing/nvdimm/test/ndtest.c b/tools/testing/nvdimm/test/ndtest.c
index 45d42cd25e82f..6622e8adbd112 100644
--- a/tools/testing/nvdimm/test/ndtest.c
+++ b/tools/testing/nvdimm/test/ndtest.c
@@ -13,6 +13,8 @@
#include <nd-core.h>
#include <linux/printk.h>
#include <linux/seq_buf.h>
+#include <linux/papr_scm.h>
+#include <uapi/linux/papr_pdsm.h>
#include "../watermark.h"
#include "nfit_test.h"
diff --git a/tools/testing/nvdimm/test/ndtest.h b/tools/testing/nvdimm/test/ndtest.h
index b9b3810213132..e18b3b006fa22 100644
--- a/tools/testing/nvdimm/test/ndtest.h
+++ b/tools/testing/nvdimm/test/ndtest.h
@@ -5,39 +5,6 @@
#include <linux/platform_device.h>
#include <linux/libnvdimm.h>
-/* SCM device is unable to persist memory contents */
-#define PAPR_PMEM_UNARMED (1ULL << (63 - 0))
-/* SCM device failed to persist memory contents */
-#define PAPR_PMEM_SHUTDOWN_DIRTY (1ULL << (63 - 1))
-/* SCM device contents are not persisted from previous IPL */
-#define PAPR_PMEM_EMPTY (1ULL << (63 - 3))
-#define PAPR_PMEM_HEALTH_CRITICAL (1ULL << (63 - 4))
-/* SCM device will be garded off next IPL due to failure */
-#define PAPR_PMEM_HEALTH_FATAL (1ULL << (63 - 5))
-/* SCM contents cannot persist due to current platform health status */
-#define PAPR_PMEM_HEALTH_UNHEALTHY (1ULL << (63 - 6))
-/* SCM device is unable to persist memory contents in certain conditions */
-#define PAPR_PMEM_HEALTH_NON_CRITICAL (1ULL << (63 - 7))
-
-/* Bits status indicators for health bitmap indicating unarmed dimm */
-#define PAPR_PMEM_UNARMED_MASK (PAPR_PMEM_UNARMED | \
- PAPR_PMEM_HEALTH_UNHEALTHY)
-
-#define PAPR_PMEM_SAVE_FAILED (1ULL << (63 - 10))
-
-/* Bits status indicators for health bitmap indicating unflushed dimm */
-#define PAPR_PMEM_BAD_SHUTDOWN_MASK (PAPR_PMEM_SHUTDOWN_DIRTY)
-
-/* Bits status indicators for health bitmap indicating unrestored dimm */
-#define PAPR_PMEM_BAD_RESTORE_MASK (PAPR_PMEM_EMPTY)
-
-/* Bit status indicators for smart event notification */
-#define PAPR_PMEM_SMART_EVENT_MASK (PAPR_PMEM_HEALTH_CRITICAL | \
- PAPR_PMEM_HEALTH_FATAL | \
- PAPR_PMEM_HEALTH_UNHEALTHY)
-
-#define PAPR_PMEM_SAVE_MASK (PAPR_PMEM_SAVE_FAILED)
-
struct ndtest_config;
struct ndtest_priv {
@@ -107,93 +74,6 @@ struct ndtest_region {
u8 range_index;
};
-#define ND_PDSM_PAYLOAD_MAX_SIZE 184
-/*
- * Methods to be embedded in ND_CMD_CALL request. These are sent to the kernel
- * via 'nd_cmd_pkg.nd_command' member of the ioctl struct
- */
-enum papr_pdsm {
- PAPR_PDSM_MIN = 0x0,
- PAPR_PDSM_HEALTH,
- PAPR_PDSM_SMART_INJECT,
- PAPR_PDSM_MAX,
-};
-
-/* Various nvdimm health indicators */
-#define PAPR_PDSM_DIMM_HEALTHY 0
-#define PAPR_PDSM_DIMM_UNHEALTHY 1
-#define PAPR_PDSM_DIMM_CRITICAL 2
-#define PAPR_PDSM_DIMM_FATAL 3
-
-/* struct nd_papr_pdsm_health.extension_flags field flags */
-
-/* Indicate that the 'dimm_fuel_gauge' field is valid */
-#define PDSM_DIMM_HEALTH_RUN_GAUGE_VALID 1
-
-/* Indicate that the 'dimm_dsc' field is valid */
-#define PDSM_DIMM_DSC_VALID 2
-
-/*
- * Struct exchanged between kernel & ndctl in for PAPR_PDSM_HEALTH
- * Various flags indicate the health status of the dimm.
- */
-struct nd_papr_pdsm_health {
- union {
- struct {
- __u32 extension_flags;
- __u8 dimm_unarmed;
- __u8 dimm_bad_shutdown;
- __u8 dimm_bad_restore;
- __u8 dimm_scrubbed;
- __u8 dimm_locked;
- __u8 dimm_encrypted;
- __u16 dimm_health;
-
- /* Extension flag PDSM_DIMM_HEALTH_RUN_GAUGE_VALID */
- __u16 dimm_fuel_gauge;
-
- /* Extension flag PDSM_DIMM_DSC_VALID */
- __u64 dimm_dsc;
- };
- __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
- };
-};
-
-/* Flags for injecting specific smart errors */
-#define PDSM_SMART_INJECT_HEALTH_FATAL (1 << 0)
-#define PDSM_SMART_INJECT_BAD_SHUTDOWN (1 << 1)
-
-struct nd_papr_pdsm_smart_inject {
- union {
- struct {
- /* One or more of PDSM_SMART_INJECT_ */
- __u32 flags;
- __u8 fatal_enable;
- __u8 unsafe_shutdown_enable;
- };
- __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
- };
-};
-
-/* Maximal union that can hold all possible payload types */
-union nd_pdsm_payload {
- struct nd_papr_pdsm_health health;
- struct nd_papr_pdsm_smart_inject smart_inject;
- __u8 buf[ND_PDSM_PAYLOAD_MAX_SIZE];
-} __packed;
-
-/*
- * PDSM-header + payload expected with ND_CMD_CALL ioctl from libnvdimm
- * Valid member of union 'payload' is identified via 'nd_cmd_pkg.nd_command'
- * that should always precede this struct when sent to papr_scm via CMD_CALL
- * interface.
- */
-struct nd_pkg_pdsm {
- __s32 cmd_status; /* Out: Sub-cmd status returned back */
- __u16 reserved[2]; /* Ignored and to be set as '0' */
- union nd_pdsm_payload payload;
-} __packed;
-
struct ndtest_config {
struct ndtest_dimm *dimms;
struct ndtest_region *regions;
^ permalink raw reply related
* [RESEND PATCH v2 0/3] Update crashkernel offset to allow kernel to boot on large config LPARs
From: Sourabh Jain @ 2021-10-18 8:44 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aneesh.kumar, hbathini, mahesh
As the crashkernel reserve memory at 128MB offset in the first memory
block, it leaves less than 128MB memory to accommodate other essential
system resources that need memory reservation in the same block. This
creates kernel boot failure on large config LPARs having core count
greater than 192.
Setting the crashkernel to mid of RMA size which can be 512MB or more
instead of capping it to 128MB by default leaves enough space to allocate
memory to another system resource in the first memory block.
Now keeping the crashkernel at mid of RMA size works fine for the primary
kernel but creates boot failure for the kdump kernel when the crashekernel
reservation start offset crosses 256MB. The reason is, in the early boot
MMU feature of 1T segments support is not detected which restricts the paca
allocation for boot CPU below 256MB. When the crashkernel itself is
starting at 256MB offset, attempt to allocate paca below 256MB leads to the
kdump kernel boot failure.
Moving the detection of segment sizes before identifying the boot CPU
removes the restriction of 256MB limit for boot CPU paca allocation
which allows the kdump kernel to successfully boot and capture vmcore.
While allocating paca for boot CPU we found that there is a small window
during kernel boot where early_radix_enabled returns True even though
the radix is disabled using command-line. This leads to an invalid bolated
size calculation on which paca limit of boot CPU is dependent. Patch 0001
closes that window that by fixing the radix bit in mmu_feature.
changes in v2:
0001: Rename the function name to update_cpu_features() as per
the review comment.
0002: Fixed compilation error reported by kernel test robot.
0003: Added comment and updated commit message
Resending:
forgot to add version tag last time
forgot to CCed PowerPC maintainer
Mahesh Salgaonkar (2):
fixup mmu_features immediately after getting cpu pa features.
Remove 256MB limit restriction for boot cpu paca allocation
Sourabh Jain (1):
powerpc: Set crashkernel offset to mid of RMA region
arch/powerpc/include/asm/book3s/64/mmu.h | 2 ++
arch/powerpc/include/asm/mmu.h | 1 +
arch/powerpc/kernel/prom.c | 8 ++++++++
arch/powerpc/kernel/rtas.c | 4 ++++
arch/powerpc/kexec/core.c | 15 +++++++++++----
arch/powerpc/mm/book3s64/hash_utils.c | 5 ++++-
arch/powerpc/mm/init_64.c | 5 ++++-
7 files changed, 34 insertions(+), 6 deletions(-)
--
2.31.1
^ permalink raw reply
* [RESEND PATCH v2 2/3] Remove 256MB limit restriction for boot cpu paca allocation
From: Sourabh Jain @ 2021-10-18 8:44 UTC (permalink / raw)
To: linuxppc-dev
Cc: Mahesh Salgaonkar, mahesh, Abdul haleem, aneesh.kumar, hbathini
In-Reply-To: <20211018084434.217772-1-sourabhjain@linux.ibm.com>
From: Mahesh Salgaonkar <mahesh@linux.ibm.com>
At the time when we detect and allocate paca for boot cpu, we havn't yet
detected mmu feature of 1T segments support (not until
mmu_early_init_devtree() call). This causes ppc64_bolted_size() to return
256MB as limit forcing boot cpu paca allocation below 256MB always.
This works fine for kdump kernel boot as long as crashkernel reservation is
at offset below 256MB. But when we move kdump offset to 256MB or above,
kdump kernel fails to allocate paca for boot cpu below 256MB and crashes in
allocate_paca().
Moving the detection of segment sizes just before paca allocation for boot
cpu removes this restriction of 256MB limit. This allows kdump kernel to
successfully boot and capture vmcore.
Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Reported-and-tested-by: Abdul haleem <abdhalee@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/book3s/64/mmu.h | 1 +
arch/powerpc/kernel/prom.c | 6 ++++++
arch/powerpc/mm/book3s64/hash_utils.c | 5 ++++-
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
index d60be5051d60..9b05a84313bb 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu.h
@@ -199,6 +199,7 @@ extern int mmu_io_psize;
/* MMU initialization */
void update_cpu_features(void);
void mmu_early_init_devtree(void);
+void hash__early_detect_seg_size(void);
void hash__early_init_devtree(void);
void radix__early_init_devtree(void);
#ifdef CONFIG_PPC_PKEY
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 889c909e4ed4..5da2bfff4dea 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -385,6 +385,12 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
identical_pvr_fixup(node);
init_mmu_slb_size(node);
+#ifdef CONFIG_PPC_BOOK3S_64
+ /* Initialize segment sizes */
+ if (!early_radix_enabled())
+ hash__early_detect_seg_size();
+#endif
+
#ifdef CONFIG_PPC64
if (nthreads == 1)
cur_cpu_spec->cpu_features &= ~CPU_FTR_SMT;
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index c145776d3ae5..ef4fc6bb1b30 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1020,11 +1020,14 @@ static void __init htab_initialize(void)
#undef KB
#undef MB
-void __init hash__early_init_devtree(void)
+void __init hash__early_detect_seg_size(void)
{
/* Initialize segment sizes */
of_scan_flat_dt(htab_dt_scan_seg_sizes, NULL);
+}
+void __init hash__early_init_devtree(void)
+{
/* Initialize page sizes */
htab_scan_page_sizes();
}
--
2.31.1
^ permalink raw reply related
* [RESEND PATCH v2 1/3] fixup mmu_features immediately after getting cpu pa features.
From: Sourabh Jain @ 2021-10-18 8:44 UTC (permalink / raw)
To: linuxppc-dev
Cc: Mahesh Salgaonkar, mahesh, Abdul haleem, aneesh.kumar, hbathini
In-Reply-To: <20211018084434.217772-1-sourabhjain@linux.ibm.com>
From: Mahesh Salgaonkar <mahesh@linux.ibm.com>
On system with radix support available, early_radix_enabled() starts
returning true for a small window (until mmu_early_init_devtree() is
called) even when radix mode disabled on kernel command line. This causes
ppc64_bolted_size() to return ULONG_MAX in HPT mode instead of supported
segment size, during boot cpu paca allocation.
Withi kernel command line = "... disable_radix":
early_init_devtree: <- early_radix_enabled() = false
early_init_dt_scan_cpus: <- early_radix_enabled() = false
...
check_cpu_pa_features: <- early_radix_enabled() = false
... ^ <- early_radix_enabled() = TRUE
allocate_paca: | <- early_radix_enabled() = TRUE
... |
ppc64_bolted_size: | <- early_radix_enabled() = TRUE
if (early_radix_enabled())| <- early_radix_enabled() = TRUE
return ULONG_MAX; |
... |
... | <- early_radix_enabled() = TRUE
... | <- early_radix_enabled() = TRUE
mmu_early_init_devtree() V
... <- early_radix_enabled() = false
So far we have not seen any issue because allocate_paca() takes minimum of
ppc64_bolted_size and rma_size while allocating paca. However it is better
to close this window by fixing up the mmu features as early as possible.
This fixes early_radix_enabled() and ppc64_bolted_size() to return valid
values in radix disable mode. This patch will help subsequent patch to
depend on early_radix_enabled() check while detecting supported segment
size in HPT mode.
Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Reported-and-tested-by: Abdul haleem <abdhalee@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/book3s/64/mmu.h | 1 +
arch/powerpc/include/asm/mmu.h | 1 +
arch/powerpc/kernel/prom.c | 2 ++
arch/powerpc/mm/init_64.c | 5 ++++-
4 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
index c02f42d1031e..d60be5051d60 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu.h
@@ -197,6 +197,7 @@ extern int mmu_vmemmap_psize;
extern int mmu_io_psize;
/* MMU initialization */
+void update_cpu_features(void);
void mmu_early_init_devtree(void);
void hash__early_init_devtree(void);
void radix__early_init_devtree(void);
diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
index 8abe8e42e045..5eb494ea85d7 100644
--- a/arch/powerpc/include/asm/mmu.h
+++ b/arch/powerpc/include/asm/mmu.h
@@ -401,6 +401,7 @@ extern void early_init_mmu(void);
extern void early_init_mmu_secondary(void);
extern void setup_initial_memory_limit(phys_addr_t first_memblock_base,
phys_addr_t first_memblock_size);
+static inline void update_cpu_features(void) { }
static inline void mmu_early_init_devtree(void) { }
static inline void pkey_early_init_devtree(void) {}
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 2e67588f6f6e..889c909e4ed4 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -380,6 +380,8 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
check_cpu_pa_features(node);
}
+ /* Update cpu features based on kernel command line */
+ update_cpu_features();
identical_pvr_fixup(node);
init_mmu_slb_size(node);
diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
index 386be136026e..19680b42898f 100644
--- a/arch/powerpc/mm/init_64.c
+++ b/arch/powerpc/mm/init_64.c
@@ -437,12 +437,15 @@ static void __init early_check_vec5(void)
}
}
-void __init mmu_early_init_devtree(void)
+void __init update_cpu_features(void)
{
/* Disable radix mode based on kernel command line. */
if (disable_radix)
cur_cpu_spec->mmu_features &= ~MMU_FTR_TYPE_RADIX;
+}
+void __init mmu_early_init_devtree(void)
+{
/*
* Check /chosen/ibm,architecture-vec-5 if running as a guest.
* When running bare-metal, we can use radix if we like
--
2.31.1
^ permalink raw reply related
* [RESEND PATCH v2 3/3] powerpc: Set crashkernel offset to mid of RMA region
From: Sourabh Jain @ 2021-10-18 8:44 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Abdul haleem, aneesh.kumar, hbathini, mahesh
In-Reply-To: <20211018084434.217772-1-sourabhjain@linux.ibm.com>
On large config LPARs (having 192 and more cores), Linux fails to boot
due to insufficient memory in the first memblock. It is due to the
memory reservation for the crash kernel which starts at 128MB offset of
the first memblock. This memory reservation for the crash kernel doesn't
leave enough space in the first memblock to accommodate other essential
system resources.
The crash kernel start address was set to 128MB offset by default to
ensure that the crash kernel get some memory below the RMA region which
is used to be of size 256MB. But given that the RMA region size can be
512MB or more, setting the crash kernel offset to mid of RMA size will
leave enough space for kernel to allocate memory for other system
resources.
Since the above crash kernel offset change is only applicable to the LPAR
platform, the LPAR feature detection is pushed before the crash kernel
reservation. The rest of LPAR specific initialization will still
be done during pseries_probe_fw_features as usual.
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Reported-and-tested-by: Abdul haleem <abdhalee@linux.vnet.ibm.com>
---
arch/powerpc/kernel/rtas.c | 4 ++++
arch/powerpc/kexec/core.c | 15 +++++++++++----
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index ff80bbad22a5..a49137727370 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1235,6 +1235,10 @@ int __init early_init_dt_scan_rtas(unsigned long node,
entryp = of_get_flat_dt_prop(node, "linux,rtas-entry", NULL);
sizep = of_get_flat_dt_prop(node, "rtas-size", NULL);
+ /* need this feature to decide the crashkernel offset */
+ if (of_get_flat_dt_prop(node, "ibm,hypertas-functions", NULL))
+ powerpc_firmware_features |= FW_FEATURE_LPAR;
+
if (basep && entryp && sizep) {
rtas.base = *basep;
rtas.entry = *entryp;
diff --git a/arch/powerpc/kexec/core.c b/arch/powerpc/kexec/core.c
index 48525e8b5730..71b1bfdadd76 100644
--- a/arch/powerpc/kexec/core.c
+++ b/arch/powerpc/kexec/core.c
@@ -147,11 +147,18 @@ void __init reserve_crashkernel(void)
if (!crashk_res.start) {
#ifdef CONFIG_PPC64
/*
- * On 64bit we split the RMO in half but cap it at half of
- * a small SLB (128MB) since the crash kernel needs to place
- * itself and some stacks to be in the first segment.
+ * On the LPAR platform place the crash kernel to mid of
+ * RMA size (512MB or more) to ensure the crash kernel
+ * gets enough space to place itself and some stack to be
+ * in the first segment. At the same time normal kernel
+ * also get enough space to allocate memory for essential
+ * system resource in the first segment. Keep the crash
+ * kernel starts at 128MB offset on other platforms.
*/
- crashk_res.start = min(0x8000000ULL, (ppc64_rma_size / 2));
+ if (firmware_has_feature(FW_FEATURE_LPAR))
+ crashk_res.start = ppc64_rma_size / 2;
+ else
+ crashk_res.start = min(0x8000000ULL, (ppc64_rma_size / 2));
#else
crashk_res.start = KDUMP_KERNELBASE;
#endif
--
2.31.1
^ permalink raw reply related
* Re: [PATCH v3 07/12] asm-generic: Define 'func_desc_t' to commonly describe function descriptors
From: Nicholas Piggin @ 2021-10-18 9:16 UTC (permalink / raw)
To: Andrew Morton, Arnd Bergmann, Benjamin Herrenschmidt,
Christophe Leroy, Helge Deller, Greg Kroah-Hartman,
James E.J. Bottomley, Kees Cook, Michael Ellerman, Paul Mackerras
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <802b3ff9-8ada-b45b-2b69-b6a23f0c3664@csgroup.eu>
Excerpts from Christophe Leroy's message of October 18, 2021 5:07 pm:
>
>
> Le 18/10/2021 à 08:29, Nicholas Piggin a écrit :
>> Excerpts from Christophe Leroy's message of October 17, 2021 10:38 pm:
>>> We have three architectures using function descriptors, each with its
>>> own type and name.
>>>
>>> Add a common typedef that can be used in generic code.
>>>
>>> Also add a stub typedef for architecture without function descriptors,
>>> to avoid a forest of #ifdefs.
>>>
>>> It replaces the similar 'func_desc_t' previously defined in
>>> arch/powerpc/kernel/module_64.c
>>>
>>> Reviewed-by: Kees Cook <keescook@chromium.org>
>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>> ---
>>
>> [...]
>>
>>> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
>>> index a918388d9bf6..33b51efe3a24 100644
>>> --- a/include/asm-generic/sections.h
>>> +++ b/include/asm-generic/sections.h
>>> @@ -63,6 +63,9 @@ extern __visible const void __nosave_begin, __nosave_end;
>>> #else
>>> #define dereference_function_descriptor(p) ((void *)(p))
>>> #define dereference_kernel_function_descriptor(p) ((void *)(p))
>>> +typedef struct {
>>> + unsigned long addr;
>>> +} func_desc_t;
>>> #endif
>>>
>>
>> I think that deserves a comment. If it's just to allow ifdef to be
>> avoided, I guess that's okay with a comment. Would be nice if you could
>> cause it to generate a link time error if it was ever used like
>> undefined functions, but I guess you can't. It's not a necessity though.
>>
>
> I tried to explain it in the commit message, but I can add a comment
> here in addition for sure.
Thanks.
>
> By the way, it IS used in powerpc's module_64.c:
Ah yes of course. I guess the point is function descriptors don't exist
so it should not be used (in general). powerpc module code knows what it
is doing, I guess it's okay for it to use it.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v3 0/4] Add mem_hops field in perf_mem_data_src structure
From: Peter Zijlstra @ 2021-10-18 9:48 UTC (permalink / raw)
To: Michael Ellerman
Cc: mark.rutland, atrajeev, ak, daniel, rnsastry, alexander.shishkin,
Kajol Jain, linux-kernel, acme, ast, linux-perf-users, yao.jin,
mingo, paulus, maddy, jolsa, namhyung, songliubraving,
linuxppc-dev, kan.liang
In-Reply-To: <87pms3c7w5.fsf@mpe.ellerman.id.au>
On Mon, Oct 18, 2021 at 02:46:18PM +1100, Michael Ellerman wrote:
> Peter Zijlstra <peterz@infradead.org> writes:
> > On Wed, Oct 06, 2021 at 07:36:50PM +0530, Kajol Jain wrote:
> >
> >> Kajol Jain (4):
> >> perf: Add comment about current state of PERF_MEM_LVL_* namespace and
> >> remove an extra line
> >> perf: Add mem_hops field in perf_mem_data_src structure
> >> tools/perf: Add mem_hops field in perf_mem_data_src structure
> >> powerpc/perf: Fix data source encodings for L2.1 and L3.1 accesses
> >>
> >> arch/powerpc/perf/isa207-common.c | 26 +++++++++++++++++++++-----
> >> arch/powerpc/perf/isa207-common.h | 2 ++
> >> include/uapi/linux/perf_event.h | 19 ++++++++++++++++---
> >> tools/include/uapi/linux/perf_event.h | 19 ++++++++++++++++---
> >> tools/perf/util/mem-events.c | 20 ++++++++++++++++++--
> >> 5 files changed, 73 insertions(+), 13 deletions(-)
> >
> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> >
> > How do we want this routed? Shall I take it, or does Michael want it in
> > the Power tree?
>
> It's mostly non-powerpc, so I think you should take it.
>
> There's a slim chance we could end up with a conflict in the powerpc
> part, but that's no big deal.
Sure thing, into perf/core it goes. Thanks!
^ permalink raw reply
* Re: [PATCH] tracing: Have all levels of checks prevent recursion
From: Petr Mladek @ 2021-10-18 10:19 UTC (permalink / raw)
To: Steven Rostedt
Cc: 王贇, Peter Zijlstra (Intel), Paul Walmsley,
James E.J. Bottomley, Paul Mackerras, Jisheng Zhang,
H. Peter Anvin, live-patching, linux-riscv, Miroslav Benes,
Joe Lawrence, Helge Deller, x86, linux-csky, Ingo Molnar,
Albert Ou, Jiri Kosina, Nicholas Piggin, Borislav Petkov,
Josh Poimboeuf, Thomas Gleixner, linux-parisc, LKML,
Palmer Dabbelt, Masami Hiramatsu, Guo Ren, Colin Ian King,
linuxppc-dev
In-Reply-To: <20211015110035.14813389@gandalf.local.home>
On Fri 2021-10-15 11:00:35, Steven Rostedt wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> While writing an email explaining the "bit = 0" logic for a discussion on
> making ftrace_test_recursion_trylock() disable preemption, I discovered a
> path that makes the "not do the logic if bit is zero" unsafe.
>
> Since we want to encourage architectures to implement all ftrace features,
> having them slow down due to this extra logic may encourage the
> maintainers to update to the latest ftrace features. And because this
> logic is only safe for them, remove it completely.
>
> [*] There is on layer of recursion that is allowed, and that is to allow
> for the transition between interrupt context (normal -> softirq ->
> irq -> NMI), because a trace may occur before the context update is
> visible to the trace recursion logic.
>
> diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h
> index a9f9c5714e65..168fdf07419a 100644
> --- a/include/linux/trace_recursion.h
> +++ b/include/linux/trace_recursion.h
> @@ -165,40 +147,29 @@ static __always_inline int trace_test_and_set_recursion(unsigned long ip, unsign
> unsigned int val = READ_ONCE(current->trace_recursion);
> int bit;
>
> - /* A previous recursion check was made */
> - if ((val & TRACE_CONTEXT_MASK) > max)
> - return 0;
@max parameter is no longer used.
> -
> bit = trace_get_context_bit() + start;
> if (unlikely(val & (1 << bit))) {
> /*
> * It could be that preempt_count has not been updated during
> * a switch between contexts. Allow for a single recursion.
> */
> - bit = TRACE_TRANSITION_BIT;
> + bit = TRACE_CTX_TRANSITION + start;
I just want to be sure that I understand it correctly.
The transition bit is the same for all contexts. It will allow one
recursion only in one context.
IMHO, the same problem (not-yet-updated preempt_count) might happen
in any transition between contexts: normal -> soft IRQ -> IRQ -> NMI.
Well, I am not sure what exacly it means "preempt_count has not been
updated during a switch between contexts."
Is it that a function in the interrupt entry code is traced before
preempt_count is updated?
Or that an interrupt entry is interrupted by a higher level
interrupt, e.g. IRQ entry code interrupted by NMI?
I guess that it is the first case. It would mean that the above
function allows one mistake (one traced funtion in an interrupt entry
code before the entry code updates preempt_count).
Do I get it correctly?
Is this what we want?
Could we please update the comment? I mean to say if it is a race
or if we trace a function that should not get traced.
> if (val & (1 << bit)) {
> do_ftrace_record_recursion(ip, pip);
> return -1;
> }
> - } else {
> - /* Normal check passed, clear the transition to allow it again */
> - val &= ~(1 << TRACE_TRANSITION_BIT);
> }
>
> val |= 1 << bit;
> current->trace_recursion = val;
> barrier();
>
> - return bit + 1;
> + return bit;
> }
>
> static __always_inline void trace_clear_recursion(int bit)
> {
> - if (!bit)
> - return;
> -
> barrier();
> - bit--;
> trace_recursion_clear(bit);
> }
Otherwise, the change looks great. It simplifies that logic a lot.
I think that I start understanding it ;-)
Best Regards,
Petr
^ permalink raw reply
* [V4 0/2] tools/perf: Add instruction and data address registers to extended regs in powerpc
From: Athira Rajeev @ 2021-10-18 11:49 UTC (permalink / raw)
To: acme, jolsa; +Cc: maddy, rnsastry, linux-perf-users, kjain, linuxppc-dev
Patch set adds PMU registers namely Sampled Instruction Address Register
(SIAR) and Sampled Data Address Register (SDAR) as part of extended regs
in PowerPC. These registers provides the instruction/data address and
adding these to extended regs helps in debug purposes.
Patch 1/2 refactors the existing macro definition of
PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 to make it more
readable.
Patch 2/2 includes perf tools side changes to add the SPRs to
sample_reg_mask to use with -I? option.
Changelog:
Change from v3 -> v4:
- Spilt tools side patches separately since kernel side
changes are in powerpc/next. There is no code wise changes
from v3.
Link to previous version:
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=265811&state=*
Kernel patches are taken to powerpc/next:
[1/4] powerpc/perf: Refactor the code definition of perf reg extended mask
https://git.kernel.org/powerpc/c/02b182e67482d9167a13a0ff19b55037b70b21ad
[3/4] powerpc/perf: Expose instruction and data address registers as part of extended regs
https://git.kernel.org/powerpc/c/29908bbf7b8960d261dfdd428bbaa656275e80f3
Change from v2 -> v3:
Addressed review comments from Michael Ellerman
- Fixed the macro definition to use "unsigned long long"
which otherwise will cause build error with perf on
32-bit.
- Added Reviewed-by from Daniel Axtens for patch3.
Change from v1 -> v2:
Addressed review comments from Michael Ellerman
- Refactored the perf reg extended mask value macros for
PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 to
make it more readable. Also moved PERF_REG_EXTENDED_MAX
along with enum definition similar to PERF_REG_POWERPC_MAX.
Athira Rajeev (2):
tools/perf: Refactor the code definition of perf reg extended mask in
tools side header file
tools/perf: Add perf tools support to expose instruction and data
address registers as part of extended regs
.../arch/powerpc/include/uapi/asm/perf_regs.h | 28 ++++++++++++-------
tools/perf/arch/powerpc/include/perf_regs.h | 2 ++
tools/perf/arch/powerpc/util/perf_regs.c | 2 ++
3 files changed, 22 insertions(+), 10 deletions(-)
--
2.33.0
^ permalink raw reply
* [V4 2/2] tools/perf: Add perf tools support to expose instruction and data address registers as part of extended regs
From: Athira Rajeev @ 2021-10-18 11:49 UTC (permalink / raw)
To: acme, jolsa; +Cc: maddy, rnsastry, linux-perf-users, kjain, linuxppc-dev
In-Reply-To: <20211018114948.16830-1-atrajeev@linux.vnet.ibm.com>
Patch enables presenting of Sampled Instruction Address Register (SIAR)
and Sampled Data Address Register (SDAR) SPRs as part of extended regsiters
for perf tool. Add these SPR's to sample_reg_mask in the tool side (to use
with -I? option).
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Reviewed-by: Kajol Jain<kjain@linux.ibm.com>
---
tools/arch/powerpc/include/uapi/asm/perf_regs.h | 11 +++++++----
tools/perf/arch/powerpc/include/perf_regs.h | 2 ++
tools/perf/arch/powerpc/util/perf_regs.c | 2 ++
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/tools/arch/powerpc/include/uapi/asm/perf_regs.h b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
index 085094553f3b..749a2e3af89e 100644
--- a/tools/arch/powerpc/include/uapi/asm/perf_regs.h
+++ b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
@@ -61,17 +61,19 @@ enum perf_event_powerpc_regs {
PERF_REG_POWERPC_PMC4,
PERF_REG_POWERPC_PMC5,
PERF_REG_POWERPC_PMC6,
+ PERF_REG_POWERPC_SDAR,
+ PERF_REG_POWERPC_SIAR,
/* Max mask value for interrupt regs w/o extended regs */
PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1,
/* Max mask value for interrupt regs including extended regs */
- PERF_REG_EXTENDED_MAX = PERF_REG_POWERPC_PMC6 + 1,
+ PERF_REG_EXTENDED_MAX = PERF_REG_POWERPC_SIAR + 1,
};
#define PERF_REG_PMU_MASK ((1ULL << PERF_REG_POWERPC_MAX) - 1)
/*
* PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_300
- * includes 9 SPRS from MMCR0 to PMC6 excluding the
+ * includes 11 SPRS from MMCR0 to SIAR excluding the
* unsupported SPRS MMCR3, SIER2 and SIER3.
*/
#define PERF_REG_PMU_MASK_300 \
@@ -79,11 +81,12 @@ enum perf_event_powerpc_regs {
(1ULL << PERF_REG_POWERPC_MMCR2) | (1ULL << PERF_REG_POWERPC_PMC1) | \
(1ULL << PERF_REG_POWERPC_PMC2) | (1ULL << PERF_REG_POWERPC_PMC3) | \
(1ULL << PERF_REG_POWERPC_PMC4) | (1ULL << PERF_REG_POWERPC_PMC5) | \
- (1ULL << PERF_REG_POWERPC_PMC6))
+ (1ULL << PERF_REG_POWERPC_PMC6) | (1ULL << PERF_REG_POWERPC_SDAR) | \
+ (1ULL << PERF_REG_POWERPC_SIAR))
/*
* PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_31
- * includes 12 SPRs from MMCR0 to PMC6.
+ * includes 14 SPRs from MMCR0 to SIAR.
*/
#define PERF_REG_PMU_MASK_31 \
(PERF_REG_PMU_MASK_300 | (1ULL << PERF_REG_POWERPC_MMCR3) | \
diff --git a/tools/perf/arch/powerpc/include/perf_regs.h b/tools/perf/arch/powerpc/include/perf_regs.h
index 04e5dc07e93f..93339d17acc4 100644
--- a/tools/perf/arch/powerpc/include/perf_regs.h
+++ b/tools/perf/arch/powerpc/include/perf_regs.h
@@ -77,6 +77,8 @@ static const char *reg_names[] = {
[PERF_REG_POWERPC_PMC4] = "pmc4",
[PERF_REG_POWERPC_PMC5] = "pmc5",
[PERF_REG_POWERPC_PMC6] = "pmc6",
+ [PERF_REG_POWERPC_SDAR] = "sdar",
+ [PERF_REG_POWERPC_SIAR] = "siar",
};
static inline const char *__perf_reg_name(int id)
diff --git a/tools/perf/arch/powerpc/util/perf_regs.c b/tools/perf/arch/powerpc/util/perf_regs.c
index 8116a253f91f..8d07a78e742a 100644
--- a/tools/perf/arch/powerpc/util/perf_regs.c
+++ b/tools/perf/arch/powerpc/util/perf_regs.c
@@ -74,6 +74,8 @@ const struct sample_reg sample_reg_masks[] = {
SMPL_REG(pmc4, PERF_REG_POWERPC_PMC4),
SMPL_REG(pmc5, PERF_REG_POWERPC_PMC5),
SMPL_REG(pmc6, PERF_REG_POWERPC_PMC6),
+ SMPL_REG(sdar, PERF_REG_POWERPC_SDAR),
+ SMPL_REG(siar, PERF_REG_POWERPC_SIAR),
SMPL_REG_END
};
--
2.33.0
^ permalink raw reply related
* [V4 1/2] tools/perf: Refactor the code definition of perf reg extended mask in tools side header file
From: Athira Rajeev @ 2021-10-18 11:49 UTC (permalink / raw)
To: acme, jolsa; +Cc: maddy, rnsastry, linux-perf-users, kjain, linuxppc-dev
In-Reply-To: <20211018114948.16830-1-atrajeev@linux.vnet.ibm.com>
PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 defines the mask
value for extended registers. Current definition of these mask values
uses hex constant and does not use registers by name, making it less
readable. Patch refactor the macro values in perf tools side header file
by or'ing together the actual register value constants.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Reviewed-by: Kajol Jain<kjain@linux.ibm.com>
---
.../arch/powerpc/include/uapi/asm/perf_regs.h | 21 ++++++++++++-------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/tools/arch/powerpc/include/uapi/asm/perf_regs.h b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
index 578b3ee86105..085094553f3b 100644
--- a/tools/arch/powerpc/include/uapi/asm/perf_regs.h
+++ b/tools/arch/powerpc/include/uapi/asm/perf_regs.h
@@ -61,27 +61,32 @@ enum perf_event_powerpc_regs {
PERF_REG_POWERPC_PMC4,
PERF_REG_POWERPC_PMC5,
PERF_REG_POWERPC_PMC6,
- /* Max regs without the extended regs */
+ /* Max mask value for interrupt regs w/o extended regs */
PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1,
+ /* Max mask value for interrupt regs including extended regs */
+ PERF_REG_EXTENDED_MAX = PERF_REG_POWERPC_PMC6 + 1,
};
#define PERF_REG_PMU_MASK ((1ULL << PERF_REG_POWERPC_MAX) - 1)
-/* Exclude MMCR3, SIER2, SIER3 for CPU_FTR_ARCH_300 */
-#define PERF_EXCLUDE_REG_EXT_300 (7ULL << PERF_REG_POWERPC_MMCR3)
-
/*
* PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_300
* includes 9 SPRS from MMCR0 to PMC6 excluding the
- * unsupported SPRS in PERF_EXCLUDE_REG_EXT_300.
+ * unsupported SPRS MMCR3, SIER2 and SIER3.
*/
-#define PERF_REG_PMU_MASK_300 ((0xfffULL << PERF_REG_POWERPC_MMCR0) - PERF_EXCLUDE_REG_EXT_300)
+#define PERF_REG_PMU_MASK_300 \
+ ((1ULL << PERF_REG_POWERPC_MMCR0) | (1ULL << PERF_REG_POWERPC_MMCR1) | \
+ (1ULL << PERF_REG_POWERPC_MMCR2) | (1ULL << PERF_REG_POWERPC_PMC1) | \
+ (1ULL << PERF_REG_POWERPC_PMC2) | (1ULL << PERF_REG_POWERPC_PMC3) | \
+ (1ULL << PERF_REG_POWERPC_PMC4) | (1ULL << PERF_REG_POWERPC_PMC5) | \
+ (1ULL << PERF_REG_POWERPC_PMC6))
/*
* PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_31
* includes 12 SPRs from MMCR0 to PMC6.
*/
-#define PERF_REG_PMU_MASK_31 (0xfffULL << PERF_REG_POWERPC_MMCR0)
+#define PERF_REG_PMU_MASK_31 \
+ (PERF_REG_PMU_MASK_300 | (1ULL << PERF_REG_POWERPC_MMCR3) | \
+ (1ULL << PERF_REG_POWERPC_SIER2) | (1ULL << PERF_REG_POWERPC_SIER3))
-#define PERF_REG_EXTENDED_MAX (PERF_REG_POWERPC_PMC6 + 1)
#endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */
--
2.33.0
^ permalink raw reply related
* Re: [PATCH 01/12] of: Add of_get_cpu_hwid() to read hardware ID from CPU nodes
From: Sudeep Holla @ 2021-10-18 13:26 UTC (permalink / raw)
To: Rob Herring
Cc: Rich Felker, Rafael J. Wysocki, linux-kernel, Guo Ren,
H. Peter Anvin, linux-riscv, Will Deacon, Thomas Gleixner,
Jonas Bonn, Florian Fainelli, Yoshinori Sato, linux-sh, x86,
Russell King, linux-csky, Ingo Molnar, bcm-kernel-feedback-list,
Catalin Marinas, Palmer Dabbelt, devicetree, Albert Ou, Ray Jui,
Stefan Kristiansson, openrisc, Borislav Petkov, Paul Walmsley,
Stafford Horne, linux-arm-kernel, Scott Branden,
Greg Kroah-Hartman, Frank Rowand, James Morse, Sudeep Holla,
Paul Mackerras, linuxppc-dev
In-Reply-To: <20211006164332.1981454-2-robh@kernel.org>
On Wed, Oct 06, 2021 at 11:43:21AM -0500, Rob Herring wrote:
> There are various open coded implementions parsing the CPU node 'reg'
> property which contains the CPU's hardware ID. Introduce a new function,
> of_get_cpu_hwid(), to read the hardware ID.
>
> All the callers should be DT only code, so no need for an empty
> function.
>
Thanks for doing this. I postponed and forgot about this though I had
planned for this when I touched code around this.
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH 04/12] arm64: Use of_get_cpu_hwid()
From: Sudeep Holla @ 2021-10-18 13:27 UTC (permalink / raw)
To: Rob Herring
Cc: Rich Felker, Rafael J. Wysocki, linux-kernel, Guo Ren,
H. Peter Anvin, linux-riscv, Will Deacon, Thomas Gleixner,
Jonas Bonn, Florian Fainelli, Yoshinori Sato, linux-sh, x86,
Russell King, linux-csky, Ingo Molnar, bcm-kernel-feedback-list,
Catalin Marinas, Palmer Dabbelt, devicetree, Albert Ou, Ray Jui,
Stefan Kristiansson, openrisc, Borislav Petkov, Paul Walmsley,
Stafford Horne, linux-arm-kernel, Scott Branden,
Greg Kroah-Hartman, Frank Rowand, James Morse, Sudeep Holla,
Paul Mackerras, linuxppc-dev
In-Reply-To: <20211006164332.1981454-5-robh@kernel.org>
On Wed, Oct 06, 2021 at 11:43:24AM -0500, Rob Herring wrote:
> Replace the open coded parsing of CPU nodes' 'reg' property with
> of_get_cpu_hwid().
>
> This change drops an error message for missing 'reg' property, but that
> should not be necessary as the DT tools will ensure 'reg' is present.
>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH 11/12] cacheinfo: Allow for >32-bit cache 'id'
From: Sudeep Holla @ 2021-10-18 13:30 UTC (permalink / raw)
To: Rob Herring
Cc: Rich Felker, Rafael J. Wysocki, linux-kernel, Guo Ren,
H. Peter Anvin, linux-riscv, Will Deacon, Thomas Gleixner,
Jonas Bonn, Florian Fainelli, Yoshinori Sato, linux-sh, x86,
Russell King, linux-csky, Ingo Molnar, bcm-kernel-feedback-list,
Catalin Marinas, Palmer Dabbelt, devicetree, Albert Ou, Ray Jui,
Stefan Kristiansson, openrisc, Borislav Petkov, Paul Walmsley,
Stafford Horne, linux-arm-kernel, Scott Branden,
Greg Kroah-Hartman, Frank Rowand, James Morse, Sudeep Holla,
Paul Mackerras, linuxppc-dev
In-Reply-To: <20211006164332.1981454-12-robh@kernel.org>
On Wed, Oct 06, 2021 at 11:43:31AM -0500, Rob Herring wrote:
> In preparation to set the cache 'id' based on the CPU h/w ids, allow for
> 64-bit bit 'id' value. The only case that needs this is arm64, so
> unsigned long is sufficient.
>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH 12/12] cacheinfo: Set cache 'id' based on DT data
From: Sudeep Holla @ 2021-10-18 13:31 UTC (permalink / raw)
To: Rob Herring
Cc: Rich Felker, Rafael J. Wysocki, linux-kernel, Guo Ren,
H. Peter Anvin, linux-riscv, Will Deacon, Thomas Gleixner,
Jonas Bonn, Florian Fainelli, Yoshinori Sato, linux-sh, x86,
Russell King, linux-csky, Ingo Molnar, bcm-kernel-feedback-list,
Catalin Marinas, Palmer Dabbelt, devicetree, Albert Ou, Ray Jui,
Stefan Kristiansson, openrisc, Borislav Petkov, Paul Walmsley,
Stafford Horne, linux-arm-kernel, Scott Branden,
Greg Kroah-Hartman, Frank Rowand, James Morse, Sudeep Holla,
Paul Mackerras, linuxppc-dev
In-Reply-To: <20211006164332.1981454-13-robh@kernel.org>
On Wed, Oct 06, 2021 at 11:43:32AM -0500, Rob Herring wrote:
> Use the minimum CPU h/w id of the CPUs associated with the cache for the
> cache 'id'. This will provide a stable id value for a given system. As
> we need to check all possible CPUs, we can't use the shared_cpu_map
> which is just online CPUs. As there's not a cache to CPUs mapping in DT,
> we have to walk all CPU nodes and then walk cache levels.
>
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH] tracing: Have all levels of checks prevent recursion
From: Steven Rostedt @ 2021-10-18 13:50 UTC (permalink / raw)
To: Petr Mladek
Cc: 王贇, Peter Zijlstra (Intel), Paul Walmsley,
James E.J. Bottomley, Paul Mackerras, Jisheng Zhang,
H. Peter Anvin, live-patching, linux-riscv, Miroslav Benes,
Joe Lawrence, Helge Deller, x86, linux-csky, Ingo Molnar,
Albert Ou, Jiri Kosina, Nicholas Piggin, Borislav Petkov,
Josh Poimboeuf, Thomas Gleixner, linux-parisc, LKML,
Palmer Dabbelt, Masami Hiramatsu, Guo Ren, Colin Ian King,
linuxppc-dev
In-Reply-To: <YW1KKCFallDG+E01@alley>
On Mon, 18 Oct 2021 12:19:20 +0200
Petr Mladek <pmladek@suse.com> wrote:
> On Fri 2021-10-15 11:00:35, Steven Rostedt wrote:
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> >
> > While writing an email explaining the "bit = 0" logic for a discussion on
> > making ftrace_test_recursion_trylock() disable preemption, I discovered a
> > path that makes the "not do the logic if bit is zero" unsafe.
> >
> > Since we want to encourage architectures to implement all ftrace features,
> > having them slow down due to this extra logic may encourage the
> > maintainers to update to the latest ftrace features. And because this
> > logic is only safe for them, remove it completely.
> >
> > [*] There is on layer of recursion that is allowed, and that is to allow
> > for the transition between interrupt context (normal -> softirq ->
> > irq -> NMI), because a trace may occur before the context update is
> > visible to the trace recursion logic.
> >
> > diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h
> > index a9f9c5714e65..168fdf07419a 100644
> > --- a/include/linux/trace_recursion.h
> > +++ b/include/linux/trace_recursion.h
> > @@ -165,40 +147,29 @@ static __always_inline int trace_test_and_set_recursion(unsigned long ip, unsign
> > unsigned int val = READ_ONCE(current->trace_recursion);
> > int bit;
> >
> > - /* A previous recursion check was made */
> > - if ((val & TRACE_CONTEXT_MASK) > max)
> > - return 0;
>
> @max parameter is no longer used.
Thanks for noticing!
>
> > -
> > bit = trace_get_context_bit() + start;
> > if (unlikely(val & (1 << bit))) {
> > /*
> > * It could be that preempt_count has not been updated during
> > * a switch between contexts. Allow for a single recursion.
> > */
> > - bit = TRACE_TRANSITION_BIT;
> > + bit = TRACE_CTX_TRANSITION + start;
>
> I just want to be sure that I understand it correctly.
>
> The transition bit is the same for all contexts. It will allow one
> recursion only in one context.
Right.
>
> IMHO, the same problem (not-yet-updated preempt_count) might happen
> in any transition between contexts: normal -> soft IRQ -> IRQ -> NMI.
Yes, and then we will drop the event if it happens twice, otherwise, we
will need to have a 4 layer transition bit mask, and allow 4 recursions,
which is more than I want to allow.
>
> Well, I am not sure what exacly it means "preempt_count has not been
> updated during a switch between contexts."
>
> Is it that a function in the interrupt entry code is traced before
> preempt_count is updated?
>
> Or that an interrupt entry is interrupted by a higher level
> interrupt, e.g. IRQ entry code interrupted by NMI?
Both actually ;-)
There are places that can trigger a trace between the time the interrupt is
triggered, and the time the preempt_count updates the interrupt context it
is in. Thus the tracer will still think it is in the previous context. But
that is OK, unless, that interrupt happened while the previous context was
in the middle of tracing:
trace() {
context = get_context(preempt_count);
test_and_set_bit(context)
<<--- interrupt --->>>
trace() {
context = get_context(preempt_count);
test_and_set_bit(context); <-- detects recursion!
}
update_preempt_count(irq_context);
By allowing a single recursion, it still does the above trace.
Yes, if an NMI happens during that first interrupt trace, and it too traces
before the preempt_count is updated, it will detect it as a recursion.
But this can only happen for that one trace. After the trace returns, the
transition bit is cleared, and the tracing that happens in the rest of the
interrupt is using the proper context. Thus, to drop due to needing two
transition bits, it would require that an interrupt triggered during a
trace, and while it was tracing before the preempt_count update, it too
needed to be interrupted by something (NMI) and that needs to trace before
the preempt_count update.
Although, I think we were able to remove all the NMI tracing before the
update, there's a game of whack-a-mole to handle the interrupt cases.
>
>
> I guess that it is the first case. It would mean that the above
> function allows one mistake (one traced funtion in an interrupt entry
> code before the entry code updates preempt_count).
>
> Do I get it correctly?
> Is this what we want?
Pretty much, which my above explanation to hopefully fill in the details.
>
>
> Could we please update the comment? I mean to say if it is a race
> or if we trace a function that should not get traced.
Comments can always use some loving ;-)
>
> > if (val & (1 << bit)) {
> > do_ftrace_record_recursion(ip, pip);
> > return -1;
> > }
> > - } else {
> > - /* Normal check passed, clear the transition to allow it again */
> > - val &= ~(1 << TRACE_TRANSITION_BIT);
> > }
> >
> > val |= 1 << bit;
> > current->trace_recursion = val;
> > barrier();
> >
> > - return bit + 1;
> > + return bit;
> > }
> >
> > static __always_inline void trace_clear_recursion(int bit)
> > {
> > - if (!bit)
> > - return;
> > -
> > barrier();
> > - bit--;
> > trace_recursion_clear(bit);
> > }
>
> Otherwise, the change looks great. It simplifies that logic a lot.
> I think that I start understanding it ;-)
Awesome. I'll make some more updates.
-- Steve
^ permalink raw reply
* Re: [PATCH] tracing: Have all levels of checks prevent recursion
From: Petr Mladek @ 2021-10-18 15:09 UTC (permalink / raw)
To: Steven Rostedt
Cc: 王贇, Peter Zijlstra (Intel), Paul Walmsley,
James E.J. Bottomley, Paul Mackerras, Jisheng Zhang,
H. Peter Anvin, live-patching, linux-riscv, Miroslav Benes,
Joe Lawrence, Helge Deller, x86, linux-csky, Ingo Molnar,
Albert Ou, Jiri Kosina, Nicholas Piggin, Borislav Petkov,
Josh Poimboeuf, Thomas Gleixner, linux-parisc, LKML,
Palmer Dabbelt, Masami Hiramatsu, Guo Ren, Colin Ian King,
linuxppc-dev
In-Reply-To: <20211018095027.52a23ff0@gandalf.local.home>
On Mon 2021-10-18 09:50:27, Steven Rostedt wrote:
> On Mon, 18 Oct 2021 12:19:20 +0200
> Petr Mladek <pmladek@suse.com> wrote:
>
> > On Fri 2021-10-15 11:00:35, Steven Rostedt wrote:
> > > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> > >
> > > While writing an email explaining the "bit = 0" logic for a discussion on
> > > making ftrace_test_recursion_trylock() disable preemption, I discovered a
> > > path that makes the "not do the logic if bit is zero" unsafe.
> > >
> > > Since we want to encourage architectures to implement all ftrace features,
> > > having them slow down due to this extra logic may encourage the
> > > maintainers to update to the latest ftrace features. And because this
> > > logic is only safe for them, remove it completely.
> > >
> > > [*] There is on layer of recursion that is allowed, and that is to allow
> > > for the transition between interrupt context (normal -> softirq ->
> > > irq -> NMI), because a trace may occur before the context update is
> > > visible to the trace recursion logic.
> > >
> > > diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h
> > > index a9f9c5714e65..168fdf07419a 100644
> > > --- a/include/linux/trace_recursion.h
> > > +++ b/include/linux/trace_recursion.h
> > > @@ -165,40 +147,29 @@ static __always_inline int trace_test_and_set_recursion(unsigned long ip, unsign
> > > unsigned int val = READ_ONCE(current->trace_recursion);
> > > int bit;
> > >
> > > - /* A previous recursion check was made */
> > > - if ((val & TRACE_CONTEXT_MASK) > max)
> > > - return 0;
> >
> > @max parameter is no longer used.
>
> Thanks for noticing!
>
> >
> > > -
> > > bit = trace_get_context_bit() + start;
> > > if (unlikely(val & (1 << bit))) {
> > > /*
> > > * It could be that preempt_count has not been updated during
> > > * a switch between contexts. Allow for a single recursion.
> > > */
> > > - bit = TRACE_TRANSITION_BIT;
> > > + bit = TRACE_CTX_TRANSITION + start;
> >
> > I just want to be sure that I understand it correctly.
> >
> > The transition bit is the same for all contexts. It will allow one
> > recursion only in one context.
>
> Right.
>
> >
> > IMHO, the same problem (not-yet-updated preempt_count) might happen
> > in any transition between contexts: normal -> soft IRQ -> IRQ -> NMI.
>
> Yes, and then we will drop the event if it happens twice, otherwise, we
> will need to have a 4 layer transition bit mask, and allow 4 recursions,
> which is more than I want to allow.
Fair enough. I am still not sure if we want to allow the recursion
at all, see below.
>
> >
> > Well, I am not sure what exacly it means "preempt_count has not been
> > updated during a switch between contexts."
> >
> > Is it that a function in the interrupt entry code is traced before
> > preempt_count is updated?
> >
> > Or that an interrupt entry is interrupted by a higher level
> > interrupt, e.g. IRQ entry code interrupted by NMI?
>
> Both actually ;-)
I see. But only one is a problem. See below.
> There are places that can trigger a trace between the time the interrupt is
> triggered, and the time the preempt_count updates the interrupt context it
> is in.
By other words, these locations trace a function that is called
between entering the context and updating preempt_count.
> Thus the tracer will still think it is in the previous context. But
> that is OK, unless, that interrupt happened while the previous context was
> in the middle of tracing:
My understanding is that this is _not_ OK. Peter Zijlstra fixed the
generic entry and no function should be traced there.
The problem is that some architectures still allow to trace some code
in this entry context code. And this is why you currently tolerate
one level of recursion here. Do I get it correctly?
But wait. If you tolerate the recursion, you actually tolerate
tracing the code between entering context and setting preempt count.
It is in your example:
> trace() {
> context = get_context(preempt_count);
> test_and_set_bit(context)
> <<--- interrupt --->>>
> trace() {
> context = get_context(preempt_count);
> test_and_set_bit(context); <-- detects recursion!
> }
> update_preempt_count(irq_context);
>
> By allowing a single recursion, it still does the above trace.
This happens only when the nested trace() is tracing something in
the IRQ entry before update_preempt_count(irq_context).
My understanding is that Peter Zijlstra do _not_ want to allow
the nested trace because this code should not get traced.
The outer trace() will work even if we reject the inner trace().
Let me show that interrupting context entry is fine:
<<--- normal context -->>
trace()
context = get_context(preempt_count); // normal context
test_and_set_bit(context)
<<--- interrupt --->>>
interrupt_entry()
// tracing is _not_ allowed here
update_preempt_count(irq_context);
// tracing should work after this point
interrupt_handler();
trace()
context = get_context(preempt_count); // IRQ context
test_and_set_bit(context); // Passes; it sees IRQ context
// trace code is allowed
<<-- back to normal context -->
// outer trace code is allowed. It sees normal context.
Best Regards,
Petr
^ permalink raw reply
* [PATCH][linux-next] soc: fsl: dpio: Unsigned compared against 0 in qbman_swp_set_irq_coalescing()
From: Tim Gardner @ 2021-10-18 16:05 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Roy Pledge, tim.gardner, linuxppc-dev, linux-kernel, Li Yang
Coverity complains of unsigned compare against 0. There are 2 cases in
this function:
1821 itp = (irq_holdoff * 1000) / p->desc->qman_256_cycles_per_ns;
CID 121131 (#1 of 1): Unsigned compared against 0 (NO_EFFECT)
unsigned_compare: This less-than-zero comparison of an unsigned value is never true. itp < 0U.
1822 if (itp < 0 || itp > 4096) {
1823 max_holdoff = (p->desc->qman_256_cycles_per_ns * 4096) / 1000;
1824 pr_err("irq_holdoff must be between 0..%dus\n", max_holdoff);
1825 return -EINVAL;
1826 }
1827
unsigned_compare: This less-than-zero comparison of an unsigned value is never true. irq_threshold < 0U.
1828 if (irq_threshold >= p->dqrr.dqrr_size || irq_threshold < 0) {
1829 pr_err("irq_threshold must be between 0..%d\n",
1830 p->dqrr.dqrr_size - 1);
1831 return -EINVAL;
1832 }
Fix this by checking for 0. Also fix a minor comment typo.
Fixes ed1d2143fee53755ec601eb4d48a337a93933f71 ("soc: fsl: dpio: add support for
irq coalescing per software portal")
Cc: Roy Pledge <Roy.Pledge@nxp.com>
Cc: Li Yang <leoyang.li@nxp.com>
Cc: linux-kernel@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
I'm not 100% sure this is the right way to fix the warning, but according to the
pr_err() comments these values should never be 0.
---
drivers/soc/fsl/dpio/qbman-portal.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/soc/fsl/dpio/qbman-portal.c b/drivers/soc/fsl/dpio/qbman-portal.c
index d3c58df6240d..b768a14bb271 100644
--- a/drivers/soc/fsl/dpio/qbman-portal.c
+++ b/drivers/soc/fsl/dpio/qbman-portal.c
@@ -1816,16 +1816,16 @@ int qbman_swp_set_irq_coalescing(struct qbman_swp *p, u32 irq_threshold,
u32 itp, max_holdoff;
/* Convert irq_holdoff value from usecs to 256 QBMAN clock cycles
- * increments. This depends to the QBMAN internal frequency.
+ * increments. This depends on the QBMAN internal frequency.
*/
itp = (irq_holdoff * 1000) / p->desc->qman_256_cycles_per_ns;
- if (itp < 0 || itp > 4096) {
+ if (!itp || itp > 4096) {
max_holdoff = (p->desc->qman_256_cycles_per_ns * 4096) / 1000;
pr_err("irq_holdoff must be between 0..%dus\n", max_holdoff);
return -EINVAL;
}
- if (irq_threshold >= p->dqrr.dqrr_size || irq_threshold < 0) {
+ if (irq_threshold >= p->dqrr.dqrr_size || !irq_threshold) {
pr_err("irq_threshold must be between 0..%d\n",
p->dqrr.dqrr_size - 1);
return -EINVAL;
--
2.33.1
^ permalink raw reply related
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