<html> <head> <meta content="text/html; charset=utf-8" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">We don't need the validate buffer and even when you need it putting it into the VM structure is illegal, because it can be used by more than one thread at the same time. This leaves only the csa_bo_va in the amdgpu_vm_virt structure and having a structure with only one field is rather pointless. Additional to that I really don't see CSA handling as something related to virtualization. Regards, Christian. Am 10.01.2017 um 04:05 schrieb Liu, Monk: </div> <blockquote cite="mid:BY2PR1201MB1110963B2DE72B2892D4AC1F84670@BY2PR1201MB1110.namprd12.prod.outlook.com" type="cite"> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <meta name="Generator" content="Microsoft Exchange Server">  <style></style> <meta content="text/html; charset=UTF-8"> <style type="text/css" style="">  </style> <div dir="ltr"> <div id="x_divtagdefaultwrapper" dir="ltr" style="font-size:12pt; color:#000000; font-family:Calibri,Arial,Helvetica,sans-serif"> > +}; > + > +struct amdgpu_vm_virt { > + /* each VM will map on CSA */ > + struct ttm_validate_buffer csa_tv; > + struct amdgpu_bo_va *csa_bo_va; Please put that directly into the amdgpu_vm structure. [ML] they are not a tiny used by bare-metal case so I don't think they should be put into common structure at all. see we have "struct amdgpu_virt" in "struct amdgpu_device", the same style BR Monk </div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_divRplyFwdMsg" dir="ltr">发件人: Christian König <a class="moz-txt-link-rfc2396E" href="mailto:deathsimple@vodafone.de"><deathsimple@vodafone.de></a> 发送时间: 2017年1月9日 18:57:06 收件人: Liu, Monk; <a class="moz-txt-link-abbreviated" href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> 主题: Re: [PATCH 2/7] drm/amdgpu:new field members for SRIOV <div> </div> </div> </div> <div class="PlainText">Am 09.01.2017 um 09:02 schrieb Monk Liu: > Change-Id: Ife0eff7b13b8b5946f005a39f6ecb8db1cb72c38 > Signed-off-by: Monk Liu <a class="moz-txt-link-rfc2396E" href="mailto:Monk.Liu@amd.com"><Monk.Liu@amd.com></a> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 19 +++++++++++++++++++ > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 ++ > 2 files changed, 21 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h > index 0d821d9..5aa7f0c 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h > @@ -28,9 +28,28 @@ > #define AMDGPU_SRIOV_CAPS_ENABLE_IOV (1 << 1) /* sr-iov is enabled on this GPU */ > #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 */ > + > +struct amdgpu_vm; > /* GPU virtualization */ > struct amdgpu_virt { > uint32_t caps; > + uint32_t csa_size; > + struct amdgpu_bo *csa_obj; > + uint64_t csa_vmid0_addr; > + uint64_t gds_vmid0_addr; > + int (*allocate_csa)(struct amdgpu_device *adev); > + void (*deallocate_csa)(struct amdgpu_device *adev); > + int (*map_csa)(struct amdgpu_device *adev, struct amdgpu_vm *vm); > + void (*unmap_csa)(struct amdgpu_device *adev, struct amdgpu_vm *vm); Why callbacks for this? > +}; > + > +struct amdgpu_vm_virt { > + /* each VM will map on CSA */ > + struct ttm_validate_buffer csa_tv; > + struct amdgpu_bo_va *csa_bo_va; Please put that directly into the amdgpu_vm structure. > + /* virtual MC address of CSA & GDS for each VM */ > + uint64_t vm_csa_addr; > + uint64_t vm_gds_addr; That should be constant, shouldn't it? Regards, Christian. > }; > > #define amdgpu_sriov_enabled(adev) \ > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > index 42a629b..27cbcbc 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h > @@ -29,6 +29,7 @@ > #include "gpu_scheduler.h" > #include "amdgpu_sync.h" > #include "amdgpu_ring.h" > +#include "amdgpu_virt.h" > > struct amdgpu_bo_va; > struct amdgpu_job; > @@ -111,6 +112,7 @@ struct amdgpu_vm { > > /* client id */ > u64 client_id; > + struct amdgpu_vm_virt vm_virt; > }; > > struct amdgpu_vm_id { </div> </blockquote> </body> </html>