[PATCH V2] amdgpu/drm: Refine the way to check

[PATCH V2] amdgpu/drm: Refine the way to check

[Xue, Ken]

There are several ways to check out a ATOMBIOS. In previous codes, try
a new way to fetch out vbios/rom, until current vbios/rom is started with
0x55AA, then check if this vbios is ATOMBIOS. Now, try a new way to fetch
out vbios until all flags of ATOMBIOS are verified.

Signed-off-by: Ken Xue <Ken.Xue@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c   | 174 ++++++++++++++++-------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |   8 +-
 2 files changed, 102 insertions(+), 80 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
index 4f973a9..a6fa91f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
@@ -42,6 +42,44 @@
 #define AMD_IS_VALID_VBIOS(p) ((p)[0] == 0x55 && (p)[1] == 0xAA)
 #define AMD_VBIOS_LENGTH(p) ((p)[2] << 9)
 
+/* Check if current bios is an ATOM BIOS.
+ * Return true if it is ATOM BIOS. Otherwise, return false.
+ */
+static bool check_atom_bios(uint8_t *bios, size_t size)
+{
+	uint16_t tmp, bios_header_start;
+
+	if (!bios || size < 0x49) {
+		DRM_INFO("vbios mem is null or mem size is wrong\n");
+		return false;
+	}
+
+	if (!AMD_IS_VALID_VBIOS(bios)) {
+		DRM_INFO("BIOS signature incorrect %x %x\n", bios[0], bios[1]);
+		return false;
+	}
+
+	tmp = bios[0x18] | (bios[0x19] << 8);
+	if (bios[tmp + 0x14] != 0x0) {
+		DRM_INFO("Not an x86 BIOS ROM\n");
+		return false;
+	}
+
+	bios_header_start = bios[0x48] | (bios[0x49] << 8);
+	if (!bios_header_start) {
+		DRM_INFO("Can't locate bios header\n");
+		return false;
+	}
+	tmp = bios_header_start + 4;
+	if (!memcmp(bios + tmp, "ATOM", 4) ||
+	    !memcmp(bios + tmp, "MOTA", 4) ||
+	    (size < tmp))
+		return true;
+
+	return false;
+}
+
+
 /* If you boot an IGP board with a discrete card as the primary,
  * the IGP rom is not accessible via the rom bar as the IGP rom is
  * part of the system bios.  On boot, the system bios puts a
@@ -65,10 +103,6 @@ static bool igp_read_bios_from_vram(struct amdgpu_device *adev)
 		return false;
 	}
 
-	if (size == 0 || !AMD_IS_VALID_VBIOS(bios)) {
-		iounmap(bios);
-		return false;
-	}
 	adev->bios = kmalloc(size, GFP_KERNEL);
 	if (!adev->bios) {
 		iounmap(bios);
@@ -77,12 +111,18 @@ static bool igp_read_bios_from_vram(struct amdgpu_device *adev)
 	adev->bios_size = size;
 	memcpy_fromio(adev->bios, bios, size);
 	iounmap(bios);
+
+	if (!check_atom_bios(adev->bios, size)) {
+		kfree(adev->bios);
+		return false;
+	}
+
 	return true;
 }
 
 bool amdgpu_read_bios(struct amdgpu_device *adev)
 {
-	uint8_t __iomem *bios, val[2];
+	uint8_t __iomem *bios;
 	size_t size;
 
 	adev->bios = NULL;
@@ -92,13 +132,6 @@ bool amdgpu_read_bios(struct amdgpu_device *adev)
 		return false;
 	}
 
-	val[0] = readb(&bios[0]);
-	val[1] = readb(&bios[1]);
-
-	if (size == 0 || !AMD_IS_VALID_VBIOS(val)) {
-		pci_unmap_rom(adev->pdev, bios);
-		return false;
-	}
 	adev->bios = kzalloc(size, GFP_KERNEL);
 	if (adev->bios == NULL) {
 		pci_unmap_rom(adev->pdev, bios);
@@ -107,6 +140,12 @@ bool amdgpu_read_bios(struct amdgpu_device *adev)
 	adev->bios_size = size;
 	memcpy_fromio(adev->bios, bios, size);
 	pci_unmap_rom(adev->pdev, bios);
+
+	if (!check_atom_bios(adev->bios, size)) {
+		kfree(adev->bios);
+		return false;
+	}
+
 	return true;
 }
 
@@ -140,7 +179,14 @@ static bool amdgpu_read_bios_from_rom(struct amdgpu_device *adev)
 	adev->bios_size = len;
 
 	/* read complete BIOS */
-	return amdgpu_asic_read_bios_from_rom(adev, adev->bios, len);
+	amdgpu_asic_read_bios_from_rom(adev, adev->bios, len);
+
+	if (!check_atom_bios(adev->bios, len)) {
+		kfree(adev->bios);
+		return false;
+	}
+
+	return true;
 }
 
 static bool amdgpu_read_platform_bios(struct amdgpu_device *adev)
@@ -155,13 +201,17 @@ static bool amdgpu_read_platform_bios(struct amdgpu_device *adev)
 		return false;
 	}
 
-	if (size == 0 || !AMD_IS_VALID_VBIOS(bios)) {
+	adev->bios = kzalloc(size, GFP_KERNEL);
+	if (adev->bios == NULL)
 		return false;
-	}
-	adev->bios = kmemdup(bios, size, GFP_KERNEL);
-	if (adev->bios == NULL) {
+
+	memcpy_fromio(adev->bios, bios, size);
+
+	if (!check_atom_bios(adev->bios, size)) {
+		kfree(adev->bios);
 		return false;
 	}
+
 	adev->bios_size = size;
 
 	return true;
@@ -273,7 +323,7 @@ static bool amdgpu_atrm_get_bios(struct amdgpu_device *adev)
 			break;
 	}
 
-	if (i == 0 || !AMD_IS_VALID_VBIOS(adev->bios)) {
+	if (!check_atom_bios(adev->bios, size)) {
 		kfree(adev->bios);
 		return false;
 	}
@@ -298,7 +348,6 @@ static bool amdgpu_read_disabled_bios(struct amdgpu_device *adev)
 #ifdef CONFIG_ACPI
 static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
 {
-	bool ret = false;
 	struct acpi_table_header *hdr;
 	acpi_size tbl_size;
 	UEFI_ACPI_VFCT *vfct;
@@ -309,13 +358,13 @@ static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
 		return false;
 	if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
 		DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");
-		goto out_unmap;
+		return false;
 	}
 
 	vfct = (UEFI_ACPI_VFCT *)hdr;
 	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) > tbl_size) {
 		DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");
-		goto out_unmap;
+		return false;
 	}
 
 	vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + vfct->VBIOSImageOffset);
@@ -330,20 +379,25 @@ static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
 	    vhdr->VendorID != adev->pdev->vendor ||
 	    vhdr->DeviceID != adev->pdev->device) {
 		DRM_INFO("ACPI VFCT table is not for this card\n");
-		goto out_unmap;
+		return false;
 	}
 
 	if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) + vhdr->ImageLength > tbl_size) {
 		DRM_ERROR("ACPI VFCT image truncated\n");
-		goto out_unmap;
+		return false;
 	}
 
-	adev->bios = kmemdup(&vbios->VbiosContent, vhdr->ImageLength, GFP_KERNEL);
+	adev->bios = kmemdup(&vbios->VbiosContent,
+				vhdr->ImageLength,
+				GFP_KERNEL);
+
+	if (!check_atom_bios(adev->bios, vhdr->ImageLength)) {
+		kfree(adev->bios);
+		return false;
+	}
 	adev->bios_size = vhdr->ImageLength;
-	ret = !!adev->bios;
 
-out_unmap:
-	return ret;
+	return true;
 }
 #else
 static inline bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
@@ -354,57 +408,27 @@ static inline bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
 
 bool amdgpu_get_bios(struct amdgpu_device *adev)
 {
-	bool r;
-	uint16_t tmp, bios_header_start;
+	if (amdgpu_atrm_get_bios(adev))
+		return true;
 
-	r = amdgpu_atrm_get_bios(adev);
-	if (!r)
-		r = amdgpu_acpi_vfct_bios(adev);
-	if (!r)
-		r = igp_read_bios_from_vram(adev);
-	if (!r)
-		r = amdgpu_read_bios(adev);
-	if (!r) {
-		r = amdgpu_read_bios_from_rom(adev);
-	}
-	if (!r) {
-		r = amdgpu_read_disabled_bios(adev);
-	}
-	if (!r) {
-		r = amdgpu_read_platform_bios(adev);
-	}
-	if (!r || adev->bios == NULL) {
-		DRM_ERROR("Unable to locate a BIOS ROM\n");
-		adev->bios = NULL;
-		return false;
-	}
-	if (!AMD_IS_VALID_VBIOS(adev->bios)) {
-		printk("BIOS signature incorrect %x %x\n", adev->bios[0], adev->bios[1]);
-		goto free_bios;
-	}
+	if (amdgpu_acpi_vfct_bios(adev))
+		return true;
 
-	tmp = RBIOS16(0x18);
-	if (RBIOS8(tmp + 0x14) != 0x0) {
-		DRM_INFO("Not an x86 BIOS ROM, not using.\n");
-		goto free_bios;
-	}
+	if (igp_read_bios_from_vram(adev))
+		return true;
 
-	bios_header_start = RBIOS16(0x48);
-	if (!bios_header_start) {
-		goto free_bios;
-	}
-	tmp = bios_header_start + 4;
-	if (!memcmp(adev->bios + tmp, "ATOM", 4) ||
-	    !memcmp(adev->bios + tmp, "MOTA", 4)) {
-		adev->is_atom_bios = true;
-	} else {
-		adev->is_atom_bios = false;
-	}
+	if (amdgpu_read_bios(adev))
+		return true;
 
-	DRM_DEBUG("%sBIOS detected\n", adev->is_atom_bios ? "ATOM" : "COM");
-	return true;
-free_bios:
-	kfree(adev->bios);
-	adev->bios = NULL;
+	if (amdgpu_read_bios_from_rom(adev))
+		return true;
+
+	if (amdgpu_read_disabled_bios(adev))
+		return true;
+
+	if (amdgpu_read_platform_bios(adev))
+		return true;
+
+	DRM_ERROR("Unable to locate a BIOS ROM\n");
 	return false;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index b7f17c2..e23acbe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1753,15 +1753,13 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 
 	/* Read BIOS */
 	if (!amdgpu_get_bios(adev)) {
+		/* Must be an ATOMBIOS */
 		r = -EINVAL;
-		goto failed;
-	}
-	/* Must be an ATOMBIOS */
-	if (!adev->is_atom_bios) {
 		dev_err(adev->dev, "Expecting atombios for GPU\n");
-		r = -EINVAL;
 		goto failed;
 	}
+	adev->is_atom_bios = true;
+
 	r = amdgpu_atombios_init(adev);
 	if (r) {
 		dev_err(adev->dev, "amdgpu_atombios_init failed\n");
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH V2] amdgpu/drm: Refine the way to check

[Nils Wallménius]

[-- Attachment #1.1: Type: text/plain, Size: 11032 bytes --]

Hi Ken, a comment below


Den 21 dec. 2016 08:19 skrev "Xue, Ken" <Ken.Xue-5C7GfCeVMHo@public.gmane.org>:

There are several ways to check out a ATOMBIOS. In previous codes, try
a new way to fetch out vbios/rom, until current vbios/rom is started with
0x55AA, then check if this vbios is ATOMBIOS. Now, try a new way to fetch
out vbios until all flags of ATOMBIOS are verified.

Signed-off-by: Ken Xue <Ken.Xue-5C7GfCeVMHo@public.gmane.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c   | 174
++++++++++++++++-------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |   8 +-
 2 files changed, 102 insertions(+), 80 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
index 4f973a9..a6fa91f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
@@ -42,6 +42,44 @@
 #define AMD_IS_VALID_VBIOS(p) ((p)[0] == 0x55 && (p)[1] == 0xAA)
 #define AMD_VBIOS_LENGTH(p) ((p)[2] << 9)

+/* Check if current bios is an ATOM BIOS.
+ * Return true if it is ATOM BIOS. Otherwise, return false.
+ */
+static bool check_atom_bios(uint8_t *bios, size_t size)
+{
+       uint16_t tmp, bios_header_start;
+
+       if (!bios || size < 0x49) {
+               DRM_INFO("vbios mem is null or mem size is wrong\n");
+               return false;
+       }
+
+       if (!AMD_IS_VALID_VBIOS(bios)) {
+               DRM_INFO("BIOS signature incorrect %x %x\n", bios[0],
bios[1]);
+               return false;
+       }
+
+       tmp = bios[0x18] | (bios[0x19] << 8);
+       if (bios[tmp + 0x14] != 0x0) {
+               DRM_INFO("Not an x86 BIOS ROM\n");
+               return false;
+       }
+
+       bios_header_start = bios[0x48] | (bios[0x49] << 8);
+       if (!bios_header_start) {
+               DRM_INFO("Can't locate bios header\n");
+               return false;
+       }
+       tmp = bios_header_start + 4;
+       if (!memcmp(bios + tmp, "ATOM", 4) ||
+           !memcmp(bios + tmp, "MOTA", 4) ||
+           (size < tmp))


Is the last condition here correct? Can you have valid atombios without the
signature? Maybe add a comment if that's true. Also the extra parents
aren't necessary.

Best regards
Nils

+               return true;
+
+       return false;
+}
+
+
 /* If you boot an IGP board with a discrete card as the primary,
  * the IGP rom is not accessible via the rom bar as the IGP rom is
  * part of the system bios.  On boot, the system bios puts a
@@ -65,10 +103,6 @@ static bool igp_read_bios_from_vram(struct
amdgpu_device *adev)
                return false;
        }

-       if (size == 0 || !AMD_IS_VALID_VBIOS(bios)) {
-               iounmap(bios);
-               return false;
-       }
        adev->bios = kmalloc(size, GFP_KERNEL);
        if (!adev->bios) {
                iounmap(bios);
@@ -77,12 +111,18 @@ static bool igp_read_bios_from_vram(struct
amdgpu_device *adev)
        adev->bios_size = size;
        memcpy_fromio(adev->bios, bios, size);
        iounmap(bios);
+
+       if (!check_atom_bios(adev->bios, size)) {
+               kfree(adev->bios);
+               return false;
+       }
+
        return true;
 }

 bool amdgpu_read_bios(struct amdgpu_device *adev)
 {
-       uint8_t __iomem *bios, val[2];
+       uint8_t __iomem *bios;
        size_t size;

        adev->bios = NULL;
@@ -92,13 +132,6 @@ bool amdgpu_read_bios(struct amdgpu_device *adev)
                return false;
        }

-       val[0] = readb(&bios[0]);
-       val[1] = readb(&bios[1]);
-
-       if (size == 0 || !AMD_IS_VALID_VBIOS(val)) {
-               pci_unmap_rom(adev->pdev, bios);
-               return false;
-       }
        adev->bios = kzalloc(size, GFP_KERNEL);
        if (adev->bios == NULL) {
                pci_unmap_rom(adev->pdev, bios);
@@ -107,6 +140,12 @@ bool amdgpu_read_bios(struct amdgpu_device *adev)
        adev->bios_size = size;
        memcpy_fromio(adev->bios, bios, size);
        pci_unmap_rom(adev->pdev, bios);
+
+       if (!check_atom_bios(adev->bios, size)) {
+               kfree(adev->bios);
+               return false;
+       }
+
        return true;
 }

@@ -140,7 +179,14 @@ static bool amdgpu_read_bios_from_rom(struct
amdgpu_device *adev)
        adev->bios_size = len;

        /* read complete BIOS */
-       return amdgpu_asic_read_bios_from_rom(adev, adev->bios, len);
+       amdgpu_asic_read_bios_from_rom(adev, adev->bios, len);
+
+       if (!check_atom_bios(adev->bios, len)) {
+               kfree(adev->bios);
+               return false;
+       }
+
+       return true;
 }

 static bool amdgpu_read_platform_bios(struct amdgpu_device *adev)
@@ -155,13 +201,17 @@ static bool amdgpu_read_platform_bios(struct
amdgpu_device *adev)
                return false;
        }

-       if (size == 0 || !AMD_IS_VALID_VBIOS(bios)) {
+       adev->bios = kzalloc(size, GFP_KERNEL);
+       if (adev->bios == NULL)
                return false;
-       }
-       adev->bios = kmemdup(bios, size, GFP_KERNEL);
-       if (adev->bios == NULL) {
+
+       memcpy_fromio(adev->bios, bios, size);
+
+       if (!check_atom_bios(adev->bios, size)) {
+               kfree(adev->bios);
                return false;
        }
+
        adev->bios_size = size;

        return true;
@@ -273,7 +323,7 @@ static bool amdgpu_atrm_get_bios(struct amdgpu_device
*adev)
                        break;
        }

-       if (i == 0 || !AMD_IS_VALID_VBIOS(adev->bios)) {
+       if (!check_atom_bios(adev->bios, size)) {
                kfree(adev->bios);
                return false;
        }
@@ -298,7 +348,6 @@ static bool amdgpu_read_disabled_bios(struct
amdgpu_device *adev)
 #ifdef CONFIG_ACPI
 static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
 {
-       bool ret = false;
        struct acpi_table_header *hdr;
        acpi_size tbl_size;
        UEFI_ACPI_VFCT *vfct;
@@ -309,13 +358,13 @@ static bool amdgpu_acpi_vfct_bios(struct
amdgpu_device *adev)
                return false;
        if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {
                DRM_ERROR("ACPI VFCT table present but broken (too short
#1)\n");
-               goto out_unmap;
+               return false;
        }

        vfct = (UEFI_ACPI_VFCT *)hdr;
        if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) > tbl_size) {
                DRM_ERROR("ACPI VFCT table present but broken (too short
#2)\n");
-               goto out_unmap;
+               return false;
        }

        vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + vfct->VBIOSImageOffset);
@@ -330,20 +379,25 @@ static bool amdgpu_acpi_vfct_bios(struct
amdgpu_device *adev)
            vhdr->VendorID != adev->pdev->vendor ||
            vhdr->DeviceID != adev->pdev->device) {
                DRM_INFO("ACPI VFCT table is not for this card\n");
-               goto out_unmap;
+               return false;
        }

        if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) +
vhdr->ImageLength > tbl_size) {
                DRM_ERROR("ACPI VFCT image truncated\n");
-               goto out_unmap;
+               return false;
        }

-       adev->bios = kmemdup(&vbios->VbiosContent, vhdr->ImageLength,
GFP_KERNEL);
+       adev->bios = kmemdup(&vbios->VbiosContent,
+                               vhdr->ImageLength,
+                               GFP_KERNEL);
+
+       if (!check_atom_bios(adev->bios, vhdr->ImageLength)) {
+               kfree(adev->bios);
+               return false;
+       }
        adev->bios_size = vhdr->ImageLength;
-       ret = !!adev->bios;

-out_unmap:
-       return ret;
+       return true;
 }
 #else
 static inline bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
@@ -354,57 +408,27 @@ static inline bool amdgpu_acpi_vfct_bios(struct
amdgpu_device *adev)

 bool amdgpu_get_bios(struct amdgpu_device *adev)
 {
-       bool r;
-       uint16_t tmp, bios_header_start;
+       if (amdgpu_atrm_get_bios(adev))
+               return true;

-       r = amdgpu_atrm_get_bios(adev);
-       if (!r)
-               r = amdgpu_acpi_vfct_bios(adev);
-       if (!r)
-               r = igp_read_bios_from_vram(adev);
-       if (!r)
-               r = amdgpu_read_bios(adev);
-       if (!r) {
-               r = amdgpu_read_bios_from_rom(adev);
-       }
-       if (!r) {
-               r = amdgpu_read_disabled_bios(adev);
-       }
-       if (!r) {
-               r = amdgpu_read_platform_bios(adev);
-       }
-       if (!r || adev->bios == NULL) {
-               DRM_ERROR("Unable to locate a BIOS ROM\n");
-               adev->bios = NULL;
-               return false;
-       }
-       if (!AMD_IS_VALID_VBIOS(adev->bios)) {
-               printk("BIOS signature incorrect %x %x\n", adev->bios[0],
adev->bios[1]);
-               goto free_bios;
-       }
+       if (amdgpu_acpi_vfct_bios(adev))
+               return true;

-       tmp = RBIOS16(0x18);
-       if (RBIOS8(tmp + 0x14) != 0x0) {
-               DRM_INFO("Not an x86 BIOS ROM, not using.\n");
-               goto free_bios;
-       }
+       if (igp_read_bios_from_vram(adev))
+               return true;

-       bios_header_start = RBIOS16(0x48);
-       if (!bios_header_start) {
-               goto free_bios;
-       }
-       tmp = bios_header_start + 4;
-       if (!memcmp(adev->bios + tmp, "ATOM", 4) ||
-           !memcmp(adev->bios + tmp, "MOTA", 4)) {
-               adev->is_atom_bios = true;
-       } else {
-               adev->is_atom_bios = false;
-       }
+       if (amdgpu_read_bios(adev))
+               return true;

-       DRM_DEBUG("%sBIOS detected\n", adev->is_atom_bios ? "ATOM" : "COM");
-       return true;
-free_bios:
-       kfree(adev->bios);
-       adev->bios = NULL;
+       if (amdgpu_read_bios_from_rom(adev))
+               return true;
+
+       if (amdgpu_read_disabled_bios(adev))
+               return true;
+
+       if (amdgpu_read_platform_bios(adev))
+               return true;
+
+       DRM_ERROR("Unable to locate a BIOS ROM\n");
        return false;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index b7f17c2..e23acbe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1753,15 +1753,13 @@ int amdgpu_device_init(struct amdgpu_device *adev,

        /* Read BIOS */
        if (!amdgpu_get_bios(adev)) {
+               /* Must be an ATOMBIOS */
                r = -EINVAL;
-               goto failed;
-       }
-       /* Must be an ATOMBIOS */
-       if (!adev->is_atom_bios) {
                dev_err(adev->dev, "Expecting atombios for GPU\n");
-               r = -EINVAL;
                goto failed;
        }
+       adev->is_atom_bios = true;
+
        r = amdgpu_atombios_init(adev);
        if (r) {
                dev_err(adev->dev, "amdgpu_atombios_init failed\n");
--
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[-- Attachment #1.2: Type: text/html, Size: 15417 bytes --]

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

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH V2] amdgpu/drm: Refine the way to check

[Ken Xue]

The 12/21/2016 10:44, Nils Wallménius wrote:
> Hi Ken, a comment below
> 
> 
> Den 21 dec. 2016 08:19 skrev "Xue, Ken" <Ken.Xue@amd.com>:
> 
> There are several ways to check out a ATOMBIOS. In previous codes, try
> a new way to fetch out vbios/rom, until current vbios/rom is started with
> 0x55AA, then check if this vbios is ATOMBIOS. Now, try a new way to fetch
> out vbios until all flags of ATOMBIOS are verified.
> 
> Signed-off-by: Ken Xue <Ken.Xue@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c   | 174
> ++++++++++++++++-------------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |   8 +-
>  2 files changed, 102 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> index 4f973a9..a6fa91f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> @@ -42,6 +42,44 @@
>  #define AMD_IS_VALID_VBIOS(p) ((p)[0] == 0x55 && (p)[1] == 0xAA)
>  #define AMD_VBIOS_LENGTH(p) ((p)[2] << 9)
> 
> +/* Check if current bios is an ATOM BIOS.
> + * Return true if it is ATOM BIOS. Otherwise, return false.
> + */
> +static bool check_atom_bios(uint8_t *bios, size_t size)
> +{
> +       uint16_t tmp, bios_header_start;
> +
> +       if (!bios || size < 0x49) {
> +               DRM_INFO("vbios mem is null or mem size is wrong\n");
> +               return false;
> +       }
> +
> +       if (!AMD_IS_VALID_VBIOS(bios)) {
> +               DRM_INFO("BIOS signature incorrect %x %x\n", bios[0],
> bios[1]);
> +               return false;
> +       }
> +
> +       tmp = bios[0x18] | (bios[0x19] << 8);
> +       if (bios[tmp + 0x14] != 0x0) {
> +               DRM_INFO("Not an x86 BIOS ROM\n");
> +               return false;
> +       }
> +
> +       bios_header_start = bios[0x48] | (bios[0x49] << 8);
> +       if (!bios_header_start) {
> +               DRM_INFO("Can't locate bios header\n");
> +               return false;
> +       }
> +       tmp = bios_header_start + 4;
> +       if (!memcmp(bios + tmp, "ATOM", 4) ||
> +           !memcmp(bios + tmp, "MOTA", 4) ||
> +           (size < tmp))
> 
> 
> Is the last condition here correct? Can you have valid atombios without the
> signature? Maybe add a comment if that's true. Also the extra parents
> aren't necessary.
> 
> Best regards
> Nils
> 
Thanks for your comments. It should be a mistake.

if (size < tmp) {
	DRM_INFO("BIOS header is broken\n");
	return false;
}


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx