Re: [PATCH v2] arm64: dts: foundation-v8: Enable PSCI mode

2017-10-03 Thread Sudeep Holla 


On 03/10/17 10:12, Daniel Thompson wrote:
> On 02/10/17 18:26, Sudeep Holla wrote:
>> Sorry for late response, I thought I had sent this mail out long back
>> but was sitting in my draft :(
> 
> No worries. I've been at Linaro connect this last week anyway.
> 
> 
>> On 20/09/17 12:17, Daniel Thompson wrote:
>>> On 20/09/17 10:42, Sudeep Holla wrote:


 On 19/09/17 19:32, Daniel Thompson wrote:
> Currently if the Foundation model is running ARM Trusted Firmware then
> the kernel, which is configured to use spin tables, cannot start
> secondary
> processors or "power off" the simulation.
>
> After adding a couple of labels to the include file and splitting out
> the
> spin-table configuration into a header, we add a couple of new headers
> together with two new DTs (GICv2+PSCI and GICv3+PSCI).
>
> The new GICv3+PSCI DT has been boot tested, the remaining three
> (two of
> which existed prior to this patch) have been "tested" by
> decompiling the
> blobs and comparing them against a reference.
>

 How different are these from the ones hosted in [1] ?
>>>
>>> They look like they were either independently written or diverged a long
>>> time ago. The existing kernel DTs describe hardware absent from the ARM
>>> TF ones and vice versa.
>>>
>>
>> OK.
>>
>>> With specific reference to PSCI it looks like my patches could perhaps
>>> be improved by adding idle-state support.
>>
>> Yes I know.
> 
> You want a v3 with it added?
> 

No, that's fine. I have pushed this already [1] with Mark's ack for now.
We can add it later after some testing, not urgent.

--
Regards,
Sudeep

[1] https://git.kernel.org/sudeep.holla/linux/h/for-next/juno

Re: [PATCH v2] arm64: dts: foundation-v8: Enable PSCI mode

2017-10-03 Thread Sudeep Holla 


On 03/10/17 10:12, Daniel Thompson wrote:
> On 02/10/17 18:26, Sudeep Holla wrote:
>> Sorry for late response, I thought I had sent this mail out long back
>> but was sitting in my draft :(
> 
> No worries. I've been at Linaro connect this last week anyway.
> 
> 
>> On 20/09/17 12:17, Daniel Thompson wrote:
>>> On 20/09/17 10:42, Sudeep Holla wrote:


 On 19/09/17 19:32, Daniel Thompson wrote:
> Currently if the Foundation model is running ARM Trusted Firmware then
> the kernel, which is configured to use spin tables, cannot start
> secondary
> processors or "power off" the simulation.
>
> After adding a couple of labels to the include file and splitting out
> the
> spin-table configuration into a header, we add a couple of new headers
> together with two new DTs (GICv2+PSCI and GICv3+PSCI).
>
> The new GICv3+PSCI DT has been boot tested, the remaining three
> (two of
> which existed prior to this patch) have been "tested" by
> decompiling the
> blobs and comparing them against a reference.
>

 How different are these from the ones hosted in [1] ?
>>>
>>> They look like they were either independently written or diverged a long
>>> time ago. The existing kernel DTs describe hardware absent from the ARM
>>> TF ones and vice versa.
>>>
>>
>> OK.
>>
>>> With specific reference to PSCI it looks like my patches could perhaps
>>> be improved by adding idle-state support.
>>
>> Yes I know.
> 
> You want a v3 with it added?
> 

No, that's fine. I have pushed this already [1] with Mark's ack for now.
We can add it later after some testing, not urgent.

--
Regards,
Sudeep

[1] https://git.kernel.org/sudeep.holla/linux/h/for-next/juno

Re: [PATCH v2] arm64: dts: foundation-v8: Enable PSCI mode

2017-10-03 Thread Ard Biesheuvel 
On 3 October 2017 at 10:12, Daniel Thompson  wrote:
> On 02/10/17 18:26, Sudeep Holla wrote:
>>
>> Sorry for late response, I thought I had sent this mail out long back
>> but was sitting in my draft :(
>
>
> No worries. I've been at Linaro connect this last week anyway.
>
>
>> On 20/09/17 12:17, Daniel Thompson wrote:
>>>
>>> On 20/09/17 10:42, Sudeep Holla wrote:



 On 19/09/17 19:32, Daniel Thompson wrote:
>
> Currently if the Foundation model is running ARM Trusted Firmware then
> the kernel, which is configured to use spin tables, cannot start
> secondary
> processors or "power off" the simulation.
>
> After adding a couple of labels to the include file and splitting out
> the
> spin-table configuration into a header, we add a couple of new headers
> together with two new DTs (GICv2+PSCI and GICv3+PSCI).
>
> The new GICv3+PSCI DT has been boot tested, the remaining three (two of
> which existed prior to this patch) have been "tested" by decompiling
> the
> blobs and comparing them against a reference.
>

 How different are these from the ones hosted in [1] ?
>>>
>>>
>>> They look like they were either independently written or diverged a long
>>> time ago. The existing kernel DTs describe hardware absent from the ARM
>>> TF ones and vice versa.
>>>
>>
>> OK.
>>
>>> With specific reference to PSCI it looks like my patches could perhaps
>>> be improved by adding idle-state support.
>>
>>
>> Yes I know.
>
>
> You want a v3 with it added?
>
>
 On argument is that we want to take the DTS out of device tree as
 firmware is responsible for generating them. Alternatively, we may be
 duplicating resulting in discrepancies over time by coping it into
 kernel.
>>>
>>>
>>> The general problem is copying from where?
>>>
>>> The kernel DTs are a well maintained centralized repository which is
>>> *really* useful. git grep across the kernel DTs is a hugely powerful
>>> tool when trying to better understand an ecosystem as sprawling and
>>> diverse as ARMs. In fact I've even seen those sort of searchs used as a
>>> basis to clean up unused code. Seeing that centralized repository
>>> splinter into separate per-vendor silos would be a huge loss for kernel
>>> developers.
>>>
>>
>> Agreed. But models are configurable and last time this discussion came
>> up, some argued that the DTs must be modified based on the configuration
>> automatically by models or some external scripts.
>
>
> Indeed. I can definitely understand why the *models* might want
> to be bundled with DTs (or a DT generator, or a
> --just-give-me-a-working-dt-for-my-command-line-options-option).
>
>

The UEFI firmware for the FVP models does implement automatic
selection between a variety of builtin DTBs. I.e., FVP base or
foundation model, with either GICv2 for GICv3. I think the maintainer
of the binary releases chose not to enable this in the builds he
distributes, but it is certainly unnecessary to fiddle with DT images
if you build the firmware yourself (but you need to set DTB_DIR when
building)

Re: [PATCH v2] arm64: dts: foundation-v8: Enable PSCI mode

2017-10-03 Thread Ard Biesheuvel 
On 3 October 2017 at 10:12, Daniel Thompson  wrote:
> On 02/10/17 18:26, Sudeep Holla wrote:
>>
>> Sorry for late response, I thought I had sent this mail out long back
>> but was sitting in my draft :(
>
>
> No worries. I've been at Linaro connect this last week anyway.
>
>
>> On 20/09/17 12:17, Daniel Thompson wrote:
>>>
>>> On 20/09/17 10:42, Sudeep Holla wrote:



 On 19/09/17 19:32, Daniel Thompson wrote:
>
> Currently if the Foundation model is running ARM Trusted Firmware then
> the kernel, which is configured to use spin tables, cannot start
> secondary
> processors or "power off" the simulation.
>
> After adding a couple of labels to the include file and splitting out
> the
> spin-table configuration into a header, we add a couple of new headers
> together with two new DTs (GICv2+PSCI and GICv3+PSCI).
>
> The new GICv3+PSCI DT has been boot tested, the remaining three (two of
> which existed prior to this patch) have been "tested" by decompiling
> the
> blobs and comparing them against a reference.
>

 How different are these from the ones hosted in [1] ?
>>>
>>>
>>> They look like they were either independently written or diverged a long
>>> time ago. The existing kernel DTs describe hardware absent from the ARM
>>> TF ones and vice versa.
>>>
>>
>> OK.
>>
>>> With specific reference to PSCI it looks like my patches could perhaps
>>> be improved by adding idle-state support.
>>
>>
>> Yes I know.
>
>
> You want a v3 with it added?
>
>
 On argument is that we want to take the DTS out of device tree as
 firmware is responsible for generating them. Alternatively, we may be
 duplicating resulting in discrepancies over time by coping it into
 kernel.
>>>
>>>
>>> The general problem is copying from where?
>>>
>>> The kernel DTs are a well maintained centralized repository which is
>>> *really* useful. git grep across the kernel DTs is a hugely powerful
>>> tool when trying to better understand an ecosystem as sprawling and
>>> diverse as ARMs. In fact I've even seen those sort of searchs used as a
>>> basis to clean up unused code. Seeing that centralized repository
>>> splinter into separate per-vendor silos would be a huge loss for kernel
>>> developers.
>>>
>>
>> Agreed. But models are configurable and last time this discussion came
>> up, some argued that the DTs must be modified based on the configuration
>> automatically by models or some external scripts.
>
>
> Indeed. I can definitely understand why the *models* might want
> to be bundled with DTs (or a DT generator, or a
> --just-give-me-a-working-dt-for-my-command-line-options-option).
>
>

The UEFI firmware for the FVP models does implement automatic
selection between a variety of builtin DTBs. I.e., FVP base or
foundation model, with either GICv2 for GICv3. I think the maintainer
of the binary releases chose not to enable this in the builds he
distributes, but it is certainly unnecessary to fiddle with DT images
if you build the firmware yourself (but you need to set DTB_DIR when
building)

Re: [PATCH v2] arm64: dts: foundation-v8: Enable PSCI mode

2017-10-03 Thread Daniel Thompson 

On 02/10/17 18:26, Sudeep Holla wrote:

Sorry for late response, I thought I had sent this mail out long back
but was sitting in my draft :(


No worries. I've been at Linaro connect this last week anyway.



On 20/09/17 12:17, Daniel Thompson wrote:

On 20/09/17 10:42, Sudeep Holla wrote:



On 19/09/17 19:32, Daniel Thompson wrote:

Currently if the Foundation model is running ARM Trusted Firmware then
the kernel, which is configured to use spin tables, cannot start
secondary
processors or "power off" the simulation.

After adding a couple of labels to the include file and splitting out
the
spin-table configuration into a header, we add a couple of new headers
together with two new DTs (GICv2+PSCI and GICv3+PSCI).

The new GICv3+PSCI DT has been boot tested, the remaining three (two of
which existed prior to this patch) have been "tested" by decompiling the
blobs and comparing them against a reference.



How different are these from the ones hosted in [1] ?


They look like they were either independently written or diverged a long
time ago. The existing kernel DTs describe hardware absent from the ARM
TF ones and vice versa.



OK.


With specific reference to PSCI it looks like my patches could perhaps
be improved by adding idle-state support.


Yes I know.


You want a v3 with it added?



On argument is that we want to take the DTS out of device tree as
firmware is responsible for generating them. Alternatively, we may be
duplicating resulting in discrepancies over time by coping it into
kernel.


The general problem is copying from where?

The kernel DTs are a well maintained centralized repository which is
*really* useful. git grep across the kernel DTs is a hugely powerful
tool when trying to better understand an ecosystem as sprawling and
diverse as ARMs. In fact I've even seen those sort of searchs used as a
basis to clean up unused code. Seeing that centralized repository
splinter into separate per-vendor silos would be a huge loss for kernel
developers.



Agreed. But models are configurable and last time this discussion came
up, some argued that the DTs must be modified based on the configuration
automatically by models or some external scripts.


Indeed. I can definitely understand why the *models* might want
to be bundled with DTs (or a DT generator, or a
--just-give-me-a-working-dt-for-my-command-line-options-option).




In other words, whilst people could discuss alternative ways to manage
DTs[1], I can't see any universe where ARM TF would be a logical place
to keep them.

[1] ... and I'd further suggest that only perhaps people who are
     prepared to put resources into fixing it should convene such a
     discussion.


While I agree with you, I am worried on the scalability. Models provide
tons of options(may not be foundation platform but for sure the base AEM
ones). It may so get unmanageable if we keep adding one DTS per
configuration.

If we are OK restricting the set of configurations to what you have
proposed, then it seems fine.

I will try to push this for v4.15, but I still want to hear more
opinions on the scalability part here.


I'm sure you mean other peoples opinions ;-) (especially so, since after 
reading things back, I realized I expressed mine rather more forefully 
than intended) but I think there is a reason why PSCI is special... 
basically its to do with the docs.


I figured out how to use foundation model by reading the documentation. 
That documentation says "to run Linux go read the Linaro stuff". As it 
happens the Linaro "stuff" loops you through to a different ARM doc 
which tells you how to download ARM TF and bootloader binaries from 
Linaro. Thus if you follow the docs (and are patient enough) you come 
out the other side with a model running ARM TF and ready for PSCI.


At that point you're ready for mainline kernel development... and you 
wonder why there is only one CPU running ;-).



Daniel.

Re: [PATCH v2] arm64: dts: foundation-v8: Enable PSCI mode

2017-10-03 Thread Daniel Thompson 

On 02/10/17 18:26, Sudeep Holla wrote:

Sorry for late response, I thought I had sent this mail out long back
but was sitting in my draft :(


No worries. I've been at Linaro connect this last week anyway.



On 20/09/17 12:17, Daniel Thompson wrote:

On 20/09/17 10:42, Sudeep Holla wrote:



On 19/09/17 19:32, Daniel Thompson wrote:

Currently if the Foundation model is running ARM Trusted Firmware then
the kernel, which is configured to use spin tables, cannot start
secondary
processors or "power off" the simulation.

After adding a couple of labels to the include file and splitting out
the
spin-table configuration into a header, we add a couple of new headers
together with two new DTs (GICv2+PSCI and GICv3+PSCI).

The new GICv3+PSCI DT has been boot tested, the remaining three (two of
which existed prior to this patch) have been "tested" by decompiling the
blobs and comparing them against a reference.



How different are these from the ones hosted in [1] ?


They look like they were either independently written or diverged a long
time ago. The existing kernel DTs describe hardware absent from the ARM
TF ones and vice versa.



OK.


With specific reference to PSCI it looks like my patches could perhaps
be improved by adding idle-state support.


Yes I know.


You want a v3 with it added?



On argument is that we want to take the DTS out of device tree as
firmware is responsible for generating them. Alternatively, we may be
duplicating resulting in discrepancies over time by coping it into
kernel.


The general problem is copying from where?

The kernel DTs are a well maintained centralized repository which is
*really* useful. git grep across the kernel DTs is a hugely powerful
tool when trying to better understand an ecosystem as sprawling and
diverse as ARMs. In fact I've even seen those sort of searchs used as a
basis to clean up unused code. Seeing that centralized repository
splinter into separate per-vendor silos would be a huge loss for kernel
developers.



Agreed. But models are configurable and last time this discussion came
up, some argued that the DTs must be modified based on the configuration
automatically by models or some external scripts.


Indeed. I can definitely understand why the *models* might want
to be bundled with DTs (or a DT generator, or a
--just-give-me-a-working-dt-for-my-command-line-options-option).




In other words, whilst people could discuss alternative ways to manage
DTs[1], I can't see any universe where ARM TF would be a logical place
to keep them.

[1] ... and I'd further suggest that only perhaps people who are
     prepared to put resources into fixing it should convene such a
     discussion.


While I agree with you, I am worried on the scalability. Models provide
tons of options(may not be foundation platform but for sure the base AEM
ones). It may so get unmanageable if we keep adding one DTS per
configuration.

If we are OK restricting the set of configurations to what you have
proposed, then it seems fine.

I will try to push this for v4.15, but I still want to hear more
opinions on the scalability part here.


I'm sure you mean other peoples opinions ;-) (especially so, since after 
reading things back, I realized I expressed mine rather more forefully 
than intended) but I think there is a reason why PSCI is special... 
basically its to do with the docs.


I figured out how to use foundation model by reading the documentation. 
That documentation says "to run Linux go read the Linaro stuff". As it 
happens the Linaro "stuff" loops you through to a different ARM doc 
which tells you how to download ARM TF and bootloader binaries from 
Linaro. Thus if you follow the docs (and are patient enough) you come 
out the other side with a model running ARM TF and ready for PSCI.


At that point you're ready for mainline kernel development... and you 
wonder why there is only one CPU running ;-).



Daniel.

Re: [PATCH v2] arm64: dts: foundation-v8: Enable PSCI mode

2017-10-02 Thread Sudeep Holla 
Sorry for late response, I thought I had sent this mail out long back
but was sitting in my draft :(

On 20/09/17 12:17, Daniel Thompson wrote:
> On 20/09/17 10:42, Sudeep Holla wrote:
>>
>>
>> On 19/09/17 19:32, Daniel Thompson wrote:
>>> Currently if the Foundation model is running ARM Trusted Firmware then
>>> the kernel, which is configured to use spin tables, cannot start
>>> secondary
>>> processors or "power off" the simulation.
>>>
>>> After adding a couple of labels to the include file and splitting out
>>> the
>>> spin-table configuration into a header, we add a couple of new headers
>>> together with two new DTs (GICv2+PSCI and GICv3+PSCI).
>>>
>>> The new GICv3+PSCI DT has been boot tested, the remaining three (two of
>>> which existed prior to this patch) have been "tested" by decompiling the
>>> blobs and comparing them against a reference.
>>>
>>
>> How different are these from the ones hosted in [1] ?
> 
> They look like they were either independently written or diverged a long
> time ago. The existing kernel DTs describe hardware absent from the ARM
> TF ones and vice versa.
> 

OK.

> With specific reference to PSCI it looks like my patches could perhaps
> be improved by adding idle-state support.
> 
> 

Yes I know.

>> On argument is that we want to take the DTS out of device tree as
>> firmware is responsible for generating them. Alternatively, we may be
>> duplicating resulting in discrepancies over time by coping it into
>> kernel.
> 
> The general problem is copying from where?
> 
> The kernel DTs are a well maintained centralized repository which is
> *really* useful. git grep across the kernel DTs is a hugely powerful
> tool when trying to better understand an ecosystem as sprawling and
> diverse as ARMs. In fact I've even seen those sort of searchs used as a
> basis to clean up unused code. Seeing that centralized repository
> splinter into separate per-vendor silos would be a huge loss for kernel
> developers.
> 

Agreed. But models are configurable and last time this discussion came
up, some argued that the DTs must be modified based on the configuration
automatically by models or some external scripts.
> 
>> Since users of ARM TF must be able to access these, I am not sure if it
>> makes sense to merge these. Or we remove it from ARM TF to avoid any
>> conflicts/discrepancies. >
>> Thoughts ?
> 
> I kind of agree that maintaining DTs and DT documentation in the kernel
> is a little odd given that the kernel is not the only player here
> (FreeBSD, u-boot, etc). However it is sufficiently well maintained that
> projects are content(ish) to regard the kernel as the canonical source
> for these things (u-boot, for example, seeks to shadow kernel DTs
> without modifying them).
> 

No argument there.

> However regardless of the above I'd say they should be removed from ARM
> TF. ARM TF does not use, modify, pass on or in any way consume DT... it
> has no skin in the game here. Why does it want to own a few of blobs for
> a small subset of the platforms it supports? I'm afraid that makes no
> sense to me, to the extent that it didn't even occur to me to *look* in
> the ARM TF sources to find any DTs for FVP until you pointed them out.
> 

Agreed, I can talk to TF guys if required.

> In other words, whilst people could discuss alternative ways to manage
> DTs[1], I can't see any universe where ARM TF would be a logical place
> to keep them.
> 
> [1] ... and I'd further suggest that only perhaps people who are
>     prepared to put resources into fixing it should convene such a
>     discussion.

While I agree with you, I am worried on the scalability. Models provide
tons of options(may not be foundation platform but for sure the base AEM
ones). It may so get unmanageable if we keep adding one DTS per
configuration.

If we are OK restricting the set of configurations to what you have
proposed, then it seems fine.

I will try to push this for v4.15, but I still want to hear more
opinions on the scalability part here.

-- 
Regards,
Sudeep

Re: [PATCH v2] arm64: dts: foundation-v8: Enable PSCI mode

2017-10-02 Thread Sudeep Holla 
Sorry for late response, I thought I had sent this mail out long back
but was sitting in my draft :(

On 20/09/17 12:17, Daniel Thompson wrote:
> On 20/09/17 10:42, Sudeep Holla wrote:
>>
>>
>> On 19/09/17 19:32, Daniel Thompson wrote:
>>> Currently if the Foundation model is running ARM Trusted Firmware then
>>> the kernel, which is configured to use spin tables, cannot start
>>> secondary
>>> processors or "power off" the simulation.
>>>
>>> After adding a couple of labels to the include file and splitting out
>>> the
>>> spin-table configuration into a header, we add a couple of new headers
>>> together with two new DTs (GICv2+PSCI and GICv3+PSCI).
>>>
>>> The new GICv3+PSCI DT has been boot tested, the remaining three (two of
>>> which existed prior to this patch) have been "tested" by decompiling the
>>> blobs and comparing them against a reference.
>>>
>>
>> How different are these from the ones hosted in [1] ?
> 
> They look like they were either independently written or diverged a long
> time ago. The existing kernel DTs describe hardware absent from the ARM
> TF ones and vice versa.
> 

OK.

> With specific reference to PSCI it looks like my patches could perhaps
> be improved by adding idle-state support.
> 
> 

Yes I know.

>> On argument is that we want to take the DTS out of device tree as
>> firmware is responsible for generating them. Alternatively, we may be
>> duplicating resulting in discrepancies over time by coping it into
>> kernel.
> 
> The general problem is copying from where?
> 
> The kernel DTs are a well maintained centralized repository which is
> *really* useful. git grep across the kernel DTs is a hugely powerful
> tool when trying to better understand an ecosystem as sprawling and
> diverse as ARMs. In fact I've even seen those sort of searchs used as a
> basis to clean up unused code. Seeing that centralized repository
> splinter into separate per-vendor silos would be a huge loss for kernel
> developers.
> 

Agreed. But models are configurable and last time this discussion came
up, some argued that the DTs must be modified based on the configuration
automatically by models or some external scripts.
> 
>> Since users of ARM TF must be able to access these, I am not sure if it
>> makes sense to merge these. Or we remove it from ARM TF to avoid any
>> conflicts/discrepancies. >
>> Thoughts ?
> 
> I kind of agree that maintaining DTs and DT documentation in the kernel
> is a little odd given that the kernel is not the only player here
> (FreeBSD, u-boot, etc). However it is sufficiently well maintained that
> projects are content(ish) to regard the kernel as the canonical source
> for these things (u-boot, for example, seeks to shadow kernel DTs
> without modifying them).
> 

No argument there.

> However regardless of the above I'd say they should be removed from ARM
> TF. ARM TF does not use, modify, pass on or in any way consume DT... it
> has no skin in the game here. Why does it want to own a few of blobs for
> a small subset of the platforms it supports? I'm afraid that makes no
> sense to me, to the extent that it didn't even occur to me to *look* in
> the ARM TF sources to find any DTs for FVP until you pointed them out.
> 

Agreed, I can talk to TF guys if required.

> In other words, whilst people could discuss alternative ways to manage
> DTs[1], I can't see any universe where ARM TF would be a logical place
> to keep them.
> 
> [1] ... and I'd further suggest that only perhaps people who are
>     prepared to put resources into fixing it should convene such a
>     discussion.

While I agree with you, I am worried on the scalability. Models provide
tons of options(may not be foundation platform but for sure the base AEM
ones). It may so get unmanageable if we keep adding one DTS per
configuration.

If we are OK restricting the set of configurations to what you have
proposed, then it seems fine.

I will try to push this for v4.15, but I still want to hear more
opinions on the scalability part here.

-- 
Regards,
Sudeep

Re: [PATCH v2] arm64: dts: foundation-v8: Enable PSCI mode

2017-09-20 Thread Daniel Thompson 

On 20/09/17 10:42, Sudeep Holla wrote:



On 19/09/17 19:32, Daniel Thompson wrote:

Currently if the Foundation model is running ARM Trusted Firmware then
the kernel, which is configured to use spin tables, cannot start secondary
processors or "power off" the simulation.

After adding a couple of labels to the include file and splitting out the
spin-table configuration into a header, we add a couple of new headers
together with two new DTs (GICv2+PSCI and GICv3+PSCI).

The new GICv3+PSCI DT has been boot tested, the remaining three (two of
which existed prior to this patch) have been "tested" by decompiling the
blobs and comparing them against a reference.



How different are these from the ones hosted in [1] ?


They look like they were either independently written or diverged a long 
time ago. The existing kernel DTs describe hardware absent from the ARM 
TF ones and vice versa.


With specific reference to PSCI it looks like my patches could perhaps 
be improved by adding idle-state support.




On argument is that we want to take the DTS out of device tree as
firmware is responsible for generating them. Alternatively, we may be
duplicating resulting in discrepancies over time by coping it into kernel.


The general problem is copying from where?

The kernel DTs are a well maintained centralized repository which is 
*really* useful. git grep across the kernel DTs is a hugely powerful 
tool when trying to better understand an ecosystem as sprawling and 
diverse as ARMs. In fact I've even seen those sort of searchs used as a 
basis to clean up unused code. Seeing that centralized repository 
splinter into separate per-vendor silos would be a huge loss for kernel 
developers.




Since users of ARM TF must be able to access these, I am not sure if it
makes sense to merge these. Or we remove it from ARM TF to avoid any
conflicts/discrepancies. >
Thoughts ?


I kind of agree that maintaining DTs and DT documentation in the kernel 
is a little odd given that the kernel is not the only player here 
(FreeBSD, u-boot, etc). However it is sufficiently well maintained that 
projects are content(ish) to regard the kernel as the canonical source 
for these things (u-boot, for example, seeks to shadow kernel DTs 
without modifying them).


However regardless of the above I'd say they should be removed from ARM 
TF. ARM TF does not use, modify, pass on or in any way consume DT... it 
has no skin in the game here. Why does it want to own a few of blobs for 
a small subset of the platforms it supports? I'm afraid that makes no 
sense to me, to the extent that it didn't even occur to me to *look* in 
the ARM TF sources to find any DTs for FVP until you pointed them out.


In other words, whilst people could discuss alternative ways to manage 
DTs[1], I can't see any universe where ARM TF would be a logical place 
to keep them.



Daniel.


[1] ... and I'd further suggest that only perhaps people who are
prepared to put resources into fixing it should convene such a
discussion.

Re: [PATCH v2] arm64: dts: foundation-v8: Enable PSCI mode

2017-09-20 Thread Daniel Thompson 

On 20/09/17 10:42, Sudeep Holla wrote:



On 19/09/17 19:32, Daniel Thompson wrote:

Currently if the Foundation model is running ARM Trusted Firmware then
the kernel, which is configured to use spin tables, cannot start secondary
processors or "power off" the simulation.

After adding a couple of labels to the include file and splitting out the
spin-table configuration into a header, we add a couple of new headers
together with two new DTs (GICv2+PSCI and GICv3+PSCI).

The new GICv3+PSCI DT has been boot tested, the remaining three (two of
which existed prior to this patch) have been "tested" by decompiling the
blobs and comparing them against a reference.



How different are these from the ones hosted in [1] ?


They look like they were either independently written or diverged a long 
time ago. The existing kernel DTs describe hardware absent from the ARM 
TF ones and vice versa.


With specific reference to PSCI it looks like my patches could perhaps 
be improved by adding idle-state support.




On argument is that we want to take the DTS out of device tree as
firmware is responsible for generating them. Alternatively, we may be
duplicating resulting in discrepancies over time by coping it into kernel.


The general problem is copying from where?

The kernel DTs are a well maintained centralized repository which is 
*really* useful. git grep across the kernel DTs is a hugely powerful 
tool when trying to better understand an ecosystem as sprawling and 
diverse as ARMs. In fact I've even seen those sort of searchs used as a 
basis to clean up unused code. Seeing that centralized repository 
splinter into separate per-vendor silos would be a huge loss for kernel 
developers.




Since users of ARM TF must be able to access these, I am not sure if it
makes sense to merge these. Or we remove it from ARM TF to avoid any
conflicts/discrepancies. >
Thoughts ?


I kind of agree that maintaining DTs and DT documentation in the kernel 
is a little odd given that the kernel is not the only player here 
(FreeBSD, u-boot, etc). However it is sufficiently well maintained that 
projects are content(ish) to regard the kernel as the canonical source 
for these things (u-boot, for example, seeks to shadow kernel DTs 
without modifying them).


However regardless of the above I'd say they should be removed from ARM 
TF. ARM TF does not use, modify, pass on or in any way consume DT... it 
has no skin in the game here. Why does it want to own a few of blobs for 
a small subset of the platforms it supports? I'm afraid that makes no 
sense to me, to the extent that it didn't even occur to me to *look* in 
the ARM TF sources to find any DTs for FVP until you pointed them out.


In other words, whilst people could discuss alternative ways to manage 
DTs[1], I can't see any universe where ARM TF would be a logical place 
to keep them.



Daniel.


[1] ... and I'd further suggest that only perhaps people who are
prepared to put resources into fixing it should convene such a
discussion.

Re: [PATCH v2] arm64: dts: foundation-v8: Enable PSCI mode

2017-09-20 Thread Sudeep Holla 


On 19/09/17 19:32, Daniel Thompson wrote:
> Currently if the Foundation model is running ARM Trusted Firmware then
> the kernel, which is configured to use spin tables, cannot start secondary
> processors or "power off" the simulation.
> 
> After adding a couple of labels to the include file and splitting out the
> spin-table configuration into a header, we add a couple of new headers
> together with two new DTs (GICv2+PSCI and GICv3+PSCI).
> 
> The new GICv3+PSCI DT has been boot tested, the remaining three (two of
> which existed prior to this patch) have been "tested" by decompiling the
> blobs and comparing them against a reference.
> 

How different are these from the ones hosted in [1] ?

On argument is that we want to take the DTS out of device tree as
firmware is responsible for generating them. Alternatively, we may be
duplicating resulting in discrepancies over time by coping it into kernel.

Since users of ARM TF must be able to access these, I am not sure if it
makes sense to merge these. Or we remove it from ARM TF to avoid any
conflicts/discrepancies.

Thoughts ?

--
Regards,
Sudeep

[1] https://github.com/ARM-software/arm-trusted-firmware/tree/master/fdts

Re: [PATCH v2] arm64: dts: foundation-v8: Enable PSCI mode

2017-09-20 Thread Sudeep Holla 


On 19/09/17 19:32, Daniel Thompson wrote:
> Currently if the Foundation model is running ARM Trusted Firmware then
> the kernel, which is configured to use spin tables, cannot start secondary
> processors or "power off" the simulation.
> 
> After adding a couple of labels to the include file and splitting out the
> spin-table configuration into a header, we add a couple of new headers
> together with two new DTs (GICv2+PSCI and GICv3+PSCI).
> 
> The new GICv3+PSCI DT has been boot tested, the remaining three (two of
> which existed prior to this patch) have been "tested" by decompiling the
> blobs and comparing them against a reference.
> 

How different are these from the ones hosted in [1] ?

On argument is that we want to take the DTS out of device tree as
firmware is responsible for generating them. Alternatively, we may be
duplicating resulting in discrepancies over time by coping it into kernel.

Since users of ARM TF must be able to access these, I am not sure if it
makes sense to merge these. Or we remove it from ARM TF to avoid any
conflicts/discrepancies.

Thoughts ?

--
Regards,
Sudeep

[1] https://github.com/ARM-software/arm-trusted-firmware/tree/master/fdts

[PATCH v2] arm64: dts: foundation-v8: Enable PSCI mode

2017-09-19 Thread Daniel Thompson 
Currently if the Foundation model is running ARM Trusted Firmware then
the kernel, which is configured to use spin tables, cannot start secondary
processors or "power off" the simulation.

After adding a couple of labels to the include file and splitting out the
spin-table configuration into a header, we add a couple of new headers
together with two new DTs (GICv2+PSCI and GICv3+PSCI).

The new GICv3+PSCI DT has been boot tested, the remaining three (two of
which existed prior to this patch) have been "tested" by decompiling the
blobs and comparing them against a reference.

Acked-by: Mark Rutland 
Signed-off-by: Daniel Thompson 
---
 arch/arm64/boot/dts/arm/Makefile   |  4 +++-
 arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi   | 19 +++
 .../boot/dts/arm/foundation-v8-gicv3-psci.dts  |  9 +++
 arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts| 25 ++-
 arch/arm64/boot/dts/arm/foundation-v8-gicv3.dtsi   | 28 ++
 arch/arm64/boot/dts/arm/foundation-v8-psci.dts |  9 +++
 arch/arm64/boot/dts/arm/foundation-v8-psci.dtsi| 28 ++
 .../boot/dts/arm/foundation-v8-spin-table.dtsi | 25 +++
 arch/arm64/boot/dts/arm/foundation-v8.dts  | 16 ++---
 arch/arm64/boot/dts/arm/foundation-v8.dtsi | 16 -
 10 files changed, 129 insertions(+), 50 deletions(-)
 create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi
 create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts
 create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-gicv3.dtsi
 create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-psci.dts
 create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-psci.dtsi
 create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-spin-table.dtsi

diff --git a/arch/arm64/boot/dts/arm/Makefile b/arch/arm64/boot/dts/arm/Makefile
index 75cc2aa10101..25f82c377f67 100644
--- a/arch/arm64/boot/dts/arm/Makefile
+++ b/arch/arm64/boot/dts/arm/Makefile
@@ -1,4 +1,6 @@
-dtb-$(CONFIG_ARCH_VEXPRESS) += foundation-v8.dtb foundation-v8-gicv3.dtb
+dtb-$(CONFIG_ARCH_VEXPRESS) += \
+   foundation-v8.dtb foundation-v8-psci.dtb \
+   foundation-v8-gicv3.dtb foundation-v8-gicv3-psci.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += juno.dtb juno-r1.dtb juno-r2.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += vexpress-v2f-1xv7-ca53x2.dtb
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi 
b/arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi
new file mode 100644
index ..851abf34fc80
--- /dev/null
+++ b/arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi
@@ -0,0 +1,19 @@
+/*
+ * ARM Ltd.
+ *
+ * ARMv8 Foundation model DTS (GICv2 configuration)
+ */
+
+/ {
+   gic: interrupt-controller@2c001000 {
+   compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
+   #interrupt-cells = <3>;
+   #address-cells = <2>;
+   interrupt-controller;
+   reg = <0x0 0x2c001000 0 0x1000>,
+ <0x0 0x2c002000 0 0x2000>,
+ <0x0 0x2c004000 0 0x2000>,
+ <0x0 0x2c006000 0 0x2000>;
+   interrupts = <1 9 0xf04>;
+   };
+};
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts 
b/arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts
new file mode 100644
index ..e096e670bec3
--- /dev/null
+++ b/arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts
@@ -0,0 +1,9 @@
+/*
+ * ARM Ltd.
+ *
+ * ARMv8 Foundation model DTS (GICv3+PSCI configuration)
+ */
+
+#include "foundation-v8.dtsi"
+#include "foundation-v8-gicv3.dtsi"
+#include "foundation-v8-psci.dtsi"
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts 
b/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts
index 35588dfa095c..c5d834d7d0ba 100644
--- a/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts
+++ b/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts
@@ -5,26 +5,5 @@
  */

 #include "foundation-v8.dtsi"
-
-/ {
-   gic: interrupt-controller@2f00 {
-   compatible = "arm,gic-v3";
-   #interrupt-cells = <3>;
-   #address-cells = <2>;
-   #size-cells = <2>;
-   ranges;
-   interrupt-controller;
-   reg =   <0x0 0x2f00 0x0 0x1>,
-   <0x0 0x2f10 0x0 0x20>,
-   <0x0 0x2c00 0x0 0x2000>,
-   <0x0 0x2c01 0x0 0x2000>,
-   <0x0 0x2c02f000 0x0 0x2000>;
-   interrupts = <1 9 4>;
-
-   its: its@2f02 {
-   compatible = "arm,gic-v3-its";
-   msi-controller;
-   reg = <0x0 0x2f02 0x0 0x2>;
-   };
-   };
-};
+#include "foundation-v8-gicv3.dtsi"
+#include "foundation-v8-spin-table.dtsi"
diff --git

[PATCH v2] arm64: dts: foundation-v8: Enable PSCI mode

2017-09-19 Thread Daniel Thompson 
Currently if the Foundation model is running ARM Trusted Firmware then
the kernel, which is configured to use spin tables, cannot start secondary
processors or "power off" the simulation.

After adding a couple of labels to the include file and splitting out the
spin-table configuration into a header, we add a couple of new headers
together with two new DTs (GICv2+PSCI and GICv3+PSCI).

The new GICv3+PSCI DT has been boot tested, the remaining three (two of
which existed prior to this patch) have been "tested" by decompiling the
blobs and comparing them against a reference.

Acked-by: Mark Rutland 
Signed-off-by: Daniel Thompson 
---
 arch/arm64/boot/dts/arm/Makefile   |  4 +++-
 arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi   | 19 +++
 .../boot/dts/arm/foundation-v8-gicv3-psci.dts  |  9 +++
 arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts| 25 ++-
 arch/arm64/boot/dts/arm/foundation-v8-gicv3.dtsi   | 28 ++
 arch/arm64/boot/dts/arm/foundation-v8-psci.dts |  9 +++
 arch/arm64/boot/dts/arm/foundation-v8-psci.dtsi| 28 ++
 .../boot/dts/arm/foundation-v8-spin-table.dtsi | 25 +++
 arch/arm64/boot/dts/arm/foundation-v8.dts  | 16 ++---
 arch/arm64/boot/dts/arm/foundation-v8.dtsi | 16 -
 10 files changed, 129 insertions(+), 50 deletions(-)
 create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi
 create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts
 create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-gicv3.dtsi
 create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-psci.dts
 create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-psci.dtsi
 create mode 100644 arch/arm64/boot/dts/arm/foundation-v8-spin-table.dtsi

diff --git a/arch/arm64/boot/dts/arm/Makefile b/arch/arm64/boot/dts/arm/Makefile
index 75cc2aa10101..25f82c377f67 100644
--- a/arch/arm64/boot/dts/arm/Makefile
+++ b/arch/arm64/boot/dts/arm/Makefile
@@ -1,4 +1,6 @@
-dtb-$(CONFIG_ARCH_VEXPRESS) += foundation-v8.dtb foundation-v8-gicv3.dtb
+dtb-$(CONFIG_ARCH_VEXPRESS) += \
+   foundation-v8.dtb foundation-v8-psci.dtb \
+   foundation-v8-gicv3.dtb foundation-v8-gicv3-psci.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += juno.dtb juno-r1.dtb juno-r2.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += rtsm_ve-aemv8a.dtb
 dtb-$(CONFIG_ARCH_VEXPRESS) += vexpress-v2f-1xv7-ca53x2.dtb
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi 
b/arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi
new file mode 100644
index ..851abf34fc80
--- /dev/null
+++ b/arch/arm64/boot/dts/arm/foundation-v8-gicv2.dtsi
@@ -0,0 +1,19 @@
+/*
+ * ARM Ltd.
+ *
+ * ARMv8 Foundation model DTS (GICv2 configuration)
+ */
+
+/ {
+   gic: interrupt-controller@2c001000 {
+   compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
+   #interrupt-cells = <3>;
+   #address-cells = <2>;
+   interrupt-controller;
+   reg = <0x0 0x2c001000 0 0x1000>,
+ <0x0 0x2c002000 0 0x2000>,
+ <0x0 0x2c004000 0 0x2000>,
+ <0x0 0x2c006000 0 0x2000>;
+   interrupts = <1 9 0xf04>;
+   };
+};
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts 
b/arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts
new file mode 100644
index ..e096e670bec3
--- /dev/null
+++ b/arch/arm64/boot/dts/arm/foundation-v8-gicv3-psci.dts
@@ -0,0 +1,9 @@
+/*
+ * ARM Ltd.
+ *
+ * ARMv8 Foundation model DTS (GICv3+PSCI configuration)
+ */
+
+#include "foundation-v8.dtsi"
+#include "foundation-v8-gicv3.dtsi"
+#include "foundation-v8-psci.dtsi"
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts 
b/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts
index 35588dfa095c..c5d834d7d0ba 100644
--- a/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts
+++ b/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dts
@@ -5,26 +5,5 @@
  */

 #include "foundation-v8.dtsi"
-
-/ {
-   gic: interrupt-controller@2f00 {
-   compatible = "arm,gic-v3";
-   #interrupt-cells = <3>;
-   #address-cells = <2>;
-   #size-cells = <2>;
-   ranges;
-   interrupt-controller;
-   reg =   <0x0 0x2f00 0x0 0x1>,
-   <0x0 0x2f10 0x0 0x20>,
-   <0x0 0x2c00 0x0 0x2000>,
-   <0x0 0x2c01 0x0 0x2000>,
-   <0x0 0x2c02f000 0x0 0x2000>;
-   interrupts = <1 9 4>;
-
-   its: its@2f02 {
-   compatible = "arm,gic-v3-its";
-   msi-controller;
-   reg = <0x0 0x2f02 0x0 0x2>;
-   };
-   };
-};
+#include "foundation-v8-gicv3.dtsi"
+#include "foundation-v8-spin-table.dtsi"
diff --git a/arch/arm64/boot/dts/arm/foundation-v8-gicv3.dtsi