From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-il1-f181.google.com (mail-il1-f181.google.com [209.85.166.181]) by mx.groups.io with SMTP id smtpd.web12.27440.1629134157695837251 for ; Mon, 16 Aug 2021 10:15:57 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20161025 header.b=GM4tyssL; spf=pass (domain: gmail.com, ip: 209.85.166.181, mailfrom: benjamin.doron00@gmail.com) Received: by mail-il1-f181.google.com with SMTP id v2so13107091ilg.12 for ; Mon, 16 Aug 2021 10:15:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=C6cJU+G0m0Ht/xAF2JSXgNKS2zSb43CdaNqcghn+VQA=; b=GM4tyssLrHL8UeUUPhxhuMvb8nWmN/mfvkLK+/NkAEHuVS9fs3SRqcmaDzgevxnXfc AMdKocce5SgbFH/Ouy0oF3qSH+UA/RXA0boZ7xod4bwMDzRlvKF+It6QbRwWvn7ibfjN riusWU0wMz6zovS2d8IezHr++LjGBJodSs2MjM8g7SPS2hbOg63shFE7KUxEAEE7IZeJ fZapA3oygvi0GAjvF037cNruXTFl1HDoGgIoGmgjM/avwqebgrvQZlyYVRNfNqdV59Cy IUUzUfm56AdTITP4uWf49fFW0Tgqg8FdQEfW3PkMqiJy21OZVSMtY4UIs6YJ/0ElV4Gp qPxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=C6cJU+G0m0Ht/xAF2JSXgNKS2zSb43CdaNqcghn+VQA=; b=TrwG+TmlQxcbmjEUjfbojWRF18H+M7+4+IovfjA54g+X43l+MU+5uoxWWCN6d7B1aM SjicTyAvB8qhceXPw/gpO8DJppXgHBcINyXjjK/yJ9WNw2L3nrCVzR42tED7ZYubOR/w wriLMsqTuZwPd1fSwzlGpxZazN05Ic8AUwb3qjBCLsoh4YXpc26gcWv/Z87D5fQfE36q 6qQYhKHmDT12mA7mYJ6zZWq8+enRj6btVeI9UfFKJqn/FbGktUoL3245uBa5DRp837d7 /c5l6n7p4trB9LOUO5c6rMbwr31pDdflcFVAsm+dg56um9fCfJiZiDFZPxKew5KFBNRa rNTA== X-Gm-Message-State: AOAM530WDQxKGNj9f9BB/MLCrGSd7fnmeZYvNNIuaokOncQhxVVFcVpT 2EWZLiVf1+SnaKNNc2LlQfzNMO7f6jH8X/4ZdgY= X-Google-Smtp-Source: ABdhPJxXowZjfigc8MV4y/P9f8YnCz0eayflE5a0m4cZK+uAzu5dQBaY15535DBU8WYL7Q2qVnL5l5J878+PTgcDxMM= X-Received: by 2002:a05:6e02:d03:: with SMTP id g3mr12533025ilj.127.1629134157127; Mon, 16 Aug 2021 10:15:57 -0700 (PDT) MIME-Version: 1.0 References: <20210816040238.29564-1-nathaniel.l.desimone@intel.com> In-Reply-To: From: "Benjamin Doron" Date: Mon, 16 Aug 2021 13:15:59 -0400 Message-ID: Subject: Re: [edk2-devel] [edk2-platforms] [PATCH V1] KabylakeSiliconPkg: Default for PeciC10Reset should be 1 To: Michael Kubacki Cc: devel@edk2.groups.io, "Desimone, Nathaniel L" , Chasel Chiu , Sai Chaganty , Michael Kubacki Content-Type: multipart/alternative; boundary="000000000000ba22bc05c9b05a1d" --000000000000ba22bc05c9b05a1d Content-Type: text/plain; charset="UTF-8" The MCH BAR field is the 38:15 bit range. Are the higher bits guaranteed to be clear, so that a 32 bit read is sufficient? Best regards, Benjamin On Mon, Aug 16, 2021 at 11:53 AM Michael Kubacki < mikuback@linux.microsoft.com> wrote: > Reviewed-by: Michael Kubacki > > On 8/16/2021 12:02 AM, Nate DeSimone wrote: > > The default value for CpuConfigLibPreMemConfig->PeciC10Reset > > should be 1 so that Peci Reset on C10 exit is disabled. > > > > Other bug fixes in > > KabylakeSiliconPkg\Cpu\Library\PeiCpuPolicyLibPreMem\PeiCpuPolicyLib.c > > > > 1. PCI configuration space can only be read 32-bits at a time. > > Converted MmioRead64 to MmioRead32. > > 2. Added a RShiftU64() call to prevent compiler instrinsics from > > being inserted. Since this is a 64-bit integer shift done in > > IA-32 mode it is possible for intrinsic calls to be added. > > > > Cc: Chasel Chiu > > Cc: Sai Chaganty > > Cc: Benjamin Doron > > Cc: Michael Kubacki > > Signed-off-by: Nate DeSimone > > --- > > .../Cpu/Library/PeiCpuPolicyLibPreMem/PeiCpuPolicyLib.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git > a/Silicon/Intel/KabylakeSiliconPkg/Cpu/Library/PeiCpuPolicyLibPreMem/PeiCpuPolicyLib.c > b/Silicon/Intel/KabylakeSiliconPkg/Cpu/Library/PeiCpuPolicyLibPreMem/PeiCpuPolicyLib.c > > index 35041322a7..85baa46208 100644 > > --- > a/Silicon/Intel/KabylakeSiliconPkg/Cpu/Library/PeiCpuPolicyLibPreMem/PeiCpuPolicyLib.c > > +++ > b/Silicon/Intel/KabylakeSiliconPkg/Cpu/Library/PeiCpuPolicyLibPreMem/PeiCpuPolicyLib.c > > @@ -1,7 +1,7 @@ > > /** @file > > This file is PeiCpuPolicy library. > > > > -Copyright (c) 2017, Intel Corporation. All rights reserved.
> > +Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.
> > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > **/ > > @@ -45,13 +45,14 @@ LoadCpuConfigLibPreMemConfigDefault ( > > CpuConfigLibPreMemConfig->BootFrequency = 1; // Maximum > non-turbo Performance > > CpuConfigLibPreMemConfig->ActiveCoreCount = 0; // All > cores active > > CpuConfigLibPreMemConfig->VmxEnable = > CPU_FEATURE_ENABLE; > > - CpuConfigLibPreMemConfig->CpuRatio = ((AsmReadMsr64 > (MSR_PLATFORM_INFO) >> N_PLATFORM_INFO_MAX_RATIO) & > B_PLATFORM_INFO_RATIO_MASK); > > + CpuConfigLibPreMemConfig->CpuRatio = RShiftU64 (AsmReadMsr64 > (MSR_PLATFORM_INFO), N_PLATFORM_INFO_MAX_RATIO) & > B_PLATFORM_INFO_RATIO_MASK; > > + > > /// > > /// FCLK Frequency > > /// > > CpuFamily = GetCpuFamily(); > > CpuSku = GetCpuSku(); > > - MchBar = MmioRead64 (MmPciBase (SA_MC_BUS, SA_MC_DEV, SA_MC_FUN) + > R_SA_MCHBAR) &~BIT0; > > + MchBar = MmioRead32 (MmPciBase (SA_MC_BUS, SA_MC_DEV, SA_MC_FUN) + > R_SA_MCHBAR) &~BIT0; > > if (IsPchLinkDmi (CpuFamily) && (MmioRead16 (MmPciBase > (SA_PEG_BUS_NUM, SA_PEG_DEV_NUM, SA_PEG10_FUN_NUM) + PCI_VENDOR_ID_OFFSET) > != 0xFFFF)) { > > PegDisabled = MmioRead32 ((UINTN) MchBar + > R_SA_MCHBAR_BIOS_RESET_CPL_OFFSET) & BIT3; > > } else { > > @@ -67,6 +68,8 @@ LoadCpuConfigLibPreMemConfigDefault ( > > } else { > > CpuConfigLibPreMemConfig->FClkFrequency = 0; // 800MHz > > } > > + > > + CpuConfigLibPreMemConfig->PeciC10Reset = 1; // Disables Peci Reset > on C10 exit > > } > > > > /** > > --000000000000ba22bc05c9b05a1d Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
The MCH BAR field is the 38:15 bit range. Are the higher b= its guaranteed to be clear, so that a 32 bit read is sufficient?

Best regards,
Benjamin


On Mon, Aug 16, 2021 at 1= 1:53 AM Michael Kubacki <mikuback@linux.microsoft.com> wrote:
Reviewed-by: Michael Kubacki <michael.kubacki@micro= soft.com>

On 8/16/2021 12:02 AM, Nate DeSimone wrote:
> The default value for CpuConfigLibPreMemConfig->PeciC10Reset
> should be 1 so that Peci Reset on C10 exit is disabled.
>
> Other bug fixes in
> KabylakeSiliconPkg\Cpu\Library\PeiCpuPolicyLibPreMem\PeiCpuPolicyLib.c=
>
>=C2=A0 =C2=A01. PCI configuration space can only be read 32-bits at a t= ime.
>=C2=A0 =C2=A0 =C2=A0 Converted MmioRead64 to MmioRead32.
>=C2=A0 =C2=A02. Added a RShiftU64() call to prevent compiler instrinsic= s from
>=C2=A0 =C2=A0 =C2=A0 being inserted. Since this is a 64-bit integer shi= ft done in
>=C2=A0 =C2=A0 =C2=A0 IA-32 mode it is possible for intrinsic calls to b= e added.
>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Benjamin Doron <benjamin.doron00@gmail.com>
> Cc: Michael Kubacki <michael.kubacki@microsoft.com>
> Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> ---
>=C2=A0 =C2=A0.../Cpu/Library/PeiCpuPolicyLibPreMem/PeiCpuPolicyLib.c=C2= =A0 | 9 ++++++---
>=C2=A0 =C2=A01 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/Silicon/Intel/KabylakeSiliconPkg/Cpu/Library/PeiCpuPolicy= LibPreMem/PeiCpuPolicyLib.c b/Silicon/Intel/KabylakeSiliconPkg/Cpu/Library/= PeiCpuPolicyLibPreMem/PeiCpuPolicyLib.c
> index 35041322a7..85baa46208 100644
> --- a/Silicon/Intel/KabylakeSiliconPkg/Cpu/Library/PeiCpuPolicyLibPreM= em/PeiCpuPolicyLib.c
> +++ b/Silicon/Intel/KabylakeSiliconPkg/Cpu/Library/PeiCpuPolicyLibPreM= em/PeiCpuPolicyLib.c
> @@ -1,7 +1,7 @@
>=C2=A0 =C2=A0/** @file
>=C2=A0 =C2=A0 =C2=A0This file is PeiCpuPolicy library.
>=C2=A0 =C2=A0
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>=
> +Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<= ;BR>
>=C2=A0 =C2=A0SPDX-License-Identifier: BSD-2-Clause-Patent
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0**/
> @@ -45,13 +45,14 @@ LoadCpuConfigLibPreMemConfigDefault (
>=C2=A0 =C2=A0 =C2=A0CpuConfigLibPreMemConfig->BootFrequency=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D 1;=C2=A0 =C2=A0 // Maximum non-turbo = Performance
>=C2=A0 =C2=A0 =C2=A0CpuConfigLibPreMemConfig->ActiveCoreCount=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D 0;=C2=A0 =C2=A0 // All cores active
>=C2=A0 =C2=A0 =C2=A0CpuConfigLibPreMemConfig->VmxEnable=C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=3D CPU_FEATURE_ENABLE;
> -=C2=A0 CpuConfigLibPreMemConfig->CpuRatio =3D ((AsmReadMsr64 (MSR_= PLATFORM_INFO) >> N_PLATFORM_INFO_MAX_RATIO) & B_PLATFORM_INFO_RA= TIO_MASK);
> +=C2=A0 CpuConfigLibPreMemConfig->CpuRatio =3D RShiftU64 (AsmReadMs= r64 (MSR_PLATFORM_INFO), N_PLATFORM_INFO_MAX_RATIO) & B_PLATFORM_INFO_R= ATIO_MASK;
> +
>=C2=A0 =C2=A0 =C2=A0///
>=C2=A0 =C2=A0 =C2=A0/// FCLK Frequency
>=C2=A0 =C2=A0 =C2=A0///
>=C2=A0 =C2=A0 =C2=A0CpuFamily=C2=A0 =3D GetCpuFamily();
>=C2=A0 =C2=A0 =C2=A0CpuSku=C2=A0 =C2=A0 =C2=A0=3D GetCpuSku();
> -=C2=A0 MchBar =3D MmioRead64 (MmPciBase (SA_MC_BUS, SA_MC_DEV, SA_MC_= FUN) + R_SA_MCHBAR) &~BIT0;
> +=C2=A0 MchBar =3D MmioRead32 (MmPciBase (SA_MC_BUS, SA_MC_DEV, SA_MC_= FUN) + R_SA_MCHBAR) &~BIT0;
>=C2=A0 =C2=A0 =C2=A0if (IsPchLinkDmi (CpuFamily) && (MmioRead16= (MmPciBase (SA_PEG_BUS_NUM, SA_PEG_DEV_NUM, SA_PEG10_FUN_NUM) + PCI_VENDOR= _ID_OFFSET) !=3D 0xFFFF)) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0PegDisabled =3D MmioRead32 ((UINTN) MchBar += R_SA_MCHBAR_BIOS_RESET_CPL_OFFSET) & BIT3;
>=C2=A0 =C2=A0 =C2=A0} else {
> @@ -67,6 +68,8 @@ LoadCpuConfigLibPreMemConfigDefault (
>=C2=A0 =C2=A0 =C2=A0} else {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0CpuConfigLibPreMemConfig->FClkFrequency = =3D 0;=C2=A0 // 800MHz
>=C2=A0 =C2=A0 =C2=A0}
> +
> +=C2=A0 CpuConfigLibPreMemConfig->PeciC10Reset =3D 1;=C2=A0 // Disa= bles Peci Reset on C10 exit
>=C2=A0 =C2=A0}
>=C2=A0 =C2=A0
>=C2=A0 =C2=A0/**

--000000000000ba22bc05c9b05a1d--