[PATCH v3] xen/dt: Remove loop in dt_read_number()

Alejandro Vallejo posted 1 patch 4 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20250619114206.1014-1-agarciav@amd.com
xen/include/xen/device_tree.h | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
[PATCH v3] xen/dt: Remove loop in dt_read_number()
Posted by Alejandro Vallejo 4 months, 1 week ago
The DT spec declares only two number types for a property: u32 and u64,
as per Table 2.3 in Section 2.2.4. Remove unbounded loop and replace
with a switch statement. Default to a size of 1 cell in the nonsensical
size case, with a warning printed on the Xen console.

Suggested-by: Daniel P. Smith" <dpsmith@apertussolutions.com>
Signed-off-by: Alejandro Vallejo <agarciav@amd.com>
---
If there's any #address-size > 2 out there at least we'll know in debug builds.
It's not like this function can cope with them either way.

I haven't tested things past CI.

pipeline: https://gitlab.com/xen-project/people/agvallejo/xen/-/pipelines/1876848415

v3:
  * s/WARNING/ERR
  * //-style comment to /**/-style comment
  * break after ASSERT_UNREACHABLE() to please MISRA
---
 xen/include/xen/device_tree.h | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 75017e4266..d2de7c3a13 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -261,10 +261,22 @@ void intc_dt_preinit(void);
 /* Helper to read a big number; size is in cells (not bytes) */
 static inline u64 dt_read_number(const __be32 *cell, int size)
 {
-    u64 r = 0;
+    u64 r = be32_to_cpu(*cell);
+
+    switch ( size )
+    {
+    case 1:
+        break;
+    case 2:
+        r = (r << 32) | be32_to_cpu(cell[1]);
+        break;
+    default:
+        /* Nonsensical size. default to 1 */
+        printk(XENLOG_ERR "dt_read_number(,%d) bad size\n", size);
+        ASSERT_UNREACHABLE();
+        break;
+    };
 
-    while ( size-- )
-        r = (r << 32) | be32_to_cpu(*(cell++));
     return r;
 }
 

base-commit: 22650d6054625be10172fe0c78b9cadd1a39bd63
-- 
2.43.0
Re: [PATCH v3] xen/dt: Remove loop in dt_read_number()
Posted by Stefano Stabellini 4 months, 1 week ago
On Thu, 19 Jun 2025, Alejandro Vallejo wrote:
> The DT spec declares only two number types for a property: u32 and u64,
> as per Table 2.3 in Section 2.2.4. Remove unbounded loop and replace
> with a switch statement. Default to a size of 1 cell in the nonsensical
> size case, with a warning printed on the Xen console.
> 
> Suggested-by: Daniel P. Smith" <dpsmith@apertussolutions.com>
> Signed-off-by: Alejandro Vallejo <agarciav@amd.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> If there's any #address-size > 2 out there at least we'll know in debug builds.
> It's not like this function can cope with them either way.
> 
> I haven't tested things past CI.
> 
> pipeline: https://gitlab.com/xen-project/people/agvallejo/xen/-/pipelines/1876848415
> 
> v3:
>   * s/WARNING/ERR
>   * //-style comment to /**/-style comment
>   * break after ASSERT_UNREACHABLE() to please MISRA
> ---
>  xen/include/xen/device_tree.h | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index 75017e4266..d2de7c3a13 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -261,10 +261,22 @@ void intc_dt_preinit(void);
>  /* Helper to read a big number; size is in cells (not bytes) */
>  static inline u64 dt_read_number(const __be32 *cell, int size)
>  {
> -    u64 r = 0;
> +    u64 r = be32_to_cpu(*cell);
> +
> +    switch ( size )
> +    {
> +    case 1:
> +        break;
> +    case 2:
> +        r = (r << 32) | be32_to_cpu(cell[1]);
> +        break;
> +    default:
> +        /* Nonsensical size. default to 1 */
> +        printk(XENLOG_ERR "dt_read_number(,%d) bad size\n", size);
> +        ASSERT_UNREACHABLE();
> +        break;
> +    };
>  
> -    while ( size-- )
> -        r = (r << 32) | be32_to_cpu(*(cell++));
>      return r;
>  }
>  
> 
> base-commit: 22650d6054625be10172fe0c78b9cadd1a39bd63
> -- 
> 2.43.0
>