On 2026-10-15 03:16, Christian König wrote:
> On 10/14/26 17:19, Benjamin Cheng wrote:
>> On 2026-10-14 11:05, Benjamin Cheng wrote:
>>> Each vmhub has it's own vmid to pasid mapping, but the code was assuming
>>> that a gfxhub vmid would also apply to mmhub and flushing mmhub using
>>> the gfxhub vmids.
>>>
>>> Instead we need to find the vmid mapping for each hub individually and
>>> flush with that.
>>
>> gmc_v{9,10}_0 have a similar issue, but at least by my reading they won't be 
>> affected since only KFD uses it there.
>>
>> gmc_v12_1 might need a similar fix, but I'm not sure how it plays out with 
>> the IH_VMID_LUT_INDEX. I'm looking for some advice on that.
> As far as I can see the fix is currently unnecessary on gfx11 and gfx12 and 
> only academically interesting on gfx9.
>
> The per PASID TLB invalidation is only necessary for VMHUBs which are used 
> together with userqueues, e.g. the classic KFD user queues or the new ones 
> for starting with gfx11.
>
> Both variants only support GFX, Compute and SDMA queues at the moment, so 
> only on gfx9 where the SDMA is connected to the MMHUB we actually need to 
> invalidate the MMHUB as well.
>
> Now on gfx9 the HWS always uses the same VMID for all VMHUBs, so the current 
> handling is actually correct. It is just extremely awkward and not documented 
> at all.


Right, I had this train of thought as well, thus why I did not send any patches 
for gfx9.


> We could argue that we are going to need this when the MM engines start using 
> userqueues as well, but a) I currently don't see that in the near future and 
> b) we then need to use the MES for the invalidation.


I agree that without the MM engines using userqueues this doesn't fix any 
functionality.

But currently all_hub=true for non-KFD cases, so we are unnecessarily flushing 
MMHUB when gfx userqueues are enabled, and also I'd argue the code is "more 
correct" with such a fix?

I am doing some internal experiments with UMSCH on GFX11 which would require 
this, but I can keep this local if preferred (won't be upstreamed in the near 
future, as you said).


> Talking about MES we should potentially remove this code altogether for gfx12 
> and rather make sure that we always use the MES packet to do this.
>
> Regards,
> Christian.
>
>>
>>> Signed-off-by: Benjamin Cheng <[email protected]>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c | 47 ++++++++++++++++----------
>>>  1 file changed, 30 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c 
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
>>> index 709c3d1b9998..715ccce8e4eb 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v11_0.c
>>> @@ -204,10 +204,25 @@ static bool gmc_v11_0_use_invalidate_semaphore(struct 
>>> amdgpu_device *adev,
>>>  
>>>  static bool gmc_v11_0_get_vmid_pasid_mapping_info(
>>>                                     struct amdgpu_device *adev,
>>> -                                   uint8_t vmid, uint16_t *p_pasid)
>>> +                                   uint32_t vmhub, uint8_t vmid,
>>> +                                   uint16_t *p_pasid)
>>>  {
>>> -   *p_pasid = RREG32(SOC15_REG_OFFSET(OSSSYS, 0, regIH_VMID_0_LUT) + vmid) 
>>> & 0xffff;
>>> +   int reg;
>>>  
>>> +   switch (vmhub) {
>>> +   case AMDGPU_GFXHUB(0):
>>> +           reg = SOC15_REG_OFFSET(OSSSYS, 0, regIH_VMID_0_LUT);
>>> +           break;
>>> +   case AMDGPU_MMHUB0(0):
>>> +           reg = SOC15_REG_OFFSET(OSSSYS, 0, regIH_VMID_0_LUT_MM);
>>> +           break;
>>> +   default:
>>> +           /* There shouldn't be any other VMHUB. */
>>> +           WARN_ON_ONCE(true);
>>> +           return false;
>>> +   }
>>> +
>>> +   *p_pasid = RREG32(reg + vmid) & 0xffff;
>>>     return !!(*p_pasid);
>>>  }
>>>  
>>> @@ -330,24 +345,22 @@ static void gmc_v11_0_flush_gpu_tlb_pasid(struct 
>>> amdgpu_device *adev,
>>>                                       bool all_hub, uint32_t inst)
>>>  {
>>>     uint16_t queried;
>>> -   int vmid, i;
>>> +   int vmid, hub;
>>>  
>>> -   for (vmid = 1; vmid < 16; vmid++) {
>>> -           bool valid;
>>> -
>>> -           valid = gmc_v11_0_get_vmid_pasid_mapping_info(adev, vmid,
>>> -                                                         &queried);
>>> -           if (!valid || queried != pasid)
>>> +   for_each_set_bit(hub, adev->vmhubs_mask, AMDGPU_MAX_VMHUBS) {
>>> +           if (!all_hub && hub != AMDGPU_GFXHUB(0))
>>>                     continue;
>>>  
>>> -           if (all_hub) {
>>> -                   for_each_set_bit(i, adev->vmhubs_mask,
>>> -                                    AMDGPU_MAX_VMHUBS)
>>> -                           gmc_v11_0_flush_gpu_tlb(adev, vmid, i,
>>> -                                                   flush_type);
>>> -           } else {
>>> -                   gmc_v11_0_flush_gpu_tlb(adev, vmid, AMDGPU_GFXHUB(0),
>>> -                                           flush_type);
>>> +           for (vmid = 1; vmid < 16; vmid++) {
>>> +                   bool valid;
>>> +
>>> +                   valid = gmc_v11_0_get_vmid_pasid_mapping_info(adev, hub,
>>> +                                                                 vmid,
>>> +                                                                 &queried);
>>> +                   if (!valid || queried != pasid)
>>> +                           continue;
>>> +
>>> +                   gmc_v11_0_flush_gpu_tlb(adev, vmid, hub, flush_type);
>>>             }
>>>     }
>>>  }

Reply via email to