Re: [Xen-devel] [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

2015-03-05 Thread Julien Grall

Hi Ian,

On 05/03/2015 14:43, Ian Campbell wrote:

On Wed, 2015-02-18 at 15:16 +, Julien Grall wrote:

ranges; means there is not translation necessary. But nothing prevent
to have a the property ranges set.


I don't suppose you know of a system where this translation is needed do
you? So I've got something to test with...


AFAIR, no. I guess, it should not be to difficult to tweak a device tree 
for testing.


Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

2015-02-18 Thread Julien Grall

Hi Ian,

On 18/02/2015 13:50, Ian Campbell wrote:

On Tue, 2015-02-17 at 17:33 +, Julien Grall wrote:

Hi Ian,

On 24/10/14 10:58, Ian Campbell wrote:

These properties are defined in ePAPR and the OpenFirmware PCI Bus Binding
Specification (IEEE Std 1275-1994).

This replaces the xgene specific mapping. Tested on Mustang and on a model with
a PCI virtio controller.


I'm wondering why you choose to map everything at Xen boot time rather
than implementing PHYSDEVOP_pci_device_add to do the job?


Does pci_device_add contain sufficient information to do so?


Hmmm... for the interrupt the SBDF is enough. Although for the MMIO it 
looks like there is no difference between PCI bars.



The regions which are being mapped are essentially the PCI host
controllers MMIO, IO and CFG windows which are then consumed by the
various bars of the devices on the bus.

So mapping based on pci_device_add would require us to go from the SBDF
to a set of BARS which need mapping, which is a whole lot more complex
than just mapping all of the resources owned by the root complex through
to the h/w domain.


I gave a look to the code which parse the host bridge resource (see 
of_pci_get_host_bridge_resources). They seem to re-use to the 
of_translate_* function. Would not it be possible to do the same?



Or perhaps I've misunderstood what you were suggesting?


I was suggesting to do it via pci_add_device but it looks like it's only 
possible for IRQ not MMIO.



This would allow us to re-use most of the interrupts/mmio decoding
provided in the device tree library. It would also avoid missing support
of cascade ranges/interrupt-map.


I *think* (if I'm remembering right) I decided we don't need to worry
about cascades of these things because the second level resources are
all fully contained within the first (top level) one and so with the
approach I've taken here are all fully mapped already. That's why I made
this patch stop descending into children when such a bus node is
found.


I don't understand this paragraph, sorry.

The address range you decoded via the PCI bus may be an intermediate 
address which needs to be translated in the physical hardware address.


Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

2015-02-18 Thread Ian Campbell
On Wed, 2015-02-18 at 14:19 +, Julien Grall wrote:
 Hi Ian,
 
 On 18/02/2015 13:50, Ian Campbell wrote:
  On Tue, 2015-02-17 at 17:33 +, Julien Grall wrote:
  Hi Ian,
 
  On 24/10/14 10:58, Ian Campbell wrote:
  These properties are defined in ePAPR and the OpenFirmware PCI Bus Binding
  Specification (IEEE Std 1275-1994).
 
  This replaces the xgene specific mapping. Tested on Mustang and on a 
  model with
  a PCI virtio controller.
 
  I'm wondering why you choose to map everything at Xen boot time rather
  than implementing PHYSDEVOP_pci_device_add to do the job?
 
  Does pci_device_add contain sufficient information to do so?
 
 Hmmm... for the interrupt the SBDF is enough. Although for the MMIO it 
 looks like there is no difference between PCI bars.
 
  The regions which are being mapped are essentially the PCI host
  controllers MMIO, IO and CFG windows which are then consumed by the
  various bars of the devices on the bus.
 
  So mapping based on pci_device_add would require us to go from the SBDF
  to a set of BARS which need mapping, which is a whole lot more complex
  than just mapping all of the resources owned by the root complex through
  to the h/w domain.
 
 I gave a look to the code which parse the host bridge resource (see 
 of_pci_get_host_bridge_resources). They seem to re-use to the 
 of_translate_* function. Would not it be possible to do the same?
 
  Or perhaps I've misunderstood what you were suggesting?
 
 I was suggesting to do it via pci_add_device but it looks like it's only 
 possible for IRQ not MMIO.

I think so, and we probably should consider the two cases separately
since the right answer could reasonably differ for different resource
types.

I am reasonably convinced that for MMIO (+IO+CFG space) we should map
everything as described by the ranges property of the top most node, it
can be considered an analogue to / extension of the reg property of that
node.

For IRQ I'm not so sure, it's possible that routing the IRQ at
pci_add_device time might be better, or fit in better with e.g. the ACPI
architecture, but mapping everything described in interrupt-map at start
of day is also an option and a reasonably simple one, probably.

(My memory is fuzzy, but I think the concerns I had with this patch were
precisely to do with IRQs and how to parse the interrupt-map without a
specific SBDF in hand -- but only because the existing helper functions
assume an SBDF is present)

  This would allow us to re-use most of the interrupts/mmio decoding
  provided in the device tree library. It would also avoid missing support
  of cascade ranges/interrupt-map.
 
  I *think* (if I'm remembering right) I decided we don't need to worry
  about cascades of these things because the second level resources are
  all fully contained within the first (top level) one and so with the
  approach I've taken here are all fully mapped already. That's why I made
  this patch stop descending into children when such a bus node is
  found.
 
 I don't understand this paragraph, sorry.
 
 The address range you decoded via the PCI bus may be an intermediate 
 address which needs to be translated in the physical hardware address.

This isn't to do with IPA-PA translations but to do with translations
between different PA addressing regimes. i.e. the different addressing
schemes of difference busses.

Lets say we have a system with a PCI-ROOT device exposing a PCI bus,
which in turn contains a PCI-BRIDGE which for the sake of argument lets
say is a PCI-FOOBUS bridge.

Lets just consider the MMIO hole for now, but IRQ is basically the same.

The ranges property on a node describes a mapping from a parent
address space into a child address space.

For PCI-ROOT parent is the host physical address space and child is
the PCI MMIO/IO/CFG address spaces.

For PCI-BRIDGE parent is the PCI-ROOT's child address space (i.e. PCI
MMIO/IO/CFG) and child is the FOOBUS address space.

The inputs (parents) of the PCI-BRIDGE ranges property must therefore
by definition be valid outputs of the PCI-ROOT ranges property (i.e. be
child addresses).

Therefore if we map all of the input/parent ranges described by
PCI-ROOT's ranges property we do not need to recurse further and
consider PCI-BRIDGE's ranges property -- we've effectively already dealt
with it.

Does that make more sense?

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

2015-02-18 Thread Julien Grall



On 18/02/2015 15:18, Ian Campbell wrote:

On Wed, 2015-02-18 at 15:05 +, Julien Grall wrote:


On 18/02/2015 14:37, Ian Campbell wrote:

On Wed, 2015-02-18 at 14:19 +, Julien Grall wrote:
I think so, and we probably should consider the two cases separately
since the right answer could reasonably differ for different resource
types.

I am reasonably convinced that for MMIO (+IO+CFG space) we should map
everything as described by the ranges property of the top most node, it
can be considered an analogue to / extension of the reg property of that
node.


Agreed.


For IRQ I'm not so sure, it's possible that routing the IRQ at
pci_add_device time might be better, or fit in better with e.g. the ACPI
architecture, but mapping everything described in interrupt-map at start
of day is also an option and a reasonably simple one, probably.


I agree that it's simple. Are we sure that we would be able to get a
better solution later without modifying the kernel?

If not, we may need to keep this solution forever.


True. I suppose feature flags would be one way out, but not a very
convenient one..


This isn't to do with IPA-PA translations but to do with translations
between different PA addressing regimes. i.e. the different addressing
schemes of difference busses.


I meant bus address. The name intermediate address was misused, sorry.


Lets say we have a system with a PCI-ROOT device exposing a PCI bus,
which in turn contains a PCI-BRIDGE which for the sake of argument lets
say is a PCI-FOOBUS bridge.



Lets just consider the MMIO hole for now, but IRQ is basically the same.

The ranges property on a node describes a mapping from a parent
address space into a child address space.

For PCI-ROOT parent is the host physical address space and child is
the PCI MMIO/IO/CFG address spaces.

For PCI-BRIDGE parent is the PCI-ROOT's child address space (i.e. PCI
MMIO/IO/CFG) and child is the FOOBUS address space.

The inputs (parents) of the PCI-BRIDGE ranges property must therefore
by definition be valid outputs of the PCI-ROOT ranges property (i.e. be
child addresses).

Therefore if we map all of the input/parent ranges described by
PCI-ROOT's ranges property we do not need to recurse further and
consider PCI-BRIDGE's ranges property -- we've effectively already dealt
with it.

Does that make more sense?


I'm still confused, what prevents the PCI-ROOT device to not be
connected to another bus?

In device tree format, that would give something like:

/ {

soc {
   ranges = ...;

   pcie {
 ranges = ...;
   }
}
}

The address retrieved from the PCI-ROOT would be a bus address and not a
physical address.


Hrm, nothing, I see what you are getting at now.

Either soc has a device_type property which we understand, in which case
we would handle it and stop recursing or (more likely for an soc) it
does not, in which case we would handle the pcie ranges property, but it
needs to be translated through the ranges property of soc, which the
patch doesn't do and probably it should.


The code to do it is quite complicate and hard to maintain (actually 
it's a copy of the Linux one). It would be good if you can re-use the 
functions to translate in common/device_tree.c.


I think we may have the same problem for interrupts too.

Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

2015-02-18 Thread Ian Campbell
On Wed, 2015-02-18 at 15:31 +, Julien Grall wrote:

  Either soc has a device_type property which we understand, in which case
  we would handle it and stop recursing or (more likely for an soc) it
  does not, in which case we would handle the pcie ranges property, but it
  needs to be translated through the ranges property of soc, which the
  patch doesn't do and probably it should.
 
 The code to do it is quite complicate and hard to maintain (actually 
 it's a copy of the Linux one). It would be good if you can re-use the 
 functions to translate in common/device_tree.c.

Of course.

 I think we may have the same problem for interrupts too.

Yes. In that case ISTR the existing functions needed an SBDF, so weren't
quite right for the up front mapping approach -- but that can surely
be fixed via some refactoring.

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

2015-02-18 Thread Ian Campbell
On Tue, 2015-02-17 at 17:33 +, Julien Grall wrote:
 Hi Ian,
 
 On 24/10/14 10:58, Ian Campbell wrote:
  These properties are defined in ePAPR and the OpenFirmware PCI Bus Binding
  Specification (IEEE Std 1275-1994).
  
  This replaces the xgene specific mapping. Tested on Mustang and on a model 
  with
  a PCI virtio controller.
 
 I'm wondering why you choose to map everything at Xen boot time rather
 than implementing PHYSDEVOP_pci_device_add to do the job?

Does pci_device_add contain sufficient information to do so?

The regions which are being mapped are essentially the PCI host
controllers MMIO, IO and CFG windows which are then consumed by the
various bars of the devices on the bus.

So mapping based on pci_device_add would require us to go from the SBDF
to a set of BARS which need mapping, which is a whole lot more complex
than just mapping all of the resources owned by the root complex through
to the h/w domain.

Or perhaps I've misunderstood what you were suggesting?

 This would allow us to re-use most of the interrupts/mmio decoding
 provided in the device tree library. It would also avoid missing support
 of cascade ranges/interrupt-map.

I *think* (if I'm remembering right) I decided we don't need to worry
about cascades of these things because the second level resources are
all fully contained within the first (top level) one and so with the
approach I've taken here are all fully mapped already. That's why I made
this patch stop descending into children when such a bus node is
found.

Ian.




___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

2015-02-18 Thread Julien Grall



On 18/02/2015 14:37, Ian Campbell wrote:

On Wed, 2015-02-18 at 14:19 +, Julien Grall wrote:
I think so, and we probably should consider the two cases separately
since the right answer could reasonably differ for different resource
types.

I am reasonably convinced that for MMIO (+IO+CFG space) we should map
everything as described by the ranges property of the top most node, it
can be considered an analogue to / extension of the reg property of that
node.


Agreed.


For IRQ I'm not so sure, it's possible that routing the IRQ at
pci_add_device time might be better, or fit in better with e.g. the ACPI
architecture, but mapping everything described in interrupt-map at start
of day is also an option and a reasonably simple one, probably.


I agree that it's simple. Are we sure that we would be able to get a 
better solution later without modifying the kernel?


If not, we may need to keep this solution forever.


This isn't to do with IPA-PA translations but to do with translations
between different PA addressing regimes. i.e. the different addressing
schemes of difference busses.


I meant bus address. The name intermediate address was misused, sorry.


Lets say we have a system with a PCI-ROOT device exposing a PCI bus,
which in turn contains a PCI-BRIDGE which for the sake of argument lets
say is a PCI-FOOBUS bridge.



Lets just consider the MMIO hole for now, but IRQ is basically the same.

The ranges property on a node describes a mapping from a parent
address space into a child address space.

For PCI-ROOT parent is the host physical address space and child is
the PCI MMIO/IO/CFG address spaces.

For PCI-BRIDGE parent is the PCI-ROOT's child address space (i.e. PCI
MMIO/IO/CFG) and child is the FOOBUS address space.

The inputs (parents) of the PCI-BRIDGE ranges property must therefore
by definition be valid outputs of the PCI-ROOT ranges property (i.e. be
child addresses).

Therefore if we map all of the input/parent ranges described by
PCI-ROOT's ranges property we do not need to recurse further and
consider PCI-BRIDGE's ranges property -- we've effectively already dealt
with it.

Does that make more sense?


I'm still confused, what prevents the PCI-ROOT device to not be 
connected to another bus?


In device tree format, that would give something like:

/ {

  soc {
 ranges = ...;

 pcie {
   ranges = ...;
 }
  }
}

The address retrieved from the PCI-ROOT would be a bus address and not a 
physical address.


Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

2015-02-18 Thread Ian Campbell
On Wed, 2015-02-18 at 15:05 +, Julien Grall wrote:
 
 On 18/02/2015 14:37, Ian Campbell wrote:
  On Wed, 2015-02-18 at 14:19 +, Julien Grall wrote:
  I think so, and we probably should consider the two cases separately
  since the right answer could reasonably differ for different resource
  types.
 
  I am reasonably convinced that for MMIO (+IO+CFG space) we should map
  everything as described by the ranges property of the top most node, it
  can be considered an analogue to / extension of the reg property of that
  node.
 
 Agreed.
 
  For IRQ I'm not so sure, it's possible that routing the IRQ at
  pci_add_device time might be better, or fit in better with e.g. the ACPI
  architecture, but mapping everything described in interrupt-map at start
  of day is also an option and a reasonably simple one, probably.
 
 I agree that it's simple. Are we sure that we would be able to get a 
 better solution later without modifying the kernel?
 
 If not, we may need to keep this solution forever.

True. I suppose feature flags would be one way out, but not a very
convenient one..

  This isn't to do with IPA-PA translations but to do with translations
  between different PA addressing regimes. i.e. the different addressing
  schemes of difference busses.
 
 I meant bus address. The name intermediate address was misused, sorry.
 
  Lets say we have a system with a PCI-ROOT device exposing a PCI bus,
  which in turn contains a PCI-BRIDGE which for the sake of argument lets
  say is a PCI-FOOBUS bridge.
 
  Lets just consider the MMIO hole for now, but IRQ is basically the same.
 
  The ranges property on a node describes a mapping from a parent
  address space into a child address space.
 
  For PCI-ROOT parent is the host physical address space and child is
  the PCI MMIO/IO/CFG address spaces.
 
  For PCI-BRIDGE parent is the PCI-ROOT's child address space (i.e. PCI
  MMIO/IO/CFG) and child is the FOOBUS address space.
 
  The inputs (parents) of the PCI-BRIDGE ranges property must therefore
  by definition be valid outputs of the PCI-ROOT ranges property (i.e. be
  child addresses).
 
  Therefore if we map all of the input/parent ranges described by
  PCI-ROOT's ranges property we do not need to recurse further and
  consider PCI-BRIDGE's ranges property -- we've effectively already dealt
  with it.
 
  Does that make more sense?
 
 I'm still confused, what prevents the PCI-ROOT device to not be 
 connected to another bus?

 In device tree format, that would give something like:
 
 / {
 
soc {
   ranges = ...;
 
   pcie {
 ranges = ...;
   }
}
 }
 
 The address retrieved from the PCI-ROOT would be a bus address and not a 
 physical address.

Hrm, nothing, I see what you are getting at now.

Either soc has a device_type property which we understand, in which case
we would handle it and stop recursing or (more likely for an soc) it
does not, in which case we would handle the pcie ranges property, but it
needs to be translated through the ranges property of soc, which the
patch doesn't do and probably it should.

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

2015-02-18 Thread Ian Campbell
On Wed, 2015-02-18 at 15:13 +, Julien Grall wrote:
 
 On 18/02/2015 14:37, Ian Campbell wrote:
  I am reasonably convinced that for MMIO (+IO+CFG space) we should map
  everything as described by the ranges property of the top most node, it
  can be considered an analogue to / extension of the reg property of that
  node.
 
 BTW, the CFG space is part of the reg property, which is already 
 mapped. The ranges property only covers the IO/MMIO BARs.

Right, I keep forgetting...

Ian.



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

2015-02-18 Thread Julien Grall



On 18/02/2015 14:37, Ian Campbell wrote:

I am reasonably convinced that for MMIO (+IO+CFG space) we should map
everything as described by the ranges property of the top most node, it
can be considered an analogue to / extension of the reg property of that
node.


BTW, the CFG space is part of the reg property, which is already 
mapped. The ranges property only covers the IO/MMIO BARs.


--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

2015-02-18 Thread Julien Grall



On 18/02/2015 15:05, Julien Grall wrote:



On 18/02/2015 14:37, Ian Campbell wrote:

On Wed, 2015-02-18 at 14:19 +, Julien Grall wrote:
I think so, and we probably should consider the two cases separately
since the right answer could reasonably differ for different resource
types.

I am reasonably convinced that for MMIO (+IO+CFG space) we should map
everything as described by the ranges property of the top most node, it
can be considered an analogue to / extension of the reg property of that
node.


Agreed.


For IRQ I'm not so sure, it's possible that routing the IRQ at
pci_add_device time might be better, or fit in better with e.g. the ACPI
architecture, but mapping everything described in interrupt-map at start
of day is also an option and a reasonably simple one, probably.


I agree that it's simple. Are we sure that we would be able to get a
better solution later without modifying the kernel?

If not, we may need to keep this solution forever.


This isn't to do with IPA-PA translations but to do with translations
between different PA addressing regimes. i.e. the different addressing
schemes of difference busses.


I meant bus address. The name intermediate address was misused, sorry.


Lets say we have a system with a PCI-ROOT device exposing a PCI bus,
which in turn contains a PCI-BRIDGE which for the sake of argument lets
say is a PCI-FOOBUS bridge.



Lets just consider the MMIO hole for now, but IRQ is basically the same.

The ranges property on a node describes a mapping from a parent
address space into a child address space.

For PCI-ROOT parent is the host physical address space and child is
the PCI MMIO/IO/CFG address spaces.

For PCI-BRIDGE parent is the PCI-ROOT's child address space (i.e. PCI
MMIO/IO/CFG) and child is the FOOBUS address space.

The inputs (parents) of the PCI-BRIDGE ranges property must therefore
by definition be valid outputs of the PCI-ROOT ranges property (i.e. be
child addresses).

Therefore if we map all of the input/parent ranges described by
PCI-ROOT's ranges property we do not need to recurse further and
consider PCI-BRIDGE's ranges property -- we've effectively already dealt
with it.

Does that make more sense?


I'm still confused, what prevents the PCI-ROOT device to not be
connected to another bus?

In device tree format, that would give something like:

/ {

   soc {
  ranges = ...;

  pcie {
ranges = ...;
  }
   }
}



Actually the device tree of the x-gene board has something similar.

/ {
  soc {
ranges;

pcie {
  ranges = ...;
}
}

ranges; means there is not translation necessary. But nothing prevent 
to have a the property ranges set.


Regards,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties

2015-02-17 Thread Julien Grall
Hi Ian,

On 24/10/14 10:58, Ian Campbell wrote:
 These properties are defined in ePAPR and the OpenFirmware PCI Bus Binding
 Specification (IEEE Std 1275-1994).
 
 This replaces the xgene specific mapping. Tested on Mustang and on a model 
 with
 a PCI virtio controller.

I'm wondering why you choose to map everything at Xen boot time rather
than implementing PHYSDEVOP_pci_device_add to do the job?

This would allow us to re-use most of the interrupts/mmio decoding
provided in the device tree library. It would also avoid missing support
of cascade ranges/interrupt-map.

Regards,

-- 
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel