[Intel-xe] [RFC v1 04/17] drm/xe: Add uAPI to query micro-controler firmware version

[Souza, Jose]

On Wed, 2023-10-11 at 13:07 -0700, John Harrison wrote:
> On 10/11/2023 06:59, Francois Dugast wrote:
> > From: José Roberto de Souza <jose.souza at intel.com>
> > 
> > Due to a bug in GuC firmware, Mesa can't enable by default the usage of
> > compute engines in DG2 and newer.
> > 
> > A new GuC firmware fixed the issue but until now there was no way
> > for Mesa to know if KMD was running with the fixed GuC version or not,
> > so this uAPI is required.
> > 
> > It may be expanded in future to query other firmware versions too.
> > 
> > More information: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/23661
> > Mesa usage: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25233
> > 
> > v2:
> > - changed to submission version
> > - added branch version to be future proof
> > - checking if pads and reserved are zero
> > 
> > v3:
> > - add braces around case XE_QUERY_UC_TYPE_GUC to make CI happy
> > 
> > Cc: John Harrison <John.C.Harrison at Intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > ---
> >   drivers/gpu/drm/xe/xe_query.c | 41 +++++++++++++++++++++++++++++++++++
> >   include/uapi/drm/xe_drm.h     | 31 ++++++++++++++++++++++++++
> >   2 files changed, 72 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> > index 10b9878ec95a..efbcf2ef436e 100644
> > --- a/drivers/gpu/drm/xe/xe_query.c
> > +++ b/drivers/gpu/drm/xe/xe_query.c
> > @@ -498,6 +498,46 @@ static int query_gt_topology(struct xe_device *xe,
> >   	return 0;
> >   }
> >   
> > +static int
> > +query_uc_fw_version(struct xe_device *xe, struct drm_xe_device_query *query)
> > +{
> > +	struct drm_xe_query_uc_fw_version __user *query_ptr = u64_to_user_ptr(query->data);
> > +	size_t size = sizeof(struct drm_xe_query_uc_fw_version);
> > +	struct drm_xe_query_uc_fw_version resp;
> > +
> > +	if (query->size == 0) {
> > +		query->size = size;
> > +		return 0;
> > +	} else if (XE_IOCTL_DBG(xe, query->size != size)) {
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (copy_from_user(&resp, query_ptr, size))
> > +		return -EFAULT;
> > +
> > +	if (XE_IOCTL_DBG(xe, resp.pad || resp.pad2 || resp.reserved))
> > +		return -EINVAL;
> > +
> > +	switch (resp.uc_type) {
> > +	case XE_QUERY_UC_TYPE_GUC: {
> > +		struct xe_guc *guc = &xe->tiles[0].primary_gt->uc.guc;
> > +
> > +		resp.major_ver = guc->submission_state.version.major;
> > +		resp.minor_ver = guc->submission_state.version.minor;
> > +		resp.patch_ver = guc->submission_state.version.patch;
> > +		resp.branch_ver = 0;
> > +		break;
> > +	}
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (copy_to_user(query_ptr, &resp, size))
> > +		return -EFAULT;
> > +
> > +	return 0;
> > +}
> > +
> >   static int (* const xe_query_funcs[])(struct xe_device *xe,
> >   				      struct drm_xe_device_query *query) = {
> >   	query_engines,
> > @@ -507,6 +547,7 @@ static int (* const xe_query_funcs[])(struct xe_device *xe,
> >   	query_hwconfig,
> >   	query_gt_topology,
> >   	query_engine_cycles,
> > +	query_uc_fw_version,
> >   };
> >   
> >   int xe_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> > index 47d2d0468339..f7ca52aba79b 100644
> > --- a/include/uapi/drm/xe_drm.h
> > +++ b/include/uapi/drm/xe_drm.h
> > @@ -466,6 +466,36 @@ struct drm_xe_query_topology_mask {
> >   	__u8 mask[];
> >   };
> >   
> > +/**
> > + * struct drm_xe_query_uc_fw_version - query a micro-controller firmware version
> > + *
> > + * Given a uc_type this will return the major, minor, patch and branch version
> > + * of the micro-controller firmware.
> > + */
> > +struct drm_xe_query_uc_fw_version {
> > +	/** @uc: The micro-controller type to query firmware version */
> > +#define XE_QUERY_UC_TYPE_GUC 0
> I thought it was agreed to use TYPE_GUC_SUBMISSION to differentiate from 
> the file version which may need to be added at some point (e.g. for 
> reporting status in a control panel or something).

It is being renamed in '[RFC v1 16/17] drm/xe/uapi: Be more specific around GuC Version Type'

In my opinion it should be squashed but up to Francois and Rodrigo.

> 
> > +	__u16 uc_type;
> > +
> > +	/** @pad: MBZ */
> > +	__u16 pad;
> And wasn't there a comment about just using a u32 for the type? What is 
> the advantage to splitting it into two u16 values?

yeah good point or rename to rsvd so it can be used in future.


> 
> John.
> 
> > +
> > +	/* @major_ver: major uc fw version */
> > +	__u32 major_ver;
> > +	/* @minor_ver: minor uc fw version */
> > +	__u32 minor_ver;
> > +	/* @patch_ver: patch uc fw version */
> > +	__u32 patch_ver;
> > +	/* @branch_ver: branch uc fw version */
> > +	__u32 branch_ver;
> > +
> > +	/** @pad2: MBZ */
> > +	__u32 pad2;
> > +
> > +	/** @reserved: Reserved */
> > +	__u64 reserved;
> > +};
> > +
> >   /**
> >    * struct drm_xe_device_query - main structure to query device information
> >    *
> > @@ -518,6 +548,7 @@ struct drm_xe_device_query {
> >   #define DRM_XE_DEVICE_QUERY_HWCONFIG		4
> >   #define DRM_XE_DEVICE_QUERY_GT_TOPOLOGY		5
> >   #define DRM_XE_DEVICE_QUERY_ENGINE_CYCLES	6
> > +#define DRM_XE_DEVICE_QUERY_UC_FW_VERSION	7
> >   	/** @query: The type of data to query */
> >   	__u32 query;
> >   
>