Re: [Qemu-devel] [PATCH v3] OvmfPkg/PlatformPei: Initialise RCBA (B0:D31:F0 0xf0) register

[Laszlo Ersek <lersek@redhat.com>]

On 06/09/15 00:07, Paulo Alcantara wrote:
> This patch initialises root complex register block BAR in order to
> support TCO watchdog emulation features (e.g. reboot upon NO_REBOOT bit
> not set) on QEMU.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> ---
>  OvmfPkg/Include/IndustryStandard/Q35MchIch9.h |  5 +++++
>  OvmfPkg/PlatformPei/Platform.c                | 14 +++++++++++++-
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
> index 4f59a7c..18b34a3 100644
> --- a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
> +++ b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
> @@ -77,6 +77,9 @@
>  #define ICH9_GEN_PMCON_1          0xA0
>  #define ICH9_GEN_PMCON_1_SMI_LOCK   BIT4
>  
> +#define ICH9_RCBA                 0xF0
> +#define ICH9_RCBA_EN                BIT0
> +
>  //
>  // IO ports
>  //
> @@ -90,4 +93,6 @@
>  #define ICH9_SMI_EN_APMC_EN      BIT5
>  #define ICH9_SMI_EN_GBL_SMI_EN   BIT0
>  
> +#define ICH9_ROOT_COMPLEX_BASE 0xFED1C000
> +
>  #endif
> diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
> index 1126c65..3811162 100644
> --- a/OvmfPkg/PlatformPei/Platform.c
> +++ b/OvmfPkg/PlatformPei/Platform.c
> @@ -212,13 +212,16 @@ MemMapInitialization (
>      // 0xFEC00000    IO-APIC                        4 KB
>      // 0xFEC01000    gap                         1020 KB
>      // 0xFED00000    HPET                           1 KB
> -    // 0xFED00400    gap                         1023 KB
> +    // 0xFED00400    gap                          111 KB
> +    // 0xFED1C000    RCRB                          16 KB
> +    // 0xFED20000    gap                          896 KB
>      // 0xFEE00000    LAPIC                          1 MB
>      //
>      AddIoMemoryRangeHob (TopOfLowRam < BASE_2GB ?
>                           BASE_2GB : TopOfLowRam, 0xFC000000);
>      AddIoMemoryBaseSizeHob (0xFEC00000, SIZE_4KB);
>      AddIoMemoryBaseSizeHob (0xFED00000, SIZE_1KB);
> +    AddIoMemoryBaseSizeHob (ICH9_ROOT_COMPLEX_BASE, SIZE_16KB);
>      AddIoMemoryBaseSizeHob (PcdGet32(PcdCpuLocalApicBaseAddress), SIZE_1MB);
>    }
>  }
> @@ -292,6 +295,15 @@ MiscInitialization (
>      //
>      PciOr8 (AcpiCtlReg, AcpiEnBit);
>    }
> +
> +  if (HostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) {
> +    //
> +    // Set Root Complex Register Block BAR
> +    //
> +    PciWrite32 (POWER_MGMT_REGISTER_Q35 (ICH9_RCBA),
> +                ICH9_ROOT_COMPLEX_BASE | ICH9_RCBA_EN
> +               );
> +  }
>  }

The right formatting for the last function call would be

PciWrite32 (
  POWER_MGMT_REGISTER_Q35 (ICH9_RCBA),
  ICH9_ROOT_COMPLEX_BASE | ICH9_RCBA_EN
  );

or

PciWrite32 (POWER_MGMT_REGISTER_Q35 (ICH9_RCBA),
  ICH9_ROOT_COMPLEX_BASE | ICH9_RCBA_EN);

but I won't obsess about it.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Let's see what Jordan says.

Thanks
Laszlo