-
Notifications
You must be signed in to change notification settings - Fork 3k
Enforce sector alignmnent for managed bootloader builds #5818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5009fdb
a219508
c74de99
86d4ce1
41dac7a
6f39d5a
785dd10
623b171
58330fb
695dc00
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,3 +11,4 @@ mbed-host-tests>=1.1.2 | |
mbed-greentea>=0.2.24 | ||
beautifulsoup4>=4 | ||
fuzzywuzzy>=0.11 | ||
pyelftools>=0.24 |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -498,6 +498,20 @@ def has_regions(self): | |
else: | ||
return False | ||
|
||
@property | ||
def sectors(self): | ||
"""Return a list of tuples of sector start,size""" | ||
cache = Cache(False, False) | ||
if self.target.device_name not in cache.index: | ||
raise ConfigException("Bootloader not supported on this target: " | ||
"targets.json `device_name` not found in " | ||
"arm_pack_manager index.") | ||
cmsis_part = cache.index[self.target.device_name] | ||
sectors = cmsis_part['sectors'] | ||
if sectors: | ||
return sectors | ||
raise ConfigException("No sector info available") | ||
|
||
@property | ||
def regions(self): | ||
"""Generate a list of regions from the config""" | ||
|
@@ -526,12 +540,16 @@ def regions(self): | |
rom_size = int(cmsis_part['memory']['IROM1']['size'], 0) | ||
rom_start = int(cmsis_part['memory']['IROM1']['start'], 0) | ||
except KeyError: | ||
raise ConfigException("Not enough information in CMSIS packs to " | ||
"build a bootloader project") | ||
try: | ||
rom_size = int(cmsis_part['memory']['PROGRAM_FLASH']['size'], 0) | ||
rom_start = int(cmsis_part['memory']['PROGRAM_FLASH']['start'], 0) | ||
except KeyError: | ||
raise ConfigException("Not enough information in CMSIS packs to " | ||
"build a bootloader project") | ||
if ('target.bootloader_img' in target_overrides or | ||
'target.restrict_size' in target_overrides): | ||
return self._generate_booloader_build(target_overrides, | ||
rom_start, rom_size) | ||
return self._generate_bootloader_build(target_overrides, | ||
rom_start, rom_size) | ||
elif ('target.mbed_app_start' in target_overrides or | ||
'target.mbed_app_size' in target_overrides): | ||
return self._generate_linker_overrides(target_overrides, | ||
|
@@ -540,8 +558,8 @@ def regions(self): | |
raise ConfigException( | ||
"Bootloader build requested but no bootlader configuration") | ||
|
||
def _generate_booloader_build(self, target_overrides, rom_start, rom_size): | ||
start = 0 | ||
def _generate_bootloader_build(self, target_overrides, rom_start, rom_size): | ||
start = rom_start | ||
if 'target.bootloader_img' in target_overrides: | ||
basedir = abspath(dirname(self.app_config_location)) | ||
filename = join(basedir, target_overrides['target.bootloader_img']) | ||
|
@@ -552,21 +570,48 @@ def _generate_booloader_build(self, target_overrides, rom_start, rom_size): | |
raise ConfigException("bootloader executable does not " | ||
"start at 0x%x" % rom_start) | ||
part_size = (part.maxaddr() - part.minaddr()) + 1 | ||
yield Region("bootloader", rom_start + start, part_size, False, | ||
part_size = Config._align_ceiling(rom_start + part_size, self.sectors) - rom_start | ||
yield Region("bootloader", rom_start, part_size, False, | ||
filename) | ||
start += part_size | ||
start = rom_start + part_size | ||
if 'target.restrict_size' in target_overrides: | ||
new_size = int(target_overrides['target.restrict_size'], 0) | ||
yield Region("application", rom_start + start, new_size, True, None) | ||
new_size = Config._align_floor(start + new_size, self.sectors) - start | ||
yield Region("application", start, new_size, True, None) | ||
start += new_size | ||
yield Region("post_application", rom_start +start, rom_size - start, | ||
yield Region("post_application", start, rom_size - start, | ||
False, None) | ||
else: | ||
yield Region("application", rom_start + start, rom_size - start, | ||
yield Region("application", start, rom_size - start, | ||
True, None) | ||
if start > rom_size: | ||
if start > rom_start + rom_size: | ||
raise ConfigException("Not enough memory on device to fit all " | ||
"application regions") | ||
|
||
@staticmethod | ||
def _find_sector(address, sectors): | ||
target_size = -1 | ||
target_start = -1 | ||
for (start, size) in sectors: | ||
if address < start: | ||
break | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this just rounds up, could it return start here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. The list of sectors is not a list of all possible sectors. It is a list of tuples with (start of block, size of sectors in that block). Maybe I should have made that more clear. |
||
target_start = start | ||
target_size = size | ||
if (target_size < 0): | ||
raise ConfigException("No valid sector found") | ||
return target_start, target_size | ||
|
||
@staticmethod | ||
def _align_floor(address, sectors): | ||
target_start, target_size = Config._find_sector(address, sectors) | ||
sector_num = (address - target_start) // target_size | ||
return target_start + (sector_num * target_size) | ||
|
||
@staticmethod | ||
def _align_ceiling(address, sectors): | ||
target_start, target_size = Config._find_sector(address, sectors) | ||
sector_num = ((address - target_start) + target_size - 1) // target_size | ||
return target_start + (sector_num * target_size) | ||
|
||
@property | ||
def report(self): | ||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
bootloader_img
is not present, thenstart
will be0
, and incorrect ifrom_start
is not0
.Alternatively, You are mixing types for
start
. It begins life as an offset, and changes to an address in theif 'target.bootloader_img' in target_overrides:
branch. This leads tostart
being either an offset or address in this block, which assumes thatstart
is an address.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@theotherjimmy, the latest commit should fix this. Good catch.