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]

Message-ID: <1520471460.55728.73.camel@intel.com>
Date:   Wed, 07 Mar 2018 17:11:00 -0800
From:   Ivan Gorinov <ivan.gorinov@...el.com>
To:     Rob Herring <robh+dt@...nel.org>
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, 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.

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.