From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 26BC381940 for ; Wed, 4 Jan 2017 03:19:23 -0800 (PST) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A01E713A9A; Wed, 4 Jan 2017 11:19:23 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-72.phx2.redhat.com [10.3.116.72]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v04BJMmu023590; Wed, 4 Jan 2017 06:19:22 -0500 To: Jordan Justen , edk2-devel-01 References: <20161202104844.6093-1-lersek@redhat.com> <148349340944.16413.4237312013765520247@jljusten-ivb> From: Laszlo Ersek Message-ID: <401681ee-93af-e490-88aa-28291bf41c1a@redhat.com> Date: Wed, 4 Jan 2017 12:19:20 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <148349340944.16413.4237312013765520247@jljusten-ivb> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Wed, 04 Jan 2017 11:19:23 +0000 (UTC) Subject: Re: [PATCH] OvmfPkg/SmmControl2Dxe: correct PCI_CONFIG_READ_WRITE in S3 boot script X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Jan 2017 11:19:23 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 01/04/17 02:30, Jordan Justen wrote: > On 2016-12-02 02:48:44, Laszlo Ersek wrote: >> EFI_BOOT_SCRIPT_PCI_CONFIG_READ_WRITE_OPCODE expects the PCI address to >> access in UEFI encoding, not in edk2/PciLib encoding. Convert the >> ICH9_GEN_PMCON_1 register's address to UEFI representation before storing >> it in the boot script. >> >> Cc: Jordan Justen >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek >> --- >> OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c | 32 +++++++++++++++++++++++++++++++- >> 1 file changed, 31 insertions(+), 1 deletion(-) >> >> diff --git a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c >> index c5e5ed02f5ad..3694282c82ad 100644 >> --- a/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c >> +++ b/OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c >> @@ -33,6 +33,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> >> @@ -307,6 +308,33 @@ FatalError: >> } >> >> /** >> + Convert a PCI address originally composed with PCI_LIB_ADDRESS() to >> + EFI_PCI_ADDRESS() representation (see Table 111. "PCI Configuration Address" >> + in UEFI-2.6). >> + >> + @param[in] PciLibAddress A PCI address originally composed with >> + PCI_LIB_ADDRESS(). >> + >> + @return The converted address suitable for consumers that expect >> + EFI_PCI_ADDRESS() representation. >> +**/ >> +STATIC >> +UINT64 >> +ConvertPciLibToEfiPciAddress ( >> + IN UINT32 PciLibAddress >> + ) >> +{ >> + UINT32 Bus, Device, Function, Register; >> + >> + Register = BitFieldRead32 (PciLibAddress, 0, 11); >> + Function = BitFieldRead32 (PciLibAddress, 12, 14); >> + Device = BitFieldRead32 (PciLibAddress, 15, 19); >> + Bus = BitFieldRead32 (PciLibAddress, 20, 27); >> + >> + return EFI_PCI_ADDRESS (Bus, Device, Function, Register); >> +} >> + >> +/** >> Notification callback for S3SaveState installation. >> >> @param[in] Event Event whose notification function is being invoked. >> @@ -362,7 +390,9 @@ OnS3SaveStateInstalled ( >> S3SaveState, >> EFI_BOOT_SCRIPT_PCI_CONFIG_READ_WRITE_OPCODE, >> EfiBootScriptWidthUint16, >> - (UINT64)POWER_MGMT_REGISTER_Q35 (ICH9_GEN_PMCON_1), >> + ConvertPciLibToEfiPciAddress ( >> + POWER_MGMT_REGISTER_Q35 (ICH9_GEN_PMCON_1) > > I think we should just add a EFI_POWER_MGMT_REGISTER_Q35 macro. I thought of that, but I didn't want to use the EFI_ prefix for a macro that has nothing to do with the UEFI / PI specs. Can you suggest an alternative name? Perhaps POWER_MGMT_REGISTER_Q35_EFI? Thanks! Laszlo > > -Jordan > >> + ), >> &GenPmCon1OrMask, >> &GenPmCon1AndMask >> ); >> -- >> 2.9.2 >>