[v2,0/6] ARM64: meson: GXBaby (S905) and Vega S95 enablement
diff mbox

Message ID 1456886101-22967-1-git-send-email-afaerber@suse.de
State New, archived
Headers show

Commit Message

Andreas Färber March 2, 2016, 2:34 a.m. UTC
Hello,

This series adds initial support for the Amlogic S905 based
Tronsmart Vega S95 Pro, Meta and Telos TV boxes.

v2:
* Pick up previously acked "tronsmart" patch instead (Matthias)
* Drop ARM_GIC selection (Sudeep)
* Change some compatible strings (Sudeep, André)
* Squash some node changes/additions

Note: On the Vega S95 I need to change TEXT_OFFSET as follows,
in order to avoid the vendor U-Boot overwriting itself (fwiu);
for the Mini Mx that's reportedly not necessary.


https://en.opensuse.org/HCL:VegaS95

https://github.com/afaerber/linux/commits/vegas95-next

Regards,
Andreas

Cc: André Przywara <andre.przywara@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: devicetree@vger.kernel.org

Andreas Färber (5):
  ARM64: Enable Amlogic Meson GXBaby platform
  Documentation: devicetree: amlogic: Document Meson GXBaby
  ARM64: dts: Prepare configs for Amlogic Meson GXBaby
  Documentation: devicetree: amlogic: Document Tronsmart Vega S95 boards
  ARM64: dts: amlogic: Add Tronsmart Vega S95 configs

Matthias Brugger (1):
  devicetree: bindings: Add vendor prefix for Tronsmart

 Documentation/devicetree/bindings/arm/amlogic.txt  |   7 +
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 arch/arm64/Kconfig.platforms                       |   5 +
 arch/arm64/boot/dts/Makefile                       |   1 +
 arch/arm64/boot/dts/amlogic/Makefile               |   7 +
 .../boot/dts/amlogic/meson-gxbb-vega-s95-meta.dts  |  55 +++++++
 .../boot/dts/amlogic/meson-gxbb-vega-s95-pro.dts   |  55 +++++++
 .../boot/dts/amlogic/meson-gxbb-vega-s95-telos.dts |  55 +++++++
 .../boot/dts/amlogic/meson-gxbb-vega-s95.dtsi      |  55 +++++++
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi        | 183 +++++++++++++++++++++
 10 files changed, 424 insertions(+)
 create mode 100644 arch/arm64/boot/dts/amlogic/Makefile
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-meta.dts
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-pro.dts
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95-telos.dts
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi

Comments

Mark Rutland March 2, 2016, 1:52 p.m. UTC | #1
On Wed, Mar 02, 2016 at 03:34:55AM +0100, Andreas Färber wrote:
> Hello,
> 
> This series adds initial support for the Amlogic S905 based
> Tronsmart Vega S95 Pro, Meta and Telos TV boxes.
> 
> v2:
> * Pick up previously acked "tronsmart" patch instead (Matthias)
> * Drop ARM_GIC selection (Sudeep)
> * Change some compatible strings (Sudeep, André)
> * Squash some node changes/additions
> 
> Note: On the Vega S95 I need to change TEXT_OFFSET as follows,
> in order to avoid the vendor U-Boot overwriting itself (fwiu);
> for the Mini Mx that's reportedly not necessary.
> 
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 354d75402ace..b7cebdb8b1ce 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -62,7 +62,7 @@ head-y                := arch/arm64/kernel/head.o
>  ifeq ($(CONFIG_ARM64_RANDOMIZE_TEXT_OFFSET), y)
>  TEXT_OFFSET := $(shell awk 'BEGIN {srand(); printf "0x%03x000\n", int(512 * rand())}')
>  else
> -TEXT_OFFSET := 0x00080000
> +TEXT_OFFSET := 0x01080000
>  endif
>  
>  # KASAN_SHADOW_OFFSET = VA_START + (1 << (VA_BITS - 3)) - (1 << 61)

Absolute NAK to this. TEXT_OFFSET is not open for platform-specific
modification.

Why can you not just load the Image 2MB higher regardless? Does the
U-Boot on this platform actually read TEXT_OFFSET and take it into
account?

> This in turn runs into an apparent regression introduced with the
> text offset randomization:
> 
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 6ebd204da16a..afdec27c8871 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -48,7 +48,7 @@
>  #elif (PAGE_OFFSET & 0x1fffff) != 0
>  #error PAGE_OFFSET must be at least 2MB aligned
>  #elif TEXT_OFFSET > 0x1fffff
> -#error TEXT_OFFSET must be less than 2MB
> +//#error TEXT_OFFSET must be less than 2MB
>  #endif
>  
>  #define KERNEL_START   _text

This is not a regression. As above, TEXT_OFFSET is not supposed to be
modified in a platform-specific manner.

Mark.
Andreas Färber March 2, 2016, 2:31 p.m. UTC | #2
Am 02.03.2016 um 14:52 schrieb Mark Rutland:
> On Wed, Mar 02, 2016 at 03:34:55AM +0100, Andreas Färber wrote:
>> Note: On the Vega S95 I need to change TEXT_OFFSET as follows,
>> in order to avoid the vendor U-Boot overwriting itself (fwiu);
>> for the Mini Mx that's reportedly not necessary.
>>
>> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
>> index 354d75402ace..b7cebdb8b1ce 100644
>> --- a/arch/arm64/Makefile
>> +++ b/arch/arm64/Makefile
>> @@ -62,7 +62,7 @@ head-y                := arch/arm64/kernel/head.o
>>  ifeq ($(CONFIG_ARM64_RANDOMIZE_TEXT_OFFSET), y)
>>  TEXT_OFFSET := $(shell awk 'BEGIN {srand(); printf "0x%03x000\n", int(512 * rand())}')
>>  else
>> -TEXT_OFFSET := 0x00080000
>> +TEXT_OFFSET := 0x01080000
>>  endif
>>  
>>  # KASAN_SHADOW_OFFSET = VA_START + (1 << (VA_BITS - 3)) - (1 << 61)
> 
> Absolute NAK to this. TEXT_OFFSET is not open for platform-specific
> modification.

Please read again. There is nothing to NAK here, it's a workaround for
testing my patches on my device! Even my own git queue has it clearly
labeled as "HACK:".

Nothing you say here indicates that this is breaking any particular
kernel feature or damaging the device, so unless you propose a different
way to solve the problem I see no way around it for now.

> Why can you not just load the Image 2MB higher regardless? Does the
> U-Boot on this platform actually read TEXT_OFFSET and take it into
> account?

Yes, U-Boot checks the ELF(?) header and tries to copy the image to the
indicated offset if it isn't loaded there already. The vendor's kernel
has the adjusted offset and works; if I use unmodified mainline kernels
then I get weird exceptions from before entering the kernel, my
assumption being that U-Boot code gets overwritten.

http://openlinux.amlogic.com:8000/download/ARM/u-boot/

This problem might go away if we had a proper upstream-based U-Boot; I'm
not familiar enough with U-Boot to fix that myself and would hate to
mess with U-Boot on eMMC, for lack of JTAG pins on this device.

The Odroid-C2 (which I do not have access to yet) has instructions how
to place U-Boot on an SD card, making it safer to experiment with.
http://odroid.com/dokuwiki/doku.php?id=en:c2_partition_table

>> This in turn runs into an apparent regression introduced with the
>> text offset randomization:
>>
>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> index 6ebd204da16a..afdec27c8871 100644
>> --- a/arch/arm64/kernel/head.S
>> +++ b/arch/arm64/kernel/head.S
>> @@ -48,7 +48,7 @@
>>  #elif (PAGE_OFFSET & 0x1fffff) != 0
>>  #error PAGE_OFFSET must be at least 2MB aligned
>>  #elif TEXT_OFFSET > 0x1fffff
>> -#error TEXT_OFFSET must be less than 2MB
>> +//#error TEXT_OFFSET must be less than 2MB
>>  #endif
>>  
>>  #define KERNEL_START   _text
> 
> This is not a regression. As above, TEXT_OFFSET is not supposed to be
> modified in a platform-specific manner.

It is in fact an unexplained behavioral change in
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=da57a369d3bc5cd61db90f7e9555840381db9b09

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 3ba0fc0..69dafe9 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -37,8 +37,12 @@

 #define KERNEL_RAM_VADDR	(PAGE_OFFSET + TEXT_OFFSET)

-#if (KERNEL_RAM_VADDR & 0xfffff) != 0x80000
-#error KERNEL_RAM_VADDR must start at 0xXXX80000
+#if (TEXT_OFFSET & 0xf) != 0
+#error TEXT_OFFSET must be at least 16B aligned
+#elif (PAGE_OFFSET & 0xfffff) != 0
+#error PAGE_OFFSET must be at least 2MB aligned
+#elif TEXT_OFFSET > 0xfffff
+#error TEXT_OFFSET must be less than 2MB
 #endif

 	.macro	pgtbl, ttb0, ttb1, virt_to_phys

As you can see, previously 0x1080000 was a valid value, and this
regressed with the new randomization feature. It obviously works with
the larger offset for me, so the "must be" seems questionable.

Regards,
Andreas
Mark Rutland March 2, 2016, 3:17 p.m. UTC | #3
On Wed, Mar 02, 2016 at 03:31:52PM +0100, Andreas Färber wrote:
> Am 02.03.2016 um 14:52 schrieb Mark Rutland:
> > On Wed, Mar 02, 2016 at 03:34:55AM +0100, Andreas Färber wrote:
> >> Note: On the Vega S95 I need to change TEXT_OFFSET as follows,
> >> in order to avoid the vendor U-Boot overwriting itself (fwiu);
> >> for the Mini Mx that's reportedly not necessary.
> >>
> >> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> >> index 354d75402ace..b7cebdb8b1ce 100644
> >> --- a/arch/arm64/Makefile
> >> +++ b/arch/arm64/Makefile
> >> @@ -62,7 +62,7 @@ head-y                := arch/arm64/kernel/head.o
> >>  ifeq ($(CONFIG_ARM64_RANDOMIZE_TEXT_OFFSET), y)
> >>  TEXT_OFFSET := $(shell awk 'BEGIN {srand(); printf "0x%03x000\n", int(512 * rand())}')
> >>  else
> >> -TEXT_OFFSET := 0x00080000
> >> +TEXT_OFFSET := 0x01080000
> >>  endif
> >>  
> >>  # KASAN_SHADOW_OFFSET = VA_START + (1 << (VA_BITS - 3)) - (1 << 61)
> > 
> > Absolute NAK to this. TEXT_OFFSET is not open for platform-specific
> > modification.
> 
> Please read again. There is nothing to NAK here, it's a workaround for
> testing my patches on my device! Even my own git queue has it clearly
> labeled as "HACK:".

Sure; I appreciate you need a workaround for the broken bootloader
currently on the device.

I'm simply heading off any attempts to upstream such changes.

> Nothing you say here indicates that this is breaking any particular
> kernel feature or damaging the device, so unless you propose a different
> way to solve the problem I see no way around it for now.

As far as I am aware, this should not result in damage to your device.

However, it runs directly counter to single-image, and other parts of
the kernel _may_ rely on the existing limits on TEXT_OFFSET. So it's not
suitable for upstream, and you _may_ encounter issues as a result.

> > Why can you not just load the Image 2MB higher regardless? Does the
> > U-Boot on this platform actually read TEXT_OFFSET and take it into
> > account?
> 
> Yes, U-Boot checks the ELF(?) header and tries to copy the image to the
> indicated offset if it isn't loaded there already. The vendor's kernel
> has the adjusted offset and works;

So the vendor deliberately changed the kernel Image in an unsupported
fashion, resulting in a divergent boot protocol.

This is incredibly unfortunate.

> if I use unmodified mainline kernels then I get weird exceptions from
> before entering the kernel, my assumption being that U-Boot code gets
> overwritten.
> 
> http://openlinux.amlogic.com:8000/download/ARM/u-boot/
> 
> This problem might go away if we had a proper upstream-based U-Boot; I'm
> not familiar enough with U-Boot to fix that myself and would hate to
> mess with U-Boot on eMMC, for lack of JTAG pins on this device.
 
> The Odroid-C2 (which I do not have access to yet) has instructions how
> to place U-Boot on an SD card, making it safer to experiment with.
> http://odroid.com/dokuwiki/doku.php?id=en:c2_partition_table

This is probably the right path to a solution.

> >> This in turn runs into an apparent regression introduced with the
> >> text offset randomization:
> >>
> >> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> >> index 6ebd204da16a..afdec27c8871 100644
> >> --- a/arch/arm64/kernel/head.S
> >> +++ b/arch/arm64/kernel/head.S
> >> @@ -48,7 +48,7 @@
> >>  #elif (PAGE_OFFSET & 0x1fffff) != 0
> >>  #error PAGE_OFFSET must be at least 2MB aligned
> >>  #elif TEXT_OFFSET > 0x1fffff
> >> -#error TEXT_OFFSET must be less than 2MB
> >> +//#error TEXT_OFFSET must be less than 2MB
> >>  #endif
> >>  
> >>  #define KERNEL_START   _text
> > 
> > This is not a regression. As above, TEXT_OFFSET is not supposed to be
> > modified in a platform-specific manner.
> 
> It is in fact an unexplained behavioral change in
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=da57a369d3bc5cd61db90f7e9555840381db9b09

Yes, strictly speaking it is a change.

No, it is not a regression, since TEXT_OFFSET was _never_ open to such
platform-specific modification.

Thanks,
Mark.
Carlo Caione March 7, 2016, 8:20 a.m. UTC | #4
On Wed, Mar 2, 2016 at 3:34 AM, Andreas Färber <afaerber@suse.de> wrote:
> Hello,
>
> This series adds initial support for the Amlogic S905 based
> Tronsmart Vega S95 Pro, Meta and Telos TV boxes.

Thank you for working on this. If there will be no more comments in
the few next days I'll pick the whole series up.

Cheers,
Kevin Hilman March 21, 2016, 10:36 p.m. UTC | #5
On Tue, Mar 1, 2016 at 6:34 PM, Andreas Färber <afaerber@suse.de> wrote:

> This series adds initial support for the Amlogic S905 based
> Tronsmart Vega S95 Pro, Meta and Telos TV boxes.
>
> v2:
> * Pick up previously acked "tronsmart" patch instead (Matthias)
> * Drop ARM_GIC selection (Sudeep)
> * Change some compatible strings (Sudeep, André)
> * Squash some node changes/additions
>
> Note: On the Vega S95 I need to change TEXT_OFFSET as follows,
> in order to avoid the vendor U-Boot overwriting itself (fwiu);
> for the Mini Mx that's reportedly not necessary.

FYI, the Amlogic P200 dev board also needs this hack with the factory u-boot.

Kevin
Andreas Färber March 22, 2016, 8:29 p.m. UTC | #6
Am 21.03.2016 um 23:36 schrieb Kevin Hilman:
> On Tue, Mar 1, 2016 at 6:34 PM, Andreas Färber <afaerber@suse.de> wrote:
> 
>> Note: On the Vega S95 I need to change TEXT_OFFSET as follows,
>> in order to avoid the vendor U-Boot overwriting itself (fwiu);
>> for the Mini Mx that's reportedly not necessary.
> 
> FYI, the Amlogic P200 dev board also needs this hack with the factory u-boot.

I have meanwhile found that

mkimage -A arm64 -O linux -T kernel -C none -a 0x1080000 -e 0x1080000 \
-n linux-next -d arch/arm64/boot/Image ../uImage

and then using bootm instead of booti works even without the above hack
on the Vega S95. Not a satisfactory solution yet, but better than
patching the kernel in a distro-incompatible way.

Regards,
Andreas
Carlo Caione March 23, 2016, 8:06 a.m. UTC | #7
On Tue, Mar 22, 2016 at 9:29 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 21.03.2016 um 23:36 schrieb Kevin Hilman:
>> On Tue, Mar 1, 2016 at 6:34 PM, Andreas Färber <afaerber@suse.de> wrote:
>>
>>> Note: On the Vega S95 I need to change TEXT_OFFSET as follows,
>>> in order to avoid the vendor U-Boot overwriting itself (fwiu);
>>> for the Mini Mx that's reportedly not necessary.
>>
>> FYI, the Amlogic P200 dev board also needs this hack with the factory u-boot.
>
> I have meanwhile found that
>
> mkimage -A arm64 -O linux -T kernel -C none -a 0x1080000 -e 0x1080000 \
> -n linux-next -d arch/arm64/boot/Image ../uImage
>
> and then using bootm instead of booti works even without the above hack
> on the Vega S95. Not a satisfactory solution yet, but better than
> patching the kernel in a distro-incompatible way.

I wonder if we can add this kind of information in Documentation/arm/meson/.
Probably it could be handy until we have a proper u-boot porting.
Kevin Hilman March 23, 2016, 6:10 p.m. UTC | #8
Andreas Färber <afaerber@suse.de> writes:

> Am 21.03.2016 um 23:36 schrieb Kevin Hilman:
>> On Tue, Mar 1, 2016 at 6:34 PM, Andreas Färber <afaerber@suse.de> wrote:
>> 
>>> Note: On the Vega S95 I need to change TEXT_OFFSET as follows,
>>> in order to avoid the vendor U-Boot overwriting itself (fwiu);
>>> for the Mini Mx that's reportedly not necessary.
>> 
>> FYI, the Amlogic P200 dev board also needs this hack with the factory u-boot.
>
> I have meanwhile found that
>
> mkimage -A arm64 -O linux -T kernel -C none -a 0x1080000 -e 0x1080000 \
> -n linux-next -d arch/arm64/boot/Image ../uImage
>
> and then using bootm instead of booti works even without the above hack
> on the Vega S95. Not a satisfactory solution yet, but better than
> patching the kernel in a distro-incompatible way.

Thanks for sharing.  I can confirm this is working for me too.

Kevin

Patch
diff mbox

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 354d75402ace..b7cebdb8b1ce 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -62,7 +62,7 @@  head-y                := arch/arm64/kernel/head.o
 ifeq ($(CONFIG_ARM64_RANDOMIZE_TEXT_OFFSET), y)
 TEXT_OFFSET := $(shell awk 'BEGIN {srand(); printf "0x%03x000\n", int(512 * rand())}')
 else
-TEXT_OFFSET := 0x00080000
+TEXT_OFFSET := 0x01080000
 endif
 
 # KASAN_SHADOW_OFFSET = VA_START + (1 << (VA_BITS - 3)) - (1 << 61)

This in turn runs into an apparent regression introduced with the
text offset randomization:

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 6ebd204da16a..afdec27c8871 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -48,7 +48,7 @@ 
 #elif (PAGE_OFFSET & 0x1fffff) != 0
 #error PAGE_OFFSET must be at least 2MB aligned
 #elif TEXT_OFFSET > 0x1fffff
-#error TEXT_OFFSET must be less than 2MB
+//#error TEXT_OFFSET must be less than 2MB
 #endif
 
 #define KERNEL_START   _text