Skip to contentSkip to conversation

arm64: move the PTE_VALID to pgtable-hwdef.h

PATCH New Huang Shijie 2 participants 5 messages May 30, 2016 – Jun 3, 2016 arm64

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 arm64: move the PTE_VALID to pgtable-hwdef.h

The PTE_VALID is used to check whether the page descriptor is valid. It's not a software defined PTE bits, such as PTE_WRITE/PTE_DIRTY.

So move it to the proper header file: pgtable-hwdef.h.

---
 arch/arm64/include/asm/pgtable-hwdef.h | 1 +
 arch/arm64/include/asm/pgtable-prot.h  | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)
arch/arm64/include/asm/pgtable-hwdef.h +1 -0
index 2813748..eeb3269 100644
@@ -154,6 +154,7 @@
 #define PTE_TYPE_MASK		(_AT(pteval_t, 3) << 0)
 #define PTE_TYPE_FAULT		(_AT(pteval_t, 0) << 0)
 #define PTE_TYPE_PAGE		(_AT(pteval_t, 3) << 0)
+#define PTE_VALID		(_AT(pteval_t, 1) << 0)
 #define PTE_TABLE_BIT		(_AT(pteval_t, 1) << 1)
 #define PTE_USER		(_AT(pteval_t, 1) << 6)		/* AP[1] */
 #define PTE_RDONLY		(_AT(pteval_t, 1) << 7)		/* AP[2] */
arch/arm64/include/asm/pgtable-prot.h +0 -1
index 29fcb33..5ac71f4 100644
@@ -24,7 +24,6 @@
 /*
  * Software defined PTE bits definition.
  */
-#define PTE_VALID		(_AT(pteval_t, 1) << 0)
 #define PTE_WRITE		(PTE_DBM)		 /* same as DBM (51) */
 #define PTE_DIRTY		(_AT(pteval_t, 1) << 55)
 #define PTE_SPECIAL		(_AT(pteval_t, 1) << 56)
Signature · 1 line
2.5.5
#1 lore raw

PATCH 2/3 arm64: fix the wrong type

We should use the pudval_t for the PUD_* macros, not the pgdval_t. This patch fixes them.

---
 arch/arm64/include/asm/pgtable-hwdef.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/arm64/include/asm/pgtable-hwdef.h +3 -3
index eeb3269..54e2fbf 100644
@@ -116,9 +116,9 @@
  * Level 1 descriptor (PUD).
  */
 #define PUD_TYPE_TABLE		(_AT(pudval_t, 3) << 0)
-#define PUD_TABLE_BIT		(_AT(pgdval_t, 1) << 1)
-#define PUD_TYPE_MASK		(_AT(pgdval_t, 3) << 0)
-#define PUD_TYPE_SECT		(_AT(pgdval_t, 1) << 0)
+#define PUD_TABLE_BIT		(_AT(pudval_t, 1) << 1)
+#define PUD_TYPE_MASK		(_AT(pudval_t, 3) << 0)
+#define PUD_TYPE_SECT		(_AT(pudval_t, 1) << 0)
 
 /*
  * Level 2 descriptor (PMD).
Signature · 1 line
2.5.5
#2 lore raw

PATCH 3/3 arm64: add a new macro for the pgd table descriptor

In reply to Huang Shijie

Add a new macro for the pgd table descriptor to define the "Table" type, and use it to replace the numerical constant in the pgd_bad().

This patch makes the code more legible.

---
 arch/arm64/include/asm/pgtable-hwdef.h | 5 +++++
 arch/arm64/include/asm/pgtable.h       | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)
arch/arm64/include/asm/pgtable-hwdef.h +5 -0
index 54e2fbf..f648c28 100644
@@ -113,6 +113,11 @@
 /*
  * Hardware page table definitions.
  *
+ * Level 0 descriptor (PGD).
+ */
+#define PGD_TABLE_BIT		(_AT(pgdval_t, 1) << 1)
+
+/*
  * Level 1 descriptor (PUD).
  */
 #define PUD_TYPE_TABLE		(_AT(pudval_t, 3) << 0)
arch/arm64/include/asm/pgtable.h +1 -1
index 46472a9..bfa0092 100644
@@ -453,7 +453,7 @@ static inline phys_addr_t pud_page_paddr(pud_t pud)
 #define pud_ERROR(pud)		__pud_error(__FILE__, __LINE__, pud_val(pud))
 
 #define pgd_none(pgd)		(!pgd_val(pgd))
-#define pgd_bad(pgd)		(!(pgd_val(pgd) & 2))
+#define pgd_bad(pgd)		(!(pgd_val(pgd) & PGD_TABLE_BIT))
 #define pgd_present(pgd)	(pgd_val(pgd))
 
 static inline void set_pgd(pgd_t *pgdp, pgd_t pgd)
Signature · 1 line
2.5.5
#3 lore raw

PATCH 1/3 arm64: move the PTE_VALID to pgtable-hwdef.h

In reply to Huang Shijie
Quoted 28 lines · Huang Shijie

The PTE_VALID is used to check whether the page descriptor is valid. It's not a software defined PTE bits, such as PTE_WRITE/PTE_DIRTY.

So move it to the proper header file: pgtable-hwdef.h.

---
 arch/arm64/include/asm/pgtable-hwdef.h | 1 +
 arch/arm64/include/asm/pgtable-prot.h  | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)
arch/arm64/include/asm/pgtable-hwdef.h +1 -0
index 2813748..eeb3269 100644
@@ -154,6 +154,7 @@
 #define PTE_TYPE_MASK		(_AT(pteval_t, 3) << 0)
 #define PTE_TYPE_FAULT		(_AT(pteval_t, 0) << 0)
 #define PTE_TYPE_PAGE		(_AT(pteval_t, 3) << 0)
+#define PTE_VALID		(_AT(pteval_t, 1) << 0)
 #define PTE_TABLE_BIT		(_AT(pteval_t, 1) << 1)
 #define PTE_USER		(_AT(pteval_t, 1) << 6)		/* AP[1] */
 #define PTE_RDONLY		(_AT(pteval_t, 1) << 7)		/* AP[2] */
arch/arm64/include/asm/pgtable-prot.h +0 -0
index 29fcb33..5ac71f4 100644

arch/arm64/include/asm/pgtable-prot.h line 27

Quoted 5 lines
arch/arm64/include/asm/pgtable-prot.h +0 -1
@@ -24,7 +24,6 @@
 /*
  * Software defined PTE bits definition.
  */
-#define PTE_VALID		(_AT(pteval_t, 1) << 0)
 #define PTE_WRITE		(PTE_DBM)		 /* same as DBM (51) */

So how would you define PTE_WRITE? It's a software bit in v8.0 and a hardware bit in v8.1 ;)

I don't mind shuffling the #defines about, I just think the boundaries are somewhat blurred and maybe it's a mistake to put software bits and hardware bits in separate headers.

Will

#4 lore raw

PATCH 1/3 arm64: move the PTE_VALID to pgtable-hwdef.h

Quoted 38 lines · Will Deacon
Quoted 28 lines · Huang Shijie

The PTE_VALID is used to check whether the page descriptor is valid. It's not a software defined PTE bits, such as PTE_WRITE/PTE_DIRTY.

So move it to the proper header file: pgtable-hwdef.h.

---
 arch/arm64/include/asm/pgtable-hwdef.h | 1 +
 arch/arm64/include/asm/pgtable-prot.h  | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)
arch/arm64/include/asm/pgtable-hwdef.h +1 -0
index 2813748..eeb3269 100644
@@ -154,6 +154,7 @@
 #define PTE_TYPE_MASK		(_AT(pteval_t, 3) << 0)
 #define PTE_TYPE_FAULT		(_AT(pteval_t, 0) << 0)
 #define PTE_TYPE_PAGE		(_AT(pteval_t, 3) << 0)
+#define PTE_VALID		(_AT(pteval_t, 1) << 0)
 #define PTE_TABLE_BIT		(_AT(pteval_t, 1) << 1)
 #define PTE_USER		(_AT(pteval_t, 1) << 6)		/* AP[1] */
 #define PTE_RDONLY		(_AT(pteval_t, 1) << 7)		/* AP[2] */
arch/arm64/include/asm/pgtable-prot.h +0 -0
index 29fcb33..5ac71f4 100644

arch/arm64/include/asm/pgtable-prot.h line 27

Quoted 5 lines
arch/arm64/include/asm/pgtable-prot.h +0 -1
@@ -24,7 +24,6 @@
 /*
  * Software defined PTE bits definition.
  */
-#define PTE_VALID		(_AT(pteval_t, 1) << 0)
 #define PTE_WRITE		(PTE_DBM)		 /* same as DBM (51) */

So how would you define PTE_WRITE? It's a software bit in v8.0 and a hardware bit in v8.1 ;)

Quoted 3 lines

I don't mind shuffling the #defines about, I just think the boundaries are somewhat blurred and maybe it's a mistake to put software bits and hardware bits in separate headers.

Do you mind I move all the software bits(PTE_VALID/PTE_WRITE/PTE_DIRTY...) to the pgtable-hwdef.h?

thanks
Huang Shijie