Re: [PATCH RFC] drm/radeon: combios tables on older cards

On Sun, Jul 21, 2013 at 11:21 AM, Mark Kettenis mark.kettenis@xs4all.nl wrote:

Noticed that my old Radeon 7500 hung after printing

drm: GPU not posted. posting now...

when it wasn't selected as the primary card the BIOS. Some digging revealed that it was hanging in combios_parse_mmio_table() while parsing the ASIC INIT 3 table. Looking at the BIOS ROM for the card, it becomes obvious that there is no ASIC INIT 3 table in the BIOS. The code is just processing random garbage. No surprise it hangs!

Why do I say that there is no ASIC INIT 3 table is the BIOS? This table is found through the MISC INFO table. The MISC INFO table can be found at offset 0x5e in the COMBIOS header. But the header is smaller than that. The COMBIOS header starts at offset 0x126. The standard PCI Data Structure (the bit that starts with 'PCIR') lives at offset 0x180. That means that the COMBIOS header can not be larger than 0x5a bytes and therefore cannot contain a MISC INFO table.

I looked at a dozen or so BIOS images, some my own, some downloaded from:

<http://www.techpowerup.com/vgabios/index.php?manufacturer=ATI&page=1>

It is fairly obvious that the size of the COMBIOS header can be found at offset 0x6 of the header. Not sure if it is a 16-bit number or just an 8-bit number, but that doesn't really matter since the tables seems to be always smaller than 256 bytes.

So I think combios_get_table_offset() should check if the requested table is present. This can be done by checking the offset against the size of the header. See the diff below. The diff is against the WIP OpenBSD codebase that roughly corresponds to Linux 3.8.13 at this point. But I don't think this bit of the code changed much since then.

Your assessment is correct. I've gone ahead and applied the patch.

Thanks!

Alex

For what it is worth:

Signed-off-by: Mark Kettenis kettenis@openbsd.org

diff --git a/sys/dev/pci/drm/radeon/radeon_combios.c b/sys/dev/pci/drm/radeon/radeon_combios.c index c2b3535..81c3f00 100644 --- a/sys/dev/pci/drm/radeon/radeon_combios.c +++ b/sys/dev/pci/drm/radeon/radeon_combios.c @@ -144,7 +144,7 @@ static uint16_t combios_get_table_offset(struct drm_device *dev, enum radeon_combios_table_offset table) { struct radeon_device *rdev = dev->dev_private;

•   int rev;
  

•   int rev, size;
    uint16_t offset = 0, check_offset;
  
    if (!rdev->bios)
  

@@ -153,174 +153,106 @@ static uint16_t combios_get_table_offset(struct drm_device *dev, switch (table) { /* absolute offset tables */ case COMBIOS_ASIC_INIT_1_TABLE:

•           check_offset = RBIOS16(rdev->bios_header_start + 0xc);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0xc;
            break;
    case COMBIOS_BIOS_SUPPORT_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x14);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x14;
            break;
    case COMBIOS_DAC_PROGRAMMING_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x2a);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x2a;
            break;
    case COMBIOS_MAX_COLOR_DEPTH_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x2c);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x2c;
            break;
    case COMBIOS_CRTC_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x2e);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x2e;
            break;
    case COMBIOS_PLL_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x30);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x30;
            break;
    case COMBIOS_TV_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x32);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x32;
            break;
    case COMBIOS_DFP_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x34);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x34;
            break;
    case COMBIOS_HW_CONFIG_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x36);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x36;
            break;
    case COMBIOS_MULTIMEDIA_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x38);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x38;
            break;
    case COMBIOS_TV_STD_PATCH_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x3e);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x3e;
            break;
    case COMBIOS_LCD_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x40);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x40;
            break;
    case COMBIOS_MOBILE_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x42);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x42;
            break;
    case COMBIOS_PLL_INIT_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x46);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x46;
            break;
    case COMBIOS_MEM_CONFIG_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x48);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x48;
            break;
    case COMBIOS_SAVE_MASK_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x4a);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x4a;
            break;
    case COMBIOS_HARDCODED_EDID_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x4c);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x4c;
            break;
    case COMBIOS_ASIC_INIT_2_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x4e);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x4e;
            break;
    case COMBIOS_CONNECTOR_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x50);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x50;
            break;
    case COMBIOS_DYN_CLK_1_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x52);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x52;
            break;
    case COMBIOS_RESERVED_MEM_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x54);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x54;
            break;
    case COMBIOS_EXT_TMDS_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x58);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x58;
            break;
    case COMBIOS_MEM_CLK_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x5a);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x5a;
            break;
    case COMBIOS_EXT_DAC_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x5c);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x5c;
            break;
    case COMBIOS_MISC_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x5e);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x5e;
            break;
    case COMBIOS_CRT_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x60);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x60;
            break;
    case COMBIOS_INTEGRATED_SYSTEM_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x62);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x62;
            break;
    case COMBIOS_COMPONENT_VIDEO_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x64);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x64;
            break;
    case COMBIOS_FAN_SPEED_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x66);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x66;
            break;
    case COMBIOS_OVERDRIVE_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x68);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x68;
            break;
    case COMBIOS_OEM_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x6a);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x6a;
            break;
    case COMBIOS_DYN_CLK_2_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x6c);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x6c;
            break;
    case COMBIOS_POWER_CONNECTOR_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x6e);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x6e;
            break;
    case COMBIOS_I2C_INFO_TABLE:
  

•           check_offset = RBIOS16(rdev->bios_header_start + 0x70);
  

•           if (check_offset)
  

•                   offset = check_offset;
  

•           check_offset = 0x70;
            break;
            /* relative offset tables */
    case COMBIOS_ASIC_INIT_3_TABLE: /* offset from misc info */
  

@@ -439,8 +371,11 @@ static uint16_t combios_get_table_offset(struct drm_device *dev, break; }

•   return offset;
  

•   size = RBIOS8(rdev->bios_header_start + 0x6);
  

•   if (table < COMBIOS_ASIC_INIT_3_TABLE && check_offset < size)
  

•           offset = RBIOS16(rdev->bios_header_start + check_offset);
  
  

•   return offset;
  

}

bool radeon_combios_check_hardcoded_edid(struct radeon_device *rdev) _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel