[PATCH] watchdog: iTCO refactoring plus Apollo Lake and ICH9 support

[Christian Storm]

From: Christian Storm <christi...@siemens.com>

Refactoring of the iTCO watchdog support module to make
integration of further supported iTCOs easier.

Support for the following iTCO watchdogs is added/improved:
* Apollo Lake for IPC127E
* ICH9 for QEmu's q35 machine

Signed-off-by: Christian Storm <christi...@siemens.com>
---
drivers/watchdog/itco.c | 281 +++++++++++++++++++++++++++++-----------
1 file changed, 202 insertions(+), 79 deletions(-)

diff --git a/drivers/watchdog/itco.c b/drivers/watchdog/itco.c
index 8346f36..dc1733e 100644
--- a/drivers/watchdog/itco.c
+++ b/drivers/watchdog/itco.c
@@ -1,10 +1,11 @@
/*
* EFI Boot Guard, iTCO support (Version 3 and later)
*
- * Copyright (c) Siemens AG, 2017
+ * Copyright (c) Siemens AG, 2019
*
* Authors:
* Jan Kiszka <jan.k...@siemens.com>
+ * Christian Storm <christi...@siemens.com>
*
* This work is licensed under the terms of the GNU GPL, version 2. See
* the COPYING file in the top-level directory.
@@ -16,136 +17,258 @@
#include <efilib.h>
#include <pci/header.h>

-#define PCI_DEVICE_ID_INTEL_BAYTRAIL 0x0f1c
-#define PCI_DEVICE_ID_INTEL_WPT_LP 0x9cc3
-#define PCI_DEVICE_ID_INTEL_APL 0x5ae8
+#define PMBASE_ADDRMASK 0x0000ff80
+#define PMCBASE_ADDRMASK 0xfffffe00

-#define PMBASE_REG 0x40
-# define PMBASE_ADDRMASK 0xff00
-#define PMCBASE_REG 0x44
-# define PMCBASE_ADDRMASK 0xfffffe00
+#define SMI_TCO_MASK (1 << 13)

-#define SMI_EN_REG 0x30
-# define TCO_EN (1 << 13)
+#define TCO_RLD_REG 0x00
+#define TCO1_CNT_REG 0x08
+#define TCO_TMR_HLT_MASK (1 << 11)
+#define TCO_TMR_REG 0x12

-#define TCO_RLD_REG 0x00
-#define TCO1_CNT_REG 0x08
-# define TCO_TMR_HLT (1 << 11)
-#define TCO_TMR_REG 0x12
+#define PMC_NO_REBOOT_MASK (1 << 4)

-#define PMC_REG 0x08
-# define PMC_NO_REBOOT (1 << 4)
+enum iTCO_chipsets {
+ ITCO_INTEL_APL = 0,
+ ITCO_INTEL_BAYTRAIL,
+ ITCO_INTEL_WPT_LP,
+ ITCO_INTEL_ICH9,
+};

-static EFI_STATUS __attribute__((constructor))
-init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 pci_device_id,
- UINTN timeout)
+typedef struct {
+ CHAR16 name[16];
+ UINT32 pci_id;
+ UINT32 pmbase;
+ UINT32 pmcbasereg;
+ UINT32 pmcreg;
+ UINT32 smireg;
+} iTCO_info;
+
+static iTCO_info iTCO_chipset_info[] = {
+ [ITCO_INTEL_APL] =
+ {
+ .name = L"Apollo Lake",
+ .pci_id = 0x5ae8,
+ .pmcbasereg = 0x10,
+ .pmcreg = 0x1008,
+ .smireg = 0x40,
+ .pmbase = 0x400,
+ },
+ [ITCO_INTEL_BAYTRAIL] =
+ {
+ .name = L"Baytrail",
+ .pci_id = 0x0f1c,
+ .pmcbasereg = 0x44,
+ .pmcreg = 0x08,
+ .smireg = 0x30,
+ },
+ [ITCO_INTEL_WPT_LP] =
+ {
+ .name = L"Wildcat",
+ .pci_id = 0x9cc3,
+ .pmcbasereg = 0x44,
+ .pmcreg = 0x08,
+ .smireg = 0x30,
+ },
+ [ITCO_INTEL_ICH9] =
+ {
+ .name = L"ICH9", /* QEmu machine q35 */
+ .pci_id = 0x2918,
+ .pmcbasereg = 0x44,
+ .pmcreg = 0x08,
+ .smireg = 0x30,
+ },
+};
+
+static BOOLEAN itco_supported(UINT16 pci_device_id, UINT8 *index)
{
- UINT32 pmbase, tcobase, pmcbase, value;
+ for (UINT8 i = 0;
+ i < (sizeof(iTCO_chipset_info) / sizeof(iTCO_chipset_info[0]));
+ i++) {
+ if (pci_device_id == iTCO_chipset_info[i].pci_id) {
+ *index = i;
+ return TRUE;
+ }
+ }
+ return FALSE;
+}
+
+static UINT32 get_pmbase(EFI_PCI_IO *pci_io, iTCO_info *itco)
+{
+ UINT32 pmbase;
EFI_STATUS status;

- if (!pci_io || pci_vendor_id != PCI_VENDOR_ID_INTEL ||
- (pci_device_id != PCI_DEVICE_ID_INTEL_BAYTRAIL &&
- pci_device_id != PCI_DEVICE_ID_INTEL_WPT_LP &&
- pci_device_id != PCI_DEVICE_ID_INTEL_APL)) {
- return EFI_UNSUPPORTED;
+ if (itco->pmbase) {
+ return itco->pmbase & PMBASE_ADDRMASK;
}

- Print(L"Detected Intel TCO watchdog\n");
-
- /* Get PMBASE and TCOBASE */
status = uefi_call_wrapper(pci_io->Pci.Read, 5, pci_io,
- EfiPciIoWidthUint32, PMBASE_REG,
- 1, &pmbase);
+ EfiPciIoWidthUint32, 0x40, 1, &pmbase);
if (EFI_ERROR(status)) {
- return status;
+ Print(L"Error reading PMBASE: %r\n", status);
+ return 0;
}
- pmbase &= PMBASE_ADDRMASK;
- tcobase = (pmbase & PMBASE_ADDRMASK) + 0x60;
+ return pmbase & PMBASE_ADDRMASK;
+}

- /* Get PMCBASE address */
- status = uefi_call_wrapper(pci_io->Pci.Read, 5, pci_io,
- EfiPciIoWidthUint32, PMCBASE_REG,
- 1, &pmcbase);
+static EFI_STATUS update_no_reboot_flag(EFI_PCI_IO *pci_io, iTCO_info *itco)
+{
+ EFI_STATUS status;
+ UINT32 pmcbase, value;
+
+ status =
+ uefi_call_wrapper(pci_io->Pci.Read, 5, pci_io, EfiPciIoWidthUint32,
+ itco->pmcbasereg, 1, &pmcbase);
if (EFI_ERROR(status)) {
return status;
}
pmcbase &= PMCBASE_ADDRMASK;

+ status = uefi_call_wrapper(
+ pci_io->Mem.Read, 6, pci_io, EfiPciIoWidthUint32,
+ EFI_PCI_IO_PASS_THROUGH_BAR, pmcbase + itco->pmcreg, 1, &value);
+ if (EFI_ERROR(status)) {
+ return status;
+ }
+ value &= ~PMC_NO_REBOOT_MASK;
+ status = uefi_call_wrapper(
+ pci_io->Mem.Write, 6, pci_io, EfiPciIoWidthUint32,
+ EFI_PCI_IO_PASS_THROUGH_BAR, pmcbase + itco->pmcreg, 1, &value);
+ if (EFI_ERROR(status)) {
+ return status;
+ }
+ return status;
+}
+
+static EFI_STATUS update_no_reboot_flag_apl(__attribute__((unused))
+ EFI_PCI_IO *pci_io,
+ iTCO_info *itco)
+{
+#define MM_PCI_OFFSET(Bus, Device, Function) \
+ ((UINTN)(Bus << 20) + (UINTN)(Device << 15) + (UINTN)(Function << 12))
+
+#define PCIE_MMCFG_BASE 0xE0000000
+
+#define MmPciBase(Bus, Device, Function, Offset) \
+ ((UINTN)(PCIE_MMCFG_BASE) + MM_PCI_OFFSET(Bus, Device, Function) + \
+ Offset)
+
+#define MmPciBarAddr(Bus, Device, Function, Bar, Type) \
+ ((UINTN)(*( \
+ volatile Type *)(((UINTN)MmPciBase(Bus, Device, Function, \
+ 0x10 + (Bar) * sizeof(Type))) & \
+ ~0x0f)))
+
+ /* Unhide the P2SB device if it's hidden. */
+ BOOLEAN p2sbvisible =
+ *(volatile UINT16 *)MmPciBase(0, 13, 0, 0) != 0xFFFF;
+ if (!p2sbvisible) {
+ *(volatile UINT8 *)MmPciBase(0, 13, 0, 0xE1) = 0;
+ }
+
+ /* Get PMC_BASE from PMC Controller Register. */
+ volatile UINT8 *reg =
+ (volatile UINT8 *)(MmPciBase(0, 13, 1, (UINTN)itco->pmcreg));
+ UINT8 value = *reg;
+ value &= ~PMC_NO_REBOOT_MASK;
+ *reg = value;
+
+ if (p2sbvisible) {
+ *(volatile UINT8 *)MmPciBase(0, 13, 0, 0xE1) = 1;
+ }
+
+ return EFI_SUCCESS;
+}
+
+static EFI_STATUS __attribute__((constructor))
+init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 pci_device_id,
+ UINTN timeout)
+{
+ UINT8 itco_chip;
+ iTCO_info *itco;
+ UINT32 pmbase, tcobase, value;
+ EFI_STATUS status;
+
+ if (!pci_io || pci_vendor_id != PCI_VENDOR_ID_INTEL ||
+ !itco_supported(pci_device_id, &itco_chip)) {
+ return EFI_UNSUPPORTED;
+ }
+ itco = &iTCO_chipset_info[itco_chip];
+
+ Print(L"Detected Intel TCO %s watchdog\n", itco->name);
+
+ /* Get PMBASE and TCOBASE */
+ if ((pmbase = get_pmbase(pci_io, itco)) == 0) {
+ return EFI_UNSUPPORTED;
+ }
+ tcobase = pmbase + 0x60;
+
/* Enable TCO SMIs */
- status = uefi_call_wrapper(pci_io->Io.Read, 6, pci_io,
- EfiPciIoWidthUint32,
- EFI_PCI_IO_PASS_THROUGH_BAR,
- pmbase + SMI_EN_REG, 1, &value);
+ status = uefi_call_wrapper(
+ pci_io->Io.Read, 6, pci_io, EfiPciIoWidthUint32,
+ EFI_PCI_IO_PASS_THROUGH_BAR, pmbase + itco->smireg, 1, &value);
if (EFI_ERROR(status)) {
return status;
}
- value |= TCO_EN;
- status = uefi_call_wrapper(pci_io->Io.Write, 6, pci_io,
- EfiPciIoWidthUint32,
- EFI_PCI_IO_PASS_THROUGH_BAR,
- pmbase + SMI_EN_REG, 1, &value);
+ value |= SMI_TCO_MASK;
+ status = uefi_call_wrapper(
+ pci_io->Io.Write, 6, pci_io, EfiPciIoWidthUint32,
+ EFI_PCI_IO_PASS_THROUGH_BAR, pmbase + itco->smireg, 1, &value);
if (EFI_ERROR(status)) {
return status;
}

/* Set timer value */
- status = uefi_call_wrapper(pci_io->Io.Read, 6, pci_io,
- EfiPciIoWidthUint16,
- EFI_PCI_IO_PASS_THROUGH_BAR,
- tcobase + TCO_TMR_REG, 1, &value);
+ status = uefi_call_wrapper(
+ pci_io->Io.Read, 6, pci_io, EfiPciIoWidthUint16,
+ EFI_PCI_IO_PASS_THROUGH_BAR, tcobase + TCO_TMR_REG, 1, &value);
if (EFI_ERROR(status)) {
return status;
}
value &= 0xfc00;
- value |= timeout & 0x3ff;
- status = uefi_call_wrapper(pci_io->Io.Write, 6, pci_io,
- EfiPciIoWidthUint16,
- EFI_PCI_IO_PASS_THROUGH_BAR,
- tcobase + TCO_TMR_REG, 1, &value);
+ value |= ((timeout * 10) / 6) & 0x3ff;
+ status = uefi_call_wrapper(
+ pci_io->Io.Write, 6, pci_io, EfiPciIoWidthUint16,
+ EFI_PCI_IO_PASS_THROUGH_BAR, tcobase + TCO_TMR_REG, 1, &value);
if (EFI_ERROR(status)) {
return status;
}

/* Force reloading of timer value */
value = 1;
- status = uefi_call_wrapper(pci_io->Io.Write, 6, pci_io,
- EfiPciIoWidthUint16,
- EFI_PCI_IO_PASS_THROUGH_BAR,
- tcobase + TCO_RLD_REG, 1, &value);
+ status = uefi_call_wrapper(
+ pci_io->Io.Write, 6, pci_io, EfiPciIoWidthUint16,
+ EFI_PCI_IO_PASS_THROUGH_BAR, tcobase + TCO_RLD_REG, 1, &value);
if (EFI_ERROR(status)) {
return status;
}

/* Clear NO_REBOOT flag */
- status = uefi_call_wrapper(pci_io->Mem.Read, 6, pci_io,
- EfiPciIoWidthUint32,
- EFI_PCI_IO_PASS_THROUGH_BAR,
- pmcbase + PMC_REG, 1, &value);
- if (EFI_ERROR(status)) {
- return status;
+ switch (itco_chip) {
+ case ITCO_INTEL_APL:
+ status = update_no_reboot_flag_apl(pci_io, itco);
+ break;
+ case ITCO_INTEL_BAYTRAIL:
+ case ITCO_INTEL_WPT_LP:
+ status = update_no_reboot_flag(pci_io, itco);
+ break;
}
- value &= ~PMC_NO_REBOOT;
- status = uefi_call_wrapper(pci_io->Mem.Write, 6, pci_io,
- EfiPciIoWidthUint32,
- EFI_PCI_IO_PASS_THROUGH_BAR,
- pmcbase + PMC_REG, 1, &value);
if (EFI_ERROR(status)) {
return status;
}

/* Clear HLT flag to start timer */
- status = uefi_call_wrapper(pci_io->Io.Read, 6, pci_io,
- EfiPciIoWidthUint16,
- EFI_PCI_IO_PASS_THROUGH_BAR,
- tcobase + TCO1_CNT_REG, 1, &value);
+ status = uefi_call_wrapper(
+ pci_io->Io.Read, 6, pci_io, EfiPciIoWidthUint16,
+ EFI_PCI_IO_PASS_THROUGH_BAR, tcobase + TCO1_CNT_REG, 1, &value);
if (EFI_ERROR(status)) {
return status;
}
- value &= ~TCO_TMR_HLT;
- status = uefi_call_wrapper(pci_io->Io.Write, 6, pci_io,
- EfiPciIoWidthUint16,
- EFI_PCI_IO_PASS_THROUGH_BAR,
- tcobase + TCO1_CNT_REG, 1, &value);
+ value &= ~TCO_TMR_HLT_MASK;
+ status = uefi_call_wrapper(
+ pci_io->Io.Write, 6, pci_io, EfiPciIoWidthUint16,
+ EFI_PCI_IO_PASS_THROUGH_BAR, tcobase + TCO1_CNT_REG, 1, &value);

return status;
}
--
2.23.0

[Jan Kiszka]

On 09.10.19 17:24, [ext] Christian Storm wrote:
> From: Christian Storm <christi...@siemens.com>
>
> Refactoring of the iTCO watchdog support module to make
> integration of further supported iTCOs easier.
>
> Support for the following iTCO watchdogs is added/improved:
> * Apollo Lake for IPC127E
> * ICH9 for QEmu's q35 machine

On which targets were you able to test? Is any case missing?

You probably had some automatic style converter active, but it is not
working consistently (compare here to the call a few lines above). In
any case, let's keep the original style, specifically to avoid
style-only changes below.

I have a couple of style remarkes on this. Let me try to refactor things
to more consistent look&feel. Will post the result.

Thanks,
Jan

--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

[Christian Storm]

Hi Jan,

> > Refactoring of the iTCO watchdog support module to make
> > integration of further supported iTCOs easier.
> >
> > Support for the following iTCO watchdogs is added/improved:
> > * Apollo Lake for IPC127E
> > * ICH9 for QEmu's q35 machine
>
> On which targets were you able to test? Is any case missing?

I did test on QEmu with iTCO and the IPC127E. The other machines
probably need a retest to be on the safe side if staring at code
is not sufficient for QA that is :)

Agreed. I did run a
$ clang-format -i drivers/watchdog/itco.c
using the .clang-format of your commit 29b0191 prior to committing
and did run it just again resulting in no modification to
drivers/watchdog/itco.c. That said, if you do, e.g., a
$ clang-format -i main.c
$ clang-format -i drivers/watchdog/i6300esb.c
...
it does result in formatting changes, i.e., these files are not conforming to
the style embodied in clang-format's formatting configuration.
So, either we should ditch it and (re-)format manually, referring to an
authoritative style guideline, or we should run anything through
clang-format. I don't have a strong opinion here though...

Sure, go ahead.


Kind regards,
Christian

--
Dr. Christian Storm

Siemens AG, Corporate Technology, CT RDA IOT SES-DE

Otto-Hahn-Ring 6, 81739 München, Germany

[mathieu.alexa...@gmail.com]

Hi Christian,

I was just about to send a patch for supporting iTCO v2. I will wait until you push your changes before rebasing my work on this refact. Until then I got some interrogations/questions.

    >+#define PMBASE_ADDRMASK 0x0000ff80
    >+#define PMCBASE_ADDRMASK 0xfffffe00
    > 
    >-#define PMBASE_REG                        0x40
    >-# define PMBASE_ADDRMASK                0xff00

Was this intentional? Changing PMBASE_ADDRMASK from 0xff00 to 0x0000ff80? 

>+typedef struct {
>+        CHAR16 name[16];
>+        UINT32 pci_id;
>+        UINT32 pmbase;
>+        UINT32 pmcbasereg;
>+        UINT32 pmcreg;
>+        UINT32 smireg;
>+} iTCO_info;

This is just a thought here. The iTCO v2 vs v3 uses differents registers.

- PMC_NO_REBOOT : GCS_NO_REBOOT

- PMCBASE_ADDRMASK : RCBABASE_ADDRMASK

- PMCBASE_REG : RCBABASE_REG

- PMC_REG : GCS_REG

For the reboot flag, I would suggest that we add a field no_reboo_flag. For the other three, I am not sure what would be the best between choosing a generic term that would fit both iTCO version or adding the iTCO v2 register names.

>-        value |= timeout & 0x3ff;
>-        status = uefi_call_wrapper(pci_io->Io.Write, 6, pci_io,
>-                                   EfiPciIoWidthUint16,
>-                                   EFI_PCI_IO_PASS_THROUGH_BAR,
>-                                   tcobase + TCO_TMR_REG, 1, &value);
>+        value |= ((timeout * 10) / 6) & 0x3ff;

This should be value |= timeout & 0x3ff;

 

Cheers,

Mathieu

[Christian Storm]

Hi Mathieu,

thanks for doing the review!

> I was just about to send a patch for supporting iTCO v2. I will wait until
> you push your changes before rebasing my work on this refact. Until then I
> got some interrogations/questions.
>
> >+#define PMBASE_ADDRMASK 0x0000ff80
> >+#define PMCBASE_ADDRMASK 0xfffffe00
> >
> >-#define PMBASE_REG 0x40
> >-# define PMBASE_ADDRMASK 0xff00
>
> Was this intentional? Changing PMBASE_ADDRMASK from 0xff00 to 0x0000ff80?

Yes, but I can't remember the exact reason :)
Just speculating, but I think it was because of some bits needed to be
masked for the Apollo Lake and this did also apply to the others as per
spec. But as said, I'm not really sure now, it definitely needs either
a confirmation or revert.

> >+typedef struct {
> > >+ CHAR16 name[16];
> > >+ UINT32 pci_id;
> > >+ UINT32 pmbase;
> > >+ UINT32 pmcbasereg;
> > >+ UINT32 pmcreg;
> > >+ UINT32 smireg;
> > >+} iTCO_info;
> >
>
> This is just a thought here. The iTCO v2 vs v3 uses differents registers.
> - PMC_NO_REBOOT : GCS_NO_REBOOT
> - PMCBASE_ADDRMASK : RCBABASE_ADDRMASK
> - PMCBASE_REG : RCBABASE_REG
> - PMC_REG : GCS_REG
>
> For the reboot flag, I would suggest that we add a field no_reboo_flag. For
> the other three, I am not sure what would be the best between choosing a
> generic term that would fit both iTCO version or adding the iTCO v2
> register names.

Hm, the NO_REBOOT handling has specific implementations, as you can see
in the switch/case routing to update_no_reboot_flag{,_apl}(). Instead of
the switch/case, you may, e.g., add a field holding the respective
function pointers. It's not a change in semantics but more a change in
style then. I opted for the switch/case since it's more explicit but
I don't have strong opinions about this...

For the struct iTCO_info naming, I would prefer to try to use generic
terms that fit all iTCOs. The concrete values are then set in struct
iTCO_info iTCO_chipset_info[] for the respective chipsets. That said,
we may rename them to, e.g., get rid of the pmc prefix though.
Adding the specifics of the iTCO v2 register names and having to do
a special implementation for this may end up qualifying as another
watchdog driver module particularly for iTCO v2 if there are not enough
synergies with the other iTCO support implementations to justify keeping
them all in one module.

> > >- value |= timeout & 0x3ff;
> > >- status = uefi_call_wrapper(pci_io->Io.Write, 6, pci_io,
> > >- EfiPciIoWidthUint16,
> > >- EFI_PCI_IO_PASS_THROUGH_BAR,
> > >- tcobase + TCO_TMR_REG, 1, &value);
> > >+ value |= ((timeout * 10) / 6) & 0x3ff;
> >
> This should be value |= timeout & 0x3ff;

True, I did confuse the patch while rebasing on yours, good spotting!
Will make it into a V2 of the patch I guess...

[Jan Kiszka]

From: Christian Storm <christi...@siemens.com>

Refactoring of the iTCO watchdog support module to make
integration of further supported iTCOs easier.

Support for the following iTCO watchdogs is added/improved:
* Apollo Lake for IPC127E
* ICH9 for QEmu's q35 machine

Signed-off-by: Christian Storm <christi...@siemens.com>
[Jan: do not touch timeout settings, style cleanups]
Signed-off-by: Jan Kiszka <jan.k...@siemens.com>

---

As Christian is not in the office the upcoming weeks, I picked this up,
hopefully without causing regressions. Review / testing would be
appreciated.

drivers/watchdog/itco.c | 230 +++++++++++++++++++++++++++++++++++++-----------
1 file changed, 178 insertions(+), 52 deletions(-)

diff --git a/drivers/watchdog/itco.c b/drivers/watchdog/itco.c
index 8346f36..0f7294d 100644

--- a/drivers/watchdog/itco.c
+++ b/drivers/watchdog/itco.c
@@ -1,10 +1,11 @@
/*
* EFI Boot Guard, iTCO support (Version 3 and later)
*
- * Copyright (c) Siemens AG, 2017
+ * Copyright (c) Siemens AG, 2019
*
* Authors:
* Jan Kiszka <jan.k...@siemens.com>
+ * Christian Storm <christi...@siemens.com>
*
* This work is licensed under the terms of the GNU GPL, version 2. See
* the COPYING file in the top-level directory.

@@ -16,74 +17,202 @@

#include <efilib.h>
#include <pci/header.h>

-#define PCI_DEVICE_ID_INTEL_BAYTRAIL 0x0f1c
-#define PCI_DEVICE_ID_INTEL_WPT_LP 0x9cc3
-#define PCI_DEVICE_ID_INTEL_APL 0x5ae8

+#define PMBASE_ADDRMASK 0x0000ff80
+#define PMCBASE_ADDRMASK 0xfffffe00

-#define PMBASE_REG 0x40
-# define PMBASE_ADDRMASK 0xff00

+typedef struct {
+ CHAR16 name[16];
+ UINT32 pci_id;
+ UINT32 pmbase;
+ UINT32 pmcbasereg;
+ UINT32 pmcreg;
+ UINT32 smireg;
+} iTCO_info;

+{

+ for (UINT8 i = 0;
+ i < (sizeof(iTCO_chipset_info) / sizeof(iTCO_chipset_info[0]));
+ i++) {
+ if (pci_device_id == iTCO_chipset_info[i].pci_id) {
+ *index = i;
+ return TRUE;
+ }
+ }
+ return FALSE;
+}
+
+static UINT32 get_pmbase(EFI_PCI_IO *pci_io, iTCO_info *itco)
{
- UINT32 pmbase, tcobase, pmcbase, value;

+ status = uefi_call_wrapper(pci_io->Mem.Read, 6, pci_io,
+ EfiPciIoWidthUint32,
+ EFI_PCI_IO_PASS_THROUGH_BAR,
+ pmcbase + itco->pmcreg, 1, &value);

+ if (EFI_ERROR(status)) {
+ return status;
+ }

+ value &= ~PMC_NO_REBOOT_MASK;
+ status = uefi_call_wrapper(pci_io->Mem.Write, 6, pci_io,
+ EfiPciIoWidthUint32,
+ EFI_PCI_IO_PASS_THROUGH_BAR,
+ pmcbase + itco->pmcreg, 1, &value);

+ if (EFI_ERROR(status)) {
+ return status;
+ }
+ return status;
+}
+

+#define APL_MMCFG_BASE 0xE0000000
+
+static UINTN apl_mmcfg_address(UINTN bus, UINTN device, UINTN function,
+ UINTN offset)
+{
+ return APL_MMCFG_BASE + (bus << 20) + (device << 15) +
+ (function << 12) + offset;

+}
+
+static EFI_STATUS update_no_reboot_flag_apl(__attribute__((unused))
+ EFI_PCI_IO *pci_io,
+ iTCO_info *itco)
+{

+ /* Unhide the P2SB device if it's hidden. */
+ BOOLEAN p2sbvisible =

+ *(volatile UINT16 *)apl_mmcfg_address(0, 13, 0, 0) != 0xFFFF;
+ if (!p2sbvisible) {
+ *(volatile UINT8 *)apl_mmcfg_address(0, 13, 0, 0xE1) = 0;

+ }
+
+ /* Get PMC_BASE from PMC Controller Register. */
+ volatile UINT8 *reg =

+ (volatile UINT8 *)apl_mmcfg_address(0, 13, 1, (UINTN)itco->pmcreg);

+ UINT8 value = *reg;
+ value &= ~PMC_NO_REBOOT_MASK;
+ *reg = value;
+
+ if (p2sbvisible) {

+ *(volatile UINT8 *)apl_mmcfg_address(0, 13, 0, 0xE1) = 1;

status = uefi_call_wrapper(pci_io->Io.Read, 6, pci_io,

EfiPciIoWidthUint32,

EFI_PCI_IO_PASS_THROUGH_BAR,
- pmbase + SMI_EN_REG, 1, &value);

+ pmbase + itco->smireg, 1, &value);

if (EFI_ERROR(status)) {
return status;
}
- value |= TCO_EN;

+ value |= SMI_TCO_MASK;

status = uefi_call_wrapper(pci_io->Io.Write, 6, pci_io,

EfiPciIoWidthUint32,

EFI_PCI_IO_PASS_THROUGH_BAR,
- pmbase + SMI_EN_REG, 1, &value);

+ pmbase + itco->smireg, 1, &value);
if (EFI_ERROR(status)) {
return status;
}
@@ -117,18 +246,15 @@ init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 pci_device_id,

}

/* Clear NO_REBOOT flag */
- status = uefi_call_wrapper(pci_io->Mem.Read, 6, pci_io,
- EfiPciIoWidthUint32,
- EFI_PCI_IO_PASS_THROUGH_BAR,
- pmcbase + PMC_REG, 1, &value);
- if (EFI_ERROR(status)) {
- return status;
+ switch (itco_chip) {
+ case ITCO_INTEL_APL:
+ status = update_no_reboot_flag_apl(pci_io, itco);
+ break;
+ case ITCO_INTEL_BAYTRAIL:
+ case ITCO_INTEL_WPT_LP:
+ status = update_no_reboot_flag(pci_io, itco);
+ break;
}
- value &= ~PMC_NO_REBOOT;
- status = uefi_call_wrapper(pci_io->Mem.Write, 6, pci_io,
- EfiPciIoWidthUint32,
- EFI_PCI_IO_PASS_THROUGH_BAR,
- pmcbase + PMC_REG, 1, &value);
if (EFI_ERROR(status)) {
return status;
}

@@ -141,7 +267,7 @@ init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 pci_device_id,

if (EFI_ERROR(status)) {
return status;
}
- value &= ~TCO_TMR_HLT;

+ value &= ~TCO_TMR_HLT_MASK;

status = uefi_call_wrapper(pci_io->Io.Write, 6, pci_io,

EfiPciIoWidthUint16,
EFI_PCI_IO_PASS_THROUGH_BAR,
--
2.16.4


--

Siemens AG, Corporate Technology, CT RDA IOT SES-DE

[Mathieu Alexandre-Tétreault]

Seems fine to me. What have being tested so far?

Cheers,

Mathieu

[Jan Kiszka]

On 16.10.19 14:45, Mathieu Alexandre-Tétreault wrote:
> Seems fine to me. What have being tested so far?
>

My version: untested so far. I still need to find some time to try it
out. Christian ran his on Apollo Lake and in QEMU.

Jan

[Jan Kiszka]

On 16.10.19 18:22, Jan Kiszka wrote:
> On 16.10.19 14:45, Mathieu Alexandre-Tétreault wrote:
>> Seems fine to me. What have being tested so far?
>>
>
> My version: untested so far. I still need to find some time to try it
> out. Christian ran his on Apollo Lake and in QEMU.
>

Meanwhile I tested my patch on an Apollo Lake, and it works fine there.