On Wednesday, March 27, 2013 09:28:58 PM Yinghai Lu wrote: > Found problem on system that firmware that could handle pci aer. > Firmware get error reporting after pci injecting error, before os boots. > But after os boots, firmware can not get report anymore, even pci=noaer > is passed. > > Root cause: BIOS _OSC has problem with query bit checking. > It turns out that BIOS vendor is copying example code from ACPI Spec. > In ACPI Spec 5.0, page 290: > > If (Not(And(CDW1,1))) // Query flag clear? > { // Disable GPEs for features granted native control. > If (And(CTRL,0x01)) // Hot plug control granted? > { > Store(0,HPCE) // clear the hot plug SCI enable bit > Store(1,HPCS) // clear the hot plug SCI status bit > } > ... > } > > When Query flag is set, And(CDW1,1) will be 1, Not(1) will return 0xfffffffe. > So it will get into code path that should be for control set only. > BIOS acpi code should be changed to "If (LEqual(And(CDW1,1), 0)))" > > Current kernel code is using _OSC query to notify firmware about support > from OS and then use _OSC to set control bits. > During query support, current code is using all possible controls. > So will execute code that should be only for control set stage. > > That will have problem when pci=noaer or aer firmware_first is used. > As firmware have that control set for os aer already in query support stage, > but later will not os aer handling. > > We should avoid passing all possible controls, just use osc_control_set > instead. > That should workaround BIOS bugs with affected systems on the field > as more bios vendors are copying sample code from ACPI spec. > > Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx> > Cc: stable@xxxxxxxxxx > > --- > drivers/acpi/pci_root.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > Index: linux-2.6/drivers/acpi/pci_root.c > =================================================================== > --- linux-2.6.orig/drivers/acpi/pci_root.c > +++ linux-2.6/drivers/acpi/pci_root.c > @@ -201,8 +201,8 @@ static acpi_status acpi_pci_query_osc(st > *control &= OSC_PCI_CONTROL_MASKS; > capbuf[OSC_CONTROL_TYPE] = *control | root->osc_control_set; > } else { > - /* Run _OSC query for all possible controls. */ > - capbuf[OSC_CONTROL_TYPE] = OSC_PCI_CONTROL_MASKS; > + /* Run _OSC query only with existing controls. */ > + capbuf[OSC_CONTROL_TYPE] = root->osc_control_set; I suppose we can do that, but then why this should be root->osc_control_set and not just 0? > } > > status = acpi_pci_run_osc(root->device->handle, capbuf, &result); > -- Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html