[PATCH 01/12] amdgpu: add UAPI for creating encrypted buffers
Christian König
christian.koenig at amd.com
Tue Nov 26 10:08:51 UTC 2019
Am 20.11.19 um 20:08 schrieb Luben Tuikov:
> On 2019-11-20 13:40, Christian König wrote:
>> 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.
> I see. Hmm, interesting. My experience differs.
>
> Regardless, here is the question:
>
> Is it then okay to embed hard TAB char anywhere in an already indented
> line of code?
>
> For instance:
>
> for (i = 0; i <\t10; i++) {
>
> int table[][3] = {
> { 2,\t3, 5 },
> { 2, 4,\t6 },
> { 1,\t1,\t2 },
> };
>
> Because it would render correct on an 8-tab stop configured editor.
At least to those examples I would say no cause they use tab and spaces
inconsistently.
> Is this okay, and acceptable, according to the LKCS (linux/Documentation/process/coding-style.rst)?
As long as you indent consistently I think it is ok. For example most
defines seem to look like the pattern "#define NAME\t+VALUE".
Regards,
Christian.
>
> Regards,
> Luben
>
>> 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