Nouveau - Jan 2016 - [PATCH v2 2/5] core: add support for secure boot

On 21 January 2016 at 12:13, Ben Skeggs <skeggsb at gmail.com>
wrote:
> On 01/21/2016 10:09 PM, Emil Velikov wrote:
>> Hi Alexandre,
>>
>> On 18 January 2016 at 06:10, Alexandre Courbot <acourbot at
nvidia.com> wrote:
>>
>> [snip]
>>> +static const char *
>>> +managed_falcons_names[] = {
>>> +       [NVKM_SECBOOT_FALCON_PMU] = "PMU",
>>> +       [NVKM_SECBOOT_FALCON_RESERVED] =
"<invalid>",
>> "<reserved>" perhaps ? we already have one invalid
below.
> Does <reserved> really mean: "we don't want to tell
you?" here? :)
>
That or we have some secret WIP that we're haven't decided if it'll
work out :-)
>>
>>> +       [NVKM_SECBOOT_FALCON_FECS] = "FECS",
>>> +       [NVKM_SECBOOT_FALCON_GPCCS] = "GPCCS",
>>> +       [NVKM_SECBOOT_FALCON_END] = "<invalid>",
>>> +};
>>> +
>>
>> [snip]
>>> +int
>>> +nvkm_secboot_ctor(const struct nvkm_secboot_func *func,
>>> +                 struct nvkm_device *device, int index,
>>> +                 struct nvkm_secboot *sb)
>>> +{
>>> +       unsigned long fid;
>>> +
>>> +       nvkm_subdev_ctor(&nvkm_secboot, device, index, 0,
&sb->subdev);
>>> +       sb->func = func;
>>> +
>> Move these two after the switch statement ?
> They need to be done here to make the failure path cleanup stuff work
> correctly, so it's correct as-is.
>
| always get confused which ones needed to setup their own dtors and
which ones didn't. Thanks !

-Emil

On Thu, Jan 21, 2016 at 9:31 PM, Emil Velikov <emil.l.velikov at
gmail.com> wrote:
> On 21 January 2016 at 12:13, Ben Skeggs <skeggsb at gmail.com> wrote:
>> On 01/21/2016 10:09 PM, Emil Velikov wrote:
>>> Hi Alexandre,
>>>
>>> On 18 January 2016 at 06:10, Alexandre Courbot <acourbot at
nvidia.com> wrote:
>>>
>>> [snip]
>>>> +static const char *
>>>> +managed_falcons_names[] = {
>>>> +       [NVKM_SECBOOT_FALCON_PMU] = "PMU",
>>>> +       [NVKM_SECBOOT_FALCON_RESERVED] =
"<invalid>",
>>> "<reserved>" perhaps ? we already have one invalid
below.
>> Does <reserved> really mean: "we don't want to tell
you?" here? :)
>>
> That or we have some secret WIP that we're haven't decided if
it'll work out :-)
That's just how it appears on our end. :) But since this enum does not
seem to be used by the hardware anyway, I will simply remove the
"_RESERVED" entry. Thanks.

On Mon, Jan 25, 2016 at 11:15 AM, Alexandre Courbot <gnurou at gmail.com>
wrote:
> On Thu, Jan 21, 2016 at 9:31 PM, Emil Velikov <emil.l.velikov at
gmail.com> wrote:
>> On 21 January 2016 at 12:13, Ben Skeggs <skeggsb at gmail.com>
wrote:
>>> On 01/21/2016 10:09 PM, Emil Velikov wrote:
>>>> Hi Alexandre,
>>>>
>>>> On 18 January 2016 at 06:10, Alexandre Courbot <acourbot at
nvidia.com> wrote:
>>>>
>>>> [snip]
>>>>> +static const char *
>>>>> +managed_falcons_names[] = {
>>>>> +       [NVKM_SECBOOT_FALCON_PMU] = "PMU",
>>>>> +       [NVKM_SECBOOT_FALCON_RESERVED] =
"<invalid>",
>>>> "<reserved>" perhaps ? we already have one
invalid below.
>>> Does <reserved> really mean: "we don't want to tell
you?" here? :)
>>>
>> That or we have some secret WIP that we're haven't decided if
it'll work out :-)
>
> That's just how it appears on our end. :) But since this enum does not
> seem to be used by the hardware anyway, I will simply remove the
> "_RESERVED" entry. Thanks.
Meh, stupid me - these values also appear in the firmware files, so I
must keep the same numbering, and thus the _RESERVED entry. Sorry for
the noise.