* [Qemu-devel] [PATCH] target/arm: fix CBAR register for AArch64 CPUs
@ 2019-09-12 11:01 Luc Michel
2019-09-12 16:03 ` Peter Maydell
0 siblings, 1 reply; 6+ messages in thread
From: Luc Michel @ 2019-09-12 11:01 UTC (permalink / raw)
To: qemu-devel, qemu-arm, Peter Maydell; +Cc: Luc Michel
For AArch64 CPUs with a CBAR register, we have two views for it:
- in AArch64 state, the CBAR_EL1 register (S3_1_C15_C3_0), returns the
full 64 bits CBAR value
- in AArch32 state, the CBAR register (cp15, opc1=1, CRn=15, CRm=3, opc2=0)
returns a 32 bits view such that:
CBAR = CBAR_EL1[31:18] 0..0 CBAR_EL1[43:32]
This commit fixes the current implementation where:
- CBAR_EL1 was returning the 32 bits view instead of the full 64 bits
value,
- CBAR was returning a truncated 32 bits version of the full 64 bits
one, instead of the 32 bits view
- CBAR was declared as cp15, opc1=4, CRn=15, CRm=0, opc2=0, which is
the CBAR register found in the ARMv7 Cortex-Ax CPUs, but not in
ARMv8 CPUs.
Signed-off-by: Luc Michel <luc.michel@greensocs.com>
---
target/arm/helper.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 507026c915..755aa18a2d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6740,12 +6740,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
ARMCPRegInfo cbar_reginfo[] = {
{ .name = "CBAR",
.type = ARM_CP_CONST,
- .cp = 15, .crn = 15, .crm = 0, .opc1 = 4, .opc2 = 0,
- .access = PL1_R, .resetvalue = cpu->reset_cbar },
+ .cp = 15, .crn = 15, .crm = 3, .opc1 = 1, .opc2 = 0,
+ .access = PL1_R, .resetvalue = cbar32 },
{ .name = "CBAR_EL1", .state = ARM_CP_STATE_AA64,
.type = ARM_CP_CONST,
.opc0 = 3, .opc1 = 1, .crn = 15, .crm = 3, .opc2 = 0,
- .access = PL1_R, .resetvalue = cbar32 },
+ .access = PL1_R, .resetvalue = cpu->reset_cbar },
REGINFO_SENTINEL
};
/* We don't implement a r/w 64 bit CBAR currently */
--
2.23.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: fix CBAR register for AArch64 CPUs
2019-09-12 11:01 [Qemu-devel] [PATCH] target/arm: fix CBAR register for AArch64 CPUs Luc Michel
@ 2019-09-12 16:03 ` Peter Maydell
2019-09-13 7:26 ` Luc Michel
0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2019-09-12 16:03 UTC (permalink / raw)
To: Luc Michel; +Cc: qemu-arm, QEMU Developers
On Thu, 12 Sep 2019 at 12:01, Luc Michel <luc.michel@greensocs.com> wrote:
>
> For AArch64 CPUs with a CBAR register, we have two views for it:
> - in AArch64 state, the CBAR_EL1 register (S3_1_C15_C3_0), returns the
> full 64 bits CBAR value
> - in AArch32 state, the CBAR register (cp15, opc1=1, CRn=15, CRm=3, opc2=0)
> returns a 32 bits view such that:
> CBAR = CBAR_EL1[31:18] 0..0 CBAR_EL1[43:32]
>
> This commit fixes the current implementation where:
> - CBAR_EL1 was returning the 32 bits view instead of the full 64 bits
> value,
> - CBAR was returning a truncated 32 bits version of the full 64 bits
> one, instead of the 32 bits view
> - CBAR was declared as cp15, opc1=4, CRn=15, CRm=0, opc2=0, which is
> the CBAR register found in the ARMv7 Cortex-Ax CPUs, but not in
> ARMv8 CPUs.
>
> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
> ---
> target/arm/helper.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 507026c915..755aa18a2d 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6740,12 +6740,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> ARMCPRegInfo cbar_reginfo[] = {
> { .name = "CBAR",
> .type = ARM_CP_CONST,
> - .cp = 15, .crn = 15, .crm = 0, .opc1 = 4, .opc2 = 0,
> - .access = PL1_R, .resetvalue = cpu->reset_cbar },
> + .cp = 15, .crn = 15, .crm = 3, .opc1 = 1, .opc2 = 0,
> + .access = PL1_R, .resetvalue = cbar32 },
This will break the Cortex-A9 &c which use the 15/0/4/0 encoding
and the un-rearranged value for this register.
I think we need to check through the TRMs to confirm which CPUs use
which format for the CBAR, and have a different feature bit for the
newer format/sysreg encoding, so we can provide the right sysregs for
the right cores.
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: fix CBAR register for AArch64 CPUs
2019-09-12 16:03 ` Peter Maydell
@ 2019-09-13 7:26 ` Luc Michel
2019-09-17 8:43 ` Luc Michel
0 siblings, 1 reply; 6+ messages in thread
From: Luc Michel @ 2019-09-13 7:26 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers
Hi Peter,
On 9/12/19 6:03 PM, Peter Maydell wrote:
> On Thu, 12 Sep 2019 at 12:01, Luc Michel <luc.michel@greensocs.com> wrote:
>>
>> For AArch64 CPUs with a CBAR register, we have two views for it:
>> - in AArch64 state, the CBAR_EL1 register (S3_1_C15_C3_0), returns the
>> full 64 bits CBAR value
>> - in AArch32 state, the CBAR register (cp15, opc1=1, CRn=15, CRm=3, opc2=0)
>> returns a 32 bits view such that:
>> CBAR = CBAR_EL1[31:18] 0..0 CBAR_EL1[43:32]
>>
>> This commit fixes the current implementation where:
>> - CBAR_EL1 was returning the 32 bits view instead of the full 64 bits
>> value,
>> - CBAR was returning a truncated 32 bits version of the full 64 bits
>> one, instead of the 32 bits view
>> - CBAR was declared as cp15, opc1=4, CRn=15, CRm=0, opc2=0, which is
>> the CBAR register found in the ARMv7 Cortex-Ax CPUs, but not in
>> ARMv8 CPUs.
>>
>> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
>> ---
>> target/arm/helper.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index 507026c915..755aa18a2d 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -6740,12 +6740,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>> ARMCPRegInfo cbar_reginfo[] = {
>> { .name = "CBAR",
>> .type = ARM_CP_CONST,
>> - .cp = 15, .crn = 15, .crm = 0, .opc1 = 4, .opc2 = 0,
>> - .access = PL1_R, .resetvalue = cpu->reset_cbar },
>> + .cp = 15, .crn = 15, .crm = 3, .opc1 = 1, .opc2 = 0,
>> + .access = PL1_R, .resetvalue = cbar32 },
>
> This will break the Cortex-A9 &c which use the 15/0/4/0 encoding
> and the un-rearranged value for this register.
I don't think so because we are in the "if (arm_feature(env,
ARM_FEATURE_AARCH64))" branch of the code. The else branch still maps
15/0/4/0 for non-AArch64 CPUs.
>
> I think we need to check through the TRMs to confirm which CPUs use
> which format for the CBAR, and have a different feature bit for the
> newer format/sysreg encoding, so we can provide the right sysregs for
> the right cores.
I checked all the AArch64 Cortex's TRMs, for those having a PERIPHBASE
signal and CBAR register (namely Cortex-A53, 57, 72, 73), they all match
the mapping I put in this patch, so I think we don't need to split the
CBAR feature further. I believe more recent Cortex's address the GIC
using coprocessor registers, and CBAR does not exist in those ones.
--
Luc
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: fix CBAR register for AArch64 CPUs
2019-09-13 7:26 ` Luc Michel
@ 2019-09-17 8:43 ` Luc Michel
2019-09-17 10:56 ` Peter Maydell
0 siblings, 1 reply; 6+ messages in thread
From: Luc Michel @ 2019-09-17 8:43 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers
On 9/13/19 9:26 AM, Luc Michel wrote:
> Hi Peter,
>
> On 9/12/19 6:03 PM, Peter Maydell wrote:
>> On Thu, 12 Sep 2019 at 12:01, Luc Michel <luc.michel@greensocs.com> wrote:
>>>
>>> For AArch64 CPUs with a CBAR register, we have two views for it:
>>> - in AArch64 state, the CBAR_EL1 register (S3_1_C15_C3_0), returns the
>>> full 64 bits CBAR value
>>> - in AArch32 state, the CBAR register (cp15, opc1=1, CRn=15, CRm=3, opc2=0)
>>> returns a 32 bits view such that:
>>> CBAR = CBAR_EL1[31:18] 0..0 CBAR_EL1[43:32]
>>>
>>> This commit fixes the current implementation where:
>>> - CBAR_EL1 was returning the 32 bits view instead of the full 64 bits
>>> value,
>>> - CBAR was returning a truncated 32 bits version of the full 64 bits
>>> one, instead of the 32 bits view
>>> - CBAR was declared as cp15, opc1=4, CRn=15, CRm=0, opc2=0, which is
>>> the CBAR register found in the ARMv7 Cortex-Ax CPUs, but not in
>>> ARMv8 CPUs.
>>>
>>> Signed-off-by: Luc Michel <luc.michel@greensocs.com>
>>> ---
>>> target/arm/helper.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>>> index 507026c915..755aa18a2d 100644
>>> --- a/target/arm/helper.c
>>> +++ b/target/arm/helper.c
>>> @@ -6740,12 +6740,12 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>>> ARMCPRegInfo cbar_reginfo[] = {
>>> { .name = "CBAR",
>>> .type = ARM_CP_CONST,
>>> - .cp = 15, .crn = 15, .crm = 0, .opc1 = 4, .opc2 = 0,
>>> - .access = PL1_R, .resetvalue = cpu->reset_cbar },
>>> + .cp = 15, .crn = 15, .crm = 3, .opc1 = 1, .opc2 = 0,
>>> + .access = PL1_R, .resetvalue = cbar32 },
>>
>> This will break the Cortex-A9 &c which use the 15/0/4/0 encoding
>> and the un-rearranged value for this register.
> I don't think so because we are in the "if (arm_feature(env,
> ARM_FEATURE_AARCH64))" branch of the code. The else branch still maps
> 15/0/4/0 for non-AArch64 CPUs.
>
>>
>> I think we need to check through the TRMs to confirm which CPUs use
>> which format for the CBAR, and have a different feature bit for the
>> newer format/sysreg encoding, so we can provide the right sysregs for
>> the right cores.
> I checked all the AArch64 Cortex's TRMs, for those having a PERIPHBASE
> signal and CBAR register (namely Cortex-A53, 57, 72, 73), they all match
> the mapping I put in this patch, so I think we don't need to split the
> CBAR feature further. I believe more recent Cortex's address the GIC
> using coprocessor registers, and CBAR does not exist in those ones.
Hi Peter,
Do you want me to re-roll this patch with some modifications?
Thanks.
--
Luc
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: fix CBAR register for AArch64 CPUs
2019-09-17 8:43 ` Luc Michel
@ 2019-09-17 10:56 ` Peter Maydell
2019-09-17 12:52 ` Luc Michel
0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2019-09-17 10:56 UTC (permalink / raw)
To: Luc Michel; +Cc: qemu-arm, QEMU Developers
On Tue, 17 Sep 2019 at 09:43, Luc Michel <luc.michel@greensocs.com> wrote:
>
> On 9/13/19 9:26 AM, Luc Michel wrote:
> > Hi Peter,
> >
> > On 9/12/19 6:03 PM, Peter Maydell wrote:
> >> I think we need to check through the TRMs to confirm which CPUs use
> >> which format for the CBAR, and have a different feature bit for the
> >> newer format/sysreg encoding, so we can provide the right sysregs for
> >> the right cores.
> > I checked all the AArch64 Cortex's TRMs, for those having a PERIPHBASE
> > signal and CBAR register (namely Cortex-A53, 57, 72, 73), they all match
> > the mapping I put in this patch, so I think we don't need to split the
> > CBAR feature further. I believe more recent Cortex's address the GIC
> > using coprocessor registers, and CBAR does not exist in those ones.
>
> Hi Peter,
>
> Do you want me to re-roll this patch with some modifications?
No, that's OK. Thanks for checking the TRMs. I think what I'll
do is squash in this comment to the patch:
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 755aa18a2dc..bc1130d989d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6733,6 +6733,19 @@ void register_cp_regs_for_features(ARMCPU *cpu)
}
if (arm_feature(env, ARM_FEATURE_CBAR)) {
+ /*
+ * CBAR is IMPDEF, but common on Arm Cortex-A implementations.
+ * There are two flavours:
+ * (1) older 32-bit only cores have a simple 32-bit CBAR
+ * (2) 64-bit cores have a 64-bit CBAR visible to AArch64, plus a
+ * 32-bit register visible to AArch32 at a different encoding
+ * to the "flavour 1" register and with the bits rearranged to
+ * be able to squash a 64-bit address into the 32-bit view.
+ * We distinguish the two via the ARM_FEATURE_AARCH64 flag, but
+ * in future if we support AArch32-only configs of some of the
+ * AArch64 cores we might need to add a specific feature flag
+ * to indicate cores with "flavour 2" CBAR.
+ */
if (arm_feature(env, ARM_FEATURE_AARCH64)) {
/* 32 bit view is [31:18] 0...0 [43:32]. */
uint32_t cbar32 = (extract64(cpu->reset_cbar, 18, 14) << 18)
and apply it to target-arm.next, if that's OK with you.
thanks
-- PMM
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] target/arm: fix CBAR register for AArch64 CPUs
2019-09-17 10:56 ` Peter Maydell
@ 2019-09-17 12:52 ` Luc Michel
0 siblings, 0 replies; 6+ messages in thread
From: Luc Michel @ 2019-09-17 12:52 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, QEMU Developers
On 9/17/19 12:56 PM, Peter Maydell wrote:
> On Tue, 17 Sep 2019 at 09:43, Luc Michel <luc.michel@greensocs.com> wrote:
>>
>> On 9/13/19 9:26 AM, Luc Michel wrote:
>>> Hi Peter,
>>>
>>> On 9/12/19 6:03 PM, Peter Maydell wrote:
>>>> I think we need to check through the TRMs to confirm which CPUs use
>>>> which format for the CBAR, and have a different feature bit for the
>>>> newer format/sysreg encoding, so we can provide the right sysregs for
>>>> the right cores.
>>> I checked all the AArch64 Cortex's TRMs, for those having a PERIPHBASE
>>> signal and CBAR register (namely Cortex-A53, 57, 72, 73), they all match
>>> the mapping I put in this patch, so I think we don't need to split the
>>> CBAR feature further. I believe more recent Cortex's address the GIC
>>> using coprocessor registers, and CBAR does not exist in those ones.
>>
>> Hi Peter,
>>
>> Do you want me to re-roll this patch with some modifications?
>
> No, that's OK. Thanks for checking the TRMs. I think what I'll
> do is squash in this comment to the patch:
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 755aa18a2dc..bc1130d989d 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6733,6 +6733,19 @@ void register_cp_regs_for_features(ARMCPU *cpu)
> }
>
> if (arm_feature(env, ARM_FEATURE_CBAR)) {
> + /*
> + * CBAR is IMPDEF, but common on Arm Cortex-A implementations.
> + * There are two flavours:
> + * (1) older 32-bit only cores have a simple 32-bit CBAR
> + * (2) 64-bit cores have a 64-bit CBAR visible to AArch64, plus a
> + * 32-bit register visible to AArch32 at a different encoding
> + * to the "flavour 1" register and with the bits rearranged to
> + * be able to squash a 64-bit address into the 32-bit view.
> + * We distinguish the two via the ARM_FEATURE_AARCH64 flag, but
> + * in future if we support AArch32-only configs of some of the
> + * AArch64 cores we might need to add a specific feature flag
> + * to indicate cores with "flavour 2" CBAR.
> + */
> if (arm_feature(env, ARM_FEATURE_AARCH64)) {
> /* 32 bit view is [31:18] 0...0 [43:32]. */
> uint32_t cbar32 = (extract64(cpu->reset_cbar, 18, 14) << 18)
>
>
> and apply it to target-arm.next, if that's OK with you.
Yes sure!
Thanks.
--
Luc
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-17 12:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-12 11:01 [Qemu-devel] [PATCH] target/arm: fix CBAR register for AArch64 CPUs Luc Michel
2019-09-12 16:03 ` Peter Maydell
2019-09-13 7:26 ` Luc Michel
2019-09-17 8:43 ` Luc Michel
2019-09-17 10:56 ` Peter Maydell
2019-09-17 12:52 ` Luc Michel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).