| Message ID | 1541113468-22097-2-git-send-email-atish.patra@wdc.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add RISC-V cpu topology | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Thu, Nov 1, 2018 at 6:04 PM Atish Patra <atish.patra@wdc.com> wrote: > > Define a RISC-V cpu topology. This is based on cpu-map in ARM world. > But it doesn't need a separate thread node for defining SMT systems. > Multiple cpu phandle properties can be parsed to identify the sibling > hardware threads. Moreover, we do not have cluster concept in RISC-V. > So package is a better word choice than cluster for RISC-V. There was a proposal to add package info for ARM recently. Not sure what happened to that, but we don't need 2 different ways. There's never going to be clusters for RISC-V? What prevents that? Seems shortsighted to me. > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > --- > .../devicetree/bindings/riscv/topology.txt | 154 +++++++++++++++++++++ > 1 file changed, 154 insertions(+) > create mode 100644 Documentation/devicetree/bindings/riscv/topology.txt > > diff --git a/Documentation/devicetree/bindings/riscv/topology.txt b/Documentation/devicetree/bindings/riscv/topology.txt > new file mode 100644 > index 00000000..96039ed3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/riscv/topology.txt > @@ -0,0 +1,154 @@ > +=========================================== > +RISC-V cpu topology binding description > +=========================================== > + > +=========================================== > +1 - Introduction > +=========================================== > + > +In a RISC-V system, the hierarchy of CPUs can be defined through following nodes that > +are used to describe the layout of physical CPUs in the system: > + > +- packages > +- core > + > +The cpu nodes (bindings defined in [1]) represent the devices that > +correspond to physical CPUs and are to be mapped to the hierarchy levels. > +Simultaneous multi-threading (SMT) systems can also represent their topology > +by defining multiple cpu phandles inside core node. The details are explained > +in paragraph 3. I don't see a reason to do this differently than ARM. That said, I don't think the thread part is in use on ARM, so it could possibly be changed. > + > +The remainder of this document provides the topology bindings for ARM, based for ARM? > +on the Devicetree Specification, available from: > + > +https://www.devicetree.org/specifications/ > + > +If not stated otherwise, whenever a reference to a cpu node phandle is made its > +value must point to a cpu node compliant with the cpu node bindings as > +documented in [1]. > +A topology description containing phandles to cpu nodes that are not compliant > +with bindings standardized in [1] is therefore considered invalid. > + > +This cpu topology binding description is mostly based on the topology defined > +in ARM [2]. > +=========================================== > +2 - cpu-topology node cpu-map. Why change this? What I would like to see is the ARM topology binding reworked to be common or some good reasons why it doesn't work for RISC-V as-is. Rob
On Fri, Nov 02, 2018 at 08:09:39AM -0500, Rob Herring wrote: > On Thu, Nov 1, 2018 at 6:04 PM Atish Patra <atish.patra@wdc.com> wrote: > > > > Define a RISC-V cpu topology. This is based on cpu-map in ARM world. > > But it doesn't need a separate thread node for defining SMT systems. > > Multiple cpu phandle properties can be parsed to identify the sibling > > hardware threads. Moreover, we do not have cluster concept in RISC-V. > > So package is a better word choice than cluster for RISC-V. > > There was a proposal to add package info for ARM recently. Not sure > what happened to that, but we don't need 2 different ways. > We still need that, I can brush it up and post what Lorenzo had previously proposed[1]. We want to keep both DT and ACPI CPU topology story aligned. -- Regards, Sudeep [1] https://marc.info/?l=devicetree&m=151817774202854&w=2
On Fri, Nov 2, 2018 at 8:31 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Fri, Nov 02, 2018 at 08:09:39AM -0500, Rob Herring wrote: > > On Thu, Nov 1, 2018 at 6:04 PM Atish Patra <atish.patra@wdc.com> wrote: > > > > > > Define a RISC-V cpu topology. This is based on cpu-map in ARM world. > > > But it doesn't need a separate thread node for defining SMT systems. > > > Multiple cpu phandle properties can be parsed to identify the sibling > > > hardware threads. Moreover, we do not have cluster concept in RISC-V. > > > So package is a better word choice than cluster for RISC-V. > > > > There was a proposal to add package info for ARM recently. Not sure > > what happened to that, but we don't need 2 different ways. > > > > We still need that, I can brush it up and post what Lorenzo had previously > proposed[1]. We want to keep both DT and ACPI CPU topology story aligned. Frankly, I don't care what the ACPI story is. I care whether each cpu arch does its own thing in DT or not. If a package prop works for RISC-V folks and that happens to align with ACPI, then okay. Though I tend to prefer a package represented as a node rather than a property as I think that's more consistent. Any comments on the thread aspect (whether it has ever been used)? Though I think thread as a node level is more consistent with each topology level being a node (same with package). Rob
On Fri, Nov 02, 2018 at 10:11:38AM -0500, Rob Herring wrote: > On Fri, Nov 2, 2018 at 8:31 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > On Fri, Nov 02, 2018 at 08:09:39AM -0500, Rob Herring wrote: > > > On Thu, Nov 1, 2018 at 6:04 PM Atish Patra <atish.patra@wdc.com> wrote: > > > > > > > > Define a RISC-V cpu topology. This is based on cpu-map in ARM world. > > > > But it doesn't need a separate thread node for defining SMT systems. > > > > Multiple cpu phandle properties can be parsed to identify the sibling > > > > hardware threads. Moreover, we do not have cluster concept in RISC-V. > > > > So package is a better word choice than cluster for RISC-V. > > > > > > There was a proposal to add package info for ARM recently. Not sure > > > what happened to that, but we don't need 2 different ways. > > > > > > > We still need that, I can brush it up and post what Lorenzo had previously > > proposed[1]. We want to keep both DT and ACPI CPU topology story aligned. > > Frankly, I don't care what the ACPI story is. I care whether each cpu Sorry I meant feature parity with ACPI and didn't refer to the mechanics. > arch does its own thing in DT or not. If a package prop works for > RISC-V folks and that happens to align with ACPI, then okay. Though I > tend to prefer a package represented as a node rather than a property > as I think that's more consistent. > Sounds good. One of the reason for making it *optional* property is for backward compatibility. But we should be able to deal with that even with node. > Any comments on the thread aspect (whether it has ever been used)? > Though I think thread as a node level is more consistent with each > topology level being a node (same with package). > Not 100% sure, the only multi threaded core in the market I know is Cavium TX2 which is ACPI. -- Regards, Sudeep
On 11/2/18 6:09 AM, Rob Herring wrote: > On Thu, Nov 1, 2018 at 6:04 PM Atish Patra <atish.patra@wdc.com> wrote: >> >> Define a RISC-V cpu topology. This is based on cpu-map in ARM world. >> But it doesn't need a separate thread node for defining SMT systems. >> Multiple cpu phandle properties can be parsed to identify the sibling >> hardware threads. Moreover, we do not have cluster concept in RISC-V. >> So package is a better word choice than cluster for RISC-V. > > There was a proposal to add package info for ARM recently. Not sure > what happened to that, but we don't need 2 different ways. > > There's never going to be clusters for RISC-V? What prevents that? > Seems shortsighted to me. > Agreed. My intention was to keep it simple at first go. If package node is added, that would work for us as well. >> >> Signed-off-by: Atish Patra <atish.patra@wdc.com> >> --- >> .../devicetree/bindings/riscv/topology.txt | 154 +++++++++++++++++++++ >> 1 file changed, 154 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/riscv/topology.txt >> >> diff --git a/Documentation/devicetree/bindings/riscv/topology.txt b/Documentation/devicetree/bindings/riscv/topology.txt >> new file mode 100644 >> index 00000000..96039ed3 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/riscv/topology.txt >> @@ -0,0 +1,154 @@ >> +=========================================== >> +RISC-V cpu topology binding description >> +=========================================== >> + >> +=========================================== >> +1 - Introduction >> +=========================================== >> + >> +In a RISC-V system, the hierarchy of CPUs can be defined through following nodes that >> +are used to describe the layout of physical CPUs in the system: >> + >> +- packages >> +- core >> + >> +The cpu nodes (bindings defined in [1]) represent the devices that >> +correspond to physical CPUs and are to be mapped to the hierarchy levels. >> +Simultaneous multi-threading (SMT) systems can also represent their topology >> +by defining multiple cpu phandles inside core node. The details are explained >> +in paragraph 3. > > I don't see a reason to do this differently than ARM. That said, I > don't think the thread part is in use on ARM, so it could possibly be > changed. > >> + >> +The remainder of this document provides the topology bindings for ARM, based > > for ARM? > Sorry for the typo. >> +on the Devicetree Specification, available from: >> + >> +https://www.devicetree.org/specifications/ >> + >> +If not stated otherwise, whenever a reference to a cpu node phandle is made its >> +value must point to a cpu node compliant with the cpu node bindings as >> +documented in [1]. >> +A topology description containing phandles to cpu nodes that are not compliant >> +with bindings standardized in [1] is therefore considered invalid. >> + >> +This cpu topology binding description is mostly based on the topology defined >> +in ARM [2]. >> +=========================================== >> +2 - cpu-topology node > > cpu-map. Why change this? > To my ears, it felt a better name. But I don't mind dropping it in favor of cpu-map if we are going to standardize cpu-map for both ARM & RISC-V. > What I would like to see is the ARM topology binding reworked to be > common or some good reasons why it doesn't work for RISC-V as-is. > IMHO, ARM topology can be reworked and put it in a common place so that RISC-V can leverage that. My intention for this RFC patch was to start the ball rolling on cpu topology in RISC-V and see if DT approach is fine with everybody. I would be happy to see ARM code to move it to a common code base where RISC-V can reuse it. Regards, Atish > Rob >
On 11/2/18 8:50 AM, Sudeep Holla wrote: > On Fri, Nov 02, 2018 at 10:11:38AM -0500, Rob Herring wrote: >> On Fri, Nov 2, 2018 at 8:31 AM Sudeep Holla <sudeep.holla@arm.com> wrote: >>> >>> On Fri, Nov 02, 2018 at 08:09:39AM -0500, Rob Herring wrote: >>>> On Thu, Nov 1, 2018 at 6:04 PM Atish Patra <atish.patra@wdc.com> wrote: >>>>> >>>>> Define a RISC-V cpu topology. This is based on cpu-map in ARM world. >>>>> But it doesn't need a separate thread node for defining SMT systems. >>>>> Multiple cpu phandle properties can be parsed to identify the sibling >>>>> hardware threads. Moreover, we do not have cluster concept in RISC-V. >>>>> So package is a better word choice than cluster for RISC-V. >>>> >>>> There was a proposal to add package info for ARM recently. Not sure >>>> what happened to that, but we don't need 2 different ways. >>>> >>> >>> We still need that, I can brush it up and post what Lorenzo had previously >>> proposed[1]. We want to keep both DT and ACPI CPU topology story aligned. >> >> Frankly, I don't care what the ACPI story is. I care whether each cpu > > Sorry I meant feature parity with ACPI and didn't refer to the mechanics. > >> arch does its own thing in DT or not. If a package prop works for >> RISC-V folks and that happens to align with ACPI, then okay. Though I >> tend to prefer a package represented as a node rather than a property >> as I think that's more consistent. >> > > Sounds good. One of the reason for making it *optional* property is for > backward compatibility. But we should be able to deal with that even with > node. > If you are introducing a package node, can we make cluster node optional? I feel it is a redundant node for use cases where one doesn't have a different grouped cpus in a package. We may have some architecture that requires cluster, it can be added to the DT at that time, I believe. >> Any comments on the thread aspect (whether it has ever been used)? >> Though I think thread as a node level is more consistent with each >> topology level being a node (same with package). >> > Not 100% sure, the only multi threaded core in the market I know is > Cavium TX2 which is ACPI. > Any advantages of keeping thread node if it's not being used. If I am not wrong, we can always use multiple cpuN phandles to represent SMT nodes. It will result in less code and DT documentation as well. Regards, Atish > -- > Regards, > Sudeep >
On Fri, Nov 2, 2018 at 3:53 PM Atish Patra <atish.patra@wdc.com> wrote: > > On 11/2/18 8:50 AM, Sudeep Holla wrote: > > On Fri, Nov 02, 2018 at 10:11:38AM -0500, Rob Herring wrote: > >> On Fri, Nov 2, 2018 at 8:31 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > >>> > >>> On Fri, Nov 02, 2018 at 08:09:39AM -0500, Rob Herring wrote: > >>>> On Thu, Nov 1, 2018 at 6:04 PM Atish Patra <atish.patra@wdc.com> wrote: > >>>>> > >>>>> Define a RISC-V cpu topology. This is based on cpu-map in ARM world. > >>>>> But it doesn't need a separate thread node for defining SMT systems. > >>>>> Multiple cpu phandle properties can be parsed to identify the sibling > >>>>> hardware threads. Moreover, we do not have cluster concept in RISC-V. > >>>>> So package is a better word choice than cluster for RISC-V. > >>>> > >>>> There was a proposal to add package info for ARM recently. Not sure > >>>> what happened to that, but we don't need 2 different ways. > >>>> > >>> > >>> We still need that, I can brush it up and post what Lorenzo had previously > >>> proposed[1]. We want to keep both DT and ACPI CPU topology story aligned. > >> > >> Frankly, I don't care what the ACPI story is. I care whether each cpu > > > > Sorry I meant feature parity with ACPI and didn't refer to the mechanics. > > > >> arch does its own thing in DT or not. If a package prop works for > >> RISC-V folks and that happens to align with ACPI, then okay. Though I > >> tend to prefer a package represented as a node rather than a property > >> as I think that's more consistent. > >> > > > > Sounds good. One of the reason for making it *optional* property is for > > backward compatibility. But we should be able to deal with that even with > > node. > > > > If you are introducing a package node, can we make cluster node > optional? I feel it is a redundant node for use cases where one doesn't > have a different grouped cpus in a package. Certainly not. A common doc can make every level optional and each arch can define what's mandatory. > We may have some architecture that requires cluster, it can be added to > the DT at that time, I believe. > > >> Any comments on the thread aspect (whether it has ever been used)? > >> Though I think thread as a node level is more consistent with each > >> topology level being a node (same with package). > >> > > Not 100% sure, the only multi threaded core in the market I know is > > Cavium TX2 which is ACPI. > > > > Any advantages of keeping thread node if it's not being used. If I am > not wrong, we can always use multiple cpuN phandles to represent SMT > nodes. It will result in less code and DT documentation as well. It's more consistent to make each level a node IMO and we've already discussed and defined it this way. I don't see how it's really less code or documentation. BTW, powerpc defined threads with multiple reg entries in the cpu nodes. You could do that if you wanted. Rob
On Fri, 02 Nov 2018 06:09:39 PDT (-0700), robh+dt@kernel.org wrote: > On Thu, Nov 1, 2018 at 6:04 PM Atish Patra <atish.patra@wdc.com> wrote: >> >> Define a RISC-V cpu topology. This is based on cpu-map in ARM world. >> But it doesn't need a separate thread node for defining SMT systems. >> Multiple cpu phandle properties can be parsed to identify the sibling >> hardware threads. Moreover, we do not have cluster concept in RISC-V. >> So package is a better word choice than cluster for RISC-V. > > There was a proposal to add package info for ARM recently. Not sure > what happened to that, but we don't need 2 different ways. > > There's never going to be clusters for RISC-V? What prevents that? > Seems shortsighted to me. > >> >> Signed-off-by: Atish Patra <atish.patra@wdc.com> >> --- >> .../devicetree/bindings/riscv/topology.txt | 154 +++++++++++++++++++++ >> 1 file changed, 154 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/riscv/topology.txt >> >> diff --git a/Documentation/devicetree/bindings/riscv/topology.txt b/Documentation/devicetree/bindings/riscv/topology.txt >> new file mode 100644 >> index 00000000..96039ed3 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/riscv/topology.txt >> @@ -0,0 +1,154 @@ >> +=========================================== >> +RISC-V cpu topology binding description >> +=========================================== >> + >> +=========================================== >> +1 - Introduction >> +=========================================== >> + >> +In a RISC-V system, the hierarchy of CPUs can be defined through following nodes that >> +are used to describe the layout of physical CPUs in the system: >> + >> +- packages >> +- core >> + >> +The cpu nodes (bindings defined in [1]) represent the devices that >> +correspond to physical CPUs and are to be mapped to the hierarchy levels. >> +Simultaneous multi-threading (SMT) systems can also represent their topology >> +by defining multiple cpu phandles inside core node. The details are explained >> +in paragraph 3. > > I don't see a reason to do this differently than ARM. That said, I > don't think the thread part is in use on ARM, so it could possibly be > changed. > >> + >> +The remainder of this document provides the topology bindings for ARM, based > > for ARM? > >> +on the Devicetree Specification, available from: >> + >> +https://www.devicetree.org/specifications/ >> + >> +If not stated otherwise, whenever a reference to a cpu node phandle is made its >> +value must point to a cpu node compliant with the cpu node bindings as >> +documented in [1]. >> +A topology description containing phandles to cpu nodes that are not compliant >> +with bindings standardized in [1] is therefore considered invalid. >> + >> +This cpu topology binding description is mostly based on the topology defined >> +in ARM [2]. >> +=========================================== >> +2 - cpu-topology node > > cpu-map. Why change this? > > What I would like to see is the ARM topology binding reworked to be > common or some good reasons why it doesn't work for RISC-V as-is. I think it would be great if CPU topologies were not a RISC-V specific thing. We don't really do anything different than anyone else, so it'd be great if we could all share the same spec and code. Looking quickly at the ARM cpu-map bindings, I don't see any reason why we can't just use the same thing on RISC-V -- it's not quite how I'd do it, but I don't think the differences are worth having another implementation. Mechanically I'm not sure how to do this: should there just be a "Documentation/devicetree/bindings/cpu-map.txt"? If everyone is OK with that then I vote we just go ahead and genericise the ARM "cpu-map" stuff for CPU topology. Sharing the implementation looks fairly straight-forward as well.
On Mon, Nov 5, 2018 at 1:39 PM Palmer Dabbelt <palmer@sifive.com> wrote: > > On Fri, 02 Nov 2018 06:09:39 PDT (-0700), robh+dt@kernel.org wrote: > > On Thu, Nov 1, 2018 at 6:04 PM Atish Patra <atish.patra@wdc.com> wrote: > >> > >> Define a RISC-V cpu topology. This is based on cpu-map in ARM world. > >> But it doesn't need a separate thread node for defining SMT systems. > >> Multiple cpu phandle properties can be parsed to identify the sibling > >> hardware threads. Moreover, we do not have cluster concept in RISC-V. > >> So package is a better word choice than cluster for RISC-V. > > > > There was a proposal to add package info for ARM recently. Not sure > > what happened to that, but we don't need 2 different ways. > > > > There's never going to be clusters for RISC-V? What prevents that? > > Seems shortsighted to me. > > > >> > >> Signed-off-by: Atish Patra <atish.patra@wdc.com> > >> --- > >> .../devicetree/bindings/riscv/topology.txt | 154 +++++++++++++++++++++ > >> 1 file changed, 154 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/riscv/topology.txt > >> > >> diff --git a/Documentation/devicetree/bindings/riscv/topology.txt b/Documentation/devicetree/bindings/riscv/topology.txt > >> new file mode 100644 > >> index 00000000..96039ed3 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/riscv/topology.txt > >> @@ -0,0 +1,154 @@ > >> +=========================================== > >> +RISC-V cpu topology binding description > >> +=========================================== > >> + > >> +=========================================== > >> +1 - Introduction > >> +=========================================== > >> + > >> +In a RISC-V system, the hierarchy of CPUs can be defined through following nodes that > >> +are used to describe the layout of physical CPUs in the system: > >> + > >> +- packages > >> +- core > >> + > >> +The cpu nodes (bindings defined in [1]) represent the devices that > >> +correspond to physical CPUs and are to be mapped to the hierarchy levels. > >> +Simultaneous multi-threading (SMT) systems can also represent their topology > >> +by defining multiple cpu phandles inside core node. The details are explained > >> +in paragraph 3. > > > > I don't see a reason to do this differently than ARM. That said, I > > don't think the thread part is in use on ARM, so it could possibly be > > changed. > > > >> + > >> +The remainder of this document provides the topology bindings for ARM, based > > > > for ARM? > > > >> +on the Devicetree Specification, available from: > >> + > >> +https://www.devicetree.org/specifications/ > >> + > >> +If not stated otherwise, whenever a reference to a cpu node phandle is made its > >> +value must point to a cpu node compliant with the cpu node bindings as > >> +documented in [1]. > >> +A topology description containing phandles to cpu nodes that are not compliant > >> +with bindings standardized in [1] is therefore considered invalid. > >> + > >> +This cpu topology binding description is mostly based on the topology defined > >> +in ARM [2]. > >> +=========================================== > >> +2 - cpu-topology node > > > > cpu-map. Why change this? > > > > What I would like to see is the ARM topology binding reworked to be > > common or some good reasons why it doesn't work for RISC-V as-is. > > I think it would be great if CPU topologies were not a RISC-V specific thing. > We don't really do anything different than anyone else, so it'd be great if we > could all share the same spec and code. Looking quickly at the ARM cpu-map > bindings, I don't see any reason why we can't just use the same thing on RISC-V > -- it's not quite how I'd do it, but I don't think the differences are worth > having another implementation. Mechanically I'm not sure how to do this: > should there just be a "Documentation/devicetree/bindings/cpu-map.txt"? Yes, but ".../bindings/cpu/cpu-topology.txt". And if we need $arch extensions, they can be moved there. (Really, I'd like to get rid of /bindings/$arch/* except for maybe a few things.) > If everyone is OK with that then I vote we just go ahead and genericise the ARM > "cpu-map" stuff for CPU topology. Sharing the implementation looks fairly > straight-forward as well.
On 11/5/18 12:11 PM, Rob Herring wrote: > On Mon, Nov 5, 2018 at 1:39 PM Palmer Dabbelt <palmer@sifive.com> wrote: >> >> On Fri, 02 Nov 2018 06:09:39 PDT (-0700), robh+dt@kernel.org wrote: >>> On Thu, Nov 1, 2018 at 6:04 PM Atish Patra <atish.patra@wdc.com> wrote: >>>> >>>> Define a RISC-V cpu topology. This is based on cpu-map in ARM world. >>>> But it doesn't need a separate thread node for defining SMT systems. >>>> Multiple cpu phandle properties can be parsed to identify the sibling >>>> hardware threads. Moreover, we do not have cluster concept in RISC-V. >>>> So package is a better word choice than cluster for RISC-V. >>> >>> There was a proposal to add package info for ARM recently. Not sure >>> what happened to that, but we don't need 2 different ways. >>> >>> There's never going to be clusters for RISC-V? What prevents that? >>> Seems shortsighted to me. >>> >>>> >>>> Signed-off-by: Atish Patra <atish.patra@wdc.com> >>>> --- >>>> .../devicetree/bindings/riscv/topology.txt | 154 +++++++++++++++++++++ >>>> 1 file changed, 154 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/riscv/topology.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/riscv/topology.txt b/Documentation/devicetree/bindings/riscv/topology.txt >>>> new file mode 100644 >>>> index 00000000..96039ed3 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/riscv/topology.txt >>>> @@ -0,0 +1,154 @@ >>>> +=========================================== >>>> +RISC-V cpu topology binding description >>>> +=========================================== >>>> + >>>> +=========================================== >>>> +1 - Introduction >>>> +=========================================== >>>> + >>>> +In a RISC-V system, the hierarchy of CPUs can be defined through following nodes that >>>> +are used to describe the layout of physical CPUs in the system: >>>> + >>>> +- packages >>>> +- core >>>> + >>>> +The cpu nodes (bindings defined in [1]) represent the devices that >>>> +correspond to physical CPUs and are to be mapped to the hierarchy levels. >>>> +Simultaneous multi-threading (SMT) systems can also represent their topology >>>> +by defining multiple cpu phandles inside core node. The details are explained >>>> +in paragraph 3. >>> >>> I don't see a reason to do this differently than ARM. That said, I >>> don't think the thread part is in use on ARM, so it could possibly be >>> changed. >>> >>>> + >>>> +The remainder of this document provides the topology bindings for ARM, based >>> >>> for ARM? >>> >>>> +on the Devicetree Specification, available from: >>>> + >>>> +https://www.devicetree.org/specifications/ >>>> + >>>> +If not stated otherwise, whenever a reference to a cpu node phandle is made its >>>> +value must point to a cpu node compliant with the cpu node bindings as >>>> +documented in [1]. >>>> +A topology description containing phandles to cpu nodes that are not compliant >>>> +with bindings standardized in [1] is therefore considered invalid. >>>> + >>>> +This cpu topology binding description is mostly based on the topology defined >>>> +in ARM [2]. >>>> +=========================================== >>>> +2 - cpu-topology node >>> >>> cpu-map. Why change this? >>> >>> What I would like to see is the ARM topology binding reworked to be >>> common or some good reasons why it doesn't work for RISC-V as-is. >> >> I think it would be great if CPU topologies were not a RISC-V specific thing. >> We don't really do anything different than anyone else, so it'd be great if we >> could all share the same spec and code. Looking quickly at the ARM cpu-map >> bindings, I don't see any reason why we can't just use the same thing on RISC-V >> -- it's not quite how I'd do it, but I don't think the differences are worth >> having another implementation. Mechanically I'm not sure how to do this: >> should there just be a "Documentation/devicetree/bindings/cpu-map.txt"? > > Yes, but ".../bindings/cpu/cpu-topology.txt". > > And if we need $arch extensions, they can be moved there. (Really, I'd > like to get rid of /bindings/$arch/* except for maybe a few things.) > >> If everyone is OK with that then I vote we just go ahead and genericise the ARM >> "cpu-map" stuff for CPU topology. Sharing the implementation looks fairly >> straight-forward as well. > +1 for a common code base. I am happy to take it up if nobody else has not already started working on it. Is there a ARM hardware test farm that can be used to test such changes? Regards, Atish
Στις 2018-11-05 21:38, Palmer Dabbelt έγραψε: > On Fri, 02 Nov 2018 06:09:39 PDT (-0700), robh+dt@kernel.org wrote: >> On Thu, Nov 1, 2018 at 6:04 PM Atish Patra <atish.patra@wdc.com> >> wrote: >>> >>> Define a RISC-V cpu topology. This is based on cpu-map in ARM world. >>> But it doesn't need a separate thread node for defining SMT systems. >>> Multiple cpu phandle properties can be parsed to identify the sibling >>> hardware threads. Moreover, we do not have cluster concept in RISC-V. >>> So package is a better word choice than cluster for RISC-V. >> >> There was a proposal to add package info for ARM recently. Not sure >> what happened to that, but we don't need 2 different ways. >> >> There's never going to be clusters for RISC-V? What prevents that? >> Seems shortsighted to me. >> >>> >>> Signed-off-by: Atish Patra <atish.patra@wdc.com> >>> --- >>> .../devicetree/bindings/riscv/topology.txt | 154 >>> +++++++++++++++++++++ >>> 1 file changed, 154 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/riscv/topology.txt >>> >>> diff --git a/Documentation/devicetree/bindings/riscv/topology.txt >>> b/Documentation/devicetree/bindings/riscv/topology.txt >>> new file mode 100644 >>> index 00000000..96039ed3 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/riscv/topology.txt >>> @@ -0,0 +1,154 @@ >>> +=========================================== >>> +RISC-V cpu topology binding description >>> +=========================================== >>> + >>> +=========================================== >>> +1 - Introduction >>> +=========================================== >>> + >>> +In a RISC-V system, the hierarchy of CPUs can be defined through >>> following nodes that >>> +are used to describe the layout of physical CPUs in the system: >>> + >>> +- packages >>> +- core >>> + >>> +The cpu nodes (bindings defined in [1]) represent the devices that >>> +correspond to physical CPUs and are to be mapped to the hierarchy >>> levels. >>> +Simultaneous multi-threading (SMT) systems can also represent their >>> topology >>> +by defining multiple cpu phandles inside core node. The details are >>> explained >>> +in paragraph 3. >> >> I don't see a reason to do this differently than ARM. That said, I >> don't think the thread part is in use on ARM, so it could possibly be >> changed. >> >>> + >>> +The remainder of this document provides the topology bindings for >>> ARM, based >> >> for ARM? >> >>> +on the Devicetree Specification, available from: >>> + >>> +https://www.devicetree.org/specifications/ >>> + >>> +If not stated otherwise, whenever a reference to a cpu node phandle >>> is made its >>> +value must point to a cpu node compliant with the cpu node bindings >>> as >>> +documented in [1]. >>> +A topology description containing phandles to cpu nodes that are not >>> compliant >>> +with bindings standardized in [1] is therefore considered invalid. >>> + >>> +This cpu topology binding description is mostly based on the >>> topology defined >>> +in ARM [2]. >>> +=========================================== >>> +2 - cpu-topology node >> >> cpu-map. Why change this? >> >> What I would like to see is the ARM topology binding reworked to be >> common or some good reasons why it doesn't work for RISC-V as-is. > > I think it would be great if CPU topologies were not a RISC-V specific > thing. We don't really do anything different than anyone else, so > it'd be great if we could all share the same spec and code. Looking > quickly at the ARM cpu-map bindings, I don't see any reason why we > can't just use the same thing on RISC-V -- it's not quite how I'd do > it, but I don't think the differences are worth having another > implementation. Mechanically I'm not sure how to do this: should > there just be a "Documentation/devicetree/bindings/cpu-map.txt"? > > If everyone is OK with that then I vote we just go ahead and > genericise the ARM "cpu-map" stuff for CPU topology. Sharing the > implementation looks fairly straight-forward as well. > Please check this out... https://lkml.org/lkml/2018/11/3/99 It's also non arch-dependent and it can handle the scheduler's capabilities better than cpu-map.
On Tue, Nov 06, 2018 at 12:03:17PM +0200, Nick Kossifidis wrote: > Στις 2018-11-05 21:38, Palmer Dabbelt έγραψε: > > On Fri, 02 Nov 2018 06:09:39 PDT (-0700), robh+dt@kernel.org wrote: > > > On Thu, Nov 1, 2018 at 6:04 PM Atish Patra <atish.patra@wdc.com> > > > wrote: > > > > > > > > Define a RISC-V cpu topology. This is based on cpu-map in ARM world. > > > > But it doesn't need a separate thread node for defining SMT systems. > > > > Multiple cpu phandle properties can be parsed to identify the sibling > > > > hardware threads. Moreover, we do not have cluster concept in RISC-V. > > > > So package is a better word choice than cluster for RISC-V. > > > > > > There was a proposal to add package info for ARM recently. Not sure > > > what happened to that, but we don't need 2 different ways. > > > > > > There's never going to be clusters for RISC-V? What prevents that? > > > Seems shortsighted to me. > > > > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > > --- > > > > .../devicetree/bindings/riscv/topology.txt | 154 > > > > +++++++++++++++++++++ > > > > 1 file changed, 154 insertions(+) > > > > create mode 100644 > > > > Documentation/devicetree/bindings/riscv/topology.txt > > > > > > > > diff --git > > > > a/Documentation/devicetree/bindings/riscv/topology.txt > > > > b/Documentation/devicetree/bindings/riscv/topology.txt > > > > new file mode 100644 > > > > index 00000000..96039ed3 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/riscv/topology.txt > > > > @@ -0,0 +1,154 @@ > > > > +=========================================== > > > > +RISC-V cpu topology binding description > > > > +=========================================== > > > > + > > > > +=========================================== > > > > +1 - Introduction > > > > +=========================================== > > > > + > > > > +In a RISC-V system, the hierarchy of CPUs can be defined > > > > through following nodes that > > > > +are used to describe the layout of physical CPUs in the system: > > > > + > > > > +- packages > > > > +- core > > > > + > > > > +The cpu nodes (bindings defined in [1]) represent the devices that > > > > +correspond to physical CPUs and are to be mapped to the > > > > hierarchy levels. > > > > +Simultaneous multi-threading (SMT) systems can also represent > > > > their topology > > > > +by defining multiple cpu phandles inside core node. The details > > > > are explained > > > > +in paragraph 3. > > > > > > I don't see a reason to do this differently than ARM. That said, I > > > don't think the thread part is in use on ARM, so it could possibly be > > > changed. > > > > > > > + > > > > +The remainder of this document provides the topology bindings > > > > for ARM, based > > > > > > for ARM? > > > > > > > +on the Devicetree Specification, available from: > > > > + > > > > +https://www.devicetree.org/specifications/ > > > > + > > > > +If not stated otherwise, whenever a reference to a cpu node > > > > phandle is made its > > > > +value must point to a cpu node compliant with the cpu node > > > > bindings as > > > > +documented in [1]. > > > > +A topology description containing phandles to cpu nodes that > > > > are not compliant > > > > +with bindings standardized in [1] is therefore considered invalid. > > > > + > > > > +This cpu topology binding description is mostly based on the > > > > topology defined > > > > +in ARM [2]. > > > > +=========================================== > > > > +2 - cpu-topology node > > > > > > cpu-map. Why change this? > > > > > > What I would like to see is the ARM topology binding reworked to be > > > common or some good reasons why it doesn't work for RISC-V as-is. > > > > I think it would be great if CPU topologies were not a RISC-V specific > > thing. We don't really do anything different than anyone else, so > > it'd be great if we could all share the same spec and code. Looking > > quickly at the ARM cpu-map bindings, I don't see any reason why we > > can't just use the same thing on RISC-V -- it's not quite how I'd do > > it, but I don't think the differences are worth having another > > implementation. Mechanically I'm not sure how to do this: should > > there just be a "Documentation/devicetree/bindings/cpu-map.txt"? > > > > If everyone is OK with that then I vote we just go ahead and > > genericise the ARM "cpu-map" stuff for CPU topology. Sharing the > > implementation looks fairly straight-forward as well. > > > > Please check this out... > https://lkml.org/lkml/2018/11/3/99 > > It's also non arch-dependent and it can handle the scheduler's capabilities > better than cpu-map. Could you elaborate on what this does better than cpu-map? I'd be curious to see if we could/should augment cpu-map to cater for those details. Generally, we deliberately avoid placing software abstractions in the DT (e.g. scheduler domains), because the design of these abstractions are specific to a given software stack rather than the hardware, and even in the context of that specific software stack these abstractions change over time. Placing software abstractions in the DT may seem simpler initially, but longer term it ends up requiring translation anyway as abstractions change, and typically it involves placing some policy in the DT, which restricts the ability of software to do something better. Having some translation between the DT and the Linux-internal abstraction is how we expect things to work generally, and allows for a more natural representation in the DT (which may be more or less powerful than the current Linux abstraction). Thanks, Mark.
diff --git a/Documentation/devicetree/bindings/riscv/topology.txt b/Documentation/devicetree/bindings/riscv/topology.txt new file mode 100644 index 00000000..96039ed3 --- /dev/null +++ b/Documentation/devicetree/bindings/riscv/topology.txt @@ -0,0 +1,154 @@ +=========================================== +RISC-V cpu topology binding description +=========================================== + +=========================================== +1 - Introduction +=========================================== + +In a RISC-V system, the hierarchy of CPUs can be defined through following nodes that +are used to describe the layout of physical CPUs in the system: + +- packages +- core + +The cpu nodes (bindings defined in [1]) represent the devices that +correspond to physical CPUs and are to be mapped to the hierarchy levels. +Simultaneous multi-threading (SMT) systems can also represent their topology +by defining multiple cpu phandles inside core node. The details are explained +in paragraph 3. + +The remainder of this document provides the topology bindings for ARM, based +on the Devicetree Specification, available from: + +https://www.devicetree.org/specifications/ + +If not stated otherwise, whenever a reference to a cpu node phandle is made its +value must point to a cpu node compliant with the cpu node bindings as +documented in [1]. +A topology description containing phandles to cpu nodes that are not compliant +with bindings standardized in [1] is therefore considered invalid. + +This cpu topology binding description is mostly based on the topology defined +in ARM [2]. +=========================================== +2 - cpu-topology node +=========================================== + +The RISC-V CPU topology is defined within the "cpu-topology" node, which is a direct +child of the "cpus" node and provides a container where the actual topology +nodes are listed. + +- cpu-topology node + + Usage: Optional - RISC-V SMP systems need to provide CPUs topology to + the OS. RISC-V uniprocessor systems do not require a + topology description and therefore should not define a + cpu-topology node. + + Description: The cpu-topology node is just a container node where its + subnodes describe the CPU topology. + + Node name must be "cpu-topology". + + The cpu-topology node's parent node must be the cpus node. + + The cpu-topology node's child nodes can be: + + - one or more package nodes + + Any other configuration is considered invalid. + +The cpu-topology node can only contain two types of child nodes: + +- package node +- core node + +whose bindings are described in paragraph 3. + +================================================= +2.1 - cpu-topology child nodes naming convention +================================================= + +cpu-topology child nodes must follow a naming convention where the node name +must be "packageN", "coreN" depending on the node type (i.e. package/core). +For SMT systems, coreN node can contain several cpuN to indicate individual +SMT harts (where N = {0, 1, ...} is the node number; nodes which are siblings +within a single common parent node must be given a unique and sequential N +value, starting from 0). cpu-topology child nodes which do not share a common +parent node can have the same name (i.e. same number N as other cpu-topology +child nodes at different device tree levels) since name uniqueness will be +guaranteed by the device tree hierarchy. + +=========================================== +3 - package/core node bindings +=========================================== + +Bindings for package/core nodes are defined as follows: + +- package node + + Description: must be declared within a cpu-topology node, one node + per package. A system can contain several layers of + package nodes. It can also be contained in parent + package nodes. + + The package node name must be "packageN" as described in 2.1 above. + A package node can not be a leaf node. + + A package node's child nodes must be: + + - one or more package nodes; or + - one or more core nodes + + Any other configuration is considered invalid. + +- core node + + Description: must be declared in a package node, one node per core in + the package. + + The core node name must be "coreN" as described in 2.1 above. + + A core node must always be a leaf node. + + Properties for core nodes : + + - cpuN + Usage: required + Value type: <phandle> + Definition: a phandle to the cpu node that corresponds to the + core node. + For SMT systems, a core node will contain multiple cpuN phandles. + + Any other configuration is considered invalid. + +=========================================== +4 - Example dts +=========================================== + +Example : HiFive Unleashed (RISC-V 64 bit, 4 core system) + +L100: cpu-topology { + package0 { + core0 { + cpu0 = <&L12>; + }; + core1 { + cpu0 = <&L15>; + }; + core2 { + cpu0 = <&L18>; + }; + core3 { + cpu0 = <&L21>; + }; + }; + }; +=============================================================================== +[1] RISC-V cpus documentation + Documentation/devicetree/binding/riscv/cpus.txt +[2] ARM topology documentation + Documentation/devicetree/binding/arm/topology.txt + +===============================================================================
Define a RISC-V cpu topology. This is based on cpu-map in ARM world. But it doesn't need a separate thread node for defining SMT systems. Multiple cpu phandle properties can be parsed to identify the sibling hardware threads. Moreover, we do not have cluster concept in RISC-V. So package is a better word choice than cluster for RISC-V. Signed-off-by: Atish Patra <atish.patra@wdc.com> --- .../devicetree/bindings/riscv/topology.txt | 154 +++++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 Documentation/devicetree/bindings/riscv/topology.txt