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);
>>> }
>>> }
>>> }