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

From: Alex Deucher
Date: Thu Nov 10 2022 - 11:01:53 EST

Next message: Andrew Lunn: "Re: [PATCH net-next v2 00/11] net: pcs: Add support for devices probed in the "usual" manner"
Previous message: Sakari Ailus: "Re: [PATCH v4 1/1] i2c: Restore initial power state when we are done."
In reply to: Alex Deucher: "Re: [PATCH] [next] drm/amdgpu: Replace one-element array with flex-array member"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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
> >

Next message: Andrew Lunn: "Re: [PATCH net-next v2 00/11] net: pcs: Add support for devices probed in the "usual" manner"
Previous message: Sakari Ailus: "Re: [PATCH v4 1/1] i2c: Restore initial power state when we are done."
In reply to: Alex Deucher: "Re: [PATCH] [next] drm/amdgpu: Replace one-element array with flex-array member"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]