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

["Greathouse, Joseph" <Joseph.Greathouse-5C7GfCeVMHo@public.gmane.org>]

> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Thursday, July 18, 2019 3:14 AM
> To: Kuehling, Felix <Felix.Kuehling@amd.com>; Greathouse, Joseph
> <Joseph.Greathouse@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH v2] drm/amdgpu: Default disable GDS for compute
> VMIDs
> 
> 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@amd.com>
> > Reviewed-by: Felix Kuehling <Felix.Kuehling@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@amd.com>.

Hmm, good point. It looks like graphics jobs will eventually call through to emit_gds_switch() to set these when launching a job, but it may be worthwhile to set these to zero as a default. I didn't want to step on any toes on the graphics side without checking first.

Do you have opinions on the most reasonable location to do this? early_init(), late_init()? The various gfx_v*_set_gds_init() might be a good place -- a quick test of setting all 16 VMIDs in gfx_v9_0_set_gds_init() appears to work fine on my Vega 20.

-Joe
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx