-
Couldn't load subscription status.
- Fork 5
Add Script to generate or update FvUpdate.xml file with Firmware entries #25
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
base: main
Are you sure you want to change the base?
Conversation
|
Can you add a commit message body explaining why the changes? |
|
@quicthogaru Thank you for working on this! ODM/OEMs may customize the reference There are customers who are preparing to ship production devices on QLI 1.4/1.5/1.6 (which supports EFI capsule updates) but the configurations for those versions of QLI are different than in the reference repo: We've also seen at least 1 ODM add a custom partition in LUN4 to hold serial # / MAC information and for this reason they have overridden the default Currently, there is no way to point to a customized file vs. the online reference file in the Perhaps we could have a choice between running: Thank you for this consideration. |
|
@quicthogaru regarding Ricardo's comment asking a commit description. I always like to think of the description as a note to someone who has no knowledge of the project. They would need the context of the patch explained in detail. Using this logic, my description for your commit would look something like this: |
|
|
||
| def main(): | ||
| parser = argparse.ArgumentParser(description="Generate FvUpdate.xml from partitions.conf") | ||
| parser.add_argument("-S", "--StorageType", choices=["UFS", "EMMC"], required=True, help="Specify storage type: UFS or EMMC") |
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.
Kind of off-topic, but are we missing some EMMC partition.conf files in the qcom-ptool repo? In theory, those would only have LUN 0 and 1.
Example for QLI 1.5: https://github.com/qualcomm-linux/meta-qcom-hwe/blob/scarthgap/recipes-devtools/partition-utils/qcom-partition-confs/emmc/qcm6490-partitions.conf
RB3 Gen2 dir in the ptool repo:
https://github.com/qualcomm-linux/qcom-ptool/tree/main/platforms/qcs6490-rb3gen2
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.
Yes, the qcom-ptool is missing an EMMC-specific partitions.conf.
Since GUIDs are the same for both boot EMMC and UFS, the tool uses the same identifiers.
Based on the -S argument, the tool generates FvUpdate.xml using:
- Logical partition names for EMMC
- Partition names/GUIDs from UFS partitions.conf
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.
We just merged support for having both emmc and ufs partitions.conf files, and it is currently only available for qcs615-adp-air, but we should had for others soon.
e.g. https://github.com/qualcomm-linux/qcom-ptool/tree/main/platforms/qcs615-adp-air
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.
Thanks for the info @ricardosalveti .
We can update the script once it is available for all targets.
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.
JFYI, Updated the script with the below usage.
Usage: UpdateFvXml.py [-h] (-T TARGET & -S {UFS,EMMC}) | [-F PARTITIONS_CONF]
python3 UpdateFvXml.py -S -T
or
python3 UpdateFvXml.py -F
As currently we don't have emmc directories for all targets, going to use ufs partitions in case of -T & -S.
For -F, we directly fetch the storage type from provided conf file.
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.
I didn't do an in detail review yet, as I'm hoping we can add a parameter which points to a local partition.conf file, however, I found a typo and make a comment on the storage selection parameter.
ba3d8d3 to
f145bda
Compare
The current FvUpdate.xml was created as part of a proof of concept to demonstrate EFI capsule functionality. For real HW, this file is much more detailed and can be difficult / error prone to create. There are already partition files in the https://github.com/qualcomm-linux/qcom-ptool repo for most supported Qualcomm Linux HW and various storage types (emmc and UFS). Let's introduce a script which parses one of those `partitions.conf` files as a way of generating an initial `FvUpdate.xml` which can then be customized by the user as needed. The script takes either: - a file path to a local partition file - parameters specifying the type of HW and storage type in order to choose the reference partition.conf from the online qcom-ptool repo NOTE: Once the new FvUpdate.xml file is generated, please update the operation to be performed on each firmware entry. By default the operation type will be set to IGNORE. Signed-off-by: Venkat Thogaru <[email protected]>
|
Changes LGTM, not tested. |
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.
Added comments
| ***Once the FvUpdate.xml file is generated, please update the Operation to be performed on the each Firmware entry***<p> | ||
| ***By default the Operation type will be set to IGNORE***<p> | ||
|
|
||
|
|
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.
these two blank lines are not needed
| - QCS8300 ESRT GUID: 8BF4424F-E842-409C-80BE-1418E91EF343<p> | ||
| - QCS615 ESRT GUID: 9FD379D2-670E-4BB3-86A1-40497E6E17B0<p> | ||
|
|
||
|
|
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.
please drop these two lines
|
|
||
| def write_xml(doc, output_file="FvUpdate.xml"): | ||
| with open(output_file, "w", encoding="utf-8") as f: | ||
| f.write('<?xml version="1.0" encoding="utf-8"?>\n') |
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.
the whole block can be just replaced with:
xml_str = doc.toprettyxml(indent=" ", encoding="utf-8")
f.write(xml_str)|
|
||
| 3. **Generate/Update FvUpdate.xml with Firmware entries:** | ||
|
|
||
| This script will generate FvUdpate.xml file, with Firmware Entries, |
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.
| This script will generate FvUdpate.xml file, with Firmware Entries, | |
| This script will generate the `FvUpdate.xml` file with firmware entries. |
| partition_info = {} | ||
| if storage_type == "EMMC" and args.F : | ||
| pattern = re.compile( | ||
| r"--partition\s+--name=([\w\-]+)\s+--size=\d+KB\s+--type-guid=([\w\-]+)(?:\s+--type=emmc)?(?:\s+--filename=([\w\.\-]+))?", |
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.
It's just a matter of preference, but why not use just one regex for both eMMC/UFS (to avoid code duplication and possible confusion), where LUN and filename are optional and the type might be ufs/emmc (I haven't tested, just a sketch to show an idea). For example, the regex might be :
pattern = re.compile(
r"--partition\s+"
r"(?:--lun=(?P<lun>\d+)\s+)?"
r"--name=(?P<name>[\w\-]+)\s+"
r"--size=\d+KB\s+"
r"--type-guid=(?P<guid>[\w\-]+)"
r"(?:\s+--type=(?P<type>ufs|emmc))?"
r"(?:\s+--filename=(?P<filename>[\w.\-]+))?",
re.IGNORECASE,
)and then we just process a dict containing all the named subgroups of the match, enforcing the per-mode (EMMC vs UFS) constraints:
gd = m.groupdict()
lun = gd.get("lun")
name = gd["name"]
guid = gd["guid"]
want_lun = (storage_type == "UFS") or (storage_type == "EMMC" and not args.F)
if want_lun: # Storage type UFS
if lun not in ['1', '4']:
continue
else: # Storage type eMMC
if lun is not None:
continue
entry = {"guid": guid, "filename": filename}
if want_lun:
entry["lun"] = lun
partition_info[name] = entry| repo_url = "https://github.com/qualcomm-linux/qcom-ptool.git" | ||
| repo_dir = "qcom-ptool" | ||
| safe_clone(repo_url, repo_dir) | ||
| if args.T == "QCS6490": |
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.
I believe this list will be extended with additional platforms in the future, so it's better to have it defined separately at the beginning of the script (so it's easier to find where it's):
SUPPORTED_PLATFORMS = {
"QCS6490": "qcs6490-rb3gen2",
"QCS9100": "qcs9100-ride-sx",
"QCS8300": "qcs8300-ride-sx",
"QCS615": "qcs615-adp-air",
}and here we can just loop of the dict items or even have a simple function wrapper:
def get_target_name(soc_name):
for platform, target in SUPPORTED_PLATFORMS.items():
if soc_name == platform:
return target
target = get_target_name(args.T)| elif args.T : | ||
| if not args.StorageType: | ||
| print("Error: You must provide -S/--StorageType when using -T/--target.") | ||
| sys.exit(1) |
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.
Can we move these two lines to the top of the script and make them all CAPS (just to follow PEP 8):
REPO_URL = "https://github.com/qualcomm-linux/qcom-ptool.git"
REPO_DIR = "qcom-ptool"
The current FvUpdate.xml was created as part of a proof of concept to demonstrate EFI capsule functionality.
For real HW, this file is much more detailed and can be difficult / error prone to create.
There are already partition files in the https://github.com/qualcomm-linux/qcom-ptool repo for most supported Qualcomm Linux HW and various storage types (emmc and UFS).
Let's introduce a script which parses one of those
partitions.conffiles as a way of generating an initialFvUpdate.xmlwhich can then be customized by the user as needed. The script takes either:NOTE: Once the new FvUpdate.xml file is generated, please update the operation to be performed on each firmware entry. By default the operation type will be set to IGNORE.