Re: newbus' ivar's limitation..

On Thursday, July 12, 2012 3:01:36 am Arnaud Lacombe wrote:
> Hi,
> 
> On Thu, Jul 12, 2012 at 1:20 AM, Warner Losh <imp_at_bsdimp.com> wrote:
> > I'm sorry you feel that way.
> >
> > Honestly, though, I think you'll be more pissed when you find out that the 
N:1 interface that you want is being done in the wrong domain.  But I've been 
wrong before and look forward to seeing your replacement.
> >
> > acpi_pcib_acpi.c, btw, implements both PCIB interfaces and ACPI 
interfaces.
> >
> Does it ? From the definition of `acpi_pcib_acpi_methods', I can only
> see a single pcib(4) interface being exported. Moreover, I do not seem
> to be able to find any clue that would led any ACPI devices to attach
> on acpi_pcib_acpi(4), neither to find how could acpi_get_flags() ends
> up in acpi_pcib_read_ivar() ?

acpi_get_handle() is certainly supported.

Relevant code snippets:

sys/dev/acpica/acpivar.h:

/*
 * Note that the low ivar values are reserved to provide
 * interface compatibility with ISA drivers which can also
 * attach to ACPI.
 */
#define ACPI_IVAR_HANDLE	0x100
#define ACPI_IVAR_UNUSED	0x101	/* Unused/reserved. */
#define ACPI_IVAR_PRIVATE	0x102
#define ACPI_IVAR_FLAGS		0x103

/*
 * Accessor functions for our ivars.  Default value for BUS_READ_IVAR is
 * (type) 0.  The <sys/bus.h> accessor functions don't check return values.
 */
#define __ACPI_BUS_ACCESSOR(varp, var, ivarp, ivar, type)	\
								\
static __inline type varp ## _get_ ## var(device_t dev)		\
{								\
    uintptr_t v = 0;						\
    BUS_READ_IVAR(device_get_parent(dev), dev,			\
	ivarp ## _IVAR_ ## ivar, &v);				\
    return ((type) v);						\
}								\
								\
static __inline void varp ## _set_ ## var(device_t dev, type t)	\
{								\
    uintptr_t v = (uintptr_t) t;				\
    BUS_WRITE_IVAR(device_get_parent(dev), dev,			\
	ivarp ## _IVAR_ ## ivar, v);				\
}

__ACPI_BUS_ACCESSOR(acpi, handle, ACPI, HANDLE, ACPI_HANDLE)
__ACPI_BUS_ACCESSOR(acpi, private, ACPI, PRIVATE, void *)
__ACPI_BUS_ACCESSOR(acpi, flags, ACPI, FLAGS, int)

sys/dev/acpica/acpi_pcib_acpi.c:

/*
 * Support for standard PCI bridge ivars.
 */
static int
acpi_pcib_read_ivar(device_t dev, device_t child, int which, uintptr_t 
*result)
{
    struct acpi_hpcib_softc	*sc = device_get_softc(dev);

    switch (which) {
    case PCIB_IVAR_DOMAIN:
	*result = sc->ap_segment;
	return (0);
    case PCIB_IVAR_BUS:
	*result = sc->ap_bus;
	return (0);
    case ACPI_IVAR_HANDLE:
	*result = (uintptr_t)sc->ap_handle;
	return (0);
    case ACPI_IVAR_FLAGS:
	*result = (uintptr_t)sc->ap_flags;
	return (0);
    }
    return (ENOENT);
}

sys/dev/acpica/acpi_pcib_pci.c:

static int
acpi_pcib_read_ivar(device_t dev, device_t child, int which, uintptr_t 
*result)
{
    struct acpi_pcib_softc *sc = device_get_softc(dev);

    switch (which) {
    case ACPI_IVAR_HANDLE:
	*result = (uintptr_t)sc->ap_handle;
	return (0);
    }
    return (pcib_read_ivar(dev, child, which, result));
}

This is used by the ACPI PCI bus driver to detect buses that are enumerated 
via ACPI and to then provide the ACPI_IVAR_HANDLE for all such PCI devices.
Note that ACPI PCI uses its own ivars structure (acpi_pci_devinfo) that 
extends the base PCI ivars to add ACPI handle and flags.

sys/dev/acpi/acpi_pci.c:

struct acpi_pci_devinfo {
	struct pci_devinfo	ap_dinfo;
	ACPI_HANDLE		ap_handle;
	int			ap_flags;
};

...

static int
acpi_pci_read_ivar(device_t dev, device_t child, int which, uintptr_t *result)
{
    struct acpi_pci_devinfo *dinfo;

    dinfo = device_get_ivars(child);
    switch (which) {
    case ACPI_IVAR_HANDLE:
	*result = (uintptr_t)dinfo->ap_handle;
	return (0);
    case ACPI_IVAR_FLAGS:
	*result = (uintptr_t)dinfo->ap_flags;
	return (0);
    }
    return (pci_read_ivar(dev, child, which, result));
}

...

static int
acpi_pci_attach(device_t dev)
{
	...
	/*
	 * First, PCI devices are added as in the normal PCI bus driver.
	 * Afterwards, the ACPI namespace under the bridge driver is
	 * walked to save ACPI handles to all the devices that appear in
	 * the ACPI namespace as immediate descendants of the bridge.
	 *
	 * XXX: Sometimes PCI devices show up in the ACPI namespace that
	 * pci_add_children() doesn't find.  We currently just ignore
	 * these devices.
	 */
	pci_add_children(dev, domain, busno, sizeof(struct acpi_pci_devinfo));
	AcpiWalkNamespace(ACPI_TYPE_DEVICE, acpi_get_handle(dev), 1,
	    acpi_pci_save_handle, NULL, dev, NULL);
	...
}

The main use of this feature is to get PCI interrupt routing to work correctly 
for ACPI bridges (i.e. using ACPI's _PRT method to route interrupts for all 
ACPI-enumerated bridges).  The first way this is accomplished is by having 
each ACPI PCI bridge driver export it's own handle as the handle for it's 
child.  This allows the ACPI PCI bus driver to detect a bus that is enumerated 
via ACPI and export ACPI handle ivars for each PCI device:

sys/dev/acpica/acpi_pci.c:

static int
acpi_pci_probe(device_t dev)
{

	if (acpi_get_handle(dev) == NULL)
		return (ENXIO);
	device_set_desc(dev, "ACPI PCI bus");
	return (0);
}

(Note: it is calling acpi_get_handle() on a pcib device!)

Secondly, to handle PCI-PCI bridges the ACPI PCI-PCI bridge driver checks for 
a valid ACPI handle on each PCI-PCI bridge device and returns a higher 
priority to claim that bridge (rather than the generic PCI-PCI bridge driver).  
Note again that it is calling acpi_get_handle() on a PCI device:

sys/dev/acpica/acpi_pcib_pci.c:

static int
acpi_pcib_pci_probe(device_t dev)
{

    if (pci_get_class(dev) != PCIC_BRIDGE ||
	pci_get_subclass(dev) != PCIS_BRIDGE_PCI ||
	acpi_disabled("pci"))
	return (ENXIO);
    if (acpi_get_handle(dev) == NULL)
	return (ENXIO);
    if (pci_cfgregopen() == 0)
	return (ENXIO);

    device_set_desc(dev, "ACPI PCI-PCI bridge");
    return (-1000);
}

New-bus is certainly not the only way to organize a device hierarchy and is 
not perfect, but in your case I suggest you tone down your language until you 
have enough information to develop an informed opinion.

-- 
John Baldwin