Skip to content
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

Nano updates for master-next #5

Merged
merged 5 commits into from
Nov 3, 2020
Merged

Conversation

madisongh
Copy link
Member

  • U-Boot patches:
    • replace one of the mender-core U-Boot patches with a copy that patches cleanly into the v2020.07 code base
    • add Mender integration for the Nano 2GB dev kit
  • Flash layout updates:
    • Fix an issue with the SPI flash layout for jetson-nano-qspi-sd
    • Add support for jetson-nano-2gb-devkit (identical layout as for the 4GB model)

These changes track the master branch in meta-tegra, which is now at L4T R32.4.4 and has support for the Nano 2GB devkit.

The flash layout fix for jetson-nano-qspi-sd also needs back-porting to dunfell, since the problem occurs with L4T R32.4.3 as well.

This eliminates the patch fuzz warning for patching into
the latest u-boot-tegra code base.

Signed-off-by: Matt Madison <matt@madison.systems>
@madisongh madisongh linked an issue Oct 31, 2020 that may be closed by this pull request
<allocation_policy> sequential </allocation_policy>
<filesystem_type> basic </filesystem_type>
<size> 3342336 </size>
<start_location> 0x3B0000 </start_location>
<size> 262144 </size>
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for catching this @madisongh .

I like having the offset in the flash.xml now.

I've added a "Nano-uboot-PR5" sheet showing the default values for 32.4.4 and 32.4.3. I'm wondering if it would be better to keep UBENV out of VER_b and VER partitions and leave these partitions at their default size of 32768 in flash_l4t_t210_spi_sd_p3448.xml. I noticed the L4T Developer Page doesn't mention much about how the VER partitions are used so I'm not sure what the implications are of changing these.

If we moved UBENV start location back to 0x3A0000 we could leave VER_b and VER at the same location/size NVIDIA uses and we should still have unused space from 0xB0000 to 0x39FFFF after the NXC_R partition.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm wondering if it would be better to keep UBENV out of VER_b and VER partitions

I played around with different locations for these partitions. The VER/VER_b partitions are only used by the l4t_payload_updater_t210 script to compare the current version against the version in the BUP payload. The data stored in them is very small, on the order of 100 bytes or so, and they're looked up via the pseudo-GPT in the SPI flash, so there's no problem with relocating them and reducing the size (at least for now). Relocating the U-Boot environment block, on the other hand, would affect upgrades/downgrades between pre-32.4.x and post-32.4.x unless we add some state scripts to save and restore at least the mender_ variables so they're propagated correctly.

So in the end it looked like relocating and shrinking the VER/VER_b partitions was the safer choice.

Copy link
Member

Choose a reason for hiding this comment

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

Relocating the U-Boot environment block, on the other hand, would affect upgrades/downgrades between pre-32.4.x and post-32.4.x

Agreed, however I'm not sure if this really matters as much for the sdcard nano versions since I don't expect anyone is using in production. So if there's a a tradeoff between future support headaches and breaking upgrades between the dunfell release we just did weeks ago and this version I'd tend to lean toward the breaking upgrades for the sake of future compatibility on this particular platform. I haven't tried upgrades from zeus to dunfell on this platform myself.

I'm wondering if it would be a good exercise to go back through each platform and look for differences between the stock partition layout and mender layout to make sure we didn't miss synchronizing with any L4T flash layout updates. It looks like we missed one on this platform at least. I haven't done this myself, not sure if you or @kekiefer have already. If not I can look at this.

Copy link
Member Author

@madisongh madisongh Nov 2, 2020

Choose a reason for hiding this comment

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

So if there's a a tradeoff between future support headaches and breaking upgrades between the dunfell release we just did weeks ago and this version I'd tend to lean toward the breaking upgrades for the sake of future compatibility on this particular platform.

I'm fine either way; given what VER is used for, I think this change is fairly low risk, but you never know.

Another option would be for us to reduce the size of the U-Boot environment (by setting BOOTENV_SIZE for the SPI/SD platforms) to 0x1800 (96KiB) or even 0x1000 (64KiB) so that two copies can fit between the UBOOTENV start location and VER_b's start location. The default size that Mender assigns is quite large, and we only use about 6KiB in the default environment setup, so it's unlikely that the reduction would have any impact for most use cases.

Thanks for the spreadsheet, btw...

Copy link
Member

Choose a reason for hiding this comment

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

Another option would be for us to reduce the size of the U-Boot environment (by setting BOOTENV_SIZE for the SPI/SD platforms) to 0x1800 (96KiB) or even 0x1000 (64KiB) so that two copies can fit between the UBOOTENV start location and VER_b's start location

I like this idea the best of all proposed thus far.

Copy link
Member Author

Choose a reason for hiding this comment

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

I like this idea the best of all proposed thus far.

OK, I'll update the PR to do it that way, then.

I'm wondering if it would be a good exercise to go back through each platform and look for differences between the stock partition layout and mender layout

Good idea. I haven't done a comprehensive comparison.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I'll update the PR to do it that way, then.

I've refreshed and rebased the patches for this, using 64KiB for the environment size.

This updates the defconfig file for the 2GB dev kit for
Mender integration.  Changes are similar to those for the
4GB Nano.

Signed-off-by: Matt Madison <matt@madison.systems>
To maintain compatibility with the layout of the NVIDIA partitions
in the SPI flash for L4T R32.4.3+ and still keep the U-Boot environment
start at the same offset in SPI flash we've been using, we need to
reduce the size of the environment to allow it to fit into the
available space.  To do this, set BOOTENV_SIZE on SPI flash-equipped
tegra210 platforms to 64KiB instead of the Mender default of 128KiB.

Signed-off-by: Matt Madison <matt@madison.systems>
The flashing tools and flash layout for the QSPI-equipped Nanos were
updated in L4T R32.4.3, and our existing layout file generates an
error during USB flashing because the updated tools think the VER
partition runs off the end of the flash. Either we added up the
sizes incorrectly, or there's a bug in the tools.

This patch updates the flash layout to keep the U-Boot environment
area we reserve in the QSPI flash at the same location and with the
same size.  The VER_b and VER partitions that follow are reduced
in size to prevent the flashing tools from complaining.  This should
retain compatibility for an OTA upgrade that updates the SPI flash
without having to save and restore the U-Boot environment.  The
VER partitions are smaller than the default L4T settings (which
were also reduced in R32.4.3), but still plenty large enough to
hold the version information.

Signed-off-by: Matt Madison <matt@madison.systems>
The flash layout is identical to the one for the 4GB model, so
just use a symlink to share the XML file between the two machines.

Signed-off-by: Matt Madison <matt@madison.systems>
Copy link
Member

@dwalkes dwalkes left a comment

Choose a reason for hiding this comment

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

Verified the build with https://github.com/BoulderAI/tegra-demo-distro/tree/master-next, haven't actually tried deploying yet. I can submit a PR for this if you like or if you'd rather merge from your test distro that's fine with me.

I'll try to get through the rest of the flash layouts this week and work on a backport of these to dunfell and upstream PR.

@madisongh madisongh merged commit d6ed135 into OE4T:master-next Nov 3, 2020
@madisongh
Copy link
Member Author

@dwalkes I've updated tegra-demo-distro master with the R32.4.4 changes ported from my test distro. I did find one more update to the custom flash layouts here which I merged in this morning prior to the demo distro update.

I'll try to get through the rest of the flash layouts this week and work on a backport of these to dunfell and upstream PR.

Thanks!

@dwalkes
Copy link
Member

dwalkes commented Nov 3, 2020

I did find one more update to the custom flash layouts here

I noticed this at #6, thanks for catching it.

I've also updated https://github.com/OE4T/meta-tegra/wiki/L4T-Integration-Issues to reference headaches with flash.xml layouts.

@dwalkes
Copy link
Member

dwalkes commented Nov 8, 2020

I'll try to get through the rest of the flash layouts this week

See updated sheet at https://docs.google.com/spreadsheets/d/14zVltS2LTwCNywoje6_8wzFzHQ3sYVhY6C7T0DkOnx0/edit#gid=2052166477 and r32.4.3 tabs. I think this covers all configurations we support. I've tried to make it easier to keep up up with this going forward by including some xml parsing commands that can generate the tables automagically.

Can you refresh my memory about why the tegra194 and tegra210 directories exist at https://github.com/mendersoftware/meta-mender-community/tree/dunfell/meta-mender-tegra/recipes-bsp/tegra-binaries/mender-custom-flash-layout ? I didn't check these.

I found one issue, it looks like NVIDIA moved VER partitions on tx2/nano-emmc and these now overlap the uboot area they previously advised for r32.3.1. I've added a comment at here to ask about this, I'm tempted to try to change the default layout related to VER partitions to match NVIDIA, at least for jetson-nano-emmc. Not sure if we should try this for tx2 or not. At minimum I think we should probably update the flash layouts to include the UBENV partition with start offset to make these align better with the new definitions from nvidia.

and work on a backport of these to dunfell and upstream PR.

See mendersoftware#188

@dwalkes
Copy link
Member

dwalkes commented Nov 9, 2020

Can you refresh my memory about why the tegra194 and tegra210 directories exist at https://github.com/mendersoftware/meta-mender-community/tree/dunfell/meta-mender-tegra/recipes-bsp/tegra-binaries/mender-custom-flash-layout

The t194 is for AGX Xavier jetson-xavier. I added a tab for that one. Shouldn't this line be RECROOTFSSIZE? Looks like it was set with a fixed value during r32.3.1 here.

Still not sure when the tegra210 file would be used. Would that be for jetson-tx1? Do we actually support that one? I've never tested myself with this platform and don't have a TX1. It was originally added with mendersoftware@f9e465c

@madisongh
Copy link
Member Author

The t194 is for AGX Xavier jetson-xavier. I added a tab for that one. Shouldn't this line be RECROOTFSSIZE? Looks like it was set with a fixed value during r32.3.1 here.

Yes, tegra194 is for any machine with the Xavier SoC (and for which we don't have a machine-specific layout). In L4T R32.3.1, the size was hard-coded. In R32.4.x NVIDIA changed to use RECROOTFSSIZE.

Still not sure when the tegra210 file would be used. Would that be for jetson-tx1?

Yes, and I have tested with a TX1 dev kit.

@dwalkes
Copy link
Member

dwalkes commented Nov 9, 2020

Yes, and I have tested with a TX1 dev kit.

OK I've added a tx1 worksheet as well. Looks like it has the same issue as AGX regarding RECROOTFSSIZE and same issue as tx2 and nano-emmc regarding swapping of the VER locations.

For the RECROOTFSSIZE change on AGX and TX1 it seems we should update both of these and I can't think of a downside.

For the VER and UENV changes I think this is going to be more complicated. No response yet on my question about relocating. For nano I think it's easier to relocate, since we don't update the bootloader I wouldn't expect this to really be something anyone will notice unless they are flashing the sdcard and paying very close attention to which rootfs they are using. See related discussion here. For TX1 since support is new, not advertised on mender hub and we don't know of anyone actually using I'm tempted to move these to match NVIDIA as well.

For TX2 if we change the uboot environment and VER location this would introduce incompatibility in the location of VER across mender u-boot bootloader updates and corrupt the VER location with u-boot environment content if booting an old version of u-boot bootloader on a new flash layout. The response to this post may help determine the urgency or whether we need to attempt to match. For the tx2 cboot install it seems it would be safer to move VER partitions, since we don't have the concern about uboot environment, however this would require special logic to select a different partition layout based on PREFERRED_PROVIDER_virtual/bootloader. This is another reason why encouraging folks starting out with TX2/mender to use PREFERRED_PROVIDER_virtual/bootloader selecting cboot might be a good approach too. See related headaches in #7.

I'm tempted to fix the TX2 issue by just moving the VER location to match NVIDIAs r32.4.3 layout for cboot based builds and making the default configuration for tegrademo-mender use cboot and having yet another version of flash layout for cboot based tx2 builds. What do you think?

@madisongh
Copy link
Member Author

OK I've added a tx1 worksheet as well. Looks like it has the same issue as AGX regarding RECROOTFSSIZE and same issue as tx2 and nano-emmc regarding swapping of the VER locations.

For the RECROOTFSSIZE change on AGX and TX1 it seems we should update both of these and I can't think of a downside.

meta-tegra sets RECROOTFSSIZE to 314572800 (300MiB) by default, so as long as folks keep that default, it should be compatible. L4T R32.3.1 did not have a RECROOTFSSIZE setting at all, and when NV added it in R32.4.x, they set it to 100MiB in their config files (flash.sh still uses 300MiB as a default if unset).

Since it's unlikely we'll ever use the L4T recovery rootfs in a Mender setup (since we're doing full A/B), the size probably doesn't matter - in fact, we could get rid of it completely. The only consideration is compatibility for upgrades and not surprising anyone with the change in layout. Documenting the change could handle that.

For the VER and UENV changes I think this is going to be more complicated. No response yet on my question about relocating. For nano I think it's easier to relocate, since we don't update the bootloader I wouldn't expect this to really be something anyone will notice unless they are flashing the sdcard and paying very close attention to which rootfs they are using. See related discussion here. For TX1 since support is new, not advertised on mender hub and we don't know of anyone actually using I'm tempted to move these to match NVIDIA as well.

That should be fine. TX1 is like Nano in that it has no A/B support in the bootloader.

For TX2 if we change the uboot environment and VER location this would introduce incompatibility in the location of VER across mender u-boot bootloader updates and corrupt the VER location with u-boot environment content if booting an old version of u-boot bootloader on a new flash layout. The response to this post may help determine the urgency or whether we need to attempt to match. For the tx2 cboot install it seems it would be safer to move VER partitions, since we don't have the concern about uboot environment, however this would require special logic to select a different partition layout based on PREFERRED_PROVIDER_virtual/bootloader. This is another reason why encouraging folks starting out with TX2/mender to use PREFERRED_PROVIDER_virtual/bootloader selecting cboot might be a good approach too. See related headaches in #7.

I'm tempted to fix the TX2 issue by just moving the VER location to match NVIDIAs r32.4.3 layout for cboot based builds and making the default configuration for tegrademo-mender use cboot and having yet another version of flash layout for cboot based tx2 builds. What do you think?

The only programs that locate and parse the VER partitions are the updating tools that look them up by partition name rather than a hard-coded offset, so moving them should not be a big deal, and there shouldn't be a problem with those partitions on a downgrade. They aren't used by the bootloaders themselves. I'd be more concerned about losing U-Boot environment due to relocation/resizing across an upgrade/downgrade.

@dwalkes
Copy link
Member

dwalkes commented Nov 9, 2020

The only consideration is compatibility for upgrades and not surprising anyone with the change in layout. Documenting the change could handle that.

So would your preference be to match NVIDIA's layout regarding RECROOTFSSIZE and document the fact that this changed in the pull request as well as in some mender tegra release notes we add?

I'd be more concerned about losing U-Boot environment due to relocation/resizing across an upgrade/downgrade.

Agreed. I was thinking uboot environment location could possibly be handled with state scripts like the ones you wrote earlier but it would be more difficult to handle any dependencies on VER location for bootloader tools.

It seems you are suggesting we don't bother to change our tx2 layout for cboot or uboot based builds and just stick with our current VER location from r32.3.1?

@madisongh
Copy link
Member Author

So would your preference be to match NVIDIA's layout regarding RECROOTFSSIZE and document the fact that this changed in the pull request as well as in some mender tegra release notes we add?

Sort of. Since we've kept the same 300MiB default for RECROOTFSSIZE in meta-tegra across versions (which differs from L4T's defaults, as it turns out), it's not exactly matching, other than to use the RECROOTFSSIZE token in the XML file instead of the hard-coded number. But making that change is fine, and yes, it should be mentioned in the release notes.

It seems you are suggesting we don't bother to change our tx2 layout for cboot or uboot based builds and just stick with our current VER location from r32.3.1?

I'm just saying that we should try to preserve the U-Boot environment across upgrades/downgrades as part of the integration -- the upgrade/downgrade considerations are, IMO, more important than alignment with stock L4T layouts in this specific case, since the VER partitions are not actually used by NV's bootloaders, just the update tools that run in Linux (and can look up the partitions by name).

Upgrades/downgrades will be easier for the U-Boot TX2 if the environment area remains at the same location with the same size... otherwise we'd have to hack in a way to transfer the environment settings like I did in those state scripts I originally put together.

This is a non-issue for the cboot-only platforms (or TX2 configured for cboot only), of course, and aligning with the L4T layout makes more sense for those cases. (Although I'd suggest that using a single layout for TX2 that works for either cboot-only or cboot+U-Boot would be better than having different ones.)

irodriguez-veridas pushed a commit to irodriguez-veridas/oe4t-meta-mender-community that referenced this pull request May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Jetson Nano 2GB
2 participants