Re: [PATCH v3] EFI/runtime: switch to xv[mz]alloc_array()

[Daniel P. Smith]

On 7/23/25 09:39, Jan Beulich wrote:
Use the more "modern" form, thus doing away with effectively open-coding
xmalloc_array() at the same time. While there is a difference in
generated code, as xmalloc_bytes() forces SMP_CACHE_BYTES alignment, if
code really cared about such higher than default alignment, it should
request so explicitly.

While I don't object to the change itself, I think this description is a 
bit over simplification of the change. If the allocation is under 
PAGE_SIZE, then they are equivalent, but if it is over the page size 
there are a few more differences than just cache alignment. It 
completely changes the underlying allocator. I personally also find it a 
bit of a stretch to call xmalloc_bytes(size) an open coded version of 
xmalloc_array(char, size).

Signed-off-by: Jan Beulich <jbeul...@suse.com>
---
v3: Use xv[mz]alloc_array().

--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -6,6 +6,7 @@
  #include <xen/irq.h>
  #include <xen/sections.h>
  #include <xen/time.h>
+#include <xen/xvmalloc.h>
  
  DEFINE_XEN_GUEST_HANDLE(CHAR16);
  
@@ -500,23 +501,23 @@ int efi_runtime_call(struct xenpf_efi_ru
          len = gwstrlen(guest_handle_cast(op->u.get_variable.name, CHAR16));
          if ( len < 0 )
              return len;
-        name = xmalloc_array(CHAR16, ++len);
+        name = xvmalloc_array(CHAR16, ++len);
          if ( !name )
             return -ENOMEM;
          if ( __copy_from_guest(name, op->u.get_variable.name, len) ||
               wmemchr(name, 0, len) != name + len - 1 )
          {
-            xfree(name);
+            xvfree(name);
              return -EIO;
          }
  
          size = op->u.get_variable.size;
          if ( size )
          {
-            data = xmalloc_bytes(size);
+            data = xvmalloc_array(unsigned char, size);
              if ( !data )
              {
-                xfree(name);
+                xvfree(name);
                  return -ENOMEM;
              }
          }
@@ -539,8 +540,8 @@ int efi_runtime_call(struct xenpf_efi_ru
          else
              rc = -EOPNOTSUPP;
  
-        xfree(data);
-        xfree(name);
+        xvfree(data);
+        xvfree(name);
      }
      break;
  
@@ -553,17 +554,17 @@ int efi_runtime_call(struct xenpf_efi_ru
          len = gwstrlen(guest_handle_cast(op->u.set_variable.name, CHAR16));
          if ( len < 0 )
              return len;
-        name = xmalloc_array(CHAR16, ++len);
+        name = xvmalloc_array(CHAR16, ++len);
          if ( !name )
             return -ENOMEM;
          if ( __copy_from_guest(name, op->u.set_variable.name, len) ||
               wmemchr(name, 0, len) != name + len - 1 )
          {
-            xfree(name);
+            xvfree(name);
              return -EIO;
          }
  
-        data = xmalloc_bytes(op->u.set_variable.size);
+        data = xvmalloc_array(unsigned char, op->u.set_variable.size);
          if ( !data )
              rc = -ENOMEM;
          else if ( copy_from_guest(data, op->u.set_variable.data,
@@ -581,8 +582,8 @@ int efi_runtime_call(struct xenpf_efi_ru
              efi_rs_leave(&state);
          }
  
-        xfree(data);
-        xfree(name);
+        xvfree(data);
+        xvfree(name);
      }
      break;
  
@@ -598,13 +599,13 @@ int efi_runtime_call(struct xenpf_efi_ru
              return -EINVAL;
  
          size = op->u.get_next_variable_name.size;
-        name.raw = xzalloc_bytes(size);
+        name.raw = xvzalloc_array(unsigned char, size);
          if ( !name.raw )
              return -ENOMEM;
          if ( copy_from_guest(name.raw, op->u.get_next_variable_name.name,
                               size) )
          {
-            xfree(name.raw);
+            xvfree(name.raw);
              return -EFAULT;
          }
  
@@ -629,7 +630,7 @@ int efi_runtime_call(struct xenpf_efi_ru
          else
              rc = -EOPNOTSUPP;
  
-        xfree(name.raw);
+        xvfree(name.raw);
      }
      break;
  

With a stronger description of the change,

Acked-by: Daniel P. Smith <dpsm...@apertussolutions.com>