Skip to content

Commit 0f32114

Browse files
bjorn-helgaasbjorn-helgaas
committed
Merge branch 'pci/aspm'
- Disable ASPM on MFD function removal to avoid use-after-free (Ding Hui) - Tighten up pci_enable_link_state() and pci_disable_link_state() interfaces so they don't enable/disable states the driver didn't specify (Ajay Agarwal) - Avoid link retraining race that can happen if ASPM sets link control parameters while the link is in the midst of training for some other reason (Ilpo Järvinen) * pci/aspm: PCI/ASPM: Avoid link retraining race PCI/ASPM: Factor out pcie_wait_for_retrain() PCI/ASPM: Return 0 or -ETIMEDOUT from pcie_retrain_link() PCI/ASPM: Remove unnecessary ASPM_STATE_L1SS check PCI/ASPM: Rename L1.2-specific functions from 'l1ss' to 'l12' PCI/ASPM: Set ASPM_STATE_L1 when driver enables L1.1 or L1.2 PCI/ASPM: Set only ASPM_STATE_L1 when driver enables L1 PCI/ASPM: Disable only ASPM_STATE_L1 when driver disables L1 PCI/ASPM: Disable ASPM on MFD function removal to avoid use-after-free
2 parents a274a4e + e7e3975 commit 0f32114

File tree

1 file changed

+64
-46
lines changed

1 file changed

+64
-46
lines changed

‎drivers/pci/pcie/aspm.c

Lines changed: 64 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -193,12 +193,39 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
193193
link->clkpm_disable = blacklist ? 1 : 0;
194194
}
195195

196-
static bool pcie_retrain_link(struct pcie_link_state *link)
196+
static int pcie_wait_for_retrain(struct pci_dev *pdev)
197197
{
198-
struct pci_dev *parent = link->pdev;
199198
unsigned long end_jiffies;
200199
u16 reg16;
201200

201+
/* Wait for Link Training to be cleared by hardware */
202+
end_jiffies = jiffies + LINK_RETRAIN_TIMEOUT;
203+
do {
204+
pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &reg16);
205+
if (!(reg16 & PCI_EXP_LNKSTA_LT))
206+
return 0;
207+
msleep(1);
208+
} while (time_before(jiffies, end_jiffies));
209+
210+
return -ETIMEDOUT;
211+
}
212+
213+
static int pcie_retrain_link(struct pcie_link_state *link)
214+
{
215+
struct pci_dev *parent = link->pdev;
216+
int rc;
217+
u16 reg16;
218+
219+
/*
220+
* Ensure the updated LNKCTL parameters are used during link
221+
* training by checking that there is no ongoing link training to
222+
* avoid LTSSM race as recommended in Implementation Note at the
223+
* end of PCIe r6.0.1 sec 7.5.3.7.
224+
*/
225+
rc = pcie_wait_for_retrain(parent);
226+
if (rc)
227+
return rc;
228+
202229
pcie_capability_read_word(parent, PCI_EXP_LNKCTL, &reg16);
203230
reg16 |= PCI_EXP_LNKCTL_RL;
204231
pcie_capability_write_word(parent, PCI_EXP_LNKCTL, reg16);
@@ -212,15 +239,7 @@ static bool pcie_retrain_link(struct pcie_link_state *link)
212239
pcie_capability_write_word(parent, PCI_EXP_LNKCTL, reg16);
213240
}
214241

215-
/* Wait for link training end. Break out after waiting for timeout */
216-
end_jiffies = jiffies + LINK_RETRAIN_TIMEOUT;
217-
do {
218-
pcie_capability_read_word(parent, PCI_EXP_LNKSTA, &reg16);
219-
if (!(reg16 & PCI_EXP_LNKSTA_LT))
220-
break;
221-
msleep(1);
222-
} while (time_before(jiffies, end_jiffies));
223-
return !(reg16 & PCI_EXP_LNKSTA_LT);
242+
return pcie_wait_for_retrain(parent);
224243
}
225244

226245
/*
@@ -289,15 +308,15 @@ static void pcie_aspm_configure_common_clock(struct pcie_link_state *link)
289308
reg16 &= ~PCI_EXP_LNKCTL_CCC;
290309
pcie_capability_write_word(parent, PCI_EXP_LNKCTL, reg16);
291310

292-
if (pcie_retrain_link(link))
293-
return;
311+
if (pcie_retrain_link(link)) {
294312

295-
/* Training failed. Restore common clock configurations */
296-
pci_err(parent, "ASPM: Could not configure common clock\n");
297-
list_for_each_entry(child, &linkbus->devices, bus_list)
298-
pcie_capability_write_word(child, PCI_EXP_LNKCTL,
313+
/* Training failed. Restore common clock configurations */
314+
pci_err(parent, "ASPM: Could not configure common clock\n");
315+
list_for_each_entry(child, &linkbus->devices, bus_list)
316+
pcie_capability_write_word(child, PCI_EXP_LNKCTL,
299317
child_reg[PCI_FUNC(child->devfn)]);
300-
pcie_capability_write_word(parent, PCI_EXP_LNKCTL, parent_reg);
318+
pcie_capability_write_word(parent, PCI_EXP_LNKCTL, parent_reg);
319+
}
301320
}
302321

303322
/* Convert L0s latency encoding to ns */
@@ -337,7 +356,7 @@ static u32 calc_l1_acceptable(u32 encoding)
337356
}
338357

339358
/* Convert L1SS T_pwr encoding to usec */
340-
static u32 calc_l1ss_pwron(struct pci_dev *pdev, u32 scale, u32 val)
359+
static u32 calc_l12_pwron(struct pci_dev *pdev, u32 scale, u32 val)
341360
{
342361
switch (scale) {
343362
case 0:
@@ -471,7 +490,7 @@ static void pci_clear_and_set_dword(struct pci_dev *pdev, int pos,
471490
}
472491

473492
/* Calculate L1.2 PM substate timing parameters */
474-
static void aspm_calc_l1ss_info(struct pcie_link_state *link,
493+
static void aspm_calc_l12_info(struct pcie_link_state *link,
475494
u32 parent_l1ss_cap, u32 child_l1ss_cap)
476495
{
477496
struct pci_dev *child = link->downstream, *parent = link->pdev;
@@ -481,9 +500,6 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link,
481500
u32 pctl1, pctl2, cctl1, cctl2;
482501
u32 pl1_2_enables, cl1_2_enables;
483502

484-
if (!(link->aspm_support & ASPM_STATE_L1_2_MASK))
485-
return;
486-
487503
/* Choose the greater of the two Port Common_Mode_Restore_Times */
488504
val1 = (parent_l1ss_cap & PCI_L1SS_CAP_CM_RESTORE_TIME) >> 8;
489505
val2 = (child_l1ss_cap & PCI_L1SS_CAP_CM_RESTORE_TIME) >> 8;
@@ -495,13 +511,13 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link,
495511
val2 = (child_l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_VALUE) >> 19;
496512
scale2 = (child_l1ss_cap & PCI_L1SS_CAP_P_PWR_ON_SCALE) >> 16;
497513

498-
if (calc_l1ss_pwron(parent, scale1, val1) >
499-
calc_l1ss_pwron(child, scale2, val2)) {
514+
if (calc_l12_pwron(parent, scale1, val1) >
515+
calc_l12_pwron(child, scale2, val2)) {
500516
ctl2 |= scale1 | (val1 << 3);
501-
t_power_on = calc_l1ss_pwron(parent, scale1, val1);
517+
t_power_on = calc_l12_pwron(parent, scale1, val1);
502518
} else {
503519
ctl2 |= scale2 | (val2 << 3);
504-
t_power_on = calc_l1ss_pwron(child, scale2, val2);
520+
t_power_on = calc_l12_pwron(child, scale2, val2);
505521
}
506522

507523
/*
@@ -616,8 +632,8 @@ static void aspm_l1ss_init(struct pcie_link_state *link)
616632
if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2)
617633
link->aspm_enabled |= ASPM_STATE_L1_2_PCIPM;
618634

619-
if (link->aspm_support & ASPM_STATE_L1SS)
620-
aspm_calc_l1ss_info(link, parent_l1ss_cap, child_l1ss_cap);
635+
if (link->aspm_support & ASPM_STATE_L1_2_MASK)
636+
aspm_calc_l12_info(link, parent_l1ss_cap, child_l1ss_cap);
621637
}
622638

623639
static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
@@ -1010,29 +1026,32 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
10101026

10111027
down_read(&pci_bus_sem);
10121028
mutex_lock(&aspm_lock);
1013-
/*
1014-
* All PCIe functions are in one slot, remove one function will remove
1015-
* the whole slot, so just wait until we are the last function left.
1016-
*/
1017-
if (!list_empty(&parent->subordinate->devices))
1018-
goto out;
10191029

10201030
link = parent->link_state;
10211031
root = link->root;
10221032
parent_link = link->parent;
10231033

1024-
/* All functions are removed, so just disable ASPM for the link */
1034+
/*
1035+
* link->downstream is a pointer to the pci_dev of function 0. If
1036+
* we remove that function, the pci_dev is about to be deallocated,
1037+
* so we can't use link->downstream again. Free the link state to
1038+
* avoid this.
1039+
*
1040+
* If we're removing a non-0 function, it's possible we could
1041+
* retain the link state, but PCIe r6.0, sec 7.5.3.7, recommends
1042+
* programming the same ASPM Control value for all functions of
1043+
* multi-function devices, so disable ASPM for all of them.
1044+
*/
10251045
pcie_config_aspm_link(link, 0);
10261046
list_del(&link->sibling);
1027-
/* Clock PM is for endpoint device */
10281047
free_link_state(link);
10291048

10301049
/* Recheck latencies and configure upstream links */
10311050
if (parent_link) {
10321051
pcie_update_aspm_capable(root);
10331052
pcie_config_aspm_path(parent_link);
10341053
}
1035-
out:
1054+
10361055
mutex_unlock(&aspm_lock);
10371056
up_read(&pci_bus_sem);
10381057
}
@@ -1095,8 +1114,7 @@ static int __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem)
10951114
if (state & PCIE_LINK_STATE_L0S)
10961115
link->aspm_disable |= ASPM_STATE_L0S;
10971116
if (state & PCIE_LINK_STATE_L1)
1098-
/* L1 PM substates require L1 */
1099-
link->aspm_disable |= ASPM_STATE_L1 | ASPM_STATE_L1SS;
1117+
link->aspm_disable |= ASPM_STATE_L1;
11001118
if (state & PCIE_LINK_STATE_L1_1)
11011119
link->aspm_disable |= ASPM_STATE_L1_1;
11021120
if (state & PCIE_LINK_STATE_L1_2)
@@ -1171,16 +1189,16 @@ int pci_enable_link_state(struct pci_dev *pdev, int state)
11711189
if (state & PCIE_LINK_STATE_L0S)
11721190
link->aspm_default |= ASPM_STATE_L0S;
11731191
if (state & PCIE_LINK_STATE_L1)
1174-
/* L1 PM substates require L1 */
1175-
link->aspm_default |= ASPM_STATE_L1 | ASPM_STATE_L1SS;
1192+
link->aspm_default |= ASPM_STATE_L1;
1193+
/* L1 PM substates require L1 */
11761194
if (state & PCIE_LINK_STATE_L1_1)
1177-
link->aspm_default |= ASPM_STATE_L1_1;
1195+
link->aspm_default |= ASPM_STATE_L1_1 | ASPM_STATE_L1;
11781196
if (state & PCIE_LINK_STATE_L1_2)
1179-
link->aspm_default |= ASPM_STATE_L1_2;
1197+
link->aspm_default |= ASPM_STATE_L1_2 | ASPM_STATE_L1;
11801198
if (state & PCIE_LINK_STATE_L1_1_PCIPM)
1181-
link->aspm_default |= ASPM_STATE_L1_1_PCIPM;
1199+
link->aspm_default |= ASPM_STATE_L1_1_PCIPM | ASPM_STATE_L1;
11821200
if (state & PCIE_LINK_STATE_L1_2_PCIPM)
1183-
link->aspm_default |= ASPM_STATE_L1_2_PCIPM;
1201+
link->aspm_default |= ASPM_STATE_L1_2_PCIPM | ASPM_STATE_L1;
11841202
pcie_config_aspm_link(link, policy_to_aspm_state(link));
11851203

11861204
link->clkpm_default = (state & PCIE_LINK_STATE_CLKPM) ? 1 : 0;

0 commit comments

Comments
 (0)