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

[Jordan Justen <jordan.l.justen@intel.com>]

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 <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  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 <Library/PciLib.h>
>  #include <Library/QemuFwCfgLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
> +#include <Protocol/PciRootBridgeIo.h>
>  #include <Protocol/S3SaveState.h>
>  #include <Protocol/SmmControl2.h>
>  
> @@ -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.

-Jordan

> +                            ),
>                            &GenPmCon1OrMask,
>                            &GenPmCon1AndMask
>                            );
> -- 
> 2.9.2
>