* [PATCH v2 2/3] selftests/powerpc: Add test for real address error handling
2021-08-05 9:20 [PATCH v2 1/3] powerpc/pseries: Parse control memory access error Ganesh Goudar
@ 2021-08-05 9:20 ` Ganesh Goudar
2021-08-24 12:48 ` Michael Ellerman
2021-08-05 9:20 ` [PATCH v2 3/3] powerpc/mce: Modify the real address error logging messages Ganesh Goudar
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Ganesh Goudar @ 2021-08-05 9:20 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: mikey, Ganesh Goudar, mahesh, npiggin
Add test for real address or control memory address access
error handling, using NX-GZIP engine.
The error is injected by accessing the control memory address
using illegal instruction, on successful handling the process
attempting to access control memory address using illegal
instruction receives SIGBUS.
Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
---
v2: Fix build error.
---
tools/testing/selftests/powerpc/Makefile | 3 +-
tools/testing/selftests/powerpc/mce/Makefile | 6 +++
.../selftests/powerpc/mce/inject-ra-err.c | 42 +++++++++++++++++++
.../selftests/powerpc/mce/inject-ra-err.sh | 18 ++++++++
tools/testing/selftests/powerpc/mce/vas-api.h | 1 +
5 files changed, 69 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/powerpc/mce/Makefile
create mode 100644 tools/testing/selftests/powerpc/mce/inject-ra-err.c
create mode 100755 tools/testing/selftests/powerpc/mce/inject-ra-err.sh
create mode 120000 tools/testing/selftests/powerpc/mce/vas-api.h
diff --git a/tools/testing/selftests/powerpc/Makefile b/tools/testing/selftests/powerpc/Makefile
index 0830e63818c1..4830372d7416 100644
--- a/tools/testing/selftests/powerpc/Makefile
+++ b/tools/testing/selftests/powerpc/Makefile
@@ -31,7 +31,8 @@ SUB_DIRS = alignment \
vphn \
math \
ptrace \
- security
+ security \
+ mce
endif
diff --git a/tools/testing/selftests/powerpc/mce/Makefile b/tools/testing/selftests/powerpc/mce/Makefile
new file mode 100644
index 000000000000..0f537ce86370
--- /dev/null
+++ b/tools/testing/selftests/powerpc/mce/Makefile
@@ -0,0 +1,6 @@
+#SPDX-License-Identifier: GPL-2.0-or-later
+
+TEST_PROGS := inject-ra-err.sh
+TEST_GEN_FILES := inject-ra-err
+
+include ../../lib.mk
diff --git a/tools/testing/selftests/powerpc/mce/inject-ra-err.c b/tools/testing/selftests/powerpc/mce/inject-ra-err.c
new file mode 100644
index 000000000000..05ab11cec3da
--- /dev/null
+++ b/tools/testing/selftests/powerpc/mce/inject-ra-err.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+#include "vas-api.h"
+
+int main(void)
+{
+ int fd, ret;
+ int *paste_addr;
+ struct vas_tx_win_open_attr attr;
+ char *devname = "/dev/crypto/nx-gzip";
+
+ memset(&attr, 0, sizeof(attr));
+ attr.version = 1;
+ attr.vas_id = 0;
+
+ fd = open(devname, O_RDWR);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open device %s\n", devname);
+ return -errno;
+ }
+ ret = ioctl(fd, VAS_TX_WIN_OPEN, &attr);
+ if (ret < 0) {
+ fprintf(stderr, "ioctl() n %d, error %d\n", ret, errno);
+ ret = -errno;
+ goto out;
+ }
+ paste_addr = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0ULL);
+ /* The following assignment triggers exception */
+ *paste_addr = 1;
+ ret = 0;
+out:
+ close(fd);
+ return ret;
+}
diff --git a/tools/testing/selftests/powerpc/mce/inject-ra-err.sh b/tools/testing/selftests/powerpc/mce/inject-ra-err.sh
new file mode 100755
index 000000000000..3633cdc651a1
--- /dev/null
+++ b/tools/testing/selftests/powerpc/mce/inject-ra-err.sh
@@ -0,0 +1,18 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+if [[ ! -w /dev/crypto/nx-gzip ]]; then
+ echo "WARN: Can't access /dev/crypto/nx-gzip, skipping"
+ exit 0
+fi
+
+timeout 5 ./inject-ra-err
+
+# 128 + 7 (SIGBUS) = 135, 128 is a exit code with special meaning.
+if [ $? -ne 135 ]; then
+ echo "FAILED: Real address or Control memory access error not handled"
+ exit $?
+fi
+
+echo "OK: Real address or Control memory access error is handled"
+exit 0
diff --git a/tools/testing/selftests/powerpc/mce/vas-api.h b/tools/testing/selftests/powerpc/mce/vas-api.h
new file mode 120000
index 000000000000..1455c1bcd351
--- /dev/null
+++ b/tools/testing/selftests/powerpc/mce/vas-api.h
@@ -0,0 +1 @@
+../../../../../arch/powerpc/include/uapi/asm/vas-api.h
\ No newline at end of file
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/3] selftests/powerpc: Add test for real address error handling
2021-08-05 9:20 ` [PATCH v2 2/3] selftests/powerpc: Add test for real address error handling Ganesh Goudar
@ 2021-08-24 12:48 ` Michael Ellerman
2021-08-25 11:31 ` Ganesh
0 siblings, 1 reply; 14+ messages in thread
From: Michael Ellerman @ 2021-08-24 12:48 UTC (permalink / raw)
To: Ganesh Goudar, linuxppc-dev; +Cc: mikey, Ganesh Goudar, mahesh, npiggin
Ganesh Goudar <ganeshgr@linux.ibm.com> writes:
> Add test for real address or control memory address access
> error handling, using NX-GZIP engine.
>
> The error is injected by accessing the control memory address
> using illegal instruction, on successful handling the process
> attempting to access control memory address using illegal
> instruction receives SIGBUS.
...
> diff --git a/tools/testing/selftests/powerpc/mce/inject-ra-err.sh b/tools/testing/selftests/powerpc/mce/inject-ra-err.sh
> new file mode 100755
> index 000000000000..3633cdc651a1
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/mce/inject-ra-err.sh
> @@ -0,0 +1,18 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +if [[ ! -w /dev/crypto/nx-gzip ]]; then
> + echo "WARN: Can't access /dev/crypto/nx-gzip, skipping"
> + exit 0
> +fi
> +
> +timeout 5 ./inject-ra-err
> +
> +# 128 + 7 (SIGBUS) = 135, 128 is a exit code with special meaning.
> +if [ $? -ne 135 ]; then
> + echo "FAILED: Real address or Control memory access error not handled"
> + exit $?
> +fi
> +
> +echo "OK: Real address or Control memory access error is handled"
> +exit 0
I don't think we really need the shell script, we should be able to do
all that in the C code.
Can you try this?
cheers
diff --git a/tools/testing/selftests/powerpc/mce/Makefile b/tools/testing/selftests/powerpc/mce/Makefile
new file mode 100644
index 000000000000..2424513982d9
--- /dev/null
+++ b/tools/testing/selftests/powerpc/mce/Makefile
@@ -0,0 +1,7 @@
+#SPDX-License-Identifier: GPL-2.0-or-later
+
+TEST_GEN_PROGS := inject-ra-err
+
+include ../../lib.mk
+
+$(TEST_GEN_PROGS): ../harness.c
diff --git a/tools/testing/selftests/powerpc/mce/inject-ra-err.c b/tools/testing/selftests/powerpc/mce/inject-ra-err.c
new file mode 100644
index 000000000000..ba0f9c28f786
--- /dev/null
+++ b/tools/testing/selftests/powerpc/mce/inject-ra-err.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "vas-api.h"
+#include "utils.h"
+
+static bool faulted;
+
+static void sigbus_handler(int n, siginfo_t *info, void *ctxt_v)
+{
+ ucontext_t *ctxt = (ucontext_t *)ctxt_v;
+ struct pt_regs *regs = ctxt->uc_mcontext.regs;
+
+ faulted = true;
+ regs->nip += 4;
+}
+
+static int test_ra_error(void)
+{
+ struct vas_tx_win_open_attr attr;
+ int fd, *paste_addr;
+ char *devname = "/dev/crypto/nx-gzip";
+ struct sigaction act = {
+ .sa_sigaction = sigbus_handler,
+ .sa_flags = SA_SIGINFO,
+ };
+
+ memset(&attr, 0, sizeof(attr));
+ attr.version = 1;
+ attr.vas_id = 0;
+
+ SKIP_IF(!access(devname, F_OK));
+
+ fd = open(devname, O_RDWR);
+ FAIL_IF(fd < 0);
+ FAIL_IF(ioctl(fd, VAS_TX_WIN_OPEN, &attr) < 0);
+ FAIL_IF(sigaction(SIGBUS, &act, NULL) != 0);
+
+ paste_addr = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0ULL);
+
+ /* The following assignment triggers exception */
+ mb();
+ *paste_addr = 1;
+ mb();
+
+ FAIL_IF(!faulted);
+
+ return 0;
+}
+
+int main(void)
+{
+ return test_harness(test_ra_error, "inject-ra-err");
+}
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/3] selftests/powerpc: Add test for real address error handling
2021-08-24 12:48 ` Michael Ellerman
@ 2021-08-25 11:31 ` Ganesh
2021-08-26 3:27 ` Michael Ellerman
0 siblings, 1 reply; 14+ messages in thread
From: Ganesh @ 2021-08-25 11:31 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: mikey, mahesh, npiggin
[-- Attachment #1: Type: text/plain, Size: 3575 bytes --]
On 8/24/21 6:18 PM, Michael Ellerman wrote:
> Ganesh Goudar <ganeshgr@linux.ibm.com> writes:
>> Add test for real address or control memory address access
>> error handling, using NX-GZIP engine.
>>
>> The error is injected by accessing the control memory address
>> using illegal instruction, on successful handling the process
>> attempting to access control memory address using illegal
>> instruction receives SIGBUS.
> ...
>
>> diff --git a/tools/testing/selftests/powerpc/mce/inject-ra-err.sh b/tools/testing/selftests/powerpc/mce/inject-ra-err.sh
>> new file mode 100755
>> index 000000000000..3633cdc651a1
>> --- /dev/null
>> +++ b/tools/testing/selftests/powerpc/mce/inject-ra-err.sh
>> @@ -0,0 +1,18 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +
>> +if [[ ! -w /dev/crypto/nx-gzip ]]; then
>> + echo "WARN: Can't access /dev/crypto/nx-gzip, skipping"
>> + exit 0
>> +fi
>> +
>> +timeout 5 ./inject-ra-err
>> +
>> +# 128 + 7 (SIGBUS) = 135, 128 is a exit code with special meaning.
>> +if [ $? -ne 135 ]; then
>> + echo "FAILED: Real address or Control memory access error not handled"
>> + exit $?
>> +fi
>> +
>> +echo "OK: Real address or Control memory access error is handled"
>> +exit 0
> I don't think we really need the shell script, we should be able to do
> all that in the C code.
>
> Can you try this?
it works!, We need to set timeout, with 120 sec timeout we may flood the dmesg.
Thanks.
>
> cheers
>
> diff --git a/tools/testing/selftests/powerpc/mce/Makefile b/tools/testing/selftests/powerpc/mce/Makefile
> new file mode 100644
> index 000000000000..2424513982d9
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/mce/Makefile
> @@ -0,0 +1,7 @@
> +#SPDX-License-Identifier: GPL-2.0-or-later
> +
> +TEST_GEN_PROGS := inject-ra-err
> +
> +include ../../lib.mk
> +
> +$(TEST_GEN_PROGS): ../harness.c
> diff --git a/tools/testing/selftests/powerpc/mce/inject-ra-err.c b/tools/testing/selftests/powerpc/mce/inject-ra-err.c
> new file mode 100644
> index 000000000000..ba0f9c28f786
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/mce/inject-ra-err.c
> @@ -0,0 +1,64 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <signal.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include "vas-api.h"
> +#include "utils.h"
> +
> +static bool faulted;
> +
> +static void sigbus_handler(int n, siginfo_t *info, void *ctxt_v)
> +{
> + ucontext_t *ctxt = (ucontext_t *)ctxt_v;
> + struct pt_regs *regs = ctxt->uc_mcontext.regs;
> +
> + faulted = true;
> + regs->nip += 4;
> +}
> +
> +static int test_ra_error(void)
> +{
> + struct vas_tx_win_open_attr attr;
> + int fd, *paste_addr;
> + char *devname = "/dev/crypto/nx-gzip";
> + struct sigaction act = {
> + .sa_sigaction = sigbus_handler,
> + .sa_flags = SA_SIGINFO,
> + };
> +
> + memset(&attr, 0, sizeof(attr));
> + attr.version = 1;
> + attr.vas_id = 0;
> +
> + SKIP_IF(!access(devname, F_OK));
> +
> + fd = open(devname, O_RDWR);
> + FAIL_IF(fd < 0);
> + FAIL_IF(ioctl(fd, VAS_TX_WIN_OPEN, &attr) < 0);
> + FAIL_IF(sigaction(SIGBUS, &act, NULL) != 0);
> +
> + paste_addr = mmap(NULL, 4096, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0ULL);
> +
> + /* The following assignment triggers exception */
> + mb();
> + *paste_addr = 1;
> + mb();
> +
> + FAIL_IF(!faulted);
> +
> + return 0;
> +}
> +
> +int main(void)
> +{
> + return test_harness(test_ra_error, "inject-ra-err");
> +}
[-- Attachment #2: Type: text/html, Size: 4197 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 2/3] selftests/powerpc: Add test for real address error handling
2021-08-25 11:31 ` Ganesh
@ 2021-08-26 3:27 ` Michael Ellerman
2021-08-26 12:57 ` Ganesh
0 siblings, 1 reply; 14+ messages in thread
From: Michael Ellerman @ 2021-08-26 3:27 UTC (permalink / raw)
To: Ganesh, linuxppc-dev; +Cc: mikey, mahesh, npiggin
Ganesh <ganeshgr@linux.ibm.com> writes:
> On 8/24/21 6:18 PM, Michael Ellerman wrote:
>
>> Ganesh Goudar <ganeshgr@linux.ibm.com> writes:
>>> Add test for real address or control memory address access
>>> error handling, using NX-GZIP engine.
>>>
>>> The error is injected by accessing the control memory address
>>> using illegal instruction, on successful handling the process
>>> attempting to access control memory address using illegal
>>> instruction receives SIGBUS.
>> ...
>>
>>> diff --git a/tools/testing/selftests/powerpc/mce/inject-ra-err.sh b/tools/testing/selftests/powerpc/mce/inject-ra-err.sh
>>> new file mode 100755
>>> index 000000000000..3633cdc651a1
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/powerpc/mce/inject-ra-err.sh
>>> @@ -0,0 +1,18 @@
>>> +#!/bin/bash
>>> +# SPDX-License-Identifier: GPL-2.0-or-later
>>> +
>>> +if [[ ! -w /dev/crypto/nx-gzip ]]; then
>>> + echo "WARN: Can't access /dev/crypto/nx-gzip, skipping"
>>> + exit 0
>>> +fi
>>> +
>>> +timeout 5 ./inject-ra-err
>>> +
>>> +# 128 + 7 (SIGBUS) = 135, 128 is a exit code with special meaning.
>>> +if [ $? -ne 135 ]; then
>>> + echo "FAILED: Real address or Control memory access error not handled"
>>> + exit $?
>>> +fi
>>> +
>>> +echo "OK: Real address or Control memory access error is handled"
>>> +exit 0
>> I don't think we really need the shell script, we should be able to do
>> all that in the C code.
>>
>> Can you try this?
>
> it works!, We need to set timeout, with 120 sec timeout we may flood the dmesg.
Hmm. Does it keep faulting? The regs->nip += 4 is meant to avoid that.
cheers
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 2/3] selftests/powerpc: Add test for real address error handling
2021-08-26 3:27 ` Michael Ellerman
@ 2021-08-26 12:57 ` Ganesh
0 siblings, 0 replies; 14+ messages in thread
From: Ganesh @ 2021-08-26 12:57 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: mikey, mahesh, npiggin
[-- Attachment #1: Type: text/plain, Size: 1767 bytes --]
On 8/26/21 8:57 AM, Michael Ellerman wrote:
> Ganesh <ganeshgr@linux.ibm.com> writes:
>> On 8/24/21 6:18 PM, Michael Ellerman wrote:
>>
>>> Ganesh Goudar <ganeshgr@linux.ibm.com> writes:
>>>> Add test for real address or control memory address access
>>>> error handling, using NX-GZIP engine.
>>>>
>>>> The error is injected by accessing the control memory address
>>>> using illegal instruction, on successful handling the process
>>>> attempting to access control memory address using illegal
>>>> instruction receives SIGBUS.
>>> ...
>>>
>>>> diff --git a/tools/testing/selftests/powerpc/mce/inject-ra-err.sh b/tools/testing/selftests/powerpc/mce/inject-ra-err.sh
>>>> new file mode 100755
>>>> index 000000000000..3633cdc651a1
>>>> --- /dev/null
>>>> +++ b/tools/testing/selftests/powerpc/mce/inject-ra-err.sh
>>>> @@ -0,0 +1,18 @@
>>>> +#!/bin/bash
>>>> +# SPDX-License-Identifier: GPL-2.0-or-later
>>>> +
>>>> +if [[ ! -w /dev/crypto/nx-gzip ]]; then
>>>> + echo "WARN: Can't access /dev/crypto/nx-gzip, skipping"
>>>> + exit 0
>>>> +fi
>>>> +
>>>> +timeout 5 ./inject-ra-err
>>>> +
>>>> +# 128 + 7 (SIGBUS) = 135, 128 is a exit code with special meaning.
>>>> +if [ $? -ne 135 ]; then
>>>> + echo "FAILED: Real address or Control memory access error not handled"
>>>> + exit $?
>>>> +fi
>>>> +
>>>> +echo "OK: Real address or Control memory access error is handled"
>>>> +exit 0
>>> I don't think we really need the shell script, we should be able to do
>>> all that in the C code.
>>>
>>> Can you try this?
>> it works!, We need to set timeout, with 120 sec timeout we may flood the dmesg.
> Hmm. Does it keep faulting? The regs->nip += 4 is meant to avoid that.
Yes, it keeps faulting, if we fail to handle and not send SIGBUS to the process.
>
> cheers
[-- Attachment #2: Type: text/html, Size: 2865 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/3] powerpc/mce: Modify the real address error logging messages
2021-08-05 9:20 [PATCH v2 1/3] powerpc/pseries: Parse control memory access error Ganesh Goudar
2021-08-05 9:20 ` [PATCH v2 2/3] selftests/powerpc: Add test for real address error handling Ganesh Goudar
@ 2021-08-05 9:20 ` Ganesh Goudar
2021-08-23 18:53 ` [PATCH v2 1/3] powerpc/pseries: Parse control memory access error Ganesh
2021-08-24 6:39 ` Michael Ellerman
3 siblings, 0 replies; 14+ messages in thread
From: Ganesh Goudar @ 2021-08-05 9:20 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: mikey, Ganesh Goudar, mahesh, npiggin
To avoid ambiguity, modify the strings in real address error
logging messages to "foreign/control memory" from "foreign",
Since the error discriptions in P9 user manual and P10 user
manual are different for same type of errors.
P9 User Manual for MCE:
DSISR:59 Host real address to foreign space during translation.
DSISR:60 Host real address to foreign space on a load or store
access.
P10 User Manual for MCE:
DSISR:59 D-side tablewalk used a host real address in the
control memory address range.
DSISR:60 D-side operand access to control memory address space.
Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
---
v2: No changes in this patch.
---
arch/powerpc/kernel/mce.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/mce.c b/arch/powerpc/kernel/mce.c
index 47a683cd00d2..f3ef480bb739 100644
--- a/arch/powerpc/kernel/mce.c
+++ b/arch/powerpc/kernel/mce.c
@@ -388,14 +388,14 @@ void machine_check_print_event_info(struct machine_check_event *evt,
static const char *mc_ra_types[] = {
"Indeterminate",
"Instruction fetch (bad)",
- "Instruction fetch (foreign)",
+ "Instruction fetch (foreign/control memory)",
"Page table walk ifetch (bad)",
- "Page table walk ifetch (foreign)",
+ "Page table walk ifetch (foreign/control memory)",
"Load (bad)",
"Store (bad)",
"Page table walk Load/Store (bad)",
- "Page table walk Load/Store (foreign)",
- "Load/Store (foreign)",
+ "Page table walk Load/Store (foreign/control memory)",
+ "Load/Store (foreign/control memory)",
};
static const char *mc_link_types[] = {
"Indeterminate",
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v2 1/3] powerpc/pseries: Parse control memory access error
2021-08-05 9:20 [PATCH v2 1/3] powerpc/pseries: Parse control memory access error Ganesh Goudar
2021-08-05 9:20 ` [PATCH v2 2/3] selftests/powerpc: Add test for real address error handling Ganesh Goudar
2021-08-05 9:20 ` [PATCH v2 3/3] powerpc/mce: Modify the real address error logging messages Ganesh Goudar
@ 2021-08-23 18:53 ` Ganesh
2021-08-24 6:39 ` Michael Ellerman
3 siblings, 0 replies; 14+ messages in thread
From: Ganesh @ 2021-08-23 18:53 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: mikey, mahesh, npiggin
[-- Attachment #1: Type: text/plain, Size: 2353 bytes --]
Hi mpe, Any comments on this patchset?
On 8/5/21 2:50 PM, Ganesh Goudar wrote:
> Add support to parse and log control memory access
> error for pseries.
>
> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
> ---
> v2: No changes in this patch.
> ---
> arch/powerpc/platforms/pseries/ras.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
> index 167f2e1b8d39..608c35cad0c3 100644
> --- a/arch/powerpc/platforms/pseries/ras.c
> +++ b/arch/powerpc/platforms/pseries/ras.c
> @@ -80,6 +80,7 @@ struct pseries_mc_errorlog {
> #define MC_ERROR_TYPE_TLB 0x04
> #define MC_ERROR_TYPE_D_CACHE 0x05
> #define MC_ERROR_TYPE_I_CACHE 0x07
> +#define MC_ERROR_TYPE_CTRL_MEM_ACCESS 0x08
>
> /* RTAS pseries MCE error sub types */
> #define MC_ERROR_UE_INDETERMINATE 0
> @@ -103,6 +104,9 @@ struct pseries_mc_errorlog {
> #define MC_ERROR_TLB_MULTIHIT 2
> #define MC_ERROR_TLB_INDETERMINATE 3
>
> +#define MC_ERROR_CTRL_MEM_ACCESS_PTABLE_WALK 0
> +#define MC_ERROR_CTRL_MEM_ACCESS_OP_ACCESS 1
> +
> static inline u8 rtas_mc_error_sub_type(const struct pseries_mc_errorlog *mlog)
> {
> switch (mlog->error_type) {
> @@ -112,6 +116,8 @@ static inline u8 rtas_mc_error_sub_type(const struct pseries_mc_errorlog *mlog)
> case MC_ERROR_TYPE_ERAT:
> case MC_ERROR_TYPE_TLB:
> return (mlog->sub_err_type & 0x03);
> + case MC_ERROR_TYPE_CTRL_MEM_ACCESS:
> + return (mlog->sub_err_type & 0x70) >> 4;
> default:
> return 0;
> }
> @@ -699,6 +705,21 @@ static int mce_handle_err_virtmode(struct pt_regs *regs,
> case MC_ERROR_TYPE_I_CACHE:
> mce_err.error_type = MCE_ERROR_TYPE_ICACHE;
> break;
> + case MC_ERROR_TYPE_CTRL_MEM_ACCESS:
> + mce_err.error_type = MCE_ERROR_TYPE_RA;
> + if (mce_log->sub_err_type & 0x80)
> + eaddr = be64_to_cpu(mce_log->effective_address);
> + switch (err_sub_type) {
> + case MC_ERROR_CTRL_MEM_ACCESS_PTABLE_WALK:
> + mce_err.u.ra_error_type =
> + MCE_RA_ERROR_PAGE_TABLE_WALK_LOAD_STORE_FOREIGN;
> + break;
> + case MC_ERROR_CTRL_MEM_ACCESS_OP_ACCESS:
> + mce_err.u.ra_error_type =
> + MCE_RA_ERROR_LOAD_STORE_FOREIGN;
> + break;
> + }
> + break;
> case MC_ERROR_TYPE_UNKNOWN:
> default:
> mce_err.error_type = MCE_ERROR_TYPE_UNKNOWN;
[-- Attachment #2: Type: text/html, Size: 2676 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 1/3] powerpc/pseries: Parse control memory access error
2021-08-05 9:20 [PATCH v2 1/3] powerpc/pseries: Parse control memory access error Ganesh Goudar
` (2 preceding siblings ...)
2021-08-23 18:53 ` [PATCH v2 1/3] powerpc/pseries: Parse control memory access error Ganesh
@ 2021-08-24 6:39 ` Michael Ellerman
2021-08-24 21:24 ` Segher Boessenkool
2021-08-25 11:03 ` Ganesh
3 siblings, 2 replies; 14+ messages in thread
From: Michael Ellerman @ 2021-08-24 6:39 UTC (permalink / raw)
To: Ganesh Goudar, linuxppc-dev; +Cc: mikey, Ganesh Goudar, mahesh, npiggin
Hi Ganesh,
Some comments below ...
Ganesh Goudar <ganeshgr@linux.ibm.com> writes:
> Add support to parse and log control memory access
> error for pseries.
>
> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
> ---
> v2: No changes in this patch.
> ---
> arch/powerpc/platforms/pseries/ras.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
> index 167f2e1b8d39..608c35cad0c3 100644
> --- a/arch/powerpc/platforms/pseries/ras.c
> +++ b/arch/powerpc/platforms/pseries/ras.c
> @@ -80,6 +80,7 @@ struct pseries_mc_errorlog {
> #define MC_ERROR_TYPE_TLB 0x04
> #define MC_ERROR_TYPE_D_CACHE 0x05
> #define MC_ERROR_TYPE_I_CACHE 0x07
> +#define MC_ERROR_TYPE_CTRL_MEM_ACCESS 0x08
...
>
> +#define MC_ERROR_CTRL_MEM_ACCESS_PTABLE_WALK 0
> +#define MC_ERROR_CTRL_MEM_ACCESS_OP_ACCESS 1
Where do the above values come from?
> +
> static inline u8 rtas_mc_error_sub_type(const struct pseries_mc_errorlog *mlog)
> {
> switch (mlog->error_type) {
> @@ -112,6 +116,8 @@ static inline u8 rtas_mc_error_sub_type(const struct pseries_mc_errorlog *mlog)
> case MC_ERROR_TYPE_ERAT:
> case MC_ERROR_TYPE_TLB:
> return (mlog->sub_err_type & 0x03);
> + case MC_ERROR_TYPE_CTRL_MEM_ACCESS:
> + return (mlog->sub_err_type & 0x70) >> 4;
Can you add to the comment above sub_err_type explaining what these bits are.
> default:
> return 0;
> }
> @@ -699,6 +705,21 @@ static int mce_handle_err_virtmode(struct pt_regs *regs,
> case MC_ERROR_TYPE_I_CACHE:
> mce_err.error_type = MCE_ERROR_TYPE_ICACHE;
> break;
> + case MC_ERROR_TYPE_CTRL_MEM_ACCESS:
> + mce_err.error_type = MCE_ERROR_TYPE_RA;
> + if (mce_log->sub_err_type & 0x80)
This appears many times in the file.
Can we add eg. MC_EFFECTIVE_ADDR_PROVIDED?
> + eaddr = be64_to_cpu(mce_log->effective_address);
> + switch (err_sub_type) {
> + case MC_ERROR_CTRL_MEM_ACCESS_PTABLE_WALK:
> + mce_err.u.ra_error_type =
> + MCE_RA_ERROR_PAGE_TABLE_WALK_LOAD_STORE_FOREIGN;
That name is ridiculously long, but I guess that's not your fault :)
We can fix it up in a later patch.
> + break;
> + case MC_ERROR_CTRL_MEM_ACCESS_OP_ACCESS:
> + mce_err.u.ra_error_type =
> + MCE_RA_ERROR_LOAD_STORE_FOREIGN;
> + break;
> + }
> + break;
cheers
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 1/3] powerpc/pseries: Parse control memory access error
2021-08-24 6:39 ` Michael Ellerman
@ 2021-08-24 21:24 ` Segher Boessenkool
2021-08-25 11:36 ` Ganesh
2021-08-25 11:03 ` Ganesh
1 sibling, 1 reply; 14+ messages in thread
From: Segher Boessenkool @ 2021-08-24 21:24 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, mikey, Ganesh Goudar, mahesh, npiggin
On Tue, Aug 24, 2021 at 04:39:57PM +1000, Michael Ellerman wrote:
> > + case MC_ERROR_CTRL_MEM_ACCESS_PTABLE_WALK:
> > + mce_err.u.ra_error_type =
> > + MCE_RA_ERROR_PAGE_TABLE_WALK_LOAD_STORE_FOREIGN;
>
> That name is ridiculously long, but I guess that's not your fault :)
> We can fix it up in a later patch.
It also has surprisingly little information content for the 47 chars
length it has :-) What does this even mean?!
Segher
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] powerpc/pseries: Parse control memory access error
2021-08-24 21:24 ` Segher Boessenkool
@ 2021-08-25 11:36 ` Ganesh
2021-08-25 14:47 ` Segher Boessenkool
0 siblings, 1 reply; 14+ messages in thread
From: Ganesh @ 2021-08-25 11:36 UTC (permalink / raw)
To: Segher Boessenkool, Michael Ellerman; +Cc: mikey, linuxppc-dev, mahesh, npiggin
[-- Attachment #1: Type: text/plain, Size: 591 bytes --]
On 8/25/21 2:54 AM, Segher Boessenkool wrote:
> On Tue, Aug 24, 2021 at 04:39:57PM +1000, Michael Ellerman wrote:
>>> + case MC_ERROR_CTRL_MEM_ACCESS_PTABLE_WALK:
>>> + mce_err.u.ra_error_type =
>>> + MCE_RA_ERROR_PAGE_TABLE_WALK_LOAD_STORE_FOREIGN;
>> That name is ridiculously long, but I guess that's not your fault :)
>> We can fix it up in a later patch.
> It also has surprisingly little information content for the 47 chars
> length it has :-) What does this even mean?!
It means control memory access error/real address error is detected during page
table walk.
>
> Segher
[-- Attachment #2: Type: text/html, Size: 1340 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] powerpc/pseries: Parse control memory access error
2021-08-25 11:36 ` Ganesh
@ 2021-08-25 14:47 ` Segher Boessenkool
0 siblings, 0 replies; 14+ messages in thread
From: Segher Boessenkool @ 2021-08-25 14:47 UTC (permalink / raw)
To: Ganesh; +Cc: mikey, linuxppc-dev, mahesh, npiggin
On Wed, Aug 25, 2021 at 05:06:29PM +0530, Ganesh wrote:
>
> On 8/25/21 2:54 AM, Segher Boessenkool wrote:
> >On Tue, Aug 24, 2021 at 04:39:57PM +1000, Michael Ellerman wrote:
> >>>+ case MC_ERROR_CTRL_MEM_ACCESS_PTABLE_WALK:
> >>>+ mce_err.u.ra_error_type =
> >>>+ MCE_RA_ERROR_PAGE_TABLE_WALK_LOAD_STORE_FOREIGN;
> >>That name is ridiculously long, but I guess that's not your fault :)
> >>We can fix it up in a later patch.
> >It also has surprisingly little information content for the 47 chars
> >length it has :-) What does this even mean?!
>
> It means control memory access error/real address error is detected during
> page
> table walk.
This isn't obvious from the name. The name contains some words your
explanation does not, as well: LOAD, STORE, FOREIGN. Most importantly,
the name is just a jumble of words, with no apparent connection between
them.
I didn't ask for an explanation, sorry if you misunderstood. I was just
exploring the many ways this name is baffling :-)
Segher
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 1/3] powerpc/pseries: Parse control memory access error
2021-08-24 6:39 ` Michael Ellerman
2021-08-24 21:24 ` Segher Boessenkool
@ 2021-08-25 11:03 ` Ganesh
2021-08-26 2:36 ` Michael Ellerman
1 sibling, 1 reply; 14+ messages in thread
From: Ganesh @ 2021-08-25 11:03 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: mikey, mahesh, npiggin
[-- Attachment #1: Type: text/plain, Size: 2659 bytes --]
On 8/24/21 12:09 PM, Michael Ellerman wrote:
> Hi Ganesh,
>
> Some comments below ...
>
> Ganesh Goudar <ganeshgr@linux.ibm.com> writes:
>> Add support to parse and log control memory access
>> error for pseries.
>>
>> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
>> ---
>> v2: No changes in this patch.
>> ---
>> arch/powerpc/platforms/pseries/ras.c | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
>> index 167f2e1b8d39..608c35cad0c3 100644
>> --- a/arch/powerpc/platforms/pseries/ras.c
>> +++ b/arch/powerpc/platforms/pseries/ras.c
>> @@ -80,6 +80,7 @@ struct pseries_mc_errorlog {
>> #define MC_ERROR_TYPE_TLB 0x04
>> #define MC_ERROR_TYPE_D_CACHE 0x05
>> #define MC_ERROR_TYPE_I_CACHE 0x07
>> +#define MC_ERROR_TYPE_CTRL_MEM_ACCESS 0x08
> ...
>>
>> +#define MC_ERROR_CTRL_MEM_ACCESS_PTABLE_WALK 0
>> +#define MC_ERROR_CTRL_MEM_ACCESS_OP_ACCESS 1
>
> Where do the above values come from?
It is from latest PAPR that added support for control memory error.
>> +
>> static inline u8 rtas_mc_error_sub_type(const struct pseries_mc_errorlog *mlog)
>> {
>> switch (mlog->error_type) {
>> @@ -112,6 +116,8 @@ static inline u8 rtas_mc_error_sub_type(const struct pseries_mc_errorlog *mlog)
>> case MC_ERROR_TYPE_ERAT:
>> case MC_ERROR_TYPE_TLB:
>> return (mlog->sub_err_type & 0x03);
>> + case MC_ERROR_TYPE_CTRL_MEM_ACCESS:
>> + return (mlog->sub_err_type & 0x70) >> 4;
> Can you add to the comment above sub_err_type explaining what these bits are.
Sure, for other errors it is explained in pseries_mc_errorlog definition, ill add it there.
>> default:
>> return 0;
>> }
>> @@ -699,6 +705,21 @@ static int mce_handle_err_virtmode(struct pt_regs *regs,
>> case MC_ERROR_TYPE_I_CACHE:
>> mce_err.error_type = MCE_ERROR_TYPE_ICACHE;
>> break;
>> + case MC_ERROR_TYPE_CTRL_MEM_ACCESS:
>> + mce_err.error_type = MCE_ERROR_TYPE_RA;
>> + if (mce_log->sub_err_type & 0x80)
> This appears many times in the file.
>
> Can we add eg. MC_EFFECTIVE_ADDR_PROVIDED?
ok, thanks.
>> + eaddr = be64_to_cpu(mce_log->effective_address);
>> + switch (err_sub_type) {
>> + case MC_ERROR_CTRL_MEM_ACCESS_PTABLE_WALK:
>> + mce_err.u.ra_error_type =
>> + MCE_RA_ERROR_PAGE_TABLE_WALK_LOAD_STORE_FOREIGN;
> That name is ridiculously long, but I guess that's not your fault :)
> We can fix it up in a later patch.
>
>> + break;
>> + case MC_ERROR_CTRL_MEM_ACCESS_OP_ACCESS:
>> + mce_err.u.ra_error_type =
>> + MCE_RA_ERROR_LOAD_STORE_FOREIGN;
>> + break;
>> + }
>> + break;
> cheers
[-- Attachment #2: Type: text/html, Size: 4268 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 1/3] powerpc/pseries: Parse control memory access error
2021-08-25 11:03 ` Ganesh
@ 2021-08-26 2:36 ` Michael Ellerman
0 siblings, 0 replies; 14+ messages in thread
From: Michael Ellerman @ 2021-08-26 2:36 UTC (permalink / raw)
To: Ganesh, linuxppc-dev; +Cc: mikey, mahesh, npiggin
Ganesh <ganeshgr@linux.ibm.com> writes:
> On 8/24/21 12:09 PM, Michael Ellerman wrote:
>> Ganesh Goudar <ganeshgr@linux.ibm.com> writes:
>>> Add support to parse and log control memory access
>>> error for pseries.
>>>
>>> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
>>> ---
>>> v2: No changes in this patch.
>>> ---
>>> arch/powerpc/platforms/pseries/ras.c | 21 +++++++++++++++++++++
>>> 1 file changed, 21 insertions(+)
>>>
>>> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
>>> index 167f2e1b8d39..608c35cad0c3 100644
>>> --- a/arch/powerpc/platforms/pseries/ras.c
>>> +++ b/arch/powerpc/platforms/pseries/ras.c
>>> @@ -80,6 +80,7 @@ struct pseries_mc_errorlog {
>>> #define MC_ERROR_TYPE_TLB 0x04
>>> #define MC_ERROR_TYPE_D_CACHE 0x05
>>> #define MC_ERROR_TYPE_I_CACHE 0x07
>>> +#define MC_ERROR_TYPE_CTRL_MEM_ACCESS 0x08
>> ...
>>>
>>> +#define MC_ERROR_CTRL_MEM_ACCESS_PTABLE_WALK 0
>>> +#define MC_ERROR_CTRL_MEM_ACCESS_OP_ACCESS 1
>>
>> Where do the above values come from?
>
> It is from latest PAPR that added support for control memory error.
Please cite the version of the document and the section number in the
change log.
cheers
^ permalink raw reply [flat|nested] 14+ messages in thread