Page MenuHomeFreeBSD

Port sbsawdt (ARM SBSA generic watchdog) driver from NetBSD
Needs ReviewPublic

Authored by greg_unrelenting.technology on Jul 16 2019, 6:39 PM.

Details

Summary

Looking at NetBSD on Ampere eMAG dmesg I noticed sbsawdt0… and decided that we need this :)

sbsawdt0: <ARM SBSA Generic Watchdog> iomem 0xf0600000-0xf0600fff,0xf0610000-0xf0610fff on acpi0
sbsawdt0: default watchdog period is 172 seconds
Test Plan

Tested on: Marvell MACCHIATObin (Armada8k) with upstream EDK2 firmware

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

@greg_unrelenting.technology do you have a git repo available with your work in it? It'd be slightly easier for me to just add your repo and grab changes directly.

@greg_unrelenting.technology do you have a git repo available with your work in it? It'd be slightly easier for me to just add your repo and grab changes directly.

https://github.com/myfreeweb/freebsd — I don't always update it before posting on phab but right now it's up to date

I can confirm this attaches on my eMAG but I cannot verify the watchdog functionality - were you able to test it?

I can confirm this attaches on my eMAG but I cannot verify the watchdog functionality - were you able to test it?

Yeah, I couldn't get it to trip on the eMAG at Packet either, but it works on my MACCHIATObin (dropping to debugger with "watchdog timeout" description on the first interrupt and then rebooting after some more time).

Would be nice to test on ThunderX2 :)

Would be nice to test on ThunderX2 :)

Does not attach on ThunderX2 (no sbsawdt0 in dmesg).

Would be nice to test on ThunderX2 :)

Does not attach on ThunderX2 (no sbsawdt0 in dmesg).

oh.. https://www.mail-archive.com/kernel-packages@lists.launchpad.net/msg272248.html

I guess we don't have any more platforms to test..

Can you show a diff of the NetBSD version to this? i.e. what you had to change in the port?
(I assume that we don't want to treat this as contrib code and will just adopt it.)

Can you show a diff of the NetBSD version to this? i.e. what you had to change in the port?
(I assume that we don't want to treat this as contrib code and will just adopt it.)

ACPI attachment is different (because API is different) and I changed bus_space_map / bus_space_write_4 / etc to bus_alloc_resource / bus_write_4 / etc because we have resources and setting them via BUS_SET_RESOURCE when walking the ACPI table is rather convenient.

The part that touches the device (in sbsawdt_acpi_watchdog_fn) is pretty much the same (that function is only adapted to our watchdog API which calls one function with a cmd).

The original is https://github.com/NetBSD/src/blob/trunk/sys/arch/arm/acpi/sbsawdt_acpi.c — a true diff would show most things (except the defines) as different, it's better to just visually compare :)

The original is https://github.com/NetBSD/src/blob/trunk/sys/arch/arm/acpi/sbsawdt_acpi.c — a true diff would show most things (except the defines) as different, it's better to just visually compare :)

Thanks; my interest is really just in being able to track the version you started from, so we can review and manually apply any future changes/bug fixes from NetBSD. Maybe just add a comment along the lines of "Obtained from NetBSD sbsawdt_acpi.c r1.1" or such.

Would be nice to test on ThunderX2 :)

Does not attach on ThunderX2 (no sbsawdt0 in dmesg).

oh.. https://www.mail-archive.com/kernel-packages@lists.launchpad.net/msg272248.html

I guess we don't have any more platforms to test..

SBSA watchdog should work on ThunderX2, the specific issue was due to the linux driver not handling the full spec.
It is fixed here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=93ac3deb7c220cbcec032a967220a1f109d58431

SBSA watchdog should work on ThunderX2, the specific issue was due to the linux driver not handling the full spec.

But the firmware version I have here does not even expose it so I cannot test on FreeBSD. Is there a known-good firmware version I should install?

sys/arm64/arm64/sbsawdt_acpi.c
69–71

This is normally written as either

#define	 C_WCS_WS1		(1 << 2)
...

or

#define	 C_WCS_WS1		0x4
...
79–80

These should have a sc_ prefix for constancy.

158

What happens if the counter frequency is different on each CPU?

sys/arm64/arm64/sbsawdt_acpi.c
158

Is that even possible?

CNTFRQ_EL0 provides the "frequency of the **system** counter", looking at the timer guide it seems the system counter is explicitly global, not the same thing as per-core ones:

The Generic Timer includes a System Counter and set of per-core timers

Our generic_timer driver doesn't do anything per-core either:

#define	get_el0(x)	READ_SPECIALREG(x ##_el0)
static int get_freq(void) { return (get_el0(cntfrq)); }
static int arm_tmr_attach(device_t dev) { …
		sc->clkfreq = get_freq(); … }

Remove __BIT NetBSD-ism, add sc_ to all sc fields

oops, actually include buildable patch with *all* the renames done