On 2017-08-14 11:15 AM, Christian König wrote:
> 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.Kuehling at amd.com> wrote:
>>>> From: Moses Reuben <moses.reuben at amd.com>
>>>>
>>>> Signed-off-by: Moses Reuben <moses.reuben at amd.com>
>>>> Signed-off-by: Ben Goz <ben.goz at amd.com>
>>>> Signed-off-by: Felix Kuehling <Felix.Kuehling at 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.
Repeating the same argument I made on another email:
Changing the ioctl numbers to something different to our ROCm releases,
means you can't test until we make another release with updated ioctl
numbers.
If testability is not a priority, then I'd rather upstream the new
ioctls in the order in which we made them. The only reason I'm going
out-of-order is to make testing as easy as possible, as early as
possible, with the most recently released user mode stack.
>
>>
>> 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?
Scratch memory is private memory per work-item. It's used when a shader
program has too few registers available. With HSA we use flat scratch
addressing, where shaders can access private memory in a special scratch
aperture using normal memory instructions. Using the same virtual
address, each work item gets its own private piece of memory. The
hardware does the address translation from the VA in the private
aperture to a scratch-backing VA. The application is responsible for
allocating the memory to back that scratch area, and to map it somewhere
in its virtual address space.
This ioctl tells the hardware (or HWS firmware) the VA of the scratch
backing memory.
Regards,
Felix
>
> Regards,
> Christian.
>
>>
>> Regards,
>> Felix
>>
>>>> #endif
>>>> --
>>>> 2.7.4
>>>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>