Re: [PATCH 9/9] drm/xe/vf: Custom HuC initialization if VF

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Jun 20, 2024 at 01:14:29AM +0200, Michal Wajdeczko wrote:
> 
> 
> On 20.06.2024 01:11, Matthew Brost wrote:
> > On Wed, Jun 19, 2024 at 11:45:57PM +0200, Michal Wajdeczko wrote:
> >> The HuC firmware is loaded and initialized by the PF driver. Make
> >> sure VF driver performs only limited data structure initialization.
> >>
> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> >> ---
> >>  drivers/gpu/drm/xe/xe_huc.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/xe/xe_huc.c b/drivers/gpu/drm/xe/xe_huc.c
> >> index 6238fb354914..c88761fe31c9 100644
> >> --- a/drivers/gpu/drm/xe/xe_huc.c
> >> +++ b/drivers/gpu/drm/xe/xe_huc.c
> >> @@ -21,6 +21,7 @@
> >>  #include "xe_guc.h"
> >>  #include "xe_map.h"
> >>  #include "xe_mmio.h"
> >> +#include "xe_sriov.h"
> >>  #include "xe_uc_fw.h"
> >>  
> >>  static struct xe_gt *
> >> @@ -92,6 +93,9 @@ int xe_huc_init(struct xe_huc *huc)
> >>  	if (!xe_uc_fw_is_enabled(&huc->fw))
> >>  		return 0;
> >>  
> >> +	if (IS_SRIOV_VF(xe))
> >> +		return 0;
> >> +
> > 
> > With this change I assume the main part of xe_huc_auth is never called
> > on a VF?
> > 
> > Does xe_uc_fw_is_loadable return false on a VF?
> 
> yes, as on VF it is marked as PRELOADED, so:
> 
> static inline bool xe_uc_fw_is_loadable(struct xe_uc_fw *uc_fw)
> {
> 	return __xe_uc_fw_status(uc_fw) >= XE_UC_FIRMWARE_LOADABLE &&
> 		__xe_uc_fw_status(uc_fw) != XE_UC_FIRMWARE_PRELOADED;
> }
> 
> returns false
> 

To be clear, I'd add asserts to parts of functions which should not be
executed on a VF if it is expected to short circuit on another function.

e.g.

xe_huc_auth()
	if (!xe_uc_fw_is_loadable)
		return

	xe_assert(!VF);

This patch LGTM to me though. With that:
Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> > 
> > Matt
> > 
> >>  	if (huc->fw.has_gsc_headers) {
> >>  		ret = huc_alloc_gsc_pkt(huc);
> >>  		if (ret)
> >> -- 
> >> 2.43.0
> >>