[PATCH] ata: Many CPUs cause seabios not to find boot disk
When CPUs are 76, mptable_setup will alloc much fseg memory,and init_atadrive allocation fails. when CPUs are 86, mptable_setup allocation is too much to be met, and init_atadrive allocs success. So when CPUs are 76~85, seabios cannot find boot disk because init_atadrive allocation fails. Set a reserved area from fsegzone for atadrive allocation, and the fsegzone space also increases by the reserved area space. Signed-off-by: Huaitong Han <huaitong.han@intel.com> diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py index b976fb0..a7bc5b1 100755 --- a/scripts/layoutrom.py +++ b/scripts/layoutrom.py @@ -62,7 +62,7 @@ BUILD_BIOS_SIZE = 0x10000 BUILD_ROM_START = 0xc0000 BUILD_LOWRAM_END = 0xa0000 # Space to reserve in f-segment for dynamic allocations -BUILD_MIN_BIOSTABLE = 2048 +BUILD_MIN_BIOSTABLE = 2304 # Layout the 16bit code. This ensures sections with fixed offset # requirements are placed in the correct location. It also places the diff --git a/src/hw/ata.c b/src/hw/ata.c index fbbbbc1..12bd01f 100644 --- a/src/hw/ata.c +++ b/src/hw/ata.c @@ -710,7 +710,7 @@ ata_extract_model(char *model, u32 size, u16 *buffer) static struct atadrive_s * init_atadrive(struct atadrive_s *dummy, u16 *buffer) { - struct atadrive_s *adrive = malloc_fseg(sizeof(*adrive)); + struct atadrive_s *adrive = malloc_reserved_fseg(sizeof(*adrive)); if (!adrive) { warn_noalloc(); return NULL; @@ -920,7 +920,7 @@ init_controller(struct pci_device *pci, int chanid, int irq , u32 port1, u32 port2, u32 master) { static int ataid = 0; - struct ata_channel_s *chan_gf = malloc_fseg(sizeof(*chan_gf)); + struct ata_channel_s *chan_gf = malloc_reserved_fseg(sizeof(*chan_gf)); if (!chan_gf) { warn_noalloc(); return; diff --git a/src/malloc.c b/src/malloc.c index c4cb171..16114f5 100644 --- a/src/malloc.c +++ b/src/malloc.c @@ -296,6 +296,19 @@ malloc_getspace(struct zone_s *zone) return maxspace - reserve; } +u32 malloc_get_zone_space(struct zone_s *zone) +{ + // XXX - results not reliable when CONFIG_THREAD_OPTIONROMS + u32 totalspace = 0; + struct allocinfo_s *info; + hlist_for_each_entry(info, &zone->head, node) { + u32 space = info->allocend - info->dataend; + totalspace += space; + } + + return totalspace; +} + // Set a handle associated with an allocation. void malloc_sethandle(void *data, u32 handle) diff --git a/src/malloc.h b/src/malloc.h index 2bcb5bf..c70a468 100644 --- a/src/malloc.h +++ b/src/malloc.h @@ -20,6 +20,10 @@ int _free(void *data); u32 malloc_getspace(struct zone_s *zone); void malloc_sethandle(void *data, u32 handle); void *malloc_findhandle(u32 handle); +u32 malloc_get_zone_space(struct zone_s *zone); + +//Set a reserved zone to prevent key memory allocation failure +#define RESERVED_FSEG_ZONE 256 #define MALLOC_DEFAULT_HANDLE 0xFFFFFFFF // Minimum alignment of malloc'd memory @@ -32,6 +36,13 @@ static inline void *malloc_high(u32 size) { return _malloc(&ZoneHigh, size, MALLOC_MIN_ALIGN); } static inline void *malloc_fseg(u32 size) { + u32 allspace = malloc_get_zone_space(&ZoneFSeg); + if (allspace < (size + RESERVED_FSEG_ZONE)) + return NULL; + + return _malloc(&ZoneFSeg, size, MALLOC_MIN_ALIGN); +} +static inline void *malloc_reserved_fseg(u32 size){ return _malloc(&ZoneFSeg, size, MALLOC_MIN_ALIGN); } static inline void *malloc_tmplow(u32 size) { -- 1.9.1
On Mon, Aug 03, 2015 at 01:59:25PM +0800, Huaitong Han wrote:
When CPUs are 76, mptable_setup will alloc much fseg memory,and init_atadrive allocation fails. when CPUs are 86, mptable_setup allocation is too much to be met, and init_atadrive allocs success. So when CPUs are 76~85, seabios cannot find boot disk because init_atadrive allocation fails.
Set a reserved area from fsegzone for atadrive allocation, and the fsegzone space also increases by the reserved area space.
If I understand your patch, it seems to be trying to force large mptable allocations to fail while still allowing other f-seg allocations. I think a simpler approach would be to just limit the mptable allocation and leave the malloc implementation unchanged. See patch below. -Kevin From: Kevin O'Connor <kevin@koconnor.net> Date: Mon, 3 Aug 2015 10:16:39 -0400 Subject: [PATCH] mptable: Don't create mptable if it is very large Very large mptable structures can fill up the space in the f-segment and cause other important f-segment allocations to fail. Limit the maximum size of the mptable to prevent this. On QEMU, with the current maximum size of 600 bytes, the mptable will not be created in configurations of ~20 cpus or more. The mptable is rarely used in modern OSes so this should not be a problem. Reported-by: Huaitong Han <huaitong.han@intel.com> Signed-off-by: Kevin O'Connor <kevin@koconnor.net> --- src/config.h | 2 ++ src/fw/biostables.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/config.h b/src/config.h index 4bfebe8..6c47f16 100644 --- a/src/config.h +++ b/src/config.h @@ -22,6 +22,8 @@ #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 +// Maximum number of bytes the mptable may be and still be copied to f-segment +#define BUILD_MAX_MPTABLE_FSEG 600 #define BUILD_MODEL_ID 0xFC #define BUILD_SUBMODEL_ID 0x00 diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 450aca2..cf714e8 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -54,6 +54,8 @@ copy_mptable(void *pos) return; u32 length = p->length * 16; u16 mpclength = ((struct mptable_config_s *)p->physaddr)->length; + if (length + mpclength > BUILD_MAX_MPTABLE_FSEG) + return; // Allocate final memory location. (In theory the config // structure can go in high memory, but Linux kernels before // v2.6.30 crash with that.)
On Mon, 2015-08-03 at 10:32 -0400, Kevin O'Connor wrote:
If I understand your patch, it seems to be trying to force large mptable allocations to fail while still allowing other f-seg allocations. I think a simpler approach would be to just limit the mptable allocation and leave the malloc implementation unchanged. See patch below.
Yes, it's a better way to solve problem.
u16 mpclength = ((struct mptable_config_s *)p->physaddr)->length; + if (length + mpclength > BUILD_MAX_MPTABLE_FSEG)
it's better that warning dprintf is added before return.
+ return;
On Mon, Aug 03, 2015 at 03:49:14PM +0000, Han, Huaitong wrote:
On Mon, 2015-08-03 at 10:32 -0400, Kevin O'Connor wrote:
If I understand your patch, it seems to be trying to force large mptable allocations to fail while still allowing other f-seg allocations. I think a simpler approach would be to just limit the mptable allocation and leave the malloc implementation unchanged. See patch below.
Yes, it's a better way to solve problem.
u16 mpclength = ((struct mptable_config_s *)p->physaddr)->length; + if (length + mpclength > BUILD_MAX_MPTABLE_FSEG)
it's better that warning dprintf is added before return.
Good point. New patch below. -Kevin
From 194b750f3735cb8b400ead1cba4ecd2e6e16d5ed Mon Sep 17 00:00:00 2001 From: Kevin O'Connor <kevin@koconnor.net> Date: Mon, 3 Aug 2015 10:16:39 -0400 Subject: [PATCH] mptable: Don't create mptable if it is very large
Very large mptable structures can fill up the space in the f-segment and cause other important f-segment allocations to fail. Limit the maximum size of the mptable to prevent this. On QEMU, with the current maximum size of 600 bytes, the mptable will not be created in configurations of ~20 cpus or more. The mptable is rarely used in modern OSes so this should not be a problem. Reported-by: Huaitong Han <huaitong.han@intel.com> Signed-off-by: Kevin O'Connor <kevin@koconnor.net> --- src/config.h | 2 ++ src/fw/biostables.c | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/src/config.h b/src/config.h index 4bfebe8..6c47f16 100644 --- a/src/config.h +++ b/src/config.h @@ -22,6 +22,8 @@ #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 +// Maximum number of bytes the mptable may be and still be copied to f-segment +#define BUILD_MAX_MPTABLE_FSEG 600 #define BUILD_MODEL_ID 0xFC #define BUILD_SUBMODEL_ID 0x00 diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 450aca2..71a1a0d 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -54,6 +54,11 @@ copy_mptable(void *pos) return; u32 length = p->length * 16; u16 mpclength = ((struct mptable_config_s *)p->physaddr)->length; + if (length + mpclength > BUILD_MAX_MPTABLE_FSEG) { + dprintf(1, "Skipping MPTABLE copy due to large size (%d bytes)\n" + , length + mpclength); + return; + } // Allocate final memory location. (In theory the config // structure can go in high memory, but Linux kernels before // v2.6.30 crash with that.) -- 1.9.3
It's OK. On Mon, 2015-08-03 at 20:42 -0400, Kevin O'Connor wrote:
On Mon, Aug 03, 2015 at 03:49:14PM +0000, Han, Huaitong wrote:
On Mon, 2015-08-03 at 10:32 -0400, Kevin O'Connor wrote:
If I understand your patch, it seems to be trying to force large mptable allocations to fail while still allowing other f-seg allocations. I think a simpler approach would be to just limit the mptable allocation and leave the malloc implementation unchanged. See patch below.
Yes, it's a better way to solve problem.
u16 mpclength = ((struct mptable_config_s *)p->physaddr)->length; + if (length + mpclength > BUILD_MAX_MPTABLE_FSEG)
it's better that warning dprintf is added before return.
Good point. New patch below.
-Kevin
From 194b750f3735cb8b400ead1cba4ecd2e6e16d5ed Mon Sep 17 00:00:00 2001 From: Kevin O'Connor <kevin@koconnor.net> Date: Mon, 3 Aug 2015 10:16:39 -0400 Subject: [PATCH] mptable: Don't create mptable if it is very large
Very large mptable structures can fill up the space in the f-segment and cause other important f-segment allocations to fail. Limit the maximum size of the mptable to prevent this.
On QEMU, with the current maximum size of 600 bytes, the mptable will not be created in configurations of ~20 cpus or more. The mptable is rarely used in modern OSes so this should not be a problem.
Reported-by: Huaitong Han <huaitong.han@intel.com> Signed-off-by: Kevin O'Connor <kevin@koconnor.net> --- src/config.h | 2 ++ src/fw/biostables.c | 5 +++++ 2 files changed, 7 insertions(+)
diff --git a/src/config.h b/src/config.h index 4bfebe8..6c47f16 100644 --- a/src/config.h +++ b/src/config.h @@ -22,6 +22,8 @@ #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 +// Maximum number of bytes the mptable may be and still be copied to f-segment +#define BUILD_MAX_MPTABLE_FSEG 600
#define BUILD_MODEL_ID 0xFC #define BUILD_SUBMODEL_ID 0x00 diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 450aca2..71a1a0d 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -54,6 +54,11 @@ copy_mptable(void *pos) return; u32 length = p->length * 16; u16 mpclength = ((struct mptable_config_s *)p->physaddr)->length; + if (length + mpclength > BUILD_MAX_MPTABLE_FSEG) { + dprintf(1, "Skipping MPTABLE copy due to large size (%d bytes)\n" + , length + mpclength); + return; + } // Allocate final memory location. (In theory the config // structure can go in high memory, but Linux kernels before // v2.6.30 crash with that.)
On Tue, Aug 04, 2015 at 02:11:45AM +0000, Han, Huaitong wrote:
It's OK.
Thanks - I pushed the patch. -Kevin
On Mon, 2015-08-03 at 20:42 -0400, Kevin O'Connor wrote:
On Mon, Aug 03, 2015 at 03:49:14PM +0000, Han, Huaitong wrote:
On Mon, 2015-08-03 at 10:32 -0400, Kevin O'Connor wrote:
If I understand your patch, it seems to be trying to force large mptable allocations to fail while still allowing other f-seg allocations. I think a simpler approach would be to just limit the mptable allocation and leave the malloc implementation unchanged. See patch below.
Yes, it's a better way to solve problem.
u16 mpclength = ((struct mptable_config_s *)p->physaddr)->length; + if (length + mpclength > BUILD_MAX_MPTABLE_FSEG)
it's better that warning dprintf is added before return.
Good point. New patch below.
-Kevin
From 194b750f3735cb8b400ead1cba4ecd2e6e16d5ed Mon Sep 17 00:00:00 2001 From: Kevin O'Connor <kevin@koconnor.net> Date: Mon, 3 Aug 2015 10:16:39 -0400 Subject: [PATCH] mptable: Don't create mptable if it is very large
Very large mptable structures can fill up the space in the f-segment and cause other important f-segment allocations to fail. Limit the maximum size of the mptable to prevent this.
On QEMU, with the current maximum size of 600 bytes, the mptable will not be created in configurations of ~20 cpus or more. The mptable is rarely used in modern OSes so this should not be a problem.
Reported-by: Huaitong Han <huaitong.han@intel.com> Signed-off-by: Kevin O'Connor <kevin@koconnor.net> --- src/config.h | 2 ++ src/fw/biostables.c | 5 +++++ 2 files changed, 7 insertions(+)
diff --git a/src/config.h b/src/config.h index 4bfebe8..6c47f16 100644 --- a/src/config.h +++ b/src/config.h @@ -22,6 +22,8 @@ #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 +// Maximum number of bytes the mptable may be and still be copied to f-segment +#define BUILD_MAX_MPTABLE_FSEG 600
#define BUILD_MODEL_ID 0xFC #define BUILD_SUBMODEL_ID 0x00 diff --git a/src/fw/biostables.c b/src/fw/biostables.c index 450aca2..71a1a0d 100644 --- a/src/fw/biostables.c +++ b/src/fw/biostables.c @@ -54,6 +54,11 @@ copy_mptable(void *pos) return; u32 length = p->length * 16; u16 mpclength = ((struct mptable_config_s *)p->physaddr)->length; + if (length + mpclength > BUILD_MAX_MPTABLE_FSEG) { + dprintf(1, "Skipping MPTABLE copy due to large size (%d bytes)\n" + , length + mpclength); + return; + } // Allocate final memory location. (In theory the config // structure can go in high memory, but Linux kernels before // v2.6.30 crash with that.)
participants (3)
-
Han, Huaitong -
Huaitong Han
-
Kevin O'Connor