diff mbox

[RFC,v2,4/8] arm/mem_access: Add short-descriptor pte typedefs

Message ID 20170601151906.10213-5-proskurin@sec.in.tum.de (mailing list archive)
State New, archived
Headers show

Commit Message

Sergej Proskurin June 1, 2017, 3:18 p.m. UTC
The current implementation does not provide appropriate types for
short-descriptor translation table entries. As such, this commit adds new
types, which simplify managing the respective translation table entries.

Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
 xen/include/asm-arm/page.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Julien Grall June 2, 2017, 8:50 a.m. UTC | #1
Hi Sergej,

On 06/01/2017 04:18 PM, Sergej Proskurin wrote:
> The current implementation does not provide appropriate types for
> short-descriptor translation table entries. As such, this commit adds new
> types, which simplify managing the respective translation table entries.
> 
> Signed-off-by: Sergej Proskurin <proskurin@sec.in.tum.de>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> ---
>   xen/include/asm-arm/page.h | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index 6222b1d4a2..5ea97ba95b 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -205,6 +205,25 @@ typedef union {
>       lpae_walk_t walk;
>   } lpae_t;
>   
> +/*
> + * Comprises the bits required to walk page tables adhering to the
> + * short-descriptor translation table format.
> + */
> +typedef struct __packed {
> +    unsigned int dt:2;          /* Descriptor type */
> +    unsigned int pad1:8;
> +    unsigned int base:22;       /* Base address of block or next table */

This is clearly confusing. The base address size varies with the level 
you are currently walking. Without looking at the code, I can guess that 
you would need shift/mask to adapt the base address. This is a call for 
providing structure for each level (there is only 2 anyway).

> +} pte_sd_walk_t;

I am a bit surprised this is the only bits you required for the walking 
as you also need to return the permissions.

Looking at the patch doing the implement in patch #7, there is a lot of 
hardcoding value. This is a call for a better structure definition here.

> +/*
> + * Represents page table entries adhering to the short-descriptor translation
> + * table format.
> + */
> +typedef union {
> +    uint32_t bits;
> +    pte_sd_walk_t walk;
> +} pte_sd_t;
> +
>   /* Standard entry type that we'll use to build Xen's own pagetables.
>    * We put the same permissions at every level, because they're ignored
>    * by the walker in non-leaf entries. */
> 

Cheers,
diff mbox

Patch

diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 6222b1d4a2..5ea97ba95b 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -205,6 +205,25 @@  typedef union {
     lpae_walk_t walk;
 } lpae_t;
 
+/*
+ * Comprises the bits required to walk page tables adhering to the
+ * short-descriptor translation table format.
+ */
+typedef struct __packed {
+    unsigned int dt:2;          /* Descriptor type */
+    unsigned int pad1:8;
+    unsigned int base:22;       /* Base address of block or next table */
+} pte_sd_walk_t;
+
+/*
+ * Represents page table entries adhering to the short-descriptor translation
+ * table format.
+ */
+typedef union {
+    uint32_t bits;
+    pte_sd_walk_t walk;
+} pte_sd_t;
+
 /* Standard entry type that we'll use to build Xen's own pagetables.
  * We put the same permissions at every level, because they're ignored
  * by the walker in non-leaf entries. */