Nouveau - Oct 2015 - [PATCH] bios: fix OF loading

Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
---

Tested on a PowerMac7,3 with a NV34 AGP adapter.

 drm/nouveau/nvkm/subdev/bios/priv.h     |  3 +++
 drm/nouveau/nvkm/subdev/bios/shadow.c   | 27 ++++++++++++++++++---------
 drm/nouveau/nvkm/subdev/bios/shadowof.c | 18 ++++++++++++++++--
 3 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/drm/nouveau/nvkm/subdev/bios/priv.h
b/drm/nouveau/nvkm/subdev/bios/priv.h
index e0ec2a6..212800e 100644
--- a/drm/nouveau/nvkm/subdev/bios/priv.h
+++ b/drm/nouveau/nvkm/subdev/bios/priv.h
@@ -8,7 +8,10 @@ struct nvbios_source {
 	void *(*init)(struct nvkm_bios *, const char *);
 	void  (*fini)(void *);
 	u32   (*read)(void *, u32 offset, u32 length, struct nvkm_bios *);
+	u32   (*size)(void *);
 	bool rw;
+	bool ignore_checksum;
+	bool no_pcir;
 };
 
 int nvbios_extend(struct nvkm_bios *, u32 length);
diff --git a/drm/nouveau/nvkm/subdev/bios/shadow.c
b/drm/nouveau/nvkm/subdev/bios/shadow.c
index 792f017..b2557e8 100644
--- a/drm/nouveau/nvkm/subdev/bios/shadow.c
+++ b/drm/nouveau/nvkm/subdev/bios/shadow.c
@@ -45,7 +45,7 @@ shadow_fetch(struct nvkm_bios *bios, struct shadow *mthd, u32
upto)
 		u32 read = mthd->func->read(data, start, limit - start, bios);
 		bios->size = start + read;
 	}
-	return bios->size >= limit;
+	return bios->size >= upto;
 }
 
 static int
@@ -55,14 +55,22 @@ shadow_image(struct nvkm_bios *bios, int idx, u32 offset,
struct shadow *mthd)
 	struct nvbios_image image;
 	int score = 1;
 
-	if (!shadow_fetch(bios, mthd, offset + 0x1000)) {
-		nvkm_debug(subdev, "%08x: header fetch failed\n", offset);
-		return 0;
-	}
+	if (mthd->func->no_pcir) {
+		image.base = 0;
+		image.type = 0;
+		image.size = mthd->func->size(mthd->data);
+		image.last = 1;
+	} else {
+		if (!shadow_fetch(bios, mthd, offset + 0x1000)) {
+			nvkm_debug(subdev, "%08x: header fetch failed\n",
+				   offset);
+			return 0;
+		}
 
-	if (!nvbios_image(bios, idx, &image)) {
-		nvkm_debug(subdev, "image %d invalid\n", idx);
-		return 0;
+		if (!nvbios_image(bios, idx, &image)) {
+			nvkm_debug(subdev, "image %d invalid\n", idx);
+			return 0;
+		}
 	}
 	nvkm_debug(subdev, "%08x: type %02x, %d bytes\n",
 		   image.base, image.type, image.size);
@@ -74,7 +82,8 @@ shadow_image(struct nvkm_bios *bios, int idx, u32 offset,
struct shadow *mthd)
 
 	switch (image.type) {
 	case 0x00:
-		if (nvbios_checksum(&bios->data[image.base], image.size)) {
+		if (!mthd->func->ignore_checksum &&
+		    nvbios_checksum(&bios->data[image.base], image.size)) {
 			nvkm_debug(subdev, "%08x: checksum failed\n",
 				   image.base);
 			if (mthd->func->rw)
diff --git a/drm/nouveau/nvkm/subdev/bios/shadowof.c
b/drm/nouveau/nvkm/subdev/bios/shadowof.c
index 29a37f0..4a20584 100644
--- a/drm/nouveau/nvkm/subdev/bios/shadowof.c
+++ b/drm/nouveau/nvkm/subdev/bios/shadowof.c
@@ -22,6 +22,7 @@
  */
 #include "priv.h"
 
+#include <core/pci.h>
 
 #if defined(__powerpc__)
 struct priv {
@@ -33,17 +34,27 @@ static u32
 of_read(void *data, u32 offset, u32 length, struct nvkm_bios *bios)
 {
 	struct priv *priv = data;
-	if (offset + length <= priv->size) {
+	if (offset < priv->size) {
+		length = min_t(u32, length, priv->size - offset);
 		memcpy_fromio(bios->data + offset, priv->data + offset, length);
 		return length;
 	}
 	return 0;
 }
 
+static u32
+of_size(void *data)
+{
+	struct priv *priv = data;
+
+	return priv->size;
+}
+
 static void *
 of_init(struct nvkm_bios *bios, const char *name)
 {
-	struct pci_dev *pdev = bios->subdev.device->pdev;
+	struct nvkm_device *device = bios->subdev.device;
+	struct pci_dev *pdev = device->func->pci(device)->pdev;
 	struct device_node *dn;
 	struct priv *priv;
 	if (!(dn = pci_device_to_OF_node(pdev)))
@@ -62,7 +73,10 @@ nvbios_of = {
 	.init = of_init,
 	.fini = (void(*)(void *))kfree,
 	.read = of_read,
+	.size = of_size,
 	.rw = false,
+	.ignore_checksum = true,
+	.no_pcir = true,
 };
 #else
 const struct nvbios_source
-- 
2.4.9

Hi,

On 02-10-15 05:41, Ilia Mirkin wrote:

<nothing>

As someone who has recently started following nouveau I must say that
it would greatly help me (and likely others) if patches likes this would
come with a somewhat more descriptive commit message.

Otherwise keep up the good work!

Regards,

Hans


p.s.

I've seen your message on the mesa fire demo on nv34, firing that up now.
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
>
> Tested on a PowerMac7,3 with a NV34 AGP adapter.
>
>   drm/nouveau/nvkm/subdev/bios/priv.h     |  3 +++
>   drm/nouveau/nvkm/subdev/bios/shadow.c   | 27 ++++++++++++++++++---------
>   drm/nouveau/nvkm/subdev/bios/shadowof.c | 18 ++++++++++++++++--
>   3 files changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/drm/nouveau/nvkm/subdev/bios/priv.h
b/drm/nouveau/nvkm/subdev/bios/priv.h
> index e0ec2a6..212800e 100644
> --- a/drm/nouveau/nvkm/subdev/bios/priv.h
> +++ b/drm/nouveau/nvkm/subdev/bios/priv.h
> @@ -8,7 +8,10 @@ struct nvbios_source {
>   	void *(*init)(struct nvkm_bios *, const char *);
>   	void  (*fini)(void *);
>   	u32   (*read)(void *, u32 offset, u32 length, struct nvkm_bios *);
> +	u32   (*size)(void *);
>   	bool rw;
> +	bool ignore_checksum;
> +	bool no_pcir;
>   };
>
>   int nvbios_extend(struct nvkm_bios *, u32 length);
> diff --git a/drm/nouveau/nvkm/subdev/bios/shadow.c
b/drm/nouveau/nvkm/subdev/bios/shadow.c
> index 792f017..b2557e8 100644
> --- a/drm/nouveau/nvkm/subdev/bios/shadow.c
> +++ b/drm/nouveau/nvkm/subdev/bios/shadow.c
> @@ -45,7 +45,7 @@ shadow_fetch(struct nvkm_bios *bios, struct shadow *mthd,
u32 upto)
>   		u32 read = mthd->func->read(data, start, limit - start, bios);
>   		bios->size = start + read;
>   	}
> -	return bios->size >= limit;
> +	return bios->size >= upto;
>   }
>
>   static int
> @@ -55,14 +55,22 @@ shadow_image(struct nvkm_bios *bios, int idx, u32
offset, struct shadow *mthd)
>   	struct nvbios_image image;
>   	int score = 1;
>
> -	if (!shadow_fetch(bios, mthd, offset + 0x1000)) {
> -		nvkm_debug(subdev, "%08x: header fetch failed\n", offset);
> -		return 0;
> -	}
> +	if (mthd->func->no_pcir) {
> +		image.base = 0;
> +		image.type = 0;
> +		image.size = mthd->func->size(mthd->data);
> +		image.last = 1;
> +	} else {
> +		if (!shadow_fetch(bios, mthd, offset + 0x1000)) {
> +			nvkm_debug(subdev, "%08x: header fetch failed\n",
> +				   offset);
> +			return 0;
> +		}
>
> -	if (!nvbios_image(bios, idx, &image)) {
> -		nvkm_debug(subdev, "image %d invalid\n", idx);
> -		return 0;
> +		if (!nvbios_image(bios, idx, &image)) {
> +			nvkm_debug(subdev, "image %d invalid\n", idx);
> +			return 0;
> +		}
>   	}
>   	nvkm_debug(subdev, "%08x: type %02x, %d bytes\n",
>   		   image.base, image.type, image.size);
> @@ -74,7 +82,8 @@ shadow_image(struct nvkm_bios *bios, int idx, u32 offset,
struct shadow *mthd)
>
>   	switch (image.type) {
>   	case 0x00:
> -		if (nvbios_checksum(&bios->data[image.base], image.size)) {
> +		if (!mthd->func->ignore_checksum &&
> +		    nvbios_checksum(&bios->data[image.base], image.size)) {
>   			nvkm_debug(subdev, "%08x: checksum failed\n",
>   				   image.base);
>   			if (mthd->func->rw)
> diff --git a/drm/nouveau/nvkm/subdev/bios/shadowof.c
b/drm/nouveau/nvkm/subdev/bios/shadowof.c
> index 29a37f0..4a20584 100644
> --- a/drm/nouveau/nvkm/subdev/bios/shadowof.c
> +++ b/drm/nouveau/nvkm/subdev/bios/shadowof.c
> @@ -22,6 +22,7 @@
>    */
>   #include "priv.h"
>
> +#include <core/pci.h>
>
>   #if defined(__powerpc__)
>   struct priv {
> @@ -33,17 +34,27 @@ static u32
>   of_read(void *data, u32 offset, u32 length, struct nvkm_bios *bios)
>   {
>   	struct priv *priv = data;
> -	if (offset + length <= priv->size) {
> +	if (offset < priv->size) {
> +		length = min_t(u32, length, priv->size - offset);
>   		memcpy_fromio(bios->data + offset, priv->data + offset, length);
>   		return length;
>   	}
>   	return 0;
>   }
>
> +static u32
> +of_size(void *data)
> +{
> +	struct priv *priv = data;
> +
> +	return priv->size;
> +}
> +
>   static void *
>   of_init(struct nvkm_bios *bios, const char *name)
>   {
> -	struct pci_dev *pdev = bios->subdev.device->pdev;
> +	struct nvkm_device *device = bios->subdev.device;
> +	struct pci_dev *pdev = device->func->pci(device)->pdev;
>   	struct device_node *dn;
>   	struct priv *priv;
>   	if (!(dn = pci_device_to_OF_node(pdev)))
> @@ -62,7 +73,10 @@ nvbios_of = {
>   	.init = of_init,
>   	.fini = (void(*)(void *))kfree,
>   	.read = of_read,
> +	.size = of_size,
>   	.rw = false,
> +	.ignore_checksum = true,
> +	.no_pcir = true,
>   };
>   #else
>   const struct nvbios_source
>

On Fri, Oct 2, 2015 at 3:18 AM, Hans de Goede <hdegoede at redhat.com>
wrote:
> Hi,
>
> On 02-10-15 05:41, Ilia Mirkin wrote:
>
> <nothing>
>
> As someone who has recently started following nouveau I must say that
> it would greatly help me (and likely others) if patches likes this would
> come with a somewhat more descriptive commit message.
Duly noted. I normally try to say a bit, but have gotten lazy of late.
Ben's pretty terse too :) How about something like

"""
Currently OF bios load fails for a few reasons:
 - checksum failure
 - bios size too small
 - no PCIR header
 - bios length not a multiple of 4

In this change, we resolve all of the above by ignoring any checksum
failures, and faking the PCIR data when loading from OF.
"""
>
> Otherwise keep up the good work!
>
> Regards,
>
> Hans
>
>
> p.s.
>
> I've seen your message on the mesa fire demo on nv34, firing that up
now.
>
>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>
>> Tested on a PowerMac7,3 with a NV34 AGP adapter.
>>
>>   drm/nouveau/nvkm/subdev/bios/priv.h     |  3 +++
>>   drm/nouveau/nvkm/subdev/bios/shadow.c   | 27
++++++++++++++++++---------
>>   drm/nouveau/nvkm/subdev/bios/shadowof.c | 18 ++++++++++++++++--
>>   3 files changed, 37 insertions(+), 11 deletions(-)
>>
>> diff --git a/drm/nouveau/nvkm/subdev/bios/priv.h
>> b/drm/nouveau/nvkm/subdev/bios/priv.h
>> index e0ec2a6..212800e 100644
>> --- a/drm/nouveau/nvkm/subdev/bios/priv.h
>> +++ b/drm/nouveau/nvkm/subdev/bios/priv.h
>> @@ -8,7 +8,10 @@ struct nvbios_source {
>>         void *(*init)(struct nvkm_bios *, const char *);
>>         void  (*fini)(void *);
>>         u32   (*read)(void *, u32 offset, u32 length, struct nvkm_bios
*);
>> +       u32   (*size)(void *);
>>         bool rw;
>> +       bool ignore_checksum;
>> +       bool no_pcir;
>>   };
>>
>>   int nvbios_extend(struct nvkm_bios *, u32 length);
>> diff --git a/drm/nouveau/nvkm/subdev/bios/shadow.c
>> b/drm/nouveau/nvkm/subdev/bios/shadow.c
>> index 792f017..b2557e8 100644
>> --- a/drm/nouveau/nvkm/subdev/bios/shadow.c
>> +++ b/drm/nouveau/nvkm/subdev/bios/shadow.c
>> @@ -45,7 +45,7 @@ shadow_fetch(struct nvkm_bios *bios, struct shadow
>> *mthd, u32 upto)
>>                 u32 read = mthd->func->read(data, start, limit -
start,
>> bios);
>>                 bios->size = start + read;
>>         }
>> -       return bios->size >= limit;
>> +       return bios->size >= upto;
>>   }
>>
>>   static int
>> @@ -55,14 +55,22 @@ shadow_image(struct nvkm_bios *bios, int idx, u32
>> offset, struct shadow *mthd)
>>         struct nvbios_image image;
>>         int score = 1;
>>
>> -       if (!shadow_fetch(bios, mthd, offset + 0x1000)) {
>> -               nvkm_debug(subdev, "%08x: header fetch
failed\n", offset);
>> -               return 0;
>> -       }
>> +       if (mthd->func->no_pcir) {
>> +               image.base = 0;
>> +               image.type = 0;
>> +               image.size = mthd->func->size(mthd->data);
>> +               image.last = 1;
>> +       } else {
>> +               if (!shadow_fetch(bios, mthd, offset + 0x1000)) {
>> +                       nvkm_debug(subdev, "%08x: header fetch
failed\n",
>> +                                  offset);
>> +                       return 0;
>> +               }
>>
>> -       if (!nvbios_image(bios, idx, &image)) {
>> -               nvkm_debug(subdev, "image %d invalid\n",
idx);
>> -               return 0;
>> +               if (!nvbios_image(bios, idx, &image)) {
>> +                       nvkm_debug(subdev, "image %d
invalid\n", idx);
>> +                       return 0;
>> +               }
>>         }
>>         nvkm_debug(subdev, "%08x: type %02x, %d bytes\n",
>>                    image.base, image.type, image.size);
>> @@ -74,7 +82,8 @@ shadow_image(struct nvkm_bios *bios, int idx, u32
>> offset, struct shadow *mthd)
>>
>>         switch (image.type) {
>>         case 0x00:
>> -               if (nvbios_checksum(&bios->data[image.base],
image.size))
>> {
>> +               if (!mthd->func->ignore_checksum &&
>> +                   nvbios_checksum(&bios->data[image.base],
image.size))
>> {
>>                         nvkm_debug(subdev, "%08x: checksum
failed\n",
>>                                    image.base);
>>                         if (mthd->func->rw)
>> diff --git a/drm/nouveau/nvkm/subdev/bios/shadowof.c
>> b/drm/nouveau/nvkm/subdev/bios/shadowof.c
>> index 29a37f0..4a20584 100644
>> --- a/drm/nouveau/nvkm/subdev/bios/shadowof.c
>> +++ b/drm/nouveau/nvkm/subdev/bios/shadowof.c
>> @@ -22,6 +22,7 @@
>>    */
>>   #include "priv.h"
>>
>> +#include <core/pci.h>
>>
>>   #if defined(__powerpc__)
>>   struct priv {
>> @@ -33,17 +34,27 @@ static u32
>>   of_read(void *data, u32 offset, u32 length, struct nvkm_bios *bios)
>>   {
>>         struct priv *priv = data;
>> -       if (offset + length <= priv->size) {
>> +       if (offset < priv->size) {
>> +               length = min_t(u32, length, priv->size - offset);
>>                 memcpy_fromio(bios->data + offset, priv->data +
offset,
>> length);
>>                 return length;
>>         }
>>         return 0;
>>   }
>>
>> +static u32
>> +of_size(void *data)
>> +{
>> +       struct priv *priv = data;
>> +
>> +       return priv->size;
>> +}
>> +
>>   static void *
>>   of_init(struct nvkm_bios *bios, const char *name)
>>   {
>> -       struct pci_dev *pdev = bios->subdev.device->pdev;
>> +       struct nvkm_device *device = bios->subdev.device;
>> +       struct pci_dev *pdev =
device->func->pci(device)->pdev;
>>         struct device_node *dn;
>>         struct priv *priv;
>>         if (!(dn = pci_device_to_OF_node(pdev)))
>> @@ -62,7 +73,10 @@ nvbios_of = {
>>         .init = of_init,
>>         .fini = (void(*)(void *))kfree,
>>         .read = of_read,
>> +       .size = of_size,
>>         .rw = false,
>> +       .ignore_checksum = true,
>> +       .no_pcir = true,
>>   };
>>   #else
>>   const struct nvbios_source
>>
>