diff mbox series

[23/36] xen/arch: coloring: manually calculate Xen physical addresses

Message ID 20220304174701.1453977-24-marco.solieri@minervasys.tech (mailing list archive)
State New, archived
Headers show
Series Arm cache coloring | expand

Commit Message

Marco Solieri March 4, 2022, 5:46 p.m. UTC
From: Luca Miccio <lucmiccio@gmail.com>

During Xen coloring procedure, we need to manually calculate consecutive
physical addresses that conform to the color selection. Add an helper
function that does this operation. The latter will return the next
address that conforms to Xen color selection.

The next_colored function is architecture dependent and the provided
implementation is for ARMv8.

Signed-off-by: Luca Miccio <lucmiccio@gmail.com>
Signed-off-by: Marco Solieri <marco.solieri@minervasys.tech>
---
 xen/arch/arm/coloring.c             | 43 +++++++++++++++++++++++++++++
 xen/arch/arm/include/asm/coloring.h | 14 ++++++++++
 2 files changed, 57 insertions(+)

Comments

Julien Grall March 14, 2022, 7:23 p.m. UTC | #1
Hi,

On 04/03/2022 17:46, Marco Solieri wrote:
> From: Luca Miccio <lucmiccio@gmail.com>
> 
> During Xen coloring procedure, we need to manually calculate consecutive
> physical addresses that conform to the color selection. Add an helper
> function that does this operation. The latter will return the next
> address that conforms to Xen color selection.
> 
> The next_colored function is architecture dependent and the provided
> implementation is for ARMv8.
> 
> Signed-off-by: Luca Miccio <lucmiccio@gmail.com>
> Signed-off-by: Marco Solieri <marco.solieri@minervasys.tech>
> ---
>   xen/arch/arm/coloring.c             | 43 +++++++++++++++++++++++++++++
>   xen/arch/arm/include/asm/coloring.h | 14 ++++++++++
>   2 files changed, 57 insertions(+)
> 
> diff --git a/xen/arch/arm/coloring.c b/xen/arch/arm/coloring.c
> index 761414fcd7..aae3c77a7b 100644
> --- a/xen/arch/arm/coloring.c
> +++ b/xen/arch/arm/coloring.c
> @@ -222,6 +222,49 @@ uint32_t get_max_colors(void)
>       return max_col_num;
>   }
>   
> +paddr_t next_xen_colored(paddr_t phys)
> +{
> +    unsigned int i;
> +    unsigned int col_next_number = 0;
> +    unsigned int col_cur_number = (phys & addr_col_mask) >> PAGE_SHIFT;
> +    int overrun = 0;

This only looks to be used as an unsigned value. So please use 'unsigned 
int'.

> +    paddr_t ret;
> +
> +    /*
> +     * Check if address color conforms to Xen selection. If it does, return
> +     * the address as is.
> +     */
> +    for( i = 0; i < xen_col_num; i++)

Coding style: missing space after 'for' and before ')'.

> +        if ( col_cur_number == xen_col_list[i] )
> +            return phys;
> +
> +    /* Find next col */
> +    for( i = xen_col_num -1 ; i >= 0; i--)

i is unsigned. So the 'i >= 0' will always be true as it will wrap to 
2^32 - 1. What did you intend to check?

Coding style: missing space after 'for', '-' and before ')'.

> +    {
> +        if ( col_cur_number > xen_col_list[i])

Coding style: missing space before ')'.

> +        {
> +            /* Need to start to first element and add a way_size */
> +            if ( i == (xen_col_num - 1) )
> +            {
> +                col_next_number = xen_col_list[0];
> +                overrun = 1;
> +            }
> +            else
> +            {
> +                col_next_number = xen_col_list[i+1];

Coding style: Missing space before and after '+'.

> +                overrun = 0;
> +            }
> +            break;
> +        }
> +    }
> +
> +    /* Align phys to way_size */
> +    ret = phys - (PAGE_SIZE * col_cur_number);

I am not sure to understand how the comment is matching with the code. 
 From the comment, I would expect the expression to contain 'way_size'.

> +    /* Add the offset based on color selection*/

Coding style: missing space before '*/'.

> +    ret += (PAGE_SIZE * (col_next_number)) + (way_size*overrun);
Coding style: Missing space before and after '*'.

> +    return ret;
> +}
> +
>   bool __init coloring_init(void)
>   {
>       int i, rc;
> diff --git a/xen/arch/arm/include/asm/coloring.h b/xen/arch/arm/include/asm/coloring.h
> index 22e67dc9d8..8c4525677c 100644
> --- a/xen/arch/arm/include/asm/coloring.h
> +++ b/xen/arch/arm/include/asm/coloring.h
> @@ -28,6 +28,20 @@
>   
>   bool __init coloring_init(void);
>   
> +/*
> + * Return physical page address that conforms to the colors selection
> + * given in col_selection_mask after @param phys.
> + *
> + * @param phys         Physical address start.
> + * @param addr_col_mask        Mask specifying the bits available for coloring.
> + * @param col_selection_mask   Mask asserting the color bits to be used,
> + * must not be 0.

The function belows have only one parameter. Yet, you are description 3 
parameters here.

> + *
> + * @return The lowest physical page address being greater or equal than
> + * 'phys' and belonging to Xen color selection
> + */
> +paddr_t next_xen_colored(paddr_t phys);
> +
>   /*
>    * Check colors of a given domain.
>    * Return true if check passed, false otherwise.

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/arm/coloring.c b/xen/arch/arm/coloring.c
index 761414fcd7..aae3c77a7b 100644
--- a/xen/arch/arm/coloring.c
+++ b/xen/arch/arm/coloring.c
@@ -222,6 +222,49 @@  uint32_t get_max_colors(void)
     return max_col_num;
 }
 
+paddr_t next_xen_colored(paddr_t phys)
+{
+    unsigned int i;
+    unsigned int col_next_number = 0;
+    unsigned int col_cur_number = (phys & addr_col_mask) >> PAGE_SHIFT;
+    int overrun = 0;
+    paddr_t ret;
+
+    /*
+     * Check if address color conforms to Xen selection. If it does, return
+     * the address as is.
+     */
+    for( i = 0; i < xen_col_num; i++)
+        if ( col_cur_number == xen_col_list[i] )
+            return phys;
+
+    /* Find next col */
+    for( i = xen_col_num -1 ; i >= 0; i--)
+    {
+        if ( col_cur_number > xen_col_list[i])
+        {
+            /* Need to start to first element and add a way_size */
+            if ( i == (xen_col_num - 1) )
+            {
+                col_next_number = xen_col_list[0];
+                overrun = 1;
+            }
+            else
+            {
+                col_next_number = xen_col_list[i+1];
+                overrun = 0;
+            }
+            break;
+        }
+    }
+
+    /* Align phys to way_size */
+    ret = phys - (PAGE_SIZE * col_cur_number);
+    /* Add the offset based on color selection*/
+    ret += (PAGE_SIZE * (col_next_number)) + (way_size*overrun);
+    return ret;
+}
+
 bool __init coloring_init(void)
 {
     int i, rc;
diff --git a/xen/arch/arm/include/asm/coloring.h b/xen/arch/arm/include/asm/coloring.h
index 22e67dc9d8..8c4525677c 100644
--- a/xen/arch/arm/include/asm/coloring.h
+++ b/xen/arch/arm/include/asm/coloring.h
@@ -28,6 +28,20 @@ 
 
 bool __init coloring_init(void);
 
+/*
+ * Return physical page address that conforms to the colors selection
+ * given in col_selection_mask after @param phys.
+ *
+ * @param phys         Physical address start.
+ * @param addr_col_mask        Mask specifying the bits available for coloring.
+ * @param col_selection_mask   Mask asserting the color bits to be used,
+ * must not be 0.
+ *
+ * @return The lowest physical page address being greater or equal than
+ * 'phys' and belonging to Xen color selection
+ */
+paddr_t next_xen_colored(paddr_t phys);
+
 /*
  * Check colors of a given domain.
  * Return true if check passed, false otherwise.