From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::d42; helo=mail-io1-xd42.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io1-xd42.google.com (mail-io1-xd42.google.com [IPv6:2607:f8b0:4864:20::d42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7825121190717 for ; Mon, 26 Nov 2018 03:32:10 -0800 (PST) Received: by mail-io1-xd42.google.com with SMTP id g8so13623583iop.10 for ; Mon, 26 Nov 2018 03:32:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=OfBpyB0E6Yl35D5o/6X94wSbgGck82jPoERkBGpvfjM=; b=SUfDILwVxZuDNFjTo88KF5vE/NWNPwe8i76dVpGSFs1+QqT4E9fSeFnwOjGuy0D1eZ v4vL/8OSnN+1hFgAqiZva+Qg9al9HdeO6Odue08y6XKLb3jebnAgi1XI51iAbvS4SyPB dJpr91L2vv4x6UCCmq/a6qPrkxyX6b4Dq/CkE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=OfBpyB0E6Yl35D5o/6X94wSbgGck82jPoERkBGpvfjM=; b=ihL9X0Fh/YMIH8lMUm8jb4xA4uSXg74Y/sfOzQ89VRY9YXQErR88N0dcaoz2EUlPqF D479NZo1zkyglobtSyhIUDPv2rYyVl/C9HxVTUr3MQH+cWSH707PgXh9bOBxkqGglgIb VTB6K458w4WDXns1Ftfltznyf1ulpjGNFGQdHrM7e3Aq+CSDf3RZOWGIK5d9Kqz38iYP 8I8DETH8TidNaVPofQ5f4d5lvIPvoYtSE3UYtmSnxuvmD1/JR8I+rcg7Tc6B4dEfHT4P 1jl7GSOAXL2B/N8LWVjtuVfbAGGrYVChI7BfnxM5sLQHbMbMs3NLL8E3sFzrFLOMowpV fJuQ== X-Gm-Message-State: AA+aEWZATEPLD2OjgYS6jxoaCDh6T4drHjzqCWYMilsrOfW3A8uwPbbi BG0nzkm9vWs9xmMmZi8lBbIsfj6zUxfuopptvXIjpg== X-Google-Smtp-Source: AFSGD/Vb5c2mV0fOwnd+mLPGz3Osl/fm04kLPAPNYpy0JRB4AtJI7zcLMXTNre/RqcThtm+LSge2wHQ1PkLb4/WixNk= X-Received: by 2002:a5d:8402:: with SMTP id i2mr11726143ion.173.1543231929445; Mon, 26 Nov 2018 03:32:09 -0800 (PST) MIME-Version: 1.0 References: <20181123121431.22353-1-ard.biesheuvel@linaro.org> <060a1e19-26b5-e62b-ca13-03cf6db95517@redhat.com> In-Reply-To: <060a1e19-26b5-e62b-ca13-03cf6db95517@redhat.com> From: Ard Biesheuvel Date: Mon, 26 Nov 2018 12:31:58 +0100 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Leif Lindholm , Auger Eric , Andrew Jones , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= , Julien Grall Subject: Re: [PATCH 0/5] ArmPkg, ArmVirtPkg: lift 40-bit IPA space limit X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Nov 2018 11:32:10 -0000 Content-Type: text/plain; charset="UTF-8" On Mon, 26 Nov 2018 at 11:22, Laszlo Ersek wrote: > > On 11/23/18 13:14, Ard Biesheuvel wrote: > > The ArmVirtQemu targets currently limit the size of the IPA space to > > 40 bits because that is all what KVM supports. However, this is about > > to change, and so we need to update the code if we want to ensure that > > our UEFI firmware builds can keep running on systems that set values > > other than 40 (which could be > 40 or < 40) > > > > So add a helper to ArmLib to read the number of supported address bits (#1) > > and take this into account in the page table code (#2), which allows > > PcdPrePiCpuMemorySize to assume a value that exceeds the capabilities of > > the CPU. > > > > Patch #3 is mostly a cleanup patch, to switch to the new helper added in > > patch #1. No functional changes intended. > > > > Patch #4 builds the CPU hob (and thus declares the size of the GCD memory > > space) based on the CPU capabilities rather than the value of > > PcdPrePiCpuMemorySize, to prevent any potential regressions in memory > > utilization when we bump PcdPrePiCpuMemorySize back to 48. > > > > Patch #5 drops the definitions of PcdPrePiCpuMemorySize, reverting its > > value back to the default 48. > > Staring a bit more at this, I wonder if it were helpful to reorder the > patches like this (just thinking loudly, I'm not even suggesting it, I'm > just curious about your opinion): > > - patch #1: keep in place (introduce new helper) > - patch #2: current patch #3 (ArmVirtPkg refactoring), to benefit from > the new helper at once, without any relation to the feature that's the > end goal here. > - patch #3: current patch #2, build page tables with CPU PA limits > accounted for > - patch #4: current patch #4, build GCD memory space map with CPU PA > limits accounted for > - patch #5: remove the traces of the now useless PCD from ArmVirt > > So basically, swap around #2 and #3. It's not really important; the > reason I'm thinking of it is the following though: while removing the > 40-bit limitation, my first thought is, "what are the current consumers, > what needs to be updated". > > The current structuring of the series, with patch #3 in the middle, > suggests that ArmVirtMemInfoLib instances are consumers. That's not the > case however, they already fetch the CPU PA limits dynamically. So they > need no functional updates, just a cleanup / centralization. That's why > I'd find it helpful to handle ArmVirtMemInfoLib right after the > introduction of the helper. > > The actual consumers (in need of functional updates) are the page tables > and the GCD memory space map (two concepts, not three). > > If you think this would be an improvement, please consider the > reordering. No need to repost just for this. > Yes, I think that makes sense.