[Intel-gfx] [PATCH v2 06/11] drm/i915: refactor VBT validation

[Jani Nikula]

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.

v2: move logging about valid VBT to opregion setup too (Ville)

Signed-off-by: Jani Nikula <jani.nikula at intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  1 +
 drivers/gpu/drm/i915/intel_bios.c     | 67 +++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_opregion.c | 11 ++++--
 3 files changed, 43 insertions(+), 36 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 04fb9ded1805..dc3a0fb1946c 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,12 +1308,7 @@ 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("Found valid VBT in ACPI OpRegion\n");
-	} else {
+	if (!vbt) {
 		size_t size;
 
 		bios = pci_map_rom(pdev, &size);
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 5b9fc790d300..859c9acbee73 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -986,8 +986,15 @@ 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)) {
+			DRM_DEBUG_KMS("Found valid VBT in ACPI OpRegion\n");
+			opregion->vbt = vbt;
+		}
+	}
 
 	return 0;
 
-- 
2.1.4