2023-05-24 07:54:37

by Hongchen Zhang

[permalink] [raw]
Subject: [PATCH] LoongArch: let pmd_present return true when splitting pmd

when we split a pmd into ptes, pmd_present() and pmd_trans_huge() should
return true,otherwise it would be treated as a swap pmd.
As arm64 does in
commit b65399f6111b ("arm64/mm: Change THP helpers to comply with generic MM semantics")
we add a _PAGE_PRESENT_INVALID bit for LoongArch.

Signed-off-by: Hongchen Zhang <[email protected]>
---
arch/loongarch/include/asm/pgtable-bits.h | 2 ++
arch/loongarch/include/asm/pgtable.h | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/loongarch/include/asm/pgtable-bits.h b/arch/loongarch/include/asm/pgtable-bits.h
index 8b98d22a145b..a7469d28d9d0 100644
--- a/arch/loongarch/include/asm/pgtable-bits.h
+++ b/arch/loongarch/include/asm/pgtable-bits.h
@@ -22,12 +22,14 @@
#define _PAGE_PFN_SHIFT 12
#define _PAGE_SWP_EXCLUSIVE_SHIFT 23
#define _PAGE_PFN_END_SHIFT 48
+#define _PAGE_PRESENT_INVALID_SHIFT 60
#define _PAGE_NO_READ_SHIFT 61
#define _PAGE_NO_EXEC_SHIFT 62
#define _PAGE_RPLV_SHIFT 63

/* Used by software */
#define _PAGE_PRESENT (_ULCAST_(1) << _PAGE_PRESENT_SHIFT)
+#define _PAGE_PRESENT_INVALID (_ULCAST_(1) << _PAGE_PRESENT_INVALID_SHIFT)
#define _PAGE_WRITE (_ULCAST_(1) << _PAGE_WRITE_SHIFT)
#define _PAGE_ACCESSED (_ULCAST_(1) << _PAGE_ACCESSED_SHIFT)
#define _PAGE_MODIFIED (_ULCAST_(1) << _PAGE_MODIFIED_SHIFT)
diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
index d28fb9dbec59..9a9f9ff9b709 100644
--- a/arch/loongarch/include/asm/pgtable.h
+++ b/arch/loongarch/include/asm/pgtable.h
@@ -213,7 +213,7 @@ static inline int pmd_bad(pmd_t pmd)
static inline int pmd_present(pmd_t pmd)
{
if (unlikely(pmd_val(pmd) & _PAGE_HUGE))
- return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE));
+ return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PRESENT_INVALID));

return pmd_val(pmd) != (unsigned long)invalid_pte_table;
}
@@ -558,6 +558,7 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)

static inline pmd_t pmd_mkinvalid(pmd_t pmd)
{
+ pmd_val(pmd) |= _PAGE_PRESENT_INVALID;
pmd_val(pmd) &= ~(_PAGE_PRESENT | _PAGE_VALID | _PAGE_DIRTY | _PAGE_PROTNONE);

return pmd;

base-commit: f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
--
2.31.1



2023-05-27 05:59:07

by Huacai Chen

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: let pmd_present return true when splitting pmd

Hi, Anshuman,

Excuse me, but could you please tell me something that why commit
b65399f6111b ("arm64/mm: Change THP helpers to comply with generic MM
semantics") needn't be backported to 5.4LTS and 4.19LTS series. The
discussion link you referred is as early as in 2018, before 4.19
released. I think this information is important for us because we want
to know how to handle our 4.19-loongarch codebase.

Huacai

On Wed, May 24, 2023 at 3:42 PM Hongchen Zhang
<[email protected]> wrote:
>
> when we split a pmd into ptes, pmd_present() and pmd_trans_huge() should
> return true,otherwise it would be treated as a swap pmd.
> As arm64 does in
> commit b65399f6111b ("arm64/mm: Change THP helpers to comply with generic MM semantics")
> we add a _PAGE_PRESENT_INVALID bit for LoongArch.
>
> Signed-off-by: Hongchen Zhang <[email protected]>
> ---
> arch/loongarch/include/asm/pgtable-bits.h | 2 ++
> arch/loongarch/include/asm/pgtable.h | 3 ++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/loongarch/include/asm/pgtable-bits.h b/arch/loongarch/include/asm/pgtable-bits.h
> index 8b98d22a145b..a7469d28d9d0 100644
> --- a/arch/loongarch/include/asm/pgtable-bits.h
> +++ b/arch/loongarch/include/asm/pgtable-bits.h
> @@ -22,12 +22,14 @@
> #define _PAGE_PFN_SHIFT 12
> #define _PAGE_SWP_EXCLUSIVE_SHIFT 23
> #define _PAGE_PFN_END_SHIFT 48
> +#define _PAGE_PRESENT_INVALID_SHIFT 60
> #define _PAGE_NO_READ_SHIFT 61
> #define _PAGE_NO_EXEC_SHIFT 62
> #define _PAGE_RPLV_SHIFT 63
>
> /* Used by software */
> #define _PAGE_PRESENT (_ULCAST_(1) << _PAGE_PRESENT_SHIFT)
> +#define _PAGE_PRESENT_INVALID (_ULCAST_(1) << _PAGE_PRESENT_INVALID_SHIFT)
> #define _PAGE_WRITE (_ULCAST_(1) << _PAGE_WRITE_SHIFT)
> #define _PAGE_ACCESSED (_ULCAST_(1) << _PAGE_ACCESSED_SHIFT)
> #define _PAGE_MODIFIED (_ULCAST_(1) << _PAGE_MODIFIED_SHIFT)
> diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
> index d28fb9dbec59..9a9f9ff9b709 100644
> --- a/arch/loongarch/include/asm/pgtable.h
> +++ b/arch/loongarch/include/asm/pgtable.h
> @@ -213,7 +213,7 @@ static inline int pmd_bad(pmd_t pmd)
> static inline int pmd_present(pmd_t pmd)
> {
> if (unlikely(pmd_val(pmd) & _PAGE_HUGE))
> - return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE));
> + return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PRESENT_INVALID));
>
> return pmd_val(pmd) != (unsigned long)invalid_pte_table;
> }
> @@ -558,6 +558,7 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>
> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
> {
> + pmd_val(pmd) |= _PAGE_PRESENT_INVALID;
> pmd_val(pmd) &= ~(_PAGE_PRESENT | _PAGE_VALID | _PAGE_DIRTY | _PAGE_PROTNONE);
>
> return pmd;
>
> base-commit: f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
> --
> 2.31.1
>
>

2023-06-06 11:44:18

by Hongchen Zhang

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: let pmd_present return true when splitting pmd

Hi,

Gentle ping.

On 2023/5/24 pm 3:41, Hongchen Zhang wrote:
> when we split a pmd into ptes, pmd_present() and pmd_trans_huge() should
> return true,otherwise it would be treated as a swap pmd.
> As arm64 does in
> commit b65399f6111b ("arm64/mm: Change THP helpers to comply with generic MM semantics")
> we add a _PAGE_PRESENT_INVALID bit for LoongArch.
>
> Signed-off-by: Hongchen Zhang <[email protected]>
> ---
> arch/loongarch/include/asm/pgtable-bits.h | 2 ++
> arch/loongarch/include/asm/pgtable.h | 3 ++-
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/loongarch/include/asm/pgtable-bits.h b/arch/loongarch/include/asm/pgtable-bits.h
> index 8b98d22a145b..a7469d28d9d0 100644
> --- a/arch/loongarch/include/asm/pgtable-bits.h
> +++ b/arch/loongarch/include/asm/pgtable-bits.h
> @@ -22,12 +22,14 @@
> #define _PAGE_PFN_SHIFT 12
> #define _PAGE_SWP_EXCLUSIVE_SHIFT 23
> #define _PAGE_PFN_END_SHIFT 48
> +#define _PAGE_PRESENT_INVALID_SHIFT 60
> #define _PAGE_NO_READ_SHIFT 61
> #define _PAGE_NO_EXEC_SHIFT 62
> #define _PAGE_RPLV_SHIFT 63
>
> /* Used by software */
> #define _PAGE_PRESENT (_ULCAST_(1) << _PAGE_PRESENT_SHIFT)
> +#define _PAGE_PRESENT_INVALID (_ULCAST_(1) << _PAGE_PRESENT_INVALID_SHIFT)
> #define _PAGE_WRITE (_ULCAST_(1) << _PAGE_WRITE_SHIFT)
> #define _PAGE_ACCESSED (_ULCAST_(1) << _PAGE_ACCESSED_SHIFT)
> #define _PAGE_MODIFIED (_ULCAST_(1) << _PAGE_MODIFIED_SHIFT)
> diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
> index d28fb9dbec59..9a9f9ff9b709 100644
> --- a/arch/loongarch/include/asm/pgtable.h
> +++ b/arch/loongarch/include/asm/pgtable.h
> @@ -213,7 +213,7 @@ static inline int pmd_bad(pmd_t pmd)
> static inline int pmd_present(pmd_t pmd)
> {
> if (unlikely(pmd_val(pmd) & _PAGE_HUGE))
> - return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE));
> + return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PRESENT_INVALID));
>
> return pmd_val(pmd) != (unsigned long)invalid_pte_table;
> }
> @@ -558,6 +558,7 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>
> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
> {
> + pmd_val(pmd) |= _PAGE_PRESENT_INVALID;
> pmd_val(pmd) &= ~(_PAGE_PRESENT | _PAGE_VALID | _PAGE_DIRTY | _PAGE_PROTNONE);
>
> return pmd;
>
> base-commit: f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
>

Best Regards
Hongchen Zhang


2023-06-06 14:02:42

by Bibo Mao

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: let pmd_present return true when splitting pmd

I do not object to adding sw bit for _PAGE_PRESENT_INVALID, only that can it use
bit12--bit20 since it is for pmd entry only?

Regards
Bibo, Mao

在 2023/6/6 19:38, Hongchen Zhang 写道:
> Hi,
>
> Gentle ping.
>
> On 2023/5/24 pm 3:41, Hongchen Zhang wrote:
>> when we split a pmd into ptes, pmd_present() and pmd_trans_huge() should
>> return true,otherwise it would be treated as a swap pmd.
>> As arm64 does in
>> commit b65399f6111b ("arm64/mm: Change THP helpers to comply with generic MM semantics")
>> we add a _PAGE_PRESENT_INVALID bit for LoongArch.
>>
>> Signed-off-by: Hongchen Zhang <[email protected]>
>> ---
>>   arch/loongarch/include/asm/pgtable-bits.h | 2 ++
>>   arch/loongarch/include/asm/pgtable.h      | 3 ++-
>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/loongarch/include/asm/pgtable-bits.h b/arch/loongarch/include/asm/pgtable-bits.h
>> index 8b98d22a145b..a7469d28d9d0 100644
>> --- a/arch/loongarch/include/asm/pgtable-bits.h
>> +++ b/arch/loongarch/include/asm/pgtable-bits.h
>> @@ -22,12 +22,14 @@
>>   #define    _PAGE_PFN_SHIFT        12
>>   #define    _PAGE_SWP_EXCLUSIVE_SHIFT 23
>>   #define    _PAGE_PFN_END_SHIFT    48
>> +#define _PAGE_PRESENT_INVALID_SHIFT 60
>>   #define    _PAGE_NO_READ_SHIFT    61
>>   #define    _PAGE_NO_EXEC_SHIFT    62
>>   #define    _PAGE_RPLV_SHIFT    63
>>     /* Used by software */
>>   #define _PAGE_PRESENT        (_ULCAST_(1) << _PAGE_PRESENT_SHIFT)
>> +#define _PAGE_PRESENT_INVALID    (_ULCAST_(1) << _PAGE_PRESENT_INVALID_SHIFT)
>>   #define _PAGE_WRITE        (_ULCAST_(1) << _PAGE_WRITE_SHIFT)
>>   #define _PAGE_ACCESSED        (_ULCAST_(1) << _PAGE_ACCESSED_SHIFT)
>>   #define _PAGE_MODIFIED        (_ULCAST_(1) << _PAGE_MODIFIED_SHIFT)
>> diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
>> index d28fb9dbec59..9a9f9ff9b709 100644
>> --- a/arch/loongarch/include/asm/pgtable.h
>> +++ b/arch/loongarch/include/asm/pgtable.h
>> @@ -213,7 +213,7 @@ static inline int pmd_bad(pmd_t pmd)
>>   static inline int pmd_present(pmd_t pmd)
>>   {
>>       if (unlikely(pmd_val(pmd) & _PAGE_HUGE))
>> -        return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE));
>> +        return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PRESENT_INVALID));
>>         return pmd_val(pmd) != (unsigned long)invalid_pte_table;
>>   }
>> @@ -558,6 +558,7 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>     static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>>   {
>> +    pmd_val(pmd) |= _PAGE_PRESENT_INVALID;
>>       pmd_val(pmd) &= ~(_PAGE_PRESENT | _PAGE_VALID | _PAGE_DIRTY | _PAGE_PROTNONE);
>>         return pmd;
>>
>> base-commit: f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
>>
>
> Best Regards
> Hongchen Zhang
>
> _______________________________________________
> Loongson-kernel mailing list -- [email protected]
> To unsubscribe send an email to [email protected]


2023-06-07 03:26:57

by Bibo Mao

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: let pmd_present return true when splitting pmd

I suggest to using bit12-bit20 since high weight bits such 63-61 are hw bits, it may
expand for future hw use, or you had better negotiate with hw guys.

>>> +#define _PAGE_PRESENT_INVALID_SHIFT 60
>>> #define _PAGE_NO_READ_SHIFT 61
>>> #define _PAGE_NO_EXEC_SHIFT 62
>>> #define _PAGE_RPLV_SHIFT 63

Regards
Bibo, Mao

在 2023/6/6 21:49, bibo, mao 写道:
> I do not object to adding sw bit for _PAGE_PRESENT_INVALID, only that can it use
> bit12--bit20 since it is for pmd entry only?
>
> Regards
> Bibo, Mao
>
> 在 2023/6/6 19:38, Hongchen Zhang 写道:
>> Hi,
>>
>> Gentle ping.
>>
>> On 2023/5/24 pm 3:41, Hongchen Zhang wrote:
>>> when we split a pmd into ptes, pmd_present() and pmd_trans_huge() should
>>> return true,otherwise it would be treated as a swap pmd.
>>> As arm64 does in
>>> commit b65399f6111b ("arm64/mm: Change THP helpers to comply with generic MM semantics")
>>> we add a _PAGE_PRESENT_INVALID bit for LoongArch.
>>>
>>> Signed-off-by: Hongchen Zhang <[email protected]>
>>> ---
>>>   arch/loongarch/include/asm/pgtable-bits.h | 2 ++
>>>   arch/loongarch/include/asm/pgtable.h      | 3 ++-
>>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/loongarch/include/asm/pgtable-bits.h b/arch/loongarch/include/asm/pgtable-bits.h
>>> index 8b98d22a145b..a7469d28d9d0 100644
>>> --- a/arch/loongarch/include/asm/pgtable-bits.h
>>> +++ b/arch/loongarch/include/asm/pgtable-bits.h
>>> @@ -22,12 +22,14 @@
>>>   #define    _PAGE_PFN_SHIFT        12
>>>   #define    _PAGE_SWP_EXCLUSIVE_SHIFT 23
>>>   #define    _PAGE_PFN_END_SHIFT    48
>>> +#define _PAGE_PRESENT_INVALID_SHIFT 60
>>>   #define    _PAGE_NO_READ_SHIFT    61
>>>   #define    _PAGE_NO_EXEC_SHIFT    62
>>>   #define    _PAGE_RPLV_SHIFT    63
>>>     /* Used by software */
>>>   #define _PAGE_PRESENT        (_ULCAST_(1) << _PAGE_PRESENT_SHIFT)
>>> +#define _PAGE_PRESENT_INVALID    (_ULCAST_(1) << _PAGE_PRESENT_INVALID_SHIFT)
>>>   #define _PAGE_WRITE        (_ULCAST_(1) << _PAGE_WRITE_SHIFT)
>>>   #define _PAGE_ACCESSED        (_ULCAST_(1) << _PAGE_ACCESSED_SHIFT)
>>>   #define _PAGE_MODIFIED        (_ULCAST_(1) << _PAGE_MODIFIED_SHIFT)
>>> diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
>>> index d28fb9dbec59..9a9f9ff9b709 100644
>>> --- a/arch/loongarch/include/asm/pgtable.h
>>> +++ b/arch/loongarch/include/asm/pgtable.h
>>> @@ -213,7 +213,7 @@ static inline int pmd_bad(pmd_t pmd)
>>>   static inline int pmd_present(pmd_t pmd)
>>>   {
>>>       if (unlikely(pmd_val(pmd) & _PAGE_HUGE))
>>> -        return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE));
>>> +        return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PRESENT_INVALID));
>>>         return pmd_val(pmd) != (unsigned long)invalid_pte_table;
>>>   }
>>> @@ -558,6 +558,7 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>>     static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>>>   {
>>> +    pmd_val(pmd) |= _PAGE_PRESENT_INVALID;
>>>       pmd_val(pmd) &= ~(_PAGE_PRESENT | _PAGE_VALID | _PAGE_DIRTY | _PAGE_PROTNONE);
>>>         return pmd;
>>>
>>> base-commit: f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
>>>
>>
>> Best Regards
>> Hongchen Zhang
>>
>> _______________________________________________
>> Loongson-kernel mailing list -- [email protected]
>> To unsubscribe send an email to [email protected]


2023-06-08 01:58:08

by Hongchen Zhang

[permalink] [raw]
Subject: Re: Re: [PATCH] LoongArch: let pmd_present return true when splitting pmd

Hi Bibo & Anshuman,

On 2023/6/6 下午9:49, bibo, mao wrote:
> I do not object to adding sw bit for _PAGE_PRESENT_INVALID, only that can it use
> bit12--bit20 since it is for pmd entry only?
If we change from bit60 to bit12, pmd_pfn will return an incorrect pfn,
and it seems that modifying pmd_pfn accordingly is not a good idea.

Hi Anshuman,
What's your opinion about this suggestion? I think arm64 architecture
has similar problems. Why do you choose bit59 instead of bit12--bit20?
>
> Regards
> Bibo, Mao
>
> 在 2023/6/6 19:38, Hongchen Zhang 写道:
>> Hi,
>>
>> Gentle ping.
>>
>> On 2023/5/24 pm 3:41, Hongchen Zhang wrote:
>>> when we split a pmd into ptes, pmd_present() and pmd_trans_huge() should
>>> return true,otherwise it would be treated as a swap pmd.
>>> As arm64 does in
>>> commit b65399f6111b ("arm64/mm: Change THP helpers to comply with generic MM semantics")
>>> we add a _PAGE_PRESENT_INVALID bit for LoongArch.
>>>
>>> Signed-off-by: Hongchen Zhang <[email protected]>
>>> ---
>>>   arch/loongarch/include/asm/pgtable-bits.h | 2 ++
>>>   arch/loongarch/include/asm/pgtable.h      | 3 ++-
>>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/loongarch/include/asm/pgtable-bits.h b/arch/loongarch/include/asm/pgtable-bits.h
>>> index 8b98d22a145b..a7469d28d9d0 100644
>>> --- a/arch/loongarch/include/asm/pgtable-bits.h
>>> +++ b/arch/loongarch/include/asm/pgtable-bits.h
>>> @@ -22,12 +22,14 @@
>>>   #define    _PAGE_PFN_SHIFT        12
>>>   #define    _PAGE_SWP_EXCLUSIVE_SHIFT 23
>>>   #define    _PAGE_PFN_END_SHIFT    48
>>> +#define _PAGE_PRESENT_INVALID_SHIFT 60
>>>   #define    _PAGE_NO_READ_SHIFT    61
>>>   #define    _PAGE_NO_EXEC_SHIFT    62
>>>   #define    _PAGE_RPLV_SHIFT    63
>>>     /* Used by software */
>>>   #define _PAGE_PRESENT        (_ULCAST_(1) << _PAGE_PRESENT_SHIFT)
>>> +#define _PAGE_PRESENT_INVALID    (_ULCAST_(1) << _PAGE_PRESENT_INVALID_SHIFT)
>>>   #define _PAGE_WRITE        (_ULCAST_(1) << _PAGE_WRITE_SHIFT)
>>>   #define _PAGE_ACCESSED        (_ULCAST_(1) << _PAGE_ACCESSED_SHIFT)
>>>   #define _PAGE_MODIFIED        (_ULCAST_(1) << _PAGE_MODIFIED_SHIFT)
>>> diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
>>> index d28fb9dbec59..9a9f9ff9b709 100644
>>> --- a/arch/loongarch/include/asm/pgtable.h
>>> +++ b/arch/loongarch/include/asm/pgtable.h
>>> @@ -213,7 +213,7 @@ static inline int pmd_bad(pmd_t pmd)
>>>   static inline int pmd_present(pmd_t pmd)
>>>   {
>>>       if (unlikely(pmd_val(pmd) & _PAGE_HUGE))
>>> -        return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE));
>>> +        return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PRESENT_INVALID));
>>>         return pmd_val(pmd) != (unsigned long)invalid_pte_table;
>>>   }
>>> @@ -558,6 +558,7 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>>     static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>>>   {
>>> +    pmd_val(pmd) |= _PAGE_PRESENT_INVALID;
>>>       pmd_val(pmd) &= ~(_PAGE_PRESENT | _PAGE_VALID | _PAGE_DIRTY | _PAGE_PROTNONE);
>>>         return pmd;
>>>
>>> base-commit: f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
>>>
>>
>> Best Regards
>> Hongchen Zhang
>>
>> _______________________________________________
>> Loongson-kernel mailing list -- [email protected]
>> To unsubscribe send an email to [email protected]
>
>


2023-06-08 02:23:13

by Hongchen Zhang

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: let pmd_present return true when splitting pmd

Hi Anshuman,

On 2023/5/27 下午1:32, Huacai Chen wrote:
> Hi, Anshuman,
>
> Excuse me, but could you please tell me something that why commit
> b65399f6111b ("arm64/mm: Change THP helpers to comply with generic MM
> semantics") needn't be backported to 5.4LTS and 4.19LTS series. The
> discussion link you referred is as early as in 2018, before 4.19
> released. I think this information is important for us because we want
> to know how to handle our 4.19-loongarch codebase.

What's your opinion about huacai's question? In my opinion,your commit
should be backported to 5.4LTS and 4.19LTS.

>
> Huacai
>
> On Wed, May 24, 2023 at 3:42 PM Hongchen Zhang
> <[email protected]> wrote:
>>
>> when we split a pmd into ptes, pmd_present() and pmd_trans_huge() should
>> return true,otherwise it would be treated as a swap pmd.
>> As arm64 does in
>> commit b65399f6111b ("arm64/mm: Change THP helpers to comply with generic MM semantics")
>> we add a _PAGE_PRESENT_INVALID bit for LoongArch.
>>
>> Signed-off-by: Hongchen Zhang <[email protected]>
>> ---
>> arch/loongarch/include/asm/pgtable-bits.h | 2 ++
>> arch/loongarch/include/asm/pgtable.h | 3 ++-
>> 2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/loongarch/include/asm/pgtable-bits.h b/arch/loongarch/include/asm/pgtable-bits.h
>> index 8b98d22a145b..a7469d28d9d0 100644
>> --- a/arch/loongarch/include/asm/pgtable-bits.h
>> +++ b/arch/loongarch/include/asm/pgtable-bits.h
>> @@ -22,12 +22,14 @@
>> #define _PAGE_PFN_SHIFT 12
>> #define _PAGE_SWP_EXCLUSIVE_SHIFT 23
>> #define _PAGE_PFN_END_SHIFT 48
>> +#define _PAGE_PRESENT_INVALID_SHIFT 60
>> #define _PAGE_NO_READ_SHIFT 61
>> #define _PAGE_NO_EXEC_SHIFT 62
>> #define _PAGE_RPLV_SHIFT 63
>>
>> /* Used by software */
>> #define _PAGE_PRESENT (_ULCAST_(1) << _PAGE_PRESENT_SHIFT)
>> +#define _PAGE_PRESENT_INVALID (_ULCAST_(1) << _PAGE_PRESENT_INVALID_SHIFT)
>> #define _PAGE_WRITE (_ULCAST_(1) << _PAGE_WRITE_SHIFT)
>> #define _PAGE_ACCESSED (_ULCAST_(1) << _PAGE_ACCESSED_SHIFT)
>> #define _PAGE_MODIFIED (_ULCAST_(1) << _PAGE_MODIFIED_SHIFT)
>> diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
>> index d28fb9dbec59..9a9f9ff9b709 100644
>> --- a/arch/loongarch/include/asm/pgtable.h
>> +++ b/arch/loongarch/include/asm/pgtable.h
>> @@ -213,7 +213,7 @@ static inline int pmd_bad(pmd_t pmd)
>> static inline int pmd_present(pmd_t pmd)
>> {
>> if (unlikely(pmd_val(pmd) & _PAGE_HUGE))
>> - return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE));
>> + return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PRESENT_INVALID));
>>
>> return pmd_val(pmd) != (unsigned long)invalid_pte_table;
>> }
>> @@ -558,6 +558,7 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>
>> static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>> {
>> + pmd_val(pmd) |= _PAGE_PRESENT_INVALID;
>> pmd_val(pmd) &= ~(_PAGE_PRESENT | _PAGE_VALID | _PAGE_DIRTY | _PAGE_PROTNONE);
>>
>> return pmd;
>>
>> base-commit: f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
>> --
>> 2.31.1
>>
>>

Best Regards
Hongchen Zhang


2023-06-08 02:33:59

by Bibo Mao

[permalink] [raw]
Subject: Re: [PATCH] LoongArch: let pmd_present return true when splitting pmd



在 2023/6/8 09:31, Hongchen Zhang 写道:
> Hi Bibo & Anshuman,
>
> On 2023/6/6 下午9:49, bibo, mao wrote:
>> I do not object to adding sw bit for _PAGE_PRESENT_INVALID,  only that can it use
>> bit12--bit20 since it is for pmd entry only?
> If we change from bit60 to bit12, pmd_pfn will return an incorrect pfn, and it seems that modifying pmd_pfn accordingly is not a good idea.
Good catch. Do you test it or just doubt about it by reviewing the code? bit 12 _PAGE_HGLOBAL_SHIFT is already used by pmd entry. There should be problem also.

if you worry about it, pmd_pfn should be something like x86
static inline pmdval_t pmd_pfn_mask(pmd_t pmd)
{
if (native_pmd_val(pmd) & _PAGE_PSE)
return PHYSICAL_PMD_PAGE_MASK;
else
return PTE_PFN_MASK;
}

However I think forcefully using bit 60 is not good way. It solves one issue and brings out another problem.

Regards
Bibo, Mao

>
> Hi Anshuman,
>   What's your opinion about this suggestion? I think arm64 architecture  has similar problems. Why do you choose bit59 instead of bit12--bit20?
>>
>> Regards
>> Bibo, Mao
>>
>> 在 2023/6/6 19:38, Hongchen Zhang 写道:
>>> Hi,
>>>
>>> Gentle ping.
>>>
>>> On 2023/5/24 pm 3:41, Hongchen Zhang wrote:
>>>> when we split a pmd into ptes, pmd_present() and pmd_trans_huge() should
>>>> return true,otherwise it would be treated as a swap pmd.
>>>> As arm64 does in
>>>> commit b65399f6111b ("arm64/mm: Change THP helpers to comply with generic MM semantics")
>>>> we add a _PAGE_PRESENT_INVALID bit for LoongArch.
>>>>
>>>> Signed-off-by: Hongchen Zhang <[email protected]>
>>>> ---
>>>>    arch/loongarch/include/asm/pgtable-bits.h | 2 ++
>>>>    arch/loongarch/include/asm/pgtable.h      | 3 ++-
>>>>    2 files changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/loongarch/include/asm/pgtable-bits.h b/arch/loongarch/include/asm/pgtable-bits.h
>>>> index 8b98d22a145b..a7469d28d9d0 100644
>>>> --- a/arch/loongarch/include/asm/pgtable-bits.h
>>>> +++ b/arch/loongarch/include/asm/pgtable-bits.h
>>>> @@ -22,12 +22,14 @@
>>>>    #define    _PAGE_PFN_SHIFT        12
>>>>    #define    _PAGE_SWP_EXCLUSIVE_SHIFT 23
>>>>    #define    _PAGE_PFN_END_SHIFT    48
>>>> +#define _PAGE_PRESENT_INVALID_SHIFT 60
>>>>    #define    _PAGE_NO_READ_SHIFT    61
>>>>    #define    _PAGE_NO_EXEC_SHIFT    62
>>>>    #define    _PAGE_RPLV_SHIFT    63
>>>>      /* Used by software */
>>>>    #define _PAGE_PRESENT        (_ULCAST_(1) << _PAGE_PRESENT_SHIFT)
>>>> +#define _PAGE_PRESENT_INVALID    (_ULCAST_(1) << _PAGE_PRESENT_INVALID_SHIFT)
>>>>    #define _PAGE_WRITE        (_ULCAST_(1) << _PAGE_WRITE_SHIFT)
>>>>    #define _PAGE_ACCESSED        (_ULCAST_(1) << _PAGE_ACCESSED_SHIFT)
>>>>    #define _PAGE_MODIFIED        (_ULCAST_(1) << _PAGE_MODIFIED_SHIFT)
>>>> diff --git a/arch/loongarch/include/asm/pgtable.h b/arch/loongarch/include/asm/pgtable.h
>>>> index d28fb9dbec59..9a9f9ff9b709 100644
>>>> --- a/arch/loongarch/include/asm/pgtable.h
>>>> +++ b/arch/loongarch/include/asm/pgtable.h
>>>> @@ -213,7 +213,7 @@ static inline int pmd_bad(pmd_t pmd)
>>>>    static inline int pmd_present(pmd_t pmd)
>>>>    {
>>>>        if (unlikely(pmd_val(pmd) & _PAGE_HUGE))
>>>> -        return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE));
>>>> +        return !!(pmd_val(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PRESENT_INVALID));
>>>>          return pmd_val(pmd) != (unsigned long)invalid_pte_table;
>>>>    }
>>>> @@ -558,6 +558,7 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>>>>      static inline pmd_t pmd_mkinvalid(pmd_t pmd)
>>>>    {
>>>> +    pmd_val(pmd) |= _PAGE_PRESENT_INVALID;
>>>>        pmd_val(pmd) &= ~(_PAGE_PRESENT | _PAGE_VALID | _PAGE_DIRTY | _PAGE_PROTNONE);
>>>>          return pmd;
>>>>
>>>> base-commit: f1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
>>>>
>>>
>>> Best Regards
>>> Hongchen Zhang
>>>
>>> _______________________________________________
>>> Loongson-kernel mailing list -- [email protected]
>>> To unsubscribe send an email to [email protected]
>>
>>
>
> _______________________________________________
> Loongson-kernel mailing list -- [email protected]
> To unsubscribe send an email to [email protected]