[V2 05/11] drm/amdgpu/virt: add high level interfaces for virt

[Xiangliang.Yu@xxxxxxx (Yu, Xiangliang)]

> -----Original Message-----
> From: Christian König [mailto:deathsimple at vodafone.de]
> Sent: Tuesday, January 10, 2017 9:12 PM
> To: Yu, Xiangliang <Xiangliang.Yu at amd.com>; amd-
> gfx at lists.freedesktop.org
> Subject: Re: [V2 05/11] drm/amdgpu/virt: add high level interfaces for virt
> 
> Am 10.01.2017 um 11:00 schrieb Xiangliang Yu:
> > Add high level interfaces that is not relate to specific asic. So asic
> > files just need to implement the interfaces to support virtualization.
> >
> > Signed-off-by: Xiangliang Yu <Xiangliang.Yu at amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 57
> ++++++++++++++++++++++++++++++++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 15 +++++++++
> >   2 files changed, 72 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > index 6520a4e..f32a789 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > @@ -84,3 +84,60 @@ void amdgpu_virt_kiq_wreg(struct amdgpu_device
> *adev, uint32_t reg, uint32_t v)
> >   		DRM_ERROR("wait for kiq fence error: %ld.\n", r);
> >   	fence_put(f);
> >   }
> > +
> > +/**
> > + * amdgpu_virt_request_full_gpu() - request full gpu access
> > + * @amdgpu:	amdgpu device.
> > + * @init:	is driver init time.
> > + * When start to init/fini driver, first need to request full gpu access.
> > + * Return: Zero if request success, otherwise will return error.
> > + */
> > +int amdgpu_virt_request_full_gpu(struct amdgpu_device *adev, bool
> > +init) {
> > +	struct amdgpu_virt *virt = &adev->virt;
> > +
> > +	if (virt->ops && virt->ops->req_full_gpu) {
> > +		adev->virt.caps &= ~AMDGPU_SRIOV_CAPS_RUNTIME;
> > +		return virt->ops->req_full_gpu(adev, init);
> 
> I would be conservative here and request full GPU access first and then clear
> AMDGPU_SRIOV_CAPS_RUNTIME.
> 
> Just in case the function is called concurrently with another thread checking
> the caps.

It can't be used in parallel, the problem you said shouldn't be appear.

> 
> On the other hand req_full_gpu() could need the flag to handle register
> reads/writes correctly, but in this case I would question if we shouldn't add
> special macros for this.

We must clear RUNTIME flag so that can read/write registers with mmio,
Otherwise driver will hang.

> 
> 
> 
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * amdgpu_virt_release_full_gpu() - release full gpu access
> > + * @amdgpu:	amdgpu device.
> > + * @init:	is driver init time.
> > + * When finishing driver init/fini, need to release full gpu access.
> > + * Return: Zero if release success, otherwise will returen error.
> > + */
> > +int amdgpu_virt_release_full_gpu(struct amdgpu_device *adev, bool
> > +init) {
> > +	struct amdgpu_virt *virt = &adev->virt;
> > +	int r;
> > +
> > +	if (virt->ops && virt->ops->rel_full_gpu) {
> > +		r = virt->ops->rel_full_gpu(adev, init);
> > +		adev->virt.caps |= AMDGPU_SRIOV_CAPS_RUNTIME;
> > +		return r;
> > +	}
> > +	return 0;
> > +}
> > +
> > +/**
> > + * amdgpu_virt_reset_gpu() - reset gpu
> > + * @amdgpu:	amdgpu device.
> > + * Send reset command to GPU hypervisor to reset GPU that VM is using
> > + * Return: Zero if reset success, otherwise will return error.
> > + */
> > +int amdgpu_virt_reset_gpu(struct amdgpu_device *adev) {
> > +	struct amdgpu_virt *virt = &adev->virt;
> > +
> > +	if (virt->ops && virt->ops->reset_gpu) {
> > +		adev->virt.caps &= ~AMDGPU_SRIOV_CAPS_RUNTIME;
> > +		return virt->ops->reset_gpu(adev);
> > +	}
> > +
> > +	return 0;
> > +}
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> > index 24f0590..3f8fc0f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> > @@ -29,11 +29,23 @@
> >   #define AMDGPU_SRIOV_CAPS_IS_VF        (1 << 2) /* this GPU is a virtual
> function */
> >   #define AMDGPU_PASSTHROUGH_MODE        (1 << 3) /* thw whole GPU
> is pass through for VM */
> >   #define AMDGPU_SRIOV_CAPS_RUNTIME      (1 << 4) /* is out of full
> access mode */
> > +
> > +/**
> > + * struct amdgpu_virt_ops - amdgpu device virt operations  */ struct
> > +amdgpu_virt_ops {
> > +	int (*req_full_gpu)(struct amdgpu_device *adev, bool init);
> > +	int (*rel_full_gpu)(struct amdgpu_device *adev, bool init);
> > +	int (*reset_gpu)(struct amdgpu_device *adev); };
> > +
> >   /* GPU virtualization */
> >   struct amdgpu_virt {
> >   	uint32_t		caps;
> >   	uint32_t		val_offs;
> >   	struct mutex		lock;
> > +
> > +	const struct amdgpu_virt_ops	*ops;
> >   };
> >
> >   #define amdgpu_sriov_enabled(adev) \ @@ -63,5 +75,8 @@ static inline
> > bool is_virtual_machine(void)
> >   void amdgpu_virt_init_setting(struct amdgpu_device *adev);
> >   uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t
> reg);
> >   void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg,
> > uint32_t v);
> > +int amdgpu_virt_request_full_gpu(struct amdgpu_device *adev, bool
> > +init); int amdgpu_virt_release_full_gpu(struct amdgpu_device *adev,
> > +bool init); int amdgpu_virt_reset_gpu(struct amdgpu_device *adev);
> >
> >   #endif
>