NetBSD Problem Report #55003

From www@netbsd.org  Sat Feb 22 14:30:30 2020
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 6C79C1A9217
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 22 Feb 2020 14:30:30 +0000 (UTC)
Message-Id: <20200222143029.175BB1A9218@mollari.NetBSD.org>
Date: Sat, 22 Feb 2020 14:30:29 +0000 (UTC)
From: bobs@thelibertytree.org
Reply-To: bobs@thelibertytree.org
To: gnats-bugs@NetBSD.org
Subject: Divice is not blacklisted in autoconfig, have fix
X-Send-Pr-Version: www-1.0

>Number:         55003
>Category:       port-prep
>Synopsis:       Divice is not blacklisted in autoconfig, have fix
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    port-prep-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Feb 22 14:35:01 +0000 2020
>Last-Modified:  Mon Feb 24 22:45:01 +0000 2020
>Originator:     Tim
>Release:        8.1_STABLE
>Organization:
>Environment:
I cross-compile so build machine is:
NetBSD 8.1_STABLE (GENERIC) #0: Fri Jan 24 18:50:19 UTC 2020  mkrepro@mkrepro.NetBSD.org:/usr/src/sys/arch/amd64/compile/GENERIC amd64

Target machine is:
NetBSD 8.1_STABLE (7248-132.v6) #11: Sat Feb 22 00:36:53 MST 2020  XXXX@XXXX:/usr/obj/prep/netbsd-8/objdir/sys/arch/prep/compile/GENERIC prep
>Description:
In the process of troubleshooting a different problem, I noticed that with the kernel option "NVRAM_DUMP" enabled, the "isabeep" device is being fully processed by the function "device_register" in:
/usr/src/sys/arch/prep/prep/autoconf.c
leading to a malformed fw-path result. That function has a specific check at the beginning to bypass any devices that have no capability to boot. I believe the "isabeep" device should have been added to that group of blacklisted devices.
>How-To-Repeat:
Enable kernel option "NVRAM_DUMP" in prep kernel config.
Make sure "isabeep" device is also enabled, it is by default in GENERIC kernel config.

Boot compiled kernel and review NVRAN_DUMP debug output in dmesg.
>Fix:
--- /usr/src/sys/arch/prep/prep/autoconf.c      2020-02-22 06:32:58.907436056 -0700
+++ /usr/src/sys/arch/prep/prep/autoconf.c      2020-02-22 06:36:20.788502695 -0700
@@ -117,7 +117,8 @@
            device_is_a(dev, "pckbc") || device_is_a(dev, "pckbd") ||
            device_is_a(dev, "vga") || device_is_a(dev, "wsdisplay") ||
            device_is_a(dev, "wskbd") || device_is_a(dev, "wsmouse") ||
-           device_is_a(dev, "pms") || device_is_a(dev, "cpu"))
+           device_is_a(dev, "pms") || device_is_a(dev, "cpu") ||
+           device_is_a(dev, "isabeep"))
                return;

        if (device_is_a(dev, "pchb")) {

>Audit-Trail:
From: T <bobs@thelibertytree.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: port-prep/55003
Date: Mon, 24 Feb 2020 02:54:17 -0700

 Hello, I have a modification for this ticket. There are a couple more 
 devices that should be blacklisted and I figured out a related problem 
 in the same area that overlaps with this function. The problem is 
 described in this mailing list post: 
 http://mail-index.netbsd.org/port-prep/2020/02/24/msg000180.html 
 Attached is a new patch so the prior one can be ignored.


 --- /usr/src/sys/arch/prep/prep/autoconf.c     2020-02-24 
 02:34:49.760979074 -0700
 +++ /usr/src/sys/arch/prep/prep/autoconf.c  2020-02-24 
 02:30:18.387739603 -0700
 @@ -117,7 +117,9 @@
              device_is_a(dev, "pckbc") || device_is_a(dev, "pckbd") ||
              device_is_a(dev, "vga") || device_is_a(dev, "wsdisplay") ||
              device_is_a(dev, "wskbd") || device_is_a(dev, "wsmouse") ||
 -           device_is_a(dev, "pms") || device_is_a(dev, "cpu"))
 +           device_is_a(dev, "pms") || device_is_a(dev, "cpu") ||
 +           device_is_a(dev, "nvram") || device_is_a(dev, "isabeep") ||
 +           device_is_a(dev, "audio"))
                  return;

          if (device_is_a(dev, "pchb")) {
 @@ -218,6 +220,11 @@
                  n += snprintf(devpath + n, sizeof(devpath) - n, "%d",
                      device_unit(dev));
          }
 +       /* Catch buses so they do not show up in the fw-path. DV_BUS? */
 +       if (device_is_a(dev, "isa") || device_is_a(dev, "isapnp") ||
 +           device_is_a(dev, "scsibus") || device_is_a(dev, "umass") ||
 +           device_is_a(dev, "usb") || device_is_a(dev, "uhub"))
 +               n = snprintf(devpath, sizeof(devpath), "");

          str1 = prop_string_create_cstring(devpath);
          KASSERT(str1 != NULL);

From: mlelstv@serpens.de (Michael van Elst)
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: port-prep/55003
Date: Mon, 24 Feb 2020 11:16:09 -0000 (UTC)

 bobs@thelibertytree.org (T) writes:

 > Hello, I have a modification for this ticket. There are a couple more 
 > devices that should be blacklisted and I figured out a related problem 
 > in the same area that overlaps with this function.

 Wouldn't it be much easier to stay with the whitelist?

 This would also avoid using an uninitialized devpath[]. I wonder
 why the compiler doesn't warn about it.

 -- 
 -- 
                                 Michael van Elst
 Internet: mlelstv@serpens.de
                                 "A potential Snark may lurk in every tree."

From: T <bobs@thelibertytree.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: port-prep/55003
Date: Mon, 24 Feb 2020 15:44:10 -0700

 Hi mlelstv, thanks for the response. I don't know the historical context 
 for why the original design choices where made, but simply worked with 
 the existing code, which implemented a blacklist of devices to skip from 
 further processing. If you think using a whitelist would be better, I 
 wouldn't challenge the idea because you are way more knowledgeable, I'm 
 very new to this. There are some concerns from my perspective about the 
 current situation. The main one being that I only have one PReP machine, 
 so it could potentially cause issues for other ones. The port-prep 
 mailing list does see some activity, but it is difficult to find other 
 active users for testing issues I find or potential changes. This makes 
 me hesitant to introduce major revisions to existing code since I can't 
 get verification that issues are specific to my machine. Whitelisting 
 based on device_class would probably be ideal, but as you mentioned on 
 IRC, enumerations like DEV_BUS are not currently implemented in bus 
 drivers. That would require all potential bus devices to be checked with 
 device_is_a and may be comparable in size to the existing blacklist. 
 Perhaps a comment should be added to the function to recommend a 
 re-write using a whitelist when those enumerations are widely implemented?

 On 2/24/20 4:20 AM, Michael van Elst wrote:
 > The following reply was made to PR port-prep/55003; it has been noted by GNATS.
 >
 > From: mlelstv@serpens.de (Michael van Elst)
 > To: gnats-bugs@netbsd.org
 > Cc:
 > Subject: Re: port-prep/55003
 > Date: Mon, 24 Feb 2020 11:16:09 -0000 (UTC)
 >
 >   bobs@thelibertytree.org (T) writes:
 >   
 >   > Hello, I have a modification for this ticket. There are a couple more
 >   > devices that should be blacklisted and I figured out a related problem
 >   > in the same area that overlaps with this function.
 >   
 >   Wouldn't it be much easier to stay with the whitelist?
 >   
 >   This would also avoid using an uninitialized devpath[]. I wonder
 >   why the compiler doesn't warn about it.
 >   
 >   --
 >   --
 >                                   Michael van Elst
 >   Internet: mlelstv@serpens.de
 >                                   "A potential Snark may lurk in every tree."
 >   

Home	PR Database Search