[PATCH] x86/PVH: allow Dom0 ELF parsing to be verbose

[PATCH] x86/PVH: allow Dom0 ELF parsing to be verbose

[Jan Beulich]

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>

--- 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 )
     {

Re: [PATCH] x86/PVH: allow Dom0 ELF parsing to be verbose

[Roger Pau Monné]

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.

Re: [PATCH] x86/PVH: allow Dom0 ELF parsing to be verbose

[Jan Beulich]

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