[Qemu-devel] [PATCH v1 0/2] target-arm: Add support for HSTR_EL2

[Qemu-devel] [PATCH v1 0/2] target-arm: Add support for HSTR_EL2

[Alistair Francis]

Now that EL3 is enabled by default for the A53s and A57s I see this
error when booting Linux on the ZynqMP board:
"Synchronous Abort" handler, esr 0x02000000

By adding the HSTR_EL2 register (and EL2 support) the Linux boot
progresses much further.

Alistair Francis (2):
  target-arm: Add the HSTR_EL2 register
  target-arm: Enable EL2 for the A53s and A57s

 target-arm/cpu64.c  | 2 ++
 target-arm/helper.c | 7 +++++++
 2 files changed, 9 insertions(+)

-- 
2.5.0

[Qemu-devel] [PATCH v1 1/2] target-arm: Add the HSTR_EL2 register

[Alistair Francis]

Add the Hypervisor System Trap Register for EL2.

This register is used early in the Linux boot and without it the kernel
aborts with a "Synchronous Abort" error.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 target-arm/helper.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 09638b2..65e8ff1 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3470,6 +3470,9 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
       .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
       .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
       .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "HSTR_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 3,
+      .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
     REGINFO_SENTINEL
 };
 
@@ -3703,6 +3706,10 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
       .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
       .access = PL2_RW,
       .fieldoffset = offsetof(CPUARMState, cp15.hpfar_el2) },
+    { .name = "HSTR_EL2", .state = ARM_CP_STATE_AA64,
+      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 3,
+      .access = PL2_RW,
+      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore, },
     REGINFO_SENTINEL
 };
 
-- 
2.5.0

[Qemu-devel] [PATCH v1 2/2] target-arm: Enable EL2 for the A53s and A57s

[Alistair Francis]

Enable EL2 by default for the A53s and A57s.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 target-arm/cpu64.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index 1635deb..59a2042 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -110,6 +110,7 @@ static void aarch64_a57_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
     set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
     set_feature(&cpu->env, ARM_FEATURE_CRC);
+    set_feature(&cpu->env, ARM_FEATURE_EL2);
     set_feature(&cpu->env, ARM_FEATURE_EL3);
     cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A57;
     cpu->midr = 0x411fd070;
@@ -165,6 +166,7 @@ static void aarch64_a53_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_V8_SHA256);
     set_feature(&cpu->env, ARM_FEATURE_V8_PMULL);
     set_feature(&cpu->env, ARM_FEATURE_CRC);
+    set_feature(&cpu->env, ARM_FEATURE_EL2);
     set_feature(&cpu->env, ARM_FEATURE_EL3);
     cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A53;
     cpu->midr = 0x410fd034;
-- 
2.5.0

Re: [Qemu-devel] [PATCH v1 2/2] target-arm: Enable EL2 for the A53s and A57s

[Peter Maydell]

On 6 May 2016 at 19:11, Alistair Francis <alistair.francis@xilinx.com> wrote:
> Enable EL2 by default for the A53s and A57s.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>

I don't really want to do this until we actually have
something approaching a workable EL2 implementation
(ideally including support in the GIC, since EL2's
not much good without it). I'd hope that software ought
to be able to work with an "EL3, but no EL2" CPU, since
that's a valid combination, unless it's very strongly
tied to a particular h/w implementation.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 2/2] target-arm: Enable EL2 for the A53s and A57s

[Edgar E. Iglesias]

On Fri, May 06, 2016 at 11:25:52PM +0100, Peter Maydell wrote:
> On 6 May 2016 at 19:11, Alistair Francis <alistair.francis@xilinx.com> wrote:
> > Enable EL2 by default for the A53s and A57s.
> >
> > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> 
> I don't really want to do this until we actually have
> something approaching a workable EL2 implementation
> (ideally including support in the GIC, since EL2's
> not much good without it). I'd hope that software ought
> to be able to work with an "EL3, but no EL2" CPU, since
> that's a valid combination, unless it's very strongly
> tied to a particular h/w implementation.

Yes, I agree, we've got a few more things to take care of.

Cheers,
Edgar

Re: [Qemu-devel] [PATCH v1 2/2] target-arm: Enable EL2 for the A53s and A57s

[Alistair Francis]

On Fri, May 6, 2016 at 6:51 PM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> On Fri, May 06, 2016 at 11:25:52PM +0100, Peter Maydell wrote:
>> On 6 May 2016 at 19:11, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> > Enable EL2 by default for the A53s and A57s.
>> >
>> > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>
>> I don't really want to do this until we actually have
>> something approaching a workable EL2 implementation
>> (ideally including support in the GIC, since EL2's
>> not much good without it). I'd hope that software ought
>> to be able to work with an "EL3, but no EL2" CPU, since
>> that's a valid combination, unless it's very strongly
>> tied to a particular h/w implementation.
>
> Yes, I agree, we've got a few more things to take care of.

I haven't been keeping up with the EL2 status on mainline, so if you
guys don't want to enable it yet that's fine with me. Just thought I'd
try.

I assume you don't have any fundamental problems with the first patch
then, so hopefully at least that can get in.

I might have to look at disabling EL3 support for the ZynqMP machine
form the command line, as I can't boot Linux with EL3 but not EL2.

Thanks,

Alistair

>
> Cheers,
> Edgar
>

Re: [Qemu-devel] [PATCH v1 2/2] target-arm: Enable EL2 for the A53s and A57s

[Peter Maydell]

On 9 May 2016 at 17:58, Alistair Francis <alistair.francis@xilinx.com> wrote:
> I might have to look at disabling EL3 support for the ZynqMP machine
> form the command line, as I can't boot Linux with EL3 but not EL2.

I think it's worth trying to find out why this doesn't work:
I'd hope that Linux can cope with a no-EL2 setup.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 2/2] target-arm: Enable EL2 for the A53s and A57s

[Alistair Francis]

On Mon, May 9, 2016 at 10:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 9 May 2016 at 17:58, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> I might have to look at disabling EL3 support for the ZynqMP machine
>> form the command line, as I can't boot Linux with EL3 but not EL2.
>
> I think it's worth trying to find out why this doesn't work:
> I'd hope that Linux can cope with a no-EL2 setup.

It is actually a u-boot problem. I originally just assumed it was a
Linux problem, but it happens before u-boot hands off to Linux.

It appears that u-boot won't work at all with EL3 enabled but EL2
disabled. It always moves to EL2 before moving to EL1 and there is no
code prepared to handle going from EL3 directly to EL1.

Just for the record, I'm specifically talking about what happens in
the do_nonsec_virt_switch() function.

It looks like there are three options:
 1. Add support to u-boot to drop from EL3 to EL1 (I'm assuming this
is possible, as not all implementations have EL2)
 2. Just wait until QEMU adds EL2 support
 3. Allow a QEMU command line option to start in EL1 instead of EL3

Thanks,

Alistair

>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH v1 2/2] target-arm: Enable EL2 for the A53s and A57s

[Peter Maydell]

On 10 May 2016 at 01:16, Alistair Francis <alistair.francis@xilinx.com> wrote:
> It is actually a u-boot problem. I originally just assumed it was a
> Linux problem, but it happens before u-boot hands off to Linux.

OK, that makes sense. u-boot tends to be a bit lower level and less
hardware-agnostic. I just wanted to check it wasn't caused by some
problem in QEMU's EL3 support we could easily fix.

> It appears that u-boot won't work at all with EL3 enabled but EL2
> disabled. It always moves to EL2 before moving to EL1 and there is no
> code prepared to handle going from EL3 directly to EL1.
>
> Just for the record, I'm specifically talking about what happens in
> the do_nonsec_virt_switch() function.
>
> It looks like there are three options:
>  1. Add support to u-boot to drop from EL3 to EL1 (I'm assuming this
> is possible, as not all implementations have EL2)
>  2. Just wait until QEMU adds EL2 support
>  3. Allow a QEMU command line option to start in EL1 instead of EL3

I would be OK with any of these three from a QEMU perspective.
Fixing u-boot is probably conceptually the nicest but I've never
looked at u-boot internals so it could be simple or painful...

Edgar, do you have the list of what we're still missing before we
can turn on EL2?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 2/2] target-arm: Enable EL2 for the A53s and A57s

[Edgar E. Iglesias]

On Tue, May 10, 2016 at 09:09:55AM +0100, Peter Maydell wrote:
> On 10 May 2016 at 01:16, Alistair Francis <alistair.francis@xilinx.com> wrote:
> > It is actually a u-boot problem. I originally just assumed it was a
> > Linux problem, but it happens before u-boot hands off to Linux.
> 
> OK, that makes sense. u-boot tends to be a bit lower level and less
> hardware-agnostic. I just wanted to check it wasn't caused by some
> problem in QEMU's EL3 support we could easily fix.
> 
> > It appears that u-boot won't work at all with EL3 enabled but EL2
> > disabled. It always moves to EL2 before moving to EL1 and there is no
> > code prepared to handle going from EL3 directly to EL1.
> >
> > Just for the record, I'm specifically talking about what happens in
> > the do_nonsec_virt_switch() function.
> >
> > It looks like there are three options:
> >  1. Add support to u-boot to drop from EL3 to EL1 (I'm assuming this
> > is possible, as not all implementations have EL2)
> >  2. Just wait until QEMU adds EL2 support
> >  3. Allow a QEMU command line option to start in EL1 instead of EL3

Alistair,

Our usual ZynqMP boot flow is

ATF at EL3
ATF hands over to u-boot at EL2
u-boot hands over to the Linux kernel at EL2

Our ATF has a mode where it can be instructed to start u-boot in
EL1, bypassing EL2. That would avoid this issue. You don't need to
recompile ATF. We can discuss the details off-line.


> I would be OK with any of these three from a QEMU perspective.
> Fixing u-boot is probably conceptually the nicest but I've never
> looked at u-boot internals so it could be simple or painful...
> 
> Edgar, do you have the list of what we're still missing before we
> can turn on EL2?

We're missing the Data Abort ISS:s, we're missing modelling of a few registers
(like HSTR_EL2). Some regs are quite incomplete like HCR but I think we can
live with that. GICv2 virtualization is missing. We are also missing a few
AT_ modes. AArch32 support is probably lacking alot of more stuff.

IIRC, last time this came up we talked about enabling EL2 after ISS and
GICv2 virtualization support gets merged + a few odd regs. Enough to allow
Xen and KVM to work under emulation.

Cheers,
Edgar

Re: [Qemu-devel] [PATCH v1 2/2] target-arm: Enable EL2 for the A53s and A57s

[Peter Maydell]

On 10 May 2016 at 11:36, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Tue, May 10, 2016 at 09:09:55AM +0100, Peter Maydell wrote:
>> Edgar, do you have the list of what we're still missing before we
>> can turn on EL2?
>
> We're missing the Data Abort ISS:s, we're missing modelling of a few registers
> (like HSTR_EL2). Some regs are quite incomplete like HCR but I think we can
> live with that. GICv2 virtualization is missing. We are also missing a few
> AT_ modes. AArch32 support is probably lacking alot of more stuff.
>
> IIRC, last time this came up we talked about enabling EL2 after ISS and
> GICv2 virtualization support gets merged + a few odd regs. Enough to allow
> Xen and KVM to work under emulation.

Yeah, my practical guide for when we can turn on EL2 would be
"when you can run at least one of Xen and KVM, and not break
booting of Linux", even if we're not 100% feature-complete.
I think GIC virt support is the biggie here.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 2/2] target-arm: Enable EL2 for the A53s and A57s

[Alistair Francis]

On Tue, May 10, 2016 at 1:09 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 10 May 2016 at 01:16, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> It is actually a u-boot problem. I originally just assumed it was a
>> Linux problem, but it happens before u-boot hands off to Linux.
>
> OK, that makes sense. u-boot tends to be a bit lower level and less
> hardware-agnostic. I just wanted to check it wasn't caused by some
> problem in QEMU's EL3 support we could easily fix.
>
>> It appears that u-boot won't work at all with EL3 enabled but EL2
>> disabled. It always moves to EL2 before moving to EL1 and there is no
>> code prepared to handle going from EL3 directly to EL1.
>>
>> Just for the record, I'm specifically talking about what happens in
>> the do_nonsec_virt_switch() function.
>>
>> It looks like there are three options:
>>  1. Add support to u-boot to drop from EL3 to EL1 (I'm assuming this
>> is possible, as not all implementations have EL2)
>>  2. Just wait until QEMU adds EL2 support
>>  3. Allow a QEMU command line option to start in EL1 instead of EL3
>
> I would be OK with any of these three from a QEMU perspective.
> Fixing u-boot is probably conceptually the nicest but I've never
> looked at u-boot internals so it could be simple or painful...

I had a quick look and it didn't look easy to do unfortunately.

Thanks,

Alistair

>
> Edgar, do you have the list of what we're still missing before we
> can turn on EL2?
>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH v1 2/2] target-arm: Enable EL2 for the A53s and A57s

[Alistair Francis]

On Tue, May 10, 2016 at 3:36 AM, Edgar E. Iglesias
<edgar.iglesias@gmail.com> wrote:
> On Tue, May 10, 2016 at 09:09:55AM +0100, Peter Maydell wrote:
>> On 10 May 2016 at 01:16, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> > It is actually a u-boot problem. I originally just assumed it was a
>> > Linux problem, but it happens before u-boot hands off to Linux.
>>
>> OK, that makes sense. u-boot tends to be a bit lower level and less
>> hardware-agnostic. I just wanted to check it wasn't caused by some
>> problem in QEMU's EL3 support we could easily fix.
>>
>> > It appears that u-boot won't work at all with EL3 enabled but EL2
>> > disabled. It always moves to EL2 before moving to EL1 and there is no
>> > code prepared to handle going from EL3 directly to EL1.
>> >
>> > Just for the record, I'm specifically talking about what happens in
>> > the do_nonsec_virt_switch() function.
>> >
>> > It looks like there are three options:
>> >  1. Add support to u-boot to drop from EL3 to EL1 (I'm assuming this
>> > is possible, as not all implementations have EL2)
>> >  2. Just wait until QEMU adds EL2 support
>> >  3. Allow a QEMU command line option to start in EL1 instead of EL3
>
> Alistair,
>
> Our usual ZynqMP boot flow is
>
> ATF at EL3
> ATF hands over to u-boot at EL2
> u-boot hands over to the Linux kernel at EL2
>
> Our ATF has a mode where it can be instructed to start u-boot in
> EL1, bypassing EL2. That would avoid this issue. You don't need to
> recompile ATF. We can discuss the details off-line.

Hey Edgar,

Unfortunately that doesn't work with JTAG boot mode, which is what we are using.

Just to confirm it though I edited the ATF source to handoff to u-boot
in EL1. Then using my device loader patches I managed to boot ATF to
u-boot to Linux. So besides the missing EL2 support the general flow
seems to work. Linux tried to execute code outside RAM or ROM, so I
never made it to a login prompt though.

>
>
>> I would be OK with any of these three from a QEMU perspective.
>> Fixing u-boot is probably conceptually the nicest but I've never
>> looked at u-boot internals so it could be simple or painful...
>>
>> Edgar, do you have the list of what we're still missing before we
>> can turn on EL2?
>
> We're missing the Data Abort ISS:s, we're missing modelling of a few registers
> (like HSTR_EL2). Some regs are quite incomplete like HCR but I think we can
> live with that. GICv2 virtualization is missing. We are also missing a few
> AT_ modes. AArch32 support is probably lacking alot of more stuff.
>
> IIRC, last time this came up we talked about enabling EL2 after ISS and
> GICv2 virtualization support gets merged + a few odd regs. Enough to allow
> Xen and KVM to work under emulation.

Do you have a rough time frame on this? Right now I can't see any way
to boot Linux on the up-streamed ZynqMP machine. If EL2 support will
take a long time we might need to look into one of the alternative
work arounds for the time being.

>
> Cheers,
> Edgar
>

Re: [Qemu-devel] [PATCH v1 1/2] target-arm: Add the HSTR_EL2 register

[Peter Maydell]

On 6 May 2016 at 19:11, Alistair Francis <alistair.francis@xilinx.com> wrote:
> Add the Hypervisor System Trap Register for EL2.
>
> This register is used early in the Linux boot and without it the kernel
> aborts with a "Synchronous Abort" error.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
>  target-arm/helper.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 09638b2..65e8ff1 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -3470,6 +3470,9 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
>        .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
>        .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
>        .type = ARM_CP_CONST, .resetvalue = 0 },
> +    { .name = "HSTR_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 3,
> +      .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
>      REGINFO_SENTINEL
>  };
>
> @@ -3703,6 +3706,10 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>        .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
>        .access = PL2_RW,
>        .fieldoffset = offsetof(CPUARMState, cp15.hpfar_el2) },
> +    { .name = "HSTR_EL2", .state = ARM_CP_STATE_AA64,
> +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 3,
> +      .access = PL2_RW,
> +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore, },
>      REGINFO_SENTINEL
>  };

Can we have the 32-bit version of the register too, please? (I think
it should be possible to do this with a STATE_BOTH stanza).

Can we have the el2_cp_reginfo implemented to be a reads-as-written
to a hstr_el2 field in the env->cp15 struct? That way we won't have
to come back to the regdef struct as and when we actually implement
the trap bits to do something...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 1/2] target-arm: Add the HSTR_EL2 register

[Alistair Francis]

On Fri, May 13, 2016 at 10:46 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 6 May 2016 at 19:11, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> Add the Hypervisor System Trap Register for EL2.
>>
>> This register is used early in the Linux boot and without it the kernel
>> aborts with a "Synchronous Abort" error.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>>
>>  target-arm/helper.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index 09638b2..65e8ff1 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -3470,6 +3470,9 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
>>        .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
>>        .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
>>        .type = ARM_CP_CONST, .resetvalue = 0 },
>> +    { .name = "HSTR_EL2", .state = ARM_CP_STATE_AA64,
>> +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 3,
>> +      .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
>>      REGINFO_SENTINEL
>>  };
>>
>> @@ -3703,6 +3706,10 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
>>        .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
>>        .access = PL2_RW,
>>        .fieldoffset = offsetof(CPUARMState, cp15.hpfar_el2) },
>> +    { .name = "HSTR_EL2", .state = ARM_CP_STATE_AA64,
>> +      .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 3,
>> +      .access = PL2_RW,
>> +      .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore, },
>>      REGINFO_SENTINEL
>>  };
>
> Can we have the 32-bit version of the register too, please? (I think
> it should be possible to do this with a STATE_BOTH stanza).

Yep, I think I added it correctly.

>
> Can we have the el2_cp_reginfo implemented to be a reads-as-written
> to a hstr_el2 field in the env->cp15 struct? That way we won't have
> to come back to the regdef struct as and when we actually implement
> the trap bits to do something...

Yep, I have added that as well. Sending V2 now.

Thanks,

Alistair

>
> thanks
> -- PMM
>