[RFC] projects/Amlogic: don't allow booting from internal for S905/S912

[spycat88]

I'm not sure how this PR will be received so thoughts would be welcome.

In community releases installing LE to internal eMMC/NAND has been supported for some time however this has created many problems over time.

As this feature will not be supported in official LE releases (of which I agree) this PR is intended to prevent users who try to install LE to internal via hacks or other methods from booting so this could be construed as controversial however I believe it would save problem posts on the forums in the future.

Booting from external devices is not effected and Wetek users can still boot from internal, this PR is only intended to effect S905/S912 builds.

Support for Odroid C2 would need to be added and other devices will need to be tested, LePotato/KVIM etc...

[Ray-future]

I agree. We don't need internal installs and it's causing a lot of issues. If the PR works with all our AML devices it's a +1 from me.

[CvH]

What happens if you have already installed LE to the internal storage ?

[nomandera]

As a big advocate of not flashing you may be surprised that I have universally flashed every one of my S905X for two reasons:

It is slightly faster
SD boot was failing WAF

I was finding that on far too many occasions I received family support calls that the device had somehow entered into Android and trying to remotely talk a non technical person into booting back into SD is a lesson in patience.

So i would request that we dont force existing users back onto SD. I would see this as a regression. I would be happy with a warning that the install has become unsupported with perhaps a popup "opt in " tick box and a perpetual line in the logs.

Also we will need to document and test the Android apps that exist to help user reboot into SD.

[Ray-future]

That's a good point @anoma. I don't see the advantage in speed to be honest. It may be a little faster but that's not significant and only at boot I'd say.
The other issue could be due to Kodi shutdown options, present in kszaq's build, to reboot to internal?
The main issue here is that we have a lot of AML box vendors that don't support LibreELEC out of the box. Even though it is easy to do it is still a "hack" to run LibreELEC from SD card.

So irrelevant to this PR we will never support internal NAND installs. Either we block it like suggested by @adamg88 in this PR or it will be a "Do at your own risk". And we will not support the installtointernal script cause people should know what they are doing. I'm fine with somebody posting HowTo's on our forum but nobody should expect official support.

[CvH]

As long you have just to type "installtointernal" we made it too easy (some warning text means nothing for a user), we would need a step that has to be different for all boxes so that not a universal guide could do it. So its basically to enforce that you have some kind of knowledge. Tbh no real idea how to achieve this :D (just dropping some ideas)

[nomandera]

I am absolutely on board that we cannot and should not support flashing. There are too many variations and too many risks even for the skilled users never mind those that blindly follow guides.

We could move installtointernal to a different dedicated github project where we can write more words about the risks. It should also never be shipped with the OS. Any user that cant work out how to source and upload this script has no place using it.

We could also make it so that we detect internal installs and mark the logs clearly as a non supported install and a similar notice in the About page in LE addon.

This is a bit of work but ultimately retains user choice and formally devolves us of the associated risks whilst keeping it open for independent development.

[chewitt]

If we do not support it the installtointernal script must not be in our images. If it exists it will be used. If it is used we are back where we started. If users want to fork code and revert commits to get access to it, they probably have the skill level to self-support and nobody cares.

(http://junkee.com/wp-content/uploads/2015/04/button.jpg)

[spycat88]

@CvH if a user already has installed to internal and then flashes an image with this PR it just wouldn't boot anymore and they would be forced to boot via SD/USB, this does not brick the box as they are easily recoverable.

@anoma it is no way faster using internal emmc/nand if you are using a fast enough sdcard/usb pen drive. If your family have rebooted into Android then that would most probably have been because of kszaqs reboot to internal community patch, if you are using LE in the first place then I certain minimum level of competence is required imo and it should not be that difficult for someone to simply pull a power cable and plug it back in.

Another very big reason for blocking it is that the LE kernel does not have the driver for NAND devices so if a user attempts to use some hacky method to get LE onto internal it won't boot anyway and then the forums become a Q&A in how to recover from a self inflicted brickage.

LE has never done an officially supported S905/S912 image so if this policy was enforced from day 1 it would not be confusing to users then. I know it seems somewhat of a medieval idea but I honestly think that we should encourage booting from external in the strongest possible form.

As @chewitt has pointed out if a user is competent enough they can quite easily remedy this and make an image that would boot from internal but it won't be from an official LE image that receives updates and is supported by us.

[Ray-future]

@adamg88 The question is what do we do with users trying to upgrade for your or kszaq's community builds. We will be hit by a bunch of support entries. "Why doesn't it boot while Adam's build worked without issues?"

[spycat88]

@Raybuntu I plan to use this in my community builds if the idea is supported, I have already removed the installtointernal script.

If a user asks that question then I would just reply 'Official LE images will only boot from external storage (sdcard/usb drive)'

I think forcing users to do this would be better in the longer term as recovery is far easier, any official LE image should be focused on 'ease of use' and installtointernal has been nothing but trouble since day 1, it's not much to ask people to spend $5 on an sdcard for their $30 box, or maybe it is =D.

[CvH]

Just saying, this will also "force" day1 custom builds with that function enabled. I can't imagine that ppl that currently use internal storage will switch to a official build + sd card while next to it a custom build pops up at the forum. This generates a lot fragmentation. Also if they accidentally keep auto updating activated they kill the system at an update.

No idea if this works, but could we prevent new installs to internal storage while keeping the function for existing ones at update ?

[spycat88]

@CvH nobody is producing custom builds, at least not publicly anyway with the exception of some pirate builds. People have yearned for official builds for some time now, there is an element of superiority to official builds that community builds just don't have so I don't think that will be the case at all.

The only way to achieve that really is too keep things as they are. It's something we really shouldn't advocate for and need to abandon in it's entirety at least in an official capacity.

[nomandera]

I will defer to whatever people think is best but two points I really dont like:

Updating and ending up with a non booting box is just not viable. We have not bricked the box but we have broke the install. We can do better than this.

This code change will cause our userbase to have to spend money. Lets accepts that reactions to this will range from annoyance at the inconvenience to the other extreme where users simply cant afford it.

[spycat88]

@anoma both points are invalid

Firstly updating... LE has never done an official release so there is nothing to update from other than community builds that is why it is best to take the stance from day 1.

Your second point is untrue because you have to own an sdcard or usb drive in the first place to get LE onto internal.

What I suggest if you feel so strongly about this is to go onto the forums and try helping one of the many users who have become bricked as a result of this because this is something I do on a near daily basis.

There was 1 posted just today even https://forum.libreelec.tv/thread/10301-8-90-5-libreelec-9-0-alpha-for-s905-s912-devices/?postID=85801#post85801

[spycat88]

Another point @anoma I'm not sure if you are aware but if you have kszaqs build installed to internal and update to a recently built LE image your box will no longer boot anyway.

The partition layout in the DTBs is different in the LE kernel than it is in kszaqs kernel so simply updating will result in a bricked box anyway, there is nothing that we can do better and the install will have become broken regardless of this PR or not.

[chewitt]

I wouldn't force hard change on existing installs, it will just cause bad feeling. Just remove the script from official images, withdraw the current images where it's available to minimise "install old version then update" tactics and do nothing further to sustain community images that contain it. Some will still figure out how to include and use it, but it will not be many and usage will decline over time as the official image establishes itself.

[CGarces]

Unfortunately for me, @adamg88 is right, LE was never support that experimental feature.
@anoma
"Updating and ending up with a non booting box is just not viable"

That is user fault for use a experimental feature in a non official build, I hope that can be mitigated.

Is similar to the IR remote changes... if you are using a pre-alpha comunity version you need to deal with it.

In my case, that means that I need to buy some SD cards...

[CvH]

@adamg88 I guess we can't "detect" not working update paths ? So if you try to update from an kszaq build we just refuse to do it ?

[spycat88]

@CvH I guess that could be done in the busybox update scripts to refuse an update from any version <9.0 and force users to install a fresh copy and we could ofcourse add an override file that will allow a forced update if that is what you mean?

I agree with @chewitt and this feature just needs to be abandoned.

@CGarces you make a very good point, users hated the IR remote changes and most of them dealt with it and some went back to an old version, those who want to use the latest version will follow the standards we set.

[tavoc]

A failsafe not to Upgrade an existing build would be ok. If the Script detects an internal build then there could be a Message.
I think its possible to detect if its emmc boot or sd/usb by questioning udev or lsusb....

[spycat88]

The only builds that would be eligible for upgrade would be the community ones I have produced, none of kszaqs will work for users installed on internal due to the partition layout changes in the DTBs

This PR is not about restricting users choice but rather to protect them from themselves, I think if booting from internal is not supported period then we will never have any problem posts about it in the future.

Leaving the door open for users to sideload the installer script and run it will inevitably cause problems as you can guarantee someone will write a how-to and then the problems will start from there.

If someone really wants LE on internal they could reverse this PR, compile LE, wget the script onto their box and run it and if they have the technical ability to do that then I don't think we will be seeing problems from them.

[spycat88]

On a more technical note, the if statement looking for the block device /dev/instaboot was the easiest way to achieve this as that partition only exists on Wetek devices.

[kszaq]

If you want to remove the ability to boot from internal, simply remove the whole if-else:
https://github.com/LibreELEC/LibreELEC.tv/blob/master/projects/Amlogic/initramfs/platform_init#L46-L50

This wouldn't affect Wetek as this information is provided by bootloader.

If you asked me, I would leave the platform_init as is to allow more savvy users installing to internal. I would add canupdate script for S905 and S912 devices to not allow updating internal installs.

Regarding being able to boot from internal using LE kernel - it allows to use internal memory when device is fitted with eMMC. Some of the cheapest devices come with NAND chips that requires a closed-source driver that is included in LE kernel.

[spycat88]

https://forum.libreelec.tv/thread/10301-8-90-6-libreelec-9-0-alpha-for-s905-s912-devices/?postID=85889#post85889

Second user today who was installed to internal and now can't boot...

How many times does it have to happen before the problem is recognised.

[CvH]

@adamg88 no idea how this actually works, but could we add something that checks if install to internal will work at all (I have no idea what are the problems that prevent it) and abort if there are likely problems ? Maybe check against some list if it works or not (just add known working configurations over time)? Just trying to get what limitations we face.

Installtointernal is a very popular feature, ofc the current way is too dangerous - maybe we could improve the security/robustness instead of removing it.

[kszaq]

This PR creates another reason of breakage for users with internal installs.

You should not break booting from internal, you should prevent upgrading an internal install. Providing a canupdate.sh script that prevents upgrading an internal install would prevent breakages. And if anyone DIYs an internal install, they would be savvy enough to touch /storage/.upgrade/.nocompat in case they wanted to update it.

[spycat88]

@CvH this sounds like encouraging install to internal by adding checks and not something I'm in favor off, there are a number of popular features that will never make it into official builds (reboot to internal for one) and things like this should be kept for community builds.

@kszaq there is a breakage for users currently installed to internal without this PR so that is not true, upgrading from your 8.2 builds to 9.0 is impossible and will require a fresh install for devices with installs on internal due to the partition layout changes.

However preventing an upgrade is a good idea.

If we do not block booting from internal someone will write a how to and undoubtedly someone will end up with a broken box, cause panic for other users and blame the build and then when a user asks for help in recovering their box it ultimately ends up being one of us who has to reply and guide them through the process.

The idea is not to block users from booting from internal but rather to only support booting from external and protect users from themselves, people are running the install script with very little understanding.

[spycat88]

I guess something along the lines of the following would be sufficient to prevent upgrading if your are installed on internal in canupdate.sh

for arg in $(cat /proc/cmdline); do
   case ${arg} in
     bootfrom*)
       bootfromext=1
       ;;
  esac
done

if [ "$bootfromext" = "1"]; then
  exit 0
else
  echo "LibreELEC only supports upgrading from devices booted from an external device (USB Drive/SD card)"
  sleep 15
  exit 1
fi

This can probably be expressed more simpler. Ping @MilhouseVH

[MilhouseVH]

I haven't been following along in detail, but this is a crucial question: Is the project/device/arch of the new/upgrade system going to be different to the project/device/arch of the running system, as that's the only reason for calling canupdate.sh during an upgrade. You can see the logic here:

https://github.com/LibreELEC/LibreELEC.tv/blob/master/packages/sysutils/busybox/scripts/init#L395-L405

where ${2} and ${3} are the LIBREELEC_ARCH variables from /etc/os-release of the current and new systems respectively. If these values are the same (ie. "S905.arm" == "S905.arm") then there's no reason to call canupdate.sh, and the upgrade will proceed - internal or external.

Assuming the project/device/arch is changing then canupdate.sh will be called, and you'll need to add some logic to prevent an incompatible upgrade (ie. when the user accidentally attempts to "upgrade" from S905.arm to Generic.x86_64) - exit 1 if the project/device/arch of the new system is not compatible with the current project/device/arch.

Only once you're sure the project/device/arch of the new system is compatible with the current system should you then proceed with your internal/external checks (which look fine to me, at least conceptually, as I don't really know the details of the internal/external logic itself).

I don't think you need the sleep 15 - when the upgrade is aborted the user will have 30 seconds to marvel at the various failed upgrade messages they will be shown.

[tavoc]

If internal builds gets not Supported so there are many Users with crappy not secure Android Versions. I Lieked the way to get rid of Android because there are never any Updates and therefore securityrisks.

I know thats not your responsibility, but it Was a nice to have.

If it is not Supported anymore so please Do the Check with canupdate, because LE has autoupdate enabled as default and will end otherwise with a Black Screen.

[Ray-future]

@tavoc If you only use LibreELEC from SD card and don't boot into Android it's not a security threat.

@kszaq do we have the proprietary NAND driver in our Amlogic Kernel?
Technically speaking if it's just the partition layout we should fix it. I wouldn't want to support new internal installs in offical but I would like to protect users from bricking their devices.

[vpeter4]

Another drama :)

[spycat88]

@vpeter4 its not another drama, this PR was made with the best of intentions, I never expected it to be received well nor accepted.

I wanted to preserve the integrity of official releases and protect users from themselves, I'm tired of dealing with the after effects when things go wrong and it's totally burned me out,

[kszaq]

@Raybuntu The proprietary NAND driver is not present in LE kernel. It may be possible to create a package with it.

@adamg88 Perhaps we should change partition layout to what it used to be in my builds and your builds with the older kernel?

[spycat88]

@kszaq people have already started updating to this kernel with the partition layout as it is but I removed installtointernal so nobody should have the new layout on internal

It could help in the future when official builds start to roll out and allow for seamless upgrades for people who have your builds installed on internal

I'm happy to keep as it is or change it whatever you feel is best

[kszaq]

@AdamG I think that we shouldn't disable booting from internal but also not support it. In terms of backwards compatibility, my patch to DTB to resize system partition was wrong.

For backwards compatibility with my 8.2 community builds,
https://github.com/LibreELEC/linux-amlogic/blob/amlogic-3.14.y/arch/arm64/boot/dts/amlogic/partition_mbox.dtsi#L10-L77
has to be replaced with
https://github.com/kszaq/linux-amlogic-le/blob/amlogic-3.14-nougat/arch/arm64/boot/dts/amlogic/gxl_p212_1g.dts#L566-L639

Please feel free to PR it, I don't have access to my dev PC right now. If you feel any other solution is better, also feel free to PR. I'm afraid there is no way we can prevent users from running update from 8.2 to 9.x as every pre-update check is done using scripts from 8.2 build. Like @MilhouseVH noticed, I was wrong about canupdate.sh logic.

[spycat88]

@kszaq I somewhat agree with you, in hindsight adding a limitation is probably a bad idea, this script causes so much headache for devs and users alike though.

An alternative idea would be to add some code to busybox init to see if arch = S905/S912 and if so if bootfromext!=1 then do not allow automatic upgrade to, this could be overridden with a .nocompat file and then the updates could be applied manually.

This idea would be purely to discourage users from running LE on internal but not disable it.

I'm having connection issues at present but I will send a PR for the partition change when I get chance if you haven't already by then.