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

Christian König ckoenig.leichtzumerken at gmail.com
Wed Nov 20 18:40:27 UTC 2019


Am 20.11.19 um 18:50 schrieb Luben Tuikov:
> On 2019-11-20 12:24, Christian König wrote:
>> 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.
> Yes, it says this, quote (for those who didn't open the file):
>
> --8<---------------------------------------------------------------------
>
> That's OK, we all do.  You've probably been told by your long-time Unix
> user helper that ``GNU emacs`` automatically formats the C sources for
> you, and you've noticed that yes, it does do that, but the defaults it
> uses are less than desirable (in fact, they are worse than random
> typing - an infinite number of monkeys typing into GNU emacs would never
> make a good program).
>
> So, you can either get rid of GNU emacs, or change it to use saner
> values.  To do the latter, you can stick the following in your .emacs file:
>
> --8<--------------------------------------------------------------------
>
>>> 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".
> Hmm, that's interesting. Is that in linux/include or amdgpu/include?

linux/include

>
> I've been meaning to do my own extended regex to catch those, although
> I'm using Emacs and pressing Tab key only indents and would not insert
> a Tab char if already indented. (So applying this regex into the pre-commit
> hook of all of my Git repos would never trigger.)
>
> I remember of olden days, circa 2000 when I first got involved with Linux,
> LKML didn't like loose tabs. Also lead kernel developers are using Emacs,
> so it's been my choice of editor since circa 1994 (switched from vi to Emacs
> largely due to the influence of a graphics prof I had in my seniour year of uni,
> and part due to LKML.)

Well I've been working with the Linux kernel since the mid 90ths and 
never ever heard of that.

Christian.

>
> Thanks for chiming in Christian!
>
> Regards,
> Luben
>
>> 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
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx



More information about the amd-gfx mailing list