On 19/10/2023 10:17, Kiszka, Jan (T CED) wrote: > On 19.10.23 09:31, 'Cedric Hombourger' via EFI Boot Guard wrote: >> Re-use the w83627hf code from the Linux kernel to support the >> NCT6116 chip found on the SIMATIC IPC BX-59A. The code was >> greatly simplified to only support that particular chip. >> >> The original intent was to use WDAT for this platform but it >> turned out to be impossible because it would require holding >> precious shared I/O registers. >> >> See >> https://lore.kernel.org/all/df8d53db-0056-434d-953b-991025e6c...@roeck-us.net/ >> >> Signed-off-by: Cedric Hombourger <cedric.hombour...@siemens.com> >> --- >> Makefile.am | 1 + >> README.md | 1 + >> drivers/watchdog/ipcbx59a_wdt.c | 226 ++++++++++++++++++++++++++++++++ >> include/simatic.h | 5 +- >> 4 files changed, 231 insertions(+), 2 deletions(-) >> create mode 100644 drivers/watchdog/ipcbx59a_wdt.c >> >> diff --git a/Makefile.am b/Makefile.am >> index 6d4c160..ef9df19 100644 >> --- a/Makefile.am >> +++ b/Makefile.am >> @@ -171,6 +171,7 @@ efi_sources_watchdogs = \ >> drivers/watchdog/i6300esb.c \ >> drivers/watchdog/atom-quark.c \ >> drivers/watchdog/ipc4x7e_wdt.c \ >> + drivers/watchdog/ipcbx59a_wdt.c \ >> drivers/watchdog/ipmi_wdt.c \ >> drivers/watchdog/itco.c \ >> drivers/watchdog/hpwdt.c >> diff --git a/README.md b/README.md >> index c6d13fd..ef8a2fe 100644 >> --- a/README.md >> +++ b/README.md >> @@ -33,6 +33,7 @@ The following watchdog drivers are implemented (and are >> probed in this order): >> * Intel i6300esb >> * Intel Quark >> * Siemens SIMATIC IPC4x7E >> +* Siemens SIMATIC BX-59A >> * Intel TCO >> * HPE ProLiant >> >> diff --git a/drivers/watchdog/ipcbx59a_wdt.c >> b/drivers/watchdog/ipcbx59a_wdt.c >> new file mode 100644 >> index 0000000..8163099 >> --- /dev/null >> +++ b/drivers/watchdog/ipcbx59a_wdt.c >> @@ -0,0 +1,226 @@ >> +/* >> + * EFI Boot Guard >> + * >> + * Modified version of the w83627hf_wdt.c driver found in the Linux kernel >> + * >> + * (c) Copyright 2013 Guenter Roeck >> + * converted to watchdog infrastructure >> + * >> + * (c) Copyright 2007 Vlad Drukker <v...@storewiz.com> >> + * added support for W83627THF. >> + * >> + * (c) Copyright 2003,2007 Pádraig Brady <p...@draigbrady.com> >> + * >> + * Based on advantechwdt.c which is based on wdt.c. >> + * Original copyright messages: >> + * >> + * (c) Copyright 2000-2001 Marek Michalkiewicz <mar...@linux.org.pl> >> + * >> + * (c) Copyright 1996 Alan Cox <a...@lxorguk.ukuu.org.uk>, >> + * All Rights Reserved. >> + * >> + * Neither Alan Cox nor CymruNet Ltd. admit liability nor provide >> + * warranty for any of this software. This material is provided >> + * "AS-IS" and at no charge. >> + * >> + * (c) Copyright 1995 Alan Cox <a...@lxorguk.ukuu.org.uk> >> + * >> + * Changes and EFI Boot Guard specific code: >> + * >> + * by Cedric Hombourger <cedric.hombour...@siemens.com> >> + * (c) Copyright (c) Siemens AG, 2023 >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2. See >> + * the COPYING file in the top-level directory. >> + * >> + * SPDX-License-Identifier: GPL-2.0 >> + */ >> + >> +#include <efi.h> >> +#include <efilib.h> >> +#include <pci/header.h> >> +#include <sys/io.h> >> +#include <mmio.h> >> +#include "simatic.h" >> +#include "utils.h" >> + >> +/* Use the host bridge device found on the BX-59A to limit probing to Intel >> + * based machines that may be a BX59A; technically we do not use/need PCI >> + * for this driver but only port I/Os */ >> +#define PCI_DEVICE_ID_INTEL_HOST_BRIDGE 0xa700 >> + >> +#define WDT_EFER (wdt_io+0) /* Extended Function Enable Registers */ >> +#define WDT_EFIR (wdt_io+0) /* Extended Function Index Register >> + (same as EFER) */ >> +#define WDT_EFDR (WDT_EFIR+1) /* Extended Function Data Register */ >> + >> +#define W83627HF_LD_WDT 0x08 >> + >> +#define NCT6116_ID 0xd2 >> + >> +#define NCT6102D_WDT_TIMEOUT 0xf1 >> +#define NCT6102D_WDT_CONTROL 0xf0 >> +#define NCT6102D_WDT_CSR 0xf2 >> + >> +#define WDT_CSR_STATUS 0x10 >> +#define WDT_CSR_KBD 0x40 >> +#define WDT_CSR_MOUSE 0x80 >> + >> +enum chips { nct6116 }; >> + >> +static int wdt_io; >> +static int cr_wdt_timeout; /* WDT timeout register */ >> +static int cr_wdt_control; /* WDT control register */ >> +static int cr_wdt_csr; /* WDT control & status register */ >> +static int wdt_cfg_enter = 0x87;/* key to unlock configuration space */ >> +static int wdt_cfg_leave = 0xAA;/* key to lock configuration space */ >> + >> +static void superio_outb(int reg, int val) >> +{ >> + outb(reg, WDT_EFER); >> + outb(val, WDT_EFDR); >> +} >> + >> +static inline int superio_inb(int reg) >> +{ >> + outb(reg, WDT_EFER); >> + return inb(WDT_EFDR); >> +} >> + >> +static int superio_enter(void) >> +{ >> + outb_p(wdt_cfg_enter, WDT_EFER); /* Enter extended function mode */ >> + outb_p(wdt_cfg_enter, WDT_EFER); /* Again according to manual */ >> + return 0; >> +} >> + >> +static void superio_select(int ld) >> +{ >> + superio_outb(0x07, ld); >> +} >> + >> +static void superio_exit(void) >> +{ >> + outb_p(wdt_cfg_leave, WDT_EFER); /* Leave extended function mode */ >> +} >> + >> +static int wdt_find(int addr) >> +{ >> + UINT8 val; >> + int ret; >> + >> + wdt_io = addr; >> + ret = superio_enter(); >> + if (ret) >> + return ret; >> + superio_select(W83627HF_LD_WDT); >> + val = superio_inb(0x20); >> + switch(val) { >> + case NCT6116_ID: >> + ret = nct6116; >> + cr_wdt_timeout = NCT6102D_WDT_TIMEOUT; >> + cr_wdt_control = NCT6102D_WDT_CONTROL; >> + cr_wdt_csr = NCT6102D_WDT_CSR; >> + break; >> + default: >> + ret = -1; >> + break; >> + } >> + superio_exit(); >> + return ret; >> +} >> + >> +static int w83627hf_init(enum chips chip) >> +{ >> + int ret; >> + unsigned char t; >> + >> + ret = superio_enter(); >> + if (ret) >> + return ret; >> + >> + superio_select(W83627HF_LD_WDT); >> + >> + /* set CR30 bit 0 to activate GPIO2 */ >> + t = superio_inb(0x30); >> + if (!(t & 0x01)) >> + superio_outb(0x30, t | 0x01); >> + >> + switch (chip) { >> + case nct6116: >> + /* >> + * These chips have a fixed WDTO# output pin (W83627UHG), >> + * or support more than one WDTO# output pin. >> + * Don't touch its configuration, and hope the BIOS >> + * does the right thing. >> + */ >> + t = superio_inb(cr_wdt_control); >> + t |= 0x02; /* enable the WDTO# output low pulse >> + * to the KBRST# pin */ >> + superio_outb(cr_wdt_control, t); >> + break; >> + default: >> + break; >> + } >> + >> + t = superio_inb(cr_wdt_timeout); >> + if (t != 0) { >> + WARNING(L"Watchdog already running.\n"); >> + } >> + >> + /* set second mode & disable keyboard turning off watchdog */ >> + t = superio_inb(cr_wdt_control) & ~0x0C; >> + superio_outb(cr_wdt_control, t); >> + >> + /* reset status, disable keyboard & mouse turning off watchdog */ >> + t = superio_inb(cr_wdt_csr); >> + t &= ~(WDT_CSR_STATUS | WDT_CSR_KBD | WDT_CSR_MOUSE); >> + superio_outb(cr_wdt_csr, t); >> + >> + superio_exit(); >> + >> + return 0; >> +} >> + >> +static int wdt_set_time(unsigned int timeout) >> +{ >> + int ret; >> + >> + ret = superio_enter(); >> + if (ret) >> + return ret; >> + >> + superio_select(W83627HF_LD_WDT); >> + superio_outb(cr_wdt_timeout, timeout); >> + superio_exit(); >> + >> + return 0; >> +} >> + >> +static EFI_STATUS __attribute__((constructor)) >> +init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 pci_device_id, >> + UINTN timeout) >> +{ >> + int chip, ret; >> + >> + if (!pci_io || pci_vendor_id != PCI_VENDOR_ID_INTEL || >> + pci_device_id != PCI_DEVICE_ID_INTEL_HOST_BRIDGE) { >> + return EFI_UNSUPPORTED; >> + } >> + >> + switch (simatic_station_id()) { >> + case SIMATIC_IPCBX_59A: >> + chip = wdt_find(0x2e); >> + if (chip < 0) >> + return EFI_UNSUPPORTED; >> + INFO(L"Detected SIMATIC BX59A watchdog\n"); >> + ret = w83627hf_init(chip); >> + if (ret < 0) >> + return EFI_UNSUPPORTED; >> + ret = wdt_set_time(timeout); >> + if (ret < 0) >> + return EFI_UNSUPPORTED; >> + return EFI_SUCCESS; >> + } >> + return EFI_UNSUPPORTED; >> +} >> diff --git a/include/simatic.h b/include/simatic.h >> index 33429c5..724e79d 100644 >> --- a/include/simatic.h >> +++ b/include/simatic.h >> @@ -23,8 +23,9 @@ >> >> #define SIMATIC_OEM_ENTRY_TYPE_BINARY 0xff >> >> -#define SIMATIC_IPC427E 0xa01 >> -#define SIMATIC_IPC477E 0xa02 >> +#define SIMATIC_IPC427E 0x0a01 >> +#define SIMATIC_IPC477E 0x0a02 >> +#define SIMATIC_IPCBX_59A 0x1202 >> >> typedef struct { >> UINT8 type; > In Linux, we use the pre-existing w83627hf_wdt unmodified for this > purpose. Wouldn't it make sense to at least name this driver here > generically so that we could add support for other systems using it > later on without much hassle? I was debating with myself on that. I'm happy either way. Would we just rename or are you suggesting we somehow get the station id check out of w83627hf_wdt.c (and e.g. have ipcbx59a.c call into w83627hf_wdt.c)? > Jan >
-- You received this message because you are subscribed to the Google Groups "EFI Boot Guard" group. To unsubscribe from this group and stop receiving emails from it, send an email to efibootguard-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/efibootguard-dev/2d886df2-363c-4f6c-9e5a-0b120aed4fdf%40siemens.com.
