From owner-freebsd-drivers@freebsd.org Mon Jul 17 17:33:33 2017 Return-Path: Delivered-To: freebsd-drivers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id B5753D9B45B; Mon, 17 Jul 2017 17:33:33 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from mail.baldwin.cx (bigwig.baldwin.cx [96.47.65.170]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 91C3D68C35; Mon, 17 Jul 2017 17:33:33 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from ralph.baldwin.cx (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by mail.baldwin.cx (Postfix) with ESMTPSA id B9F8110AF0F; Mon, 17 Jul 2017 13:33:26 -0400 (EDT) From: John Baldwin To: freebsd-acpi@freebsd.org Cc: Jia-Ju Bai , freebsd-drivers@freebsd.org Subject: Re: [Bug 220096][PATCH] acpi_thermal: Fix a possible sleep-under-mutex bug in acpi_tz_thread Date: Mon, 17 Jul 2017 09:38:54 -0700 Message-ID: <6854694.QsTIBa8hWt@ralph.baldwin.cx> User-Agent: KMail/4.14.10 (FreeBSD/11.0-STABLE; KDE/4.14.10; amd64; ; ) In-Reply-To: <20170618095245.40693-1-baijiaju1990@163.com> References: <20170618095245.40693-1-baijiaju1990@163.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.baldwin.cx); Mon, 17 Jul 2017 13:33:26 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.99.2 at mail.baldwin.cx X-Virus-Status: Clean X-BeenThere: freebsd-drivers@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Writing device drivers for FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Jul 2017 17:33:33 -0000 On Sunday, June 18, 2017 05:52:45 PM Jia-Ju Bai wrote: > The driver may sleep under a mutex, and the code path is: > acpi_tz_thread [line 992: acquire the mutex] > acpi_tz_thread [line 993] > acpi_tz_thread [line 1003] > acpi_tz_thread [line 1004] (msleep is excuted) > acpi_tz_thread [line 1008] > acpi_tz_thread [line 970] > acpi_tz_thread [line 971] > acpi_tz_thread [line 975] > malloc(M_WAITOK) [line 976] > > The possible fix of this bug is to replace "M_WAITOK" in malloc with > "M_NOWAIT". > > This bug is found by a static analysis tool written by myself, and it is > checked by my review of the FreeBSD code. > > Signed-off-by: Jia-Ju Bai > --- > sys/dev/acpica/acpi_thermal.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sys/dev/acpica/acpi_thermal.c b/sys/dev/acpica/acpi_thermal.c > index b2b2a13aa88..fb9f44b5711 100644 > --- a/sys/dev/acpica/acpi_thermal.c > +++ b/sys/dev/acpica/acpi_thermal.c > @@ -974,7 +974,7 @@ acpi_tz_thread(void *arg) > } > devclass_get_devices(acpi_tz_devclass, &devs, &devcount); > sc = malloc(sizeof(struct acpi_tz_softc *) * devcount, M_TEMP, > - M_WAITOK | M_ZERO); > + M_NOWAIT | M_ZERO); > for (i = 0; i < devcount; i++) > sc[i] = device_get_softc(devs[i]); > } As noted in the followup to the PR, the lock is never held when malloc is called because msleep() uses PDROP. The malloc is safe to stay as M_WAITOK. -- John Baldwin From owner-freebsd-drivers@freebsd.org Thu Jul 20 17:28:25 2017 Return-Path: Delivered-To: freebsd-drivers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 393A1C7C8C8 for ; Thu, 20 Jul 2017 17:28:25 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from mail.baldwin.cx (bigwig.baldwin.cx [96.47.65.170]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id EF074712DA for ; Thu, 20 Jul 2017 17:28:24 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from ralph.baldwin.cx (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by mail.baldwin.cx (Postfix) with ESMTPSA id 287B910AF09; Thu, 20 Jul 2017 13:28:18 -0400 (EDT) From: John Baldwin To: freebsd-drivers@freebsd.org Cc: Krishna Yenduri Subject: Re: ACPI mediated Hotplug? Date: Thu, 20 Jul 2017 10:19:38 -0700 Message-ID: <3585281.TQq0CdjIrL@ralph.baldwin.cx> User-Agent: KMail/4.14.10 (FreeBSD/11.0-STABLE; KDE/4.14.10; amd64; ; ) In-Reply-To: <3DDBEFB3-AB51-4AD2-845D-A8B81ECB417A@brkt.com> References: <3DDBEFB3-AB51-4AD2-845D-A8B81ECB417A@brkt.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.baldwin.cx); Thu, 20 Jul 2017 13:28:18 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.99.2 at mail.baldwin.cx X-Virus-Status: Clean X-BeenThere: freebsd-drivers@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Writing device drivers for FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Jul 2017 17:28:25 -0000 On Monday, May 22, 2017 11:12:43 AM Krishna Yenduri wrote: > Hi Folks, > > I have a AWS instance (m4.large) running FreeBSD 11.0-STABLE-amd64-2017-04-20. > The system came up with a primary interface ixv0. Per pciconf - > > ixv0@pci0:0:3:0: class=0x020000 card=0x00000000 chip=0x10ed8086 rev=0x01 hdr=0x00 > vendor = 'Intel Corporation' > device = '82599 Ethernet Controller Virtual Function' > class = network > subclass = ethernet > > > I attached another network interface while the system is up. But, it does > not show up in the pciconf -l output. Doing > # devctl rescan pci0 > causes it to appear. > > I want to make the interface appear automatically rather > than having to run the devctl command. BTW, this works on Linux. > Is there a way to use the ACPI framework in FreeBSD so > that the Hotplug happens automatically? > > DTracing shows that adding a new interface triggers calls to > AcpiEvSciXruptHandler and AcpiEvAsynchExecuteGpeMethod. > Is it safe to add a hook in to pci_rescan_method() from these > routines? Any pointers to the ACPI<->FreeBSD kernel interface > are much appreciated. It is possible to support HotPlug via ACPI, however it is a bit more involved than that. There are ACPI methods for handling hotplug that are generic to ACPI (not PCI specific) which on some systems handle PCI hotplug. I believe there is a section discussing it in the ACPI spec. More details are present in the "Device Insertion, Removal, and Status Objects" section of the "Device Configuration" section of the ACPI spec (at least as of 6.1). You might be able to make this work by installing a notify handler in the ACPI PCI bus driver (sys/dev/acpica/acpi_pci.c) and triggering a rescan in response to notifications with a value of 0. Something like this perhaps (untested): Index: acpi_pci.c =================================================================== --- acpi_pci.c (revision 321295) +++ acpi_pci.c (working copy) @@ -71,9 +71,11 @@ CTASSERT(ACPI_STATE_D2 == PCI_POWERSTATE_D2); CTASSERT(ACPI_STATE_D3 == PCI_POWERSTATE_D3); static struct pci_devinfo *acpi_pci_alloc_devinfo(device_t dev); +static int acpi_pci_attach(device_t dev); static void acpi_pci_child_deleted(device_t dev, device_t child); static int acpi_pci_child_location_str_method(device_t cbdev, device_t child, char *buf, size_t buflen); +static int acpi_pci_detach(device_t dev); static int acpi_pci_probe(device_t dev); static int acpi_pci_read_ivar(device_t dev, device_t child, int which, uintptr_t *result); @@ -89,6 +91,8 @@ static bus_dma_tag_t acpi_pci_get_dma_tag(device_t static device_method_t acpi_pci_methods[] = { /* Device interface */ DEVMETHOD(device_probe, acpi_pci_probe), + DEVMETHOD(device_attach, acpi_pci_attach), + DEVMETHOD(device_detach, acpi_pci_detach), /* Bus interface */ DEVMETHOD(bus_read_ivar, acpi_pci_read_ivar), @@ -326,6 +330,45 @@ acpi_pci_probe(device_t dev) return (BUS_PROBE_DEFAULT); } +static void +acpi_pcib_notify_handler(ACPI_HANDLE h, UINT32 notify, void *context) +{ + device_t dev; + + dev = context; + + switch (notify) { + case ACPI_NOTIFY_BUS_CHECK: + BUS_RESCAN(dev); + break; + default: + device_printf(dev, "unknown notify %#x\n", notify); + break; + } +} + +static int +acpi_pci_attach(device_t dev) +{ + int error; + + error = pci_attach(dev); + if (error) + return (error); + AcpiInstallNotifyHandler(acpi_get_handle(dev), ACPI_DEVICE_NOTIFY, + acpi_pci_notify_handler, dev); + return (0); +} + +static int +acpi_pci_detach(device_t dev) +{ + + AcpiRemoveNotifyHandler(acpi_get_handle(dev), ACPI_DEVICE_NOTIFY, + acpi_pci_notify_handler); + return (pci_detach(dev)); +} + #ifdef ACPI_DMAR bus_dma_tag_t dmar_get_dma_tag(device_t dev, device_t child); static bus_dma_tag_t -- John Baldwin From owner-freebsd-drivers@freebsd.org Thu Jul 20 17:28:25 2017 Return-Path: Delivered-To: freebsd-drivers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 1E76DC7C8C6 for ; Thu, 20 Jul 2017 17:28:25 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from mail.baldwin.cx (bigwig.baldwin.cx [96.47.65.170]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id EF0F6712DB; Thu, 20 Jul 2017 17:28:24 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from ralph.baldwin.cx (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by mail.baldwin.cx (Postfix) with ESMTPSA id 78DF610AF07; Thu, 20 Jul 2017 13:28:16 -0400 (EDT) From: John Baldwin To: freebsd-drivers@freebsd.org, Ram Kishore Vegesna Cc: "mav@freebsd.org" , Stephen Hurd Subject: Re: Upstream Request: Fiber channel driver for Broadcom/Emulex FC host bus adapter. Date: Thu, 20 Jul 2017 10:28:08 -0700 Message-ID: <1624453.JHbxFsdJUZ@ralph.baldwin.cx> User-Agent: KMail/4.14.10 (FreeBSD/11.0-STABLE; KDE/4.14.10; amd64; ; ) In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (mail.baldwin.cx); Thu, 20 Jul 2017 13:28:16 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.99.2 at mail.baldwin.cx X-Virus-Status: Clean X-BeenThere: freebsd-drivers@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Writing device drivers for FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Jul 2017 17:28:25 -0000 On Wednesday, May 10, 2017 11:50:21 AM Ram Kishore Vegesna via freebsd-drivers wrote: > Hi all, > > We are planning to upstream/inbox our FreeBSD cam driver which supports > Emulex FC host bus adapters (LPe16xx and LPe32xx family). > > Please provide me the inputs on up-streaming driver process for FreeBSD. If > you can share any documents related to that will be of great help. Unfortunately there isn't a white paper that describes how to upstream drivers very well. However, here are some suggestions: 1) Create a phabricator account at reviews.freebsd.org and upload a diff there to begin the code review process. There is a command line client ("arcanist") for phabricator that you can use to upload a diff from a git or hg branch or from a modified svn checkout. 2) Find a reviewer. This can happen in a couple of ways. I've cc'd Alexander Motin (mav@) who has worked on the isp(4) driver for QLogic HBAs that support FC. He will hopefully be able to either review or help forward you to another person who can. I've also cc'd Stephen Hurd who is a committer who works at Broadcom on the bnxt(4) NIC driver as he might be a good resource, and might be able to give useful advice on the upstreaming process. Also, apologies for the latency in the reply. -- John Baldwin