[PATCH 01/12] amdgpu: add UAPI for creating encrypted buffers

[Harry Wentland]

On 2019-11-20 12:05 p.m., Harry Wentland wrote:
> On 2019-11-20 11:49 a.m., Luben Tuikov wrote:
>> On 2019-11-19 21:41, Marek Olšák wrote:
>>> On Tue, Nov 19, 2019 at 8:52 PM Luben Tuikov <luben.tuikov at amd.com <mailto:luben.tuikov at amd.com>> wrote:
>>>
>>>     On 2019-11-14 10:34 p.m., Aaron Liu wrote:
>>>     > From: Huang Rui <ray.huang at amd.com <mailto:ray.huang at amd.com>>
>>>     >
>>>     > To align the kernel uapi change from Alex:
>>>     >
>>>     > "Add a flag to the GEM_CREATE ioctl to create encrypted buffers. Buffers with
>>>     > this flag set will be created with the TMZ bit set in the PTEs or engines
>>>     > accessing them. This is required in order to properly access the data from the
>>>     > engines."
>>>     >
>>>     > We will use GEM_CREATE_ENCRYPTED flag for secure buffer allocation.
>>>     >
>>>     > Signed-off-by: Huang Rui <ray.huang at amd.com <mailto:ray.huang at amd.com>>
>>>     > Reviewed-by: Alex Deucher <alexander.deucher at amd.com <mailto:alexander.deucher at amd.com>>
>>>     > ---
>>>     >  include/drm/amdgpu_drm.h | 5 +++++
>>>     >  1 file changed, 5 insertions(+)
>>>     >
>>>     > diff --git a/include/drm/amdgpu_drm.h b/include/drm/amdgpu_drm.h
>>>     > index 5c28aa7..1a95e37 100644
>>>     > --- a/include/drm/amdgpu_drm.h
>>>     > +++ b/include/drm/amdgpu_drm.h
>>>     > @@ -141,6 +141,11 @@ extern "C" {
>>>     >   * releasing the memory
>>>     >   */
>>>     >  #define AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE       (1 << 9)
>>>     > +/* Flag that BO will be encrypted and that the TMZ bit should be
>>>     > + * set in the PTEs when mapping this buffer via GPUVM or
>>>     > + * accessing it with various hw blocks
>>>     > + */
>>>     > +#define AMDGPU_GEM_CREATE_ENCRYPTED          (1 << 10)
>>>
>>>     Style!
>>>     TAB char?!
>>>
>>>     You have a TAB char between ".._ENCRYPTED" and "(1 << 10)"
>>>     Do NOT add/insert TAB chars instead of space to align colunmns!
>>>     If when you press Tab key a tab is inserted, as opposed to the line
>>>     indented, then DO NOT use this editor.
>>>     The Tab key should "indent according to mode" by inserting TAB chars.
>>>     If the line is already indented, as this one is, then it should do nothing.
>>>
>>>
>>> I disagree with this 100%. Tabs or spaces don't matter here from my perspective. I also disagree with your language. It's overly impolite.
>>
>> But it's the coding style of Linux: leading tabs only. Try it with Emacs as described and given in
>>
>> linux/Documentation/process/coding-style.rst
>>
>> starting at line 589. And press the Tab key on an already indented line--nothing will happen. Linux has traditionally
>> shunned from loose TAB chars in already indented lines: leading tabs only mode. In a proper code editor
>> pressing the Tab key only indents according to buffer mode, it shouldn't insert a Tab char willy-nilly.
>> People may set their tab stops differently for different tab positions and inserting a tab char may display
>> incorrectly. The most portable way to align columns in an already indented-according-to-mode line, is
>> using spaces. (Of course this doesn't matter when using spaces to indent, but Linux uses hard TAB chars
>> to indent: linux/Documentation/process/coding-style.rst. (which also seem to be set to 8 chars))
>>
>> It's a code review, there is no "language".
> 
> May I remind you that freedesktop.org hosted projects follow a code of
> conduct [1]. This applies whether the interaction is a code review or
> any other interaction.
> 
> I don't think your language was overly impolite but it did come across a
> bit strong. Please consider how your statements might be perceived by

Just to clarify, and I did intend to highlight this in the previous
sentence, I don't think there is any violation of the CoC here. I am
merely trying to say that language matters, even for code reviews.

Harry

> the person they're addressed to.
> 
> [1] https://www.freedesktop.org/wiki/CodeOfConduct/
> 
> Harry
> 
>>
>> Regards,
>> Luben
>>
>>>
>>> Marek
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>