[PATCH v2] drm/amdgpu: Default disable GDS for compute VMIDs

[Christian König]

Am 17.07.19 um 22:09 schrieb Kuehling, Felix:
> On 2019-07-17 14:23, Greathouse, Joseph wrote:
>> The GDS and GWS blocks default to allowing all VMIDs to
>> access all entries. Graphics VMIDs can handle setting
>> these limits when the driver launches work. However,
>> compute workloads under HWS control don't go through the
>> kernel driver. Instead, HWS firmware should set these
>> limits when a process is put into a VMID slot.
>>
>> Disable access to these devices by default by turning off
>> all mask bits (for OA) and setting BASE=SIZE=0 (for GDS
>> and GWS) for all compute VMIDs. If a process wants to use
>> these resources, they can request this from the HWS
>> firmware (when such capabilities are enabled). HWS will
>> then handle setting the base and limit for the process when
>> it is assigned to a VMID.
>>
>> This will also prevent user kernels from getting 'stuck' in
>> GWS by accident if they write GWS-using code but HWS
>> firmware is not set up to handle GWS reset. Until HWS is
>> enabled to handle GWS properly, all GWS accesses will
>> MEM_VIOL fault the kernel.
>>
>> v2: Move initialization outside of SRBM mutex
>>
>> Change-Id: I8edcea9d0b14d16a7444bcf9fbf9451aef8b707d
>> Signed-off-by: Joseph Greathouse <Joseph.Greathouse at amd.com>
> Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>

Might be a good idea to do this for all VMIDs during initialization and 
not just for the ones used for compute.

But anyway patch is Reviewed-by: Christian König <christian.koenig at amd.com>.

>
>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 9 +++++++++
>>    drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c  | 9 +++++++++
>>    drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 9 +++++++++
>>    drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 9 +++++++++
>>    4 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> index 618291df659b..73dcb632a3ce 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> @@ -1516,6 +1516,15 @@ static void gfx_v10_0_init_compute_vmid(struct amdgpu_device *adev)
>>    	}
>>    	nv_grbm_select(adev, 0, 0, 0, 0);
>>    	mutex_unlock(&adev->srbm_mutex);
>> +
>> +	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
>> +	   acccess. These should be enabled by FW for target VMIDs. */
>> +	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
>> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
>> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
>> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
>> +	}
>>    }
>>    
>>    static void gfx_v10_0_tcp_harvest(struct amdgpu_device *adev)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> index e1e2a44ee13c..3f98624772a4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>> @@ -1877,6 +1877,15 @@ static void gfx_v7_0_init_compute_vmid(struct amdgpu_device *adev)
>>    	}
>>    	cik_srbm_select(adev, 0, 0, 0, 0);
>>    	mutex_unlock(&adev->srbm_mutex);
>> +
>> +	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
>> +	   acccess. These should be enabled by FW for target VMIDs. */
>> +	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>> +		WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
>> +		WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
>> +		WREG32(amdgpu_gds_reg_offset[i].gws, 0);
>> +		WREG32(amdgpu_gds_reg_offset[i].oa, 0);
>> +	}
>>    }
>>    
>>    static void gfx_v7_0_config_init(struct amdgpu_device *adev)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> index 8c590a554675..e4028d54f8f7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> @@ -3702,6 +3702,15 @@ static void gfx_v8_0_init_compute_vmid(struct amdgpu_device *adev)
>>    	}
>>    	vi_srbm_select(adev, 0, 0, 0, 0);
>>    	mutex_unlock(&adev->srbm_mutex);
>> +
>> +	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
>> +	   acccess. These should be enabled by FW for target VMIDs. */
>> +	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>> +		WREG32(amdgpu_gds_reg_offset[i].mem_base, 0);
>> +		WREG32(amdgpu_gds_reg_offset[i].mem_size, 0);
>> +		WREG32(amdgpu_gds_reg_offset[i].gws, 0);
>> +		WREG32(amdgpu_gds_reg_offset[i].oa, 0);
>> +	}
>>    }
>>    
>>    static void gfx_v8_0_config_init(struct amdgpu_device *adev)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> index 5af60e1c735a..259a35395fec 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> @@ -2025,6 +2025,15 @@ static void gfx_v9_0_init_compute_vmid(struct amdgpu_device *adev)
>>    	}
>>    	soc15_grbm_select(adev, 0, 0, 0, 0);
>>    	mutex_unlock(&adev->srbm_mutex);
>> +
>> +	/* Initialize all compute VMIDs to have no GDS, GWS, or OA
>> +	   acccess. These should be enabled by FW for target VMIDs. */
>> +	for (i = FIRST_COMPUTE_VMID; i < LAST_COMPUTE_VMID; i++) {
>> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_BASE, 2 * i, 0);
>> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_VMID0_SIZE, 2 * i, 0);
>> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_GWS_VMID0, i, 0);
>> +		WREG32_SOC15_OFFSET(GC, 0, mmGDS_OA_VMID0, i, 0);
>> +	}
>>    }
>>    
>>    static void gfx_v9_0_constants_init(struct amdgpu_device *adev)
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx