On Sun, Jul 21, 2013 at 11:21 AM, Mark Kettenis <mark.kettenis@xxxxxxxxx> 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@xxxxxxxxxxx> > > > 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
