From c4be8c68e6900b1811bc64f74cb13d5032a389ce Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Wed, 31 May 2017 15:58:21 +0800 Subject: clk: sunxi-ng: h3: Export PLL_PERIPH0 clock for the PRCM The PRCM takes PLL_PERIPH0 as one of its parents for the AR100 clock. As such we need to be able to describe this relationship in the device tree. Export the PLL_PERIPH0 clock so we can reference it in the PRCM node. Signed-off-by: Chen-Yu Tsai Signed-off-by: Maxime Ripard --- include/dt-bindings/clock/sun8i-h3-ccu.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include') diff --git a/include/dt-bindings/clock/sun8i-h3-ccu.h b/include/dt-bindings/clock/sun8i-h3-ccu.h index c2afc41d6964..e139fe5c62ec 100644 --- a/include/dt-bindings/clock/sun8i-h3-ccu.h +++ b/include/dt-bindings/clock/sun8i-h3-ccu.h @@ -43,6 +43,8 @@ #ifndef _DT_BINDINGS_CLK_SUN8I_H3_H_ #define _DT_BINDINGS_CLK_SUN8I_H3_H_ +#define CLK_PLL_PERIPH0 9 + #define CLK_CPUX 14 #define CLK_BUS_CE 20 -- cgit v1.3-14-g43fede From d85da227c3ae43d9ca513d60f244213cb4e55485 Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Wed, 31 May 2017 15:58:23 +0800 Subject: clk: sunxi-ng: a64: Export PLL_PERIPH0 clock for the PRCM The PRCM takes PLL_PERIPH0 as one of its parents for the AR100 clock. As such we need to be able to describe this relationship in the device tree. Export the PLL_PERIPH0 clock so we can reference it in the PRCM node. Signed-off-by: Chen-Yu Tsai Signed-off-by: Maxime Ripard --- drivers/clk/sunxi-ng/ccu-sun50i-a64.h | 4 +++- include/dt-bindings/clock/sun50i-a64-ccu.h | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.h b/drivers/clk/sunxi-ng/ccu-sun50i-a64.h index 9b3cd24b78d2..061b6fbb4f95 100644 --- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.h +++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.h @@ -31,7 +31,9 @@ #define CLK_PLL_VIDEO0_2X 8 #define CLK_PLL_VE 9 #define CLK_PLL_DDR0 10 -#define CLK_PLL_PERIPH0 11 + +/* PLL_PERIPH0 exported for PRCM */ + #define CLK_PLL_PERIPH0_2X 12 #define CLK_PLL_PERIPH1 13 #define CLK_PLL_PERIPH1_2X 14 diff --git a/include/dt-bindings/clock/sun50i-a64-ccu.h b/include/dt-bindings/clock/sun50i-a64-ccu.h index 370c0a0473fc..d66432c6e675 100644 --- a/include/dt-bindings/clock/sun50i-a64-ccu.h +++ b/include/dt-bindings/clock/sun50i-a64-ccu.h @@ -43,6 +43,8 @@ #ifndef _DT_BINDINGS_CLK_SUN50I_A64_H_ #define _DT_BINDINGS_CLK_SUN50I_A64_H_ +#define CLK_PLL_PERIPH0 11 + #define CLK_BUS_MIPI_DSI 28 #define CLK_BUS_CE 29 #define CLK_BUS_DMA 30 -- cgit v1.3-14-g43fede From e918d79a5d0a1b431e2cac0e6e6ac9452fd9ab32 Mon Sep 17 00:00:00 2001 From: Rodrigo Vivi Date: Tue, 6 Jun 2017 13:30:32 -0700 Subject: drm/i915/cnl: Add Cannonlake PCI IDs for U-skus. Platform enabling and its power-on are organized in different skus (U x Y x S x H, etc). So instead of organizing it in GT1 x GT2 x GT3 let's also use the platform sku. This is also the new Spec style what makes the review much more easy and straightforward. v2: Really include the PCI IDs to the picidlist[]; v3: Remove PCI IDs not present in spec. v4: Rebase. Signed-off-by: Anusha Srivatsa Signed-off-by: Rodrigo Vivi Reviewed-by: Clinton Taylor Link: http://patchwork.freedesktop.org/patch/msgid/1496781040-20888-3-git-send-email-rodrigo.vivi@intel.com --- drivers/gpu/drm/i915/i915_pci.c | 1 + include/drm/i915_pciids.h | 10 ++++++++++ 2 files changed, 11 insertions(+) (limited to 'include') diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index feb3425b8174..e9d80df85cf3 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -477,6 +477,7 @@ static const struct pci_device_id pciidlist[] = { INTEL_KBL_GT2_IDS(&intel_kabylake_info), INTEL_KBL_GT3_IDS(&intel_kabylake_gt3_info), INTEL_KBL_GT4_IDS(&intel_kabylake_gt3_info), + INTEL_CNL_IDS(&intel_cannonlake_info), {0, 0, 0} }; MODULE_DEVICE_TABLE(pci, pciidlist); diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 27e0dbaa6c0e..7f1bb3b0ce5b 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -334,4 +334,14 @@ INTEL_KBL_GT3_IDS(info), \ INTEL_KBL_GT4_IDS(info) +/* CNL U 2+2 */ +#define INTEL_CNL_U_GT2_IDS(info) \ + INTEL_VGA_DEVICE(0x5A52, info), \ + INTEL_VGA_DEVICE(0x5A5A, info), \ + INTEL_VGA_DEVICE(0x5A42, info), \ + INTEL_VGA_DEVICE(0x5A4A, info) + +#define INTEL_CNL_IDS(info) \ + INTEL_CNL_U_GT2_IDS(info) + #endif /* _I915_PCIIDS_H */ -- cgit v1.3-14-g43fede From 95578277cbdb60e3c68cb92c843cafc1f77c4f55 Mon Sep 17 00:00:00 2001 From: Rodrigo Vivi Date: Tue, 6 Jun 2017 13:30:33 -0700 Subject: drm/i915/cnl: Add Cannonlake PCI IDs for Y-skus. By the Spec all CNL Y skus are 2+2, i.e. GT2. v2: Really include the PCI IDs to the picidlist[]; Reviewed-by: Anusha Srivatsa Signed-off-by: Rodrigo Vivi Link: http://patchwork.freedesktop.org/patch/msgid/1496781040-20888-4-git-send-email-rodrigo.vivi@intel.com --- include/drm/i915_pciids.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 7f1bb3b0ce5b..7d2696a6588e 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -341,7 +341,17 @@ INTEL_VGA_DEVICE(0x5A42, info), \ INTEL_VGA_DEVICE(0x5A4A, info) +/* CNL Y 2+2 */ +#define INTEL_CNL_Y_GT2_IDS(info) \ + INTEL_VGA_DEVICE(0x5A51, info), \ + INTEL_VGA_DEVICE(0x5A59, info), \ + INTEL_VGA_DEVICE(0x5A41, info), \ + INTEL_VGA_DEVICE(0x5A49, info), \ + INTEL_VGA_DEVICE(0x5A71, info), \ + INTEL_VGA_DEVICE(0x5A79, info) + #define INTEL_CNL_IDS(info) \ - INTEL_CNL_U_GT2_IDS(info) + INTEL_CNL_U_GT2_IDS(info), \ + INTEL_CNL_Y_GT2_IDS(info) #endif /* _I915_PCIIDS_H */ -- cgit v1.3-14-g43fede From b056f8f3d6b900e8afd19f312719160346d263b4 Mon Sep 17 00:00:00 2001 From: Anusha Srivatsa Date: Thu, 8 Jun 2017 16:41:05 -0700 Subject: drm/i915/cfl: Add Coffee Lake PCI IDs for S Skus. Add PCI Ids for S Sku following the BSpec. v2: Remove the unused INTEL_CFL_IDS.(Rodrigo) v3: Add missing IDs(Rodrigo) Cc: Rodrigo Vivi Signed-off-by: Anusha Srivatsa Reviewed-by: Rodrigo Vivi Signed-off-by: Rodrigo Vivi Link: http://patchwork.freedesktop.org/patch/msgid/1496965267-21725-1-git-send-email-anusha.srivatsa@intel.com --- drivers/gpu/drm/i915/i915_pci.c | 1 + include/drm/i915_pciids.h | 8 ++++++++ 2 files changed, 9 insertions(+) (limited to 'include') diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index ba0443cc874a..4dfc4d37165a 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -489,6 +489,7 @@ static const struct pci_device_id pciidlist[] = { INTEL_KBL_GT2_IDS(&intel_kabylake_info), INTEL_KBL_GT3_IDS(&intel_kabylake_gt3_info), INTEL_KBL_GT4_IDS(&intel_kabylake_gt3_info), + INTEL_CFL_S_IDS(&intel_coffeelake_info), INTEL_CNL_IDS(&intel_cannonlake_info), {0, 0, 0} }; diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 7d2696a6588e..4858debd4b58 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -334,6 +334,14 @@ INTEL_KBL_GT3_IDS(info), \ INTEL_KBL_GT4_IDS(info) +/* CFL S */ +#define INTEL_CFL_S_IDS(info) \ + INTEL_VGA_DEVICE(0x3E90, info), /* SRV GT1 */ \ + INTEL_VGA_DEVICE(0x3E93, info), /* SRV GT1 */ \ + INTEL_VGA_DEVICE(0x3E91, info), /* SRV GT2 */ \ + INTEL_VGA_DEVICE(0x3E92, info), /* SRV GT2 */ \ + INTEL_VGA_DEVICE(0x3E96, info) /* SRV GT2 */ + /* CNL U 2+2 */ #define INTEL_CNL_U_GT2_IDS(info) \ INTEL_VGA_DEVICE(0x5A52, info), \ -- cgit v1.3-14-g43fede From ccfd13215fd25a0e8c28221f3acc0dcaec11cd15 Mon Sep 17 00:00:00 2001 From: Anusha Srivatsa Date: Thu, 8 Jun 2017 16:41:06 -0700 Subject: drm/i915/cfl: Add Coffee Lake PCI IDs for H Sku. Add PCI Ids for H Sku by following the BSpec. v2: Remove unused INTEL_CFL_IDS.(Rodrigo). v3: Add missing IDs(Rodrigo) Cc: Rodrigo Vivi Signed-off-by: Anusha Srivatsa Reviewed-by: Rodrigo Vivi Signed-off-by: Rodrigo Vivi Link: http://patchwork.freedesktop.org/patch/msgid/1496965267-21725-2-git-send-email-anusha.srivatsa@intel.com --- drivers/gpu/drm/i915/i915_pci.c | 1 + include/drm/i915_pciids.h | 5 +++++ 2 files changed, 6 insertions(+) (limited to 'include') diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 4dfc4d37165a..9202a68fc7c1 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -490,6 +490,7 @@ static const struct pci_device_id pciidlist[] = { INTEL_KBL_GT3_IDS(&intel_kabylake_gt3_info), INTEL_KBL_GT4_IDS(&intel_kabylake_gt3_info), INTEL_CFL_S_IDS(&intel_coffeelake_info), + INTEL_CFL_H_IDS(&intel_coffeelake_info), INTEL_CNL_IDS(&intel_cannonlake_info), {0, 0, 0} }; diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 4858debd4b58..8f25316013ba 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -342,6 +342,11 @@ INTEL_VGA_DEVICE(0x3E92, info), /* SRV GT2 */ \ INTEL_VGA_DEVICE(0x3E96, info) /* SRV GT2 */ +/* CFL H */ +#define INTEL_CFL_H_IDS(info) \ + INTEL_VGA_DEVICE(0x3E9B, info), /* Halo GT2 */ \ + INTEL_VGA_DEVICE(0x3E94, info) /* Halo GT2 */ + /* CNL U 2+2 */ #define INTEL_CNL_U_GT2_IDS(info) \ INTEL_VGA_DEVICE(0x5A52, info), \ -- cgit v1.3-14-g43fede From d29fe702c9cb682df99146d24d06e5455f043101 Mon Sep 17 00:00:00 2001 From: Anusha Srivatsa Date: Thu, 8 Jun 2017 16:41:07 -0700 Subject: drm/i915/cfl: Add Coffee Lake PCI IDs for U Sku. Add PCI Ids for U Skus of Coffeelake. v2: Use intel_coffeelake_gt3_info, in accordance to- Rodrigo's patch: v3: rebased v3: Remove unused INTEL_CFL_IDS(Rodrigo). Cc: Rodrigo Vivi Signed-off-by: Anusha Srivatsa Reviewed-by: Rodrigo Vivi Signed-off-by: Rodrigo Vivi Link: http://patchwork.freedesktop.org/patch/msgid/1496965267-21725-3-git-send-email-anusha.srivatsa@intel.com --- drivers/gpu/drm/i915/i915_pci.c | 1 + include/drm/i915_pciids.h | 7 +++++++ 2 files changed, 8 insertions(+) (limited to 'include') diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 9202a68fc7c1..4f5fd1ab9f2c 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -491,6 +491,7 @@ static const struct pci_device_id pciidlist[] = { INTEL_KBL_GT4_IDS(&intel_kabylake_gt3_info), INTEL_CFL_S_IDS(&intel_coffeelake_info), INTEL_CFL_H_IDS(&intel_coffeelake_info), + INTEL_CFL_U_IDS(&intel_coffeelake_gt3_info), INTEL_CNL_IDS(&intel_cannonlake_info), {0, 0, 0} }; diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h index 8f25316013ba..34c8f5600ce0 100644 --- a/include/drm/i915_pciids.h +++ b/include/drm/i915_pciids.h @@ -347,6 +347,13 @@ INTEL_VGA_DEVICE(0x3E9B, info), /* Halo GT2 */ \ INTEL_VGA_DEVICE(0x3E94, info) /* Halo GT2 */ +/* CFL U */ +#define INTEL_CFL_U_IDS(info) \ + INTEL_VGA_DEVICE(0x3EA6, info), /* ULT GT3 */ \ + INTEL_VGA_DEVICE(0x3EA7, info), /* ULT GT3 */ \ + INTEL_VGA_DEVICE(0x3EA8, info), /* ULT GT3 */ \ + INTEL_VGA_DEVICE(0x3EA5, info) /* ULT GT3 */ + /* CNL U 2+2 */ #define INTEL_CNL_U_GT2_IDS(info) \ INTEL_VGA_DEVICE(0x5A52, info), \ -- cgit v1.3-14-g43fede From 68dd02d19c811ca8ea60220a9d73e13b4bdad73a Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Wed, 14 Jun 2017 09:28:11 +0200 Subject: dev_ioctl: copy only the smaller struct iwreq for wext Unfortunately, struct iwreq isn't a proper subset of struct ifreq, but is still handled by the same code path. Robert reported that then applications may (randomly) fault if the struct iwreq they pass happens to land within 8 bytes of the end of a mapping (the struct is only 32 bytes, vs. struct ifreq's 40 bytes). To fix this, pull out the code handling wireless extension ioctls and copy only the smaller structure in this case. This bug goes back a long time, I tracked that it was introduced into mainline in 2.1.15, over 20 years ago! This fixes https://bugzilla.kernel.org/show_bug.cgi?id=195869 Reported-by: Robert O'Callahan Signed-off-by: Johannes Berg --- include/net/wext.h | 4 ++-- net/core/dev_ioctl.c | 19 ++++++++++++++++--- net/wireless/wext-core.c | 6 +++--- 3 files changed, 21 insertions(+), 8 deletions(-) (limited to 'include') diff --git a/include/net/wext.h b/include/net/wext.h index 345911965dbb..454ff763eeba 100644 --- a/include/net/wext.h +++ b/include/net/wext.h @@ -6,7 +6,7 @@ struct net; #ifdef CONFIG_WEXT_CORE -int wext_handle_ioctl(struct net *net, struct ifreq *ifr, unsigned int cmd, +int wext_handle_ioctl(struct net *net, struct iwreq *iwr, unsigned int cmd, void __user *arg); int compat_wext_handle_ioctl(struct net *net, unsigned int cmd, unsigned long arg); @@ -14,7 +14,7 @@ int compat_wext_handle_ioctl(struct net *net, unsigned int cmd, struct iw_statistics *get_wireless_stats(struct net_device *dev); int call_commit_handler(struct net_device *dev); #else -static inline int wext_handle_ioctl(struct net *net, struct ifreq *ifr, unsigned int cmd, +static inline int wext_handle_ioctl(struct net *net, struct iwreq *iwr, unsigned int cmd, void __user *arg) { return -EINVAL; diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c index b94b1d293506..27fad31784a8 100644 --- a/net/core/dev_ioctl.c +++ b/net/core/dev_ioctl.c @@ -410,6 +410,22 @@ int dev_ioctl(struct net *net, unsigned int cmd, void __user *arg) if (cmd == SIOCGIFNAME) return dev_ifname(net, (struct ifreq __user *)arg); + /* + * Take care of Wireless Extensions. Unfortunately struct iwreq + * isn't a proper subset of struct ifreq (it's 8 byte shorter) + * so we need to treat it specially, otherwise applications may + * fault if the struct they're passing happens to land at the + * end of a mapped page. + */ + if (cmd >= SIOCIWFIRST && cmd <= SIOCIWLAST) { + struct iwreq iwr; + + if (copy_from_user(&iwr, arg, sizeof(iwr))) + return -EFAULT; + + return wext_handle_ioctl(net, &iwr, cmd, arg); + } + if (copy_from_user(&ifr, arg, sizeof(struct ifreq))) return -EFAULT; @@ -559,9 +575,6 @@ int dev_ioctl(struct net *net, unsigned int cmd, void __user *arg) ret = -EFAULT; return ret; } - /* Take care of Wireless Extensions */ - if (cmd >= SIOCIWFIRST && cmd <= SIOCIWLAST) - return wext_handle_ioctl(net, &ifr, cmd, arg); return -ENOTTY; } } diff --git a/net/wireless/wext-core.c b/net/wireless/wext-core.c index 12949c8d3e5f..6cdb054484d6 100644 --- a/net/wireless/wext-core.c +++ b/net/wireless/wext-core.c @@ -1035,18 +1035,18 @@ static int ioctl_standard_call(struct net_device * dev, } -int wext_handle_ioctl(struct net *net, struct ifreq *ifr, unsigned int cmd, +int wext_handle_ioctl(struct net *net, struct iwreq *iwr, unsigned int cmd, void __user *arg) { struct iw_request_info info = { .cmd = cmd, .flags = 0 }; int ret; - ret = wext_ioctl_dispatch(net, (void *)ifr, cmd, &info, + ret = wext_ioctl_dispatch(net, iwr, cmd, &info, ioctl_standard_call, ioctl_private_call); if (ret >= 0 && IW_IS_GET(cmd) && - copy_to_user(arg, ifr, sizeof(struct iwreq))) + copy_to_user(arg, iwr, sizeof(struct iwreq))) return -EFAULT; return ret; -- cgit v1.3-14-g43fede From 7fed555c02f754af44f8963890b5ef8e30e97391 Mon Sep 17 00:00:00 2001 From: Robert Bragg Date: Tue, 13 Jun 2017 12:22:59 +0100 Subject: drm/i915: expose _SLICE_MASK GETPARM Enables userspace to determine the maximum number of slices that can be enabled on the device and also know what specific slices can be enabled. This information is required, for example, to be able to analyse some OA counter reports where the counter configuration depends on the HW slice configuration. Signed-off-by: Robert Bragg Reviewed-by: Matthew Auld Signed-off-by: Lionel Landwerlin Signed-off-by: Ben Widawsky --- drivers/gpu/drm/i915/i915_drv.c | 5 +++++ include/uapi/drm/i915_drm.h | 3 +++ 2 files changed, 8 insertions(+) (limited to 'include') diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 1f802de7b94b..d503612a7fc1 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -374,6 +374,11 @@ static int i915_getparam(struct drm_device *dev, void *data, */ value = 1; break; + case I915_PARAM_SLICE_MASK: + value = INTEL_INFO(dev_priv)->sseu.slice_mask; + if (!value) + return -ENODEV; + break; default: DRM_DEBUG("Unknown parameter %d\n", param->param); return -EINVAL; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index f24a80d2d42e..25695c3d9a76 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -418,6 +418,9 @@ typedef struct drm_i915_irq_wait { */ #define I915_PARAM_HAS_EXEC_CAPTURE 45 +/* Query the mask of slices available for this system */ +#define I915_PARAM_SLICE_MASK 46 + typedef struct drm_i915_getparam { __s32 param; /* -- cgit v1.3-14-g43fede From f532023381df49ac00cb2d1e70df607cf534720d Mon Sep 17 00:00:00 2001 From: Robert Bragg Date: Tue, 13 Jun 2017 12:23:00 +0100 Subject: drm/i915: expose _SUBSLICE_MASK GETPARM Assuming a uniform mask across all slices, this enables userspace to determine the specific sub slices can be enabled. This information is required, for example, to be able to analyse some OA counter reports where the counter configuration depends on the HW sub slice configuration. Signed-off-by: Robert Bragg Reviewed-by: Matthew Auld Signed-off-by: Lionel Landwerlin Signed-off-by: Ben Widawsky --- drivers/gpu/drm/i915/i915_drv.c | 5 +++++ include/uapi/drm/i915_drm.h | 5 +++++ 2 files changed, 10 insertions(+) (limited to 'include') diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index d503612a7fc1..a534412a5551 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -379,6 +379,11 @@ static int i915_getparam(struct drm_device *dev, void *data, if (!value) return -ENODEV; break; + case I915_PARAM_SUBSLICE_MASK: + value = INTEL_INFO(dev_priv)->sseu.subslice_mask; + if (!value) + return -ENODEV; + break; default: DRM_DEBUG("Unknown parameter %d\n", param->param); return -EINVAL; diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 25695c3d9a76..464547d08173 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -421,6 +421,11 @@ typedef struct drm_i915_irq_wait { /* Query the mask of slices available for this system */ #define I915_PARAM_SLICE_MASK 46 +/* Assuming it's uniform for each slice, this queries the mask of subslices + * per-slice for this system. + */ +#define I915_PARAM_SUBSLICE_MASK 47 + typedef struct drm_i915_getparam { __s32 param; /* -- cgit v1.3-14-g43fede From 19f81df2859eb10e92d68991cefa39f826dea013 Mon Sep 17 00:00:00 2001 From: Robert Bragg Date: Tue, 13 Jun 2017 12:23:03 +0100 Subject: drm/i915/perf: Add OA unit support for Gen 8+ Enables access to OA unit metrics for BDW, CHV, SKL and BXT which all share (more-or-less) the same OA unit design. Of particular note in comparison to Haswell: some OA unit HW config state has become per-context state and as a consequence it is somewhat more complicated to manage synchronous state changes from the cpu while there's no guarantee of what context (if any) is currently actively running on the gpu. The periodic sampling frequency which can be particularly useful for system-wide analysis (as opposed to command stream synchronised MI_REPORT_PERF_COUNT commands) is perhaps the most surprising state to have become per-context save and restored (while the OABUFFER destination is still a shared, system-wide resource). This support for gen8+ takes care to consider a number of timing challenges involved in synchronously updating per-context state primarily by programming all config state from the cpu and updating all current and saved contexts synchronously while the OA unit is still disabled. The driver intentionally avoids depending on command streamer programming to update OA state considering the lack of synchronization between the automatic loading of OACTXCONTROL state (that includes the periodic sampling state and enable state) on context restore and the parsing of any general purpose BB the driver can control. I.e. this implementation is careful to avoid the possibility of a context restore temporarily enabling any out-of-date periodic sampling state. In addition to the risk of transiently-out-of-date state being loaded automatically; there are also internal HW latencies involved in the loading of MUX configurations which would be difficult to account for from the command streamer (and we only want to enable the unit when once the MUX configuration is complete). Since the Gen8+ OA unit design no longer supports clock gating the unit off for a single given context (which effectively stopped any progress of counters while any other context was running) and instead supports tagging OA reports with a context ID for filtering on the CPU, it means we can no longer hide the system-wide progress of counters from a non-privileged application only interested in metrics for its own context. Although we could theoretically try and subtract the progress of other contexts before forwarding reports via read() we aren't in a position to filter reports captured via MI_REPORT_PERF_COUNT commands. As a result, for Gen8+, we always require the dev.i915.perf_stream_paranoid to be unset for any access to OA metrics if not root. v5: Drain submitted requests when enabling metric set to ensure no lite-restore erases the context image we just updated (Lionel) v6: In addition to drain, switch to kernel context & update all context in place (Chris) v7: Add missing mutex_unlock() if switching to kernel context fails (Matthew) v8: Simplify OA period/flex-eu-counters programming by using the batchbuffer instead of modifying ctx-image (Lionel) v9: Back to updating the context image (due to erroneous testing, batchbuffer programming the OA unit doesn't actually work) (Lionel) Pin context before updating context image (Chris) Drop MMIO programming now that we switch to a kernel context with right values in initial context image (Chris) v10: Just pin_map the contexts we want to modify or let the configuration happen on first use (Chris) v11: Update kernel context OA config through the batchbuffer rather than on the fly ctx-image update (Lionel) v12: Rework OA context registers update again by swithing away from user contexts and reconfiguring the kernel context through the batchbuffer and updating all the other contexts' context image. Also take care to lock slice/subslice configuration when OA is on. (Lionel) v13: Request rpcs updates on all engine when updating the OA config (Lionel) v14: Drop any kind of rpcs management now that we monitor sseu configuration changes in a later patch (Lionel) Remove usleep after programming the NOA configs on Gen8+, this doesn't seem to be needed (Lionel) v15: Respect coding style for block comments (Chris) v16: Add missing i915_add_request() in case we fail to emit OA configuration (Matthew) Signed-off-by: Robert Bragg Signed-off-by: Lionel Landwerlin Reviewed-by: Matthew Auld \o/ Signed-off-by: Ben Widawsky --- drivers/gpu/drm/i915/i915_drv.h | 46 +- drivers/gpu/drm/i915/i915_perf.c | 1034 +++++++++++++++++++++++++++++++++++--- drivers/gpu/drm/i915/i915_reg.h | 22 + drivers/gpu/drm/i915/intel_lrc.c | 2 + include/uapi/drm/i915_drm.h | 19 +- 5 files changed, 1028 insertions(+), 95 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2faad94c9ef2..eefb35a5d27d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2018,9 +2018,17 @@ struct i915_oa_ops { void (*init_oa_buffer)(struct drm_i915_private *dev_priv); /** - * @enable_metric_set: Applies any MUX configuration to set up the - * Boolean and Custom (B/C) counters that are part of the counter - * reports being sampled. May apply system constraints such as + * @select_metric_set: The auto generated code that checks whether a + * requested OA config is applicable to the system and if so sets up + * the mux, oa and flex eu register config pointers according to the + * current dev_priv->perf.oa.metrics_set. + */ + int (*select_metric_set)(struct drm_i915_private *dev_priv); + + /** + * @enable_metric_set: Selects and applies any MUX configuration to set + * up the Boolean and Custom (B/C) counters that are part of the + * counter reports being sampled. May apply system constraints such as * disabling EU clock gating as required. */ int (*enable_metric_set)(struct drm_i915_private *dev_priv); @@ -2051,20 +2059,13 @@ struct i915_oa_ops { size_t *offset); /** - * @oa_buffer_check: Check for OA buffer data + update tail - * - * This is either called via fops or the poll check hrtimer (atomic - * ctx) without any locks taken. + * @oa_hw_tail_read: read the OA tail pointer register * - * It's safe to read OA config state here unlocked, assuming that this - * is only called while the stream is enabled, while the global OA - * configuration can't be modified. - * - * Efficiency is more important than avoiding some false positives - * here, which will be handled gracefully - likely resulting in an - * %EAGAIN error for userspace. + * In particular this enables us to share all the fiddly code for + * handling the OA unit tail pointer race that affects multiple + * generations. */ - bool (*oa_buffer_check)(struct drm_i915_private *dev_priv); + u32 (*oa_hw_tail_read)(struct drm_i915_private *dev_priv); }; struct intel_cdclk_state { @@ -2429,6 +2430,7 @@ struct drm_i915_private { struct { struct i915_vma *vma; u8 *vaddr; + u32 last_ctx_id; int format; int format_size; @@ -2498,6 +2500,15 @@ struct drm_i915_private { } oa_buffer; u32 gen7_latched_oastatus1; + u32 ctx_oactxctrl_offset; + u32 ctx_flexeu0_offset; + + /** + * The RPT_ID/reason field for Gen8+ includes a bit + * to determine if the CTX ID in the report is valid + * but the specific bit differs between Gen 8 and 9 + */ + u32 gen8_valid_ctx_bit; struct i915_oa_ops ops; const struct i915_oa_format *oa_formats; @@ -2810,6 +2821,8 @@ intel_info(const struct drm_i915_private *dev_priv) #define IS_KBL_ULX(dev_priv) (INTEL_DEVID(dev_priv) == 0x590E || \ INTEL_DEVID(dev_priv) == 0x5915 || \ INTEL_DEVID(dev_priv) == 0x591E) +#define IS_SKL_GT2(dev_priv) (IS_SKYLAKE(dev_priv) && \ + (INTEL_DEVID(dev_priv) & 0x00F0) == 0x0010) #define IS_SKL_GT3(dev_priv) (IS_SKYLAKE(dev_priv) && \ (INTEL_DEVID(dev_priv) & 0x00F0) == 0x0020) #define IS_SKL_GT4(dev_priv) (IS_SKYLAKE(dev_priv) && \ @@ -3554,6 +3567,9 @@ i915_gem_context_lookup_timeline(struct i915_gem_context *ctx, int i915_perf_open_ioctl(struct drm_device *dev, void *data, struct drm_file *file); +void i915_oa_init_reg_state(struct intel_engine_cs *engine, + struct i915_gem_context *ctx, + uint32_t *reg_state); /* i915_gem_evict.c */ int __must_check i915_gem_evict_something(struct i915_address_space *vm, diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 7e56b895fd34..a6af4d7dc4d6 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -196,6 +196,12 @@ #include "i915_drv.h" #include "i915_oa_hsw.h" +#include "i915_oa_bdw.h" +#include "i915_oa_chv.h" +#include "i915_oa_sklgt2.h" +#include "i915_oa_sklgt3.h" +#include "i915_oa_sklgt4.h" +#include "i915_oa_bxt.h" /* HW requires this to be a power of two, between 128k and 16M, though driver * is currently generally designed assuming the largest 16M size is used such @@ -215,7 +221,7 @@ * * Although this can be observed explicitly while copying reports to userspace * by checking for a zeroed report-id field in tail reports, we want to account - * for this earlier, as part of the _oa_buffer_check to avoid lots of redundant + * for this earlier, as part of the oa_buffer_check to avoid lots of redundant * read() attempts. * * In effect we define a tail pointer for reading that lags the real tail @@ -237,7 +243,7 @@ * indicates that an updated tail pointer is needed. * * Most of the implementation details for this workaround are in - * gen7_oa_buffer_check_unlocked() and gen7_appand_oa_reports() + * oa_buffer_check_unlocked() and _append_oa_reports() * * Note for posterity: previously the driver used to define an effective tail * pointer that lagged the real pointer by a 'tail margin' measured in bytes @@ -272,6 +278,13 @@ static u32 i915_perf_stream_paranoid = true; #define INVALID_CTX_ID 0xffffffff +/* On Gen8+ automatically triggered OA reports include a 'reason' field... */ +#define OAREPORT_REASON_MASK 0x3f +#define OAREPORT_REASON_SHIFT 19 +#define OAREPORT_REASON_TIMER (1<<0) +#define OAREPORT_REASON_CTX_SWITCH (1<<3) +#define OAREPORT_REASON_CLK_RATIO (1<<5) + /* For sysctl proc_dointvec_minmax of i915_oa_max_sample_rate * @@ -303,6 +316,13 @@ static struct i915_oa_format hsw_oa_formats[I915_OA_FORMAT_MAX] = { [I915_OA_FORMAT_C4_B8] = { 7, 64 }, }; +static struct i915_oa_format gen8_plus_oa_formats[I915_OA_FORMAT_MAX] = { + [I915_OA_FORMAT_A12] = { 0, 64 }, + [I915_OA_FORMAT_A12_B8_C8] = { 2, 128 }, + [I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 }, + [I915_OA_FORMAT_C4_B8] = { 7, 64 }, +}; + #define SAMPLE_OA_REPORT (1<<0) /** @@ -332,8 +352,20 @@ struct perf_open_properties { int oa_period_exponent; }; +static u32 gen8_oa_hw_tail_read(struct drm_i915_private *dev_priv) +{ + return I915_READ(GEN8_OATAILPTR) & GEN8_OATAILPTR_MASK; +} + +static u32 gen7_oa_hw_tail_read(struct drm_i915_private *dev_priv) +{ + u32 oastatus1 = I915_READ(GEN7_OASTATUS1); + + return oastatus1 & GEN7_OASTATUS1_TAIL_MASK; +} + /** - * gen7_oa_buffer_check_unlocked - check for data and update tail ptr state + * oa_buffer_check_unlocked - check for data and update tail ptr state * @dev_priv: i915 device instance * * This is either called via fops (for blocking reads in user ctx) or the poll @@ -356,12 +388,11 @@ struct perf_open_properties { * * Returns: %true if the OA buffer contains data, else %false */ -static bool gen7_oa_buffer_check_unlocked(struct drm_i915_private *dev_priv) +static bool oa_buffer_check_unlocked(struct drm_i915_private *dev_priv) { int report_size = dev_priv->perf.oa.oa_buffer.format_size; unsigned long flags; unsigned int aged_idx; - u32 oastatus1; u32 head, hw_tail, aged_tail, aging_tail; u64 now; @@ -381,8 +412,7 @@ static bool gen7_oa_buffer_check_unlocked(struct drm_i915_private *dev_priv) aged_tail = dev_priv->perf.oa.oa_buffer.tails[aged_idx].offset; aging_tail = dev_priv->perf.oa.oa_buffer.tails[!aged_idx].offset; - oastatus1 = I915_READ(GEN7_OASTATUS1); - hw_tail = oastatus1 & GEN7_OASTATUS1_TAIL_MASK; + hw_tail = dev_priv->perf.oa.ops.oa_hw_tail_read(dev_priv); /* The tail pointer increases in 64 byte increments, * not in report_size steps... @@ -404,6 +434,7 @@ static bool gen7_oa_buffer_check_unlocked(struct drm_i915_private *dev_priv) if (aging_tail != INVALID_TAIL_PTR && ((now - dev_priv->perf.oa.oa_buffer.aging_timestamp) > OA_TAIL_MARGIN_NSEC)) { + aged_idx ^= 1; dev_priv->perf.oa.oa_buffer.aged_tail_idx = aged_idx; @@ -533,6 +564,300 @@ static int append_oa_sample(struct i915_perf_stream *stream, return 0; } +/** + * Copies all buffered OA reports into userspace read() buffer. + * @stream: An i915-perf stream opened for OA metrics + * @buf: destination buffer given by userspace + * @count: the number of bytes userspace wants to read + * @offset: (inout): the current position for writing into @buf + * + * Notably any error condition resulting in a short read (-%ENOSPC or + * -%EFAULT) will be returned even though one or more records may + * have been successfully copied. In this case it's up to the caller + * to decide if the error should be squashed before returning to + * userspace. + * + * Note: reports are consumed from the head, and appended to the + * tail, so the tail chases the head?... If you think that's mad + * and back-to-front you're not alone, but this follows the + * Gen PRM naming convention. + * + * Returns: 0 on success, negative error code on failure. + */ +static int gen8_append_oa_reports(struct i915_perf_stream *stream, + char __user *buf, + size_t count, + size_t *offset) +{ + struct drm_i915_private *dev_priv = stream->dev_priv; + int report_size = dev_priv->perf.oa.oa_buffer.format_size; + u8 *oa_buf_base = dev_priv->perf.oa.oa_buffer.vaddr; + u32 gtt_offset = i915_ggtt_offset(dev_priv->perf.oa.oa_buffer.vma); + u32 mask = (OA_BUFFER_SIZE - 1); + size_t start_offset = *offset; + unsigned long flags; + unsigned int aged_tail_idx; + u32 head, tail; + u32 taken; + int ret = 0; + + if (WARN_ON(!stream->enabled)) + return -EIO; + + spin_lock_irqsave(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags); + + head = dev_priv->perf.oa.oa_buffer.head; + aged_tail_idx = dev_priv->perf.oa.oa_buffer.aged_tail_idx; + tail = dev_priv->perf.oa.oa_buffer.tails[aged_tail_idx].offset; + + spin_unlock_irqrestore(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags); + + /* + * An invalid tail pointer here means we're still waiting for the poll + * hrtimer callback to give us a pointer + */ + if (tail == INVALID_TAIL_PTR) + return -EAGAIN; + + /* + * NB: oa_buffer.head/tail include the gtt_offset which we don't want + * while indexing relative to oa_buf_base. + */ + head -= gtt_offset; + tail -= gtt_offset; + + /* + * An out of bounds or misaligned head or tail pointer implies a driver + * bug since we validate + align the tail pointers we read from the + * hardware and we are in full control of the head pointer which should + * only be incremented by multiples of the report size (notably also + * all a power of two). + */ + if (WARN_ONCE(head > OA_BUFFER_SIZE || head % report_size || + tail > OA_BUFFER_SIZE || tail % report_size, + "Inconsistent OA buffer pointers: head = %u, tail = %u\n", + head, tail)) + return -EIO; + + + for (/* none */; + (taken = OA_TAKEN(tail, head)); + head = (head + report_size) & mask) { + u8 *report = oa_buf_base + head; + u32 *report32 = (void *)report; + u32 ctx_id; + u32 reason; + + /* + * All the report sizes factor neatly into the buffer + * size so we never expect to see a report split + * between the beginning and end of the buffer. + * + * Given the initial alignment check a misalignment + * here would imply a driver bug that would result + * in an overrun. + */ + if (WARN_ON((OA_BUFFER_SIZE - head) < report_size)) { + DRM_ERROR("Spurious OA head ptr: non-integral report offset\n"); + break; + } + + /* + * The reason field includes flags identifying what + * triggered this specific report (mostly timer + * triggered or e.g. due to a context switch). + * + * This field is never expected to be zero so we can + * check that the report isn't invalid before copying + * it to userspace... + */ + reason = ((report32[0] >> OAREPORT_REASON_SHIFT) & + OAREPORT_REASON_MASK); + if (reason == 0) { + if (__ratelimit(&dev_priv->perf.oa.spurious_report_rs)) + DRM_NOTE("Skipping spurious, invalid OA report\n"); + continue; + } + + /* + * XXX: Just keep the lower 21 bits for now since I'm not + * entirely sure if the HW touches any of the higher bits in + * this field + */ + ctx_id = report32[2] & 0x1fffff; + + /* + * Squash whatever is in the CTX_ID field if it's marked as + * invalid to be sure we avoid false-positive, single-context + * filtering below... + * + * Note: that we don't clear the valid_ctx_bit so userspace can + * understand that the ID has been squashed by the kernel. + */ + if (!(report32[0] & dev_priv->perf.oa.gen8_valid_ctx_bit)) + ctx_id = report32[2] = INVALID_CTX_ID; + + /* + * NB: For Gen 8 the OA unit no longer supports clock gating + * off for a specific context and the kernel can't securely + * stop the counters from updating as system-wide / global + * values. + * + * Automatic reports now include a context ID so reports can be + * filtered on the cpu but it's not worth trying to + * automatically subtract/hide counter progress for other + * contexts while filtering since we can't stop userspace + * issuing MI_REPORT_PERF_COUNT commands which would still + * provide a side-band view of the real values. + * + * To allow userspace (such as Mesa/GL_INTEL_performance_query) + * to normalize counters for a single filtered context then it + * needs be forwarded bookend context-switch reports so that it + * can track switches in between MI_REPORT_PERF_COUNT commands + * and can itself subtract/ignore the progress of counters + * associated with other contexts. Note that the hardware + * automatically triggers reports when switching to a new + * context which are tagged with the ID of the newly active + * context. To avoid the complexity (and likely fragility) of + * reading ahead while parsing reports to try and minimize + * forwarding redundant context switch reports (i.e. between + * other, unrelated contexts) we simply elect to forward them + * all. + * + * We don't rely solely on the reason field to identify context + * switches since it's not-uncommon for periodic samples to + * identify a switch before any 'context switch' report. + */ + if (!dev_priv->perf.oa.exclusive_stream->ctx || + dev_priv->perf.oa.specific_ctx_id == ctx_id || + (dev_priv->perf.oa.oa_buffer.last_ctx_id == + dev_priv->perf.oa.specific_ctx_id) || + reason & OAREPORT_REASON_CTX_SWITCH) { + + /* + * While filtering for a single context we avoid + * leaking the IDs of other contexts. + */ + if (dev_priv->perf.oa.exclusive_stream->ctx && + dev_priv->perf.oa.specific_ctx_id != ctx_id) { + report32[2] = INVALID_CTX_ID; + } + + ret = append_oa_sample(stream, buf, count, offset, + report); + if (ret) + break; + + dev_priv->perf.oa.oa_buffer.last_ctx_id = ctx_id; + } + + /* + * The above reason field sanity check is based on + * the assumption that the OA buffer is initially + * zeroed and we reset the field after copying so the + * check is still meaningful once old reports start + * being overwritten. + */ + report32[0] = 0; + } + + if (start_offset != *offset) { + spin_lock_irqsave(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags); + + /* + * We removed the gtt_offset for the copy loop above, indexing + * relative to oa_buf_base so put back here... + */ + head += gtt_offset; + + I915_WRITE(GEN8_OAHEADPTR, head & GEN8_OAHEADPTR_MASK); + dev_priv->perf.oa.oa_buffer.head = head; + + spin_unlock_irqrestore(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags); + } + + return ret; +} + +/** + * gen8_oa_read - copy status records then buffered OA reports + * @stream: An i915-perf stream opened for OA metrics + * @buf: destination buffer given by userspace + * @count: the number of bytes userspace wants to read + * @offset: (inout): the current position for writing into @buf + * + * Checks OA unit status registers and if necessary appends corresponding + * status records for userspace (such as for a buffer full condition) and then + * initiate appending any buffered OA reports. + * + * Updates @offset according to the number of bytes successfully copied into + * the userspace buffer. + * + * NB: some data may be successfully copied to the userspace buffer + * even if an error is returned, and this is reflected in the + * updated @offset. + * + * Returns: zero on success or a negative error code + */ +static int gen8_oa_read(struct i915_perf_stream *stream, + char __user *buf, + size_t count, + size_t *offset) +{ + struct drm_i915_private *dev_priv = stream->dev_priv; + u32 oastatus; + int ret; + + if (WARN_ON(!dev_priv->perf.oa.oa_buffer.vaddr)) + return -EIO; + + oastatus = I915_READ(GEN8_OASTATUS); + + /* + * We treat OABUFFER_OVERFLOW as a significant error: + * + * Although theoretically we could handle this more gracefully + * sometimes, some Gens don't correctly suppress certain + * automatically triggered reports in this condition and so we + * have to assume that old reports are now being trampled + * over. + * + * Considering how we don't currently give userspace control + * over the OA buffer size and always configure a large 16MB + * buffer, then a buffer overflow does anyway likely indicate + * that something has gone quite badly wrong. + */ + if (oastatus & GEN8_OASTATUS_OABUFFER_OVERFLOW) { + ret = append_oa_status(stream, buf, count, offset, + DRM_I915_PERF_RECORD_OA_BUFFER_LOST); + if (ret) + return ret; + + DRM_DEBUG("OA buffer overflow (exponent = %d): force restart\n", + dev_priv->perf.oa.period_exponent); + + dev_priv->perf.oa.ops.oa_disable(dev_priv); + dev_priv->perf.oa.ops.oa_enable(dev_priv); + + /* + * Note: .oa_enable() is expected to re-init the oabuffer and + * reset GEN8_OASTATUS for us + */ + oastatus = I915_READ(GEN8_OASTATUS); + } + + if (oastatus & GEN8_OASTATUS_REPORT_LOST) { + ret = append_oa_status(stream, buf, count, offset, + DRM_I915_PERF_RECORD_OA_REPORT_LOST); + if (ret) + return ret; + I915_WRITE(GEN8_OASTATUS, + oastatus & ~GEN8_OASTATUS_REPORT_LOST); + } + + return gen8_append_oa_reports(stream, buf, count, offset); +} + /** * Copies all buffered OA reports into userspace read() buffer. * @stream: An i915-perf stream opened for OA metrics @@ -732,7 +1057,8 @@ static int gen7_oa_read(struct i915_perf_stream *stream, if (ret) return ret; - DRM_DEBUG("OA buffer overflow: force restart\n"); + DRM_DEBUG("OA buffer overflow (exponent = %d): force restart\n", + dev_priv->perf.oa.period_exponent); dev_priv->perf.oa.ops.oa_disable(dev_priv); dev_priv->perf.oa.ops.oa_enable(dev_priv); @@ -775,7 +1101,7 @@ static int i915_oa_wait_unlocked(struct i915_perf_stream *stream) return -EIO; return wait_event_interruptible(dev_priv->perf.oa.poll_wq, - dev_priv->perf.oa.ops.oa_buffer_check(dev_priv)); + oa_buffer_check_unlocked(dev_priv)); } /** @@ -832,30 +1158,38 @@ static int i915_oa_read(struct i915_perf_stream *stream, static int oa_get_render_ctx_id(struct i915_perf_stream *stream) { struct drm_i915_private *dev_priv = stream->dev_priv; - struct intel_engine_cs *engine = dev_priv->engine[RCS]; - struct intel_ring *ring; - int ret; - ret = i915_mutex_lock_interruptible(&dev_priv->drm); - if (ret) - return ret; + if (i915.enable_execlists) + dev_priv->perf.oa.specific_ctx_id = stream->ctx->hw_id; + else { + struct intel_engine_cs *engine = dev_priv->engine[RCS]; + struct intel_ring *ring; + int ret; - /* As the ID is the gtt offset of the context's vma we pin - * the vma to ensure the ID remains fixed. - * - * NB: implied RCS engine... - */ - ring = engine->context_pin(engine, stream->ctx); - mutex_unlock(&dev_priv->drm.struct_mutex); - if (IS_ERR(ring)) - return PTR_ERR(ring); + ret = i915_mutex_lock_interruptible(&dev_priv->drm); + if (ret) + return ret; + + /* + * As the ID is the gtt offset of the context's vma we + * pin the vma to ensure the ID remains fixed. + * + * NB: implied RCS engine... + */ + ring = engine->context_pin(engine, stream->ctx); + mutex_unlock(&dev_priv->drm.struct_mutex); + if (IS_ERR(ring)) + return PTR_ERR(ring); - /* Explicitly track the ID (instead of calling i915_ggtt_offset() - * on the fly) considering the difference with gen8+ and - * execlists - */ - dev_priv->perf.oa.specific_ctx_id = - i915_ggtt_offset(stream->ctx->engine[engine->id].state); + + /* + * Explicitly track the ID (instead of calling + * i915_ggtt_offset() on the fly) considering the difference + * with gen8+ and execlists + */ + dev_priv->perf.oa.specific_ctx_id = + i915_ggtt_offset(stream->ctx->engine[engine->id].state); + } return 0; } @@ -870,14 +1204,19 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream) static void oa_put_render_ctx_id(struct i915_perf_stream *stream) { struct drm_i915_private *dev_priv = stream->dev_priv; - struct intel_engine_cs *engine = dev_priv->engine[RCS]; - mutex_lock(&dev_priv->drm.struct_mutex); + if (i915.enable_execlists) { + dev_priv->perf.oa.specific_ctx_id = INVALID_CTX_ID; + } else { + struct intel_engine_cs *engine = dev_priv->engine[RCS]; - dev_priv->perf.oa.specific_ctx_id = INVALID_CTX_ID; - engine->context_unpin(engine, stream->ctx); + mutex_lock(&dev_priv->drm.struct_mutex); - mutex_unlock(&dev_priv->drm.struct_mutex); + dev_priv->perf.oa.specific_ctx_id = INVALID_CTX_ID; + engine->context_unpin(engine, stream->ctx); + + mutex_unlock(&dev_priv->drm.struct_mutex); + } } static void @@ -901,6 +1240,12 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream) BUG_ON(stream != dev_priv->perf.oa.exclusive_stream); + /* + * Unset exclusive_stream first, it might be checked while + * disabling the metric set on gen8+. + */ + dev_priv->perf.oa.exclusive_stream = NULL; + dev_priv->perf.oa.ops.disable_metric_set(dev_priv); free_oa_buffer(dev_priv); @@ -911,8 +1256,6 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream) if (stream->ctx) oa_put_render_ctx_id(stream); - dev_priv->perf.oa.exclusive_stream = NULL; - if (dev_priv->perf.oa.spurious_report_rs.missed) { DRM_NOTE("%d spurious OA report notices suppressed due to ratelimiting\n", dev_priv->perf.oa.spurious_report_rs.missed); @@ -967,6 +1310,65 @@ static void gen7_init_oa_buffer(struct drm_i915_private *dev_priv) dev_priv->perf.oa.pollin = false; } +static void gen8_init_oa_buffer(struct drm_i915_private *dev_priv) +{ + u32 gtt_offset = i915_ggtt_offset(dev_priv->perf.oa.oa_buffer.vma); + unsigned long flags; + + spin_lock_irqsave(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags); + + I915_WRITE(GEN8_OASTATUS, 0); + I915_WRITE(GEN8_OAHEADPTR, gtt_offset); + dev_priv->perf.oa.oa_buffer.head = gtt_offset; + + I915_WRITE(GEN8_OABUFFER_UDW, 0); + + /* + * PRM says: + * + * "This MMIO must be set before the OATAILPTR + * register and after the OAHEADPTR register. This is + * to enable proper functionality of the overflow + * bit." + */ + I915_WRITE(GEN8_OABUFFER, gtt_offset | + OABUFFER_SIZE_16M | OA_MEM_SELECT_GGTT); + I915_WRITE(GEN8_OATAILPTR, gtt_offset & GEN8_OATAILPTR_MASK); + + /* Mark that we need updated tail pointers to read from... */ + dev_priv->perf.oa.oa_buffer.tails[0].offset = INVALID_TAIL_PTR; + dev_priv->perf.oa.oa_buffer.tails[1].offset = INVALID_TAIL_PTR; + + /* + * Reset state used to recognise context switches, affecting which + * reports we will forward to userspace while filtering for a single + * context. + */ + dev_priv->perf.oa.oa_buffer.last_ctx_id = INVALID_CTX_ID; + + spin_unlock_irqrestore(&dev_priv->perf.oa.oa_buffer.ptr_lock, flags); + + /* + * NB: although the OA buffer will initially be allocated + * zeroed via shmfs (and so this memset is redundant when + * first allocating), we may re-init the OA buffer, either + * when re-enabling a stream or in error/reset paths. + * + * The reason we clear the buffer for each re-init is for the + * sanity check in gen8_append_oa_reports() that looks at the + * reason field to make sure it's non-zero which relies on + * the assumption that new reports are being written to zeroed + * memory... + */ + memset(dev_priv->perf.oa.oa_buffer.vaddr, 0, OA_BUFFER_SIZE); + + /* + * Maybe make ->pollin per-stream state if we support multiple + * concurrent streams in the future. + */ + dev_priv->perf.oa.pollin = false; +} + static int alloc_oa_buffer(struct drm_i915_private *dev_priv) { struct drm_i915_gem_object *bo; @@ -1114,6 +1516,324 @@ static void hsw_disable_metric_set(struct drm_i915_private *dev_priv) ~GT_NOA_ENABLE)); } +/* + * NB: It must always remain pointer safe to run this even if the OA unit + * has been disabled. + * + * It's fine to put out-of-date values into these per-context registers + * in the case that the OA unit has been disabled. + */ +static void gen8_update_reg_state_unlocked(struct i915_gem_context *ctx, + u32 *reg_state) +{ + struct drm_i915_private *dev_priv = ctx->i915; + const struct i915_oa_reg *flex_regs = dev_priv->perf.oa.flex_regs; + int n_flex_regs = dev_priv->perf.oa.flex_regs_len; + u32 ctx_oactxctrl = dev_priv->perf.oa.ctx_oactxctrl_offset; + u32 ctx_flexeu0 = dev_priv->perf.oa.ctx_flexeu0_offset; + /* The MMIO offsets for Flex EU registers aren't contiguous */ + u32 flex_mmio[] = { + i915_mmio_reg_offset(EU_PERF_CNTL0), + i915_mmio_reg_offset(EU_PERF_CNTL1), + i915_mmio_reg_offset(EU_PERF_CNTL2), + i915_mmio_reg_offset(EU_PERF_CNTL3), + i915_mmio_reg_offset(EU_PERF_CNTL4), + i915_mmio_reg_offset(EU_PERF_CNTL5), + i915_mmio_reg_offset(EU_PERF_CNTL6), + }; + int i; + + reg_state[ctx_oactxctrl] = i915_mmio_reg_offset(GEN8_OACTXCONTROL); + reg_state[ctx_oactxctrl+1] = (dev_priv->perf.oa.period_exponent << + GEN8_OA_TIMER_PERIOD_SHIFT) | + (dev_priv->perf.oa.periodic ? + GEN8_OA_TIMER_ENABLE : 0) | + GEN8_OA_COUNTER_RESUME; + + for (i = 0; i < ARRAY_SIZE(flex_mmio); i++) { + u32 state_offset = ctx_flexeu0 + i * 2; + u32 mmio = flex_mmio[i]; + + /* + * This arbitrary default will select the 'EU FPU0 Pipeline + * Active' event. In the future it's anticipated that there + * will be an explicit 'No Event' we can select, but not yet... + */ + u32 value = 0; + int j; + + for (j = 0; j < n_flex_regs; j++) { + if (i915_mmio_reg_offset(flex_regs[j].addr) == mmio) { + value = flex_regs[j].value; + break; + } + } + + reg_state[state_offset] = mmio; + reg_state[state_offset+1] = value; + } +} + +/* + * Same as gen8_update_reg_state_unlocked only through the batchbuffer. This + * is only used by the kernel context. + */ +static int gen8_emit_oa_config(struct drm_i915_gem_request *req) +{ + struct drm_i915_private *dev_priv = req->i915; + const struct i915_oa_reg *flex_regs = dev_priv->perf.oa.flex_regs; + int n_flex_regs = dev_priv->perf.oa.flex_regs_len; + /* The MMIO offsets for Flex EU registers aren't contiguous */ + u32 flex_mmio[] = { + i915_mmio_reg_offset(EU_PERF_CNTL0), + i915_mmio_reg_offset(EU_PERF_CNTL1), + i915_mmio_reg_offset(EU_PERF_CNTL2), + i915_mmio_reg_offset(EU_PERF_CNTL3), + i915_mmio_reg_offset(EU_PERF_CNTL4), + i915_mmio_reg_offset(EU_PERF_CNTL5), + i915_mmio_reg_offset(EU_PERF_CNTL6), + }; + u32 *cs; + int i; + + cs = intel_ring_begin(req, n_flex_regs * 2 + 4); + if (IS_ERR(cs)) + return PTR_ERR(cs); + + *cs++ = MI_LOAD_REGISTER_IMM(n_flex_regs + 1); + + *cs++ = i915_mmio_reg_offset(GEN8_OACTXCONTROL); + *cs++ = (dev_priv->perf.oa.period_exponent << GEN8_OA_TIMER_PERIOD_SHIFT) | + (dev_priv->perf.oa.periodic ? GEN8_OA_TIMER_ENABLE : 0) | + GEN8_OA_COUNTER_RESUME; + + for (i = 0; i < ARRAY_SIZE(flex_mmio); i++) { + u32 mmio = flex_mmio[i]; + + /* + * This arbitrary default will select the 'EU FPU0 Pipeline + * Active' event. In the future it's anticipated that there + * will be an explicit 'No Event' we can select, but not + * yet... + */ + u32 value = 0; + int j; + + for (j = 0; j < n_flex_regs; j++) { + if (i915_mmio_reg_offset(flex_regs[j].addr) == mmio) { + value = flex_regs[j].value; + break; + } + } + + *cs++ = mmio; + *cs++ = value; + } + + *cs++ = MI_NOOP; + intel_ring_advance(req, cs); + + return 0; +} + +static int gen8_switch_to_updated_kernel_context(struct drm_i915_private *dev_priv) +{ + struct intel_engine_cs *engine = dev_priv->engine[RCS]; + struct i915_gem_timeline *timeline; + struct drm_i915_gem_request *req; + int ret; + + lockdep_assert_held(&dev_priv->drm.struct_mutex); + + i915_gem_retire_requests(dev_priv); + + req = i915_gem_request_alloc(engine, dev_priv->kernel_context); + if (IS_ERR(req)) + return PTR_ERR(req); + + ret = gen8_emit_oa_config(req); + if (ret) { + i915_add_request(req); + return ret; + } + + /* Queue this switch after all other activity */ + list_for_each_entry(timeline, &dev_priv->gt.timelines, link) { + struct drm_i915_gem_request *prev; + struct intel_timeline *tl; + + tl = &timeline->engine[engine->id]; + prev = i915_gem_active_raw(&tl->last_request, + &dev_priv->drm.struct_mutex); + if (prev) + i915_sw_fence_await_sw_fence_gfp(&req->submit, + &prev->submit, + GFP_KERNEL); + } + + ret = i915_switch_context(req); + i915_add_request(req); + + return ret; +} + +/* + * Manages updating the per-context aspects of the OA stream + * configuration across all contexts. + * + * The awkward consideration here is that OACTXCONTROL controls the + * exponent for periodic sampling which is primarily used for system + * wide profiling where we'd like a consistent sampling period even in + * the face of context switches. + * + * Our approach of updating the register state context (as opposed to + * say using a workaround batch buffer) ensures that the hardware + * won't automatically reload an out-of-date timer exponent even + * transiently before a WA BB could be parsed. + * + * This function needs to: + * - Ensure the currently running context's per-context OA state is + * updated + * - Ensure that all existing contexts will have the correct per-context + * OA state if they are scheduled for use. + * - Ensure any new contexts will be initialized with the correct + * per-context OA state. + * + * Note: it's only the RCS/Render context that has any OA state. + */ +static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv, + bool interruptible) +{ + struct i915_gem_context *ctx; + int ret; + unsigned int wait_flags = I915_WAIT_LOCKED; + + if (interruptible) { + ret = i915_mutex_lock_interruptible(&dev_priv->drm); + if (ret) + return ret; + + wait_flags |= I915_WAIT_INTERRUPTIBLE; + } else { + mutex_lock(&dev_priv->drm.struct_mutex); + } + + /* Switch away from any user context. */ + ret = gen8_switch_to_updated_kernel_context(dev_priv); + if (ret) + goto out; + + /* + * The OA register config is setup through the context image. This image + * might be written to by the GPU on context switch (in particular on + * lite-restore). This means we can't safely update a context's image, + * if this context is scheduled/submitted to run on the GPU. + * + * We could emit the OA register config through the batch buffer but + * this might leave small interval of time where the OA unit is + * configured at an invalid sampling period. + * + * So far the best way to work around this issue seems to be draining + * the GPU from any submitted work. + */ + ret = i915_gem_wait_for_idle(dev_priv, wait_flags); + if (ret) + goto out; + + /* Update all contexts now that we've stalled the submission. */ + list_for_each_entry(ctx, &dev_priv->context_list, link) { + struct intel_context *ce = &ctx->engine[RCS]; + u32 *regs; + + /* OA settings will be set upon first use */ + if (!ce->state) + continue; + + regs = i915_gem_object_pin_map(ce->state->obj, I915_MAP_WB); + if (IS_ERR(regs)) { + ret = PTR_ERR(regs); + goto out; + } + + ce->state->obj->mm.dirty = true; + regs += LRC_STATE_PN * PAGE_SIZE / sizeof(*regs); + + gen8_update_reg_state_unlocked(ctx, regs); + + i915_gem_object_unpin_map(ce->state->obj); + } + + out: + mutex_unlock(&dev_priv->drm.struct_mutex); + + return ret; +} + +static int gen8_enable_metric_set(struct drm_i915_private *dev_priv) +{ + int ret = dev_priv->perf.oa.ops.select_metric_set(dev_priv); + int i; + + if (ret) + return ret; + + /* + * We disable slice/unslice clock ratio change reports on SKL since + * they are too noisy. The HW generates a lot of redundant reports + * where the ratio hasn't really changed causing a lot of redundant + * work to processes and increasing the chances we'll hit buffer + * overruns. + * + * Although we don't currently use the 'disable overrun' OABUFFER + * feature it's worth noting that clock ratio reports have to be + * disabled before considering to use that feature since the HW doesn't + * correctly block these reports. + * + * Currently none of the high-level metrics we have depend on knowing + * this ratio to normalize. + * + * Note: This register is not power context saved and restored, but + * that's OK considering that we disable RC6 while the OA unit is + * enabled. + * + * The _INCLUDE_CLK_RATIO bit allows the slice/unslice frequency to + * be read back from automatically triggered reports, as part of the + * RPT_ID field. + */ + if (IS_SKYLAKE(dev_priv) || IS_BROXTON(dev_priv)) { + I915_WRITE(GEN8_OA_DEBUG, + _MASKED_BIT_ENABLE(GEN9_OA_DEBUG_DISABLE_CLK_RATIO_REPORTS | + GEN9_OA_DEBUG_INCLUDE_CLK_RATIO)); + } + + /* + * Update all contexts prior writing the mux configurations as we need + * to make sure all slices/subslices are ON before writing to NOA + * registers. + */ + ret = gen8_configure_all_contexts(dev_priv, true); + if (ret) + return ret; + + I915_WRITE(GDT_CHICKEN_BITS, 0xA0); + for (i = 0; i < dev_priv->perf.oa.n_mux_configs; i++) { + config_oa_regs(dev_priv, dev_priv->perf.oa.mux_regs[i], + dev_priv->perf.oa.mux_regs_lens[i]); + } + I915_WRITE(GDT_CHICKEN_BITS, 0x80); + + config_oa_regs(dev_priv, dev_priv->perf.oa.b_counter_regs, + dev_priv->perf.oa.b_counter_regs_len); + + return 0; +} + +static void gen8_disable_metric_set(struct drm_i915_private *dev_priv) +{ + /* Reset all contexts' slices/subslices configurations. */ + gen8_configure_all_contexts(dev_priv, false); +} + static void gen7_update_oacontrol_locked(struct drm_i915_private *dev_priv) { lockdep_assert_held(&dev_priv->perf.hook_lock); @@ -1158,6 +1878,31 @@ static void gen7_oa_enable(struct drm_i915_private *dev_priv) spin_unlock_irqrestore(&dev_priv->perf.hook_lock, flags); } +static void gen8_oa_enable(struct drm_i915_private *dev_priv) +{ + u32 report_format = dev_priv->perf.oa.oa_buffer.format; + + /* + * Reset buf pointers so we don't forward reports from before now. + * + * Think carefully if considering trying to avoid this, since it + * also ensures status flags and the buffer itself are cleared + * in error paths, and we have checks for invalid reports based + * on the assumption that certain fields are written to zeroed + * memory which this helps maintains. + */ + gen8_init_oa_buffer(dev_priv); + + /* + * Note: we don't rely on the hardware to perform single context + * filtering and instead filter on the cpu based on the context-id + * field of reports + */ + I915_WRITE(GEN8_OACONTROL, (report_format << + GEN8_OA_REPORT_FORMAT_SHIFT) | + GEN8_OA_COUNTER_ENABLE); +} + /** * i915_oa_stream_enable - handle `I915_PERF_IOCTL_ENABLE` for OA stream * @stream: An i915 perf stream opened for OA metrics @@ -1184,6 +1929,11 @@ static void gen7_oa_disable(struct drm_i915_private *dev_priv) I915_WRITE(GEN7_OACONTROL, 0); } +static void gen8_oa_disable(struct drm_i915_private *dev_priv) +{ + I915_WRITE(GEN8_OACONTROL, 0); +} + /** * i915_oa_stream_disable - handle `I915_PERF_IOCTL_DISABLE` for OA stream * @stream: An i915 perf stream opened for OA metrics @@ -1362,6 +2112,21 @@ err_oa_buf_alloc: return ret; } +void i915_oa_init_reg_state(struct intel_engine_cs *engine, + struct i915_gem_context *ctx, + u32 *reg_state) +{ + struct drm_i915_private *dev_priv = engine->i915; + + if (engine->id != RCS) + return; + + if (!dev_priv->perf.initialized) + return; + + gen8_update_reg_state_unlocked(ctx, reg_state); +} + /** * i915_perf_read_locked - &i915_perf_stream_ops->read with error normalisation * @stream: An i915 perf stream @@ -1487,7 +2252,7 @@ static enum hrtimer_restart oa_poll_check_timer_cb(struct hrtimer *hrtimer) container_of(hrtimer, typeof(*dev_priv), perf.oa.poll_check_timer); - if (dev_priv->perf.oa.ops.oa_buffer_check(dev_priv)) { + if (oa_buffer_check_unlocked(dev_priv)) { dev_priv->perf.oa.pollin = true; wake_up(&dev_priv->perf.oa.poll_wq); } @@ -1776,6 +2541,7 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv, struct i915_gem_context *specific_ctx = NULL; struct i915_perf_stream *stream = NULL; unsigned long f_flags = 0; + bool privileged_op = true; int stream_fd; int ret; @@ -1793,12 +2559,29 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv, } } + /* + * On Haswell the OA unit supports clock gating off for a specific + * context and in this mode there's no visibility of metrics for the + * rest of the system, which we consider acceptable for a + * non-privileged client. + * + * For Gen8+ the OA unit no longer supports clock gating off for a + * specific context and the kernel can't securely stop the counters + * from updating as system-wide / global values. Even though we can + * filter reports based on the included context ID we can't block + * clients from seeing the raw / global counter values via + * MI_REPORT_PERF_COUNT commands and so consider it a privileged op to + * enable the OA unit by default. + */ + if (IS_HASWELL(dev_priv) && specific_ctx) + privileged_op = false; + /* Similar to perf's kernel.perf_paranoid_cpu sysctl option * we check a dev.i915.perf_stream_paranoid sysctl option * to determine if it's ok to access system wide OA counters * without CAP_SYS_ADMIN privileges. */ - if (!specific_ctx && + if (privileged_op && i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) { DRM_DEBUG("Insufficient privileges to open system-wide i915 perf stream\n"); ret = -EACCES; @@ -2070,9 +2853,6 @@ int i915_perf_open_ioctl(struct drm_device *dev, void *data, */ void i915_perf_register(struct drm_i915_private *dev_priv) { - if (!IS_HASWELL(dev_priv)) - return; - if (!dev_priv->perf.initialized) return; @@ -2088,11 +2868,38 @@ void i915_perf_register(struct drm_i915_private *dev_priv) if (!dev_priv->perf.metrics_kobj) goto exit; - if (i915_perf_register_sysfs_hsw(dev_priv)) { - kobject_put(dev_priv->perf.metrics_kobj); - dev_priv->perf.metrics_kobj = NULL; + if (IS_HASWELL(dev_priv)) { + if (i915_perf_register_sysfs_hsw(dev_priv)) + goto sysfs_error; + } else if (IS_BROADWELL(dev_priv)) { + if (i915_perf_register_sysfs_bdw(dev_priv)) + goto sysfs_error; + } else if (IS_CHERRYVIEW(dev_priv)) { + if (i915_perf_register_sysfs_chv(dev_priv)) + goto sysfs_error; + } else if (IS_SKYLAKE(dev_priv)) { + if (IS_SKL_GT2(dev_priv)) { + if (i915_perf_register_sysfs_sklgt2(dev_priv)) + goto sysfs_error; + } else if (IS_SKL_GT3(dev_priv)) { + if (i915_perf_register_sysfs_sklgt3(dev_priv)) + goto sysfs_error; + } else if (IS_SKL_GT4(dev_priv)) { + if (i915_perf_register_sysfs_sklgt4(dev_priv)) + goto sysfs_error; + } else + goto sysfs_error; + } else if (IS_BROXTON(dev_priv)) { + if (i915_perf_register_sysfs_bxt(dev_priv)) + goto sysfs_error; } + goto exit; + +sysfs_error: + kobject_put(dev_priv->perf.metrics_kobj); + dev_priv->perf.metrics_kobj = NULL; + exit: mutex_unlock(&dev_priv->perf.lock); } @@ -2108,13 +2915,24 @@ exit: */ void i915_perf_unregister(struct drm_i915_private *dev_priv) { - if (!IS_HASWELL(dev_priv)) - return; - if (!dev_priv->perf.metrics_kobj) return; - i915_perf_unregister_sysfs_hsw(dev_priv); + if (IS_HASWELL(dev_priv)) + i915_perf_unregister_sysfs_hsw(dev_priv); + else if (IS_BROADWELL(dev_priv)) + i915_perf_unregister_sysfs_bdw(dev_priv); + else if (IS_CHERRYVIEW(dev_priv)) + i915_perf_unregister_sysfs_chv(dev_priv); + else if (IS_SKYLAKE(dev_priv)) { + if (IS_SKL_GT2(dev_priv)) + i915_perf_unregister_sysfs_sklgt2(dev_priv); + else if (IS_SKL_GT3(dev_priv)) + i915_perf_unregister_sysfs_sklgt3(dev_priv); + else if (IS_SKL_GT4(dev_priv)) + i915_perf_unregister_sysfs_sklgt4(dev_priv); + } else if (IS_BROXTON(dev_priv)) + i915_perf_unregister_sysfs_bxt(dev_priv); kobject_put(dev_priv->perf.metrics_kobj); dev_priv->perf.metrics_kobj = NULL; @@ -2173,36 +2991,105 @@ static struct ctl_table dev_root[] = { */ void i915_perf_init(struct drm_i915_private *dev_priv) { - if (!IS_HASWELL(dev_priv)) - return; - - hrtimer_init(&dev_priv->perf.oa.poll_check_timer, - CLOCK_MONOTONIC, HRTIMER_MODE_REL); - dev_priv->perf.oa.poll_check_timer.function = oa_poll_check_timer_cb; - init_waitqueue_head(&dev_priv->perf.oa.poll_wq); + dev_priv->perf.oa.n_builtin_sets = 0; + + if (IS_HASWELL(dev_priv)) { + dev_priv->perf.oa.ops.init_oa_buffer = gen7_init_oa_buffer; + dev_priv->perf.oa.ops.enable_metric_set = hsw_enable_metric_set; + dev_priv->perf.oa.ops.disable_metric_set = hsw_disable_metric_set; + dev_priv->perf.oa.ops.oa_enable = gen7_oa_enable; + dev_priv->perf.oa.ops.oa_disable = gen7_oa_disable; + dev_priv->perf.oa.ops.read = gen7_oa_read; + dev_priv->perf.oa.ops.oa_hw_tail_read = + gen7_oa_hw_tail_read; + + dev_priv->perf.oa.oa_formats = hsw_oa_formats; + + dev_priv->perf.oa.n_builtin_sets = + i915_oa_n_builtin_metric_sets_hsw; + } else if (i915.enable_execlists) { + /* Note: that although we could theoretically also support the + * legacy ringbuffer mode on BDW (and earlier iterations of + * this driver, before upstreaming did this) it didn't seem + * worth the complexity to maintain now that BDW+ enable + * execlist mode by default. + */ - INIT_LIST_HEAD(&dev_priv->perf.streams); - mutex_init(&dev_priv->perf.lock); - spin_lock_init(&dev_priv->perf.hook_lock); - spin_lock_init(&dev_priv->perf.oa.oa_buffer.ptr_lock); + if (IS_GEN8(dev_priv)) { + dev_priv->perf.oa.ctx_oactxctrl_offset = 0x120; + dev_priv->perf.oa.ctx_flexeu0_offset = 0x2ce; + dev_priv->perf.oa.gen8_valid_ctx_bit = (1<<25); + + if (IS_BROADWELL(dev_priv)) { + dev_priv->perf.oa.n_builtin_sets = + i915_oa_n_builtin_metric_sets_bdw; + dev_priv->perf.oa.ops.select_metric_set = + i915_oa_select_metric_set_bdw; + } else if (IS_CHERRYVIEW(dev_priv)) { + dev_priv->perf.oa.n_builtin_sets = + i915_oa_n_builtin_metric_sets_chv; + dev_priv->perf.oa.ops.select_metric_set = + i915_oa_select_metric_set_chv; + } + } else if (IS_GEN9(dev_priv)) { + dev_priv->perf.oa.ctx_oactxctrl_offset = 0x128; + dev_priv->perf.oa.ctx_flexeu0_offset = 0x3de; + dev_priv->perf.oa.gen8_valid_ctx_bit = (1<<16); + + if (IS_SKL_GT2(dev_priv)) { + dev_priv->perf.oa.n_builtin_sets = + i915_oa_n_builtin_metric_sets_sklgt2; + dev_priv->perf.oa.ops.select_metric_set = + i915_oa_select_metric_set_sklgt2; + } else if (IS_SKL_GT3(dev_priv)) { + dev_priv->perf.oa.n_builtin_sets = + i915_oa_n_builtin_metric_sets_sklgt3; + dev_priv->perf.oa.ops.select_metric_set = + i915_oa_select_metric_set_sklgt3; + } else if (IS_SKL_GT4(dev_priv)) { + dev_priv->perf.oa.n_builtin_sets = + i915_oa_n_builtin_metric_sets_sklgt4; + dev_priv->perf.oa.ops.select_metric_set = + i915_oa_select_metric_set_sklgt4; + } else if (IS_BROXTON(dev_priv)) { + dev_priv->perf.oa.n_builtin_sets = + i915_oa_n_builtin_metric_sets_bxt; + dev_priv->perf.oa.ops.select_metric_set = + i915_oa_select_metric_set_bxt; + } + } - dev_priv->perf.oa.ops.init_oa_buffer = gen7_init_oa_buffer; - dev_priv->perf.oa.ops.enable_metric_set = hsw_enable_metric_set; - dev_priv->perf.oa.ops.disable_metric_set = hsw_disable_metric_set; - dev_priv->perf.oa.ops.oa_enable = gen7_oa_enable; - dev_priv->perf.oa.ops.oa_disable = gen7_oa_disable; - dev_priv->perf.oa.ops.read = gen7_oa_read; - dev_priv->perf.oa.ops.oa_buffer_check = - gen7_oa_buffer_check_unlocked; + if (dev_priv->perf.oa.n_builtin_sets) { + dev_priv->perf.oa.ops.init_oa_buffer = gen8_init_oa_buffer; + dev_priv->perf.oa.ops.enable_metric_set = + gen8_enable_metric_set; + dev_priv->perf.oa.ops.disable_metric_set = + gen8_disable_metric_set; + dev_priv->perf.oa.ops.oa_enable = gen8_oa_enable; + dev_priv->perf.oa.ops.oa_disable = gen8_oa_disable; + dev_priv->perf.oa.ops.read = gen8_oa_read; + dev_priv->perf.oa.ops.oa_hw_tail_read = + gen8_oa_hw_tail_read; + + dev_priv->perf.oa.oa_formats = gen8_plus_oa_formats; + } + } - dev_priv->perf.oa.oa_formats = hsw_oa_formats; + if (dev_priv->perf.oa.n_builtin_sets) { + hrtimer_init(&dev_priv->perf.oa.poll_check_timer, + CLOCK_MONOTONIC, HRTIMER_MODE_REL); + dev_priv->perf.oa.poll_check_timer.function = oa_poll_check_timer_cb; + init_waitqueue_head(&dev_priv->perf.oa.poll_wq); - dev_priv->perf.oa.n_builtin_sets = - i915_oa_n_builtin_metric_sets_hsw; + INIT_LIST_HEAD(&dev_priv->perf.streams); + mutex_init(&dev_priv->perf.lock); + spin_lock_init(&dev_priv->perf.hook_lock); + spin_lock_init(&dev_priv->perf.oa.oa_buffer.ptr_lock); - dev_priv->perf.sysctl_header = register_sysctl_table(dev_root); + dev_priv->perf.sysctl_header = register_sysctl_table(dev_root); - dev_priv->perf.initialized = true; + dev_priv->perf.initialized = true; + } } /** @@ -2217,5 +3104,6 @@ void i915_perf_fini(struct drm_i915_private *dev_priv) unregister_sysctl_table(dev_priv->perf.sysctl_header); memset(&dev_priv->perf.oa.ops, 0, sizeof(dev_priv->perf.oa.ops)); + dev_priv->perf.initialized = false; } diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 88e4707f571d..bd535f12db18 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -656,6 +656,12 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define GEN8_OACTXID _MMIO(0x2364) +#define GEN8_OA_DEBUG _MMIO(0x2B04) +#define GEN9_OA_DEBUG_DISABLE_CLK_RATIO_REPORTS (1<<5) +#define GEN9_OA_DEBUG_INCLUDE_CLK_RATIO (1<<6) +#define GEN9_OA_DEBUG_DISABLE_GO_1_0_REPORTS (1<<2) +#define GEN9_OA_DEBUG_DISABLE_CTX_SWITCH_REPORTS (1<<1) + #define GEN8_OACONTROL _MMIO(0x2B00) #define GEN8_OA_REPORT_FORMAT_A12 (0<<2) #define GEN8_OA_REPORT_FORMAT_A12_B8_C8 (2<<2) @@ -677,6 +683,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define GEN7_OABUFFER_STOP_RESUME_ENABLE (1<<1) #define GEN7_OABUFFER_RESUME (1<<0) +#define GEN8_OABUFFER_UDW _MMIO(0x23b4) #define GEN8_OABUFFER _MMIO(0x2b14) #define GEN7_OASTATUS1 _MMIO(0x2364) @@ -695,7 +702,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define GEN8_OASTATUS_REPORT_LOST (1<<0) #define GEN8_OAHEADPTR _MMIO(0x2B0C) +#define GEN8_OAHEADPTR_MASK 0xffffffc0 #define GEN8_OATAILPTR _MMIO(0x2B10) +#define GEN8_OATAILPTR_MASK 0xffffffc0 #define OABUFFER_SIZE_128K (0<<3) #define OABUFFER_SIZE_256K (1<<3) @@ -708,7 +717,17 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg) #define OA_MEM_SELECT_GGTT (1<<0) +/* + * Flexible, Aggregate EU Counter Registers. + * Note: these aren't contiguous + */ #define EU_PERF_CNTL0 _MMIO(0xe458) +#define EU_PERF_CNTL1 _MMIO(0xe558) +#define EU_PERF_CNTL2 _MMIO(0xe658) +#define EU_PERF_CNTL3 _MMIO(0xe758) +#define EU_PERF_CNTL4 _MMIO(0xe45c) +#define EU_PERF_CNTL5 _MMIO(0xe55c) +#define EU_PERF_CNTL6 _MMIO(0xe65c) #define GDT_CHICKEN_BITS _MMIO(0x9840) #define GT_NOA_ENABLE 0x00000080 @@ -2494,6 +2513,9 @@ enum skl_disp_power_wells { #define GEN8_RC_SEMA_IDLE_MSG_DISABLE (1 << 12) #define GEN8_FF_DOP_CLOCK_GATE_DISABLE (1<<10) +#define GEN6_RCS_PWR_FSM _MMIO(0x22ac) +#define GEN9_RCS_FE_FSM2 _MMIO(0x22a4) + /* Fuse readout registers for GT */ #define CHV_FUSE_GT _MMIO(VLV_DISPLAY_BASE + 0x2168) #define CHV_FGT_DISABLE_SS0 (1 << 10) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index d49dbaa931b5..7404cf2aac28 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1962,6 +1962,8 @@ static void execlists_init_reg_state(u32 *regs, regs[CTX_LRI_HEADER_2] = MI_LOAD_REGISTER_IMM(1); CTX_REG(regs, CTX_R_PWR_CLK_STATE, GEN8_R_PWR_CLK_STATE, make_rpcs(dev_priv)); + + i915_oa_init_reg_state(engine, ctx, regs); } } diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 464547d08173..15bc9f78ba4d 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -1316,13 +1316,18 @@ struct drm_i915_gem_context_param { }; enum drm_i915_oa_format { - I915_OA_FORMAT_A13 = 1, - I915_OA_FORMAT_A29, - I915_OA_FORMAT_A13_B8_C8, - I915_OA_FORMAT_B4_C8, - I915_OA_FORMAT_A45_B8_C8, - I915_OA_FORMAT_B4_C8_A16, - I915_OA_FORMAT_C4_B8, + I915_OA_FORMAT_A13 = 1, /* HSW only */ + I915_OA_FORMAT_A29, /* HSW only */ + I915_OA_FORMAT_A13_B8_C8, /* HSW only */ + I915_OA_FORMAT_B4_C8, /* HSW only */ + I915_OA_FORMAT_A45_B8_C8, /* HSW only */ + I915_OA_FORMAT_B4_C8_A16, /* HSW only */ + I915_OA_FORMAT_C4_B8, /* HSW+ */ + + /* Gen8+ */ + I915_OA_FORMAT_A12, + I915_OA_FORMAT_A12_B8_C8, + I915_OA_FORMAT_A32u40_A4u32_B8_C8, I915_OA_FORMAT_MAX /* non-ABI */ }; -- cgit v1.3-14-g43fede From 167b606aa262270ab6aeb5700adca6b1f33da26a Mon Sep 17 00:00:00 2001 From: Jordan Crouse Date: Mon, 8 May 2017 14:34:59 -0600 Subject: drm/msm: Remove DRM_MSM_NUM_IOCTLS The ioctl array is sparsely populated but the compiler will make sure that it is sufficiently sized for all the values that we have so we can safely use ARRAY_SIZE() instead of having a constantly changing #define in the uapi header. Signed-off-by: Jordan Crouse Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_drv.c | 2 +- include/uapi/drm/msm_drm.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 9d498eb81906..18d02ccb6c9b 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -842,7 +842,7 @@ static struct drm_driver msm_driver = { .debugfs_init = msm_debugfs_init, #endif .ioctls = msm_ioctls, - .num_ioctls = DRM_MSM_NUM_IOCTLS, + .num_ioctls = ARRAY_SIZE(msm_ioctls), .fops = &fops, .name = "msm", .desc = "MSM Snapdragon DRM", diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h index a4a189a240d7..a9985fe6efcd 100644 --- a/include/uapi/drm/msm_drm.h +++ b/include/uapi/drm/msm_drm.h @@ -261,7 +261,6 @@ struct drm_msm_gem_madvise { #define DRM_MSM_GEM_SUBMIT 0x06 #define DRM_MSM_WAIT_FENCE 0x07 #define DRM_MSM_GEM_MADVISE 0x08 -#define DRM_MSM_NUM_IOCTLS 0x09 #define DRM_IOCTL_MSM_GET_PARAM DRM_IOWR(DRM_COMMAND_BASE + DRM_MSM_GET_PARAM, struct drm_msm_param) #define DRM_IOCTL_MSM_GEM_NEW DRM_IOWR(DRM_COMMAND_BASE + DRM_MSM_GEM_NEW, struct drm_msm_gem_new) -- cgit v1.3-14-g43fede From 49fd08baa36ac10b13ea7b23fc6bbee8b4a6fcfe Mon Sep 17 00:00:00 2001 From: Jordan Crouse Date: Mon, 8 May 2017 14:35:01 -0600 Subject: drm/msm: Add hint to DRM_IOCTL_MSM_GEM_INFO to return an object IOVA Modify the 'pad' member of struct drm_msm_gem_info to 'flags'. If the user sets 'flags' to non-zero it means that they want a IOVA for the GEM object instead of a mmap() offset. Return the iova in the 'offset' member. Signed-off-by: Jordan Crouse [robclark: s/hint/flags in commit msg] Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_drv.c | 23 +++++++++++++++++++++-- include/uapi/drm/msm_drm.h | 8 ++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 18d02ccb6c9b..beb4f6b3ac70 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -699,6 +699,17 @@ static int msm_ioctl_gem_cpu_fini(struct drm_device *dev, void *data, return ret; } +static int msm_ioctl_gem_info_iova(struct drm_device *dev, + struct drm_gem_object *obj, uint64_t *iova) +{ + struct msm_drm_private *priv = dev->dev_private; + + if (!priv->gpu) + return -EINVAL; + + return msm_gem_get_iova(obj, priv->gpu->id, iova); +} + static int msm_ioctl_gem_info(struct drm_device *dev, void *data, struct drm_file *file) { @@ -706,14 +717,22 @@ static int msm_ioctl_gem_info(struct drm_device *dev, void *data, struct drm_gem_object *obj; int ret = 0; - if (args->pad) + if (args->flags & ~MSM_INFO_FLAGS) return -EINVAL; obj = drm_gem_object_lookup(file, args->handle); if (!obj) return -ENOENT; - args->offset = msm_gem_mmap_offset(obj); + if (args->flags & MSM_INFO_IOVA) { + uint64_t iova; + + ret = msm_ioctl_gem_info_iova(dev, obj, &iova); + if (!ret) + args->offset = iova; + } else { + args->offset = msm_gem_mmap_offset(obj); + } drm_gem_object_unreference_unlocked(obj); diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h index a9985fe6efcd..26c54f6d595d 100644 --- a/include/uapi/drm/msm_drm.h +++ b/include/uapi/drm/msm_drm.h @@ -104,10 +104,14 @@ struct drm_msm_gem_new { __u32 handle; /* out */ }; +#define MSM_INFO_IOVA 0x01 + +#define MSM_INFO_FLAGS (MSM_INFO_IOVA) + struct drm_msm_gem_info { __u32 handle; /* in */ - __u32 pad; - __u64 offset; /* out, offset to pass to mmap() */ + __u32 flags; /* in - combination of MSM_INFO_* flags */ + __u64 offset; /* out, mmap() offset or iova */ }; #define MSM_PREP_READ 0x01 -- cgit v1.3-14-g43fede From 1a71cf2fa646799d4397a49b223549d8617fece0 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 16 Jun 2017 15:05:23 +0100 Subject: drm/i915: Allow execbuffer to use the first object as the batch Currently, the last object in the execlist is the always the batch. However, when building the batch buffer we often know the batch object first and if we can use the first slot in the execlist we can emit relocation instructions relative to it immediately and avoid a separate pass to adjust the relocations to point to the last execlist slot. Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen --- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 ++++- include/uapi/drm/i915_drm.h | 19 +++++++++++++++++-- 3 files changed, 22 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index e33a2ed9244c..3c2af70034cf 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -367,6 +367,7 @@ static int i915_getparam(struct drm_device *dev, void *data, case I915_PARAM_HAS_EXEC_ASYNC: case I915_PARAM_HAS_EXEC_FENCE: case I915_PARAM_HAS_EXEC_CAPTURE: + case I915_PARAM_HAS_EXEC_BATCH_FIRST: /* For the time being all of these are always true; * if some supported hardware does not have one of these * features this value needs to be provided from diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index f4b02ef3987f..e262133a7cf5 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -645,7 +645,10 @@ ht_needs_resize(const struct i915_gem_context_vma_lut *lut) static unsigned int eb_batch_index(const struct i915_execbuffer *eb) { - return eb->buffer_count - 1; + if (eb->args->flags & I915_EXEC_BATCH_FIRST) + return 0; + else + return eb->buffer_count - 1; } static int eb_select_context(struct i915_execbuffer *eb) diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 15bc9f78ba4d..7ccbd6a2bbe0 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -418,7 +418,6 @@ typedef struct drm_i915_irq_wait { */ #define I915_PARAM_HAS_EXEC_CAPTURE 45 -/* Query the mask of slices available for this system */ #define I915_PARAM_SLICE_MASK 46 /* Assuming it's uniform for each slice, this queries the mask of subslices @@ -426,6 +425,12 @@ typedef struct drm_i915_irq_wait { */ #define I915_PARAM_SUBSLICE_MASK 47 +/* + * Query whether DRM_I915_GEM_EXECBUFFER2 supports supplying the batch buffer + * as the first execobject as opposed to the last. See I915_EXEC_BATCH_FIRST. + */ +#define I915_PARAM_HAS_EXEC_BATCH_FIRST 48 + typedef struct drm_i915_getparam { __s32 param; /* @@ -912,7 +917,17 @@ struct drm_i915_gem_execbuffer2 { */ #define I915_EXEC_FENCE_OUT (1<<17) -#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_OUT<<1)) +/* + * Traditionally the execbuf ioctl has only considered the final element in + * the execobject[] to be the executable batch. Often though, the client + * will known the batch object prior to construction and being able to place + * it into the execobject[] array first can simplify the relocation tracking. + * Setting I915_EXEC_BATCH_FIRST tells execbuf to use element 0 of the + * execobject[] as the * batch instead (the default is to use the last + * element). + */ +#define I915_EXEC_BATCH_FIRST (1<<18) +#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_BATCH_FIRST<<1)) #define I915_EXEC_CONTEXT_ID_MASK (0xffffffff) #define i915_execbuffer2_set_context_id(eb2, context) \ -- cgit v1.3-14-g43fede From ceea5e3771ed2378668455fa21861bead7504df5 Mon Sep 17 00:00:00 2001 From: John Stultz Date: Thu, 8 Jun 2017 16:44:20 -0700 Subject: time: Fix clock->read(clock) race around clocksource changes In tests, which excercise switching of clocksources, a NULL pointer dereference can be observed on AMR64 platforms in the clocksource read() function: u64 clocksource_mmio_readl_down(struct clocksource *c) { return ~(u64)readl_relaxed(to_mmio_clksrc(c)->reg) & c->mask; } This is called from the core timekeeping code via: cycle_now = tkr->read(tkr->clock); tkr->read is the cached tkr->clock->read() function pointer. When the clocksource is changed then tkr->clock and tkr->read are updated sequentially. The code above results in a sequential load operation of tkr->read and tkr->clock as well. If the store to tkr->clock hits between the loads of tkr->read and tkr->clock, then the old read() function is called with the new clock pointer. As a consequence the read() function dereferences a different data structure and the resulting 'reg' pointer can point anywhere including NULL. This problem was introduced when the timekeeping code was switched over to use struct tk_read_base. Before that, it was theoretically possible as well when the compiler decided to reload clock in the code sequence: now = tk->clock->read(tk->clock); Add a helper function which avoids the issue by reading tk_read_base->clock once into a local variable clk and then issue the read function via clk->read(clk). This guarantees that the read() function always gets the proper clocksource pointer handed in. Since there is now no use for the tkr.read pointer, this patch also removes it, and to address stopping the fast timekeeper during suspend/resume, it introduces a dummy clocksource to use rather then just a dummy read function. Signed-off-by: John Stultz Acked-by: Ingo Molnar Cc: Prarit Bhargava Cc: Richard Cochran Cc: Stephen Boyd Cc: stable Cc: Miroslav Lichvar Cc: Daniel Mentz Link: http://lkml.kernel.org/r/1496965462-20003-2-git-send-email-john.stultz@linaro.org Signed-off-by: Thomas Gleixner --- include/linux/timekeeper_internal.h | 1 - kernel/time/timekeeping.c | 52 +++++++++++++++++++++++++------------ 2 files changed, 36 insertions(+), 17 deletions(-) (limited to 'include') diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h index 110f4532188c..e9834ada4d0c 100644 --- a/include/linux/timekeeper_internal.h +++ b/include/linux/timekeeper_internal.h @@ -29,7 +29,6 @@ */ struct tk_read_base { struct clocksource *clock; - u64 (*read)(struct clocksource *cs); u64 mask; u64 cycle_last; u32 mult; diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 9652bc57fd09..eff94cb8e89e 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -118,6 +118,26 @@ static inline void tk_update_sleep_time(struct timekeeper *tk, ktime_t delta) tk->offs_boot = ktime_add(tk->offs_boot, delta); } +/* + * tk_clock_read - atomic clocksource read() helper + * + * This helper is necessary to use in the read paths because, while the + * seqlock ensures we don't return a bad value while structures are updated, + * it doesn't protect from potential crashes. There is the possibility that + * the tkr's clocksource may change between the read reference, and the + * clock reference passed to the read function. This can cause crashes if + * the wrong clocksource is passed to the wrong read function. + * This isn't necessary to use when holding the timekeeper_lock or doing + * a read of the fast-timekeeper tkrs (which is protected by its own locking + * and update logic). + */ +static inline u64 tk_clock_read(struct tk_read_base *tkr) +{ + struct clocksource *clock = READ_ONCE(tkr->clock); + + return clock->read(clock); +} + #ifdef CONFIG_DEBUG_TIMEKEEPING #define WARNING_FREQ (HZ*300) /* 5 minute rate-limiting */ @@ -175,7 +195,7 @@ static inline u64 timekeeping_get_delta(struct tk_read_base *tkr) */ do { seq = read_seqcount_begin(&tk_core.seq); - now = tkr->read(tkr->clock); + now = tk_clock_read(tkr); last = tkr->cycle_last; mask = tkr->mask; max = tkr->clock->max_cycles; @@ -209,7 +229,7 @@ static inline u64 timekeeping_get_delta(struct tk_read_base *tkr) u64 cycle_now, delta; /* read clocksource */ - cycle_now = tkr->read(tkr->clock); + cycle_now = tk_clock_read(tkr); /* calculate the delta since the last update_wall_time */ delta = clocksource_delta(cycle_now, tkr->cycle_last, tkr->mask); @@ -238,12 +258,10 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock) ++tk->cs_was_changed_seq; old_clock = tk->tkr_mono.clock; tk->tkr_mono.clock = clock; - tk->tkr_mono.read = clock->read; tk->tkr_mono.mask = clock->mask; - tk->tkr_mono.cycle_last = tk->tkr_mono.read(clock); + tk->tkr_mono.cycle_last = tk_clock_read(&tk->tkr_mono); tk->tkr_raw.clock = clock; - tk->tkr_raw.read = clock->read; tk->tkr_raw.mask = clock->mask; tk->tkr_raw.cycle_last = tk->tkr_mono.cycle_last; @@ -404,7 +422,7 @@ static __always_inline u64 __ktime_get_fast_ns(struct tk_fast *tkf) now += timekeeping_delta_to_ns(tkr, clocksource_delta( - tkr->read(tkr->clock), + tk_clock_read(tkr), tkr->cycle_last, tkr->mask)); } while (read_seqcount_retry(&tkf->seq, seq)); @@ -461,6 +479,10 @@ static u64 dummy_clock_read(struct clocksource *cs) return cycles_at_suspend; } +static struct clocksource dummy_clock = { + .read = dummy_clock_read, +}; + /** * halt_fast_timekeeper - Prevent fast timekeeper from accessing clocksource. * @tk: Timekeeper to snapshot. @@ -477,13 +499,13 @@ static void halt_fast_timekeeper(struct timekeeper *tk) struct tk_read_base *tkr = &tk->tkr_mono; memcpy(&tkr_dummy, tkr, sizeof(tkr_dummy)); - cycles_at_suspend = tkr->read(tkr->clock); - tkr_dummy.read = dummy_clock_read; + cycles_at_suspend = tk_clock_read(tkr); + tkr_dummy.clock = &dummy_clock; update_fast_timekeeper(&tkr_dummy, &tk_fast_mono); tkr = &tk->tkr_raw; memcpy(&tkr_dummy, tkr, sizeof(tkr_dummy)); - tkr_dummy.read = dummy_clock_read; + tkr_dummy.clock = &dummy_clock; update_fast_timekeeper(&tkr_dummy, &tk_fast_raw); } @@ -649,11 +671,10 @@ static void timekeeping_update(struct timekeeper *tk, unsigned int action) */ static void timekeeping_forward_now(struct timekeeper *tk) { - struct clocksource *clock = tk->tkr_mono.clock; u64 cycle_now, delta; u64 nsec; - cycle_now = tk->tkr_mono.read(clock); + cycle_now = tk_clock_read(&tk->tkr_mono); delta = clocksource_delta(cycle_now, tk->tkr_mono.cycle_last, tk->tkr_mono.mask); tk->tkr_mono.cycle_last = cycle_now; tk->tkr_raw.cycle_last = cycle_now; @@ -929,8 +950,7 @@ void ktime_get_snapshot(struct system_time_snapshot *systime_snapshot) do { seq = read_seqcount_begin(&tk_core.seq); - - now = tk->tkr_mono.read(tk->tkr_mono.clock); + now = tk_clock_read(&tk->tkr_mono); systime_snapshot->cs_was_changed_seq = tk->cs_was_changed_seq; systime_snapshot->clock_was_set_seq = tk->clock_was_set_seq; base_real = ktime_add(tk->tkr_mono.base, @@ -1108,7 +1128,7 @@ int get_device_system_crosststamp(int (*get_time_fn) * Check whether the system counter value provided by the * device driver is on the current timekeeping interval. */ - now = tk->tkr_mono.read(tk->tkr_mono.clock); + now = tk_clock_read(&tk->tkr_mono); interval_start = tk->tkr_mono.cycle_last; if (!cycle_between(interval_start, cycles, now)) { clock_was_set_seq = tk->clock_was_set_seq; @@ -1629,7 +1649,7 @@ void timekeeping_resume(void) * The less preferred source will only be tried if there is no better * usable source. The rtc part is handled separately in rtc core code. */ - cycle_now = tk->tkr_mono.read(clock); + cycle_now = tk_clock_read(&tk->tkr_mono); if ((clock->flags & CLOCK_SOURCE_SUSPEND_NONSTOP) && cycle_now > tk->tkr_mono.cycle_last) { u64 nsec, cyc_delta; @@ -2030,7 +2050,7 @@ void update_wall_time(void) #ifdef CONFIG_ARCH_USES_GETTIMEOFFSET offset = real_tk->cycle_interval; #else - offset = clocksource_delta(tk->tkr_mono.read(tk->tkr_mono.clock), + offset = clocksource_delta(tk_clock_read(&tk->tkr_mono), tk->tkr_mono.cycle_last, tk->tkr_mono.mask); #endif -- cgit v1.3-14-g43fede From 3d88d56c5873f6eebe23e05c3da701960146b801 Mon Sep 17 00:00:00 2001 From: John Stultz Date: Thu, 8 Jun 2017 16:44:21 -0700 Subject: time: Fix CLOCK_MONOTONIC_RAW sub-nanosecond accounting Due to how the MONOTONIC_RAW accumulation logic was handled, there is the potential for a 1ns discontinuity when we do accumulations. This small discontinuity has for the most part gone un-noticed, but since ARM64 enabled CLOCK_MONOTONIC_RAW in their vDSO clock_gettime implementation, we've seen failures with the inconsistency-check test in kselftest. This patch addresses the issue by using the same sub-ns accumulation handling that CLOCK_MONOTONIC uses, which avoids the issue for in-kernel users. Since the ARM64 vDSO implementation has its own clock_gettime calculation logic, this patch reduces the frequency of errors, but failures are still seen. The ARM64 vDSO will need to be updated to include the sub-nanosecond xtime_nsec values in its calculation for this issue to be completely fixed. Signed-off-by: John Stultz Tested-by: Daniel Mentz Cc: Prarit Bhargava Cc: Kevin Brodsky Cc: Richard Cochran Cc: Stephen Boyd Cc: Will Deacon Cc: "stable #4 . 8+" Cc: Miroslav Lichvar Link: http://lkml.kernel.org/r/1496965462-20003-3-git-send-email-john.stultz@linaro.org Signed-off-by: Thomas Gleixner --- include/linux/timekeeper_internal.h | 4 ++-- kernel/time/timekeeping.c | 19 ++++++++++--------- 2 files changed, 12 insertions(+), 11 deletions(-) (limited to 'include') diff --git a/include/linux/timekeeper_internal.h b/include/linux/timekeeper_internal.h index e9834ada4d0c..f7043ccca81c 100644 --- a/include/linux/timekeeper_internal.h +++ b/include/linux/timekeeper_internal.h @@ -57,7 +57,7 @@ struct tk_read_base { * interval. * @xtime_remainder: Shifted nano seconds left over when rounding * @cycle_interval - * @raw_interval: Raw nano seconds accumulated per NTP interval. + * @raw_interval: Shifted raw nano seconds accumulated per NTP interval. * @ntp_error: Difference between accumulated time and NTP time in ntp * shifted nano seconds. * @ntp_error_shift: Shift conversion between clock shifted nano seconds and @@ -99,7 +99,7 @@ struct timekeeper { u64 cycle_interval; u64 xtime_interval; s64 xtime_remainder; - u32 raw_interval; + u64 raw_interval; /* The ntp_tick_length() value currently being used. * This cached copy ensures we consistently apply the tick * length for an entire tick, as ntp_tick_length may change diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index eff94cb8e89e..b602c48cb841 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -280,7 +280,7 @@ static void tk_setup_internals(struct timekeeper *tk, struct clocksource *clock) /* Go back from cycles -> shifted ns */ tk->xtime_interval = interval * clock->mult; tk->xtime_remainder = ntpinterval - tk->xtime_interval; - tk->raw_interval = (interval * clock->mult) >> clock->shift; + tk->raw_interval = interval * clock->mult; /* if changing clocks, convert xtime_nsec shift units */ if (old_clock) { @@ -1996,7 +1996,7 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset, u32 shift, unsigned int *clock_set) { u64 interval = tk->cycle_interval << shift; - u64 raw_nsecs; + u64 snsec_per_sec; /* If the offset is smaller than a shifted interval, do nothing */ if (offset < interval) @@ -2011,14 +2011,15 @@ static u64 logarithmic_accumulation(struct timekeeper *tk, u64 offset, *clock_set |= accumulate_nsecs_to_secs(tk); /* Accumulate raw time */ - raw_nsecs = (u64)tk->raw_interval << shift; - raw_nsecs += tk->raw_time.tv_nsec; - if (raw_nsecs >= NSEC_PER_SEC) { - u64 raw_secs = raw_nsecs; - raw_nsecs = do_div(raw_secs, NSEC_PER_SEC); - tk->raw_time.tv_sec += raw_secs; + tk->tkr_raw.xtime_nsec += (u64)tk->raw_time.tv_nsec << tk->tkr_raw.shift; + tk->tkr_raw.xtime_nsec += tk->raw_interval << shift; + snsec_per_sec = (u64)NSEC_PER_SEC << tk->tkr_raw.shift; + while (tk->tkr_raw.xtime_nsec >= snsec_per_sec) { + tk->tkr_raw.xtime_nsec -= snsec_per_sec; + tk->raw_time.tv_sec++; } - tk->raw_time.tv_nsec = raw_nsecs; + tk->raw_time.tv_nsec = tk->tkr_raw.xtime_nsec >> tk->tkr_raw.shift; + tk->tkr_raw.xtime_nsec -= (u64)tk->raw_time.tv_nsec << tk->tkr_raw.shift; /* Accumulate error between NTP and clock interval */ tk->ntp_error += tk->ntp_tick << shift; -- cgit v1.3-14-g43fede From 8e8320c9315c47a6a090188720ccff32a6a6ba18 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Tue, 20 Jun 2017 17:56:13 -0600 Subject: blk-mq: fix performance regression with shared tags If we have shared tags enabled, then every IO completion will trigger a full loop of every queue belonging to a tag set, and every hardware queue for each of those queues, even if nothing needs to be done. This causes a massive performance regression if you have a lot of shared devices. Instead of doing this huge full scan on every IO, add an atomic counter to the main queue that tracks how many hardware queues have been marked as needing a restart. With that, we can avoid looking for restartable queues, if we don't have to. Max reports that this restores performance. Before this patch, 4K IOPS was limited to 22-23K IOPS. With the patch, we are running at 950-970K IOPS. Fixes: 6d8c6c0f97ad ("blk-mq: Restart a single queue if tag sets are shared") Reported-by: Max Gurtovoy Tested-by: Max Gurtovoy Reviewed-by: Bart Van Assche Tested-by: Bart Van Assche Signed-off-by: Jens Axboe --- block/blk-mq-sched.c | 58 +++++++++++++++++++++++++++++++++++++++----------- block/blk-mq-sched.h | 9 -------- block/blk-mq.c | 16 +++++++++++--- include/linux/blkdev.h | 2 ++ 4 files changed, 61 insertions(+), 24 deletions(-) (limited to 'include') diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c index 1f5b692526ae..0ded5e846335 100644 --- a/block/blk-mq-sched.c +++ b/block/blk-mq-sched.c @@ -68,6 +68,45 @@ static void blk_mq_sched_assign_ioc(struct request_queue *q, __blk_mq_sched_assign_ioc(q, rq, bio, ioc); } +/* + * Mark a hardware queue as needing a restart. For shared queues, maintain + * a count of how many hardware queues are marked for restart. + */ +static void blk_mq_sched_mark_restart_hctx(struct blk_mq_hw_ctx *hctx) +{ + if (test_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state)) + return; + + if (hctx->flags & BLK_MQ_F_TAG_SHARED) { + struct request_queue *q = hctx->queue; + + if (!test_and_set_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state)) + atomic_inc(&q->shared_hctx_restart); + } else + set_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state); +} + +static bool blk_mq_sched_restart_hctx(struct blk_mq_hw_ctx *hctx) +{ + if (!test_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state)) + return false; + + if (hctx->flags & BLK_MQ_F_TAG_SHARED) { + struct request_queue *q = hctx->queue; + + if (test_and_clear_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state)) + atomic_dec(&q->shared_hctx_restart); + } else + clear_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state); + + if (blk_mq_hctx_has_pending(hctx)) { + blk_mq_run_hw_queue(hctx, true); + return true; + } + + return false; +} + struct request *blk_mq_sched_get_request(struct request_queue *q, struct bio *bio, unsigned int op, @@ -266,18 +305,6 @@ static bool blk_mq_sched_bypass_insert(struct blk_mq_hw_ctx *hctx, return true; } -static bool blk_mq_sched_restart_hctx(struct blk_mq_hw_ctx *hctx) -{ - if (test_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state)) { - clear_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state); - if (blk_mq_hctx_has_pending(hctx)) { - blk_mq_run_hw_queue(hctx, true); - return true; - } - } - return false; -} - /** * list_for_each_entry_rcu_rr - iterate in a round-robin fashion over rcu list * @pos: loop cursor. @@ -309,6 +336,13 @@ void blk_mq_sched_restart(struct blk_mq_hw_ctx *const hctx) unsigned int i, j; if (set->flags & BLK_MQ_F_TAG_SHARED) { + /* + * If this is 0, then we know that no hardware queues + * have RESTART marked. We're done. + */ + if (!atomic_read(&queue->shared_hctx_restart)) + return; + rcu_read_lock(); list_for_each_entry_rcu_rr(q, queue, &set->tag_list, tag_set_list) { diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h index edafb5383b7b..5007edece51a 100644 --- a/block/blk-mq-sched.h +++ b/block/blk-mq-sched.h @@ -115,15 +115,6 @@ static inline bool blk_mq_sched_has_work(struct blk_mq_hw_ctx *hctx) return false; } -/* - * Mark a hardware queue as needing a restart. - */ -static inline void blk_mq_sched_mark_restart_hctx(struct blk_mq_hw_ctx *hctx) -{ - if (!test_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state)) - set_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state); -} - static inline bool blk_mq_sched_needs_restart(struct blk_mq_hw_ctx *hctx) { return test_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state); diff --git a/block/blk-mq.c b/block/blk-mq.c index bb66c96850b1..958cedaff8b8 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -2103,20 +2103,30 @@ static void blk_mq_map_swqueue(struct request_queue *q, } } +/* + * Caller needs to ensure that we're either frozen/quiesced, or that + * the queue isn't live yet. + */ static void queue_set_hctx_shared(struct request_queue *q, bool shared) { struct blk_mq_hw_ctx *hctx; int i; queue_for_each_hw_ctx(q, hctx, i) { - if (shared) + if (shared) { + if (test_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state)) + atomic_inc(&q->shared_hctx_restart); hctx->flags |= BLK_MQ_F_TAG_SHARED; - else + } else { + if (test_bit(BLK_MQ_S_SCHED_RESTART, &hctx->state)) + atomic_dec(&q->shared_hctx_restart); hctx->flags &= ~BLK_MQ_F_TAG_SHARED; + } } } -static void blk_mq_update_tag_set_depth(struct blk_mq_tag_set *set, bool shared) +static void blk_mq_update_tag_set_depth(struct blk_mq_tag_set *set, + bool shared) { struct request_queue *q; diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index b74a3edcb3da..1ddd36bd2173 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -391,6 +391,8 @@ struct request_queue { int nr_rqs[2]; /* # allocated [a]sync rqs */ int nr_rqs_elvpriv; /* # allocated rqs w/ elvpriv */ + atomic_t shared_hctx_restart; + struct blk_queue_stats *stats; struct rq_wb *rq_wb; -- cgit v1.3-14-g43fede From e4330d8bf669139a983255d1801733b64c2ae841 Mon Sep 17 00:00:00 2001 From: Jarkko Nikula Date: Mon, 19 Jun 2017 15:53:01 +0300 Subject: ACPI / scan: Fix enumeration for special SPI and I2C devices Commit f406270bf73d ("ACPI / scan: Set the visited flag for all enumerated devices") caused that two group of special SPI or I2C devices do not enumerate. SPI and I2C devices are expected to be enumerated by the SPI and I2C subsystems but change caused that acpi_bus_attach() marks those devices with acpi_device_set_enumerated(). First group of devices are matched using Device Tree compatible property with special _HID "PRP0001". Those devices have matched scan handler, acpi_scan_attach_handler() retuns 1 and acpi_bus_attach() marks them with acpi_device_set_enumerated(). Second group of devices without valid _HID such as "LNXVIDEO" have device->pnp.type.platform_id set to zero and change again marks them with acpi_device_set_enumerated(). Fix this by flagging the SPI and I2C devices during struct acpi_device object initialization time and let the code in acpi_bus_attach() to go through the device_attach() and acpi_default_enumeration() path for all SPI and I2C devices. Fixes: f406270bf73d (ACPI / scan: Set the visited flag for all enumerated devices) Signed-off-by: Jarkko Nikula Acked-by: Mika Westerberg Cc: 4.11+ # 4.11+ Signed-off-by: Rafael J. Wysocki --- drivers/acpi/scan.c | 67 +++++++++++++++++++++++++++---------------------- include/acpi/acpi_bus.h | 3 ++- 2 files changed, 39 insertions(+), 31 deletions(-) (limited to 'include') diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 3a10d7573477..d53162997f32 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1428,6 +1428,37 @@ static void acpi_init_coherency(struct acpi_device *adev) adev->flags.coherent_dma = cca; } +static int acpi_check_spi_i2c_slave(struct acpi_resource *ares, void *data) +{ + bool *is_spi_i2c_slave_p = data; + + if (ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS) + return 1; + + /* + * devices that are connected to UART still need to be enumerated to + * platform bus + */ + if (ares->data.common_serial_bus.type != ACPI_RESOURCE_SERIAL_TYPE_UART) + *is_spi_i2c_slave_p = true; + + /* no need to do more checking */ + return -1; +} + +static bool acpi_is_spi_i2c_slave(struct acpi_device *device) +{ + struct list_head resource_list; + bool is_spi_i2c_slave = false; + + INIT_LIST_HEAD(&resource_list); + acpi_dev_get_resources(device, &resource_list, acpi_check_spi_i2c_slave, + &is_spi_i2c_slave); + acpi_dev_free_resource_list(&resource_list); + + return is_spi_i2c_slave; +} + void acpi_init_device_object(struct acpi_device *device, acpi_handle handle, int type, unsigned long long sta) { @@ -1443,6 +1474,7 @@ void acpi_init_device_object(struct acpi_device *device, acpi_handle handle, acpi_bus_get_flags(device); device->flags.match_driver = false; device->flags.initialized = true; + device->flags.spi_i2c_slave = acpi_is_spi_i2c_slave(device); acpi_device_clear_enumerated(device); device_initialize(&device->dev); dev_set_uevent_suppress(&device->dev, true); @@ -1727,38 +1759,13 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, u32 lvl_not_used, return AE_OK; } -static int acpi_check_spi_i2c_slave(struct acpi_resource *ares, void *data) -{ - bool *is_spi_i2c_slave_p = data; - - if (ares->type != ACPI_RESOURCE_TYPE_SERIAL_BUS) - return 1; - - /* - * devices that are connected to UART still need to be enumerated to - * platform bus - */ - if (ares->data.common_serial_bus.type != ACPI_RESOURCE_SERIAL_TYPE_UART) - *is_spi_i2c_slave_p = true; - - /* no need to do more checking */ - return -1; -} - static void acpi_default_enumeration(struct acpi_device *device) { - struct list_head resource_list; - bool is_spi_i2c_slave = false; - /* * Do not enumerate SPI/I2C slaves as they will be enumerated by their * respective parents. */ - INIT_LIST_HEAD(&resource_list); - acpi_dev_get_resources(device, &resource_list, acpi_check_spi_i2c_slave, - &is_spi_i2c_slave); - acpi_dev_free_resource_list(&resource_list); - if (!is_spi_i2c_slave) { + if (!device->flags.spi_i2c_slave) { acpi_create_platform_device(device, NULL); acpi_device_set_enumerated(device); } else { @@ -1854,7 +1861,7 @@ static void acpi_bus_attach(struct acpi_device *device) return; device->flags.match_driver = true; - if (ret > 0) { + if (ret > 0 && !device->flags.spi_i2c_slave) { acpi_device_set_enumerated(device); goto ok; } @@ -1863,10 +1870,10 @@ static void acpi_bus_attach(struct acpi_device *device) if (ret < 0) return; - if (device->pnp.type.platform_id) - acpi_default_enumeration(device); - else + if (!device->pnp.type.platform_id && !device->flags.spi_i2c_slave) acpi_device_set_enumerated(device); + else + acpi_default_enumeration(device); ok: list_for_each_entry(child, &device->children, node) diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 197f3fffc9a7..408c7820e200 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -210,7 +210,8 @@ struct acpi_device_flags { u32 of_compatible_ok:1; u32 coherent_dma:1; u32 cca_seen:1; - u32 reserved:20; + u32 spi_i2c_slave:1; + u32 reserved:19; }; /* File System */ -- cgit v1.3-14-g43fede From 3b7b314053d021601940c50b07f5f1423ae67e21 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Fri, 23 Jun 2017 15:08:52 -0700 Subject: slub: make sysfs file removal asynchronous Commit bf5eb3de3847 ("slub: separate out sysfs_slab_release() from sysfs_slab_remove()") made slub sysfs file removals synchronous to kmem_cache shutdown. Unfortunately, this created a possible ABBA deadlock between slab_mutex and sysfs draining mechanism triggering the following lockdep warning. ====================================================== [ INFO: possible circular locking dependency detected ] 4.10.0-test+ #48 Not tainted ------------------------------------------------------- rmmod/1211 is trying to acquire lock: (s_active#120){++++.+}, at: [] kernfs_remove+0x23/0x40 but task is already holding lock: (slab_mutex){+.+.+.}, at: [] kmem_cache_destroy+0x41/0x2d0 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (slab_mutex){+.+.+.}: lock_acquire+0xf6/0x1f0 __mutex_lock+0x75/0x950 mutex_lock_nested+0x1b/0x20 slab_attr_store+0x75/0xd0 sysfs_kf_write+0x45/0x60 kernfs_fop_write+0x13c/0x1c0 __vfs_write+0x28/0x120 vfs_write+0xc8/0x1e0 SyS_write+0x49/0xa0 entry_SYSCALL_64_fastpath+0x1f/0xc2 -> #0 (s_active#120){++++.+}: __lock_acquire+0x10ed/0x1260 lock_acquire+0xf6/0x1f0 __kernfs_remove+0x254/0x320 kernfs_remove+0x23/0x40 sysfs_remove_dir+0x51/0x80 kobject_del+0x18/0x50 __kmem_cache_shutdown+0x3e6/0x460 kmem_cache_destroy+0x1fb/0x2d0 kvm_exit+0x2d/0x80 [kvm] vmx_exit+0x19/0xa1b [kvm_intel] SyS_delete_module+0x198/0x1f0 entry_SYSCALL_64_fastpath+0x1f/0xc2 other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(slab_mutex); lock(s_active#120); lock(slab_mutex); lock(s_active#120); *** DEADLOCK *** 2 locks held by rmmod/1211: #0: (cpu_hotplug.dep_map){++++++}, at: [] get_online_cpus+0x37/0x80 #1: (slab_mutex){+.+.+.}, at: [] kmem_cache_destroy+0x41/0x2d0 stack backtrace: CPU: 3 PID: 1211 Comm: rmmod Not tainted 4.10.0-test+ #48 Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v02.05 05/07/2012 Call Trace: print_circular_bug+0x1be/0x210 __lock_acquire+0x10ed/0x1260 lock_acquire+0xf6/0x1f0 __kernfs_remove+0x254/0x320 kernfs_remove+0x23/0x40 sysfs_remove_dir+0x51/0x80 kobject_del+0x18/0x50 __kmem_cache_shutdown+0x3e6/0x460 kmem_cache_destroy+0x1fb/0x2d0 kvm_exit+0x2d/0x80 [kvm] vmx_exit+0x19/0xa1b [kvm_intel] SyS_delete_module+0x198/0x1f0 ? SyS_delete_module+0x5/0x1f0 entry_SYSCALL_64_fastpath+0x1f/0xc2 It'd be the cleanest to deal with the issue by removing sysfs files without holding slab_mutex before the rest of shutdown; however, given the current code structure, it is pretty difficult to do so. This patch punts sysfs file removal to a work item. Before commit bf5eb3de3847, the removal was punted to a RCU delayed work item which is executed after release. Now, we're punting to a different work item on shutdown which still maintains the goal removing the sysfs files earlier when destroying kmem_caches. Link: http://lkml.kernel.org/r/20170620204512.GI21326@htj.duckdns.org Fixes: bf5eb3de3847 ("slub: separate out sysfs_slab_release() from sysfs_slab_remove()") Signed-off-by: Tejun Heo Reported-by: Steven Rostedt (VMware) Tested-by: Steven Rostedt (VMware) Cc: Christoph Lameter Cc: Pekka Enberg Cc: David Rientjes Cc: Joonsoo Kim Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- include/linux/slub_def.h | 1 + mm/slub.c | 40 ++++++++++++++++++++++++++-------------- 2 files changed, 27 insertions(+), 14 deletions(-) (limited to 'include') diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h index 07ef550c6627..93315d6b21a8 100644 --- a/include/linux/slub_def.h +++ b/include/linux/slub_def.h @@ -84,6 +84,7 @@ struct kmem_cache { int red_left_pad; /* Left redzone padding size */ #ifdef CONFIG_SYSFS struct kobject kobj; /* For sysfs */ + struct work_struct kobj_remove_work; #endif #ifdef CONFIG_MEMCG struct memcg_cache_params memcg_params; diff --git a/mm/slub.c b/mm/slub.c index 7449593fca72..8addc535bcdc 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -5625,6 +5625,28 @@ static char *create_unique_id(struct kmem_cache *s) return name; } +static void sysfs_slab_remove_workfn(struct work_struct *work) +{ + struct kmem_cache *s = + container_of(work, struct kmem_cache, kobj_remove_work); + + if (!s->kobj.state_in_sysfs) + /* + * For a memcg cache, this may be called during + * deactivation and again on shutdown. Remove only once. + * A cache is never shut down before deactivation is + * complete, so no need to worry about synchronization. + */ + return; + +#ifdef CONFIG_MEMCG + kset_unregister(s->memcg_kset); +#endif + kobject_uevent(&s->kobj, KOBJ_REMOVE); + kobject_del(&s->kobj); + kobject_put(&s->kobj); +} + static int sysfs_slab_add(struct kmem_cache *s) { int err; @@ -5632,6 +5654,8 @@ static int sysfs_slab_add(struct kmem_cache *s) struct kset *kset = cache_kset(s); int unmergeable = slab_unmergeable(s); + INIT_WORK(&s->kobj_remove_work, sysfs_slab_remove_workfn); + if (!kset) { kobject_init(&s->kobj, &slab_ktype); return 0; @@ -5695,20 +5719,8 @@ static void sysfs_slab_remove(struct kmem_cache *s) */ return; - if (!s->kobj.state_in_sysfs) - /* - * For a memcg cache, this may be called during - * deactivation and again on shutdown. Remove only once. - * A cache is never shut down before deactivation is - * complete, so no need to worry about synchronization. - */ - return; - -#ifdef CONFIG_MEMCG - kset_unregister(s->memcg_kset); -#endif - kobject_uevent(&s->kobj, KOBJ_REMOVE); - kobject_del(&s->kobj); + kobject_get(&s->kobj); + schedule_work(&s->kobj_remove_work); } void sysfs_slab_release(struct kmem_cache *s) -- cgit v1.3-14-g43fede