<div dir="auto">Hi Ken, a comment below<div><br><div class="gmail_extra"><br><div class="gmail_quote">Den 21 dec. 2016 08:19 skrev "Xue, Ken" <<a href="mailto:Ken.Xue@amd.com">Ken.Xue@amd.com</a>>:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">There are several ways to check out a ATOMBIOS. In previous codes, try<br>
a new way to fetch out vbios/rom, until current vbios/rom is started with<br>
0x55AA, then check if this vbios is ATOMBIOS. Now, try a new way to fetch<br>
out vbios until all flags of ATOMBIOS are verified.<br>
<br>
Signed-off-by: Ken Xue <<a href="mailto:Ken.Xue@amd.com">Ken.Xue@amd.com</a>><br>
---<br>
 drivers/gpu/drm/amd/amdgpu/<wbr>amdgpu_bios.c   | 174 ++++++++++++++++-------------<br>
 drivers/gpu/drm/amd/amdgpu/<wbr>amdgpu_device.c |   8 +-<br>
 2 files changed, 102 insertions(+), 80 deletions(-)<br>
<br>
diff --git a/drivers/gpu/drm/amd/amdgpu/<wbr>amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/<wbr>amdgpu_bios.c<br>
index 4f973a9..a6fa91f 100644<br>
--- a/drivers/gpu/drm/amd/amdgpu/<wbr>amdgpu_bios.c<br>
+++ b/drivers/gpu/drm/amd/amdgpu/<wbr>amdgpu_bios.c<br>
@@ -42,6 +42,44 @@<br>
 #define AMD_IS_VALID_VBIOS(p) ((p)[0] == 0x55 && (p)[1] == 0xAA)<br>
 #define AMD_VBIOS_LENGTH(p) ((p)[2] << 9)<br>
<br>
+/* Check if current bios is an ATOM BIOS.<br>
+ * Return true if it is ATOM BIOS. Otherwise, return false.<br>
+ */<br>
+static bool check_atom_bios(uint8_t *bios, size_t size)<br>
+{<br>
+       uint16_t tmp, bios_header_start;<br>
+<br>
+       if (!bios || size < 0x49) {<br>
+               DRM_INFO("vbios mem is null or mem size is wrong\n");<br>
+               return false;<br>
+       }<br>
+<br>
+       if (!AMD_IS_VALID_VBIOS(bios)) {<br>
+               DRM_INFO("BIOS signature incorrect %x %x\n", bios[0], bios[1]);<br>
+               return false;<br>
+       }<br>
+<br>
+       tmp = bios[0x18] | (bios[0x19] << 8);<br>
+       if (bios[tmp + 0x14] != 0x0) {<br>
+               DRM_INFO("Not an x86 BIOS ROM\n");<br>
+               return false;<br>
+       }<br>
+<br>
+       bios_header_start = bios[0x48] | (bios[0x49] << 8);<br>
+       if (!bios_header_start) {<br>
+               DRM_INFO("Can't locate bios header\n");<br>
+               return false;<br>
+       }<br>
+       tmp = bios_header_start + 4;<br>
+       if (!memcmp(bios + tmp, "ATOM", 4) ||<br>
+           !memcmp(bios + tmp, "MOTA", 4) ||<br>
+           (size < tmp))<br></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto">Best regards</div><div dir="auto">Nils</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+               return true;<br>
+<br>
+       return false;<br>
+}<br>
+<br>
+<br>
 /* If you boot an IGP board with a discrete card as the primary,<br>
  * the IGP rom is not accessible via the rom bar as the IGP rom is<br>
  * part of the system bios.  On boot, the system bios puts a<br>
@@ -65,10 +103,6 @@ static bool igp_read_bios_from_vram(struct amdgpu_device *adev)<br>
                return false;<br>
        }<br>
<br>
-       if (size == 0 || !AMD_IS_VALID_VBIOS(bios)) {<br>
-               iounmap(bios);<br>
-               return false;<br>
-       }<br>
        adev->bios = kmalloc(size, GFP_KERNEL);<br>
        if (!adev->bios) {<br>
                iounmap(bios);<br>
@@ -77,12 +111,18 @@ static bool igp_read_bios_from_vram(struct amdgpu_device *adev)<br>
        adev->bios_size = size;<br>
        memcpy_fromio(adev->bios, bios, size);<br>
        iounmap(bios);<br>
+<br>
+       if (!check_atom_bios(adev->bios, size)) {<br>
+               kfree(adev->bios);<br>
+               return false;<br>
+       }<br>
+<br>
        return true;<br>
 }<br>
<br>
 bool amdgpu_read_bios(struct amdgpu_device *adev)<br>
 {<br>
-       uint8_t __iomem *bios, val[2];<br>
+       uint8_t __iomem *bios;<br>
        size_t size;<br>
<br>
        adev->bios = NULL;<br>
@@ -92,13 +132,6 @@ bool amdgpu_read_bios(struct amdgpu_device *adev)<br>
                return false;<br>
        }<br>
<br>
-       val[0] = readb(&bios[0]);<br>
-       val[1] = readb(&bios[1]);<br>
-<br>
-       if (size == 0 || !AMD_IS_VALID_VBIOS(val)) {<br>
-               pci_unmap_rom(adev->pdev, bios);<br>
-               return false;<br>
-       }<br>
        adev->bios = kzalloc(size, GFP_KERNEL);<br>
        if (adev->bios == NULL) {<br>
                pci_unmap_rom(adev->pdev, bios);<br>
@@ -107,6 +140,12 @@ bool amdgpu_read_bios(struct amdgpu_device *adev)<br>
        adev->bios_size = size;<br>
        memcpy_fromio(adev->bios, bios, size);<br>
        pci_unmap_rom(adev->pdev, bios);<br>
+<br>
+       if (!check_atom_bios(adev->bios, size)) {<br>
+               kfree(adev->bios);<br>
+               return false;<br>
+       }<br>
+<br>
        return true;<br>
 }<br>
<br>
@@ -140,7 +179,14 @@ static bool amdgpu_read_bios_from_rom(<wbr>struct amdgpu_device *adev)<br>
        adev->bios_size = len;<br>
<br>
        /* read complete BIOS */<br>
-       return amdgpu_asic_read_bios_from_<wbr>rom(adev, adev->bios, len);<br>
+       amdgpu_asic_read_bios_from_<wbr>rom(adev, adev->bios, len);<br>
+<br>
+       if (!check_atom_bios(adev->bios, len)) {<br>
+               kfree(adev->bios);<br>
+               return false;<br>
+       }<br>
+<br>
+       return true;<br>
 }<br>
<br>
 static bool amdgpu_read_platform_bios(<wbr>struct amdgpu_device *adev)<br>
@@ -155,13 +201,17 @@ static bool amdgpu_read_platform_bios(<wbr>struct amdgpu_device *adev)<br>
                return false;<br>
        }<br>
<br>
-       if (size == 0 || !AMD_IS_VALID_VBIOS(bios)) {<br>
+       adev->bios = kzalloc(size, GFP_KERNEL);<br>
+       if (adev->bios == NULL)<br>
                return false;<br>
-       }<br>
-       adev->bios = kmemdup(bios, size, GFP_KERNEL);<br>
-       if (adev->bios == NULL) {<br>
+<br>
+       memcpy_fromio(adev->bios, bios, size);<br>
+<br>
+       if (!check_atom_bios(adev->bios, size)) {<br>
+               kfree(adev->bios);<br>
                return false;<br>
        }<br>
+<br>
        adev->bios_size = size;<br>
<br>
        return true;<br>
@@ -273,7 +323,7 @@ static bool amdgpu_atrm_get_bios(struct amdgpu_device *adev)<br>
                        break;<br>
        }<br>
<br>
-       if (i == 0 || !AMD_IS_VALID_VBIOS(adev-><wbr>bios)) {<br>
+       if (!check_atom_bios(adev->bios, size)) {<br>
                kfree(adev->bios);<br>
                return false;<br>
        }<br>
@@ -298,7 +348,6 @@ static bool amdgpu_read_disabled_bios(<wbr>struct amdgpu_device *adev)<br>
 #ifdef CONFIG_ACPI<br>
 static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)<br>
 {<br>
-       bool ret = false;<br>
        struct acpi_table_header *hdr;<br>
        acpi_size tbl_size;<br>
        UEFI_ACPI_VFCT *vfct;<br>
@@ -309,13 +358,13 @@ static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)<br>
                return false;<br>
        if (tbl_size < sizeof(UEFI_ACPI_VFCT)) {<br>
                DRM_ERROR("ACPI VFCT table present but broken (too short #1)\n");<br>
-               goto out_unmap;<br>
+               return false;<br>
        }<br>
<br>
        vfct = (UEFI_ACPI_VFCT *)hdr;<br>
        if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) > tbl_size) {<br>
                DRM_ERROR("ACPI VFCT table present but broken (too short #2)\n");<br>
-               goto out_unmap;<br>
+               return false;<br>
        }<br>
<br>
        vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + vfct->VBIOSImageOffset);<br>
@@ -330,20 +379,25 @@ static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)<br>
            vhdr->VendorID != adev->pdev->vendor ||<br>
            vhdr->DeviceID != adev->pdev->device) {<br>
                DRM_INFO("ACPI VFCT table is not for this card\n");<br>
-               goto out_unmap;<br>
+               return false;<br>
        }<br>
<br>
        if (vfct->VBIOSImageOffset + sizeof(VFCT_IMAGE_HEADER) + vhdr->ImageLength > tbl_size) {<br>
                DRM_ERROR("ACPI VFCT image truncated\n");<br>
-               goto out_unmap;<br>
+               return false;<br>
        }<br>
<br>
-       adev->bios = kmemdup(&vbios->VbiosContent, vhdr->ImageLength, GFP_KERNEL);<br>
+       adev->bios = kmemdup(&vbios->VbiosContent,<br>
+                               vhdr->ImageLength,<br>
+                               GFP_KERNEL);<br>
+<br>
+       if (!check_atom_bios(adev->bios, vhdr->ImageLength)) {<br>
+               kfree(adev->bios);<br>
+               return false;<br>
+       }<br>
        adev->bios_size = vhdr->ImageLength;<br>
-       ret = !!adev->bios;<br>
<br>
-out_unmap:<br>
-       return ret;<br>
+       return true;<br>
 }<br>
 #else<br>
 static inline bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)<br>
@@ -354,57 +408,27 @@ static inline bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)<br>
<br>
 bool amdgpu_get_bios(struct amdgpu_device *adev)<br>
 {<br>
-       bool r;<br>
-       uint16_t tmp, bios_header_start;<br>
+       if (amdgpu_atrm_get_bios(adev))<br>
+               return true;<br>
<br>
-       r = amdgpu_atrm_get_bios(adev);<br>
-       if (!r)<br>
-               r = amdgpu_acpi_vfct_bios(adev);<br>
-       if (!r)<br>
-               r = igp_read_bios_from_vram(adev);<br>
-       if (!r)<br>
-               r = amdgpu_read_bios(adev);<br>
-       if (!r) {<br>
-               r = amdgpu_read_bios_from_rom(<wbr>adev);<br>
-       }<br>
-       if (!r) {<br>
-               r = amdgpu_read_disabled_bios(<wbr>adev);<br>
-       }<br>
-       if (!r) {<br>
-               r = amdgpu_read_platform_bios(<wbr>adev);<br>
-       }<br>
-       if (!r || adev->bios == NULL) {<br>
-               DRM_ERROR("Unable to locate a BIOS ROM\n");<br>
-               adev->bios = NULL;<br>
-               return false;<br>
-       }<br>
-       if (!AMD_IS_VALID_VBIOS(adev-><wbr>bios)) {<br>
-               printk("BIOS signature incorrect %x %x\n", adev->bios[0], adev->bios[1]);<br>
-               goto free_bios;<br>
-       }<br>
+       if (amdgpu_acpi_vfct_bios(adev))<br>
+               return true;<br>
<br>
-       tmp = RBIOS16(0x18);<br>
-       if (RBIOS8(tmp + 0x14) != 0x0) {<br>
-               DRM_INFO("Not an x86 BIOS ROM, not using.\n");<br>
-               goto free_bios;<br>
-       }<br>
+       if (igp_read_bios_from_vram(adev)<wbr>)<br>
+               return true;<br>
<br>
-       bios_header_start = RBIOS16(0x48);<br>
-       if (!bios_header_start) {<br>
-               goto free_bios;<br>
-       }<br>
-       tmp = bios_header_start + 4;<br>
-       if (!memcmp(adev->bios + tmp, "ATOM", 4) ||<br>
-           !memcmp(adev->bios + tmp, "MOTA", 4)) {<br>
-               adev->is_atom_bios = true;<br>
-       } else {<br>
-               adev->is_atom_bios = false;<br>
-       }<br>
+       if (amdgpu_read_bios(adev))<br>
+               return true;<br>
<br>
-       DRM_DEBUG("%sBIOS detected\n", adev->is_atom_bios ? "ATOM" : "COM");<br>
-       return true;<br>
-free_bios:<br>
-       kfree(adev->bios);<br>
-       adev->bios = NULL;<br>
+       if (amdgpu_read_bios_from_rom(<wbr>adev))<br>
+               return true;<br>
+<br>
+       if (amdgpu_read_disabled_bios(<wbr>adev))<br>
+               return true;<br>
+<br>
+       if (amdgpu_read_platform_bios(<wbr>adev))<br>
+               return true;<br>
+<br>
+       DRM_ERROR("Unable to locate a BIOS ROM\n");<br>
        return false;<br>
 }<br>
diff --git a/drivers/gpu/drm/amd/amdgpu/<wbr>amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/<wbr>amdgpu_device.c<br>
index b7f17c2..e23acbe 100644<br>
--- a/drivers/gpu/drm/amd/amdgpu/<wbr>amdgpu_device.c<br>
+++ b/drivers/gpu/drm/amd/amdgpu/<wbr>amdgpu_device.c<br>
@@ -1753,15 +1753,13 @@ int amdgpu_device_init(struct amdgpu_device *adev,<br>
<br>
        /* Read BIOS */<br>
        if (!amdgpu_get_bios(adev)) {<br>
+               /* Must be an ATOMBIOS */<br>
                r = -EINVAL;<br>
-               goto failed;<br>
-       }<br>
-       /* Must be an ATOMBIOS */<br>
-       if (!adev->is_atom_bios) {<br>
                dev_err(adev->dev, "Expecting atombios for GPU\n");<br>
-               r = -EINVAL;<br>
                goto failed;<br>
        }<br>
+       adev->is_atom_bios = true;<br>
+<br>
        r = amdgpu_atombios_init(adev);<br>
        if (r) {<br>
                dev_err(adev->dev, "amdgpu_atombios_init failed\n");<br>
<font color="#888888">--<br>
2.7.4<br>
<br>
______________________________<wbr>_________________<br>
amd-gfx mailing list<br>
<a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/amd-gfx" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/amd-gfx</a><br>
</font></blockquote></div><br></div></div></div>