On Mon, 12 Apr 2021, Matthew Auld <[email protected]> wrote:
> From: Anshuman Gupta <[email protected]>
>
> Sanitize OPROM header, CPD signature and OPROM PCI version.
> OPROM_HEADER, EXPANSION_ROM_HEADER and OPROM_MEU_BLOB structures
> and PCI struct offsets are provided by GSC counterparts.
> These are yet to be Documented in B.Spec.
> After successful sanitization, extract VBT from opregion
> image.

So I don't understand what the point is with two consecutive patches
where the latter rewrites a lot of the former. 

BR,
Jani.


>
> v2:
> - Used macro for OPROM header magic 0xaa55 [Rodrigo]
> - Added a OPROM layout. [Uma]
> - Extract opregion from OPROM package and then extract
>   VBT from opregion to have backward compatibility with
>   older IFWI.
>
> v3:
> - Moved opreg stuff to intel_opregion.{c,h}. [Uma]
> - Memory leak and intel_oprom_verify_signature return
>   value fixes. [Uma]
>
> v4:
>  - Fix return code storage for oprom_image_parse_helper (Matt)
>
> v5 by Jani:
> - switch to intel_uncore_read/intel_uncore_write
>
> v6 by Khajapasha:
> - Rename intel_oprom_verify_signature() to
>   intel_spi_get_oprom_opreg() [Jani, Nikula]
> - Use u32 data type for opregion size [Jani, Nikula]
>
> Cc: Jani Nikula <[email protected]>
> Cc: Uma Shankar <[email protected]>
> Signed-off-by: Anshuman Gupta <[email protected]>
> Signed-off-by: Jani Nikula <[email protected]>
> Signed-off-by: Mohammed Khajapasha <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c     |  47 +++--
>  drivers/gpu/drm/i915/display/intel_opregion.c | 169 ++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_opregion.h |  38 +++-
>  3 files changed, 227 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c 
> b/drivers/gpu/drm/i915/display/intel_bios.c
> index f9dc651f1652..59eec8333723 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -2240,37 +2240,36 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t 
> size)
>  
>  static struct vbt_header *spi_oprom_get_vbt(struct drm_i915_private *i915)
>  {
> -     u32 count, data, found, store = 0;
> -     u32 static_region, oprom_offset;
> -     u32 oprom_size = 0x200000;
> +     u32 count, found, opreg_size;
> +     u32 *vbt, *oprom_opreg = NULL;
>       u16 vbt_size;
> -     u32 *vbt;
> +     u8 *parse_ptr;
>  
> -     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);
> -
> -     oprom_offset = intel_uncore_read(&i915->uncore, OROM_OFFSET);
> -     oprom_offset &= OROM_OFFSET_MASK;
> +     if (intel_spi_get_oprom_opreg(i915, &oprom_opreg, &opreg_size)) {
> +             drm_err(&i915->drm, "oprom signature verification failed\n");
> +             goto err_not_found;
> +     }
>  
> -     for (count = 0; count < oprom_size; count += 4) {
> -             intel_uncore_write(&i915->uncore, PRIMARY_SPI_ADDRESS, 
> oprom_offset + count);
> -             data = intel_uncore_read(&i915->uncore, PRIMARY_SPI_TRIGGER);
> +     if (!oprom_opreg) {
> +             drm_err(&i915->drm, "opregion not found\n");
> +             goto err_not_found;
> +     }
>  
> -             if (data == *((const u32 *)"$VBT")) {
> -                     found = oprom_offset + count;
> +     for (count = 0; count < opreg_size; count += 4) {
> +             if (oprom_opreg[count / 4] == *((const u32 *)"$VBT")) {
> +                     found = count;
>                       break;
>               }
>       }
>  
> -     if (count >= oprom_size)
> +     if (count >= opreg_size) {
> +             drm_err(&i915->drm, "VBT not found in opregion\n");
>               goto err_not_found;
> +     }
>  
>       /* Get VBT size and allocate space for the VBT */
> -     intel_uncore_write(&i915->uncore, PRIMARY_SPI_ADDRESS, found +
> -                offsetof(struct vbt_header, vbt_size));
> -     vbt_size = intel_uncore_read(&i915->uncore, PRIMARY_SPI_TRIGGER);
> -     vbt_size &= 0xffff;
> +     parse_ptr = (u8 *)oprom_opreg + found;
> +     vbt_size = ((struct vbt_header *)parse_ptr)->vbt_size;
>  
>       vbt = kzalloc(vbt_size, GFP_KERNEL);
>       if (!vbt) {
> @@ -2279,16 +2278,12 @@ static struct vbt_header *spi_oprom_get_vbt(struct 
> drm_i915_private *i915)
>               goto err_not_found;
>       }
>  
> -     for (count = 0; count < vbt_size; count += 4) {
> -             intel_uncore_write(&i915->uncore, PRIMARY_SPI_ADDRESS, found + 
> count);
> -             data = intel_uncore_read(&i915->uncore, PRIMARY_SPI_TRIGGER);
> -             *(vbt + store++) = data;
> -     }
> -
> +     memcpy(vbt, parse_ptr, vbt_size);
>       if (!intel_bios_is_valid_vbt(vbt, vbt_size))
>               goto err_free_vbt;
>  
>       DRM_DEBUG_KMS("Found valid VBT in SPI flash\n");
> +     kfree(oprom_opreg);
>  
>       return (struct vbt_header *)vbt;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.c 
> b/drivers/gpu/drm/i915/display/intel_opregion.c
> index dfd724e506b5..e9ccd8265a1f 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.c
> @@ -983,6 +983,175 @@ int intel_opregion_setup(struct drm_i915_private 
> *dev_priv)
>       return err;
>  }
>  
> +static int oprom_image_parse_helper(u8 *parse_ptr, u8 *last_img, u8 
> *code_type,
> +                                 struct drm_i915_private *i915)
> +{
> +     u8 size_512_bytes;
> +
> +     if (((union oprom_header *)parse_ptr)->signature != OPROM_IMAGE_MAGIC) {
> +             drm_err(&i915->drm, "Wrong OPROM header signature.\n");
> +             return -EINVAL;
> +     }
> +
> +     size_512_bytes = parse_ptr[((struct expansion_rom_header 
> *)parse_ptr)->pcistructoffset + PCI_IMAGE_LENGTH_OFFSET];
> +     *code_type = parse_ptr[((struct expansion_rom_header 
> *)parse_ptr)->pcistructoffset + PCI_CODE_TYPE_OFFSET];
> +     *last_img = parse_ptr[((struct expansion_rom_header 
> *)parse_ptr)->pcistructoffset + PCI_LAST_IMAGE_INDICATOR_OFFSET];
> +
> +     return size_512_bytes;
> +}
> +
> +static void spi_read_oprom_helper(size_t len, u32 offset, u32 *buf,
> +                               struct drm_i915_private *dev_priv)
> +{
> +     u32 count, data;
> +
> +     for (count = 0; count < len; count += 4) {
> +             intel_uncore_write(&dev_priv->uncore, PRIMARY_SPI_ADDRESS, 
> offset + count);
> +             data = intel_uncore_read(&dev_priv->uncore, 
> PRIMARY_SPI_TRIGGER);
> +             buf[count / 4] = data;
> +     }
> +}
> +
> +/**
> + *   +        DASH+G OPROM IMAGE LAYOUT           +
> + *   +--------+-------+---------------------------+
> + *   | Offset | Value |   ROM Header Fields       +-----> Image 1 (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     |
> + *   |    .       .             .                 |
> + *   +--------------------------------------------+
> + *   |               MEU BLOB                     |
> + *   +--------------------------------------------+
> + *   |              CPD Header                    |
> + *   |              CPD Entry                     |
> + *   |              Reserved                      |
> + *   |           SignedDataPart1                  |
> + *   |              PublicKey                     |
> + *   |            RSA Signature                   |
> + *   |           SignedDataPart2                  |
> + *   |            IFWI Metadata                   |
> + *   +--------+-------+---------------------------+
> + *   |    .   |   .   |         .                 |
> + *   |    .   |   .   |         .                 |
> + *   +--------------------------------------------+
> + *   | Offset | Value |   ROM Header Fields       +-----> Image 2 (Config 
> Data) (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
> + *   +--------+-----------------------------------+
> + *
> + * intel_spi_get_oprom_opreg() get OPROM image.
> + * @i915: pointer to i915 device.
> + * @opreg: pointer to opregion buffer output.
> + * @opreg_size: pointer to opregion size output.
> + */
> +int
> +intel_spi_get_oprom_opreg(struct drm_i915_private *i915, u32 **opreg,
> +                       u32 *opreg_size)
> +{
> +     u8 img_sig[sizeof(OPREGION_SIGNATURE)];
> +     u8 code_type, last_img;
> +     u32 static_region, offset, img_len;
> +     u32 *oprom_img, *oprom_img_hdr;
> +     u16 opreg_base;
> +     u8 *parse_ptr;
> +     int img_size;
> +     int ret = -EINVAL;
> +
> +     /* 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);
> +     offset &= OROM_OFFSET_MASK;
> +
> +     oprom_img_hdr = kzalloc(OPROM_INITIAL_READ_SIZE, GFP_KERNEL);
> +     if (!oprom_img_hdr)
> +             return -ENOMEM;
> +
> +     do {
> +             spi_read_oprom_helper(OPROM_INITIAL_READ_SIZE, offset,
> +                                   oprom_img_hdr, i915);
> +             img_size = oprom_image_parse_helper((u8 *)oprom_img_hdr, 
> &last_img,
> +                                                 &code_type, i915);
> +             if (img_size <= 0) {
> +                     ret = -EINVAL;
> +                     goto err_free_hdr;
> +             }
> +
> +             img_len = img_size * OPROM_BYTE_BOUNDARY;
> +             oprom_img = kzalloc(img_len, GFP_KERNEL);
> +             if (!oprom_img) {
> +                     ret = -ENOMEM;
> +                     goto err_free_hdr;
> +             }
> +
> +             spi_read_oprom_helper(img_len, offset, oprom_img, i915);
> +             parse_ptr = (u8 *)oprom_img;
> +             offset = offset + img_len;
> +
> +             /* opregion base offset */
> +             opreg_base = ((struct expansion_rom_header 
> *)parse_ptr)->opregion_base;
> +             /* CPD or opreg signature is present at opregion_base offset */
> +             memcpy(img_sig, parse_ptr + opreg_base, 
> sizeof(OPREGION_SIGNATURE));
> +
> +             if (!memcmp(img_sig, OPREGION_SIGNATURE, 
> sizeof(OPREGION_SIGNATURE) - 1)) {
> +                     *opreg = oprom_img;
> +                     *opreg_size = img_len;
> +                     drm_dbg_kms(&i915->drm, "Found opregion image\n");
> +                     ret = 0;
> +                     break;
> +             } else if (!memcmp(img_sig, CPD_SIGNATURE, NUM_CPD_BYTES)) {
> +                     if (code_type != OPROM_CSS_CODE_TYPE) {
> +                             drm_err(&i915->drm, "Invalid OPROM\n");
> +                             ret = -EINVAL;
> +                             goto err_free_img;
> +                     }
> +                     drm_dbg_kms(&i915->drm, "Found CSS image\n");
> +                     /* proceed here onwards for signature authentication */
> +                     kfree(oprom_img);
> +                     continue;
> +             }
> +
> +     } while (last_img != LAST_IMG_INDICATOR);
> +
> +     return ret;
> +
> +err_free_img:
> +     kfree(oprom_img);
> +err_free_hdr:
> +     kfree(oprom_img_hdr);
> +
> +     return ret;
> +}
> +
>  static int intel_use_opregion_panel_type_callback(const struct dmi_system_id 
> *id)
>  {
>       DRM_INFO("Using panel type from OpRegion on %s\n", id->ident);
> diff --git a/drivers/gpu/drm/i915/display/intel_opregion.h 
> b/drivers/gpu/drm/i915/display/intel_opregion.h
> index 4aa68ffbd30e..de53dde10dd9 100644
> --- a/drivers/gpu/drm/i915/display/intel_opregion.h
> +++ b/drivers/gpu/drm/i915/display/intel_opregion.h
> @@ -54,6 +54,34 @@ struct intel_opregion {
>  
>  #define OPREGION_SIZE            (8 * 1024)
>  
> +#define CPD_SIGNATURE "$CPD"                  /* CPD Signature */
> +#define NUM_CPD_BYTES 4
> +#define PCI_IMAGE_LENGTH_OFFSET 0x10
> +#define PCI_CODE_TYPE_OFFSET 0x14
> +#define PCI_LAST_IMAGE_INDICATOR_OFFSET 0x15
> +#define LAST_IMG_INDICATOR 0x80
> +#define OPROM_IMAGE_MAGIC 0xAA55       /* Little Endian */
> +#define OPROM_CSS_CODE_TYPE 0xF0
> +#define OPROM_BYTE_BOUNDARY 512        /* OPROM image sizes are indicated in 
> 512 byte boundaries */
> +#define OPROM_INITIAL_READ_SIZE 60     /* Read 60 bytes to compute the Img 
> Len from PCI structure */
> +
> +union oprom_header {
> +     u32 data;
> +     struct {
> +             u16 signature;  /* Offset[0x0]: Header 0x55 0xAA */
> +             u8 sizein512bytes;
> +             u8 reserved;
> +     };
> +};
> +
> +struct expansion_rom_header {
> +     union oprom_header header;      /* Offset[0x0]: Oprom Header */
> +     u16 vbiospostoffset;    /* Offset[0x4]: pointer to VBIOS entry point */
> +     u8 resvd[0x12];
> +     u16 pcistructoffset;    /* Offset[0x18]: Contains pointer PCI Data 
> Structure */
> +     u16 opregion_base;      /* Offset[0x1A]: Offset to Opregion Base start 
> */
> +};
> +
>  #ifdef CONFIG_ACPI
>  
>  int intel_opregion_setup(struct drm_i915_private *dev_priv);
> @@ -72,6 +100,9 @@ int intel_opregion_notify_adapter(struct drm_i915_private 
> *dev_priv,
>                                 pci_power_t state);
>  int intel_opregion_get_panel_type(struct drm_i915_private *dev_priv);
>  
> +int intel_spi_get_oprom_opreg(struct drm_i915_private *i915, u32 **opreg,
> +                           u32 *opreg_size);
> +
>  #else /* CONFIG_ACPI*/
>  
>  static inline int intel_opregion_setup(struct drm_i915_private *dev_priv)
> @@ -117,6 +148,11 @@ static inline int intel_opregion_get_panel_type(struct 
> drm_i915_private *dev)
>       return -ENODEV;
>  }
>  
> -#endif /* CONFIG_ACPI */
> +static int intel_spi_get_oprom_opreg(struct drm_i915_private *i915, u32 
> **opreg,
> +                                  u32 *opreg_size)
> +{
> +     return 0;
> +}
>  
> +#endif /* CONFIG_ACPI */
>  #endif

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to