SPE data source filtering (optional from Armv8.8) requires that traps to
the filter register PMSDSFR be disabled. Document the requirements and
disable the traps if the feature is present.
Tested-by: Leo Yan <leo.yan@arm.com>
Signed-off-by: James Clark <james.clark@linaro.org>
---
Documentation/arch/arm64/booting.rst | 11 +++++++++++
arch/arm64/include/asm/el2_setup.h | 14 ++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/Documentation/arch/arm64/booting.rst b/Documentation/arch/arm64/booting.rst
index dee7b6de864f..abd75085a239 100644
--- a/Documentation/arch/arm64/booting.rst
+++ b/Documentation/arch/arm64/booting.rst
@@ -404,6 +404,17 @@ Before jumping into the kernel, the following conditions must be met:
- HDFGWTR2_EL2.nPMICFILTR_EL0 (bit 3) must be initialised to 0b1.
- HDFGWTR2_EL2.nPMUACR_EL1 (bit 4) must be initialised to 0b1.
+ For CPUs with SPE data source filtering (FEAT_SPE_FDS):
+
+ - If EL3 is present:
+
+ - MDCR_EL3.EnPMS3 (bit 42) must be initialised to 0b1.
+
+ - If the kernel is entered at EL1 and EL2 is present:
+
+ - HDFGRTR2_EL2.nPMSDSFR_EL1 (bit 19) must be initialised to 0b1.
+ - HDFGWTR2_EL2.nPMSDSFR_EL1 (bit 19) must be initialised to 0b1.
+
For CPUs with Memory Copy and Memory Set instructions (FEAT_MOPS):
- If the kernel is entered at EL1 and EL2 is present:
diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
index 1e7c7475e43f..02b4a7fc016e 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h
@@ -279,6 +279,20 @@
orr x0, x0, #HDFGRTR2_EL2_nPMICFILTR_EL0
orr x0, x0, #HDFGRTR2_EL2_nPMUACR_EL1
.Lskip_pmuv3p9_\@:
+ mrs x1, id_aa64dfr0_el1
+ ubfx x1, x1, #ID_AA64DFR0_EL1_PMSVer_SHIFT, #4
+ /* If SPE is implemented, */
+ cmp x1, #ID_AA64DFR0_EL1_PMSVer_IMP
+ b.lt .Lskip_spefds_\@
+ /* we can read PMSIDR and */
+ mrs_s x1, SYS_PMSIDR_EL1
+ and x1, x1, #PMSIDR_EL1_FDS
+ /* if FEAT_SPE_FDS is implemented, */
+ cbz x1, .Lskip_spefds_\@
+ /* disable traps to PMSDSFR. */
+ orr x0, x0, #HDFGRTR2_EL2_nPMSDSFR_EL1
+
+.Lskip_spefds_\@:
msr_s SYS_HDFGRTR2_EL2, x0
msr_s SYS_HDFGWTR2_EL2, x0
msr_s SYS_HFGRTR2_EL2, xzr
--
2.34.1On Thu, Jun 05, 2025 at 11:49:02AM +0100, James Clark wrote: > SPE data source filtering (optional from Armv8.8) requires that traps to > the filter register PMSDSFR be disabled. Document the requirements and > disable the traps if the feature is present. > > Tested-by: Leo Yan <leo.yan@arm.com> > Signed-off-by: James Clark <james.clark@linaro.org> > --- > Documentation/arch/arm64/booting.rst | 11 +++++++++++ > arch/arm64/include/asm/el2_setup.h | 14 ++++++++++++++ > 2 files changed, 25 insertions(+) > > diff --git a/Documentation/arch/arm64/booting.rst b/Documentation/arch/arm64/booting.rst > index dee7b6de864f..abd75085a239 100644 > --- a/Documentation/arch/arm64/booting.rst > +++ b/Documentation/arch/arm64/booting.rst > @@ -404,6 +404,17 @@ Before jumping into the kernel, the following conditions must be met: > - HDFGWTR2_EL2.nPMICFILTR_EL0 (bit 3) must be initialised to 0b1. > - HDFGWTR2_EL2.nPMUACR_EL1 (bit 4) must be initialised to 0b1. > > + For CPUs with SPE data source filtering (FEAT_SPE_FDS): > + > + - If EL3 is present: > + > + - MDCR_EL3.EnPMS3 (bit 42) must be initialised to 0b1. > + > + - If the kernel is entered at EL1 and EL2 is present: > + > + - HDFGRTR2_EL2.nPMSDSFR_EL1 (bit 19) must be initialised to 0b1. > + - HDFGWTR2_EL2.nPMSDSFR_EL1 (bit 19) must be initialised to 0b1. > + > For CPUs with Memory Copy and Memory Set instructions (FEAT_MOPS): > > - If the kernel is entered at EL1 and EL2 is present: > diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h > index 1e7c7475e43f..02b4a7fc016e 100644 > --- a/arch/arm64/include/asm/el2_setup.h > +++ b/arch/arm64/include/asm/el2_setup.h > @@ -279,6 +279,20 @@ > orr x0, x0, #HDFGRTR2_EL2_nPMICFILTR_EL0 > orr x0, x0, #HDFGRTR2_EL2_nPMUACR_EL1 > .Lskip_pmuv3p9_\@: > + mrs x1, id_aa64dfr0_el1 > + ubfx x1, x1, #ID_AA64DFR0_EL1_PMSVer_SHIFT, #4 > + /* If SPE is implemented, */ > + cmp x1, #ID_AA64DFR0_EL1_PMSVer_IMP > + b.lt .Lskip_spefds_\@ > + /* we can read PMSIDR and */ > + mrs_s x1, SYS_PMSIDR_EL1 > + and x1, x1, #PMSIDR_EL1_FDS > + /* if FEAT_SPE_FDS is implemented, */ > + cbz x1, .Lskip_spefds_\@ > + /* disable traps to PMSDSFR. */ > + orr x0, x0, #HDFGRTR2_EL2_nPMSDSFR_EL1 Why is this being done here rather than alongside the existing SPE configuration of HDFGRTR_EL2 and HDFGWTR_EL2 near the start of __init_el2_fgt? Will
On 14/07/2025 2:54 pm, Will Deacon wrote: > On Thu, Jun 05, 2025 at 11:49:02AM +0100, James Clark wrote: >> SPE data source filtering (optional from Armv8.8) requires that traps to >> the filter register PMSDSFR be disabled. Document the requirements and >> disable the traps if the feature is present. >> >> Tested-by: Leo Yan <leo.yan@arm.com> >> Signed-off-by: James Clark <james.clark@linaro.org> >> --- >> Documentation/arch/arm64/booting.rst | 11 +++++++++++ >> arch/arm64/include/asm/el2_setup.h | 14 ++++++++++++++ >> 2 files changed, 25 insertions(+) >> >> diff --git a/Documentation/arch/arm64/booting.rst b/Documentation/arch/arm64/booting.rst >> index dee7b6de864f..abd75085a239 100644 >> --- a/Documentation/arch/arm64/booting.rst >> +++ b/Documentation/arch/arm64/booting.rst >> @@ -404,6 +404,17 @@ Before jumping into the kernel, the following conditions must be met: >> - HDFGWTR2_EL2.nPMICFILTR_EL0 (bit 3) must be initialised to 0b1. >> - HDFGWTR2_EL2.nPMUACR_EL1 (bit 4) must be initialised to 0b1. >> >> + For CPUs with SPE data source filtering (FEAT_SPE_FDS): >> + >> + - If EL3 is present: >> + >> + - MDCR_EL3.EnPMS3 (bit 42) must be initialised to 0b1. >> + >> + - If the kernel is entered at EL1 and EL2 is present: >> + >> + - HDFGRTR2_EL2.nPMSDSFR_EL1 (bit 19) must be initialised to 0b1. >> + - HDFGWTR2_EL2.nPMSDSFR_EL1 (bit 19) must be initialised to 0b1. >> + >> For CPUs with Memory Copy and Memory Set instructions (FEAT_MOPS): >> >> - If the kernel is entered at EL1 and EL2 is present: >> diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h >> index 1e7c7475e43f..02b4a7fc016e 100644 >> --- a/arch/arm64/include/asm/el2_setup.h >> +++ b/arch/arm64/include/asm/el2_setup.h >> @@ -279,6 +279,20 @@ >> orr x0, x0, #HDFGRTR2_EL2_nPMICFILTR_EL0 >> orr x0, x0, #HDFGRTR2_EL2_nPMUACR_EL1 >> .Lskip_pmuv3p9_\@: >> + mrs x1, id_aa64dfr0_el1 >> + ubfx x1, x1, #ID_AA64DFR0_EL1_PMSVer_SHIFT, #4 >> + /* If SPE is implemented, */ >> + cmp x1, #ID_AA64DFR0_EL1_PMSVer_IMP >> + b.lt .Lskip_spefds_\@ >> + /* we can read PMSIDR and */ >> + mrs_s x1, SYS_PMSIDR_EL1 >> + and x1, x1, #PMSIDR_EL1_FDS >> + /* if FEAT_SPE_FDS is implemented, */ >> + cbz x1, .Lskip_spefds_\@ >> + /* disable traps to PMSDSFR. */ >> + orr x0, x0, #HDFGRTR2_EL2_nPMSDSFR_EL1 > > Why is this being done here rather than alongside the existing SPE > configuration of HDFGRTR_EL2 and HDFGWTR_EL2 near the start of > __init_el2_fgt? > > Will I thought everything was separated by which trap configs it writes to, rather than the feature. This SPE feature is in HDFGRTR2 so I put it in __init_el2_fgt2 rather than __init_el2_fgt. I suppose we could have a single __init_el2_spe that writes to both HDFGRTR and HDFGRTR2 but we'd have to be careful to not overwrite what was already done in the other sections.
On Tue, Jul 15, 2025 at 01:48:03PM +0100, James Clark wrote: > > > On 14/07/2025 2:54 pm, Will Deacon wrote: > > On Thu, Jun 05, 2025 at 11:49:02AM +0100, James Clark wrote: > > > SPE data source filtering (optional from Armv8.8) requires that traps to > > > the filter register PMSDSFR be disabled. Document the requirements and > > > disable the traps if the feature is present. > > > > > > Tested-by: Leo Yan <leo.yan@arm.com> > > > Signed-off-by: James Clark <james.clark@linaro.org> > > > --- > > > Documentation/arch/arm64/booting.rst | 11 +++++++++++ > > > arch/arm64/include/asm/el2_setup.h | 14 ++++++++++++++ > > > 2 files changed, 25 insertions(+) > > > > > > diff --git a/Documentation/arch/arm64/booting.rst b/Documentation/arch/arm64/booting.rst > > > index dee7b6de864f..abd75085a239 100644 > > > --- a/Documentation/arch/arm64/booting.rst > > > +++ b/Documentation/arch/arm64/booting.rst > > > @@ -404,6 +404,17 @@ Before jumping into the kernel, the following conditions must be met: > > > - HDFGWTR2_EL2.nPMICFILTR_EL0 (bit 3) must be initialised to 0b1. > > > - HDFGWTR2_EL2.nPMUACR_EL1 (bit 4) must be initialised to 0b1. > > > + For CPUs with SPE data source filtering (FEAT_SPE_FDS): > > > + > > > + - If EL3 is present: > > > + > > > + - MDCR_EL3.EnPMS3 (bit 42) must be initialised to 0b1. > > > + > > > + - If the kernel is entered at EL1 and EL2 is present: > > > + > > > + - HDFGRTR2_EL2.nPMSDSFR_EL1 (bit 19) must be initialised to 0b1. > > > + - HDFGWTR2_EL2.nPMSDSFR_EL1 (bit 19) must be initialised to 0b1. > > > + > > > For CPUs with Memory Copy and Memory Set instructions (FEAT_MOPS): > > > - If the kernel is entered at EL1 and EL2 is present: > > > diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h > > > index 1e7c7475e43f..02b4a7fc016e 100644 > > > --- a/arch/arm64/include/asm/el2_setup.h > > > +++ b/arch/arm64/include/asm/el2_setup.h > > > @@ -279,6 +279,20 @@ > > > orr x0, x0, #HDFGRTR2_EL2_nPMICFILTR_EL0 > > > orr x0, x0, #HDFGRTR2_EL2_nPMUACR_EL1 > > > .Lskip_pmuv3p9_\@: > > > + mrs x1, id_aa64dfr0_el1 > > > + ubfx x1, x1, #ID_AA64DFR0_EL1_PMSVer_SHIFT, #4 > > > + /* If SPE is implemented, */ > > > + cmp x1, #ID_AA64DFR0_EL1_PMSVer_IMP > > > + b.lt .Lskip_spefds_\@ > > > + /* we can read PMSIDR and */ > > > + mrs_s x1, SYS_PMSIDR_EL1 > > > + and x1, x1, #PMSIDR_EL1_FDS > > > + /* if FEAT_SPE_FDS is implemented, */ > > > + cbz x1, .Lskip_spefds_\@ > > > + /* disable traps to PMSDSFR. */ > > > + orr x0, x0, #HDFGRTR2_EL2_nPMSDSFR_EL1 > > > > Why is this being done here rather than alongside the existing SPE > > configuration of HDFGRTR_EL2 and HDFGWTR_EL2 near the start of > > __init_el2_fgt? > > > I thought everything was separated by which trap configs it writes to, > rather than the feature. This SPE feature is in HDFGRTR2 so I put it in > __init_el2_fgt2 rather than __init_el2_fgt. That's fair; __init_el2_fgt isn't the right place. But the redundancy of re-reading PMSVer from DFR0 is a little jarring. > I suppose we could have a single __init_el2_spe that writes to both HDFGRTR > and HDFGRTR2 but we'd have to be careful to not overwrite what was already > done in the other sections. Right, perhaps it would be clearer to have trap-preserving macros for features in a specific ID register rather than per-trap configuration register macros. In other words, we have something like __init_fgt_aa64dfr0 which would configure the FGT and FGT2 registers based on features in aa64dfr0. I think you'd need to have a play to see how it ends up looking but the main thing to avoid is having duplicate ID register parsing code for setting up FGT and FGT2 traps. Will
On 15/07/2025 1:57 pm, Will Deacon wrote: > On Tue, Jul 15, 2025 at 01:48:03PM +0100, James Clark wrote: >> >> >> On 14/07/2025 2:54 pm, Will Deacon wrote: >>> On Thu, Jun 05, 2025 at 11:49:02AM +0100, James Clark wrote: >>>> SPE data source filtering (optional from Armv8.8) requires that traps to >>>> the filter register PMSDSFR be disabled. Document the requirements and >>>> disable the traps if the feature is present. >>>> >>>> Tested-by: Leo Yan <leo.yan@arm.com> >>>> Signed-off-by: James Clark <james.clark@linaro.org> >>>> --- >>>> Documentation/arch/arm64/booting.rst | 11 +++++++++++ >>>> arch/arm64/include/asm/el2_setup.h | 14 ++++++++++++++ >>>> 2 files changed, 25 insertions(+) >>>> >>>> diff --git a/Documentation/arch/arm64/booting.rst b/Documentation/arch/arm64/booting.rst >>>> index dee7b6de864f..abd75085a239 100644 >>>> --- a/Documentation/arch/arm64/booting.rst >>>> +++ b/Documentation/arch/arm64/booting.rst >>>> @@ -404,6 +404,17 @@ Before jumping into the kernel, the following conditions must be met: >>>> - HDFGWTR2_EL2.nPMICFILTR_EL0 (bit 3) must be initialised to 0b1. >>>> - HDFGWTR2_EL2.nPMUACR_EL1 (bit 4) must be initialised to 0b1. >>>> + For CPUs with SPE data source filtering (FEAT_SPE_FDS): >>>> + >>>> + - If EL3 is present: >>>> + >>>> + - MDCR_EL3.EnPMS3 (bit 42) must be initialised to 0b1. >>>> + >>>> + - If the kernel is entered at EL1 and EL2 is present: >>>> + >>>> + - HDFGRTR2_EL2.nPMSDSFR_EL1 (bit 19) must be initialised to 0b1. >>>> + - HDFGWTR2_EL2.nPMSDSFR_EL1 (bit 19) must be initialised to 0b1. >>>> + >>>> For CPUs with Memory Copy and Memory Set instructions (FEAT_MOPS): >>>> - If the kernel is entered at EL1 and EL2 is present: >>>> diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h >>>> index 1e7c7475e43f..02b4a7fc016e 100644 >>>> --- a/arch/arm64/include/asm/el2_setup.h >>>> +++ b/arch/arm64/include/asm/el2_setup.h >>>> @@ -279,6 +279,20 @@ >>>> orr x0, x0, #HDFGRTR2_EL2_nPMICFILTR_EL0 >>>> orr x0, x0, #HDFGRTR2_EL2_nPMUACR_EL1 >>>> .Lskip_pmuv3p9_\@: >>>> + mrs x1, id_aa64dfr0_el1 >>>> + ubfx x1, x1, #ID_AA64DFR0_EL1_PMSVer_SHIFT, #4 >>>> + /* If SPE is implemented, */ >>>> + cmp x1, #ID_AA64DFR0_EL1_PMSVer_IMP >>>> + b.lt .Lskip_spefds_\@ >>>> + /* we can read PMSIDR and */ >>>> + mrs_s x1, SYS_PMSIDR_EL1 >>>> + and x1, x1, #PMSIDR_EL1_FDS >>>> + /* if FEAT_SPE_FDS is implemented, */ >>>> + cbz x1, .Lskip_spefds_\@ >>>> + /* disable traps to PMSDSFR. */ >>>> + orr x0, x0, #HDFGRTR2_EL2_nPMSDSFR_EL1 >>> >>> Why is this being done here rather than alongside the existing SPE >>> configuration of HDFGRTR_EL2 and HDFGWTR_EL2 near the start of >>> __init_el2_fgt? >>> >> I thought everything was separated by which trap configs it writes to, >> rather than the feature. This SPE feature is in HDFGRTR2 so I put it in >> __init_el2_fgt2 rather than __init_el2_fgt. > > That's fair; __init_el2_fgt isn't the right place. But the redundancy of > re-reading PMSVer from DFR0 is a little jarring. > >> I suppose we could have a single __init_el2_spe that writes to both HDFGRTR >> and HDFGRTR2 but we'd have to be careful to not overwrite what was already >> done in the other sections. > > Right, perhaps it would be clearer to have trap-preserving macros for > features in a specific ID register rather than per-trap configuration > register macros. > > In other words, we have something like __init_fgt_aa64dfr0 which would > configure the FGT and FGT2 registers based on features in aa64dfr0. I > think you'd need to have a play to see how it ends up looking but the > main thing to avoid is having duplicate ID register parsing code for > setting up FGT and FGT2 traps. > > Will I'll give it a go but that could end up being fragile to something that is dependent on two different ID registers in the future. Then we'd end up in the same situation for a different reason.
On 15/07/2025 2:10 pm, James Clark wrote: > > > On 15/07/2025 1:57 pm, Will Deacon wrote: >> On Tue, Jul 15, 2025 at 01:48:03PM +0100, James Clark wrote: >>> >>> >>> On 14/07/2025 2:54 pm, Will Deacon wrote: >>>> On Thu, Jun 05, 2025 at 11:49:02AM +0100, James Clark wrote: >>>>> SPE data source filtering (optional from Armv8.8) requires that >>>>> traps to >>>>> the filter register PMSDSFR be disabled. Document the requirements and >>>>> disable the traps if the feature is present. >>>>> >>>>> Tested-by: Leo Yan <leo.yan@arm.com> >>>>> Signed-off-by: James Clark <james.clark@linaro.org> >>>>> --- >>>>> Documentation/arch/arm64/booting.rst | 11 +++++++++++ >>>>> arch/arm64/include/asm/el2_setup.h | 14 ++++++++++++++ >>>>> 2 files changed, 25 insertions(+) >>>>> >>>>> diff --git a/Documentation/arch/arm64/booting.rst b/Documentation/ >>>>> arch/arm64/booting.rst >>>>> index dee7b6de864f..abd75085a239 100644 >>>>> --- a/Documentation/arch/arm64/booting.rst >>>>> +++ b/Documentation/arch/arm64/booting.rst >>>>> @@ -404,6 +404,17 @@ Before jumping into the kernel, the following >>>>> conditions must be met: >>>>> - HDFGWTR2_EL2.nPMICFILTR_EL0 (bit 3) must be initialised to >>>>> 0b1. >>>>> - HDFGWTR2_EL2.nPMUACR_EL1 (bit 4) must be initialised to 0b1. >>>>> + For CPUs with SPE data source filtering (FEAT_SPE_FDS): >>>>> + >>>>> + - If EL3 is present: >>>>> + >>>>> + - MDCR_EL3.EnPMS3 (bit 42) must be initialised to 0b1. >>>>> + >>>>> + - If the kernel is entered at EL1 and EL2 is present: >>>>> + >>>>> + - HDFGRTR2_EL2.nPMSDSFR_EL1 (bit 19) must be initialised to 0b1. >>>>> + - HDFGWTR2_EL2.nPMSDSFR_EL1 (bit 19) must be initialised to 0b1. >>>>> + >>>>> For CPUs with Memory Copy and Memory Set instructions >>>>> (FEAT_MOPS): >>>>> - If the kernel is entered at EL1 and EL2 is present: >>>>> diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/ >>>>> include/asm/el2_setup.h >>>>> index 1e7c7475e43f..02b4a7fc016e 100644 >>>>> --- a/arch/arm64/include/asm/el2_setup.h >>>>> +++ b/arch/arm64/include/asm/el2_setup.h >>>>> @@ -279,6 +279,20 @@ >>>>> orr x0, x0, #HDFGRTR2_EL2_nPMICFILTR_EL0 >>>>> orr x0, x0, #HDFGRTR2_EL2_nPMUACR_EL1 >>>>> .Lskip_pmuv3p9_\@: >>>>> + mrs x1, id_aa64dfr0_el1 >>>>> + ubfx x1, x1, #ID_AA64DFR0_EL1_PMSVer_SHIFT, #4 >>>>> + /* If SPE is implemented, */ >>>>> + cmp x1, #ID_AA64DFR0_EL1_PMSVer_IMP >>>>> + b.lt .Lskip_spefds_\@ >>>>> + /* we can read PMSIDR and */ >>>>> + mrs_s x1, SYS_PMSIDR_EL1 >>>>> + and x1, x1, #PMSIDR_EL1_FDS >>>>> + /* if FEAT_SPE_FDS is implemented, */ >>>>> + cbz x1, .Lskip_spefds_\@ >>>>> + /* disable traps to PMSDSFR. */ >>>>> + orr x0, x0, #HDFGRTR2_EL2_nPMSDSFR_EL1 >>>> >>>> Why is this being done here rather than alongside the existing SPE >>>> configuration of HDFGRTR_EL2 and HDFGWTR_EL2 near the start of >>>> __init_el2_fgt? >>>> >>> I thought everything was separated by which trap configs it writes to, >>> rather than the feature. This SPE feature is in HDFGRTR2 so I put it in >>> __init_el2_fgt2 rather than __init_el2_fgt. >> >> That's fair; __init_el2_fgt isn't the right place. But the redundancy of >> re-reading PMSVer from DFR0 is a little jarring. >> >>> I suppose we could have a single __init_el2_spe that writes to both >>> HDFGRTR >>> and HDFGRTR2 but we'd have to be careful to not overwrite what was >>> already >>> done in the other sections. >> >> Right, perhaps it would be clearer to have trap-preserving macros for >> features in a specific ID register rather than per-trap configuration >> register macros. >> >> In other words, we have something like __init_fgt_aa64dfr0 which would >> configure the FGT and FGT2 registers based on features in aa64dfr0. I >> think you'd need to have a play to see how it ends up looking but the >> main thing to avoid is having duplicate ID register parsing code for >> setting up FGT and FGT2 traps. >> >> Will > > I'll give it a go but that could end up being fragile to something that > is dependent on two different ID registers in the future. Then we'd end > up in the same situation for a different reason. > I think I've run into it already. Wouldn't checking for FGT and FGT2 have to be repeated when doing each ID register? Now we only do that once at the start of __init_el2_fgt and __init_el2_fgt2, even if we might sometimes check a different ID register twice. But if we flipped it we'd always have to repeat those.
On Tue, Jul 15, 2025 at 02:28:19PM +0100, James Clark wrote: > > > On 15/07/2025 2:10 pm, James Clark wrote: > > > > > > On 15/07/2025 1:57 pm, Will Deacon wrote: > > > On Tue, Jul 15, 2025 at 01:48:03PM +0100, James Clark wrote: > > > > > > > > > > > > On 14/07/2025 2:54 pm, Will Deacon wrote: > > > > > On Thu, Jun 05, 2025 at 11:49:02AM +0100, James Clark wrote: > > > > > > SPE data source filtering (optional from Armv8.8) > > > > > > requires that traps to > > > > > > the filter register PMSDSFR be disabled. Document the requirements and > > > > > > disable the traps if the feature is present. > > > > > > > > > > > > Tested-by: Leo Yan <leo.yan@arm.com> > > > > > > Signed-off-by: James Clark <james.clark@linaro.org> > > > > > > --- > > > > > > Documentation/arch/arm64/booting.rst | 11 +++++++++++ > > > > > > arch/arm64/include/asm/el2_setup.h | 14 ++++++++++++++ > > > > > > 2 files changed, 25 insertions(+) > > > > > > > > > > > > diff --git a/Documentation/arch/arm64/booting.rst > > > > > > b/Documentation/ arch/arm64/booting.rst > > > > > > index dee7b6de864f..abd75085a239 100644 > > > > > > --- a/Documentation/arch/arm64/booting.rst > > > > > > +++ b/Documentation/arch/arm64/booting.rst > > > > > > @@ -404,6 +404,17 @@ Before jumping into the kernel, the > > > > > > following conditions must be met: > > > > > > - HDFGWTR2_EL2.nPMICFILTR_EL0 (bit 3) must be > > > > > > initialised to 0b1. > > > > > > - HDFGWTR2_EL2.nPMUACR_EL1 (bit 4) must be initialised to 0b1. > > > > > > + For CPUs with SPE data source filtering (FEAT_SPE_FDS): > > > > > > + > > > > > > + - If EL3 is present: > > > > > > + > > > > > > + - MDCR_EL3.EnPMS3 (bit 42) must be initialised to 0b1. > > > > > > + > > > > > > + - If the kernel is entered at EL1 and EL2 is present: > > > > > > + > > > > > > + - HDFGRTR2_EL2.nPMSDSFR_EL1 (bit 19) must be initialised to 0b1. > > > > > > + - HDFGWTR2_EL2.nPMSDSFR_EL1 (bit 19) must be initialised to 0b1. > > > > > > + > > > > > > For CPUs with Memory Copy and Memory Set > > > > > > instructions (FEAT_MOPS): > > > > > > - If the kernel is entered at EL1 and EL2 is present: > > > > > > diff --git a/arch/arm64/include/asm/el2_setup.h > > > > > > b/arch/arm64/ include/asm/el2_setup.h > > > > > > index 1e7c7475e43f..02b4a7fc016e 100644 > > > > > > --- a/arch/arm64/include/asm/el2_setup.h > > > > > > +++ b/arch/arm64/include/asm/el2_setup.h > > > > > > @@ -279,6 +279,20 @@ > > > > > > orr x0, x0, #HDFGRTR2_EL2_nPMICFILTR_EL0 > > > > > > orr x0, x0, #HDFGRTR2_EL2_nPMUACR_EL1 > > > > > > .Lskip_pmuv3p9_\@: > > > > > > + mrs x1, id_aa64dfr0_el1 > > > > > > + ubfx x1, x1, #ID_AA64DFR0_EL1_PMSVer_SHIFT, #4 > > > > > > + /* If SPE is implemented, */ > > > > > > + cmp x1, #ID_AA64DFR0_EL1_PMSVer_IMP > > > > > > + b.lt .Lskip_spefds_\@ > > > > > > + /* we can read PMSIDR and */ > > > > > > + mrs_s x1, SYS_PMSIDR_EL1 > > > > > > + and x1, x1, #PMSIDR_EL1_FDS > > > > > > + /* if FEAT_SPE_FDS is implemented, */ > > > > > > + cbz x1, .Lskip_spefds_\@ > > > > > > + /* disable traps to PMSDSFR. */ > > > > > > + orr x0, x0, #HDFGRTR2_EL2_nPMSDSFR_EL1 > > > > > > > > > > Why is this being done here rather than alongside the existing SPE > > > > > configuration of HDFGRTR_EL2 and HDFGWTR_EL2 near the start of > > > > > __init_el2_fgt? > > > > > > > > > I thought everything was separated by which trap configs it writes to, > > > > rather than the feature. This SPE feature is in HDFGRTR2 so I put it in > > > > __init_el2_fgt2 rather than __init_el2_fgt. > > > > > > That's fair; __init_el2_fgt isn't the right place. But the redundancy of > > > re-reading PMSVer from DFR0 is a little jarring. > > > > > > > I suppose we could have a single __init_el2_spe that writes to > > > > both HDFGRTR > > > > and HDFGRTR2 but we'd have to be careful to not overwrite what > > > > was already > > > > done in the other sections. > > > > > > Right, perhaps it would be clearer to have trap-preserving macros for > > > features in a specific ID register rather than per-trap configuration > > > register macros. > > > > > > In other words, we have something like __init_fgt_aa64dfr0 which would > > > configure the FGT and FGT2 registers based on features in aa64dfr0. I > > > think you'd need to have a play to see how it ends up looking but the > > > main thing to avoid is having duplicate ID register parsing code for > > > setting up FGT and FGT2 traps. > > > > > > > I'll give it a go but that could end up being fragile to something that > > is dependent on two different ID registers in the future. Then we'd end > > up in the same situation for a different reason. > > > > I think I've run into it already. Wouldn't checking for FGT and FGT2 have to > be repeated when doing each ID register? Now we only do that once at the > start of __init_el2_fgt and __init_el2_fgt2, even if we might sometimes > check a different ID register twice. But if we flipped it we'd always have > to repeat those. Bah, this is quite horrible! Maybe the best we can do for now is have a macro for safely getting at PMBIDR? At some point, I suspect this whole FGT-configuration logic will need reworking but at the moment it's hard to see what the best approach would be. Will
© 2016 - 2025 Red Hat, Inc.