[PATCH v2] drm/amdgpu: set vm_update_mode=0 as default for Sienna Cichlid in SRIOV case

[Slivka, Danijel]

[AMD Official Use Only - General]

Thank you for the input.

Regarding function amdgpu_virt_mmio_blocked, this is not returning correct results as it is checking if read operation is successful,
which for VF MMIO access protection is always allowed.

Regarding other suggestion to set this in some capability flag,  this flag should be set after VF MMIO access protection is enabled,
after driver is amdgpu driver is loaded,  thus it would require reinitialization of vm instances, to set vm_update function to sdma instead of cpu.

Would it be better to keep this fix in amdgpu_vm_manager_init() when initially setting the sm_update_mode?

BR,
Danijel

-----Original Message-----
From: Kuehling, Felix <Felix.Kuehling at amd.com>
Sent: Wednesday, October 5, 2022 6:43 PM
To: amd-gfx at lists.freedesktop.org; Slivka, Danijel <Danijel.Slivka at amd.com>
Subject: Re: [PATCH v2] drm/amdgpu: set vm_update_mode=0 as default for Sienna Cichlid in SRIOV case

Am 2022-10-05 um 07:03 schrieb Danijel Slivka:
> CPU pagetable updates have issues with HDP flush as VF MMIO access
> protection is not allowing write during sriov runtime to
> mmBIF_BX_DEV0_EPF0_VF0_HDP_MEM_COHERENCY_FLUSH_CNTL
>
> Signed-off-by: Danijel Slivka <danijel.slivka at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 83b0c5d86e48..32088ac0666c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2338,7 +2338,9 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
>        */
>   #ifdef CONFIG_X86_64
>       if (amdgpu_vm_update_mode == -1) {
> -             if (amdgpu_gmc_vram_full_visible(&adev->gmc))
> +             if (amdgpu_gmc_vram_full_visible(&adev->gmc) &&
> +                 !(adev->asic_type == CHIP_SIENNA_CICHLID &&
> +                 amdgpu_sriov_vf(adev)))

This would need at least a code comment. But I'd prefer a more general solution that expresses that some ASICs don't allow any MMIO access under SRIOV.

I found that there is this function defined in amdgpu_virt.c/h: bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev). Would this return the correct result and could you use it here instead of a hard-coded asic_type?

Or maybe this could be added as a flag in (adev)->virt.caps and get initialized in some ASIC-specific code path.

Regards,
   Felix


>                       adev->vm_manager.vm_update_mode =
>                               AMDGPU_VM_USE_CPU_FOR_COMPUTE;
>               else