From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR04-DB3-obe.outbound.protection.outlook.com (EUR04-DB3-obe.outbound.protection.outlook.com [40.107.6.44]) by mx.groups.io with SMTP id smtpd.web12.1660.1588836513584290244 for ; Thu, 07 May 2020 00:28:34 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nxp1.onmicrosoft.com header.s=selector2-nxp1-onmicrosoft-com header.b=OA+WkI1I; spf=pass (domain: oss.nxp.com, ip: 40.107.6.44, mailfrom: pankaj.bansal@oss.nxp.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ebFdGnE2UolM7nRAd4zhuWMVCpcoa4sVNJxyMf1HO/W++HM5Lb/3+9XGwNM6nj4zRsdYFO/IyC51PXu6ZRASYnVeR1x7iUewTANYfWUA2+gyntdCXExzIo+UHOLTkkRFzO+1/4hVTDZ3gGvqx6kWjsgPzIiv2UCruPFoLqUtYBYRFkZANyVIuMDZrFaZpJnysob/MU/KuiDQ1BP/FlmOymDU2rc8sd9YDNoPxitgWvU7Gw9Y2sywM9Ia8X9oizcjkDGWyVrCmiYCUJpKn0L5/Ob10PC4gI0mqnLYPCl9MVaRdvXI/V/2BDODDKCa7K5Izb/G0WsVDwJdJOGUJM9xQQ== 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=0f1XHXxMCP01cjtly0Qy6iMfGEboduVdovIHacZVA8g=; b=bvOBH+IeXdHbv5Xs4vbKjSOk73+D5fO4caVX4FySaSFdZVGg+PjB6L7wccIdmcwNWxLMcInVVDUovTX1m6jqK/wmKiSDRIWIx+m+bnn5N6hYOo2rANp21Kn7wnhGJEgBXBnGmrYLgFmyGlF81+jUjQ+/tWhizcvEv3nlrKXYoUg3QOKqdehJv7h1yPh+5cMb95almfgV/O+zUWzFcpodHCgMH3uK9Hh8cGnn9j5tWJMn4r5b8keT0OFHSrppfLuuLVTfimWtbbvbbWdSiF/BwO5y6VPSeZEPWOHCithEf0JLGYTRpPrrfhk9+DBrsoGm0/Xz8WGi0dTyWy9sDYifLA== 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=0f1XHXxMCP01cjtly0Qy6iMfGEboduVdovIHacZVA8g=; b=OA+WkI1IRvPBLzTy35E5pl2DdH+TykzUaNtAo4VR0KBHw1hqP4FS4Xn+11xkovkfC/kcb6wrGEcSQqvIXhq3SdH6e1GBBX9uZcvGvIg5ppUxcDRuv/wH2SC/F4IKXt7KAtW577bbZtYJdjQzcWA4YrukTlZgfTKeLWo1ozrbENo= Received: from VI1PR04MB5933.eurprd04.prod.outlook.com (2603:10a6:803:ec::16) by VI1PR04MB6894.eurprd04.prod.outlook.com (2603:10a6:803:13a::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2958.21; Thu, 7 May 2020 07:28:30 +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.2958.034; Thu, 7 May 2020 07:28:30 +0000 From: "Pankaj Bansal" To: Leif Lindholm , "Pankaj Bansal (OSS)" CC: Meenakshi Aggarwal , Michael D Kinney , "devel@edk2.groups.io" , Varun Sethi , Samer El-Haj-Mahmoud , Jon Nettleton , Ard Biesheuvel Subject: Re: [PATCH edk2-platforms v4 12/24] Silicon/NXP: Move RAM retrieval from SocLib Thread-Topic: [PATCH edk2-platforms v4 12/24] Silicon/NXP: Move RAM retrieval from SocLib Thread-Index: AQHWJEEaec8kUtll4Em/u/A6RSfN9A== Date: Thu, 7 May 2020 07:28:30 +0000 Message-ID: References: <20200501054955.13025-1-pankaj.bansal@oss.nxp.com> <20200501054955.13025-13-pankaj.bansal@oss.nxp.com> <20200506104405.GL21486@vanye> In-Reply-To: <20200506104405.GL21486@vanye> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: nuviainc.com; dkim=none (message not signed) header.d=none;nuviainc.com; dmarc=none action=none header.from=oss.nxp.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [49.36.133.207] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: ea399872-4bcb-4c15-cf67-08d7f2583dac x-ms-traffictypediagnostic: VI1PR04MB6894:|VI1PR04MB6894: x-ms-exchange-sharedmailbox-routingagent-processed: True x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 03965EFC76 x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: N954NwS93c0BK4NRYcZldsDYhk7aXCeiC+ajPJSZSC8/dJ72Hn7NsEd5NL2qcHvTm2CTxjoKRb+Hk6m8PVkZMC16tShZpB9lww6JgWPozQ4FWWrdploEW7mgBoO08pL/Z0vxXFS2ebWmrqQMRsF7L+xUZGOkocconQ2PitDcYD8vTsz5+xGaYKedNVAKx4RWKjZD5/inWyY9xoWsY/h5Bb/SeInzsvIcNDg8J8PVL85J6pR3qDeVmzJy4HGk69KG51Fzwoeq/AKNz4NbZJv/7Z24cLozCoKwkQMjUSi8OGDRldrXIWDPveODu2N6J1QjZ9M+YVkcFZ0v7pofDhICYhAHcpsf46PKvnSsWJTmARH/8x00vlaShUosB3va301brRhGpo/D/GFJSV3YdkEjd5tCeEm0Hs8loTfwJZVpnruR9j7lX/+erEhj8tBIfIqlBBS0bC6e570PLcUra/FJl7TP2m8YEKmR7iMucAaFw0/d2JLE14IaweZzWhDzvW7S1Fzw35VcR//nqJoLSONvyjTUiVh2fP1SZj9E22cyc6vPoW0jwJS3MZMri3k8BLiTsroim9eaN52M0Y0vgRxOyeImsQ4qtJlL3ub5zxxIRYc= 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:(4636009)(396003)(346002)(376002)(39860400002)(136003)(366004)(33430700001)(6506007)(71200400001)(26005)(52536014)(186003)(8676002)(8936002)(66446008)(64756008)(66476007)(66946007)(76116006)(66556008)(5660300002)(86362001)(7696005)(966005)(478600001)(83320400001)(83280400001)(83300400001)(2906002)(83310400001)(83290400001)(33440700001)(4326008)(316002)(54906003)(110136005)(33656002)(55016002)(9686003);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata: DTS13GjC74nvVD9OV+rvWdsuvRDKqJTyq8RSn+Wew2VID+m4Q4qtDB2DtY/adswDmSC9F+XTM5Kp4DCGLHUMd6R/+m7L6D511uOhnwS7sbrafh5w1hBZg9RmehBSSP4QHoWNOoh3V26eXmNxO6NEbKRjmkP8w87pF4RSAzVQXakyRGiHU0K8Kfw6/E3Bwi1yKRuppCjbTpP98vbHR2YIEb+Y3mOnGh8N60X5VXI5g0K5sikOShgrOty3SOIYSudkqe0RaTc5Q2CpkXc4qMJxg/+vfMI0TixV/GUMA0q1cH4hmu2S414n013Gkn+NabWEN8RcHlOn7KsyejBTjEL+RvSOhPvEkSYQEvL8r4dQS63leDvJFKO0djx7NqXnXoszmuILADuQIo+Gn8rv8tLnc9FdmMwmC++v+Q1/1LU2rrYqnvQdFaG2EiJRlgakyrfRwyJvQtamu+7qTh+ZSBSG4wH/l9pYmotr2OS4wtVAzyt6eWT7PnDbayjiYwpMLvgLbze99svul3OSueVR2H92FnTJ24tnzHNeAlbT8V/1YULfJFqKDi0TEzxY4DVPqUTWi2G6iqmm3N1QubhCD2loTK7NzpfwkQb7OPDgLbwKpow6lPH6Ry0FLMY9wYhZ0f+1MQhQxMdmypPHj/8Bhk0GYLrArOowt8qwx8+QxF+W5DIF+yrQ81ZpAP4DEiZYAcnNHvkdjjL5L4Qt81xn9Kj0yU+MPTJg/275fTNDfCUbbCZIYBZmezvq61GiMfzhtky3jpn13jI4ntNZmKxrB6EjOw3Qtu6oVgkkSsZjeIoLVuI= MIME-Version: 1.0 X-OriginatorOrg: oss.nxp.com X-MS-Exchange-CrossTenant-Network-Message-Id: ea399872-4bcb-4c15-cf67-08d7f2583dac X-MS-Exchange-CrossTenant-originalarrivaltime: 07 May 2020 07:28:30.0934 (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: V0aCYFsttT/VA/3KXSIF7d7Y836MdoHrBRibvJPJpzYoAAvRI+D7nfz1ZbROwCe1QaG41vH+x9SFlBZ33PyuTA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB6894 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Leif, > > + ARM_SMC_ARGS ArmSmcArgs; > > > > -Routine Description: > > + ArmSmcArgs.Arg0 =3D SMC_DRAM_BANK_INFO; > > + ArmSmcArgs.Arg1 =3D -1; >=20 > Should this be SMC_UNK? No. SMC_OK / SMC_UNK is returned values. While x0, x1 are arguments. I have explained this in the MemoryInitPeiLib.h // This SMC call works in this way: // x1 =3D -1 : return x0: SMC_OK, x1: total DDR Ram size // x1 >=3D number of DRAM regions to which DDR RAM is mapped : return x0: S= MC_UNK // 0 <=3D x1 < number of DRAM regions to which DDR RAM is mapped : return // x0: SMC_OK, x1: Base address of DRAM region, // x2: Size of DRAM region >=20 > > > > + ArmCallSmc (&ArmSmcArgs); > > > > + if (ArmSmcArgs.Arg0 =3D=3D SMC_OK) { > > + return ArmSmcArgs.Arg1; > > + } > > .... > > - { > > - Found =3D TRUE; > > - break; > > - } > > - NextHob.Raw =3D GET_NEXT_HOB (NextHob); > > + Status =3D GetDramRegionsInfo (DramRegions, ARRAY_SIZE (DramRegions)= ); > > + ASSERT_EFI_ERROR (Status); >=20 > Slightly concerned here because we end up with a variable Status that > is only *used* in DEBUG builds. That could lead to toolchain warnings. I don't think this would cause build warnings in RELEASE builds. I have tes= ted it. Also the Status is frequently being handled in this way in other places in = edk2: https://github.com/tianocore/edk2/blob/master/ArmPlatformPkg/PlatformPei/Pl= atformPeim.c#L90 >=20 > And I was completely serious last time around, I am OK with the return > value being cast away explicitly. What I meant with that is: > (VOID)GetDramRegionsInfo (DramRegions, ARRAY_SIZE (DramRegions)); >=20 I agreed with your suggestion to return EFI_BUFFER_TOO_SMALL from GetDramRe= gionsInfo, But If we discard the return value it means we are OK with some RAM not bei= ng reported to UEFI firmware and subsequently to OS. Isn't this a critical error ? > / > Leif >=20 > > + > > + FdBase =3D (UINTN)FixedPcdGet64 (PcdFdBaseAddress); > > + FdTop =3D FdBase + (UINTN)FixedPcdGet32 (PcdFdSize); > > + > > + // Declare memory regions to system