From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web11.23.1582739129943207205 for ; Wed, 26 Feb 2020 09:45:30 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=MPRUlOoh; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1582739129; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=N28vNaloDwu3xqrrLXSOPiY86g6QNW8fwixWCwD6+AA=; b=MPRUlOohF1ZlNEAk6HFecSFOXVFwwbZpwed+f4f61rv9MFROdAMCJPmP7e7cWQo3hXe8Z9 b7TjhrF7Jt6XBh65ihUEkJkOFxY5FQdDxU4vPOS7I9aEGRf6R7Zm3Fud/6Cx5SrA2RaZwg gKR4087O9eCjAD4+2D9N1xiRvd86HnU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-364-H97EKfR-Nru7CxFhjGmgOg-1; Wed, 26 Feb 2020 12:45:25 -0500 X-MC-Unique: H97EKfR-Nru7CxFhjGmgOg-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C260DDB31; Wed, 26 Feb 2020 17:45:23 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-185.ams2.redhat.com [10.36.116.185]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3828392985; Wed, 26 Feb 2020 17:45:22 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 0/2] UefiCpuPkg/Library: Fix bug in MpInitLib To: "Duran, Leo" , "Ni, Ray" , "devel@edk2.groups.io" , "Wu, Hao A" , "Fu, Siyuan" Cc: "Dong, Eric" References: <1582659566-9893-1-git-send-email-leo.duran@amd.com> <734D49CCEBEEF84792F5B80ED585239D5C4542DA@SHSMSX104.ccr.corp.intel.com> <444c59ea-70dc-0edd-d680-add054dad2c5@redhat.com> From: "Laszlo Ersek" Message-ID: Date: Wed, 26 Feb 2020 18:45:21 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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 = FALSE; >>> else { >>> PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID); >>> CpuMpData->CpuData[ProcessorNumber].PlatformId = >> (UINT8)PlatformIdMsr.Bits.PlatformId; >>> CpuMpData->CpuData[ProcessorNumber].IsValidPlatformId = 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 global 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. Thanks Laszlo