This is John/Rodrigo's 2 patches with some minor changes, and I added 2 patches.
"drm/i915/uapi: Add query for hwconfig blob" was changed:
* Rename DRM_I915_QUERY_HWCONFIG_TABLE to DRM_I915_QUERY_HWCONFIG_BLOB as requested by Joonas.
* Reword commit message
* I added Acked-by to this patch, but this Acked-by only applies in the context of this version of the patchset.
"drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item" adds a struct to the uAPI and documents the return value for DRM_I915_QUERY_HWCONFIG_BLOB. (Except, keys / values are still deferred to the PRM.)
"drm/i915/guc: Verify hwconfig blob matches supported format" does the simple verification of the blob to make sure it matches what the uAPI documents.
v2: * Fix -Werror errors. * Rebase to drm-intel/for-linux-next instead of drm-intel/for-linux-next-gt, as this seems to be what CI wants. * Fix u32 -> __u32. * Add commit message for "Verify hwconfig blob" patch as requested by Tvrtko. * Reword text added to i915_drm.h as requested by Tvrtko. (Attempting to indicate the overall blob ends right at the last blob item.)
v3: * Add several changes suggested by Tvrtko in the "Verify hwconfig blob", along with some tweaks to i915_drm.h from the feedback for the same patch.
v4: * Rewrite verify_hwconfig_blob() to hopefully be clearer without relying on comments so much, and add various suggestions from Michal. * Michal also had some suggestions in John's "drm/i915/guc: Add fetch of hwconfig table" patch. I held off on making any of these changes in this version.
v5: * Add many changes suggested by Michal in John's "drm/i915/guc: Add fetch of hwconfig table" patch. * Fix documenation formatting as suggested by Daniel in "drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item"
v6: * Updated "drm/i915/guc: Add fetch of hwconfig table" by merging in John's v2 patch which saves the hwconfig blob at the GT level. I also added a few changes requested by Michal on the v5 posting. * Tvrtko requested an example of UMD using the i915 hwconfig interface. Mesa support for this interface can be seen in this MR: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14511
John Harrison (1): drm/i915/guc: Add fetch of hwconfig table
Jordan Justen (2): drm/i915/uapi: Add struct drm_i915_query_hwconfig_blob_item drm/i915/guc: Verify hwconfig blob matches supported format
Rodrigo Vivi (1): drm/i915/uapi: Add query for hwconfig blob
drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gt/intel_gt.c | 6 + drivers/gpu/drm/i915/gt/intel_gt_types.h | 4 + drivers/gpu/drm/i915/gt/intel_hwconfig.h | 21 ++ .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 1 + .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h | 4 + .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c | 203 ++++++++++++++++++ drivers/gpu/drm/i915/i915_query.c | 23 ++ include/uapi/drm/i915_drm.h | 44 ++++ 9 files changed, 307 insertions(+) create mode 100644 drivers/gpu/drm/i915/gt/intel_hwconfig.h create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
From: John Harrison John.C.Harrison@Intel.com
Implement support for fetching the hardware description table from the GuC. The call is made twice - once without a destination buffer to query the size and then a second time to fill in the buffer.
The table is stored in the GT structure so that it can be fetched once at driver load time. Keeping inside a GuC structure would mean it would be release and reloaded on a GuC reset (part of a full GT reset). However, the table does not change just because the GT has been reset and the GuC reloaded. Also, dynamic memory allocations inside the reset path are a problem.
Note that the table is only available on ADL-P and later platforms.
v2 (John's v2 patch): * Move to GT level to avoid memory allocation during reset path (and unnecessary re-read of the table on a reset).
v5 (of Jordan's posting): * Various changes made by Jordan and recommended by Michal - Makefile ordering - Adjust "struct intel_guc_hwconfig hwconfig" comment - Set Copyright year to 2022 in intel_guc_hwconfig.c/.h - Drop inline from hwconfig_to_guc() - Replace hwconfig param with guc in __guc_action_get_hwconfig() - Move zero size check into guc_hwconfig_discover_size() - Change comment to say zero size offset/size is needed to get size - Add has_guc_hwconfig to devinfo and drop has_table() - Change drm_err to notice in __uc_init_hw() and use %pe
v6 (of Jordan's posting): * Added a couple more small changes recommended by Michal * Merge in John's v2 patch, but note: - Using drm_notice as recommended by Michal - Reverted Michal's suggestion of using devinfo
Cc: Michal Wajdeczko michal.wajdeczko@intel.com Signed-off-by: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: John Harrison John.C.Harrison@Intel.com Reviewed-by: Matthew Brost matthew.brost@intel.com Acked-by: Jon Bloomfield jon.bloomfield@intel.com Signed-off-by: Jordan Justen jordan.l.justen@intel.com Reviewed-by: Michal Wajdeczko michal.wajdeczko@intel.com --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/gt/intel_gt.c | 6 + drivers/gpu/drm/i915/gt/intel_gt_types.h | 4 + drivers/gpu/drm/i915/gt/intel_hwconfig.h | 21 +++ .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 1 + .../gpu/drm/i915/gt/uc/abi/guc_errors_abi.h | 4 + .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c | 162 ++++++++++++++++++ 7 files changed, 199 insertions(+) create mode 100644 drivers/gpu/drm/i915/gt/intel_hwconfig.h create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 9d588d936e3d..61b078bd1b32 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -187,6 +187,7 @@ i915-y += gt/uc/intel_uc.o \ gt/uc/intel_guc_ct.o \ gt/uc/intel_guc_debugfs.o \ gt/uc/intel_guc_fw.o \ + gt/uc/intel_guc_hwconfig.o \ gt/uc/intel_guc_log.o \ gt/uc/intel_guc_log_debugfs.o \ gt/uc/intel_guc_rc.o \ diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index e8403fa53909..bf02fb28562a 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -712,6 +712,11 @@ int intel_gt_init(struct intel_gt *gt) if (err) goto err_uc_init;
+ err = intel_gt_init_hwconfig(gt); + if (err) + drm_notice(>->i915->drm, "Failed to retrieve hwconfig table: %pe\n", + ERR_PTR(err)); + err = __engines_record_defaults(gt); if (err) goto err_gt; @@ -793,6 +798,7 @@ void intel_gt_driver_release(struct intel_gt *gt) intel_gt_pm_fini(gt); intel_gt_fini_scratch(gt); intel_gt_fini_buffer_pool(gt); + intel_gt_fini_hwconfig(gt); }
void intel_gt_driver_late_release(struct intel_gt *gt) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h index f20687796490..514b92cff9b0 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h @@ -20,6 +20,7 @@ #include "i915_vma.h" #include "intel_engine_types.h" #include "intel_gt_buffer_pool_types.h" +#include "intel_hwconfig.h" #include "intel_llc_types.h" #include "intel_reset_types.h" #include "intel_rc6_types.h" @@ -199,6 +200,9 @@ struct intel_gt { struct sseu_dev_info sseu;
unsigned long mslice_mask; + + /** @hwconfig: hardware configuration data */ + struct intel_hwconfig hwconfig; } info;
struct { diff --git a/drivers/gpu/drm/i915/gt/intel_hwconfig.h b/drivers/gpu/drm/i915/gt/intel_hwconfig.h new file mode 100644 index 000000000000..322290780b67 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/intel_hwconfig.h @@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright © 2022 Intel Corporation + */ + +#ifndef _INTEL_HWCONFIG_H_ +#define _INTEL_HWCONFIG_H_ + +#include <linux/types.h> + +struct intel_gt; + +struct intel_hwconfig { + u32 size; + void *ptr; +}; + +int intel_gt_init_hwconfig(struct intel_gt *gt); +void intel_gt_fini_hwconfig(struct intel_gt *gt); + +#endif /* _INTEL_HWCONFIG_H_ */ diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h index 7afdadc7656f..a9a329e53c35 100644 --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h @@ -129,6 +129,7 @@ enum intel_guc_action { INTEL_GUC_ACTION_ENGINE_FAILURE_NOTIFICATION = 0x1009, INTEL_GUC_ACTION_SETUP_PC_GUCRC = 0x3004, INTEL_GUC_ACTION_AUTHENTICATE_HUC = 0x4000, + INTEL_GUC_ACTION_GET_HWCONFIG = 0x4100, INTEL_GUC_ACTION_REGISTER_CONTEXT = 0x4502, INTEL_GUC_ACTION_DEREGISTER_CONTEXT = 0x4503, INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER = 0x4505, diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h index c20658ee85a5..8085fb181274 100644 --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_errors_abi.h @@ -8,6 +8,10 @@
enum intel_guc_response_status { INTEL_GUC_RESPONSE_STATUS_SUCCESS = 0x0, + INTEL_GUC_RESPONSE_NOT_SUPPORTED = 0x20, + INTEL_GUC_RESPONSE_NO_ATTRIBUTE_TABLE = 0x201, + INTEL_GUC_RESPONSE_NO_DECRYPTION_KEY = 0x202, + INTEL_GUC_RESPONSE_DECRYPTION_FAILED = 0x204, INTEL_GUC_RESPONSE_STATUS_GENERIC_FAIL = 0xF000, };
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c new file mode 100644 index 000000000000..e0f65bdd1c84 --- /dev/null +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2022 Intel Corporation + */ + +#include "gt/intel_gt.h" +#include "gt/intel_hwconfig.h" +#include "i915_drv.h" +#include "i915_memcpy.h" + +/* + * GuC has a blob containing hardware configuration information (HWConfig). + * This is formatted as a simple and flexible KLV (Key/Length/Value) table. + * + * For example, a minimal version could be: + * enum device_attr { + * ATTR_SOME_VALUE = 0, + * ATTR_SOME_MASK = 1, + * }; + * + * static const u32 hwconfig[] = { + * ATTR_SOME_VALUE, + * 1, // Value Length in DWords + * 8, // Value + * + * ATTR_SOME_MASK, + * 3, + * 0x00FFFFFFFF, 0xFFFFFFFF, 0xFF000000, + * }; + * + * The attribute ids are defined in a hardware spec. + */ + +static int __guc_action_get_hwconfig(struct intel_guc *guc, + u32 ggtt_offset, u32 ggtt_size) +{ + u32 action[] = { + INTEL_GUC_ACTION_GET_HWCONFIG, + lower_32_bits(ggtt_offset), + upper_32_bits(ggtt_offset), + ggtt_size, + }; + int ret; + + ret = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0); + if (ret == -ENXIO) + return -ENOENT; + + return ret; +} + +static int guc_hwconfig_discover_size(struct intel_guc *guc, struct intel_hwconfig *hwconfig) +{ + int ret; + + /* + * Sending a query with zero offset and size will return the + * size of the blob. + */ + ret = __guc_action_get_hwconfig(guc, 0, 0); + if (ret < 0) + return ret; + + if (ret == 0) + return -EINVAL; + + hwconfig->size = ret; + return 0; +} + +static int guc_hwconfig_fill_buffer(struct intel_guc *guc, struct intel_hwconfig *hwconfig) +{ + struct i915_vma *vma; + u32 ggtt_offset; + void *vaddr; + int ret; + + GEM_BUG_ON(!hwconfig->size); + + ret = intel_guc_allocate_and_map_vma(guc, hwconfig->size, &vma, &vaddr); + if (ret) + return ret; + + ggtt_offset = intel_guc_ggtt_offset(guc, vma); + + ret = __guc_action_get_hwconfig(guc, ggtt_offset, hwconfig->size); + if (ret >= 0) + memcpy(hwconfig->ptr, vaddr, hwconfig->size); + + i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP); + + return ret; +} + +static bool has_table(struct drm_i915_private *i915) +{ + if (IS_ALDERLAKE_P(i915)) + return true; + + return false; +} + +/** + * intel_guc_hwconfig_init - Initialize the HWConfig + * + * Retrieve the HWConfig table from the GuC and save it locally. + * It can then be queried on demand by other users later on. + */ +static int guc_hwconfig_init(struct intel_gt *gt) +{ + struct intel_hwconfig *hwconfig = >->info.hwconfig; + struct intel_guc *guc = >->uc.guc; + int ret; + + if (!has_table(gt->i915)) + return 0; + + ret = guc_hwconfig_discover_size(guc, hwconfig); + if (ret) + return ret; + + hwconfig->ptr = kmalloc(hwconfig->size, GFP_KERNEL); + if (!hwconfig->ptr) { + hwconfig->size = 0; + return -ENOMEM; + } + + ret = guc_hwconfig_fill_buffer(guc, hwconfig); + if (ret < 0) { + intel_gt_fini_hwconfig(gt); + return ret; + } + + return 0; +} + +/** + * intel_gt_init_hwconfig - Initialize the HWConfig if available + * + * Retrieve the HWConfig table if available on the current platform. + */ +int intel_gt_init_hwconfig(struct intel_gt *gt) +{ + if (!intel_uc_uses_guc(>->uc)) + return 0; + + return guc_hwconfig_init(gt); +} + +/** + * intel_gt_fini_hwconfig - Finalize the HWConfig + * + * Free up the memory allocation holding the table. + */ +void intel_gt_fini_hwconfig(struct intel_gt *gt) +{ + struct intel_hwconfig *hwconfig = >->info.hwconfig; + + kfree(hwconfig->ptr); + hwconfig->size = 0; + hwconfig->ptr = NULL; +}
Hi Jordan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next drm/drm-next tegra-drm/drm/tegra/for-next v5.17-rc5 next-20220225] [cannot apply to airlied/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Jordan-Justen/GuC-HWCONFIG-with-doc... base: git://anongit.freedesktop.org/drm-intel for-linux-next config: i386-randconfig-a011 (https://download.01.org/0day-ci/archive/20220227/202202271904.1Rr5ckwF-lkp@i...) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/7a92eba9714ffe68202fee73b9916d35b0da... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Jordan-Justen/GuC-HWCONFIG-with-documentation/20220227-161945 git checkout 7a92eba9714ffe68202fee73b9916d35b0da2968 # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c:110: warning: Function parameter or member 'gt' not described in 'guc_hwconfig_init'
drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c:110: warning: expecting prototype for intel_guc_hwconfig_init(). Prototype was for guc_hwconfig_init() instead
drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c:143: warning: Function parameter or member 'gt' not described in 'intel_gt_init_hwconfig' drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c:156: warning: Function parameter or member 'gt' not described in 'intel_gt_fini_hwconfig'
vim +110 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
102 103 /** 104 * intel_guc_hwconfig_init - Initialize the HWConfig 105 * 106 * Retrieve the HWConfig table from the GuC and save it locally. 107 * It can then be queried on demand by other users later on. 108 */ 109 static int guc_hwconfig_init(struct intel_gt *gt)
110 {
111 struct intel_hwconfig *hwconfig = >->info.hwconfig; 112 struct intel_guc *guc = >->uc.guc; 113 int ret; 114 115 if (!has_table(gt->i915)) 116 return 0; 117 118 ret = guc_hwconfig_discover_size(guc, hwconfig); 119 if (ret) 120 return ret; 121 122 hwconfig->ptr = kmalloc(hwconfig->size, GFP_KERNEL); 123 if (!hwconfig->ptr) { 124 hwconfig->size = 0; 125 return -ENOMEM; 126 } 127 128 ret = guc_hwconfig_fill_buffer(guc, hwconfig); 129 if (ret < 0) { 130 intel_gt_fini_hwconfig(gt); 131 return ret; 132 } 133 134 return 0; 135 } 136
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Jordan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next drm/drm-next tegra-drm/drm/tegra/for-next v5.17-rc5 next-20220225] [cannot apply to airlied/drm-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Jordan-Justen/GuC-HWCONFIG-with-doc... base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-a011 (https://download.01.org/0day-ci/archive/20220227/202202272041.Kr2QxFem-lkp@i...) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/7a92eba9714ffe68202fee73b9916d35b0da... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Jordan-Justen/GuC-HWCONFIG-with-documentation/20220227-161945 git checkout 7a92eba9714ffe68202fee73b9916d35b0da2968 # save the config file to linux build tree mkdir build_dir make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c:110: warning: Function parameter or member 'gt' not described in 'guc_hwconfig_init'
drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c:110: warning: expecting prototype for intel_guc_hwconfig_init(). Prototype was for guc_hwconfig_init() instead
drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c:143: warning: Function parameter or member 'gt' not described in 'intel_gt_init_hwconfig' drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c:156: warning: Function parameter or member 'gt' not described in 'intel_gt_fini_hwconfig'
vim +110 drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
102 103 /** 104 * intel_guc_hwconfig_init - Initialize the HWConfig 105 * 106 * Retrieve the HWConfig table from the GuC and save it locally. 107 * It can then be queried on demand by other users later on. 108 */ 109 static int guc_hwconfig_init(struct intel_gt *gt)
110 {
111 struct intel_hwconfig *hwconfig = >->info.hwconfig; 112 struct intel_guc *guc = >->uc.guc; 113 int ret; 114 115 if (!has_table(gt->i915)) 116 return 0; 117 118 ret = guc_hwconfig_discover_size(guc, hwconfig); 119 if (ret) 120 return ret; 121 122 hwconfig->ptr = kmalloc(hwconfig->size, GFP_KERNEL); 123 if (!hwconfig->ptr) { 124 hwconfig->size = 0; 125 return -ENOMEM; 126 } 127 128 ret = guc_hwconfig_fill_buffer(guc, hwconfig); 129 if (ret < 0) { 130 intel_gt_fini_hwconfig(gt); 131 return ret; 132 } 133 134 return 0; 135 } 136
--- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
From: Rodrigo Vivi rodrigo.vivi@intel.com
The DRM_I915_QUERY_HWCONFIG_BLOB query item returns a blob of data which it receives from the GuC software. This blob provides some useful data about the hardware for drivers.
Although the blob is not fully documented at this time, the basic format is an array of u32 values. The array is a simple and flexible KLV (Key/Length/Value) formatted table. For example, it could be just: enum device_attr { ATTR_SOME_VALUE = 0, ATTR_SOME_MASK = 1, };
static const u32 hwconfig[] = { ATTR_SOME_VALUE, 1, // Value Length in DWords 8, // Value
ATTR_SOME_MASK, 3, 0x00FFFFFFFF, 0xFFFFFFFF, 0xFF000000, };
The attribute ids and meaning of the values will be documented in the Programmer Reference Manuals when released.
Cc: Tvrtko Ursulin tvrtko.ursulin@linux.intel.com Cc: Kenneth Graunke kenneth.w.graunke@intel.com Cc: Michal Wajdeczko michal.wajdeczko@intel.com Cc: Slawomir Milczarek slawomir.milczarek@intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Signed-off-by: Rodrigo Vivi rodrigo.vivi@intel.com Signed-off-by: John Harrison John.C.Harrison@Intel.com Reviewed-by: Matthew Brost matthew.brost@intel.com Acked-by: Jordan Justen jordan.l.justen@intel.com Tested-by: Jordan Justen jordan.l.justen@intel.com Acked-by: Jon Bloomfield jon.bloomfield@intel.com --- drivers/gpu/drm/i915/i915_query.c | 23 +++++++++++++++++++++++ include/uapi/drm/i915_drm.h | 1 + 2 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c index 2dfbc22857a3..b5ca00cb6cf6 100644 --- a/drivers/gpu/drm/i915/i915_query.c +++ b/drivers/gpu/drm/i915/i915_query.c @@ -479,12 +479,35 @@ static int query_memregion_info(struct drm_i915_private *i915, return total_length; }
+static int query_hwconfig_blob(struct drm_i915_private *i915, + struct drm_i915_query_item *query_item) +{ + struct intel_gt *gt = to_gt(i915); + struct intel_hwconfig *hwconfig = >->info.hwconfig; + + if (!hwconfig->size || !hwconfig->ptr) + return -ENODEV; + + if (query_item->length == 0) + return hwconfig->size; + + if (query_item->length < hwconfig->size) + return -EINVAL; + + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr), + hwconfig->ptr, hwconfig->size)) + return -EFAULT; + + return hwconfig->size; +} + static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv, struct drm_i915_query_item *query_item) = { query_topology_info, query_engine_info, query_perf_config, query_memregion_info, + query_hwconfig_blob, };
int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file) diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 914ebd9290e5..069d2fadfbd9 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -2685,6 +2685,7 @@ struct drm_i915_query_item { #define DRM_I915_QUERY_ENGINE_INFO 2 #define DRM_I915_QUERY_PERF_CONFIG 3 #define DRM_I915_QUERY_MEMORY_REGIONS 4 +#define DRM_I915_QUERY_HWCONFIG_BLOB 5 /* Must be kept compact -- no holes and well documented */
/**
Also, document DRM_I915_QUERY_HWCONFIG_BLOB with this struct.
v3: * Add various changes suggested by Tvrtko
v5: * Fix documenation formatting and verified with `make htmldocs` as suggested by Daniel
Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Jordan Justen jordan.l.justen@intel.com Acked-by: Jon Bloomfield jon.bloomfield@intel.com Acked-by: Daniel Vetter daniel.vetter@ffwll.ch --- include/uapi/drm/i915_drm.h | 43 +++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 069d2fadfbd9..d033211cb862 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -3279,6 +3279,49 @@ struct drm_i915_gem_create_ext_protected_content { /* ID of the protected content session managed by i915 when PXP is active */ #define I915_PROTECTED_CONTENT_DEFAULT_SESSION 0xf
+/** + * DOC: GuC HWCONFIG blob uAPI + * + * The GuC produces a blob with information about the current device. + * i915 reads this blob from GuC and makes it available via this uAPI. + * + * The returned blob is a sequence of items of variable length + * described by struct drm_i915_query_hwconfig_blob_item. + * + * The overall blob returned by DRM_I915_QUERY_HWCONFIG_BLOB will end + * at the same location as the end of the final struct + * drm_i915_query_hwconfig_blob_item. In other words, walking through + * the individual items is guaranteed to eventually arrive at the + * exact end of the entire blob. + */ + +/** + * struct drm_i915_query_hwconfig_blob_item - A single hwconfig item + * within the sequence of hwconfig items returned by + * DRM_I915_QUERY_HWCONFIG_BLOB. + * + * The length field gives the length of the data[] array. The length + * is the number of u32 items in the data[] array, and *not* the + * number of bytes. + * + * The key and length fields are required, so the minimum item size is + * 2 x u32, or 8 bytes, when the length field is 0. If the length + * field is 1, then the item's size is 12 bytes. + * + * The meaning of the key field and the data values are documented in + * the Programmer's Reference Manual. + */ +struct drm_i915_query_hwconfig_blob_item { + /** @key: Enum which defines how to interpret @data values. */ + __u32 key; + + /** @length: The number of u32 values in the @data array. */ + __u32 length; + + /** @data: Array of values with meaning defined by @key. */ + __u32 data[]; +}; + #if defined(__cplusplus) } #endif
i915_drm.h now defines the format of the returned DRM_I915_QUERY_HWCONFIG_BLOB query item. Since i915 receives this from the black box GuC software, it should verify that the data matches that format before sending it to user-space.
The verification makes a single simple pass through the blob contents, so this verification step should not add a significant amount of init time to i915.
v3: * Add various changes suggested by Tvrtko
v4: * Rewrite verify_hwconfig_blob() to hopefully be clearer without relying on comments so much, and add various suggestions from Michal.
v6: * Rework based on John's updated "drm/i915/guc: Add fetch of hwconfig table" v2 which stores the hwconfig blob at the GT level.
Signed-off-by: Jordan Justen jordan.l.justen@intel.com Acked-by: Jon Bloomfield jon.bloomfield@intel.com Reviewed-by: Tvrtko Ursulin tvrtko.ursulin@intel.com --- .../gpu/drm/i915/gt/uc/intel_guc_hwconfig.c | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c index e0f65bdd1c84..1a3134d3d434 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c @@ -68,8 +68,44 @@ static int guc_hwconfig_discover_size(struct intel_guc *guc, struct intel_hwconf return 0; }
+static int verify_hwconfig_blob(struct intel_guc *guc, struct intel_hwconfig *hwconfig) +{ + struct drm_device *drm = &guc_to_gt(guc)->i915->drm; + struct drm_i915_query_hwconfig_blob_item *item = hwconfig->ptr; + u64 offset = 0; + u64 remaining = hwconfig->size; + /* Everything before the data field is required */ + u64 min_item_size = offsetof(struct drm_i915_query_hwconfig_blob_item, data); + u64 item_size; + + if (!IS_ALIGNED(hwconfig->size, sizeof(u32))) { + drm_err(drm, "hwconfig blob size (%d) is not u32 aligned\n", hwconfig->size); + return -EINVAL; + } + + while (offset < hwconfig->size) { + if (remaining < min_item_size) { + drm_err(drm, "hwconfig blob invalid (no room for item required fields at offset %lld)\n", + offset); + return -EINVAL; + } + item_size = min_item_size + sizeof(u32) * item->length; + if (item_size > remaining) { + drm_err(drm, "hwconfig blob invalid (no room for data array of item at offset %lld)\n", + offset); + return -EINVAL; + } + offset += item_size; + remaining -= item_size; + item = (void *)&item->data[item->length]; + } + + return 0; +} + static int guc_hwconfig_fill_buffer(struct intel_guc *guc, struct intel_hwconfig *hwconfig) { + struct drm_device *drm = &guc_to_gt(guc)->i915->drm; struct i915_vma *vma; u32 ggtt_offset; void *vaddr; @@ -84,8 +120,13 @@ static int guc_hwconfig_fill_buffer(struct intel_guc *guc, struct intel_hwconfig ggtt_offset = intel_guc_ggtt_offset(guc, vma);
ret = __guc_action_get_hwconfig(guc, ggtt_offset, hwconfig->size); - if (ret >= 0) + if (ret >= 0) { memcpy(hwconfig->ptr, vaddr, hwconfig->size); + if (verify_hwconfig_blob(guc, hwconfig)) { + drm_err(drm, "Ignoring invalid hwconfig blob received from GuC!\n"); + ret = -EINVAL; + } + }
i915_vma_unpin_and_release(&vma, I915_VMA_RELEASE_MAP);
dri-devel@lists.freedesktop.org