Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1204395 > unrolled thread

Re: [RFC PATCH 01/10] arm64: feature registers: Documentation

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 01/10] arm64: feature registers: Documentation Catalin Marinas <catalin.marinas@arm.com> - 2015-08-10 18:10 +0200
    Re: [RFC PATCH 01/10] arm64: feature registers: Documentation "Suzuki K. Poulose" <Suzuki.Poulose@arm.com> - 2015-08-10 19:40 +0200
      Re: [RFC PATCH 01/10] arm64: feature registers: Documentation Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2015-08-10 19:50 +0200
        Re: [RFC PATCH 01/10] arm64: feature registers: Documentation Catalin Marinas <catalin.marinas@arm.com> - 2015-08-11 16:30 +0200
          Re: [RFC PATCH 01/10] arm64: feature registers: Documentation "Suzuki K. Poulose" <Suzuki.Poulose@arm.com> - 2015-08-11 17:40 +0200
      Re: [RFC PATCH 01/10] arm64: feature registers: Documentation Andrew Haley <aph@redhat.com> - 2015-08-10 20:20 +0200
        Re: [RFC PATCH 01/10] arm64: feature registers: Documentation "Suzuki K. Poulose" <Suzuki.Poulose@arm.com> - 2015-08-11 10:50 +0200
          Re: [RFC PATCH 01/10] arm64: feature registers: Documentation Andrew Haley <aph@redhat.com> - 2015-08-11 11:00 +0200
      Re: [RFC PATCH 01/10] arm64: feature registers: Documentation Catalin Marinas <catalin.marinas@arm.com> - 2015-08-11 16:50 +0200
        Re: [RFC PATCH 01/10] arm64: feature registers: Documentation "Suzuki K. Poulose" <Suzuki.Poulose@arm.com> - 2015-08-11 17:20 +0200

#1204395 — Re: [RFC PATCH 01/10] arm64: feature registers: Documentation

Hi Suzuki,

On Fri, Jul 24, 2015 at 10:43:47AM +0100, Suzuki K. Poulose wrote:
> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> 
> Documentation of the infrastructure
> 
> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>

The implementation looks fine but I think the main discussion will be
around the goal of this feature and the ABI that it introduces. So I'll
just write my thoughts on this patch (I could as well have replied to
the cover letter).

Another question: who's going to use this feature? I know people asked
in private but I'd like to have some public statements.

> --- /dev/null
> +++ b/Documentation/arm64/cpu-feature-registers.txt
> @@ -0,0 +1,185 @@
> +		ARM64 CPU Feature Registers
> +		===========================
> +
> +Author: Suzuki K. Poulose <suzuki.poulose@arm.com>
> +
> +
> +This file describes the API for exporting the AArch64 CPU ID/feature registers
> +to userspace.
> +
> +1. Motivation
> +---------------
> +
> +The ARM architecture defines a set of feature registers, which describe
> +the capabilities of the CPU/system. Access to these system registers is
> +restricted from EL0 and there is no reliable way for an application to
> +extract this information to make better decisions at runtime. There is
> +limited information available to the application via ELF_HWCAPs, however
> +there are some issues with their usage.
> +
> + a) Any change to the HWCAPs requires an update to userspace (e.g libc)
> +    to detect the new changes, which can take a long time to appear in
> +    distributions. Exposing the registers allows applications to get the
> +    information without requiring other userspace components to be updated.

How does it help if you have a new CPUID field or even a new value in an
existing field? Doesn't userspace need to be changed anyway to make use
of the new feature? I don't think that's a valid argument.

> + b) Access to HWCAPs is sometimes restricted (e.g prior to libc, or when ld is
> +    initialised at startup time).

That's useful indeed.

> + c) HWCAPs cannot represent non-boolean information effectively. The
> +    architecture defines a canonical format for representing features
> +    in the ID registers; this is well defined and is capable of
> +    representing all valid architecture variations. Exposing the ID
> +    registers avoids having to come up with HWCAP representations
> +    and parsing code.

So far we've managed to cope with the boolean state of HWCAP, at least
for information relevant to user space. One thing it doesn't cover is
MIDR_EL1.

But the question here is whether we continue to add HWCAP bits even when
we exposed the CPUID registers to user. IMO, we should continue to add
the HWCAP bits matching new CPUID features for a few reasons:

1. It's the current interface that we have and the bits can be checked
   in standard C code without having to issue arm64-specific instructions

2. We still need features listed in /proc/cpuinfo, at least for humans
   reading this file or scripts that can't issue mrs instructions

And to debunk some of the counter arguments:

a) Running out of HWCAP bits - I really doubt this, we can always
   introduce 64 more via a new elf_hwcapX

b) Non-boolean information - The CPUID scheme (not MIDR) is pretty much
   boolean, each increment of the field adding a new feature or
   extending an existing one. We could do the same with HWCAP bits (e.g.
   HWCAP_FEATUREv4)

> +2. Requirements
> +-----------------
> +
> + a) Safety :
> +    Applications should be able to use the information provided by the
> +    infrastructure to run optimally safely across the system. This has
> +    greater implications on a system with heterogeneous CPUs. The
> +    infrastructure exports a value that is safe across all the available
> +    CPU on the system.
> +
> +    e.g, If at least one CPU doesn't implement CRC32 instructions, while others
> +    do, we should report that the CRC32 is not implemented. Otherwise an
> +    application could crash when scheduled on the CPU which doesn't support
> +    CRC32.

Agreed.

> + b) Security :
> +    Applications should only be able to receive information that is relevant
> +    to the normal operation in userspace. Hence, some of the fields
> +    are masked out and the values of the fields are set to indicate the
> +    feature is 'not supported' (See the 'visible' field in the
> +    table in Section 4). Also, the kernel may manipulate the fields based on what
> +    it supports. e.g, If FP is not supported by the kernel, the values
> +    could indicate that the FP is not available (even when the CPU provides
> +    it).

That's fine as well.

What we don't cover is what to do with emulated features. Luckily, we
don't have any for AArch64 currently. If we ever need to do this, do we
fake the CPUID to pretend we have the feature or we don't expose it at
all. I would vote for the latter but that's probably too vague to make
any decision now.

> + c) Implementation Defined Features
> +    The infrastructure doesn't expose any register which is
> +    IMPLEMENTATION DEFINED as per ARMv8-A Architecture and is set to 0.

It may be worth adding somewhere the (unwritten; yet) rules of the CPUID
fields: original 4-bit signed field is RAZ. When a feature is added or
extended, the field is incremented. If an existing feature is removed
for which the CPUID field is 0, the field becomes negative (0xf).

> + d) CPU Identification :
> +    MIDR_EL1 is exposed to help identify the processor. On a heterogeneous
> +    system, this could be racy (just like getcpu()). The process could be
> +    migrated to another CPU by the time we use the register value. Hence,

s/we use/it uses/

> +    there is no guarantee that the value reflects the processor that it is
> +    currently executing on.

You could extend this a bit, something like "unless the CPU affinity is
set".

Anyway, for this reason, we decided not to expose REVIDR since it can
only be read in conjunction with MIDR.

> +The list of supported registers and the attributes of individual
> +feature bits are listed in section 4. Unless there is absolute necessity,
> +we don't encourage the addition of new feature registers to the list.
> +In any case, it should comply to the requirements listed above.
> +
> +3. Implementation
> +--------------------
> +
> +The infrastructure is built on the emulation of the 'MRS' instruction.
> +Accessing a restricted system register from an application generates an
> +exception and ends up in SIGILL being delivered to the process.
> +The infrastructure hooks into the exception handler and emulates the
> +operation if the source belongs to the supported system register space.
> +
> +The infrastructure emulates only the following system register space:
> +	Op0=3, Op1=0, CRn=0
> +
> +(See Table C5-6 'System instruction encodings for System register accesses'
> + in ARMv8 ARM, for the list of registers).
> +
> +
> +The following rules are applied to the value returned by the infrastructure:
> +
> + a) The value of an 'IMPLEMENTATION DEFINED' field is set to 0.
> + b) The value of a reserved field is set to the reserved value(as
> +    defined by the architecture).

Do we expose any IMPLEMENTATION DEFINED or reserved field to user?

> + c) The value of a field marked as not 'visible', is set to indicate
> +    the feature is missing (as defined by the architecture).
> + d) The value of a 'visible' field holds the system wide safe value
> +    for the particular feature(except for MIDR_EL1, see section 4)

I'm slightly confused by the visible/not-visible definition. GIC for
example may be present but we don't want to expose it to user, hence you
marked it as "not visible" in the table. But the feature is definitely
not missing, it may be present and we just decided not to expose it to
EL0 since it is not relevant.

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1204450

On 10/08/15 17:06, Catalin Marinas wrote:
> Hi Suzuki,
>
> On Fri, Jul 24, 2015 at 10:43:47AM +0100, Suzuki K. Poulose wrote:
>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>>
>> Documentation of the infrastructure
>>
>> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
>
> The implementation looks fine but I think the main discussion will be
> around the goal of this feature and the ABI that it introduces. So I'll
> just write my thoughts on this patch (I could as well have replied to
> the cover letter).
>
> Another question: who's going to use this feature? I know people asked
> in private but I'd like to have some public statements.

Right, I am hoping that folks from glibc / JIT / GCC will respond to
this thread.

>
...

>> + a) Any change to the HWCAPs requires an update to userspace (e.g libc)
>> +    to detect the new changes, which can take a long time to appear in
>> +    distributions. Exposing the registers allows applications to get the
>> +    information without requiring other userspace components to be updated.
>
> How does it help if you have a new CPUID field or even a new value in an
> existing field? Doesn't userspace need to be changed anyway to make use
> of the new feature? I don't think that's a valid argument.
>

Yes, the userspace would need an update to work with the new CPUID field. I understand.
It is just that, "in the enterprise world" updates to the system libraries provided by
the distribution might take a bit longer to provide the changes than a software vendor.
I agree thats not a common case.


>> + b) Access to HWCAPs is sometimes restricted (e.g prior to libc, or when ld is
>> +    initialised at startup time).
>
> That's useful indeed.

OK

>
>> + c) HWCAPs cannot represent non-boolean information effectively. The
>> +    architecture defines a canonical format for representing features
>> +    in the ID registers; this is well defined and is capable of
>> +    representing all valid architecture variations. Exposing the ID
>> +    registers avoids having to come up with HWCAP representations
>> +    and parsing code.
>
> So far we've managed to cope with the boolean state of HWCAP, at least
> for information relevant to user space. One thing it doesn't cover is
> MIDR_EL1.
>
> But the question here is whether we continue to add HWCAP bits even when
> we exposed the CPUID registers to user. IMO, we should continue to add
> the HWCAP bits matching new CPUID features for a few reasons:

I don't have a strong opinion against it.

>
> 1. It's the current interface that we have and the bits can be checked
>     in standard C code without having to issue arm64-specific instructions
>

I agree. May be we could provide library interface for this in the future ?

> 2. We still need features listed in /proc/cpuinfo, at least for humans
>     reading this file or scripts that can't issue mrs instructions
>

Agreed, we still need to provide the features in /proc/cpuinfo. We could do
this without HWCAP if we decide not to update the list.

> And to debunk some of the counter arguments:
>
> a) Running out of HWCAP bits - I really doubt this, we can always
>     introduce 64 more via a new elf_hwcapX

OK :)

>
> b) Non-boolean information - The CPUID scheme (not MIDR) is pretty much
>     boolean, each increment of the field adding a new feature or
>     extending an existing one. We could do the same with HWCAP bits (e.g.
>     HWCAP_FEATUREv4)

OK

>> + b) Security :
>> +    Applications should only be able to receive information that is relevant
>> +    to the normal operation in userspace. Hence, some of the fields
>> +    are masked out and the values of the fields are set to indicate the
>> +    feature is 'not supported' (See the 'visible' field in the
>> +    table in Section 4). Also, the kernel may manipulate the fields based on what
>> +    it supports. e.g, If FP is not supported by the kernel, the values
>> +    could indicate that the FP is not available (even when the CPU provides
>> +    it).
>
> That's fine as well.
>
> What we don't cover is what to do with emulated features. Luckily, we
> don't have any for AArch64 currently. If we ever need to do this, do we
> fake the CPUID to pretend we have the feature or we don't expose it at
> all. I would vote for the latter but that's probably too vague to make
> any decision now.

Right. We could pretend to have a feature which the userspace can safely
operate, given kernel/hardware can handle it.

>
>> + c) Implementation Defined Features
>> +    The infrastructure doesn't expose any register which is
>> +    IMPLEMENTATION DEFINED as per ARMv8-A Architecture and is set to 0.
>
> It may be worth adding somewhere the (unwritten; yet) rules of the CPUID
> fields: original 4-bit signed field is RAZ. When a feature is added or
> extended, the field is incremented. If an existing feature is removed
> for which the CPUID field is 0, the field becomes negative (0xf).

May be I can add it as an 'Notes' section at the end ?

>
>> + d) CPU Identification :
>> +    MIDR_EL1 is exposed to help identify the processor. On a heterogeneous
>> +    system, this could be racy (just like getcpu()). The process could be
>> +    migrated to another CPU by the time we use the register value. Hence,
>
> s/we use/it uses/

Will fix it.
  

>> +    there is no guarantee that the value reflects the processor that it is
>> +    currently executing on.
>
> You could extend this a bit, something like "unless the CPU affinity is
> set".

Sure, makes sense.

>
> Anyway, for this reason, we decided not to expose REVIDR since it can
> only be read in conjunction with MIDR.

Right.

...
>> +3. Implementation
>> +--------------------
>> +
...
>> +The infrastructure emulates only the following system register space:
>> +	Op0=3, Op1=0, CRn=0
>> +
>> +(See Table C5-6 'System instruction encodings for System register accesses'
>> + in ARMv8 ARM, for the list of registers).
>> +
>> +
>> +The following rules are applied to the value returned by the infrastructure:
>> +
>> + a) The value of an 'IMPLEMENTATION DEFINED' field is set to 0.
>> + b) The value of a reserved field is set to the reserved value(as
>> +    defined by the architecture).
>
> Do we expose any IMPLEMENTATION DEFINED or reserved field to user?

We don't. All such fields are marked invisible. The above rules define
how we fill those 'special' (invisible) fields. We 'emulate' all
access to the space (as defined above) with Op0=3, Op1=0 & CRn=0.
Out of this space, there are only a very few 'visible' fields(listed in
section 4). These rules, define how the values are emulated.

>
>> + c) The value of a field marked as not 'visible', is set to indicate
>> +    the feature is missing (as defined by the architecture).
>> + d) The value of a 'visible' field holds the system wide safe value
>> +    for the particular feature(except for MIDR_EL1, see section 4)
>
> I'm slightly confused by the visible/not-visible definition. GIC for
> example may be present but we don't want to expose it to user, hence you
> marked it as "not visible" in the table. But the feature is definitely
> not missing, it may be present and we just decided not to expose it to
> EL0 since it is not relevant.

Thats right. In this case, the userspace will see that 'GIC' is not present
even though it is available. Btw, the system wide value(exposed to the system
wide users) could be different from what the user gets. e.g, if all the CPUs
have GIC system register access, the system view will have 'GIC' available.

Taking another example to explain rule (d), if all CPUs but one supports CRC32
instructions, both the system view and the user view will have CRC32 disabled.


Thanks
Suzuki




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1204456

On 10 August 2015 at 19:36, Suzuki K. Poulose <Suzuki.Poulose@arm.com> wrote:
> On 10/08/15 17:06, Catalin Marinas wrote:
>>
>> Hi Suzuki,
>>
>> On Fri, Jul 24, 2015 at 10:43:47AM +0100, Suzuki K. Poulose wrote:
>>>
>>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>>>
>>> Documentation of the infrastructure
>>>
>>> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
>>
>>
>> The implementation looks fine but I think the main discussion will be
>> around the goal of this feature and the ABI that it introduces. So I'll
>> just write my thoughts on this patch (I could as well have replied to
>> the cover letter).
>>
>> Another question: who's going to use this feature? I know people asked
>> in private but I'd like to have some public statements.
>
>
> Right, I am hoping that folks from glibc / JIT / GCC will respond to
> this thread.
>
>>
> ...
>
>>> + a) Any change to the HWCAPs requires an update to userspace (e.g libc)
>>> +    to detect the new changes, which can take a long time to appear in
>>> +    distributions. Exposing the registers allows applications to get the
>>> +    information without requiring other userspace components to be
>>> updated.
>>
>>
>> How does it help if you have a new CPUID field or even a new value in an
>> existing field? Doesn't userspace need to be changed anyway to make use
>> of the new feature? I don't think that's a valid argument.
>>
>
> Yes, the userspace would need an update to work with the new CPUID field. I
> understand.
> It is just that, "in the enterprise world" updates to the system libraries
> provided by
> the distribution might take a bit longer to provide the changes than a
> software vendor.
> I agree thats not a common case.
>
>
>>> + b) Access to HWCAPs is sometimes restricted (e.g prior to libc, or when
>>> ld is
>>> +    initialised at startup time).
>>
>>
>> That's useful indeed.
>
>
> OK
>
>>
>>> + c) HWCAPs cannot represent non-boolean information effectively. The
>>> +    architecture defines a canonical format for representing features
>>> +    in the ID registers; this is well defined and is capable of
>>> +    representing all valid architecture variations. Exposing the ID
>>> +    registers avoids having to come up with HWCAP representations
>>> +    and parsing code.
>>
>>
>> So far we've managed to cope with the boolean state of HWCAP, at least
>> for information relevant to user space. One thing it doesn't cover is
>> MIDR_EL1.
>>
>> But the question here is whether we continue to add HWCAP bits even when
>> we exposed the CPUID registers to user. IMO, we should continue to add
>> the HWCAP bits matching new CPUID features for a few reasons:
>
>
> I don't have a strong opinion against it.
>
>>
>> 1. It's the current interface that we have and the bits can be checked
>>     in standard C code without having to issue arm64-specific instructions
>>
>
> I agree. May be we could provide library interface for this in the future ?
>
>> 2. We still need features listed in /proc/cpuinfo, at least for humans
>>     reading this file or scripts that can't issue mrs instructions
>>
>
> Agreed, we still need to provide the features in /proc/cpuinfo. We could do
> this without HWCAP if we decide not to update the list.
>
>> And to debunk some of the counter arguments:
>>
>> a) Running out of HWCAP bits - I really doubt this, we can always
>>     introduce 64 more via a new elf_hwcapX
>

Note that ELF_HWCAP is also wired into ifunc resolution of GNU
indirect functions, which looks like a useful feature although it
isn't used that widely yet.

The ifunc prototype for aarch64 has only one 'long' parameter, and I
don't know if it is possible to extend that without having a bit in
HWCAPn to indicate that HWCAPn+1 is valid. Also, the ifunc resolvers
are restricted in the sense that they cannot use shared libraries or
code that uses constructors (AFAIR) so it may require a special static
library to call this CPU feature interface from such a resolver if
features are not covered by HWCAP bits.

So treating HWCAP bits as an endless supply may not be the wisest
approach here. Also, I think some alignment with the libc folks is
indeed in order.

-- 
Ard.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1205174

On Mon, Aug 10, 2015 at 07:48:48PM +0200, Ard Biesheuvel wrote:
> > On 10/08/15 17:06, Catalin Marinas wrote:
> >> And to debunk some of the counter arguments:
> >>
> >> a) Running out of HWCAP bits - I really doubt this, we can always
> >>     introduce 64 more via a new elf_hwcapX
> 
> Note that ELF_HWCAP is also wired into ifunc resolution of GNU
> indirect functions, which looks like a useful feature although it
> isn't used that widely yet.

I forgot to mention, we also need an HWCAP_CPUID with these patches when
we expose the MRS interface. The ifunc resolver could use MRS when
available. But I would still keep adding HWCAP bits for new features,
even if we risk running out of the 64-bit we have now.

> The ifunc prototype for aarch64 has only one 'long' parameter, and I
> don't know if it is possible to extend that without having a bit in
> HWCAPn to indicate that HWCAPn+1 is valid. Also, the ifunc resolvers
> are restricted in the sense that they cannot use shared libraries or
> code that uses constructors (AFAIR) so it may require a special static
> library to call this CPU feature interface from such a resolver if
> features are not covered by HWCAP bits.

Or we could get some compiler intrinsics that generate the instruction
inline, just to avoid explicit asm.

> So treating HWCAP bits as an endless supply may not be the wisest
> approach here.

Probably not for ifunc, otherwise I don't think it hurts.

> Also, I think some alignment with the libc folks is indeed in order.

I agree (not sure how they feel about cross-posting though).

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1205222

On 11/08/15 15:23, Catalin Marinas wrote:
> On Mon, Aug 10, 2015 at 07:48:48PM +0200, Ard Biesheuvel wrote:
>>> On 10/08/15 17:06, Catalin Marinas wrote:
>>>> And to debunk some of the counter arguments:
>>>>
>>>> a) Running out of HWCAP bits - I really doubt this, we can always
>>>>      introduce 64 more via a new elf_hwcapX
>>
>> Note that ELF_HWCAP is also wired into ifunc resolution of GNU
>> indirect functions, which looks like a useful feature although it
>> isn't used that widely yet.
>
> I forgot to mention, we also need an HWCAP_CPUID with these patches when
> we expose the MRS interface. The ifunc resolver could use MRS when
> available. But I would still keep adding HWCAP bits for new features,
> even if we risk running out of the 64-bit we have now.
>

Sure, I will add the HWCAP_CPUID in the next version of the series.

Thanks
Suzuki

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1204461

On 08/10/2015 06:36 PM, Suzuki K. Poulose wrote:
> On 10/08/15 17:06, Catalin Marinas wrote:
>> Hi Suzuki,
>>
>> On Fri, Jul 24, 2015 at 10:43:47AM +0100, Suzuki K. Poulose wrote:
>>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>>>
>>> Documentation of the infrastructure
>>>
>>> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
>>
>> The implementation looks fine but I think the main discussion will be
>> around the goal of this feature and the ABI that it introduces. So I'll
>> just write my thoughts on this patch (I could as well have replied to
>> the cover letter).
>>
>> Another question: who's going to use this feature? I know people asked
>> in private but I'd like to have some public statements.
> 
> Right, I am hoping that folks from glibc / JIT / GCC will respond to
> this thread.

We certainly need it for OpenJDK.  We need to know the manufacturer,
part number, revision id, etc.  We already have workarounds in
OpenJDK for various bugs, and we also can generate better code if we
know the exact part.

I note that the REVIDR is not in this patch.  That seems odd, because
it can be used to identify minor revisions.

Andrew.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1204887

On 10/08/15 19:19, Andrew Haley wrote:
> On 08/10/2015 06:36 PM, Suzuki K. Poulose wrote:
>> On 10/08/15 17:06, Catalin Marinas wrote:
>>> Hi Suzuki,
>>>
>>> On Fri, Jul 24, 2015 at 10:43:47AM +0100, Suzuki K. Poulose wrote:
>>>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>>>>
>>>> Documentation of the infrastructure
>>>>
>>>> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
>>>
>>> The implementation looks fine but I think the main discussion will be
>>> around the goal of this feature and the ABI that it introduces. So I'll
>>> just write my thoughts on this patch (I could as well have replied to
>>> the cover letter).
>>>
>>> Another question: who's going to use this feature? I know people asked
>>> in private but I'd like to have some public statements.
>>
>> Right, I am hoping that folks from glibc / JIT / GCC will respond to
>> this thread.
>
> We certainly need it for OpenJDK.  We need to know the manufacturer,
> part number, revision id, etc.  We already have workarounds in
> OpenJDK for various bugs, and we also can generate better code if we
> know the exact part.

OK.

>
> I note that the REVIDR is not in this patch.  That seems odd, because
> it can be used to identify minor revisions.

The REVIDR has to be used in conjunction with the MIDR to make real sense.
We cannot guarantee that the REVIDR that we read (would) belong to the CPU
where MIDR would have been read (unless the process is pinned) and hence the
user may not be able to make any use of the information. Steve has a patch [1]
to expose the MIDR,REVIDR info via sysfs.

[1] https://lkml.org/lkml/2015/7/24/420

Thanks
Suzuki

>
> Andrew.
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1204891

On 11/08/15 09:41, Suzuki K. Poulose wrote:
> The REVIDR has to be used in conjunction with the MIDR to make real sense.

Sure, of course.

> We cannot guarantee that the REVIDR that we read (would) belong to the CPU
> where MIDR would have been read (unless the process is pinned) and hence the
> user may not be able to make any use of the information.

Well, yes, nothing is perfect.  The situation we're in now, for example,
is that if we see Cortex A57 we have to assume Cortex A53 and include
workarounds.  People might mix and match processors.  So it goes.

> Steve has a patch [1]
> to expose the MIDR,REVIDR info via sysfs.
> 
> [1] https://lkml.org/lkml/2015/7/24/420

OK.  As long as I can get at it I'm happy.

Andrew.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1205185

On Mon, Aug 10, 2015 at 06:36:46PM +0100, Suzuki K. Poulose wrote:
> On 10/08/15 17:06, Catalin Marinas wrote:
> >On Fri, Jul 24, 2015 at 10:43:47AM +0100, Suzuki K. Poulose wrote:
> >>From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> >>
> >>Documentation of the infrastructure
> >>
> >>Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
> >
> >The implementation looks fine but I think the main discussion will be
> >around the goal of this feature and the ABI that it introduces. So I'll
> >just write my thoughts on this patch (I could as well have replied to
> >the cover letter).
> >
> >Another question: who's going to use this feature? I know people asked
> >in private but I'd like to have some public statements.
> 
> Right, I am hoping that folks from glibc / JIT / GCC will respond to
> this thread.

Some of them didn't even want to be cc'ed ;)

> >>+ a) Any change to the HWCAPs requires an update to userspace (e.g libc)
> >>+    to detect the new changes, which can take a long time to appear in
> >>+    distributions. Exposing the registers allows applications to get the
> >>+    information without requiring other userspace components to be updated.
> >
> >How does it help if you have a new CPUID field or even a new value in an
> >existing field? Doesn't userspace need to be changed anyway to make use
> >of the new feature? I don't think that's a valid argument.
> 
> Yes, the userspace would need an update to work with the new CPUID field. I understand.
> It is just that, "in the enterprise world" updates to the system libraries provided by
> the distribution might take a bit longer to provide the changes than a software vendor.
> I agree thats not a common case.

What I meant is that for a new CPU feature, the user space needs
updating anyway to make use of (add support for) it, whether it checks
its presence via HWCAP or MRS. Let's say we get new crypto instructions,
existing user space won't even check for it because it doesn't know
there is a new CPUID field (or HWCAP bit).

> >2. We still need features listed in /proc/cpuinfo, at least for humans
> >    reading this file or scripts that can't issue mrs instructions
> 
> Agreed, we still need to provide the features in /proc/cpuinfo. We could do
> this without HWCAP if we decide not to update the list.

I agree, /proc/cpuinfo is doable without HWCAP. But since some software
ends up parsing /proc/cpuinfo anyway, I don't see why we should hide
HWCAP.

> >>+ c) Implementation Defined Features
> >>+    The infrastructure doesn't expose any register which is
> >>+    IMPLEMENTATION DEFINED as per ARMv8-A Architecture and is set to 0.
> >
> >It may be worth adding somewhere the (unwritten; yet) rules of the CPUID
> >fields: original 4-bit signed field is RAZ. When a feature is added or
> >extended, the field is incremented. If an existing feature is removed
> >for which the CPUID field is 0, the field becomes negative (0xf).
> 
> May be I can add it as an 'Notes' section at the end ?

Fine.

> >>+3. Implementation
> >>+--------------------
> >>+
> ...
> >>+The infrastructure emulates only the following system register space:
> >>+	Op0=3, Op1=0, CRn=0
> >>+
> >>+(See Table C5-6 'System instruction encodings for System register accesses'
> >>+ in ARMv8 ARM, for the list of registers).
> >>+
> >>+
> >>+The following rules are applied to the value returned by the infrastructure:
> >>+
> >>+ a) The value of an 'IMPLEMENTATION DEFINED' field is set to 0.
> >>+ b) The value of a reserved field is set to the reserved value(as
> >>+    defined by the architecture).
> >
> >Do we expose any IMPLEMENTATION DEFINED or reserved field to user?
> 
> We don't. All such fields are marked invisible. The above rules define
> how we fill those 'special' (invisible) fields. We 'emulate' all
> access to the space (as defined above) with Op0=3, Op1=0 & CRn=0.
> Out of this space, there are only a very few 'visible' fields(listed in
> section 4). These rules, define how the values are emulated.

Point b) above is a bit confusing - reserved field is set to the
reserved value. If the reserved value is non-zero, do we expose such
value to user or we return zero as for other invisible fields?

> >>+ c) The value of a field marked as not 'visible', is set to indicate
> >>+    the feature is missing (as defined by the architecture).
> >>+ d) The value of a 'visible' field holds the system wide safe value
> >>+    for the particular feature(except for MIDR_EL1, see section 4)
> >
> >I'm slightly confused by the visible/not-visible definition. GIC for
> >example may be present but we don't want to expose it to user, hence you
> >marked it as "not visible" in the table. But the feature is definitely
> >not missing, it may be present and we just decided not to expose it to
> >EL0 since it is not relevant.
> 
> Thats right. In this case, the userspace will see that 'GIC' is not present
> even though it is available. Btw, the system wide value(exposed to the system
> wide users) could be different from what the user gets. e.g, if all the CPUs
> have GIC system register access, the system view will have 'GIC' available.
> 
> Taking another example to explain rule (d), if all CPUs but one supports CRC32
> instructions, both the system view and the user view will have CRC32 disabled.

OK. I missed the difference between "system wide view" and "user view".
I guess the former is not exposed to user.

As I mentioned in my reply to Ard, we need a HWCAP entry to inform the
user of the MRS emulation. My question is whether to use a single
HWCAP_CPUID or multiple for each ID register (e.g.
HWCAP_ID_AA64ISAR0_EL1 or a shorter HWCAP_ID_ISAR0). The advantage of
the latter is that we can expose new CPUID registers if any of them
appear (or there is a useful feature in a register we don't expose).

-- 
Catalin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1205208

On 11/08/15 15:46, Catalin Marinas wrote:
> On Mon, Aug 10, 2015 at 06:36:46PM +0100, Suzuki K. Poulose wrote:
>> On 10/08/15 17:06, Catalin Marinas wrote:
>>> On Fri, Jul 24, 2015 at 10:43:47AM +0100, Suzuki K. Poulose wrote:
>>>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>>>>
>>>> Documentation of the infrastructure
>>>>
>>>> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>

...

>>> + a) Any change to the HWCAPs requires an update to userspace (e.g libc)
>>>> +    to detect the new changes, which can take a long time to appear in
>>>> +    distributions. Exposing the registers allows applications to get the
>>>> +    information without requiring other userspace components to be updated.
>>>
>>> How does it help if you have a new CPUID field or even a new value in an
>>> existing field? Doesn't userspace need to be changed anyway to make use
>>> of the new feature? I don't think that's a valid argument.
>>
>> Yes, the userspace would need an update to work with the new CPUID field. I understand.
>> It is just that, "in the enterprise world" updates to the system libraries provided by
>> the distribution might take a bit longer to provide the changes than a software vendor.
>> I agree thats not a common case.
>
> What I meant is that for a new CPU feature, the user space needs
> updating anyway to make use of (add support for) it, whether it checks
> its presence via HWCAP or MRS. Let's say we get new crypto instructions,
> existing user space won't even check for it because it doesn't know
> there is a new CPUID field (or HWCAP bit).
>

I understand that, thats why I mentioned a 'software vendor' could roll out an
update independent of the 'system libraries' provided by the distribution, where
standard distributions might have their own schedule. But, I agree that the userspace
needs update and my 'story' is not common :)

>>> 2. We still need features listed in /proc/cpuinfo, at least for humans
>>>     reading this file or scripts that can't issue mrs instructions
>>
>> Agreed, we still need to provide the features in /proc/cpuinfo. We could do
>> this without HWCAP if we decide not to update the list.
>
> I agree, /proc/cpuinfo is doable without HWCAP. But since some software
> ends up parsing /proc/cpuinfo anyway, I don't see why we should hide
> HWCAP.
>

Right.

>>>> +3. Implementation
>>>> +--------------------
>>>> +
>> ...
>>>> +The infrastructure emulates only the following system register space:
>>>> +	Op0=3, Op1=0, CRn=0
>>>> +
>>>> +(See Table C5-6 'System instruction encodings for System register accesses'
>>>> + in ARMv8 ARM, for the list of registers).
>>>> +
>>>> +
>>>> +The following rules are applied to the value returned by the infrastructure:
>>>> +
>>>> + a) The value of an 'IMPLEMENTATION DEFINED' field is set to 0.
>>>> + b) The value of a reserved field is set to the reserved value(as
>>>> +    defined by the architecture).
>>>
>>> Do we expose any IMPLEMENTATION DEFINED or reserved field to user?
>>
>> We don't. All such fields are marked invisible. The above rules define
>> how we fill those 'special' (invisible) fields. We 'emulate' all
>> access to the space (as defined above) with Op0=3, Op1=0 & CRn=0.
>> Out of this space, there are only a very few 'visible' fields(listed in
>> section 4). These rules, define how the values are emulated.
>
> Point b) above is a bit confusing - reserved field is set to the
> reserved value. If the reserved value is non-zero, do we expose such
> value to user or we return zero as for other invisible fields?

At the moment, we are exposing the reserved values to not confuse the user, which
- I thought - is safer than exposing 0. Value 0 could have a different meaning in
the future(if it is not RES0 already).

>>>> + c) The value of a field marked as not 'visible', is set to indicate
>>>> +    the feature is missing (as defined by the architecture).
>>>> + d) The value of a 'visible' field holds the system wide safe value
>>>> +    for the particular feature(except for MIDR_EL1, see section 4)
>>>
>>> I'm slightly confused by the visible/not-visible definition. GIC for
>>> example may be present but we don't want to expose it to user, hence you
>>> marked it as "not visible" in the table. But the feature is definitely
>>> not missing, it may be present and we just decided not to expose it to
>>> EL0 since it is not relevant.
>>
>> Thats right. In this case, the userspace will see that 'GIC' is not present
>> even though it is available. Btw, the system wide value(exposed to the system
>> wide users) could be different from what the user gets. e.g, if all the CPUs
>> have GIC system register access, the system view will have 'GIC' available.
>>
>> Taking another example to explain rule (d), if all CPUs but one supports CRC32
>> instructions, both the system view and the user view will have CRC32 disabled.
>
> OK. I missed the difference between "system wide view" and "user view".
> I guess the former is not exposed to user.

Right, the system view and the user view are different. User view will only get the
visible parts from the 'System view'.

>
> As I mentioned in my reply to Ard, we need a HWCAP entry to inform the
> user of the MRS emulation. My question is whether to use a single
> HWCAP_CPUID or multiple for each ID register (e.g.
> HWCAP_ID_AA64ISAR0_EL1 or a shorter HWCAP_ID_ISAR0). The advantage of
> the latter is that we can expose new CPUID registers if any of them
> appear (or there is a useful feature in a register we don't expose).

The current implementation, kind of solves the issue. i.e, we expose
a safe value(which implies 'feature not available') for all the registers
in the id space (including the reserved ids) we plan to emulate. So if we
decide to expose something, userspace need not make any change. It could
continue to read the register safely and decide if the feature is available or not
and take appropriate decisions.


Thanks
Suzuki

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web