From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::22a; helo=mail-it0-x22a.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x22a.google.com (mail-it0-x22a.google.com [IPv6:2607:f8b0:4001:c0b::22a]) (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 3CBDB21B00DC1 for ; Tue, 21 Nov 2017 09:07:42 -0800 (PST) Received: by mail-it0-x22a.google.com with SMTP id 187so2901347iti.1 for ; Tue, 21 Nov 2017 09:11:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=pz6kg7bIzuXk5duBN2UIdL7R3rgqYbXRpsQ2uTF59IQ=; b=XThnCqCvBniRucEPHcsIcUxBFHh0qfMaueFWmlAveFngqNJOkBacqmZCQJTVo3Vg37 YimNR32baM1MTAi2WbIp/Ft5BYHru1dluc07UzpkJiDSzQrrJXO+/0LFNaTu2HFlOeXY gk3veHrl8fRgFtMb21noV73UDwLgBwmrKxf9Q= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=pz6kg7bIzuXk5duBN2UIdL7R3rgqYbXRpsQ2uTF59IQ=; b=aw9w7gG66tAxeXkCW5NAFjnRr7W8FuT9/xFLCmuZ1J1+cAJQEJ6SlumV0Yl2zCntwm Uf+wgHIvcMRhkXgfQ2rGF82KCfZsvvcfUoqLh4PAyniHZEJgYiCB1kRkYzP4bsT7QONn j9eh21FzhjOxzCknJ0ouHO1/la4zkwrOAFwA+ZPs+pcXUr06hCipmPZDq0G2NoIFffix a+UVbm5v2CaAcpvGe33oVUWsEIgmVtieS7J3+Mt6O1jHIP2QmJzwporfMBFP1s/2s9ti yrCERs1ntBJH/NZr4piPIJi5Foa/wW7I2fIRuWxfMZ129MWkCxj0xKpo+fOy3Josxk0y 6DZg== X-Gm-Message-State: AJaThX64xlp/DYnR8aYTJgwSecqCmgdZvp3o4iEheDgVtostnbCRJcXa xHjanLiEbChRibn5zKjoIzXBwHaH8k1Lle8JkcObsw== X-Google-Smtp-Source: AGs4zMZZY0gEN6p2OVS+kWn35Fmq9TGzZyBOliytmKYuBbHwR6aMBix/EcmphqLaaUx0D5T0ONj3PZUo7lsfxM1gKpU= X-Received: by 10.36.31.212 with SMTP id d203mr2762570itd.48.1511284317724; Tue, 21 Nov 2017 09:11:57 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.16 with HTTP; Tue, 21 Nov 2017 09:11:57 -0800 (PST) In-Reply-To: <1c112eca-96a0-be8a-5f68-cc91d17e3a1d@redhat.com> References: <20171117160913.17292-1-ard.biesheuvel@linaro.org> <20171117160913.17292-13-ard.biesheuvel@linaro.org> <1c112eca-96a0-be8a-5f68-cc91d17e3a1d@redhat.com> From: Ard Biesheuvel Date: Tue, 21 Nov 2017 17:11:57 +0000 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Leif Lindholm Subject: Re: [PATCH 12/15] ArmVirtPkg/ArmVirtQemu: add ArmVirtMemInfoLib implementation X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Nov 2017 17:07:43 -0000 Content-Type: text/plain; charset="UTF-8" On 21 November 2017 at 16:56, Laszlo Ersek wrote: > On 11/17/17 17:09, Ard Biesheuvel wrote: >> Clone the existing ArmPlatformGetVirtualMemoryMap () for this platform, >> clean it up slightly (by removing the support for uncached DRAM mappings), >> and turn it into a new ArmVirtMemInfoLib implementation. > > I've looked at this patch with "git show --find-copies-harder". It looks > OK, but the commit message could be improved: > > (1) the support for uncached DRAM mappings is removed in the copy-origin > lib instance, in patch 09/15 ("ArmVirtPkg/ArmVirtPlatformLib: remove > support for uncached mappings"). I think this sentence should be dropped > from the commit message. > > (2) There are other cleanups however: > > - factor out TableSize and TopOfMemory > > - replace EFI_D_* with DEBUG_* > > - fetch PcdFdBaseAddress with PcdGet64(), not FixedPcdGet64(). (This is > matched by the [Pcd] / [FixedPcd] sections in the new INF file.) > > Can you elaborate on the last item? I wonder if that change qualifies as > cleanup. (I'm fine if the change is justified by some other flexibility, > but it should be documented please.) > > With the commit message updated: > > Reviewed-by: Laszlo Ersek > Thanks. But is actually based on ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib, which explains the latter point. However, given that we will be sharing it between ArmVirtQemu and ArmVirtQemuKernel later on (which is apparently justified, since git can't even tell them apart), it makes sense to elaborate a bit on the differences and changes wrt the originals.