All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] amdgpu/drm: Refine the way to check atom bios
@ 2016-12-20  7:20 Xue, Ken
       [not found] ` <CY4PR12MB1349868F850BECDFF57AD41CFB900-rpdhrqHFk04TyeVEJk5hrgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Xue, Ken @ 2016-12-20  7:20 UTC (permalink / raw)
  To: amd-gfx list; +Cc: Alex Deucher, 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.h        |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c   | 152 ++++++++++++++++-------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  10 +-
 3 files changed, 89 insertions(+), 75 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index b7f521a..dd8acb9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -305,7 +305,7 @@ struct amdgpu_ih_funcs {
 /*
  * BIOS.
  */
-bool amdgpu_get_bios(struct amdgpu_device *adev);
+bool amdgpu_get_atom_bios(struct amdgpu_device *adev);
 bool amdgpu_read_bios(struct amdgpu_device *adev);
 
 /*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
index 4f973a9..86e7b43 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 __iomem *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,7 +103,7 @@ static bool igp_read_bios_from_vram(struct amdgpu_device *adev)
 		return false;
 	}
 
-	if (size == 0 || !AMD_IS_VALID_VBIOS(bios)) {
+	if (!check_atom_bios(bios, size)) {
 		iounmap(bios);
 		return false;
 	}
@@ -82,7 +120,7 @@ static bool igp_read_bios_from_vram(struct amdgpu_device *adev)
 
 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,10 +130,7 @@ 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)) {
+	if (!check_atom_bios(bios, size)) {
 		pci_unmap_rom(adev->pdev, bios);
 		return false;
 	}
@@ -140,7 +175,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,9 +197,9 @@ static bool amdgpu_read_platform_bios(struct amdgpu_device *adev)
 		return false;
 	}
 
-	if (size == 0 || !AMD_IS_VALID_VBIOS(bios)) {
+	if (!check_atom_bios(bios, size))
 		return false;
-	}
+
 	adev->bios = kmemdup(bios, size, GFP_KERNEL);
 	if (adev->bios == NULL) {
 		return false;
@@ -273,7 +315,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 +340,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 +350,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 +371,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)
@@ -352,59 +398,29 @@ static inline bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
 }
 #endif
 
-bool amdgpu_get_bios(struct amdgpu_device *adev)
+bool amdgpu_get_atom_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 c4075b7..a77331c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1752,16 +1752,14 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 		vga_switcheroo_init_domain_pm_ops(adev->dev, &adev->vga_pm_domain);
 
 	/* Read BIOS */
-	if (!amdgpu_get_bios(adev)) {
+	if (!amdgpu_get_atom_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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] amdgpu/drm: Refine the way to check atom bios
       [not found]     ` <CADnq5_OsNp6SyJqF+fxe5iTdPn5=q0ELv4WNr2akUOO3dV2KKw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-12-20  9:54       ` Ken Xue
  0 siblings, 0 replies; 3+ messages in thread
From: Ken Xue @ 2016-12-20  9:54 UTC (permalink / raw)
  To: Alex Deucher
  Cc: dl.SRDC_SW_GPUVirtualization-5C7GfCeVMHo,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ken.Xue-5C7GfCeVMHo

Th 12/20/2016 12:09, Alex Deucher wrote:
> On Tue, Dec 20, 2016 at 2:20 AM, Xue, Ken <Ken.Xue@amd.com> wrote:
> > 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.h        |   2 +-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c   | 152 ++++++++++++++++-------------
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  10 +-
> >  3 files changed, 89 insertions(+), 75 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index b7f521a..dd8acb9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -305,7 +305,7 @@ struct amdgpu_ih_funcs {
> >  /*
> >   * BIOS.
> >   */
> > -bool amdgpu_get_bios(struct amdgpu_device *adev);
> > +bool amdgpu_get_atom_bios(struct amdgpu_device *adev);
> 
> No need to change the name of this function in my opinion, but I'm ok
> either way.
> 

Ok. I will revert the name of the function.

> >  bool amdgpu_read_bios(struct amdgpu_device *adev);
> >
> >  /*
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> > index 4f973a9..86e7b43 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 __iomem *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,7 +103,7 @@ static bool igp_read_bios_from_vram(struct amdgpu_device *adev)
> >                 return false;
> >         }
> >
> > -       if (size == 0 || !AMD_IS_VALID_VBIOS(bios)) {
> > +       if (!check_atom_bios(bios, size)) {
> >                 iounmap(bios);
> >                 return false;
> >         }
> > @@ -82,7 +120,7 @@ static bool igp_read_bios_from_vram(struct amdgpu_device *adev)
> >
> >  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,10 +130,7 @@ bool amdgpu_read_bios(struct amdgpu_device *adev)
> >                 return false;
> >         }
> >
> > -       val[0] = readb(&bios[0]);
> > -       val[1] = readb(&bios[1]);
> 
> We need to use io accessors on some platforms rather than accessing
> memory directly for pcie resources.  This should be preseved.
> 
> Alex
> 

Sure, Understand it. And I can see that some part of the original file also
does not use "readb" to access iomem, I will send a new patch for keep same
coding style for the file amdgpu_bios.c.

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] amdgpu/drm: Refine the way to check atom bios
       [not found] ` <CY4PR12MB1349868F850BECDFF57AD41CFB900-rpdhrqHFk04TyeVEJk5hrgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
@ 2016-12-20 17:09   ` Alex Deucher
       [not found]     ` <CADnq5_OsNp6SyJqF+fxe5iTdPn5=q0ELv4WNr2akUOO3dV2KKw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Alex Deucher @ 2016-12-20 17:09 UTC (permalink / raw)
  To: Xue, Ken; +Cc: amd-gfx list

On Tue, Dec 20, 2016 at 2:20 AM, Xue, Ken <Ken.Xue@amd.com> wrote:
> 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.h        |   2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c   | 152 ++++++++++++++++-------------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  10 +-
>  3 files changed, 89 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index b7f521a..dd8acb9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -305,7 +305,7 @@ struct amdgpu_ih_funcs {
>  /*
>   * BIOS.
>   */
> -bool amdgpu_get_bios(struct amdgpu_device *adev);
> +bool amdgpu_get_atom_bios(struct amdgpu_device *adev);

No need to change the name of this function in my opinion, but I'm ok
either way.

>  bool amdgpu_read_bios(struct amdgpu_device *adev);
>
>  /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> index 4f973a9..86e7b43 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 __iomem *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,7 +103,7 @@ static bool igp_read_bios_from_vram(struct amdgpu_device *adev)
>                 return false;
>         }
>
> -       if (size == 0 || !AMD_IS_VALID_VBIOS(bios)) {
> +       if (!check_atom_bios(bios, size)) {
>                 iounmap(bios);
>                 return false;
>         }
> @@ -82,7 +120,7 @@ static bool igp_read_bios_from_vram(struct amdgpu_device *adev)
>
>  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,10 +130,7 @@ bool amdgpu_read_bios(struct amdgpu_device *adev)
>                 return false;
>         }
>
> -       val[0] = readb(&bios[0]);
> -       val[1] = readb(&bios[1]);

We need to use io accessors on some platforms rather than accessing
memory directly for pcie resources.  This should be preseved.

Alex

> -
> -       if (size == 0 || !AMD_IS_VALID_VBIOS(val)) {
> +       if (!check_atom_bios(bios, size)) {
>                 pci_unmap_rom(adev->pdev, bios);
>                 return false;
>         }
> @@ -140,7 +175,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,9 +197,9 @@ static bool amdgpu_read_platform_bios(struct amdgpu_device *adev)
>                 return false;
>         }
>
> -       if (size == 0 || !AMD_IS_VALID_VBIOS(bios)) {
> +       if (!check_atom_bios(bios, size))
>                 return false;
> -       }
> +
>         adev->bios = kmemdup(bios, size, GFP_KERNEL);
>         if (adev->bios == NULL) {
>                 return false;
> @@ -273,7 +315,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 +340,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 +350,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 +371,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)
> @@ -352,59 +398,29 @@ static inline bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
>  }
>  #endif
>
> -bool amdgpu_get_bios(struct amdgpu_device *adev)
> +bool amdgpu_get_atom_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 c4075b7..a77331c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1752,16 +1752,14 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>                 vga_switcheroo_init_domain_pm_ops(adev->dev, &adev->vga_pm_domain);
>
>         /* Read BIOS */
> -       if (!amdgpu_get_bios(adev)) {
> +       if (!amdgpu_get_atom_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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-12-20 17:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-20  7:20 [PATCH] amdgpu/drm: Refine the way to check atom bios Xue, Ken
     [not found] ` <CY4PR12MB1349868F850BECDFF57AD41CFB900-rpdhrqHFk04TyeVEJk5hrgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
2016-12-20 17:09   ` Alex Deucher
     [not found]     ` <CADnq5_OsNp6SyJqF+fxe5iTdPn5=q0ELv4WNr2akUOO3dV2KKw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-12-20  9:54       ` Ken Xue

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.