All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
@ 2016-01-20 16:04 Lorenzo Pieralisi
  2016-01-20 16:13 ` Sinan Kaya
  0 siblings, 1 reply; 19+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-20 16:04 UTC (permalink / raw)
  To: linux-pci; +Cc: Zhou Wang, Phil Edworthy, Bjorn Helgaas

Hi,

I noticed that:

79953dd22c1d ("PCI: rcar: Remove dependency on ARM-specific struct hw_pci")
cbce7900598c ("PCI: designware: Make driver arch-agnostic")

added code in the respective host controller drivers to size bridges
and assign resources only if the PCI_PROBE_ONLY flag is clear, which makes
me wonder if there exists PCI_PROBE_ONLY set-ups for the respective
host controllers:

- On ARM PCI_PROBE_ONLY can be set-up only via the command line or
  by DT (but the host controllers drivers have to check the DT and
  set it - see of_pci_check_probe_only()

  in drivers/pci/host/pci-host-generic.c

  neither designware nor rcar calls that function and I do not see
  any dts file with that property). Given that I can't see a way
  to set up PCI_PROBE_ONLY other than the command line on designware
  or rcar I assume the PCI_PROBE_ONLY set-ups have been tested by
  forcing it through command line parameter

- On ARM64 PCI_PROBE_ONLY can *only* be set via DT, so see above

We want to get rid of PCI_PROBE_ONLY on ARM/ARM64:

https://patchwork.ozlabs.org/patch/545671/

so unless you really have *existing* set-ups that require it, please
remove the respective checks from the host controller drivers, this is
becoming a serious issue, because either:

- we claim resources if and only if PCI_PROBE_ONLY is set

Either like this (to be done for every host controllers and ARM
bios32):

https://patchwork.ozlabs.org/patch/545670/

or in core ARM/ARM64 code - eg pcibios_fixup_bus() - (to avoid adding a
resource claiming call in ALL PCI host controllers)

- or we *always* carry out resource claiming regardless of PCI_PROBE_ONLY
  (but on ARM we can't really do that since PCI FW set-up on most of the
  platforms is not present)

On PCI_PROBE_ONLY systems resources claiming is mandatory if we want
to get rid of arches workarounds:

https://patchwork.ozlabs.org/patch/545671/

Comments very appreciated.

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-20 16:04 [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms Lorenzo Pieralisi
@ 2016-01-20 16:13 ` Sinan Kaya
  2016-01-20 18:10   ` Lorenzo Pieralisi
  2016-01-29 23:06   ` Bjorn Helgaas
  0 siblings, 2 replies; 19+ messages in thread
From: Sinan Kaya @ 2016-01-20 16:13 UTC (permalink / raw)
  To: Lorenzo Pieralisi, linux-pci; +Cc: Zhou Wang, Phil Edworthy, Bjorn Helgaas

On 1/20/2016 11:04 AM, Lorenzo Pieralisi wrote:
> Hi,
> 
> I noticed that:
> 
> 79953dd22c1d ("PCI: rcar: Remove dependency on ARM-specific struct hw_pci")
> cbce7900598c ("PCI: designware: Make driver arch-agnostic")
> 
> added code in the respective host controller drivers to size bridges
> and assign resources only if the PCI_PROBE_ONLY flag is clear, which makes
> me wonder if there exists PCI_PROBE_ONLY set-ups for the respective
> host controllers:
> 
> - On ARM PCI_PROBE_ONLY can be set-up only via the command line or
>   by DT (but the host controllers drivers have to check the DT and
>   set it - see of_pci_check_probe_only()
> 
>   in drivers/pci/host/pci-host-generic.c
> 
>   neither designware nor rcar calls that function and I do not see
>   any dts file with that property). Given that I can't see a way
>   to set up PCI_PROBE_ONLY other than the command line on designware
>   or rcar I assume the PCI_PROBE_ONLY set-ups have been tested by
>   forcing it through command line parameter
> 
> - On ARM64 PCI_PROBE_ONLY can *only* be set via DT, so see above
> 
> We want to get rid of PCI_PROBE_ONLY on ARM/ARM64:

For platforms that does not have UEFI BIOS, it makes sense to remove the probe only
option as the firmware is not doing anything.

For server like arm64 platforms, the behavior should be identical to x86 world. The UEFI
BIOS sets up the resources, kernel uses the resources.

Can we set this flag all the time for ACPI builds instead?

What's so special about arm64 that we can't do probe only builds and x86 can do?

> 
> https://patchwork.ozlabs.org/patch/545671/
> 
> so unless you really have *existing* set-ups that require it, please
> remove the respective checks from the host controller drivers, this is
> becoming a serious issue, because either:
> 
> - we claim resources if and only if PCI_PROBE_ONLY is set
> 
> Either like this (to be done for every host controllers and ARM
> bios32):
> 
> https://patchwork.ozlabs.org/patch/545670/
> 
> or in core ARM/ARM64 code - eg pcibios_fixup_bus() - (to avoid adding a
> resource claiming call in ALL PCI host controllers)
> 
> - or we *always* carry out resource claiming regardless of PCI_PROBE_ONLY
>   (but on ARM we can't really do that since PCI FW set-up on most of the
>   platforms is not present)
> 
> On PCI_PROBE_ONLY systems resources claiming is mandatory if we want
> to get rid of arches workarounds:

I'm hoping to see x86 like behavior on ARM64 without any gotchas as there is nothing
special about CPU type when it comes to PCI.

> 
> https://patchwork.ozlabs.org/patch/545671/
> 
> Comments very appreciated.
> 
> Thanks,
> Lorenzo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-20 16:13 ` Sinan Kaya
@ 2016-01-20 18:10   ` Lorenzo Pieralisi
  2016-01-20 18:15     ` Sinan Kaya
                       ` (2 more replies)
  2016-01-29 23:06   ` Bjorn Helgaas
  1 sibling, 3 replies; 19+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-20 18:10 UTC (permalink / raw)
  To: Sinan Kaya; +Cc: linux-pci, Zhou Wang, Phil Edworthy, Bjorn Helgaas

On Wed, Jan 20, 2016 at 11:13:04AM -0500, Sinan Kaya wrote:
> On 1/20/2016 11:04 AM, Lorenzo Pieralisi wrote:
> > Hi,
> > 
> > I noticed that:
> > 
> > 79953dd22c1d ("PCI: rcar: Remove dependency on ARM-specific struct hw_pci")
> > cbce7900598c ("PCI: designware: Make driver arch-agnostic")
> > 
> > added code in the respective host controller drivers to size bridges
> > and assign resources only if the PCI_PROBE_ONLY flag is clear, which makes
> > me wonder if there exists PCI_PROBE_ONLY set-ups for the respective
> > host controllers:
> > 
> > - On ARM PCI_PROBE_ONLY can be set-up only via the command line or
> >   by DT (but the host controllers drivers have to check the DT and
> >   set it - see of_pci_check_probe_only()
> > 
> >   in drivers/pci/host/pci-host-generic.c
> > 
> >   neither designware nor rcar calls that function and I do not see
> >   any dts file with that property). Given that I can't see a way
> >   to set up PCI_PROBE_ONLY other than the command line on designware
> >   or rcar I assume the PCI_PROBE_ONLY set-ups have been tested by
> >   forcing it through command line parameter
> > 
> > - On ARM64 PCI_PROBE_ONLY can *only* be set via DT, so see above
> > 
> > We want to get rid of PCI_PROBE_ONLY on ARM/ARM64:
> 
> For platforms that does not have UEFI BIOS, it makes sense to remove
> the probe only option as the firmware is not doing anything.
> 
> For server like arm64 platforms, the behavior should be identical to
> x86 world. The UEFI BIOS sets up the resources, kernel uses the
> resources.
> 
> Can we set this flag all the time for ACPI builds instead?
> 
> What's so special about arm64 that we can't do probe only builds and
> x86 can do?

Ok, let me give you a bit of background otherwise we talk at cross
purposes here.

ARM and ARM64 arch code is currently forced to prevent calling:

pci_enable_resources() (eg arch/arm/kernel/bios32.c)

because, on platforms that are "PCI_PROBE_ONLY" (whatever that means),
resources are neither assigned nor claimed. This works in practice
but it is basically a workaround (ie the resources hierarchy is not
validated at all in the kernel on PCI_PROBE_ONLY systems) and we want
to get rid of that, that's what I mean by "get rid of PCI_PROBE_ONLY".

Now, how we can get rid of that plaster in pcibios_enable_device().

To do that, we must claim resources on PCI_PROBE_ONLY systems, but
I know for certain Bjorn does not like the idea (I let you trawl
the archives - at least he does not accept the idea of claiming
resources ONLY on PCI_PROBE_ONLY systems, he thinks we should
always claim resources regardless of that flag and fall-back to
reassigning them in case claiming fails. That's perfectly reasonable,
at least on systems with FW initializing PCI). The problem is dealing
with legacy, so switching to resources claiming by default is a tad
complicated, at least for testing (code is easy to implement).

So yes, on arm64 ACPI systems, regardless of PCI_PROBE_ONLY, resources
must be claimed and reassigned if claiming fails (as x86), we start from
fresh and we *must* do it from the beginning.

We can do that on arm64 (for ACPI - where PCI FW set-up is _supposed_ to
be correct).

On other arm64 (and arm) platforms, the question is whether it is safe
to claim resources or not. Remember that there are already PCI host
controller drivers in the kernel (with equivalent FW set-up) so there
is legacy.

Resources claiming and assignment should be managed in arch code,
not in host controllers specific code, and that's the reason I
complained in this RFC about the scattering of PCI_PROBE_ONLY flag
checks in host drivers, it is becoming unmanageable (if useful
at all on designware and rcar, I would like to know if there are
PCI_PROBE_ONLY set-ups probing those host drivers).

Does it make the point clearer ?

> > https://patchwork.ozlabs.org/patch/545671/
> > 
> > so unless you really have *existing* set-ups that require it, please
> > remove the respective checks from the host controller drivers, this is
> > becoming a serious issue, because either:
> > 
> > - we claim resources if and only if PCI_PROBE_ONLY is set
> > 
> > Either like this (to be done for every host controllers and ARM
> > bios32):
> > 
> > https://patchwork.ozlabs.org/patch/545670/
> > 
> > or in core ARM/ARM64 code - eg pcibios_fixup_bus() - (to avoid adding a
> > resource claiming call in ALL PCI host controllers)
> > 
> > - or we *always* carry out resource claiming regardless of PCI_PROBE_ONLY
> >   (but on ARM we can't really do that since PCI FW set-up on most of the
> >   platforms is not present)
> > 
> > On PCI_PROBE_ONLY systems resources claiming is mandatory if we want
> > to get rid of arches workarounds:
> 
> I'm hoping to see x86 like behavior on ARM64 without any gotchas as
> there is nothing special about CPU type when it comes to PCI.

As I said, we must enforce it with ACPI, for DT platforms I am all
ears to decide how we can implement it sanely (I think the only way
to do it is by claiming the PCI resources on PCI_PROBE_ONLY systems,
that's not ideal but it simplifies things a lot).

Thanks,
Lorenzo

> 
> > 
> > https://patchwork.ozlabs.org/patch/545671/
> > 
> > Comments very appreciated.
> > 
> > Thanks,
> > Lorenzo
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> 
> -- 
> Sinan Kaya
> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-20 18:10   ` Lorenzo Pieralisi
@ 2016-01-20 18:15     ` Sinan Kaya
  2016-01-29 23:26       ` Bjorn Helgaas
  2016-01-22 16:28     ` Phil Edworthy
  2016-01-29 23:25     ` Bjorn Helgaas
  2 siblings, 1 reply; 19+ messages in thread
From: Sinan Kaya @ 2016-01-20 18:15 UTC (permalink / raw)
  To: Lorenzo Pieralisi; +Cc: linux-pci, Zhou Wang, Phil Edworthy, Bjorn Helgaas

On 1/20/2016 1:10 PM, Lorenzo Pieralisi wrote:
> On Wed, Jan 20, 2016 at 11:13:04AM -0500, Sinan Kaya wrote:
>> On 1/20/2016 11:04 AM, Lorenzo Pieralisi wrote:
>>> Hi,
>>>
>>> I noticed that:
>>>
>>> 79953dd22c1d ("PCI: rcar: Remove dependency on ARM-specific struct hw_pci")
>>> cbce7900598c ("PCI: designware: Make driver arch-agnostic")
>>>
>>> added code in the respective host controller drivers to size bridges
>>> and assign resources only if the PCI_PROBE_ONLY flag is clear, which makes
>>> me wonder if there exists PCI_PROBE_ONLY set-ups for the respective
>>> host controllers:
>>>
>>> - On ARM PCI_PROBE_ONLY can be set-up only via the command line or
>>>   by DT (but the host controllers drivers have to check the DT and
>>>   set it - see of_pci_check_probe_only()
>>>
>>>   in drivers/pci/host/pci-host-generic.c
>>>
>>>   neither designware nor rcar calls that function and I do not see
>>>   any dts file with that property). Given that I can't see a way
>>>   to set up PCI_PROBE_ONLY other than the command line on designware
>>>   or rcar I assume the PCI_PROBE_ONLY set-ups have been tested by
>>>   forcing it through command line parameter
>>>
>>> - On ARM64 PCI_PROBE_ONLY can *only* be set via DT, so see above
>>>
>>> We want to get rid of PCI_PROBE_ONLY on ARM/ARM64:
>>
>> For platforms that does not have UEFI BIOS, it makes sense to remove
>> the probe only option as the firmware is not doing anything.
>>
>> For server like arm64 platforms, the behavior should be identical to
>> x86 world. The UEFI BIOS sets up the resources, kernel uses the
>> resources.
>>
>> Can we set this flag all the time for ACPI builds instead?
>>
>> What's so special about arm64 that we can't do probe only builds and
>> x86 can do?
> 
> Ok, let me give you a bit of background otherwise we talk at cross
> purposes here.
> 
> ARM and ARM64 arch code is currently forced to prevent calling:
> 
> pci_enable_resources() (eg arch/arm/kernel/bios32.c)
> 
> because, on platforms that are "PCI_PROBE_ONLY" (whatever that means),
> resources are neither assigned nor claimed. This works in practice
> but it is basically a workaround (ie the resources hierarchy is not
> validated at all in the kernel on PCI_PROBE_ONLY systems) and we want
> to get rid of that, that's what I mean by "get rid of PCI_PROBE_ONLY".
> 
> Now, how we can get rid of that plaster in pcibios_enable_device().
> 
> To do that, we must claim resources on PCI_PROBE_ONLY systems, but
> I know for certain Bjorn does not like the idea (I let you trawl
> the archives - at least he does not accept the idea of claiming
> resources ONLY on PCI_PROBE_ONLY systems, he thinks we should
> always claim resources regardless of that flag and fall-back to
> reassigning them in case claiming fails. That's perfectly reasonable,
> at least on systems with FW initializing PCI). The problem is dealing
> with legacy, so switching to resources claiming by default is a tad
> complicated, at least for testing (code is easy to implement).
> 
> So yes, on arm64 ACPI systems, regardless of PCI_PROBE_ONLY, resources
> must be claimed and reassigned if claiming fails (as x86), we start from
> fresh and we *must* do it from the beginning.
> 
> We can do that on arm64 (for ACPI - where PCI FW set-up is _supposed_ to
> be correct).

Yes, please. ACPI is not CPU centric and the behavior should be identical across
all systems.

Footnote: I remember reading somewhere that some BIOS want to keep track
of where the endpoints are mapped. Reassigning the resources break such systems.

> 
> On other arm64 (and arm) platforms, the question is whether it is safe
> to claim resources or not. Remember that there are already PCI host
> controller drivers in the kernel (with equivalent FW set-up) so there
> is legacy.
> 
> Resources claiming and assignment should be managed in arch code,
> not in host controllers specific code, and that's the reason I
> complained in this RFC about the scattering of PCI_PROBE_ONLY flag
> checks in host drivers, it is becoming unmanageable (if useful
> at all on designware and rcar, I would like to know if there are
> PCI_PROBE_ONLY set-ups probing those host drivers).
> 
> Does it make the point clearer ?

Yes, I feel better now. I understand now that this is a plumbing work for 
non-ACPI ARM/ARM64 systems.

> 
>>> https://patchwork.ozlabs.org/patch/545671/
>>>
>>> so unless you really have *existing* set-ups that require it, please
>>> remove the respective checks from the host controller drivers, this is
>>> becoming a serious issue, because either:
>>>
>>> - we claim resources if and only if PCI_PROBE_ONLY is set
>>>
>>> Either like this (to be done for every host controllers and ARM
>>> bios32):
>>>
>>> https://patchwork.ozlabs.org/patch/545670/
>>>
>>> or in core ARM/ARM64 code - eg pcibios_fixup_bus() - (to avoid adding a
>>> resource claiming call in ALL PCI host controllers)
>>>
>>> - or we *always* carry out resource claiming regardless of PCI_PROBE_ONLY
>>>   (but on ARM we can't really do that since PCI FW set-up on most of the
>>>   platforms is not present)
>>>
>>> On PCI_PROBE_ONLY systems resources claiming is mandatory if we want
>>> to get rid of arches workarounds:
>>
>> I'm hoping to see x86 like behavior on ARM64 without any gotchas as
>> there is nothing special about CPU type when it comes to PCI.
> 
> As I said, we must enforce it with ACPI, for DT platforms I am all
> ears to decide how we can implement it sanely (I think the only way
> to do it is by claiming the PCI resources on PCI_PROBE_ONLY systems,
> that's not ideal but it simplifies things a lot).
> 

OK.

> Thanks,
> Lorenzo
> 
>>
>>>
>>> https://patchwork.ozlabs.org/patch/545671/
>>>
>>> Comments very appreciated.
>>>
>>> Thanks,
>>> Lorenzo
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>>
>> -- 
>> Sinan Kaya
>> Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
>>


-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-20 18:10   ` Lorenzo Pieralisi
  2016-01-20 18:15     ` Sinan Kaya
@ 2016-01-22 16:28     ` Phil Edworthy
  2016-01-25 17:51       ` Lorenzo Pieralisi
                         ` (2 more replies)
  2016-01-29 23:25     ` Bjorn Helgaas
  2 siblings, 3 replies; 19+ messages in thread
From: Phil Edworthy @ 2016-01-22 16:28 UTC (permalink / raw)
  To: Lorenzo Pieralisi; +Cc: linux-pci, Zhou Wang, Bjorn Helgaas, Sinan Kaya

Hi Lorenzo,

On 20 January 2016 18:10, Lorenzo Pieralisi wrote:
> On Wed, Jan 20, 2016 at 11:13:04AM -0500, Sinan Kaya wrote:
> > On 1/20/2016 11:04 AM, Lorenzo Pieralisi wrote:
> > > Hi,
> > >
> > > I noticed that:
> > >
> > > 79953dd22c1d ("PCI: rcar: Remove dependency on ARM-specific struct
> hw_pci")
> > > cbce7900598c ("PCI: designware: Make driver arch-agnostic")
> > >
> > > added code in the respective host controller drivers to size bridges
> > > and assign resources only if the PCI_PROBE_ONLY flag is clear, which makes
> > > me wonder if there exists PCI_PROBE_ONLY set-ups for the respective
> > > host controllers:
> > >
<snip>
 
> Resources claiming and assignment should be managed in arch code,
> not in host controllers specific code, and that's the reason I
> complained in this RFC about the scattering of PCI_PROBE_ONLY flag
> checks in host drivers, it is becoming unmanageable (if useful
> at all on designware and rcar, I would like to know if there are
> PCI_PROBE_ONLY set-ups probing those host drivers).

I am pretty sure  there are no rcar set-ups that use PCI_PROBE_ONLY,
so I am happy for that code to go.

Thanks
Phil

> Does it make the point clearer ?
> 
> > > https://patchwork.ozlabs.org/patch/545671/
> > >
> > > so unless you really have *existing* set-ups that require it, please
> > > remove the respective checks from the host controller drivers, this is
> > > becoming a serious issue, because either:
> > >
> > > - we claim resources if and only if PCI_PROBE_ONLY is set
> > >
> > > Either like this (to be done for every host controllers and ARM
> > > bios32):
> > >
> > > https://patchwork.ozlabs.org/patch/545670/
> > >
> > > or in core ARM/ARM64 code - eg pcibios_fixup_bus() - (to avoid adding a
> > > resource claiming call in ALL PCI host controllers)
> > >
> > > - or we *always* carry out resource claiming regardless of PCI_PROBE_ONLY
> > >   (but on ARM we can't really do that since PCI FW set-up on most of the
> > >   platforms is not present)
> > >
> > > On PCI_PROBE_ONLY systems resources claiming is mandatory if we want
> > > to get rid of arches workarounds:
> >
> > I'm hoping to see x86 like behavior on ARM64 without any gotchas as
> > there is nothing special about CPU type when it comes to PCI.
> 
> As I said, we must enforce it with ACPI, for DT platforms I am all
> ears to decide how we can implement it sanely (I think the only way
> to do it is by claiming the PCI resources on PCI_PROBE_ONLY systems,
> that's not ideal but it simplifies things a lot).
> 
> Thanks,
> Lorenzo
> 
> >
> > >
> > > https://patchwork.ozlabs.org/patch/545671/
> > >
> > > Comments very appreciated.
> > >
> > > Thanks,
> > > Lorenzo
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > >
> >
> >
> > --
> > Sinan Kaya
> > Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
> > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project
> >

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-22 16:28     ` Phil Edworthy
@ 2016-01-25 17:51       ` Lorenzo Pieralisi
  2016-01-28 17:27       ` Lorenzo Pieralisi
  2016-01-29  6:32       ` Pratyush Anand
  2 siblings, 0 replies; 19+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-25 17:51 UTC (permalink / raw)
  To: Phil Edworthy; +Cc: linux-pci, Zhou Wang, Bjorn Helgaas, Sinan Kaya

Hi Phil,

On Fri, Jan 22, 2016 at 04:28:39PM +0000, Phil Edworthy wrote:
> Hi Lorenzo,
> 
> On 20 January 2016 18:10, Lorenzo Pieralisi wrote:
> > On Wed, Jan 20, 2016 at 11:13:04AM -0500, Sinan Kaya wrote:
> > > On 1/20/2016 11:04 AM, Lorenzo Pieralisi wrote:
> > > > Hi,
> > > >
> > > > I noticed that:
> > > >
> > > > 79953dd22c1d ("PCI: rcar: Remove dependency on ARM-specific struct
> > hw_pci")
> > > > cbce7900598c ("PCI: designware: Make driver arch-agnostic")
> > > >
> > > > added code in the respective host controller drivers to size bridges
> > > > and assign resources only if the PCI_PROBE_ONLY flag is clear, which makes
> > > > me wonder if there exists PCI_PROBE_ONLY set-ups for the respective
> > > > host controllers:
> > > >
> <snip>
>  
> > Resources claiming and assignment should be managed in arch code,
> > not in host controllers specific code, and that's the reason I
> > complained in this RFC about the scattering of PCI_PROBE_ONLY flag
> > checks in host drivers, it is becoming unmanageable (if useful
> > at all on designware and rcar, I would like to know if there are
> > PCI_PROBE_ONLY set-ups probing those host drivers).
> 
> I am pretty sure  there are no rcar set-ups that use PCI_PROBE_ONLY,
> so I am happy for that code to go.

I will remove it then, thank you very much.

Lorenzo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-22 16:28     ` Phil Edworthy
  2016-01-25 17:51       ` Lorenzo Pieralisi
@ 2016-01-28 17:27       ` Lorenzo Pieralisi
  2016-01-29 12:02         ` Gabriele Paoloni
  2016-01-29  6:32       ` Pratyush Anand
  2 siblings, 1 reply; 19+ messages in thread
From: Lorenzo Pieralisi @ 2016-01-28 17:27 UTC (permalink / raw)
  To: Phil Edworthy, Pratyush Anand, Jingoo Han, Gabriele Paoloni
  Cc: linux-pci, Zhou Wang, Bjorn Helgaas, Sinan Kaya

[+ Pratyush, Jingoo, Gabriele]

any comment on this for the designware driver ?

Thanks,
Lorenzo

On Fri, Jan 22, 2016 at 04:28:39PM +0000, Phil Edworthy wrote:
> Hi Lorenzo,
> 
> On 20 January 2016 18:10, Lorenzo Pieralisi wrote:
> > On Wed, Jan 20, 2016 at 11:13:04AM -0500, Sinan Kaya wrote:
> > > On 1/20/2016 11:04 AM, Lorenzo Pieralisi wrote:
> > > > Hi,
> > > >
> > > > I noticed that:
> > > >
> > > > 79953dd22c1d ("PCI: rcar: Remove dependency on ARM-specific struct
> > hw_pci")
> > > > cbce7900598c ("PCI: designware: Make driver arch-agnostic")
> > > >
> > > > added code in the respective host controller drivers to size bridges
> > > > and assign resources only if the PCI_PROBE_ONLY flag is clear, which makes
> > > > me wonder if there exists PCI_PROBE_ONLY set-ups for the respective
> > > > host controllers:
> > > >
> <snip>
>  
> > Resources claiming and assignment should be managed in arch code,
> > not in host controllers specific code, and that's the reason I
> > complained in this RFC about the scattering of PCI_PROBE_ONLY flag
> > checks in host drivers, it is becoming unmanageable (if useful
> > at all on designware and rcar, I would like to know if there are
> > PCI_PROBE_ONLY set-ups probing those host drivers).
> 
> I am pretty sure  there are no rcar set-ups that use PCI_PROBE_ONLY,
> so I am happy for that code to go.
> 
> Thanks
> Phil
> 
> > Does it make the point clearer ?
> > 
> > > > https://patchwork.ozlabs.org/patch/545671/
> > > >
> > > > so unless you really have *existing* set-ups that require it, please
> > > > remove the respective checks from the host controller drivers, this is
> > > > becoming a serious issue, because either:
> > > >
> > > > - we claim resources if and only if PCI_PROBE_ONLY is set
> > > >
> > > > Either like this (to be done for every host controllers and ARM
> > > > bios32):
> > > >
> > > > https://patchwork.ozlabs.org/patch/545670/
> > > >
> > > > or in core ARM/ARM64 code - eg pcibios_fixup_bus() - (to avoid adding a
> > > > resource claiming call in ALL PCI host controllers)
> > > >
> > > > - or we *always* carry out resource claiming regardless of PCI_PROBE_ONLY
> > > >   (but on ARM we can't really do that since PCI FW set-up on most of the
> > > >   platforms is not present)
> > > >
> > > > On PCI_PROBE_ONLY systems resources claiming is mandatory if we want
> > > > to get rid of arches workarounds:
> > >
> > > I'm hoping to see x86 like behavior on ARM64 without any gotchas as
> > > there is nothing special about CPU type when it comes to PCI.
> > 
> > As I said, we must enforce it with ACPI, for DT platforms I am all
> > ears to decide how we can implement it sanely (I think the only way
> > to do it is by claiming the PCI resources on PCI_PROBE_ONLY systems,
> > that's not ideal but it simplifies things a lot).
> > 
> > Thanks,
> > Lorenzo
> > 
> > >
> > > >
> > > > https://patchwork.ozlabs.org/patch/545671/
> > > >
> > > > Comments very appreciated.
> > > >
> > > > Thanks,
> > > > Lorenzo
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > >
> > >
> > >
> > > --
> > > Sinan Kaya
> > > Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
> > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> > Foundation Collaborative Project
> > >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-22 16:28     ` Phil Edworthy
  2016-01-25 17:51       ` Lorenzo Pieralisi
  2016-01-28 17:27       ` Lorenzo Pieralisi
@ 2016-01-29  6:32       ` Pratyush Anand
  2 siblings, 0 replies; 19+ messages in thread
From: Pratyush Anand @ 2016-01-29  6:32 UTC (permalink / raw)
  To: Phil Edworthy
  Cc: Lorenzo Pieralisi, linux-pci, Zhou Wang, Bjorn Helgaas, Sinan Kaya

On Fri, Jan 22, 2016 at 9:58 PM, Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> > Resources claiming and assignment should be managed in arch code,
> > not in host controllers specific code, and that's the reason I
> > complained in this RFC about the scattering of PCI_PROBE_ONLY flag
> > checks in host drivers, it is becoming unmanageable (if useful
> > at all on designware and rcar, I would like to know if there are
> > PCI_PROBE_ONLY set-ups probing those host drivers).
>
> I am pretty sure  there are no rcar set-ups that use PCI_PROBE_ONLY,
> so I am happy for that code to go.
>

I do not remember either any designware platform which has required
and tested PCI_PROBE_ONLY.

~Pratyush

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-28 17:27       ` Lorenzo Pieralisi
@ 2016-01-29 12:02         ` Gabriele Paoloni
  0 siblings, 0 replies; 19+ messages in thread
From: Gabriele Paoloni @ 2016-01-29 12:02 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Phil Edworthy, Pratyush Anand, Jingoo Han
  Cc: linux-pci, Wangzhou (B), Bjorn Helgaas, Sinan Kaya

Hi Lorenzo

Sorry to be so late on this

> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-owner@vger.kernel.org]
> On Behalf Of Lorenzo Pieralisi
> Sent: 28 January 2016 17:27
> To: Phil Edworthy; Pratyush Anand; Jingoo Han; Gabriele Paoloni
> Cc: linux-pci@vger.kernel.org; Wangzhou (B); Bjorn Helgaas; Sinan Kaya
> Subject: Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
> 
> [+ Pratyush, Jingoo, Gabriele]
> 
> any comment on this for the designware driver ?

>From our side we (HiSilicon) do not use such flag.

We just modified the designware to stay in line with the
generic host bridge as we were not sure if any other
designware driver used it.

So I think it can be removed.

Thanks

Gab

> 
> Thanks,
> Lorenzo
> 
> On Fri, Jan 22, 2016 at 04:28:39PM +0000, Phil Edworthy wrote:
> > Hi Lorenzo,
> >
> > On 20 January 2016 18:10, Lorenzo Pieralisi wrote:
> > > On Wed, Jan 20, 2016 at 11:13:04AM -0500, Sinan Kaya wrote:
> > > > On 1/20/2016 11:04 AM, Lorenzo Pieralisi wrote:
> > > > > Hi,
> > > > >
> > > > > I noticed that:
> > > > >
> > > > > 79953dd22c1d ("PCI: rcar: Remove dependency on ARM-specific struct
> > > hw_pci")
> > > > > cbce7900598c ("PCI: designware: Make driver arch-agnostic")
> > > > >
> > > > > added code in the respective host controller drivers to size bridges
> > > > > and assign resources only if the PCI_PROBE_ONLY flag is clear, which
> makes
> > > > > me wonder if there exists PCI_PROBE_ONLY set-ups for the respective
> > > > > host controllers:
> > > > >
> > <snip>
> >
> > > Resources claiming and assignment should be managed in arch code,
> > > not in host controllers specific code, and that's the reason I
> > > complained in this RFC about the scattering of PCI_PROBE_ONLY flag
> > > checks in host drivers, it is becoming unmanageable (if useful
> > > at all on designware and rcar, I would like to know if there are
> > > PCI_PROBE_ONLY set-ups probing those host drivers).
> >
> > I am pretty sure  there are no rcar set-ups that use PCI_PROBE_ONLY,
> > so I am happy for that code to go.
> >
> > Thanks
> > Phil
> >
> > > Does it make the point clearer ?
> > >
> > > > > https://patchwork.ozlabs.org/patch/545671/
> > > > >
> > > > > so unless you really have *existing* set-ups that require it, please
> > > > > remove the respective checks from the host controller drivers, this is
> > > > > becoming a serious issue, because either:
> > > > >
> > > > > - we claim resources if and only if PCI_PROBE_ONLY is set
> > > > >
> > > > > Either like this (to be done for every host controllers and ARM
> > > > > bios32):
> > > > >
> > > > > https://patchwork.ozlabs.org/patch/545670/
> > > > >
> > > > > or in core ARM/ARM64 code - eg pcibios_fixup_bus() - (to avoid adding
> a
> > > > > resource claiming call in ALL PCI host controllers)
> > > > >
> > > > > - or we *always* carry out resource claiming regardless of
> PCI_PROBE_ONLY
> > > > >   (but on ARM we can't really do that since PCI FW set-up on most of
> the
> > > > >   platforms is not present)
> > > > >
> > > > > On PCI_PROBE_ONLY systems resources claiming is mandatory if we want
> > > > > to get rid of arches workarounds:
> > > >
> > > > I'm hoping to see x86 like behavior on ARM64 without any gotchas as
> > > > there is nothing special about CPU type when it comes to PCI.
> > >
> > > As I said, we must enforce it with ACPI, for DT platforms I am all
> > > ears to decide how we can implement it sanely (I think the only way
> > > to do it is by claiming the PCI resources on PCI_PROBE_ONLY systems,
> > > that's not ideal but it simplifies things a lot).
> > >
> > > Thanks,
> > > Lorenzo
> > >
> > > >
> > > > >
> > > > > https://patchwork.ozlabs.org/patch/545671/
> > > > >
> > > > > Comments very appreciated.
> > > > >
> > > > > Thanks,
> > > > > Lorenzo
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe linux-pci"
> in
> > > > > the body of a message to majordomo@vger.kernel.org
> > > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > > >
> > > >
> > > >
> > > > --
> > > > Sinan Kaya
> > > > Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
> > > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
> Linux
> > > Foundation Collaborative Project
> > > >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-20 16:13 ` Sinan Kaya
  2016-01-20 18:10   ` Lorenzo Pieralisi
@ 2016-01-29 23:06   ` Bjorn Helgaas
  2016-01-30  0:14     ` Sinan Kaya
  1 sibling, 1 reply; 19+ messages in thread
From: Bjorn Helgaas @ 2016-01-29 23:06 UTC (permalink / raw)
  To: Sinan Kaya
  Cc: Lorenzo Pieralisi, linux-pci, Zhou Wang, Phil Edworthy, Bjorn Helgaas

On Wed, Jan 20, 2016 at 11:13:04AM -0500, Sinan Kaya wrote:
> On 1/20/2016 11:04 AM, Lorenzo Pieralisi wrote:
> > 
> > We want to get rid of PCI_PROBE_ONLY on ARM/ARM64:
> 
> For platforms that does not have UEFI BIOS, it makes sense to remove the probe only
> option as the firmware is not doing anything.

I don't understand this statement.  It sounds like you mean "non-UEFI
BIOS firmware doesn't assign PCI BARs", but that's not true, so you
must mean something else.

> For server like arm64 platforms, the behavior should be identical to
> x86 world. 

> The UEFI BIOS sets up the resources, kernel uses the resources.

Is there actually a requirement that a UEFI BIOS assign resources to
PCI BARs?   I know that historically, x86 BIOS has set up most or all
PCI BARs.  But I thought there was something in UEFI that relaxed
that, so it would only have to program BARs for the boot and console
devices it actually needed.  But I can't find a spec reference either
way.

Bjorn

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-20 18:10   ` Lorenzo Pieralisi
  2016-01-20 18:15     ` Sinan Kaya
  2016-01-22 16:28     ` Phil Edworthy
@ 2016-01-29 23:25     ` Bjorn Helgaas
  2016-02-01 16:28       ` Lorenzo Pieralisi
  2 siblings, 1 reply; 19+ messages in thread
From: Bjorn Helgaas @ 2016-01-29 23:25 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Sinan Kaya, linux-pci, Zhou Wang, Phil Edworthy, Bjorn Helgaas

On Wed, Jan 20, 2016 at 06:10:03PM +0000, Lorenzo Pieralisi wrote:

> To do that, we must claim resources on PCI_PROBE_ONLY systems, but
> I know for certain Bjorn does not like the idea (I let you trawl
> the archives - at least he does not accept the idea of claiming
> resources ONLY on PCI_PROBE_ONLY systems, he thinks we should
> always claim resources regardless of that flag and fall-back to
> reassigning them in case claiming fails. That's perfectly reasonable,
> at least on systems with FW initializing PCI). The problem is dealing
> with legacy, so switching to resources claiming by default is a tad
> complicated, at least for testing (code is easy to implement).

I'd like to think of PCI_PROBE_ONLY basically as "setting
IORESOURCE_PCI_FIXED for every BAR", i.e., we can do everything we
normally do *except* write to the BAR (of course we have to write to
it to size the BAR during enumeration, but we have to restore the
original value).

I don't think PCI_PROBE_ONLY should have anything to do with the
iomem_resource and ioport_resource trees.  /proc/iomem and
/proc/ioports should still contain every resource we know about,
regardless of whether PCI_PROBE_ONLY is set or not.

Bjorn

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-20 18:15     ` Sinan Kaya
@ 2016-01-29 23:26       ` Bjorn Helgaas
  0 siblings, 0 replies; 19+ messages in thread
From: Bjorn Helgaas @ 2016-01-29 23:26 UTC (permalink / raw)
  To: Sinan Kaya
  Cc: Lorenzo Pieralisi, linux-pci, Zhou Wang, Phil Edworthy, Bjorn Helgaas

On Wed, Jan 20, 2016 at 01:15:04PM -0500, Sinan Kaya wrote:

> Footnote: I remember reading somewhere that some BIOS want to keep track
> of where the endpoints are mapped. Reassigning the resources break such systems.

I agree, I think there are systems where the BIOS sets a PCI BAR value
and depends on that remaining untouched at run-time.  For example, I'm
pretty sure there's x86 SMM code that logs errors to PCI management
devices.  I think these systems are broken.

I don't think there's a way for firmware to tell the kernel "please
don't reassign these PCI resources."  After handoff to the OS, I think
the OS owns all PCI resources and can reassign things as necessary,
subject only to a few constraints like _OSC.  Certainly the OS has to
be able to write to BARs at least temporarily to learn their size.  I
have no spec reference for this belief.

I don't have a good strategy for dealing with such systems, other
than:

  - Keep firmware BAR assignments unless we have a reason to change
    them

  - Add quirks to set IORESOURCE_PCI_FIXED for specific BARs when we
    know about issues

The new PCI "Enhanced Allocation" stuff might be a way for platforms
to do this more safely.

Bjorn

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-29 23:06   ` Bjorn Helgaas
@ 2016-01-30  0:14     ` Sinan Kaya
  2016-01-30 13:30       ` Bjorn Helgaas
  0 siblings, 1 reply; 19+ messages in thread
From: Sinan Kaya @ 2016-01-30  0:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Lorenzo Pieralisi, linux-pci, Zhou Wang, Phil Edworthy, Bjorn Helgaas

On 1/29/2016 6:06 PM, Bjorn Helgaas wrote:
> On Wed, Jan 20, 2016 at 11:13:04AM -0500, Sinan Kaya wrote:
>> On 1/20/2016 11:04 AM, Lorenzo Pieralisi wrote:
>>>
>>> We want to get rid of PCI_PROBE_ONLY on ARM/ARM64:
>>
>> For platforms that does not have UEFI BIOS, it makes sense to remove the probe only
>> option as the firmware is not doing anything.
> 
> I don't understand this statement.  It sounds like you mean "non-UEFI
> BIOS firmware doesn't assign PCI BARs", but that's not true, so you
> must mean something else.

It depends on the firmware flavor. I know u-boot does some PCI assignment but it does minimum
to use PCI itself not for OS consumption. It may not deal with with switches/bridges etc. or 
will only assign mem32 resources and not touch prefetchable. 

Most non-UEFI firmwares I have seen on ARM rely on device specific driver
like synopsys etc. to do the device initialization and ask kernel to do the enumeration.

ACPI systems on the other hand handle the resource assignment before the OS starts.

> 
>> For server like arm64 platforms, the behavior should be identical to
>> x86 world. 
> 
>> The UEFI BIOS sets up the resources, kernel uses the resources.
> 
> Is there actually a requirement that a UEFI BIOS assign resources to
> PCI BARs?   I know that historically, x86 BIOS has set up most or all
> PCI BARs.  But I thought there was something in UEFI that relaxed
> that, so it would only have to program BARs for the boot and console
> devices it actually needed.  But I can't find a spec reference either
> way.

I don't have a spec reference. This statement was purely out of x86 experience not based on some
spec. We are trying to follow the PC model.

> 
> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-30  0:14     ` Sinan Kaya
@ 2016-01-30 13:30       ` Bjorn Helgaas
  2016-01-30 17:51         ` Okaya
  2016-02-01 15:25         ` Lorenzo Pieralisi
  0 siblings, 2 replies; 19+ messages in thread
From: Bjorn Helgaas @ 2016-01-30 13:30 UTC (permalink / raw)
  To: Sinan Kaya
  Cc: Lorenzo Pieralisi, linux-pci, Zhou Wang, Phil Edworthy, Bjorn Helgaas

On Fri, Jan 29, 2016 at 07:14:29PM -0500, Sinan Kaya wrote:
> On 1/29/2016 6:06 PM, Bjorn Helgaas wrote:
> > On Wed, Jan 20, 2016 at 11:13:04AM -0500, Sinan Kaya wrote:
> >> On 1/20/2016 11:04 AM, Lorenzo Pieralisi wrote:
> >>>
> >>> We want to get rid of PCI_PROBE_ONLY on ARM/ARM64:
> >>
> >> For platforms that does not have UEFI BIOS, it makes sense to remove the probe only
> >> option as the firmware is not doing anything.
> > 
> > I don't understand this statement.  It sounds like you mean "non-UEFI
> > BIOS firmware doesn't assign PCI BARs", but that's not true, so you
> > must mean something else.
> 
> It depends on the firmware flavor. I know u-boot does some PCI
> assignment but it does minimum to use PCI itself not for OS
> consumption. It may not deal with with switches/bridges etc. or will
> only assign mem32 resources and not touch prefetchable. 

Ah, I see the problem.  When you wrote "non-UEFI BIOS," I thought
"old-fashioned x86 BIOS," which generally do assign resources.  But I
don't have much experience with other firmware like U-boot.  There are
good reasons, e.g., portability and boot-time optimization, why
firmware might not touch things it doesn't need.  That's especially
true for PCI resource assignment, where we know the OS must do it
itself anyway, and there's little point in doing it twice.

> Most non-UEFI firmwares I have seen on ARM rely on device specific
> driver like synopsys etc. to do the device initialization and ask
> kernel to do the enumeration.
> 
> ACPI systems on the other hand handle the resource assignment before
> the OS starts.

My guess is that this is more of a tradition than anything actually
required by the spec.

The bottom line is that Linux can't rely on much consistency across
the universe of architectures and firmwares.  I think the only thing
that really makes sense for us to do is:

  - Read whatever assignments the firmware may have made
  - Keep them unchanged if they seem sensible
  - Reassign them if they aren't sensible
  - Use quirks to handle exceptions

Bjorn

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-30 13:30       ` Bjorn Helgaas
@ 2016-01-30 17:51         ` Okaya
  2016-02-01 15:25         ` Lorenzo Pieralisi
  1 sibling, 0 replies; 19+ messages in thread
From: Okaya @ 2016-01-30 17:51 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Sinan Kaya, Lorenzo Pieralisi, linux-pci, Zhou Wang,
	Phil Edworthy, Bjorn Helgaas

> On Fri, Jan 29, 2016 at 07:14:29PM -0500, Sinan Kaya wrote:
>> On 1/29/2016 6:06 PM, Bjorn Helgaas wrote:
>> > On Wed, Jan 20, 2016 at 11:13:04AM -0500, Sinan Kaya wrote:
>> >> On 1/20/2016 11:04 AM, Lorenzo Pieralisi wrote:
>> >>>
>> >>> We want to get rid of PCI_PROBE_ONLY on ARM/ARM64:
>> >>
>> >> For platforms that does not have UEFI BIOS, it makes sense to remove
>> the probe only
>> >> option as the firmware is not doing anything.
>> >
>> > I don't understand this statement.  It sounds like you mean "non-UEFI
>> > BIOS firmware doesn't assign PCI BARs", but that's not true, so you
>> > must mean something else.
>>
>> It depends on the firmware flavor. I know u-boot does some PCI
>> assignment but it does minimum to use PCI itself not for OS
>> consumption. It may not deal with with switches/bridges etc. or will
>> only assign mem32 resources and not touch prefetchable.
>
> Ah, I see the problem.  When you wrote "non-UEFI BIOS," I thought
> "old-fashioned x86 BIOS," which generally do assign resources.  But I
> don't have much experience with other firmware like U-boot.  There are
> good reasons, e.g., portability and boot-time optimization, why
> firmware might not touch things it doesn't need.  That's especially
> true for PCI resource assignment, where we know the OS must do it
> itself anyway, and there's little point in doing it twice.
>
>> Most non-UEFI firmwares I have seen on ARM rely on device specific
>> driver like synopsys etc. to do the device initialization and ask
>> kernel to do the enumeration.
>>
>> ACPI systems on the other hand handle the resource assignment before
>> the OS starts.
>
> My guess is that this is more of a tradition than anything actually
> required by the spec.
>
> The bottom line is that Linux can't rely on much consistency across
> the universe of architectures and firmwares.  I think the only thing
> that really makes sense for us to do is:
>
>   - Read whatever assignments the firmware may have made
>   - Keep them unchanged if they seem sensible
>   - Reassign them if they aren't sensible
>   - Use quirks to handle exceptions
>
> Bjorn
>


+1 to this list.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-30 13:30       ` Bjorn Helgaas
  2016-01-30 17:51         ` Okaya
@ 2016-02-01 15:25         ` Lorenzo Pieralisi
  2016-02-01 21:12           ` Bjorn Helgaas
  1 sibling, 1 reply; 19+ messages in thread
From: Lorenzo Pieralisi @ 2016-02-01 15:25 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Sinan Kaya, linux-pci, Zhou Wang, Phil Edworthy, Bjorn Helgaas

On Sat, Jan 30, 2016 at 07:30:27AM -0600, Bjorn Helgaas wrote:

[...]

> > Most non-UEFI firmwares I have seen on ARM rely on device specific
> > driver like synopsys etc. to do the device initialization and ask
> > kernel to do the enumeration.
> > 
> > ACPI systems on the other hand handle the resource assignment before
> > the OS starts.
> 
> My guess is that this is more of a tradition than anything actually
> required by the spec.

I share your opinion, and that tradition on ARM64 should be built
on top of existing DT based systems where the bootloader assigns
*NOTHING* in 90% of designs.

That's why I want to see resource claiming carried out by default
on ACPI on ARM64, this would foster the tradition :), hopefully.

> The bottom line is that Linux can't rely on much consistency across
> the universe of architectures and firmwares.  I think the only thing
> that really makes sense for us to do is:
> 
>   - Read whatever assignments the firmware may have made
>   - Keep them unchanged if they seem sensible

Here I take "sensible" as "it can be successfully claimed" - ie the resource
is allocated in a valid way, though it may not be optimal (eg bridge window
apertures).

>   - Reassign them if they aren't sensible

And we reassign whatever can't be successfully claimed.

Yes, it seems like the best approach and likely the only viable one.

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-01-29 23:25     ` Bjorn Helgaas
@ 2016-02-01 16:28       ` Lorenzo Pieralisi
  2016-02-01 21:19         ` Bjorn Helgaas
  0 siblings, 1 reply; 19+ messages in thread
From: Lorenzo Pieralisi @ 2016-02-01 16:28 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Sinan Kaya, linux-pci, Zhou Wang, Phil Edworthy, Bjorn Helgaas

On Fri, Jan 29, 2016 at 05:25:38PM -0600, Bjorn Helgaas wrote:
> On Wed, Jan 20, 2016 at 06:10:03PM +0000, Lorenzo Pieralisi wrote:
> 
> > To do that, we must claim resources on PCI_PROBE_ONLY systems, but
> > I know for certain Bjorn does not like the idea (I let you trawl
> > the archives - at least he does not accept the idea of claiming
> > resources ONLY on PCI_PROBE_ONLY systems, he thinks we should
> > always claim resources regardless of that flag and fall-back to
> > reassigning them in case claiming fails. That's perfectly reasonable,
> > at least on systems with FW initializing PCI). The problem is dealing
> > with legacy, so switching to resources claiming by default is a tad
> > complicated, at least for testing (code is easy to implement).
> 
> I'd like to think of PCI_PROBE_ONLY basically as "setting
> IORESOURCE_PCI_FIXED for every BAR", i.e., we can do everything we
> normally do *except* write to the BAR (of course we have to write to
> it to size the BAR during enumeration, but we have to restore the
> original value).

I am honestly a bit confused about the distinction between
PCI_PROBE_ONLY and "IORESOURCE_PCI_FIXED for every BAR", the more
so if I read the resources assignment code, in particular:

pci_assign_resource()

in drivers/pci/setup-res.c

That function assigns the resource but does not update the BAR
if IORESOURCE_PCI_FIXED is set. I wonder why the resource is
assigned at all if IORESOURCE_PCI_FIXED is set, probably to
initialize its parent pointer and set it, still, the value in
the struct resource may differ from what's in the BAR, I am not sure
I understand why we are happy with that or I am reading code the
wrong way (does not this give a view of resource tree disconnected
from how HW is actually programmed ?)

> I don't think PCI_PROBE_ONLY should have anything to do with the
> iomem_resource and ioport_resource trees.  /proc/iomem and
> /proc/ioports should still contain every resource we know about,
> regardless of whether PCI_PROBE_ONLY is set or not.

I do not think PCI_PROBE_ONLY should have anything to do with the
resource tree either, still, if resources are not assigned, they must be
inserted in the resource tree anyway so that we are capable of enabling
the PCI devices (ie pci_enable_resources()).

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-02-01 15:25         ` Lorenzo Pieralisi
@ 2016-02-01 21:12           ` Bjorn Helgaas
  0 siblings, 0 replies; 19+ messages in thread
From: Bjorn Helgaas @ 2016-02-01 21:12 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Sinan Kaya, linux-pci, Zhou Wang, Phil Edworthy, Bjorn Helgaas

On Mon, Feb 01, 2016 at 03:25:07PM +0000, Lorenzo Pieralisi wrote:
> On Sat, Jan 30, 2016 at 07:30:27AM -0600, Bjorn Helgaas wrote:
> 
> [...]
> 
> > > Most non-UEFI firmwares I have seen on ARM rely on device specific
> > > driver like synopsys etc. to do the device initialization and ask
> > > kernel to do the enumeration.
> > > 
> > > ACPI systems on the other hand handle the resource assignment before
> > > the OS starts.
> > 
> > My guess is that this is more of a tradition than anything actually
> > required by the spec.
> 
> I share your opinion, and that tradition on ARM64 should be built
> on top of existing DT based systems where the bootloader assigns
> *NOTHING* in 90% of designs.
> 
> That's why I want to see resource claiming carried out by default
> on ACPI on ARM64, this would foster the tradition :), hopefully.
> 
> > The bottom line is that Linux can't rely on much consistency across
> > the universe of architectures and firmwares.  I think the only thing
> > that really makes sense for us to do is:
> > 
> >   - Read whatever assignments the firmware may have made
> >   - Keep them unchanged if they seem sensible
> 
> Here I take "sensible" as "it can be successfully claimed" - ie the resource
> is allocated in a valid way, though it may not be optimal (eg bridge window
> apertures).

Yes.  But even if platforms assign resources in a way they can be
successfully claimed, I don't think platforms should rely on that
assignment being unchanged.  For example, even if the boot-time
configuration is valid, the kernel may need to move things around to
deal with hotplug.  I know we don't really do that today, but I think
we should be able to do it in principle.

Bjorn

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms
  2016-02-01 16:28       ` Lorenzo Pieralisi
@ 2016-02-01 21:19         ` Bjorn Helgaas
  0 siblings, 0 replies; 19+ messages in thread
From: Bjorn Helgaas @ 2016-02-01 21:19 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: Sinan Kaya, linux-pci, Zhou Wang, Phil Edworthy, Bjorn Helgaas

On Mon, Feb 01, 2016 at 04:28:38PM +0000, Lorenzo Pieralisi wrote:
> On Fri, Jan 29, 2016 at 05:25:38PM -0600, Bjorn Helgaas wrote:
> > On Wed, Jan 20, 2016 at 06:10:03PM +0000, Lorenzo Pieralisi wrote:
> > 
> > > To do that, we must claim resources on PCI_PROBE_ONLY systems, but
> > > I know for certain Bjorn does not like the idea (I let you trawl
> > > the archives - at least he does not accept the idea of claiming
> > > resources ONLY on PCI_PROBE_ONLY systems, he thinks we should
> > > always claim resources regardless of that flag and fall-back to
> > > reassigning them in case claiming fails. That's perfectly reasonable,
> > > at least on systems with FW initializing PCI). The problem is dealing
> > > with legacy, so switching to resources claiming by default is a tad
> > > complicated, at least for testing (code is easy to implement).
> > 
> > I'd like to think of PCI_PROBE_ONLY basically as "setting
> > IORESOURCE_PCI_FIXED for every BAR", i.e., we can do everything we
> > normally do *except* write to the BAR (of course we have to write to
> > it to size the BAR during enumeration, but we have to restore the
> > original value).
> 
> I am honestly a bit confused about the distinction between
> PCI_PROBE_ONLY and "IORESOURCE_PCI_FIXED for every BAR", the more
> so if I read the resources assignment code, in particular:
> 
> pci_assign_resource()
> 
> in drivers/pci/setup-res.c
> 
> That function assigns the resource but does not update the BAR
> if IORESOURCE_PCI_FIXED is set. I wonder why the resource is
> assigned at all if IORESOURCE_PCI_FIXED is set, probably to
> initialize its parent pointer and set it, still, the value in
> the struct resource may differ from what's in the BAR, I am not sure
> I understand why we are happy with that or I am reading code the
> wrong way (does not this give a view of resource tree disconnected
> from how HW is actually programmed ?)

I'm not happy with that -- it's just one of our many problems.

The assignment code doesn't doesn't really pay attention to
IORESOURCE_PCI_FIXED.  I don't think it pays attention to
PCI_PROBE_ONLY either, except that some arches don't even call the
assignment code if it is set.

Bjorn

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-02-01 21:19 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-20 16:04 [RFC] ARM/ARM64 PCI_PROBE_ONLY platforms Lorenzo Pieralisi
2016-01-20 16:13 ` Sinan Kaya
2016-01-20 18:10   ` Lorenzo Pieralisi
2016-01-20 18:15     ` Sinan Kaya
2016-01-29 23:26       ` Bjorn Helgaas
2016-01-22 16:28     ` Phil Edworthy
2016-01-25 17:51       ` Lorenzo Pieralisi
2016-01-28 17:27       ` Lorenzo Pieralisi
2016-01-29 12:02         ` Gabriele Paoloni
2016-01-29  6:32       ` Pratyush Anand
2016-01-29 23:25     ` Bjorn Helgaas
2016-02-01 16:28       ` Lorenzo Pieralisi
2016-02-01 21:19         ` Bjorn Helgaas
2016-01-29 23:06   ` Bjorn Helgaas
2016-01-30  0:14     ` Sinan Kaya
2016-01-30 13:30       ` Bjorn Helgaas
2016-01-30 17:51         ` Okaya
2016-02-01 15:25         ` Lorenzo Pieralisi
2016-02-01 21:12           ` Bjorn Helgaas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.