Re: [PATCH] OvmfPkg/SmmControl2Dxe: correct PCI_CONFIG_READ_WRITE in S3 boot script

[Laszlo Ersek <lersek@redhat.com>]

On 01/05/17 14:09, Yao, Jiewen wrote:
> Yes, I also agree to add a new MACRO.
> 
>  
> 
> My only concern is the name - POWER_MGMT_REGISTER_Q35_EFI is too weird.
> 
> I have no idea on the meaning to use _EFI as suffix.
> 
>  
> 
> Since we already defined below in PciRootBridgeIo.h,
> 
> #defineEFI_PCI_ADDRESS(bus, dev, func, reg) \
> 
>   (UINT64) ( \
> 
>   (((UINTN) bus) << 24) | \
> 
>   (((UINTN) dev) << 16) | \
> 
>   (((UINTN) func) << 8) | \
> 
>   (((UINTN) (reg)) < 256 ? ((UINTN) (reg)) : (UINT64) (LShiftU64
> ((UINT64) (reg), 32))))
> 
>  
> 
>  
> 
> How about we use POWER_MGMT_REGISTER_Q35_EFI_PCI_ADDRESS, or
> EFI_PCI_ADDRESS_POWER_MGMT_REGISTER_Q35 ?

POWER_MGMT_REGISTER_Q35_EFI_PCI_ADDRESS() looks fine to me, if Jordan
agrees.

Thanks!
Laszlo

> 
>  
> 
> Thank you
> 
> Yao Jiewen
> 
>  
> 
>  
> 
>  
> 
> *From:*Laszlo Ersek [mailto:lersek@redhat.com]
> *Sent:* Thursday, January 5, 2017 7:48 PM
> *To:* Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; edk2-devel-01 <edk2-devel@ml01.01.org>
> *Subject:* Re: [edk2] [PATCH] OvmfPkg/SmmControl2Dxe: correct
> PCI_CONFIG_READ_WRITE in S3 boot script
> 
>  
> 
> On 01/05/17 02:45, Yao, Jiewen wrote:
>> Sorry, fix typo: I agree we do **not** use EFI_ prefix here.
>> 
>> But using _EFI as suffix is also odd. :)
>> 
>> Do we have better name?
>> 
>> Such as POWER_MGMT_REGISTER_Q35_ADDRESS ?
> 
> POWER_MGMT_REGISTER_Q35() already returns a flat address, just in PciLib
> encoding. So an _ADDRESS suffix for the variant with the UEFI spec
> encoding is not particularly telling.
> 
> The new macro name should reflect the PciLib encoding <-> UEFI spec
> encoding difference.
> 
> Thanks!
> Laszlo
> 
> 
>> 
>>  
>> 
>> Thank you
>> 
>> Yao Jiewen
>> 
>>  
>> 
>> *From:*edk2-devel [mailto:edk2-devel-bounces@lists.01.org] *On Behalf Of
>> *Yao, Jiewen
>> *Sent:* Thursday, January 5, 2017 9:03 AM
>> *To:* Justen, Jordan L <jordan.l.justen@intel.com <mailto:jordan.l.justen@intel.com>>; Laszlo Ersek
>> <lersek@redhat.com
> <mailto:lersek@redhat.com>>; edk2-devel-01 <edk2-devel@lists.01.org
> <mailto:edk2-devel@lists.01.org>>
>> *Subject:* Re: [edk2] [PATCH] OvmfPkg/SmmControl2Dxe: correct
>> PCI_CONFIG_READ_WRITE in S3 boot script
>> 
>>  
>> 
>> Hi
>> I agree we do use EFI_ prefix here.
>> 
>> But using _EFI as suffix is also odd. :)
>> 
>> Do we have better name?
>> 
>> Thank you
>> Yao Jiewen
>> 
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jordan Justen
>> Sent: Thursday, January 5, 2017 6:02 AM
>> To: Laszlo Ersek <lersek@redhat.com
> <mailto:lersek@redhat.com%0b>> <mailto:lersek@redhat.com>>; edk2-devel-01 <edk2-devel@lists.01.org
> <mailto:edk2-devel@lists.01.org%0b>> <mailto:edk2-devel@lists.01.org>>
>> Subject: Re: [edk2] [PATCH] OvmfPkg/SmmControl2Dxe: correct PCI_CONFIG_READ_WRITE in S3 boot script
>> 
>> On 2017-01-04 03:19:20, Laszlo Ersek wrote:
>>> On 01/04/17 02:30, Jordan Justen wrote:
>>> > On 2016-12-02 02:48:44, Laszlo Ersek wrote:
>>> >> @@ -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?
>> 
>> Good point. Yeah, that name seems fine to me.
>> 
>> -Jordan
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org
> <mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>
>> <mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>
>> https://lists.01.org/mailman/listinfo/edk2-devel
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
> <mailto:edk2-devel@lists.01.org> <mailto:edk2-devel@lists.01.org>
>> https://lists.01.org/mailman/listinfo/edk2-devel
>> 
>