[PATCH 2/2] drm/amdgpu: cleanup PTE flag generation
Christian König
ckoenig.leichtzumerken at gmail.com
Wed Sep 4 07:50:42 UTC 2019
Am 03.09.19 um 22:28 schrieb Kuehling, Felix:
> On 2019-09-02 10:57 a.m., Christian König wrote:
>> Move the ASIC specific code into a new callback function.
> NAK. I believe the BUG_ONs you're adding will trigger with ROCm on
> Hawaii and Kaveri. See inline ...
Those are the flags as generated by the TTM code, they shouldn't contain
the executable bit.
The mapping->flags are the one generated by ROCm.
> ROCm user mode doesn't care that the page table doesn't have an
> executable bit on those chips. If the HW makes all memory executable, we
> should just ignore the flag.
I've added this because I'm not sure how the hardware reacts when it
sees a reserved bit set.
The alternative is to just clear the flag, because it isn't really
supported in any way.
Christian.
>
>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 5 +++++
>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 29 ++-----------------------
>> drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 22 ++++++++++++++++++-
>> drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 9 ++++++++
>> drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 11 +++++++++-
>> drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 13 ++++++++++-
>> drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 24 +++++++++++++++++++-
>> 7 files changed, 82 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> index d5e4574afbc2..d3be51ba6349 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
>> @@ -104,6 +104,10 @@ struct amdgpu_gmc_funcs {
>> /* get the pde for a given mc addr */
>> void (*get_vm_pde)(struct amdgpu_device *adev, int level,
>> u64 *dst, u64 *flags);
>> + /* get the pte flags to use for a BO VA mapping */
>> + void (*get_vm_pte)(struct amdgpu_device *adev,
>> + struct amdgpu_bo_va_mapping *mapping,
>> + uint64_t *flags);
>> };
>>
>> struct amdgpu_xgmi {
>> @@ -185,6 +189,7 @@ struct amdgpu_gmc {
>> #define amdgpu_gmc_emit_pasid_mapping(r, vmid, pasid) (r)->adev->gmc.gmc_funcs->emit_pasid_mapping((r), (vmid), (pasid))
>> #define amdgpu_gmc_map_mtype(adev, flags) (adev)->gmc.gmc_funcs->map_mtype((adev),(flags))
>> #define amdgpu_gmc_get_vm_pde(adev, level, dst, flags) (adev)->gmc.gmc_funcs->get_vm_pde((adev), (level), (dst), (flags))
>> +#define amdgpu_gmc_get_vm_pte(adev, mapping, flags) (adev)->gmc.gmc_funcs->get_vm_pte((adev), (mapping), (flags))
>>
>> /**
>> * amdgpu_gmc_vram_full_visible - Check if full VRAM is visible through the BAR
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 2cb82b229802..b285ab25146d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1571,33 +1571,8 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>> if (!(mapping->flags & AMDGPU_PTE_WRITEABLE))
>> flags &= ~AMDGPU_PTE_WRITEABLE;
>>
>> - if (adev->asic_type >= CHIP_TONGA) {
>> - flags &= ~AMDGPU_PTE_EXECUTABLE;
>> - flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> - }
>> -
>> - if (adev->asic_type >= CHIP_NAVI10) {
>> - flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
>> - flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
>> - } else {
>> - flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
>> - flags |= (mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK);
>> - }
>> -
>> - if ((mapping->flags & AMDGPU_PTE_PRT) &&
>> - (adev->asic_type >= CHIP_VEGA10)) {
>> - flags |= AMDGPU_PTE_PRT;
>> - if (adev->asic_type >= CHIP_NAVI10) {
>> - flags |= AMDGPU_PTE_SNOOPED;
>> - flags |= AMDGPU_PTE_LOG;
>> - flags |= AMDGPU_PTE_SYSTEM;
>> - }
>> - flags &= ~AMDGPU_PTE_VALID;
>> - }
>> - if (adev->asic_type == CHIP_ARCTURUS &&
>> - !(flags & AMDGPU_PTE_SYSTEM) &&
>> - mapping->bo_va->is_xgmi)
>> - flags |= AMDGPU_PTE_SNOOPED;
>> + /* Apply ASIC specific mapping flags */
>> + amdgpu_gmc_get_vm_pte(adev, mapping, &flags);
>>
>> trace_amdgpu_vm_bo_update(mapping);
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> index 7eb0ba87fef9..1a8d8f528b01 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> @@ -440,12 +440,32 @@ static void gmc_v10_0_get_vm_pde(struct amdgpu_device *adev, int level,
>> }
>> }
>>
>> +static void gmc_v10_0_get_vm_pte(struct amdgpu_device *adev,
>> + struct amdgpu_bo_va_mapping *mapping,
>> + uint64_t *flags)
>> +{
>> + *flags &= ~AMDGPU_PTE_EXECUTABLE;
>> + *flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +
>> + *flags &= ~AMDGPU_PTE_MTYPE_NV10_MASK;
>> + *flags |= (mapping->flags & AMDGPU_PTE_MTYPE_NV10_MASK);
>> +
>> + if (mapping->flags & AMDGPU_PTE_PRT) {
>> + *flags |= AMDGPU_PTE_PRT;
>> + *flags |= AMDGPU_PTE_SNOOPED;
>> + *flags |= AMDGPU_PTE_LOG;
>> + *flags |= AMDGPU_PTE_SYSTEM;
>> + *flags &= ~AMDGPU_PTE_VALID;
>> + }
>> +}
>> +
>> static const struct amdgpu_gmc_funcs gmc_v10_0_gmc_funcs = {
>> .flush_gpu_tlb = gmc_v10_0_flush_gpu_tlb,
>> .emit_flush_gpu_tlb = gmc_v10_0_emit_flush_gpu_tlb,
>> .emit_pasid_mapping = gmc_v10_0_emit_pasid_mapping,
>> .map_mtype = gmc_v10_0_map_mtype,
>> - .get_vm_pde = gmc_v10_0_get_vm_pde
>> + .get_vm_pde = gmc_v10_0_get_vm_pde,
>> + .get_vm_pte = gmc_v10_0_get_vm_pte
>> };
>>
>> static void gmc_v10_0_set_gmc_funcs(struct amdgpu_device *adev)
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> index 2e305b21db69..ce591c995691 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> @@ -389,6 +389,14 @@ static void gmc_v6_0_get_vm_pde(struct amdgpu_device *adev, int level,
>> BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>> }
>>
>> +static void gmc_v6_0_get_vm_pte(struct amdgpu_device *adev,
>> + struct amdgpu_bo_va_mapping *mapping,
>> + uint64_t *flags)
>> +{
>> + BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
>> + BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +}
>> +
>> static void gmc_v6_0_set_fault_enable_default(struct amdgpu_device *adev,
>> bool value)
>> {
>> @@ -1144,6 +1152,7 @@ static const struct amdgpu_gmc_funcs gmc_v6_0_gmc_funcs = {
>> .emit_flush_gpu_tlb = gmc_v6_0_emit_flush_gpu_tlb,
>> .set_prt = gmc_v6_0_set_prt,
>> .get_vm_pde = gmc_v6_0_get_vm_pde,
>> + .get_vm_pte = gmc_v6_0_get_vm_pte,
>> };
>>
>> static const struct amdgpu_irq_src_funcs gmc_v6_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> index 3b77421234a7..e3f53fbfcd8f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> @@ -466,6 +466,14 @@ static void gmc_v7_0_get_vm_pde(struct amdgpu_device *adev, int level,
>> BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>> }
>>
>> +static void gmc_v7_0_get_vm_pte(struct amdgpu_device *adev,
>> + struct amdgpu_bo_va_mapping *mapping,
>> + uint64_t *flags)
>> +{
>> + BUG_ON(*flags & AMDGPU_PTE_EXECUTABLE);
> NAK. This is going to break ROCm on Kaveri and Hawaii.
>
> Regards,
> Felix
>
>
>> + BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +}
>> +
>> /**
>> * gmc_v8_0_set_fault_enable_default - update VM fault handling
>> *
>> @@ -1339,7 +1347,8 @@ static const struct amdgpu_gmc_funcs gmc_v7_0_gmc_funcs = {
>> .emit_flush_gpu_tlb = gmc_v7_0_emit_flush_gpu_tlb,
>> .emit_pasid_mapping = gmc_v7_0_emit_pasid_mapping,
>> .set_prt = gmc_v7_0_set_prt,
>> - .get_vm_pde = gmc_v7_0_get_vm_pde
>> + .get_vm_pde = gmc_v7_0_get_vm_pde,
>> + .get_vm_pte = gmc_v7_0_get_vm_pte
>> };
>>
>> static const struct amdgpu_irq_src_funcs gmc_v7_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> index 58444aa0af05..256d1faacada 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> @@ -689,6 +689,16 @@ static void gmc_v8_0_get_vm_pde(struct amdgpu_device *adev, int level,
>> BUG_ON(*addr & 0xFFFFFF0000000FFFULL);
>> }
>>
>> +static void gmc_v8_0_get_vm_pte(struct amdgpu_device *adev,
>> + struct amdgpu_bo_va_mapping *mapping,
>> + uint64_t *flags)
>> +{
>> + BUG_ON(*flags & AMDGPU_PTE_PRT);
>> +
>> + *flags &= ~AMDGPU_PTE_EXECUTABLE;
>> + *flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +}
>> +
>> /**
>> * gmc_v8_0_set_fault_enable_default - update VM fault handling
>> *
>> @@ -1705,7 +1715,8 @@ static const struct amdgpu_gmc_funcs gmc_v8_0_gmc_funcs = {
>> .emit_flush_gpu_tlb = gmc_v8_0_emit_flush_gpu_tlb,
>> .emit_pasid_mapping = gmc_v8_0_emit_pasid_mapping,
>> .set_prt = gmc_v8_0_set_prt,
>> - .get_vm_pde = gmc_v8_0_get_vm_pde
>> + .get_vm_pde = gmc_v8_0_get_vm_pde,
>> + .get_vm_pte = gmc_v8_0_get_vm_pte
>> };
>>
>> static const struct amdgpu_irq_src_funcs gmc_v8_0_irq_funcs = {
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index 22660e1005db..b3d2d70e84c9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -625,12 +625,34 @@ static void gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, int level,
>> }
>> }
>>
>> +static void gmc_v9_0_get_vm_pte(struct amdgpu_device *adev,
>> + struct amdgpu_bo_va_mapping *mapping,
>> + uint64_t *flags)
>> +{
>> + *flags &= ~AMDGPU_PTE_EXECUTABLE;
>> + *flags |= mapping->flags & AMDGPU_PTE_EXECUTABLE;
>> +
>> + *flags &= ~AMDGPU_PTE_MTYPE_VG10_MASK;
>> + *flags |= mapping->flags & AMDGPU_PTE_MTYPE_VG10_MASK;
>> +
>> + if (mapping->flags & AMDGPU_PTE_PRT) {
>> + *flags |= AMDGPU_PTE_PRT;
>> + *flags &= ~AMDGPU_PTE_VALID;
>> + }
>> +
>> + if (adev->asic_type == CHIP_ARCTURUS &&
>> + !(*flags & AMDGPU_PTE_SYSTEM) &&
>> + mapping->bo_va->is_xgmi)
>> + *flags |= AMDGPU_PTE_SNOOPED;
>> +}
>> +
>> static const struct amdgpu_gmc_funcs gmc_v9_0_gmc_funcs = {
>> .flush_gpu_tlb = gmc_v9_0_flush_gpu_tlb,
>> .emit_flush_gpu_tlb = gmc_v9_0_emit_flush_gpu_tlb,
>> .emit_pasid_mapping = gmc_v9_0_emit_pasid_mapping,
>> .map_mtype = gmc_v9_0_map_mtype,
>> - .get_vm_pde = gmc_v9_0_get_vm_pde
>> + .get_vm_pde = gmc_v9_0_get_vm_pde,
>> + .get_vm_pte = gmc_v9_0_get_vm_pte
>> };
>>
>> static void gmc_v9_0_set_gmc_funcs(struct amdgpu_device *adev)
More information about the amd-gfx
mailing list