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

[Ken Xue <kxue-5C7GfCeVMHo@public.gmane.org>]

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