diff mbox series

[08/12] Revert "xen/arm: setup: Add Xen as boot module before printing all boot modules"

Message ID 20220826125111.152261-9-carlo.nonato@minervasys.tech (mailing list archive)
State Superseded
Headers show
Series Arm cache coloring | expand

Commit Message

Carlo Nonato Aug. 26, 2022, 12:51 p.m. UTC
From: Luca Miccio <lucmiccio@gmail.com>

This reverts commit 48fb2a9deba11ee48dde21c5c1aa93b4d4e1043b.

The cache coloring support has the command line parsing as a prerequisite
because of the color configurations passed in this way. Also, the Xen boot
module will be placed at an address that depends on the coloring
initialization. This commit moves the Xen boot module after the coloring
initialization to allow the order of operations previously described to
take place.

Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech>
Signed-off-by: Marco Solieri <marco.solieri@minervasys.tech>
---
 xen/arch/arm/setup.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Julien Grall Sept. 10, 2022, 2:01 p.m. UTC | #1
Hi,

On 26/08/2022 13:51, Carlo Nonato wrote:
> From: Luca Miccio <lucmiccio@gmail.com>
> 
> This reverts commit 48fb2a9deba11ee48dde21c5c1aa93b4d4e1043b.
> 
> The cache coloring support has the command line parsing as a prerequisite
> because of the color configurations passed in this way. Also, the Xen boot
> module will be placed at an address that depends on the coloring
> initialization. This commit moves the Xen boot module after the coloring
> initialization to allow the order of operations previously described to
> take place.

The commit you revert was created in order to print the position of Xen 
on the console. So while I understand your aim, you are (temporarily?) 
not printing Xen address anymore.

Therefore your commit message, should contain some words explaining why 
this is fine and how this problem will be addressed.

Cheers,
Carlo Nonato Sept. 12, 2022, 1:54 p.m. UTC | #2
Hi Julien,

On Sat, Sep 10, 2022 at 4:01 PM Julien Grall <julien@xen.org> wrote:
>
> Hi,
>
> On 26/08/2022 13:51, Carlo Nonato wrote:
> > From: Luca Miccio <lucmiccio@gmail.com>
> >
> > This reverts commit 48fb2a9deba11ee48dde21c5c1aa93b4d4e1043b.
> >
> > The cache coloring support has the command line parsing as a prerequisite
> > because of the color configurations passed in this way. Also, the Xen boot
> > module will be placed at an address that depends on the coloring
> > initialization. This commit moves the Xen boot module after the coloring
> > initialization to allow the order of operations previously described to
> > take place.
>
> The commit you revert was created in order to print the position of Xen
> on the console. So while I understand your aim, you are (temporarily?)
> not printing Xen address anymore.

Yes. The address will be printed by the get_xen_paddr() function in later
patches, but only when coloring is enabled.
So I probably need to find a way to print it regardless of the configuration.
Do you have any suggestions? Is it ok to add the print to this very patch
explaining why I added that (since it would edit the clean revert)?

>
> Therefore your commit message, should contain some words explaining why
> this is fine and how this problem will be addressed.
>
> Cheers,
>
> --
> Julien Grall
Julien Grall Oct. 21, 2022, 4:52 p.m. UTC | #3
On 12/09/2022 14:54, Carlo Nonato wrote:
> Hi Julien,

Hi Carlo,

> On Sat, Sep 10, 2022 at 4:01 PM Julien Grall <julien@xen.org> wrote: > Do you have any suggestions? Is it ok to add the print to this very patch
> explaining why I added that (since it would edit the clean revert)?

I would consider to the call to early_print_info() after the Xen module 
is created.

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index c02f21c0e6..611c93ad7d 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -921,12 +921,6 @@  void __init start_xen(unsigned long boot_phys_offset,
               "Please check your bootloader.\n",
               fdt_paddr);
 
-    /* Register Xen's load address as a boot module. */
-    xen_bootmodule = add_boot_module(BOOTMOD_XEN,
-                             (paddr_t)(uintptr_t)(_start + boot_phys_offset),
-                             (paddr_t)(uintptr_t)(_end - _start), false);
-    BUG_ON(!xen_bootmodule);
-
     fdt_size = boot_fdt_info(device_tree_flattened, fdt_paddr);
 
     cmdline = boot_fdt_cmdline(device_tree_flattened);
@@ -938,6 +932,12 @@  void __init start_xen(unsigned long boot_phys_offset,
         panic("Xen Coloring support: setup failed\n");
 #endif
 
+    /* Register Xen's load address as a boot module. */
+    xen_bootmodule = add_boot_module(BOOTMOD_XEN,
+                             (paddr_t)(uintptr_t)(_start + boot_phys_offset),
+                             (paddr_t)(uintptr_t)(_end - _start + 1), false);
+    BUG_ON(!xen_bootmodule);
+
     setup_mm();
 
     /* Parse the ACPI tables for possible boot-time configuration */