svn commit: r321720 - head/sys/dev/ichwd

[John Baldwin]

On Sunday, July 30, 2017 03:19:07 PM Alexander Motin wrote:
> Author: mav
> Date: Sun Jul 30 15:19:07 2017
> New Revision: 321720
> URL: https://svnweb.freebsd.org/changeset/base/321720
> 
> Log:
>   Attach ichwd(4) only to ISA bus of the LPC bridge.
>   
>   Resource allocation for parent device does not look good by itself, but
>   attempt to allocate them for unrelated device just does not end up good.
>   On Asus X99-E WS/USB3.1 system reporting ISA bridge via both PCI and ACPI
>   this reported to cause kernel panic on shutdown due to messed resources:
>   https://bugs.freenas.org/issues/25237.
>   
>   MFC after:	1 week
> 
> Modified:
>   head/sys/dev/ichwd/ichwd.c
> 
> Modified: head/sys/dev/ichwd/ichwd.c
> ==============================================================================
> --- head/sys/dev/ichwd/ichwd.c	Sun Jul 30 11:50:16 2017	(r321719)
> +++ head/sys/dev/ichwd/ichwd.c	Sun Jul 30 15:19:07 2017	(r321720)
> @@ -533,23 +533,26 @@ ichwd_event(void *arg, unsigned int cmd, int *error)
>  }
>  
>  static device_t
> -ichwd_find_ich_lpc_bridge(struct ichwd_device **id_p)
> +ichwd_find_ich_lpc_bridge(device_t isa, struct ichwd_device **id_p)
>  {
>  	struct ichwd_device *id;
> -	device_t ich = NULL;
> +	device_t isab;
> +	uint16_t devid;
>  
> -	/* look for an ICH LPC interface bridge */
> -	for (id = ichwd_devices; id->desc != NULL; ++id)
> -		if ((ich = pci_find_device(VENDORID_INTEL, id->device)) != NULL)
> -			break;
> -
> -	if (ich == NULL)
> +	/* Check whether parent ISA bridge looks familiar. */
> +	isab = device_get_parent(isa);
> +	if (pci_get_vendor(isab) != VENDORID_INTEL)
>  		return (NULL);

You probably need to do slightly more work to verify that the isab device is a
PCI child before you use pci_get_vendor().  I believe isa0 can be a child of
legacy0 for example (though I doubt we run on such hardware without PCI anymore).

Seeing if the devclass of device_get_parent(isab) == devclass_find("pci") would
be sufficient.  It might also be nice to add a 'device_is_pci()' helper function
as I think we probably have a similar check in some other places.

The general change is definitely an improvement though.

-- 
John Baldwin