| Message ID | 1411076592-6157-3-git-send-email-sboyd@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Thu, Sep 18, 2014 at 02:43:11PM -0700, Stephen Boyd wrote: > diff --git a/arch/arm/include/asm/vfp.h b/arch/arm/include/asm/vfp.h > index f4ab34fd4f72..76d3f6907cce 100644 > --- a/arch/arm/include/asm/vfp.h > +++ b/arch/arm/include/asm/vfp.h > @@ -21,7 +21,7 @@ > #define FPSID_FORMAT_MASK (0x3 << FPSID_FORMAT_BIT) > #define FPSID_NODOUBLE (1<<20) > #define FPSID_ARCH_BIT (16) > -#define FPSID_ARCH_MASK (0xF << FPSID_ARCH_BIT) > +#define FPSID_ARCH_MASK (0x7F << FPSID_ARCH_BIT) This is incorrect. On VFPv2, the architecture field is four bits long. As you can see from the above, bit 20 indicates that there are no double operations provided, and the next two bits indicate the FSTMX/ FLDMX format. I know that you're changing this to conform with the ARM ARM, but we have to consider that before VFP was subsumed into the ARM ARM, this register had the format described as per this file, and these other bits may be set for an ARM part. Including these bits in the mask means that we will mis-identify these older parts as VFPv3. Welcome to the lack of standardisation!
On 09/18/14 15:46, Russell King - ARM Linux wrote: > On Thu, Sep 18, 2014 at 02:43:11PM -0700, Stephen Boyd wrote: >> diff --git a/arch/arm/include/asm/vfp.h b/arch/arm/include/asm/vfp.h >> index f4ab34fd4f72..76d3f6907cce 100644 >> --- a/arch/arm/include/asm/vfp.h >> +++ b/arch/arm/include/asm/vfp.h >> @@ -21,7 +21,7 @@ >> #define FPSID_FORMAT_MASK (0x3 << FPSID_FORMAT_BIT) >> #define FPSID_NODOUBLE (1<<20) >> #define FPSID_ARCH_BIT (16) >> -#define FPSID_ARCH_MASK (0xF << FPSID_ARCH_BIT) >> +#define FPSID_ARCH_MASK (0x7F << FPSID_ARCH_BIT) > This is incorrect. On VFPv2, the architecture field is four bits long. > As you can see from the above, bit 20 indicates that there are no > double operations provided, and the next two bits indicate the FSTMX/ > FLDMX format. > > I know that you're changing this to conform with the ARM ARM, but we > have to consider that before VFP was subsumed into the ARM ARM, this > register had the format described as per this file, and these other > bits may be set for an ARM part. Including these bits in the mask > means that we will mis-identify these older parts as VFPv3. > > Welcome to the lack of standardisation! > Thank you for the warm welcome! I looked at the TRMs for ARM11 and ARM9. I can't find anywhere where VFPv2 is supported and these bits are set. Bits 22-16 of FPSID: ARM1136r1p5: 0x01 ARM1136r1p3: 0x01 ARM1176: 0x01 ARM11MPCorer2p0: 0x01 ARM11MPCorer1p0: 0x01 ARM1156: 0x01 ARM9: 0x01 Do you, or anyone else, know of other implementations? I *hope* that this same exercise was done by the VFP architects before they re-purposed bits but who knows. If nobody is actually setting these higher bits then is there any problem widening the mask (besides it being slightly confusing)?
On 09/19/14 11:24, Stephen Boyd wrote: > On 09/18/14 15:46, Russell King - ARM Linux wrote: >> On Thu, Sep 18, 2014 at 02:43:11PM -0700, Stephen Boyd wrote: >>> diff --git a/arch/arm/include/asm/vfp.h b/arch/arm/include/asm/vfp.h >>> index f4ab34fd4f72..76d3f6907cce 100644 >>> --- a/arch/arm/include/asm/vfp.h >>> +++ b/arch/arm/include/asm/vfp.h >>> @@ -21,7 +21,7 @@ >>> #define FPSID_FORMAT_MASK (0x3 << FPSID_FORMAT_BIT) >>> #define FPSID_NODOUBLE (1<<20) >>> #define FPSID_ARCH_BIT (16) >>> -#define FPSID_ARCH_MASK (0xF << FPSID_ARCH_BIT) >>> +#define FPSID_ARCH_MASK (0x7F << FPSID_ARCH_BIT) >> This is incorrect. On VFPv2, the architecture field is four bits long. >> As you can see from the above, bit 20 indicates that there are no >> double operations provided, and the next two bits indicate the FSTMX/ >> FLDMX format. >> >> I know that you're changing this to conform with the ARM ARM, but we >> have to consider that before VFP was subsumed into the ARM ARM, this >> register had the format described as per this file, and these other >> bits may be set for an ARM part. Including these bits in the mask >> means that we will mis-identify these older parts as VFPv3. >> >> Welcome to the lack of standardisation! >> > Thank you for the warm welcome! I looked at the TRMs for ARM11 and ARM9. > I can't find anywhere where VFPv2 is supported and these bits are set. > > Bits 22-16 of FPSID: > > ARM1136r1p5: 0x01 > ARM1136r1p3: 0x01 > ARM1176: 0x01 > ARM11MPCorer2p0: 0x01 > ARM11MPCorer1p0: 0x01 > ARM1156: 0x01 > ARM9: 0x01 > > > Do you, or anyone else, know of other implementations? I *hope* that > this same exercise was done by the VFP architects before they > re-purposed bits but who knows. If nobody is actually setting these > higher bits then is there any problem widening the mask (besides it > being slightly confusing)? > Any thoughts?
On Fri, Sep 19, 2014 at 11:24:01AM -0700, Stephen Boyd wrote: > Thank you for the warm welcome! I looked at the TRMs for ARM11 and ARM9. > I can't find anywhere where VFPv2 is supported and these bits are set. > > Bits 22-16 of FPSID: > > ARM1136r1p5: 0x01 > ARM1136r1p3: 0x01 > ARM1176: 0x01 > ARM11MPCorer2p0: 0x01 > ARM11MPCorer1p0: 0x01 > ARM1156: 0x01 > ARM9: 0x01 > > > Do you, or anyone else, know of other implementations? I *hope* that > this same exercise was done by the VFP architects before they > re-purposed bits but who knows. If nobody is actually setting these > higher bits then is there any problem widening the mask (besides it > being slightly confusing)? There are certainly non-ARM implementations around, eg: VFP support v0.3: implementor 56 architecture 2 part 20 variant 9 rev 5 which is from Marvell Dove. I don't know how the other Marvell offerings identify. I wouldn't be surprised if there were other implementations from other vendors too. However, if we do have any implementation which indicates that it does not support both single and double precision ops (bit 20), then today the VFP support code refuses to initialise, and the system will behave as if there is no VFP present. So, the problem case is whether we do have non-ARM produced implementations where the VFP code refuses to init, which would then be misidentified as some insane architecture version.
Hi all, On Wed, Oct 01, 2014 at 06:54:18PM +0100, Stephen Boyd wrote: > On 09/19/14 11:24, Stephen Boyd wrote: > > On 09/18/14 15:46, Russell King - ARM Linux wrote: > >> I know that you're changing this to conform with the ARM ARM, but we > >> have to consider that before VFP was subsumed into the ARM ARM, this > >> register had the format described as per this file, and these other > >> bits may be set for an ARM part. Including these bits in the mask > >> means that we will mis-identify these older parts as VFPv3. > >> > > Do you, or anyone else, know of other implementations? I *hope* that > > this same exercise was done by the VFP architects before they > > re-purposed bits but who knows. If nobody is actually setting these > > higher bits then is there any problem widening the mask (besides it > > being slightly confusing)? > > > > Any thoughts? I've spoken to the architects about this, and it seems that you need to parse FPSID differently depending upon whether you are a v7 CPU or not. However, as you are well aware, detecting whether or not you are v7 isn't as simple as it sounds. cpu_architecture() checks the VMSA/PMSA fields in MMFR0, but that's not quite right for 11MPcore (and 1176 iirc), which are v6 implementations. So, it sounds like the scheme is: if (revised_cpuid_format()) // i.e. MIDR.Architecture == 0xF determine_vfp_arch_using_mvfr_regs(); else determine_vfp_arch_using_fpsid(); which in turn means that we probably need to rethink how we want to advertise floating point hardware features in the future. ARMv8, for example, adds an additional MVFR2 register, which advertises additional floating point instructions without explicitly creating something akin to VFPv5. Will -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h index 8c2b7321a478..a8329a5fd9b1 100644 --- a/arch/arm/include/asm/cputype.h +++ b/arch/arm/include/asm/cputype.h @@ -60,6 +60,7 @@ ((mpidr >> (MPIDR_LEVEL_BITS * level)) & MPIDR_LEVEL_MASK) #define ARM_CPU_IMP_ARM 0x41 +#define ARM_CPU_IMP_QCOM 0x51 #define ARM_CPU_IMP_INTEL 0x69 #define ARM_CPU_PART_ARM1136 0xB360 diff --git a/arch/arm/include/asm/vfp.h b/arch/arm/include/asm/vfp.h index f4ab34fd4f72..76d3f6907cce 100644 --- a/arch/arm/include/asm/vfp.h +++ b/arch/arm/include/asm/vfp.h @@ -21,7 +21,7 @@ #define FPSID_FORMAT_MASK (0x3 << FPSID_FORMAT_BIT) #define FPSID_NODOUBLE (1<<20) #define FPSID_ARCH_BIT (16) -#define FPSID_ARCH_MASK (0xF << FPSID_ARCH_BIT) +#define FPSID_ARCH_MASK (0x7F << FPSID_ARCH_BIT) #define FPSID_PART_BIT (8) #define FPSID_PART_MASK (0xFF << FPSID_PART_BIT) #define FPSID_VARIANT_BIT (4) diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c index 2f37e1d6cb45..e6bd8d99e916 100644 --- a/arch/arm/vfp/vfpmodule.c +++ b/arch/arm/vfp/vfpmodule.c @@ -746,6 +746,13 @@ static int __init vfp_init(void) hotcpu_notifier(vfp_hotplug, 0); VFP_arch = (vfpsid & FPSID_ARCH_MASK) >> FPSID_ARCH_BIT; /* Extract the architecture version */ + + /* + * Qualcomm implementations are VFPv3 architecture or later + * with common VFP subarchitecture v3 + */ + if (ARM_CPU_IMP_QCOM == ((vfpsid & FPSID_IMPLEMENTER_MASK) >> FPSID_IMPLEMENTER_BIT)) + VFP_arch = 4; pr_cont("implementor %02x architecture %d part %02x variant %x rev %x\n", (vfpsid & FPSID_IMPLEMENTER_MASK) >> FPSID_IMPLEMENTER_BIT, (vfpsid & FPSID_ARCH_MASK) >> FPSID_ARCH_BIT,
The subarchitecture field in the fpsid register is 7 bits wide. The topmost bit is used to designate that the subarchitecture designer is not ARM. We use this field to determine which VFP version is supported by the CPU. Since the topmost bit is ignored with the current mask we detect non-ARM subarchitectures as supporting only HWCAP_VFP. In Qualcomm's processors (Krait and Scorpion) it should see that we have HWCAP_VFPv3 but it doesn't. Use the proper width for the mask and then check to see if the implementor is 0x51 (Qualcomm). If so, indicate that the vfp architecture is compatible with VFPv3 architecture or later with common VFP subarchitecture v3. Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- arch/arm/include/asm/cputype.h | 1 + arch/arm/include/asm/vfp.h | 2 +- arch/arm/vfp/vfpmodule.c | 7 +++++++ 3 files changed, 9 insertions(+), 1 deletion(-)