[PATCH 01/12] amdgpu: add UAPI for creating encrypted buffers
Luben Tuikov
luben.tuikov at amd.com
Wed Nov 20 19:08:40 UTC 2019
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.
Is this okay, and acceptable, according to the LKCS (linux/Documentation/process/coding-style.rst)?
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