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

[Christian König]

Am 20.11.19 um 18:16 schrieb Christian König:
> Am 20.11.19 um 17:49 schrieb Luben Tuikov:
>> 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".
>
> Well the section you noted also suggest to either get rid of emacs or 
> change it to use some saner default values. We just got rid of emacs.
>
> Regarding tabs after the initial indentation, I've just done a quick 
> grep and around 14% of all defines under include/ uses that so I would 
> say that this is perfectly fine.

Fast typing with lazy eyes, that should read "around 71% of all defines".

Sorry,
Christian.

>
> Regards,
> Christian.
>
>>
>> Regards,
>> Luben
>>
>>> Marek
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>