2022-12-03 03:07:36

by Palmer Dabbelt

[permalink] [raw]
Subject: [PATCH 0/5] mm: Stop alaising VM_FAULT_HINDEX_MASK in arch code

When reviewing
<https://lore.kernel.org/r/[email protected]>
I noticed that the arch-specific VM_FAULT flags used by arm and s390
alias with VM_FAULT_HINDEX_MASK. I'm not sure if it's possible to
manifest this as a bug, but it certainly seems fragile.

The RISC-V patch is on top of the linked patch above, which isn't in any
proper tree yet. Everything else should apply to 6.1-rc1, but I'm in no
particular rush to get that cleanup in so rush on my end for 6.2. That
said I figured it would be easier to send along this now, just in case
someone who knows the MM code better thinks it can manifest as a bug.


2022-12-03 03:08:03

by Palmer Dabbelt

[permalink] [raw]
Subject: [PATCH 3/5] arm: fault: Convert to VM_FAULT_ARCH_* codes

These conflict with VM_FAULT_HINDEX_MASK, so move to some designated
arch-specific values.

Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/arm/mm/fault.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 46cccd6bf705..7063b875b05f 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -201,8 +201,8 @@ void do_bad_area(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
}

#ifdef CONFIG_MMU
-#define VM_FAULT_BADMAP ((__force vm_fault_t)0x010000)
-#define VM_FAULT_BADACCESS ((__force vm_fault_t)0x020000)
+#define VM_FAULT_BADMAP VM_FAULT_ARCH_0
+#define VM_FAULT_BADACCESS VM_FAULT_ARCH_1

static inline bool is_permission_fault(unsigned int fsr)
{
--
2.38.1

2022-12-03 03:08:14

by Palmer Dabbelt

[permalink] [raw]
Subject: [PATCH 4/5] s390: fault: Convert to VM_FAULT_ARCH_* codes

These conflict with VM_FAULT_HINDEX_MASK, so move to some designated
arch-specific values.

Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/s390/mm/fault.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index 9649d9382e0a..464a74e52465 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -46,11 +46,11 @@
#define __SUBCODE_MASK 0x0600
#define __PF_RES_FIELD 0x8000000000000000ULL

-#define VM_FAULT_BADCONTEXT ((__force vm_fault_t) 0x010000)
-#define VM_FAULT_BADMAP ((__force vm_fault_t) 0x020000)
-#define VM_FAULT_BADACCESS ((__force vm_fault_t) 0x040000)
-#define VM_FAULT_SIGNAL ((__force vm_fault_t) 0x080000)
-#define VM_FAULT_PFAULT ((__force vm_fault_t) 0x100000)
+#define VM_FAULT_BADCONTEXT VM_FAULT_ARCH_0
+#define VM_FAULT_BADMAP VM_FAULT_ARCH_1
+#define VM_FAULT_BADACCESS VM_FAULT_ARCH_2
+#define VM_FAULT_SIGNAL VM_FAULT_ARCH_3
+#define VM_FAULT_PFAULT VM_FAULT_ARCH_4

enum fault_type {
KERNEL_FAULT,
--
2.38.1

2022-12-03 03:08:33

by Palmer Dabbelt

[permalink] [raw]
Subject: [PATCH 5/5] RISC-V: fault: Convert to VM_FAULT_ARCH_* codes

These conflict with VM_FAULT_HINDEX_MASK, so move to some designated
arch-specific values.

Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/riscv/mm/fault.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 3fdc2eebdd36..bd990735aa90 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -202,8 +202,8 @@ static inline bool access_error(unsigned long cause, struct vm_area_struct *vma)
return false;
}

-#define VM_FAULT_BADMAP ((__force vm_fault_t)0x010000)
-#define VM_FAULT_BADACCESS ((__force vm_fault_t)0x020000)
+#define VM_FAULT_BADMAP VM_FAULT_ARCH_0
+#define VM_FAULT_BADACCESS VM_FAULT_ARCH_1

static vm_fault_t __do_page_fault(struct mm_struct *mm, unsigned long addr,
unsigned int mm_flags, struct pt_regs *regs)
--
2.38.1

2022-12-03 03:26:54

by Palmer Dabbelt

[permalink] [raw]
Subject: [PATCH 1/5] mm: Add a leading 0 to the VM_FAULT_* types

The next patch will add enough codes to need another character, this
adds the 0 to all the existing codes to keep alignment.

Signed-off-by: Palmer Dabbelt <[email protected]>
---
include/linux/mm_types.h | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 500e536796ca..758eb70829cb 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -862,24 +862,24 @@ typedef __bitwise unsigned int vm_fault_t;
* in DAX)
* @VM_FAULT_COMPLETED: ->fault completed, meanwhile mmap lock released
* @VM_FAULT_HINDEX_MASK: mask HINDEX value
- *
+ * @VM_FAULT_ARCH_*: Architecture-specific VM fault codes.
*/
enum vm_fault_reason {
- VM_FAULT_OOM = (__force vm_fault_t)0x000001,
- VM_FAULT_SIGBUS = (__force vm_fault_t)0x000002,
- VM_FAULT_MAJOR = (__force vm_fault_t)0x000004,
- VM_FAULT_WRITE = (__force vm_fault_t)0x000008,
- VM_FAULT_HWPOISON = (__force vm_fault_t)0x000010,
- VM_FAULT_HWPOISON_LARGE = (__force vm_fault_t)0x000020,
- VM_FAULT_SIGSEGV = (__force vm_fault_t)0x000040,
- VM_FAULT_NOPAGE = (__force vm_fault_t)0x000100,
- VM_FAULT_LOCKED = (__force vm_fault_t)0x000200,
- VM_FAULT_RETRY = (__force vm_fault_t)0x000400,
- VM_FAULT_FALLBACK = (__force vm_fault_t)0x000800,
- VM_FAULT_DONE_COW = (__force vm_fault_t)0x001000,
- VM_FAULT_NEEDDSYNC = (__force vm_fault_t)0x002000,
- VM_FAULT_COMPLETED = (__force vm_fault_t)0x004000,
- VM_FAULT_HINDEX_MASK = (__force vm_fault_t)0x0f0000,
+ VM_FAULT_OOM = (__force vm_fault_t)0x0000001,
+ VM_FAULT_SIGBUS = (__force vm_fault_t)0x0000002,
+ VM_FAULT_MAJOR = (__force vm_fault_t)0x0000004,
+ VM_FAULT_WRITE = (__force vm_fault_t)0x0000008,
+ VM_FAULT_HWPOISON = (__force vm_fault_t)0x0000010,
+ VM_FAULT_HWPOISON_LARGE = (__force vm_fault_t)0x0000020,
+ VM_FAULT_SIGSEGV = (__force vm_fault_t)0x0000040,
+ VM_FAULT_NOPAGE = (__force vm_fault_t)0x0000100,
+ VM_FAULT_LOCKED = (__force vm_fault_t)0x0000200,
+ VM_FAULT_RETRY = (__force vm_fault_t)0x0000400,
+ VM_FAULT_FALLBACK = (__force vm_fault_t)0x0000800,
+ VM_FAULT_DONE_COW = (__force vm_fault_t)0x0001000,
+ VM_FAULT_NEEDDSYNC = (__force vm_fault_t)0x0002000,
+ VM_FAULT_COMPLETED = (__force vm_fault_t)0x0004000,
+ VM_FAULT_HINDEX_MASK = (__force vm_fault_t)0x00f0000,
};

/* Encode hstate index for a hwpoisoned large page */
--
2.38.1

2022-12-03 03:58:12

by Palmer Dabbelt

[permalink] [raw]
Subject: [PATCH 2/5] mm: Add VM_FAULT_ARCH_* codes

A handful of architectures (arm, s390, and soon RISC-V) define their
own internal fault codes, so instead dedicate a few standard codes as
being architecture-specific to avoid conflicts.

Signed-off-by: Palmer Dabbelt <[email protected]>
---
include/linux/mm_types.h | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 758eb70829cb..df1aa8d58444 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -880,6 +880,11 @@ enum vm_fault_reason {
VM_FAULT_NEEDDSYNC = (__force vm_fault_t)0x0002000,
VM_FAULT_COMPLETED = (__force vm_fault_t)0x0004000,
VM_FAULT_HINDEX_MASK = (__force vm_fault_t)0x00f0000,
+ VM_FAULT_ARCH_0 = (__force vm_fault_t)0x0100000,
+ VM_FAULT_ARCH_1 = (__force vm_fault_t)0x0200000,
+ VM_FAULT_ARCH_2 = (__force vm_fault_t)0x0400000,
+ VM_FAULT_ARCH_3 = (__force vm_fault_t)0x0800000,
+ VM_FAULT_ARCH_4 = (__force vm_fault_t)0x1000000,
};

/* Encode hstate index for a hwpoisoned large page */
@@ -903,7 +908,12 @@ enum vm_fault_reason {
{ VM_FAULT_RETRY, "RETRY" }, \
{ VM_FAULT_FALLBACK, "FALLBACK" }, \
{ VM_FAULT_DONE_COW, "DONE_COW" }, \
- { VM_FAULT_NEEDDSYNC, "NEEDDSYNC" }
+ { VM_FAULT_NEEDDSYNC, "NEEDDSYNC" }, \
+ { VM_FAULT_ARCH_0, "ARCH_0" }, \
+ { VM_FAULT_ARCH_1, "ARCH_1" }, \
+ { VM_FAULT_ARCH_2, "ARCH_2" }, \
+ { VM_FAULT_ARCH_3, "ARCH_3" }, \
+ { VM_FAULT_ARCH_4, "ARCH_4" }, \

struct vm_special_mapping {
const char *name; /* The name, e.g. "[vdso]". */
--
2.38.1

2023-02-03 03:32:11

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH 0/5] mm: Stop alaising VM_FAULT_HINDEX_MASK in arch code

On Fri, 02 Dec 2022 19:03:51 PST (-0800), Palmer Dabbelt wrote:
> When reviewing
> <https://lore.kernel.org/r/[email protected]>
> I noticed that the arch-specific VM_FAULT flags used by arm and s390
> alias with VM_FAULT_HINDEX_MASK. I'm not sure if it's possible to
> manifest this as a bug, but it certainly seems fragile.
>
> The RISC-V patch is on top of the linked patch above, which isn't in any
> proper tree yet. Everything else should apply to 6.1-rc1, but I'm in no
> particular rush to get that cleanup in so rush on my end for 6.2. That
> said I figured it would be easier to send along this now, just in case
> someone who knows the MM code better thinks it can manifest as a bug.

Just pinging this one, I'm not sure if it was just bad timing on the
last one. That RISC-V patch is still sitting around outside my tree.