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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx <mailto:luben.tuikov@xxxxxxx>> wrote:

     On 2019-11-14 10:34 p.m., Aaron Liu wrote:
     > From: Huang Rui <ray.huang@xxxxxxx <mailto:ray.huang@xxxxxxx>>
     >
     > 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@xxxxxxx <mailto:ray.huang@xxxxxxx>>
     > Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx <mailto:alexander.deucher@xxxxxxx>>
     > ---
     >  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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux