Date: Thu, 27 Apr 2017 16:32:42 +0000 (UTC) From: John Baldwin <jhb@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r317510 - in head/sys/dev: acpica pci Message-ID: <201704271632.v3RGWgZH054991@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201704271632.v3RGWgZH054991>