Re: Crash when unplugging a UPS USB connection

2021-08-05 Thread joshua stein
On Thu, 05 Aug 2021 at 12:41:56 -0600, Aaron Bieber wrote:
> 
> Martin Pieuchot writes:
> 
> > On 05/08/21(Thu) 09:41, Aaron Bieber wrote:
> >> 
> >> Stuart Henderson writes:
> >> 
> >> > On 2021/07/13 01:29, Anindya Mukherjee wrote:
> >> >> I have a Cyber Power CP1500PFCLCDa UPS and get exactly the same crash 
> >> >> on the
> >> >> latest snapshot if the USB cable is unplugged. My dmesg is very similar 
> >> >> so I've
> >> >> omitted it, but I'd also be happy to help debug this issue.
> >> >> 
> >> >> Regards,
> >> >> Anindya
> >> >> 
> >> >
> >> > First try backing out the "Allow uhidev child devices to claim selective
> >> > report ids" commit from March.
> >> 
> >> Here is a diff that reverts:
> >> 
> >> 2021-03-08 6545f693 jcs   Add another Type Cover device
> >> 2021-03-08 1f85050a jcs   regen
> >> 2021-03-08 fc9d2605 jcs   Add Surface Pro Type Cover
> >> 2021-03-08 f31b43ce jcs   Allow uhidev child devices to claim 
> >> selective report ids
> >> 
> >> With them reverted, my UPS(s) no longer trigger panics on disconnect.
> >
> > The bugs lies in upd_match() and has been introduced with the change
> > containing UHIDEV_CLAIM_MULTIPLE_REPORTID.
> >
> > upd(4) owns all the reportID of a USB device, so a "uha.claimed" logic
> > similar to what has been written for umt_match() is missing.
> >
> 
> The following diff fixes it for me:
> 
> diff 2f84ac1381d01579f2b25c83b3dec5f908503765 /usr/src
> blob - ad65b77718b50851c1537a7048eb7b54aba91203
> file + sys/dev/usb/upd.c
> --- sys/dev/usb/upd.c
> +++ sys/dev/usb/upd.c
> @@ -167,6 +167,7 @@ upd_match(struct device *parent, void *match, void *au
>   if (upd_lookup_usage_entry(desc, size,
>   upd_usage_roots + i, )) {
>   ret = UMATCH_VENDOR_PRODUCT;
> + uha->claimed[item.report_ID] = 1;
>   break;
>   }

I think it needs to loop through all of the report IDs like it does 
in upd_attach_sensor_tree to mark them claimed, so it should not 
break after the first result.



Re: X windows dont boot up all the way

2021-08-05 Thread Brian Alston
Is there anything you can do help me with
X11?

Get Outlook for iOS

From: Brian Alston 
Sent: Tuesday, August 3, 2021 5:54:30 PM
To: bugs@openbsd.org 
Subject: Fw: X windows dont boot up all the way


Here is the file you could not open up  up RTF format.

From: Matthieu Herrb 
Sent: Monday, August 2, 2021 10:57 PM
To: Brian Alston 
Cc: bugs@openbsd.org 
Subject: Re: X windows dont boot up all the way

On Tue, Aug 03, 2021 at 02:55:20AM +, Brian Alston wrote:
> Please send me a message back.
> I just want to know if you got my
> email. Sorry sendbug didn't work.
> Thanks for all your help

Hi,

The attachment in your message is mangled (embedded in an RTF file or
something).

Anyways it looks like ypu're trying to run OpenBSD/i386 on a machine
that's capable of running the 64 bits (amd64) version.

There are known issues with OpenBSD/i386 in this kind of situation. So
try to install OpenBSD/amd64 instead and report back.

--
Matthieu Herrb


Re: Crash when unplugging a UPS USB connection

2021-08-05 Thread Aaron Bieber


Martin Pieuchot writes:

> On 05/08/21(Thu) 09:41, Aaron Bieber wrote:
>> 
>> Stuart Henderson writes:
>> 
>> > On 2021/07/13 01:29, Anindya Mukherjee wrote:
>> >> I have a Cyber Power CP1500PFCLCDa UPS and get exactly the same crash on 
>> >> the
>> >> latest snapshot if the USB cable is unplugged. My dmesg is very similar 
>> >> so I've
>> >> omitted it, but I'd also be happy to help debug this issue.
>> >> 
>> >> Regards,
>> >> Anindya
>> >> 
>> >
>> > First try backing out the "Allow uhidev child devices to claim selective
>> > report ids" commit from March.
>> 
>> Here is a diff that reverts:
>> 
>> 2021-03-08 6545f693 jcs   Add another Type Cover device
>> 2021-03-08 1f85050a jcs   regen
>> 2021-03-08 fc9d2605 jcs   Add Surface Pro Type Cover
>> 2021-03-08 f31b43ce jcs   Allow uhidev child devices to claim selective 
>> report ids
>> 
>> With them reverted, my UPS(s) no longer trigger panics on disconnect.
>
> The bugs lies in upd_match() and has been introduced with the change
> containing UHIDEV_CLAIM_MULTIPLE_REPORTID.
>
> upd(4) owns all the reportID of a USB device, so a "uha.claimed" logic
> similar to what has been written for umt_match() is missing.
>

The following diff fixes it for me:

diff 2f84ac1381d01579f2b25c83b3dec5f908503765 /usr/src
blob - ad65b77718b50851c1537a7048eb7b54aba91203
file + sys/dev/usb/upd.c
--- sys/dev/usb/upd.c
+++ sys/dev/usb/upd.c
@@ -167,6 +167,7 @@ upd_match(struct device *parent, void *match, void *au
if (upd_lookup_usage_entry(desc, size,
upd_usage_roots + i, )) {
ret = UMATCH_VENDOR_PRODUCT;
+   uha->claimed[item.report_ID] = 1;
break;
}
 



Re: Crash when unplugging a UPS USB connection

2021-08-05 Thread Martin Pieuchot
On 05/08/21(Thu) 09:41, Aaron Bieber wrote:
> 
> Stuart Henderson writes:
> 
> > On 2021/07/13 01:29, Anindya Mukherjee wrote:
> >> I have a Cyber Power CP1500PFCLCDa UPS and get exactly the same crash on 
> >> the
> >> latest snapshot if the USB cable is unplugged. My dmesg is very similar so 
> >> I've
> >> omitted it, but I'd also be happy to help debug this issue.
> >> 
> >> Regards,
> >> Anindya
> >> 
> >
> > First try backing out the "Allow uhidev child devices to claim selective
> > report ids" commit from March.
> 
> Here is a diff that reverts:
> 
> 2021-03-08 6545f693 jcs   Add another Type Cover device
> 2021-03-08 1f85050a jcs   regen
> 2021-03-08 fc9d2605 jcs   Add Surface Pro Type Cover
> 2021-03-08 f31b43ce jcs   Allow uhidev child devices to claim selective 
> report ids
> 
> With them reverted, my UPS(s) no longer trigger panics on disconnect.

The bugs lies in upd_match() and has been introduced with the change
containing UHIDEV_CLAIM_MULTIPLE_REPORTID.

upd(4) owns all the reportID of a USB device, so a "uha.claimed" logic
similar to what has been written for umt_match() is missing.

> diff --git a/sys/dev/usb/fido.c b/sys/dev/usb/fido.c
> index c6d846aaa84..77bd9b12175 100644
> --- a/sys/dev/usb/fido.c
> +++ b/sys/dev/usb/fido.c
> @@ -1,4 +1,4 @@
> -/*   $OpenBSD: fido.c,v 1.3 2021/03/08 14:35:57 jcs Exp $*/
> +/*   $OpenBSD: fido.c,v 1.2 2019/12/18 05:09:53 deraadt Exp $*/
>  
>  /*
>   * Copyright (c) 2019 Reyk Floeter 
> @@ -63,7 +63,7 @@ fido_match(struct device *parent, void *match, void *aux)
>   void *desc;
>   int   ret = UMATCH_NONE;
>  
> - if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID)
> + if (uha->reportid == UHIDEV_CLAIM_ALLREPORTID)
>   return (ret);
>  
>   /* Find the FIDO usage page and U2F collection */
> diff --git a/sys/dev/usb/ucycom.c b/sys/dev/usb/ucycom.c
> index ca8636f0a7f..ca6d6e9c6b2 100644
> --- a/sys/dev/usb/ucycom.c
> +++ b/sys/dev/usb/ucycom.c
> @@ -1,4 +1,4 @@
> -/*   $OpenBSD: ucycom.c,v 1.39 2021/03/08 14:35:57 jcs Exp $ */
> +/*   $OpenBSD: ucycom.c,v 1.38 2020/02/25 10:03:39 mpi Exp $ */
>  /*   $NetBSD: ucycom.c,v 1.3 2005/08/05 07:27:47 skrll Exp $ */
>  
>  /*
> @@ -165,7 +165,7 @@ ucycom_match(struct device *parent, void *match, void 
> *aux)
>  {
>   struct uhidev_attach_arg *uha = aux;
>  
> - if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID)
> + if (uha->reportid == UHIDEV_CLAIM_ALLREPORTID)
>   return (UMATCH_NONE);
>  
>   return (usb_lookup(ucycom_devs, uha->uaa->vendor, uha->uaa->product) != 
> NULL ?
> diff --git a/sys/dev/usb/ugold.c b/sys/dev/usb/ugold.c
> index 752ecff64d2..865618cc17d 100644
> --- a/sys/dev/usb/ugold.c
> +++ b/sys/dev/usb/ugold.c
> @@ -1,4 +1,4 @@
> -/*   $OpenBSD: ugold.c,v 1.17 2021/04/05 16:26:06 landry Exp $   */
> +/*   $OpenBSD: ugold.c,v 1.15 2020/08/17 04:26:57 gnezdo Exp $   */
>  
>  /*
>   * Copyright (c) 2013 Takayoshi SASANO 
> @@ -113,7 +113,7 @@ ugold_match(struct device *parent, void *match, void *aux)
>   int size;
>   void *desc;
>  
> - if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID)
> + if (uha->reportid == UHIDEV_CLAIM_ALLREPORTID)
>   return (UMATCH_NONE);
>  
>   if (usb_lookup(ugold_devs, uha->uaa->vendor, uha->uaa->product) == NULL)
> diff --git a/sys/dev/usb/uhid.c b/sys/dev/usb/uhid.c
> index 085c1523ccf..ba21e8cf96f 100644
> --- a/sys/dev/usb/uhid.c
> +++ b/sys/dev/usb/uhid.c
> @@ -1,4 +1,4 @@
> -/*   $OpenBSD: uhid.c,v 1.84 2021/03/08 14:35:57 jcs Exp $ */
> +/*   $OpenBSD: uhid.c,v 1.83 2021/01/29 16:59:41 sthen Exp $ */
>  /*   $NetBSD: uhid.c,v 1.57 2003/03/11 16:44:00 augustss Exp $   */
>  
>  /*
> @@ -115,7 +115,7 @@ uhid_match(struct device *parent, void *match, void *aux)
>  {
>   struct uhidev_attach_arg *uha = aux;
>  
> - if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID)
> + if (uha->reportid == UHIDEV_CLAIM_ALLREPORTID)
>   return (UMATCH_NONE);
>  
>   return (UMATCH_IFACECLASS_GENERIC);
> diff --git a/sys/dev/usb/uhidev.c b/sys/dev/usb/uhidev.c
> index cbd0b8336f6..2333c260d71 100644
> --- a/sys/dev/usb/uhidev.c
> +++ b/sys/dev/usb/uhidev.c
> @@ -1,5 +1,4 @@
>  /*   $OpenBSD: uhidev.c,v 1.92 2021/03/18 09:21:53 anton Exp $   */
> -/*   $NetBSD: uhidev.c,v 1.14 2003/03/11 16:44:00 augustss Exp $ */
>  
>  /*
>   * Copyright (c) 2001 The NetBSD Foundation, Inc.
> @@ -250,28 +249,21 @@ uhidev_attach(struct device *parent, struct device 
> *self, void *aux)
>  
>   uha.uaa = uaa;
>   uha.parent = sc;
> - uha.reportid = UHIDEV_CLAIM_MULTIPLE_REPORTID;
> - uha.nreports = nrepid;
> - uha.claimed = malloc(nrepid, M_TEMP, M_WAITOK|M_ZERO);
> + uha.reportid = UHIDEV_CLAIM_ALLREPORTID;
>  
> - /* Look for a driver claiming multiple report IDs first. */
> + /* Look for a driver claiming all report IDs first. */
>   dev = 

Re: Crash when unplugging a UPS USB connection

2021-08-05 Thread Aaron Bieber

Stuart Henderson writes:

> On 2021/07/13 01:29, Anindya Mukherjee wrote:
>> I have a Cyber Power CP1500PFCLCDa UPS and get exactly the same crash on the
>> latest snapshot if the USB cable is unplugged. My dmesg is very similar so 
>> I've
>> omitted it, but I'd also be happy to help debug this issue.
>> 
>> Regards,
>> Anindya
>> 
>
> First try backing out the "Allow uhidev child devices to claim selective
> report ids" commit from March.

Here is a diff that reverts:

2021-03-08 6545f693 jcs   Add another Type Cover device
2021-03-08 1f85050a jcs   regen
2021-03-08 fc9d2605 jcs   Add Surface Pro Type Cover
2021-03-08 f31b43ce jcs   Allow uhidev child devices to claim selective 
report ids

With them reverted, my UPS(s) no longer trigger panics on disconnect.

diff --git a/sys/dev/usb/fido.c b/sys/dev/usb/fido.c
index c6d846aaa84..77bd9b12175 100644
--- a/sys/dev/usb/fido.c
+++ b/sys/dev/usb/fido.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: fido.c,v 1.3 2021/03/08 14:35:57 jcs Exp $	*/
+/*	$OpenBSD: fido.c,v 1.2 2019/12/18 05:09:53 deraadt Exp $	*/
 
 /*
  * Copyright (c) 2019 Reyk Floeter 
@@ -63,7 +63,7 @@ fido_match(struct device *parent, void *match, void *aux)
 	void			 *desc;
 	int			  ret = UMATCH_NONE;
 
-	if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID)
+	if (uha->reportid == UHIDEV_CLAIM_ALLREPORTID)
 		return (ret);
 
 	/* Find the FIDO usage page and U2F collection */
diff --git a/sys/dev/usb/ucycom.c b/sys/dev/usb/ucycom.c
index ca8636f0a7f..ca6d6e9c6b2 100644
--- a/sys/dev/usb/ucycom.c
+++ b/sys/dev/usb/ucycom.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ucycom.c,v 1.39 2021/03/08 14:35:57 jcs Exp $	*/
+/*	$OpenBSD: ucycom.c,v 1.38 2020/02/25 10:03:39 mpi Exp $	*/
 /*	$NetBSD: ucycom.c,v 1.3 2005/08/05 07:27:47 skrll Exp $	*/
 
 /*
@@ -165,7 +165,7 @@ ucycom_match(struct device *parent, void *match, void *aux)
 {
 	struct uhidev_attach_arg *uha = aux;
 
-	if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID)
+	if (uha->reportid == UHIDEV_CLAIM_ALLREPORTID)
 		return (UMATCH_NONE);
 
 	return (usb_lookup(ucycom_devs, uha->uaa->vendor, uha->uaa->product) != NULL ?
diff --git a/sys/dev/usb/ugold.c b/sys/dev/usb/ugold.c
index 752ecff64d2..865618cc17d 100644
--- a/sys/dev/usb/ugold.c
+++ b/sys/dev/usb/ugold.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ugold.c,v 1.17 2021/04/05 16:26:06 landry Exp $   */
+/*	$OpenBSD: ugold.c,v 1.15 2020/08/17 04:26:57 gnezdo Exp $   */
 
 /*
  * Copyright (c) 2013 Takayoshi SASANO 
@@ -113,7 +113,7 @@ ugold_match(struct device *parent, void *match, void *aux)
 	int size;
 	void *desc;
 
-	if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID)
+	if (uha->reportid == UHIDEV_CLAIM_ALLREPORTID)
 		return (UMATCH_NONE);
 
 	if (usb_lookup(ugold_devs, uha->uaa->vendor, uha->uaa->product) == NULL)
diff --git a/sys/dev/usb/uhid.c b/sys/dev/usb/uhid.c
index 085c1523ccf..ba21e8cf96f 100644
--- a/sys/dev/usb/uhid.c
+++ b/sys/dev/usb/uhid.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uhid.c,v 1.84 2021/03/08 14:35:57 jcs Exp $ */
+/*	$OpenBSD: uhid.c,v 1.83 2021/01/29 16:59:41 sthen Exp $ */
 /*	$NetBSD: uhid.c,v 1.57 2003/03/11 16:44:00 augustss Exp $	*/
 
 /*
@@ -115,7 +115,7 @@ uhid_match(struct device *parent, void *match, void *aux)
 {
 	struct uhidev_attach_arg *uha = aux;
 
-	if (uha->reportid == UHIDEV_CLAIM_MULTIPLE_REPORTID)
+	if (uha->reportid == UHIDEV_CLAIM_ALLREPORTID)
 		return (UMATCH_NONE);
 
 	return (UMATCH_IFACECLASS_GENERIC);
diff --git a/sys/dev/usb/uhidev.c b/sys/dev/usb/uhidev.c
index cbd0b8336f6..2333c260d71 100644
--- a/sys/dev/usb/uhidev.c
+++ b/sys/dev/usb/uhidev.c
@@ -1,5 +1,4 @@
 /*	$OpenBSD: uhidev.c,v 1.92 2021/03/18 09:21:53 anton Exp $	*/
-/*	$NetBSD: uhidev.c,v 1.14 2003/03/11 16:44:00 augustss Exp $	*/
 
 /*
  * Copyright (c) 2001 The NetBSD Foundation, Inc.
@@ -250,28 +249,21 @@ uhidev_attach(struct device *parent, struct device *self, void *aux)
 
 	uha.uaa = uaa;
 	uha.parent = sc;
-	uha.reportid = UHIDEV_CLAIM_MULTIPLE_REPORTID;
-	uha.nreports = nrepid;
-	uha.claimed = malloc(nrepid, M_TEMP, M_WAITOK|M_ZERO);
+	uha.reportid = UHIDEV_CLAIM_ALLREPORTID;
 
-	/* Look for a driver claiming multiple report IDs first. */
+	/* Look for a driver claiming all report IDs first. */
 	dev = config_found_sm(self, , NULL, uhidevsubmatch);
 	if (dev != NULL) {
 		for (repid = 0; repid < nrepid; repid++) {
 			/*
 			 * Could already be assigned by uhidev_set_report_dev().
 			 */
-			if (sc->sc_subdevs[repid] != NULL)
-continue;
-
-			if (uha.claimed[repid])
+			if (sc->sc_subdevs[repid] == NULL)
 sc->sc_subdevs[repid] = (struct uhidev *)dev;
 		}
+		return;
 	}
 
-	free(uha.claimed, M_TEMP, nrepid);
-	uha.claimed = NULL;
-
 	for (repid = 0; repid < nrepid; repid++) {
 		DPRINTF(("%s: try repid=%d\n", __func__, repid));
 		if (hid_report_size(desc, size, hid_input, repid) == 0 &&
@@ -362,7 +354,7 @@ uhidevprint(void *aux, const char *pnp)
 
 	if (pnp)
 		printf("uhid at %s", pnp);
-	if (uha->reportid != 0 && uha->reportid != UHIDEV_CLAIM_MULTIPLE_REPORTID)
+	if (uha->reportid 

Re: Crash when unplugging a UPS USB connection

2021-08-05 Thread Aaron Bieber


Mike writes:

> Another user on misc@ confirmed this, so I am submitting it to bug@.
>
> Please let me know if I need to do anything else.
>
> Many thanks!
> Mike.
>
>
>
>
> I run NUT on OpenBSD to monitor a Cyperpower UPS.  The UPS plugs into
> the OpenBSD box via a USB connection.
>
> OpenBSD 6.8, I had no problems, everything ran fine.  When the power
> went out, NUT saw that and reacted according to configuration.
>
> After I upgraded to OpenBSD 6.9 (a fresh install, not an in-place
> upgrade), when the power dropped, I'd be greeted with a blue crash screen.
>
> It seems that when the power drops, the UPS temporarily drops the USB
> connection, seemingly the equivalent of unplugging the USB connector.
>
> I am able to reproduce that 100% by booting up OpenBSD 6.9 with the UPS
> communications cable plugged into the USB port.  When I unplugged that
> USB connector, the crash occurs.
>
> This first occurred on my production box which is a Supermicro
> motherboard.  I can provide that dmesg if needed.

Here is an OCR'd dump of the panic from my machine running
OpenBSD 6.9-current (GENERIC.MP) #163: Tue Aug  3 20:39:14 MDT 2021
(sorry for formatting, it's ocr after all):

panic: config_detach: detached device (uhidev?) has children
Stopped at
db enter +0x10:
popq
%rbp
TID
PID
UID
PRFLAGS
PFLAGS
42624
72916
35
ox100010
0x80
39238
43835
ox10
0x80
*179053
39355
0x14000
ox200
CPU
COMMAND
2
xconsole
3
hotp lugd
1K usbtask
db enter) at db enter+0x10
panic(8led@ed1) at panic+@xbf
config_detach(82195e00,1) at config_detach+0x3d4
usbd detach(82679b00, 80352080) at usbd detach+@x5a
uhub_port_connect(80352080,1,180) at uhub_port_connect+0x68
hub_explore(8034f100) at uhub_explore+@xbg
hub_explore(8033ed00) at uhub_explore+@xZac
hub _explore(801a9a00) at uhub_explore+BxZac
usb_explore(801a9900) at usb_explore+x123
usb _task _thread(80004010) at usb_ task thread+0x135
end trace frame: 0x0, count: 5
https://www.openbsd.org/ddb.html describes the minimum info required in bug
Insufficient info makes it difficult to find and fix bugs.

And one from 6.9 (actual text - no OCR)

config_detach: upd0 attached at uhidev0
panic: config_detach: detached device (uhidev0) has children
Starting stack trace...
panic(81e72356) at panic+0x11d
config_detach(8013d600,1) at config_detach+0x3d4
usbd_detach(8013d500,800a8980) at usbd_detach+0x5a
uhub_port_connect(800a8980,4,2a0) at uhub_port_connect+0x68
uhub_explore(800b3700) at uhub_explore+0xb9
usb_explore(800b3500) at usb_explore+0x123
usb_task_thread(8000da48) at usb_task_thread+0x135
end trace frame: 0x0, count: 250

I have a CyperPowre 1500va

>
>
> Both OpenBSD 6.8 and current below are fresh installs on a test Lenovo
> laptop.
>
> On OpenBSD 6.8, when I plug in the UPS and unplug it, here is what I see
> on the console (dmesg is included):
>
> vvv= OpenBSD 6.8 ==
>
> OpenBSD 6.8 (GENERIC.MP) #3: Sat Jun  5 10:34:16 MDT 2021
>
> t...@syspatch-68-amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> real mem = 4129800192 (3938MB)
> avail mem = 3989590016 (3804MB)
> random: good seed from bootblocks
> mpath0 at root
> scsibus0 at mpath0: 256 targets
> mainbus0 at root
> bios0 at mainbus0: SMBIOS rev. 2.6 @ 0xe0010 (78 entries)
> bios0: vendor LENOVO version "6IET75WW (1.35 )" date 02/01/2011
> bios0: LENOVO 2522DU5
> acpi0 at bios0: ACPI 4.0
> acpi0: sleep states S0 S3 S4 S5
> acpi0: tables DSDT FACP SSDT ECDT APIC MCFG HPET ASF! SLIC BOOT SSDT
> TCPA SSDT SSDT SSDT
> acpi0: wakeup devices LID_(S3) SLPB(S3) IGBE(S4) EXP1(S4) EXP2(S4)
> EXP3(S4) EXP4(S4) EXP5(S4) EHC1(S3) EHC2(S3) HDEF(S4)
> acpitimer0 at acpi0: 3579545 Hz, 24 bits
> acpiec0 at acpi0
> acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
> cpu0 at mainbus0: apid 0 (boot processor)
> cpu0: Intel(R) Core(TM) i5 CPU M 560 @ 2.67GHz, 2926.44 MHz, 06-25-05
> 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,POPCNT,AES,NXE,RDTSCP,LONG,LAHF,PERF,ITSC,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,MELTDOWN
> cpu0: 256KB 64b/line 8-way L2 cache
> cpu0: smt 0, core 0, package 0
> mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
> cpu0: apic clock running at 133MHz
> cpu0: mwait min=64, max=64, C-substates=0.2.1.1, IBE
> cpu1 at mainbus0: apid 1 (application processor)
> cpu1: Intel(R) Core(TM) i5 CPU M 560 @ 2.67GHz, 2926.01 MHz, 06-25-05
> 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,POPCNT,AES,NXE,RDTSCP,LONG,LAHF,PERF,ITSC,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,MELTDOWN
> cpu1: 256KB 64b/line 8-way L2 cache
> cpu1: 

Re: Details on OpenBSD bug on system reboot with XHCI and UEFI

2021-08-05 Thread Peter Nicolai Mathias Hansteen


> 5. aug. 2021 kl. 04:34 skrev Stéphane Paquin :
> 
> 
> The issue is super probably between the hardware, the BIOS & the EUFI drivers.
> Or just plain hard disk corruption; I've seen that on the r730xd in fact. 
> With so many hardware freezes, the firmware drivers ended up as garbage on 
> the disk. Same could/should happen with the EUFI drivers on disk.

Whether or not the firmware files have been corrupted is relatively easy to 
check, at least - doas fw_update -v will report any errors. Then fw_update -d 
the damaged drivers and reinstall them with fw_update driver.

(I had to do exactly that while fiddling with my new ASUS laptop a little while 
back, the article is fairly close to the first url in my .signature)

- Peter



—
Peter N. M. Hansteen, member of the first RFC 1149 implementation team
http://bsdly.blogspot.com/ http://www.bsdly.net/ http://www.nuug.no/
"Remember to set the evil bit on all malicious network traffic"
delilah spamd[29949]: 85.152.224.147: disconnected after 42673 seconds.






signature.asc
Description: Message signed with OpenPGP


Re: kernel: page fault trap in rw_status

2021-08-05 Thread Martin Pieuchot
Hello Thomas,

Thanks a lot for your great but report, see below for an explanation and
a possible fix.

On 04/08/21(Wed) 12:18, Thomas L. wrote:
> >Synopsis:  page fault trap in rw_status
> >Category:  kernel
> >Environment:
> System  : OpenBSD 6.9
> Details : OpenBSD 6.9 (GENERIC) #4: Mon Jun  7 08:20:14 MDT 2021
>  
> r...@syspatch-69-amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC
> 
> Architecture: OpenBSD.amd64
> Machine : amd64
> >Description:
> One of my VMs crashed with a page fault trap
> ddb> show panic
> kernel page fault
> uvm_fault(0xfd801a844120, 0x0, 0, 1) -> e
> rw_status(0) at rw_status+0x11
> end trace frame: 0x8f9991e0, count: 0
> ddb> trace
> rw_status(0) at rw_status+0x11
> amap_wipeout(fd8005893740) at amap_wipeout+0x2a
> uvm_fault_check(8f999380,8f9993b8,8f9993e0) at 
> uvm_faul
> t_check+0x272
> uvm_fault(fd801a844120,7f7c,0,2) at uvm_fault+0xab
> upageflttrap(8f9994e0,7f7c04c8) at upageflttrap+0x59

The problem comes from an error path in amap_copy().  Your workload put
some memory pressure which makes amap_chunk_get() fails when trying to
handle a page fault.
Then amap_wipeout() is called on a newly allocated amap which hasn't a
lock.

> [...]
> ddb> show uvm
> Current UVM status:
>   pagesize=4096 (0x1000), pagemask=0xfff, pageshift=12
>   121478 VM pages: 60248 active, 30019 inactive, 65 wired, 5 free (0 zero)
>   min  10% (25) anon, 10% (25) vnode, 5% (12) vtext
>   freemin=4049, free-target=5398, inactive-target=30036, wired-max=40492
>   faults=9051552, traps=9231593, intrs=26202481, ctxswitch=6058861 fpuswitch=0
>   softint=13077905, syscalls=25514901, kmapent=11
>   fault counts:
> noram=1191470, noanon=0, noamap=0, pgwait=7, pgrele=0
> ok relocks(total)=1652265(1652346), anget(retries)=4025885(1468035), 
> amapco
> py=1540576
> neighbor anon/obj pg=1512054/4165883, gets(lock/unlock)=1652565/184269
> cases: anon=3835980, anoncow=189847, obj=1363156, prcopy=288413, 
> przero=337
> 7927
>   daemon and swap counts:
> woke=1165876, revs=1187026, scans=1832267541, obscans=21522202, 
> anscans=181
> 0745339
> busy=0, freed=746569, reactivate=0, deactivate=1694102
> pageouts=94994196, pending=91077, nswget=282429
> nswapdev=1
> swpages=262143, swpginuse=262142, swpgonly=240501 paging=0
>   kernel pointers:
> objs(kern)=0x82158318

The diff below fixes this by setting the "source" amap lock to the newly
allocated one.  This is not strictly necessary on OpenBSD since the amap
is only inserted on the global list at the end of amap copy, but this
satisfies the locking requirement of amap_wipeout() and is IMHO the
simplest solution.  This has also the advantage of reducing the
differences with NetBSD.

The diff below includes other styles issues that I might commit
separately, but it is easier for me to include them since I checked
NetBSD's sources anyway.

Note that your workload, involving an application in ruby26 puts a lot
of pressure on the memory subsystem.  So I wont be surprise if you find
other bugs.  In any case I'd be delighted to look at your bug reports,
thanks a lot!

Index: uvm/uvm_amap.c
===
RCS file: /cvs/src/sys/uvm/uvm_amap.c,v
retrieving revision 1.89
diff -u -p -r1.89 uvm_amap.c
--- uvm/uvm_amap.c  26 Mar 2021 13:40:05 -  1.89
+++ uvm/uvm_amap.c  5 Aug 2021 10:11:45 -
@@ -525,7 +525,6 @@ amap_wipeout(struct vm_amap *amap)
/*
 * Finally, destroy the amap.
 */
-   amap->am_ref = 0;   /* ... was one */
amap->am_nused = 0;
amap_unlock(amap);
amap_free(amap);
@@ -555,13 +554,17 @@ amap_copy(struct vm_map *map, struct vm_
int i, j, k, n, srcslot;
struct vm_amap_chunk *chunk = NULL, *srcchunk = NULL;
struct vm_anon *anon;
+   vsize_t len;
 
KASSERT(map != kernel_map); /* we use sleeping locks */
 
+   srcamap = entry->aref.ar_amap;
+   len = entry->end - entry->start;
+
/*
 * Is there an amap to copy?  If not, create one.
 */
-   if (entry->aref.ar_amap == NULL) {
+   if (srcamap == NULL) {
/*
 * Check to see if we have a large amap that we can
 * chunk.  We align startva/endva to chunk-sized
@@ -571,7 +574,7 @@ amap_copy(struct vm_map *map, struct vm_
 * that makes it grow or shrink dynamically with
 * the number of slots.
 */
-   if (atop(entry->end - entry->start) >= UVM_AMAP_LARGE) {
+   if (atop(len) >= UVM_AMAP_LARGE) {
if (canchunk) {
/* convert slots to bytes */
chunksize = UVM_AMAP_CHUNK << PAGE_SHIFT;
@@ -586,10 +589,10 @@