From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by mx.groups.io with SMTP id smtpd.web11.2510.1585764221581993477 for ; Wed, 01 Apr 2020 11:04:07 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=xJJyOv/V; spf=pass (domain: nuviainc.com, ip: 209.85.221.66, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f66.google.com with SMTP id h15so1145432wrx.9 for ; Wed, 01 Apr 2020 11:03:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=sKbA0qMfsY5IEWeAi5WT6V9mkGJWQcE1h6KbiphQTQw=; b=xJJyOv/VkPjvr6sGlVmEscvQTMrnKYz+d7s8WyCU95BOkFh+LP+4riimMf8ewUMAtc yG9TMLP3YIkwvfM7/iddk1eObvroj8oiDqAAR3zK8+LHrzhKoJxWjUzp80jdgF+9hPRy ktSeNi1MW1I81wlyivczKPXKQFVOVCPaLdjG4EeEjcIR0dYOwhezv2volpHdhs34IcaT 8GJ/6xuyNdLwA9my5xATuRdfZsE6OuObiF8mS/q7Es2ZtD1sil2KpYqEhUrkKsK8MB/s I5Xf1gBMg5wyVo+23l14I0VPcG//7gMF6vFK1yxeE2Lb3XRYIjJ8ywOj8/zFNi1B+wmg 0gzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=sKbA0qMfsY5IEWeAi5WT6V9mkGJWQcE1h6KbiphQTQw=; b=Y6/mC0XpsqOOiYaBdmW2daLd70O6U3VL6zPnha2gpSDdEr909BBwumDA2sTPN++4A4 dCLCK0ZhGHZr0mWF69sPvO1L0zUjheQ+ZWr6L1JHCdlvQNdK60JokLSdIr8jm9w+JE96 ZslpC121pDgqg1f9QlvhHKOoMRX+xHzg6M41eEe2tQwT4t8MJo+H7D45615EOCsu4r8N vGjwIrHJfgw9cgvv9Kx7cmGkghSiTr2P3ToBQsGDMWrg2RQs55hQfVX6iA+V/SXiwf/J tHp2iybcCsa42bVRnLrCbdx9Dbk+vAQbHyVUnfjGFcc+gaN8hGxYG9XDW/+CIdb+tfnp 6UCQ== X-Gm-Message-State: ANhLgQ1vpldVLfzXaHO/ngoxX78oD9nyYc3ikEZIyit88WzTKf1q05H7 oWhquVee83YPZI2N8O3z7XrgZQ== X-Google-Smtp-Source: ADFU+vvtKTLzcrPvjlUAXs2cv8rhd7mNEIO0RW2iJeUnENfZ1DfzWiy/SR+jJ2hrYr6AUaYOs83TcQ== X-Received: by 2002:a5d:54cb:: with SMTP id x11mr28739407wrv.179.1585764220209; Wed, 01 Apr 2020 11:03:40 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id m8sm3429510wmc.28.2020.04.01.11.03.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Apr 2020 11:03:39 -0700 (PDT) Date: Wed, 1 Apr 2020 19:03:37 +0100 From: "Leif Lindholm" To: Pankaj Bansal Cc: Meenakshi Aggarwal , Michael D Kinney , devel@edk2.groups.io, Varun Sethi , Samer El-Haj-Mahmoud , Jon Nettleton Subject: Re: [PATCH v2 23/28] NXP/LS1043aRdbPkg/ArmPlatformLib: Use Allocate pool Message-ID: <20200401180337.GF7468@vanye> References: <20200320143543.18615-1-pankaj.bansal@oss.nxp.com> <20200320143543.18615-24-pankaj.bansal@oss.nxp.com> MIME-Version: 1.0 In-Reply-To: <20200320143543.18615-24-pankaj.bansal@oss.nxp.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Mar 20, 2020 at 20:05:38 +0530, Pankaj Bansal wrote: > From: Pankaj Bansal > > Allocate Pages may allocate more memory than required for > VirtualMemoryTable. > There is no special requirement that VirtualMemoryTable size should be > page size aligned. > > Therefore, replace AllocatePages with AllocatePool. > > Signed-off-by: Pankaj Bansal I don't object to this as such (although one comment), but what is the purpose of this change? My comment is that most other platforms use AllocatePages for this. So this is diverging from the norm. Secondly, while I don't necessarily *like* the current design (copied across most ARM platforms), it's somewhat mitigated by the AllocatePages giving you a minimum of 128 entries before you start overwriting things. I don't know what you'll overwrite if you do go too far, but you will overwrite it *before* the ASSERT ever gets evaluated. / Leif > --- > .../LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf | 1 + > .../LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c | 5 +++-- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf > index 1faf99b99c54..c64032f32772 100644 > --- a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf > +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf > @@ -25,6 +25,7 @@ [Packages] > > [LibraryClasses] > ArmLib > + DebugLib > SocLib > > [Sources.common] > diff --git a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c > index f5fa308551aa..f8dd642e3cff 100644 > --- a/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c > +++ b/Platform/NXP/LS1043aRdbPkg/Library/ArmPlatformLib/ArmPlatformLibMem.c > @@ -43,10 +43,11 @@ ArmPlatformGetVirtualMemoryMap ( > > ASSERT (VirtualMemoryMap != NULL); > > - VirtualMemoryTable = (ARM_MEMORY_REGION_DESCRIPTOR*)AllocatePages ( > - EFI_SIZE_TO_PAGES (sizeof (ARM_MEMORY_REGION_DESCRIPTOR) * MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS)); > + VirtualMemoryTable = AllocatePool (sizeof (ARM_MEMORY_REGION_DESCRIPTOR) * > + MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS); > > if (VirtualMemoryTable == NULL) { > + DEBUG ((DEBUG_ERROR, "%a: Error: Failed AllocatePool()\n", __FUNCTION__)); > return; > } > > -- > 2.17.1 >