Author: jhb
Date: Thu Apr 27 16:32:42 2017
New Revision: 317510
URL: https://svnweb.freebsd.org/changeset/base/317510

Log:
  Various fixes for PCI _OSC handling so HotPlug works again.
  
  - Rename the default implementation of 'pcib_request_feature' and add
    a pcib_request_feature() wrapper function (as is often done for
    new-bus APIs implemented via kobj) that accepts a single function.
    Previously the call to pcib_request_feature() ended up invoking the
    method on the great-great-grandparent of the bridge device instead
    of the grandparent.  For a bridge that was a direct child of pci0 on
    x86 this resulted in the method skipping over the Host-PCI bridge
    driver and being invoked against nexus0
  - When invoking _OSC from a Host-PCI bridge driver, invoke
    device_get_softc() against the Host-PCI bridge device instead of the
    child bridge that is requesting HotPlug.  Using the wrong softc data
    resulted in garbage being passed for the ACPI handle causing the
    _OSC call to fail.
  - While here, perform some other cleanups to _OSC handling in the ACPI
    Host-PCI bridge driver:
    - Don't invoke _OSC when requesting a control that has already been
      granted by the firmware.
    - Don't set the first word of the capability array before invoking
      _OSC.  This word is always set explicitly by acpi_EvaluateOSC()
      since it is UUID-independent.
    - Don't modify the set of granted controls unless _OSC doesn't exist
      (which is treated as always successful), or the _OSC method
      doesn't fail.
    - Don't require an _OSC status of 0 for success.  _OSC always
      returns the updated control mask even if it returns a non-zero
      status in the first word.
    - Whine if _OSC ever tries to revoke a previously-granted control.
      (It is not supposed to do that.)
  - While here, add constants for the _OSC status word in acpivar.h
    (though currently unused).
  
  Reported by:  adrian
  Reviewed by:  imp
  MFC after:    1 week
  Tested on:    Lenovo x220
  Differential Revision:        https://reviews.freebsd.org/D10520

Modified:
  head/sys/dev/acpica/acpi_pcib_acpi.c
  head/sys/dev/acpica/acpivar.h
  head/sys/dev/pci/pci_pci.c
  head/sys/dev/pci/pcib_private.h

Modified: head/sys/dev/acpica/acpi_pcib_acpi.c
==============================================================================
--- head/sys/dev/acpica/acpi_pcib_acpi.c        Thu Apr 27 16:14:32 2017        
(r317509)
+++ head/sys/dev/acpica/acpi_pcib_acpi.c        Thu Apr 27 16:32:42 2017        
(r317510)
@@ -313,32 +313,42 @@ acpi_pcib_osc(struct acpi_hpcib_softc *s
                0x96, 0x57, 0x74, 0x41, 0xc0, 0x3d, 0xd7, 0x66
        };
 
-       /* Status Field */
-       cap_set[PCI_OSC_STATUS] = 0;
+       /*
+        * Don't invoke _OSC if a control is already granted.
+        * However, always invoke _OSC during attach when 0 is passed.
+        */
+       if (osc_ctl != 0 && (sc->ap_osc_ctl & osc_ctl) == osc_ctl)
+               return (0);
 
        /* Support Field: Extended PCI Config Space, MSI */
        cap_set[PCI_OSC_SUPPORT] = PCIM_OSC_SUPPORT_EXT_PCI_CONF |
            PCIM_OSC_SUPPORT_MSI;
 
        /* Control Field */
-       sc->ap_osc_ctl |= osc_ctl;
-       cap_set[PCI_OSC_CTL] = sc->ap_osc_ctl;
+       cap_set[PCI_OSC_CTL] = sc->ap_osc_ctl | osc_ctl;
 
        status = acpi_EvaluateOSC(sc->ap_handle, pci_host_bridge_uuid, 1,
            nitems(cap_set), cap_set, cap_set, false);
        if (ACPI_FAILURE(status)) {
-               if (status == AE_NOT_FOUND)
+               if (status == AE_NOT_FOUND) {
+                       sc->ap_osc_ctl |= osc_ctl;
                        return (0);
+               }
                device_printf(sc->ap_dev, "_OSC failed: %s\n",
                    AcpiFormatException(status));
                return (EIO);
        }
 
-       if (cap_set[PCI_OSC_STATUS] == 0)
-               sc->ap_osc_ctl = cap_set[PCI_OSC_CTL];
-
-       if (cap_set[PCI_OSC_STATUS] != 0 ||
-           (cap_set[PCI_OSC_CTL] & osc_ctl) != osc_ctl)
+       /*
+        * _OSC may return an error in the status word, but will
+        * update the control mask always.  _OSC should not revoke
+        * previously-granted controls.
+        */
+       if ((cap_set[PCI_OSC_CTL] & sc->ap_osc_ctl) != sc->ap_osc_ctl)
+               device_printf(sc->ap_dev, "_OSC revoked %#x\n",
+                   (cap_set[PCI_OSC_CTL] & sc->ap_osc_ctl) ^ sc->ap_osc_ctl);
+       sc->ap_osc_ctl = cap_set[PCI_OSC_CTL];
+       if ((sc->ap_osc_ctl & osc_ctl) != osc_ctl)
                return (EIO);
 
        return (0);
@@ -727,7 +737,7 @@ acpi_pcib_request_feature(device_t pcib,
        uint32_t osc_ctl;
        struct acpi_hpcib_softc *sc;
 
-       sc = device_get_softc(dev);
+       sc = device_get_softc(pcib);
 
        switch (feature) {
        case PCI_FEATURE_HP:

Modified: head/sys/dev/acpica/acpivar.h
==============================================================================
--- head/sys/dev/acpica/acpivar.h       Thu Apr 27 16:14:32 2017        
(r317509)
+++ head/sys/dev/acpica/acpivar.h       Thu Apr 27 16:32:42 2017        
(r317510)
@@ -301,6 +301,12 @@ void               acpi_EnterDebugger(void);
        device_printf(dev, x);                                  \
 } while (0)
 
+/* Values for the first status word returned by _OSC. */
+#define        ACPI_OSC_FAILURE        (1 << 1)
+#define        ACPI_OSC_BAD_UUID       (1 << 2)
+#define        ACPI_OSC_BAD_REVISION   (1 << 3)
+#define        ACPI_OSC_CAPS_MASKED    (1 << 4)
+
 /* Values for the device _STA (status) method. */
 #define ACPI_STA_PRESENT       (1 << 0)
 #define ACPI_STA_ENABLED       (1 << 1)

Modified: head/sys/dev/pci/pci_pci.c
==============================================================================
--- head/sys/dev/pci/pci_pci.c  Thu Apr 27 16:14:32 2017        (r317509)
+++ head/sys/dev/pci/pci_pci.c  Thu Apr 27 16:32:42 2017        (r317510)
@@ -76,7 +76,7 @@ static void           pcib_pcie_ab_timeout(void *
 static void            pcib_pcie_cc_timeout(void *arg);
 static void            pcib_pcie_dll_timeout(void *arg);
 #endif
-static int             pcib_request_feature(device_t pcib, device_t dev,
+static int             pcib_request_feature_default(device_t pcib, device_t 
dev,
                            enum pci_feature feature);
 
 static device_method_t pcib_methods[] = {
@@ -121,7 +121,7 @@ static device_method_t pcib_methods[] = 
     DEVMETHOD(pcib_try_enable_ari,     pcib_try_enable_ari),
     DEVMETHOD(pcib_ari_enabled,                pcib_ari_enabled),
     DEVMETHOD(pcib_decode_rid,         pcib_ari_decode_rid),
-    DEVMETHOD(pcib_request_feature,    pcib_request_feature),
+    DEVMETHOD(pcib_request_feature,    pcib_request_feature_default),
 
     DEVMETHOD_END
 };
@@ -964,8 +964,7 @@ pcib_probe_hotplug(struct pcib_softc *sc
         * Now that we're sure we want to do hot plug, ask the
         * firmware, if any, if that's OK.
         */
-       if (pcib_request_feature(device_get_parent(device_get_parent(dev)), dev,
-               PCI_FEATURE_HP) != 0) {
+       if (pcib_request_feature(dev, PCI_FEATURE_HP) != 0) {
                if (bootverbose)
                        device_printf(dev, "Unable to activate hot plug 
feature.\n");
                return;
@@ -2863,6 +2862,17 @@ pcib_request_feature_allow(device_t pcib
        return (0);
 }
 
+int
+pcib_request_feature(device_t dev, enum pci_feature feature)
+{
+
+       /*
+        * Invoke PCIB_REQUEST_FEATURE of this bridge first in case
+        * the firmware overrides the method of PCI-PCI bridges.
+        */
+       return (PCIB_REQUEST_FEATURE(dev, dev, feature));
+}
+
 /*
  * Pass the request to use this PCI feature up the tree. Either there's a
  * firmware like ACPI that's using this feature that will approve (or deny) the
@@ -2871,7 +2881,8 @@ pcib_request_feature_allow(device_t pcib
  * to make use of the feature or render it harmless.
  */
 static int
-pcib_request_feature(device_t pcib, device_t dev, enum pci_feature feature)
+pcib_request_feature_default(device_t pcib, device_t dev,
+    enum pci_feature feature)
 {
        device_t bus;
 

Modified: head/sys/dev/pci/pcib_private.h
==============================================================================
--- head/sys/dev/pci/pcib_private.h     Thu Apr 27 16:14:32 2017        
(r317509)
+++ head/sys/dev/pci/pcib_private.h     Thu Apr 27 16:32:42 2017        
(r317510)
@@ -193,6 +193,7 @@ int         pcib_get_id(device_t pcib, device_t
                    uintptr_t *id);
 void           pcib_decode_rid(device_t pcib, uint16_t rid, int *bus, 
                    int *slot, int *func);
+int            pcib_request_feature(device_t dev, enum pci_feature feature);
 int            pcib_request_feature_allow(device_t pcib, device_t dev, enum 
pci_feature feature);
 
 #endif
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to