linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] watchdog/hpwdt: Update driver to use watchdog core.
@ 2018-02-06 22:58 Jerry Hoemann
  2018-02-06 22:58 ` [PATCH 01/10] watchdog/hpwdt: Remove legacy NMI sourcing Jerry Hoemann
                   ` (9 more replies)
  0 siblings, 10 replies; 13+ messages in thread
From: Jerry Hoemann @ 2018-02-06 22:58 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, rwright, maurice.a.saldivar, Jerry Hoemann

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 (10):
  watchdog/hpwdt: Remove legacy NMI sourcing.
  watchdog/hpwdt: remove include files no longer needed.
  watchdog/hpwdt: Update nmi_panic message.
  watchdog/hpwdt: white space changes
  watchdog/hpwdt: Update Module info.
  watchdog/hpwdt: Modify to use watchdog core.
  watchdog/hpwdt: Select WATCHDOG_CORE
  watchdog/hpwdt: Programable Pretimeout NMI
  watchdog/hpwdt: condition early return of NMI handler on iLO5
  watchdog/hpwdt: remove allow_kdump module parameter.

 drivers/watchdog/Kconfig |   1 +
 drivers/watchdog/hpwdt.c | 843 ++++++++---------------------------------------
 2 files changed, 135 insertions(+), 709 deletions(-)

-- 
2.13.6

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 01/10] watchdog/hpwdt: Remove legacy NMI sourcing.
  2018-02-06 22:58 [PATCH 00/10] watchdog/hpwdt: Update driver to use watchdog core Jerry Hoemann
@ 2018-02-06 22:58 ` Jerry Hoemann
  2018-02-11 17:52   ` [01/10] " Guenter Roeck
  2018-02-11 17:56   ` Guenter Roeck
  2018-02-06 22:58 ` [PATCH 02/10] watchdog/hpwdt: remove include files no longer needed Jerry Hoemann
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 13+ messages in thread
From: Jerry Hoemann @ 2018-02-06 22:58 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, rwright, maurice.a.saldivar, 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.

As the CRU sourcing of the NMI isn't required for handling the
NMI, 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 | 485 +----------------------------------------------
 1 file changed, 3 insertions(+), 482 deletions(-)

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index f1f00dfc0e68..01ef52c13209 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -48,6 +48,7 @@
 static unsigned int soft_margin = DEFAULT_MARGIN;	/* in seconds */
 static unsigned int reload;			/* the computed soft_margin */
 static bool nowayout = WATCHDOG_NOWAYOUT;
+static unsigned int allow_kdump = 1;
 static char expect_release;
 static unsigned long hpwdt_is_open;
 
@@ -63,373 +64,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 +120,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 +291,10 @@ 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)
 {
 	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,33 +322,17 @@ 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;
 }
 
 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);
+#endif
 }
-#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;
-}
-
-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)
@@ -814,11 +340,6 @@ static int hpwdt_init_one(struct pci_dev *dev,
 	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.
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 02/10] watchdog/hpwdt: remove include files no longer needed.
  2018-02-06 22:58 [PATCH 00/10] watchdog/hpwdt: Update driver to use watchdog core Jerry Hoemann
  2018-02-06 22:58 ` [PATCH 01/10] watchdog/hpwdt: Remove legacy NMI sourcing Jerry Hoemann
@ 2018-02-06 22:58 ` Jerry Hoemann
  2018-02-06 22:58 ` [PATCH 03/10] watchdog/hpwdt: Update nmi_panic message Jerry Hoemann
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Jerry Hoemann @ 2018-02-06 22:58 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, rwright, maurice.a.saldivar, Jerry Hoemann

remove header files used by NMI sourcing and DMI decoding.

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

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 01ef52c13209..260740051084 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)
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 03/10] watchdog/hpwdt: Update nmi_panic message.
  2018-02-06 22:58 [PATCH 00/10] watchdog/hpwdt: Update driver to use watchdog core Jerry Hoemann
  2018-02-06 22:58 ` [PATCH 01/10] watchdog/hpwdt: Remove legacy NMI sourcing Jerry Hoemann
  2018-02-06 22:58 ` [PATCH 02/10] watchdog/hpwdt: remove include files no longer needed Jerry Hoemann
@ 2018-02-06 22:58 ` Jerry Hoemann
  2018-02-06 22:58 ` [PATCH 04/10] watchdog/hpwdt: white space changes Jerry Hoemann
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Jerry Hoemann @ 2018-02-06 22:58 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, rwright, maurice.a.saldivar, 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 | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 260740051084..7f55b9bf371e 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -106,24 +106,35 @@ static int hpwdt_my_nmi(void)
 	return ioread8(hpwdt_nmistat) & 0x6;
 }
 
+static inline int hexdigit(int v)
+{
+	return (v > 9) ? (v-9+'A') : (v+'0');
+}
+
 /*
  *	NMI Handler
  */
 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");
+	panic_msg[0] = hexdigit((mynmi>>4)&0xf);
+	panic_msg[1] = hexdigit(mynmi&0xf);
+
+	nmi_panic(regs, panic_msg);
 
 	return NMI_HANDLED;
 }
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 04/10] watchdog/hpwdt: white space changes
  2018-02-06 22:58 [PATCH 00/10] watchdog/hpwdt: Update driver to use watchdog core Jerry Hoemann
                   ` (2 preceding siblings ...)
  2018-02-06 22:58 ` [PATCH 03/10] watchdog/hpwdt: Update nmi_panic message Jerry Hoemann
@ 2018-02-06 22:58 ` Jerry Hoemann
  2018-02-06 22:58 ` [PATCH 05/10] watchdog/hpwdt: Update Module info Jerry Hoemann
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Jerry Hoemann @ 2018-02-06 22:58 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, rwright, maurice.a.saldivar, Jerry Hoemann

Minor white space changes and some name clean up.

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

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 7f55b9bf371e..a275f14bbcb0 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -1,11 +1,9 @@
 /*
  *	HPE WatchDog Driver
- *	based on
  *
- *	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>
+ *	Jerry Hoemann <jerry.hoemann@hpe.com>
  *
  *	This program is free software; you can redistribute it and/or
  *	modify it under the terms of the GNU General Public License
@@ -51,7 +49,7 @@ static unsigned long __iomem *hpwdt_timer_con;
 static const struct pci_device_id hpwdt_devices[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_COMPAQ, 0xB203) },	/* iLO2 */
 	{ PCI_DEVICE(PCI_VENDOR_ID_HP, 0x3306) },	/* iLO3 */
-	{0},			/* terminate list */
+	{0},						/* terminate list */
 };
 MODULE_DEVICE_TABLE(pci, hpwdt_devices);
 
@@ -100,7 +98,7 @@ static int hpwdt_time_left(void)
 	return TICKS_TO_SECS(ioread16(hpwdt_timer_reg));
 }
 
-#ifdef CONFIG_HPWDT_NMI_DECODING
+#ifdef CONFIG_HPWDT_NMI_DECODING	/* { */
 static int hpwdt_my_nmi(void)
 {
 	return ioread8(hpwdt_nmistat) & 0x6;
@@ -138,7 +136,7 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
 
 	return NMI_HANDLED;
 }
-#endif /* CONFIG_HPWDT_NMI_DECODING */
+#endif					/* } */
 
 /*
  *	/dev/watchdog handling
@@ -203,11 +201,11 @@ static ssize_t hpwdt_write(struct file *file, const char __user *data,
 	return len;
 }
 
-static const struct watchdog_info ident = {
-	.options = WDIOF_SETTIMEOUT |
-		   WDIOF_KEEPALIVEPING |
-		   WDIOF_MAGICCLOSE,
-	.identity = "HPE iLO2+ HW Watchdog Timer",
+static const struct watchdog_info hpwdt_info = {
+	.options	= WDIOF_SETTIMEOUT    |
+			  WDIOF_KEEPALIVEPING |
+			  WDIOF_MAGICCLOSE,
+	.identity	= "HPE iLO2+ HW Watchdog Timer",
 };
 
 static long hpwdt_ioctl(struct file *file, unsigned int cmd,
@@ -221,7 +219,7 @@ static long hpwdt_ioctl(struct file *file, unsigned int cmd,
 	switch (cmd) {
 	case WDIOC_GETSUPPORT:
 		ret = 0;
-		if (copy_to_user(argp, &ident, sizeof(ident)))
+		if (copy_to_user(argp, &hpwdt_info, sizeof(hpwdt_info)))
 			ret = -EFAULT;
 		break;
 
@@ -317,7 +315,7 @@ static int hpwdt_init_nmi_decoding(struct pci_dev *dev)
 	return 0;
 
 error2:
-	unregister_nmi_handler(NMI_SERR, "hpwdt");
+	unregister_nmi_handler(NMI_SERR,    "hpwdt");
 error1:
 	unregister_nmi_handler(NMI_UNKNOWN, "hpwdt");
 error:
@@ -329,15 +327,14 @@ static int hpwdt_init_nmi_decoding(struct pci_dev *dev)
 
 static void hpwdt_exit_nmi_decoding(void)
 {
-#ifdef CONFIG_HPWDT_NMI_DECODING
-	unregister_nmi_handler(NMI_UNKNOWN, "hpwdt");
-	unregister_nmi_handler(NMI_SERR, "hpwdt");
+#ifdef CONFIG_HPWDT_NMI_DECODING	/* { */
+	unregister_nmi_handler(NMI_UNKNOWN,  "hpwdt");
+	unregister_nmi_handler(NMI_SERR,     "hpwdt");
 	unregister_nmi_handler(NMI_IO_CHECK, "hpwdt");
-#endif
+#endif					/* } */
 }
 
-static int hpwdt_init_one(struct pci_dev *dev,
-					const struct pci_device_id *ent)
+static int hpwdt_probe(struct pci_dev *dev, const struct pci_device_id *ent)
 {
 	int retval;
 
@@ -424,10 +421,10 @@ static void hpwdt_exit(struct pci_dev *dev)
 }
 
 static struct pci_driver hpwdt_driver = {
-	.name = "hpwdt",
-	.id_table = hpwdt_devices,
-	.probe = hpwdt_init_one,
-	.remove = hpwdt_exit,
+	.name		= "hpwdt",
+	.id_table	= hpwdt_devices,
+	.probe		= hpwdt_probe,
+	.remove		= hpwdt_exit,
 };
 
 MODULE_AUTHOR("Tom Mingarelli");
@@ -442,9 +439,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
+#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					/* } */
 
 module_pci_driver(hpwdt_driver);
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 05/10] watchdog/hpwdt: Update Module info.
  2018-02-06 22:58 [PATCH 00/10] watchdog/hpwdt: Update driver to use watchdog core Jerry Hoemann
                   ` (3 preceding siblings ...)
  2018-02-06 22:58 ` [PATCH 04/10] watchdog/hpwdt: white space changes Jerry Hoemann
@ 2018-02-06 22:58 ` Jerry Hoemann
  2018-02-06 22:58 ` [PATCH 06/10] watchdog/hpwdt: Modify to use watchdog core Jerry Hoemann
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Jerry Hoemann @ 2018-02-06 22:58 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, rwright, maurice.a.saldivar, Jerry Hoemann

Update Module Author and permission on parameters so that the
parameters show up in sysfs.

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

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index a275f14bbcb0..3fae3119369f 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -427,20 +427,20 @@ static struct pci_driver hpwdt_driver = {
 	.remove		= hpwdt_exit,
 };
 
-MODULE_AUTHOR("Tom Mingarelli");
-MODULE_DESCRIPTION("hp watchdog driver");
+MODULE_AUTHOR("Jerry Hoemann");
+MODULE_DESCRIPTION("hpe watchdog driver");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(HPWDT_VERSION);
 
-module_param(soft_margin, int, 0);
+module_param(soft_margin, int, 0444);
 MODULE_PARM_DESC(soft_margin, "Watchdog timeout in seconds");
 
-module_param(nowayout, bool, 0);
+module_param(nowayout, bool, 0444);
 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_param(allow_kdump, int, 0444);
 MODULE_PARM_DESC(allow_kdump, "Start a kernel dump after NMI occurs");
 #endif					/* } */
 
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 06/10] watchdog/hpwdt: Modify to use watchdog core.
  2018-02-06 22:58 [PATCH 00/10] watchdog/hpwdt: Update driver to use watchdog core Jerry Hoemann
                   ` (4 preceding siblings ...)
  2018-02-06 22:58 ` [PATCH 05/10] watchdog/hpwdt: Update Module info Jerry Hoemann
@ 2018-02-06 22:58 ` Jerry Hoemann
  2018-02-06 22:58 ` [PATCH 07/10] watchdog/hpwdt: Select WATCHDOG_CORE Jerry Hoemann
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Jerry Hoemann @ 2018-02-06 22:58 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, rwright, maurice.a.saldivar, 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

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

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 3fae3119369f..a8e5c6542406 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -14,18 +14,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>
 
 #define HPWDT_VERSION			"1.4.0"
@@ -38,8 +33,6 @@ static unsigned int soft_margin = DEFAULT_MARGIN;	/* in seconds */
 static unsigned int reload;			/* the computed soft_margin */
 static bool nowayout = WATCHDOG_NOWAYOUT;
 static unsigned int allow_kdump = 1;
-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;
@@ -53,53 +46,58 @@ static const struct pci_device_id hpwdt_devices[] = {
 };
 MODULE_DEVICE_TABLE(pci, hpwdt_devices);
 
+static struct watchdog_device hpwdt_dev;
 
 /*
  *	Watchdog operations
  */
-static void hpwdt_start(void)
+static int hpwdt_start(struct watchdog_device *dev)
 {
-	reload = SECS_TO_TICKS(soft_margin);
+	reload = SECS_TO_TICKS(dev->timeout);
+
 	iowrite16(reload, hpwdt_timer_reg);
 	iowrite8(0x85, hpwdt_timer_con);
+
+	return 0;
 }
 
-static void hpwdt_stop(void)
+static int hpwdt_stop(struct watchdog_device *dev)
 {
 	unsigned long data;
 
 	data = ioread8(hpwdt_timer_con);
 	data &= 0xFE;
 	iowrite8(data, hpwdt_timer_con);
+	return 0;
 }
 
-static void hpwdt_ping(void)
-{
-	iowrite16(reload, hpwdt_timer_reg);
-}
-
-static int hpwdt_change_timer(int new_margin)
+static int hpwdt_ping(struct watchdog_device *dev)
 {
-	if (new_margin < 1 || new_margin > HPWDT_MAX_TIMER) {
-		pr_warn("New value passed in is invalid: %d seconds\n",
-			new_margin);
-		return -EINVAL;
-	}
+	reload = SECS_TO_TICKS(dev->timeout);
 
-	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 *dev)
 {
 	return TICKS_TO_SECS(ioread16(hpwdt_timer_reg));
 }
 
+static int hpwdt_settimeout(struct watchdog_device *dev, unsigned int val)
+{
+	pr_debug("settimeout = %d\n", val);
+
+	soft_margin = dev->timeout = val;
+	hpwdt_ping(dev);
+
+	return 0;
+}
+
 #ifdef CONFIG_HPWDT_NMI_DECODING	/* { */
-static int hpwdt_my_nmi(void)
+
+static unsigned int hpwdt_my_nmi(void)
 {
 	return ioread8(hpwdt_nmistat) & 0x6;
 }
@@ -126,8 +124,10 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
 	if ((ulReason == NMI_UNKNOWN) && !mynmi)
 		return NMI_DONE;
 
+	pr_debug("nmi: ulReason=%d, mynmi=0x%0x\n", ulReason, mynmi);
+
 	if (allow_kdump)
-		hpwdt_stop();
+		hpwdt_stop(&hpwdt_dev);
 
 	panic_msg[0] = hexdigit((mynmi>>4)&0xf);
 	panic_msg[1] = hexdigit(mynmi&0xf);
@@ -138,68 +138,6 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
 }
 #endif					/* } */
 
-/*
- *	/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 hpwdt_info = {
 	.options	= WDIOF_SETTIMEOUT    |
@@ -208,92 +146,13 @@ static const struct watchdog_info hpwdt_info = {
 	.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, &hpwdt_info, sizeof(hpwdt_info)))
-			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 struct miscdevice hpwdt_miscdev = {
-	.minor = WATCHDOG_MINOR,
-	.name = "watchdog",
-	.fops = &hpwdt_fops,
-};
-
 /*
  *	Init & Exit
  */
 
-
 static int hpwdt_init_nmi_decoding(struct pci_dev *dev)
 {
+#ifdef CONFIG_HPWDT_NMI_DECODING	/* { */
 	int retval;
 	/*
 	 * Only one function can register for NMI_UNKNOWN
@@ -308,10 +167,7 @@ static int hpwdt_init_nmi_decoding(struct pci_dev *dev)
 	if (retval)
 		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");
+	dev_info(&dev->dev, "HPE Watchdog Timer Driver: NMI decoding initialized");
 	return 0;
 
 error2:
@@ -323,6 +179,8 @@ static int hpwdt_init_nmi_decoding(struct pci_dev *dev)
 		"Unable to register a die notifier (err=%d).\n",
 		retval);
 	return retval;
+#endif					/* } */
+	return 0;
 }
 
 static void hpwdt_exit_nmi_decoding(void)
@@ -376,31 +234,32 @@ static int hpwdt_probe(struct pci_dev *dev, const struct pci_device_id *ent)
 	hpwdt_timer_con = pci_mem_addr + 0x72;
 
 	/* 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);
+	hpwdt_stop(&hpwdt_dev);
 
 	/* Initialize NMI Decoding functionality */
 	retval = hpwdt_init_nmi_decoding(dev);
 	if (retval != 0)
 		goto error_init_nmi_decoding;
 
-	retval = misc_register(&hpwdt_miscdev);
+	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_warn(&dev->dev, "Unable to register hpe watchdog (err=%d).\n", retval);
+		goto error_wd_register;
+	}
+
+	watchdog_set_nowayout(&hpwdt_dev, nowayout);
+	if (watchdog_init_timeout(&hpwdt_dev, soft_margin, NULL)) {
+		dev_warn(&dev->dev, "Invalid soft_margin: %d. Using default\n", soft_margin);
+		soft_margin = DEFAULT_MARGIN;
 	}
 
 	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);
@@ -412,9 +271,9 @@ static int hpwdt_probe(struct pci_dev *dev, const struct pci_device_id *ent)
 static void hpwdt_exit(struct pci_dev *dev)
 {
 	if (!nowayout)
-		hpwdt_stop();
+		hpwdt_stop(&hpwdt_dev);
 
-	misc_deregister(&hpwdt_miscdev);
+	watchdog_unregister_device(&hpwdt_dev);
 	hpwdt_exit_nmi_decoding();
 	pci_iounmap(dev, pci_mem_addr);
 	pci_disable_device(dev);
@@ -427,6 +286,25 @@ static struct pci_driver hpwdt_driver = {
 	.remove		= hpwdt_exit,
 };
 
+
+static const struct watchdog_ops hpwdt_ops = {
+	.owner		= THIS_MODULE,
+	.start		= hpwdt_start,
+	.stop		= hpwdt_stop,
+	.ping		= hpwdt_ping,
+	.set_timeout	= hpwdt_settimeout,
+	.get_timeleft	= hpwdt_gettimeleft,
+};
+
+static struct watchdog_device hpwdt_dev = {
+	.info		= &hpwdt_info,
+	.ops		= &hpwdt_ops,
+	.min_timeout	= 1,
+	.max_timeout	= HPWDT_MAX_TIMER,
+	.timeout	= DEFAULT_MARGIN,
+};
+
+
 MODULE_AUTHOR("Jerry Hoemann");
 MODULE_DESCRIPTION("hpe watchdog driver");
 MODULE_LICENSE("GPL");
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 07/10] watchdog/hpwdt: Select WATCHDOG_CORE
  2018-02-06 22:58 [PATCH 00/10] watchdog/hpwdt: Update driver to use watchdog core Jerry Hoemann
                   ` (5 preceding siblings ...)
  2018-02-06 22:58 ` [PATCH 06/10] watchdog/hpwdt: Modify to use watchdog core Jerry Hoemann
@ 2018-02-06 22:58 ` Jerry Hoemann
  2018-02-06 22:58 ` [PATCH 08/10] watchdog/hpwdt: Programable Pretimeout NMI Jerry Hoemann
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Jerry Hoemann @ 2018-02-06 22:58 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, rwright, maurice.a.saldivar, Jerry Hoemann

Update Kconfig file to show that hpwdt now selects WATCHDOG_CORE.

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

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 6a602f70aaa4..4d219c3fa8b4 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1108,6 +1108,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
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 08/10] watchdog/hpwdt: Programable Pretimeout NMI
  2018-02-06 22:58 [PATCH 00/10] watchdog/hpwdt: Update driver to use watchdog core Jerry Hoemann
                   ` (6 preceding siblings ...)
  2018-02-06 22:58 ` [PATCH 07/10] watchdog/hpwdt: Select WATCHDOG_CORE Jerry Hoemann
@ 2018-02-06 22:58 ` Jerry Hoemann
  2018-02-06 22:58 ` [PATCH 09/10] watchdog/hpwdt: condition early return of NMI handler on iLO5 Jerry Hoemann
  2018-02-06 22:58 ` [PATCH 10/10] watchdog/hpwdt: remove allow_kdump module parameter Jerry Hoemann
  9 siblings, 0 replies; 13+ messages in thread
From: Jerry Hoemann @ 2018-02-06 22:58 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, rwright, maurice.a.saldivar, 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.

Hence, in hpwdt_set_pretimeout, val == 0 disables the NMI. Any
non-zero value sets the pretimeout length to what the hardware
supports.

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

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index a8e5c6542406..1edbcb8f1537 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -23,16 +23,21 @@
 
 #include <asm/nmi.h>
 
-#define HPWDT_VERSION			"1.4.0"
+#define HPWDT_VERSION			"1.5.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)
 #define DEFAULT_MARGIN			30
+#define PRETIMEOUT_SEC			9
 
 static unsigned int soft_margin = DEFAULT_MARGIN;	/* in seconds */
-static unsigned int reload;			/* the computed soft_margin */
 static bool nowayout = WATCHDOG_NOWAYOUT;
 static unsigned int allow_kdump = 1;
+#ifdef CONFIG_HPWDT_NMI_DECODING	/* { */
+static bool pretimeout = 1;
+#else
+static bool pretimeout;
+#endif					/* } */
 
 static void __iomem *pci_mem_addr;		/* the PCI-memory address */
 static unsigned long __iomem *hpwdt_nmistat;
@@ -53,10 +58,12 @@ static struct watchdog_device hpwdt_dev;
  */
 static int hpwdt_start(struct watchdog_device *dev)
 {
-	reload = SECS_TO_TICKS(dev->timeout);
+	int control = 0x81 | (pretimeout ? 0x4 : 0);
+	int reload = SECS_TO_TICKS(dev->timeout);
 
+	pr_debug("start watchdog 0x%08x:0x%02x\n", reload, control);
 	iowrite16(reload, hpwdt_timer_reg);
-	iowrite8(0x85, hpwdt_timer_con);
+	iowrite8(control, hpwdt_timer_con);
 
 	return 0;
 }
@@ -65,6 +72,8 @@ static int hpwdt_stop(struct watchdog_device *dev)
 {
 	unsigned long data;
 
+	pr_debug("stop  watchdog\n");
+
 	data = ioread8(hpwdt_timer_con);
 	data &= 0xFE;
 	iowrite8(data, hpwdt_timer_con);
@@ -73,8 +82,9 @@ static int hpwdt_stop(struct watchdog_device *dev)
 
 static int hpwdt_ping(struct watchdog_device *dev)
 {
-	reload = SECS_TO_TICKS(dev->timeout);
+	int reload = SECS_TO_TICKS(dev->timeout);
 
+	pr_debug("ping  watchdog 0x%08x\n", reload);
 	iowrite16(reload, hpwdt_timer_reg);
 
 	return 0;
@@ -96,6 +106,16 @@ static int hpwdt_settimeout(struct watchdog_device *dev, unsigned int val)
 }
 
 #ifdef CONFIG_HPWDT_NMI_DECODING	/* { */
+static int hpwdt_set_pretimeout(struct watchdog_device *dev, unsigned int val)
+{
+	if (val && (val != PRETIMEOUT_SEC)) {
+		pr_info("Setting pretimeout to %d\n", PRETIMEOUT_SEC);
+		val = PRETIMEOUT_SEC;
+	}
+	dev->pretimeout = val;
+	pretimeout = val ? 1 : 0;
+	return 0;
+}
 
 static unsigned int hpwdt_my_nmi(void)
 {
@@ -106,7 +126,6 @@ static inline int hexdigit(int v)
 {
 	return (v > 9) ? (v-9+'A') : (v+'0');
 }
-
 /*
  *	NMI Handler
  */
@@ -126,6 +145,9 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
 
 	pr_debug("nmi: ulReason=%d, mynmi=0x%0x\n", ulReason, mynmi);
 
+	if (!pretimeout)
+		return NMI_DONE;
+
 	if (allow_kdump)
 		hpwdt_stop(&hpwdt_dev);
 
@@ -142,7 +164,8 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
 static const struct watchdog_info hpwdt_info = {
 	.options	= WDIOF_SETTIMEOUT    |
 			  WDIOF_KEEPALIVEPING |
-			  WDIOF_MAGICCLOSE,
+			  WDIOF_MAGICCLOSE    |
+			  WDIOF_PRETIMEOUT,
 	.identity	= "HPE iLO2+ HW Watchdog Timer",
 };
 
@@ -294,6 +317,7 @@ static const struct watchdog_ops hpwdt_ops = {
 	.ping		= hpwdt_ping,
 	.set_timeout	= hpwdt_settimeout,
 	.get_timeleft	= hpwdt_gettimeleft,
+	.set_pretimeout	= hpwdt_set_pretimeout,
 };
 
 static struct watchdog_device hpwdt_dev = {
@@ -302,6 +326,7 @@ static struct watchdog_device hpwdt_dev = {
 	.min_timeout	= 1,
 	.max_timeout	= HPWDT_MAX_TIMER,
 	.timeout	= DEFAULT_MARGIN,
+	.pretimeout	= PRETIMEOUT_SEC,
 };
 
 
@@ -320,6 +345,9 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
 #ifdef CONFIG_HPWDT_NMI_DECODING	/* { */
 module_param(allow_kdump, int, 0444);
 MODULE_PARM_DESC(allow_kdump, "Start a kernel dump after NMI occurs");
+
+module_param(pretimeout, bool, 0444);
+MODULE_PARM_DESC(pretimeout, "Watchdog pretimeout enabled");
 #endif					/* } */
 
 module_pci_driver(hpwdt_driver);
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 09/10] watchdog/hpwdt: condition early return of NMI handler on iLO5
  2018-02-06 22:58 [PATCH 00/10] watchdog/hpwdt: Update driver to use watchdog core Jerry Hoemann
                   ` (7 preceding siblings ...)
  2018-02-06 22:58 ` [PATCH 08/10] watchdog/hpwdt: Programable Pretimeout NMI Jerry Hoemann
@ 2018-02-06 22:58 ` Jerry Hoemann
  2018-02-06 22:58 ` [PATCH 10/10] watchdog/hpwdt: remove allow_kdump module parameter Jerry Hoemann
  9 siblings, 0 replies; 13+ messages in thread
From: Jerry Hoemann @ 2018-02-06 22:58 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, rwright, maurice.a.saldivar, 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 | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 1edbcb8f1537..9bfb668115f8 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -33,6 +33,7 @@
 static unsigned int soft_margin = DEFAULT_MARGIN;	/* in seconds */
 static bool nowayout = WATCHDOG_NOWAYOUT;
 static unsigned int allow_kdump = 1;
+static bool iLO5;
 #ifdef CONFIG_HPWDT_NMI_DECODING	/* { */
 static bool pretimeout = 1;
 #else
@@ -140,14 +141,14 @@ 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)
-		return NMI_DONE;
-
 	pr_debug("nmi: ulReason=%d, mynmi=0x%0x\n", ulReason, mynmi);
 
 	if (!pretimeout)
 		return NMI_DONE;
 
+	if (iLO5 && (ulReason == NMI_UNKNOWN) && !mynmi)
+		return NMI_DONE;
+
 	if (allow_kdump)
 		hpwdt_stop(&hpwdt_dev);
 
@@ -280,6 +281,9 @@ static int hpwdt_probe(struct pci_dev *dev, const struct pci_device_id *ent)
 			", timer margin: %d seconds (nowayout=%d).\n",
 			HPWDT_VERSION, hpwdt_dev.timeout, nowayout);
 
+	if (dev->subsystem_vendor == PCI_VENDOR_ID_HP_3PAR)
+		iLO5 = 1;
+
 	return 0;
 
 error_wd_register:
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 10/10] watchdog/hpwdt: remove allow_kdump module parameter.
  2018-02-06 22:58 [PATCH 00/10] watchdog/hpwdt: Update driver to use watchdog core Jerry Hoemann
                   ` (8 preceding siblings ...)
  2018-02-06 22:58 ` [PATCH 09/10] watchdog/hpwdt: condition early return of NMI handler on iLO5 Jerry Hoemann
@ 2018-02-06 22:58 ` Jerry Hoemann
  9 siblings, 0 replies; 13+ messages in thread
From: Jerry Hoemann @ 2018-02-06 22:58 UTC (permalink / raw)
  To: wim, linux
  Cc: linux-watchdog, linux-kernel, rwright, maurice.a.saldivar, 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 | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 9bfb668115f8..f767ea4dba86 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -32,7 +32,6 @@
 
 static unsigned int soft_margin = DEFAULT_MARGIN;	/* in seconds */
 static bool nowayout = WATCHDOG_NOWAYOUT;
-static unsigned int allow_kdump = 1;
 static bool iLO5;
 #ifdef CONFIG_HPWDT_NMI_DECODING	/* { */
 static bool pretimeout = 1;
@@ -149,8 +148,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_dev);
+	hpwdt_stop(&hpwdt_dev);
 
 	panic_msg[0] = hexdigit((mynmi>>4)&0xf);
 	panic_msg[1] = hexdigit(mynmi&0xf);
@@ -347,9 +345,6 @@ 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, 0444);
-MODULE_PARM_DESC(allow_kdump, "Start a kernel dump after NMI occurs");
-
 module_param(pretimeout, bool, 0444);
 MODULE_PARM_DESC(pretimeout, "Watchdog pretimeout enabled");
 #endif					/* } */
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [01/10] watchdog/hpwdt: Remove legacy NMI sourcing.
  2018-02-06 22:58 ` [PATCH 01/10] watchdog/hpwdt: Remove legacy NMI sourcing Jerry Hoemann
@ 2018-02-11 17:52   ` Guenter Roeck
  2018-02-11 17:56   ` Guenter Roeck
  1 sibling, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2018-02-11 17:52 UTC (permalink / raw)
  To: Jerry Hoemann
  Cc: wim, linux-watchdog, linux-kernel, rwright, maurice.a.saldivar

On Tue, Feb 06, 2018 at 03:58:46PM -0700, Jerry Hoemann 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.
> 
> As the CRU sourcing of the NMI isn't required for handling the
> NMI, 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>

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

> ---
>  drivers/watchdog/hpwdt.c | 485 +----------------------------------------------
>  1 file changed, 3 insertions(+), 482 deletions(-)
> 
> diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
> index f1f00dfc0e68..01ef52c13209 100644
> --- a/drivers/watchdog/hpwdt.c
> +++ b/drivers/watchdog/hpwdt.c
> @@ -48,6 +48,7 @@
>  static unsigned int soft_margin = DEFAULT_MARGIN;	/* in seconds */
>  static unsigned int reload;			/* the computed soft_margin */
>  static bool nowayout = WATCHDOG_NOWAYOUT;
> +static unsigned int allow_kdump = 1;
>  static char expect_release;
>  static unsigned long hpwdt_is_open;
>  
> @@ -63,373 +64,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 +120,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 +291,10 @@ 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)
>  {
>  	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,33 +322,17 @@ 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;
>  }
>  
>  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);
> +#endif
>  }
> -#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;
> -}
> -
> -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)
> @@ -814,11 +340,6 @@ static int hpwdt_init_one(struct pci_dev *dev,
>  	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.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [01/10] watchdog/hpwdt: Remove legacy NMI sourcing.
  2018-02-06 22:58 ` [PATCH 01/10] watchdog/hpwdt: Remove legacy NMI sourcing Jerry Hoemann
  2018-02-11 17:52   ` [01/10] " Guenter Roeck
@ 2018-02-11 17:56   ` Guenter Roeck
  1 sibling, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2018-02-11 17:56 UTC (permalink / raw)
  To: Jerry Hoemann
  Cc: wim, linux-watchdog, linux-kernel, rwright, maurice.a.saldivar

On Tue, Feb 06, 2018 at 03:58:46PM -0700, Jerry Hoemann 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.
> 
> As the CRU sourcing of the NMI isn't required for handling the
> NMI, 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>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Spoke too early. This fails to compile if CONFIG_HPWDT_NMI_DECODING
is disabled. Please fix and resubmit the series, and please make sure
that each patch compiles on its own.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-02-11 17:56 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-06 22:58 [PATCH 00/10] watchdog/hpwdt: Update driver to use watchdog core Jerry Hoemann
2018-02-06 22:58 ` [PATCH 01/10] watchdog/hpwdt: Remove legacy NMI sourcing Jerry Hoemann
2018-02-11 17:52   ` [01/10] " Guenter Roeck
2018-02-11 17:56   ` Guenter Roeck
2018-02-06 22:58 ` [PATCH 02/10] watchdog/hpwdt: remove include files no longer needed Jerry Hoemann
2018-02-06 22:58 ` [PATCH 03/10] watchdog/hpwdt: Update nmi_panic message Jerry Hoemann
2018-02-06 22:58 ` [PATCH 04/10] watchdog/hpwdt: white space changes Jerry Hoemann
2018-02-06 22:58 ` [PATCH 05/10] watchdog/hpwdt: Update Module info Jerry Hoemann
2018-02-06 22:58 ` [PATCH 06/10] watchdog/hpwdt: Modify to use watchdog core Jerry Hoemann
2018-02-06 22:58 ` [PATCH 07/10] watchdog/hpwdt: Select WATCHDOG_CORE Jerry Hoemann
2018-02-06 22:58 ` [PATCH 08/10] watchdog/hpwdt: Programable Pretimeout NMI Jerry Hoemann
2018-02-06 22:58 ` [PATCH 09/10] watchdog/hpwdt: condition early return of NMI handler on iLO5 Jerry Hoemann
2018-02-06 22:58 ` [PATCH 10/10] watchdog/hpwdt: remove allow_kdump module parameter Jerry Hoemann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).