Re: FW: Re: watchdog suport for new hardware

2016-05-04 Thread Chase Davis 
This was added to GENERIC:

sel* at acpi?

and these four lines were added to files.acpi:
# SEL embedded controller
device sel
attach sel at acpi with sel_acpi
file dev/acpi/sel_acpi.c sel_acpi

With SEL0002 being the first item in the array, shouldn't it at least
match it if it exists? We will add NULL to the HID array and see what
happens.

On Tue, May 3, 2016 at 5:29 PM, Mike Larkin  wrote:
> On Tue, May 03, 2016 at 03:32:34PM -0500, Chase Davis wrote:
>> Mike,
>>
>> We took your suggestion and re-wrote the driver to model sdhc_acpi. I
>> have attached the new code. However, the match function never returns
>> a 1. We put temporary print statements in the match routine. It is
>> being called several times during the kernel boot process, but it
>> never finds a device with HID SEL0002. Do you have any suggestions for
>> why it would show up in the DSDT tables when we do an acpidump, but
>> that the match routine never finds it?
>>
>> This is a snippet from the dmesg boot log
>>
>> Attempting to match SEL: result (0)
>> acpitimer0 at acpi0: 3579545 Hz, 24 bits
>> Attempting to match SEL: result (0)
>> Attempting to match SEL: result (0)
>> Attempting to match SEL: result (0)
>> Attempting to match SEL: result (0)
>> acpihpet0 at acpi0: 14318179 Hz
>> Attempting to match SEL: result (0)
>> acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
>>
>> int
>> sel_acpi_match(struct device *parent, void *match, void *aux)
>> {
>> struct acpi_attach_args *aaa = aux;
>> struct cfdata *cf = match;
>> printf("Attempting to match SEL: ");
>> int res = acpi_matchhids(aaa, sel_hids, cf->cf_driver->cd_name);
>> printf("result (%d)\n", res);
>> return res;
>> }
>>
>> Thanks,
>> Chase
>
> I think this diff is missing some parts. Like GENERIC?
>
> Also, you need a NULL after your last HID or matchhids will walk all
> through memory until it hits a NULL.
>
> -ml
>
>> /*$OpenBSD: sel.c,v 1.0 2016/04/01 05:00:00 jsg Exp $ */
>> /*
>>  * Copyright (c) 2016 PREMIER System Integrators
>>  *
>>  * Permission to use, copy, modify, and distribute this software for any
>>  * purpose with or without fee is hereby granted, provided that the above
>>  * copyright notice and this permission notice appear in all copies.
>>  *
>>  * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
>>  * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
>>  * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
>>  * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
>>  * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
>>  * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
>>  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>>  *
>>  * Schweitzer Engineering Laboratories: SEL-3355 Embedded controller
>> */
>>
>> #include 
>> #include 
>> #include 
>> #include 
>> #include 
>> #include 
>>
>> #include 
>>
>> #include 
>> #include 
>> #include 
>> #include 
>> #include 
>>
>> #include 
>>
>> struct sel_host;
>>
>> struct sel_softc {
>>   /* sc_dev must be the first item in the struct */
>>   struct device   sc_dev;
>>   struct sel_host **sc_host;
>> };
>>
>> struct sel_acpi_softc {
>>   struct sel_softcsc;
>>   struct acpi_softc   *sc_acpi;
>>   struct aml_node *sc_node;
>>
>>   /* device access through bus space */
>>   bus_space_tag_t sc_iot;
>>   bus_space_handle_t  sc_ioh;
>>   bus_addr_t  sc_addr;
>>   bus_size_t  sc_size;
>>
>>   struct sel_host *sc_host;
>> };
>>
>> int sel_wait(bus_space_tag_t, bus_space_handle_t, bool);
>>
>> /* Autoconfiguration glue */
>> int sel_acpi_match(struct device *, void *, void *);
>> void sel_acpi_attach(struct device *, struct device *, void *);
>> int sel_print(void *, const char *);
>> int sel_wd_cb(void *, int);
>>
>> /* functions to interact with the controller */
>> void sel_write_wdog(bus_space_tag_t, bus_space_handle_t, int);
>> u_int8_t sel_read_wdog(bus_space_tag_t, bus_space_handle_t);
>> u_int8_t sel_read_status(bus_space_tag_t, bus_space_handle_t);
>> void sel_abort(bus_space_tag_t, bus_space_handle_t);
>> u_int32_t sel_read_boardid(bus_space_tag_t, bus_space_handle_t);
>> u_int32_t sel_read_mcversion(bus_space_tag_t, bus_space_handle_t);
>> u_int16_t sel_read_pciboardid(bus_space_tag_t, bus_space_handle_t);
>> u_int8_t sel_read_ledctl0(bus_space_tag_t, bus_space_handle_t);
>> void sel_write_ledctl0(bus_space_tag_t, bus_space_handle_t, u_int8_t);
>> u_int8_t sel_read_ledctl1(bus_space_tag_t, bus_space_handle_t);
>> void sel_write_ledctl1(bus_space_tag_t, bus_space_handle_t, u_int8_t);
>> u_int8_t sel_read_miscctl0(bus_space_tag_t, bus_space_handle_t);
>> u_int8_t sel_read_miscctl1(bus_space_tag_t, bus_space_handle_t);
>> void sel_read_modelno(bus_space_tag_t, bus_space_handle_t, char*);
>> void

Re: FW: Re: watchdog suport for new hardware

2016-05-03 Thread Mike Larkin 
On Tue, May 03, 2016 at 03:32:34PM -0500, Chase Davis wrote:
> Mike,
> 
> We took your suggestion and re-wrote the driver to model sdhc_acpi. I
> have attached the new code. However, the match function never returns
> a 1. We put temporary print statements in the match routine. It is
> being called several times during the kernel boot process, but it
> never finds a device with HID SEL0002. Do you have any suggestions for
> why it would show up in the DSDT tables when we do an acpidump, but
> that the match routine never finds it?
> 
> This is a snippet from the dmesg boot log
> 
> Attempting to match SEL: result (0)
> acpitimer0 at acpi0: 3579545 Hz, 24 bits
> Attempting to match SEL: result (0)
> Attempting to match SEL: result (0)
> Attempting to match SEL: result (0)
> Attempting to match SEL: result (0)
> acpihpet0 at acpi0: 14318179 Hz
> Attempting to match SEL: result (0)
> acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
> 
> int
> sel_acpi_match(struct device *parent, void *match, void *aux)
> {
> struct acpi_attach_args *aaa = aux;
> struct cfdata *cf = match;
> printf("Attempting to match SEL: ");
> int res = acpi_matchhids(aaa, sel_hids, cf->cf_driver->cd_name);
> printf("result (%d)\n", res);
> return res;
> }
> 
> Thanks,
> Chase

I think this diff is missing some parts. Like GENERIC?

Also, you need a NULL after your last HID or matchhids will walk all
through memory until it hits a NULL.

-ml

> /*$OpenBSD: sel.c,v 1.0 2016/04/01 05:00:00 jsg Exp $ */
> /*
>  * Copyright (c) 2016 PREMIER System Integrators
>  *
>  * Permission to use, copy, modify, and distribute this software for any
>  * purpose with or without fee is hereby granted, provided that the above
>  * copyright notice and this permission notice appear in all copies.
>  *
>  * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
>  * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
>  * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
>  * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
>  * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
>  * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
>  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>  *
>  * Schweitzer Engineering Laboratories: SEL-3355 Embedded controller
> */
> 
> #include 
> #include 
> #include 
> #include 
> #include 
> #include 
> 
> #include 
> 
> #include 
> #include 
> #include 
> #include 
> #include 
> 
> #include 
> 
> struct sel_host;
> 
> struct sel_softc {
>   /* sc_dev must be the first item in the struct */
>   struct device   sc_dev;
>   struct sel_host **sc_host;
> };
> 
> struct sel_acpi_softc {
>   struct sel_softcsc;
>   struct acpi_softc   *sc_acpi;
>   struct aml_node *sc_node;
> 
>   /* device access through bus space */
>   bus_space_tag_t sc_iot;
>   bus_space_handle_t  sc_ioh;
>   bus_addr_t  sc_addr;
>   bus_size_t  sc_size;
> 
>   struct sel_host *sc_host;
> };
> 
> int sel_wait(bus_space_tag_t, bus_space_handle_t, bool);
> 
> /* Autoconfiguration glue */
> int sel_acpi_match(struct device *, void *, void *);
> void sel_acpi_attach(struct device *, struct device *, void *);
> int sel_print(void *, const char *);
> int sel_wd_cb(void *, int);
> 
> /* functions to interact with the controller */
> void sel_write_wdog(bus_space_tag_t, bus_space_handle_t, int);
> u_int8_t sel_read_wdog(bus_space_tag_t, bus_space_handle_t);
> u_int8_t sel_read_status(bus_space_tag_t, bus_space_handle_t);
> void sel_abort(bus_space_tag_t, bus_space_handle_t);
> u_int32_t sel_read_boardid(bus_space_tag_t, bus_space_handle_t);
> u_int32_t sel_read_mcversion(bus_space_tag_t, bus_space_handle_t);
> u_int16_t sel_read_pciboardid(bus_space_tag_t, bus_space_handle_t);
> u_int8_t sel_read_ledctl0(bus_space_tag_t, bus_space_handle_t);
> void sel_write_ledctl0(bus_space_tag_t, bus_space_handle_t, u_int8_t);
> u_int8_t sel_read_ledctl1(bus_space_tag_t, bus_space_handle_t);
> void sel_write_ledctl1(bus_space_tag_t, bus_space_handle_t, u_int8_t);
> u_int8_t sel_read_miscctl0(bus_space_tag_t, bus_space_handle_t);
> u_int8_t sel_read_miscctl1(bus_space_tag_t, bus_space_handle_t);
> void sel_read_modelno(bus_space_tag_t, bus_space_handle_t, char*);
> void sel_read_serialno(bus_space_tag_t, bus_space_handle_t, char*);
> void sel_read_configid(bus_space_tag_t, bus_space_handle_t, char*);
> 
> /* macros to extract bits from the status register */
> #define EC_STATUS_IBF(status) (((status) >> 0x1) & 0x1)
> #define EC_STATUS_OBF(status) (((status) & 0x1))
> #define EC_STATUS_BUSY(status)(((status) >> 0x4) & 0x1)
> #define EC_STATUS_STATE(status)   (((status) >> 0x6) & 0x3)
> 
> struct cfattach sel_acpi_ca = {
>   sizeof(struct sel_acpi_softc), sel_acpi_match, sel_acpi_attach
> };
> 
> struct

Re: FW: Re: watchdog suport for new hardware

2016-05-03 Thread Chase Davis 
Mike,

We took your suggestion and re-wrote the driver to model sdhc_acpi. I
have attached the new code. However, the match function never returns
a 1. We put temporary print statements in the match routine. It is
being called several times during the kernel boot process, but it
never finds a device with HID SEL0002. Do you have any suggestions for
why it would show up in the DSDT tables when we do an acpidump, but
that the match routine never finds it?

This is a snippet from the dmesg boot log

Attempting to match SEL: result (0)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
Attempting to match SEL: result (0)
Attempting to match SEL: result (0)
Attempting to match SEL: result (0)
Attempting to match SEL: result (0)
acpihpet0 at acpi0: 14318179 Hz
Attempting to match SEL: result (0)
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat

int
sel_acpi_match(struct device *parent, void *match, void *aux)
{
struct acpi_attach_args *aaa = aux;
struct cfdata *cf = match;
printf("Attempting to match SEL: ");
int res = acpi_matchhids(aaa, sel_hids, cf->cf_driver->cd_name);
printf("result (%d)\n", res);
return res;
}

Thanks,
Chase

[demime 1.01d removed an attachment of type text/x-csrc which had a name of 
sel_acpi.c]

[demime 1.01d removed an attachment of type text/x-chdr which had a name of 
selreg.h]

Re: FW: Re: watchdog suport for new hardware

2016-04-28 Thread stan 
On Thu, Apr 28, 2016 at 02:18:25PM +0100, Stuart Henderson wrote:
> On 2016/04/28 08:56, stan wrote:
> > On Thu, Apr 28, 2016 at 08:44:49AM +0100, Stuart Henderson wrote:
> > > Stan, can you send the information that is output when you run
> > > sendbug -P as root? Just putting the whole thing inline in a
> > > reply-to-all to this mail would be fine. Please add "sysctl hw"
> > > output as well. Ideally we want a way to identify the watchdog
> > > itself rather than the general machine type etc. which is why
> > > I'm hoping they follow Microsoft's spec (which is the de-facto 
> > > standard for this).
> > 
> > 
> > Sorry got distracted and frgot to cc the list.
> 
> OK, pity, there doesn't seem to be anything to properly identify
> the watchdog in acpi tables. Just the vendor-specific thing which
> needs reading to figure things out further. If they had followed
> the usual spec then the driver would have been *very* generally
> useful.
> 
> In that case maybe the approach would be to do something similar
> to acpithinkpad, but matching SECD instead of MHKV, and then
> looking for the SEL0002 HID. But I only know a bit about how
> to find my way round the decompiled files, so ignore me if
> a real ACPI hacker steps in with a better idea ;)
> 
> > hw.vendor=Schweitzer Engineering Laboratories, Inc.
> > hw.product=SEL-3355
> 
> An alternative might be to match on vendor/product, see the last
> commit to sys/dev/ic/re.c for how to do this, but then you're
> having to look at fixed addresses which they seem to be providing
> via acpi.
> 

As I look at what the vendor did, I discover they were working in the
arch/i386 codebase. 2 questions. First should this not be in amd64, as this
is a 64 bit machine, and if so does that change any of the discussions as to
how to detect the hardware?


-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

Re: FW: Re: watchdog suport for new hardware

2016-04-28 Thread Mike Larkin 
On Thu, Apr 28, 2016 at 03:22:15PM +, Stuart Henderson wrote:
> On 2016-04-28, stan  wrote:
> > On Thu, Apr 28, 2016 at 02:18:25PM +0100, Stuart Henderson wrote:
> >> On 2016/04/28 08:56, stan wrote:
> >> > On Thu, Apr 28, 2016 at 08:44:49AM +0100, Stuart Henderson wrote:
> >> > > Stan, can you send the information that is output when you run
> >> > > sendbug -P as root? Just putting the whole thing inline in a
> >> > > reply-to-all to this mail would be fine. Please add "sysctl hw"
> >> > > output as well. Ideally we want a way to identify the watchdog
> >> > > itself rather than the general machine type etc. which is why
> >> > > I'm hoping they follow Microsoft's spec (which is the de-facto 
> >> > > standard for this).
> >> > 
> >> > 
> >> > Sorry got distracted and frgot to cc the list.
> >> 
> >> OK, pity, there doesn't seem to be anything to properly identify
> >> the watchdog in acpi tables. Just the vendor-specific thing which
> >> needs reading to figure things out further. If they had followed
> >> the usual spec then the driver would have been *very* generally
> >> useful.
> >> 
> >> In that case maybe the approach would be to do something similar
> >> to acpithinkpad, but matching SECD instead of MHKV, and then
> >> looking for the SEL0002 HID. But I only know a bit about how
> >> to find my way round the decompiled files, so ignore me if
> >> a real ACPI hacker steps in with a better idea ;)
> >> 
> >> > hw.vendor=Schweitzer Engineering Laboratories, Inc.
> >> > hw.product=SEL-3355
> >> 
> >> An alternative might be to match on vendor/product, see the last
> >> commit to sys/dev/ic/re.c for how to do this, but then you're
> >> having to look at fixed addresses which they seem to be providing
> >> via acpi.
> >> 
> >
> > Let me apologize right here for my lack of knowledge as to low level
> > hardware coding.
> >
> > So, given that, please help me to understand why reading the DSDT ACPI
> > table and finding the SEL0002 is not the correct solution here?  BTW, they
> > also identify an SEL0001 device, but I have not asked them what that is,
> > and )so far) I do not need to know. For what it is worth this hardware
> > vendor has been very helpful, and the corporate philosophy is to do things
> > "the right way". For instance, they released code to support this device for
> > Linux. When I talked to them, i brought up liscneing concerns with the
> > BSD's. The reply was, already been there, thought about that. It is dual
> > GPL, and BSD liscneced. Really a pleasant sunrise, as this is not in their
> > mainstream area of expertise. i was really pleased that the project team
> > had researched the OSS issues that well.
> 
> I was just chatting to mlarkin about this, he hasn't looked at the
> code but gave a few suggestions. 
> 
> I was wrong about the need to look for the SECD container - he gave an
> example of a better driver to crib from, sdhc_acpi, which is pretty
> simple in itself and shows the parts needed to find the device in the
> DSDT. Basically it just involves passing an array containing "SEL0002"
> to acpi_matchhids and it does the work.
> 

It does the work for the find and attach. After that, you'll need to plumb
the AML to find the device location and IRQ/etc. But sdhc_acpi is a simple
driver that has most of the system interface goo you need to get started.

-ml

Re: FW: Re: watchdog suport for new hardware

2016-04-28 Thread Stuart Henderson 
On 2016-04-28, stan  wrote:
> On Thu, Apr 28, 2016 at 02:18:25PM +0100, Stuart Henderson wrote:
>> On 2016/04/28 08:56, stan wrote:
>> > On Thu, Apr 28, 2016 at 08:44:49AM +0100, Stuart Henderson wrote:
>> > > Stan, can you send the information that is output when you run
>> > > sendbug -P as root? Just putting the whole thing inline in a
>> > > reply-to-all to this mail would be fine. Please add "sysctl hw"
>> > > output as well. Ideally we want a way to identify the watchdog
>> > > itself rather than the general machine type etc. which is why
>> > > I'm hoping they follow Microsoft's spec (which is the de-facto 
>> > > standard for this).
>> > 
>> > 
>> > Sorry got distracted and frgot to cc the list.
>> 
>> OK, pity, there doesn't seem to be anything to properly identify
>> the watchdog in acpi tables. Just the vendor-specific thing which
>> needs reading to figure things out further. If they had followed
>> the usual spec then the driver would have been *very* generally
>> useful.
>> 
>> In that case maybe the approach would be to do something similar
>> to acpithinkpad, but matching SECD instead of MHKV, and then
>> looking for the SEL0002 HID. But I only know a bit about how
>> to find my way round the decompiled files, so ignore me if
>> a real ACPI hacker steps in with a better idea ;)
>> 
>> > hw.vendor=Schweitzer Engineering Laboratories, Inc.
>> > hw.product=SEL-3355
>> 
>> An alternative might be to match on vendor/product, see the last
>> commit to sys/dev/ic/re.c for how to do this, but then you're
>> having to look at fixed addresses which they seem to be providing
>> via acpi.
>> 
>
> Let me apologize right here for my lack of knowledge as to low level
> hardware coding.
>
> So, given that, please help me to understand why reading the DSDT ACPI
> table and finding the SEL0002 is not the correct solution here?  BTW, they
> also identify an SEL0001 device, but I have not asked them what that is,
> and )so far) I do not need to know. For what it is worth this hardware
> vendor has been very helpful, and the corporate philosophy is to do things
> "the right way". For instance, they released code to support this device for
> Linux. When I talked to them, i brought up liscneing concerns with the
> BSD's. The reply was, already been there, thought about that. It is dual
> GPL, and BSD liscneced. Really a pleasant sunrise, as this is not in their
> mainstream area of expertise. i was really pleased that the project team
> had researched the OSS issues that well.

I was just chatting to mlarkin about this, he hasn't looked at the
code but gave a few suggestions. 

I was wrong about the need to look for the SECD container - he gave an
example of a better driver to crib from, sdhc_acpi, which is pretty
simple in itself and shows the parts needed to find the device in the
DSDT. Basically it just involves passing an array containing "SEL0002"
to acpi_matchhids and it does the work.

Re: FW: Re: watchdog suport for new hardware

2016-04-28 Thread stan 
On Thu, Apr 28, 2016 at 02:18:25PM +0100, Stuart Henderson wrote:
> On 2016/04/28 08:56, stan wrote:
> > On Thu, Apr 28, 2016 at 08:44:49AM +0100, Stuart Henderson wrote:
> > > Stan, can you send the information that is output when you run
> > > sendbug -P as root? Just putting the whole thing inline in a
> > > reply-to-all to this mail would be fine. Please add "sysctl hw"
> > > output as well. Ideally we want a way to identify the watchdog
> > > itself rather than the general machine type etc. which is why
> > > I'm hoping they follow Microsoft's spec (which is the de-facto 
> > > standard for this).
> > 
> > 
> > Sorry got distracted and frgot to cc the list.
> 
> OK, pity, there doesn't seem to be anything to properly identify
> the watchdog in acpi tables. Just the vendor-specific thing which
> needs reading to figure things out further. If they had followed
> the usual spec then the driver would have been *very* generally
> useful.
> 
> In that case maybe the approach would be to do something similar
> to acpithinkpad, but matching SECD instead of MHKV, and then
> looking for the SEL0002 HID. But I only know a bit about how
> to find my way round the decompiled files, so ignore me if
> a real ACPI hacker steps in with a better idea ;)
> 
> > hw.vendor=Schweitzer Engineering Laboratories, Inc.
> > hw.product=SEL-3355
> 
> An alternative might be to match on vendor/product, see the last
> commit to sys/dev/ic/re.c for how to do this, but then you're
> having to look at fixed addresses which they seem to be providing
> via acpi.
> 

Let me apologize right here for my lack of knowledge as to low level
hardware coding.

So, given that, please help me to understand why reading the DSDT ACPI
table and finding the SEL0002 is not the correct solution here?  BTW, they
also identify an SEL0001 device, but I have not asked them what that is,
and )so far) I do not need to know. For what it is worth this hardware
vendor has been very helpful, and the corporate philosophy is to do things
"the right way". For instance, they released code to support this device for
Linux. When I talked to them, i brought up liscneing concerns with the
BSD's. The reply was, already been there, thought about that. It is dual
GPL, and BSD liscneced. Really a pleasant sunrise, as this is not in their
mainstream area of expertise. i was really pleased that the project team
had researched the OSS issues that well.




-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

Re: FW: Re: watchdog suport for new hardware

2016-04-28 Thread stan 
On Thu, Apr 28, 2016 at 08:44:49AM +0100, Stuart Henderson wrote:
> Stan, can you send the information that is output when you run
> sendbug -P as root? Just putting the whole thing inline in a
> reply-to-all to this mail would be fine. Please add "sysctl hw"
> output as well. Ideally we want a way to identify the watchdog
> itself rather than the general machine type etc. which is why
> I'm hoping they follow Microsoft's spec (which is the de-facto 
> standard for this).


Sorry got distracted and frgot to cc the list.


Script started on Thu Apr 28 08:45:28 2016
# sendbug -P
SENDBUG: -*- sendbug -*-
SENDBUG: Lines starting with `SENDBUG' will be removed automatically.
SENDBUG:
SENDBUG: Choose from the following categories:
SENDBUG:
SENDBUG: system user library documentation kernel alpha amd64 arm hppa i386 
m88k mips64 powerpc sh sparc sparc64 vax
SENDBUG:
SENDBUG:
To: b...@openbsd.org
Subject: 
From: root
Cc: root
Reply-To: root

>Synopsis:  
>Category:  
>Environment:
System  : OpenBSD 5.9
Details : OpenBSD 5.9 (GENERIC.MP) #0: Wed Apr 27 18:40:39 EDT 2016
 
r...@test.power.charleston.kapstonepaper.com:/usr/src/sys/arch/amd64/compile/GENERIC.MP

Architecture: OpenBSD.amd64
Machine : amd64
>Description:

>How-To-Repeat:

>Fix:


SENDBUG: dmesg, pcidump, acpidump and usbdevs are attached.
SENDBUG: Feel free to delete or use the -D flag if they contain sensitive 
information.

dmesg:
OpenBSD 5.9 (GENERIC.MP) #0: Wed Apr 27 18:40:39 EDT 2016

r...@test.power.charleston.kapstonepaper.com:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 16991633408 (16204MB)
avail mem = 16472473600 (15709MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xe0840 (59 entries)
bios0: vendor Schweitzer Engineering Laboratories, Inc. version "B_1.5.49152.2 
X64" date 12/18/2015
bios0: Schweitzer Engineering Laboratories, Inc. SEL-3355
acpi0 at bios0: rev 2
acpi0: sleep states S0 S5
acpi0: tables DSDT FACP SSDT HPET APIC MCFG FPDT ASF! SSDT SSDT SSDT SSDT UEFI 
POAT SSDT BERT HEST EINJ ERST UEFI UEFI DBG2
acpi0: wakeup devices P0P1(S0) GLAN(S0) EHC1(S0) EHC2(S0) HDEF(S0) PXSX(S0) 
PXSX(S0) PXSX(S0) PXSX(S0) PXSX(S0) RP08(S0) PEG1(S0) PEG3(S0)
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpihpet0 at acpi0: 14318179 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Core(TM) i7-3612QE CPU @ 2.10GHz, 2095.58 MHz
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,SENSOR,ARAT
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.1.2, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Intel(R) Core(TM) i7-3612QE CPU @ 2.10GHz, 2095.24 MHz
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,SENSOR,ARAT
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 1, core 0, package 0
cpu2 at mainbus0: apid 2 (application processor)
cpu2: Intel(R) Core(TM) i7-3612QE CPU @ 2.10GHz, 2095.24 MHz
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,SENSOR,ARAT
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 0, core 1, package 0
cpu3 at mainbus0: apid 3 (application processor)
cpu3: Intel(R) Core(TM) i7-3612QE CPU @ 2.10GHz, 2095.24 MHz
cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,SENSOR,ARAT
cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 1, core 1, package 0
cpu4 at mainbus0: apid 4 (application processor)
cpu4: Intel(R) Core(TM) i7-3612QE CPU @ 2.10GHz, 2095.24 MHz
cpu4: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,SENSOR,ARAT

Re: FW: Re: watchdog suport for new hardware

2016-04-28 Thread Stuart Henderson 
On 2016/04/28 08:56, stan wrote:
> On Thu, Apr 28, 2016 at 08:44:49AM +0100, Stuart Henderson wrote:
> > Stan, can you send the information that is output when you run
> > sendbug -P as root? Just putting the whole thing inline in a
> > reply-to-all to this mail would be fine. Please add "sysctl hw"
> > output as well. Ideally we want a way to identify the watchdog
> > itself rather than the general machine type etc. which is why
> > I'm hoping they follow Microsoft's spec (which is the de-facto 
> > standard for this).
> 
> 
> Sorry got distracted and frgot to cc the list.

OK, pity, there doesn't seem to be anything to properly identify
the watchdog in acpi tables. Just the vendor-specific thing which
needs reading to figure things out further. If they had followed
the usual spec then the driver would have been *very* generally
useful.

In that case maybe the approach would be to do something similar
to acpithinkpad, but matching SECD instead of MHKV, and then
looking for the SEL0002 HID. But I only know a bit about how
to find my way round the decompiled files, so ignore me if
a real ACPI hacker steps in with a better idea ;)

> hw.vendor=Schweitzer Engineering Laboratories, Inc.
> hw.product=SEL-3355

An alternative might be to match on vendor/product, see the last
commit to sys/dev/ic/re.c for how to do this, but then you're
having to look at fixed addresses which they seem to be providing
via acpi.

Re: FW: Re: watchdog suport for new hardware

2016-04-28 Thread stan 
On Thu, Apr 28, 2016 at 08:44:49AM +0100, Stuart Henderson wrote:
> Stan, can you send the information that is output when you run
> sendbug -P as root? Just putting the whole thing inline in a
> reply-to-all to this mail would be fine. Please add "sysctl hw"
> output as well. Ideally we want a way to identify the watchdog
> itself rather than the general machine type etc. which is why
> I'm hoping they follow Microsoft's spec (which is the de-facto 
> standard for this).

Absolutely. I am in the process of bailing a 5.9 machine to install this
on, so hopefully I can send this today.

Thanks for the interest in our issues.


-- 
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

Re: FW: Re: watchdog suport for new hardware

2016-04-28 Thread Stuart Henderson 
Stan, can you send the information that is output when you run
sendbug -P as root? Just putting the whole thing inline in a
reply-to-all to this mail would be fine. Please add "sysctl hw"
output as well. Ideally we want a way to identify the watchdog
itself rather than the general machine type etc. which is why
I'm hoping they follow Microsoft's spec (which is the de-facto 
standard for this).

Re: FW: Re: watchdog suport for new hardware

2016-04-26 Thread Stuart Henderson 
On 2016-04-26, Theo de Raadt  wrote:
>> int
>> selwd_probe(struct device *parent, void *match, void *aux)
>> {
>>  struct isa_attach_args *ia = aux;
>>  bus_space_tag_t iot;
>>  bus_space_handle_t ioh;
>> 
>>  /* Match by device ID */
>>  iot = ia->ia_iot;
>>  if (bus_space_map(iot, ia->ipa_io[0].base, SELWD_IOSIZE, 0, ))
>>  return 0;
>
> ...
>
>>  /* read model number */
>>  char *model = malloc(sizeof(char)*16, M_DEVBUF, M_WAITOK | M_ZERO);
>>  selwd_read_modelno(iot, ioh, model);
>
> This is worrying.  It assumes that all systems have this hardware.
>
> And it starts by doing a "write".

Is there no WDRT or WDAT table in ACPI on this hardware? Most "modern"
(2006-on) watchdogs on PCs with ACPI support have the WDRT table described
in https://msdn.microsoft.com/en-us/windows/hardware/gg463320.aspx

Check for /tmp/acpi.WD[AR]T.* files after running acpidump -o /tmp/acpi ..

FW: Re: watchdog suport for new hardware

2016-04-26 Thread stan 
- Forwarded message from stan  -

From: stan 
To: Theo de Raadt 
Subject: Re: watchdog suport for new hardware
Date: Tue, 26 Apr 2016 09:19:20 -0400
User-Agent: Mutt/1.5.4i
X-Operating-System: Debian GNU/Linux
X-Kernel-Version: 2.4.23
X-Uptime: 09:17:17 up 91 days,  8:18,  1 user,  load average: 0.00, 0.02, 0.04
X-Editor: gVim

Hee is the core of the functionality:


/*  $OpenBSD: selwd.c,v 1.0 2016/04/01 05:00:00 jsg Exp $   */
/*
 * Copyright (c) 2016 PREMIER System Integrators
 *
 * Permission to use, copy, modify, and distribute this software for any
 * purpose with or without fee is hereby granted, provided that the above
 * copyright notice and this permission notice appear in all copies.
 *
 * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
 * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
 * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
 * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
 * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
 * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 *
 * Schweitzer Engineering Laboratories: SEL-3355 Embedded controller
*/

#include 
#include 
#include 
#include 
#include 
#include 

#include 

#include 
#include 

struct selwd_softc {
/* sc_dev must be the first item in the struct */
struct device   sc_dev;
/* device access through bus space */
bus_space_tag_t sc_iot;
bus_space_handle_t  sc_ioh;
};

int selwd_wait(bus_space_tag_t, bus_space_handle_t, bool);

/* Autoconfiguration glue */
int selwd_probe(struct device *, void *, void *);
void selwd_attach(struct device *, struct device *, void *);
int selwd_print(void *, const char *);
int selwd_wd_cb(void *, int);

/* functions to interact with the controller */
void selwd_write_wdog(bus_space_tag_t, bus_space_handle_t, int);
u_int8_t selwd_read_wdog(bus_space_tag_t, bus_space_handle_t);
u_int8_t selwd_read_status(bus_space_tag_t, bus_space_handle_t);
void selwd_abort(bus_space_tag_t, bus_space_handle_t);
u_int32_t selwd_read_boardid(bus_space_tag_t, bus_space_handle_t);
u_int32_t selwd_read_mcversion(bus_space_tag_t, bus_space_handle_t);
u_int16_t selwd_read_pciboardid(bus_space_tag_t, bus_space_handle_t);
u_int8_t selwd_read_ledctl0(bus_space_tag_t, bus_space_handle_t);
void selwd_write_ledctl0(bus_space_tag_t, bus_space_handle_t, u_int8_t);
u_int8_t selwd_read_ledctl1(bus_space_tag_t, bus_space_handle_t);
void selwd_write_ledctl1(bus_space_tag_t, bus_space_handle_t, u_int8_t);
u_int8_t selwd_read_miscctl0(bus_space_tag_t, bus_space_handle_t);
u_int8_t selwd_read_miscctl1(bus_space_tag_t, bus_space_handle_t);
void selwd_read_modelno(bus_space_tag_t, bus_space_handle_t, char*);
void selwd_read_serialno(bus_space_tag_t, bus_space_handle_t, char*);
void selwd_read_configid(bus_space_tag_t, bus_space_handle_t, char*);

/* macros to extract bits from the status register */
#define EC_STATUS_IBF(status)   (((status) >> 0x1) & 0x1)
#define EC_STATUS_OBF(status)   (((status) & 0x1))
#define EC_STATUS_BUSY(status)  (((status) >> 0x4) & 0x1)
#define EC_STATUS_STATE(status) (((status) >> 0x6) & 0x3)

struct cfattach selwd_ca = {
sizeof(struct selwd_softc), selwd_probe, selwd_attach
};

struct cfdriver selwd_cd = {
NULL, "selwd", DV_DULL
};

const char* selwd_models[] = { SELWD_DEV_3355, SELWD_DEV_3360 };

int selwd_wait(bus_space_tag_t iot, bus_space_handle_t ioh, bool useobf)
{
uint32_t timeout = 0;
uint8_t status = 0;

while (timeout < 50) {
status = bus_space_read_1(iot, ioh, SELWD_CMD);
if ((EC_STATUS_IBF(status) == 0x0) &&
(EC_STATUS_BUSY(status) == 0x0) &&
(EC_STATUS_STATE(status) != 0x3) &&
((EC_STATUS_OBF(status) == 0x0) || !useobf)) {
return 1;
}
delay(10);
++timeout;
}
return 0;
}

static __inline void
selwd_conf_enable(bus_space_tag_t iot, bus_space_handle_t ioh, u_int8_t cmd)
{
/* write the desired command to the command register */
bus_space_write_1(iot, ioh, SELWD_CMD, cmd);
/* wait for controller to be ready */
selwd_wait(iot,ioh,0);
}

static __inline u_int8_t
selwd_conf_read(bus_space_tag_t iot, bus_space_handle_t ioh, u_int8_t target)
{
/* write the target address to the data register */
bus_space_write_1(iot, ioh, SELWD_DATA, target);
/* wait for controller to be ready */
selwd_wait(iot, ioh,1);
/* return the value from the data register */
return (bus_space_read_1(iot, ioh, SELWD_DATA));
}

static __inline void
selwd_conf_write(bus_space_tag_t iot,

Re: FW: Re: watchdog suport for new hardware

2016-04-26 Thread Theo de Raadt 
> int
> selwd_probe(struct device *parent, void *match, void *aux)
> {
>   struct isa_attach_args *ia = aux;
>   bus_space_tag_t iot;
>   bus_space_handle_t ioh;
> 
>   /* Match by device ID */
>   iot = ia->ia_iot;
>   if (bus_space_map(iot, ia->ipa_io[0].base, SELWD_IOSIZE, 0, ))
>   return 0;

...

>   /* read model number */
>   char *model = malloc(sizeof(char)*16, M_DEVBUF, M_WAITOK | M_ZERO);
>   selwd_read_modelno(iot, ioh, model);

This is worrying.  It assumes that all systems have this hardware.

And it starts by doing a "write".

> void
> selwd_read_modelno(bus_space_tag_t iot, bus_space_handle_t ioh, char* model)
> {
>   /* read the MODEL NUMBER value from the controller */
>   /* make sure status is 0 before proceeding */
>   selwd_abort(iot, ioh);
>   u_int8_t reg;
>   int i = 0;
> 
>   /* write the READCFG command to the command register */
>   selwd_conf_enable(iot, ioh, SELWD_CMD_READCFG);
>   /* read the MODELNO data value from the data register */
>   reg = selwd_conf_read(iot, ioh, SELWD_CFG_MODELNO);
>   while (reg != 0 && i < 15) {
>   model[i] = reg;
>   /* make sure status is 0 before proceeding */
>   selwd_abort(iot, ioh);
>   /* write the READCFG command to the command register */
>   selwd_conf_enable(iot, ioh, SELWD_CMD_READCFG);
>   /* read the MODELNO data value from the data register */
>   reg = selwd_conf_read(iot, ioh, SELWD_CFG_MODELNO);
>   i++;
>   }
> 
> }

I do not know what IO port this lands at, but we have had bad experiences
with this strategy in the past.  Specifically the uguru(4) chip which landed
right on top of an undocumented Dell server's cpu clock-generator, that was
even worse, since the clock generator appeared to change behaviour based only
upon read's...