On 2/23/21 3:07 PM, Claudio Imbrenda wrote:
> Fix pgtable.h:
>
> * SEGMENT_ENTRY_SFAA had one extra bit set
> * pmd entries don't have a length
> * ipte does not need to clear the lower bits
> * pud entries should use SEGMENT_TABLE_LENGTH, as they point to segment tables
>
> Signed-off-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
> ---
> lib/s390x/asm/pgtable.h | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/lib/s390x/asm/pgtable.h b/lib/s390x/asm/pgtable.h
> index 277f3480..a2ff2d4e 100644
> --- a/lib/s390x/asm/pgtable.h
> +++ b/lib/s390x/asm/pgtable.h
> @@ -60,7 +60,7 @@
> #define SEGMENT_SHIFT 20
>
> #define SEGMENT_ENTRY_ORIGIN 0xfffffffffffff800UL
> -#define SEGMENT_ENTRY_SFAA 0xfffffffffff80000UL
> +#define SEGMENT_ENTRY_SFAA 0xfffffffffff00000UL
> #define SEGMENT_ENTRY_AV 0x0000000000010000UL
> #define SEGMENT_ENTRY_ACC 0x000000000000f000UL
> #define SEGMENT_ENTRY_F 0x0000000000000800UL
> @@ -183,7 +183,7 @@ static inline pmd_t *pmd_alloc(pud_t *pud, unsigned long addr)
> if (pud_none(*pud)) {
> pmd_t *pmd = pmd_alloc_one();
> pud_val(*pud) = __pa(pmd) | REGION_ENTRY_TT_REGION3 |
> - REGION_TABLE_LENGTH;
> + SEGMENT_TABLE_LENGTH;
@David: I'd much rather have REGION_ENTRY_LENGTH instead of
REGION_TABLE_LENGTH and SEGMENT_TABLE_LENGTH.
My argument is that this is not really an attribute of the table and
very much specific to the format of the (region table) entry. We already
have the table order as a length anyway...
Could you tell me what you had in mind when splitting this?
> }
> return pmd_offset(pud, addr);
> }
> @@ -202,15 +202,14 @@ static inline pte_t *pte_alloc(pmd_t *pmd, unsigned long addr)
> {
> if (pmd_none(*pmd)) {
> pte_t *pte = pte_alloc_one();
> - pmd_val(*pmd) = __pa(pte) | SEGMENT_ENTRY_TT_SEGMENT |
> - SEGMENT_TABLE_LENGTH;
> + pmd_val(*pmd) = __pa(pte) | SEGMENT_ENTRY_TT_SEGMENT;
Uhhhh good catch!
> }
> return pte_offset(pmd, addr);
> }
>
> static inline void ipte(unsigned long vaddr, pteval_t *p_pte)
> {
> - unsigned long table_origin = (unsigned long)p_pte & PAGE_MASK;
> + unsigned long table_origin = (unsigned long)p_pte;
>
> asm volatile(
> " ipte %0,%1\n"
>
IPTE ignores that data but having the page mask also doesn't hurt so
generally this is not a fix, right?
