linux-kernel - Re: [PATCH v4 3/4] of: Documentation: Add x86 local APIC ID property

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 7 Mar 2018 19:34:09 -0600
From:   Rob Herring <robh+dt@...nel.org>
To:     Ivan Gorinov <ivan.gorinov@...el.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...hat.com>,
        Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCH v4 3/4] of: Documentation: Add x86 local APIC ID property

On Wed, Mar 7, 2018 at 7:11 PM, Ivan Gorinov <ivan.gorinov@...el.com> wrote:
> On Wed, 2018-03-07 at 14:23 -0600, Rob Herring wrote:
>
>> > Add new "intel,apic-id" property to allow using CPU descriptions
>> > in Device Tree data provided by the U-Boot loader.
>> > Address specified in 'reg' to be used as default local APIC ID
>> > to avoid breaking existing systems with DTB provided by firmware.
>> Is there some reason to not always use reg? For when the numbering of
>> cpus and timers is different?
>
> Yes, local APIC ID may differ from CPU number.
> For example, in Atom E38xx (u-boot/arch/x86/dts/minnowmax.dts):
>
>         cpus {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>
>                 cpu@0 {
>                         device_type = "cpu";
>                         compatible = "intel,baytrail-cpu";
>                         reg = <0>;
>                         intel,apic-id = <0>;
>                 };
>
>                 cpu@1 {
>                         device_type = "cpu";
>                         compatible = "intel,baytrail-cpu";
>                         reg = <1>;
>                         intel,apic-id = <4>;
>                 };
>         };
>
>> Of course, we do have the situation on ARM with the GIC that the GIC
>> CPU IDs may be
>> >
>> >
>> > Signed-off-by: Ivan Gorinov <ivan.gorinov@...el.com>
>> > ---
>> >  Documentation/devicetree/bindings/x86/ce4100.txt | 6 ++++++
>> >  1 file changed, 6 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/x86/ce4100.txt b/Documentation/devicetree/bindings/x86/ce4100.txt
>> > index b49ae59..d15de48 100644
>> > --- a/Documentation/devicetree/bindings/x86/ce4100.txt
>> > +++ b/Documentation/devicetree/bindings/x86/ce4100.txt
>> > @@ -14,11 +14,17 @@ The CPU node
>> >                 compatible = "intel,ce4100";
>> >                 reg = <0>;
>> >                 lapic = <&lapic0>;
>> Isn't this enough? I can't tell because whatever this points to has no
>> binding documentation.
>
> Local APIC is a part of CPU, not an external device (except for 486 and early Pentium).
> Every CPU has access to its own local APIC registers at the same base address (0xfee00000).
> Therefore, one "lapic" device node can work for all processors in the system.

Do you need a lapic node then? If you typically don't have a node,
then just having the id should be fine.

> With more changes in the code, the local APIC description could be made optional
> because every processor can always read its local APIC base address from MSR 0x1b.
> And when x2APIC mode is enabled, the local APIC registers are accessed as model
> specific registers instead of memory-mapped I/O.
>
>> You could perhaps extend it and add a cell with the id value.
>
> This may require different DT data for Linux and U-Boot, or changes in the latter.

The latter case. There's one upstream for DT binding reviews.

Rob