| Message ID | 20240718215532.616447-1-rananta@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64/sysreg: Correct the values for GICv4.1 | expand |
On Thu, 18 Jul 2024 22:55:32 +0100, Raghavendra Rao Ananta <rananta@google.com> wrote: > > Currently, sysreg has value as 0b0010 for the presence of GICv4.1 in > ID_PFR1_EL1 and ID_AA64PFR0_EL1, instead of 0b0011 as per ARM ARM. > Hence, correct them to reflect ARM ARM. > > Signed-off-by: Raghavendra Rao Ananta <rananta@google.com> > --- > arch/arm64/tools/sysreg | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg > index a4c1dd4741a47..7ceaa1e0b4bc2 100644 > --- a/arch/arm64/tools/sysreg > +++ b/arch/arm64/tools/sysreg > @@ -149,7 +149,7 @@ Res0 63:32 > UnsignedEnum 31:28 GIC > 0b0000 NI > 0b0001 GICv3 > - 0b0010 GICv4p1 > + 0b0011 GICv4p1 > EndEnum > UnsignedEnum 27:24 Virt_frac > 0b0000 NI > @@ -903,7 +903,7 @@ EndEnum > UnsignedEnum 27:24 GIC > 0b0000 NI > 0b0001 IMP > - 0b0010 V4P1 > + 0b0011 V4P1 I wonder why we have different naming schemes for the same feature... > EndEnum > SignedEnum 23:20 AdvSIMD > 0b0000 IMP > Yup, this looks correct and checks out against revision H.b of the GICv3 spec, revision K.a of the ARM ARM, and even I.a (which the original patches were referencing). Once more, it shows that these dumps should be automatically generated from the XML instead of (creatively) hand-written. Reviewed-by: Marc Zyngier <maz@kernel.org> M.
On 2024/7/19 5:55, Raghavendra Rao Ananta wrote: > Currently, sysreg has value as 0b0010 for the presence of GICv4.1 in > ID_PFR1_EL1 and ID_AA64PFR0_EL1, instead of 0b0011 as per ARM ARM. > Hence, correct them to reflect ARM ARM. > > Signed-off-by: Raghavendra Rao Ananta <rananta@google.com> > --- > arch/arm64/tools/sysreg | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg > index a4c1dd4741a47..7ceaa1e0b4bc2 100644 > --- a/arch/arm64/tools/sysreg > +++ b/arch/arm64/tools/sysreg > @@ -149,7 +149,7 @@ Res0 63:32 > UnsignedEnum 31:28 GIC > 0b0000 NI > 0b0001 GICv3 > - 0b0010 GICv4p1 > + 0b0011 GICv4p1 > EndEnum > UnsignedEnum 27:24 Virt_frac > 0b0000 NI > @@ -903,7 +903,7 @@ EndEnum > UnsignedEnum 27:24 GIC > 0b0000 NI > 0b0001 IMP > - 0b0010 V4P1 > + 0b0011 V4P1 > EndEnum > SignedEnum 23:20 AdvSIMD > 0b0000 IMP Fortunately there is no user for this bit inside kernel. We had checked against the correct hard-coded value (0x3) in gic_cpuif_has_vsgi(). Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
On 7/19/24 13:25, Marc Zyngier wrote: > On Thu, 18 Jul 2024 22:55:32 +0100, > Raghavendra Rao Ananta <rananta@google.com> wrote: >> >> Currently, sysreg has value as 0b0010 for the presence of GICv4.1 in >> ID_PFR1_EL1 and ID_AA64PFR0_EL1, instead of 0b0011 as per ARM ARM. >> Hence, correct them to reflect ARM ARM. >> >> Signed-off-by: Raghavendra Rao Ananta <rananta@google.com> >> --- >> arch/arm64/tools/sysreg | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg >> index a4c1dd4741a47..7ceaa1e0b4bc2 100644 >> --- a/arch/arm64/tools/sysreg >> +++ b/arch/arm64/tools/sysreg >> @@ -149,7 +149,7 @@ Res0 63:32 >> UnsignedEnum 31:28 GIC >> 0b0000 NI >> 0b0001 GICv3 >> - 0b0010 GICv4p1 >> + 0b0011 GICv4p1 >> EndEnum >> UnsignedEnum 27:24 Virt_frac >> 0b0000 NI >> @@ -903,7 +903,7 @@ EndEnum >> UnsignedEnum 27:24 GIC >> 0b0000 NI >> 0b0001 IMP >> - 0b0010 V4P1 >> + 0b0011 V4P1 > > I wonder why we have different naming schemes for the same feature... Both definitions were added via different commits and different developers who might just have interpreted the following common description bit differently. "System register interface to version 4.1 of the GIC CPU interface is supported" 1224308075f1 ("arm64/sysreg: Convert ID_PFR1_EL1 to automatic generation") cea08f2bf406 ("arm64/sysreg: Convert ID_AA64PFR0_EL1 to automatic generation") But I agree that same fields should be named exactly the same both in their 32 bit and 64 bit variants. > >> EndEnum >> SignedEnum 23:20 AdvSIMD >> 0b0000 IMP >> > > Yup, this looks correct and checks out against revision H.b of the GICv3 > spec, revision K.a of the ARM ARM, and even I.a (which the original > patches were referencing). > > Once more, it shows that these dumps should be automatically generated > from the XML instead of (creatively) hand-written. > > Reviewed-by: Marc Zyngier <maz@kernel.org> > > M. >
On 7/21/24 14:59, Zenghui Yu wrote: > On 2024/7/19 5:55, Raghavendra Rao Ananta wrote: >> Currently, sysreg has value as 0b0010 for the presence of GICv4.1 in >> ID_PFR1_EL1 and ID_AA64PFR0_EL1, instead of 0b0011 as per ARM ARM. >> Hence, correct them to reflect ARM ARM. >> >> Signed-off-by: Raghavendra Rao Ananta <rananta@google.com> >> --- >> arch/arm64/tools/sysreg | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg >> index a4c1dd4741a47..7ceaa1e0b4bc2 100644 >> --- a/arch/arm64/tools/sysreg >> +++ b/arch/arm64/tools/sysreg >> @@ -149,7 +149,7 @@ Res0 63:32 >> UnsignedEnum 31:28 GIC >> 0b0000 NI >> 0b0001 GICv3 >> - 0b0010 GICv4p1 >> + 0b0011 GICv4p1 >> EndEnum >> UnsignedEnum 27:24 Virt_frac >> 0b0000 NI >> @@ -903,7 +903,7 @@ EndEnum >> UnsignedEnum 27:24 GIC >> 0b0000 NI >> 0b0001 IMP >> - 0b0010 V4P1 >> + 0b0011 V4P1 >> EndEnum >> SignedEnum 23:20 AdvSIMD >> 0b0000 IMP > > Fortunately there is no user for this bit inside kernel. We had checked > against the correct hard-coded value (0x3) in gic_cpuif_has_vsgi(). which probably helped this problem to be left unnoticed till now :) but I guess it would be better to use ID_AA64PFR0_EL1_GIC_V4P1 there instead. > > Reviewed-by: Zenghui Yu <yuzenghui@huawei.com> >
On 7/19/24 03:25, Raghavendra Rao Ananta wrote: > Currently, sysreg has value as 0b0010 for the presence of GICv4.1 in > ID_PFR1_EL1 and ID_AA64PFR0_EL1, instead of 0b0011 as per ARM ARM. > Hence, correct them to reflect ARM ARM. > > Signed-off-by: Raghavendra Rao Ananta <rananta@google.com> > --- > arch/arm64/tools/sysreg | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg > index a4c1dd4741a47..7ceaa1e0b4bc2 100644 > --- a/arch/arm64/tools/sysreg > +++ b/arch/arm64/tools/sysreg > @@ -149,7 +149,7 @@ Res0 63:32 > UnsignedEnum 31:28 GIC > 0b0000 NI > 0b0001 GICv3 > - 0b0010 GICv4p1 > + 0b0011 GICv4p1 > EndEnum > UnsignedEnum 27:24 Virt_frac > 0b0000 NI > @@ -903,7 +903,7 @@ EndEnum > UnsignedEnum 27:24 GIC > 0b0000 NI > 0b0001 IMP > - 0b0010 V4P1 > + 0b0011 V4P1 > EndEnum > SignedEnum 23:20 AdvSIMD > 0b0000 IMP > > base-commit: 0c3836482481200ead7b416ca80c68a29cfdaabd This checks against ARM DDI 0487K.a and ddi0601/2024-06 XML. Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
On Thu, 18 Jul 2024 21:55:32 +0000, Raghavendra Rao Ananta wrote: > Currently, sysreg has value as 0b0010 for the presence of GICv4.1 in > ID_PFR1_EL1 and ID_AA64PFR0_EL1, instead of 0b0011 as per ARM ARM. > Hence, correct them to reflect ARM ARM. > > Applied to arm64 (for-next/core), thanks! [1/1] arm64/sysreg: Correct the values for GICv4.1 https://git.kernel.org/arm64/c/f3dfcd25455b Cheers,
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg index a4c1dd4741a47..7ceaa1e0b4bc2 100644 --- a/arch/arm64/tools/sysreg +++ b/arch/arm64/tools/sysreg @@ -149,7 +149,7 @@ Res0 63:32 UnsignedEnum 31:28 GIC 0b0000 NI 0b0001 GICv3 - 0b0010 GICv4p1 + 0b0011 GICv4p1 EndEnum UnsignedEnum 27:24 Virt_frac 0b0000 NI @@ -903,7 +903,7 @@ EndEnum UnsignedEnum 27:24 GIC 0b0000 NI 0b0001 IMP - 0b0010 V4P1 + 0b0011 V4P1 EndEnum SignedEnum 23:20 AdvSIMD 0b0000 IMP
Currently, sysreg has value as 0b0010 for the presence of GICv4.1 in ID_PFR1_EL1 and ID_AA64PFR0_EL1, instead of 0b0011 as per ARM ARM. Hence, correct them to reflect ARM ARM. Signed-off-by: Raghavendra Rao Ananta <rananta@google.com> --- arch/arm64/tools/sysreg | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) base-commit: 0c3836482481200ead7b416ca80c68a29cfdaabd