> -----Original Message----- > From: Christian König [mailto:deathsimple at vodafone.de] > Sent: Tuesday, January 10, 2017 9:59 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 14:33 schrieb 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. > > 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 >
