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 X-Spam-Level: X-Spam-Status: No, score=-6.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E5734C433E2 for ; Mon, 14 Sep 2020 15:51:50 +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 6A548208DB for ; Mon, 14 Sep 2020 15:51:50 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=citrix.com header.i=@citrix.com header.b="YutmchZh" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6A548208DB Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=citrix.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kHqlM-0004ZF-6N; Mon, 14 Sep 2020 15:51:28 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kHqlK-0004ZA-Oj for xen-devel@lists.xenproject.org; Mon, 14 Sep 2020 15:51:26 +0000 X-Inumbo-ID: 47c26fe6-7880-45dd-89f6-ee9df3a38919 Received: from esa5.hc3370-68.iphmx.com (unknown [216.71.155.168]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 47c26fe6-7880-45dd-89f6-ee9df3a38919; Mon, 14 Sep 2020 15:51:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1600098685; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=zNmAcsDde02wdiZpyIQRwEpgrcg5DjLMnBO5JvDuNL4=; b=YutmchZhEYNdU7mQM7NfW/E+fLRYM2X8kvkcDWSFI7pxSAVcWOgnHTi6 nyJsgEdVSe/ES44EBxqxNDumyv7mYJqNFKhztu6vNolj/7ELw+ppxqNIB FnBVHy2v3UsPzV7VmYK2OS8UlU6nzB+Eut+EtWZMKuSSrc4age4z4PXEI s=; Authentication-Results: esa5.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none IronPort-SDR: 0CqRPi2S669xpTIWZ/nP3l/+62Zwd6UTPATkRLXXgeo4+eg2ZJty4teA/udU5ti5rT99+WVvoD hmhTcdn9c2qcIDHraq7yv3bGfqo+Gi79l/G8+eSKEzC8QdQK9DxgycFCd21Y1qkjw6Y3y9I5pg E/4b3ePcuvjtg/3NJdQW5vaZ5x/5kveaQwE6kyWB33BttLo5kQ9XsI4V/ATeGTNySgboyFN/Fq rPJ95/bpkWAgtKTBhc2Yik+p6ZeGl8HOKWd4RP6sI2wZSzfcNJDSxU88q6v6Vac/7m6UUIn0fe zJU= X-SBRS: 2.7 X-MesageID: 26767350 X-Ironport-Server: esa5.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.76,426,1592884800"; d="scan'208";a="26767350" Date: Mon, 14 Sep 2020 17:51:16 +0200 From: Roger Pau =?utf-8?B?TW9ubsOp?= To: Jan Beulich CC: "xen-devel@lists.xenproject.org" , "Andrew Cooper" , George Dunlap , Ian Jackson , "Julien Grall" , Wei Liu , Stefano Stabellini , Lukasz Hawrylko Subject: Re: [PATCH v2 2/2] EFI: free unused boot mem in at least some cases Message-ID: <20200914155116.GH753@Air-de-Roger> References: <5dd2fcea-d8ec-1c20-6514-c7733b59047f@suse.com> <20200914151608.GF753@Air-de-Roger> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-ClientProxiedBy: AMSPEX02CAS02.citrite.net (10.69.22.113) To FTLPEX02CL06.citrite.net (10.13.108.179) X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" On Mon, Sep 14, 2020 at 05:26:57PM +0200, Jan Beulich wrote: > On 14.09.2020 17:16, Roger Pau Monné wrote: > > On Mon, Aug 24, 2020 at 02:08:11PM +0200, Jan Beulich wrote: > >> Address at least the primary reason why 52bba67f8b87 ("efi/boot: Don't > >> free ebmalloc area at all") was put in place: Make xen_in_range() aware > >> of the freed range. This is in particular relevant for EFI-enabled > >> builds not actually running on EFI, as the entire range will be unused > >> in this case. > >> > >> Signed-off-by: Jan Beulich > > > > Acked-by: Roger Pau Monné > > Thanks much. > > >> @@ -1145,7 +1146,8 @@ void __init noreturn __start_xen(unsigne > >> > >> /* > >> * This needs to remain in sync with xen_in_range() and the > >> - * respective reserve_e820_ram() invocation below. > >> + * respective reserve_e820_ram() invocation below. No need to > >> + * query efi_boot_mem_unused() here, though. > >> */ > >> mod[mbi->mods_count].mod_start = virt_to_mfn(_stext); > >> mod[mbi->mods_count].mod_end = __2M_rwdata_end - _stext; > > > > I find this extremely confusing, we reuse mod_start/mod_end to contain > > a mfn and a size (in bytes) instead of a start and end address (not > > something that should be fixed here, but seeing this I assumed it was > > wrong). > > While perhaps somewhat confusing, I still think it was a fair thing > to do in favor of introducing a completely new way of propagating > respective information, and then having the consumer of this data > look at two different places. Maybe we could add a union there so that mod_end would alias with mod_size or something. > >> +bool efi_boot_mem_unused(unsigned long *start, unsigned long *end) > >> +{ > >> + *start = (unsigned long)ebmalloc_mem + PAGE_ALIGN(ebmalloc_allocated); > >> + *end = (unsigned long)ebmalloc_mem + sizeof(ebmalloc_mem); > >> + > >> + return *start < *end; > >> +} > >> + > >> void __init free_ebmalloc_unused_mem(void) > >> { > >> -#if 0 /* FIXME: Putting a hole in the BSS breaks the IOMMU mappings for dom0. */ > >> unsigned long start, end; > >> > >> - start = (unsigned long)ebmalloc_mem + PAGE_ALIGN(ebmalloc_allocated); > >> - end = (unsigned long)ebmalloc_mem + sizeof(ebmalloc_mem); > >> +#ifdef CONFIG_X86 > >> + /* FIXME: Putting a hole in .bss would shatter the large page mapping. */ > > > > Could you make the ebmalloc size (EBMALLOC_SIZE) 2MB (and aligned), so > > that you would only shatter the malloc'ed pages but not the > > surrounding mappings? > > > > That would be a good compromise IMO. > > Yes, that's what I've been considering as a compromise as well. In > fact I was further thinking whether to allocate the space from the > linker script instead of having a global/static object. Maybe by > extending into the .pad section, which is already 2Mb aligned anyway. We would have to adjust the calculations then, but would be fine IMO as it won't require poking a hole in the bss section. I assume we would need to then move _end to cover it. > Another option is to not further align the whole blob at all and > merely free whatever comes past the next 2Mb boundary (and is not > in use). This would avoid having an up to 2Mb block of unused, not > freed memory ahead of the ebmalloc one. I would just place it at the end of the loaded image (after bss) as it seems simpler. Roger.