acpi_throttle: quirk based on pci info

[John Baldwin]

On Wednesday 27 February 2008 02:54:38 am Andriy Gapon wrote:
> on 25/02/2008 23:03 Andriy Gapon said the following:
> > Nate,
> > these are very good points. It seems that I wrote something very
> > specific to my case and didn't consider other scenarios. I completely
> > forgot about SMP case.
> > I need to think more about this.
> > A possible blanket solution would be to probe/attach all and any cpu
> > child devices after pci bus is attached. But I don't know if this is a
> > viable solution in general and if it's possible to implement at all.
> 
> I looked through some code while pursuing the above idea. Of course,
> things are more complicated if one has to consider legacy scenarios (no
> ACPI) too, but for the scope of this issue I sticked to the case when
> ACPI is available.
> It seems that children of acpi are added in acpi_probe_children
> function. First we walk over all devices declared in DSDT and create
> child devices for those that are interesting to us. AcpiWalkNamespace
> with acpi_probe_child is used for that. When we call BUS_ADD_CHILD for a
> device we typically specify an order value based on a device's
> level/depth. For some special devices we set specific (early) order
> value in acpi_probe_order (based on HID).
> Then we add devices that are not auto-detected but rather add themselves
> via device_identify method. Then we perform probe and attach for all
> added devices via bus_generic_attach, and the devices are probed and
> attached according to their 'order' attribute.
> From this, my conclusion is that relative order of cpu and pci bridge is
> generally undetermined in this case. This is because neither of them has
> a specific order assigned. So their order depends on their level and,
> well, order in DSDT.
> Typically, processor would have a smaller or equal level compared to
> host-pci bridge. In my particular case I believe that the levels are the
> same:
> \_PR_.CPU0
> \_SB_.PCI0
> And processor appears earlier in DSDT, so they both have the same order
> value but processor is added earlier.
> 
> Another thing is that acpi_cpu and acpi_pcib_acpi (using their "full
> names") are the buses themselves, so they identify/probe/attach their
> children in their attach methods. Thus, whichever has earlier order will
> also probe and attach (all of) its children earlier.
> 
> So what I am getting at - it is certainly not hard to make
> acpi_pcib_acpi (and consecutively pci) be attached earlier than
> acpi_cpu. All is needed is to add another special case in
> acpi_probe_order after the existing ones, I actually did it as an
> experiment:
>     else if (acpi_MatchHid(handle, "PNP0A03"))
>         *order = 4;
> This worked for me as expected and without any side effects.
> 
> But I have a big concern about whether we have any devices/drivers that
> rely on a current typical order. That is, drivers that expect cpu
> device/bus to be already probed and attached at the time when
> probe/attach is called for something on pci bus.
> I guess one example here is famous ichss that adds a child to cpu bus in
> its pci probe routine, and calls device_probe_and_attach for it in the
> same place too. I guess that while BUS_ADD_CHILD should work at that
> place, device_probe_and_attach may not because cpu itself is not probed
> and attached yet.
> 
> On the other hand, as I said in the beginning, I think that even now
> there are no strict guarantees about the order, it just happens to work.
> 
> Anyway, I am not sure if the discussed change in ordering has any merit.
> It seems that whatever the order it would be convenient in some places
> and inconvenient in others, so some jumping through the hoops is inevitable.
> 
> I wonder if it would be possible to create some mechanism for attaching
> "important" devices first and then the rest later. E.g. cpu and pcib+pci
> are attached first (in whatever order), then their children are attached
> later (in whatever order). What is important is that all those "late"
> children can expect both cpu and pci to be fully available.

I actually have had patches for a while to make CPU devices attach in a 
certain order with respect to Host-PCI bridges precisely to help address 
this.  What I did was to force CPUs to probe before PCI devices so then the 
ichss drivers current method (adding devices to CPUs) always works as the 
CPUs are around when ichss goes to attach devices to them.

This is the patch I've had for a while.  It also attaches Host-PCI bridges in 
a more deterministic order:

--- //depot/vendor/freebsd/src/sys/dev/acpica/acpi.c	2007/10/09 07:52:34
+++ //depot/user/jhb/acpipci/dev/acpica/acpi.c	2008/01/22 23:22:19
@@ -1533,18 +1534,32 @@
 static int
 acpi_probe_order(ACPI_HANDLE handle, int *order)
 {
+    ACPI_OBJECT_TYPE type;
+    u_int addr;
 
     /*
      * 1. I/O port and memory system resource holders
      * 2. Embedded controllers (to handle early accesses)
-     * 3. PCI Link Devices
+     * 3. CPUs
+     * 4. PCI Link Devices
+     * 11 - 266. Host-PCI bridges sorted by _ADR
      */
+    AcpiGetType(handle, &type);
     if (acpi_MatchHid(handle, "PNP0C01") || acpi_MatchHid(handle, "PNP0C02"))
 	*order = 1;
     else if (acpi_MatchHid(handle, "PNP0C09"))
 	*order = 2;
+    else if (type == ACPI_TYPE_PROCESSOR)
+	*order = 3;
     else if (acpi_MatchHid(handle, "PNP0C0F"))
-	*order = 3;
+	*order = 4;
+    else if (acpi_MatchHid(handle, "PNP0A03")) {
+	if (ACPI_SUCCESS(acpi_GetInteger(handle, "_ADR", &addr)))
+	    *order = 11 + ACPI_ADR_PCI_SLOT(addr) * (PCI_FUNCMAX + 1) +
+	       ACPI_ADR_PCI_FUNC(addr);
+	else
+	    *order = 11;
+    }
     return (0);
 }
 
@@ -1591,14 +1606,16 @@
 		break;
 
 	    /* 
-	     * Create a placeholder device for this node.  Sort the placeholder
-	     * so that the probe/attach passes will run breadth-first.  Orders
-	     * less than ACPI_DEV_BASE_ORDER are reserved for special objects
-	     * (i.e., system resources).  Larger values are used for all other
-	     * devices.
+	     * Create a placeholder device for this node.  Sort the
+	     * placeholder so that the probe/attach passes will run
+	     * breadth-first.  Orders less than ACPI_DEV_BASE_ORDER
+	     * are reserved for special objects (i.e., system
+	     * resources).  Values between ACPI_DEV_BASE_ORDER and 300
+	     * are reserved for Host-PCI bridges.  Larger values are
+	     * used for all other devices.
 	     */
 	    ACPI_DEBUG_PRINT((ACPI_DB_OBJECTS, "scanning '%s'\n", handle_str));
-	    order = (level + 1) * ACPI_DEV_BASE_ORDER;
+	    order = level * 10 + 300;
 	    acpi_probe_order(handle, &order);
 	    child = BUS_ADD_CHILD(bus, order, NULL, -1);
 	    if (child == NULL)

With this patch PCI drivers that want to work with CPU devices would use the 
same approach as ichss.  I'm not sure that is the best approach though.  The 
fact that smist0 wants to check if ichss0 is attached probably wouldn't work 
as with this patch smist0 would attach first before ichss0 got a chance to 
probe via PCI.

We could flip this around to have PCI bridges first before CPUs.  As you 
suggest.  However, that will break ichss in its current form as cpu0 won't 
exist yet (the device wouldn't be probed, so it would be called cpu0 yet).

This is begging for multi-pass. :(  (But that's another topic).

In practice, I think you could change ichss to not use a PCI probe, but to 
just read config registers directly and hardcode the address of the ICH.  You 
could even walk PCI bus 0 looking for a PCI-ISA bridge instead of hardcoding 
the address I think.  Then ichss would just be a CPU device and not depend on 
the PCI bus at all.  I'm not sure if that is workable for the other case you 
are worried about.

BTW, once an order is decided upon of CPUs relative to Host-PCI bridges we can 
fix that order for the non-ACPI case easily enough in legacy.c.

-- 
John Baldwin