From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 18455C433F5 for ; Fri, 5 Nov 2021 15:33:45 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C9E5461245 for ; Fri, 5 Nov 2021 15:33:44 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C9E5461245 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.xenproject.org Received: from list by lists.xenproject.org with outflank-mailman.222485.384703 (Exim 4.92) (envelope-from ) id 1mj1DU-0000d8-JS; Fri, 05 Nov 2021 15:33:20 +0000 X-Outflank-Mailman: Message body and most headers restored to incoming version Received: by outflank-mailman (output) from mailman id 222485.384703; Fri, 05 Nov 2021 15:33:20 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mj1DU-0000d1-G9; Fri, 05 Nov 2021 15:33:20 +0000 Received: by outflank-mailman (input) for mailman id 222485; Fri, 05 Nov 2021 15:33:19 +0000 Received: from se1-gles-flk1-in.inumbo.com ([94.247.172.50] helo=se1-gles-flk1.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1mj1DT-0000cv-Lu for xen-devel@lists.xenproject.org; Fri, 05 Nov 2021 15:33:19 +0000 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by se1-gles-flk1.inumbo.com (Halon) with ESMTPS id b232a093-3e4d-11ec-a9d2-d9f7a1cc8784; Fri, 05 Nov 2021 16:33:18 +0100 (CET) Received: by mail.kernel.org (Postfix) with ESMTPSA id A9BC161245; Fri, 5 Nov 2021 15:33:15 +0000 (UTC) X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: b232a093-3e4d-11ec-a9d2-d9f7a1cc8784 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1636126396; bh=H9UUUqkgOcHzQWMv8LWu8HE74C6jUXBkpgUx5V8aikY=; h=Date:From:To:cc:Subject:In-Reply-To:References:From; b=KgDt1A3WFQSDVKUVHCYfq+8pn9jxEoNky+EeCT4jxow3TpokFtLbX0bX0y0cFJQ0q SRFseoY86GRcsF1k2wADtp3zCE9YYH5Hkdeb+KskWtaMrpxxF5KYizNd4ADffoyHAi aCmnuBFN3xuOaVrj/dmssRY9L42PrssGaFzYzmPJqgPAihnt5Ljq2slzHpiIOGruEZ NVYly1K/lt3g0mceW5g1zJbK7ntg1OAOTG8OsjGeIzc96/Kp+uhian+eglONw5Abrj W/FIaW+57Ypbsvdn8+x7vztXJhhoMvGf45fwohY7GIOQvCGT6Ps8DR8QenGxyjSZpi z8tNTk/DZeyaA== Date: Fri, 5 Nov 2021 08:33:14 -0700 (PDT) From: Stefano Stabellini X-X-Sender: sstabellini@ubuntu-linux-20-04-desktop To: Jan Beulich cc: Stefano Stabellini , Bertrand Marquis , wei.chen@arm.com, Ian Jackson , Julien Grall , Volodymyr Babchuk , xen-devel@lists.xenproject.org, Luca Fancellu Subject: Re: [PATCH-4.16 v2] xen/efi: Fix Grub2 boot on arm64 In-Reply-To: Message-ID: References: <20211104141206.25153-1-luca.fancellu@arm.com> <81685961-501e-7a41-6f6f-bc4491645264@suse.com> <97C884F7-0577-4996-AB79-0A07A8D48FD8@arm.com> <9E52FA33-422B-4B1C-A6AF-601CDF565700@arm.com> User-Agent: Alpine 2.22 (DEB 394 2020-01-19) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="8323329-2108625499-1636126396=:284830" This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --8323329-2108625499-1636126396=:284830 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT On Fri, 5 Nov 2021, Jan Beulich wrote: > On 04.11.2021 22:50, Stefano Stabellini wrote: > > On Thu, 4 Nov 2021, Luca Fancellu wrote: > >>> On 4 Nov 2021, at 21:35, Stefano Stabellini wrote: > >>> On Thu, 4 Nov 2021, Luca Fancellu wrote: > >>>>> On 4 Nov 2021, at 20:56, Stefano Stabellini wrote: > >>>>> @@ -851,10 +853,14 @@ static int __init handle_dom0less_domain_node(EFI_FILE_HANDLE dir_handle, > >>>>> * dom0 and domU guests to be loaded. > >>>>> * Returns the number of multiboot modules found or a negative number for error. > >>>>> */ > >>>>> -static int __init efi_check_dt_boot(EFI_FILE_HANDLE dir_handle) > >>>>> +static int __init efi_check_dt_boot(EFI_LOADED_IMAGE *loaded_image) > >>>>> { > >>>>> int chosen, node, addr_len, size_len; > >>>>> unsigned int i = 0, modules_found = 0; > >>>>> + EFI_FILE_HANDLE dir_handle; > >>>>> + CHAR16 *file_name; > >>>>> + > >>>>> + dir_handle = get_parent_handle(loaded_image, &file_name); > >>>> > >>>> We can’t use get_parent_handle here because we will end up with the same problem, > >>>> we would need to use the filesystem if and only if we need to use it, > >>> > >>> Understood, but it would work the same way as this version of the patch: > >>> if we end up calling read_file with dir_handle == NULL, then read_file > >>> would do: > >>> > >>> blexit(L"Error: No access to the filesystem"); > >>> > >>> If we don't end up calling read_file, then everything works even if > >>> dir_handle == NULL. Right? > >> > >> Oh yes sorry my bad Stefano! Having this version of the patch, it will work. > >> > >> My understanding was instead that the Jan suggestion is to revert the place > >> of call of get_parent_handle (like in your code diff), but also to remove the > >> changes to get_parent_handle and read_file. > >> I guess Jan will specify his preference, but if he meant the last one, then > >> the only way I see... > > > > I think we should keep the changes to get_parent_handle and read_file, > > otherwise it will make it awkward, and those changes are good in their > > own right anyway. > > As a maintainer of this code I'm afraid I have to say that I disagree. > These changes were actually part of the reason why I went and looked > how things could (and imo ought to be) done differently. You know this code and EFI booting better than me -- aren't you concerned about Xen calling get_parent_handle / dir_handle->Close so many times (by allocate_module_file)? My main concern is performance and resource utilization. With v3 of the patch get_parent_handle will get called for every module to be loaded. With dom0less, it could easily get called 10 times or more. Taking a look at get_parent_handle, the Xen side doesn't seem small and I have no idea how the EDK2 side looks. I am just worried that it would actually have an impact on boot times (also depending on the bootloader implementation). --8323329-2108625499-1636126396=:284830--