diff mbox series

x86/PVH: allow Dom0 ELF parsing to be verbose

Message ID b297e430-f3e3-97a7-be7d-1debdbcb0119@suse.com (mailing list archive)
State New, archived
Headers show
Series x86/PVH: allow Dom0 ELF parsing to be verbose | expand

Commit Message

Jan Beulich Oct. 30, 2023, 1:14 p.m. UTC
VERBOSE had ceased to exist already before the introduction of this ELF
parsing code.

Fixes: 62ba982424cb ("x86: parse Dom0 kernel for PVHv2")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

Comments

Roger Pau Monne Oct. 31, 2023, 9:08 a.m. UTC | #1
On Mon, Oct 30, 2023 at 02:14:44PM +0100, Jan Beulich wrote:
> VERBOSE had ceased to exist already before the introduction of this ELF
> parsing code.
> 
> Fixes: 62ba982424cb ("x86: parse Dom0 kernel for PVHv2")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Some of the printed information might not be relevant for PVH mode,
like the 'ELF: addresses:' virt_* fields.

Note also that in 62ba982424cb opt_dom0_verbose wasn't available yet,
that option got introduced a couple of years later by 525ef6584f852.

Maybe it's 679216943f545 that should have also switched the
elf_set_verbose() in the PVH dom0 builder to use opt_dom0_verbose, at
the same time that the PV one was switched?

Thanks, Roger.
Jan Beulich Oct. 31, 2023, 9:51 a.m. UTC | #2
On 31.10.2023 10:08, Roger Pau Monné wrote:
> On Mon, Oct 30, 2023 at 02:14:44PM +0100, Jan Beulich wrote:
>> VERBOSE had ceased to exist already before the introduction of this ELF
>> parsing code.
>>
>> Fixes: 62ba982424cb ("x86: parse Dom0 kernel for PVHv2")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

> Some of the printed information might not be relevant for PVH mode,
> like the 'ELF: addresses:' virt_* fields.

We may want to conditionalize some of that, but since you mention virt_*:
Just ahead of the printing we forcefully set e.g. virt_entry when hvm is
true. virt_base, otoh, is pretty clearly useless to log, for being set to
zero unconditionally when hvm is true.

In any event, what I was missing was the output from elf_xen_parse_note().
I had mistakenly tried to boot a PV-only kernel as PVH, but it didn't
occur to me to check for a typo in the kernel specification in the grub
entry. It was only the sequence of ELF notes that finally directed me
towards checking that ...

> Note also that in 62ba982424cb opt_dom0_verbose wasn't available yet,
> that option got introduced a couple of years later by 525ef6584f852.

Right; I've set the Fixes: tag solely from the wrong use of VERBOSE.

> Maybe it's 679216943f545 that should have also switched the
> elf_set_verbose() in the PVH dom0 builder to use opt_dom0_verbose, at
> the same time that the PV one was switched?

Perhaps that would have been a good point in time, but here we are.

Jan
diff mbox series

Patch

--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -557,9 +557,8 @@  static int __init pvh_load_kernel(struct
         printk("Unable to init ELF\n");
         return rc;
     }
-#ifdef VERBOSE
-    elf_set_verbose(&elf);
-#endif
+    if ( opt_dom0_verbose )
+        elf_set_verbose(&elf);
     elf_parse_binary(&elf);
     if ( (rc = elf_xen_parse(&elf, &parms, true)) != 0 )
     {