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

[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.

Regards,
Christian.

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