Skip to content

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

Merged
merged 10 commits into from
Jan 30, 2018
Merged
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ mbed-host-tests>=1.1.2
mbed-greentea>=0.2.24
beautifulsoup4>=4
fuzzywuzzy>=0.11
pyelftools>=0.24
27 changes: 25 additions & 2 deletions tools/arm_pack_manager/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import warnings
from distutils.version import LooseVersion

from tools.flash_algo import PackFlashAlgo

warnings.filterwarnings("ignore")

from fuzzywuzzy import process
Expand Down Expand Up @@ -145,12 +147,33 @@ def get_urls(self):
for pdsc in root_data.find_all("pdsc")]
return self.urls

def _get_sectors(self, device):
"""Extract sector sizes from device FLM algorithm

Will return None if there is no algorithm, pdsc URL formatted in correctly

:return: A list tuples of sector start and size
:rtype: [list]
"""
try:
pack = self.pack_from_cache(device)
algo_itr = (pack.open(path) for path in device['algorithm'].keys())
algo_bin = algo_itr.next()
flm_file = algo_bin.read()
return PackFlashAlgo(flm_file).sector_sizes
except Exception:
return None

def _extract_dict(self, device, filename, pack) :
to_ret = dict(pdsc_file=filename, pack_file=pack)
try : to_ret["memory"] = dict([(m["id"], dict(start=m["start"],
size=m["size"]))
for m in device("memory")])
except (KeyError, TypeError, IndexError) as e : pass
except (KeyError, TypeError, IndexError) as e:
try : to_ret["memory"] = dict([(m["name"], dict(start=m["start"],
size=m["size"]))
for m in device("memory")])
except (KeyError, TypeError, IndexError) as e : pass
try: algorithms = device("algorithm")
except:
try: algorithms = device.parent("algorithm")
Expand Down Expand Up @@ -219,6 +242,7 @@ def _extract_dict(self, device, filename, pack) :
del to_ret["compile"]

to_ret['debug-interface'] = []
to_ret['sectors'] = self._get_sectors(to_ret)

return to_ret

Expand Down Expand Up @@ -445,4 +469,3 @@ def cache_and_parse(self, url) :
"""
self.cache_file(url)
return self.pdsc_from_cache(url)

2 changes: 1 addition & 1 deletion tools/arm_pack_manager/index.json

Large diffs are not rendered by default.

69 changes: 57 additions & 12 deletions tools/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down Expand Up @@ -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,
Expand All @@ -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'])
Expand All @@ -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
Copy link
Contributor

@theotherjimmy theotherjimmy Jan 22, 2018

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, then start will be 0, and incorrect if rom_start is not 0.

Alternatively, You are mixing types for start. It begins life as an offset, and changes to an address in the if 'target.bootloader_img' in target_overrides: branch. This leads to start being either an offset or address in this block, which assumes that start is an address.

Copy link
Contributor Author

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.

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this just rounds up, could it return start here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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):
Expand Down
File renamed without changes.
4 changes: 2 additions & 2 deletions tools/flash_algo/extract.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import os
import argparse
from os.path import join, abspath, dirname
from flash_algo import PackFlashAlgo
from tools.flash_algo import PackFlashAlgo

# Be sure that the tools directory is in the search path
ROOT = abspath(join(dirname(__file__), "..", ".."))
Expand Down Expand Up @@ -98,7 +98,7 @@ def filter_algos(dev, algos):
except ValueError:
return algos

matching_algos = [algo for algo in algos if
matching_algos = [algo for algo in algos if
algo.flash_start == start and algo.flash_size == size]
return matching_algos if len(matching_algos) == 1 else algos

Expand Down