Am 14.08.2017 um 17:10 schrieb Felix Kuehling:
[dropping Moses and Ben, they no longer work for AMD and the email
addresses bounce]


On 2017-08-13 05:04 AM, Oded Gabbay wrote:
On Sat, Aug 12, 2017 at 7:47 AM, Felix Kuehling <felix.kuehl...@amd.com> wrote:
From: Moses Reuben <moses.reu...@amd.com>

Signed-off-by: Moses Reuben <moses.reu...@amd.com>
Signed-off-by: Ben Goz <ben....@amd.com>
Signed-off-by: Felix Kuehling <felix.kuehl...@amd.com>
[snip]
@@ -286,7 +293,17 @@ struct kfd_ioctl_wait_events_args {
  #define AMDKFD_IOC_DBG_WAVE_CONTROL            \
                 AMDKFD_IOW(0x10, struct kfd_ioctl_dbg_wave_control_args)

+/* TODO:
+ * - AMDKFD_IOC_ALLOC_MEMORY_OF_GPU
+ * - AMDKFD_IOC_FREE_MEMORY_OF_GPU
+ * - AMDKFD_IOC_MAP_MEMORY_TO_GPU
+ * - AMDKFD_IOC_UNMAP_MEMORY_FROM_GPU
+ */
+
+#define AMDKFD_IOC_ALLOC_MEMORY_OF_SCRATCH     \
+               AMDKFD_IOWR(0x15, struct kfd_ioctl_alloc_memory_of_scratch_args)
+
  #define AMDKFD_COMMAND_START           0x01
-#define AMDKFD_COMMAND_END             0x11
+#define AMDKFD_COMMAND_END             0x16
You create a hole here, between 0x11 to 0x16. This would make the
sanity check in kfd_ioctl() to be useless.
I'm creating the hole to match the ABI with our current ROCm releases.
The TODO comment lists the ioctls that will slot into the hole in future
patches.

That's usually not such a good idea, cause interface rare upstream as they are designed internally.

Probably better to just add them to the end of the list.


kfd_ioctl checks that the function pointer is initialized and fails
otherwise. So I think this hole should work OK and attempts to call
unassigned ioctls should fail as expected:

         func = ioctl->func;

         if (unlikely(!func)) {
                 dev_dbg(kfd_device, "no function\n");
                 retcode = -EINVAL;
                 goto err_i1;
         }


Also, why not do a generic IOCTL for allocating GPU memory, and pass
parameter if its scratch or something else, similar to amdgpu's
"AMDGPU_GEM_DOMAIN_*" defines ?
The real memory allocation ioctl has more parameters. Also, I always
thought the name "ALLOC_MEMORY_OF_SCRATCH" was misleading, because this
ioctl doesn't really allocate anything, and there is no corresponding
"free". What it really does, it configures the virtual address of the
scratch backing memory for the process.

Maybe we could change the name of the ioctl (without changing the ABI)
to something like SET_SCRATCH_BACKING_VA. What do you think?

That's better, but I think something in the direction of VA address config would be better.

BTW: What exactly this this good for?

Regards,
Christian.


Regards,
   Felix

  #endif
--
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to