NetBSD Problem Report #48960

From campbell@mumble.net  Wed Jul  2 19:46:56 2014
Return-Path: <campbell@mumble.net>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id AC029A6526
	for <gnats-bugs@gnats.NetBSD.org>; Wed,  2 Jul 2014 19:46:56 +0000 (UTC)
Message-Id: <20140702194215.9643D6050C@jupiter.mumble.net>
Date: Wed,  2 Jul 2014 19:42:15 +0000 (UTC)
From: Taylor R Campbell <campbell+netbsd@mumble.net>
Reply-To: Taylor R Campbell <campbell+netbsd@mumble.net>
To: gnats-bugs@gnats.NetBSD.org
Subject: ichlpcib maps I/O space from ACPI BAR which ichsmb wants
X-Send-Pr-Version: 3.95

>Number:         48960
>Category:       kern
>Synopsis:       ichlpcib maps I/O space from ACPI BAR which ichsmb wants
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Jul 02 19:50:00 +0000 2014
>Closed-Date:    Sun May 29 23:49:47 +0000 2016
>Last-Modified:  Sun May 29 23:49:47 +0000 2016
>Originator:     Taylor R Campbell <campbell+netbsd@mumble.net>
>Release:        NetBSD-current as of 2014-07-02
>Organization:
>Environment:
Architecture: x86_64
Machine: amd64
>Description:

	ichlpcib(4) maps the space in PCI register 0x40, ACPI PMBASE,
	for acpipmtimer, tcotimer, and speedstep support.  ichsmb(4)
	tries to map space which is sometimes in the middle of this
	region.

>How-To-Repeat:

	Boot a system with ichlpcib and ichsmb.  Observe that ichsmb0
	fails to map its I/O space because ichlpcib0 already mapped
	it.

>Fix:

	matt@ and jakllsch@ say it is wrong for ichlpcib(4) to map the
	ACPI PMBASE region, which it does for acpipmtimer, tcotimer,
	and speedstep support.

	I imagine the Right Thing is to attach these only if ACPI is
	disabled, and to write proper ACPI drivers under sys/dev/acpi
	for them.  Disabling the ad hoc drivers until proper ACPI
	drivers are written is probably not a good idea, however, and I
	don't have time or the datasheets or hardware necessary to port
	the ad hoc drivers to the ACPI subsystem.

>Release-Note:

>Audit-Trail:
From: SAITOH Masanobu <msaitoh@execsw.org>
To: gnats-bugs@NetBSD.org, kern-bug-people@netbsd.org, 
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc: msaitoh@execsw.org
Subject: Re: kern/48960: ichlpcib maps I/O space from ACPI BAR which ichsmb
 wants
Date: Sat, 20 Dec 2014 12:13:29 +0900

 On 2014/07/03 4:50, Taylor R Campbell wrote:
 >> Number:         48960
 >> Category:       kern
 >> Synopsis:       ichlpcib maps I/O space from ACPI BAR which ichsmb wants
 >> Confidential:   no
 >> Severity:       serious
 >> Priority:       medium
 >> Responsible:    kern-bug-people
 >> State:          open
 >> Class:          sw-bug
 >> Submitter-Id:   net
 >> Arrival-Date:   Wed Jul 02 19:50:00 +0000 2014
 >> Originator:     Taylor R Campbell <campbell+netbsd@mumble.net>
 >> Release:        NetBSD-current as of 2014-07-02
 >> Organization:
 >> Environment:
 > Architecture: x86_64
 > Machine: amd64
 >> Description:
 > 
 > 	ichlpcib(4) maps the space in PCI register 0x40, ACPI PMBASE,
 > 	for acpipmtimer, tcotimer, and speedstep support.  ichsmb(4)
 > 	tries to map space which is sometimes in the middle of this
 > 	region.
 > 
 >> How-To-Repeat:
 > 
 > 	Boot a system with ichlpcib and ichsmb.  Observe that ichsmb0
 > 	fails to map its I/O space because ichlpcib0 already mapped
 > 	it.
 > 
 >> Fix:
 > 
 > 	matt@ and jakllsch@ say it is wrong for ichlpcib(4) to map the
 > 	ACPI PMBASE region, which it does for acpipmtimer, tcotimer,
 > 	and speedstep support.
 > 
 > 	I imagine the Right Thing is to attach these only if ACPI is
 > 	disabled, and to write proper ACPI drivers under sys/dev/acpi
 > 	for them.  Disabling the ad hoc drivers until proper ACPI
 > 	drivers are written is probably not a good idea, however, and I
 > 	don't have time or the datasheets or hardware necessary to port
 > 	the ad hoc drivers to the ACPI subsystem.
 > 

  How about the following patch?

  This fixes a bug that ichlpcib(4) maps I/O area incorrectly.
 - The LPCIB_PCI_PMBASE and LPCIB_PCI_GPIO register are alike PCI BAR but not
   completely compatible with it. The PMBASE and GPIO registers define the
   base address and the type but not describe the size. The size is fixed
   to 128bytes. So use pci_mapreg_submap().
 - Make pci_mapreg_submap() extern again.
 - Fix the calculation of the map size in pci_mapreg_submap().



 Index: sys/dev/pci/pcivar.h
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/pci/pcivar.h,v
 retrieving revision 1.100
 diff -u -p -r1.100 pcivar.h
 --- sys/dev/pci/pcivar.h	16 Oct 2014 12:31:23 -0000	1.100
 +++ sys/dev/pci/pcivar.h	19 Dec 2014 17:08:14 -0000
 @@ -269,6 +269,10 @@ int	pci_mapreg_info(pci_chipset_tag_t, p
  int	pci_mapreg_map(const struct pci_attach_args *, int, pcireg_t, int,
  	    bus_space_tag_t *, bus_space_handle_t *, bus_addr_t *,
  	    bus_size_t *);
 +int	pci_mapreg_submap(const struct pci_attach_args *, int, pcireg_t, int,
 +    bus_size_t, bus_size_t, bus_space_tag_t *, bus_space_handle_t *,
 +    bus_addr_t *, bus_size_t *);
 +

  int pci_find_rom(const struct pci_attach_args *, bus_space_tag_t,
  	    bus_space_handle_t, bus_size_t,
 Index: sys/dev/pci/pci_map.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/pci/pci_map.c,v
 retrieving revision 1.31
 diff -u -p -r1.31 pci_map.c
 --- sys/dev/pci/pci_map.c	16 Oct 2014 12:31:23 -0000	1.31
 +++ sys/dev/pci/pci_map.c	19 Dec 2014 17:08:14 -0000
 @@ -43,10 +43,6 @@ __KERNEL_RCSID(0, "$NetBSD: pci_map.c,v
  #include <dev/pci/pcireg.h>
  #include <dev/pci/pcivar.h>

 -static int pci_mapreg_submap(const struct pci_attach_args *, int, pcireg_t, int,
 -    bus_size_t, bus_size_t, bus_space_tag_t *, bus_space_handle_t *,
 -    bus_addr_t *, bus_size_t *);
 -
  static int
  pci_io_find(pci_chipset_tag_t pc, pcitag_t tag, int reg, pcireg_t type,
      bus_addr_t *basep, bus_size_t *sizep, int *flagsp)
 @@ -282,7 +278,7 @@ pci_mapreg_map(const struct pci_attach_a
  	    handlep, basep, sizep);
  }

 -static int
 +int
  pci_mapreg_submap(const struct pci_attach_args *pa, int reg, pcireg_t type,
      int busflags, bus_size_t maxsize, bus_size_t offset, bus_space_tag_t *tagp,
  	bus_space_handle_t *handlep, bus_addr_t *basep, bus_size_t *sizep)
 @@ -324,10 +320,10 @@ pci_mapreg_submap(const struct pci_attac
  	 * pci_mapreg_map.
  	 */

 -	maxsize = (maxsize && offset) ? maxsize : size;
 +	maxsize = (maxsize != 0) ? maxsize : size;
  	base += offset;

 -	if ((maxsize < size && offset + maxsize <= size) || offset != 0)
 +	if ((size < maxsize) || (size < (offset + maxsize)))
  		return 1;

  	if (bus_space_map(tag, base, maxsize, busflags | flags, &handle))
 Index: sys/dev/ic/i82801lpcreg.h
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/ic/i82801lpcreg.h,v
 retrieving revision 1.11
 diff -u -p -r1.11 i82801lpcreg.h
 --- sys/dev/ic/i82801lpcreg.h	23 Jul 2010 02:23:58 -0000	1.11
 +++ sys/dev/ic/i82801lpcreg.h	19 Dec 2014 17:08:14 -0000
 @@ -40,6 +40,7 @@
   * PCI configuration registers
   */
  #define LPCIB_PCI_PMBASE	0x40
 +#define LPCIB_PCI_PM_SIZE	0x00000080
  #define LPCIB_PCI_ACPI_CNTL	0x44
  # define LPCIB_PCI_ACPI_CNTL_EN	(1 << 4)
  /* GPIO config registers ICH6+ */
 @@ -51,6 +52,7 @@
  #define LPCIB_PCI_TCO_CNTL	0x54
  /* GPIO config registers ICH0-ICH5 */
  #define LPCIB_PCI_GPIO_BASE	0x58
 +#define LPCIB_PCI_GPIO_SIZE	0x00000080
  #define LPCIB_PCI_GPIO_CNTL	0x5c
  #define LPCIB_PCI_GPIO_CNTL_EN	(1 << 4)
  #define LPCIB_PCI_PIRQA_ROUT	0x60
 Index: sys/arch/x86/pci/ichlpcib.c
 ===================================================================
 RCS file: /cvsroot/src/sys/arch/x86/pci/ichlpcib.c,v
 retrieving revision 1.44
 diff -u -p -r1.44 ichlpcib.c
 --- sys/arch/x86/pci/ichlpcib.c	15 Dec 2014 13:29:42 -0000	1.44
 +++ sys/arch/x86/pci/ichlpcib.c	19 Dec 2014 17:08:14 -0000
 @@ -321,9 +321,14 @@ lpcibattach(device_t parent, device_t se
  	 * Part of our I/O registers are used as ACPI PM regs.
  	 * Since our ACPI subsystem accesses the I/O space directly so far,
  	 * we do not have to bother bus_space I/O map confliction.
 +	 *
 +	 * The PMBASE register is alike PCI BAR but not completely compatible
 +	 * with it. The PMBASE define the base address and the type but
 +	 * not describe the size.
  	 */
 -	if (pci_mapreg_map(pa, LPCIB_PCI_PMBASE, PCI_MAPREG_TYPE_IO, 0,
 -			   &sc->sc_iot, &sc->sc_ioh, NULL, &sc->sc_iosize)) {
 +	if (pci_mapreg_submap(pa, LPCIB_PCI_PMBASE, PCI_MAPREG_TYPE_IO, 0,
 +		LPCIB_PCI_PM_SIZE, 0, &sc->sc_iot, &sc->sc_ioh, NULL,
 +		&sc->sc_iosize)) {
  		aprint_error_dev(self, "can't map power management i/o space\n");
  		return;
  	}
 @@ -1057,6 +1062,7 @@ lpcib_gpio_configure(device_t self)
  	pcireg_t gpio_cntl;
  	uint32_t use, io, bit;
  	int pin, shift, base_reg, cntl_reg, reg;
 +	int rv;

  	/* this implies ICH >= 6, and thus different mapreg */
  	if (sc->sc_has_rcba) {
 @@ -1073,11 +1079,16 @@ lpcib_gpio_configure(device_t self)
  	/* Is GPIO enabled? */
  	if ((gpio_cntl & LPCIB_PCI_GPIO_CNTL_EN) == 0)
  		return;
 -		
 -	if (pci_mapreg_map(&sc->sc_pa, base_reg, PCI_MAPREG_TYPE_IO, 0,
 -			   &sc->sc_gpio_iot, &sc->sc_gpio_ioh,
 -			   NULL, &sc->sc_gpio_ios)) {
 -		aprint_error_dev(self, "can't map general purpose i/o space\n");
 +	/*
 +	 * The GPIO_BASE register is alike PCI BAR but not completely
 +	 * compatible with it. The PMBASE define the base address and the type
 +	 * but not describe the size.
 +	 */
 +	rv = pci_mapreg_submap(&sc->sc_pa, base_reg, PCI_MAPREG_TYPE_IO, 0,
 +	    LPCIB_PCI_GPIO_SIZE, 0, &sc->sc_gpio_iot, &sc->sc_gpio_ioh,
 +	    NULL, &sc->sc_gpio_ios);
 +	if (rv != 0) {
 +		aprint_error_dev(self, "can't map general purpose i/o space(rv = %d)\n", rv);
  		return;
  	}



 -- 
 -----------------------------------------------
                 SAITOH Masanobu (msaitoh@execsw.org
                                  msaitoh@netbsd.org)

From: "SAITOH Masanobu" <msaitoh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/48960 CVS commit: src/sys
Date: Fri, 26 Dec 2014 05:09:04 +0000

 Module Name:	src
 Committed By:	msaitoh
 Date:		Fri Dec 26 05:09:04 UTC 2014

 Modified Files:
 	src/sys/arch/x86/pci: ichlpcib.c
 	src/sys/dev/ic: i82801lpcreg.h
 	src/sys/dev/pci: pci_map.c pcivar.h

 Log Message:
 Fix a bug that ichlpcib(4) maps I/O area incorrectly and then fails to attach
 gpio. It might also fix ACPI related problem described in PR#48960:
  - The LPCIB_PCI_PMBASE and LPCIB_PCI_GPIO register are alike PCI BAR but not
    completely compatible with it. It's ok because the registers' addresses are
    out of BAR0-BAR5(0x10-0x24) and are located in the device-dependent header.
    The PMBASE and GPIO registers define the base address and the type but not
    describe the size. The size is fixed to 128bytes. So use
    pci_mapreg_submap().
  - Make pci_mapreg_submap() extern again.
  - Fix the calculation of the map size in pci_mapreg_submap().


 To generate a diff of this commit:
 cvs rdiff -u -r1.44 -r1.45 src/sys/arch/x86/pci/ichlpcib.c
 cvs rdiff -u -r1.11 -r1.12 src/sys/dev/ic/i82801lpcreg.h
 cvs rdiff -u -r1.31 -r1.32 src/sys/dev/pci/pci_map.c
 cvs rdiff -u -r1.100 -r1.101 src/sys/dev/pci/pcivar.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/48960 CVS commit: [netbsd-7] src/sys
Date: Thu, 8 Jan 2015 11:39:38 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Thu Jan  8 11:39:38 UTC 2015

 Modified Files:
 	src/sys/arch/x86/pci [netbsd-7]: ichlpcib.c
 	src/sys/dev/ic [netbsd-7]: i82801lpcreg.h
 	src/sys/dev/pci [netbsd-7]: pci_map.c pcivar.h

 Log Message:
 Pull up following revision(s) (requested by msaitoh in ticket #394):
 	sys/dev/pci/pcivar.h: revision 1.101
 	sys/dev/pci/pci_map.c: revision 1.32
 	sys/dev/ic/i82801lpcreg.h: revision 1.12
 	sys/arch/x86/pci/ichlpcib.c: revision 1.45
 Fix a bug that ichlpcib(4) maps I/O area incorrectly and then fails to attach
 gpio. It might also fix ACPI related problem described in PR#48960:
   - The LPCIB_PCI_PMBASE and LPCIB_PCI_GPIO register are alike PCI BAR but not
     completely compatible with it. It's ok because the registers' addresses are
     out of BAR0-BAR5(0x10-0x24) and are located in the device-dependent header.
     The PMBASE and GPIO registers define the base address and the type but not
     describe the size. The size is fixed to 128bytes. So use
     pci_mapreg_submap().
   - Make pci_mapreg_submap() extern again.
   - Fix the calculation of the map size in pci_mapreg_submap().


 To generate a diff of this commit:
 cvs rdiff -u -r1.43 -r1.43.4.1 src/sys/arch/x86/pci/ichlpcib.c
 cvs rdiff -u -r1.11 -r1.11.34.1 src/sys/dev/ic/i82801lpcreg.h
 cvs rdiff -u -r1.30.12.1 -r1.30.12.2 src/sys/dev/pci/pci_map.c
 cvs rdiff -u -r1.99.4.1 -r1.99.4.2 src/sys/dev/pci/pcivar.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->feedback
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Fri, 16 Jan 2015 05:41:04 +0000
State-Changed-Why:
Is this fixed now?


From: "Soren Jacobsen" <snj@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/48960 CVS commit: [netbsd-6] src/sys
Date: Fri, 16 Jan 2015 08:22:25 +0000

 Module Name:	src
 Committed By:	snj
 Date:		Fri Jan 16 08:22:25 UTC 2015

 Modified Files:
 	src/sys/arch/x86/pci [netbsd-6]: ichlpcib.c
 	src/sys/dev/ic [netbsd-6]: i82801lpcreg.h
 	src/sys/dev/pci [netbsd-6]: pci_map.c pcivar.h

 Log Message:
 Pull up following revision(s) (requested by msaitoh in ticket #1229):
 	sys/arch/x86/pci/ichlpcib.c: revision 1.40, 1.45
 	sys/dev/pci/pcivar.h: revision 1.101
 	sys/dev/pci/pci_map.c: revision 1.32
 	sys/dev/ic/i82801lpcreg.h: revision 1.12
 Use '\n' at the end of all aprint_error_dev() format strings.
 --
 Fix a bug that ichlpcib(4) maps I/O area incorrectly and then fails to attach
 gpio. It might also fixes ACPI related problem described in PR#48960:
  - The LPCIB_PCI_PMBASE and LPCIB_PCI_GPIO register are alike PCI BAR but not
    completely compatible with it. It's ok because the registers' addresses are
    out of BAR0-BAR5(0x10-0x24) and are located in the device-dependent header.
    The PMBASE and GPIO registers define the base address and the type but not
    describe the size. The size is fixed to 128bytes. So use
    pci_mapreg_submap().
  - Make pci_mapreg_submap() extern again.
  - Fix the calculation of the map size in pci_mapreg_submap().


 To generate a diff of this commit:
 cvs rdiff -u -r1.34 -r1.34.6.1 src/sys/arch/x86/pci/ichlpcib.c
 cvs rdiff -u -r1.11 -r1.11.14.1 src/sys/dev/ic/i82801lpcreg.h
 cvs rdiff -u -r1.29 -r1.29.10.1 src/sys/dev/pci/pci_map.c
 cvs rdiff -u -r1.98 -r1.98.2.1 src/sys/dev/pci/pcivar.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/48960 CVS commit: [netbsd-5] src/sys
Date: Fri, 23 Jan 2015 16:24:55 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Fri Jan 23 16:24:55 UTC 2015

 Modified Files:
 	src/sys/arch/x86/pci [netbsd-5]: ichlpcib.c
 	src/sys/dev/ic [netbsd-5]: i82801lpcreg.h
 	src/sys/dev/pci [netbsd-5]: pci_map.c

 Log Message:
 Pull up the following changes, requested by msaitoh in ticket #1942:

 sys/arch/x86/pci/ichlpcib.c		1.40, 1.45 via patch
 sys/dev/ic/i82801lpcreg.h		1.12
 sys/dev/pci/pci_map.c			1.32 via patch

 	- Fix a bug that ichlpcib(4) maps I/O area incorrectly. It might also
 	  fixes ACPI related problem described in PR#48960:
 	  - The LPCIB_PCI_PMBASE and LPCIB_PCI_GPIO register are alike PCI BAR
 	    but not completely compatible with it. It's ok because the
 	    registers' addresses are out of BAR0-BAR5(0x10-0x24) and are
 	    located in the device-dependent header. The PMBASE and GPIO
 	    registers define the base address and the type but not describe
 	    the size. The size is fixed to 128bytes. So use
 	    pci_mapreg_submap().
 	  - Fix the calculation of the map size in pci_mapreg_submap().
 	- Use '\n' at the end of aprint_error_dev() format strings.


 To generate a diff of this commit:
 cvs rdiff -u -r1.14.4.2 -r1.14.4.3 src/sys/arch/x86/pci/ichlpcib.c
 cvs rdiff -u -r1.8 -r1.8.10.1 src/sys/dev/ic/i82801lpcreg.h
 cvs rdiff -u -r1.24 -r1.24.4.1 src/sys/dev/pci/pci_map.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: feedback->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sun, 29 May 2016 23:49:47 +0000
State-Changed-Why:
I guess this is fixed.


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.