Re: [Intel-xe] [PATCH v3 27/30] drm/xe: Extend uAPI to query HuC micro-controler firmware version

["Souza, Jose" <jose.souza@intel.com>]

+ John Harrison

On Wed, 2023-09-27 at 13:04 -0400, Rodrigo Vivi wrote:
> On Tue, Sep 26, 2023 at 04:46:36PM +0000, Souza, Jose wrote:
> > On Tue, 2023-09-26 at 12:55 +0000, Francois Dugast wrote:
> > > The infrastructure to query GuC firmware version is already in place. It
> > > is extended with a new micro-controller type to query the HuC firmware
> > > version. It can be used from user space to know if HuC is running.
> > > 
> > > Signed-off-by: Francois Dugast <francois.dugast@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_query.c | 9 +++++++++
> > >  include/uapi/drm/xe_drm.h     | 1 +
> > >  2 files changed, 10 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> > > index 7a0ffd9a654a..c250ca534bb9 100644
> > > --- a/drivers/gpu/drm/xe/xe_query.c
> > > +++ b/drivers/gpu/drm/xe/xe_query.c
> > > @@ -530,6 +530,15 @@ query_uc_fw_version(struct xe_device *xe, struct drm_xe_device_query *query)
> > >  		resp.branch_ver = 0;
> > >  		break;
> > >  	}
> > > +	case XE_QUERY_UC_TYPE_HUC: {
> > > +		struct xe_huc *huc = &xe->tiles[0].primary_gt->uc.huc;
> > > +
> > > +		resp.major_ver = huc->fw.major_ver_found;
> > > +		resp.minor_ver = huc->fw.minor_ver_found;
> > > +		resp.patch_ver = huc->fw.patch_ver_found;
> > 
> > Have you confirmed that HuC will not have something like submission version like GuC have?
> 
> Nah... GuC is the only complicated fw in our set of fw...
> 
> > At least in GuC, when running in SRIOV mode the VFs will not have access to the actual GuC version, that is why it have submission version.
> > 
> > Not sure if providing a complete different firmware version from one kernel version to other would be considered a uAPI break...
> 
> hmmm... but now what I'm asking myself is if we shouldn't move the guc one to
> have the current loaded firmware and create a special category for the
> submission version:
> 
> XE_QUERY_UC_TYPE_GUC
> XE_QUERY_UC_TYPE_GUC_SUBMISSION
> XE_QUERY_UC_TYPE_HUC

I don't think any UMD would fetch the actual GUC FW version and risk fail when running under SRIOV VF.
If needed we can map a submission version to a actual version...

> 
> But to be really really honest, there's something really fishy on this
> submission version. Why the VF cannot read the running firmware and
> get the submission version from there?

Got this information from John, he can explain it better.

> 
> > 
> > > +		resp.branch_ver = 0;
> > > +		break;
> > > +	}
> > >  	default:
> > >  		return -EINVAL;
> > >  	}
> > > diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> > > index 84091860c7d2..fe7e83a5bd3e 100644
> > > --- a/include/uapi/drm/xe_drm.h
> > > +++ b/include/uapi/drm/xe_drm.h
> > > @@ -478,6 +478,7 @@ struct drm_xe_query_topology_mask {
> > >  struct drm_xe_query_uc_fw_version {
> > >  	/** @uc: The micro-controller type to query firmware version */
> > >  #define XE_QUERY_UC_TYPE_GUC 0
> > > +#define XE_QUERY_UC_TYPE_HUC 1
> > >  	__u16 uc_type;
> > >  
> > >  	/** @pad: MBZ */
> >