2024-04-25 13:20:54

by Claudio Imbrenda

[permalink] [raw]
Subject: [PATCH v1 0/2] s390/pgtable: misc small improvements

This series has two small improvements for the s390 page tables.

The first aligns the layout of large puds with that of large pmds;
there is no reason for the large pud read and write softbits to be
swapped compared to large pmds.

The second introduces _REGION3_ENTRY_HARDWARE_BITS_LARGE, for
completeness, which is a bitmask of the hardware bits for large puds.

Claudio Imbrenda (2):
s390/pgtable: switch read and write softbits for puds
s390/pgtable: introduce _REGION3_ENTRY_HARDWARE_BITS_LARGE

arch/s390/include/asm/pgtable.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

--
2.44.0



2024-04-25 13:21:02

by Claudio Imbrenda

[permalink] [raw]
Subject: [PATCH v1 1/2] s390/pgtable: switch read and write softbits for puds

There is no reason for the read and write softbits to be swapped in the
puds compared to pmds. They are different only because the softbits for
puds were introduced at the same time when the softbits for pmds were
swapped.

The current implementation is not wrong per se, since the macros are
defined correctly; only the documentation does not reflect reality.

With this patch, the read and write softbits for large pmd and large
puds will have the same layout, and will match the existing
documentation.

Signed-off-by: Claudio Imbrenda <[email protected]>
---
arch/s390/include/asm/pgtable.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h
index 60950e7a25f5..3da2995fd196 100644
--- a/arch/s390/include/asm/pgtable.h
+++ b/arch/s390/include/asm/pgtable.h
@@ -266,8 +266,8 @@ static inline int is_module_addr(void *addr)
#define _REGION3_ENTRY_DIRTY 0x2000 /* SW region dirty bit */
#define _REGION3_ENTRY_YOUNG 0x1000 /* SW region young bit */
#define _REGION3_ENTRY_LARGE 0x0400 /* RTTE-format control, large page */
-#define _REGION3_ENTRY_READ 0x0002 /* SW region read bit */
-#define _REGION3_ENTRY_WRITE 0x0001 /* SW region write bit */
+#define _REGION3_ENTRY_WRITE 0x0002 /* SW region write bit */
+#define _REGION3_ENTRY_READ 0x0001 /* SW region read bit */

#ifdef CONFIG_MEM_SOFT_DIRTY
#define _REGION3_ENTRY_SOFT_DIRTY 0x4000 /* SW region soft dirty bit */
--
2.44.0


2024-04-25 13:42:00

by Claudio Imbrenda

[permalink] [raw]
Subject: [PATCH v1 2/2] s390/pgtable: introduce _REGION3_ENTRY_HARDWARE_BITS_LARGE

For completeness, introduce _REGION3_ENTRY_HARDWARE_BITS_LARGE,
containing the hardware bits used for large puds.

Signed-off-by: Claudio Imbrenda <[email protected]>
---
arch/s390/include/asm/pgtable.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h
index 3da2995fd196..5c0f840eee2a 100644
--- a/arch/s390/include/asm/pgtable.h
+++ b/arch/s390/include/asm/pgtable.h
@@ -262,6 +262,7 @@ static inline int is_module_addr(void *addr)
#define _REGION3_ENTRY (_REGION_ENTRY_TYPE_R3 | _REGION_ENTRY_LENGTH)
#define _REGION3_ENTRY_EMPTY (_REGION_ENTRY_TYPE_R3 | _REGION_ENTRY_INVALID)

+#define _REGION3_ENTRY_HARDWARE_BITS_LARGE 0xffffffff8000073cUL
#define _REGION3_ENTRY_ORIGIN_LARGE ~0x7fffffffUL /* large page address */
#define _REGION3_ENTRY_DIRTY 0x2000 /* SW region dirty bit */
#define _REGION3_ENTRY_YOUNG 0x1000 /* SW region young bit */
--
2.44.0


2024-04-26 08:50:42

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] s390/pgtable: switch read and write softbits for puds

On Thu, Apr 25, 2024 at 03:05:54PM +0200, Claudio Imbrenda wrote:
> There is no reason for the read and write softbits to be swapped in the
> puds compared to pmds. They are different only because the softbits for
> puds were introduced at the same time when the softbits for pmds were
> swapped.
>
> The current implementation is not wrong per se, since the macros are
> defined correctly; only the documentation does not reflect reality.
>
> With this patch, the read and write softbits for large pmd and large
> puds will have the same layout, and will match the existing
> documentation.
>
> Signed-off-by: Claudio Imbrenda <[email protected]>
> ---
> arch/s390/include/asm/pgtable.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Heiko Carstens <[email protected]>

2024-04-26 08:57:37

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] s390/pgtable: introduce _REGION3_ENTRY_HARDWARE_BITS_LARGE

On Thu, Apr 25, 2024 at 03:05:55PM +0200, Claudio Imbrenda wrote:
> For completeness, introduce _REGION3_ENTRY_HARDWARE_BITS_LARGE,
> containing the hardware bits used for large puds.
>
> Signed-off-by: Claudio Imbrenda <[email protected]>
> ---
> arch/s390/include/asm/pgtable.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h
> index 3da2995fd196..5c0f840eee2a 100644
> --- a/arch/s390/include/asm/pgtable.h
> +++ b/arch/s390/include/asm/pgtable.h
> @@ -262,6 +262,7 @@ static inline int is_module_addr(void *addr)
> #define _REGION3_ENTRY (_REGION_ENTRY_TYPE_R3 | _REGION_ENTRY_LENGTH)
> #define _REGION3_ENTRY_EMPTY (_REGION_ENTRY_TYPE_R3 | _REGION_ENTRY_INVALID)
>
> +#define _REGION3_ENTRY_HARDWARE_BITS_LARGE 0xffffffff8000073cUL

_REGION_ENTRY_HARDWARE_BITS is missing too. :)

And this definition also raises the question if the definition of
_SEGMENT_ENTRY_HARDWARE_BITS_LARGE should be changed so it also includes
the table type bits, which it probably should.

These masks are really a bit randomly defined and assume that the
ACCF-Validity control bit is never set, and therefore the ACC bitfield can
be assumed to be software bits (and they are used as such for format 1
segment table entries).

But the ACCF bit is also a hardware bit in any case... oh well.

2024-04-26 09:23:10

by Claudio Imbrenda

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] s390/pgtable: introduce _REGION3_ENTRY_HARDWARE_BITS_LARGE

On Fri, 26 Apr 2024 10:57:14 +0200
Heiko Carstens <[email protected]> wrote:

> On Thu, Apr 25, 2024 at 03:05:55PM +0200, Claudio Imbrenda wrote:
> > For completeness, introduce _REGION3_ENTRY_HARDWARE_BITS_LARGE,
> > containing the hardware bits used for large puds.
> >
> > Signed-off-by: Claudio Imbrenda <[email protected]>
> > ---
> > arch/s390/include/asm/pgtable.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h
> > index 3da2995fd196..5c0f840eee2a 100644
> > --- a/arch/s390/include/asm/pgtable.h
> > +++ b/arch/s390/include/asm/pgtable.h
> > @@ -262,6 +262,7 @@ static inline int is_module_addr(void *addr)
> > #define _REGION3_ENTRY (_REGION_ENTRY_TYPE_R3 | _REGION_ENTRY_LENGTH)
> > #define _REGION3_ENTRY_EMPTY (_REGION_ENTRY_TYPE_R3 | _REGION_ENTRY_INVALID)
> >
> > +#define _REGION3_ENTRY_HARDWARE_BITS_LARGE 0xffffffff8000073cUL
>
> _REGION_ENTRY_HARDWARE_BITS is missing too. :)

right, I will fix it

>
> And this definition also raises the question if the definition of
> _SEGMENT_ENTRY_HARDWARE_BITS_LARGE should be changed so it also includes
> the table type bits, which it probably should.

tbh I agree

>
> These masks are really a bit randomly defined and assume that the
> ACCF-Validity control bit is never set, and therefore the ACC bitfield can
> be assumed to be software bits (and they are used as such for format 1
> segment table entries).
>
> But the ACCF bit is also a hardware bit in any case... oh well.

probably the ACCF bit should also be marked as hardware bit (I had
actually thought about it, but we don't do it for segments and I wanted
it to be consistent)

I'll send a v2 with:

segment table type bits
ACCF bit (both for segments and region3
_REGION3_ENTRY_HARDWARE_BITS