[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] x86/pv: Provide more helpful error when CONFIG_PV32 is absent


  • To: Jane Malalane <jane.malalane@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 12 Aug 2021 14:22:30 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=jc5UOJNVYigHrspVHTFQjALjNXWNmEuCuqHYBt8M8Ik=; b=Sul0AixpDfmrv5uiCbULLIkuRCduVj7Ykl/EPjQWzS0EnfYKQoxCALW6zEo9y6euWsunaCANw2J0D6nckwhgLKNMLdKfNCyfvWRCGXp491Tb9mA9fpzMl0HINVRCuRwGsBgvotk1K5Jn361u+qXS+BzZaInZkKtaHNiLR+/AApYTNnRxKZKMIwAydZGwSitxYoBAZ7tRM+jqzKbqUip3Q5Q6ZSyfHEU+1mqkncbJ5zgbdziGYIC868dEf/dJ7i7tuvbpPcB/c3vMM6FYhe25bWqQmtj9ZIV18Ld84UqpLKD7AB+vFUaZ8mbXW+hSbHbOiXnyg4A7Ro72Vr063HMo3w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LixRwQuXdwPKbnR79oHfitpLqyhT8VFam4cnynmA+KEklZPTS/hS4OjfObvX/nGba9OT89gR/zIxv0qGaSatMh84/y2OJbbfSj/k/BAtVOqITI+MHics1ucaIrm/zsTeWdVHlGkh9jvEd5W5bhx62sio9ZcujXD3CVv9i55mnKuT09kER9wosslKay7Ykfs2aS0s8GCj0soDOkMErnYTxFOrIZogUYb8MDBA+ZNdsG/XgfOnShZLFeyHa2xa5AtQl1n2ESg1vwkenPprFArp+GtkMFc2s6I46A/5Q6NOj1NL6TzOQKGYaa+gj8wXNST7Il+yrTNQy7dukkrSS2uYMQ==
  • Authentication-results: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Thu, 12 Aug 2021 12:22:58 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 12.08.2021 13:48, Jane Malalane wrote:
> Currently, when booting a 32bit dom0 kernel, the message isn't very
> helpful:
> 
>   (XEN)  Xen  kernel: 64-bit, lsb
>   (XEN)  Dom0 kernel: 32-bit, PAE, lsb, paddr 0x100000 -> 0x112000
>   (XEN) Mismatch between Xen and DOM0 kernel
>   (XEN)
>   (XEN) ****************************************
>   (XEN) Panic on CPU 0:
>   (XEN) Could not construct domain 0
>   (XEN) ****************************************
> 
> With this adjustment, it now looks like this:
> 
>   (XEN)  Xen  kernel: 64-bit, lsb
>   (XEN) Found 32-bit PV kernel, but CONFIG_PV32 missing
>   (XEN)
>   (XEN) ****************************************
>   (XEN) Panic on CPU 0:
>   (XEN) Could not construct domain 0
>   (XEN) ****************************************
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Jane Malalane <jane.malalane@xxxxxxxxxx>
> ---
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
> CC: Wei Liu <wl@xxxxxxx>
> ---
>  xen/arch/x86/pv/dom0_build.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
> index af47615b22..80e6c6e35b 100644
> --- a/xen/arch/x86/pv/dom0_build.c
> +++ b/xen/arch/x86/pv/dom0_build.c
> @@ -362,9 +362,9 @@ int __init dom0_construct_pv(struct domain *d,
>      compatible = false;
>      machine = elf_uval(&elf, elf.ehdr, e_machine);
>  
> -#ifdef CONFIG_PV32
>      if ( elf_32bit(&elf) )
>      {
> +#ifdef CONFIG_PV32
>          if ( parms.pae == XEN_PAE_BIMODAL )
>              parms.pae = XEN_PAE_EXTCR3;
>          if ( parms.pae && machine == EM_386 )
> @@ -377,8 +377,12 @@ int __init dom0_construct_pv(struct domain *d,
>  
>              compatible = true;
>          }
> -    }
> +#else
> +        printk("Found 32-bit PV kernel, but CONFIG_PV32 missing\n");
> +        rc = -ENODEV;
> +        goto out;

I don't see the "goto" as warranted here, not the least because the
code fragment right above the "#else" you add also uses plain "return".
With just "return -ENODEV;" (or maybe better "return -EOPNOTSUPP;")
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

In fact I think most "goto out;" are unwarranted in this function.
The only two places that want elf_check_broken() to be invoked are
after elf_xen_parse() and after elf_load_binary(). You'd be welcome
to add a 2nd patch to clean this up.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.