[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: Fri, 8 Oct 2021 16:14:42 +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=aXv1ERi9cd99hYeUcIgKcsjZuWenVVS8l730ZkjOxsQ=; b=GZkIEnlZoyKv+1VAm1/LMgl01K3SKtL1sKOTVc2kcXgnBHTkqSABf9RnOgBU5v/mozGD59Su4g0OrUHBr9isHpyyJbwzOpDGALAidiWWLegYw1YCgnSVmGqhwxoOR+rTWoFDy/laq8nRPhD96A8zDm6XTy9O4TCe6lGGqUoLtDjhPMynyJwawBCsmmhqsICcrko9wyZMPQu+1Mb7O+jFGfzh9zCpF0oCWdYfRhwlvL9fIXfZ45mrCCHrOZev4yxk0ouYGVdWVvKPBj9fU5NqX2h6CH1UzhK/bzfGPYubFKPk+YnIdc+Erfy7bNEr7ssM8eHjdf5c/w2JGzRWJGbsxQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=GUYQN6gTACL1wpowQg9tnuPeMCkMMTpJOJP3wZ7SDFcUb6yZgshCR+neA7do7rllP1X1+vmpyLUZaeSCeRcEV1G4rYhMQZLmoJvxU8XRsn84xMHSsydLE6+NYxOxqDcA987jxmJxwOfSWlqH5ndgvkQZvYQC/5aOo/BZzB6F8nvuvnhiE+V5ri95tWL4V1/x3kKcZXli6yO9vegN4eM4Pi3zRJ76Ix+MpG+iCutUAFhgYAAn4lN1KJ2jYBXTUs04KYCXQNQcRQ9TwNdchUQm1JdhmlDj1VwFsy+URHEE/zkw6P+PHuyQSyMfIKp0jfNS/Sw5lEr5cu7001YGbwyjJg==
  • 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: Fri, 08 Oct 2021 14:15:04 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 08.10.2021 15:38, Luca Fancellu wrote:
>> On 7 Oct 2021, at 08:15, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> 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);
>>>>>>>   }
>>>>>>> +#ifdef CONFIG_HAS_DEVICE_TREE
>>>>>>> +    /* 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.
> Yes right, in this case I did in another way because declaring the stub in 
> the .c file
> was (in my opinion) not the right thing to do, since also the name 
> (efi_arch_*) recalls
> something arch oriented and so not to be put in the common code.

Feel free to drop "arch" from the hook name.

> In this way any future architecture supporting DT, can just provide the 
> function (or a 
> stub) and we don’t have stubs in architectures that won’t ever support DT.
> In your opinion that solution could be acceptable?

Yes, but not preferable.




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