Re: [Xen-devel] [PATCH 3/3] x86/pvh: Print the PVH start info more concisely

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.01.19 at 16:37, <andrew.cooper3@xxxxxxxxxx> wrote:
> The current rendering of PVH start info in unnecessarily verbose, and doesn't
> clearly separate decimal and hex numbers.

As expressed on earlier occasions, I think blindly adding 0x in
all cases goes too far. When context makes sufficiently clear
that it's a hex number (like when addresses get printed) I don't
see the need. Nevertheless, knowing you disagree,
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
with two instances of one further question below.

> --- a/xen/arch/x86/guest/pvh-boot.c
> +++ b/xen/arch/x86/guest/pvh-boot.c
> @@ -123,28 +123,29 @@ void __init pvh_print_info(void)
>      const struct hvm_modlist_entry *entry;
>      unsigned int i;
>  
> -    ASSERT(pvh_info->magic == XEN_HVM_START_MAGIC_VALUE);
> -
> -    printk("PVH start info: (pa %08x)\n", pvh_start_info_pa);
> -    printk("  version:    %u\n", pvh_info->version);
> -    printk("  flags:      %#"PRIx32"\n", pvh_info->flags);
> -    printk("  nr_modules: %u\n", pvh_info->nr_modules);
> -    printk("  modlist_pa: %016"PRIx64"\n", pvh_info->modlist_paddr);
> -    printk("  cmdline_pa: %016"PRIx64"\n", pvh_info->cmdline_paddr);
> +    printk("PVH start info: (pa 0x%08x)\n", pvh_start_info_pa);
> +    printk("  version %u, flags %#x\n", pvh_info->version, pvh_info->flags);
> +
> +    printk("  cmdline 0x%08"PRIx64, pvh_info->cmdline_paddr);
>      if ( pvh_info->cmdline_paddr )
> -        printk("  cmdline:    '%s'\n", (char 
> *)__va(pvh_info->cmdline_paddr));
> -    printk("  rsdp_pa:    %016"PRIx64"\n", pvh_info->rsdp_paddr);
> +        printk(" '%s'", (char *)__va(pvh_info->cmdline_paddr));

Is the cast here really necessary?

> +    printk("\n");
> +
> +    printk("  rsdp    0x%08"PRIx64"\n", pvh_info->rsdp_paddr);
> +
> +    printk("  modlist 0x%08"PRIx64", nr %u\n",
> +           pvh_info->modlist_paddr, pvh_info->nr_modules);
>  
>      entry = __va(pvh_info->modlist_paddr);
>      for ( i = 0; i < pvh_info->nr_modules; i++ )
>      {
> -        printk("    mod[%u].pa:         %016"PRIx64"\n", i, entry[i].paddr);
> -        printk("    mod[%u].size:       %016"PRIu64"\n", i, entry[i].size);
> -        printk("    mod[%u].cmdline_pa: %016"PRIx64"\n",
> -               i, entry[i].cmdline_paddr);
> +        printk("    mod%u pa 0x%08"PRIx64", sz 0x%08"PRIx64", cmdline 0x%08" 
> PRIx64,
> +               i, entry[i].paddr, entry[i].size, entry[i].cmdline_paddr);
> +
>          if ( entry[i].cmdline_paddr )
> -            printk("    mod[%1u].cmdline:    '%s'\n", i,
> -                   (char *)__va(entry[i].cmdline_paddr));
> +            printk(" '%s'\n", (char *)__va(entry[i].cmdline_paddr));

Same here then.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel