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

Re: [PATCH v4 2/3] arm/efi: Use dom0less configuration when using EFI boot

  • To: Luca Fancellu <luca.fancellu@xxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 7 Oct 2021 09:15:18 +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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=1Jf4CdjZAvVVn44BTSBs4eFZdi/nAtCGtwAbF5bgeUQ=; b=nlw81IOl99V+JFqY1XJzKDIEkRHySt3sqz0S7BhMId8q7ZBJRPso7xMzsGA7aKA+FThOVhB/S8gJQfD++5NUiJoONPzdxiZRk/HEaS7YF9mTNz1gjtZrn7AVxA1zeYdPq1j28WSRx9wuIflxWg1NniNKyCwEFEX9+r36M8wDftDVewrOM/Dq3EOiL7lxFEfzypr97BaKc7J1HDstBOK5wpavLHB12PbL/bb4b+rw1aJYSWsLrQvdPDdkBqtSvW+ES0J5zh5JzJDnNhq4o4VVnyHpSA5aeewga8woSEMNirZK1zXRbMGbSUpn269mSaKyFsEEbGf3b2JdoQr5fQK2EQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=jiW3rrFxcdv4Qe15sAzPApjeOd0MSQoA3KwUY+/CyAEW/BhzL0593l4Qb7+MGyJFds6z0Cwtzb3kbtJHtf6ALfBVhcS13zYxAq0RXBfE1Ozi+XwzlxQ0Vm+plMAKVsStJWmdliHa6JV2VB05cgzHXwQZ6NYo6IIaP8X1h+cX0xP/x6SoJwmXG9AdTRx22NWzMmc0iC4SmK7gB0eEsOF2D9LNS5ZbiWqOEcpddjhzbDo7lvlwVRzXUSXVr8uYxTxHbud7P5wf+G8Ppa5Xnoj6byieSUBHx10jowQfFOPCSsyME1Uqs/pzrMWCq2r8Y1r3rfF1xrbwx79IVdSSsXQDig==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=suse.com;
  • Cc: Bertrand Marquis <bertrand.marquis@xxxxxxx>, wei.chen@xxxxxxx, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Thu, 07 Oct 2021 07:15:36 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 01.10.2021 17:13, Luca Fancellu wrote:
>> On 1 Oct 2021, at 15:22, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> On 01.10.2021 15:55, Luca Fancellu wrote:
>>>> On 1 Oct 2021, at 12:02, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>> On 30.09.2021 16:28, Luca Fancellu wrote:
>>>>> @@ -1361,12 +1361,30 @@ efi_start(EFI_HANDLE ImageHandle, 
>>>>> EFI_SYSTEM_TABLE *SystemTable)
>>>>>        efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
>>>>>        cfg.addr = 0;
>>>>> -        dir_handle->Close(dir_handle);
>>>>> -
>>>>>        if ( gop && !base_video )
>>>>>            gop_mode = efi_find_gop_mode(gop, cols, rows, depth);
>>>>>    }
>>>>> +    /* Get the number of boot modules specified on the DT or an error 
>>>>> (<0) */
>>>>> +    dt_modules_found = efi_arch_check_dt_boot(dir_handle);
>>>>> +#endif
>>>> So I had asked to add a stub enclosed in such an #ifdef, to avoid the
>>>> #ifdef here. I may be willing to let you keep things as you have them
>>>> now, but I'd like to understand why you've picked that different
>>>> approach despite the prior discussion.
>>> There must be a misunderstanding, your message in the v3 was:
>>> "Every time I see this addition I'm getting puzzled. As a result I'm
>>> afraid I now need to finally ask you to do something about this (and
>>> I'm sorry for doing so only now). There would better be no notion of
>>> DT in x86 code, and there would better also not be a need for
>>> architectures not supporting DT to each supply such a stub. Instead
>>> I think you want to put this stub in xen/common/efi/boot.c, inside a
>>> suitable #ifdef.”
>>> So I thought you wanted me to remove the stub in x86 (since it doesn’t 
>>> support DT)
>>> and put this call under #ifdef so it won’t be compiled for arch not 
>>> supporting DT.
>> So FTAOD I'll repeat the crucial part: "I think you want to put this
>> stub in xen/common/efi/boot.c". There was nothing about removing the
>> stub altogether.
> Oh ok, now I see, so in your opinion this is a better idea:
> static inline int __init efi_arch_check_dt_boot(EFI_FILE_HANDLE dir_handle)
> {
>     return 0;
> }
> #endif
> But I would like to understand the advantage respect of my approach, could you
> explain me?

Well, to a degree it's a matter of taste. Your approach may lead to a long
series of various #ifdef sections in a single function, harming readability.
Having stubs instead (usually placed in headers, albeit not in this case)
allows the main bodies of code to remain more tidy.




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