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

Submitted by Yu, Xiangliang on Jan. 10, 2017, 10 a.m.

Details

Message ID 1484042450-22987-6-git-send-email-Xiangliang.Yu@amd.com
State New
Headers show
Series "Add support AMD GPU virtualization soultion" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Yu, Xiangliang Jan. 10, 2017, 10 a.m.
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@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(+)

Patch hide | download patch | download mbox

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);
+	}
+
+	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

Comments

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@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.

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.

Christian.



> +	}
> +
> +	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
> -----Original Message-----

> From: Christian König [mailto:deathsimple@vodafone.de]

> Sent: Tuesday, January 10, 2017 9:12 PM

> To: Yu, Xiangliang <Xiangliang.Yu@amd.com>; amd-

> gfx@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@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

>
Am 10.01.2017 um 14:33 schrieb Yu, Xiangliang:
>> -----Original Message-----
>> From: Christian König [mailto:deathsimple@vodafone.de]
>> Sent: Tuesday, January 10, 2017 9:12 PM
>> To: Yu, Xiangliang <Xiangliang.Yu@amd.com>; amd-
>> gfx@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@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.

Yeah, that's what I expected. Would be nice if we could better split 
that, e.g. have explicit macros for direct register write.

But that's really only nice to have, patch is ok as it is as far as I 
can see.

Christian.

>
>>
>>
>>> +	}
>>> +
>>> +	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
> -----Original Message-----

> From: Christian König [mailto:deathsimple@vodafone.de]

> Sent: Tuesday, January 10, 2017 9:59 PM

> To: Yu, Xiangliang <Xiangliang.Yu@amd.com>; amd-

> gfx@lists.freedesktop.org

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

> 

> Am 10.01.2017 um 14:33 schrieb Yu, Xiangliang:

> >> -----Original Message-----

> >> From: Christian König [mailto:deathsimple@vodafone.de]

> >> Sent: Tuesday, January 10, 2017 9:12 PM

> >> To: Yu, Xiangliang <Xiangliang.Yu@amd.com>; amd-

> >> gfx@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@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.

> 

> Yeah, that's what I expected. Would be nice if we could better split that, e.g.

> have explicit macros for direct register write.

> 

> But that's really only nice to have, patch is ok as it is as far as I can see.


I think it is also very clear. Actually, I have a try but it is not good choice.
If anyone have good idea, can change it later.

> >

> >>

> >>

> >>> +	}

> >>> +

> >>> +	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

>
> -----Original Message-----

> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf

> Of Xiangliang Yu

> Sent: Tuesday, January 10, 2017 5:01 AM

> To: amd-gfx@lists.freedesktop.org

> Cc: Yu, Xiangliang

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

> 

> 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@amd.com>


Reviewed-by: Alex Deucher <alexander.deucher@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);

> +	}

> +

> +	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

> --

> 2.7.4

> 

> _______________________________________________

> amd-gfx mailing list

> amd-gfx@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/amd-gfx