On Mon, Mar 20, 2017 at 09:11:05PM +0800, Chenhui Sun wrote:

The size of the updated DTB file may be increased, so we need to allocate more memory than the original DTB size,or memory overflow may happen.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: shaochangliang shaochangliang@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Signed-off-by: Yi Li phoenix.liyi@huawei.com

---

Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c b/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c index 8586e33..699a820 100644 --- a/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c +++ b/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c @@ -112,7 +112,7 @@ EFIAPI UpdateFdt ( Size = (UINTN)fdt_totalsize ((VOID*)(PcdGet64(FdtFileAddress))); NewFdtBlobSize = Size + ADD_FILE_LENGTH;

• Status = gBS->AllocatePages (AllocateAnyPages, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES(Size), &NewFdtBlobBase);

• Status = gBS->AllocatePages (AllocateAnyPages, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES(NewFdtBlobSize), &NewFdtBlobBase);

This is clearly a fix. However, can you clarify whether (VOID) CopyMem((VOID*)NewFdtBlobBase, Fdt, Size); also need to be updated?

 if (EFI_ERROR (Status))
 {
     return EFI_OUT_OF_RESOURCES;

-- 1.9.1

On Tue, Mar 21, 2017 at 04:05:35PM +0800, Chenhui Sun wrote:

Hi Leif,

在 2017/3/20 23:49, Leif Lindholm 写道:

...

On Mon, Mar 20, 2017 at 09:11:05PM +0800, Chenhui Sun wrote:

...

The size of the updated DTB file may be increased, so we need to allocate more memory than the original DTB size,or memory overflow may happen.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: shaochangliang shaochangliang@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Signed-off-by: Yi Li phoenix.liyi@huawei.com

---

Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c b/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c index 8586e33..699a820 100644 --- a/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c +++ b/Chips/Hisilicon/Drivers/UpdateFdtDxe/UpdateFdtDxe.c @@ -112,7 +112,7 @@ EFIAPI UpdateFdt ( Size = (UINTN)fdt_totalsize ((VOID*)(PcdGet64(FdtFileAddress))); NewFdtBlobSize = Size + ADD_FILE_LENGTH;

• Status = gBS->AllocatePages (AllocateAnyPages, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES(Size), &NewFdtBlobBase);

• Status = gBS->AllocatePages (AllocateAnyPages, EfiRuntimeServicesData, EFI_SIZE_TO_PAGES(NewFdtBlobSize), &NewFdtBlobBase);

This is clearly a fix. However, can you clarify whether (VOID) CopyMem((VOID*)NewFdtBlobBase, Fdt, Size); also need to be updated?

Copy the source fdt file to NewFdtBlobBase, then update it, the size may increase after updating, so there just copy the origin data.

OK, in that case: Reviewed-by: Leif Lindholm leif.lindholm@linaro.org

thanks. Chenhui

...

...

 if (EFI_ERROR (Status))
 {
     return EFI_OUT_OF_RESOURCES;

-- 1.9.1

On Mon, Mar 20, 2017 at 09:11:06PM +0800, Chenhui Sun wrote:

The I350 Hilink state is not stable, so we need to modify the rx_tx_status_cfg to fix it, or the I350 enumeration fail may happen.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: shaochangliang shaochangliang@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Signed-off-by: Yi Li phoenix.liyi@huawei.com

---

Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c | 5 +++++ 1 file changed, 5 insertions(+)

diff --git a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c index 0b5a659..3bad240 100644 --- a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c +++ b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c @@ -23,6 +23,9 @@ #include <Library/TimerLib.h> #define PCIE_SYS_REG_OFFSET 0x1000 +#define MUX_LOS_ALOS_REG_OFFSET 0x508 +#define CH_RXTX_STATUS_CFG_EN BIT1 +#define CH_RXTX_STATUS_CFG BIT2 static PCIE_INIT_CFG mPcieIntCfg; UINT64 pcie_subctrl_base[2] = {0xb0000000, BASE_4TB + 0xb0000000}; @@ -470,6 +473,8 @@ VOID PciePcsInit(UINT32 soctype, UINT32 HostBridgeNum, UINT32 Port) RegRead(PCIE_PHY_BASE_1610[HostBridgeNum][Port] + PCS_SDS_CFG_REG + i * SDS_CFG_STRIDE, Value); Value |= (1 << 20); //bit 20: rxvalid enable RegWrite(PCIE_PHY_BASE_1610[HostBridgeNum][Port] + PCS_SDS_CFG_REG + i * SDS_CFG_STRIDE, Value);

•        RegWrite(PCIE_PHY_BASE_1610[HostBridgeNum][Port] + MUX_LOS_ALOS_REG_OFFSET + i*0x4, \
  

Why 0x4? Can we have a descriptive #define instead? Also, spaces around '*'.

•          CH_RXTX_STATUS_CFG_EN|CH_RXTX_STATUS_CFG);
  

Spaces around '|'.

     }
     PcieRxValidCtrl(soctype, HostBridgeNum, Port, 0);
     RegWrite(PCIE_PHY_BASE_1610[HostBridgeNum][Port] + 0x264, 0x3D090);

-- 1.9.1

On Mon, Mar 20, 2017 at 09:11:07PM +0800, Chenhui Sun wrote:

The M3(the corprocessor)PCIe driver will read Option Rom header

"corprocessor" -> "coprocessor".

durning enumeration, this opration will cause a completion error

"opration" -> "operation".

when there is no device inserted to the RC port, and the Option rom is uesless now. So we need to disable the RC Option Rom.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chenhui Sun sunchenhui@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Signed-off-by: Yi Li phoenix.liyi@huawei.com

---

.../Hi1610/Drivers/PcieInit1610/PcieInitLib.c | 40 ++++++++++++++++++++++ 1 file changed, 40 insertions(+)

diff --git a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c index 3bad240..57699e0 100644 --- a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c +++ b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c @@ -904,6 +904,44 @@ void PcieConfigContextHi1610(UINT32 soctype, UINT32 HostBridgeNum, UINT32 Port) return; } +UINT32 SysRegRead(UINT32 SocType, UINT32 HostBridgeNum, UINT32 Port, UINTN Reg)

Coding style says this should be

UINT32 SysRegRead ( UINT32 SocType, UINT32 HostBridgeNum, UINT32 Port, UINTN Reg )

Also, whether the arguments are IN, OUT or IN OUT. 5.7.1.11 of https://github.com/tianocore-docs/Docs/raw/master/Specifications/CCS_2_1_Dra...

+{

• UINT32 Value;
• if (SocType == 0x1610) {
• RegRead(PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + Reg, Value);

Space before "(".

• } else {
• //PCIE_APB_SLVAE_BASE is for 660,and each PCIe Ccontroller has the same APB_SLVAE_BASE
• //in the same hostbridge.
• RegRead(PCIE_APB_SLVAE_BASE[HostBridgeNum] + Reg, Value);

Space before "(".

• }
• return Value;

+}

+VOID +DisableRcOptionRom (

• UINT32 soctype,

Since this is a new function, could you rename this variable "SocType", like in the previous function? For coding style compliance.

• UINT32 HostBridgeNum,
• UINT32 Port,
• PCIE_PORT_TYPE PcieType

+)

Please add IN/OUT indicators.

+{

• UINT32 Value = 0;
• if (PcieType == PCIE_ROOT_COMPLEX) {
• Value = SysRegRead(soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG);

Space before "(".

• Value |= BIT2; //cs2 enable
• SysRegWrite(soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG, Value);

Space before "(".

• Value = SysRegRead(soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_EP_PCI_CFG_HDR12_REG);

Space before "(".

• Value &= ~BIT0; //disable option rom
• SysRegWrite(soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_EP_PCI_CFG_HDR12_REG, Value);

Space before "(".

• Value = SysRegRead(soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG);

Space before "(".

• Value &= ~BIT2; //cs2 disable
• SysRegWrite(soctype, HostBridgeNum, Port, PCIE_SYS_REG_OFFSET + PCIE_SYS_CTRL21_REG, Value);

Space before "(".

• }
• return;

+}

EFI_STATUS EFIAPI PciePortInit ( @@ -964,6 +1002,8 @@ PciePortInit ( /* Pcie Equalization*/ (VOID)PcieEqualization(soctype ,HostBridgeNum, PortIndex);

• /* Disable RC Option Rom */
  

• DisableRcOptionRom(soctype, HostBridgeNum, PortIndex, PcieCfg->PortInfo.PortType);
  

Space before "(".

  /* assert LTSSM enable */
  (VOID)PcieEnableItssm(soctype, HostBridgeNum, PortIndex);
  if (FeaturePcdGet(PcdIsPciPerfTuningEnable)) {

-- 1.9.1

Hi Leif,

在 2017/3/21 3:28, Leif Lindholm 写道:

On Mon, Mar 20, 2017 at 09:11:08PM +0800, Chenhui Sun wrote:

...

In some cases, the PCIe device may close part of lanes in config state of LTSSM, the hip06 RC should reconfig lane number and try to linkup again.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chenhui Sun sunchenhui@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Signed-off-by: Yi Li phoenix.liyi@huawei.com

---

.../Hi1610/Drivers/PcieInit1610/PcieInitLib.c | 121 ++++++++++++++++++++- Chips/Hisilicon/Include/Regs/HisiPcieV1RegOffset.h | 3 + 2 files changed, 122 insertions(+), 2 deletions(-)

diff --git a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c index 57699e0..b45d54f 100644 --- a/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c +++ b/Chips/Hisilicon/Hi1610/Drivers/PcieInit1610/PcieInitLib.c @@ -42,6 +42,7 @@ extern PCIE_DRIVER_CFG gastr_pcie_driver_cfg; extern PCIE_IATU gastr_pcie_iatu_cfg; extern PCIE_IATU_VA mPcieIatuTable; +EFI_STATUS PciePortInit (UINT32 soctype, UINT32 HostBridgeNum, PCIE_DRIVER_CFG *PcieCfg);

A blank line after this definition would be nice. Also, I think the EFI_API needs to be here as well.

EFI_STATUS EFIAPI PciePortInit ( IN UINT32 soctype, IN UINT32 HostBridgeNum, IN PCIE_DRIVER_CFG *PcieCfg );

Is this the correct style?

...

VOID PcieRegWrite(UINT32 Port, UINTN Offset, UINT32 Value) { RegWrite((UINT64)mPcieIntCfg.RegResource[Port] + Offset, Value);

Space between function name and "(". This applies globally in this patch (I won't repeat this comment).

(But no space between the (UINT64) and mPcieInitCfg, that bit is done the correct way.)

This code is not the new added, so I will refine the follow new added code according your comments. is that ok?

Thanks Chenhui

...

@@ -152,8 +153,123 @@ VOID PcieRxValidCtrl(UINT32 soctype, UINT32 HostBridgeNum, UINT32 Port, BOOLEAN } } } +/*

• • The ltssm register is assigned in an asynchronous way, the value
• • of register may not right in metastable state.
• • Read the register twice to get stable value.
• */

+VOID PcieGetLtssmValue (

• UINT32 HostBridgeNum,
• UINT32 Port,
• UINT32* Value

  UINT32 *Value

...

+) +{

• UINT32 ValueA;
• UINT32 ValueB = 0;
• UINT32 Count;
• RegRead(PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + PCIE_SYS_REG_OFFSET + PCIE_SYS_STATE4_REG, ValueA);
• ValueA = ValueA & PCIE_LTSSM_STATE_MASK;
• Count = 0;
• while (Count < 2) {
• RegRead(PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + PCIE_SYS_REG_OFFSET + PCIE_SYS_STATE4_REG, ValueB);
• ValueB = ValueB & PCIE_LTSSM_STATE_MASK;
• /* Get the same state in continuous two times*/
• if (ValueA == ValueB) {

•  break;
  

• }
• /* If the second value not equel to the first,

"equel" -> "equal".

...

• • we return the second one as the stable

Also, this fits on a single line, so please join them.

...

• */
• ValueA = ValueB;
• Count++;
• }
• *Value = ValueB;
• return;

+}

+/*

• • In some cases, the PCIe device may close part of lanes in
• • config state of LTSSM, the hip06 RC should reconfig lane num
• • and try to linkup again.
• */

+VOID PcieReconfigLaneNum (

• UINT32 soctype,
• UINT32 HostBridgeNum,
• UINT32 Port,
• PCIE_DRIVER_CFG *PcieCfg

+) +{

• EFI_STATUS Status;
• UINT32 LtssmStatus;
• UINT32 RegVal;
• UINT32 LoopCnt = 0;
• UINT32 LaneNumCnt = 0;
• PCIE_PORT_WIDTH PortWidth = PcieCfg->PortInfo.PortWidth;
• // 500 * 200us = 100ms, so it takes 100 ms must to reconfig lane numbers
• while (LoopCnt < 500) {
• /*
• • The minimum lanenum is 1, no need to try any more.
• */

Please align the * characters.

...

• if (PortWidth <= 1) {

•  DEBUG((DEBUG_ERROR, "PcieReconfigLanenum  PortWidth <= 1 !\n"));
  

•  return;
  

• }
• /*
• • Check the lane num config state is normal or not.
• */
• PcieGetLtssmValue(HostBridgeNum, Port, &LtssmStatus);
• if ((LtssmStatus == PCIE_LTSSM_CFG_LANENUM_ACPT) || (LtssmStatus == PCIE_LTSSM_CFG_COMPLETE)) {

•  LaneNumCnt++;
  

• } else if (LtssmStatus == PCIE_LTSSM_LINKUP_STATE) {

•  PcieGetLtssmValue(HostBridgeNum, Port, &LtssmStatus);
  

•  if (LtssmStatus == PCIE_LTSSM_LINKUP_STATE) {
  

•      break;
  

•  }
  

• } else {

•  LaneNumCnt = 0;
  

• }
• /*
• • The lane num config state is abnormal, need to reconfig
• • the lane num and try to establish link again.
• */
• if (LaneNumCnt > 5) {

Why 5? Could there be a #define with a descriptive name?

...

•  /* Disable LTSSM */
  

•  RegRead(PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + PCIE_CTRL_7_REG, RegVal);
  

•  RegVal &= ~(BIT11);
  

What is BIT11? Can there be a comment explaining

...

•  RegWrite(PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + PCIE_CTRL_7_REG, RegVal);
  

•  PcieCfg->PortInfo.PortWidth = (PCIE_PORT_WIDTH)((UINT8)PcieCfg->PortInfo.PortWidth >> 1);
  

This shift deserves a comment. Also, consider if PcieCfg->PortInfo.PortWidth >>= 1; would not be more clear.

...

•  Status = PciePortInit(soctype, HostBridgeNum, PcieCfg);
  

•  if(EFI_ERROR(Status)) {
  

•      DEBUG((DEBUG_ERROR, "PcieReconfigLanenum HostBridge %d, Pcie Port %d Init Failed! \n", HostBridgeNum, Port));
  

•  }
  

•  return;
  

• }
• LoopCnt++;
• /* Pcie 3.0 Spec,part 4.2.6.3.4.1: the Upstream Lanes are permitted

• * delay up to 1 ms before transitioning to Configuration.Lanenum.Accept.
  

• * So the delay time 200 us * 5(LanNumCnt) = 1ms, not beyond the reasonable range.
  

• */
  

This comment is excellent, thank you.

...

• MicroSecondDelay(200);
• }
• return ;

+} -EFI_STATUS PcieEnableItssm(UINT32 soctype, UINT32 HostBridgeNum, UINT32 Port) +EFI_STATUS PcieEnableItssm(UINT32 soctype, UINT32 HostBridgeNum, UINT32 Port, PCIE_DRIVER_CFG *PcieCfg) { PCIE_CTRL_7_U pcie_ctrl7; UINT32 Value = 0; @@ -168,6 +284,7 @@ EFI_STATUS PcieEnableItssm(UINT32 soctype, UINT32 HostBridgeNum, UINT32 Port) Value |= BIT11|BIT30|BIT31; RegWrite(PCIE_APB_SLAVE_BASE_1610[HostBridgeNum][Port] + 0x1114, Value); (VOID)PcieRxValidCtrl(soctype, HostBridgeNum, Port, 1);

•    PcieReconfigLaneNum(soctype, HostBridgeNum, Port, PcieCfg);
      return EFI_SUCCESS;
  }
  else
  

@@ -1005,7 +1122,7 @@ PciePortInit ( /* Disable RC Option Rom */ DisableRcOptionRom(soctype, HostBridgeNum, PortIndex, PcieCfg->PortInfo.PortType); /* assert LTSSM enable */

• (VOID)PcieEnableItssm(soctype, HostBridgeNum, PortIndex);
  

• (VOID)PcieEnableItssm(soctype, HostBridgeNum, PortIndex, PcieCfg);
   if (FeaturePcdGet(PcdIsPciPerfTuningEnable)) {
     //PCIe will still work even if performance tuning fails,
     //and there is warning message inside the function to print
  

diff --git a/Chips/Hisilicon/Include/Regs/HisiPcieV1RegOffset.h b/Chips/Hisilicon/Include/Regs/HisiPcieV1RegOffset.h index 539d567..b750910 100644 --- a/Chips/Hisilicon/Include/Regs/HisiPcieV1RegOffset.h +++ b/Chips/Hisilicon/Include/Regs/HisiPcieV1RegOffset.h @@ -8982,6 +8982,7 @@ typedef union tagIepMsiCtrlIntStatus #define PCIE_SYS_CTRL28_REG (PCI_SYS_BASE + 0x1c4) #define PCIE_SYS_CTRL29_REG (PCI_SYS_BASE + 0x1c8) #define PCIE_SYS_CTRL54_REG (PCI_SYS_BASE + 0x274) +#define PCIE_SYS_STATE4_REG (PCI_SYS_BASE + 0x31C) #define PCIE_SYS_STATE5_REG (PCI_SYS_BASE + 0x30) #define PCIE_SYS_STATE6_REG (PCI_SYS_BASE + 0x34) #define PCIE_SYS_STATE7_REG (PCI_SYS_BASE + 0x38) @@ -12694,6 +12695,8 @@ typedef union tagPortlogic93 #define PCIE_SUBCTRL_SC_PCIE0_SYS_STATE3_REG (PCIE_SUBCTRL_BASE + 0x6814) #define PCIE_SUBCTRL_SC_PCIE0_SYS_STATE4_REG (PCIE_SUBCTRL_BASE + 0x6818) #define PCIE_LTSSM_STATE_MASK (0x3f) +#define PCIE_LTSSM_CFG_LANENUM_ACPT 0x0a +#define PCIE_LTSSM_CFG_COMPLETE 0x0b #define PCIE_LTSSM_LINKUP_STATE (0x11) #define PCIE_SUBCTRL_SC_PCIE0_AXI_MSTR_OOO_WR_STS0_REG (PCIE_SUBCTRL_BASE + 0x6880)

#define PCIE_SUBCTRL_SC_PCIE0_AXI_MSTR_OOO_WR_STS1_REG (PCIE_SUBCTRL_BASE + 0x6884)

1.9.1

Hi Leif,

I'm sorry I forgot to look into this...

Regards,

Heyi

在 3/21/2017 10:34 PM, Leif Lindholm 写道:

We discussed this back in December, for the previous iteration: https://lists.linaro.org/pipermail/linaro-uefi/2016-December/003730.html

The suggestion then was to move over to using the FIRMWARE_VER mechanism, and Heyi said he was going to look into this after the release. This does not appear to have happened.

Will you have time to revisit this less confusing option while I am away on holiday?

Regards,

Leif

On Mon, Mar 20, 2017 at 09:11:09PM +0800, Chenhui Sun wrote:

...

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chenhui Sun sunchenhui@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Signed-off-by: Yi Li phoenix.liyi@huawei.com

---

Platforms/Hisilicon/D02/Pv660D02.dsc | 2 +- Platforms/Hisilicon/D03/D03.dsc | 2 +- Platforms/Hisilicon/D05/D05.dsc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Platforms/Hisilicon/D02/Pv660D02.dsc b/Platforms/Hisilicon/D02/Pv660D02.dsc index b611579..3f2ea31 100644 --- a/Platforms/Hisilicon/D02/Pv660D02.dsc +++ b/Platforms/Hisilicon/D02/Pv660D02.dsc @@ -145,7 +145,7 @@ gHisiTokenSpaceGuid.PcdPcieSmmuBaseAddress|0xb0040000 gHisiTokenSpaceGuid.PcdDsaSmmuBaseAddress|0xc0040000 gHisiTokenSpaceGuid.PcdAlgSmmuBaseAddress|0xd0040000

• gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Hisilicon D02 UEFI 16.08 RC1"

• gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Hisilicon D02 UEFI ROCK1.6"

gHisiTokenSpaceGuid.PcdSystemProductName|L"D02" gHisiTokenSpaceGuid.PcdSystemVersion|L"Estuary" diff --git a/Platforms/Hisilicon/D03/D03.dsc b/Platforms/Hisilicon/D03/D03.dsc index 9c73494..b44a5a3 100644 --- a/Platforms/Hisilicon/D03/D03.dsc +++ b/Platforms/Hisilicon/D03/D03.dsc @@ -170,7 +170,7 @@ gHisiTokenSpaceGuid.PcdAlgSmmuBaseAddress|0xd0040000

• gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Hisilicon D03 UEFI 16.12 Release"

• gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Hisilicon D03 UEFI ROCK1.6"

gHisiTokenSpaceGuid.PcdBiosVersionString|L"10.01.01T18" diff --git a/Platforms/Hisilicon/D05/D05.dsc b/Platforms/Hisilicon/D05/D05.dsc index 2877da0..b17903c 100644 --- a/Platforms/Hisilicon/D05/D05.dsc +++ b/Platforms/Hisilicon/D05/D05.dsc @@ -188,7 +188,7 @@ gHisiTokenSpaceGuid.PcdIsMPBoot|1 gHisiTokenSpaceGuid.PcdSocketMask|0x3

• gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Hisilicon D05 UEFI 16.12 Release"

• gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"Hisilicon D05 UEFI ROCK1.6"

gHisiTokenSpaceGuid.PcdBiosVersionString|L"10.01.01T18" -- 1.9.1

On Mon, Mar 20, 2017 at 09:11:10PM +0800, Chenhui Sun wrote:

From: Chenhui Sun sunchenhui@huawei.com

Refresh checksum after changing DSDT table.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: wanglijun wanglijun@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Signed-off-by: Yi Li phoenix.liyi@huawei.com

---

Chips/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)

diff --git a/Chips/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c b/Chips/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c index 41f5692..98be4dc 100644 --- a/Chips/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c +++ b/Chips/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c @@ -442,6 +442,27 @@ static EFI_STATUS ProcessDSDT( return EFI_SUCCESS; }

STATIC

+VOID +AcpiPlatformChecksum (

EDK2 in general seems to camel-case also the final S in CheckSum, so for consistency, please to the same (and also in variables below).

I do not see anything "Platform" about this. Since it is only a local function, name is not very important, "AcpiCheckSum" would be sufficient.

• IN UINT8 *Buffer,

It would be nicer to accept an EFI_ACPI_SDT_HEADER *, so that the caller did not always need to cast when calling. And then add a local UINT8 * to operate on. We would also then not need to pass the Size separately.

Also, this is IN OUT. The buffer is being modified.

• IN UINTN Size
• )

+{

• UINTN ChecksumOffset;
• ChecksumOffset = OFFSET_OF (EFI_ACPI_DESCRIPTION_HEADER, Checksum);
• //
• // set checksum to 0 first
• //
• Buffer[ChecksumOffset] = 0;

Could this not be skipped if ...

• //
• // Update checksum value
• //
• Buffer[ChecksumOffset] = CalculateCheckSum8(Buffer, Size);

... we passed in "Size - 1" here?

+}

EFI_STATUS EthMacInit(void) { EFI_STATUS Status; @@ -478,6 +499,7 @@ EFI_STATUS EthMacInit(void) ProcessDSDT(AcpiTableProtocol, TableHandle); AcpiTableProtocol->Close(TableHandle);

• AcpiPlatformChecksum ((UINT8*)Table, Table->Length); }

return EFI_SUCCESS; -- 1.9.1

On Fri, Mar 31, 2017 at 05:00:38PM +0800, Chenhui Sun wrote:

Hi Leif,

在 2017/3/21 22:48, Leif Lindholm 写道:

...

On Mon, Mar 20, 2017 at 09:11:10PM +0800, Chenhui Sun wrote:

...

From: Chenhui Sun sunchenhui@huawei.com

Refresh checksum after changing DSDT table.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: wanglijun wanglijun@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Signed-off-by: Yi Li phoenix.liyi@huawei.com

---

Chips/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)

diff --git a/Chips/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c b/Chips/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c index 41f5692..98be4dc 100644 --- a/Chips/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c +++ b/Chips/Hisilicon/Drivers/AcpiPlatformDxe/EthMac.c @@ -442,6 +442,27 @@ static EFI_STATUS ProcessDSDT( return EFI_SUCCESS; }

STATIC

...

+VOID +AcpiPlatformChecksum (

EDK2 in general seems to camel-case also the final S in CheckSum, so for consistency, please to the same (and also in variables below).

Checksum is a word I think, not "Check" + "Sum", it is spelled as Checksum in Edk2 base code. Could you check it again please? :)

So, I actually agree with your argument. Which was why I went to look. And MdePkg/Include/Library/BaseLib.h uses "CheckSum" consistently.

And it sticks out in this file because it calls those functions, with the other form.

...

I do not see anything "Platform" about this. Since it is only a local function, name is not very important, "AcpiCheckSum" would be sufficient.

Ok

...

...

• IN UINT8 *Buffer,

It would be nicer to accept an EFI_ACPI_SDT_HEADER *, so that the caller did not always need to cast when calling. And then add a local UINT8 * to operate on. We would also then not need to pass the Size separately.

It should calculate all the table data checksum, not only the header.

Yes. But the current code does AcpiPlatformChecksum ((UINT8*)Table, Table->Length);

Where it could do AcpiPlatformChecksum (Table);

and the function could do: UINT8 *Buffer; Buffer = (UINT8 *)Table; Buffer[ChecksumOffset] = CalculateCheckSum8 (Buffer, Table->Length);

Regards,

Leif

Chenhui, Thanks and Regards

...

Also, this is IN OUT. The buffer is being modified.

...

• IN UINTN Size
• )

+{

• UINTN ChecksumOffset;
• ChecksumOffset = OFFSET_OF (EFI_ACPI_DESCRIPTION_HEADER, Checksum);
• //
• // set checksum to 0 first
• //
• Buffer[ChecksumOffset] = 0;

Could this not be skipped if ...

...

• //
• // Update checksum value
• //
• Buffer[ChecksumOffset] = CalculateCheckSum8(Buffer, Size);

... we passed in "Size - 1" here?

...

+}

EFI_STATUS EthMacInit(void) { EFI_STATUS Status; @@ -478,6 +499,7 @@ EFI_STATUS EthMacInit(void) ProcessDSDT(AcpiTableProtocol, TableHandle); AcpiTableProtocol->Close(TableHandle);

• AcpiPlatformChecksum ((UINT8*)Table, Table->Length); } return EFI_SUCCESS;

-- 1.9.1

On Mon, Mar 20, 2017 at 09:11:11PM +0800, Chenhui Sun wrote:

CPM should also be 1 when CCA is 1.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: wanglijun wanglijun@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Signed-off-by: Yi Li phoenix.liyi@huawei.com

IMO these three paches changelog could be improved by showing the DSDT snippet that conflicts and the test output.

So that people coming later why have a similar issue find the solution.

Graeme

---

Chips/Hisilicon/Pv660/Pv660AcpiTables/Iort.asl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Chips/Hisilicon/Pv660/Pv660AcpiTables/Iort.asl b/Chips/Hisilicon/Pv660/Pv660AcpiTables/Iort.asl index bcd31d6..8f38359 100644 --- a/Chips/Hisilicon/Pv660/Pv660AcpiTables/Iort.asl +++ b/Chips/Hisilicon/Pv660/Pv660AcpiTables/Iort.asl @@ -205,8 +205,8 @@ Read Allocate : 0 Override : 0 [0002] Reserved : 0000 -[0001] Memory Flags (decoded below) : 00

•                            Coherency : 0
  

+[0001] Memory Flags (decoded below) : 01

•                            Coherency : 1
                      Device Attribute : 0
  

[0004] ATS Attribute : 00000000 [0004] PCI Segment Number : 00000001 @@ -234,8 +234,8 @@ Read Allocate : 0 Override : 0 [0002] Reserved : 0000 -[0001] Memory Flags (decoded below) : 00

•                            Coherency : 0
  

+[0001] Memory Flags (decoded below) : 01

•                            Coherency : 1
                      Device Attribute : 0
  

[0004] ATS Attribute : 00000000 [0004] PCI Segment Number : 00000002 -- 1.9.1

Hi Leif,

在 2017/3/21 23:28, Leif Lindholm 写道:

On Mon, Mar 20, 2017 at 09:11:15PM +0800, Chenhui Sun wrote:

...

EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL/EFI_PCI_IO_PROTOCOL must returns EFI_INVALID_PARAMETER, change return value.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: wanglijun wanglijun@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Signed-off-by: Yi Li phoenix.liyi@huawei.com

---

.../Drivers/PciHostBridgeDxe/PciRootBridgeIo.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c b/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c index 03edcf1..30619f5 100644 --- a/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c +++ b/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c @@ -824,8 +824,8 @@ RootBridgeConstructor ( PrivateData->Supports = EFI_PCI_ATTRIBUTE_IDE_PRIMARY_IO | EFI_PCI_ATTRIBUTE_IDE_SECONDARY_IO | \ EFI_PCI_ATTRIBUTE_ISA_IO_16 | EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO | \ EFI_PCI_ATTRIBUTE_VGA_MEMORY | \

•                         EFI_PCI_ATTRIBUTE_VGA_IO_16  | EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16 | \
  

•                         EFI_PCI_ATTRIBUTE_VALID_FOR_ALLOCATE_BUFFER;
  

•                         EFI_PCI_ATTRIBUTE_VGA_IO_16  | EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16;
  

This deletion of EFI_PCI_ATTRIBUTE_VALID_FOR_ALLOCATE_BUFFER does not obviously relate to what the commit message describes. Can you expand the explanation, please?

There seems to be something wrong here, we will check it first. thanks.

Regards, Chenhui

...

PrivateData->Attributes  = PrivateData->Supports;

Protocol->ParentHandle = HostBridgeHandle; @@ -1275,6 +1275,9 @@ RootBridgeIoPollMem ( return EFI_INVALID_PARAMETER; }

• if (Width > EfiPciWidthUint64) {
• return EFI_INVALID_PARAMETER;
• }

Add blank line after?

...

//
// No matter what, always do a single poll.
//

@@ -2010,7 +2013,10 @@ RootBridgeIoMap ( ) { DMA_MAP_OPERATION DmaOperation;

Why delete this blank line?

...

• if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress == NULL ||

•  Mapping == NULL) {
  

• return EFI_INVALID_PARAMETER;
• }

I would prefer a blank line here.

...

if (Operation == EfiPciOperationBusMasterRead) {
  DmaOperation = MapOperationBusMasterRead;
} else if (Operation == EfiPciOperationBusMasterWrite) {

@@ -2245,6 +2251,11 @@ RootBridgeIoSetAttributes ( PrivateData = DRIVER_INSTANCE_FROM_PCI_ROOT_BRIDGE_IO_THIS(This);

• if((Attributes & (EFI_PCI_ATTRIBUTE_MEMORY_WRITE_COMBINE | EFI_PCI_ATTRIBUTE_MEMORY_CACHED)) &&

• (ResourceBase == NULL || ResourceLength == NULL)) {
  

• return EFI_INVALID_PARAMETER;
• }
• if (Attributes != 0) { if ((Attributes & (~(PrivateData->Supports))) != 0) { return EFI_UNSUPPORTED;

-- 1.9.1

On Mon, Mar 20, 2017 at 09:11:16PM +0800, Chenhui Sun wrote:

As the PCIe driver is updated in kernel 4.10, and it will read the PCIe resource in the PCIe device, so the resource description should also be put into the Device(PCIx).

1. add _OSC support
2. put the pcie resource description into the Device(PCIx), in order to compatible with kernel 4.10 PCIe driver.

The actual changes in this file are hidden in the indentation changes. Are the indentation changes actually required?

The changes don't look wrong in themselves.

Graeme

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Heyi Guo heyi.guo@linaro.org Signed-off-by: Yi Li phoenix.liyi@huawei.com Signed-off-by: Chenhui Sun sunchenhui@huawei.com

---

.../Hisilicon/Hi1616/D05AcpiTables/Dsdt/D05Pci.asl | 641 ++++++++++++++++----- 1 file changed, 508 insertions(+), 133 deletions(-)

diff --git a/Chips/Hisilicon/Hi1616/D05AcpiTables/Dsdt/D05Pci.asl b/Chips/Hisilicon/Hi1616/D05AcpiTables/Dsdt/D05Pci.asl index f9b4722..deca155 100644 --- a/Chips/Hisilicon/Hi1616/D05AcpiTables/Dsdt/D05Pci.asl +++ b/Chips/Hisilicon/Hi1616/D05AcpiTables/Dsdt/D05Pci.asl @@ -40,6 +40,49 @@ Scope(_SB) } } +/*

• See ACPI 6.1 Spec, 6.2.11, PCI Firmware Spec 3.0, 4.5

+*/ +#define PCI_OSC_SUPPORT() \

• Name(SUPP, Zero) /* PCI _OSC Support Field value */ \
• Name(CTRL, Zero) /* PCI _OSC Control Field value */ \
• Method(_OSC,4) { \
• If(LEqual(Arg0,ToUUID("33DB4D5B-1FF7-401C-9657-7441C03DD766"))) { \

•  /* Create DWord-adressable fields from the Capabilities Buffer */ \
  

•  CreateDWordField(Arg3,0,CDW1) \
  

•  CreateDWordField(Arg3,4,CDW2) \
  

•  CreateDWordField(Arg3,8,CDW3) \
  

•  /* Save Capabilities DWord2 & 3 */ \
  

•  Store(CDW2,SUPP) \
  

•  Store(CDW3,CTRL) \
  

•  /* Only allow native hot plug control if OS supports: */ \
  

•  /* ASPM */ \
  

•  /* Clock PM */ \
  

•  /* MSI/MSI-X */ \
  

•  If(LNotEqual(And(SUPP, 0x16), 0x16)) { \
  

•    And(CTRL,0x1E,CTRL) \
  

•  }\
  

•  \
  

•  /* Do not allow native PME, AER */ \
  

•  /* Never allow SHPC (no SHPC controller in this system)*/ \
  

•  And(CTRL,0x10,CTRL) \
  

•  If(LNotEqual(Arg1,One)) { /* Unknown revision */ \
  

•    Or(CDW1,0x08,CDW1) \
  

•  } \
  

•  \
  

•  If(LNotEqual(CDW3,CTRL)) { /* Capabilities bits were masked */ \
  

•    Or(CDW1,0x10,CDW1) \
  

•  } \
  

•  \
  

•  /* Update DWORD3 in the buffer */ \
  

•  Store(CTRL,CDW3) \
  

•  Return(Arg3) \
  

• } Else { \

•  Or(CDW1,4,CDW1) /* Unrecognized UUID */ \
  

•  Return(Arg3) \
  

• } \
• } // End _OSC
• // 1P NA PCIe2 Device (PCI2) {

@@ -89,17 +132,43 @@ Scope(_SB) Device (RES2) { Name (_HID, "HISI0081") // HiSi PCIe RC config base address

•  Name (_CID, "PNP0C02")  // Motherboard reserved resource
   Name (_CRS, ResourceTemplate (){
  

•    Memory32Fixed (ReadWrite, 0xa00a0000 , 0x10000)
  

•    Memory32Fixed (ReadWrite, 0xa00a0000 , 0x10000) //host bridge register space
   })
  

  }
• PCI_OSC_SUPPORT() Method (_STA, 0x0, NotSerialized) { Return (0xf) }

• } // Device(PCI2)

• Device (RES2)
• {
• Name (_HID, "HISI0081") // HiSi PCIe RC config base address
• Name (_CID, "PNP0C02") // Motherboard reserved resource
• Name (_UID, 0x2) // Unique ID
• Name (_CRS, ResourceTemplate (){

•  Memory32Fixed (ReadWrite, 0xa00a0000 , 0x10000) //host bridge register space
  

• })
• Method (_STA, 0x0, NotSerialized)
• {

•  Return (0xf)
  

• }
• }
• Device (R1NA) // reserve 1p NA ECAM resource
• {
• Name (_HID, "PNP0C02") // Motherboard reserved resource
• Name (_CRS, ResourceTemplate (){

•  Memory32Fixed (ReadWrite, 0xa8000000 , 0x800000) //ECAM space for [bus 80-87]
  

• })
• Method (_STA, 0x0, NotSerialized)
• {

•  Return (0xf)
  

• }
• }
• // 1p NB PCIe0 Device (PCI4) {

@@ -149,30 +218,55 @@ Scope(_SB) Device (RES4) { Name (_HID, "HISI0081") // HiSi PCIe RC config base address

•  Name (_CID, "PNP0C02")  // Motherboard reserved resource
   Name (_CRS, ResourceTemplate (){
  

•    QwordMemory (
  

•      ResourceProducer,
  

•      PosDecode,
  

•      MinFixed,
  

•      MaxFixed,
  

•      NonCacheable,
  

•      ReadWrite,
  

•      0x0, // Granularity
  

•      0x8a0090000, // Min Base Address
  

•      0x8a009ffff, // Max Base Address
  

•      0x0, // Translate
  

•      0x10000 // Length
  

•   )
  

•    QwordMemory ( //host bridge register space
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x8a0090000, // Min Base Address
  

•    0x8a009ffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x10000 // Length
  

•    )
   })
  

  }
• PCI_OSC_SUPPORT() Method (_STA, 0x0, NotSerialized) { Return (RBYV()) }

• } // Device(PCI4)

• Device (RES4)
• {
• Name (_HID, "HISI0081") // HiSi PCIe RC config base address
• Name (_CID, "PNP0C02") // Motherboard reserved resource
• Name (_UID, 0x4) // Unique ID
• Name (_CRS, ResourceTemplate (){

•    QwordMemory ( //host bridge register space
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x8a0090000, // Min Base Address
  

•    0x8a009ffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x10000 // Length
  

•  )
  

• })
• Method (_STA, 0x0, NotSerialized)
• {

•  Return (RBYV())
  

• }
• }
• // 1P NB PCI1 Device (PCI5) {

@@ -219,31 +313,57 @@ Scope(_SB) }) // Name(RBUF) Return (RBUF) } // Method(_CRS)

• Device (RES5)

• Device (RES5)
• {
• Name (_HID, "HISI0081") // HiSi PCIe RC config base address
• Name (_CRS, ResourceTemplate (){

•  QwordMemory (  //host bridge register space
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x8a0200000, // Min Base Address
  

•    0x8a020ffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x10000 // Length
  

•  )
  

• })
• }
• PCI_OSC_SUPPORT()
• Method (_STA, 0x0, NotSerialized) {

•  Name (_HID, "HISI0081") // HiSi PCIe RC config base address
  

•  Name (_CID, "PNP0C02")  // Motherboard reserved resource
  

•  Name (_CRS, ResourceTemplate (){
  

•    QwordMemory (
  

•      ResourceProducer,
  

•      PosDecode,
  

•      MinFixed,
  

•      MaxFixed,
  

•      NonCacheable,
  

•      ReadWrite,
  

•      0x0, // Granularity
  

•      0x8a0200000, // Min Base Address
  

•      0x8a020ffff, // Max Base Address
  

•      0x0, // Translate
  

•      0x10000 // Length
  

•   )
  

•  })
  

•  Return (RBYV())
  

  }
• } // Device(PCI5)
• Device (RES5)
• {
• Name (_HID, "HISI0081") // HiSi PCIe RC config base address
• Name (_CID, "PNP0C02") // Motherboard reserved resource
• Name (_UID, 0x5) // Unique ID
• Name (_CRS, ResourceTemplate (){

•  QwordMemory (  //host bridge register space
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x8a0200000, // Min Base Address
  

•    0x8a020ffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x10000 // Length
  

•  )
  

• }) Method (_STA, 0x0, NotSerialized) { Return (RBYV()) }

• } // Device(PCI5)

• }

// 1P NB PCIe2 Device (PCI6) @@ -291,31 +411,58 @@ Scope(_SB) }) // Name(RBUF) Return (RBUF) } // Method(_CRS)

• Device (RES6)

• Device (RES6)
• {
• Name (_HID, "HISI0081") // HiSi PCIe RC config base address
• Name (_CRS, ResourceTemplate (){

•  QwordMemory ( //host bridge register space
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x8a00a0000, // Min Base Address
  

•    0x8a00affff, // Max Base Address
  

•    0x0, // Translate
  

•    0x10000 // Length
  

•  )
  

• })
• }
• PCI_OSC_SUPPORT()
• Method (_STA, 0x0, NotSerialized) {

•  Name (_HID, "HISI0081") // HiSi PCIe RC config base address
  

•  Name (_CID, "PNP0C02")  // Motherboard reserved resource
  

•  Name (_CRS, ResourceTemplate (){
  

•    QwordMemory (
  

•      ResourceProducer,
  

•      PosDecode,
  

•      MinFixed,
  

•      MaxFixed,
  

•      NonCacheable,
  

•      ReadWrite,
  

•      0x0, // Granularity
  

•      0x8a00a0000, // Min Base Address
  

•      0x8a00affff, // Max Base Address
  

•      0x0, // Translate
  

•      0x10000 // Length
  

• )

• })
  

•  Return (RBYV())
  

  }
• } // Device(PCI6)
• Device (RES6)
• {
• Name (_HID, "HISI0081") // HiSi PCIe RC config base address
• Name (_CID, "PNP0C02") // Motherboard reserved resource
• Name (_UID, 0x6) // Unique ID
• Name (_CRS, ResourceTemplate (){

•  QwordMemory ( //host bridge register space
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x8a00a0000, // Min Base Address
  

•    0x8a00affff, // Max Base Address
  

•    0x0, // Translate
  

•    0x10000 // Length
  

•  )
  

• }) Method (_STA, 0x0, NotSerialized) { Return (RBYV()) }

• } // Device(PCI6)

• }
• // 1P NB PCIe3 Device (PCI7) {

@@ -362,31 +509,120 @@ Scope(_SB) }) // Name(RBUF) Return (RBUF) } // Method(_CRS)

• Device (RES7)

• Device (RES7)
• {
• Name (_HID, "HISI0081") // HiSi PCIe RC config base address
• Name (_CRS, ResourceTemplate (){

•  QwordMemory (  //host bridge register space
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x8a00b0000, // Min Base Address
  

•    0x8a00bffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x10000 // Length
  

•  )
  

• })
• }
• PCI_OSC_SUPPORT()
• Method (_STA, 0x0, NotSerialized) {

•  Name (_HID, "HISI0081") // HiSi PCIe RC config base address
  

•  Name (_CID, "PNP0C02")  // Motherboard reserved resource
  

•  Name (_CRS, ResourceTemplate (){
  

•    QwordMemory (
  

•      ResourceProducer,
  

•      PosDecode,
  

•      MinFixed,
  

•      MaxFixed,
  

•      NonCacheable,
  

•      ReadWrite,
  

•      0x0, // Granularity
  

•      0x8a00b0000, // Min Base Address
  

•      0x8a00bffff, // Max Base Address
  

•      0x0, // Translate
  

•      0x10000 // Length
  

•    )
  

•  })
  

•  Return (RBYV())
  

  }
• } // Device(PCI7)
• Device (RES7)
• {
• Name (_HID, "HISI0081") // HiSi PCIe RC config base address
• Name (_CID, "PNP0C02") // Motherboard reserved resource
• Name (_UID, 0x7) // Unique ID
• Name (_CRS, ResourceTemplate (){

•  QwordMemory (  //host bridge register space
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x8a00b0000, // Min Base Address
  

•    0x8a00bffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x10000 // Length
  

•  )
  

• }) Method (_STA, 0x0, NotSerialized) { Return (RBYV()) }

• } // Device(PCI7)

• }
• Device (R1NB) // reserve 1p NB ECAM resource
• {
• Name (_HID, "PNP0C02") // Motherboard reserved resource
• Name (_CRS, ResourceTemplate (){

•  QwordMemory ( //ECAM space for [bus 88-8f]
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x8a8800000, // Min Base Address
  

•    0x8a8ffffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x800000 // Length
  

•  )
  

•  QwordMemory ( //ECAM space for [bus 0-7]
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x8b0000000, // Min Base Address
  

•    0x8b07fffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x800000 // Length
  

•  )
  

•  QwordMemory (  //ECAM space for [bus c0-c7]
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x8ac000000, // Min Base Address
  

•    0x8ac7fffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x800000 // Length
  

•  )
  

•  QwordMemory (   //ECAM space for [bus 90-97]
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x8b9000000, // Min Base Address
  

•    0x8b97fffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x800000 // Length
  

•  )
  

• })
• Method (_STA, 0x0, NotSerialized)
• {

•  Return (RBYV())
  

• }
• }
• // 2P NA PCIe2 Device (PCIa) {

@@ -433,31 +669,82 @@ Scope(_SB) }) // Name(RBUF) Return (RBUF) } // Method(_CRS)

• Device (RESa)

• Device (RESa)
• {
• Name (_HID, "HISI0081") // HiSi PCIe RC config base address
• Name (_CRS, ResourceTemplate (){

•  QwordMemory ( //host bridge register space
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x600a00a0000, // Min Base Address
  

•    0x600a00affff, // Max Base Address
  

•    0x0, // Translate
  

•    0x10000 // Length
  

•  )
  

• })
• }
• PCI_OSC_SUPPORT()
• Method (_STA, 0x0, NotSerialized) {

•  Name (_HID, "HISI0081") // HiSi PCIe RC config base address
  

•  Name (_CID, "PNP0C02")  // Motherboard reserved resource
  

•  Name (_CRS, ResourceTemplate (){
  

•    QwordMemory (
  

•      ResourceProducer,
  

•      PosDecode,
  

•      MinFixed,
  

•      MaxFixed,
  

•      NonCacheable,
  

•      ReadWrite,
  

•      0x0, // Granularity
  

•      0x600a00a0000, // Min Base Address
  

•      0x600a00affff, // Max Base Address
  

•      0x0, // Translate
  

•      0x10000 // Length
  

•    )
  

•  })
  

•  Return (0xf)
  

  }
• } // Device(PCIa)
• Device (RESa)
• {
• Name (_HID, "HISI0081") // HiSi PCIe RC config base address
• Name (_CID, "PNP0C02") // Motherboard reserved resource
• Name (_UID, 0xa) // Unique ID
• Name (_CRS, ResourceTemplate (){

•  QwordMemory ( //host bridge register space
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x600a00a0000, // Min Base Address
  

•    0x600a00affff, // Max Base Address
  

•    0x0, // Translate
  

•    0x10000 // Length
  

•  )
  

• }) Method (_STA, 0x0, NotSerialized) { Return (0xf) }

• } // Device(PCIa)

• }
• Device (R2NA) //reserve for 2p NA ecam resource
• {
• Name (_HID, "PNP0C02") // Motherboard reserved resource
• Name (_CRS, ResourceTemplate (){

•  QwordMemory ( //ECAM space for [bus 10-1f]
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x64001000000, // Min Base Address
  

•    0x64001ffffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x1000000 // Length
  

•  )
  

• })
• Method (_STA, 0x0, NotSerialized)
• {

•  Return (0xf)
  

• }
• }
• // 2P NB PCIe0 Device (PCIc) {

@@ -504,32 +791,57 @@ Scope(_SB) }) // Name(RBUF) Return (RBUF) } // Method(_CRS)

• Device (RESc)
• {

•  Name (_HID, "HISI0081") // HiSi PCIe RC config base address
  

•  Name (_CID, "PNP0C02")  // Motherboard reserved resource
  

•  Name (_CRS, ResourceTemplate (){
  

•    QwordMemory (
  

•      ResourceProducer,
  

•      PosDecode,
  

•      MinFixed,
  

•      MaxFixed,
  

•      NonCacheable,
  

•      ReadWrite,
  

•      0x0, // Granularity
  

•      0x700a0090000, // Min Base Address
  

•      0x700a009ffff, // Max Base Address
  

•      0x0, // Translate
  

•      0x10000 // Length
  

•    )
  

•  })
  

• }

• Device (RESc)
• {
• Name (_HID, "HISI0081") // HiSi PCIe RC config base address
• Name (_CRS, ResourceTemplate (){

•  QwordMemory ( //host bridge register space
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x700a0090000, // Min Base Address
  

•    0x700a009ffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x10000 // Length
  

•  )
  

• })
• }
• PCI_OSC_SUPPORT() Method (_STA, 0x0, NotSerialized) { Return (RBYV()) } } // Device(PCIc)

• Device (RESc)
• {
• Name (_HID, "HISI0081") // HiSi PCIe RC config base address
• Name (_CID, "PNP0C02") // Motherboard reserved resource
• Name (_UID, 0xc) // Unique ID
• Name (_CRS, ResourceTemplate (){

•  QwordMemory ( //host bridge register space
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x700a0090000, // Min Base Address
  

•    0x700a009ffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x10000 // Length
  

•  )
  

• })
• Method (_STA, 0x0, NotSerialized)
• {

•  Return (RBYV())
  

• }
• } //2P NB PCIe1 Device (PCId) {

@@ -576,30 +888,93 @@ Scope(_SB) }) // Name(RBUF) Return (RBUF) } // Method(_CRS)

• Device (RESd)

• Device (RESd)
• {
• Name (_HID, "HISI0081") // HiSi PCIe RC config base address
• Name (_CRS, ResourceTemplate (){ //host bridge register space

•  QwordMemory (
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x700a0200000, // Min Base Address
  

•    0x700a020ffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x10000 // Length
  

•  )
  

• })
• }
• PCI_OSC_SUPPORT()
• Method (_STA, 0x0, NotSerialized) {

•  Name (_HID, "HISI0081") // HiSi PCIe RC config base address
  

•  Name (_CID, "PNP0C02")  // Motherboard reserved resource
  

•  Name (_CRS, ResourceTemplate (){
  

•    QwordMemory (
  

•      ResourceProducer,
  

•      PosDecode,
  

•      MinFixed,
  

•      MaxFixed,
  

•      NonCacheable,
  

•      ReadWrite,
  

•      0x0, // Granularity
  

•      0x700a0200000, // Min Base Address
  

•      0x700a020ffff, // Max Base Address
  

•      0x0, // Translate
  

•      0x10000 // Length
  

•    )
  

•  })
  

•  Return (RBYV())
  

• }
• } // Device(PCId)
• Device (RESd)
• {
• Name (_HID, "HISI0081") // HiSi PCIe RC config base address
• Name (_CID, "PNP0C02") // Motherboard reserved resource
• Name (_UID, 0xd) // Unique ID
• Name (_CRS, ResourceTemplate (){ //host bridge register space

•  QwordMemory (
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x700a0200000, // Min Base Address
  

•    0x700a020ffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x10000 // Length
  

•  )
  

• })
• Method (_STA, 0x0, NotSerialized)
• {

•  Return (RBYV())
  

  }
• }
• Device (R2NB) //reserve for 2p NB ecam resource
• {
• Name (_HID, "PNP0C02") // Motherboard reserved resource
• Name (_CRS, ResourceTemplate (){

•  QwordMemory (  //ECAM space for [bus 20-2f]
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x74002000000, // Min Base Address
  

•    0x74002ffffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x1000000 // Length
  

•  )
  

•  QwordMemory (  //ECAM space for [bus 30-3f]
  

•    ResourceConsumer,
  

•    PosDecode,
  

•    MinFixed,
  

•    MaxFixed,
  

•    NonCacheable,
  

•    ReadWrite,
  

•    0x0, // Granularity
  

•    0x78003000000, // Min Base Address
  

•    0x78003ffffff, // Max Base Address
  

•    0x0, // Translate
  

•    0x1000000 // Length
  

•  )
  

• }) Method (_STA, 0x0, NotSerialized) { Return (RBYV()) }

• } // Device(PCId)

• }

} -- 1.9.1

Hi Leif,

在 2017/3/22 0:46, Leif Lindholm 写道:

On Mon, Mar 20, 2017 at 09:11:19PM +0800, Chenhui Sun wrote:

...

SP805 watchdog driver does not really take effect the platforms, so we switch to generic watchdog driver.

Again, this description does not explain what problem is being resolved. Please clarify the commit message.

If there is no need for a hardware watchdog, the change itself looks plausible.

The system hung during SCT SetWatchdogTimer_Func testing, but we found that it is not caused by this driver, we will continue to debug it, I will drop this patch first.

Thanks and Regards, Chenhui

Regards,

Leif

...

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: wanglijun wanglijun@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Signed-off-by: Yi Li phoenix.liyi@huawei.com

---

Platforms/Hisilicon/D02/Pv660D02.dsc | 2 +- Platforms/Hisilicon/D02/Pv660D02.fdf | 2 +- Platforms/Hisilicon/D03/D03.dsc | 2 +- Platforms/Hisilicon/D03/D03.fdf | 2 +- Platforms/Hisilicon/D05/D05.dsc | 2 +- Platforms/Hisilicon/D05/D05.fdf | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Platforms/Hisilicon/D02/Pv660D02.dsc b/Platforms/Hisilicon/D02/Pv660D02.dsc index 3f2ea31..2a70974 100644 --- a/Platforms/Hisilicon/D02/Pv660D02.dsc +++ b/Platforms/Hisilicon/D02/Pv660D02.dsc @@ -339,7 +339,7 @@ ArmPkg/Drivers/ArmGic/ArmGicDxe.inf ArmPkg/Drivers/TimerDxe/TimerDxe.inf

• ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf

• MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf

IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCodeRuntimeDxe.inf # diff --git a/Platforms/Hisilicon/D02/Pv660D02.fdf b/Platforms/Hisilicon/D02/Pv660D02.fdf index 406b501..b430712 100644 --- a/Platforms/Hisilicon/D02/Pv660D02.fdf +++ b/Platforms/Hisilicon/D02/Pv660D02.fdf @@ -196,7 +196,7 @@ READ_LOCK_STATUS = TRUE INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf

• INF ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf

• INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf

# # FAT filesystem + GPT/MBR partitioning diff --git a/Platforms/Hisilicon/D03/D03.dsc b/Platforms/Hisilicon/D03/D03.dsc index 81b8271..24c88a3 100644 --- a/Platforms/Hisilicon/D03/D03.dsc +++ b/Platforms/Hisilicon/D03/D03.dsc @@ -396,7 +396,7 @@ ArmPkg/Drivers/TimerDxe/TimerDxe.inf

• ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf

• MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCodeRuntimeDxe.inf # #ACPI

diff --git a/Platforms/Hisilicon/D03/D03.fdf b/Platforms/Hisilicon/D03/D03.fdf index 3102f94..b254667 100644 --- a/Platforms/Hisilicon/D03/D03.fdf +++ b/Platforms/Hisilicon/D03/D03.fdf @@ -189,7 +189,7 @@ READ_LOCK_STATUS = TRUE INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf

• INF ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf

• INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf

# # FAT filesystem + GPT/MBR partitioning diff --git a/Platforms/Hisilicon/D05/D05.dsc b/Platforms/Hisilicon/D05/D05.dsc index 78c7913..9de1be6 100644 --- a/Platforms/Hisilicon/D05/D05.dsc +++ b/Platforms/Hisilicon/D05/D05.dsc @@ -516,7 +516,7 @@ ArmPkg/Drivers/TimerDxe/TimerDxe.inf

• ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf

• MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCodeRuntimeDxe.inf # #ACPI

diff --git a/Platforms/Hisilicon/D05/D05.fdf b/Platforms/Hisilicon/D05/D05.fdf index 5a646f3..987427c 100644 --- a/Platforms/Hisilicon/D05/D05.fdf +++ b/Platforms/Hisilicon/D05/D05.fdf @@ -193,7 +193,7 @@ READ_LOCK_STATUS = TRUE INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf

• INF ArmPlatformPkg/Drivers/SP805WatchdogDxe/SP805WatchdogDxe.inf

• INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf

# # FAT filesystem + GPT/MBR partitioning -- 1.9.1

On Mon, Mar 20, 2017 at 09:11:20PM +0800, Chenhui Sun wrote:

Io BAR should be based IoBase and Mem BAR should be based PciRegionBase.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ming Huang huangming23@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Signed-off-by: Yi Li phoenix.liyi@huawei.com

---

.../Drivers/PciHostBridgeDxe/PciHostBridge.c | 29 ++++++++++++++-------- .../Drivers/PciHostBridgeDxe/PciRootBridgeIo.c | 15 +++++++++-- 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciHostBridge.c b/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciHostBridge.c index a970da6..2289d4b 100644 --- a/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciHostBridge.c +++ b/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciHostBridge.c @@ -1410,9 +1410,8 @@ SetResource( Ptr->ResType = 1; Ptr->GenFlag = 0; Ptr->SpecificFlag = 0;

•    /* This is PCIE Device Bus which start address is the low 32bit of mem base*/
  

•    Ptr->AddrRangeMin = (RootBridgeInstance->ResAllocNode[Index].Base - RootBridgeInstance->MemBase) +
  

•                        (RootBridgeInstance->MemBase & 0xFFFFFFFF);
  

•    /* PCIE Device Iobar address should be based on IoBase */
  

•    Ptr->AddrRangeMin = RootBridgeInstance->IoBase;
     Ptr->AddrRangeMax = 0;
     Ptr->AddrTranslationOffset = \
          (ResStatus == ResAllocated) ? EFI_RESOURCE_SATISFIED : EFI_RESOURCE_LESS;
  

@@ -1429,9 +1428,13 @@ SetResource( Ptr->GenFlag = 0; Ptr->SpecificFlag = 0; Ptr->AddrSpaceGranularity = 32;

•    /* This is PCIE Device Bus which start address is the low 32bit of mem base*/
  

•    /* PCIE device Bar should be based on PciRegionBase */
  

•    if (RootBridgeInstance->PciRegionBase > 0xFFFFFFFF) {
  

•      DEBUG((DEBUG_ERROR, "PCIE Res(TypeMem32) unsupported.\n"));
  

Space before "(("..

•      return EFI_UNSUPPORTED;
  

The commit message describes nothing obviously to do with this modification. Why is a RegionBase above 32 bit invalid?

•    }
     Ptr->AddrRangeMin = (RootBridgeInstance->ResAllocNode[Index].Base - RootBridgeInstance->MemBase) +
  

•                         (RootBridgeInstance->MemBase & 0xFFFFFFFF);
  

•                         (RootBridgeInstance->PciRegionBase & 0xFFFFFFFF);
  

The current code above has already returned if any bits > 4GB are set, so this mask is now redundant.

     Ptr->AddrRangeMax = 0;
     Ptr->AddrTranslationOffset = \
          (ResStatus == ResAllocated) ? EFI_RESOURCE_SATISFIED : EFI_RESOURCE_LESS;

@@ -1448,9 +1451,13 @@ SetResource( Ptr->GenFlag = 0; Ptr->SpecificFlag = 6; Ptr->AddrSpaceGranularity = 32;

•    /* This is PCIE Device Bus which start address is the low 32bit of mem base*/
  

•    /* PCIE device Bar should be based on PciRegionBase */
  

•    if (RootBridgeInstance->PciRegionBase > 0xFFFFFFFF) {
  

•      DEBUG((DEBUG_ERROR, "PCIE Res(TypePMem32) unsupported.\n"));
  

•      return EFI_UNSUPPORTED;
  

•    }
     Ptr->AddrRangeMin = (RootBridgeInstance->ResAllocNode[Index].Base - RootBridgeInstance->MemBase) +
  

•                         (RootBridgeInstance->MemBase & 0xFFFFFFFF);
  

•                         (RootBridgeInstance->PciRegionBase & 0xFFFFFFFF);
  

Same three comments as above.

     Ptr->AddrRangeMax = 0;
     Ptr->AddrTranslationOffset = \
          (ResStatus == ResAllocated) ? EFI_RESOURCE_SATISFIED : EFI_RESOURCE_LESS;

@@ -1467,9 +1474,9 @@ SetResource( Ptr->GenFlag = 0; Ptr->SpecificFlag = 0; Ptr->AddrSpaceGranularity = 64;

•    /* This is PCIE Device Bus which start address is the low 32bit of mem base*/
  

•    /* PCIE device Bar should be based on PciRegionBase */
     Ptr->AddrRangeMin = (RootBridgeInstance->ResAllocNode[Index].Base - RootBridgeInstance->MemBase) +
  

•                         (RootBridgeInstance->MemBase & 0xFFFFFFFFFFFFFFFF);
  

•                         (RootBridgeInstance->PciRegionBase & 0xFFFFFFFFFFFFFFFF);
  

I don't believe these are type safe. Preferably replace the direct literal masks on lines you are modifying with MAX_UINT32 and MAX_UINT64 to ensure to avoiding amusing integer promotion disasters.

     Ptr->AddrRangeMax = 0;
     Ptr->AddrTranslationOffset = \
          (ResStatus == ResAllocated) ? EFI_RESOURCE_SATISFIED : EFI_RESOURCE_LESS;

@@ -1486,9 +1493,9 @@ SetResource( Ptr->GenFlag = 0; Ptr->SpecificFlag = 6; Ptr->AddrSpaceGranularity = 64;

•    /* This is PCIE Device Bus which start address is the low 32bit of mem base*/
  

•    /* PCIE device Bar should be based on PciRegionBase */
     Ptr->AddrRangeMin = (RootBridgeInstance->ResAllocNode[Index].Base - RootBridgeInstance->MemBase) +
  

•                         (RootBridgeInstance->MemBase & 0xFFFFFFFFFFFFFFFF);
  

•                         (RootBridgeInstance->PciRegionBase & 0xFFFFFFFFFFFFFFFF);
     Ptr->AddrRangeMax = 0;
     Ptr->AddrTranslationOffset = \
          (ResStatus == ResAllocated) ? EFI_RESOURCE_SATISFIED : EFI_RESOURCE_LESS;
  

In general, there is a spectacular amount of duplicated code in this function. It would be worth refactoring so that the lines that are identical between all cases are not repeated.

At the same time it would be useful to replace the hard-coded numerals in this switch statement with more descriptive #defines.

And a default: target with an assert would look a bit neater.

diff --git a/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c b/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c index 30619f5..7902831 100644 --- a/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c +++ b/Chips/Hisilicon/Drivers/PciHostBridgeDxe/PciRootBridgeIo.c @@ -2312,8 +2312,19 @@ RootBridgeIoConfiguration ( PrivateData = DRIVER_INSTANCE_FROM_PCI_ROOT_BRIDGE_IO_THIS (This); for (Index = 0; Index < TypeMax; Index++) { if (PrivateData->ResAllocNode[Index].Status == ResAllocated) {

•  Configuration.SpaceDesp[Index].AddrRangeMin = PrivateData->ResAllocNode[Index].Base;
  

•  Configuration.SpaceDesp[Index].AddrRangeMax = PrivateData->ResAllocNode[Index].Base + PrivateData->ResAllocNode[Index].Length - 1;
  

•  switch (Index) {
  

•  case TypeIo:
  

•    Configuration.SpaceDesp[Index].AddrRangeMin = PrivateData->IoBase;
  

•    break;
  

•  case TypeBus:
  

•    Configuration.SpaceDesp[Index].AddrRangeMin = PrivateData->ResAllocNode[Index].Base;
  

•    break;
  

•  default:
  

•  /* PCIE Device bar address should be base on PciRegionBase */
  

•  Configuration.SpaceDesp[Index].AddrRangeMin = (PrivateData->ResAllocNode[Index].Base - PrivateData->MemBase) +
  

•                         (PrivateData->PciRegionBase & 0xFFFFFFFFFFFFFFFF);
  

Please replace this hand-coded mask with MAX_UINT64.

/ Leif

•  }
  

•  Configuration.SpaceDesp[Index].AddrRangeMax = Configuration.SpaceDesp[Index].AddrRangeMin + PrivateData->ResAllocNode[Index].Length - 1;
   Configuration.SpaceDesp[Index].AddrLen      = PrivateData->ResAllocNode[Index].Length;
  

  } }

-- 1.9.1

patches 17-19 seem to be fairly normal standard upgrade and all looks good to me.

Reviewed-by: Graeme Gregory graeme.gregory@linaro.org

On Mon, Mar 20, 2017 at 09:11:21PM +0800, Chenhui Sun wrote:

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chenhui Sun sunchenhui@huawei.com Signed-off-by: Heyi Guo heyi.guo@linaro.org Signed-off-by: Yi Li phoenix.liyi@huawei.com

---

.../Hisilicon/Hi1610/Hi1610AcpiTables/D03Mcfg.aslc | 20 ++-- Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Facs.aslc | 8 +- Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Fadt.aslc | 39 +++---- Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Gtdt.aslc | 30 +++--- .../Hi1610/Hi1610AcpiTables/MadtHi1610.aslc | 120 ++++++++++----------- Chips/Hisilicon/Include/Library/AcpiNextLib.h | 2 +- 6 files changed, 110 insertions(+), 109 deletions(-)

diff --git a/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/D03Mcfg.aslc b/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/D03Mcfg.aslc index ed47a44..7e5c8ef 100644 --- a/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/D03Mcfg.aslc +++ b/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/D03Mcfg.aslc @@ -11,7 +11,7 @@ #include <IndustryStandard/Acpi.h> #include "Hi1610Platform.h" -#define ACPI_5_0_MCFG_VERSION 0x1 +#define ACPI_6_1_MCFG_VERSION 0x1 #pragma pack(1) typedef struct @@ -21,28 +21,28 @@ typedef struct UINT8 ucStartBusNum; UINT8 ucEndBusNum; UINT32 Reserved2; -}EFI_ACPI_5_0_MCFG_CONFIG_STRUCTURE; +}EFI_ACPI_6_1_MCFG_CONFIG_STRUCTURE; typedef struct { EFI_ACPI_DESCRIPTION_HEADER Header; UINT64 Reserved1; -}EFI_ACPI_5_0_MCFG_TABLE_CONFIG; +}EFI_ACPI_6_1_MCFG_TABLE_CONFIG; typedef struct {

• EFI_ACPI_5_0_MCFG_TABLE_CONFIG Acpi_Table_Mcfg;
• EFI_ACPI_5_0_MCFG_CONFIG_STRUCTURE Config_Structure[3];

-}EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_TABLE;

• EFI_ACPI_6_1_MCFG_TABLE_CONFIG Acpi_Table_Mcfg;
• EFI_ACPI_6_1_MCFG_CONFIG_STRUCTURE Config_Structure[3];

+}EFI_ACPI_6_1_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_TABLE; #pragma pack() -EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_TABLE Mcfg= +EFI_ACPI_6_1_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_TABLE Mcfg= { { {

•    EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
  

•    sizeof (EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_TABLE),
  

•    ACPI_5_0_MCFG_VERSION,
  

•    EFI_ACPI_6_1_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE,
  

•    sizeof (EFI_ACPI_6_1_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_TABLE),
  

•    ACPI_6_1_MCFG_VERSION,
     0x00,                                                     // Checksum will be updated at runtime
     {EFI_ACPI_ARM_OEM_ID},
     EFI_ACPI_ARM_OEM_TABLE_ID,
  

diff --git a/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Facs.aslc b/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Facs.aslc index 72cc66c..d5bc299 100644 --- a/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Facs.aslc +++ b/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Facs.aslc @@ -19,15 +19,15 @@ #include <IndustryStandard/Acpi.h> -EFI_ACPI_5_0_FIRMWARE_ACPI_CONTROL_STRUCTURE Facs = {

• EFI_ACPI_5_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE, // UINT32 Signature
• sizeof (EFI_ACPI_5_0_FIRMWARE_ACPI_CONTROL_STRUCTURE), // UINT32 Length

+EFI_ACPI_6_1_FIRMWARE_ACPI_CONTROL_STRUCTURE Facs = {

• EFI_ACPI_6_1_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE, // UINT32 Signature
• sizeof (EFI_ACPI_6_1_FIRMWARE_ACPI_CONTROL_STRUCTURE), // UINT32 Length 0xA152, // UINT32 HardwareSignature 0, // UINT32 FirmwareWakingVector 0, // UINT32 GlobalLock 0, // UINT32 Flags 0, // UINT64 XFirmwareWakingVector

• EFI_ACPI_5_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_VERSION, // UINT8 Version;

• EFI_ACPI_6_1_FIRMWARE_ACPI_CONTROL_STRUCTURE_VERSION, // UINT8 Version; { EFI_ACPI_RESERVED_BYTE, // UINT8 Reserved0[0] EFI_ACPI_RESERVED_BYTE, // UINT8 Reserved0[1] EFI_ACPI_RESERVED_BYTE }, // UINT8 Reserved0[2]

diff --git a/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Fadt.aslc b/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Fadt.aslc index 5307041..025b42c 100644 --- a/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Fadt.aslc +++ b/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Fadt.aslc @@ -22,16 +22,16 @@ #include <Library/AcpiLib.h> #include <IndustryStandard/Acpi.h> -EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE Fadt = { +EFI_ACPI_6_1_FIXED_ACPI_DESCRIPTION_TABLE Fadt = { ARM_ACPI_HEADER (

• EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
• EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE,
• EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE_REVISION

• EFI_ACPI_6_1_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
• EFI_ACPI_6_1_FIXED_ACPI_DESCRIPTION_TABLE,
• EFI_ACPI_6_1_FIXED_ACPI_DESCRIPTION_TABLE_REVISION ), 0, // UINT32 FirmwareCtrl 0, // UINT32 Dsdt EFI_ACPI_RESERVED_BYTE, // UINT8 Reserved0

• EFI_ACPI_5_0_PM_PROFILE_UNSPECIFIED, // UINT8 PreferredPmProfile

• EFI_ACPI_6_1_PM_PROFILE_UNSPECIFIED, // UINT8 PreferredPmProfile 0, // UINT16 SciInt 0, // UINT32 SmiCmd 0, // UINT8 AcpiEnable

@@ -65,23 +65,24 @@ EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE Fadt = { 0, // UINT8 Century 0, // UINT16 IaPcBootArch 0, // UINT8 Reserved1

• EFI_ACPI_5_0_HW_REDUCED_ACPI | EFI_ACPI_5_0_LOW_POWER_S0_IDLE_CAPABLE, // UINT32 Flags
• NULL_GAS, // EFI_ACPI_5_0_GENERIC_ADDRESS_STRUCTURE ResetReg

• EFI_ACPI_6_1_HW_REDUCED_ACPI | EFI_ACPI_6_1_LOW_POWER_S0_IDLE_CAPABLE, // UINT32 Flags
• NULL_GAS, // EFI_ACPI_6_1_GENERIC_ADDRESS_STRUCTURE ResetReg 0, // UINT8 ResetValue

• EFI_ACPI_5_1_ARM_PSCI_COMPLIANT, // UINT16 ArmBootArchFlags
• EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE_MINOR_REVISION, // UINT8 MinorRevision

• EFI_ACPI_6_1_ARM_PSCI_COMPLIANT, // UINT16 ArmBootArchFlags
• EFI_ACPI_6_1_FIXED_ACPI_DESCRIPTION_TABLE_MINOR_REVISION, // UINT8 MinorRevision 0, // UINT64 XFirmwareCtrl 0, // UINT64 XDsdt

• NULL_GAS, // EFI_ACPI_5_0_GENERIC_ADDRESS_STRUCTURE XPm1aEvtBlk
• NULL_GAS, // EFI_ACPI_5_0_GENERIC_ADDRESS_STRUCTURE XPm1bEvtBlk
• NULL_GAS, // EFI_ACPI_5_0_GENERIC_ADDRESS_STRUCTURE XPm1aCntBlk
• NULL_GAS, // EFI_ACPI_5_0_GENERIC_ADDRESS_STRUCTURE XPm1bCntBlk
• NULL_GAS, // EFI_ACPI_5_0_GENERIC_ADDRESS_STRUCTURE XPm2CntBlk
• NULL_GAS, // EFI_ACPI_5_0_GENERIC_ADDRESS_STRUCTURE XPmTmrBlk
• NULL_GAS, // EFI_ACPI_5_0_GENERIC_ADDRESS_STRUCTURE XGpe0Blk
• NULL_GAS, // EFI_ACPI_5_0_GENERIC_ADDRESS_STRUCTURE XGpe1Blk
• NULL_GAS, // EFI_ACPI_5_0_GENERIC_ADDRESS_STRUCTURE SleepControlReg
• NULL_GAS // EFI_ACPI_5_0_GENERIC_ADDRESS_STRUCTURE SleepStatusReg

• NULL_GAS, // EFI_ACPI_6_1_GENERIC_ADDRESS_STRUCTURE XPm1aEvtBlk
• NULL_GAS, // EFI_ACPI_6_1_GENERIC_ADDRESS_STRUCTURE XPm1bEvtBlk
• NULL_GAS, // EFI_ACPI_6_1_GENERIC_ADDRESS_STRUCTURE XPm1aCntBlk
• NULL_GAS, // EFI_ACPI_6_1_GENERIC_ADDRESS_STRUCTURE XPm1bCntBlk
• NULL_GAS, // EFI_ACPI_6_1_GENERIC_ADDRESS_STRUCTURE XPm2CntBlk
• NULL_GAS, // EFI_ACPI_6_1_GENERIC_ADDRESS_STRUCTURE XPmTmrBlk
• NULL_GAS, // EFI_ACPI_6_1_GENERIC_ADDRESS_STRUCTURE XGpe0Blk
• NULL_GAS, // EFI_ACPI_6_1_GENERIC_ADDRESS_STRUCTURE XGpe1Blk
• NULL_GAS, // EFI_ACPI_6_1_GENERIC_ADDRESS_STRUCTURE SleepControlReg
• NULL_GAS, // EFI_ACPI_6_1_GENERIC_ADDRESS_STRUCTURE SleepStatusReg
• 0 // UINT64 Hypervisor Vendor Identify

}; // diff --git a/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Gtdt.aslc b/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Gtdt.aslc index 922f5c3..4c1050a 100644 --- a/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Gtdt.aslc +++ b/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/Gtdt.aslc @@ -23,9 +23,9 @@ #include <Library/PcdLib.h> #include <IndustryStandard/Acpi.h> -#define GTDT_GLOBAL_FLAGS_MAPPED EFI_ACPI_5_0_GTDT_GLOBAL_FLAG_MEMORY_MAPPED_BLOCK_PRESENT +#define GTDT_GLOBAL_FLAGS_MAPPED EFI_ACPI_6_1_GTDT_GLOBAL_FLAG_MEMORY_MAPPED_BLOCK_PRESENT #define GTDT_GLOBAL_FLAGS_NOT_MAPPED 0 -#define GTDT_GLOBAL_FLAGS_EDGE EFI_ACPI_5_0_GTDT_GLOBAL_FLAG_INTERRUPT_MODE +#define GTDT_GLOBAL_FLAGS_EDGE EFI_ACPI_6_1_GTDT_GLOBAL_FLAG_INTERRUPT_MODE #define GTDT_GLOBAL_FLAGS_LEVEL 0 // Note: We could have a build flag that switches between memory mapped/non-memory mapped timer @@ -36,9 +36,9 @@ #define SYSTEM_TIMER_BASE_ADDRESS 0xFFFFFFFFFFFFFFFF #endif -#define GTDT_TIMER_EDGE_TRIGGERED EFI_ACPI_5_0_GTDT_TIMER_FLAG_TIMER_INTERRUPT_MODE +#define GTDT_TIMER_EDGE_TRIGGERED EFI_ACPI_6_1_GTDT_TIMER_FLAG_TIMER_INTERRUPT_MODE #define GTDT_TIMER_LEVEL_TRIGGERED 0 -#define GTDT_TIMER_ACTIVE_LOW EFI_ACPI_5_0_GTDT_TIMER_FLAG_TIMER_INTERRUPT_POLARITY +#define GTDT_TIMER_ACTIVE_LOW EFI_ACPI_6_1_GTDT_TIMER_FLAG_TIMER_INTERRUPT_POLARITY #define GTDT_TIMER_ACTIVE_HIGH 0 #define GTDT_GTIMER_FLAGS (GTDT_TIMER_ACTIVE_LOW | GTDT_TIMER_LEVEL_TRIGGERED) @@ -46,18 +46,18 @@ #pragma pack (1) typedef struct {

• EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE Gtdt;
• EFI_ACPI_5_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE Watchdogs[HI1610_WATCHDOG_COUNT];

-} EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES;

• EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE Gtdt;
• EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_STRUCTURE Watchdogs[HI1610_WATCHDOG_COUNT];

+} EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLES; #pragma pack () -EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = { +EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = { { ARM_ACPI_HEADER(

•  EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
  

•  EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES,
  

•  EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION
  

•  EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
  

•  EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLES,
  

•  EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE_REVISION
  

  ), SYSTEM_TIMER_BASE_ADDRESS, // UINT64 PhysicalAddress 0, // UINT32 Reserved

@@ -72,14 +72,14 @@ EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = { 0xFFFFFFFFFFFFFFFF, // UINT64 CntReadBasePhysicalAddress #ifdef notyet PV660_WATCHDOG_COUNT, // UINT32 PlatformTimerCount

• sizeof (EFI_ACPI_5_1_GENERIC_TIMER_DESCRIPTION_TABLE) // UINT32 PlatfromTimerOffset

• sizeof (EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE) // UINT32 PlatfromTimerOffset }, {

• EFI_ACPI_5_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(

• EFI_ACPI_6_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT( //FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 (PcdGenericWatchdogControlBase), 93, 0), 0, 0, 0, 0),

• EFI_ACPI_5_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(

•    //FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 (PcdGenericWatchdogControlBase), 94, EFI_ACPI_5_1_GTDT_SBSA_GENERIC_WATCHDOG_FLAG_SECURE_TIMER)
  

• EFI_ACPI_6_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(

•    //FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 (PcdGenericWatchdogControlBase), 94, EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_FLAG_SECURE_TIMER)
     0, 0, 0, 0)
  

  }

#else /* !notyet */ diff --git a/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/MadtHi1610.aslc b/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/MadtHi1610.aslc index 7bebe8f..f302dd6 100644 --- a/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/MadtHi1610.aslc +++ b/Chips/Hisilicon/Hi1610/Hi1610AcpiTables/MadtHi1610.aslc @@ -37,20 +37,20 @@ #pragma pack (1) typedef struct {

• EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER Header;
• EFI_ACPI_5_1_GIC_STRUCTURE GicInterfaces[16];
• EFI_ACPI_6_0_GIC_DISTRIBUTOR_STRUCTURE GicDistributor;
• EFI_ACPI_6_0_GIC_ITS_STRUCTURE GicITS[1];

-} EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE;

• EFI_ACPI_6_1_MULTIPLE_APIC_DESCRIPTION_TABLE_HEADER Header;
• EFI_ACPI_6_1_GIC_STRUCTURE GicInterfaces[16];
• EFI_ACPI_6_1_GIC_DISTRIBUTOR_STRUCTURE GicDistributor;
• EFI_ACPI_6_1_GIC_ITS_STRUCTURE GicITS[1];

+} EFI_ACPI_6_1_MULTIPLE_APIC_DESCRIPTION_TABLE; #pragma pack () -EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE Madt = { +EFI_ACPI_6_1_MULTIPLE_APIC_DESCRIPTION_TABLE Madt = { { ARM_ACPI_HEADER (

•  EFI_ACPI_1_0_APIC_SIGNATURE,
  

•  EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE,
  

•  EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION
  

•  EFI_ACPI_6_1_MULTIPLE_APIC_DESCRIPTION_TABLE_SIGNATURE,
  

•  EFI_ACPI_6_1_MULTIPLE_APIC_DESCRIPTION_TABLE,
  

•  EFI_ACPI_6_1_MULTIPLE_APIC_DESCRIPTION_TABLE_REVISION
  

  ), // // MADT specific fields

@@ -59,65 +59,65 @@ EFI_ACPI_1_0_MULTIPLE_APIC_DESCRIPTION_TABLE Madt = { 0, // Flags }, {

• // Format: EFI_ACPI_5_1_GICC_STRUCTURE_INIT(GicId, AcpiCpuUid, Flags, PmuIrq, GicBase, GicVBase, GicHBase,

• // Format: EFI_ACPI_6_1_GICC_STRUCTURE_INIT(GicId, AcpiCpuUid, Flags, PmuIrq, GicBase, GicVBase, GicHBase, // GsivId, GicRBase, Mpidr) // Note: The GIC Structure of the primary CPU must be the first entry (see note in 5.2.12.14 GICC Structure of // ACPI v5.1). // The cores from a same cluster are kept together. It is not an ACPI requirement but in case the OSPM uses // the ACPI ARM Parking protocol, it might want to wake up the cores in the order of this table.

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    0, 0, PLATFORM_GET_MPID(0, 0),  EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x100000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    1, 1, PLATFORM_GET_MPID(0, 1),  EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x130000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    2, 2, PLATFORM_GET_MPID(0, 2),  EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x160000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    3, 3, PLATFORM_GET_MPID(0, 3),  EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x190000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    4, 4, PLATFORM_GET_MPID(1, 0), EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x1C0000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    5, 5, PLATFORM_GET_MPID(1, 1),  EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x1F0000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    6, 6, PLATFORM_GET_MPID(1, 2),  EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x220000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    7, 7, PLATFORM_GET_MPID(1, 3),  EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x250000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    8, 8, PLATFORM_GET_MPID(2, 0),  EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x280000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    9, 9, PLATFORM_GET_MPID(2, 1),  EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x2B0000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    10, 10, PLATFORM_GET_MPID(2, 2),  EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x2E0000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    11, 11, PLATFORM_GET_MPID(2, 3),  EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x310000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    12, 12, PLATFORM_GET_MPID(3, 0), EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x340000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    13, 13, PLATFORM_GET_MPID(3, 1),  EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x370000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    14, 14, PLATFORM_GET_MPID(3, 2),  EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x3A0000 /* GicRBase */),
  

• EFI_ACPI_5_1_GICC_STRUCTURE_INIT(

•    15, 15, PLATFORM_GET_MPID(3, 3),  EFI_ACPI_5_0_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x3D0000 /* GicRBase */),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    0, 0, PLATFORM_GET_MPID(0, 0),  EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x100000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    1, 1, PLATFORM_GET_MPID(0, 1),  EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x130000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    2, 2, PLATFORM_GET_MPID(0, 2),  EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x160000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    3, 3, PLATFORM_GET_MPID(0, 3),  EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x190000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    4, 4, PLATFORM_GET_MPID(1, 0), EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x1C0000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    5, 5, PLATFORM_GET_MPID(1, 1),  EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x1F0000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    6, 6, PLATFORM_GET_MPID(1, 2),  EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x220000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    7, 7, PLATFORM_GET_MPID(1, 3),  EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x250000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    8, 8, PLATFORM_GET_MPID(2, 0),  EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x280000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    9, 9, PLATFORM_GET_MPID(2, 1),  EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x2B0000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    10, 10, PLATFORM_GET_MPID(2, 2),  EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x2E0000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    11, 11, PLATFORM_GET_MPID(2, 3),  EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x310000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    12, 12, PLATFORM_GET_MPID(3, 0), EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x340000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    13, 13, PLATFORM_GET_MPID(3, 1),  EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x370000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    14, 14, PLATFORM_GET_MPID(3, 2),  EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x3A0000 /* GicRBase */, 0),
  

• EFI_ACPI_6_1_GICC_STRUCTURE_INIT(

•    15, 15, PLATFORM_GET_MPID(3, 3),  EFI_ACPI_6_1_GIC_ENABLED, 23, FixedPcdGet32 (PcdGicInterruptInterfaceBase),
  

•    FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x20000, FixedPcdGet32 (PcdGicInterruptInterfaceBase) + 0x10000, 25, FixedPcdGet32 (PcdGicDistributorBase) + 0x3D0000 /* GicRBase */, 0),
  

  },

• EFI_ACPI_6_0_GIC_DISTRIBUTOR_INIT(0, FixedPcdGet32 (PcdGicDistributorBase), 0, 0x4),

• EFI_ACPI_6_1_GIC_DISTRIBUTOR_INIT(0, FixedPcdGet32 (PcdGicDistributorBase), 0, 0x4), {

• EFI_ACPI_6_0_GIC_ITS_INIT(0,0xC6000000),

• EFI_ACPI_6_1_GIC_ITS_INIT(0,0xC6000000), }

}; diff --git a/Chips/Hisilicon/Include/Library/AcpiNextLib.h b/Chips/Hisilicon/Include/Library/AcpiNextLib.h index 5a810ec..0e65b1f 100644 --- a/Chips/Hisilicon/Include/Library/AcpiNextLib.h +++ b/Chips/Hisilicon/Include/Library/AcpiNextLib.h @@ -35,7 +35,7 @@ #define EFI_ACPI_6_0_GICC_AFFINITY_STRUCTURE_INIT( \ ProximityDomain, ACPIProcessorUID, Flags, ClockDomain) \ { \

• 3, sizeof (EFI_ACPI_5_1_GICC_AFFINITY_STRUCTURE),ProximityDomain , \

• 3, sizeof (EFI_ACPI_6_1_GICC_AFFINITY_STRUCTURE),ProximityDomain , \ ACPIProcessorUID, Flags, ClockDomain \ }

1.9.1