Re: [PATCH] [next] drm/amdgpu: Replace one-element array with flex-array member

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

 



On Thu, Nov 10, 2022 at 10:57 AM Alex Deucher <alexdeucher@xxxxxxxxx> wrote:
>
> On Wed, Nov 9, 2022 at 2:33 AM Paulo Miguel Almeida
> <paulo.miguel.almeida.rodenas@xxxxxxxxx> wrote:
> >
> > One-element arrays are deprecated, and we are replacing them with
> > flexible array members instead. So, replace one-element array with
> > flexible-array member in structs _ATOM_CONNECTOR_DEVICE_TAG_RECORD,
> > _ATOM_OBJECT_GPIO_CNTL_RECORD, _ATOM_BRACKET_LAYOUT_RECORD,
> > _ATOM_BRACKET_LAYOUT_RECORD, _ATOM_LEAKAGE_VOLTAGE_OBJECT_V3,
> > _ATOM_FUSION_SYSTEM_INFO_V3, _ATOM_I2C_DATA_RECORD,
> > _ATOM_I2C_DEVICE_SETUP_INFO, _ATOM_ASIC_MVDD_INFO and refactor the
> > rest of the code accordingly. While at it, removed a redundant casting.
> >
> > Important to mention is that doing a build before/after this patch results
> > in no binary output differences.
> >
> > This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
> > routines on memcpy() and help us make progress towards globally
> > enabling -fstrict-flex-arrays=3 [1].
> >
> > Link: https://github.com/KSPP/linux/issues/79
> > Link: https://github.com/KSPP/linux/issues/238
> > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
> >
> > Signed-off-by: Paulo Miguel Almeida <paulo.miguel.almeida.rodenas@xxxxxxxxx>
> > ---
> >
> > Alex, I noticed a few structs in atombios.h that were not referenced. Is
> > there any appetite for removing them? Or is that one of those cases
> > where the structs are there should one driver ever need it?
>
> A lot of userspace tools use these tables and I'd like to keep the
> headers compatible.  Also, as I mentioned in the other thread, the
> atombios command tables often parse these tables when initializing the
> hardware so when you are debugging a command table, it helps to have
> the relevant structures defined so it's easier to understand what the
> command tables are doing.

Also, because this header is shared by multiple components within AMD
(linux, windows, bios, etc.), it would make things a lot easier if we
maintained compatibility.

Applied this patch.  Thanks,

Alex

>
> Alex
>
>
> >
> > Ex.:
> >         struct _ATOM_I2C_DATA_RECORD
> >         struct _ATOM_I2C_DEVICE_SETUP_INFO
> >         struct _ATOM_ASIC_MVDD_INFO
> > ---
> >  .../gpu/drm/amd/display/dc/bios/bios_parser.c    |  5 ++---
> >  drivers/gpu/drm/amd/include/atombios.h           | 16 ++++++++--------
> >  2 files changed, 10 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
> > index 39dd8b2dc254..6b9e64cd4379 100644
> > --- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
> > +++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
> > @@ -2606,8 +2606,7 @@ static enum bp_result update_slot_layout_info(
> >
> >         for (;;) {
> >
> > -               record_header = (ATOM_COMMON_RECORD_HEADER *)
> > -                       GET_IMAGE(ATOM_COMMON_RECORD_HEADER, record_offset);
> > +               record_header = GET_IMAGE(ATOM_COMMON_RECORD_HEADER, record_offset);
> >                 if (record_header == NULL) {
> >                         result = BP_RESULT_BADBIOSTABLE;
> >                         break;
> > @@ -2621,7 +2620,7 @@ static enum bp_result update_slot_layout_info(
> >
> >                 if (record_header->ucRecordType ==
> >                         ATOM_BRACKET_LAYOUT_RECORD_TYPE &&
> > -                       sizeof(ATOM_BRACKET_LAYOUT_RECORD)
> > +                       struct_size(record, asConnInfo, 1)
> >                         <= record_header->ucRecordSize) {
> >                         record = (ATOM_BRACKET_LAYOUT_RECORD *)
> >                                 (record_header);
> > diff --git a/drivers/gpu/drm/amd/include/atombios.h b/drivers/gpu/drm/amd/include/atombios.h
> > index 55ae93c1e365..60c44a8a067f 100644
> > --- a/drivers/gpu/drm/amd/include/atombios.h
> > +++ b/drivers/gpu/drm/amd/include/atombios.h
> > @@ -4733,7 +4733,7 @@ typedef struct  _ATOM_CONNECTOR_DEVICE_TAG_RECORD
> >    ATOM_COMMON_RECORD_HEADER   sheader;
> >    UCHAR                       ucNumberOfDevice;
> >    UCHAR                       ucReserved;
> > -  ATOM_CONNECTOR_DEVICE_TAG   asDeviceTag[1];         //This Id is same as "ATOM_DEVICE_XXX_SUPPORT", 1 is only for allocation
> > +  ATOM_CONNECTOR_DEVICE_TAG   asDeviceTag[];          //This Id is same as "ATOM_DEVICE_XXX_SUPPORT"
> >  }ATOM_CONNECTOR_DEVICE_TAG_RECORD;
> >
> >
> > @@ -4793,7 +4793,7 @@ typedef struct  _ATOM_OBJECT_GPIO_CNTL_RECORD
> >    ATOM_COMMON_RECORD_HEADER   sheader;
> >    UCHAR                       ucFlags;                // Future expnadibility
> >    UCHAR                       ucNumberOfPins;         // Number of GPIO pins used to control the object
> > -  ATOM_GPIO_PIN_CONTROL_PAIR  asGpio[1];              // the real gpio pin pair determined by number of pins ucNumberOfPins
> > +  ATOM_GPIO_PIN_CONTROL_PAIR  asGpio[];               // the real gpio pin pair determined by number of pins ucNumberOfPins
> >  }ATOM_OBJECT_GPIO_CNTL_RECORD;
> >
> >  //Definitions for GPIO pin state
> > @@ -4982,7 +4982,7 @@ typedef struct  _ATOM_BRACKET_LAYOUT_RECORD
> >    UCHAR                       ucWidth;
> >    UCHAR                       ucConnNum;
> >    UCHAR                       ucReserved;
> > -  ATOM_CONNECTOR_LAYOUT_INFO  asConnInfo[1];
> > +  ATOM_CONNECTOR_LAYOUT_INFO  asConnInfo[];
> >  }ATOM_BRACKET_LAYOUT_RECORD;
> >
> >
> > @@ -5161,7 +5161,7 @@ typedef struct  _ATOM_GPIO_VOLTAGE_OBJECT_V3
> >     UCHAR  ucPhaseDelay;                      // phase delay in unit of micro second
> >     UCHAR  ucReserved;
> >     ULONG  ulGpioMaskVal;                     // GPIO Mask value
> > -   VOLTAGE_LUT_ENTRY_V2 asVolGpioLut[1];
> > +   VOLTAGE_LUT_ENTRY_V2 asVolGpioLut[];
> >  }ATOM_GPIO_VOLTAGE_OBJECT_V3;
> >
> >  typedef struct  _ATOM_LEAKAGE_VOLTAGE_OBJECT_V3
> > @@ -5171,7 +5171,7 @@ typedef struct  _ATOM_LEAKAGE_VOLTAGE_OBJECT_V3
> >     UCHAR    ucLeakageEntryNum;               // indicate the entry number of LeakageId/Voltage Lut table
> >     UCHAR    ucReserved[2];
> >     ULONG    ulMaxVoltageLevel;
> > -   LEAKAGE_VOLTAGE_LUT_ENTRY_V2 asLeakageIdLut[1];
> > +   LEAKAGE_VOLTAGE_LUT_ENTRY_V2 asLeakageIdLut[];
> >  }ATOM_LEAKAGE_VOLTAGE_OBJECT_V3;
> >
> >
> > @@ -6599,7 +6599,7 @@ typedef struct _ATOM_FUSION_SYSTEM_INFO_V3
> >  typedef struct _ATOM_I2C_DATA_RECORD
> >  {
> >    UCHAR         ucNunberOfBytes;                                              //Indicates how many bytes SW needs to write to the external ASIC for one block, besides to "Start" and "Stop"
> > -  UCHAR         ucI2CData[1];                                                 //I2C data in bytes, should be less than 16 bytes usually
> > +  UCHAR         ucI2CData[];                                                  //I2C data in bytes, should be less than 16 bytes usually
> >  }ATOM_I2C_DATA_RECORD;
> >
> >
> > @@ -6610,14 +6610,14 @@ typedef struct _ATOM_I2C_DEVICE_SETUP_INFO
> >    UCHAR                              ucSSChipID;             //SS chip being used
> >    UCHAR                              ucSSChipSlaveAddr;      //Slave Address to set up this SS chip
> >    UCHAR                           ucNumOfI2CDataRecords;  //number of data block
> > -  ATOM_I2C_DATA_RECORD            asI2CData[1];
> > +  ATOM_I2C_DATA_RECORD            asI2CData[];
> >  }ATOM_I2C_DEVICE_SETUP_INFO;
> >
> >  //==========================================================================================
> >  typedef struct  _ATOM_ASIC_MVDD_INFO
> >  {
> >    ATOM_COMMON_TABLE_HEADER         sHeader;
> > -  ATOM_I2C_DEVICE_SETUP_INFO      asI2CSetup[1];
> > +  ATOM_I2C_DEVICE_SETUP_INFO      asI2CSetup[];
> >  }ATOM_ASIC_MVDD_INFO;
> >
> >  //==========================================================================================
> > --
> > 2.37.3
> >



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

  Powered by Linux