Re: [PATCH v8 00/13] bootstd: Convert rockchip and add various fixes and tweaks
Hi Tom, On Wed, 12 Apr 2023 at 17:03, Simon Glass wrote: > > Hi Tom, > > On Sun, 9 Apr 2023 at 18:07, Tom Rini wrote: > > > > On Sun, Apr 09, 2023 at 08:18:31AM +1200, Simon Glass wrote: > > > Hi Tom, > > > [..] > > > > Because that's the key here, on the 564 platforms that use > > > > distro_bootcmd today, there shouldn't be any growth when we switch to > > > > bootstd, and there's even more platforms that enable > > > > CONFIG_DISTRO_DEFAULTS than there are that use distro_bootcmd so the > > > > cases like xilinx mini should be the exception, not the rule. And > > > > that's what I keep circling back to. The logic to keep the defaults that > > > > generic distro support needs are (almost) always already set on the > > > > platforms that are using generic distro boot, so we shouldn't grow in > > > > size when bootstd turns it on. And if I'm changing my mind about > > > > forcing this on, on platforms that hadn't been doing generic distro > > > > before, I guess I'm changing my mind, sorry, there's too many platforms > > > > that are doing other things (not generic distro or Android or Special > > > > Things). > > > > > > Yes I agree there should be no size growth for platforms which use > > > distro_bootcmd today and move to bootstd. That is easy enough to check > > > as part of migration. For the rpi and rockchip series the size reduces > > > from where we are today by 1-12KB. > > > > It shouldn't change it at all tho, let alone reduce? Why is it reducing? Just on this point, which I missed, dropping the scripts in the environment reduces the size by 4-5KB depending on the board. Dropping hush can reduce by quite a lot. Standard boot does not need scripts or hush. Regards, Simon
Re: [PATCH v8 00/13] bootstd: Convert rockchip and add various fixes and tweaks
Hi Tom, On Sun, 9 Apr 2023 at 18:07, Tom Rini wrote: > > On Sun, Apr 09, 2023 at 08:18:31AM +1200, Simon Glass wrote: > > Hi Tom, > > > > On Sun, 9 Apr 2023 at 03:14, Tom Rini wrote: > > > > > > On Sat, Apr 08, 2023 at 12:08:37PM +1200, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Sat, 8 Apr 2023 at 09:55, Tom Rini wrote: > > > > > > > > > > On Sat, Apr 08, 2023 at 09:35:48AM +1200, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Sat, 8 Apr 2023 at 08:22, Tom Rini wrote: > > > > > > > > > > > > > > On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote: > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > On Sat, 8 Apr 2023 at 07:39, Tom Rini > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote: > > > > > > > > > > > > > > > > > > > This series converts rockchip boards over to use standard > > > > > > > > > > boot. It also > > > > > > > > > > fixes various problems which have come up recently, showing > > > > > > > > > > differences > > > > > > > > > > between the current implementation and the distroboot > > > > > > > > > > scripts. > > > > > > > > > > > > > > > > > > > > This should get us closer to being able to turn down the > > > > > > > > > > scripts. > > > > > > > > > > > > > > > > > > Alright, so I grabbed a few parts of this series to > > > > > > > > > investigate the > > > > > > > > > points I'm trying to grasp better, and I think this is going > > > > > > > > > the wrong > > > > > > > > > track. We should start off by dropping "default y" from > > > > > > > > > BOOTSTD, and > > > > > > > > > then start adding "default y if" for SoCs as we convert them. > > > > > > > > > The end > > > > > > > > > goal should be that we get to the point where we can "default > > > > > > > > > y if ARM > > > > > > > > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" > > > > > > > > > as it's > > > > > > > > > just a few architectures that haven't ended up being > > > > > > > > > converted. But > > > > > > > > > today, there's too much churn on platforms that aren't making > > > > > > > > > any use of > > > > > > > > > this. And I don't think this is going to be functionally > > > > > > > > > worse than all > > > > > > > > > of the places we "imply DISTRO_DEFAULTS" today, as > > > > > > > > > functionally we can > > > > > > > > > replace that with "imply BOOTSTD" as they get migrated. > > > > > > > > > > > > > > > > That would really be a backward step. I'm not sure what to say > > > > > > > > at this > > > > > > > > point. I've put a lot of effort in trying to get this over the > > > > > > > > line, > > > > > > > > but the only way we get feedback is when it is applied. > > > > > > > > > > > > > > Having bootstd enabled and not functional (because boot_targets > > > > > > > aren't > > > > > > > set) isn't helping the migration happen. And the hard part of the > > > > > > > migration isn't knowing it's possible, or enabling 1-2 options, > > > > > > > it's > > > > > > > testing it and also it really just being 1-2 options. > > > > > > > > > > > > Standard boot does not need boot_targets to be set. It works fine > > > > > > without it. It just goes through the boot devices in a pre-defined > > > > > > order, from fastest to slowest. It matches what most boards do > > > > > > anyway. > > > > > > The main reason we kept it is for compatibility with distro boot. > > > > > > > > > > What most boards do today is just sit at the prompt and wait for > > > > > input, > > > > > which this changes, which is part of the big source of size churn > > > > > here. > > > > > > > > Yes. > > > > > > > > > > > > > > > I don't think testing in advance is a feasible approach in general. > > > > > > See for example the rpi series which hasn't got any comment. It > > > > > > likely > > > > > > won't until it is applied. That's how we get feedback. We have > > > > > > months > > > > > > to resolve issues and I believe that the code is fundamentally > > > > > > sound. > > > > > > > > > > We need some spot testing here and there to see how things react and > > > > > how > > > > > people use it. You found a lot of things with just rk3399, and now the > > > > > rest of rockchip looks to be fairly direct. You found more things > > > > > doing > > > > > x86. When you can convert some other SoC and the change is just > > > > > dropping > > > > > distro_bootcmd and calling bootflow scan instead (or that + setting > > > > > the > > > > > order again), that'll be good. > > > > > > > > So long as we are aware that we generally only find problems when > > > > patches land, yes. The QEMU stuff is easier since it doesn't need a > > > > board. It's also not all that useful in the real world :-) I'd like to > > > > try a programmatic conversion, too, although I haven't looked at it > > > > yet. > > > > > > Well, I think you're misjudging when we get most of the testing done. > > > It's at that last one or
Re: [PATCH v8 00/13] bootstd: Convert rockchip and add various fixes and tweaks
On Sun, Apr 09, 2023 at 08:18:31AM +1200, Simon Glass wrote: > Hi Tom, > > On Sun, 9 Apr 2023 at 03:14, Tom Rini wrote: > > > > On Sat, Apr 08, 2023 at 12:08:37PM +1200, Simon Glass wrote: > > > Hi Tom, > > > > > > On Sat, 8 Apr 2023 at 09:55, Tom Rini wrote: > > > > > > > > On Sat, Apr 08, 2023 at 09:35:48AM +1200, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Sat, 8 Apr 2023 at 08:22, Tom Rini wrote: > > > > > > > > > > > > On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote: > > > > > > > Hi Tom, > > > > > > > > > > > > > > On Sat, 8 Apr 2023 at 07:39, Tom Rini wrote: > > > > > > > > > > > > > > > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote: > > > > > > > > > > > > > > > > > This series converts rockchip boards over to use standard > > > > > > > > > boot. It also > > > > > > > > > fixes various problems which have come up recently, showing > > > > > > > > > differences > > > > > > > > > between the current implementation and the distroboot scripts. > > > > > > > > > > > > > > > > > > This should get us closer to being able to turn down the > > > > > > > > > scripts. > > > > > > > > > > > > > > > > Alright, so I grabbed a few parts of this series to investigate > > > > > > > > the > > > > > > > > points I'm trying to grasp better, and I think this is going > > > > > > > > the wrong > > > > > > > > track. We should start off by dropping "default y" from > > > > > > > > BOOTSTD, and > > > > > > > > then start adding "default y if" for SoCs as we convert them. > > > > > > > > The end > > > > > > > > goal should be that we get to the point where we can "default y > > > > > > > > if ARM > > > > > > > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" > > > > > > > > as it's > > > > > > > > just a few architectures that haven't ended up being converted. > > > > > > > > But > > > > > > > > today, there's too much churn on platforms that aren't making > > > > > > > > any use of > > > > > > > > this. And I don't think this is going to be functionally worse > > > > > > > > than all > > > > > > > > of the places we "imply DISTRO_DEFAULTS" today, as functionally > > > > > > > > we can > > > > > > > > replace that with "imply BOOTSTD" as they get migrated. > > > > > > > > > > > > > > That would really be a backward step. I'm not sure what to say at > > > > > > > this > > > > > > > point. I've put a lot of effort in trying to get this over the > > > > > > > line, > > > > > > > but the only way we get feedback is when it is applied. > > > > > > > > > > > > Having bootstd enabled and not functional (because boot_targets > > > > > > aren't > > > > > > set) isn't helping the migration happen. And the hard part of the > > > > > > migration isn't knowing it's possible, or enabling 1-2 options, it's > > > > > > testing it and also it really just being 1-2 options. > > > > > > > > > > Standard boot does not need boot_targets to be set. It works fine > > > > > without it. It just goes through the boot devices in a pre-defined > > > > > order, from fastest to slowest. It matches what most boards do anyway. > > > > > The main reason we kept it is for compatibility with distro boot. > > > > > > > > What most boards do today is just sit at the prompt and wait for input, > > > > which this changes, which is part of the big source of size churn here. > > > > > > Yes. > > > > > > > > > > > > I don't think testing in advance is a feasible approach in general. > > > > > See for example the rpi series which hasn't got any comment. It likely > > > > > won't until it is applied. That's how we get feedback. We have months > > > > > to resolve issues and I believe that the code is fundamentally sound. > > > > > > > > We need some spot testing here and there to see how things react and how > > > > people use it. You found a lot of things with just rk3399, and now the > > > > rest of rockchip looks to be fairly direct. You found more things doing > > > > x86. When you can convert some other SoC and the change is just dropping > > > > distro_bootcmd and calling bootflow scan instead (or that + setting the > > > > order again), that'll be good. > > > > > > So long as we are aware that we generally only find problems when > > > patches land, yes. The QEMU stuff is easier since it doesn't need a > > > board. It's also not all that useful in the real world :-) I'd like to > > > try a programmatic conversion, too, although I haven't looked at it > > > yet. > > > > Well, I think you're misjudging when we get most of the testing done. > > It's at that last one or two -rc points where it seems people test > > things, and surprises are very much not welcome then, which is why I > > want more unit testing of things like this. Especially as we're just > > getting started on the conversions. > > The unit tests are in test/boot and there's quite a bit. But it does > not cover some of the minor details though, e.g. how the fdt file is > selected. It can certainly be added, once we
Re: [PATCH v8 00/13] bootstd: Convert rockchip and add various fixes and tweaks
Hi Tom, On Sun, 9 Apr 2023 at 03:14, Tom Rini wrote: > > On Sat, Apr 08, 2023 at 12:08:37PM +1200, Simon Glass wrote: > > Hi Tom, > > > > On Sat, 8 Apr 2023 at 09:55, Tom Rini wrote: > > > > > > On Sat, Apr 08, 2023 at 09:35:48AM +1200, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Sat, 8 Apr 2023 at 08:22, Tom Rini wrote: > > > > > > > > > > On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Sat, 8 Apr 2023 at 07:39, Tom Rini wrote: > > > > > > > > > > > > > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote: > > > > > > > > > > > > > > > This series converts rockchip boards over to use standard boot. > > > > > > > > It also > > > > > > > > fixes various problems which have come up recently, showing > > > > > > > > differences > > > > > > > > between the current implementation and the distroboot scripts. > > > > > > > > > > > > > > > > This should get us closer to being able to turn down the > > > > > > > > scripts. > > > > > > > > > > > > > > Alright, so I grabbed a few parts of this series to investigate > > > > > > > the > > > > > > > points I'm trying to grasp better, and I think this is going the > > > > > > > wrong > > > > > > > track. We should start off by dropping "default y" from BOOTSTD, > > > > > > > and > > > > > > > then start adding "default y if" for SoCs as we convert them. The > > > > > > > end > > > > > > > goal should be that we get to the point where we can "default y > > > > > > > if ARM > > > > > > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as > > > > > > > it's > > > > > > > just a few architectures that haven't ended up being converted. > > > > > > > But > > > > > > > today, there's too much churn on platforms that aren't making any > > > > > > > use of > > > > > > > this. And I don't think this is going to be functionally worse > > > > > > > than all > > > > > > > of the places we "imply DISTRO_DEFAULTS" today, as functionally > > > > > > > we can > > > > > > > replace that with "imply BOOTSTD" as they get migrated. > > > > > > > > > > > > That would really be a backward step. I'm not sure what to say at > > > > > > this > > > > > > point. I've put a lot of effort in trying to get this over the line, > > > > > > but the only way we get feedback is when it is applied. > > > > > > > > > > Having bootstd enabled and not functional (because boot_targets aren't > > > > > set) isn't helping the migration happen. And the hard part of the > > > > > migration isn't knowing it's possible, or enabling 1-2 options, it's > > > > > testing it and also it really just being 1-2 options. > > > > > > > > Standard boot does not need boot_targets to be set. It works fine > > > > without it. It just goes through the boot devices in a pre-defined > > > > order, from fastest to slowest. It matches what most boards do anyway. > > > > The main reason we kept it is for compatibility with distro boot. > > > > > > What most boards do today is just sit at the prompt and wait for input, > > > which this changes, which is part of the big source of size churn here. > > > > Yes. > > > > > > > > > I don't think testing in advance is a feasible approach in general. > > > > See for example the rpi series which hasn't got any comment. It likely > > > > won't until it is applied. That's how we get feedback. We have months > > > > to resolve issues and I believe that the code is fundamentally sound. > > > > > > We need some spot testing here and there to see how things react and how > > > people use it. You found a lot of things with just rk3399, and now the > > > rest of rockchip looks to be fairly direct. You found more things doing > > > x86. When you can convert some other SoC and the change is just dropping > > > distro_bootcmd and calling bootflow scan instead (or that + setting the > > > order again), that'll be good. > > > > So long as we are aware that we generally only find problems when > > patches land, yes. The QEMU stuff is easier since it doesn't need a > > board. It's also not all that useful in the real world :-) I'd like to > > try a programmatic conversion, too, although I haven't looked at it > > yet. > > Well, I think you're misjudging when we get most of the testing done. > It's at that last one or two -rc points where it seems people test > things, and surprises are very much not welcome then, which is why I > want more unit testing of things like this. Especially as we're just > getting started on the conversions. The unit tests are in test/boot and there's quite a bit. But it does not cover some of the minor details though, e.g. how the fdt file is selected. It can certainly be added, once we know what the correct behaviour is. I think it needs more users, before I can tell how close we are. The nice thing with bootstd is that we can create unit tests for this behaviour. That isn't possible with the scripts. > > > > But I'm not sure that changing the platforms that don't
Re: [PATCH v8 00/13] bootstd: Convert rockchip and add various fixes and tweaks
On Sat, Apr 08, 2023 at 12:08:37PM +1200, Simon Glass wrote: > Hi Tom, > > On Sat, 8 Apr 2023 at 09:55, Tom Rini wrote: > > > > On Sat, Apr 08, 2023 at 09:35:48AM +1200, Simon Glass wrote: > > > Hi Tom, > > > > > > On Sat, 8 Apr 2023 at 08:22, Tom Rini wrote: > > > > > > > > On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Sat, 8 Apr 2023 at 07:39, Tom Rini wrote: > > > > > > > > > > > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote: > > > > > > > > > > > > > This series converts rockchip boards over to use standard boot. > > > > > > > It also > > > > > > > fixes various problems which have come up recently, showing > > > > > > > differences > > > > > > > between the current implementation and the distroboot scripts. > > > > > > > > > > > > > > This should get us closer to being able to turn down the scripts. > > > > > > > > > > > > Alright, so I grabbed a few parts of this series to investigate the > > > > > > points I'm trying to grasp better, and I think this is going the > > > > > > wrong > > > > > > track. We should start off by dropping "default y" from BOOTSTD, and > > > > > > then start adding "default y if" for SoCs as we convert them. The > > > > > > end > > > > > > goal should be that we get to the point where we can "default y if > > > > > > ARM > > > > > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as > > > > > > it's > > > > > > just a few architectures that haven't ended up being converted. But > > > > > > today, there's too much churn on platforms that aren't making any > > > > > > use of > > > > > > this. And I don't think this is going to be functionally worse than > > > > > > all > > > > > > of the places we "imply DISTRO_DEFAULTS" today, as functionally we > > > > > > can > > > > > > replace that with "imply BOOTSTD" as they get migrated. > > > > > > > > > > That would really be a backward step. I'm not sure what to say at this > > > > > point. I've put a lot of effort in trying to get this over the line, > > > > > but the only way we get feedback is when it is applied. > > > > > > > > Having bootstd enabled and not functional (because boot_targets aren't > > > > set) isn't helping the migration happen. And the hard part of the > > > > migration isn't knowing it's possible, or enabling 1-2 options, it's > > > > testing it and also it really just being 1-2 options. > > > > > > Standard boot does not need boot_targets to be set. It works fine > > > without it. It just goes through the boot devices in a pre-defined > > > order, from fastest to slowest. It matches what most boards do anyway. > > > The main reason we kept it is for compatibility with distro boot. > > > > What most boards do today is just sit at the prompt and wait for input, > > which this changes, which is part of the big source of size churn here. > > Yes. > > > > > > I don't think testing in advance is a feasible approach in general. > > > See for example the rpi series which hasn't got any comment. It likely > > > won't until it is applied. That's how we get feedback. We have months > > > to resolve issues and I believe that the code is fundamentally sound. > > > > We need some spot testing here and there to see how things react and how > > people use it. You found a lot of things with just rk3399, and now the > > rest of rockchip looks to be fairly direct. You found more things doing > > x86. When you can convert some other SoC and the change is just dropping > > distro_bootcmd and calling bootflow scan instead (or that + setting the > > order again), that'll be good. > > So long as we are aware that we generally only find problems when > patches land, yes. The QEMU stuff is easier since it doesn't need a > board. It's also not all that useful in the real world :-) I'd like to > try a programmatic conversion, too, although I haven't looked at it > yet. Well, I think you're misjudging when we get most of the testing done. It's at that last one or two -rc points where it seems people test things, and surprises are very much not welcome then, which is why I want more unit testing of things like this. Especially as we're just getting started on the conversions. > > But I'm not sure that changing the platforms that don't today opt-in to > > distro_bootcmd (which has been a thing for a long time now) to force > > opt-in to this is the right call. It might be in some cases (mediatek > > maybe? Or maybe no, everyone does android of some flavour so a different > > bootstd option) but not others (those *_evm_r5_defconfig boards). > > Fair enough, so long as we actually turn down distro_bootcmd. So long > as it is still there, boards will enable it. See SPL_FIT_GENERATOR. Yes, it will take follow-through to get everyone converted, and making sure the new tool covers all the use cases. > > > > > What churn are you seeing? Do you mean: > > > > > > > > > > disable BOOTSTD for boards with custom commands? You asked for that >
Re: [PATCH v8 00/13] bootstd: Convert rockchip and add various fixes and tweaks
Hi Tom, On Sat, 8 Apr 2023 at 09:55, Tom Rini wrote: > > On Sat, Apr 08, 2023 at 09:35:48AM +1200, Simon Glass wrote: > > Hi Tom, > > > > On Sat, 8 Apr 2023 at 08:22, Tom Rini wrote: > > > > > > On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Sat, 8 Apr 2023 at 07:39, Tom Rini wrote: > > > > > > > > > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote: > > > > > > > > > > > This series converts rockchip boards over to use standard boot. It > > > > > > also > > > > > > fixes various problems which have come up recently, showing > > > > > > differences > > > > > > between the current implementation and the distroboot scripts. > > > > > > > > > > > > This should get us closer to being able to turn down the scripts. > > > > > > > > > > Alright, so I grabbed a few parts of this series to investigate the > > > > > points I'm trying to grasp better, and I think this is going the wrong > > > > > track. We should start off by dropping "default y" from BOOTSTD, and > > > > > then start adding "default y if" for SoCs as we convert them. The end > > > > > goal should be that we get to the point where we can "default y if ARM > > > > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's > > > > > just a few architectures that haven't ended up being converted. But > > > > > today, there's too much churn on platforms that aren't making any use > > > > > of > > > > > this. And I don't think this is going to be functionally worse than > > > > > all > > > > > of the places we "imply DISTRO_DEFAULTS" today, as functionally we can > > > > > replace that with "imply BOOTSTD" as they get migrated. > > > > > > > > That would really be a backward step. I'm not sure what to say at this > > > > point. I've put a lot of effort in trying to get this over the line, > > > > but the only way we get feedback is when it is applied. > > > > > > Having bootstd enabled and not functional (because boot_targets aren't > > > set) isn't helping the migration happen. And the hard part of the > > > migration isn't knowing it's possible, or enabling 1-2 options, it's > > > testing it and also it really just being 1-2 options. > > > > Standard boot does not need boot_targets to be set. It works fine > > without it. It just goes through the boot devices in a pre-defined > > order, from fastest to slowest. It matches what most boards do anyway. > > The main reason we kept it is for compatibility with distro boot. > > What most boards do today is just sit at the prompt and wait for input, > which this changes, which is part of the big source of size churn here. Yes. > > > I don't think testing in advance is a feasible approach in general. > > See for example the rpi series which hasn't got any comment. It likely > > won't until it is applied. That's how we get feedback. We have months > > to resolve issues and I believe that the code is fundamentally sound. > > We need some spot testing here and there to see how things react and how > people use it. You found a lot of things with just rk3399, and now the > rest of rockchip looks to be fairly direct. You found more things doing > x86. When you can convert some other SoC and the change is just dropping > distro_bootcmd and calling bootflow scan instead (or that + setting the > order again), that'll be good. So long as we are aware that we generally only find problems when patches land, yes. The QEMU stuff is easier since it doesn't need a board. It's also not all that useful in the real world :-) I'd like to try a programmatic conversion, too, although I haven't looked at it yet. > > But I'm not sure that changing the platforms that don't today opt-in to > distro_bootcmd (which has been a thing for a long time now) to force > opt-in to this is the right call. It might be in some cases (mediatek > maybe? Or maybe no, everyone does android of some flavour so a different > bootstd option) but not others (those *_evm_r5_defconfig boards). Fair enough, so long as we actually turn down distro_bootcmd. So long as it is still there, boards will enable it. See SPL_FIT_GENERATOR. > > > > > What churn are you seeing? Do you mean: > > > > > > > > disable BOOTSTD for boards with custom commands? You asked for that > > > > patch > > > > disabling BOOTSTD_DEFAULTS? You asked for that patch > > > > enable BOOTSTD_DEFAULTS by default? We can drop it if you like > > > > > > We need all 3 of those patches because without the 3rd you don't get a > > > good experience when you do enable bootstd for a platform. The problem > > > is that unless you have distro_bootcmd today you're getting between 63kB > > > (a lot of mediatek platforms) and 5k (silk, as a semi-random example) of > > > growth because bootstd is on and now is the default bootcmd, when before > > > they had nothing. And probably had board docs saying "now do ... to > > > boot". And that's largely setting aside the *_r5_* platforms that I know > > > are just doing
Re: [PATCH v8 00/13] bootstd: Convert rockchip and add various fixes and tweaks
On Sat, Apr 08, 2023 at 09:35:48AM +1200, Simon Glass wrote: > Hi Tom, > > On Sat, 8 Apr 2023 at 08:22, Tom Rini wrote: > > > > On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote: > > > Hi Tom, > > > > > > On Sat, 8 Apr 2023 at 07:39, Tom Rini wrote: > > > > > > > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote: > > > > > > > > > This series converts rockchip boards over to use standard boot. It > > > > > also > > > > > fixes various problems which have come up recently, showing > > > > > differences > > > > > between the current implementation and the distroboot scripts. > > > > > > > > > > This should get us closer to being able to turn down the scripts. > > > > > > > > Alright, so I grabbed a few parts of this series to investigate the > > > > points I'm trying to grasp better, and I think this is going the wrong > > > > track. We should start off by dropping "default y" from BOOTSTD, and > > > > then start adding "default y if" for SoCs as we convert them. The end > > > > goal should be that we get to the point where we can "default y if ARM > > > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's > > > > just a few architectures that haven't ended up being converted. But > > > > today, there's too much churn on platforms that aren't making any use of > > > > this. And I don't think this is going to be functionally worse than all > > > > of the places we "imply DISTRO_DEFAULTS" today, as functionally we can > > > > replace that with "imply BOOTSTD" as they get migrated. > > > > > > That would really be a backward step. I'm not sure what to say at this > > > point. I've put a lot of effort in trying to get this over the line, > > > but the only way we get feedback is when it is applied. > > > > Having bootstd enabled and not functional (because boot_targets aren't > > set) isn't helping the migration happen. And the hard part of the > > migration isn't knowing it's possible, or enabling 1-2 options, it's > > testing it and also it really just being 1-2 options. > > Standard boot does not need boot_targets to be set. It works fine > without it. It just goes through the boot devices in a pre-defined > order, from fastest to slowest. It matches what most boards do anyway. > The main reason we kept it is for compatibility with distro boot. What most boards do today is just sit at the prompt and wait for input, which this changes, which is part of the big source of size churn here. > I don't think testing in advance is a feasible approach in general. > See for example the rpi series which hasn't got any comment. It likely > won't until it is applied. That's how we get feedback. We have months > to resolve issues and I believe that the code is fundamentally sound. We need some spot testing here and there to see how things react and how people use it. You found a lot of things with just rk3399, and now the rest of rockchip looks to be fairly direct. You found more things doing x86. When you can convert some other SoC and the change is just dropping distro_bootcmd and calling bootflow scan instead (or that + setting the order again), that'll be good. But I'm not sure that changing the platforms that don't today opt-in to distro_bootcmd (which has been a thing for a long time now) to force opt-in to this is the right call. It might be in some cases (mediatek maybe? Or maybe no, everyone does android of some flavour so a different bootstd option) but not others (those *_evm_r5_defconfig boards). > > > What churn are you seeing? Do you mean: > > > > > > disable BOOTSTD for boards with custom commands? You asked for that patch > > > disabling BOOTSTD_DEFAULTS? You asked for that patch > > > enable BOOTSTD_DEFAULTS by default? We can drop it if you like > > > > We need all 3 of those patches because without the 3rd you don't get a > > good experience when you do enable bootstd for a platform. The problem > > is that unless you have distro_bootcmd today you're getting between 63kB > > (a lot of mediatek platforms) and 5k (silk, as a semi-random example) of > > growth because bootstd is on and now is the default bootcmd, when before > > they had nothing. And probably had board docs saying "now do ... to > > boot". And that's largely setting aside the *_r5_* platforms that I know > > are just doing something else, and could disable it. > > Er, I thought you wanted it to default on if the boards has no > bootcmd? If not, we can disable it for those as well. If you don't > want any increase we can disable it for boards without DISTRO_DEFAULTS > too. After all, presumably those boards are doing something custom > anyway. I think I might have originally, but now that I'm looking at the results it was too optimistic. Every branch I build I look at the per-board breakdown, not just the summary. And it's too much on all of these platforms that had no default bootcmd today. > > We want to convert everyone doing distro_bootcmd over to this, that's > > good. The problem
Re: [PATCH v8 00/13] bootstd: Convert rockchip and add various fixes and tweaks
Hi Tom, On Sat, 8 Apr 2023 at 08:22, Tom Rini wrote: > > On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote: > > Hi Tom, > > > > On Sat, 8 Apr 2023 at 07:39, Tom Rini wrote: > > > > > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote: > > > > > > > This series converts rockchip boards over to use standard boot. It also > > > > fixes various problems which have come up recently, showing differences > > > > between the current implementation and the distroboot scripts. > > > > > > > > This should get us closer to being able to turn down the scripts. > > > > > > Alright, so I grabbed a few parts of this series to investigate the > > > points I'm trying to grasp better, and I think this is going the wrong > > > track. We should start off by dropping "default y" from BOOTSTD, and > > > then start adding "default y if" for SoCs as we convert them. The end > > > goal should be that we get to the point where we can "default y if ARM > > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's > > > just a few architectures that haven't ended up being converted. But > > > today, there's too much churn on platforms that aren't making any use of > > > this. And I don't think this is going to be functionally worse than all > > > of the places we "imply DISTRO_DEFAULTS" today, as functionally we can > > > replace that with "imply BOOTSTD" as they get migrated. > > > > That would really be a backward step. I'm not sure what to say at this > > point. I've put a lot of effort in trying to get this over the line, > > but the only way we get feedback is when it is applied. > > Having bootstd enabled and not functional (because boot_targets aren't > set) isn't helping the migration happen. And the hard part of the > migration isn't knowing it's possible, or enabling 1-2 options, it's > testing it and also it really just being 1-2 options. Standard boot does not need boot_targets to be set. It works fine without it. It just goes through the boot devices in a pre-defined order, from fastest to slowest. It matches what most boards do anyway. The main reason we kept it is for compatibility with distro boot. I don't think testing in advance is a feasible approach in general. See for example the rpi series which hasn't got any comment. It likely won't until it is applied. That's how we get feedback. We have months to resolve issues and I believe that the code is fundamentally sound. > > > What churn are you seeing? Do you mean: > > > > disable BOOTSTD for boards with custom commands? You asked for that patch > > disabling BOOTSTD_DEFAULTS? You asked for that patch > > enable BOOTSTD_DEFAULTS by default? We can drop it if you like > > We need all 3 of those patches because without the 3rd you don't get a > good experience when you do enable bootstd for a platform. The problem > is that unless you have distro_bootcmd today you're getting between 63kB > (a lot of mediatek platforms) and 5k (silk, as a semi-random example) of > growth because bootstd is on and now is the default bootcmd, when before > they had nothing. And probably had board docs saying "now do ... to > boot". And that's largely setting aside the *_r5_* platforms that I know > are just doing something else, and could disable it. Er, I thought you wanted it to default on if the boards has no bootcmd? If not, we can disable it for those as well. If you don't want any increase we can disable it for boards without DISTRO_DEFAULTS too. After all, presumably those boards are doing something custom anyway. > > We want to convert everyone doing distro_bootcmd over to this, that's > good. The problem is we don't have a symbol today that means "we want > distro_bootcmd" and also isn't overloaded (DISTRO_DEFAULTS is overloaded > in this sense). > > The wrong direction part of this series is that for platforms that > aren't in the middle of converting we're increasing their size between > somewhat and very very much, and we haven't tested that it'll work. And > yes, there's some automatic guessing logic, which hasn't been tested on > these platforms either, so we don't actually know if going from no > bootcmd (and so drops to prompt) to attempts to autoboot something is an > improvement. So, the wrong direction comes from the last three patches. Is that right? Fundamentally the problem I have is that I know where I would like this to head, which is everything using standard boot and turning down the scripts. But it feels like every time I touch bootstd we have to have the EFI discussion again. You can imagine how I feel about disabling BOOTSTD by default...it would basically kill it. This is not really an arch-specific thing, nor an SoC-specific thing. The underlying logic is the same for everything. The reason I think we need to do a few cases before we enable it everywhere is that we need to find the little tweaks needed in that logic. How about we apply the first patches in this series, skipping the last three, then apply the
Re: [PATCH v8 00/13] bootstd: Convert rockchip and add various fixes and tweaks
On Sat, Apr 08, 2023 at 07:53:10AM +1200, Simon Glass wrote: > Hi Tom, > > On Sat, 8 Apr 2023 at 07:39, Tom Rini wrote: > > > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote: > > > > > This series converts rockchip boards over to use standard boot. It also > > > fixes various problems which have come up recently, showing differences > > > between the current implementation and the distroboot scripts. > > > > > > This should get us closer to being able to turn down the scripts. > > > > Alright, so I grabbed a few parts of this series to investigate the > > points I'm trying to grasp better, and I think this is going the wrong > > track. We should start off by dropping "default y" from BOOTSTD, and > > then start adding "default y if" for SoCs as we convert them. The end > > goal should be that we get to the point where we can "default y if ARM > > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's > > just a few architectures that haven't ended up being converted. But > > today, there's too much churn on platforms that aren't making any use of > > this. And I don't think this is going to be functionally worse than all > > of the places we "imply DISTRO_DEFAULTS" today, as functionally we can > > replace that with "imply BOOTSTD" as they get migrated. > > That would really be a backward step. I'm not sure what to say at this > point. I've put a lot of effort in trying to get this over the line, > but the only way we get feedback is when it is applied. Having bootstd enabled and not functional (because boot_targets aren't set) isn't helping the migration happen. And the hard part of the migration isn't knowing it's possible, or enabling 1-2 options, it's testing it and also it really just being 1-2 options. > What churn are you seeing? Do you mean: > > disable BOOTSTD for boards with custom commands? You asked for that patch > disabling BOOTSTD_DEFAULTS? You asked for that patch > enable BOOTSTD_DEFAULTS by default? We can drop it if you like We need all 3 of those patches because without the 3rd you don't get a good experience when you do enable bootstd for a platform. The problem is that unless you have distro_bootcmd today you're getting between 63kB (a lot of mediatek platforms) and 5k (silk, as a semi-random example) of growth because bootstd is on and now is the default bootcmd, when before they had nothing. And probably had board docs saying "now do ... to boot". And that's largely setting aside the *_r5_* platforms that I know are just doing something else, and could disable it. We want to convert everyone doing distro_bootcmd over to this, that's good. The problem is we don't have a symbol today that means "we want distro_bootcmd" and also isn't overloaded (DISTRO_DEFAULTS is overloaded in this sense). The wrong direction part of this series is that for platforms that aren't in the middle of converting we're increasing their size between somewhat and very very much, and we haven't tested that it'll work. And yes, there's some automatic guessing logic, which hasn't been tested on these platforms either, so we don't actually know if going from no bootcmd (and so drops to prompt) to attempts to autoboot something is an improvement. -- Tom signature.asc Description: PGP signature
Re: [PATCH v8 00/13] bootstd: Convert rockchip and add various fixes and tweaks
Hi Tom, On Sat, 8 Apr 2023 at 07:39, Tom Rini wrote: > > On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote: > > > This series converts rockchip boards over to use standard boot. It also > > fixes various problems which have come up recently, showing differences > > between the current implementation and the distroboot scripts. > > > > This should get us closer to being able to turn down the scripts. > > Alright, so I grabbed a few parts of this series to investigate the > points I'm trying to grasp better, and I think this is going the wrong > track. We should start off by dropping "default y" from BOOTSTD, and > then start adding "default y if" for SoCs as we convert them. The end > goal should be that we get to the point where we can "default y if ARM > || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's > just a few architectures that haven't ended up being converted. But > today, there's too much churn on platforms that aren't making any use of > this. And I don't think this is going to be functionally worse than all > of the places we "imply DISTRO_DEFAULTS" today, as functionally we can > replace that with "imply BOOTSTD" as they get migrated. That would really be a backward step. I'm not sure what to say at this point. I've put a lot of effort in trying to get this over the line, but the only way we get feedback is when it is applied. What churn are you seeing? Do you mean: disable BOOTSTD for boards with custom commands? You asked for that patch disabling BOOTSTD_DEFAULTS? You asked for that patch enable BOOTSTD_DEFAULTS by default? We can drop it if you like The other patches are bug fixes as noted below. Passing run at https://source.denx.de/u-boot/custodians/u-boot-dm/-/pipelines/15934 Size trace: 02: bootstd: Tweak bootflow logic for device tree arm: brsmarc1 j7200_evm_r5 j721e_evm_r5 ge_b1x5v2 stv0991 imxrt1020-evk imxrt1050-evk imxrt1170-evk aarch64: (for 383/383 boards) all +36.6 text +36.6 arm: (for 649/649 boards) all +13.8 text +13.8 riscv32: (for 7/7 boards) all +34.0 text +34.0 riscv64: (for 14/14 boards) all +20.4 text +20.4 sandbox: (for 7/7 boards) all +10.3 text +10.3 x86: (for 29/29 boards) all +20.9 text +20.9 this is a bug fix, really 03: virtio: Ensure PCI is set up first aarch64: (for 383/383 boards) all +0.1 text +0.1 arm: (for 649/649 boards) all +0.0 text +0.0 powerpc: (for 81/81 boards) all +0.2 text +0.2 riscv32: (for 7/7 boards) all +6.9 text +6.9 riscv64: (for 14/14 boards) all +3.9 text +3.9 sandbox: (for 7/7 boards) all +17.1 text +17.1 x86: (for 29/29 boards) all +1.1 text +1.1 this makes it match the scripts 04: bootstd: Support booting EFI where multiple options exist aarch64: (for 383/383 boards) all +24.0 rodata +0.3 text +23.8 arm: (for 649/649 boards) all +13.0 rodata +0.1 text +13.0 riscv32: (for 7/7 boards) all +40.0 rodata -4.0 text +44.0 riscv64: (for 14/14 boards) all -13.4 rodata -4.6 text -8.9 sandbox: (for 7/7 boards) all +36.6 text +36.6 x86: (for 29/29 boards) all +13.4 text +13.4 bug fix 05: bootstd: Report missing labels only when asked aarch64: (for 383/383 boards) all +36.9 rodata -9.5 text +46.5 arc: (for 11/11 boards) all +13.8 rodata -5.1 text +18.9 arm: (for 649/649 boards) all +30.0 rodata -8.7 text +38.6 m68k: (for 18/18 boards) all -0.3 rodata -0.5 text +0.2 mips: (for 45/45 boards) all +42.7 rodata -3.9 text +46.6 powerpc: (for 81/81 boards) all +43.8 rodata -7.0 text +50.7 riscv32: (for 7/7 boards) all +48.0 rodata -8.0 text +56.0 riscv64: (for 14/14 boards) all +48.3 rodata -8.0 text +56.3 sandbox: (for 7/7 boards) all -253.7 rodata -64.0 spl/u-boot-spl:all -11.4 spl/u-boot-spl:rodata -9.1 spl/u-boot-spl:text -2.3 text -189.7 sh: (for 1/1 boards) all +24.0 rodata -8.0 text +32.0 x86: (for 29/29 boards) all +8.6 rodata -8.4 text +17.0 Patch you requested 06: bootstd: Show a message sometimes if no bootflows are found aarch64: (for 383/383 boards) all +0.2 rodata +0.1 text +0.1 arm: (for 649/649 boards) all +0.2 rodata +0.1 text +0.1 sandbox: (for 7/7 boards) all +57.1 rodata +36.6 text +20.6 Patch you requested 07: rockchip: Move to standard boot aarch64: (for 383/383 boards) all -180.1 rodata -180.1 arm: (for 649/649 boards) all -108.8 rodata -108.8 08: rockchip: Use the same boot_targets for all boards aarch64: (for 383/383 boards) all +0.9 rodata +0.9 arm: (for 649/649 boards) all +0.5 rodata +0.5 Long-standing conversion patches 09: bootstd: Disable BOOTSTD for boards with custom commands aarch64: (for 383/383 boards) all -3314.4 bss -3.6 data -224.2 rodata -312.8 text -2773.8 arm: (for 649/649 boards) all -6150.4 data -276.3 rodata -640.6 text -5233.5 mips: (for 45/45 boards) all -1090.7 data -30.9 rodata -103.8 text -955.9 powerpc: (for 81/81 boards) all -21042.0 bss -9.7 data -951.4
Re: [PATCH v8 00/13] bootstd: Convert rockchip and add various fixes and tweaks
On Fri, Apr 07, 2023 at 10:36:38PM +1200, Simon Glass wrote: > This series converts rockchip boards over to use standard boot. It also > fixes various problems which have come up recently, showing differences > between the current implementation and the distroboot scripts. > > This should get us closer to being able to turn down the scripts. Alright, so I grabbed a few parts of this series to investigate the points I'm trying to grasp better, and I think this is going the wrong track. We should start off by dropping "default y" from BOOTSTD, and then start adding "default y if" for SoCs as we convert them. The end goal should be that we get to the point where we can "default y if ARM || RISCV || X86" or perhaps "default y !(PPC || M68K || ...)" as it's just a few architectures that haven't ended up being converted. But today, there's too much churn on platforms that aren't making any use of this. And I don't think this is going to be functionally worse than all of the places we "imply DISTRO_DEFAULTS" today, as functionally we can replace that with "imply BOOTSTD" as they get migrated. -- Tom signature.asc Description: PGP signature
