kern/119675: [acpi] apic_hpet0 probe causes divide by zero kernel panic

[John Baldwin]

The following reply was made to PR kern/119675; it has been noted by GNATS.

From: John Baldwin <jhb at FreeBSD.org>
To: bug-followup at freebsd.org,
 bicknell at ufp.org
Cc:  
Subject: Re: kern/119675: [acpi] apic_hpet0 probe causes divide by zero kernel panic
Date: Tue, 15 Jan 2008 10:13:07 -0500

 You can try the patch below.  It fixes a couple of places where we don't
 honor the spec (we don't shut it off in S1 and S2 as required and we don't
 preserve reserved bits in the global configuration register).  It also
 fails the attach if the period is zero which should fix your panic and
 just leave you with no HPET.
 
 Index: acpi_hpet.c
 ===================================================================
 RCS file: /host/cvs/usr/cvs/src/sys/dev/acpica/acpi_hpet.c,v
 retrieving revision 1.12
 diff -u -r1.12 acpi_hpet.c
 --- acpi_hpet.c	9 Oct 2007 07:48:07 -0000	1.12
 +++ acpi_hpet.c	15 Jan 2008 14:53:21 -0000
 @@ -82,6 +82,24 @@
  	return (bus_read_4(sc->mem_res, HPET_OFFSET_VALUE));
  }
  
 +static void
 +hpet_enable(struct acpi_hpet_softc *sc)
 +{
 +	uint32_t val;
 +	
 +	val = bus_read_4(sc->mem_res, HPET_OFFSET_ENABLE);
 +	bus_write_4(sc->mem_res, HPET_OFFSET_ENABLE, val | 1);
 +}
 +
 +static void
 +hpet_disable(struct acpi_hpet_softc *sc)
 +{
 +	uint32_t val;
 +	
 +	val = bus_read_4(sc->mem_res, HPET_OFFSET_ENABLE);
 +	bus_write_4(sc->mem_res, HPET_OFFSET_ENABLE, val & ~1);
 +}
 +
  /* Discover the HPET via the ACPI table of the same name. */
  static void 
  acpi_hpet_identify(driver_t *driver, device_t parent)
 @@ -166,10 +184,16 @@
  	}
  
  	/* Be sure timer is enabled. */
 -	bus_write_4(sc->mem_res, HPET_OFFSET_ENABLE, 1);
 +	hpet_enable(sc);
  
  	/* Read basic statistics about the timer. */
  	val = bus_read_4(sc->mem_res, HPET_OFFSET_PERIOD);
 +	if (val == 0) {
 +		device_printf(dev, "invalid period\n");
 +		hpet_disable(sc);
 +		bus_free_resource(dev, SYS_RES_MEMORY, sc->mem_res);
 +	}
 +
  	freq = (1000000000000000LL + val / 2) / val;
  	if (bootverbose) {
  		val = bus_read_4(sc->mem_res, HPET_OFFSET_INFO);
 @@ -192,7 +216,7 @@
  	val2 = bus_read_4(sc->mem_res, HPET_OFFSET_VALUE);
  	if (val == val2) {
  		device_printf(dev, "HPET never increments, disabling\n");
 -		bus_write_4(sc->mem_res, HPET_OFFSET_ENABLE, 0);
 +		hpet_disable(sc);
  		bus_free_resource(dev, SYS_RES_MEMORY, sc->mem_res);
  		return (ENXIO);
  	}
 @@ -214,13 +238,29 @@
  }
  
  static int
 +acpi_hpet_suspend(device_t dev)
 +{
 +	struct acpi_hpet_softc *sc;
 +
 +	/*
 +	 * Disable the timer during suspend.  The timer will not lose
 +	 * its state in S1 or S2, but we are required to disable
 +	 * it.
 +	 */
 +	sc = device_get_softc(dev);
 +	hpet_disable(sc);
 +
 +	return (0);
 +}
 +
 +static int
  acpi_hpet_resume(device_t dev)
  {
  	struct acpi_hpet_softc *sc;
  
  	/* Re-enable the timer after a resume to keep the clock advancing. */
  	sc = device_get_softc(dev);
 -	bus_write_4(sc->mem_res, HPET_OFFSET_ENABLE, 1);
 +	hpet_enable(sc);
  
  	return (0);
  }
 @@ -260,6 +300,7 @@
  	DEVMETHOD(device_probe, acpi_hpet_probe),
  	DEVMETHOD(device_attach, acpi_hpet_attach),
  	DEVMETHOD(device_detach, acpi_hpet_detach),
 +	DEVMETHOD(device_suspend, acpi_hpet_suspend),
  	DEVMETHOD(device_resume, acpi_hpet_resume),
  
  	{0, 0}
 
 -- 
 John Baldwin