[RFC PATCH] gr: did you try turning it off and on again.

[RFC PATCH] gr: did you try turning it off and on again.

[Karol Herbst]

Fixes secure boot on my gp107. No idea why. Otherwise the GPU enters
complete lockdown after starting the gpccs and fecs with the LS images
loaded.

Signed-off-by: Karol Herbst <kherbst@redhat.com>
---
 drm/nouveau/nvkm/engine/gr/gf100.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drm/nouveau/nvkm/engine/gr/gf100.c b/drm/nouveau/nvkm/engine/gr/gf100.c
index 2f8dc107..322d9fa6 100644
--- a/drm/nouveau/nvkm/engine/gr/gf100.c
+++ b/drm/nouveau/nvkm/engine/gr/gf100.c
@@ -1731,8 +1731,15 @@ gf100_gr_init_(struct nvkm_gr *base)
 {
 	struct gf100_gr *gr = gf100_gr(base);
 	struct nvkm_subdev *subdev = &base->engine.subdev;
+	struct nvkm_device *device = subdev->device;
 	u32 ret;
 
+	/* did you try turning it off and on again? Apparently we need this
+	 * on pascal, otherwise secboot will just fail.
+	 */
+	nvkm_mask(device, 0x200, 0x1000, 0x0000);
+	nvkm_mask(device, 0x200, 0x1000, 0x1000);
+
 	nvkm_pmu_pgob(gr->base.engine.subdev.device->pmu, false);
 
 	ret = nvkm_falcon_get(gr->fecs, subdev);
-- 
2.14.3

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [RFC PATCH] gr: did you try turning it off and on again.

[Tobias Klausmann]

Hi,

comments inline

On 11/28/17 2:11 PM, Karol Herbst wrote:
> Fixes secure boot on my gp107. No idea why. Otherwise the GPU enters
> complete lockdown after starting the gpccs and fecs with the LS images
> loaded.
>
> Signed-off-by: Karol Herbst <kherbst@redhat.com>
> ---
>   drm/nouveau/nvkm/engine/gr/gf100.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/drm/nouveau/nvkm/engine/gr/gf100.c b/drm/nouveau/nvkm/engine/gr/gf100.c
> index 2f8dc107..322d9fa6 100644
> --- a/drm/nouveau/nvkm/engine/gr/gf100.c
> +++ b/drm/nouveau/nvkm/engine/gr/gf100.c
> @@ -1731,8 +1731,15 @@ gf100_gr_init_(struct nvkm_gr *base)
>   {
>   	struct gf100_gr *gr = gf100_gr(base);
>   	struct nvkm_subdev *subdev = &base->engine.subdev;
> +	struct nvkm_device *device = subdev->device;
>   	u32 ret;
>   
> +	/* did you try turning it off and on again? Apparently we need this
> +	 * on pascal, otherwise secboot will just fail.


The comments about the off and on looks silly, at least put it in 
quotation marks, or rewrite it, e.g. "Apparently we need to turn it off 
and on for the pascal generation, otherwise secboot will just fail."

> +	 */
> +	nvkm_mask(device, 0x200, 0x1000, 0x0000);
> +	nvkm_mask(device, 0x200, 0x1000, 0x1000);
> +


It is needed with pascal, but does it harm other generations calling 
this init? Maybe guard it against exectution on maxwell


Greetings,

Tobias


>   	nvkm_pmu_pgob(gr->base.engine.subdev.device->pmu, false);
>   
>   	ret = nvkm_falcon_get(gr->fecs, subdev);
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [RFC PATCH] gr: did you try turning it off and on again.

[Karol Herbst]

On Tue, Nov 28, 2017 at 4:36 PM, Tobias Klausmann
<tobias.johannes.klausmann@mni.thm.de> wrote:
> Hi,
>
> comments inline
>
> On 11/28/17 2:11 PM, Karol Herbst wrote:
>>
>> Fixes secure boot on my gp107. No idea why. Otherwise the GPU enters
>> complete lockdown after starting the gpccs and fecs with the LS images
>> loaded.
>>
>> Signed-off-by: Karol Herbst <kherbst@redhat.com>
>> ---
>>   drm/nouveau/nvkm/engine/gr/gf100.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drm/nouveau/nvkm/engine/gr/gf100.c
>> b/drm/nouveau/nvkm/engine/gr/gf100.c
>> index 2f8dc107..322d9fa6 100644
>> --- a/drm/nouveau/nvkm/engine/gr/gf100.c
>> +++ b/drm/nouveau/nvkm/engine/gr/gf100.c
>> @@ -1731,8 +1731,15 @@ gf100_gr_init_(struct nvkm_gr *base)
>>   {
>>         struct gf100_gr *gr = gf100_gr(base);
>>         struct nvkm_subdev *subdev = &base->engine.subdev;
>> +       struct nvkm_device *device = subdev->device;
>>         u32 ret;
>>   +     /* did you try turning it off and on again? Apparently we need
>> this
>> +        * on pascal, otherwise secboot will just fail.
>
>
>
> The comments about the off and on looks silly, at least put it in quotation
> marks, or rewrite it, e.g. "Apparently we need to turn it off and on for the
> pascal generation, otherwise secboot will just fail."
>

right... I wasn't entirely serious about this one though. Just a
little annoyed, that this was just a painful issue, because it just
doesn't make any sense at all in the end. And because this is
involving secure boot here, who knows how many bugs of this kind are
still there?

>> +        */
>> +       nvkm_mask(device, 0x200, 0x1000, 0x0000);
>> +       nvkm_mask(device, 0x200, 0x1000, 0x1000);
>> +
>
>
>
> It is needed with pascal, but does it harm other generations calling this
> init? Maybe guard it against exectution on maxwell
>

Maybe, maybe not? I think it is fairly safe, but I kind of hoped
somebody with more knowledge about these kind of things would respond
and tells us this has to be done or maybe not.

>
> Greetings,
>
> Tobias
>
>
>
>>         nvkm_pmu_pgob(gr->base.engine.subdev.device->pmu, false);
>>         ret = nvkm_falcon_get(gr->fecs, subdev);
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [RFC PATCH] gr: did you try turning it off and on again.

[Ben Skeggs]

[-- Attachment #1.1.1: Type: text/plain, Size: 2821 bytes --]

On 11/29/2017 01:47 PM, Karol Herbst wrote:
> On Tue, Nov 28, 2017 at 4:36 PM, Tobias Klausmann
> <tobias.johannes.klausmann-AqjdNwhu20eELgA04lAiVw@public.gmane.org> wrote:
>> Hi,
>>
>> comments inline
>>
>> On 11/28/17 2:11 PM, Karol Herbst wrote:
>>>
>>> Fixes secure boot on my gp107. No idea why. Otherwise the GPU enters
>>> complete lockdown after starting the gpccs and fecs with the LS images
>>> loaded.
>>>
>>> Signed-off-by: Karol Herbst <kherbst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>   drm/nouveau/nvkm/engine/gr/gf100.c | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drm/nouveau/nvkm/engine/gr/gf100.c
>>> b/drm/nouveau/nvkm/engine/gr/gf100.c
>>> index 2f8dc107..322d9fa6 100644
>>> --- a/drm/nouveau/nvkm/engine/gr/gf100.c
>>> +++ b/drm/nouveau/nvkm/engine/gr/gf100.c
>>> @@ -1731,8 +1731,15 @@ gf100_gr_init_(struct nvkm_gr *base)
>>>   {
>>>         struct gf100_gr *gr = gf100_gr(base);
>>>         struct nvkm_subdev *subdev = &base->engine.subdev;
>>> +       struct nvkm_device *device = subdev->device;
>>>         u32 ret;
>>>   +     /* did you try turning it off and on again? Apparently we need
>>> this
>>> +        * on pascal, otherwise secboot will just fail.
>>
>>
>>
>> The comments about the off and on looks silly, at least put it in quotation
>> marks, or rewrite it, e.g. "Apparently we need to turn it off and on for the
>> pascal generation, otherwise secboot will just fail."
>>
> 
> right... I wasn't entirely serious about this one though. Just a
> little annoyed, that this was just a painful issue, because it just
> doesn't make any sense at all in the end. And because this is
> involving secure boot here, who knows how many bugs of this kind are
> still there?
> 
>>> +        */
>>> +       nvkm_mask(device, 0x200, 0x1000, 0x0000);
>>> +       nvkm_mask(device, 0x200, 0x1000, 0x1000);
>>> +
>>
>>
>>
>> It is needed with pascal, but does it harm other generations calling this
>> init? Maybe guard it against exectution on maxwell
>>
> 
> Maybe, maybe not? I think it is fairly safe, but I kind of hoped
> somebody with more knowledge about these kind of things would respond
> and tells us this has to be done or maybe not.
This should already be happening actually, so it's definitely weird that
adding it there explicitly does anything at all...

Can you mmiotrace nouveau for me?

Ben.

> 
>>
>> Greetings,
>>
>> Tobias
>>
>>
>>
>>>         nvkm_pmu_pgob(gr->base.engine.subdev.device->pmu, false);
>>>         ret = nvkm_falcon_get(gr->fecs, subdev);
> _______________________________________________
> Nouveau mailing list
> Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
> 


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau