[PATCH 3/9] drm/amdkfd: Make IOMMUv2 code conditional

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 31, 2018 at 4:56 PM, Oded Gabbay <oded.gabbay at gmail.com> wrote:
> Hi Felix,
> Please don't spread 19 #ifdefs throughout the code.
> I suggest to put one #ifdef in linux/amd-iommu.h itself around all the
> functions declarations and in the #else section put macros with empty
> implementations. This is much more readable and maintainable.
>
> Oded

To emphasize my point, there is a call to amd_iommu_bind_pasid in
kfd_bind_processes_to_device() which isn't wrapped with the #ifdef so
the compliation breaks. Putting the #ifdefs around the calls is simply
not scalable.

Oded

>
>
> On Fri, Jan 5, 2018 at 12:17 AM, Felix Kuehling <Felix.Kuehling at amd.com> wrote:
>> dGPUs work without IOMMUv2. Make IOMMUv2 initialization dependent on
>> ASIC information. Also allow building KFD without IOMMUv2 support.
>> This is still useful for dGPUs and prepares for enabling KFD on
>> architectures that don't support AMD IOMMUv2.
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdkfd/Kconfig        |  2 +-
>>  drivers/gpu/drm/amd/amdkfd/kfd_crat.c     |  8 +++-
>>  drivers/gpu/drm/amd/amdkfd/kfd_device.c   | 62 +++++++++++++++++++++----------
>>  drivers/gpu/drm/amd/amdkfd/kfd_events.c   |  2 +
>>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h     |  5 +++
>>  drivers/gpu/drm/amd/amdkfd/kfd_process.c  | 17 ++++++---
>>  drivers/gpu/drm/amd/amdkfd/kfd_topology.c |  2 +
>>  drivers/gpu/drm/amd/amdkfd/kfd_topology.h |  2 +
>>  8 files changed, 74 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/Kconfig b/drivers/gpu/drm/amd/amdkfd/Kconfig
>> index bc5a294..5bbeb95 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/Kconfig
>> +++ b/drivers/gpu/drm/amd/amdkfd/Kconfig
>> @@ -4,6 +4,6 @@
>>
>>  config HSA_AMD
>>         tristate "HSA kernel driver for AMD GPU devices"
>> -       depends on DRM_AMDGPU && AMD_IOMMU_V2 && X86_64
>> +       depends on DRM_AMDGPU && X86_64
>>         help
>>           Enable this if you want to use HSA features on AMD GPU devices.
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
>> index 2bc2816..3478270 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
>> @@ -22,7 +22,9 @@
>>
>>  #include <linux/pci.h>
>>  #include <linux/acpi.h>
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>  #include <linux/amd-iommu.h>
>> +#endif
>>  #include "kfd_crat.h"
>>  #include "kfd_priv.h"
>>  #include "kfd_topology.h"
>> @@ -1037,15 +1039,17 @@ static int kfd_create_vcrat_image_gpu(void *pcrat_image,
>>         struct crat_subtype_generic *sub_type_hdr;
>>         struct crat_subtype_computeunit *cu;
>>         struct kfd_cu_info cu_info;
>> -       struct amd_iommu_device_info iommu_info;
>>         int avail_size = *size;
>>         uint32_t total_num_of_cu;
>>         int num_of_cache_entries = 0;
>>         int cache_mem_filled = 0;
>>         int ret = 0;
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>> +       struct amd_iommu_device_info iommu_info;
>>         const u32 required_iommu_flags = AMD_IOMMU_DEVICE_FLAG_ATS_SUP |
>>                                          AMD_IOMMU_DEVICE_FLAG_PRI_SUP |
>>                                          AMD_IOMMU_DEVICE_FLAG_PASID_SUP;
>> +#endif
>>         struct kfd_local_mem_info local_mem_info;
>>
>>         if (!pcrat_image || avail_size < VCRAT_SIZE_FOR_GPU)
>> @@ -1106,12 +1110,14 @@ static int kfd_create_vcrat_image_gpu(void *pcrat_image,
>>         /* Check if this node supports IOMMU. During parsing this flag will
>>          * translate to HSA_CAP_ATS_PRESENT
>>          */
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>         iommu_info.flags = 0;
>>         if (amd_iommu_device_info(kdev->pdev, &iommu_info) == 0) {
>>                 if ((iommu_info.flags & required_iommu_flags) ==
>>                                 required_iommu_flags)
>>                         cu->hsa_capability |= CRAT_CU_FLAGS_IOMMU_PRESENT;
>>         }
>> +#endif
>>
>>         crat_table->length += sub_type_hdr->length;
>>         crat_table->total_entries++;
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>> index fafe971..5205b34 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>> @@ -20,7 +20,9 @@
>>   * OTHER DEALINGS IN THE SOFTWARE.
>>   */
>>
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>  #include <linux/amd-iommu.h>
>> +#endif
>>  #include <linux/bsearch.h>
>>  #include <linux/pci.h>
>>  #include <linux/slab.h>
>> @@ -31,6 +33,7 @@
>>
>>  #define MQD_SIZE_ALIGNED 768
>>
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>  static const struct kfd_device_info kaveri_device_info = {
>>         .asic_family = CHIP_KAVERI,
>>         .max_pasid_bits = 16,
>> @@ -41,6 +44,7 @@ static const struct kfd_device_info kaveri_device_info = {
>>         .num_of_watch_points = 4,
>>         .mqd_size_aligned = MQD_SIZE_ALIGNED,
>>         .supports_cwsr = false,
>> +       .needs_iommu_device = true,
>>         .needs_pci_atomics = false,
>>  };
>>
>> @@ -54,8 +58,10 @@ static const struct kfd_device_info carrizo_device_info = {
>>         .num_of_watch_points = 4,
>>         .mqd_size_aligned = MQD_SIZE_ALIGNED,
>>         .supports_cwsr = true,
>> +       .needs_iommu_device = true,
>>         .needs_pci_atomics = false,
>>  };
>> +#endif
>>
>>  struct kfd_deviceid {
>>         unsigned short did;
>> @@ -64,6 +70,7 @@ struct kfd_deviceid {
>>
>>  /* Please keep this sorted by increasing device id. */
>>  static const struct kfd_deviceid supported_devices[] = {
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>         { 0x1304, &kaveri_device_info },        /* Kaveri */
>>         { 0x1305, &kaveri_device_info },        /* Kaveri */
>>         { 0x1306, &kaveri_device_info },        /* Kaveri */
>> @@ -91,6 +98,7 @@ static const struct kfd_deviceid supported_devices[] = {
>>         { 0x9875, &carrizo_device_info },       /* Carrizo */
>>         { 0x9876, &carrizo_device_info },       /* Carrizo */
>>         { 0x9877, &carrizo_device_info }        /* Carrizo */
>> +#endif
>>  };
>>
>>  static int kfd_gtt_sa_init(struct kfd_dev *kfd, unsigned int buf_size,
>> @@ -161,6 +169,7 @@ struct kfd_dev *kgd2kfd_probe(struct kgd_dev *kgd,
>>         return kfd;
>>  }
>>
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>  static bool device_iommu_pasid_init(struct kfd_dev *kfd)
>>  {
>>         const u32 required_iommu_flags = AMD_IOMMU_DEVICE_FLAG_ATS_SUP |
>> @@ -231,6 +240,7 @@ static int iommu_invalid_ppr_cb(struct pci_dev *pdev, int pasid,
>>
>>         return AMD_IOMMU_INV_PRI_RSP_INVALID;
>>  }
>> +#endif /* CONFIG_AMD_IOMMU_V2 */
>>
>>  static void kfd_cwsr_init(struct kfd_dev *kfd)
>>  {
>> @@ -321,12 +331,14 @@ bool kgd2kfd_device_init(struct kfd_dev *kfd,
>>                 goto device_queue_manager_error;
>>         }
>>
>> -       if (!device_iommu_pasid_init(kfd)) {
>> -               dev_err(kfd_device,
>> -                       "Error initializing iommuv2 for device %x:%x\n",
>> -                       kfd->pdev->vendor, kfd->pdev->device);
>> -               goto device_iommu_pasid_error;
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>> +       if (kfd->device_info->needs_iommu_device) {
>> +               if (!device_iommu_pasid_init(kfd)) {
>> +                       dev_err(kfd_device, "Error initializing iommuv2\n");
>> +                       goto device_iommu_pasid_error;
>> +               }
>>         }
>> +#endif
>>
>>         kfd_cwsr_init(kfd);
>>
>> @@ -386,11 +398,16 @@ void kgd2kfd_suspend(struct kfd_dev *kfd)
>>
>>         kfd->dqm->ops.stop(kfd->dqm);
>>
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>> +       if (!kfd->device_info->needs_iommu_device)
>> +               return;
>> +
>>         kfd_unbind_processes_from_device(kfd);
>>
>>         amd_iommu_set_invalidate_ctx_cb(kfd->pdev, NULL);
>>         amd_iommu_set_invalid_ppr_cb(kfd->pdev, NULL);
>>         amd_iommu_free_device(kfd->pdev);
>> +#endif
>>  }
>>
>>  int kgd2kfd_resume(struct kfd_dev *kfd)
>> @@ -405,19 +422,24 @@ int kgd2kfd_resume(struct kfd_dev *kfd)
>>  static int kfd_resume(struct kfd_dev *kfd)
>>  {
>>         int err = 0;
>> -       unsigned int pasid_limit = kfd_get_pasid_limit();
>>
>> -       err = amd_iommu_init_device(kfd->pdev, pasid_limit);
>> -       if (err)
>> -               return -ENXIO;
>> -       amd_iommu_set_invalidate_ctx_cb(kfd->pdev,
>> -                                       iommu_pasid_shutdown_callback);
>> -       amd_iommu_set_invalid_ppr_cb(kfd->pdev,
>> -                                    iommu_invalid_ppr_cb);
>> -
>> -       err = kfd_bind_processes_to_device(kfd);
>> -       if (err)
>> -               goto processes_bind_error;
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>> +       if (kfd->device_info->needs_iommu_device) {
>> +               unsigned int pasid_limit = kfd_get_pasid_limit();
>> +
>> +               err = amd_iommu_init_device(kfd->pdev, pasid_limit);
>> +               if (err)
>> +                       return -ENXIO;
>> +               amd_iommu_set_invalidate_ctx_cb(kfd->pdev,
>> +                                               iommu_pasid_shutdown_callback);
>> +               amd_iommu_set_invalid_ppr_cb(kfd->pdev,
>> +                                            iommu_invalid_ppr_cb);
>> +
>> +               err = kfd_bind_processes_to_device(kfd);
>> +               if (err)
>> +                       goto processes_bind_error;
>> +       }
>> +#endif
>>
>>         err = kfd->dqm->ops.start(kfd->dqm);
>>         if (err) {
>> @@ -431,8 +453,10 @@ static int kfd_resume(struct kfd_dev *kfd)
>>
>>  dqm_start_error:
>>  processes_bind_error:
>> -       amd_iommu_free_device(kfd->pdev);
>> -
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>> +       if (kfd->device_info->needs_iommu_device)
>> +               amd_iommu_free_device(kfd->pdev);
>> +#endif
>>         return err;
>>  }
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_events.c b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
>> index 93aae5c..f770dc7 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_events.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_events.c
>> @@ -837,6 +837,7 @@ static void lookup_events_by_type_and_signal(struct kfd_process *p,
>>         }
>>  }
>>
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>  void kfd_signal_iommu_event(struct kfd_dev *dev, unsigned int pasid,
>>                 unsigned long address, bool is_write_requested,
>>                 bool is_execute_requested)
>> @@ -905,6 +906,7 @@ void kfd_signal_iommu_event(struct kfd_dev *dev, unsigned int pasid,
>>         mutex_unlock(&p->event_mutex);
>>         kfd_unref_process(p);
>>  }
>> +#endif /* CONFIG_AMD_IOMMU_V2_MODULE */
>>
>>  void kfd_signal_hw_exception_event(unsigned int pasid)
>>  {
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> index eebfb1e..9f4766c 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> @@ -158,6 +158,7 @@ struct kfd_device_info {
>>         uint8_t num_of_watch_points;
>>         uint16_t mqd_size_aligned;
>>         bool supports_cwsr;
>> +       bool needs_iommu_device;
>>         bool needs_pci_atomics;
>>  };
>>
>> @@ -617,9 +618,11 @@ void kfd_unref_process(struct kfd_process *p);
>>
>>  struct kfd_process_device *kfd_bind_process_to_device(struct kfd_dev *dev,
>>                                                 struct kfd_process *p);
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>  int kfd_bind_processes_to_device(struct kfd_dev *dev);
>>  void kfd_unbind_processes_from_device(struct kfd_dev *dev);
>>  void kfd_process_iommu_unbind_callback(struct kfd_dev *dev, unsigned int pasid);
>> +#endif
>>  struct kfd_process_device *kfd_get_process_device_data(struct kfd_dev *dev,
>>                                                         struct kfd_process *p);
>>  struct kfd_process_device *kfd_create_process_device_data(struct kfd_dev *dev,
>> @@ -784,9 +787,11 @@ int kfd_wait_on_events(struct kfd_process *p,
>>                        uint32_t *wait_result);
>>  void kfd_signal_event_interrupt(unsigned int pasid, uint32_t partial_id,
>>                                 uint32_t valid_id_bits);
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>  void kfd_signal_iommu_event(struct kfd_dev *dev,
>>                 unsigned int pasid, unsigned long address,
>>                 bool is_write_requested, bool is_execute_requested);
>> +#endif
>>  void kfd_signal_hw_exception_event(unsigned int pasid);
>>  int kfd_set_event(struct kfd_process *p, uint32_t event_id);
>>  int kfd_reset_event(struct kfd_process *p, uint32_t event_id);
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> index a22fb071..1d0e02c 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> @@ -173,14 +173,17 @@ static void kfd_process_wq_release(struct work_struct *work)
>>  {
>>         struct kfd_process *p = container_of(work, struct kfd_process,
>>                                              release_work);
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>         struct kfd_process_device *pdd;
>>
>>         pr_debug("Releasing process (pasid %d) in workqueue\n", p->pasid);
>>
>>         list_for_each_entry(pdd, &p->per_device_data, per_device_list) {
>> -               if (pdd->bound == PDD_BOUND)
>> +               if (pdd->bound == PDD_BOUND &&
>> +                   pdd->dev->device_info->needs_iommu_device)
>>                         amd_iommu_unbind_pasid(pdd->dev->pdev, p->pasid);
>>         }
>> +#endif
>>
>>         kfd_process_destroy_pdds(p);
>>
>> @@ -421,7 +424,6 @@ struct kfd_process_device *kfd_bind_process_to_device(struct kfd_dev *dev,
>>                                                         struct kfd_process *p)
>>  {
>>         struct kfd_process_device *pdd;
>> -       int err;
>>
>>         pdd = kfd_get_process_device_data(dev, p);
>>         if (!pdd) {
>> @@ -436,9 +438,14 @@ struct kfd_process_device *kfd_bind_process_to_device(struct kfd_dev *dev,
>>                 return ERR_PTR(-EINVAL);
>>         }
>>
>> -       err = amd_iommu_bind_pasid(dev->pdev, p->pasid, p->lead_thread);
>> -       if (err < 0)
>> -               return ERR_PTR(err);
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>> +       if (dev->device_info->needs_iommu_device) {
>> +               int err = amd_iommu_bind_pasid(dev->pdev, p->pasid,
>> +                                              p->lead_thread);
>> +               if (err < 0)
>> +                       return ERR_PTR(err);
>> +       }
>> +#endif
>>
>>         pdd->bound = PDD_BOUND;
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>> index c6a7609..f57c305 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
>> @@ -875,6 +875,7 @@ static void find_system_memory(const struct dmi_header *dm,
>>   */
>>  static int kfd_add_perf_to_topology(struct kfd_topology_device *kdev)
>>  {
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>         struct kfd_perf_properties *props;
>>
>>         if (amd_iommu_pc_supported()) {
>> @@ -886,6 +887,7 @@ static int kfd_add_perf_to_topology(struct kfd_topology_device *kdev)
>>                         amd_iommu_pc_get_max_counters(0); /* assume one iommu */
>>                 list_add_tail(&props->list, &kdev->perf_props);
>>         }
>> +#endif
>>
>>         return 0;
>>  }
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
>> index 53fca1f..111fda2 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.h
>> @@ -183,8 +183,10 @@ struct kfd_topology_device *kfd_create_topology_device(
>>                 struct list_head *device_list);
>>  void kfd_release_topology_device_list(struct list_head *device_list);
>>
>> +#if defined(CONFIG_AMD_IOMMU_V2_MODULE) || defined(CONFIG_AMD_IOMMU_V2)
>>  extern bool amd_iommu_pc_supported(void);
>>  extern u8 amd_iommu_pc_get_max_banks(u16 devid);
>>  extern u8 amd_iommu_pc_get_max_counters(u16 devid);
>> +#endif
>>
>>  #endif /* __KFD_TOPOLOGY_H__ */
>> --
>> 2.7.4
>>


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux