Am 2020-08-21 um 9:42 p.m. schrieb Huang Rui:
> On Sat, Aug 22, 2020 at 12:48:00AM +0800, Kuehling, Felix wrote:
>> Am 2020-08-21 um 8:50 a.m. schrieb Huang Rui:
>>> We still have a few iommu issues which need to address, so force raven
>>> as "dgpu" path for the moment.
>>>
>>> This is to add the fallback path to bypass IOMMU if IOMMU v2 is disabled
>>> or ACPI CRAT table not correct.
>>>
>>> v2: Use ignore_crat parameter to decide whether it will go with IOMMUv2.
>>> v3: Align with existed thunk, don't change the way of raven, only renoir
>>> will use "dgpu" path by default.
>>> v4: don't update global ignore_crat in the driver, and revise fallback
>>> function if CRAT is broken.
>>> v5: refine acpi crat good but no iommu support case, and rename the
>>> title.
>>>
>>> Signed-off-by: Huang Rui <ray.hu...@amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 5 ++-
>>> drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +-
>>> drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 22 ++++++++++--
>>> drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 4 +--
>>> drivers/gpu/drm/amd/amdkfd/kfd_device.c | 5 ++-
>>> .../amd/amdkfd/kfd_device_queue_manager_v9.c | 2 +-
>>> drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c | 4 +--
>>> drivers/gpu/drm/amd/amdkfd/kfd_iommu.c | 10 +++---
>>> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 5 ++-
>>> drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 36 +++++++++++++++++--
>>> 10 files changed, 76 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> index a9a4319c24ae..189f9d7e190d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> @@ -684,11 +684,14 @@ MODULE_PARM_DESC(debug_largebar,
>>> * Ignore CRAT table during KFD initialization. By default, KFD uses the
>>> ACPI CRAT
>>> * table to get information about AMD APUs. This option can serve as a
>>> workaround on
>>> * systems with a broken CRAT table.
>>> + *
>>> + * Default is auto (according to asic type, iommu_v2, and crat table, to
>>> decide
>>> + * whehter use CRAT)
>>> */
>>> int ignore_crat;
>>> module_param(ignore_crat, int, 0444);
>>> MODULE_PARM_DESC(ignore_crat,
>>> - "Ignore CRAT table during KFD initialization (0 = use CRAT (default), 1
>>> = ignore CRAT)");
>>> + "Ignore CRAT table during KFD initialization (0 = auto (default), 1 =
>>> ignore CRAT)");
>>>
>>> /**
>>> * DOC: halt_if_hws_hang (int)
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>> index 1b60e0ed6b5c..3ec5322ba88d 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>>> @@ -1258,7 +1258,7 @@ bool kfd_dev_is_large_bar(struct kfd_dev *dev)
>>> return true;
>>> }
>>>
>>> - if (dev->device_info->needs_iommu_device)
>>> + if (dev->use_iommu_v2)
>>> return false;
>>>
>>> amdgpu_amdkfd_get_local_mem_info(dev->kgd, &mem_info);
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
>>> index 59557e3e206a..aba40df189d9 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
>>> @@ -740,6 +740,25 @@ static int kfd_fill_gpu_cache_info(struct kfd_dev
>>> *kdev,
>>> return 0;
>>> }
>>>
>>> +
>>> +#ifdef CONFIG_ACPI
>>> +
>>> +static bool kfd_ignore_crat(void)
>>> +{
>>> + bool ret;
>>> +
>>> + if (ignore_crat)
>>> + return true;
>>> +
>>> +#ifndef KFD_SUPPORT_IOMMU_V2
>>> + ret = true;
>>> +#else
>>> + ret = false;
>>> +#endif
>>> +
>>> + return ret;
>>> +}
>>> +
>>> /*
>>> * kfd_create_crat_image_acpi - Allocates memory for CRAT image and
>>> * copies CRAT from ACPI (if available).
>>> @@ -751,7 +770,6 @@ static int kfd_fill_gpu_cache_info(struct kfd_dev *kdev,
>>> *
>>> * Return 0 if successful else return error code
>>> */
>>> -#ifdef CONFIG_ACPI
>>> int kfd_create_crat_image_acpi(void **crat_image, size_t *size)
>>> {
>>> struct acpi_table_header *crat_table;
>>> @@ -775,7 +793,7 @@ int kfd_create_crat_image_acpi(void **crat_image,
>>> size_t *size)
>>> return -EINVAL;
>>> }
>>>
>>> - if (ignore_crat) {
>>> + if (kfd_ignore_crat()) {
>>> pr_info("CRAT table disabled by module option\n");
>>> return -ENODATA;
>>> }
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>>> index 3e5904f8876a..e4159b5f83d5 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
>>> @@ -309,7 +309,7 @@ static int dbgdev_address_watch_nodiq(struct kfd_dbgdev
>>> *dbgdev,
>>> for (i = 0; i < adw_info->num_watch_points; i++) {
>>> dbgdev_address_watch_set_registers(adw_info, &addrHi, &addrLo,
>>> &cntl, i, pdd->qpd.vmid,
>>> - dbgdev->dev->device_info->needs_iommu_device);
>>> + dbgdev->dev->use_iommu_v2);
>>>
>>> pr_debug("\t\t%30s\n", "* * * * * * * * * * * * * * * * * *");
>>> pr_debug("\t\t%20s %08x\n", "register index :", i);
>>> @@ -399,7 +399,7 @@ static int dbgdev_address_watch_diq(struct kfd_dbgdev
>>> *dbgdev,
>>> for (i = 0; i < adw_info->num_watch_points; i++) {
>>> dbgdev_address_watch_set_registers(adw_info, &addrHi, &addrLo,
>>> &cntl, i, vmid,
>>> - dbgdev->dev->device_info->needs_iommu_device);
>>> + dbgdev->dev->use_iommu_v2);
>>>
>>> pr_debug("\t\t%30s\n", "* * * * * * * * * * * * * * * * * *");
>>> pr_debug("\t\t%20s %08x\n", "register index :", i);
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> index 2c030c2b5b8d..fdf64d361be3 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>> @@ -112,6 +112,7 @@ static const struct kfd_device_info carrizo_device_info
>>> = {
>>> .num_xgmi_sdma_engines = 0,
>>> .num_sdma_queues_per_engine = 2,
>>> };
>>> +#endif
>>>
>>> static const struct kfd_device_info raven_device_info = {
>>> .asic_family = CHIP_RAVEN,
>>> @@ -130,7 +131,6 @@ static const struct kfd_device_info raven_device_info =
>>> {
>>> .num_xgmi_sdma_engines = 0,
>>> .num_sdma_queues_per_engine = 2,
>>> };
>>> -#endif
>>>
>>> static const struct kfd_device_info hawaii_device_info = {
>>> .asic_family = CHIP_HAWAII,
>>> @@ -688,6 +688,9 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
>>> goto gws_error;
>>> }
>>>
>>> + /* If CRAT is broken, won't set iommu enabled */
>>> + kfd_double_confirm_iommu_support(kfd);
>>> +
>>> if (kfd_iommu_device_init(kfd)) {
>>> dev_err(kfd_device, "Error initializing iommuv2\n");
>>> goto device_iommu_error;
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_v9.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_v9.c
>>> index 95a82ac455f2..309f63a0b34a 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_v9.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_v9.c
>>> @@ -62,7 +62,7 @@ static int update_qpd_v9(struct device_queue_manager *dqm,
>>> SH_MEM_ALIGNMENT_MODE_UNALIGNED <<
>>> SH_MEM_CONFIG__ALIGNMENT_MODE__SHIFT;
>>> if (amdgpu_noretry &&
>>> - !dqm->dev->device_info->needs_iommu_device)
>>> + !dqm->dev->use_iommu_v2)
>>> qpd->sh_mem_config |=
>>> 1 << SH_MEM_CONFIG__RETRY_DISABLE__SHIFT;
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c
>>> index 78714f9a8b11..798303c3c7ac 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_flat_memory.c
>>> @@ -321,7 +321,7 @@ static void kfd_init_apertures_vi(struct
>>> kfd_process_device *pdd, uint8_t id)
>>> pdd->lds_base = MAKE_LDS_APP_BASE_VI();
>>> pdd->lds_limit = MAKE_LDS_APP_LIMIT(pdd->lds_base);
>>>
>>> - if (!pdd->dev->device_info->needs_iommu_device) {
>>> + if (!pdd->dev->use_iommu_v2) {
>>> /* dGPUs: SVM aperture starting at 0
>>> * with small reserved space for kernel.
>>> * Set them to CANONICAL addresses.
>>> @@ -423,7 +423,7 @@ int kfd_init_apertures(struct kfd_process *process)
>>> return -EINVAL;
>>> }
>>>
>>> - if (!dev->device_info->needs_iommu_device) {
>>> + if (!dev->use_iommu_v2) {
>>> /* dGPUs: the reserved space for kernel
>>> * before SVM
>>> */
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
>>> index 4d3b4188b9a1..0c0269497e51 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_iommu.c
>>> @@ -41,7 +41,7 @@ int kfd_iommu_check_device(struct kfd_dev *kfd)
>>> struct amd_iommu_device_info iommu_info;
>>> int err;
>>>
>>> - if (!kfd->device_info->needs_iommu_device)
>>> + if (!kfd->use_iommu_v2)
>>> return -ENODEV;
>>>
>>> iommu_info.flags = 0;
>>> @@ -63,7 +63,7 @@ int kfd_iommu_device_init(struct kfd_dev *kfd)
>>> unsigned int pasid_limit;
>>> int err;
>>>
>>> - if (!kfd->device_info->needs_iommu_device)
>>> + if (!kfd->use_iommu_v2)
>>> return 0;
>>>
>>> iommu_info.flags = 0;
>>> @@ -109,7 +109,7 @@ int kfd_iommu_bind_process_to_device(struct
>>> kfd_process_device *pdd)
>>> struct kfd_process *p = pdd->process;
>>> int err;
>>>
>>> - if (!dev->device_info->needs_iommu_device || pdd->bound == PDD_BOUND)
>>> + if (!dev->use_iommu_v2 || pdd->bound == PDD_BOUND)
>>> return 0;
>>>
>>> if (unlikely(pdd->bound == PDD_BOUND_SUSPENDED)) {
>>> @@ -284,7 +284,7 @@ static void kfd_unbind_processes_from_device(struct
>>> kfd_dev *kfd)
>>> */
>>> void kfd_iommu_suspend(struct kfd_dev *kfd)
>>> {
>>> - if (!kfd->device_info->needs_iommu_device)
>>> + if (!kfd->use_iommu_v2)
>>> return;
>>>
>>> kfd_unbind_processes_from_device(kfd);
>>> @@ -304,7 +304,7 @@ int kfd_iommu_resume(struct kfd_dev *kfd)
>>> unsigned int pasid_limit;
>>> int err;
>>>
>>> - if (!kfd->device_info->needs_iommu_device)
>>> + if (!kfd->use_iommu_v2)
>>> return 0;
>>>
>>> pasid_limit = kfd_get_pasid_limit();
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> index 60243798cce2..641b964387ec 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> @@ -308,12 +308,14 @@ struct kfd_dev {
>>>
>>> /* xGMI */
>>> uint64_t hive_id;
>>> -
>>> /* UUID */
>>> uint64_t unique_id;
>>>
>>> bool pci_atomic_requested;
>>>
>>> + /* Use IOMMU v2 flag */
>>> + bool use_iommu_v2;
>>> +
>>> /* SRAM ECC flag */
>>> atomic_t sram_ecc_flag;
>>>
>>> @@ -1009,6 +1011,7 @@ struct kfd_dev *kfd_device_by_pci_dev(const struct
>>> pci_dev *pdev);
>>> struct kfd_dev *kfd_device_by_kgd(const struct kgd_dev *kgd);
>>> int kfd_topology_enum_kfd_devices(uint8_t idx, struct kfd_dev **kdev);
>>> int kfd_numa_node_to_apic_id(int numa_node_id);
>>> +void kfd_double_confirm_iommu_support(struct kfd_dev *gpu);
>>>
>>> /* Interrupts */
>>> int kfd_interrupt_init(struct kfd_dev *dev);
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>>> index cbb8535abf0c..d141f726d037 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>>> @@ -545,7 +545,7 @@ static ssize_t node_show(struct kobject *kobj, struct
>>> attribute *attr,
>>> * If the ASIC is APU except Kaveri, set local memory size
>>> * to 0 to disable local memory support
>>> */
>>> - if (!dev->gpu->device_info->needs_iommu_device
>>> + if (!dev->gpu->use_iommu_v2
>>> || dev->gpu->device_info->asic_family == CHIP_KAVERI) {
>>> amdgpu_amdkfd_get_local_mem_info(dev->gpu->kgd,
>>> &local_mem_info);
>>> @@ -1197,7 +1197,7 @@ static struct kfd_topology_device
>>> *kfd_assign_gpu(struct kfd_dev *gpu)
>>> /* Discrete GPUs need their own topology device list
>>> * entries. Don't assign them to CPU/APU nodes.
>>> */
>>> - if (!gpu->device_info->needs_iommu_device &&
>>> + if (!gpu->use_iommu_v2 &&
>>> dev->node_props.cpu_cores_count)
>>> continue;
>>>
>>> @@ -1452,7 +1452,7 @@ int kfd_topology_add_device(struct kfd_dev *gpu)
>>> * Overwrite ATS capability according to needs_iommu_device to fix
>>> * potential missing corresponding bit in CRAT of BIOS.
>>> */
>>> - if (dev->gpu->device_info->needs_iommu_device)
>>> + if (dev->gpu->use_iommu_v2)
>>> dev->node_props.capability |= HSA_CAP_ATS_PRESENT;
>>> else
>>> dev->node_props.capability &= ~HSA_CAP_ATS_PRESENT;
>>> @@ -1579,6 +1579,36 @@ int kfd_numa_node_to_apic_id(int numa_node_id)
>>> return kfd_cpumask_to_apic_id(cpumask_of_node(numa_node_id));
>>> }
>>>
>>> +void kfd_double_confirm_iommu_support(struct kfd_dev *gpu)
>>> +{
>>> + struct kfd_topology_device *dev;
>>> +
>>> + gpu->use_iommu_v2 = false;
>>> +
>>> + down_read(&topology_lock);
>>> +
>>> + /* Only use IOMMUv2 if there is an APU topology node with no GPU
>>> + * assigned yet. This GPU will be assigned to it.
>>> + */
>>> + list_for_each_entry(dev, &topology_device_list, list) {
>>> + if (dev->node_props.cpu_cores_count &&
>>> + dev->node_props.simd_count &&
>>> + !dev->gpu) {
>>> + /* If device_info indicates no iommu support, force
>>> + * simd_count as 0 to tell user mode no GPU in the APU
>>> + * node.
>>> + */
>>> + if (!gpu->device_info->needs_iommu_device)
>>> + dev->node_props.simd_count = 0;
>> This assumes that the APU's built-in GPU is the first to be initialized.
>> If there is a dGPU and that gets initialized first, you will set
>> dev->node_props.simd_count = 0 here, which will force the APU to run in
>> dGPU mode.
>>
>> Maybe that's intentional, but it seems weird that this depends on the
>> order in which GPUs are initialized. That order may not be stable
>> between reboots, so relying on that can lead to unexpected behaviour and
>> confusing bug reports.
>>
> This is to fix the case if ACPI CRAT is good on Renoir. If ACPI CRAT good
> by default on Renoir, we will initialize the APU node at that time. If it
> checked needs_iommu_device here, then we can report the simd_count as 0 to
> user mode to treat this node as pure CPU. This is in spired by below
> comments:
>
>>> I'd suggest trying a more general solution in node_show in
>>> kfd_topology.c. If we see an APU node (that has CPU and GPU cores) with
>>> no associated GPU (dev->gpu_id is 0 or dev->gpu is NULL), we can report
>>> it as a pure CPU node to user mode by just reporting the simd_count as 0.
The best place to do that without depending on the order in which GPUs
are initialized is in node_show:
sysfs_show_32bit_prop(buffer, offs, "simd_count",
dev->gpu ? dev->node_props.simd_count : 0);
Regards,
Felix
> Thanks,
> Ray
>
>> Regards,
>> Felix
>>
>>
>>> + else
>>> + gpu->use_iommu_v2 = true;
>>> + break;
>>> + }
>>> + }
>>> +
>>> + up_read(&topology_lock);
>>> +}
>>> +
>>> #if defined(CONFIG_DEBUG_FS)
>>>
>>> int kfd_debugfs_hqds_by_device(struct seq_file *m, void *data)
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
