Re: [PATCH v2 5/6] drm/i915/dgfx: OPROM OpRegion Setup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 15, 2022 at 07:07:26PM +0530, Anshuman Gupta wrote:
> On igfx cards ACPI OpRegion retrieve through ASLS.
> System BIOS writes ASLS address to pci config space(0xFC) but
> on discrete cards OpRegion is part of PCI Option ROM(OPROM) along
> with other firmware images, i915 is interested only in
> Code Signature System(CSS) and OpRegion + VBT image.
> 
> DGFX Cards has it dedicated flash, where OPROM reside.
> DGFX card provides SPI controller interface to read the OPROM.
> Read OPROM through SPI MMIO because PCI ROM mapping may does not
> work on some platforms due to the BIOS not leaving the OPROM mapped.
> 
> In order to setup OpRegion and retrieve VBT from OpRegion,
> it is required to do OPROM sanity check.
> 
> OPROM Sanity checks involves below steps.
> 
> Verify OPROM images Signature as Documented in PCI firmware Specs 3.2.
> Verify Intel CSS image signature.
> Verify the Intel CSS image code type.
> Authenticate OPROM RSA Signature. (TODO)
> Verify OpRegion image Signature.
> 
> After successful sanity check, driver will consume the OPROM
> config data to get opreg and vbt accordingly.
> 
> v2:
> - Add kzalloc NULL check for oprom_opreg pointer.
> - Fixed memory leak in intel_spi_get_oprom_opreg().
> 
> PCI Firmware Spec: ID:12886
> https://pcisig.com/specifications
> 
> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Cc: Uma Shankar <uma.shankar@xxxxxxxxx>
> Cc: Badal Nilawar <badal.nilawar@xxxxxxxxx>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/display/intel_opregion.c | 343 +++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_opregion.h |   1 +
>  2 files changed, 327 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c b/drivers/gpu/drm/i915/display/intel_opregion.c
> index 562161a929d6..8af3a92582cb 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.c
> @@ -31,6 +31,7 @@
>  #include <acpi/video.h>
>  
>  #include "i915_drv.h"
> +#include "i915_reg.h"
>  #include "intel_acpi.h"
>  #include "intel_backlight.h"
>  #include "intel_display_types.h"
> @@ -145,6 +146,34 @@ struct i915_opregion_func {
>  	void (*free_opregion)(struct drm_i915_private *i915);
>  };
>  
> +/* Refer 8_PCI_Firmware_v3.2_01-26-2015_ts_clean_Firmware_Final Page 77 */
> +struct expansion_rom_header {
> +	u16 signature;		/* Offset[0x0]: Header 0x55 0xAA */
> +	u8 resvd[0x16];
> +	u16 pcistructoffset;	/* Offset[0x18]: Contains pointer PCI Data Structure */
> +	u16 img_base;		/* Offset[0x1A]: Offset to Oprom Image Base start */
> +} __packed;
> +
> +struct pci_data_structure {
> +	u32 signature;
> +	u8 resvd[12];
> +	u16 img_len;
> +	u8 resvd1[2];
> +	u8 code_type;
> +	u8 last_img;
> +	u8 resvd2[6];
> +} __packed;
> +
> +/* PCI Firmware Spec specific Macro */
> +#define LAST_IMG_INDICATOR		0x80
> +#define OPROM_IMAGE_MAGIC		0xAA55       /* Little Endian */
> +#define OPROM_IMAGE_PCIR_MAGIC		0x52494350   /* "PCIR" */
> +#define OPROM_BYTE_BOUNDARY		512          /* OPROM image sizes are in 512 byte */
> +
> +#define INTEL_CSS_SIGNATURE		"$CPD"	/* Code Signature System Signature */
> +#define NUM_CSS_BYTES			4
> +#define INTEL_OPROM_CSS_CODE_TYPE	0xF0
> +
>  /* Driver readiness indicator */
>  #define ASLE_ARDY_READY		(1 << 0)
>  #define ASLE_ARDY_NOT_READY	(0 << 0)
> @@ -880,6 +909,196 @@ static int intel_load_vbt_firmware(struct drm_i915_private *dev_priv)
>  	return ret;
>  }
>  
> +/* Refer PCI Firmware Spec Chapter 5 */
> +static int
> +pci_exp_rom_check_signature(struct drm_i915_private *i915,
> +			    struct expansion_rom_header *exprom_hdr,
> +			    struct pci_data_structure *exprom_pci_data)
> +{
> +	if (exprom_hdr->signature != OPROM_IMAGE_MAGIC) {
> +		drm_err(&i915->drm, "Invalid PCI ROM header signature.\n");
> +		return -EINVAL;
> +	}
> +
> +	if (exprom_pci_data->signature != OPROM_IMAGE_PCIR_MAGIC) {
> +		drm_err(&i915->drm, "Invalid PCI ROM data signature.\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static u32 intel_spi_oprom_offset(struct drm_i915_private *i915)
> +{
> +	u32 static_region, offset;
> +
> +	/* initialize SPI to read the OPROM */
> +	static_region = intel_uncore_read(&i915->uncore, SPI_STATIC_REGIONS);
> +	static_region &= OPTIONROM_SPI_REGIONID_MASK;
> +	intel_uncore_write(&i915->uncore, PRIMARY_SPI_REGIONID, static_region);
> +
> +	/* read OPROM offset in SPI flash */
> +	offset = intel_uncore_read(&i915->uncore, OROM_OFFSET);
> +
> +	return offset;
> +}
> +
> +static void intel_spi_read_oprom(struct drm_i915_private *i915,
> +				 u32 offset, size_t len, void *buf)
> +{
> +	u32 count, data;
> +	u32 *word = buf;
> +
> +	drm_WARN_ON(&i915->drm, !IS_ALIGNED(len, 4));
> +
> +	for (count = 0; count < len; count += 4) {
> +		intel_uncore_write(&i915->uncore, PRIMARY_SPI_ADDRESS, offset + count);
> +		data = intel_uncore_read(&i915->uncore, PRIMARY_SPI_TRIGGER);
> +		word[count >> 2] = data;
> +	}
> +}
> +
> +static int intel_verify_css(struct drm_i915_private *i915,
> +			    struct expansion_rom_header *exprom_hdr,
> +			    struct pci_data_structure *exprom_pci_data)
> +{
> +	if (exprom_pci_data->code_type != INTEL_OPROM_CSS_CODE_TYPE) {
> +		drm_dbg_kms(&i915->drm, "Invalid OPROM CSS Code\n");
> +		return -EINVAL;
> +	}
> +	drm_dbg_kms(&i915->drm, "Found CSS image\n");
> +	/*
> +	 * TODO: Authticate OPROM RSA Signature if required in future
> +	 * pubic key and signature are present in CSS image.
> +	 */
> +
> +	return 0;
> +}
> +
> +/**
> + * intel_spi_get_oprom_opreg() get OPROM OpRegion image.
> + * @i915: pointer to i915 device.
> + *
> + * This function parses the DGFX OPROM to retieve the opregion.
> + * OPROM has bundled multiple images but i915 only interested
> + * in CSS and opregion image.
> + *
> + *	+        DGFX OPROM IMAGE LAYOUT             +
> + *	+--------+-------+---------------------------+
> + *	| Offset | Value |   ROM Header Fields       +-----> Image1 (CSS)
> + *	+--------------------------------------------+
> + *	|    0h  |  55h  |   ROM Signature Byte1     |
> + *	|    1h  |  AAh  |   ROM Signature Byte2     |
> + *	|    2h  |  xx   |        Reserved           |
> + *	|  18+19h|  xx   |  Ptr to PCI DataStructure |
> + *	+----------------+---------------------------+
> + *	|           PCI Data Structure               |
> + *	+--------------------------------------------+
> + *	|    .       .             .                 |
> + *	|    .       .             .                 |
> + *	|    10  +  xx   +     Image Length          |
> + *	|    14  +  xx   +     Code Type             |
> + *	|    15  +  xx   +  Last Image Indicator     |
> + *	|    .       .             .                 |
> + *	+--------------------------------------------+
> + *	|         Signature and Public Key           |
> + *	+--------+-------+---------------------------+
> + *	|    .   |   .   |         .                 |
> + *	|    .   |   .   |         .                 |
> + *	+--------------------------------------------+
> + *	| Offset | Value |   ROM Header Fields       +-----> Image2 (opregion, vbt) (Offset: 0x800)
> + *	+--------------------------------------------+
> + *	|    0h  |  55h  |   ROM Signature Byte1     |
> + *	|    1h  |  AAh  |   ROM Signature Byte2     |
> + *	|    2h  |  xx   |        Reserved           |
> + *	|  18+19h|  xx   |  Ptr to PCI DataStructure |
> + *	+----------------+---------------------------+
> + *	|           PCI Data Structure               |
> + *	+--------------------------------------------+
> + *	|    .       .             .                 |
> + *	|    .       .             .                 |
> + *	|    10  +  xx   +     Image Length          |
> + *	|    14  +  xx   +      Code Type            |
> + *	|    15  +  xx   +   Last Image Indicator    |
> + *	|    .       .             .                 |
> + *	|    1A  +  3C   + Ptr to Opregion Signature |
> + *	|    .       .             .                 |
> + *	|    .       .             .                 |
> + *	|   83Ch + IntelGraphicsMem                  | <---+ Opregion Signature
> + *	+--------+-----------------------------------+
> + *
> + * Return : Returns the opregion image blob which starts from opregion
> + * signature "IntelGraphicsMem". Error value in case of error
> + */
> +static void *
> +intel_spi_get_oprom_opreg(struct drm_i915_private *i915)
> +{
> +	struct expansion_rom_header *exprom_hdr;
> +	struct pci_data_structure *exprom_pci_data;
> +	u8 img_sig[sizeof(OPREGION_SIGNATURE)];
> +	u32 oprom_offset, offset;
> +	size_t img_len, opreg_len;
> +	void *opreg = ERR_PTR(-ENXIO);
> +	int ret;
> +
> +	oprom_offset = intel_spi_oprom_offset(i915);
> +
> +	exprom_hdr = kzalloc(sizeof(struct expansion_rom_header), GFP_KERNEL);
> +	exprom_pci_data = kzalloc(sizeof(struct pci_data_structure), GFP_KERNEL);
> +	if (!exprom_hdr || !exprom_pci_data) {
> +		opreg = ERR_PTR(-ENOMEM);
> +		goto err_free_hdr;
> +	}
> +
> +	for (offset = oprom_offset; exprom_pci_data->last_img != LAST_IMG_INDICATOR;
> +	     offset = offset + img_len) {
> +		intel_spi_read_oprom(i915, offset, sizeof(struct expansion_rom_header),
> +				     exprom_hdr);
> +		intel_spi_read_oprom(i915, offset + exprom_hdr->pcistructoffset,
> +				     sizeof(struct pci_data_structure), exprom_pci_data);
> +		ret = pci_exp_rom_check_signature(i915, exprom_hdr, exprom_pci_data);
> +		if (ret) {
> +			opreg = ERR_PTR(ret);
> +			goto err_free_hdr;
> +		}
> +
> +		img_len = exprom_pci_data->img_len * OPROM_BYTE_BOUNDARY;
> +
> +		/* CSS or OpReg signature is present at exprom_hdr->img_base offset */
> +		intel_spi_read_oprom(i915, offset + exprom_hdr->img_base,
> +				     sizeof(OPREGION_SIGNATURE) - 1, img_sig);
> +
> +		if (!memcmp(img_sig, INTEL_CSS_SIGNATURE, NUM_CSS_BYTES)) {
> +			ret = intel_verify_css(i915, exprom_hdr, exprom_pci_data);
> +			if (ret) {
> +				opreg = ERR_PTR(ret);
> +				goto err_free_hdr;
> +			}
> +		} else if (!memcmp(img_sig, OPREGION_SIGNATURE, sizeof(OPREGION_SIGNATURE) - 1)) {
> +			opreg_len = img_len - exprom_hdr->img_base;
> +			opreg_len = ALIGN(opreg_len, 4);
> +			opreg = kzalloc(opreg_len, GFP_KERNEL);
> +
> +			if (!opreg) {
> +				opreg = ERR_PTR(-ENOMEM);
> +				goto err_free_hdr;
> +			}
> +
> +			intel_spi_read_oprom(i915, offset + exprom_hdr->img_base,
> +					     opreg_len, opreg);
> +			drm_dbg_kms(&i915->drm, "Found opregion image of size %zu\n", opreg_len);
> +			break;
> +		}
> +	}
> +
> +err_free_hdr:
> +
> +	kfree(exprom_pci_data);
> +	kfree(exprom_hdr);
> +
> +	return opreg;
> +}
> +
>  static int intel_opregion_setup(struct drm_i915_private *dev_priv)
>  {
>  	struct intel_opregion *opregion = &dev_priv->opregion;
> @@ -1006,6 +1225,17 @@ static int intel_opregion_setup(struct drm_i915_private *dev_priv)
>  	}
>  
>  out:
> +	/*
> +	 * We might got VBT from OPROM OpRegion but we can't use OPROM OpRegion
> +	 * to write ACPI OpRegion MBOX.
> +	 */
> +	if (!opregion->asls) {
> +		drm_dbg(&dev_priv->drm, "ACPI OpRegion MBOX is not supported!\n");
> +		opregion->acpi = NULL;
> +		opregion->swsci = NULL;
> +		opregion->asle = NULL;
> +	}
> +
>  	return 0;
>  
>  }
> @@ -1218,45 +1448,54 @@ intel_opregion_get_asls(struct drm_i915_private *i915)
>  	pci_read_config_dword(pdev, ASLS, &asls);
>  	drm_dbg(&i915->drm, "graphic opregion physical addr: 0x%x\n",
>  		asls);
> -	if (asls == 0) {
> -		drm_dbg(&i915->drm, "ACPI OpRegion not supported!\n");
> +	if (asls == 0)
>  		return -EINVAL;
> -	}
>  
>  	opregion->asls = asls;
>  
>  	return 0;
>  }
>  
> +static int
> +intel_opregion_verify_signature(struct drm_i915_private *i915, const void *base)
> +{
> +	char buf[sizeof(OPREGION_SIGNATURE)];
> +
> +	memcpy(buf, base, sizeof(buf));
> +
> +	if (memcmp(buf, OPREGION_SIGNATURE, 16)) {
> +		drm_dbg(&i915->drm, "opregion signature mismatch\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static void *intel_igfx_alloc_opregion(struct drm_i915_private *i915)
>  {
>  	struct intel_opregion *opregion = &i915->opregion;
> -	char buf[sizeof(OPREGION_SIGNATURE)];
>  	int err = 0;
>  	void *base;
>  
>  	err = intel_opregion_get_asls(i915);
> -	if (err)
> +	if (err) {
> +		if (!opregion->asls)
> +			drm_dbg(&i915->drm, "ACPI OpRegion not supported!\n");
> +
>  		return ERR_PTR(err);
> +	}
>  
>  	base = memremap(opregion->asls, OPREGION_SIZE, MEMREMAP_WB);
>  	if (!base)
>  		return ERR_PTR(-ENOMEM);
>  
> -	memcpy(buf, base, sizeof(buf));
> -
> -	if (memcmp(buf, OPREGION_SIGNATURE, 16)) {
> -		drm_dbg(&i915->drm, "opregion signature mismatch\n");
> -		err = -EINVAL;
> -		goto err_out;
> +	err = intel_opregion_verify_signature(i915, base);
> +	if (err) {
> +		memunmap(base);
> +		return ERR_PTR(err);
>  	}
>  
>  	return base;
> -
> -err_out:
> -	memunmap(base);
> -
> -	return ERR_PTR(err);
>  }
>  
>  static void *intel_igfx_alloc_rvda(struct drm_i915_private *i915)
> @@ -1315,9 +1554,73 @@ static const struct i915_opregion_func igfx_opregion_func = {
>  	.free_opregion = intel_igfx_free_opregion,
>  };
>  
> +static void *intel_dgfx_setup_asls(struct drm_i915_private *i915)
> +{
> +	struct intel_opregion *opregion = &i915->opregion;
> +	struct opregion_asle *asls_asle;
> +	const struct opregion_asle *spi_asle;
> +	void *base;
> +	int ret;
> +
> +	if (!opregion->dgfx_oprom_opreg)
> +		return ERR_PTR(-EINVAL);
> +
> +	spi_asle = opregion->dgfx_oprom_opreg + OPREGION_ASLE_OFFSET;
> +
> +	/*
> +	 * DGFX MBD configs supports ASL storage.
> +	 * Populate the RVDA and RVDS field from OPROM opregion.
> +	 */
> +	base = memremap(opregion->asls, OPREGION_SIZE, MEMREMAP_WB);
> +	if (!base)
> +		return ERR_PTR(-ENOMEM);
> +
> +	ret = intel_opregion_verify_signature(i915, base);
> +	if (ret) {
> +		memunmap(base);
> +		return ERR_PTR(ret);
> +	}
> +
> +	asls_asle = base + OPREGION_ASLE_OFFSET;
> +	asls_asle->rvda = spi_asle->rvda;
> +	asls_asle->rvds = spi_asle->rvds;
> +
> +	return base;
> +}
> +
>  static void *intel_dgfx_alloc_opregion(struct drm_i915_private *i915)
>  {
> -	return ERR_PTR(-EOPNOTSUPP);
> +	struct intel_opregion *opregion = &i915->opregion;
> +	void *oprom_opreg;
> +	void *asls_opreg;
> +
> +	BUILD_BUG_ON(sizeof(struct expansion_rom_header) != 28);
> +	BUILD_BUG_ON(sizeof(struct pci_data_structure) != 28);
> +
> +	oprom_opreg = intel_spi_get_oprom_opreg(i915);
> +
> +	if (IS_ERR(oprom_opreg)) {
> +		drm_err(&i915->drm, "Unable to get opregion image from dgfx oprom Err: %ld\n",
> +			PTR_ERR(oprom_opreg));
> +		return oprom_opreg;
> +	}
> +
> +	/* Cache the OPROM opregion + vbt image to retrieve vbt later */
> +	opregion->dgfx_oprom_opreg = oprom_opreg;
> +
> +	if (!intel_opregion_get_asls(i915)) {
> +		asls_opreg = intel_dgfx_setup_asls(i915);
> +		if (!IS_ERR(asls_opreg))
> +			return asls_opreg;
> +	}
> +
> +	oprom_opreg = kzalloc(OPREGION_SIZE, GFP_KERNEL);
> +	if (!oprom_opreg)
> +		return ERR_PTR(-ENOMEM);
> +
> +	memcpy(oprom_opreg, opregion->dgfx_oprom_opreg, OPREGION_SIZE);

Here the suggestion was to use the kmemdup instead of kzalloc and memcpy, I have sent patch for this in internal could you make that here as well?

oprom_opreg = kmemdup(opregion->dgfx_oprom_opreg, OPREGION_SIZE, GFP_KERNEL);

return oprom_opreg ?: ERR_PTR(-ENOMEM);

This was suggestion made by Chris Wilson.

Also please add the version change log for fixing the NULL pointer and kmemdup changes as made by me on DII patchset

Manasi

> +
> +	return oprom_opreg;
>  }
>  
>  static void *intel_dgfx_alloc_rvda(struct drm_i915_private *i915)
> @@ -1331,6 +1634,12 @@ static void intel_dgfx_free_rvda(struct drm_i915_private *i915)
>  
>  static void intel_dgfx_free_opregion(struct drm_i915_private *i915)
>  {
> +	struct intel_opregion *opregion = &i915->opregion;
> +
> +	if (opregion->asls)
> +		memunmap(opregion->header);
> +	else
> +		kfree(opregion->header);
>  }
>  
>  static const struct i915_opregion_func dgfx_opregion_func = {
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h b/drivers/gpu/drm/i915/display/intel_opregion.h
> index 7500c396b74d..65a9aa4fdb59 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.h
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.h
> @@ -52,6 +52,7 @@ struct intel_opregion {
>  	void *rvda;
>  	void *vbt_firmware;
>  	const void *vbt;
> +	const void *dgfx_oprom_opreg;
>  	u32 vbt_size;
>  	u32 *lid_state;
>  	struct work_struct asle_work;
> -- 
> 2.26.2
> 



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux