diff mbox series

[v2,06/17] PCI: Fix us->ms conversion in pci_acpi_optimize_delay

Message ID 20200302184429.12880-7-stanspas@amazon.com (mailing list archive)
State Superseded, archived
Headers show
Series Improve PCI device post-reset readiness polling | expand

Commit Message

Stanislav Spassov March 2, 2020, 6:44 p.m. UTC 
From: Stanislav Spassov <stanspas@amazon.de>

_DSM Function 9 returns device readiness durations in microseconds.

Without this fix, integer truncation could cause msleep()-ing for up to
999us less than actually requested by the firmware.

Specifically, if the firmware specifies a 500us delay, msleep(0) would
be invoked by the users of the delay values optimized here.

Signed-off-by: Stanislav Spassov <stanspas@amazon.de>
---
 drivers/pci/pci-acpi.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

kernel test robot March 3, 2020, 4:19 a.m. UTC | #1 
Hi Stanislav,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9]

url:    https://github.com/0day-ci/linux/commits/Stanislav-Spassov/Improve-PCI-device-post-reset-readiness-polling/20200303-043307
base:    bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9
config: i386-randconfig-h002-20200302 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-5) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/pci/pci-acpi.o: in function `pci_acpi_setup':
>> pci-acpi.c:(.text+0xb9a): undefined reference to `__udivdi3'
>> ld: pci-acpi.c:(.text+0xbb5): undefined reference to `__umoddi3'
   ld: pci-acpi.c:(.text+0xc39): undefined reference to `__udivdi3'
   ld: pci-acpi.c:(.text+0xc54): undefined reference to `__umoddi3'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot March 3, 2020, 5:54 a.m. UTC | #2 
Hi Stanislav,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9]

url:    https://github.com/0day-ci/linux/commits/Stanislav-Spassov/Improve-PCI-device-post-reset-readiness-polling/20200303-043307
base:    bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9
config: i386-randconfig-d001-20200302 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-5) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/pci/pci-acpi.o: in function `pci_acpi_optimize_delay':
>> drivers/pci/pci-acpi.c:1242: undefined reference to `__udivdi3'
>> ld: drivers/pci/pci-acpi.c:1243: undefined reference to `__umoddi3'
   ld: drivers/pci/pci-acpi.c:1250: undefined reference to `__udivdi3'
   ld: drivers/pci/pci-acpi.c:1251: undefined reference to `__umoddi3'

vim +1242 drivers/pci/pci-acpi.c

  1178	
  1179	/**
  1180	 * pci_acpi_optimize_delay - optimize PCI D3 and D3cold delay from ACPI
  1181	 * @pdev: the PCI device whose delay is to be updated
  1182	 * @handle: ACPI handle of this device
  1183	 *
  1184	 * Update the d3_delay and d3cold_delay of a PCI device from the ACPI _DSM
  1185	 * Function 9 of the device, and cache the parent host bridge's flag for
  1186	 * ignoring reset delay upon Sx Resume (the flag is originally set in
  1187	 * acpi_pci_add_bus through _DSM Function 8).
  1188	 *
  1189	 * Function 9, "Device Readiness Durations," applies only to the object
  1190	 * where it is located.  It returns delay durations required after various
  1191	 * events if the device requires less time than the spec requires.
  1192	 * Values provided by this function can only be used to lower (reduce) the
  1193	 * latency required by specification or values discovered from device.
  1194	 *
  1195	 * This _DSM function is defined by the PCI Firmware Specification Rev 3.2
  1196	 * (January 26, 2015), after originally introduced by a draft ECN of
  1197	 * January 28, 2014, titled "ACPI additions for FW latency optimizations."
  1198	 *
  1199	 * XXX The PCI Firmware Specification contradicts itself by stating, in addition
  1200	 * to the above "can only be used to lower (reduce)", that also:
  1201	 * Values must be interpreted as overriding any Configuration Ready indicator
  1202	 * from hardware, whether increasing or decreasing required delays. This
  1203	 * includes ignoring FRS and DRS notifications where overridden by this
  1204	 * _DSM function, as well as ignoring values specified in the Readiness Time
  1205	 * Reporting Extended Capability, if present.
  1206	 * Meanwhile, the PCI Express Base Specification Revision 5.0 Version 1.0
  1207	 * (22 May 2019) states in section 7.9.17 Readiness Time Reporting Extended
  1208	 * Capability:
  1209	 * Software is permitted to issue requests upon the earliest of:
  1210	 * - Receiving a Readiness Notification messages
  1211	 * - Waiting the appropriate time as per relevant specifications
  1212	 * - Waiting the time indicated in the associated field of this capability
  1213	 * - Waiting the time defined by system software or firmware
  1214	 * The kernel does not yet support Readiness Notifications, and does not yet
  1215	 * use a Readiness Time Reporting capability if present, so we do not need to
  1216	 * worry about the prioritization for now.
  1217	 */
  1218	static void pci_acpi_optimize_delay(struct pci_dev *pdev,
  1219					    acpi_handle handle)
  1220	{
  1221		struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
  1222		/*
  1223		 * _DSM 9 provides values in microseconds, but the kernel uses msleep()
  1224		 * when waiting, so the code below rounds up when setting value in ms
  1225		 */
  1226		u64 value_us;
  1227		int value;
  1228		union acpi_object *obj, *elements;
  1229	
  1230		pdev->ignore_reset_delay_on_sx_resume =
  1231			bridge->ignore_reset_delay_on_sx_resume;
  1232	
  1233		obj = acpi_evaluate_dsm(handle, &pci_acpi_dsm_guid, 3,
  1234					FUNCTION_DELAY_DSM, NULL);
  1235		if (!obj)
  1236			return;
  1237	
  1238		if (obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 5) {
  1239			elements = obj->package.elements;
  1240			if (elements[0].type == ACPI_TYPE_INTEGER) {
  1241				value_us = elements[0].integer.value;
> 1242				value = (int)(value_us / 1000);
> 1243				if (value_us % 1000 > 0)
  1244					value++;
  1245				if (value < PCI_PM_D3COLD_WAIT)
  1246					pdev->d3cold_delay = value;
  1247			}
  1248			if (elements[3].type == ACPI_TYPE_INTEGER) {
  1249				value_us = elements[3].integer.value;
  1250				value = (int)(value_us / 1000);
  1251				if (value_us % 1000 > 0)
  1252					value++;
  1253				if (value < PCI_PM_D3_WAIT)
  1254					pdev->d3_delay = value;
  1255			}
  1256		}
  1257		ACPI_FREE(obj);
  1258	}
  1259	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index a8fa13d6089d..508924377bff 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1219,6 +1219,11 @@  static void pci_acpi_optimize_delay(struct pci_dev *pdev,
 				    acpi_handle handle)
 {
 	struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
+	/*
+	 * _DSM 9 provides values in microseconds, but the kernel uses msleep()
+	 * when waiting, so the code below rounds up when setting value in ms
+	 */
+	u64 value_us;
 	int value;
 	union acpi_object *obj, *elements;
 
@@ -1233,12 +1238,18 @@  static void pci_acpi_optimize_delay(struct pci_dev *pdev,
 	if (obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 5) {
 		elements = obj->package.elements;
 		if (elements[0].type == ACPI_TYPE_INTEGER) {
-			value = (int)elements[0].integer.value / 1000;
+			value_us = elements[0].integer.value;
+			value = (int)(value_us / 1000);
+			if (value_us % 1000 > 0)
+				value++;
 			if (value < PCI_PM_D3COLD_WAIT)
 				pdev->d3cold_delay = value;
 		}
 		if (elements[3].type == ACPI_TYPE_INTEGER) {
-			value = (int)elements[3].integer.value / 1000;
+			value_us = elements[3].integer.value;
+			value = (int)(value_us / 1000);
+			if (value_us % 1000 > 0)
+				value++;
 			if (value < PCI_PM_D3_WAIT)
 				pdev->d3_delay = value;
 		}