[PATCH v5 0/9] watchdog/hpwdt: Update driver to use watchdog core.

[PATCH v5 0/9] watchdog/hpwdt: Update driver to use watchdog core.

[Jerry Hoemann]

== v5 ==

Patch numbers changing due to squash and reorder.
Using Prior (v4) Patch number noting *new* patch numbers as bullet item.


Patch 0001 & 0004 Merged into: Update-Module-info-and-copyright
1) New Patch 0001

Patch 0002 Remove legacy NMI sourcing
1) Unmodified

Patch 0003 Update nmi_panic message.
1) Unmodified

Patch 0004 Update Module info.
1) Merged into 0001 above.

Patch 0005 Modify to use watchdog core
1) syntatic sugar.
2) Now Patch 0004

Patch 0006 condition-early-return-of-NMI-handler
1) Now Patch 0005
2) Casing on iLO5
3) true for boolean

Patch 0007 remove allow_kdump module param
1) Now patch 0006

Patch 0008 Programable Pretimeout NMI
1) Now patch 00007

Patch 0009 Add dynamic debug
1) Now patch 0008

Patch 0010 Update driver version.
1) Now Patch 0009


== v4 ==

Modifications from prior version:

Manual merging and patch reordering has cause minor white
space and diff changes.  Such change shouldn't have effected
the logic and are omitted below:

Patch 001  Remove legacy NMI sourcing.
1) Updated patch documentation to reflect Spectre concerns.
2) Restored comment at #endif
>>>>>>>>> 3) New Patch 0002  <<<<<<<<

Patch 002 watchdog/hpwdt: remove include files no longer
1) Squashed onto Patch001.
>>>>>>>>> 3) Still Patch 0002  <<<<<<<<


Patch 0003: watchdog/hpwdt: Update nmi_panic message.
1) Unchanged


Patch 0004:  watchdog/hpwdt: white space changes
1) Redacted.

Patch 0005: watchdog/hpwdt: Update Module info.
1) Now Patch 0004
2) Update only module description to reflect branding changes.
3) Rest of patch redacted.

Patch 0006 Select CORE
1) Merged with 0007

Patch 0007 Modify to use watchdog core
1) squashed on Patch 0006.
2) Now Patch 0005
3) Requested white space changes.
4) Use "wdd" for watchdog_device like other watchdog drivers do

Patch 0008 Programable-Pretimeout
1) hpwdt_settimeout re-worked to take into account timeout <= pretimeout.
   Patterned after watchdog_set_timeout.
2) hpwdt_set_pretimeout re-worked to take into account requested
   pretimeout <= timeout.
3) Clarify patch doc to reflect above.

Patch 0009 condition-early-return-of-NMI
1) Requested syntatic surgar.
2) Now Patch 0006

Patch 0010 remove-allow_kdump
1) Now Patch 0007

Patch 0011 Add-dynamic-debug
1) Now Patch 0009
2) Typo fix
3) New dev_dbg resulting from changes to set timeout and pretimeout.

Patch 0012 Update driver version.
1) Now Patch 0010

 
New Patch
1) Patch 0001 Update Copyright



== v3 ==

Incorperating code review feedback.

1) Patch 0003: Use existing hex_byte_pack instead of creating new function.
2) Patch 0005: Redacted change in module_param permission.
3) Patch 0006: switch from pr_debug etc., to dev_dbg where possible.
4) Patch 0006: No longer updating soft_margin post module load.
5) Patch 0006: Initialize hpwdt_dev.parent before registering watchdog.
6) Patch 0006: Redacted change to dev_info message w.r.t. allow_kdump
7) Patch 0006 & 0007: Reorder patches to maintain bisectability.
8) Patch 0008: Change pr_debug to dev_dbg
9) Patch 0010: Change dev_info message w.r.t. allow_kdump where feature
		is removed.

Note, I am explicitly ignoring the checkpatch error on Patch 0008
for specifying permisson of "0" instead of "0000".

== v2 ==

1) Fix compiler error when CONFIG_HPWDT_NMI_DECODING is not defined.

2) Break out driver version change to its own patch (0011).


== v1 ==

The primary purposes of this patch set are to

1) Update the hpwdt driver to use the watchdog core.
2) Reduce complexity by removing unnecessary features.
3) Add customer requested features like optional pretimeout.
4) Enhance readability/maintainability of the driver.

The size of the resultant driver is reduced from over 900
lines to 350 lines.

Patch 1& 2 remove legacy NMI sourcing.
Patch 3	adds useful indication of NMI cause to panic message
Patch 4 & 5 are general cleanup
Patch 6 & 7 updates the driver to user the watchdog core.
Patch 8 makes the pretimeout NMI programmable.
Patch 9 modifies whether the NMI handler claims the NMI.
Patch 10 retracts the allow_kdump module parameter.



Jerry Hoemann (9):
  watchdog/hpwdt: Update Module info and copyright.
  watchdog/hpwdt: Remove legacy NMI sourcing.
  watchdog/hpwdt: Update nmi_panic message.
  watchdog/hpwdt: Modify to use watchdog core.
  watchdog/hpwdt: condition early return of NMI handler on iLO5
  watchdog/hpwdt: remove allow_kdump module parameter.
  watchdog/hpwdt: Programable Pretimeout NMI
  watchdog/hpwdt: Add dynamic debug
  watchdog/hpwdt: Update driver version.

 drivers/watchdog/Kconfig |   1 +
 drivers/watchdog/hpwdt.c | 791 +++++++----------------------------------------
 2 files changed, 121 insertions(+), 671 deletions(-)

-- 
2.13.6

[PATCH v5 1/9] watchdog/hpwdt: Update Module info and copyright.

[Jerry Hoemann]

Update Copyright and Module description to reflect branding changes.

Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
 drivers/watchdog/hpwdt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index f1f00dfc0e68..542dfc598716 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -4,7 +4,7 @@
  *
  *	SoftDog	0.05:	A Software Watchdog Device
  *
- *	(c) Copyright 2015 Hewlett Packard Enterprise Development LP
+ *	(c) Copyright 2018 Hewlett Packard Enterprise Development LP
  *	Thomas Mingarelli <thomas.mingarelli@hpe.com>
  *
  *	This program is free software; you can redistribute it and/or
@@ -908,7 +908,7 @@ static struct pci_driver hpwdt_driver = {
 };
 
 MODULE_AUTHOR("Tom Mingarelli");
-MODULE_DESCRIPTION("hp watchdog driver");
+MODULE_DESCRIPTION("hpe watchdog driver");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(HPWDT_VERSION);
 
-- 
2.13.6

[PATCH v5 2/9] watchdog/hpwdt: Remove legacy NMI sourcing.

[Jerry Hoemann]

Gen8 and prior Proliant systems supported the "CRU" interface
to firmware.  This interfaces allows linux to "call back" into firmware
to source the cause of an NMI.  This feature isn't fully utilized
as the actual source of the NMI isn't printed, the driver only
indicates that the source couldn't be determined when the call
fails.

With the advent of Gen9, iCRU replaces the CRU. The call back
feature is no longer available in firmware.  To be compatible and
not attempt to call back into firmware on system not supporting CRU,
the SMBIOS table is consulted to determine if it is safe to
make the call back or not.

This results in about half of the driver code being devoted
to either making CRU calls or determing if it is safe to make
CRU calls.  As noted, the driver isn't really using the results of
the CRU calls.

Furthermore, as a consequence of the Spectre security issue, the
BIOS/EFI calls are being wrapped into Spectre-disabling section.
Removing the call back in hpwdt_pretimeout assists in this effort.

As the CRU sourcing of the NMI isn't required for handling the
NMI and there are security concerns with making the call back, remove
the legacy (pre Gen9) NMI sourcing and the DMI code to determine if
the system had the CRU interface.

Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
 drivers/watchdog/hpwdt.c | 501 +----------------------------------------------
 1 file changed, 9 insertions(+), 492 deletions(-)

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 542dfc598716..1d20bc6d2c44 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -28,16 +28,7 @@
 #include <linux/types.h>
 #include <linux/uaccess.h>
 #include <linux/watchdog.h>
-#ifdef CONFIG_HPWDT_NMI_DECODING
-#include <linux/dmi.h>
-#include <linux/spinlock.h>
-#include <linux/nmi.h>
-#include <linux/kdebug.h>
-#include <linux/notifier.h>
-#include <asm/set_memory.h>
-#endif /* CONFIG_HPWDT_NMI_DECODING */
 #include <asm/nmi.h>
-#include <asm/frame.h>
 
 #define HPWDT_VERSION			"1.4.0"
 #define SECS_TO_TICKS(secs)		((secs) * 1000 / 128)
@@ -48,6 +39,9 @@
 static unsigned int soft_margin = DEFAULT_MARGIN;	/* in seconds */
 static unsigned int reload;			/* the computed soft_margin */
 static bool nowayout = WATCHDOG_NOWAYOUT;
+#ifdef CONFIG_HPWDT_NMI_DECODING
+static unsigned int allow_kdump = 1;
+#endif
 static char expect_release;
 static unsigned long hpwdt_is_open;
 
@@ -63,373 +57,6 @@ static const struct pci_device_id hpwdt_devices[] = {
 };
 MODULE_DEVICE_TABLE(pci, hpwdt_devices);
 
-#ifdef CONFIG_HPWDT_NMI_DECODING
-#define PCI_BIOS32_SD_VALUE		0x5F32335F	/* "_32_" */
-#define CRU_BIOS_SIGNATURE_VALUE	0x55524324
-#define PCI_BIOS32_PARAGRAPH_LEN	16
-#define PCI_ROM_BASE1			0x000F0000
-#define ROM_SIZE			0x10000
-
-struct bios32_service_dir {
-	u32 signature;
-	u32 entry_point;
-	u8 revision;
-	u8 length;
-	u8 checksum;
-	u8 reserved[5];
-};
-
-/* type 212 */
-struct smbios_cru64_info {
-	u8 type;
-	u8 byte_length;
-	u16 handle;
-	u32 signature;
-	u64 physical_address;
-	u32 double_length;
-	u32 double_offset;
-};
-#define SMBIOS_CRU64_INFORMATION	212
-
-/* type 219 */
-struct smbios_proliant_info {
-	u8 type;
-	u8 byte_length;
-	u16 handle;
-	u32 power_features;
-	u32 omega_features;
-	u32 reserved;
-	u32 misc_features;
-};
-#define SMBIOS_ICRU_INFORMATION		219
-
-
-struct cmn_registers {
-	union {
-		struct {
-			u8 ral;
-			u8 rah;
-			u16 rea2;
-		};
-		u32 reax;
-	} u1;
-	union {
-		struct {
-			u8 rbl;
-			u8 rbh;
-			u8 reb2l;
-			u8 reb2h;
-		};
-		u32 rebx;
-	} u2;
-	union {
-		struct {
-			u8 rcl;
-			u8 rch;
-			u16 rec2;
-		};
-		u32 recx;
-	} u3;
-	union {
-		struct {
-			u8 rdl;
-			u8 rdh;
-			u16 red2;
-		};
-		u32 redx;
-	} u4;
-
-	u32 resi;
-	u32 redi;
-	u16 rds;
-	u16 res;
-	u32 reflags;
-}  __attribute__((packed));
-
-static unsigned int hpwdt_nmi_decoding;
-static unsigned int allow_kdump = 1;
-static unsigned int is_icru;
-static unsigned int is_uefi;
-static DEFINE_SPINLOCK(rom_lock);
-static void *cru_rom_addr;
-static struct cmn_registers cmn_regs;
-
-extern asmlinkage void asminline_call(struct cmn_registers *pi86Regs,
-						unsigned long *pRomEntry);
-
-#ifdef CONFIG_X86_32
-/* --32 Bit Bios------------------------------------------------------------ */
-
-#define HPWDT_ARCH	32
-
-asm(".text                          \n\t"
-    ".align 4                       \n\t"
-    ".globl asminline_call	    \n"
-    "asminline_call:                \n\t"
-    "pushl       %ebp               \n\t"
-    "movl        %esp, %ebp         \n\t"
-    "pusha                          \n\t"
-    "pushf                          \n\t"
-    "push        %es                \n\t"
-    "push        %ds                \n\t"
-    "pop         %es                \n\t"
-    "movl        8(%ebp),%eax       \n\t"
-    "movl        4(%eax),%ebx       \n\t"
-    "movl        8(%eax),%ecx       \n\t"
-    "movl        12(%eax),%edx      \n\t"
-    "movl        16(%eax),%esi      \n\t"
-    "movl        20(%eax),%edi      \n\t"
-    "movl        (%eax),%eax        \n\t"
-    "push        %cs                \n\t"
-    "call        *12(%ebp)          \n\t"
-    "pushf                          \n\t"
-    "pushl       %eax               \n\t"
-    "movl        8(%ebp),%eax       \n\t"
-    "movl        %ebx,4(%eax)       \n\t"
-    "movl        %ecx,8(%eax)       \n\t"
-    "movl        %edx,12(%eax)      \n\t"
-    "movl        %esi,16(%eax)      \n\t"
-    "movl        %edi,20(%eax)      \n\t"
-    "movw        %ds,24(%eax)       \n\t"
-    "movw        %es,26(%eax)       \n\t"
-    "popl        %ebx               \n\t"
-    "movl        %ebx,(%eax)        \n\t"
-    "popl        %ebx               \n\t"
-    "movl        %ebx,28(%eax)      \n\t"
-    "pop         %es                \n\t"
-    "popf                           \n\t"
-    "popa                           \n\t"
-    "leave                          \n\t"
-    "ret                            \n\t"
-    ".previous");
-
-
-/*
- *	cru_detect
- *
- *	Routine Description:
- *	This function uses the 32-bit BIOS Service Directory record to
- *	search for a $CRU record.
- *
- *	Return Value:
- *	0        :  SUCCESS
- *	<0       :  FAILURE
- */
-static int cru_detect(unsigned long map_entry,
-	unsigned long map_offset)
-{
-	void *bios32_map;
-	unsigned long *bios32_entrypoint;
-	unsigned long cru_physical_address;
-	unsigned long cru_length;
-	unsigned long physical_bios_base = 0;
-	unsigned long physical_bios_offset = 0;
-	int retval = -ENODEV;
-
-	bios32_map = ioremap(map_entry, (2 * PAGE_SIZE));
-
-	if (bios32_map == NULL)
-		return -ENODEV;
-
-	bios32_entrypoint = bios32_map + map_offset;
-
-	cmn_regs.u1.reax = CRU_BIOS_SIGNATURE_VALUE;
-
-	set_memory_x((unsigned long)bios32_map, 2);
-	asminline_call(&cmn_regs, bios32_entrypoint);
-
-	if (cmn_regs.u1.ral != 0) {
-		pr_warn("Call succeeded but with an error: 0x%x\n",
-			cmn_regs.u1.ral);
-	} else {
-		physical_bios_base = cmn_regs.u2.rebx;
-		physical_bios_offset = cmn_regs.u4.redx;
-		cru_length = cmn_regs.u3.recx;
-		cru_physical_address =
-			physical_bios_base + physical_bios_offset;
-
-		/* If the values look OK, then map it in. */
-		if ((physical_bios_base + physical_bios_offset)) {
-			cru_rom_addr =
-				ioremap(cru_physical_address, cru_length);
-			if (cru_rom_addr) {
-				set_memory_x((unsigned long)cru_rom_addr & PAGE_MASK,
-					(cru_length + PAGE_SIZE - 1) >> PAGE_SHIFT);
-				retval = 0;
-			}
-		}
-
-		pr_debug("CRU Base Address:   0x%lx\n", physical_bios_base);
-		pr_debug("CRU Offset Address: 0x%lx\n", physical_bios_offset);
-		pr_debug("CRU Length:         0x%lx\n", cru_length);
-		pr_debug("CRU Mapped Address: %p\n", &cru_rom_addr);
-	}
-	iounmap(bios32_map);
-	return retval;
-}
-
-/*
- *	bios_checksum
- */
-static int bios_checksum(const char __iomem *ptr, int len)
-{
-	char sum = 0;
-	int i;
-
-	/*
-	 * calculate checksum of size bytes. This should add up
-	 * to zero if we have a valid header.
-	 */
-	for (i = 0; i < len; i++)
-		sum += ptr[i];
-
-	return ((sum == 0) && (len > 0));
-}
-
-/*
- *	bios32_present
- *
- *	Routine Description:
- *	This function finds the 32-bit BIOS Service Directory
- *
- *	Return Value:
- *	0        :  SUCCESS
- *	<0       :  FAILURE
- */
-static int bios32_present(const char __iomem *p)
-{
-	struct bios32_service_dir *bios_32_ptr;
-	int length;
-	unsigned long map_entry, map_offset;
-
-	bios_32_ptr = (struct bios32_service_dir *) p;
-
-	/*
-	 * Search for signature by checking equal to the swizzled value
-	 * instead of calling another routine to perform a strcmp.
-	 */
-	if (bios_32_ptr->signature == PCI_BIOS32_SD_VALUE) {
-		length = bios_32_ptr->length * PCI_BIOS32_PARAGRAPH_LEN;
-		if (bios_checksum(p, length)) {
-			/*
-			 * According to the spec, we're looking for the
-			 * first 4KB-aligned address below the entrypoint
-			 * listed in the header. The Service Directory code
-			 * is guaranteed to occupy no more than 2 4KB pages.
-			 */
-			map_entry = bios_32_ptr->entry_point & ~(PAGE_SIZE - 1);
-			map_offset = bios_32_ptr->entry_point - map_entry;
-
-			return cru_detect(map_entry, map_offset);
-		}
-	}
-	return -ENODEV;
-}
-
-static int detect_cru_service(void)
-{
-	char __iomem *p, *q;
-	int rc = -1;
-
-	/*
-	 * Search from 0x0f0000 through 0x0fffff, inclusive.
-	 */
-	p = ioremap(PCI_ROM_BASE1, ROM_SIZE);
-	if (p == NULL)
-		return -ENOMEM;
-
-	for (q = p; q < p + ROM_SIZE; q += 16) {
-		rc = bios32_present(q);
-		if (!rc)
-			break;
-	}
-	iounmap(p);
-	return rc;
-}
-/* ------------------------------------------------------------------------- */
-#endif /* CONFIG_X86_32 */
-#ifdef CONFIG_X86_64
-/* --64 Bit Bios------------------------------------------------------------ */
-
-#define HPWDT_ARCH	64
-
-asm(".text                      \n\t"
-    ".align 4                   \n\t"
-    ".globl asminline_call	\n\t"
-    ".type asminline_call, @function \n\t"
-    "asminline_call:            \n\t"
-    FRAME_BEGIN
-    "pushq      %rax            \n\t"
-    "pushq      %rbx            \n\t"
-    "pushq      %rdx            \n\t"
-    "pushq      %r12            \n\t"
-    "pushq      %r9             \n\t"
-    "movq       %rsi, %r12      \n\t"
-    "movq       %rdi, %r9       \n\t"
-    "movl       4(%r9),%ebx     \n\t"
-    "movl       8(%r9),%ecx     \n\t"
-    "movl       12(%r9),%edx    \n\t"
-    "movl       16(%r9),%esi    \n\t"
-    "movl       20(%r9),%edi    \n\t"
-    "movl       (%r9),%eax      \n\t"
-    "call       *%r12           \n\t"
-    "pushfq                     \n\t"
-    "popq        %r12           \n\t"
-    "movl       %eax, (%r9)     \n\t"
-    "movl       %ebx, 4(%r9)    \n\t"
-    "movl       %ecx, 8(%r9)    \n\t"
-    "movl       %edx, 12(%r9)   \n\t"
-    "movl       %esi, 16(%r9)   \n\t"
-    "movl       %edi, 20(%r9)   \n\t"
-    "movq       %r12, %rax      \n\t"
-    "movl       %eax, 28(%r9)   \n\t"
-    "popq       %r9             \n\t"
-    "popq       %r12            \n\t"
-    "popq       %rdx            \n\t"
-    "popq       %rbx            \n\t"
-    "popq       %rax            \n\t"
-    FRAME_END
-    "ret                        \n\t"
-    ".previous");
-
-/*
- *	dmi_find_cru
- *
- *	Routine Description:
- *	This function checks whether or not a SMBIOS/DMI record is
- *	the 64bit CRU info or not
- */
-static void dmi_find_cru(const struct dmi_header *dm, void *dummy)
-{
-	struct smbios_cru64_info *smbios_cru64_ptr;
-	unsigned long cru_physical_address;
-
-	if (dm->type == SMBIOS_CRU64_INFORMATION) {
-		smbios_cru64_ptr = (struct smbios_cru64_info *) dm;
-		if (smbios_cru64_ptr->signature == CRU_BIOS_SIGNATURE_VALUE) {
-			cru_physical_address =
-				smbios_cru64_ptr->physical_address +
-				smbios_cru64_ptr->double_offset;
-			cru_rom_addr = ioremap(cru_physical_address,
-				smbios_cru64_ptr->double_length);
-			set_memory_x((unsigned long)cru_rom_addr & PAGE_MASK,
-				smbios_cru64_ptr->double_length >> PAGE_SHIFT);
-		}
-	}
-}
-
-static int detect_cru_service(void)
-{
-	cru_rom_addr = NULL;
-
-	dmi_walk(dmi_find_cru, NULL);
-
-	/* if cru_rom_addr has been set then we found a CRU service */
-	return ((cru_rom_addr != NULL) ? 0 : -ENODEV);
-}
-/* ------------------------------------------------------------------------- */
-#endif /* CONFIG_X86_64 */
-#endif /* CONFIG_HPWDT_NMI_DECODING */
 
 /*
  *	Watchdog operations
@@ -486,30 +113,12 @@ static int hpwdt_my_nmi(void)
  */
 static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
 {
-	unsigned long rom_pl;
-	static int die_nmi_called;
-
-	if (!hpwdt_nmi_decoding)
-		return NMI_DONE;
-
 	if ((ulReason == NMI_UNKNOWN) && !hpwdt_my_nmi())
 		return NMI_DONE;
 
-	spin_lock_irqsave(&rom_lock, rom_pl);
-	if (!die_nmi_called && !is_icru && !is_uefi)
-		asminline_call(&cmn_regs, cru_rom_addr);
-	die_nmi_called = 1;
-	spin_unlock_irqrestore(&rom_lock, rom_pl);
-
 	if (allow_kdump)
 		hpwdt_stop();
 
-	if (!is_icru && !is_uefi) {
-		if (cmn_regs.u1.ral == 0) {
-			nmi_panic(regs, "An NMI occurred, but unable to determine source.\n");
-			return NMI_HANDLED;
-		}
-	}
 	nmi_panic(regs, "An NMI occurred. Depending on your system the reason "
 		"for the NMI is logged in any one of the following "
 		"resources:\n"
@@ -675,84 +284,11 @@ static struct miscdevice hpwdt_miscdev = {
  *	Init & Exit
  */
 
-#ifdef CONFIG_HPWDT_NMI_DECODING
-#ifdef CONFIG_X86_LOCAL_APIC
-static void hpwdt_check_nmi_decoding(struct pci_dev *dev)
-{
-	/*
-	 * If nmi_watchdog is turned off then we can turn on
-	 * our nmi decoding capability.
-	 */
-	hpwdt_nmi_decoding = 1;
-}
-#else
-static void hpwdt_check_nmi_decoding(struct pci_dev *dev)
-{
-	dev_warn(&dev->dev, "NMI decoding is disabled. "
-		"Your kernel does not support a NMI Watchdog.\n");
-}
-#endif /* CONFIG_X86_LOCAL_APIC */
-
-/*
- *	dmi_find_icru
- *
- *	Routine Description:
- *	This function checks whether or not we are on an iCRU-based server.
- *	This check is independent of architecture and needs to be made for
- *	any ProLiant system.
- */
-static void dmi_find_icru(const struct dmi_header *dm, void *dummy)
-{
-	struct smbios_proliant_info *smbios_proliant_ptr;
-
-	if (dm->type == SMBIOS_ICRU_INFORMATION) {
-		smbios_proliant_ptr = (struct smbios_proliant_info *) dm;
-		if (smbios_proliant_ptr->misc_features & 0x01)
-			is_icru = 1;
-		if (smbios_proliant_ptr->misc_features & 0x1400)
-			is_uefi = 1;
-	}
-}
 
 static int hpwdt_init_nmi_decoding(struct pci_dev *dev)
 {
+#ifdef CONFIG_HPWDT_NMI_DECODING
 	int retval;
-
-	/*
-	 * On typical CRU-based systems we need to map that service in
-	 * the BIOS. For 32 bit Operating Systems we need to go through
-	 * the 32 Bit BIOS Service Directory. For 64 bit Operating
-	 * Systems we get that service through SMBIOS.
-	 *
-	 * On systems that support the new iCRU service all we need to
-	 * do is call dmi_walk to get the supported flag value and skip
-	 * the old cru detect code.
-	 */
-	dmi_walk(dmi_find_icru, NULL);
-	if (!is_icru && !is_uefi) {
-
-		/*
-		* We need to map the ROM to get the CRU service.
-		* For 32 bit Operating Systems we need to go through the 32 Bit
-		* BIOS Service Directory
-		* For 64 bit Operating Systems we get that service through SMBIOS.
-		*/
-		retval = detect_cru_service();
-		if (retval < 0) {
-			dev_warn(&dev->dev,
-				"Unable to detect the %d Bit CRU Service.\n",
-				HPWDT_ARCH);
-			return retval;
-		}
-
-		/*
-		* We know this is the only CRU call we need to make so lets keep as
-		* few instructions as possible once the NMI comes in.
-		*/
-		cmn_regs.u1.rah = 0x0D;
-		cmn_regs.u1.ral = 0x02;
-	}
-
 	/*
 	 * Only one function can register for NMI_UNKNOWN
 	 */
@@ -780,45 +316,26 @@ static int hpwdt_init_nmi_decoding(struct pci_dev *dev)
 	dev_warn(&dev->dev,
 		"Unable to register a die notifier (err=%d).\n",
 		retval);
-	if (cru_rom_addr)
-		iounmap(cru_rom_addr);
 	return retval;
+#endif	/* CONFIG_HPWDT_NMI_DECODING */
+	return 0;
 }
 
 static void hpwdt_exit_nmi_decoding(void)
 {
+#ifdef CONFIG_HPWDT_NMI_DECODING
 	unregister_nmi_handler(NMI_UNKNOWN, "hpwdt");
 	unregister_nmi_handler(NMI_SERR, "hpwdt");
 	unregister_nmi_handler(NMI_IO_CHECK, "hpwdt");
-	if (cru_rom_addr)
-		iounmap(cru_rom_addr);
-}
-#else /* !CONFIG_HPWDT_NMI_DECODING */
-static void hpwdt_check_nmi_decoding(struct pci_dev *dev)
-{
-}
-
-static int hpwdt_init_nmi_decoding(struct pci_dev *dev)
-{
-	return 0;
+#endif
 }
 
-static void hpwdt_exit_nmi_decoding(void)
-{
-}
-#endif /* CONFIG_HPWDT_NMI_DECODING */
-
 static int hpwdt_init_one(struct pci_dev *dev,
 					const struct pci_device_id *ent)
 {
 	int retval;
 
 	/*
-	 * Check if we can do NMI decoding or not
-	 */
-	hpwdt_check_nmi_decoding(dev);
-
-	/*
 	 * First let's find out if we are on an iLO2+ server. We will
 	 * not run on a legacy ASM box.
 	 * So we only support the G5 ProLiant servers and higher.
@@ -922,6 +439,6 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 #ifdef CONFIG_HPWDT_NMI_DECODING
 module_param(allow_kdump, int, 0);
 MODULE_PARM_DESC(allow_kdump, "Start a kernel dump after NMI occurs");
-#endif /* !CONFIG_HPWDT_NMI_DECODING */
+#endif /* CONFIG_HPWDT_NMI_DECODING */
 
 module_pci_driver(hpwdt_driver);
-- 
2.13.6

[PATCH v5 3/9] watchdog/hpwdt: Update nmi_panic message.

[Jerry Hoemann]

Include the nmistat in the nmi_panic message to give support
an indication why the NMI was called (e.g. a timeout or generate
nmi button.)

Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
 drivers/watchdog/hpwdt.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 1d20bc6d2c44..44c3038cc531 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -113,19 +113,23 @@ static int hpwdt_my_nmi(void)
  */
 static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
 {
-	if ((ulReason == NMI_UNKNOWN) && !hpwdt_my_nmi())
+	unsigned int mynmi = hpwdt_my_nmi();
+	static char panic_msg[] =
+		"00: An NMI occurred. Depending on your system the reason "
+		"for the NMI is logged in any one of the following resources:\n"
+		"1. Integrated Management Log (IML)\n"
+		"2. OA Syslog\n"
+		"3. OA Forward Progress Log\n"
+		"4. iLO Event Log";
+
+	if ((ulReason == NMI_UNKNOWN) && mynmi)
 		return NMI_DONE;
 
 	if (allow_kdump)
 		hpwdt_stop();
 
-	nmi_panic(regs, "An NMI occurred. Depending on your system the reason "
-		"for the NMI is logged in any one of the following "
-		"resources:\n"
-		"1. Integrated Management Log (IML)\n"
-		"2. OA Syslog\n"
-		"3. OA Forward Progress Log\n"
-		"4. iLO Event Log");
+	hex_byte_pack(panic_msg, mynmi);
+	nmi_panic(regs, panic_msg);
 
 	return NMI_HANDLED;
 }
-- 
2.13.6

[PATCH v5 4/9] watchdog/hpwdt: Modify to use watchdog core.

[Jerry Hoemann]

Follow Documentation/watchdog/convert_drivers_to_kernel_api.txt to
convert hpwdt from legacy watchdog driver to use the watchdog core.

Removed functions: hpwdt_open, hpwdt_release, hpwdt_write, hpwdt_ioctl
Removed data structures: hpwdt_fops, hpwdt_miscdev, watchdog_device
Modified functions: hpwdt_start, hpwdt_stop, hpwdt_ping, hpwdt_gettimeleft
Added functions: hpwdt_settimeout
Added structures: watchdog_device

Update Kconfig file to show that hpwdt now selects WATCHDOG_CORE.

Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
 drivers/watchdog/Kconfig |   1 +
 drivers/watchdog/hpwdt.c | 216 +++++++++++------------------------------------
 2 files changed, 48 insertions(+), 169 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 37460cd6cabb..e873522fca2d 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1118,6 +1118,7 @@ config IT87_WDT
 
 config HP_WATCHDOG
 	tristate "HP ProLiant iLO2+ Hardware Watchdog Timer"
+	select WATCHDOG_CORE
 	depends on X86 && PCI
 	help
 	  A software monitoring watchdog and NMI sourcing driver. This driver
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 44c3038cc531..0e35bb735d8e 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -16,17 +16,13 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/device.h>
-#include <linux/fs.h>
 #include <linux/io.h>
-#include <linux/bitops.h>
 #include <linux/kernel.h>
-#include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/pci.h>
 #include <linux/pci_ids.h>
 #include <linux/types.h>
-#include <linux/uaccess.h>
 #include <linux/watchdog.h>
 #include <asm/nmi.h>
 
@@ -42,8 +38,6 @@ static bool nowayout = WATCHDOG_NOWAYOUT;
 #ifdef CONFIG_HPWDT_NMI_DECODING
 static unsigned int allow_kdump = 1;
 #endif
-static char expect_release;
-static unsigned long hpwdt_is_open;
 
 static void __iomem *pci_mem_addr;		/* the PCI-memory address */
 static unsigned long __iomem *hpwdt_nmistat;
@@ -61,11 +55,14 @@ MODULE_DEVICE_TABLE(pci, hpwdt_devices);
 /*
  *	Watchdog operations
  */
-static void hpwdt_start(void)
+static int hpwdt_start(struct watchdog_device *wdd)
 {
-	reload = SECS_TO_TICKS(soft_margin);
+	reload = SECS_TO_TICKS(wdd->timeout);
+
 	iowrite16(reload, hpwdt_timer_reg);
 	iowrite8(0x85, hpwdt_timer_con);
+
+	return 0;
 }
 
 static void hpwdt_stop(void)
@@ -77,31 +74,32 @@ static void hpwdt_stop(void)
 	iowrite8(data, hpwdt_timer_con);
 }
 
-static void hpwdt_ping(void)
+static int hpwdt_stop_core(struct watchdog_device *wdd)
 {
-	iowrite16(reload, hpwdt_timer_reg);
+	hpwdt_stop();
+
+	return 0;
 }
 
-static int hpwdt_change_timer(int new_margin)
+static int hpwdt_ping(struct watchdog_device *wdd)
 {
-	if (new_margin < 1 || new_margin > HPWDT_MAX_TIMER) {
-		pr_warn("New value passed in is invalid: %d seconds\n",
-			new_margin);
-		return -EINVAL;
-	}
-
-	soft_margin = new_margin;
-	pr_debug("New timer passed in is %d seconds\n", new_margin);
-	reload = SECS_TO_TICKS(soft_margin);
-
+	iowrite16(reload, hpwdt_timer_reg);
 	return 0;
 }
 
-static int hpwdt_time_left(void)
+static unsigned int hpwdt_gettimeleft(struct watchdog_device *wdd)
 {
 	return TICKS_TO_SECS(ioread16(hpwdt_timer_reg));
 }
 
+static int hpwdt_settimeout(struct watchdog_device *wdd, unsigned int val)
+{
+	wdd->timeout = val;
+	hpwdt_ping(wdd);
+
+	return 0;
+}
+
 #ifdef CONFIG_HPWDT_NMI_DECODING
 static int hpwdt_my_nmi(void)
 {
@@ -135,68 +133,6 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
 }
 #endif /* CONFIG_HPWDT_NMI_DECODING */
 
-/*
- *	/dev/watchdog handling
- */
-static int hpwdt_open(struct inode *inode, struct file *file)
-{
-	/* /dev/watchdog can only be opened once */
-	if (test_and_set_bit(0, &hpwdt_is_open))
-		return -EBUSY;
-
-	/* Start the watchdog */
-	hpwdt_start();
-	hpwdt_ping();
-
-	return nonseekable_open(inode, file);
-}
-
-static int hpwdt_release(struct inode *inode, struct file *file)
-{
-	/* Stop the watchdog */
-	if (expect_release == 42) {
-		hpwdt_stop();
-	} else {
-		pr_crit("Unexpected close, not stopping watchdog!\n");
-		hpwdt_ping();
-	}
-
-	expect_release = 0;
-
-	/* /dev/watchdog is being closed, make sure it can be re-opened */
-	clear_bit(0, &hpwdt_is_open);
-
-	return 0;
-}
-
-static ssize_t hpwdt_write(struct file *file, const char __user *data,
-	size_t len, loff_t *ppos)
-{
-	/* See if we got the magic character 'V' and reload the timer */
-	if (len) {
-		if (!nowayout) {
-			size_t i;
-
-			/* note: just in case someone wrote the magic character
-			 * five months ago... */
-			expect_release = 0;
-
-			/* scan to see whether or not we got the magic char. */
-			for (i = 0; i != len; i++) {
-				char c;
-				if (get_user(c, data + i))
-					return -EFAULT;
-				if (c == 'V')
-					expect_release = 42;
-			}
-		}
-
-		/* someone wrote to us, we should reload the timer */
-		hpwdt_ping();
-	}
-
-	return len;
-}
 
 static const struct watchdog_info ident = {
 	.options = WDIOF_SETTIMEOUT |
@@ -205,90 +141,32 @@ static const struct watchdog_info ident = {
 	.identity = "HPE iLO2+ HW Watchdog Timer",
 };
 
-static long hpwdt_ioctl(struct file *file, unsigned int cmd,
-	unsigned long arg)
-{
-	void __user *argp = (void __user *)arg;
-	int __user *p = argp;
-	int new_margin, options;
-	int ret = -ENOTTY;
-
-	switch (cmd) {
-	case WDIOC_GETSUPPORT:
-		ret = 0;
-		if (copy_to_user(argp, &ident, sizeof(ident)))
-			ret = -EFAULT;
-		break;
-
-	case WDIOC_GETSTATUS:
-	case WDIOC_GETBOOTSTATUS:
-		ret = put_user(0, p);
-		break;
-
-	case WDIOC_KEEPALIVE:
-		hpwdt_ping();
-		ret = 0;
-		break;
-
-	case WDIOC_SETOPTIONS:
-		ret = get_user(options, p);
-		if (ret)
-			break;
-
-		if (options & WDIOS_DISABLECARD)
-			hpwdt_stop();
-
-		if (options & WDIOS_ENABLECARD) {
-			hpwdt_start();
-			hpwdt_ping();
-		}
-		break;
-
-	case WDIOC_SETTIMEOUT:
-		ret = get_user(new_margin, p);
-		if (ret)
-			break;
-
-		ret = hpwdt_change_timer(new_margin);
-		if (ret)
-			break;
-
-		hpwdt_ping();
-		/* Fall */
-	case WDIOC_GETTIMEOUT:
-		ret = put_user(soft_margin, p);
-		break;
-
-	case WDIOC_GETTIMELEFT:
-		ret = put_user(hpwdt_time_left(), p);
-		break;
-	}
-	return ret;
-}
-
 /*
  *	Kernel interfaces
  */
-static const struct file_operations hpwdt_fops = {
-	.owner = THIS_MODULE,
-	.llseek = no_llseek,
-	.write = hpwdt_write,
-	.unlocked_ioctl = hpwdt_ioctl,
-	.open = hpwdt_open,
-	.release = hpwdt_release,
+
+static const struct watchdog_ops hpwdt_ops = {
+	.owner		= THIS_MODULE,
+	.start		= hpwdt_start,
+	.stop		= hpwdt_stop_core,
+	.ping		= hpwdt_ping,
+	.set_timeout	= hpwdt_settimeout,
+	.get_timeleft	= hpwdt_gettimeleft,
 };
 
-static struct miscdevice hpwdt_miscdev = {
-	.minor = WATCHDOG_MINOR,
-	.name = "watchdog",
-	.fops = &hpwdt_fops,
+static struct watchdog_device hpwdt_dev = {
+	.info		= &ident,
+	.ops		= &hpwdt_ops,
+	.min_timeout	= 1,
+	.max_timeout	= HPWDT_MAX_TIMER,
+	.timeout	= DEFAULT_MARGIN,
 };
 
+
 /*
  *	Init & Exit
  */
 
-
 static int hpwdt_init_nmi_decoding(struct pci_dev *dev)
 {
 #ifdef CONFIG_HPWDT_NMI_DECODING
@@ -379,29 +257,29 @@ static int hpwdt_init_one(struct pci_dev *dev,
 	/* Make sure that timer is disabled until /dev/watchdog is opened */
 	hpwdt_stop();
 
-	/* Make sure that we have a valid soft_margin */
-	if (hpwdt_change_timer(soft_margin))
-		hpwdt_change_timer(DEFAULT_MARGIN);
-
 	/* Initialize NMI Decoding functionality */
 	retval = hpwdt_init_nmi_decoding(dev);
 	if (retval != 0)
 		goto error_init_nmi_decoding;
 
-	retval = misc_register(&hpwdt_miscdev);
+	watchdog_set_nowayout(&hpwdt_dev, nowayout);
+	if (watchdog_init_timeout(&hpwdt_dev, soft_margin, NULL))
+		dev_warn(&dev->dev, "Invalid soft_margin: %d.\n", soft_margin);
+
+	hpwdt_dev.parent = &dev->dev;
+	retval = watchdog_register_device(&hpwdt_dev);
 	if (retval < 0) {
-		dev_warn(&dev->dev,
-			"Unable to register miscdev on minor=%d (err=%d).\n",
-			WATCHDOG_MINOR, retval);
-		goto error_misc_register;
+		dev_err(&dev->dev, "watchdog register failed: %d.\n", retval);
+		goto error_wd_register;
 	}
 
 	dev_info(&dev->dev, "HPE Watchdog Timer Driver: %s"
 			", timer margin: %d seconds (nowayout=%d).\n",
-			HPWDT_VERSION, soft_margin, nowayout);
+			HPWDT_VERSION, hpwdt_dev.timeout, nowayout);
+
 	return 0;
 
-error_misc_register:
+error_wd_register:
 	hpwdt_exit_nmi_decoding();
 error_init_nmi_decoding:
 	pci_iounmap(dev, pci_mem_addr);
@@ -415,7 +293,7 @@ static void hpwdt_exit(struct pci_dev *dev)
 	if (!nowayout)
 		hpwdt_stop();
 
-	misc_deregister(&hpwdt_miscdev);
+	watchdog_unregister_device(&hpwdt_dev);
 	hpwdt_exit_nmi_decoding();
 	pci_iounmap(dev, pci_mem_addr);
 	pci_disable_device(dev);
-- 
2.13.6

[PATCH v5 5/9] watchdog/hpwdt: condition early return of NMI handler on iLO5

[Jerry Hoemann]

Modify prior change to not claim an NMI unless originated
from iLO to apply only to iLO5 and later going forward.
This restores hpwdt traditional behavior of calling panic
if the NMI is NMI_IO_CHECK, NMI_SERR, or NMI_UNKNOWN for
legacy hardware.

Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
 drivers/watchdog/hpwdt.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 0e35bb735d8e..171d5033d7b5 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -32,6 +32,7 @@
 #define HPWDT_MAX_TIMER			TICKS_TO_SECS(65535)
 #define DEFAULT_MARGIN			30
 
+static bool ilo5;
 static unsigned int soft_margin = DEFAULT_MARGIN;	/* in seconds */
 static unsigned int reload;			/* the computed soft_margin */
 static bool nowayout = WATCHDOG_NOWAYOUT;
@@ -120,7 +121,7 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
 		"3. OA Forward Progress Log\n"
 		"4. iLO Event Log";
 
-	if ((ulReason == NMI_UNKNOWN) && mynmi)
+	if (ilo5 && ulReason == NMI_UNKNOWN && mynmi)
 		return NMI_DONE;
 
 	if (allow_kdump)
@@ -277,6 +278,9 @@ static int hpwdt_init_one(struct pci_dev *dev,
 			", timer margin: %d seconds (nowayout=%d).\n",
 			HPWDT_VERSION, hpwdt_dev.timeout, nowayout);
 
+	if (dev->subsystem_vendor == PCI_VENDOR_ID_HP_3PAR)
+		ilo5 = true;
+
 	return 0;
 
 error_wd_register:
-- 
2.13.6

[PATCH v5 6/9] watchdog/hpwdt: remove allow_kdump module parameter.

[Jerry Hoemann]

The intent of this parameter is unclear and it sets up a
race between the reset of the system by ASR and crashdump.

The length of time between receipt of the pretimeout NMI
and the ASR reset of the system is fixed by hardware.

Turning the parameter off doesn't necessairly prevent a crash dump.
Also, having the ASR reset occur while the system is crash dumping
doesn't imply that the dump was hung given the short duration
between the NMI and the reset.

This parameter is not a substitute for having a architected watchdog
crashdump hang detection paridigm.

Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
 drivers/watchdog/hpwdt.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 171d5033d7b5..68e84a212d00 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -36,9 +36,6 @@ static bool ilo5;
 static unsigned int soft_margin = DEFAULT_MARGIN;	/* in seconds */
 static unsigned int reload;			/* the computed soft_margin */
 static bool nowayout = WATCHDOG_NOWAYOUT;
-#ifdef CONFIG_HPWDT_NMI_DECODING
-static unsigned int allow_kdump = 1;
-#endif
 
 static void __iomem *pci_mem_addr;		/* the PCI-memory address */
 static unsigned long __iomem *hpwdt_nmistat;
@@ -124,8 +121,7 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
 	if (ilo5 && ulReason == NMI_UNKNOWN && mynmi)
 		return NMI_DONE;
 
-	if (allow_kdump)
-		hpwdt_stop();
+	hpwdt_stop();
 
 	hex_byte_pack(panic_msg, mynmi);
 	nmi_panic(regs, panic_msg);
@@ -186,9 +182,8 @@ static int hpwdt_init_nmi_decoding(struct pci_dev *dev)
 		goto error2;
 
 	dev_info(&dev->dev,
-			"HPE Watchdog Timer Driver: NMI decoding initialized"
-			", allow kernel dump: %s (default = 1/ON)\n",
-			(allow_kdump == 0) ? "OFF" : "ON");
+		"HPE Watchdog Timer Driver: NMI decoding initialized\n");
+
 	return 0;
 
 error2:
@@ -322,9 +317,4 @@ module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 		__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
-#ifdef CONFIG_HPWDT_NMI_DECODING
-module_param(allow_kdump, int, 0);
-MODULE_PARM_DESC(allow_kdump, "Start a kernel dump after NMI occurs");
-#endif /* CONFIG_HPWDT_NMI_DECODING */
-
 module_pci_driver(hpwdt_driver);
-- 
2.13.6

[PATCH v5 7/9] watchdog/hpwdt: Programable Pretimeout NMI

[Jerry Hoemann]

Make whether or not the hpwdt watchdog delivers a pretimeout NMI
programable by the user.

The underlying iLO hardware is programmable as to whether or not
a pre-timeout NMI is delivered to the system before the iLO resets
the system.  However, the iLO does not allow for programming the
length of time that NMI is delivered before the system is reset.

By watchdog API, in hpwdt_set_pretimeout a val == 0 disables the NMI.
When val != 0, hpwdt_set_pretimeout will enable the pretimeout NMI
provided the current timeout is greator than the HW specified
pretimeout length. Otherwise an error is returned.

In set_timeout, if the new timeout is <= an already established pretimeout,
the pretimeout is canceled.  This matches the action watchdog_set_timeout
in the watchdog core would do if an hpwdt specific set_timeout
function wasn't specified.

Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
 drivers/watchdog/hpwdt.c | 53 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 49 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 68e84a212d00..b8205c6e61c1 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -31,11 +31,12 @@
 #define TICKS_TO_SECS(ticks)		((ticks) * 128 / 1000)
 #define HPWDT_MAX_TIMER			TICKS_TO_SECS(65535)
 #define DEFAULT_MARGIN			30
+#define PRETIMEOUT_SEC			9
 
 static bool ilo5;
 static unsigned int soft_margin = DEFAULT_MARGIN;	/* in seconds */
-static unsigned int reload;			/* the computed soft_margin */
 static bool nowayout = WATCHDOG_NOWAYOUT;
+static bool pretimeout = IS_ENABLED(CONFIG_HPWDT_NMI_DECODING);
 
 static void __iomem *pci_mem_addr;		/* the PCI-memory address */
 static unsigned long __iomem *hpwdt_nmistat;
@@ -55,10 +56,11 @@ MODULE_DEVICE_TABLE(pci, hpwdt_devices);
  */
 static int hpwdt_start(struct watchdog_device *wdd)
 {
-	reload = SECS_TO_TICKS(wdd->timeout);
+	int control = 0x81 | (pretimeout ? 0x4 : 0);
+	int reload = SECS_TO_TICKS(wdd->timeout);
 
 	iowrite16(reload, hpwdt_timer_reg);
-	iowrite8(0x85, hpwdt_timer_con);
+	iowrite8(control, hpwdt_timer_con);
 
 	return 0;
 }
@@ -81,7 +83,10 @@ static int hpwdt_stop_core(struct watchdog_device *wdd)
 
 static int hpwdt_ping(struct watchdog_device *wdd)
 {
+	int reload = SECS_TO_TICKS(wdd->timeout);
+
 	iowrite16(reload, hpwdt_timer_reg);
+
 	return 0;
 }
 
@@ -93,12 +98,37 @@ static unsigned int hpwdt_gettimeleft(struct watchdog_device *wdd)
 static int hpwdt_settimeout(struct watchdog_device *wdd, unsigned int val)
 {
 	wdd->timeout = val;
+	if (val <= wdd->pretimeout) {
+		wdd->pretimeout = 0;
+		pretimeout = 0;
+		if (watchdog_active(wdd))
+			hpwdt_start(wdd);
+	}
 	hpwdt_ping(wdd);
 
 	return 0;
 }
 
 #ifdef CONFIG_HPWDT_NMI_DECODING
+static int hpwdt_set_pretimeout(struct watchdog_device *wdd, unsigned int req)
+{
+	unsigned int val = 0;
+
+	if (req) {
+		val = PRETIMEOUT_SEC;
+		if (val >= wdd->timeout)
+			return -EINVAL;
+	}
+
+	wdd->pretimeout = val;
+	pretimeout = !!val;
+
+	if (watchdog_active(wdd))
+		hpwdt_start(wdd);
+
+	return 0;
+}
+
 static int hpwdt_my_nmi(void)
 {
 	return ioread8(hpwdt_nmistat) & 0x6;
@@ -121,6 +151,9 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
 	if (ilo5 && ulReason == NMI_UNKNOWN && mynmi)
 		return NMI_DONE;
 
+	if (ilo5 && !pretimeout)
+		return NMI_DONE;
+
 	hpwdt_stop();
 
 	hex_byte_pack(panic_msg, mynmi);
@@ -132,7 +165,8 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
 
 
 static const struct watchdog_info ident = {
-	.options = WDIOF_SETTIMEOUT |
+	.options = WDIOF_PRETIMEOUT    |
+		   WDIOF_SETTIMEOUT    |
 		   WDIOF_KEEPALIVEPING |
 		   WDIOF_MAGICCLOSE,
 	.identity = "HPE iLO2+ HW Watchdog Timer",
@@ -149,6 +183,9 @@ static const struct watchdog_ops hpwdt_ops = {
 	.ping		= hpwdt_ping,
 	.set_timeout	= hpwdt_settimeout,
 	.get_timeleft	= hpwdt_gettimeleft,
+#ifdef CONFIG_HPWDT_NMI_DECODING
+	.set_pretimeout	= hpwdt_set_pretimeout,
+#endif
 };
 
 static struct watchdog_device hpwdt_dev = {
@@ -157,6 +194,9 @@ static struct watchdog_device hpwdt_dev = {
 	.min_timeout	= 1,
 	.max_timeout	= HPWDT_MAX_TIMER,
 	.timeout	= DEFAULT_MARGIN,
+#ifdef CONFIG_HPWDT_NMI_DECODING
+	.pretimeout	= PRETIMEOUT_SEC,
+#endif
 };
 
 
@@ -317,4 +357,9 @@ module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 		__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
+#ifdef CONFIG_HPWDT_NMI_DECODING
+module_param(pretimeout, bool, 0);
+MODULE_PARM_DESC(pretimeout, "Watchdog pretimeout enabled");
+#endif
+
 module_pci_driver(hpwdt_driver);
-- 
2.13.6

[PATCH v5 8/9] watchdog/hpwdt: Add dynamic debug

[Jerry Hoemann]

Add a few dynamic debug messages to aid in module level debug.

Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
 drivers/watchdog/hpwdt.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index b8205c6e61c1..b82bbeed0e43 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -59,6 +59,7 @@ static int hpwdt_start(struct watchdog_device *wdd)
 	int control = 0x81 | (pretimeout ? 0x4 : 0);
 	int reload = SECS_TO_TICKS(wdd->timeout);
 
+	dev_dbg(wdd->parent, "start watchdog 0x%08x:0x%02x\n", reload, control);
 	iowrite16(reload, hpwdt_timer_reg);
 	iowrite8(control, hpwdt_timer_con);
 
@@ -69,6 +70,8 @@ static void hpwdt_stop(void)
 {
 	unsigned long data;
 
+	pr_debug("stop  watchdog\n");
+
 	data = ioread8(hpwdt_timer_con);
 	data &= 0xFE;
 	iowrite8(data, hpwdt_timer_con);
@@ -85,6 +88,7 @@ static int hpwdt_ping(struct watchdog_device *wdd)
 {
 	int reload = SECS_TO_TICKS(wdd->timeout);
 
+	dev_dbg(wdd->parent, "ping  watchdog 0x%08x\n", reload);
 	iowrite16(reload, hpwdt_timer_reg);
 
 	return 0;
@@ -97,8 +101,11 @@ static unsigned int hpwdt_gettimeleft(struct watchdog_device *wdd)
 
 static int hpwdt_settimeout(struct watchdog_device *wdd, unsigned int val)
 {
+	dev_dbg(wdd->parent, "set_timeout = %d\n", val);
+
 	wdd->timeout = val;
 	if (val <= wdd->pretimeout) {
+		dev_dbg(wdd->parent, "pretimeout < timeout. Setting to zero\n");
 		wdd->pretimeout = 0;
 		pretimeout = 0;
 		if (watchdog_active(wdd))
@@ -114,12 +121,16 @@ static int hpwdt_set_pretimeout(struct watchdog_device *wdd, unsigned int req)
 {
 	unsigned int val = 0;
 
+	dev_dbg(wdd->parent, "set_pretimeout = %d\n", req);
 	if (req) {
 		val = PRETIMEOUT_SEC;
 		if (val >= wdd->timeout)
 			return -EINVAL;
 	}
 
+	if (val != req)
+		dev_dbg(wdd->parent, "Rounding pretimeout to: %d\n", val);
+
 	wdd->pretimeout = val;
 	pretimeout = !!val;
 
-- 
2.13.6

[PATCH v5 9/9] watchdog/hpwdt: Update driver version.

[Jerry Hoemann]

Update driver version number to reflect changes.

Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
---
 drivers/watchdog/hpwdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index b82bbeed0e43..a43ab2cecca2 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -26,7 +26,7 @@
 #include <linux/watchdog.h>
 #include <asm/nmi.h>
 
-#define HPWDT_VERSION			"1.4.0"
+#define HPWDT_VERSION			"2.0.0"
 #define SECS_TO_TICKS(secs)		((secs) * 1000 / 128)
 #define TICKS_TO_SECS(ticks)		((ticks) * 128 / 1000)
 #define HPWDT_MAX_TIMER			TICKS_TO_SECS(65535)
-- 
2.13.6

Re: [PATCH v5 2/9] watchdog/hpwdt: Remove legacy NMI sourcing.

[Arnd Bergmann]

On Mon, Feb 26, 2018 at 4:22 AM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> Gen8 and prior Proliant systems supported the "CRU" interface
> to firmware.  This interfaces allows linux to "call back" into firmware
> to source the cause of an NMI.  This feature isn't fully utilized
> as the actual source of the NMI isn't printed, the driver only
> indicates that the source couldn't be determined when the call
> fails.
>
> With the advent of Gen9, iCRU replaces the CRU. The call back
> feature is no longer available in firmware.  To be compatible and
> not attempt to call back into firmware on system not supporting CRU,
> the SMBIOS table is consulted to determine if it is safe to
> make the call back or not.
>
> This results in about half of the driver code being devoted
> to either making CRU calls or determing if it is safe to make
> CRU calls.  As noted, the driver isn't really using the results of
> the CRU calls.
>
> Furthermore, as a consequence of the Spectre security issue, the
> BIOS/EFI calls are being wrapped into Spectre-disabling section.
> Removing the call back in hpwdt_pretimeout assists in this effort.
>
> As the CRU sourcing of the NMI isn't required for handling the
> NMI and there are security concerns with making the call back, remove
> the legacy (pre Gen9) NMI sourcing and the DMI code to determine if
> the system had the CRU interface.
>
> Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>

This avoids a warning in mainline kernels, so that's great:

drivers/watchdog/hpwdt.o: warning: objtool: .text+0x24: indirect call
found in RETPOLINE build

I wonder what we do about stable kernels. Are both this patch and the patch
that added the objtool warning message candidates for backports to
stable kernels?

       Arnd

Re: [PATCH v5 2/9] watchdog/hpwdt: Remove legacy NMI sourcing.

[Guenter Roeck]

On 02/26/2018 06:11 AM, Arnd Bergmann wrote:
> On Mon, Feb 26, 2018 at 4:22 AM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
>> Gen8 and prior Proliant systems supported the "CRU" interface
>> to firmware.  This interfaces allows linux to "call back" into firmware
>> to source the cause of an NMI.  This feature isn't fully utilized
>> as the actual source of the NMI isn't printed, the driver only
>> indicates that the source couldn't be determined when the call
>> fails.
>>
>> With the advent of Gen9, iCRU replaces the CRU. The call back
>> feature is no longer available in firmware.  To be compatible and
>> not attempt to call back into firmware on system not supporting CRU,
>> the SMBIOS table is consulted to determine if it is safe to
>> make the call back or not.
>>
>> This results in about half of the driver code being devoted
>> to either making CRU calls or determing if it is safe to make
>> CRU calls.  As noted, the driver isn't really using the results of
>> the CRU calls.
>>
>> Furthermore, as a consequence of the Spectre security issue, the
>> BIOS/EFI calls are being wrapped into Spectre-disabling section.
>> Removing the call back in hpwdt_pretimeout assists in this effort.
>>
>> As the CRU sourcing of the NMI isn't required for handling the
>> NMI and there are security concerns with making the call back, remove
>> the legacy (pre Gen9) NMI sourcing and the DMI code to determine if
>> the system had the CRU interface.
>>
>> Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
> 
> This avoids a warning in mainline kernels, so that's great:
> 
> drivers/watchdog/hpwdt.o: warning: objtool: .text+0x24: indirect call
> found in RETPOLINE build
> 
> I wonder what we do about stable kernels. Are both this patch and the patch
> that added the objtool warning message candidates for backports to
> stable kernels?
> 

Makes sense to me, but it is really a bit more than a bug fix, so I'll
leave it up to Jerry/HPE to make the call in respect to hpwdt.

Guenter

Re: [PATCH v5 2/9] watchdog/hpwdt: Remove legacy NMI sourcing.

[Jerry Hoemann]

On Mon, Feb 26, 2018 at 06:32:30AM -0800, Guenter Roeck wrote:
> On 02/26/2018 06:11 AM, Arnd Bergmann wrote:
> > On Mon, Feb 26, 2018 at 4:22 AM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> > > Gen8 and prior Proliant systems supported the "CRU" interface
> > > to firmware.  This interfaces allows linux to "call back" into firmware
> > > to source the cause of an NMI.  This feature isn't fully utilized
> > > as the actual source of the NMI isn't printed, the driver only
> > > indicates that the source couldn't be determined when the call
> > > fails.
> > > 
> > > With the advent of Gen9, iCRU replaces the CRU. The call back
> > > feature is no longer available in firmware.  To be compatible and
> > > not attempt to call back into firmware on system not supporting CRU,
> > > the SMBIOS table is consulted to determine if it is safe to
> > > make the call back or not.
> > > 
> > > This results in about half of the driver code being devoted
> > > to either making CRU calls or determing if it is safe to make
> > > CRU calls.  As noted, the driver isn't really using the results of
> > > the CRU calls.
> > > 
> > > Furthermore, as a consequence of the Spectre security issue, the
> > > BIOS/EFI calls are being wrapped into Spectre-disabling section.
> > > Removing the call back in hpwdt_pretimeout assists in this effort.
> > > 
> > > As the CRU sourcing of the NMI isn't required for handling the
> > > NMI and there are security concerns with making the call back, remove
> > > the legacy (pre Gen9) NMI sourcing and the DMI code to determine if
> > > the system had the CRU interface.
> > > 
> > > Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
> > 
> > This avoids a warning in mainline kernels, so that's great:
> > 
> > drivers/watchdog/hpwdt.o: warning: objtool: .text+0x24: indirect call
> > found in RETPOLINE build
> > 
> > I wonder what we do about stable kernels. Are both this patch and the patch
> > that added the objtool warning message candidates for backports to
> > stable kernels?
> > 
> 
> Makes sense to me, but it is really a bit more than a bug fix, so I'll
> leave it up to Jerry/HPE to make the call in respect to hpwdt.
> 

Generally speaking, HPE customers who run linux do so through a distro
vendor and pick up patches from them.  But I'm sure there are some
customers who do things differently.

The distro vendor's have their own repos and we'll work with them
to back port patches to their code base.  So, I typically don't do a lot
of kernel.org stable branch work.

Looks like objtool has been enhanced to find Spectre vulnerable code.
Are the other kernel patches related to Spectre being back ported
to stable release lines?  If yes, it probably make sense to do
the hpwdt change as well.

Is just the patch removing the firmware call back wanted/needed? Or the
whole driver rewrite?  (The older baseline don't have all the watchdog
features that the patch set uses.)

Which stable baseline(s) would need to be patched?  Priority?

Who does it?  (i.e. do you want me to submit patches to the stable baseline?)

Thanks

-- 

-----------------------------------------------------------------------------
Jerry Hoemann                  Software Engineer   Hewlett Packard Enterprise
-----------------------------------------------------------------------------

Re: [PATCH v5 2/9] watchdog/hpwdt: Remove legacy NMI sourcing.

[Guenter Roeck]

On 02/26/2018 05:02 PM, Jerry Hoemann wrote:
> On Mon, Feb 26, 2018 at 06:32:30AM -0800, Guenter Roeck wrote:
>> On 02/26/2018 06:11 AM, Arnd Bergmann wrote:
>>> On Mon, Feb 26, 2018 at 4:22 AM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
>>>> Gen8 and prior Proliant systems supported the "CRU" interface
>>>> to firmware.  This interfaces allows linux to "call back" into firmware
>>>> to source the cause of an NMI.  This feature isn't fully utilized
>>>> as the actual source of the NMI isn't printed, the driver only
>>>> indicates that the source couldn't be determined when the call
>>>> fails.
>>>>
>>>> With the advent of Gen9, iCRU replaces the CRU. The call back
>>>> feature is no longer available in firmware.  To be compatible and
>>>> not attempt to call back into firmware on system not supporting CRU,
>>>> the SMBIOS table is consulted to determine if it is safe to
>>>> make the call back or not.
>>>>
>>>> This results in about half of the driver code being devoted
>>>> to either making CRU calls or determing if it is safe to make
>>>> CRU calls.  As noted, the driver isn't really using the results of
>>>> the CRU calls.
>>>>
>>>> Furthermore, as a consequence of the Spectre security issue, the
>>>> BIOS/EFI calls are being wrapped into Spectre-disabling section.
>>>> Removing the call back in hpwdt_pretimeout assists in this effort.
>>>>
>>>> As the CRU sourcing of the NMI isn't required for handling the
>>>> NMI and there are security concerns with making the call back, remove
>>>> the legacy (pre Gen9) NMI sourcing and the DMI code to determine if
>>>> the system had the CRU interface.
>>>>
>>>> Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
>>>
>>> This avoids a warning in mainline kernels, so that's great:
>>>
>>> drivers/watchdog/hpwdt.o: warning: objtool: .text+0x24: indirect call
>>> found in RETPOLINE build
>>>
>>> I wonder what we do about stable kernels. Are both this patch and the patch
>>> that added the objtool warning message candidates for backports to
>>> stable kernels?
>>>
>>
>> Makes sense to me, but it is really a bit more than a bug fix, so I'll
>> leave it up to Jerry/HPE to make the call in respect to hpwdt.
>>
> 
> Generally speaking, HPE customers who run linux do so through a distro
> vendor and pick up patches from them.  But I'm sure there are some
> customers who do things differently.
> 
> The distro vendor's have their own repos and we'll work with them
> to back port patches to their code base.  So, I typically don't do a lot
> of kernel.org stable branch work.
> 
> Looks like objtool has been enhanced to find Spectre vulnerable code.
> Are the other kernel patches related to Spectre being back ported
> to stable release lines?  If yes, it probably make sense to do
> the hpwdt change as well.
> 

Spectre has been backported to v4.4 and later. I don't know about earlier kernels.

> Is just the patch removing the firmware call back wanted/needed? Or the
> whole driver rewrite?  (The older baseline don't have all the watchdog
> features that the patch set uses.)
> 

We would only want to backport this patch. The rest is really out of scope.

> Which stable baseline(s) would need to be patched?  Priority?
> 
> Who does it?  (i.e. do you want me to submit patches to the stable baseline?)
> 
We would tag the patch for stable (and submit it into v4.16-rc). Greg would
take care of the rest unless there are conflicts, in which case we get a note
telling us that a backport is needed.

Guenter

Re: [PATCH v5 2/9] watchdog/hpwdt: Remove legacy NMI sourcing.

[Jerry Hoemann]

On Mon, Feb 26, 2018 at 05:29:55PM -0800, Guenter Roeck wrote:
> On 02/26/2018 05:02 PM, Jerry Hoemann wrote:
> > On Mon, Feb 26, 2018 at 06:32:30AM -0800, Guenter Roeck wrote:
> > > On 02/26/2018 06:11 AM, Arnd Bergmann wrote:
> > > > On Mon, Feb 26, 2018 at 4:22 AM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
> > > > > Gen8 and prior Proliant systems supported the "CRU" interface
> > > > > to firmware.  This interfaces allows linux to "call back" into firmware
> > > > > to source the cause of an NMI.  This feature isn't fully utilized
> > > > > as the actual source of the NMI isn't printed, the driver only
> > > > > indicates that the source couldn't be determined when the call
> > > > > fails.
> > > > > 
> > > > > With the advent of Gen9, iCRU replaces the CRU. The call back
> > > > > feature is no longer available in firmware.  To be compatible and
> > > > > not attempt to call back into firmware on system not supporting CRU,
> > > > > the SMBIOS table is consulted to determine if it is safe to
> > > > > make the call back or not.
> > > > > 
> > > > > This results in about half of the driver code being devoted
> > > > > to either making CRU calls or determing if it is safe to make
> > > > > CRU calls.  As noted, the driver isn't really using the results of
> > > > > the CRU calls.
> > > > > 
> > > > > Furthermore, as a consequence of the Spectre security issue, the
> > > > > BIOS/EFI calls are being wrapped into Spectre-disabling section.
> > > > > Removing the call back in hpwdt_pretimeout assists in this effort.
> > > > > 
> > > > > As the CRU sourcing of the NMI isn't required for handling the
> > > > > NMI and there are security concerns with making the call back, remove
> > > > > the legacy (pre Gen9) NMI sourcing and the DMI code to determine if
> > > > > the system had the CRU interface.
> > > > > 
> > > > > Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
> > > > 
> > > > This avoids a warning in mainline kernels, so that's great:
> > > > 
> > > > drivers/watchdog/hpwdt.o: warning: objtool: .text+0x24: indirect call
> > > > found in RETPOLINE build
> > > > 
> > > > I wonder what we do about stable kernels. Are both this patch and the patch
> > > > that added the objtool warning message candidates for backports to
> > > > stable kernels?
> > > > 
> > > 
> > > Makes sense to me, but it is really a bit more than a bug fix, so I'll
> > > leave it up to Jerry/HPE to make the call in respect to hpwdt.
> > > 
> > 
> > Generally speaking, HPE customers who run linux do so through a distro
> > vendor and pick up patches from them.  But I'm sure there are some
> > customers who do things differently.
> > 
> > The distro vendor's have their own repos and we'll work with them
> > to back port patches to their code base.  So, I typically don't do a lot
> > of kernel.org stable branch work.
> > 
> > Looks like objtool has been enhanced to find Spectre vulnerable code.
> > Are the other kernel patches related to Spectre being back ported
> > to stable release lines?  If yes, it probably make sense to do
> > the hpwdt change as well.
> > 
> 
> Spectre has been backported to v4.4 and later. I don't know about earlier kernels.
> 
> > Is just the patch removing the firmware call back wanted/needed? Or the
> > whole driver rewrite?  (The older baseline don't have all the watchdog
> > features that the patch set uses.)
> > 
> 
> We would only want to backport this patch. The rest is really out of scope.
> 
> > Which stable baseline(s) would need to be patched?  Priority?
> > 
> > Who does it?  (i.e. do you want me to submit patches to the stable baseline?)
> > 
> We would tag the patch for stable (and submit it into v4.16-rc). Greg would
> take care of the rest unless there are conflicts, in which case we get a note
> telling us that a backport is needed.
> 

Guenter,

Are you waiting for anything more from me on this patch, or are we
good for now until the back ports to v.15 etc.,?

Thanks

Jerry

-- 

-----------------------------------------------------------------------------
Jerry Hoemann                  Software Engineer   Hewlett Packard Enterprise
-----------------------------------------------------------------------------

Re: [PATCH v5 0/9] watchdog/hpwdt: Update driver to use watchdog core.

[Guenter Roeck]

On 02/25/2018 07:22 PM, Jerry Hoemann wrote:
> == v5 ==
> 
> Patch numbers changing due to squash and reorder.
> Using Prior (v4) Patch number noting *new* patch numbers as bullet item.
> 
> 
> Patch 0001 & 0004 Merged into: Update-Module-info-and-copyright
> 1) New Patch 0001
> 
> Patch 0002 Remove legacy NMI sourcing
> 1) Unmodified
> 
> Patch 0003 Update nmi_panic message.
> 1) Unmodified
> 
> Patch 0004 Update Module info.
> 1) Merged into 0001 above.
> 
> Patch 0005 Modify to use watchdog core
> 1) syntatic sugar.
> 2) Now Patch 0004
> 
> Patch 0006 condition-early-return-of-NMI-handler
> 1) Now Patch 0005
> 2) Casing on iLO5
> 3) true for boolean
> 
> Patch 0007 remove allow_kdump module param
> 1) Now patch 0006
> 
> Patch 0008 Programable Pretimeout NMI
> 1) Now patch 00007
> 
> Patch 0009 Add dynamic debug
> 1) Now patch 0008
> 
> Patch 0010 Update driver version.
> 1) Now Patch 0009
> 
> 
> == v4 ==
> 
> Modifications from prior version:
> 
> Manual merging and patch reordering has cause minor white
> space and diff changes.  Such change shouldn't have effected
> the logic and are omitted below:
> 
> Patch 001  Remove legacy NMI sourcing.
> 1) Updated patch documentation to reflect Spectre concerns.
> 2) Restored comment at #endif
>>>>>>>>>> 3) New Patch 0002  <<<<<<<<
> 
> Patch 002 watchdog/hpwdt: remove include files no longer
> 1) Squashed onto Patch001.
>>>>>>>>>> 3) Still Patch 0002  <<<<<<<<
> 
> 
> Patch 0003: watchdog/hpwdt: Update nmi_panic message.
> 1) Unchanged
> 
> 
> Patch 0004:  watchdog/hpwdt: white space changes
> 1) Redacted.
> 
> Patch 0005: watchdog/hpwdt: Update Module info.
> 1) Now Patch 0004
> 2) Update only module description to reflect branding changes.
> 3) Rest of patch redacted.
> 
> Patch 0006 Select CORE
> 1) Merged with 0007
> 
> Patch 0007 Modify to use watchdog core
> 1) squashed on Patch 0006.
> 2) Now Patch 0005
> 3) Requested white space changes.
> 4) Use "wdd" for watchdog_device like other watchdog drivers do
> 
> Patch 0008 Programable-Pretimeout
> 1) hpwdt_settimeout re-worked to take into account timeout <= pretimeout.
>     Patterned after watchdog_set_timeout.
> 2) hpwdt_set_pretimeout re-worked to take into account requested
>     pretimeout <= timeout.
> 3) Clarify patch doc to reflect above.
> 
> Patch 0009 condition-early-return-of-NMI
> 1) Requested syntatic surgar.
> 2) Now Patch 0006
> 
> Patch 0010 remove-allow_kdump
> 1) Now Patch 0007
> 
> Patch 0011 Add-dynamic-debug
> 1) Now Patch 0009
> 2) Typo fix
> 3) New dev_dbg resulting from changes to set timeout and pretimeout.
> 
> Patch 0012 Update driver version.
> 1) Now Patch 0010
> 
>   
> New Patch
> 1) Patch 0001 Update Copyright
> 
> 
> 
> == v3 ==
> 
> Incorperating code review feedback.
> 
> 1) Patch 0003: Use existing hex_byte_pack instead of creating new function.
> 2) Patch 0005: Redacted change in module_param permission.
> 3) Patch 0006: switch from pr_debug etc., to dev_dbg where possible.
> 4) Patch 0006: No longer updating soft_margin post module load.
> 5) Patch 0006: Initialize hpwdt_dev.parent before registering watchdog.
> 6) Patch 0006: Redacted change to dev_info message w.r.t. allow_kdump
> 7) Patch 0006 & 0007: Reorder patches to maintain bisectability.
> 8) Patch 0008: Change pr_debug to dev_dbg
> 9) Patch 0010: Change dev_info message w.r.t. allow_kdump where feature
> 		is removed.
> 
> Note, I am explicitly ignoring the checkpatch error on Patch 0008
> for specifying permisson of "0" instead of "0000".
> 
> == v2 ==
> 
> 1) Fix compiler error when CONFIG_HPWDT_NMI_DECODING is not defined.
> 
> 2) Break out driver version change to its own patch (0011).
> 
> 
> == v1 ==
> 
> The primary purposes of this patch set are to
> 
> 1) Update the hpwdt driver to use the watchdog core.
> 2) Reduce complexity by removing unnecessary features.
> 3) Add customer requested features like optional pretimeout.
> 4) Enhance readability/maintainability of the driver.
> 
> The size of the resultant driver is reduced from over 900
> lines to 350 lines.
> 
> Patch 1& 2 remove legacy NMI sourcing.
> Patch 3	adds useful indication of NMI cause to panic message
> Patch 4 & 5 are general cleanup
> Patch 6 & 7 updates the driver to user the watchdog core.
> Patch 8 makes the pretimeout NMI programmable.
> Patch 9 modifies whether the NMI handler claims the NMI.
> Patch 10 retracts the allow_kdump module parameter.
> 
> 
> 
> Jerry Hoemann (9):
>    watchdog/hpwdt: Update Module info and copyright.
>    watchdog/hpwdt: Remove legacy NMI sourcing.
>    watchdog/hpwdt: Update nmi_panic message.
>    watchdog/hpwdt: Modify to use watchdog core.
>    watchdog/hpwdt: condition early return of NMI handler on iLO5
>    watchdog/hpwdt: remove allow_kdump module parameter.
>    watchdog/hpwdt: Programable Pretimeout NMI
>    watchdog/hpwdt: Add dynamic debug
>    watchdog/hpwdt: Update driver version.
> 
>   drivers/watchdog/Kconfig |   1 +
>   drivers/watchdog/hpwdt.c | 791 +++++++----------------------------------------
>   2 files changed, 121 insertions(+), 671 deletions(-)
> 
For the series:

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Re: [PATCH v5 2/9] watchdog/hpwdt: Remove legacy NMI sourcing.

[Guenter Roeck]

On 02/28/2018 11:45 AM, Jerry Hoemann wrote:
> On Mon, Feb 26, 2018 at 05:29:55PM -0800, Guenter Roeck wrote:
>> On 02/26/2018 05:02 PM, Jerry Hoemann wrote:
>>> On Mon, Feb 26, 2018 at 06:32:30AM -0800, Guenter Roeck wrote:
>>>> On 02/26/2018 06:11 AM, Arnd Bergmann wrote:
>>>>> On Mon, Feb 26, 2018 at 4:22 AM, Jerry Hoemann <jerry.hoemann@hpe.com> wrote:
>>>>>> Gen8 and prior Proliant systems supported the "CRU" interface
>>>>>> to firmware.  This interfaces allows linux to "call back" into firmware
>>>>>> to source the cause of an NMI.  This feature isn't fully utilized
>>>>>> as the actual source of the NMI isn't printed, the driver only
>>>>>> indicates that the source couldn't be determined when the call
>>>>>> fails.
>>>>>>
>>>>>> With the advent of Gen9, iCRU replaces the CRU. The call back
>>>>>> feature is no longer available in firmware.  To be compatible and
>>>>>> not attempt to call back into firmware on system not supporting CRU,
>>>>>> the SMBIOS table is consulted to determine if it is safe to
>>>>>> make the call back or not.
>>>>>>
>>>>>> This results in about half of the driver code being devoted
>>>>>> to either making CRU calls or determing if it is safe to make
>>>>>> CRU calls.  As noted, the driver isn't really using the results of
>>>>>> the CRU calls.
>>>>>>
>>>>>> Furthermore, as a consequence of the Spectre security issue, the
>>>>>> BIOS/EFI calls are being wrapped into Spectre-disabling section.
>>>>>> Removing the call back in hpwdt_pretimeout assists in this effort.
>>>>>>
>>>>>> As the CRU sourcing of the NMI isn't required for handling the
>>>>>> NMI and there are security concerns with making the call back, remove
>>>>>> the legacy (pre Gen9) NMI sourcing and the DMI code to determine if
>>>>>> the system had the CRU interface.
>>>>>>
>>>>>> Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
>>>>>
>>>>> This avoids a warning in mainline kernels, so that's great:
>>>>>
>>>>> drivers/watchdog/hpwdt.o: warning: objtool: .text+0x24: indirect call
>>>>> found in RETPOLINE build
>>>>>
>>>>> I wonder what we do about stable kernels. Are both this patch and the patch
>>>>> that added the objtool warning message candidates for backports to
>>>>> stable kernels?
>>>>>
>>>>
>>>> Makes sense to me, but it is really a bit more than a bug fix, so I'll
>>>> leave it up to Jerry/HPE to make the call in respect to hpwdt.
>>>>
>>>
>>> Generally speaking, HPE customers who run linux do so through a distro
>>> vendor and pick up patches from them.  But I'm sure there are some
>>> customers who do things differently.
>>>
>>> The distro vendor's have their own repos and we'll work with them
>>> to back port patches to their code base.  So, I typically don't do a lot
>>> of kernel.org stable branch work.
>>>
>>> Looks like objtool has been enhanced to find Spectre vulnerable code.
>>> Are the other kernel patches related to Spectre being back ported
>>> to stable release lines?  If yes, it probably make sense to do
>>> the hpwdt change as well.
>>>
>>
>> Spectre has been backported to v4.4 and later. I don't know about earlier kernels.
>>
>>> Is just the patch removing the firmware call back wanted/needed? Or the
>>> whole driver rewrite?  (The older baseline don't have all the watchdog
>>> features that the patch set uses.)
>>>
>>
>> We would only want to backport this patch. The rest is really out of scope.
>>
>>> Which stable baseline(s) would need to be patched?  Priority?
>>>
>>> Who does it?  (i.e. do you want me to submit patches to the stable baseline?)
>>>
>> We would tag the patch for stable (and submit it into v4.16-rc). Greg would
>> take care of the rest unless there are conflicts, in which case we get a note
>> telling us that a backport is needed.
>>
> 
> Guenter,
> 
> Are you waiting for anything more from me on this patch, or are we
> good for now until the back ports to v.15 etc.,?
> 

We are good. I'll need to ask Wim to send a pull request to Linus.

Guenter