Skip to contentSkip to conversation

pciback: restore PCI BARs on D3->D0 transition

PATCH In review Alex Williamson 3 participants 4 messages Mar 26, 2007 – Apr 3, 2007

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 pciback: restore PCI BARs on D3->D0 transition

Ever since xen-unstable cset 14308 ("pci back: Fix registration of of
filters on subsections of config space") I've been getting an MCA on the *2nd* boot of a driver domain using an e1000 NIC. Cset 14308 allowed the proper setup of the PM control registers, so the NIC is put in the D3 power state when the driver domain shuts down. Unfortunately, pci_set_power_state() won't restore the BARs on a D3hot/cold to D0 transition. There are some comments in this upstream cset about why that is:

http://www.kernel.org/hg/linux-2.6/rev/8f33e29c89a7

However, we obviously can't expect PCI devices to work without their BARs properly programmed. The patch below adds a call to pci_restore_bars() as a workaround until we can figure out why the PCI code doesn't do this for us. Thanks,

Alex

---

linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_capability_pm.c +12 -1
diff -r 56caf0e37e6a linux-2.6-xen-sparse/drivers/xen/pciback/conf_space_capability_pm.c
@@ -33,7 +33,7 @@ static int pm_ctrl_write(struct pci_dev 
 {
 	int err;
 	u16 cur_value;
-	pci_power_t new_state;
+	pci_power_t new_state, old_state;
 
 	/* Handle setting power state separately */
 	new_state = (pci_power_t)(new_value & PCI_PM_CTRL_STATE_MASK);
@@ -52,9 +52,20 @@ static int pm_ctrl_write(struct pci_dev 
 
 	/* Let pci core handle the power management change */
 	dev_dbg(&dev->dev, "set power state to %x\n", new_state);
+	old_state = dev->current_state;
 	err = pci_set_power_state(dev, new_state);
 	if (err)
 		err = PCIBIOS_SET_FAILED;
+
+	/*
+	 * Linux does not currently restore the BARs on a transition from
+	 * D3hot/cold to D0.  Fixup here.
+	 */
+	if (new_state == PCI_D0 &&
+	    (old_state == PCI_D3hot || old_state == PCI_D3cold)) {
+		if (!(cur_value & PCI_PM_CTRL_NO_SOFT_RESET))
+			pci_restore_bars(dev);
+	}
 
       out:
 	return err;
#1 lore raw
Quoted 4 lines · Alex Williamson

However, we obviously can't expect PCI devices to work without their BARs properly programmed. The patch below adds a call to pci_restore_bars() as a workaround until we can figure out why the PCI code doesn't do this for us. Thanks,

From what I can gather, it seems when a driver induces a transition
to D3, its expected to use pci_save_state() and pci_restore_state(). I don't think we necessarily need to go to that extent for pciback, but it does appear to be pciback's responsibility to restore some state on the card when transitioning out of D3, since it is effectively the driver for the device. So, I think the pci_restore_bars() call and the patch I sent previously are correct. Thanks,

Alex

Signature · 1 line
Alex Williamson                             HP Open Source & Linux Org.
#2 lore raw
Quoted 4 lines · Alex Williamson

don't think we necessarily need to go to that extent for pciback, but it does appear to be pciback's responsibility to restore some state on the card when transitioning out of D3, since it is effectively the driver for the device.

Well, really it's supposed to be a filter (and perhaps translator) for PCI config space accesses. It's not really a driver. But obviously something like this is needed. The driver maintainer (I think!) is Chris Bookholt (cc'ed).

-- Keir

#3 lore raw
Quoted 10 lines · Keir Fraser
Quoted 4 lines · Alex Williamson

don't think we necessarily need to go to that extent for pciback, but it does appear to be pciback's responsibility to restore some state on the card when transitioning out of D3, since it is effectively the driver for the device.

Well, really it's supposed to be a filter (and perhaps translator) for PCI config space accesses. It's not really a driver. But obviously something like this is needed.

Sorry for the delay, I've been out of the office for a while.

Indeed pciback was intended to be more of a filter than a driver. For the time being it seems reasonable to implement BAR-related PM functions in pciback as you've done (many thanks for that). As time permits I'll double-check that your solution makes the fix in the most appropriate location.

Cheers,
Chris