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

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

[Claudio Imbrenda]

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

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

[Claudio Imbrenda]

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 <imbrenda@linux.ibm.com>
---
 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

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

[Heiko Carstens]

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 <imbrenda@linux.ibm.com>
> ---
>  arch/s390/include/asm/pgtable.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Heiko Carstens <hca@linux.ibm.com>

[PATCH v1 2/2] s390/pgtable: introduce _REGION3_ENTRY_HARDWARE_BITS_LARGE

[Claudio Imbrenda]

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

Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
 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

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

[Heiko Carstens]

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 <imbrenda@linux.ibm.com>
> ---
>  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.

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

[Claudio Imbrenda]

On Fri, 26 Apr 2024 10:57:14 +0200
Heiko Carstens <hca@linux.ibm.com> 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 <imbrenda@linux.ibm.com>
> > ---
> >  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