(2010/07/30 0:45), Rafael J. Wysocki wrote: > On Thursday, July 29, 2010, Kenji Kaneshige wrote: >> (2010/07/29 6:43), Jesse Barnes wrote: >>> On Wed, 28 Jul 2010 23:23:56 +0200 >>> "Rafael J. Wysocki"<rjw@xxxxxxx> wrote: >>> >>>> From: Rafael J. Wysocki<rjw@xxxxxxx> >>>> >>>> PCIe port service drivers ask the BIOS, through _OSC, for control of >>>> the services they handle. Unfortunately, each of them individually >>>> asks for control of the PCIe capability structure and if that is >>>> granted, some BIOSes expect that the other PCIe port services will be >>>> configured and handled by the kernel as well. If that is not the >>>> case (eg. one of the PCIe port service drivers is not loaded), the >>>> BIOS may be confused and may cause the system as a whole to misbehave >>>> (eg. on one of such systems enabling the native PCIe PME service >>>> without loading the native PCIe hot-plug service driver causes a >>>> storm of ACPI notify requests to appear). >>>> >>>> For this reason rework the PCIe port driver so that (1) it checks >>>> which native PCIe port services can be enabled, according to the >>>> BIOS, and (2) it requests control of all these services >>>> simultaneously. In particular, this causes pcie_portdrv_probe() to >>>> fail if the BIOS refuses to grant control of the PCIe capability >>>> structure, which means that no native PCIe port services can be >>>> enabled for the PCIe root complex the given port belongs to. >>>> >>>> Make it possible to override this behavior using a new command line >>>> switch pcie_ports= that can be set to 'auto' (ask the BIOS, the >>>> default), 'native' (use the PCIe native services regardless of the >>>> BIOS response to the control request), or 'compat' (do not use the >>>> PCIe native services at all). >>>> >>>> Accordingly, rework the existing PCIe port service drivers so that >>>> they don't request control of the services directly. >>>> >>>> Signed-off-by: Rafael J. Wysocki<rjw@xxxxxxx> >>>> --- >>> >>> Kenji-san, are you ok with this version? I would like to get your ack >>> (and ideally your tested-by) for this one since it affects >>> functionality you need. >>> >> >> Hi Jesse, Rafael, >> >> I've just started reviewing the latest version of the patch. I have good >> impression about this version so far. Please give me a few days for deeper >> review. And I've booked the test machine for this patch in next week. I'll >> try to test the patch and send you the result as soon as I can. >> >> Here are two comments I have so far. >> >> Though it is not directly related to Rafael's patch, I found one problem >> in _OSC query handling while reviewing this patch. Currently, all the >> _OSC controls are queried at the same time in acpi_pci_osc_support() and >> the result is preserved for later acpi_pci_osc_control_set() call. But >> query result can vary depending on the combination of requested controls. > > In principle it can, but acpi_pci_query_osc() asks for all aplicable control > bits, in which case the formware can only clear those bits in the response if > the corresponding features are unsupported. Doing otherwise would be in > violation of Section 6.2.9.1. of ACPI spec 3.0b, the following in particular: > > "If any bits in the Control Field are returned cleared (masked to zero) by the _OSC control method, > the respective feature is designated unsupported by the platform and must not be enabled by the OS." > My concern is as follows. For easy, assume as follows: - all the _OSC controls are A, B, C -B depends on C. - When requesting A, B and C, all A, B and C are granted to OS. - When requesting A, B, only A is granted to OS. Current linux queries all A, B and C at the boot time and preserve the result (all A, B and C can be granted to OS). Here, if a service driver requests A and B using acpi_pci_osc_control_set(), acpi_pci_osc_control_set() checks if both of those controls can be granted to OS. The expected result here is only A can be granted to OS, but current linux judges both of them can be granted because linux does this check by seeing preserved result. As a result, acpi_pci_osc_control_set() evaluates _OSC with query flag cleared to request A and B. Since the control B returned cleared, acpi_pci_osc_control() returns error to the service driver. As a result, control A is granted to OS even though there is no driver to handle the control A. >> So I think query result must not be preserved. Since Rafael's patch uses >> query result, this problem should be fixed at the same time. I'll try to >> make a patch for this and update Rafael's patch if needed. > > It would be sufficient to change acpi_pci_osc_control_get(), introduced > by my patch, to call acpi_pci_query_osc() unconditionally, but as I said above, > I don't think it's necessary. I made a patch for this and updated your patch slightly. I'll send them soon. > >> I think the following changes should be done in the separated patch. I >> guess this change is for the BIOS which enables hotplug interrupt at when >> _OSC is evaluated with native hot-plug control. Right? > > Yes, it is. > >> Anyway, it should be separated patch with appropriate description. > > But the $subject patch does analogous things for AER and PCIe PME. Do you > think they also should be introduced by separate patches? I think the following changes in your patch should be introduced by separate patches. - optimizing the checks in acpi_pci_osc_control_set() - Disabling hot-plug interrupt - Removing module_exit() Thanks, Kenji Kaneshige _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
