[bootwrapper PATCH 0/2] aarch64: Prepare EL3 for MDSELR_EL1

[bootwrapper PATCH 0/2] aarch64: Prepare EL3 for MDSELR_EL1

[Anshuman Khandual]

MDSELR_EL1 register will be used to configure breakpoint and watchpoint
register banks when FEAT_Debugv8p9 is enabled. This series prepares EL3
for MDSELR_EL1 access traps into EL3, and into EL2 via new Fine Grained
Traps Enable 2 (FEAT_FGT2) registers.

Anshuman Khandual (2):
  aarch64: Disable trapping into EL3 while accessing MDSELR_EL1
  aarch64: Disable trapping into EL3 while accessing FEAT_FGT2 registers

 arch/aarch64/include/asm/cpu.h | 3 +++
 arch/aarch64/init.c            | 6 ++++++
 2 files changed, 9 insertions(+)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[bootwrapper PATCH 1/2] aarch64: Disable trapping into EL3 while accessing MDSELR_EL1

[Anshuman Khandual]

This disables trapping into EL3 while accessing MDSELR_EL1 either in EL1 or
EL2 via setting MDCR_EL3.EBWE. But first ensure that FEAT_Debugv8p9 feature
is implemented and enabled looking into ID_AA64DFR0_EL1.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/aarch64/include/asm/cpu.h | 2 ++
 arch/aarch64/init.c            | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index d1f8fd9..124ef91 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -41,6 +41,7 @@
 #define MDCR_EL3_NSTB_NS_NOTRAP			(UL(3) << 24)
 #define MDCR_EL3_SBRBE_NOTRAP_NOPROHIBIT	(UL(3) << 32)
 #define MDCR_EL3_ENPMSN				BIT(36)
+#define MDCR_EL3_EBWE				BIT(43)
 
 #define SCR_EL3_RES1			BITS(5, 4)
 #define SCR_EL3_NS			BIT(0)
@@ -62,6 +63,7 @@
 #define ID_AA64DFR0_EL1_PMSVER		BITS(35, 32)
 #define ID_AA64DFR0_EL1_TRACEBUFFER	BITS(47, 44)
 #define ID_AA64DFR0_EL1_BRBE		BITS(55, 52)
+#define ID_AA64DFR0_EL1_DEBUGVER	BITS(3, 0)
 
 #define ID_AA64ISAR0_EL1_TME		BITS(27, 24)
 
diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
index c4e91e4..37cb45f 100644
--- a/arch/aarch64/init.c
+++ b/arch/aarch64/init.c
@@ -105,6 +105,9 @@ void cpu_init_el3(void)
 	if (mrs_field(ID_AA64DFR0_EL1, BRBE))
 		mdcr |= MDCR_EL3_SBRBE_NOTRAP_NOPROHIBIT;
 
+	if (mrs_field(ID_AA64DFR0_EL1, DEBUGVER) >= 11)
+		mdcr |= MDCR_EL3_EBWE;
+
 	msr(MDCR_EL3, mdcr);
 
 	if (mrs_field(ID_AA64PFR0_EL1, SVE)) {
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[bootwrapper PATCH 2/2] aarch64: Disable trapping into EL3 while accessing FEAT_FGT2 registers

[Anshuman Khandual]

This disables trapping into EL3 while accessing Fine Grained Traps Enable 2
(i.e FEAT_FGT2) registers such as HDFGRTR2_EL2, HDFGWTR2_EL2, HFGITR2_EL2,
HFGRTR2_EL2 and HFGWTR2_EL2 via setting SCR_EL3.FGTEN2. But first ensure
that FEAT_FGT2 feature is implemented looking into ID_AA64MMFR0_EL1.

Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 arch/aarch64/include/asm/cpu.h | 1 +
 arch/aarch64/init.c            | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
index 124ef91..56f319a 100644
--- a/arch/aarch64/include/asm/cpu.h
+++ b/arch/aarch64/include/asm/cpu.h
@@ -57,6 +57,7 @@
 #define SCR_EL3_EnTP2			BIT(41)
 #define SCR_EL3_TCR2EN			BIT(43)
 #define SCR_EL3_PIEN			BIT(45)
+#define SCR_EL3_FGTEN2			BIT(59)
 
 #define HCR_EL2_RES1			BIT(1)
 
diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
index 37cb45f..557266b 100644
--- a/arch/aarch64/init.c
+++ b/arch/aarch64/init.c
@@ -68,6 +68,9 @@ void cpu_init_el3(void)
 	if (mrs_field(ID_AA64MMFR0_EL1, FGT))
 		scr |= SCR_EL3_FGTEN;
 
+	if (mrs_field(ID_AA64MMFR0_EL1, FGT) >= 2)
+		scr |= SCR_EL3_FGTEN2;
+
 	if (mrs_field(ID_AA64MMFR0_EL1, ECV) >= 2)
 		scr |= SCR_EL3_ECVEN;
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [bootwrapper PATCH 0/2] aarch64: Prepare EL3 for MDSELR_EL1

[Anshuman Khandual]

On 4/4/24 13:07, Anshuman Khandual wrote:
> MDSELR_EL1 register will be used to configure breakpoint and watchpoint
> register banks when FEAT_Debugv8p9 is enabled. This series prepares EL3
> for MDSELR_EL1 access traps into EL3, and into EL2 via new Fine Grained
> Traps Enable 2 (FEAT_FGT2) registers.
> 
> Anshuman Khandual (2):
>   aarch64: Disable trapping into EL3 while accessing MDSELR_EL1
>   aarch64: Disable trapping into EL3 while accessing FEAT_FGT2 registers
> 
>  arch/aarch64/include/asm/cpu.h | 3 +++
>  arch/aarch64/init.c            | 6 ++++++
>  2 files changed, 9 insertions(+)
> 

Hello Mark,

Gentle ping, any updates on this series ?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [bootwrapper PATCH 1/2] aarch64: Disable trapping into EL3 while accessing MDSELR_EL1

[Mark Rutland]

On Thu, Apr 04, 2024 at 01:07:25PM +0530, Anshuman Khandual wrote:
> This disables trapping into EL3 while accessing MDSELR_EL1 either in EL1 or
> EL2 via setting MDCR_EL3.EBWE. But first ensure that FEAT_Debugv8p9 feature
> is implemented and enabled looking into ID_AA64DFR0_EL1.

The patch itself looks fine, but I'm going to reword the commit message for clarity:

| aarch64: Enable access to MDSELR_EL1 from EL2 and below
| 
| FEAT_Debugv8p9 adds a new MDSELR_EL1 register to select between banks of
| breakpoints and watchpoints. Accesses to MDSELR_EL1 from EL2 and below trap
| to EL3 unless MDCR_EL3.EBWE is set.
| 
| Enable access to MDSELR_EL1 when it is implemented.

Mark.

> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  arch/aarch64/include/asm/cpu.h | 2 ++
>  arch/aarch64/init.c            | 3 +++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
> index d1f8fd9..124ef91 100644
> --- a/arch/aarch64/include/asm/cpu.h
> +++ b/arch/aarch64/include/asm/cpu.h
> @@ -41,6 +41,7 @@
>  #define MDCR_EL3_NSTB_NS_NOTRAP			(UL(3) << 24)
>  #define MDCR_EL3_SBRBE_NOTRAP_NOPROHIBIT	(UL(3) << 32)
>  #define MDCR_EL3_ENPMSN				BIT(36)
> +#define MDCR_EL3_EBWE				BIT(43)
>  
>  #define SCR_EL3_RES1			BITS(5, 4)
>  #define SCR_EL3_NS			BIT(0)
> @@ -62,6 +63,7 @@
>  #define ID_AA64DFR0_EL1_PMSVER		BITS(35, 32)
>  #define ID_AA64DFR0_EL1_TRACEBUFFER	BITS(47, 44)
>  #define ID_AA64DFR0_EL1_BRBE		BITS(55, 52)
> +#define ID_AA64DFR0_EL1_DEBUGVER	BITS(3, 0)
>  
>  #define ID_AA64ISAR0_EL1_TME		BITS(27, 24)
>  
> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
> index c4e91e4..37cb45f 100644
> --- a/arch/aarch64/init.c
> +++ b/arch/aarch64/init.c
> @@ -105,6 +105,9 @@ void cpu_init_el3(void)
>  	if (mrs_field(ID_AA64DFR0_EL1, BRBE))
>  		mdcr |= MDCR_EL3_SBRBE_NOTRAP_NOPROHIBIT;
>  
> +	if (mrs_field(ID_AA64DFR0_EL1, DEBUGVER) >= 11)
> +		mdcr |= MDCR_EL3_EBWE;
> +
>  	msr(MDCR_EL3, mdcr);
>  
>  	if (mrs_field(ID_AA64PFR0_EL1, SVE)) {
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [bootwrapper PATCH 2/2] aarch64: Disable trapping into EL3 while accessing FEAT_FGT2 registers

[Mark Rutland]

On Thu, Apr 04, 2024 at 01:07:26PM +0530, Anshuman Khandual wrote:
> This disables trapping into EL3 while accessing Fine Grained Traps Enable 2
> (i.e FEAT_FGT2) registers such as HDFGRTR2_EL2, HDFGWTR2_EL2, HFGITR2_EL2,
> HFGRTR2_EL2 and HFGWTR2_EL2 via setting SCR_EL3.FGTEN2. But first ensure
> that FEAT_FGT2 feature is implemented looking into ID_AA64MMFR0_EL1.

I think for clarity that should be:

| aarch64: Enable access to FGT2 registers from EL2 and below
| 
| FEAT_FGT2 adds a number of registers: HFGITR2_EL2, HFGRTR2_EL2,
| HFGWTR_EL2, HDFGRTR2_EL2, and HDFGWTR2_EL2. Acceses to these from EL2
| trap to EL3 unless SCR_EL3.FGTEN2 is set.
| 
| Enable access to the FGT2 registers when they are implemented.

However, I note that those registers have fields that reset to UNKNOWN values
when resets are taken to EL3. That means we must initialise those to sane
values to avoid breaking existing EL2 code with unexpected traps.

I know that we didn't do that for FGT, which really was a mistake.

I reckon we should reset all of the FGT regsiters to sane values (i.e. existing
features shouldn't trap, new features should trap). We'll also need to ensure
that Trusted Firmware does likewise...

Mark.

> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  arch/aarch64/include/asm/cpu.h | 1 +
>  arch/aarch64/init.c            | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
> index 124ef91..56f319a 100644
> --- a/arch/aarch64/include/asm/cpu.h
> +++ b/arch/aarch64/include/asm/cpu.h
> @@ -57,6 +57,7 @@
>  #define SCR_EL3_EnTP2			BIT(41)
>  #define SCR_EL3_TCR2EN			BIT(43)
>  #define SCR_EL3_PIEN			BIT(45)
> +#define SCR_EL3_FGTEN2			BIT(59)
>  
>  #define HCR_EL2_RES1			BIT(1)
>  
> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
> index 37cb45f..557266b 100644
> --- a/arch/aarch64/init.c
> +++ b/arch/aarch64/init.c
> @@ -68,6 +68,9 @@ void cpu_init_el3(void)
>  	if (mrs_field(ID_AA64MMFR0_EL1, FGT))
>  		scr |= SCR_EL3_FGTEN;
>  
> +	if (mrs_field(ID_AA64MMFR0_EL1, FGT) >= 2)
> +		scr |= SCR_EL3_FGTEN2;
> +
>  	if (mrs_field(ID_AA64MMFR0_EL1, ECV) >= 2)
>  		scr |= SCR_EL3_ECVEN;
>  
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [bootwrapper PATCH 0/2] aarch64: Prepare EL3 for MDSELR_EL1

[Mark Rutland]

On Wed, Apr 17, 2024 at 08:55:15AM +0530, Anshuman Khandual wrote:
> On 4/4/24 13:07, Anshuman Khandual wrote:
> > MDSELR_EL1 register will be used to configure breakpoint and watchpoint
> > register banks when FEAT_Debugv8p9 is enabled. This series prepares EL3
> > for MDSELR_EL1 access traps into EL3, and into EL2 via new Fine Grained
> > Traps Enable 2 (FEAT_FGT2) registers.
> > 
> > Anshuman Khandual (2):
> >   aarch64: Disable trapping into EL3 while accessing MDSELR_EL1
> >   aarch64: Disable trapping into EL3 while accessing FEAT_FGT2 registers
> > 
> >  arch/aarch64/include/asm/cpu.h | 3 +++
> >  arch/aarch64/init.c            | 6 ++++++
> >  2 files changed, 9 insertions(+)
> > 
> 
> Hello Mark,
> 
> Gentle ping, any updates on this series ?

I've picked up the first patch, but I don't beleive that the second is
complete. Please see my comments there.

Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [bootwrapper PATCH 1/2] aarch64: Disable trapping into EL3 while accessing MDSELR_EL1

[Anshuman Khandual]

On 4/18/24 15:57, Mark Rutland wrote:
> On Thu, Apr 04, 2024 at 01:07:25PM +0530, Anshuman Khandual wrote:
>> This disables trapping into EL3 while accessing MDSELR_EL1 either in EL1 or
>> EL2 via setting MDCR_EL3.EBWE. But first ensure that FEAT_Debugv8p9 feature
>> is implemented and enabled looking into ID_AA64DFR0_EL1.
> 
> The patch itself looks fine, but I'm going to reword the commit message for clarity:
> 
> | aarch64: Enable access to MDSELR_EL1 from EL2 and below
> | 
> | FEAT_Debugv8p9 adds a new MDSELR_EL1 register to select between banks of
> | breakpoints and watchpoints. Accesses to MDSELR_EL1 from EL2 and below trap
> | to EL3 unless MDCR_EL3.EBWE is set.
> | 
> | Enable access to MDSELR_EL1 when it is implemented.

Sure, makes sense.

> 
> Mark.
> 
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  arch/aarch64/include/asm/cpu.h | 2 ++
>>  arch/aarch64/init.c            | 3 +++
>>  2 files changed, 5 insertions(+)
>>
>> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
>> index d1f8fd9..124ef91 100644
>> --- a/arch/aarch64/include/asm/cpu.h
>> +++ b/arch/aarch64/include/asm/cpu.h
>> @@ -41,6 +41,7 @@
>>  #define MDCR_EL3_NSTB_NS_NOTRAP			(UL(3) << 24)
>>  #define MDCR_EL3_SBRBE_NOTRAP_NOPROHIBIT	(UL(3) << 32)
>>  #define MDCR_EL3_ENPMSN				BIT(36)
>> +#define MDCR_EL3_EBWE				BIT(43)
>>  
>>  #define SCR_EL3_RES1			BITS(5, 4)
>>  #define SCR_EL3_NS			BIT(0)
>> @@ -62,6 +63,7 @@
>>  #define ID_AA64DFR0_EL1_PMSVER		BITS(35, 32)
>>  #define ID_AA64DFR0_EL1_TRACEBUFFER	BITS(47, 44)
>>  #define ID_AA64DFR0_EL1_BRBE		BITS(55, 52)
>> +#define ID_AA64DFR0_EL1_DEBUGVER	BITS(3, 0)
>>  
>>  #define ID_AA64ISAR0_EL1_TME		BITS(27, 24)
>>  
>> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
>> index c4e91e4..37cb45f 100644
>> --- a/arch/aarch64/init.c
>> +++ b/arch/aarch64/init.c
>> @@ -105,6 +105,9 @@ void cpu_init_el3(void)
>>  	if (mrs_field(ID_AA64DFR0_EL1, BRBE))
>>  		mdcr |= MDCR_EL3_SBRBE_NOTRAP_NOPROHIBIT;
>>  
>> +	if (mrs_field(ID_AA64DFR0_EL1, DEBUGVER) >= 11)
>> +		mdcr |= MDCR_EL3_EBWE;
>> +
>>  	msr(MDCR_EL3, mdcr);
>>  
>>  	if (mrs_field(ID_AA64PFR0_EL1, SVE)) {
>> -- 
>> 2.25.1
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [bootwrapper PATCH 2/2] aarch64: Disable trapping into EL3 while accessing FEAT_FGT2 registers

[Anshuman Khandual]

On 4/18/24 16:10, Mark Rutland wrote:
> On Thu, Apr 04, 2024 at 01:07:26PM +0530, Anshuman Khandual wrote:
>> This disables trapping into EL3 while accessing Fine Grained Traps Enable 2
>> (i.e FEAT_FGT2) registers such as HDFGRTR2_EL2, HDFGWTR2_EL2, HFGITR2_EL2,
>> HFGRTR2_EL2 and HFGWTR2_EL2 via setting SCR_EL3.FGTEN2. But first ensure
>> that FEAT_FGT2 feature is implemented looking into ID_AA64MMFR0_EL1.
> 
> I think for clarity that should be:
> 
> | aarch64: Enable access to FGT2 registers from EL2 and below
> | 
> | FEAT_FGT2 adds a number of registers: HFGITR2_EL2, HFGRTR2_EL2,
> | HFGWTR_EL2, HDFGRTR2_EL2, and HDFGWTR2_EL2. Acceses to these from EL2
> | trap to EL3 unless SCR_EL3.FGTEN2 is set.
> | 
> | Enable access to the FGT2 registers when they are implemented.
> 
> However, I note that those registers have fields that reset to UNKNOWN values

Are you referring to the following rules in the register descriptions ?


The reset behavior of this field is:

On a Warm reset:
— When the highest implemented Exception level is EL2, this field resets to 0 .
— Otherwise, this field resets to an architecturally UNKNOWN value. <========

> when resets are taken to EL3. That means we must initialise those to sane
> values to avoid breaking existing EL2 code with unexpected traps.

FEAT_FGT2 is a new feature, so are its corresponding registers as listed
above. Hence I guess resetting them all to zero should be fine ?

        if (mrs_field(ID_AA64MMFR0_EL1, FGT) >= 2) {
                scr |= SCR_EL3_FGTEN2;
                msr(HDFGRTR2_EL2, 0);
                msr(HDFGWTR2_EL2, 0);
                msr(HFGITR2_EL2, 0);
                msr(HFGRTR2_EL2, 0);
                msr(HFGWTR2_EL2, 0);
        }
> 
> I know that we didn't do that for FGT, which really was a mistake.
> 
> I reckon we should reset all of the FGT regsiters to sane values (i.e. existing
> features shouldn't trap, new features should trap). We'll also need to ensure
> that Trusted Firmware does likewise...

Sure, will audit for existing FEAT_FGT register reset values depending on what
is currently supported in the kernel and follow up on with TFA changes as well.

> 
> Mark.
> 
>>
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  arch/aarch64/include/asm/cpu.h | 1 +
>>  arch/aarch64/init.c            | 3 +++
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
>> index 124ef91..56f319a 100644
>> --- a/arch/aarch64/include/asm/cpu.h
>> +++ b/arch/aarch64/include/asm/cpu.h
>> @@ -57,6 +57,7 @@
>>  #define SCR_EL3_EnTP2			BIT(41)
>>  #define SCR_EL3_TCR2EN			BIT(43)
>>  #define SCR_EL3_PIEN			BIT(45)
>> +#define SCR_EL3_FGTEN2			BIT(59)
>>  
>>  #define HCR_EL2_RES1			BIT(1)
>>  
>> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
>> index 37cb45f..557266b 100644
>> --- a/arch/aarch64/init.c
>> +++ b/arch/aarch64/init.c
>> @@ -68,6 +68,9 @@ void cpu_init_el3(void)
>>  	if (mrs_field(ID_AA64MMFR0_EL1, FGT))
>>  		scr |= SCR_EL3_FGTEN;
>>  
>> +	if (mrs_field(ID_AA64MMFR0_EL1, FGT) >= 2)
>> +		scr |= SCR_EL3_FGTEN2;
>> +
>>  	if (mrs_field(ID_AA64MMFR0_EL1, ECV) >= 2)
>>  		scr |= SCR_EL3_ECVEN;
>>  
>> -- 
>> 2.25.1
>>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel