1. Patchew
  2. Xen
  3. View series

[PATCH] tools/libxl: Remove page_size and page_shift from struct libxl_acpi_ctxt

Kevin Stefanov posted 1 patch 3 years, 2 months ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20210924110500.25412-1-kevin.stefanov@citrix.com
tools/libs/light/libxl_x86_acpi.c | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
[PATCH] tools/libxl: Remove page_size and page_shift from struct libxl_acpi_ctxt
Posted by Kevin Stefanov 3 years, 2 months ago 
As a result of recent work, two members of struct libxl_acpi_ctxt were
left with only one user. Thus, it becomes illogical for them to be
members of the struct at all.

Drop the two struct members and instead let the only function using
them have them as local variables.

Signed-off-by: Kevin Stefanov <kevin.stefanov@citrix.com>
---
CC: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Jackson <iwj@xenproject.org>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
---
 tools/libs/light/libxl_x86_acpi.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/tools/libs/light/libxl_x86_acpi.c b/tools/libs/light/libxl_x86_acpi.c
index 57a6b63790..68902e7809 100644
--- a/tools/libs/light/libxl_x86_acpi.c
+++ b/tools/libs/light/libxl_x86_acpi.c
@@ -25,9 +25,6 @@
 struct libxl_acpi_ctxt {
     struct acpi_ctxt c;
 
-    unsigned int page_size;
-    unsigned int page_shift;
-
     /* Memory allocator */
     unsigned long guest_start;
     unsigned long guest_curr;
@@ -159,12 +156,13 @@ int libxl__dom_load_acpi(libxl__gc *gc,
     struct acpi_config config = {0};
     struct libxl_acpi_ctxt libxl_ctxt;
     int rc = 0, acpi_pages_num;
+    unsigned int page_size, page_shift;
 
     if (b_info->type != LIBXL_DOMAIN_TYPE_PVH)
         goto out;
 
-    libxl_ctxt.page_size = XC_DOM_PAGE_SIZE(dom);
-    libxl_ctxt.page_shift =  XC_DOM_PAGE_SHIFT(dom);
+    page_size = XC_DOM_PAGE_SIZE(dom);
+    page_shift = XC_DOM_PAGE_SHIFT(dom);
 
     libxl_ctxt.c.mem_ops.alloc = mem_alloc;
     libxl_ctxt.c.mem_ops.v2p = virt_to_phys;
@@ -176,20 +174,19 @@ int libxl__dom_load_acpi(libxl__gc *gc,
         goto out;
     }
 
-    config.rsdp = (unsigned long)libxl__malloc(gc, libxl_ctxt.page_size);
-    config.infop = (unsigned long)libxl__malloc(gc, libxl_ctxt.page_size);
+    config.rsdp = (unsigned long)libxl__malloc(gc, page_size);
+    config.infop = (unsigned long)libxl__malloc(gc, page_size);
     /* Pages to hold ACPI tables */
-    libxl_ctxt.buf = libxl__malloc(gc, NUM_ACPI_PAGES *
-                                   libxl_ctxt.page_size);
+    libxl_ctxt.buf = libxl__malloc(gc, NUM_ACPI_PAGES * page_size);
 
     /*
      * Set up allocator memory.
      * Start next to acpi_info page to avoid fracturing e820.
      */
     libxl_ctxt.guest_start = libxl_ctxt.guest_curr = libxl_ctxt.guest_end =
-        ACPI_INFO_PHYSICAL_ADDRESS + libxl_ctxt.page_size;
+        ACPI_INFO_PHYSICAL_ADDRESS + page_size;
 
-    libxl_ctxt.guest_end += NUM_ACPI_PAGES * libxl_ctxt.page_size;
+    libxl_ctxt.guest_end += NUM_ACPI_PAGES * page_size;
 
     /* Build the tables. */
     rc = acpi_build_tables(&libxl_ctxt.c, &config);
@@ -199,8 +196,8 @@ int libxl__dom_load_acpi(libxl__gc *gc,
     }
 
     /* Calculate how many pages are needed for the tables. */
-    acpi_pages_num = (ALIGN(libxl_ctxt.guest_curr, libxl_ctxt.page_size) -
-                      libxl_ctxt.guest_start) >> libxl_ctxt.page_shift;
+    acpi_pages_num = (ALIGN(libxl_ctxt.guest_curr, page_size) -
+                      libxl_ctxt.guest_start) >> page_shift;
 
     dom->acpi_modules[0].data = (void *)config.rsdp;
     dom->acpi_modules[0].length = 64;
@@ -212,7 +209,7 @@ int libxl__dom_load_acpi(libxl__gc *gc,
     if (strcmp(xc_dom_guest_os(dom), "linux") ||
         xc_dom_feature_get(dom, XENFEAT_linux_rsdp_unrestricted))
         dom->acpi_modules[0].guest_addr_out = ACPI_INFO_PHYSICAL_ADDRESS +
-            (1 + acpi_pages_num) * libxl_ctxt.page_size;
+            (1 + acpi_pages_num) * page_size;
     else
         dom->acpi_modules[0].guest_addr_out = 0x100000 - 64;
 
@@ -221,9 +218,9 @@ int libxl__dom_load_acpi(libxl__gc *gc,
     dom->acpi_modules[1].guest_addr_out = ACPI_INFO_PHYSICAL_ADDRESS;
 
     dom->acpi_modules[2].data = libxl_ctxt.buf;
-    dom->acpi_modules[2].length = acpi_pages_num  << libxl_ctxt.page_shift;
+    dom->acpi_modules[2].length = acpi_pages_num << page_shift;
     dom->acpi_modules[2].guest_addr_out = ACPI_INFO_PHYSICAL_ADDRESS +
-        libxl_ctxt.page_size;
+        page_size;
 
 out:
     return rc;
-- 
2.25.1
Re: [PATCH] tools/libxl: Remove page_size and page_shift from struct libxl_acpi_ctxt
Posted by Jan Beulich 3 years, 2 months ago 
On 24.09.2021 13:05, Kevin Stefanov wrote:
> As a result of recent work, two members of struct libxl_acpi_ctxt were
> left with only one user. Thus, it becomes illogical for them to be
> members of the struct at all.
> 
> Drop the two struct members and instead let the only function using
> them have them as local variables.
> 
> Signed-off-by: Kevin Stefanov <kevin.stefanov@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

I would like to suggest though to consider ...

> @@ -176,20 +174,19 @@ int libxl__dom_load_acpi(libxl__gc *gc,
>          goto out;
>      }
>  
> -    config.rsdp = (unsigned long)libxl__malloc(gc, libxl_ctxt.page_size);
> -    config.infop = (unsigned long)libxl__malloc(gc, libxl_ctxt.page_size);
> +    config.rsdp = (unsigned long)libxl__malloc(gc, page_size);
> +    config.infop = (unsigned long)libxl__malloc(gc, page_size);
>      /* Pages to hold ACPI tables */
> -    libxl_ctxt.buf = libxl__malloc(gc, NUM_ACPI_PAGES *
> -                                   libxl_ctxt.page_size);
> +    libxl_ctxt.buf = libxl__malloc(gc, NUM_ACPI_PAGES * page_size);

... using page_shift to replace all multiplications like the one here
at this occasion.

Jan
Re: [PATCH] tools/libxl: Remove page_size and page_shift from struct libxl_acpi_ctxt
Posted by Ian Jackson 3 years, 2 months ago 
Jan Beulich writes ("Re: [PATCH] tools/libxl: Remove page_size and page_shift from struct libxl_acpi_ctxt"):
> On 24.09.2021 13:05, Kevin Stefanov wrote:
> > As a result of recent work, two members of struct libxl_acpi_ctxt were
> > left with only one user. Thus, it becomes illogical for them to be
> > members of the struct at all.
> > 
> > Drop the two struct members and instead let the only function using
> > them have them as local variables.
> > 
> > Signed-off-by: Kevin Stefanov <kevin.stefanov@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Ian Jackson <iwj@xenproject.org>

> I would like to suggest though to consider ...
> 
> > @@ -176,20 +174,19 @@ int libxl__dom_load_acpi(libxl__gc *gc,
> >          goto out;
> >      }
> >  
> > -    config.rsdp = (unsigned long)libxl__malloc(gc, libxl_ctxt.page_size);
> > -    config.infop = (unsigned long)libxl__malloc(gc, libxl_ctxt.page_size);
> > +    config.rsdp = (unsigned long)libxl__malloc(gc, page_size);
> > +    config.infop = (unsigned long)libxl__malloc(gc, page_size);
> >      /* Pages to hold ACPI tables */
> > -    libxl_ctxt.buf = libxl__malloc(gc, NUM_ACPI_PAGES *
> > -                                   libxl_ctxt.page_size);
> > +    libxl_ctxt.buf = libxl__malloc(gc, NUM_ACPI_PAGES * page_size);
> 
> ... using page_shift to replace all multiplications like the one here
> at this occasion.

I don't have an opinion about this; my tools ack can stand if this
change is made and reviewed.

Ian.

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.