[RFC PATCH 0/2] Introduce pte_isset for arm and arm64

[RFC PATCH 0/2] Introduce pte_isset for arm and arm64

[Steve Capper]

Long descriptors on ARM and all ptes on ARM64 are 64 bits. Some pte
attribute test functions such as pte_dirty return the bitwise-and of a
flag with the pte value. If the flag to be tested resides in the
upper 32 bits of the pte, then we run into the danger of the result
being dropped if downcast.

For example:
	gather_stats(page, md, pte_dirty(*pte), 1);
where pte_dirty(*pte) is downcast to an int.

Under ARM with LPAE, functions such as huge_pte_write also perform a
downcast to unsigned long (which is 32 bits). 

This series introduces a new macro pte_isset which performs the bitwise
and, then performs a double logical invert to ensure predictable
downcasting.

The reasoning behind a new macro was that it creates an obvious
pattern which can be followed by future changes to the pte functions.
Rather than target specific pte functions, I opted to wrap all of them
with this macro for the sake of simplicity (also the flags may move in
future).

I was toying with using (bool), but decided that the !! looked slightly
better. Under testing (with an Arndale and the Fast Model on 3.14-rc4)
both strategies worked as expected.

My compilers, gcc 4.7.3 for arm and aarch64, both optimised cases such
as "if(pte_dirty(blah))" to a simple single bit test.

Any feedback on this would be very welcome, as both my PTE_WRITE patch
for ARM, and PTE_SPECIAL introduction patch for ARM (needed for
fast_gup on ARM and ARM64) depend on this.

Thanks,
-- 
Steve

Steve Capper (2):
  arm: mm: Introduce pte_isset(pte,flag)
  arm64: Introduce pte_isset(pte,flag)

 arch/arm/include/asm/pgtable.h   | 12 +++++++-----
 arch/arm64/include/asm/pgtable.h | 13 +++++++------
 2 files changed, 14 insertions(+), 11 deletions(-)

-- 
1.8.1.4

[RFC PATCH 1/2] arm: mm: Introduce pte_isset(pte,flag)

[Steve Capper]

Long descriptors on ARM are 64 bits, and some pte functions such as
pte_dirty return a bitwise-and of a flag with the pte value. If the
flag to be tested resides in the upper 32 bits of the pte, then we run
into the danger of the result being dropped if downcast.

For example:
	gather_stats(page, md, pte_dirty(*pte), 1);
where pte_dirty(*pte) is downcast to an int.

This patch introduces a new macro pte_isset which performs the bitwise
and, then performs a double logical invert to ensure predictable
downcasting.

Signed-off-by: Steve Capper <steve.capper@linaro.org>
---
 arch/arm/include/asm/pgtable.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index 7d59b52..13626b8 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -214,12 +214,14 @@ static inline pte_t *pmd_page_vaddr(pmd_t pmd)
 
 #define pte_clear(mm,addr,ptep)	set_pte_ext(ptep, __pte(0), 0)
 
+#define pte_isset(pte, val)	(!!(pte_val(pte) & (val)))
+
 #define pte_none(pte)		(!pte_val(pte))
-#define pte_present(pte)	(pte_val(pte) & L_PTE_PRESENT)
-#define pte_write(pte)		(!(pte_val(pte) & L_PTE_RDONLY))
-#define pte_dirty(pte)		(pte_val(pte) & L_PTE_DIRTY)
-#define pte_young(pte)		(pte_val(pte) & L_PTE_YOUNG)
-#define pte_exec(pte)		(!(pte_val(pte) & L_PTE_XN))
+#define pte_present(pte)	(pte_isset((pte), L_PTE_PRESENT))
+#define pte_write(pte)		(!pte_isset((pte), L_PTE_RDONLY))
+#define pte_dirty(pte)		(pte_isset((pte), L_PTE_DIRTY))
+#define pte_young(pte)		(pte_isset((pte), L_PTE_YOUNG))
+#define pte_exec(pte)		(!pte_isset((pte), L_PTE_XN))
 #define pte_special(pte)	(0)
 
 #define pte_present_user(pte)  (pte_present(pte) && (pte_val(pte) & L_PTE_USER))
-- 
1.8.1.4

[RFC PATCH 2/2] arm64: Introduce pte_isset(pte,flag)

[Steve Capper]

Page table entries on ARM64 are 64 bits, and some pte functions such as
pte_dirty return a bitwise-and of a flag with the pte value. If the
flag to be tested resides in the upper 32 bits of the pte, then we run
into the danger of the result being dropped if downcast.

For example:
        gather_stats(page, md, pte_dirty(*pte), 1);
where pte_dirty(*pte) is downcast to an int.

This patch introduces a new macro pte_isset which performs the bitwise
and, then performs a double logical invert to ensure predictable
downcasting.

Signed-off-by: Steve Capper <steve.capper@linaro.org>
---
 arch/arm64/include/asm/pgtable.h | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index b524dcd..5130dd8 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -133,15 +133,16 @@ extern struct page *empty_zero_page;
 #define pte_unmap(pte)			do { } while (0)
 #define pte_unmap_nested(pte)		do { } while (0)
 
+#define pte_isset(pte, val)	(!!(pte_val(pte) & (val)))
 /*
  * The following only work if pte_present(). Undefined behaviour otherwise.
  */
-#define pte_present(pte)	(pte_val(pte) & (PTE_VALID | PTE_PROT_NONE))
-#define pte_dirty(pte)		(pte_val(pte) & PTE_DIRTY)
-#define pte_young(pte)		(pte_val(pte) & PTE_AF)
-#define pte_special(pte)	(pte_val(pte) & PTE_SPECIAL)
-#define pte_write(pte)		(pte_val(pte) & PTE_WRITE)
-#define pte_exec(pte)		(!(pte_val(pte) & PTE_UXN))
+#define pte_present(pte)	(pte_isset((pte), (PTE_VALID | PTE_PROT_NONE)))
+#define pte_dirty(pte)		(pte_isset((pte), PTE_DIRTY))
+#define pte_young(pte)		(pte_isset((pte), PTE_AF))
+#define pte_special(pte)	(pte_isset((pte), PTE_SPECIAL))
+#define pte_write(pte)		(pte_isset((pte), PTE_WRITE))
+#define pte_exec(pte)		(!(pte_isset((pte), PTE_UXN)))
 
 #define pte_valid_user(pte) \
 	((pte_val(pte) & (PTE_VALID | PTE_USER)) == (PTE_VALID | PTE_USER))
-- 
1.8.1.4

[RFC PATCH 0/2] Introduce pte_isset for arm and arm64

[Will Deacon]

Hi Steve,

On Mon, Feb 24, 2014 at 03:33:23PM +0000, Steve Capper wrote:
> Long descriptors on ARM and all ptes on ARM64 are 64 bits. Some pte
> attribute test functions such as pte_dirty return the bitwise-and of a
> flag with the pte value. If the flag to be tested resides in the
> upper 32 bits of the pte, then we run into the danger of the result
> being dropped if downcast.
> 
> For example:
> 	gather_stats(page, md, pte_dirty(*pte), 1);
> where pte_dirty(*pte) is downcast to an int.
> 
> Under ARM with LPAE, functions such as huge_pte_write also perform a
> downcast to unsigned long (which is 32 bits). 
> 
> This series introduces a new macro pte_isset which performs the bitwise
> and, then performs a double logical invert to ensure predictable
> downcasting.
> 
> The reasoning behind a new macro was that it creates an obvious
> pattern which can be followed by future changes to the pte functions.
> Rather than target specific pte functions, I opted to wrap all of them
> with this macro for the sake of simplicity (also the flags may move in
> future).
> 
> I was toying with using (bool), but decided that the !! looked slightly
> better. Under testing (with an Arndale and the Fast Model on 3.14-rc4)
> both strategies worked as expected.

Ok, but I still don't understand why this is needed over your previous
series. Just inlining the '!!' into the existing accessors should be fine,
no? Basically, I don't see what the new macro gains us.

Will

[RFC PATCH 0/2] Introduce pte_isset for arm and arm64

[Steve Capper]

On 24 February 2014 17:36, Will Deacon <will.deacon@arm.com> wrote:
> Hi Steve,

Hey Will,

>
> On Mon, Feb 24, 2014 at 03:33:23PM +0000, Steve Capper wrote:
>> Long descriptors on ARM and all ptes on ARM64 are 64 bits. Some pte
>> attribute test functions such as pte_dirty return the bitwise-and of a
>> flag with the pte value. If the flag to be tested resides in the
>> upper 32 bits of the pte, then we run into the danger of the result
>> being dropped if downcast.
>>
>> For example:
>>       gather_stats(page, md, pte_dirty(*pte), 1);
>> where pte_dirty(*pte) is downcast to an int.
>>
>> Under ARM with LPAE, functions such as huge_pte_write also perform a
>> downcast to unsigned long (which is 32 bits).
>>
>> This series introduces a new macro pte_isset which performs the bitwise
>> and, then performs a double logical invert to ensure predictable
>> downcasting.
>>
>> The reasoning behind a new macro was that it creates an obvious
>> pattern which can be followed by future changes to the pte functions.
>> Rather than target specific pte functions, I opted to wrap all of them
>> with this macro for the sake of simplicity (also the flags may move in
>> future).
>>
>> I was toying with using (bool), but decided that the !! looked slightly
>> better. Under testing (with an Arndale and the Fast Model on 3.14-rc4)
>> both strategies worked as expected.
>
> Ok, but I still don't understand why this is needed over your previous
> series. Just inlining the '!!' into the existing accessors should be fine,
> no? Basically, I don't see what the new macro gains us.
>

I thought it would be slightly easier to read, but it probably is
somewhat superfluous. I will just expand out to !! for all the
accessors that don't have a logical inverse already.

Cheers,
-- 
Steve