From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (NAM10-MW2-obe.outbound.protection.outlook.com [40.107.94.85]) by mx.groups.io with SMTP id smtpd.web10.17087.1582907937833646280 for ; Fri, 28 Feb 2020 08:38:58 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@amdcloud.onmicrosoft.com header.s=selector2-amdcloud-onmicrosoft-com header.b=l9OSL0BF; spf=none, err=SPF record not found (domain: amd.com, ip: 40.107.94.85, mailfrom: leo.duran@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=cV6csXJZcrM7zISahhzmI6auoxFiPZX5t9Wx7aEWe7fqu0DkjAH7h5vNkr07qKPV0UazyhH3ecebl4DylzO+nTbRLmUjCzO4Td9V4OyefD3vX6wIuPdL+QzvE9ySUgvJZoQCe5qBtiAk2LBSme4cs3XiLsTOdNuHgYBHxNymS8KWK5yCmzR4E/Ha9BLky/UoMWIOAQiPBpJysKphNHIqnzC/tmT7F9CM9o/meygFAZMq8r66WVNUbvRsLxSj23UYq2GP/7LK6ejzodqIwOD0JlKCOBc/EHDaaMxXp6mgOH1q9uZUaTj4gvv8P/bG6d8nIzfyZMUsA00iwLUvg/9fRQ== 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=jdvulYCSdhbmVrZizGBLVuyyY0VEEgiqO6o4AtxtM3g=; b=G6zaZjiMj+oqlUoCIKwClLXVNdN+ynw2luNMqau/nRphTPnIAOwNkAR3wcwQ0OZu5s7Xqt8xScx9Um2GG9gLBxgOFctsgZVf/HUQqOebfBynUT/Lym95jmWqld2UPgniWVqAExEYXMwlnolSZrbyH2+P64hAJKOq8YxUzge81jtuw5lIYepB0W40XwCf8zTd+kQ8Ca84Ul2sB1AgGlasyLEd+AiqMDyu6X6vtw/qWAq2S2pw3hoEriKw62fCm1ZfaTR0AzGq5Z4h3A2cEQHrmpGmDqrd0aHI1iaxdZ7YP977qE/iwBioWO3pUnT1x5Iedoj1TxfzfiqciLut0WVXCg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amdcloud.onmicrosoft.com; s=selector2-amdcloud-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=jdvulYCSdhbmVrZizGBLVuyyY0VEEgiqO6o4AtxtM3g=; b=l9OSL0BF3EvQDOEy+NS29I0NrTJrAmasCl4wkjyJ9sESs0PW5zi1QtI4bE6rokHyLs9EesTlHl6U9Q9pf8O+9+7hAA9pKq3S6H0dBf4WWYReNcAPs1FcmoPC/r5oSODFo65PRrBL9aVRizmTDDsNKwVe5rmDIaFuyWr01OJPMk4= Received: from BN6PR12MB1922.namprd12.prod.outlook.com (2603:10b6:404:106::14) by BN6PR12MB1218.namprd12.prod.outlook.com (2603:10b6:404:1b::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2772.15; Fri, 28 Feb 2020 16:38:54 +0000 Received: from BN6PR12MB1922.namprd12.prod.outlook.com ([fe80::d931:1942:a6b5:d74c]) by BN6PR12MB1922.namprd12.prod.outlook.com ([fe80::d931:1942:a6b5:d74c%7]) with mapi id 15.20.2772.018; Fri, 28 Feb 2020 16:38:54 +0000 From: "Duran, Leo" To: "Ni, Ray" , Laszlo Ersek , "devel@edk2.groups.io" , "Wu, Hao A" , "Fu, Siyuan" CC: "Dong, Eric" Subject: Re: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug in MpInitLib Thread-Topic: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug in MpInitLib Thread-Index: AQHV7BNuZBsSC+kbJkGHn9DFW2NUaKgspoeAgAB2EYCAAHmSsIAABrIAgAAMX4CAAAR2sIAAEzCAgAAAykCAAMs0AIAAzrJAgADSHgCAAJkIMA== Date: Fri, 28 Feb 2020 16:38:54 +0000 Message-ID: References: <1582659566-9893-1-git-send-email-leo.duran@amd.com> <734D49CCEBEEF84792F5B80ED585239D5C4542DA@SHSMSX104.ccr.corp.intel.com> <444c59ea-70dc-0edd-d680-add054dad2c5@redhat.com> <734D49CCEBEEF84792F5B80ED585239D5C463F32@SHSMSX104.ccr.corp.intel.com> <734D49CCEBEEF84792F5B80ED585239D5C46BE23@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C46BE23@SHSMSX104.ccr.corp.intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=leo.duran@amd.com; x-originating-ip: [173.170.80.115] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 0f22758d-ca84-4717-8c11-08d7bc6cb373 x-ms-traffictypediagnostic: BN6PR12MB1218: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-forefront-prvs: 0327618309 x-forefront-antispam-report: SFV:NSPM;SFS:(10001)(10009020)(6029001)(4636009)(346002)(136003)(39860400002)(376002)(396003)(366004)(199004)(189003)(8936002)(76116006)(33656002)(66946007)(66556008)(66476007)(71200400001)(64756008)(110136005)(4326008)(7696005)(86362001)(19627235002)(316002)(66446008)(81166006)(52536014)(45080400002)(6506007)(186003)(53546011)(478600001)(8676002)(55016002)(2906002)(966005)(5660300002)(81156014)(9686003)(26005);DIR:OUT;SFP:1101;SCL:1;SRVR:BN6PR12MB1218;H:BN6PR12MB1922.namprd12.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;A:1;MX:1; received-spf: None (protection.outlook.com: amd.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: EVGBTN4MLtAegs/CK/aB7jGSMihGOeR/oaPFy7La0SHQcvczlKIbjFr3imlK6VZ8ay/coV6wIyuWPMY/KI4Yib6eJQvzUQ8bcsr56hYDmUtLoNuwfke2YADXx+/bYoQrm6KvX2XhFtdYLYOTbUTrO5VJXI6Qtq3HKvtdEGzI0DUiDmQkyeZcK0bJXTLduCdLT5n3dt9pUSsAxQfWLd2RcjjASmG2D0xvxtu9uq2LwB4GRMS7yjWnFX0YJS44GG7QwS2Ud01rNtiQx87V5exj6s05I54WmJ/65i48lZ6sk3eWd8k2W925HXzR23c7CEOwHQyuPtmivCQqFnkAdBpH9+3nMN6EpM/QRjNSwa6uLbraXRGQb/LRv5IJ1otKGdcNl4lH3g31bmJcY5t/xki+6BHlBvaIWHxjJ/q5a/jbkpByYYtg1tA7pwGxud4QED03DZgH/MzoEct3Tq53p9JzVX1msSvBe0hsNmoWex8h4S+t67AFGdBz+Q54C56ZS+g/3etrrHUumVhoZi3K5NIT+C14FCuBWos44g/B+50TkgKN0txRhstdkNdcXTF77RXyjEsg6V52tXKjx8kmgYWh8/wEZkBvJrljGWohTz5Be8shxUzfaQldGr7z/hRgP6t2 x-ms-exchange-antispam-messagedata: xAM561L5lPkSvehWBScxEMuRgLvFBM5cY8+3P9Bb+ooCTQzM6cJpWsDuM+Y0jc9GGJiOk4AUAXht1yHHN35p/ftSMl4Z7wVhVMemuSH3O8e6kiXl0vkSepP8nATNJBp52WJW/X4tSzRdTpDxYDkJ6g== x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 0f22758d-ca84-4717-8c11-08d7bc6cb373 X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Feb 2020 16:38:54.8239 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: VtcTDTol0dqKrlRs6GXBNow+68TBr1UtBF2IPy2BFKM40eFo/Dt6097IBvXpnq1/ X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR12MB1218 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Ni, Ray [mailto:ray.ni@intel.com] > Sent: Friday, February 28, 2020 1:47 AM > To: Duran, Leo ; Laszlo Ersek ; > devel@edk2.groups.io; Wu, Hao A ; Fu, Siyuan > > Cc: Dong, Eric > Subject: RE: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug in > MpInitLib >=20 > Leo, > Thanks for your quick reply. >=20 > My most concern part to your patch is to expose a new API in LocalApicLib > checking whether the processor is AMD type. LocalApicLib is a library tha= t > operates on Local APIC registers while checking CPU type deals with CPUID > signature. It's not proper to mix the two things together just because > LocalApicLib needs to know CPU type in some of the operation. [Duran, Leo]=20 I can't argue that point strongly. It just seemed "related enough" to warrant the export to avoid duplication. >=20 > Because BaseLib already provides AsmCpuid() API and the check of CPU ID > signature is just one line of comparison, I prefer to call AsmCpuid() dir= ectly in > MpInitLib. >=20 > And in the patch to MpInitLib, since the AMD platform can set the two > microcode PCDs to 0 to skip the microcode detection, I think the only pla= ce > that needs to take care is in InitializeApData(). >=20 > What do you think? [Duran, Leo]=20 I take this to mean duplicating the Signature function in MpLib.c So, I will submit that patch instead. >=20 > Thanks, > Ray >=20 >=20 >=20 > > -----Original Message----- > > From: Duran, Leo > > Sent: Friday, February 28, 2020 2:17 AM > > To: Ni, Ray ; Laszlo Ersek ; > > devel@edk2.groups.io; Wu, Hao A ; Fu, Siyuan > > > > Cc: Dong, Eric > > Subject: RE: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug in > > MpInitLib > > > > > > > > > -----Original Message----- > > > From: Ni, Ray [mailto:ray.ni@intel.com] > > > Sent: Thursday, February 27, 2020 12:55 AM > > > To: Duran, Leo ; Laszlo Ersek > > > ; devel@edk2.groups.io; Wu, Hao A > > > ; Fu, Siyuan > > > Cc: Dong, Eric > > > Subject: RE: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug in > > > MpInitLib > > > > > > Leo and all, > > > I replied in > > > https://nam11.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fb= u > > > gzill > > > > a.tianocore.org%2Fshow_bug.cgi%3Fid%3D2556&data=3D02%7C01%7Cleo. > > > > duran%40amd.com%7C40786a41798d4173dd8508d7bb49aaa7%7C3dd8961 > > > > fe4884e608e11a82d994e183d%7C0%7C0%7C637183797396444421&s > > > > data=3DUoLRg%2ByFl%2BxyPB41xu1wOHpsf2euBrSe2HuD4CskTWg%3D&r > > > eserved=3D0 for a more general question about how uCode is used in AM= D > > > processors. > > > > > > Because this package recently exposed a new interface > > > ShadowMicrocodePpi, I try to involve Leo in the review from AMD > > > uCode's needs. > > > > > > Thanks, > > > Ray > > [Duran, Leo] Hi Ray, I just updated the ticket to say: > > AMD handles microcode patches outside of the context of UEFI. So EDK2 > > hooks (ShadowMicrocode PPI, et al) are not required. > > (Hence my comments in the MpInitLib patch I just submitted) > > > > > > > > > -----Original Message----- > > > > From: Duran, Leo > > > > Sent: Thursday, February 27, 2020 1:52 AM > > > > To: Laszlo Ersek ; Ni, Ray ; > > > > devel@edk2.groups.io; Wu, Hao A ; Fu, Siyuan > > > > > > > > Cc: Dong, Eric > > > > Subject: RE: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug > > > > in MpInitLib > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Laszlo Ersek [mailto:lersek@redhat.com] > > > > > Sent: Wednesday, February 26, 2020 12:45 PM > > > > > To: Duran, Leo ; Ni, Ray ; > > > > > devel@edk2.groups.io; Wu, Hao A ; Fu, Siyuan > > > > > > > > > > Cc: Dong, Eric > > > > > Subject: Re: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix > > > > > bug in MpInitLib > > > > > > > > > > On 02/26/20 17:39, Duran, Leo wrote: > > > > > > > > > > > > > > > > > >> -----Original Message----- > > > > > >> From: Laszlo Ersek [mailto:lersek@redhat.com] > > > > > >> Sent: Wednesday, February 26, 2020 11:21 AM > > > > > >> To: Duran, Leo ; Ni, Ray > > > > > >> ; devel@edk2.groups.io; Wu, Hao A > > > > > >> ; Fu, Siyuan > > > > > >> Cc: Dong, Eric > > > > > >> Subject: Re: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix > > > > > >> bug in MpInitLib > > > > > >> > > > > > >> On 02/26/20 16:46, Duran, Leo wrote: > > > > > >>> BTW, > > > > > >>> > > > > > >>> I also considered adding a flag to CPU_MP_DATA to make the > > > > > >>> usage of > > > > > >> PlatformId a bit more explicit. > > > > > >>> E.g., something like CpuMpData- > > > > > >>> CpuData[ProcessorNumber].IsValidPlatformId... So the init > > > > > >>> code would look > > > > > >> like this: > > > > > >>> > > > > > >>> // > > > > > >>> // NOTE: PlatformId is not relevant on AMD platforms. > > > > > >>> // > > > > > >>> if (StandardSignatureIsAuthenticAMD ()) { > > > > > >>> CpuMpData->CpuData[ProcessorNumber].IsValidPlatformId =3D > > > FALSE; > > > > > >>> else { > > > > > >>> PlatformIdMsr.Uint64 =3D AsmReadMsr64 > > > (MSR_IA32_PLATFORM_ID); > > > > > >>> CpuMpData->CpuData[ProcessorNumber].PlatformId =3D > > > > > >> (UINT8)PlatformIdMsr.Bits.PlatformId; > > > > > >>> CpuMpData->CpuData[ProcessorNumber].IsValidPlatformId =3D > TRUE; > > > > > >>> } > > > > > >>> > > > > > >>> This way "IsValidPlatformId" could be checked prior to using > > > "PlatformId". > > > > > >>> Anyway, that seemed a bit overkill, so I opted against it... > thoughts? > > > > > >> > > > > > >> I think a global flag is justified; in the above approach, > > > "IsValidPlatformId" > > > > > >> would not vary across "ProcessorNumber", so it does look like > > > > > >> useless generality. > > > > > > [Duran, Leo] > > > > > > Great point, Laszlo. > > > > > > Indeed, global makes senses in the case! > > > > > > I can prepare a v2-set to incorporate that. > > > > > > > > > > No, sorry, that wasn't what I meant. I didn't try to suggest a gl= obal > variable. > > > > > Instead, I meant that a "global check" (conceptually, i.e. > > > > > regardless of particular processor number) made sense. > > > > > > > > > > I'm also not particularly *against* a global variable. In other > > > > > words, I didn't try to comment on using a global variable *at all= *. > > > > > > > > > > Using a global variable might as well work, I just feel that > > > > > your current patches are good enough. > > > > [Duran, Leo] > > > > Great... I hear you. > > > > Then, I'd prefer not refactoring further at this point.... I hope > > > > Ray & Eric > > > agree. > > > > > > > > Thanks for your feedback! > > > > Leo. > > > > > > > > > > > > > > Thanks > > > > > Laszlo