Re: [PATCHv2] ARM64: Add AT_ARM64_MIDR to the aux vector

[Mark Rutland]

On Tue, Sep 01, 2015 at 05:51:44PM +0100, pinskia@xxxxxxxxx wrote:
> 
> 
> 
> 
> > On Sep 2, 2015, at 12:33 AM, Mark Rutland <mark.rutland@xxxxxxx> wrote:
> > 
> > Hi,
> > 
> >> On Sat, Aug 29, 2015 at 07:46:22PM +0100, Andrew Pinski wrote:
> >> It is useful to pass down MIDR register down to userland if all of
> >> the online cores are all the same type.  This adds AT_ARM64_MIDR
> >> aux vector type and passes down the midr system register.
> >> 
> >> This is alternative to MIDR_EL1 part of
> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/358995.html.
> >> It allows for faster access to midr_el1 than going through a trap and
> >> does not exist if the set of cores are not the same.
> > 
> > I'm not sure I follow the rationale. If speed is important the
> > application can cache the value the first time it reads it with a trap.
> 
> It is also about compatibility also. Exposing the register is not
> backwards compatible but using the aux vector is. 

So long as we have HWCAP_CPUID describing the availability of register
access [2], then userspace can test for that before attempting to access
the MIDR.

Other than that, I don't see a backwards or forwards compatibility
issue.

> >> +u32 get_arm64_midr(void)
> >> +{
> >> +    int i;
> >> +    u32 midr = 0;
> >> +
> >> +    for_each_online_cpu(i) {
> >> +        struct cpuinfo_arm64 *cpuinfo = &per_cpu(cpu_data, i);
> >> +        u32 oldmidr = midr;
> >> +
> >> +        midr = cpuinfo->reg_midr;
> >> +        /*
> >> +         * If there are cpus which have a different
> >> +         * midr just return 0.
> >> +         */
> >> +        if (oldmidr && oldmidr != midr)
> >> +            return 0;
> >> +    }
> >> +
> >> +    return midr;
> >> +}
> > 
> > If I have a big.LITTLE system where all the big CPUs are currently
> > offline, this will leave the MIDR the little CPUs in the auxvec.
> > However, at any point after this has run, I could hotplug the big CPUs
> > on and the little CPUs off, leaving this reporting a MIDR that
> > represents none of the online CPUs.
> > 
> > Given big.LITTLE and the potential for physical/dynamic hotplug (where
> > we won't know all the MIDRs in advance), I don't think that we can
> > generally expose a common MIDR in this fashion, and I don't think that
> > we should give the impression that we can.
> 
> This is standard issue with hot plug and big.little. Really big.little
> is a design flaw but I am not going into that here. 

Regardless of our personal feelings on big.LITTLE, it's something we
have to deal with.

Hopefully it's a non-issue anyway; a MIDR provided by this interface can
really only be used to derive optimisation criteria rather than
non-architected properties required for correctness.

> > I think that the only things we can do are expose the MIDR for CPU the
> > code is currently executing on (as Suzuki's patches do), and/or expose
> > all the MIDRs for currently online CPUs (as Steve's [1] patch does).
> > Anything else leaves us trying to provide semantics that we cannot
> > guarantee.
> 
> Except they are not backwards compatible which means nobody in their
> right mind would use the register to get the midr that way.

I assume you missed the discussion of HWCAP_CPUID, which prevents the
compatibility issue I believe you're considering here.

> I am sorry but having a newer version of glibc working on a year old
> kernel is not going to fly. 

I'm not sure I follow this, unless you meant _not_ working.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/359127.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-August/363559.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/