From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR05-DB8-obe.outbound.protection.outlook.com (EUR05-DB8-obe.outbound.protection.outlook.com [40.107.20.81]) by mx.groups.io with SMTP id smtpd.web12.10103.1586758282067957766 for ; Sun, 12 Apr 2020 23:11:23 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nxp1.onmicrosoft.com header.s=selector2-nxp1-onmicrosoft-com header.b=LVRuH09y; spf=pass (domain: oss.nxp.com, ip: 40.107.20.81, mailfrom: pankaj.bansal@oss.nxp.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=A4iGCWf5LflkhpzcCGntajhb3IgNEUKAGilX1I0q8piSNpQIqGC3VwV8NYleXcdwLZRsXWhGVKPBGIKoH5dX6SfCB0uVWAtYPH1cPI7hi55yvMoK6qss59FpyzDyAsKG7kmoa6wty+Uzy1PEm+ZN21aaMIAgJ5SXuwcz8hpuO1ygey7ZR72VDA+6pz7rjD//mCWRF7SnfGWB1FN5f5/5F6lVI2l9WW9IWeMOvVaPfZHA/NkOzaRafJSJaMi/WFrKrCbyiJ/fo52S1j9XyI/qjpgtamvRN6MxnF4q+Jow/7aIKoUowPboluDKbfeZfDSNVkQN4Z/EOtldMtu0hbBcaA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=WDGRDCPDXdd7ya/FQwnDqhuc4roo5slr/+sOH6K7s0o=; b=me1nu1sLRqm+OkaudbJztW+I+JSrSJ5S/XyMu5qkezv/ef3p7i2emIibipclH3h9rTDLfYbPc0tFMQZiot5Ev6A59ywJzb2Rz+jf+Qe8fFofdqFwpmLq/C0wIr+8VYl1zj4/7cc0zJK/Za0NiaTqQqde5y5RQRjuqWQDaizegtK8x7NJwfTAW40jMbyfXdkC1svUlXw/FA+93++8p0BeBMM7KlMwrEBMmxk962t//TECN6JgF/kChp0UCxEBn7iz29VTBP/u9cwv4IJBw2hESZ+7fbQDMaA3/EOcQtF7sRlqu91YN9pWbE5F90i+0t4RUsK7cAFFjnj+qNq9pE2eQA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oss.nxp.com; dmarc=pass action=none header.from=oss.nxp.com; dkim=pass header.d=oss.nxp.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=NXP1.onmicrosoft.com; s=selector2-NXP1-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=WDGRDCPDXdd7ya/FQwnDqhuc4roo5slr/+sOH6K7s0o=; b=LVRuH09ygaprMouKkiEzEjx517R1cjSFa8kFa4PsMwGI8E/8cEGtblxH8FB2uCUAJ22DvIicZYQ2+403LBtCRIAwBP8nqI1zkZC349KvQ+qaCtvn2Maf/TH91vvgJTkxh+Z2MxieUsBKrxpOiCc4/PYMN+vWJOcOAlev57OAchI= Received: from VI1PR04MB5933.eurprd04.prod.outlook.com (2603:10a6:803:ec::16) by VI1PR04MB4480.eurprd04.prod.outlook.com (2603:10a6:803:65::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2900.20; Mon, 13 Apr 2020 06:11:19 +0000 Received: from VI1PR04MB5933.eurprd04.prod.outlook.com ([fe80::45c4:8846:5327:9513]) by VI1PR04MB5933.eurprd04.prod.outlook.com ([fe80::45c4:8846:5327:9513%7]) with mapi id 15.20.2900.028; Mon, 13 Apr 2020 06:11:19 +0000 From: "Pankaj Bansal" To: Leif Lindholm , "Pankaj Bansal (OSS)" , 'Ard Biesheuvel' CC: Meenakshi Aggarwal , Michael D Kinney , "devel@edk2.groups.io" , Varun Sethi , Samer El-Haj-Mahmoud , Jon Nettleton , "ard.biesheuvel@arm.com" Subject: Re: [PATCH v2 23/28] NXP/LS1043aRdbPkg/ArmPlatformLib: Use Allocate pool Thread-Topic: [PATCH v2 23/28] NXP/LS1043aRdbPkg/ArmPlatformLib: Use Allocate pool Thread-Index: AQHWEVpY3bg5QBv5ekaWO4K4LgJbpA== Date: Mon, 13 Apr 2020 06:11:18 +0000 Message-ID: References: <20200320143543.18615-1-pankaj.bansal@oss.nxp.com> <20200320143543.18615-24-pankaj.bansal@oss.nxp.com> <20200401180337.GF7468@vanye> <20200407130826.GP14075@vanye> In-Reply-To: <20200407130826.GP14075@vanye> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=pankaj.bansal@oss.nxp.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [49.36.131.169] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 7b6dc491-ab30-45fb-5fe6-08d7df717b91 x-ms-traffictypediagnostic: VI1PR04MB4480:|VI1PR04MB4480: x-ms-exchange-sharedmailbox-routingagent-processed: True x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-forefront-prvs: 037291602B x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI1PR04MB5933.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(10009020)(4636009)(136003)(376002)(346002)(366004)(396003)(39860400002)(9686003)(55016002)(4326008)(316002)(26005)(2906002)(186003)(33656002)(5660300002)(110136005)(54906003)(71200400001)(64756008)(52536014)(66476007)(66556008)(66446008)(86362001)(81156014)(76116006)(478600001)(66946007)(7696005)(6506007)(8936002)(8676002);DIR:OUT;SFP:1101; received-spf: None (protection.outlook.com: oss.nxp.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: iEQnYlE2MlgTN6A9e2h6GIBJkFtk11l+XmCJjVnF5dCSCqkNr9Mi3AuFgJlCAX6kChUK0R5lHJyZXYbE7rSAr1I+TbAl6Jqbpax9WXXQjnJGNRzjX99bgjpjWWeXVRP9ySP90h263xAGwjjx/y6ATexXG68sku5zDjJuG7jkm0aQJysJhbChLAblSN9QYDRfDyHcR0rf7s2CFzxTicSp5PDuVf4I+gsMjsB8FgipRXq/7d/qgXRoz509DUshv6bc/KAg2uA6t1I2rPILDMgb6Eq0nTVO9m2XUc8qz7ibWy7E51nWfTq0yuZRku9oXJR+ePH3ZzlZN8uA8iXhKmjbeAngB6P/GpGTohsoy4dBengsA3pINiQcYIIzstPiRmndYwWv+83RpUkwTtuKH8MT8rtOwlHg+IM/NBbgMCuBM00h+GI8UiLHPBUcgpFEReQ1 x-ms-exchange-antispam-messagedata: 9KSS9CcmhQrdYO3XCGIUY0hpmeaTId71r2hgt4jFRjuLawc/fjcU0UvQj71bh4COvkzgb4ILRsYB70lKFc+iyoFEK726K/MXELpUM5Up6qlFRmTVhBvRlyXXT8khcVtJEy3F0SZf4chEHVh7T3PIVg== MIME-Version: 1.0 X-OriginatorOrg: oss.nxp.com X-MS-Exchange-CrossTenant-Network-Message-Id: 7b6dc491-ab30-45fb-5fe6-08d7df717b91 X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Apr 2020 06:11:19.3118 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: mejlINI3mGUK7DAQ1VOiNV30vrhzFCvn7Zh9Byh3yQIVWoPr9aIHqMpoCslAkrAyj8a0Ay3JOI6aP9Lfm10kBA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB4480 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Ard, > > > > > > I don't object to this as such (although one comment), but what is th= e > > > purpose of this change? > > > > > > My comment is that most other platforms use AllocatePages for this. S= o > > > this is diverging from the norm. > > > > I referred ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c >=20 > Sure. This one was converted to AllocatePool because the QEMU virt > machine is very simple (because it does not emulate much real > hardware) and the port rarely changes. >=20 > Ard's what's your opinion - do you think this worth it even for a > platform that has > #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 25 > ? >=20 > Clearly there is still wasteage going on, but it's 16% less bad. >=20 Can you please reply ? I have to send v3 of these patches. Please let me know if I need to keep or remove these changes ? > > > 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* th= e > > > ASSERT ever gets evaluated. > > > > > > > We can improve this by evaluating ASSERT after each entry like this : > > VirtualMemoryTable[Index].PhysicalBase =3D 0xXXXXXXXX; > > VirtualMemoryTable[Index].VirtualBase =3D 0xXXXXXXXX; > > VirtualMemoryTable[Index].Length =3D 0xXXXXXXXX; > > VirtualMemoryTable[Index++].Attributes =3D 0xXXXXXXXX; > > > > ASSERT (Index < MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS); >=20 > Whilst functionally preferable, I think that would make for very > tedious reading. I'll let Ard call this one. >=20 This is also a minor improvement that I am proposing. Please comment if this should be done or not ?