There are plenty of coreboot platforms whose MPTABLE size is just slightly larger than the current uneven limit of 600 bytes, which prevents these important tables from being copied. For example, G505S has 628 bytes and A88XM-E has 740 bytes. I propose 768 bytes as a new saner default for MPTABLE size, to replace the current uneven limit of 600. SMBIOS size should be bumped the similar way as well.
Signed-off-by: Mike Banon <mikebdp2 at gmail.com> --- diff --git a/src/config.h b/src/config.h index 93c8dbc..71333f8 100644 --- a/src/config.h +++ b/src/config.h @@ -21,9 +21,9 @@ // Largest supported externaly facing drive id #define BUILD_MAX_EXTDRIVE 16 // Number of bytes the smbios may be and still live in the f-segment -#define BUILD_MAX_SMBIOS_FSEG 600 +#define BUILD_MAX_SMBIOS_FSEG 768 // Maximum number of bytes the mptable may be and still be copied to f-segment -#define BUILD_MAX_MPTABLE_FSEG 600 +#define BUILD_MAX_MPTABLE_FSEG 768
#define BUILD_MODEL_ID 0xFC #define BUILD_SUBMODEL_ID 0x00
On Tue, Mar 02, 2021 at 11:21:27PM +0300, Mike Banon wrote:
There are plenty of coreboot platforms whose MPTABLE size is just slightly larger than the current uneven limit of 600 bytes, which prevents these important tables from being copied. For example, G505S has 628 bytes and A88XM-E has 740 bytes. I propose 768 bytes as a new saner default for MPTABLE size, to replace the current uneven limit of 600. SMBIOS size should be bumped the similar way as well.
This doesn't look correct to me. There is only a limited amount of space in the f-segment, and it shouldn't be necessary to store large tables there. All recent OSes should support the SMBIOS in high-memory, and recent OSes don't need the mptable at all.
The concern with increasing the size is that it could cause allocation failures for other critical information that does need to be stored in the f-segment (like drive layout).
-Kevin
According to my calculations, my proposed 768 default - which is just slightly larger than a previous 600 - should also allow having ~20 CPUs, although the systems with 20 CPUs are significantly less popular than i.e. 16. Also, the uneven (unrelated to the power of 2's) values of 600 weren't really justified in the first place: 768 decimal is 0x300, while 600 is a weird 0x258.
On Tue, Mar 9, 2021 at 2:28 AM Kevin O'Connor kevin@koconnor.net wrote:
On Tue, Mar 02, 2021 at 11:21:27PM +0300, Mike Banon wrote:
There are plenty of coreboot platforms whose MPTABLE size is just slightly larger than the current uneven limit of 600 bytes, which prevents these important tables from being copied. For example, G505S has 628 bytes and A88XM-E has 740 bytes. I propose 768 bytes as a new saner default for MPTABLE size, to replace the current uneven limit of 600. SMBIOS size should be bumped the similar way as well.
This doesn't look correct to me. There is only a limited amount of space in the f-segment, and it shouldn't be necessary to store large tables there. All recent OSes should support the SMBIOS in high-memory, and recent OSes don't need the mptable at all.
The concern with increasing the size is that it could cause allocation failures for other critical information that does need to be stored in the f-segment (like drive layout).
-Kevin
Good day Kevin, please tell if there's anything I can add for us to proceed with a review of this patch.
On Tue, Mar 9, 2021 at 12:54 PM Mike Banon mikebdp2@gmail.com wrote:
According to my calculations, my proposed 768 default - which is just slightly larger than a previous 600 - should also allow having ~20 CPUs, although the systems with 20 CPUs are significantly less popular than i.e. 16. Also, the uneven (unrelated to the power of 2's) values of 600 weren't really justified in the first place: 768 decimal is 0x300, while 600 is a weird 0x258.
On Tue, Mar 9, 2021 at 2:28 AM Kevin O'Connor kevin@koconnor.net wrote:
On Tue, Mar 02, 2021 at 11:21:27PM +0300, Mike Banon wrote:
There are plenty of coreboot platforms whose MPTABLE size is just slightly larger than the current uneven limit of 600 bytes, which prevents these important tables from being copied. For example, G505S has 628 bytes and A88XM-E has 740 bytes. I propose 768 bytes as a new saner default for MPTABLE size, to replace the current uneven limit of 600. SMBIOS size should be bumped the similar way as well.
This doesn't look correct to me. There is only a limited amount of space in the f-segment, and it shouldn't be necessary to store large tables there. All recent OSes should support the SMBIOS in high-memory, and recent OSes don't need the mptable at all.
The concern with increasing the size is that it could cause allocation failures for other critical information that does need to be stored in the f-segment (like drive layout).
-Kevin
-- Best regards, Mike Banon Open Source Community Manager of 3mdeb - https://3mdeb.com/
On Thu, Apr 01, 2021 at 05:44:06PM +0300, Mike Banon wrote:
Good day Kevin, please tell if there's anything I can add for us to proceed with a review of this patch.
On Tue, Mar 9, 2021 at 12:54 PM Mike Banon mikebdp2@gmail.com wrote:
According to my calculations, my proposed 768 default - which is just slightly larger than a previous 600 - should also allow having ~20 CPUs, although the systems with 20 CPUs are significantly less popular than i.e. 16. Also, the uneven (unrelated to the power of 2's) values of 600 weren't really justified in the first place: 768 decimal is 0x300, while 600 is a weird 0x258.
On Tue, Mar 9, 2021 at 2:28 AM Kevin O'Connor kevin@koconnor.net wrote:
On Tue, Mar 02, 2021 at 11:21:27PM +0300, Mike Banon wrote:
There are plenty of coreboot platforms whose MPTABLE size is just slightly larger than the current uneven limit of 600 bytes, which prevents these important tables from being copied. For example, G505S has 628 bytes and A88XM-E has 740 bytes. I propose 768 bytes as a new saner default for MPTABLE size, to replace the current uneven limit of 600. SMBIOS size should be bumped the similar way as well.
This doesn't look correct to me. There is only a limited amount of space in the f-segment, and it shouldn't be necessary to store large tables there. All recent OSes should support the SMBIOS in high-memory, and recent OSes don't need the mptable at all.
The concern with increasing the size is that it could cause allocation failures for other critical information that does need to be stored in the f-segment (like drive layout).
-Kevin
I don't agree with the patch due to the reasons described above.
-Kevin
Is my reasoning above invalid? If yes, please clarify what is wrong:
According to my calculations, my proposed 768 default - which is just slightly larger than a previous 600 - should also allow having ~20 CPUs, although the systems with 20 CPUs are significantly less popular than i.e. 16. Also, the uneven (unrelated to the power of 2's) values of 600 weren't really justified in the first place: 768 decimal is 0x300, while 600 is a weird 0x258.
On Sat, Apr 3, 2021 at 3:49 AM Kevin O'Connor kevin@koconnor.net wrote:
On Thu, Apr 01, 2021 at 05:44:06PM +0300, Mike Banon wrote:
Good day Kevin, please tell if there's anything I can add for us to proceed with a review of this patch.
On Tue, Mar 9, 2021 at 12:54 PM Mike Banon mikebdp2@gmail.com wrote:
According to my calculations, my proposed 768 default - which is just slightly larger than a previous 600 - should also allow having ~20 CPUs, although the systems with 20 CPUs are significantly less popular than i.e. 16. Also, the uneven (unrelated to the power of 2's) values of 600 weren't really justified in the first place: 768 decimal is 0x300, while 600 is a weird 0x258.
On Tue, Mar 9, 2021 at 2:28 AM Kevin O'Connor kevin@koconnor.net wrote:
On Tue, Mar 02, 2021 at 11:21:27PM +0300, Mike Banon wrote:
There are plenty of coreboot platforms whose MPTABLE size is just slightly larger than the current uneven limit of 600 bytes, which prevents these important tables from being copied. For example, G505S has 628 bytes and A88XM-E has 740 bytes. I propose 768 bytes as a new saner default for MPTABLE size, to replace the current uneven limit of 600. SMBIOS size should be bumped the similar way as well.
This doesn't look correct to me. There is only a limited amount of space in the f-segment, and it shouldn't be necessary to store large tables there. All recent OSes should support the SMBIOS in high-memory, and recent OSes don't need the mptable at all.
The concern with increasing the size is that it could cause allocation failures for other critical information that does need to be stored in the f-segment (like drive layout).
-Kevin
I don't agree with the patch due to the reasons described above.
-Kevin
-
Kevin O'Connor -
Mike Banon