| Message ID | 068ea5809a2ebe1e79aef85bc91d05ef69041f1d.1450089383.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Dec 14, 2015 at 12:50:50PM +0200, Jani Nikula wrote: > Make the validation function a boolean operating on a buffer of given > size, removing the extra pointer dances. > > Move the OpRegion based VBT validation to intel_opregion_setup(), only > initializing opregion->vbt if it's valid. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_bios.c | 63 ++++++++++++++++++----------------- > drivers/gpu/drm/i915/intel_opregion.c | 9 +++-- > 3 files changed, 40 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index a2bd4c6a86a1..ef15f1710b50 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -3354,6 +3354,7 @@ extern void intel_i2c_reset(struct drm_device *dev); > > /* intel_bios.c */ > int intel_bios_init(struct drm_device *dev); > +bool intel_bios_is_valid_vbt(const void *buf, size_t size); > > /* intel_opregion.c */ > #ifdef CONFIG_ACPI > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index f2b79b7d12b8..007b2b759600 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -1221,37 +1221,40 @@ static const struct bdb_header *get_bdb_header(const struct vbt_header *vbt) > return _vbt + vbt->bdb_offset; > } > > -static const struct vbt_header *validate_vbt(const void *base, > - size_t size, > - const void *_vbt) > +/** > + * intel_bios_is_valid_vbt - does the given buffer contain a valid VBT > + * @buf: pointer to a buffer to validate > + * @size: size of the buffer > + * > + * Returns true on valid VBT. > + */ > +bool intel_bios_is_valid_vbt(const void *buf, size_t size) > { > - size_t offset = _vbt - base; > - const struct vbt_header *vbt = _vbt; > + const struct vbt_header *vbt = buf; > const struct bdb_header *bdb; > > if (!vbt) > - return NULL; > + return false; > > - if (offset + sizeof(struct vbt_header) > size) { > + if (sizeof(struct vbt_header) > size) { > DRM_DEBUG_DRIVER("VBT header incomplete\n"); > - return NULL; > + return false; > } > > if (memcmp(vbt->signature, "$VBT", 4)) { > DRM_DEBUG_DRIVER("VBT invalid signature\n"); > - return NULL; > + return false; > } > > - offset += vbt->bdb_offset; > - if (offset + sizeof(struct bdb_header) > size) { > + if (vbt->bdb_offset + sizeof(struct bdb_header) > size) { > DRM_DEBUG_DRIVER("BDB header incomplete\n"); > - return NULL; > + return false; > } > > bdb = get_bdb_header(vbt); > - if (offset + bdb->bdb_size > size) { > + if (vbt->bdb_offset + bdb->bdb_size > size) { > DRM_DEBUG_DRIVER("BDB incomplete\n"); > - return NULL; > + return false; > } > > return vbt; > @@ -1259,26 +1262,27 @@ static const struct vbt_header *validate_vbt(const void *base, > > static const struct vbt_header *find_vbt(void __iomem *bios, size_t size) > { > - const struct vbt_header *vbt = NULL; > size_t i; > > /* Scour memory looking for the VBT signature. */ > for (i = 0; i + 4 < size; i++) { > - if (ioread32(bios + i) == *((const u32 *) "$VBT")) { > - /* > - * This is the one place where we explicitly discard the > - * address space (__iomem) of the BIOS/VBT. From now on > - * everything is based on 'base', and treated as regular > - * memory. > - */ > - void *_bios = (void __force *) bios; > + void *vbt; > > - vbt = validate_vbt(_bios, size, _bios + i); > - break; > - } > + if (ioread32(bios + i) != *((const u32 *) "$VBT")) > + continue; > + > + /* > + * This is the one place where we explicitly discard the address > + * space (__iomem) of the BIOS/VBT. > + */ > + vbt = (void __force *) bios + i; > + if (intel_bios_is_valid_vbt(vbt, size - i)) > + return vbt; > + > + break; > } > > - return vbt; > + return NULL; > } > > /** > @@ -1295,7 +1299,7 @@ intel_bios_init(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > struct pci_dev *pdev = dev->pdev; > - const struct vbt_header *vbt; > + const struct vbt_header *vbt = dev_priv->opregion.vbt; > const struct bdb_header *bdb; > u8 __iomem *bios = NULL; > > @@ -1304,9 +1308,6 @@ intel_bios_init(struct drm_device *dev) > > init_vbt_defaults(dev_priv); > > - /* XXX Should this validation be moved to intel_opregion.c? */ > - vbt = validate_vbt(dev_priv->opregion.header, OPREGION_SIZE, > - dev_priv->opregion.vbt); > if (vbt) { > DRM_DEBUG_KMS("Using VBT from ACPI OpRegion: %20s\n", > vbt->signature); Maybe the debug message should be moved too? It looks a bit out of place after the validation gets moved. > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c > index 5b9fc790d300..9f992e7c6185 100644 > --- a/drivers/gpu/drm/i915/intel_opregion.c > +++ b/drivers/gpu/drm/i915/intel_opregion.c > @@ -986,8 +986,13 @@ int intel_opregion_setup(struct drm_device *dev) > if (mboxes & MBOX_ASLE_EXT) > DRM_DEBUG_DRIVER("ASLE extension supported\n"); > > - if (!dmi_check_system(intel_no_opregion_vbt)) > - opregion->vbt = base + OPREGION_VBT_OFFSET; > + if (!dmi_check_system(intel_no_opregion_vbt)) { > + void *vbt = base + OPREGION_VBT_OFFSET; > + u32 vbt_size = OPREGION_SIZE - OPREGION_VBT_OFFSET; > + > + if (intel_bios_is_valid_vbt(vbt, vbt_size)) > + opregion->vbt = vbt; > + } > > return 0; > > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, 14 Dec 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Mon, Dec 14, 2015 at 12:50:50PM +0200, Jani Nikula wrote: >> Make the validation function a boolean operating on a buffer of given >> size, removing the extra pointer dances. >> >> Move the OpRegion based VBT validation to intel_opregion_setup(), only >> initializing opregion->vbt if it's valid. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 1 + >> drivers/gpu/drm/i915/intel_bios.c | 63 ++++++++++++++++++----------------- >> drivers/gpu/drm/i915/intel_opregion.c | 9 +++-- >> 3 files changed, 40 insertions(+), 33 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index a2bd4c6a86a1..ef15f1710b50 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -3354,6 +3354,7 @@ extern void intel_i2c_reset(struct drm_device *dev); >> >> /* intel_bios.c */ >> int intel_bios_init(struct drm_device *dev); >> +bool intel_bios_is_valid_vbt(const void *buf, size_t size); >> >> /* intel_opregion.c */ >> #ifdef CONFIG_ACPI >> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c >> index f2b79b7d12b8..007b2b759600 100644 >> --- a/drivers/gpu/drm/i915/intel_bios.c >> +++ b/drivers/gpu/drm/i915/intel_bios.c >> @@ -1221,37 +1221,40 @@ static const struct bdb_header *get_bdb_header(const struct vbt_header *vbt) >> return _vbt + vbt->bdb_offset; >> } >> >> -static const struct vbt_header *validate_vbt(const void *base, >> - size_t size, >> - const void *_vbt) >> +/** >> + * intel_bios_is_valid_vbt - does the given buffer contain a valid VBT >> + * @buf: pointer to a buffer to validate >> + * @size: size of the buffer >> + * >> + * Returns true on valid VBT. >> + */ >> +bool intel_bios_is_valid_vbt(const void *buf, size_t size) >> { >> - size_t offset = _vbt - base; >> - const struct vbt_header *vbt = _vbt; >> + const struct vbt_header *vbt = buf; >> const struct bdb_header *bdb; >> >> if (!vbt) >> - return NULL; >> + return false; >> >> - if (offset + sizeof(struct vbt_header) > size) { >> + if (sizeof(struct vbt_header) > size) { >> DRM_DEBUG_DRIVER("VBT header incomplete\n"); >> - return NULL; >> + return false; >> } >> >> if (memcmp(vbt->signature, "$VBT", 4)) { >> DRM_DEBUG_DRIVER("VBT invalid signature\n"); >> - return NULL; >> + return false; >> } >> >> - offset += vbt->bdb_offset; >> - if (offset + sizeof(struct bdb_header) > size) { >> + if (vbt->bdb_offset + sizeof(struct bdb_header) > size) { >> DRM_DEBUG_DRIVER("BDB header incomplete\n"); >> - return NULL; >> + return false; >> } >> >> bdb = get_bdb_header(vbt); >> - if (offset + bdb->bdb_size > size) { >> + if (vbt->bdb_offset + bdb->bdb_size > size) { >> DRM_DEBUG_DRIVER("BDB incomplete\n"); >> - return NULL; >> + return false; >> } >> >> return vbt; >> @@ -1259,26 +1262,27 @@ static const struct vbt_header *validate_vbt(const void *base, >> >> static const struct vbt_header *find_vbt(void __iomem *bios, size_t size) >> { >> - const struct vbt_header *vbt = NULL; >> size_t i; >> >> /* Scour memory looking for the VBT signature. */ >> for (i = 0; i + 4 < size; i++) { >> - if (ioread32(bios + i) == *((const u32 *) "$VBT")) { >> - /* >> - * This is the one place where we explicitly discard the >> - * address space (__iomem) of the BIOS/VBT. From now on >> - * everything is based on 'base', and treated as regular >> - * memory. >> - */ >> - void *_bios = (void __force *) bios; >> + void *vbt; >> >> - vbt = validate_vbt(_bios, size, _bios + i); >> - break; >> - } >> + if (ioread32(bios + i) != *((const u32 *) "$VBT")) >> + continue; >> + >> + /* >> + * This is the one place where we explicitly discard the address >> + * space (__iomem) of the BIOS/VBT. >> + */ >> + vbt = (void __force *) bios + i; >> + if (intel_bios_is_valid_vbt(vbt, size - i)) >> + return vbt; >> + >> + break; >> } >> >> - return vbt; >> + return NULL; >> } >> >> /** >> @@ -1295,7 +1299,7 @@ intel_bios_init(struct drm_device *dev) >> { >> struct drm_i915_private *dev_priv = dev->dev_private; >> struct pci_dev *pdev = dev->pdev; >> - const struct vbt_header *vbt; >> + const struct vbt_header *vbt = dev_priv->opregion.vbt; >> const struct bdb_header *bdb; >> u8 __iomem *bios = NULL; >> >> @@ -1304,9 +1308,6 @@ intel_bios_init(struct drm_device *dev) >> >> init_vbt_defaults(dev_priv); >> >> - /* XXX Should this validation be moved to intel_opregion.c? */ >> - vbt = validate_vbt(dev_priv->opregion.header, OPREGION_SIZE, >> - dev_priv->opregion.vbt); >> if (vbt) { >> DRM_DEBUG_KMS("Using VBT from ACPI OpRegion: %20s\n", >> vbt->signature); > > Maybe the debug message should be moved too? It looks a bit out of place > after the validation gets moved. This is intentional (see patch 4). In a way this is where the decision is made to use the opregion VBT. The opregion code just validates it, but doesn't actually *use* it. BR, Jani. > >> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c >> index 5b9fc790d300..9f992e7c6185 100644 >> --- a/drivers/gpu/drm/i915/intel_opregion.c >> +++ b/drivers/gpu/drm/i915/intel_opregion.c >> @@ -986,8 +986,13 @@ int intel_opregion_setup(struct drm_device *dev) >> if (mboxes & MBOX_ASLE_EXT) >> DRM_DEBUG_DRIVER("ASLE extension supported\n"); >> >> - if (!dmi_check_system(intel_no_opregion_vbt)) >> - opregion->vbt = base + OPREGION_VBT_OFFSET; >> + if (!dmi_check_system(intel_no_opregion_vbt)) { >> + void *vbt = base + OPREGION_VBT_OFFSET; >> + u32 vbt_size = OPREGION_SIZE - OPREGION_VBT_OFFSET; >> + >> + if (intel_bios_is_valid_vbt(vbt, vbt_size)) >> + opregion->vbt = vbt; >> + } >> >> return 0; >> >> -- >> 2.1.4 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Dec 14, 2015 at 04:34:50PM +0200, Jani Nikula wrote: > On Mon, 14 Dec 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Mon, Dec 14, 2015 at 12:50:50PM +0200, Jani Nikula wrote: > >> Make the validation function a boolean operating on a buffer of given > >> size, removing the extra pointer dances. > >> > >> Move the OpRegion based VBT validation to intel_opregion_setup(), only > >> initializing opregion->vbt if it's valid. > >> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_drv.h | 1 + > >> drivers/gpu/drm/i915/intel_bios.c | 63 ++++++++++++++++++----------------- > >> drivers/gpu/drm/i915/intel_opregion.c | 9 +++-- > >> 3 files changed, 40 insertions(+), 33 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > >> index a2bd4c6a86a1..ef15f1710b50 100644 > >> --- a/drivers/gpu/drm/i915/i915_drv.h > >> +++ b/drivers/gpu/drm/i915/i915_drv.h > >> @@ -3354,6 +3354,7 @@ extern void intel_i2c_reset(struct drm_device *dev); > >> > >> /* intel_bios.c */ > >> int intel_bios_init(struct drm_device *dev); > >> +bool intel_bios_is_valid_vbt(const void *buf, size_t size); > >> > >> /* intel_opregion.c */ > >> #ifdef CONFIG_ACPI > >> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > >> index f2b79b7d12b8..007b2b759600 100644 > >> --- a/drivers/gpu/drm/i915/intel_bios.c > >> +++ b/drivers/gpu/drm/i915/intel_bios.c > >> @@ -1221,37 +1221,40 @@ static const struct bdb_header *get_bdb_header(const struct vbt_header *vbt) > >> return _vbt + vbt->bdb_offset; > >> } > >> > >> -static const struct vbt_header *validate_vbt(const void *base, > >> - size_t size, > >> - const void *_vbt) > >> +/** > >> + * intel_bios_is_valid_vbt - does the given buffer contain a valid VBT > >> + * @buf: pointer to a buffer to validate > >> + * @size: size of the buffer > >> + * > >> + * Returns true on valid VBT. > >> + */ > >> +bool intel_bios_is_valid_vbt(const void *buf, size_t size) > >> { > >> - size_t offset = _vbt - base; > >> - const struct vbt_header *vbt = _vbt; > >> + const struct vbt_header *vbt = buf; > >> const struct bdb_header *bdb; > >> > >> if (!vbt) > >> - return NULL; > >> + return false; > >> > >> - if (offset + sizeof(struct vbt_header) > size) { > >> + if (sizeof(struct vbt_header) > size) { > >> DRM_DEBUG_DRIVER("VBT header incomplete\n"); > >> - return NULL; > >> + return false; > >> } > >> > >> if (memcmp(vbt->signature, "$VBT", 4)) { > >> DRM_DEBUG_DRIVER("VBT invalid signature\n"); > >> - return NULL; > >> + return false; > >> } > >> > >> - offset += vbt->bdb_offset; > >> - if (offset + sizeof(struct bdb_header) > size) { > >> + if (vbt->bdb_offset + sizeof(struct bdb_header) > size) { > >> DRM_DEBUG_DRIVER("BDB header incomplete\n"); > >> - return NULL; > >> + return false; > >> } > >> > >> bdb = get_bdb_header(vbt); > >> - if (offset + bdb->bdb_size > size) { > >> + if (vbt->bdb_offset + bdb->bdb_size > size) { > >> DRM_DEBUG_DRIVER("BDB incomplete\n"); > >> - return NULL; > >> + return false; > >> } > >> > >> return vbt; > >> @@ -1259,26 +1262,27 @@ static const struct vbt_header *validate_vbt(const void *base, > >> > >> static const struct vbt_header *find_vbt(void __iomem *bios, size_t size) > >> { > >> - const struct vbt_header *vbt = NULL; > >> size_t i; > >> > >> /* Scour memory looking for the VBT signature. */ > >> for (i = 0; i + 4 < size; i++) { > >> - if (ioread32(bios + i) == *((const u32 *) "$VBT")) { > >> - /* > >> - * This is the one place where we explicitly discard the > >> - * address space (__iomem) of the BIOS/VBT. From now on > >> - * everything is based on 'base', and treated as regular > >> - * memory. > >> - */ > >> - void *_bios = (void __force *) bios; > >> + void *vbt; > >> > >> - vbt = validate_vbt(_bios, size, _bios + i); > >> - break; > >> - } > >> + if (ioread32(bios + i) != *((const u32 *) "$VBT")) > >> + continue; > >> + > >> + /* > >> + * This is the one place where we explicitly discard the address > >> + * space (__iomem) of the BIOS/VBT. > >> + */ > >> + vbt = (void __force *) bios + i; > >> + if (intel_bios_is_valid_vbt(vbt, size - i)) > >> + return vbt; > >> + > >> + break; > >> } > >> > >> - return vbt; > >> + return NULL; > >> } > >> > >> /** > >> @@ -1295,7 +1299,7 @@ intel_bios_init(struct drm_device *dev) > >> { > >> struct drm_i915_private *dev_priv = dev->dev_private; > >> struct pci_dev *pdev = dev->pdev; > >> - const struct vbt_header *vbt; > >> + const struct vbt_header *vbt = dev_priv->opregion.vbt; > >> const struct bdb_header *bdb; > >> u8 __iomem *bios = NULL; > >> > >> @@ -1304,9 +1308,6 @@ intel_bios_init(struct drm_device *dev) > >> > >> init_vbt_defaults(dev_priv); > >> > >> - /* XXX Should this validation be moved to intel_opregion.c? */ > >> - vbt = validate_vbt(dev_priv->opregion.header, OPREGION_SIZE, > >> - dev_priv->opregion.vbt); > >> if (vbt) { > >> DRM_DEBUG_KMS("Using VBT from ACPI OpRegion: %20s\n", > >> vbt->signature); > > > > Maybe the debug message should be moved too? It looks a bit out of place > > after the validation gets moved. > > This is intentional (see patch 4). In a way this is where the decision > is made to use the opregion VBT. The opregion code just validates it, > but doesn't actually *use* it. But when you add the RVDA thing, the debug print for that is in the opregion code, so with that we get two debug prints? > > BR, > Jani. > > > > > > >> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c > >> index 5b9fc790d300..9f992e7c6185 100644 > >> --- a/drivers/gpu/drm/i915/intel_opregion.c > >> +++ b/drivers/gpu/drm/i915/intel_opregion.c > >> @@ -986,8 +986,13 @@ int intel_opregion_setup(struct drm_device *dev) > >> if (mboxes & MBOX_ASLE_EXT) > >> DRM_DEBUG_DRIVER("ASLE extension supported\n"); > >> > >> - if (!dmi_check_system(intel_no_opregion_vbt)) > >> - opregion->vbt = base + OPREGION_VBT_OFFSET; > >> + if (!dmi_check_system(intel_no_opregion_vbt)) { > >> + void *vbt = base + OPREGION_VBT_OFFSET; > >> + u32 vbt_size = OPREGION_SIZE - OPREGION_VBT_OFFSET; > >> + > >> + if (intel_bios_is_valid_vbt(vbt, vbt_size)) > >> + opregion->vbt = vbt; > >> + } > >> > >> return 0; > >> > >> -- > >> 2.1.4 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Jani Nikula, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a2bd4c6a86a1..ef15f1710b50 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3354,6 +3354,7 @@ extern void intel_i2c_reset(struct drm_device *dev); /* intel_bios.c */ int intel_bios_init(struct drm_device *dev); +bool intel_bios_is_valid_vbt(const void *buf, size_t size); /* intel_opregion.c */ #ifdef CONFIG_ACPI diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index f2b79b7d12b8..007b2b759600 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -1221,37 +1221,40 @@ static const struct bdb_header *get_bdb_header(const struct vbt_header *vbt) return _vbt + vbt->bdb_offset; } -static const struct vbt_header *validate_vbt(const void *base, - size_t size, - const void *_vbt) +/** + * intel_bios_is_valid_vbt - does the given buffer contain a valid VBT + * @buf: pointer to a buffer to validate + * @size: size of the buffer + * + * Returns true on valid VBT. + */ +bool intel_bios_is_valid_vbt(const void *buf, size_t size) { - size_t offset = _vbt - base; - const struct vbt_header *vbt = _vbt; + const struct vbt_header *vbt = buf; const struct bdb_header *bdb; if (!vbt) - return NULL; + return false; - if (offset + sizeof(struct vbt_header) > size) { + if (sizeof(struct vbt_header) > size) { DRM_DEBUG_DRIVER("VBT header incomplete\n"); - return NULL; + return false; } if (memcmp(vbt->signature, "$VBT", 4)) { DRM_DEBUG_DRIVER("VBT invalid signature\n"); - return NULL; + return false; } - offset += vbt->bdb_offset; - if (offset + sizeof(struct bdb_header) > size) { + if (vbt->bdb_offset + sizeof(struct bdb_header) > size) { DRM_DEBUG_DRIVER("BDB header incomplete\n"); - return NULL; + return false; } bdb = get_bdb_header(vbt); - if (offset + bdb->bdb_size > size) { + if (vbt->bdb_offset + bdb->bdb_size > size) { DRM_DEBUG_DRIVER("BDB incomplete\n"); - return NULL; + return false; } return vbt; @@ -1259,26 +1262,27 @@ static const struct vbt_header *validate_vbt(const void *base, static const struct vbt_header *find_vbt(void __iomem *bios, size_t size) { - const struct vbt_header *vbt = NULL; size_t i; /* Scour memory looking for the VBT signature. */ for (i = 0; i + 4 < size; i++) { - if (ioread32(bios + i) == *((const u32 *) "$VBT")) { - /* - * This is the one place where we explicitly discard the - * address space (__iomem) of the BIOS/VBT. From now on - * everything is based on 'base', and treated as regular - * memory. - */ - void *_bios = (void __force *) bios; + void *vbt; - vbt = validate_vbt(_bios, size, _bios + i); - break; - } + if (ioread32(bios + i) != *((const u32 *) "$VBT")) + continue; + + /* + * This is the one place where we explicitly discard the address + * space (__iomem) of the BIOS/VBT. + */ + vbt = (void __force *) bios + i; + if (intel_bios_is_valid_vbt(vbt, size - i)) + return vbt; + + break; } - return vbt; + return NULL; } /** @@ -1295,7 +1299,7 @@ intel_bios_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; struct pci_dev *pdev = dev->pdev; - const struct vbt_header *vbt; + const struct vbt_header *vbt = dev_priv->opregion.vbt; const struct bdb_header *bdb; u8 __iomem *bios = NULL; @@ -1304,9 +1308,6 @@ intel_bios_init(struct drm_device *dev) init_vbt_defaults(dev_priv); - /* XXX Should this validation be moved to intel_opregion.c? */ - vbt = validate_vbt(dev_priv->opregion.header, OPREGION_SIZE, - dev_priv->opregion.vbt); if (vbt) { DRM_DEBUG_KMS("Using VBT from ACPI OpRegion: %20s\n", vbt->signature); diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c index 5b9fc790d300..9f992e7c6185 100644 --- a/drivers/gpu/drm/i915/intel_opregion.c +++ b/drivers/gpu/drm/i915/intel_opregion.c @@ -986,8 +986,13 @@ int intel_opregion_setup(struct drm_device *dev) if (mboxes & MBOX_ASLE_EXT) DRM_DEBUG_DRIVER("ASLE extension supported\n"); - if (!dmi_check_system(intel_no_opregion_vbt)) - opregion->vbt = base + OPREGION_VBT_OFFSET; + if (!dmi_check_system(intel_no_opregion_vbt)) { + void *vbt = base + OPREGION_VBT_OFFSET; + u32 vbt_size = OPREGION_SIZE - OPREGION_VBT_OFFSET; + + if (intel_bios_is_valid_vbt(vbt, vbt_size)) + opregion->vbt = vbt; + } return 0;
Make the validation function a boolean operating on a buffer of given size, removing the extra pointer dances. Move the OpRegion based VBT validation to intel_opregion_setup(), only initializing opregion->vbt if it's valid. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_bios.c | 63 ++++++++++++++++++----------------- drivers/gpu/drm/i915/intel_opregion.c | 9 +++-- 3 files changed, 40 insertions(+), 33 deletions(-)