* [PATCH] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()
@ 2026-07-16 8:10 Mike Rapoport (Microsoft)
2026-07-16 8:37 ` Jürgen Groß
0 siblings, 1 reply; 11+ messages in thread
From: Mike Rapoport (Microsoft) @ 2026-07-16 8:10 UTC (permalink / raw)
To: Dave Hansen
Cc: Andy Lutomirski, Borislav Petkov, Ingo Molnar, Ingo Molnar,
Juergen Gross, Mike Rapoport (Microsoft), H. Peter Anvin,
Peter Zijlstra, Thomas Gleixner, x86, linux-kernel
lookup_address_in_pgd_attr() accumulates the effective NX and RW bits of
the walked page table levels so that verify_rwx() can detect mappings that
are both writable and executable.
The RW bits are folded into a bool with
*rw &= pXd_flags(*pXd) & _PAGE_RW;
but _PAGE_RW is bit 1 while *rw only ever holds 0 or 1, so the AND is
always 0. *rw becomes false at the first level walked, regardless of the
actual permissions, and verify_rwx() treats every mapping as non-writable
and never reports a W^X violation.
Accumulate NX and RW in unsigned long locals in their native bit positions
and store the result into the bool outputs once.
Fixes: ceb647b4b529 ("x86/pat: Introduce lookup_address_in_pgd_attr()")
Assisted-by: Copilot:claude-opus-4.8
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
arch/x86/mm/pat/set_memory.c | 58 ++++++++++++++++++++++++++------------------
1 file changed, 35 insertions(+), 23 deletions(-)
diff --git a/arch/x86/mm/pat/set_memory.c b/arch/x86/mm/pat/set_memory.c
index d023a40a1e03..05de76887fd3 100644
--- a/arch/x86/mm/pat/set_memory.c
+++ b/arch/x86/mm/pat/set_memory.c
@@ -716,57 +716,69 @@ static inline pgprot_t verify_rwx(pgprot_t old, pgprot_t new, unsigned long star
* page table levels.
*/
pte_t *lookup_address_in_pgd_attr(pgd_t *pgd, unsigned long address,
- unsigned int *level, bool *nx, bool *rw)
+ unsigned int *level,
+ bool *ret_nx, bool *ret_rw)
{
+ unsigned long rw = _PAGE_RW;
+ unsigned long nx = 0;
+ pte_t *pte = NULL;
p4d_t *p4d;
pud_t *pud;
pmd_t *pmd;
*level = PG_LEVEL_256T;
- *nx = false;
- *rw = true;
-
if (pgd_none(*pgd))
- return NULL;
+ goto out;
*level = PG_LEVEL_512G;
- *nx |= pgd_flags(*pgd) & _PAGE_NX;
- *rw &= pgd_flags(*pgd) & _PAGE_RW;
+ nx |= pgd_flags(*pgd) & _PAGE_NX;
+ rw &= pgd_flags(*pgd) & _PAGE_RW;
p4d = p4d_offset(pgd, address);
if (p4d_none(*p4d))
- return NULL;
+ goto out;
- if (p4d_leaf(*p4d) || !p4d_present(*p4d))
- return (pte_t *)p4d;
+ if (p4d_leaf(*p4d) || !p4d_present(*p4d)) {
+ pte = (pte_t *)p4d;
+ goto out;
+ }
*level = PG_LEVEL_1G;
- *nx |= p4d_flags(*p4d) & _PAGE_NX;
- *rw &= p4d_flags(*p4d) & _PAGE_RW;
+ nx |= p4d_flags(*p4d) & _PAGE_NX;
+ rw &= p4d_flags(*p4d) & _PAGE_RW;
pud = pud_offset(p4d, address);
if (pud_none(*pud))
- return NULL;
+ goto out;
- if (pud_leaf(*pud) || !pud_present(*pud))
- return (pte_t *)pud;
+ if (pud_leaf(*pud) || !pud_present(*pud)) {
+ pte = (pte_t *)pud;
+ goto out;
+ }
*level = PG_LEVEL_2M;
- *nx |= pud_flags(*pud) & _PAGE_NX;
- *rw &= pud_flags(*pud) & _PAGE_RW;
+ nx |= pud_flags(*pud) & _PAGE_NX;
+ rw &= pud_flags(*pud) & _PAGE_RW;
pmd = pmd_offset(pud, address);
if (pmd_none(*pmd))
- return NULL;
+ goto out;
- if (pmd_leaf(*pmd) || !pmd_present(*pmd))
- return (pte_t *)pmd;
+ if (pmd_leaf(*pmd) || !pmd_present(*pmd)) {
+ pte = (pte_t *)pmd;
+ goto out;
+ }
*level = PG_LEVEL_4K;
- *nx |= pmd_flags(*pmd) & _PAGE_NX;
- *rw &= pmd_flags(*pmd) & _PAGE_RW;
+ nx |= pmd_flags(*pmd) & _PAGE_NX;
+ rw &= pmd_flags(*pmd) & _PAGE_RW;
+ pte = pte_offset_kernel(pmd, address);
+
+out:
+ *ret_nx = !!nx;
+ *ret_rw = !!rw;
- return pte_offset_kernel(pmd, address);
+ return pte;
}
/*
---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260716-verify-rwx-fix-3b0bd3e51244
--
Sincerely yours,
Mike.
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()
2026-07-16 8:10 [PATCH] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr() Mike Rapoport (Microsoft)
@ 2026-07-16 8:37 ` Jürgen Groß
2026-07-16 8:46 ` Juergen Gross
2026-07-16 9:18 ` Mike Rapoport
0 siblings, 2 replies; 11+ messages in thread
From: Jürgen Groß @ 2026-07-16 8:37 UTC (permalink / raw)
To: Mike Rapoport (Microsoft), Dave Hansen
Cc: Andy Lutomirski, Borislav Petkov, Ingo Molnar, Ingo Molnar,
H. Peter Anvin, Peter Zijlstra, Thomas Gleixner, x86,
linux-kernel
[-- Attachment #1.1.1: Type: text/plain, Size: 1234 bytes --]
On 16.07.26 10:10, Mike Rapoport (Microsoft) wrote:
> lookup_address_in_pgd_attr() accumulates the effective NX and RW bits of
> the walked page table levels so that verify_rwx() can detect mappings that
> are both writable and executable.
>
> The RW bits are folded into a bool with
>
> *rw &= pXd_flags(*pXd) & _PAGE_RW;
>
> but _PAGE_RW is bit 1 while *rw only ever holds 0 or 1, so the AND is
> always 0. *rw becomes false at the first level walked, regardless of the
> actual permissions, and verify_rwx() treats every mapping as non-writable
> and never reports a W^X violation.
>
> Accumulate NX and RW in unsigned long locals in their native bit positions
> and store the result into the bool outputs once.
>
> Fixes: ceb647b4b529 ("x86/pat: Introduce lookup_address_in_pgd_attr()")
> Assisted-by: Copilot:claude-opus-4.8
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
Thanks for catching this.
Reviewed-by: Juergen Gross <jgross@suse.com>
Just one remark: instead of using additional local variables the fix could
just look like:
*rw |= !!(pXd_flags(*pXd) & _PAGE_RW);
I don't really care how the issue is fixed, but this would result in less
code.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()
2026-07-16 8:37 ` Jürgen Groß
@ 2026-07-16 8:46 ` Juergen Gross
2026-07-16 9:18 ` Mike Rapoport
1 sibling, 0 replies; 11+ messages in thread
From: Juergen Gross @ 2026-07-16 8:46 UTC (permalink / raw)
To: Mike Rapoport (Microsoft), Dave Hansen
Cc: Andy Lutomirski, Borislav Petkov, Ingo Molnar, Ingo Molnar,
H. Peter Anvin, Peter Zijlstra, Thomas Gleixner, x86,
linux-kernel
[-- Attachment #1.1.1: Type: text/plain, Size: 1256 bytes --]
On 16.07.26 10:37, Jürgen Groß wrote:
> On 16.07.26 10:10, Mike Rapoport (Microsoft) wrote:
>> lookup_address_in_pgd_attr() accumulates the effective NX and RW bits of
>> the walked page table levels so that verify_rwx() can detect mappings that
>> are both writable and executable.
>>
>> The RW bits are folded into a bool with
>>
>> *rw &= pXd_flags(*pXd) & _PAGE_RW;
>>
>> but _PAGE_RW is bit 1 while *rw only ever holds 0 or 1, so the AND is
>> always 0. *rw becomes false at the first level walked, regardless of the
>> actual permissions, and verify_rwx() treats every mapping as non-writable
>> and never reports a W^X violation.
>>
>> Accumulate NX and RW in unsigned long locals in their native bit positions
>> and store the result into the bool outputs once.
>>
>> Fixes: ceb647b4b529 ("x86/pat: Introduce lookup_address_in_pgd_attr()")
>> Assisted-by: Copilot:claude-opus-4.8
>> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
>
> Thanks for catching this.
>
> Reviewed-by: Juergen Gross <jgross@suse.com>
>
> Just one remark: instead of using additional local variables the fix could
> just look like:
>
> *rw |= !!(pXd_flags(*pXd) & _PAGE_RW);
&=, of course.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()
2026-07-16 8:37 ` Jürgen Groß
2026-07-16 8:46 ` Juergen Gross
@ 2026-07-16 9:18 ` Mike Rapoport
2026-07-16 12:35 ` David Laight
2026-07-16 13:27 ` Dave Hansen
1 sibling, 2 replies; 11+ messages in thread
From: Mike Rapoport @ 2026-07-16 9:18 UTC (permalink / raw)
To: Jürgen Groß
Cc: Dave Hansen, Andy Lutomirski, Borislav Petkov, Ingo Molnar,
Ingo Molnar, H. Peter Anvin, Peter Zijlstra, Thomas Gleixner, x86,
linux-kernel
On Thu, Jul 16, 2026 at 10:37:00AM +0200, Jürgen Groß wrote:
> On 16.07.26 10:10, Mike Rapoport (Microsoft) wrote:
> > lookup_address_in_pgd_attr() accumulates the effective NX and RW bits of
> > the walked page table levels so that verify_rwx() can detect mappings that
> > are both writable and executable.
> >
> > The RW bits are folded into a bool with
> >
> > *rw &= pXd_flags(*pXd) & _PAGE_RW;
> >
> > but _PAGE_RW is bit 1 while *rw only ever holds 0 or 1, so the AND is
> > always 0. *rw becomes false at the first level walked, regardless of the
> > actual permissions, and verify_rwx() treats every mapping as non-writable
> > and never reports a W^X violation.
> >
> > Accumulate NX and RW in unsigned long locals in their native bit positions
> > and store the result into the bool outputs once.
> >
> > Fixes: ceb647b4b529 ("x86/pat: Introduce lookup_address_in_pgd_attr()")
> > Assisted-by: Copilot:claude-opus-4.8
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
>
> Thanks for catching this.
>
> Reviewed-by: Juergen Gross <jgross@suse.com>
>
> Just one remark: instead of using additional local variables the fix could
> just look like:
>
> *rw |= !!(pXd_flags(*pXd) & _PAGE_RW);
>
> I don't really care how the issue is fixed, but this would result in less
> code.
But it will be slower :)
I instrumented cpa-test and I see ~2% improvement with additional local
variables.
> Juergen
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()
2026-07-16 9:18 ` Mike Rapoport
@ 2026-07-16 12:35 ` David Laight
2026-07-16 12:46 ` Juergen Gross
2026-07-16 13:27 ` Dave Hansen
1 sibling, 1 reply; 11+ messages in thread
From: David Laight @ 2026-07-16 12:35 UTC (permalink / raw)
To: Mike Rapoport
Cc: Jürgen Groß, Dave Hansen, Andy Lutomirski,
Borislav Petkov, Ingo Molnar, Ingo Molnar, H. Peter Anvin,
Peter Zijlstra, Thomas Gleixner, x86, linux-kernel
On Thu, 16 Jul 2026 12:18:48 +0300
Mike Rapoport <rppt@kernel.org> wrote:
> On Thu, Jul 16, 2026 at 10:37:00AM +0200, Jürgen Groß wrote:
> > On 16.07.26 10:10, Mike Rapoport (Microsoft) wrote:
> > > lookup_address_in_pgd_attr() accumulates the effective NX and RW bits of
> > > the walked page table levels so that verify_rwx() can detect mappings that
> > > are both writable and executable.
> > >
> > > The RW bits are folded into a bool with
> > >
> > > *rw &= pXd_flags(*pXd) & _PAGE_RW;
> > >
> > > but _PAGE_RW is bit 1 while *rw only ever holds 0 or 1, so the AND is
> > > always 0. *rw becomes false at the first level walked, regardless of the
> > > actual permissions, and verify_rwx() treats every mapping as non-writable
> > > and never reports a W^X violation.
> > >
> > > Accumulate NX and RW in unsigned long locals in their native bit positions
> > > and store the result into the bool outputs once.
> > >
> > > Fixes: ceb647b4b529 ("x86/pat: Introduce lookup_address_in_pgd_attr()")
> > > Assisted-by: Copilot:claude-opus-4.8
> > > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> >
> > Thanks for catching this.
> >
> > Reviewed-by: Juergen Gross <jgross@suse.com>
> >
> > Just one remark: instead of using additional local variables the fix could
> > just look like:
> >
> > *rw |= !!(pXd_flags(*pXd) & _PAGE_RW);
> >
> > I don't really care how the issue is fixed, but this would result in less
> > code.
>
> But it will be slower :)
>
> I instrumented cpa-test and I see ~2% improvement with additional local
> variables.
Does it improve further if you defer the '& _PAGE_NX' to the final assigment?
David
>
> > Juergen
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()
2026-07-16 12:35 ` David Laight
@ 2026-07-16 12:46 ` Juergen Gross
2026-07-16 12:49 ` David Laight
0 siblings, 1 reply; 11+ messages in thread
From: Juergen Gross @ 2026-07-16 12:46 UTC (permalink / raw)
To: David Laight, Mike Rapoport
Cc: Dave Hansen, Andy Lutomirski, Borislav Petkov, Ingo Molnar,
Ingo Molnar, H. Peter Anvin, Peter Zijlstra, Thomas Gleixner, x86,
linux-kernel
[-- Attachment #1.1.1: Type: text/plain, Size: 1937 bytes --]
On 16.07.26 14:35, David Laight wrote:
> On Thu, 16 Jul 2026 12:18:48 +0300
> Mike Rapoport <rppt@kernel.org> wrote:
>
>> On Thu, Jul 16, 2026 at 10:37:00AM +0200, Jürgen Groß wrote:
>>> On 16.07.26 10:10, Mike Rapoport (Microsoft) wrote:
>>>> lookup_address_in_pgd_attr() accumulates the effective NX and RW bits of
>>>> the walked page table levels so that verify_rwx() can detect mappings that
>>>> are both writable and executable.
>>>>
>>>> The RW bits are folded into a bool with
>>>>
>>>> *rw &= pXd_flags(*pXd) & _PAGE_RW;
>>>>
>>>> but _PAGE_RW is bit 1 while *rw only ever holds 0 or 1, so the AND is
>>>> always 0. *rw becomes false at the first level walked, regardless of the
>>>> actual permissions, and verify_rwx() treats every mapping as non-writable
>>>> and never reports a W^X violation.
>>>>
>>>> Accumulate NX and RW in unsigned long locals in their native bit positions
>>>> and store the result into the bool outputs once.
>>>>
>>>> Fixes: ceb647b4b529 ("x86/pat: Introduce lookup_address_in_pgd_attr()")
>>>> Assisted-by: Copilot:claude-opus-4.8
>>>> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
>>>
>>> Thanks for catching this.
>>>
>>> Reviewed-by: Juergen Gross <jgross@suse.com>
>>>
>>> Just one remark: instead of using additional local variables the fix could
>>> just look like:
>>>
>>> *rw |= !!(pXd_flags(*pXd) & _PAGE_RW);
>>>
>>> I don't really care how the issue is fixed, but this would result in less
>>> code.
>>
>> But it will be slower :)
>>
>> I instrumented cpa-test and I see ~2% improvement with additional local
>> variables.
>
> Does it improve further if you defer the '& _PAGE_NX' to the final assigment?
If so, it would probably be beneficial to have one local variable for the
logical OR of all page table entries involved (used for *nx), and one for
the logical AND of all entries (used for *rw).
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()
2026-07-16 12:46 ` Juergen Gross
@ 2026-07-16 12:49 ` David Laight
2026-07-16 12:55 ` Jürgen Groß
0 siblings, 1 reply; 11+ messages in thread
From: David Laight @ 2026-07-16 12:49 UTC (permalink / raw)
To: Juergen Gross
Cc: Mike Rapoport, Dave Hansen, Andy Lutomirski, Borislav Petkov,
Ingo Molnar, Ingo Molnar, H. Peter Anvin, Peter Zijlstra,
Thomas Gleixner, x86, linux-kernel
On Thu, 16 Jul 2026 14:46:48 +0200
Juergen Gross <jgross@suse.com> wrote:
> On 16.07.26 14:35, David Laight wrote:
> > On Thu, 16 Jul 2026 12:18:48 +0300
> > Mike Rapoport <rppt@kernel.org> wrote:
> >
> >> On Thu, Jul 16, 2026 at 10:37:00AM +0200, Jürgen Groß wrote:
> >>> On 16.07.26 10:10, Mike Rapoport (Microsoft) wrote:
> >>>> lookup_address_in_pgd_attr() accumulates the effective NX and RW bits of
> >>>> the walked page table levels so that verify_rwx() can detect mappings that
> >>>> are both writable and executable.
> >>>>
> >>>> The RW bits are folded into a bool with
> >>>>
> >>>> *rw &= pXd_flags(*pXd) & _PAGE_RW;
> >>>>
> >>>> but _PAGE_RW is bit 1 while *rw only ever holds 0 or 1, so the AND is
> >>>> always 0. *rw becomes false at the first level walked, regardless of the
> >>>> actual permissions, and verify_rwx() treats every mapping as non-writable
> >>>> and never reports a W^X violation.
> >>>>
> >>>> Accumulate NX and RW in unsigned long locals in their native bit positions
> >>>> and store the result into the bool outputs once.
> >>>>
> >>>> Fixes: ceb647b4b529 ("x86/pat: Introduce lookup_address_in_pgd_attr()")
> >>>> Assisted-by: Copilot:claude-opus-4.8
> >>>> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> >>>
> >>> Thanks for catching this.
> >>>
> >>> Reviewed-by: Juergen Gross <jgross@suse.com>
> >>>
> >>> Just one remark: instead of using additional local variables the fix could
> >>> just look like:
> >>>
> >>> *rw |= !!(pXd_flags(*pXd) & _PAGE_RW);
> >>>
> >>> I don't really care how the issue is fixed, but this would result in less
> >>> code.
> >>
> >> But it will be slower :)
> >>
> >> I instrumented cpa-test and I see ~2% improvement with additional local
> >> variables.
> >
> > Does it improve further if you defer the '& _PAGE_NX' to the final assigment?
>
> If so, it would probably be beneficial to have one local variable for the
> logical OR of all page table entries involved (used for *nx), and one for
> the logical AND of all entries (used for *rw).
Which is what the patch does...
David
>
>
> Juergen
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()
2026-07-16 12:49 ` David Laight
@ 2026-07-16 12:55 ` Jürgen Groß
0 siblings, 0 replies; 11+ messages in thread
From: Jürgen Groß @ 2026-07-16 12:55 UTC (permalink / raw)
To: David Laight
Cc: Mike Rapoport, Dave Hansen, Andy Lutomirski, Borislav Petkov,
Ingo Molnar, Ingo Molnar, H. Peter Anvin, Peter Zijlstra,
Thomas Gleixner, x86, linux-kernel
[-- Attachment #1.1.1: Type: text/plain, Size: 2647 bytes --]
On 16.07.26 14:49, David Laight wrote:
> On Thu, 16 Jul 2026 14:46:48 +0200
> Juergen Gross <jgross@suse.com> wrote:
>
>> On 16.07.26 14:35, David Laight wrote:
>>> On Thu, 16 Jul 2026 12:18:48 +0300
>>> Mike Rapoport <rppt@kernel.org> wrote:
>>>
>>>> On Thu, Jul 16, 2026 at 10:37:00AM +0200, Jürgen Groß wrote:
>>>>> On 16.07.26 10:10, Mike Rapoport (Microsoft) wrote:
>>>>>> lookup_address_in_pgd_attr() accumulates the effective NX and RW bits of
>>>>>> the walked page table levels so that verify_rwx() can detect mappings that
>>>>>> are both writable and executable.
>>>>>>
>>>>>> The RW bits are folded into a bool with
>>>>>>
>>>>>> *rw &= pXd_flags(*pXd) & _PAGE_RW;
>>>>>>
>>>>>> but _PAGE_RW is bit 1 while *rw only ever holds 0 or 1, so the AND is
>>>>>> always 0. *rw becomes false at the first level walked, regardless of the
>>>>>> actual permissions, and verify_rwx() treats every mapping as non-writable
>>>>>> and never reports a W^X violation.
>>>>>>
>>>>>> Accumulate NX and RW in unsigned long locals in their native bit positions
>>>>>> and store the result into the bool outputs once.
>>>>>>
>>>>>> Fixes: ceb647b4b529 ("x86/pat: Introduce lookup_address_in_pgd_attr()")
>>>>>> Assisted-by: Copilot:claude-opus-4.8
>>>>>> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
>>>>>
>>>>> Thanks for catching this.
>>>>>
>>>>> Reviewed-by: Juergen Gross <jgross@suse.com>
>>>>>
>>>>> Just one remark: instead of using additional local variables the fix could
>>>>> just look like:
>>>>>
>>>>> *rw |= !!(pXd_flags(*pXd) & _PAGE_RW);
>>>>>
>>>>> I don't really care how the issue is fixed, but this would result in less
>>>>> code.
>>>>
>>>> But it will be slower :)
>>>>
>>>> I instrumented cpa-test and I see ~2% improvement with additional local
>>>> variables.
>>>
>>> Does it improve further if you defer the '& _PAGE_NX' to the final assigment?
>>
>> If so, it would probably be beneficial to have one local variable for the
>> logical OR of all page table entries involved (used for *nx), and one for
>> the logical AND of all entries (used for *rw).
>
> Which is what the patch does...
Yes, of course, but you could delay masking the RW and NX bits until the final
assignment to *ret_nx and *ret_rw. At the same time I'd fetch the flags only
once per level. So something like:
unsigned long flags_ored = 0;
unsigned long flags_anded = ~0UL;
unsigned long flags;
flags = pgd_flags(*pgd)
flags_ored |= flags;
flags_anded &= flags;
...
*ret_nx = !!(flags_ored & _PAGE_NX);
*ret_rw = !!(flags_anded & _PAGE_RW);
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()
2026-07-16 9:18 ` Mike Rapoport
2026-07-16 12:35 ` David Laight
@ 2026-07-16 13:27 ` Dave Hansen
2026-07-16 13:44 ` Mike Rapoport
1 sibling, 1 reply; 11+ messages in thread
From: Dave Hansen @ 2026-07-16 13:27 UTC (permalink / raw)
To: Mike Rapoport, Jürgen Groß
Cc: Dave Hansen, Andy Lutomirski, Borislav Petkov, Ingo Molnar,
Ingo Molnar, H. Peter Anvin, Peter Zijlstra, Thomas Gleixner, x86,
linux-kernel
On 7/16/26 02:18, Mike Rapoport wrote:
>> I don't really care how the issue is fixed, but this would result in less
>> code.
> But it will be slower 🙂
What does "slower" mean here?
Does it matter in _practice_? Does it measurably slow down something an
end user might see?
Also, I thought we had tests for this gunk. But maybe we're only testing
the leaf entry permissions or something and not the upper-level
permissions. I guess we don't often muck with those so this is
relatively unlikely to have hidden real bugs.
BTW, my pre-coffee brain struggled with the changelog here:
but _PAGE_RW is bit 1 while *rw only ever holds 0 or 1, so the
Maybe it was a 0 vs. 1 bit thing, but it still took me way too long.
Could we do something like:
_PAGE_RW is 0x2. So consider the accumulation line:
rw &= pXd_flags(*pXd) & _PAGE_RW;
where rw=0x1 and the right side evaluates down to 0x2. It'll end up doing:
rw = 0x1 & 0x2
and rw always ends up 0.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()
2026-07-16 13:27 ` Dave Hansen
@ 2026-07-16 13:44 ` Mike Rapoport
2026-07-16 13:50 ` Dave Hansen
0 siblings, 1 reply; 11+ messages in thread
From: Mike Rapoport @ 2026-07-16 13:44 UTC (permalink / raw)
To: Dave Hansen
Cc: Jürgen Groß, Dave Hansen, Andy Lutomirski,
Borislav Petkov, Ingo Molnar, Ingo Molnar, H. Peter Anvin,
Peter Zijlstra, Thomas Gleixner, x86, linux-kernel
On Thu, Jul 16, 2026 at 06:27:34AM -0700, Dave Hansen wrote:
> On 7/16/26 02:18, Mike Rapoport wrote:
> >> I don't really care how the issue is fixed, but this would result in less
> >> code.
> > But it will be slower 🙂
>
> What does "slower" mean here?
I've seen ~2% difference per set_memory call in instrumented cpa-test.
> Does it matter in _practice_? Does it measurably slow down something an
> end user might see?
Yes, if a user is loading/unloading a BPF program in a tight loop :)
We can also accumulate flags and push the masking to the final assignment
like Jürgen suggested earlier in this thread:
https://lore.kernel.org/all/f0d7dbe5-5472-409c-9f61-eeefe7d08591@suse.com/
I'd prefer to keep the current structure though as I'm planning to pull
lookup_address_in_pgd_attr() to generic code and replace bit checks with
pXd_write()/pXd_exec() helpers.
> Also, I thought we had tests for this gunk. But maybe we're only testing
> the leaf entry permissions or something and not the upper-level
> permissions. I guess we don't often muck with those so this is
> relatively unlikely to have hidden real bugs.
>
> BTW, my pre-coffee brain struggled with the changelog here:
>
> but _PAGE_RW is bit 1 while *rw only ever holds 0 or 1, so the
>
> Maybe it was a 0 vs. 1 bit thing, but it still took me way too long.
> Could we do something like:
>
> _PAGE_RW is 0x2. So consider the accumulation line:
>
> rw &= pXd_flags(*pXd) & _PAGE_RW;
>
> where rw=0x1 and the right side evaluates down to 0x2. It'll end up doing:
>
> rw = 0x1 & 0x2
>
> and rw always ends up 0.
Works for me.
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr()
2026-07-16 13:44 ` Mike Rapoport
@ 2026-07-16 13:50 ` Dave Hansen
0 siblings, 0 replies; 11+ messages in thread
From: Dave Hansen @ 2026-07-16 13:50 UTC (permalink / raw)
To: Mike Rapoport
Cc: Jürgen Groß, Dave Hansen, Andy Lutomirski,
Borislav Petkov, Ingo Molnar, Ingo Molnar, H. Peter Anvin,
Peter Zijlstra, Thomas Gleixner, x86, linux-kernel
On 7/16/26 06:44, Mike Rapoport wrote:
>> What does "slower" mean here?
>
> I've seen ~2% difference per set_memory call in instrumented cpa-test.
>
>> Does it matter in _practice_? Does it measurably slow down something an
>> end user might see?
> Yes, if a user is loading/unloading a BPF program in a tight loop 🙂
IMNHO, it may be worth optimizing if you can measure the difference in
BPF program load times (even in a tight loop).
But if set_memory() is the only thing you can measure a delta in, I'm
less convinced.
It's really sounding to me like we should do the simplest thing for a
bug fix and then arm wrestle later about how to micro-optimize the sucker.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-07-16 13:50 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16 8:10 [PATCH] x86/mm/pat: fix effective RW computation in lookup_address_in_pgd_attr() Mike Rapoport (Microsoft)
2026-07-16 8:37 ` Jürgen Groß
2026-07-16 8:46 ` Juergen Gross
2026-07-16 9:18 ` Mike Rapoport
2026-07-16 12:35 ` David Laight
2026-07-16 12:46 ` Juergen Gross
2026-07-16 12:49 ` David Laight
2026-07-16 12:55 ` Jürgen Groß
2026-07-16 13:27 ` Dave Hansen
2026-07-16 13:44 ` Mike Rapoport
2026-07-16 13:50 ` Dave Hansen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.