[PATCH] [next] i915/gvt: Replace one-element array with flexible-array member
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 struct gvt_firmware_header and refactor the
rest of the code accordingly.
Additionally, previous implementation was allocating 8 bytes more than
required to represent firmware_header + cfg_space data + mmio data.
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://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
Signed-off-by: Paulo Miguel Almeida
---
To make reviewing this patch easier, I'm pasting before/after struct
sizes.
pahole -C gvt_firmware_header before/drivers/gpu/drm/i915/gvt/firmware.o
struct gvt_firmware_header {
u64magic;/* 0 8 */
u32crc32;/* 8 4 */
u32version; /*12 4 */
u64cfg_space_size; /*16 8 */
u64cfg_space_offset; /*24 8 */
u64mmio_size;/*32 8 */
u64mmio_offset; /*40 8 */
unsigned char data[1]; /*48 1 */
/* size: 56, cachelines: 1, members: 8 */
/* padding: 7 */
/* last cacheline: 56 bytes */
};
pahole -C gvt_firmware_header after/drivers/gpu/drm/i915/gvt/firmware.o
struct gvt_firmware_header {
u64magic;/* 0 8 */
u32crc32;/* 8 4 */
u32version; /*12 4 */
u64cfg_space_size; /*16 8 */
u64cfg_space_offset; /*24 8 */
u64mmio_size;/*32 8 */
u64mmio_offset; /*40 8 */
unsigned char data[]; /*48 0 */
/* size: 48, cachelines: 1, members: 8 */
/* last cacheline: 48 bytes */
};
As you can see the additional byte of the fake-flexible array (data[1])
forced the compiler to pad the struct but those bytes aren't actually used
as first & last bytes (of both cfg_space and mmio) are controlled by the
<>_size and <>_offset members present in the gvt_firmware_header struct.
---
drivers/gpu/drm/i915/gvt/firmware.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gvt/firmware.c
b/drivers/gpu/drm/i915/gvt/firmware.c
index a683c22d5b64..dce93738e98a 100644
--- a/drivers/gpu/drm/i915/gvt/firmware.c
+++ b/drivers/gpu/drm/i915/gvt/firmware.c
@@ -45,7 +45,7 @@ struct gvt_firmware_header {
u64 cfg_space_offset; /* offset in the file */
u64 mmio_size;
u64 mmio_offset;/* offset in the file */
- unsigned char data[1];
+ unsigned char data[];
};
#define dev_to_drm_minor(d) dev_get_drvdata((d))
@@ -77,7 +77,7 @@ static int expose_firmware_sysfs(struct intel_gvt *gvt)
unsigned long size, crc32_start;
int ret;
- size = sizeof(*h) + info->mmio_size + info->cfg_space_size;
+ size = offsetof(struct gvt_firmware_header, data) + info->mmio_size +
info->cfg_space_size;
firmware = vzalloc(size);
if (!firmware)
return -ENOMEM;
--
2.38.1
[PATCH] [next] drm/radeon: Replace 1-element arrays with flexible-array members
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_DISPLAY_OBJECT_PATH,
_ATOM_DISPLAY_OBJECT_PATH_TABLE, _ATOM_OBJECT_TABLE, GOP_VBIOS_CONTENT
_ATOM_GPIO_VOLTAGE_OBJECT_V3 and refactor the rest of the code accordingly.
It's worth mentioning 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/239
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
Signed-off-by: Paulo Miguel Almeida
---
Notes for the maintainer:
- These are all fake-flexible arrays with references in source code for
the radeon driver. Given the way they are used, no change to *.c files
were required.
---
drivers/gpu/drm/radeon/atombios.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios.h
b/drivers/gpu/drm/radeon/atombios.h
index 235e59b547a1..8a6621f1e82c 100644
--- a/drivers/gpu/drm/radeon/atombios.h
+++ b/drivers/gpu/drm/radeon/atombios.h
@@ -4020,7 +4020,7 @@ typedef struct _ATOM_DISPLAY_OBJECT_PATH
USHORTusSize;//the size of
ATOM_DISPLAY_OBJECT_PATH
USHORTusConnObjectId;//Connector Object
ID
USHORTusGPUObjectId; //GPU ID
- USHORTusGraphicObjIds[1]; //1st Encoder Obj
source from GPU to last Graphic Obj destinate to connector.
+ USHORTusGraphicObjIds[]; //1st Encoder Obj
source from GPU to last Graphic Obj destinate to connector.
}ATOM_DISPLAY_OBJECT_PATH;
typedef struct _ATOM_DISPLAY_EXTERNAL_OBJECT_PATH
@@ -4037,7 +4037,7 @@ typedef struct _ATOM_DISPLAY_OBJECT_PATH_TABLE
UCHAR ucNumOfDispPath;
UCHAR ucVersion;
UCHAR ucPadding[2];
- ATOM_DISPLAY_OBJECT_PATHasDispPath[1];
+ ATOM_DISPLAY_OBJECT_PATHasDispPath[];
}ATOM_DISPLAY_OBJECT_PATH_TABLE;
@@ -4053,7 +4053,7 @@ typedef struct _ATOM_OBJECT_TABLE
//Above 4 object table
{
UCHAR ucNumberOfObjects;
UCHAR ucPadding[3];
- ATOM_OBJECT asObjects[1];
+ ATOM_OBJECT asObjects[];
}ATOM_OBJECT_TABLE;
typedef struct _ATOM_SRC_DST_TABLE_FOR_ONE_OBJECT
//usSrcDstTableOffset pointing to this structure
@@ -4615,7 +4615,7 @@ typedef struct _ATOM_GPIO_VOLTAGE_OBJECT_V3
UCHARucPhaseDelay;// phase delay in unit of micro second
UCHARucReserved;
ULONGulGpioMaskVal; // 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
@@ -7964,7 +7964,7 @@ typedef struct {
typedef struct {
VFCT_IMAGE_HEADERVbiosHeader;
- UCHARVbiosContent[1];
+ UCHARVbiosContent[];
}GOP_VBIOS_CONTENT;
typedef struct {
--
2.38.1
[PATCH] [next] drm/amdgpu: Replace remaining 1-element array with flex-array
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 struct GOP_VBIOS_CONTENT and refactor the
rest of the code accordingly.
Important to mention is that doing a build before/after this patch
results in no functional 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
---
This should be the last one-element array that had references in source
code. Given the way it was used, no *.c code change was required.
I will move on to the atombios.h in the radeon driver.
---
drivers/gpu/drm/amd/include/atombios.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/include/atombios.h
b/drivers/gpu/drm/amd/include/atombios.h
index 4dc738c51771..b78360a71bc9 100644
--- a/drivers/gpu/drm/amd/include/atombios.h
+++ b/drivers/gpu/drm/amd/include/atombios.h
@@ -9292,7 +9292,7 @@ typedef struct {
typedef struct {
VFCT_IMAGE_HEADER VbiosHeader;
- UCHAR VbiosContent[1];
+ UCHAR VbiosContent[];
}GOP_VBIOS_CONTENT;
typedef struct {
--
2.37.3
[PATCH] [next] drm/amdgpu: Replace one-elements array with flex-array members
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_I2C_VOLTAGE_OBJECT_V3, ATOM_ASIC_INTERNAL_SS_INFO_V2, ATOM_ASIC_INTERNAL_SS_INFO_V3, and refactor the rest of the code accordingly. Important to mention is that doing a build before/after this patch results in no functional 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 --- Binary difference findings: Some changes took more than a single line which changed the line number parameter passed to the drm_dbg function (which leverages kernel's dynamic debugging). Functionally-wise, nothing changed after doing a before/after patch build. --- .../gpu/drm/amd/display/dc/bios/bios_parser.c | 22 --- drivers/gpu/drm/amd/include/atombios.h| 6 ++--- 2 files changed, 17 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 6b9e64cd4379..a1a00f432168 100644 --- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c +++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c @@ -665,8 +665,9 @@ static enum bp_result get_ss_info_v3_1( if (!DATA_TABLES(ASIC_InternalSS_Info)) return BP_RESULT_UNSUPPORTED; - ss_table_header_include = GET_IMAGE(ATOM_ASIC_INTERNAL_SS_INFO_V3, - DATA_TABLES(ASIC_InternalSS_Info)); + ss_table_header_include = ((ATOM_ASIC_INTERNAL_SS_INFO_V3 *) bios_get_image(>base, + DATA_TABLES(ASIC_InternalSS_Info), + struct_size(ss_table_header_include, asSpreadSpectrum, 1))); table_size = (le16_to_cpu(ss_table_header_include->sHeader.usStructureSize) - sizeof(ATOM_COMMON_TABLE_HEADER)) @@ -1032,8 +1033,10 @@ static enum bp_result get_ss_info_from_internal_ss_info_tbl_V2_1( if (!DATA_TABLES(ASIC_InternalSS_Info)) return result; - header = GET_IMAGE(ATOM_ASIC_INTERNAL_SS_INFO_V2, - DATA_TABLES(ASIC_InternalSS_Info)); + header = ((ATOM_ASIC_INTERNAL_SS_INFO_V2 *) bios_get_image( + >base, + DATA_TABLES(ASIC_InternalSS_Info), + struct_size(header, asSpreadSpectrum, 1))); memset(info, 0, sizeof(struct spread_spectrum_info)); @@ -1712,8 +1715,10 @@ static uint32_t get_ss_entry_number_from_internal_ss_info_tbl_v2_1( if (!DATA_TABLES(ASIC_InternalSS_Info)) return 0; - header_include = GET_IMAGE(ATOM_ASIC_INTERNAL_SS_INFO_V2, - DATA_TABLES(ASIC_InternalSS_Info)); + header_include = ((ATOM_ASIC_INTERNAL_SS_INFO_V2 *) bios_get_image( + >base, + DATA_TABLES(ASIC_InternalSS_Info), + struct_size(header_include, asSpreadSpectrum, 1))); size = (le16_to_cpu(header_include->sHeader.usStructureSize) - sizeof(ATOM_COMMON_TABLE_HEADER)) @@ -1749,8 +1754,9 @@ static uint32_t get_ss_entry_number_from_internal_ss_info_tbl_V3_1( if (!DATA_TABLES(ASIC_InternalSS_Info)) return number; - header_include = GET_IMAGE(ATOM_ASIC_INTERNAL_SS_INFO_V3, - DATA_TABLES(ASIC_InternalSS_Info)); + header_include = ((ATOM_ASIC_INTERNAL_SS_INFO_V3 *) bios_get_image(>base, + DATA_TABLES(ASIC_InternalSS_Info), + struct_size(header_include, asSpreadSpectrum, 1))); size = (le16_to_cpu(header_include->sHeader.usStructureSize) - sizeof(ATOM_COMMON_TABLE_HEADER)) / sizeof(ATOM_ASIC_SS_ASSIGNMENT_V3); diff --git a/drivers/gpu/drm/amd/include/atombios.h b/drivers/gpu/drm/amd/include/atombios.h index 60c44a8a067f..4dc738c51771 100644 --- a/drivers/gpu/drm/amd/include/atombios.h +++ b/drivers/gpu/drm/amd/include/atombios.h @@ -5146,7 +5146,7 @@ typedef struct _ATOM_I2C_VOLTAGE_OBJECT_V3 UCHAR ucVoltageControlOffset; UCHAR ucVoltageControlFlag; // Bit0: 0 - One byte data; 1 - Two byte data UCHAR ulReserved[3]; - VOLTAGE_LUT_ENTRY asVolI2cLut[1]; // end with 0xff + VOLTAGE_LUT_ENTRY asVolI2cLut[]; // end with 0xff }ATOM_I2C_VOLTAGE_OBJECT_V3; // ATOM_I2C_VOLTAGE_OBJECT_V3.ucVoltageControlFlag @@ -6679,7 +6679,7 @@ typedef struct _ATOM_
Re: [RFC] Approaches to deal with a struct with multiple fake flexible arrays members
On Thu, Nov 10, 2022 at 11:39:02PM -0600, Gustavo A. R. Silva wrote:
> On Wed, Nov 09, 2022 at 10:20:34PM -0500, Alex Deucher wrote:
> > On Wed, Nov 9, 2022 at 8:31 PM Paulo Miguel Almeida
> > wrote:
> > >
> > > On Wed, Nov 09, 2022 at 06:45:57PM -0600, Gustavo A. R. Silva wrote:
> > > > On Wed, Nov 09, 2022 at 04:45:42PM +1300, Paulo Miguel Almeida wrote:
> > > >
> > > > Adding Alex, Christian and DRM lists to the thread.
> > >
> > > Thanks so much for your reply Gustavo
> > > Yep, that's a good idea.
> > >
> > > >
> > > > > struct _ATOM_INIT_REG_BLOCK {
> > > > > USHORT usRegIndexTblSize;/* 0 2 */
> > > > > USHORT usRegDataBlkSize; /* 2 2 */
> > > > > ATOM_INIT_REG_INDEX_FORMAT asRegIndexBuf[1]; /* 4 3 */
> > > > > ATOM_MEMORY_SETTING_DATA_BLOCK asRegDataBuf[1]; /* 7 8 */
> > > >
> > > > I didn't find evidence that asRegDataBuf is used anywhere in the
> > > > codebase[1].
> > > > ...
> > > >
> > > > ...
> > > > As I pointed out above, I don't see asRegDataBuf[] being used in the
> > > > codebase (of course it may describe firmware memory layout). Instead,
> > > > there is this jump to a block of data past asRegIndexBuf[]:
> > > >
> > > > drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1444:
> > > > 1444: ATOM_MEMORY_SETTING_DATA_BLOCK *reg_data =
> > > > 1445: (ATOM_MEMORY_SETTING_DATA_BLOCK *)
> > > > 1446: ((u8 *)reg_block + (2 * sizeof(u16)) +
> > > > 1447: le16_to_cpu(reg_block->usRegIndexTblSize));
> > > >
> > > > So, it seems the one relevant array, from the kernel side, is
> > > > asRegIndexBuf[]. I wonder if we really need asRegDataBuf[] in that
> > > > structure... or if we could try modifying that struct and only have
> > > > asRegIndexBuf[] as last member? and then we can transform it into a
> > > > flex-array member.
> > >
> > > I saw that one too. That would be the way it's currently accessing
> > > asRegDataBuf member as the existing struct would make asRegDataBuf[0]
> > > point
> > > to some index within the asRegIndexBuf member (as you probably got it too)
> > >
> > > you are right... asRegDataBuff isn't used from a static analysis
> > > point of view but removing it make the code a bit cryptic, right?
> > >
> > > That's pickle, ay? :-)
> > >
> > > >
> > > > If for any strong reasong we cannot remove asRegDataBuf[] then I think
> > > > we
> > > > could give it a try and use DECLARE_FLEX_ARRAY() to declare both arrays
> > > > in the structure.
> > > >
> > >
> > > Out of curiosity, why both rather than just asRegIndexBuf?
>
> Because if I understand the code and Alex's reply below correctly, both
> arrays are being used to describe data of variable size, and both arrays
> need to stay. The situation is that it'd be _strange_ to transform just
> one of them into a flex-array member and not the other, and at the same
My apologies, I tried being succinct and I ended up mistakenly leading you
to understand a different thing. I will be more careful next time :-)
What I meant was why would you use DECLARE_FLEX_ARRAY macro for both
members instead of the following ?
typedef struct _ATOM_INIT_REG_BLOCK{
USHORT usRegIndexTblSize;
USHORT usRegDataBlkSize;
DECLARE_FLEX_ARRAY(ATOM_INIT_REG_INDEX_FORMAT, asRegIndexBuf); // Macro
needed
ATOM_MEMORY_SETTING_DATA_BLOCK asRegDataBuf[];// Regular
FMA
}ATOM_INIT_REG_BLOCK;
> On the other hand, I fail to see how the current state of the code is
> problematic for things like -fstrict-flex-arrays, right now. asRegDataBuf[]
> is not being used for anything in the kernel, and asRegIndexBuf[] is
> correctly being accessed through it's only valid index zero, after which
> is decays into a pointer[2].
>
> That struct is definitely not great (I don't love it), but we might try
> to explore other cases while we determine how and if we ultimately need
> to modify it.
>
> I'll open an issue for this in the bug tracker, so we keep an eye on it.
> :)
Fair enough. Thanks heaps Gustavo, I will move on to the other fake flex
array occurences and circle back to it once a decision in made. Please
count on me to make the changes :-)
>
> > >
> > > > B
Re: [RFC] Approaches to deal with a struct with multiple fake flexible arrays members
On Wed, Nov 09, 2022 at 06:45:57PM -0600, Gustavo A. R. Silva wrote:
> On Wed, Nov 09, 2022 at 04:45:42PM +1300, Paulo Miguel Almeida wrote:
>
> Adding Alex, Christian and DRM lists to the thread.
Thanks so much for your reply Gustavo
Yep, that's a good idea.
>
> > struct _ATOM_INIT_REG_BLOCK {
> > USHORT usRegIndexTblSize;/* 0 2 */
> > USHORT usRegDataBlkSize; /* 2 2 */
> > ATOM_INIT_REG_INDEX_FORMAT asRegIndexBuf[1]; /* 4 3 */
> > ATOM_MEMORY_SETTING_DATA_BLOCK asRegDataBuf[1]; /* 7 8 */
>
> I didn't find evidence that asRegDataBuf is used anywhere in the
> codebase[1].
> ...
>
> ...
> As I pointed out above, I don't see asRegDataBuf[] being used in the
> codebase (of course it may describe firmware memory layout). Instead,
> there is this jump to a block of data past asRegIndexBuf[]:
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_atombios.c:1444:
> 1444: ATOM_MEMORY_SETTING_DATA_BLOCK *reg_data =
> 1445: (ATOM_MEMORY_SETTING_DATA_BLOCK *)
> 1446: ((u8 *)reg_block + (2 * sizeof(u16)) +
> 1447: le16_to_cpu(reg_block->usRegIndexTblSize));
>
> So, it seems the one relevant array, from the kernel side, is
> asRegIndexBuf[]. I wonder if we really need asRegDataBuf[] in that
> structure... or if we could try modifying that struct and only have
> asRegIndexBuf[] as last member? and then we can transform it into a
> flex-array member.
I saw that one too. That would be the way it's currently accessing
asRegDataBuf member as the existing struct would make asRegDataBuf[0] point
to some index within the asRegIndexBuf member (as you probably got it too)
you are right... asRegDataBuff isn't used from a static analysis
point of view but removing it make the code a bit cryptic, right?
That's pickle, ay? :-)
>
> If for any strong reasong we cannot remove asRegDataBuf[] then I think we
> could give it a try and use DECLARE_FLEX_ARRAY() to declare both arrays
> in the structure.
>
Out of curiosity, why both rather than just asRegIndexBuf?
> But first, of course, Alex, Christian, it'd be really great if we can
> have your input and feedback. :)
>
> Thanks!
> --
> Gustavo
>
- Paulo A.
[PATCH] [next] drm/amdgpu: Replace one-element array with flex-array member
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
---
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?
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
UCHARucLeakageEntryNum; // indicate the entry number of
LeakageId/Voltage Lut table
UCHARucReserved[2];
ULONGulMaxVoltageLe
[PATCH] [next] drm/amdgpu: Replace 1-element array with flexible-array member
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_GPIO_PIN_ASSIGNMENT,
_ATOM_DISPLAY_OBJECT_PATH, _ATOM_DISPLAY_OBJECT_PATH_TABLE,
_ATOM_OBJECT_TABLE and refactor the rest of the code accordingly.
Important to mention is that doing a build before/after this patch results
in no functional 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
---
Binary difference findings:
Some changes took more than a single line which changed the line
number parameter passed to the drm_dbg function (which leverages
kernel's dynamic debugging). Functionally-wise, nothing changed
after doing a before/after patch build.
Additional one-element arrays to be changed:
There are more instances of one-element arrays to be changed but
I will keep patches small so they are easy to review. [and I can
only dedicate a few hours per day on this :-) ]
---
.../gpu/drm/amd/display/dc/bios/bios_parser.c | 23 ---
drivers/gpu/drm/amd/include/atombios.h| 8 +++
2 files changed, 19 insertions(+), 12 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 9b8ea6e9a2b9..39dd8b2dc254 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser.c
@@ -138,7 +138,9 @@ static uint8_t get_number_of_objects(struct bios_parser
*bp, uint32_t offset)
uint32_t object_table_offset = bp->object_info_tbl_offset + offset;
- table = GET_IMAGE(ATOM_OBJECT_TABLE, object_table_offset);
+ table = ((ATOM_OBJECT_TABLE *) bios_get_image(>base,
+ object_table_offset,
+ struct_size(table, asObjects, 1)));
if (!table)
return 0;
@@ -166,8 +168,9 @@ static struct graphics_object_id
bios_parser_get_connector_id(
uint32_t connector_table_offset = bp->object_info_tbl_offset
+
le16_to_cpu(bp->object_info_tbl.v1_1->usConnectorObjectTableOffset);
- ATOM_OBJECT_TABLE *tbl =
- GET_IMAGE(ATOM_OBJECT_TABLE, connector_table_offset);
+ ATOM_OBJECT_TABLE *tbl = ((ATOM_OBJECT_TABLE *)
bios_get_image(>base,
+ connector_table_offset,
+ struct_size(tbl, asObjects, 1)));
if (!tbl) {
dm_error("Can't get connector table from atom bios.\n");
@@ -1789,11 +1792,13 @@ static enum bp_result bios_parser_get_gpio_pin_info(
if (!DATA_TABLES(GPIO_Pin_LUT))
return BP_RESULT_BADBIOSTABLE;
- header = GET_IMAGE(ATOM_GPIO_PIN_LUT, DATA_TABLES(GPIO_Pin_LUT));
+ header = ((ATOM_GPIO_PIN_LUT *) bios_get_image(>base,
+ DATA_TABLES(GPIO_Pin_LUT),
+ struct_size(header, asGPIO_Pin, 1)));
if (!header)
return BP_RESULT_BADBIOSTABLE;
- if (sizeof(ATOM_COMMON_TABLE_HEADER) + sizeof(ATOM_GPIO_PIN_LUT)
+ if (sizeof(ATOM_COMMON_TABLE_HEADER) + struct_size(header, asGPIO_Pin,
1)
> le16_to_cpu(header->sHeader.usStructureSize))
return BP_RESULT_BADBIOSTABLE;
@@ -1978,7 +1983,8 @@ static ATOM_OBJECT *get_bios_object(struct bios_parser
*bp,
offset += bp->object_info_tbl_offset;
- tbl = GET_IMAGE(ATOM_OBJECT_TABLE, offset);
+ tbl = ((ATOM_OBJECT_TABLE *) bios_get_image(>base, offset,
+ struct_size(tbl, asObjects, 1)));
if (!tbl)
return NULL;
@@ -2709,8 +2715,9 @@ static enum bp_result get_bracket_layout_record(
genericTableOffset = bp->object_info_tbl_offset +
bp->object_info_tbl.v1_3->usMiscObjectTableOffset;
- object_table = (ATOM_OBJECT_TABLE *)
- GET_IMAGE(ATOM_OBJECT_TABLE, genericTableOffset);
+ object_table = ((ATOM_OBJECT_TABLE *) bios_get_image(>base,
+ genericTableOffset,
+ struct_size(object_table, asObjects, 1)));
if (!object_table)
return BP_RESULT_FAILURE;
diff --git a/drivers/gpu/drm/amd/include/atombios.h
b/drivers/gpu/drm/amd/include/atombios.h
index b5b1d073f8e2..55ae93c1e365 100644
--- a/drivers/gpu/drm/amd/include/atombios.h
+++ b/drivers/gpu/drm/amd/include/atombios.h
@@ -4386,7 +4386,7 @@ typedef struct _ATOM_GPIO_PIN_ASSIGNMENT
typedef s
Re: [PATCH v2] [next] drm/radeon: Replace one-element array with flexible-array member
On Tue, Nov 01, 2022 at 10:42:14AM -0400, Alex Deucher wrote: > On Fri, Oct 28, 2022 at 11:32 PM Paulo Miguel Almeida > 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 struct _ATOM_FAKE_EDID_PATCH_RECORD and > > refactor the rest of the code accordingly. > > > > It's worth mentioning 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]. > > This seems like a worthy goal, and I'm not opposed to the patch per > se, but it seems a bit at odds with what this header represents. > atombios.h represents the memory layout of the data stored in the ROM > on the GPU. This changes the memory layout of that ROM. We can work > around that in the driver code, but if someone were to take this > header to try and write some standalone tool or use it for something > else in the kernel, it would not reflect reality. > > Alex > Hi Alex, thanks for taking the time to review this patch. I see where you are coming from and why you may be apprehensive with the approach. From my humble point of view, I think that the artificial line that separates "what we can change at will" and "what we should be extra careful not to break/confuse others" is whether the header file is part of the UAPI. Given that atombios.h isn't publicly accessible, I tend to gravitate towards the first one. > > + /* empty fake edid record must be 3 bytes long */ > +sizeof(ATOM_FAKE_EDID_PATCH_RECORD) + 1; I am assuming that this is the part of the patch that makes you feel concerned about how devs will get it (should they copy this header), is that right? If so, would a comment on the ATOM_FAKE_EDID_PATCH_RECORD struct specifying the size of the struct when empty do the trick? - Paulo A.
[PATCH] [next] i915/gvt: remove hardcoded value on crc32_start calculation
struct gvt_firmware_header has a crc32 member in which all members that come after the that field are used to calculate it. The previous implementation added the value '4' (crc32's u32 size) to calculate the crc32_start offset which came across as a bit cryptic until you take a deeper look at the struct. This patch changes crc32_start offset to the 'version' member which is the first member of the struct gvt_firmware_header after crc32. It's worth mentioning that doing a build before/after this patch results in no binary output differences. Signed-off-by: Paulo Miguel Almeida --- drivers/gpu/drm/i915/gvt/firmware.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gvt/firmware.c b/drivers/gpu/drm/i915/gvt/firmware.c index 54fe442238c6..a683c22d5b64 100644 --- a/drivers/gpu/drm/i915/gvt/firmware.c +++ b/drivers/gpu/drm/i915/gvt/firmware.c @@ -104,7 +104,7 @@ static int expose_firmware_sysfs(struct intel_gvt *gvt) memcpy(p, gvt->firmware.mmio, info->mmio_size); - crc32_start = offsetof(struct gvt_firmware_header, crc32) + 4; + crc32_start = offsetof(struct gvt_firmware_header, version); h->crc32 = crc32_le(0, firmware + crc32_start, size - crc32_start); firmware_attr.size = size; -- 2.25.4
[PATCH v2] [next] drm/radeon: Replace one-element array with flexible-array member
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 struct _ATOM_FAKE_EDID_PATCH_RECORD and
refactor the rest of the code accordingly.
It's worth mentioning 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/239
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
Signed-off-by: Paulo Miguel Almeida
---
Changelog:
v2: no binary output differences patch; report binary changes findings
on commit log. Res: Kees Cook.
This request was made in an identical, yet different, patch but the
same feedback applies.
https://lore.kernel.org/lkml/y1x3mtrj8ckxx...@mail.google.com/
v1: https://lore.kernel.org/lkml/y1trhre3nk5ia...@mail.google.com/
---
drivers/gpu/drm/radeon/atombios.h| 2 +-
drivers/gpu/drm/radeon/radeon_atombios.c | 7 +--
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios.h
b/drivers/gpu/drm/radeon/atombios.h
index da35a970fcc0..235e59b547a1 100644
--- a/drivers/gpu/drm/radeon/atombios.h
+++ b/drivers/gpu/drm/radeon/atombios.h
@@ -3615,7 +3615,7 @@ typedef struct _ATOM_FAKE_EDID_PATCH_RECORD
{
UCHAR ucRecordType;
UCHAR ucFakeEDIDLength;
- UCHAR ucFakeEDIDString[1];// This actually has ucFakeEdidLength elements.
+ UCHAR ucFakeEDIDString[];// This actually has ucFakeEdidLength elements.
} ATOM_FAKE_EDID_PATCH_RECORD;
typedef struct _ATOM_PANEL_RESOLUTION_PATCH_RECORD
diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c
b/drivers/gpu/drm/radeon/radeon_atombios.c
index 204127bad89c..4ad5a328d920 100644
--- a/drivers/gpu/drm/radeon/radeon_atombios.c
+++ b/drivers/gpu/drm/radeon/radeon_atombios.c
@@ -1727,8 +1727,11 @@ struct radeon_encoder_atom_dig
*radeon_atombios_get_lvds_info(struct
}
}
record +=
fake_edid_record->ucFakeEDIDLength ?
-
fake_edid_record->ucFakeEDIDLength + 2 :
-
sizeof(ATOM_FAKE_EDID_PATCH_RECORD);
+ struct_size(fake_edid_record,
+ ucFakeEDIDString,
+
fake_edid_record->ucFakeEDIDLength) :
+ /* empty fake edid record
must be 3 bytes long */
+
sizeof(ATOM_FAKE_EDID_PATCH_RECORD) + 1;
break;
case LCD_PANEL_RESOLUTION_RECORD_TYPE:
panel_res_record =
(ATOM_PANEL_RESOLUTION_PATCH_RECORD *)record;
--
2.37.3
[PATCH v2] [next] drm/amdgpu: Replace one-element array with flexible-array member
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 struct _ATOM_FAKE_EDID_PATCH_RECORD and
refactor the rest of the code accordingly.
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
---
Changelog:
v2: no binary output differences patch; report binary changes findings
on commit log. Res: Kees Cook
v1: https://lore.kernel.org/lkml/y1tkwdwpup+ud...@mail.google.com/
---
drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 7 +--
drivers/gpu/drm/amd/include/atombios.h | 2 +-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
index 6be9ac2b9c5b..18ae9433e463 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
@@ -2081,8 +2081,11 @@ amdgpu_atombios_encoder_get_lcd_info(struct
amdgpu_encoder *encoder)
}
}
record +=
fake_edid_record->ucFakeEDIDLength ?
-
fake_edid_record->ucFakeEDIDLength + 2 :
-
sizeof(ATOM_FAKE_EDID_PATCH_RECORD);
+ struct_size(fake_edid_record,
+ ucFakeEDIDString,
+
fake_edid_record->ucFakeEDIDLength) :
+ /* empty fake edid record
must be 3 bytes long */
+
sizeof(ATOM_FAKE_EDID_PATCH_RECORD) + 1;
break;
case LCD_PANEL_RESOLUTION_RECORD_TYPE:
panel_res_record =
(ATOM_PANEL_RESOLUTION_PATCH_RECORD *)record;
diff --git a/drivers/gpu/drm/amd/include/atombios.h
b/drivers/gpu/drm/amd/include/atombios.h
index 15943bc21bc5..b5b1d073f8e2 100644
--- a/drivers/gpu/drm/amd/include/atombios.h
+++ b/drivers/gpu/drm/amd/include/atombios.h
@@ -4107,7 +4107,7 @@ typedef struct _ATOM_FAKE_EDID_PATCH_RECORD
{
UCHAR ucRecordType;
UCHAR ucFakeEDIDLength; // = 128 means EDID length is 128 bytes,
otherwise the EDID length = ucFakeEDIDLength*128
- UCHAR ucFakeEDIDString[1];// This actually has ucFakeEdidLength elements.
+ UCHAR ucFakeEDIDString[]; // This actually has ucFakeEdidLength elements.
} ATOM_FAKE_EDID_PATCH_RECORD;
typedef struct _ATOM_PANEL_RESOLUTION_PATCH_RECORD
--
2.37.3
Re: [PATCH] [next] drm/amdgpu: Replace one-element array with flexible-array member
On Fri, Oct 28, 2022 at 07:33:17PM +0200, Christian König wrote: > Am 28.10.22 um 18:36 schrieb Kees Cook: > > > All that said, converting away from them can be tricky, and I think such > > conversions need to explicitly show how they were checked for binary > > differences[2]. > > Oh, that's a great idea! Yes, if this can be guaranteed then the change is > obviously perfectly ok. > > > > > Paulo, can you please check for deltas and report your findings in the > > commit log? Note that add struct_size() use in the same patch may result > > in binary differences, so for more complex cases, you may want to split > > the 1-element conversion from the struct_size() conversions. > > > > -Kees Noted. I will reporting my findings on commit logs from now onwards. Given that I split the if-ternary to avoid checking "fake_edid_record->ucFakeEDIDLength" twice then (for the current patch), yes, there will be changes to *.o|ko files. Knowing that Christian would feel more confident with no binary changes at this point, I will send a different patch aiming solely on the replacement of 1-elem array without incurring binary changes. -- Paulo A.
[PATCH] [next] drm/radeon: Replace one-element array with flexible-array member
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 struct _ATOM_FAKE_EDID_PATCH_RECORD and
refactor the rest of the code accordingly.
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/239
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1]
Signed-off-by: Paulo Miguel Almeida
---
drivers/gpu/drm/radeon/atombios.h| 2 +-
drivers/gpu/drm/radeon/radeon_atombios.c | 12
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios.h
b/drivers/gpu/drm/radeon/atombios.h
index da35a970fcc0..235e59b547a1 100644
--- a/drivers/gpu/drm/radeon/atombios.h
+++ b/drivers/gpu/drm/radeon/atombios.h
@@ -3615,7 +3615,7 @@ typedef struct _ATOM_FAKE_EDID_PATCH_RECORD
{
UCHAR ucRecordType;
UCHAR ucFakeEDIDLength;
- UCHAR ucFakeEDIDString[1];// This actually has ucFakeEdidLength elements.
+ UCHAR ucFakeEDIDString[];// This actually has ucFakeEdidLength elements.
} ATOM_FAKE_EDID_PATCH_RECORD;
typedef struct _ATOM_PANEL_RESOLUTION_PATCH_RECORD
diff --git a/drivers/gpu/drm/radeon/radeon_atombios.c
b/drivers/gpu/drm/radeon/radeon_atombios.c
index 204127bad89c..48de2521f253 100644
--- a/drivers/gpu/drm/radeon/radeon_atombios.c
+++ b/drivers/gpu/drm/radeon/radeon_atombios.c
@@ -1716,7 +1716,7 @@ struct radeon_encoder_atom_dig
*radeon_atombios_get_lvds_info(struct
max((int)EDID_LENGTH,
(int)fake_edid_record->ucFakeEDIDLength);
edid = kmalloc(edid_size,
GFP_KERNEL);
if (edid) {
- memcpy((u8 *)edid, (u8
*)_edid_record->ucFakeEDIDString[0],
+ memcpy((u8 *)edid, (u8
*)fake_edid_record->ucFakeEDIDString,
fake_edid_record->ucFakeEDIDLength);
if
(drm_edid_is_valid(edid)) {
@@ -1725,10 +1725,14 @@ struct radeon_encoder_atom_dig
*radeon_atombios_get_lvds_info(struct
} else
kfree(edid);
}
+
+ record +=
struct_size(fake_edid_record,
+
ucFakeEDIDString,
+
fake_edid_record->ucFakeEDIDLength);
+ } else {
+ /* empty fake edid record must
be 3 bytes long */
+ record +=
sizeof(*fake_edid_record) + 1;
}
- record +=
fake_edid_record->ucFakeEDIDLength ?
-
fake_edid_record->ucFakeEDIDLength + 2 :
-
sizeof(ATOM_FAKE_EDID_PATCH_RECORD);
break;
case LCD_PANEL_RESOLUTION_RECORD_TYPE:
panel_res_record =
(ATOM_PANEL_RESOLUTION_PATCH_RECORD *)record;
--
2.37.3
[PATCH] [next] drm/amdgpu: Replace one-element array with flexible-array member
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 struct _ATOM_FAKE_EDID_PATCH_RECORD and
refactor the rest of the code accordingly.
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
---
drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 10 +++---
drivers/gpu/drm/amd/include/atombios.h | 2 +-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
index 6be9ac2b9c5b..6b5abf1249db 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
@@ -2079,10 +2079,14 @@ amdgpu_atombios_encoder_get_lcd_info(struct
amdgpu_encoder *encoder)
} else
kfree(edid);
}
+
+ record +=
struct_size(fake_edid_record,
+
ucFakeEDIDString,
+
fake_edid_record->ucFakeEDIDLength);
+ } else {
+ /* empty fake edid record must
be 3 bytes long */
+ record +=
sizeof(ATOM_FAKE_EDID_PATCH_RECORD) + 1;
}
- record +=
fake_edid_record->ucFakeEDIDLength ?
-
fake_edid_record->ucFakeEDIDLength + 2 :
-
sizeof(ATOM_FAKE_EDID_PATCH_RECORD);
break;
case LCD_PANEL_RESOLUTION_RECORD_TYPE:
panel_res_record =
(ATOM_PANEL_RESOLUTION_PATCH_RECORD *)record;
diff --git a/drivers/gpu/drm/amd/include/atombios.h
b/drivers/gpu/drm/amd/include/atombios.h
index 15943bc21bc5..b5b1d073f8e2 100644
--- a/drivers/gpu/drm/amd/include/atombios.h
+++ b/drivers/gpu/drm/amd/include/atombios.h
@@ -4107,7 +4107,7 @@ typedef struct _ATOM_FAKE_EDID_PATCH_RECORD
{
UCHAR ucRecordType;
UCHAR ucFakeEDIDLength; // = 128 means EDID length is 128 bytes,
otherwise the EDID length = ucFakeEDIDLength*128
- UCHAR ucFakeEDIDString[1];// This actually has ucFakeEdidLength elements.
+ UCHAR ucFakeEDIDString[]; // This actually has ucFakeEdidLength elements.
} ATOM_FAKE_EDID_PATCH_RECORD;
typedef struct _ATOM_PANEL_RESOLUTION_PATCH_RECORD
--
2.37.3
[PATCH] [next] drm/amdgpu: clean up unused constants, macros and includes
Remove include directives in which header is already included via
another header (atombios.h -> atom.h). Remove unused constants and
macros.
Signed-off-by: Paulo Miguel Almeida
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 ---
drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c | 1 -
2 files changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 0e6ddf05c23c..dc55e60c2e4a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -245,10 +245,8 @@ extern int amdgpu_vcnfw_log;
#define AMDGPU_VM_MAX_NUM_CTX 4096
#define AMDGPU_SG_THRESHOLD(256*1024*1024)
#define AMDGPU_DEFAULT_GTT_SIZE_MB 3072ULL /* 3GB by default */
-#define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS 3000
#define AMDGPU_MAX_USEC_TIMEOUT10 /* 100 ms */
#define AMDGPU_FENCE_JIFFIES_TIMEOUT (HZ / 2)
-#define AMDGPU_DEBUGFS_MAX_COMPONENTS 32
#define AMDGPUFB_CONN_LIMIT4
#define AMDGPU_BIOS_NUM_SCRATCH16
@@ -1227,7 +1225,6 @@ int emu_soc_asic_init(struct amdgpu_device *adev);
#define amdgpu_asic_set_vce_clocks(adev, ev, ec)
(adev)->asic_funcs->set_vce_clocks((adev), (ev), (ec))
#define amdgpu_get_pcie_lanes(adev) (adev)->asic_funcs->get_pcie_lanes((adev))
#define amdgpu_set_pcie_lanes(adev, l)
(adev)->asic_funcs->set_pcie_lanes((adev), (l))
-#define amdgpu_asic_get_gpu_clock_counter(adev)
(adev)->asic_funcs->get_gpu_clock_counter((adev))
#define amdgpu_asic_read_disabled_bios(adev)
(adev)->asic_funcs->read_disabled_bios((adev))
#define amdgpu_asic_read_bios_from_rom(adev, b, l)
(adev)->asic_funcs->read_bios_from_rom((adev), (b), (l))
#define amdgpu_asic_read_register(adev, se, sh, offset,
v)((adev)->asic_funcs->read_register((adev), (se), (sh), (offset), (v)))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
index b81b77a9efa6..0c3448dc4951 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_atomfirmware.c
@@ -26,7 +26,6 @@
#include "atomfirmware.h"
#include "amdgpu_atomfirmware.h"
#include "atom.h"
-#include "atombios.h"
#include "soc15_hw_ip.h"
union firmware_info {
--
2.37.3
Re: [PATCH] [next] amdkfd: remove unused kfd_pm4_headers_diq header file
On Tue, Oct 25, 2022 at 03:48:33PM -0400, Felix Kuehling wrote: > Am 2022-10-25 um 05:12 schrieb Paulo Miguel Almeida: > > kfd_pm4_headers_diq.h header is a leftover from the old H/W debugger > > module support added on commit . That implementation > > was removed after a while and the last file that included that header > > was removed on commit <5bdd3eb253544b1>. > > > > This patch removes the unused header file kfd_pm4_headers_diq.h > > > > Signed-off-by: Paulo Miguel Almeida > > Thank you for this patch and the one that removes struct cdit_header. I am > applying both to our amd-staging-drm-next branch. Thanks! > I'm also fixing up the > prefix of the commit headline to match our usual convention: drm/amdkfd: ... Noted! (and much appreciated too). I will ensure I use the drm/amdkfd prefix from now onwards. Paulo A.
[PATCH] [next] amdkfd: remove unused kfd_pm4_headers_diq header file
kfd_pm4_headers_diq.h header is a leftover from the old H/W debugger
module support added on commit . That implementation
was removed after a while and the last file that included that header
was removed on commit <5bdd3eb253544b1>.
This patch removes the unused header file kfd_pm4_headers_diq.h
Signed-off-by: Paulo Miguel Almeida
---
.../gpu/drm/amd/amdkfd/kfd_pm4_headers_diq.h | 291 --
1 file changed, 291 deletions(-)
delete mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_pm4_headers_diq.h
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_pm4_headers_diq.h
b/drivers/gpu/drm/amd/amdkfd/kfd_pm4_headers_diq.h
deleted file mode 100644
index f9cd28690151..
--- a/drivers/gpu/drm/amd/amdkfd/kfd_pm4_headers_diq.h
+++ /dev/null
@@ -1,291 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 OR MIT */
-/*
- * Copyright 2014-2022 Advanced Micro Devices, Inc.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
- * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
- * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- * OTHER DEALINGS IN THE SOFTWARE.
- *
- */
-
-#ifndef KFD_PM4_HEADERS_DIQ_H_
-#define KFD_PM4_HEADERS_DIQ_H_
-
-/*_INDIRECT_BUFFER */
-
-#ifndef _PM4__INDIRECT_BUFFER_DEFINED
-#define _PM4__INDIRECT_BUFFER_DEFINED
-enum _INDIRECT_BUFFER_cache_policy_enum {
- cache_policy___indirect_buffer__lru = 0,
- cache_policy___indirect_buffer__stream = 1,
- cache_policy___indirect_buffer__bypass = 2
-};
-
-enum {
- IT_INDIRECT_BUFFER_PASID = 0x5C
-};
-
-struct pm4__indirect_buffer_pasid {
- union {
- union PM4_MES_TYPE_3_HEADER header; /* header */
- unsigned int ordinal1;
- };
-
- union {
- struct {
- unsigned int reserved1:2;
- unsigned int ib_base_lo:30;
- } bitfields2;
- unsigned int ordinal2;
- };
-
- union {
- struct {
- unsigned int ib_base_hi:16;
- unsigned int reserved2:16;
- } bitfields3;
- unsigned int ordinal3;
- };
-
- union {
- unsigned int control;
- unsigned int ordinal4;
- };
-
- union {
- struct {
- unsigned int pasid:10;
- unsigned int reserved4:22;
- } bitfields5;
- unsigned int ordinal5;
- };
-
-};
-
-#endif
-
-/*_RELEASE_MEM */
-
-#ifndef _PM4__RELEASE_MEM_DEFINED
-#define _PM4__RELEASE_MEM_DEFINED
-enum _RELEASE_MEM_event_index_enum {
- event_index___release_mem__end_of_pipe = 5,
- event_index___release_mem__shader_done = 6
-};
-
-enum _RELEASE_MEM_cache_policy_enum {
- cache_policy___release_mem__lru = 0,
- cache_policy___release_mem__stream = 1,
- cache_policy___release_mem__bypass = 2
-};
-
-enum _RELEASE_MEM_dst_sel_enum {
- dst_sel___release_mem__memory_controller = 0,
- dst_sel___release_mem__tc_l2 = 1,
- dst_sel___release_mem__queue_write_pointer_register = 2,
- dst_sel___release_mem__queue_write_pointer_poll_mask_bit = 3
-};
-
-enum _RELEASE_MEM_int_sel_enum {
- int_sel___release_mem__none = 0,
- int_sel___release_mem__send_interrupt_only = 1,
- int_sel___release_mem__send_interrupt_after_write_confirm = 2,
- int_sel___release_mem__send_data_after_write_confirm = 3
-};
-
-enum _RELEASE_MEM_data_sel_enum {
- data_sel___release_mem__none = 0,
- data_sel___release_mem__send_32_bit_low = 1,
- data_sel___release_mem__send_64_bit_data = 2,
- data_sel___release_mem__send_gpu_clock_counter = 3,
- data_sel___release_mem__send_cp_perfcounter_hi_lo = 4,
- data_sel___release_mem__store_gds_data_to_memory = 5
-};
-
-struct pm4__release_mem {
- union {
- union PM4_MES_TYPE_3_HEADER header; /*header */
- unsigned int ordinal1;
[PATCH] [next] amdkfd: remove unused struct cdit_header
struct cdit_header was never used across any of the amd drivers nor
this is exposed to UAPI so it can be removed.
This patch removes struct cdit_header and refactor code accordingly
Signed-off-by: Paulo Miguel Almeida
---
At some point, removing this structure was suggested during a review for
a different patch but the clean up patch was never sent.
https://lore.kernel.org/lkml/0eeaa0b7-7ae6-c244-cfcd-86333d8f9...@amd.com/
---
drivers/gpu/drm/amd/amdkfd/kfd_crat.h | 24 +---
1 file changed, 1 insertion(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
index 482ba84a728d..22893ff7b9a1 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.h
@@ -29,11 +29,10 @@
#pragma pack(1)
/*
- * 4CC signature values for the CRAT and CDIT ACPI tables
+ * 4CC signature value for the CRAT ACPI table
*/
#define CRAT_SIGNATURE "CRAT"
-#define CDIT_SIGNATURE "CDIT"
/*
* Component Resource Association Table (CRAT)
@@ -292,27 +291,6 @@ struct crat_subtype_generic {
uint32_tflags;
};
-/*
- * Component Locality Distance Information Table (CDIT)
- */
-#define CDIT_OEMID_LENGTH 6
-#define CDIT_OEMTABLEID_LENGTH 8
-
-struct cdit_header {
- uint32_tsignature;
- uint32_tlength;
- uint8_t revision;
- uint8_t checksum;
- uint8_t oem_id[CDIT_OEMID_LENGTH];
- uint8_t oem_table_id[CDIT_OEMTABLEID_LENGTH];
- uint32_toem_revision;
- uint32_tcreator_id;
- uint32_tcreator_revision;
- uint32_ttotal_entries;
- uint16_tnum_domains;
- uint8_t entry[1];
-};
-
#pragma pack()
struct kfd_dev;
--
2.37.3
