xen: arm: Parse PCI DT nodes' ranges and interrupt-map
cover xen: arm: Parse PCI DT nodes' ranges and interrupt-map
This series adds parsing of the DT ranges and interrupt-map properties for PCI devices, these contain the MMIOs and IRQs used by children on the bus. This replaces the specific mapping on xgene which should also mean that it works on xgene devices other than mustang which use a different PCI root controller (the xgene has several, we only map the first).
I've tested on Mustang and on a FastModel with virtio-pci based rootfs.
This is *not* for 4.5.
Ian.
PATCH 1/5 xen: arm: propagate gic's #address-cells property to dom0.
The interrupt-map property requires that the interrupt-parent node must have both #address-cells and #interrupt-cells properties (see ePAPR 2.4.3.1). Therefore propagate the property if it is present.
We must propagate (rather than invent our own value) since this value is used to size fields within other properties within the tree.
ePAPR strictly speaking requires that the interrupt-parent node always has these properties. However reality has diverged from this and implementations will recursively search parents for #*-cells properties. Hence we only copy if it is present.
--- xen/arch/arm/domain_build.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
xen/arch/arm/domain_build.c +10 -0
@@ -788,6 +788,8 @@ static int make_gic_node(const struct domain *d, void *fdt, {const struct dt_device_node *gic = dt_interrupt_controller;int res =0 ; +const void *addrcells; +u32 addrcells_len;/* * Xen currently supports only a single GIC. Discard any secondary
@@ -817,6 +819,14 @@ static int make_gic_node(const struct domain *d, void *fdt,return res; } + addrcells =dt_get_property (gic,"#address-cells" , &addrcells_len); +if ( addrcells ) + { + res =fdt_property (fdt,"#address-cells" , addrcells, addrcells_len); +if ( res ) +return res; + } + res =fdt_end_node (fdt);return res;
Signature · 1 line
1.7.10.4
Re: [PATCH 1/5] xen: arm: propagate gic's #address-cells property to dom0.
Hi Ian,
Quoted 7 lines · Ian Campbell
The interrupt-map property requires that the interrupt-parent node must have both #address-cells and #interrupt-cells properties (see ePAPR 2.4.3.1). Therefore propagate the property if it is present.
We must propagate (rather than invent our own value) since this value is used to size fields within other properties within the tree.
Quoted 6 lines
ePAPR strictly speaking requires that the interrupt-parent node always has these properties. However reality has diverged from this and implementations will recursively search parents for #*-cells properties. Hence we only copy if it is present.
Signed-off-by Ian Campbell <[email protected]>
Without this patch I can't boot Xen on the Foundation model with GIC-v3. Is it possible to push this patch for Xen 4.5 rc1?
Regards,
Signature · 1 line
Julien Grall
Re: [PATCH 1/5] xen: arm: propagate gic's #address-cells property to dom0.
Quoted 15 lines · Julien Grall
Hi Ian,
Quoted 12 lines · Ian Campbell
The interrupt-map property requires that the interrupt-parent node must have both #address-cells and #interrupt-cells properties (see ePAPR 2.4.3.1). Therefore propagate the property if it is present.
We must propagate (rather than invent our own value) since this value is used to size fields within other properties within the tree.
ePAPR strictly speaking requires that the interrupt-parent node always has these properties. However reality has diverged from this and implementations will recursively search parents for #*-cells properties. Hence we only copy if it is present.
Quoted 6 lines
Quoted 1 line
Signed-off-by Ian Campbell <[email protected]>Reviewed-by Julien Grall <[email protected]>Without this patch I can't boot Xen on the Foundation model with GIC-v3. Is it possible to push this patch for Xen 4.5 rc1?
rc1 is long gone... I'll push for rc2 though.
Ian.
Re: [PATCH 1/5] xen: arm: propagate gic's #address-cells property to dom0.
Quoted 16 lines · Ian Campbell
Quoted 15 lines · Julien Grall
Hi Ian,
Quoted 12 lines · Ian Campbell
The interrupt-map property requires that the interrupt-parent node must have both #address-cells and #interrupt-cells properties (see ePAPR 2.4.3.1). Therefore propagate the property if it is present.
We must propagate (rather than invent our own value) since this value is used to size fields within other properties within the tree.
ePAPR strictly speaking requires that the interrupt-parent node always has these properties. However reality has diverged from this and implementations will recursively search parents for #*-cells properties. Hence we only copy if it is present.
Quoted 8 lines
Quoted 6 lines
Quoted 1 line
Signed-off-by Ian Campbell <[email protected]>Reviewed-by Julien Grall <[email protected]>Without this patch I can't boot Xen on the Foundation model with GIC-v3. Is it possible to push this patch for Xen 4.5 rc1?
rc1 is long gone... I'll push for rc2 though.
Hmm right, I though the first version was rc0 not rc1.
Signature · 1 line
Julien Grall
Re: [PATCH 1/5] xen: arm: propagate gic's #address-cells property to dom0.
In reply to Julien GrallQuoted 15 lines · Julien Grall
Hi Ian,
Quoted 12 lines · Ian Campbell
The interrupt-map property requires that the interrupt-parent node must have both #address-cells and #interrupt-cells properties (see ePAPR 2.4.3.1). Therefore propagate the property if it is present.
We must propagate (rather than invent our own value) since this value is used to size fields within other properties within the tree.
ePAPR strictly speaking requires that the interrupt-parent node always has these properties. However reality has diverged from this and implementations will recursively search parents for #*-cells properties. Hence we only copy if it is present.
Quoted 6 lines
Quoted 1 line
Signed-off-by Ian Campbell <[email protected]>Reviewed-by Julien Grall <[email protected]>Without this patch I can't boot Xen on the Foundation model with GIC-v3. Is it possible to push this patch for Xen 4.5 rc1?
Konrad, I think this is needed for 4.5 since without it dom0 can fail to parse certain other constructs within the DT (in bits which we don't generate and can't easily/don't want to rewrite as we pass them through).
The risk is that some bit of DT which we do generate relies on this value being absent (as it was before this patch). I don't believe we generate any such nodes. The consumers are typically nodes representing devices which we pass through rather than messing with them.
Ian.
Re: [PATCH 1/5] xen: arm: propagate gic's #address-cells property to dom0.
Quoted 20 lines · Ian Campbell
Quoted 19 lines · Julien Grall
Hi Ian,
Quoted 7 lines · Ian Campbell
The interrupt-map property requires that the interrupt-parent node must have both #address-cells and #interrupt-cells properties (see ePAPR 2.4.3.1). Therefore propagate the property if it is present.
We must propagate (rather than invent our own value) since this value is used to size fields within other properties within the tree.
Quoted 6 lines
ePAPR strictly speaking requires that the interrupt-parent node always has these properties. However reality has diverged from this and implementations will recursively search parents for #*-cells properties. Hence we only copy if it is present.
Signed-off-by Ian Campbell <[email protected]>Reviewed-by Julien Grall <[email protected]>
Quoted 7 lines
Quoted 2 lines
Without this patch I can't boot Xen on the Foundation model with GIC-v3. Is it possible to push this patch for Xen 4.5 rc1?
Konrad, I think this is needed for 4.5 since without it dom0 can fail to parse certain other constructs within the DT (in bits which we don't generate and can't easily/don't want to rewrite as we pass them through).
Quoted 8 lines
The risk is that some bit of DT which we do generate relies on this value being absent (as it was before this patch). I don't believe we generate any such nodes. The consumers are typically nodes representing devices which we pass through rather than messing with them.
Ian.
Ian Campbell
AppliedRe: [PATCH 1/5] xen: arm: propagate gic's #address-cells property to dom0.
Quoted 24 lines · Konrad Rzeszutek Wilk
Quoted 23 lines · Ian Campbell
Quoted 21 lines · Julien Grall
Hi Ian,
Quoted 7 lines · Ian Campbell
The interrupt-map property requires that the interrupt-parent node must have both #address-cells and #interrupt-cells properties (see ePAPR 2.4.3.1). Therefore propagate the property if it is present.
We must propagate (rather than invent our own value) since this value is used to size fields within other properties within the tree.
Quoted 6 lines
ePAPR strictly speaking requires that the interrupt-parent node always has these properties. However reality has diverged from this and implementations will recursively search parents for #*-cells properties. Hence we only copy if it is present.
Signed-off-by Ian Campbell <[email protected]>Reviewed-by Julien Grall <[email protected]>Without this patch I can't boot Xen on the Foundation model with GIC-v3. Is it possible to push this patch for Xen 4.5 rc1?
Quoted 6 lines
Quoted 4 lines
Konrad, I think this is needed for 4.5 since without it dom0 can fail to parse certain other constructs within the DT (in bits which we don't generate and can't easily/don't want to rewrite as we pass them through).
Release-Acked-by Konrad Rzeszutek Wilk <[email protected]>
Applied, thanks.
Ian.
PATCH 2/5 xen: device-tree: add accessors for the addr/size-cells of a node's children.
In reply to Ian CampbellThe existing dt_n_{addr,size}_cells functions tell you which sizes will apply to this node, which are the #address/size-cells properties of the node's *parent* (or grandparent, etc).
In some cases we need to know what size applies to a nodes children, which can include the #address/size-cells properties of the node itself (or its parent).
Refactor the existing function to allow both possibilities.
Clean up the grammar of the existing doc comments a bit.
--- xen/common/device_tree.c | 35 ++++++++++++++++++++++++++---- xen/include/xen/device_tree.h | 47 ++++++++++++++++++++++++++++++++++------- 2 files changed, 70 insertions(+), 12 deletions(-)
xen/common/device_tree.c +31 -4
@@ -368,13 +368,17 @@ dt_find_matching_node(struct dt_device_node *from,return NULL ; } -int dt_n_addr_cells (const struct dt_device_node *np) +/* If parent is true then start from the parent. */ +static int dt_n_addr_cells_common (const struct dt_device_node *np,bool parent) {const __be32 *ip; do { - if ( np->parent ) + if ( parent && np->parent ) np = np->parent; + else + parent = 1; + ip = dt_get_property(np, "#address-cells", NULL); if ( ip ) return be32_to_cpup(ip);
@@ -383,13 +387,26 @@ int dt_n_addr_cells(const struct dt_device_node *np)return DT_ROOT_NODE_ADDR_CELLS_DEFAULT ; } -int dt_n_size_cells (const struct dt_device_node *np) +int dt_n_addr_cells (const struct dt_device_node *np) +{ +return dt_n_addr_cells_common (np, true); +} + +int dt_n_addr_cells_children (const struct dt_device_node *np) +{ +return dt_n_addr_cells_common (np, false); +} + +/* If parent is true then start from the parent. */ +static int dt_n_size_cells_common (const struct dt_device_node *np,bool parent) {const __be32 *ip; do { - if ( np->parent ) + if ( parent && np->parent ) np = np->parent; + else + parent = 1; ip = dt_get_property(np, "#size-cells", NULL); if ( ip ) return be32_to_cpup(ip);
@@ -398,6 +415,16 @@ int dt_n_size_cells(const struct dt_device_node *np)return DT_ROOT_NODE_SIZE_CELLS_DEFAULT ; } +int dt_n_size_cells (const struct dt_device_node *np) +{ +return dt_n_size_cells_common (np, true); +} +int dt_n_size_cells_children (const struct dt_device_node *np) +{ +return dt_n_size_cells_common (np, false); +} + +/* * Default translator (generic bus) */
xen/include/xen/device_tree.h +39 -8
63 lines not on this page · raw message
Signature · 1 line
1.7.10.4
PATCH 3/5 xen: arm: Add DT_NR_GIC_INTERRUPT_CELLS rather than hardcoding 3
In reply to Ian CampbellI'm going to need this elsewhere and would prefer to avoid writing 3 again.
--- xen/arch/arm/domain_build.c | 2 +- xen/arch/arm/gic.c | 2 +- xen/include/asm-arm/gic.h | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-)
xen/arch/arm/domain_build.c +1 -1
@@ -500,7 +500,7 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo,* This code is assuming the irq is an PPI. */ -typedef __be32 gic_interrupt_t [3 ]; +typedef __be32 gic_interrupt_t [DT_NR_GIC_INTERRUPT_CELLS ];static void set_interrupt_ppi (gic_interrupt_t interrupt,unsigned int irq,unsigned int cpumask,unsigned int level)
xen/arch/arm/gic.c +1 -1
@@ -147,7 +147,7 @@ int gic_irq_xlate(const u32 *intspec, unsigned int intsize, unsigned int *out_hwirq, unsigned int *out_type) { -if ( intsize <3 ) +if ( intsize <DT_NR_GIC_INTERRUPT_CELLS )return -EINVAL ;/* Get the interrupt number and add 16 to skip over SGIs */
xen/include/asm-arm/gic.h +2 -0
@@ -22,6 +22,8 @@#define NR_GIC_SGI 16#define MAX_RDIST_COUNT 4 +#define DT_NR_GIC_INTERRUPT_CELLS 3 +#define GICD_CTLR (0x000)#define GICD_TYPER (0x004)#define GICD_IIDR (0x008)
Signature · 1 line
1.7.10.4
PATCH 4/5 xen: refactor irq_set_type out of platform_get_irq
In reply to Ian CampbellWhen parsing a PCI nodes interrupt-map property I need to be able to configure an interrupt which has no associated dt_device_node. So refactor the code and expose.
--- xen/arch/arm/irq.c | 16 ++++++++++------ xen/include/asm-arm/irq.h | 1 + 2 files changed, 11 insertions(+), 6 deletions(-)
xen/arch/arm/irq.c +10 -6
@@ -520,6 +520,15 @@ unlock:return ret; } +int irq_set_type (unsigned int irq,unsigned int type) +{ +/* Setup the IRQ type */ +if ( irq <NR_LOCAL_IRQS ) +return irq_local_set_type (irq, type); +else +return irq_set_spi_type (irq, type); +} +int platform_get_irq (const struct dt_device_node *device,int index) {struct dt_irq dt_irq;
@@ -533,12 +542,7 @@ int platform_get_irq(const struct dt_device_node *device, int index) irq = dt_irq.irq; type = dt_irq.type; -/* Setup the IRQ type */ -if ( irq <NR_LOCAL_IRQS ) - res =irq_local_set_type (irq, type); -else - res =irq_set_spi_type (irq, type); - + res =irq_set_type (irq, type);if ( res )return -1 ;
xen/include/asm-arm/irq.h +1 -0
@@ -47,6 +47,7 @@ void arch_move_irqs(struct vcpu *v);/* Set IRQ type for an SPI */ int irq_set_spi_type (unsigned int spi,unsigned int type); +int irq_set_type (unsigned int irq,unsigned int type);int platform_get_irq (const struct dt_device_node *device,int index);void irq_set_affinity (struct irq_desc *desc,const cpumask_t *cpu_mask);
Signature · 1 line
1.7.10.4
PATCH 5/5 xen: arm: handle PCI DT node ranges and interrupt-map properties
In reply to Ian CampbellThese 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.
--- xen/arch/arm/domain_build.c | 193 ++++++++++++++++++++++++++++++++++ xen/arch/arm/platforms/xgene-storm.c | 90 ---------------- xen/include/xen/device_tree.h | 32 ++++++ 3 files changed, 225 insertions(+), 90 deletions(-)
xen/arch/arm/domain_build.c +193 -0
@@ -907,6 +907,191 @@ static int make_timer_node(const struct domain *d, void *fdt,return res; } +static int map_pci_device_ranges (struct domain *d, +const struct dt_device_node *dev, +const struct dt_pci_ranges_entry *ranges, +const u32 len) +{ +int i, nr, res; + +if ( len %sizeof (struct dt_pci_ranges_entry ) ) + { +printk (XENLOG_ERR +"%s: ranges property length is not a multiple of entry size\n" , +dt_node_name (dev)); +return -EINVAL ; + } + + nr = len /sizeof (*ranges); + +for ( i =0 ; i < nr ; i++ ) + { +const struct dt_pci_ranges_entry *range = &ranges[i]; +u64 addr, len; + + addr =fdt64_to_cpu (range->cpu_addr); + len =fdt64_to_cpu (range->length); + +DPRINT ("%s: ranges[%d]: 0x%08x.%08x.%08x 0x%" PRIx64" size 0x%" PRIx64"\n" , +dt_node_name (dev), i, +fdt32_to_cpu (range->pci_addr.hi), +fdt32_to_cpu (range->pci_addr.mid), +fdt32_to_cpu (range->pci_addr.lo), + addr, len); + + res =map_mmio_regions (d,paddr_to_pfn (addr &PAGE_MASK ), +DIV_ROUND_UP (len,PAGE_SIZE ), +paddr_to_pfn (addr &PAGE_MASK )); +if ( res <0 ) + { +printk (XENLOG_ERR "%s: ranges[%d]: " +"Unable to map 0x%" PRIx64" - 0x%" PRIx64" in domain %d\n" , +dt_node_name (dev), i, + addr &PAGE_MASK ,PAGE_ALIGN (addr + len) -1 , + d->domain_id); +return res; + } + } +return 0 ; +} + +static int map_device_ranges (struct domain *d,const struct dt_device_node *dev) +{ +const void *ranges; +u32 len; + +if ( !dev->type ) +return 0 ;/* No device type, nothing to do */ + + ranges =dt_get_property (dev,"ranges" , &len); +if ( !ranges ) +return 0 ;/* No ranges, nothing to do */ + +if (dt_device_type_is_equal (dev,"pci" ) ) +return map_pci_device_ranges (d, dev, ranges, len); +else if (dt_device_type_is_equal (dev,"<NULL>" ) ) +return 0 ;/* "<NULL>" is used for none */ + +DPRINT ("Cannot handle ranges property for non-PCI device %s type %s\n" , +dt_node_name (dev), dev->type); + +/* Non-critical. */ +return 0 ; +} + +static int map_pci_device_interrupts (struct domain *d, +const struct dt_device_node *dev, +const __be32 *cells,const u32 len) +{ +const int intc_addr_cells = +dt_n_addr_cells_children (dt_interrupt_controller); +const size_t entry_size = +sizeof (struct dt_interrupt_map_entry ) + + (intc_addr_cells +DT_NR_GIC_INTERRUPT_CELLS )*sizeof (__be32); + +int i, nr, res; +u64 parent_addr; + +if ( len % entry_size ) + { +printk (XENLOG_ERR +"%s: interrupts property length is not a multiple of entry size %zd\n" , +dt_node_name (dev), entry_size); +return -EINVAL ; + } + + nr = len / entry_size; + +for ( i =0 ; i < nr ; i++ ) + { +const struct dt_interrupt_map_entry *map = (void *)cells; +struct dt_raw_irq dt_raw_irq; +struct dt_irq dt_irq; +int j; + +if (fdt32_to_cpu (map->phandle) != dt_interrupt_controller->phandle ) + { +printk ("%s: interrupt-map[%d]: Not connected to primary controller.\n" , +dt_node_name (dev), i); +continue ; + } + + dt_raw_irq.controller = dt_interrupt_controller; + dt_raw_irq.size =DT_NR_GIC_INTERRUPT_CELLS ; + + cells = &map->irq_specifier[0 ]; + + parent_addr =dt_next_cell (intc_addr_cells, &cells); +for (j =0 ; j <DT_NR_GIC_INTERRUPT_CELLS ; j++) + dt_raw_irq.specifier[j] =dt_next_cell (1 , &cells); + + res =dt_irq_translate (&dt_raw_irq, &dt_irq); +if ( res <0 ) + { +printk (XENLOG_ERR +"%s: interrupt-map[%d]: Unable to translate raw IRQ\n" , +dt_node_name (dev), i); +continue ; + } + +DPRINT ("%s: interrupt-map[%d]: 0x%" PRIx32".%" PRIx32".%" PRIx32" " +"pin=%" PRId32" phandle=%" PRIx32" " +"paddr=%" PRIx64" type=%" PRIx32" irq=%x\n" , +dt_node_name (dev), i, +fdt32_to_cpu (map->pci_irq_mask.pci_addr.hi), +fdt32_to_cpu (map->pci_irq_mask.pci_addr.mid), +fdt32_to_cpu (map->pci_irq_mask.pci_addr.lo), +fdt32_to_cpu (map->pci_irq_mask.pci_pin), +fdt32_to_cpu (map->phandle), + parent_addr, dt_irq.type, dt_irq.irq); + +/* Setup the IRQ type */ + res =irq_set_type (dt_irq.irq, dt_irq.type); +if ( res ) + { +printk (XENLOG_ERR +"%s: interrupt-map[%d]: Unable to setup IRQ%" PRId32" to dom%d\n" , +dt_node_name (dev), i, dt_irq.irq, d->domain_id); +return res; + } + + res =route_irq_to_guest (d, dt_irq.irq,dt_node_name (dev)); +if ( res <0 ) + { +printk (XENLOG_ERR +"%s: interrupt-map[%d]: Unable to map IRQ%" PRId32" to dom%d\n" , +dt_node_name (dev), i, dt_irq.irq, d->domain_id); +return res; + } + } +return 0 ; +} + +static int map_device_interrupts (struct domain *d,const struct dt_device_node *dev) +{ +const void *map; +u32 len; + +if ( !dev->type ) +return 0 ;/* No device type, nothing to do */ + + map =dt_get_property (dev,"interrupt-map" , &len); +if ( !map )/* nothing to do */ +return 0 ; + +if (dt_device_type_is_equal (dev,"pci" ) ) +return map_pci_device_interrupts (d, dev, map, len); +else if (dt_device_type_is_equal (dev,"<NULL>" ) ) +return 0 ;/* "<NULL>" is used for none */ + +printk ("Cannot handle interrupt-map for non-PCI device %s type %s\n" , +dt_node_name (dev), dev->type); + +/* Non-critical. */ +return 0 ; +} + +/* Map the device in the domain */ static int map_device (struct domain *d,struct dt_device_node *dev) {
@@ -1015,6 +1200,14 @@ static int map_device(struct domain *d, struct dt_device_node *dev) } } + res =map_device_ranges (d, dev); +if ( res ) +return res; + + res =map_device_interrupts (d, dev); +if ( res ) +return res; +return 0 ; }
xen/arch/arm/platforms/xgene-storm.c +0 -90
102 lines not on this page · raw message
xen/include/xen/device_tree.h +32 -0
@@ -682,6 +682,38 @@ int dt_parse_phandle_with_args(const struct dt_device_node *np, const char *cells_name, int index, struct dt_phandle_args *out_args); +/* +* Definitions for implementing parts of the OpenFirmware PCI Bus Binding +* Specification (IEEE Std 1275-1994). +*/ + +struct dt_pci_unit_address { +__be32 hi, mid, lo; +} __attribute__((packed)); + +struct dt_pci_irq_mask { +struct dt_pci_unit_address pci_addr; +__be32 pci_pin; +} __attribute__((packed)); + +struct dt_pci_ranges_entry { +struct dt_pci_unit_address pci_addr; +__be64 cpu_addr; +__be64 length; +} __attribute__((packed)); + +struct dt_interrupt_map_entry { +struct dt_pci_irq_mask pci_irq_mask; +__be32 phandle; +__be32 irq_specifier[0 ]; +/* +* irq_specifier is: parent-addr and interrupt-descriptor, both +* sized according to the interrupt-parent's #address-cells and +* #interrupt-cells. +*/ +} __attribute__((packed)); + +#endif /* __XEN_DEVICE_TREE_H */ /*
Signature · 1 line
1.7.10.4
Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties
Hi Ian,
Quoted 5 lines · Ian Campbell
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,
Signature · 1 line
Julien Grall
Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties
Quoted 11 lines · Julien Grall
Hi Ian,
Quoted 5 lines · Ian Campbell
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_addto 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?
Quoted 3 lines
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.
Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties
Hi Ian,
Quoted 7 lines · Ian Campbell
Quoted 6 lines · Julien Grall
Hi Ian,
Quoted 3 lines · Ian Campbell
These properties are defined in ePAPR and the OpenFirmware PCI Bus Binding Specification (IEEE Std 1275-1994).
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
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_addto do the job?Does
pci_device_addcontain 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.
Quoted 8 lines
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_addwould 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?
Quoted 1 line
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.
Quoted 10 lines
Quoted 3 lines
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,
Signature · 1 line
Julien Grall
Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties
Quoted 30 lines · Julien Grall
Hi Ian,
Quoted 7 lines · Ian Campbell
Quoted 6 lines · Julien Grall
Hi Ian,
Quoted 3 lines · Ian Campbell
These properties are defined in ePAPR and the OpenFirmware PCI Bus Binding Specification (IEEE Std 1275-1994).
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
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_addto do the job?Does
pci_device_addcontain 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.
Quoted 8 lines
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_addwould 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.
Quoted 8 lines
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?Quoted 1 line
Or perhaps I've misunderstood what you were suggesting?
I was suggesting to do it via
pci_add_devicebut 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)
Quoted 11 lines
Quoted 10 lines
Quoted 3 lines
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.
Quoted 4 lines
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.
Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties
Quoted 9 lines · Ian Campbell
On Wed, 2015-02-18 at 14:19 +0000, 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.
Quoted 4 lines
For IRQ I'm not so sure, it's possible that routing the IRQ at
pci_add_devicetime 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.
Quoted 3 lines
This isn't to do with
IPA->PAtranslations 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.
Quoted 19 lines
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).
Quoted 6 lines
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,
Signature · 1 line
Julien Grall
Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties
Quoted 64 lines · Julien Grall
Quoted 9 lines · Ian Campbell
On Wed, 2015-02-18 at 14:19 +0000, 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.
Quoted 4 lines
For IRQ I'm not so sure, it's possible that routing the IRQ at
pci_add_devicetime 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.
Quoted 3 lines
This isn't to do with
IPA->PAtranslations 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.
Quoted 19 lines
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).
Quoted 6 lines
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:
/ {
Quoted 8 lines
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,
Signature · 1 line
Julien Grall
Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties
Quoted 2 lines · Julien Grall
"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...
Ian.
Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties
Hi Ian,
Quoted 6 lines · Ian Campbell
Quoted 2 lines · Julien Grall
"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,
Signature · 1 line
Julien Grall
Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties
In reply to Julien GrallQuoted 19 lines · Julien Grall
Quoted 9 lines · Ian Campbell
On Wed, 2015-02-18 at 14:19 +0000, 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.
Quoted 4 lines
For IRQ I'm not so sure, it's possible that routing the IRQ at
pci_add_devicetime 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.
Quoted 4 lines
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..
Quoted 42 lines
Quoted 3 lines
This isn't to do with
IPA->PAtranslations 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.
Quoted 19 lines
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).
Quoted 6 lines
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 = "...";
Quoted 8 lines
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.
Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties
Quoted 79 lines · Ian Campbell
Quoted 19 lines · Julien Grall
Quoted 9 lines · Ian Campbell
On Wed, 2015-02-18 at 14:19 +0000, 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.
Quoted 4 lines
For IRQ I'm not so sure, it's possible that routing the IRQ at
pci_add_devicetime 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.Quoted 4 lines
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..
Quoted 48 lines
Quoted 3 lines
This isn't to do with
IPA->PAtranslations 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.
Quoted 19 lines
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).
Quoted 6 lines
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 = "..."; } } }Quoted 2 lines
The address retrieved from the PCI-ROOT would be a bus address and not a physical address.
Quoted 7 lines
Hrm, nothing, I see what you are getting at now.
Either soc has a
device_typeproperty 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,
Signature · 1 line
Julien Grall
Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties
Quoted 9 lines · Julien Grall
Quoted 5 lines
Either soc has a
device_typeproperty 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.
Quoted 1 line
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.
Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties
In reply to Ian CampbellQuoted 4 lines · Ian Campbell
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.
Signature · 1 line
Julien Grall
Re: [PATCH 5/5] xen: arm: handle PCI DT node ranges and interrupt-map properties
Quoted 9 lines · Julien Grall
Quoted 4 lines · Ian Campbell
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.
Quoted 13 lines · Ian Campbell
This message has been archived. View the original item
<http://ausev2.amd.com/EnterpriseVault/ViewMessage.asp?VaultId=18C5C586ACEF6BE42BD5DBA644C96C94F1110000amdvault.amd.com&SavesetId=201411267046116~201410241000000000~Z~F141820FA2B2A99F4BA9EAD48EEB5211>
This series adds parsing of the DT ranges and interrupt-map properties for PCI devices, these contain the MMIOs and IRQs used by children on the bus. This replaces the specific mapping on xgene which should also mean that it works on xgene devices other than mustang which use a different PCI root controller (the xgene has several, we only map the first).I've tested on Mustang and on a FastModel with virtio-pci based rootfs.
This is *not* for 4.5.
Quoted 7 lines
Ian.
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Ian,
I have tested this on Seattle w/ the ToT Xen-4.6 unstable branch (commit 001324547356af86875fad5003f679571a6b8f1c), and I have PCI working in Dom0. Are you planning on committing this series at some point?
Thanks,
Surave
Hi Suravee,
Quoted 23 lines · Suravee Suthikulpanit
Quoted 14 lines · Ian Campbell
This message has been archived. View the original item
<http://ausev2.amd.com/EnterpriseVault/ViewMessage.asp?VaultId=18C5C586ACEF6BE42BD5DBA644C96C94F1110000amdvault.amd.com&SavesetId=201411267046116~201410241000000000~Z~F141820FA2B2A99F4BA9EAD48EEB5211>This series adds parsing of the DT ranges and interrupt-map properties for PCI devices, these contain the MMIOs and IRQs used by children on the bus. This replaces the specific mapping on xgene which should also mean that it works on xgene devices other than mustang which use a different PCI root controller (the xgene has several, we only map the first).
I've tested on Mustang and on a FastModel with virtio-pci based rootfs.
This is *not* for 4.5.
Quoted 7 lines
Ian.
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 5 lines
Ian,
I have tested this on Seattle w/ the ToT Xen-4.6 unstable branch (commit
001324547356af86875fad5003f679571a6b8f1c), and I have PCI working in Dom0. Are you planning on committing this series at some point?
I think the plan is to commit this series.
I will try to review it during the week.
Regards,
Signature · 1 line
Julien Grall
A message that is not in the archive (sent off-list or not yet received): [email protected]
(CC Jan and Andrew)
Hi Suravee,
Quoted 33 lines · Suravee Suthikulanit
By the way, looking at the output of "xl dmesg", I saw the following message:
(XEN) DOM0: PCI host bridge /smb/pcie@f0000000 ranges:
(XEN) DOM0: IO 0xefff0000..0xefffffff ->0x00000000
(XEN) DOM0: MEM 0x40000000..0xbfffffff ->0x40000000
(XEN) DOM0: MEM 0x100000000..0x7fffffffff ->0x100000000
(XEN) DOM0: pci-host-generic f0000000.pcie: PCI host bridge to bus 0000:00 (XEN) DOM0:pci_bus0000:00: root bus resource [bus 00-7f] (XEN) DOM0:pci_bus0000:00: root bus resource [io 0x0000-0xffff] (XEN) DOM0:pci_bus0000:00: root bus resource [mem 0x40000000-0xbfffffff] (XEN) DOM0:pci_bus0000:00: root bus resource [mem 0x100000000-0x7fffffffff]
(XEN) DOM0: pci 0000:00:00.0:of_irq_parse_pci()failed with rc=-19 (XEN)do_physdev_op16 cmd=25: not implemented yet
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:00.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN) DOM0: pci 0000:00:02.0:of_irq_parse_pci()failed with rc=-19 (XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:02.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:02.1: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:01:00.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN) DOM0: pci 0000:01:00.1:of_irq_parse_pci()failed with rc=-22 (XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:01:00.1: Failed to add - passthrough or MSI/MSI-X might fail!
Quoted 3 lines
IIUC, This is because
xen_add_device()failed, and it seems to be related to some hyper call not implemented. Not sure what is "cmd=15". Any ideas?
There is 2 commands not implemented in the log:
* cmd 15: PHYSDEVOP_manage_pci_add
* cmd 25: PHYSDEVOP_pci_device_add
Linux fallbacks on the former because the later is not implemented.
AFAICT, PHYSDEVOP_manage_pci_add should not be implemented for ARM because it doesn't support segment. I suspect that it's kept for legacy on older Xen x86. Maybe Jan or Andrew have more input on this?
Cheers,
Signature · 1 line
Julien Grall
Quoted 45 lines · Julien Grall
(CC Jan and Andrew)
Hi Suravee,
Quoted 33 lines · Suravee Suthikulanit
By the way, looking at the output of "xl dmesg", I saw the following message:
(XEN) DOM0: PCI host bridge /smb/pcie@f0000000 ranges:
(XEN) DOM0: IO 0xefff0000..0xefffffff ->0x00000000
(XEN) DOM0: MEM 0x40000000..0xbfffffff ->0x40000000
(XEN) DOM0: MEM 0x100000000..0x7fffffffff ->0x100000000
(XEN) DOM0: pci-host-generic f0000000.pcie: PCI host bridge to bus 0000:00 (XEN) DOM0:pci_bus0000:00: root bus resource [bus 00-7f] (XEN) DOM0:pci_bus0000:00: root bus resource [io 0x0000-0xffff] (XEN) DOM0:pci_bus0000:00: root bus resource [mem 0x40000000-0xbfffffff] (XEN) DOM0:pci_bus0000:00: root bus resource [mem 0x100000000-0x7fffffffff]
(XEN) DOM0: pci 0000:00:00.0:of_irq_parse_pci()failed with rc=-19 (XEN)do_physdev_op16 cmd=25: not implemented yet
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:00.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN) DOM0: pci 0000:00:02.0:of_irq_parse_pci()failed with rc=-19 (XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:02.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:02.1: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:01:00.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN) DOM0: pci 0000:01:00.1:of_irq_parse_pci()failed with rc=-22 (XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:01:00.1: Failed to add - passthrough or MSI/MSI-X might fail!Quoted 3 lines
IIUC, This is because
xen_add_device()failed, and it seems to be related to some hyper call not implemented. Not sure what is "cmd=15". Any ideas?There is 2 commands not implemented in the log:
* cmd 15:PHYSDEVOP_manage_pci_add
* cmd 25:PHYSDEVOP_pci_device_add
Quoted 5 lines
Linux fallbacks on the former because the later is not implemented.
AFAICT,
PHYSDEVOP_manage_pci_addshould not be implemented for ARM because it doesn't support segment. I suspect that it's kept for legacy on older Xen x86. Maybe Jan or Andrew have more input on this?
It needs to be kept for backwards compatibility in x86.
All new code should use PHYSDEVOP_pci_device_add.
~Andrew
Quoted 52 lines · Andrew Cooper
Quoted 45 lines · Julien Grall
(CC Jan and Andrew)
Hi Suravee,
Quoted 33 lines · Suravee Suthikulanit
By the way, looking at the output of "xl dmesg", I saw the following message:
(XEN) DOM0: PCI host bridge /smb/pcie@f0000000 ranges:
(XEN) DOM0: IO 0xefff0000..0xefffffff ->0x00000000
(XEN) DOM0: MEM 0x40000000..0xbfffffff ->0x40000000
(XEN) DOM0: MEM 0x100000000..0x7fffffffff ->0x100000000
(XEN) DOM0: pci-host-generic f0000000.pcie: PCI host bridge to bus 0000:00 (XEN) DOM0:pci_bus0000:00: root bus resource [bus 00-7f] (XEN) DOM0:pci_bus0000:00: root bus resource [io 0x0000-0xffff] (XEN) DOM0:pci_bus0000:00: root bus resource [mem 0x40000000-0xbfffffff] (XEN) DOM0:pci_bus0000:00: root bus resource [mem 0x100000000-0x7fffffffff]
(XEN) DOM0: pci 0000:00:00.0:of_irq_parse_pci()failed with rc=-19 (XEN)do_physdev_op16 cmd=25: not implemented yet
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:00.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN) DOM0: pci 0000:00:02.0:of_irq_parse_pci()failed with rc=-19 (XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:02.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:02.1: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:01:00.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN) DOM0: pci 0000:01:00.1:of_irq_parse_pci()failed with rc=-22 (XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:01:00.1: Failed to add - passthrough or MSI/MSI-X might fail!Quoted 3 lines
IIUC, This is because
xen_add_device()failed, and it seems to be related to some hyper call not implemented. Not sure what is "cmd=15". Any ideas?There is 2 commands not implemented in the log:
* cmd 15:PHYSDEVOP_manage_pci_add
* cmd 25:PHYSDEVOP_pci_device_addQuoted 5 lines
Linux fallbacks on the former because the later is not implemented.
AFAICT,
PHYSDEVOP_manage_pci_addshould not be implemented for ARM because it doesn't support segment. I suspect that it's kept for legacy on older Xen x86. Maybe Jan or Andrew have more input on this?
Quoted 6 lines
It needs to be kept for backwards compatibility in x86.
All new code should use
PHYSDEVOP_pci_device_add.~Andrew
Ok, now that I look at the arch/arm/physdev.c, I don't think the code for supporting any of the PHYSDEVOP_xxx is there. That's probably why Xen complains. In contrast, arch/x86/physdev.c has most PHYSDEVOP_xxx already supported.
My question is, are we supposed to be adding code to put the support in here?
Thanks,
Suravee.
Quoted 67 lines · Suravee Suthikulanit
Quoted 54 lines · Andrew Cooper
Quoted 47 lines · Julien Grall
(CC Jan and Andrew)
Hi Suravee,
Quoted 35 lines · Suravee Suthikulanit
By the way, looking at the output of "xl dmesg", I saw the following message:
(XEN) DOM0: PCI host bridge /smb/pcie@f0000000 ranges:
(XEN) DOM0: IO 0xefff0000..0xefffffff ->0x00000000
(XEN) DOM0: MEM 0x40000000..0xbfffffff ->0x40000000
(XEN) DOM0: MEM 0x100000000..0x7fffffffff ->0x100000000
(XEN) DOM0: pci-host-generic f0000000.pcie: PCI host bridge to bus 0000:00
(XEN) DOM0:pci_bus0000:00: root bus resource [bus 00-7f] (XEN) DOM0:pci_bus0000:00: root bus resource [io 0x0000-0xffff] (XEN) DOM0:pci_bus0000:00: root bus resource [mem 0x40000000-0xbfffffff]
(XEN) DOM0:pci_bus0000:00: root bus resource [mem 0x100000000-0x7fffffffff]
(XEN) DOM0: pci 0000:00:00.0:of_irq_parse_pci()failed with rc=-19 (XEN)do_physdev_op16 cmd=25: not implemented yet
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:00.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN) DOM0: pci 0000:00:02.0:of_irq_parse_pci()failed with rc=-19 (XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:02.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:02.1: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:01:00.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN) DOM0: pci 0000:01:00.1:of_irq_parse_pci()failed with rc=-22 (XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:01:00.1: Failed to add - passthrough or MSI/MSI-X might fail!Quoted 3 lines
IIUC, This is because
xen_add_device()failed, and it seems to be related to some hyper call not implemented. Not sure what is "cmd=15". Any ideas?There is 2 commands not implemented in the log:
* cmd 15:PHYSDEVOP_manage_pci_add
* cmd 25:PHYSDEVOP_pci_device_addQuoted 5 lines
Linux fallbacks on the former because the later is not implemented.
AFAICT,
PHYSDEVOP_manage_pci_addshould not be implemented for ARM because it doesn't support segment. I suspect that it's kept for legacy on older Xen x86. Maybe Jan or Andrew have more input on this?Quoted 6 lines
It needs to be kept for backwards compatibility in x86.
All new code should use
PHYSDEVOP_pci_device_add.~Andrew
Ok, now that I look at the
arch/arm/physdev.c, I don't think the code for supporting any of thePHYSDEVOP_xxxis there. That's probably why Xen complains. In contrast,arch/x86/physdev.chas mostPHYSDEVOP_xxxalready supported.
Quoted 6 lines
My question is, are we supposed to be adding code to put the support in here?
Thanks,
Suravee.
My guess is yes, and that would mean we need to enable building drivers/pci.c when building arm code, which then open up a can of worms with re-factoring MSI support code from x86 and etc.
Suravee
Quoted 73 lines · Suravee Suthikulanit
Quoted 72 lines · Suravee Suthikulanit
Quoted 54 lines · Andrew Cooper
Quoted 47 lines · Julien Grall
(CC Jan and Andrew)
Hi Suravee,
Quoted 35 lines · Suravee Suthikulanit
By the way, looking at the output of "xl dmesg", I saw the following message:
(XEN) DOM0: PCI host bridge /smb/pcie@f0000000 ranges:
(XEN) DOM0: IO 0xefff0000..0xefffffff ->0x00000000
(XEN) DOM0: MEM 0x40000000..0xbfffffff ->0x40000000
(XEN) DOM0: MEM 0x100000000..0x7fffffffff ->0x100000000
(XEN) DOM0: pci-host-generic f0000000.pcie: PCI host bridge to bus 0000:00
(XEN) DOM0:pci_bus0000:00: root bus resource [bus 00-7f] (XEN) DOM0:pci_bus0000:00: root bus resource [io 0x0000-0xffff] (XEN) DOM0:pci_bus0000:00: root bus resource [mem 0x40000000-0xbfffffff]
(XEN) DOM0:pci_bus0000:00: root bus resource [mem 0x100000000-0x7fffffffff]
(XEN) DOM0: pci 0000:00:00.0:of_irq_parse_pci()failed with rc=-19 (XEN)do_physdev_op16 cmd=25: not implemented yet
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:00.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN) DOM0: pci 0000:00:02.0:of_irq_parse_pci()failed with rc=-19 (XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:02.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:00:02.1: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:01:00.0: Failed to add - passthrough or MSI/MSI-X might fail!
(XEN) DOM0: pci 0000:01:00.1:of_irq_parse_pci()failed with rc=-22 (XEN)do_physdev_op16 cmd=15: not implemented yet
(XEN) DOM0: pci 0000:01:00.1: Failed to add - passthrough or MSI/MSI-X might fail!Quoted 3 lines
IIUC, This is because
xen_add_device()failed, and it seems to be related to some hyper call not implemented. Not sure what is "cmd=15". Any ideas?There is 2 commands not implemented in the log:
* cmd 15:PHYSDEVOP_manage_pci_add
* cmd 25:PHYSDEVOP_pci_device_addQuoted 5 lines
Linux fallbacks on the former because the later is not implemented.
AFAICT,
PHYSDEVOP_manage_pci_addshould not be implemented for ARM because it doesn't support segment. I suspect that it's kept for legacy on older Xen x86. Maybe Jan or Andrew have more input on this?Quoted 6 lines
It needs to be kept for backwards compatibility in x86.
All new code should use
PHYSDEVOP_pci_device_add.~Andrew
Ok, now that I look at the
arch/arm/physdev.c, I don't think the code for supporting any of thePHYSDEVOP_xxxis there. That's probably why Xen complains. In contrast,arch/x86/physdev.chas mostPHYSDEVOP_xxxalready supported.My question is, are we supposed to be adding code to put the support in here?
Thanks,
Quoted 7 lines
Quoted 1 line
Suravee.
My guess is yes, and that would mean we need to enable building
drivers/pci.cwhen building arm code, which then open up a can of worms with re-factoring MSI support code from x86 and etc.Suravee
Actually, that seems to be more related to the PCI pass-through devices. Isn't the Cavium guys already done that work to support their PCI device pass-through?
Anyways, at this point, I am able to generated Dom0 device tree with correct v2m node, and I can see Dom0 gicv2m driver probing and initializing correctly as it would on bare-metal.
# Snippet from /sys/firmware/fdt showing dom0 GIC node
interrupt-controller {
compatible = "arm,gic-400", "arm,cortex-a15-gic";
#interrupt-cells = <0x3>;
interrupt-controller;
reg = <0x0 0xe1110000 0x0 0x1000 0x0 0xe112f000 0x0 0x2000>;
phandle = <0x1>;
#address-cells = <0x2>;
#size-cells = <0x2>;
ranges = <0x0 0x0 0x0 0xe1100000 0x0 0x100000>;
v2m {
compatible = "arm,gic-v2m-frame";
msi-controller;
arm,msi-base-spi = <0x40>;
arm,msi-num-spis = <0x100>;
phandle = <0x5>;
reg = <0x0 0x80000 0x0 0x1000>;
};
};linux:~ # dmesg | grep v2m
[ 0.000000] GICv2m: Overriding V2M MSI_TYPER (base:64, num:256) [ 0.000000] GICv2m: Node v2m: range[0xe1180000:0xe1180fff], SPI[64:320]
So, during setting up v2m in hypervisor, I also call route_irq_to_guest() for the all SPIs used for MSI (i.e. 64-320 on Seattle), which will force the MSIs to Dom0. However, we would need to figure out how to detach and re-route certain interrupts to a specific DomU in case of passing through PCI devices in the future.
So, here's what I got:
linux:~ # cat /proc/interrupts
CPU0 CPU1 CPU2 CPU3 CPU4 CPU5
0: 0 0 0 0 0 0
xen-dyn-event xenbus
3: 19872 19872 19870 19861 19866 20034
GIC 27 arch_timer
4: 91 1 1 1 1 1
GIC 31 events
13: 2397 0 0 0 0 0
GIC 387 e0300000.sata
14: 0 0 0 0 0 0
GIC 389 e1000000.i2c
15: 0 0 0 0 0 0
GIC 362 pl022
16: 0 0 0 0 0 0
GIC 361 pl022
46: 90 0 0 0 0 0
xen-percpu-virq hvc_console
47: 200 0 0 0 0 0
MSI 524288 enp1s0f0-TxRx-0
48: 56 0 0 0 0 0
MSI 524289 enp1s0f0-TxRx-1
49: 47 0 0 0 0 0
MSI 524290 enp1s0f0-TxRx-2
50: 45 0 0 0 0 0
MSI 524291 enp1s0f0-TxRx-3
51: 53 0 0 0 0 0
MSI 524292 enp1s0f0-TxRx-4
52: 48 0 0 0 0 0
MSI 524293 enp1s0f0-TxRx-5
53: 4 0 0 0 0 0
MSI 524294 enp1s0f0linux:~ # ifconfig enp1s0f0
enp1s0f0 Link encap:Ethernet HWaddr 00:1B:21:55:7F:14
inet addr:10.236.19.5 Bcast:10.236.19.255 Mask:255.255.254.0 inet6 addr: fe80::21b:21ff:fe55:7f14/64 Scope:Link UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 RX packets:333 errors:0 dropped:0 overruns:0 frame:0 TX packets:111 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000
RX bytes:33406 (32.6 Kb) TX bytes:14957 (14.6 Kb)
linux:~ # lspci -vvv -s 1:0.0
01:00.0 Ethernet controller: Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network Connection (rev 01)
Subsystem: Intel Corporation Ethernet Server Adapter X520-2
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
ParErr- Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0
Interrupt: pin A routed to IRQ 44
Region 0: Memory at bfe80000 (64-bit, prefetchable) [size=512K]
Region 2: I/O ports at 0020 [size=32]
Region 4: Memory at bff04000 (64-bit, prefetchable) [size=16K]
Capabilities: [40] Power Management version 3
Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mAPME(D0+,D1-,D2-,D3hot+,D3cold-)
Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=1 PME-
Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
Address: 0000000000000000 Data: 0000
Masking: 00000000 Pending: 00000000
Capabilities: [70] MSI-X: Enable+ Count=64 Masked-
Vector table: BAR=4 offset=00000000
PBA: BAR=4 offset=00002000....
And there you have it.... GICv2m MSI(-X) supports in Dom 0 for Seattle ;) Thanks to Ian's PCI patch, which makes porting much simpler.
Next, I'll clean up the code and send out Xen patch for review. I'll also push Linux changes (for adding ARM64 PCI Generic host controller supports and MSI IRQ domain from Marc) into my Linux tree on Github. Then you could give it a try on your Seattle box.
Thanks,
Suravee
Hi Suravee,
Quoted 4 lines · Suravee Suthikulanit
Actually, that seems to be more related to the PCI pass-through devices. Isn't the Cavium guys already done that work to support their PCI device pass-through?
They were working on it, but so far there is no patch series on the ML. It would be nice to come with a common solution (i.e between GICv2m and GICv3 ITS) for MSI.
Quoted 26 lines
Anyways, at this point, I am able to generated Dom0 device tree with correct v2m node, and I can see Dom0 gicv2m driver probing and initializing correctly as it would on bare-metal.
# Snippet from
/sys/firmware/fdtshowing dom0 GIC nodeinterrupt-controller { compatible = "arm,gic-400", "arm,cortex-a15-gic"; #interrupt-cells = <0x3>; interrupt-controller; reg = <0x0 0xe1110000 0x0 0x1000 0x0 0xe112f000 0x0 0x2000>; phandle = <0x1>; #address-cells = <0x2>; #size-cells = <0x2>; ranges = <0x0 0x0 0x0 0xe1100000 0x0 0x100000>; v2m { compatible = "arm,gic-v2m-frame"; msi-controller; arm,msi-base-spi = <0x40>; arm,msi-num-spis = <0x100>; phandle = <0x5>; reg = <0x0 0x80000 0x0 0x1000>; }; };linux:~ # dmesg | grep v2m
Quoted 8 lines
[ 0.000000] GICv2m: Overriding V2M MSI_TYPER (base:64, num:256) [ 0.000000] GICv2m: Node v2m: range[0xe1180000:0xe1180fff], SPI[64:320]So, during setting up v2m in hypervisor, I also call
route_irq_to_guest()for the all SPIs used for MSI (i.e. 64-320 on Seattle), which will force the MSIs to Dom0. However, we would need to figure out how to detach and re-route certain interrupts to a specific DomU in case of passing through PCI devices in the future.
Who decide to assign the MSI n to the SPI x? DOM0 or Xen?
Wouldn't it be possible to route the SPI dynamically when the domain decide to use the MSI n? We would need to implement PHYSDEVOP_map_pirq for MSI.
[..]
Quoted 7 lines
And there you have it.... GICv2m MSI(-X) supports in Dom 0 for Seattle ;) Thanks to Ian's PCI patch, which makes porting much simpler.
Next, I'll clean up the code and send out Xen patch for review. I'll also push Linux changes (for adding ARM64 PCI Generic host controller supports and MSI IRQ domain from Marc) into my Linux tree on Github. Then you could give it a try on your Seattle box.
Congrats!
Signature · 1 line
Julien Grall
Quoted 11 lines · Julien Grall
Hi Suravee,
Quoted 4 lines · Suravee Suthikulanit
Actually, that seems to be more related to the PCI pass-through devices. Isn't the Cavium guys already done that work to support their PCI device pass-through?
They were working on it, but so far there is no patch series on the ML. It would be nice to come with a common solution (i.e between GICv2m and GICv3 ITS) for MSI.
Agree, although for supporting Dom0 MSI, I don't see anything that could be shared since this would totally be two separate drivers/interfaces.
Quoted 31 lines
Quoted 31 lines
Anyways, at this point, I am able to generated Dom0 device tree with correct v2m node, and I can see Dom0 gicv2m driver probing and initializing correctly as it would on bare-metal.
# Snippet from
/sys/firmware/fdtshowing dom0 GIC nodeinterrupt-controller { compatible = "arm,gic-400", "arm,cortex-a15-gic"; #interrupt-cells = <0x3>; interrupt-controller; reg = <0x0 0xe1110000 0x0 0x1000 0x0 0xe112f000 0x0 0x2000>; phandle = <0x1>; #address-cells = <0x2>; #size-cells = <0x2>; ranges = <0x0 0x0 0x0 0xe1100000 0x0 0x100000>; v2m { compatible = "arm,gic-v2m-frame"; msi-controller; arm,msi-base-spi = <0x40>; arm,msi-num-spis = <0x100>; phandle = <0x5>; reg = <0x0 0x80000 0x0 0x1000>; }; };linux:~ # dmesg | grep v2m
[ 0.000000] GICv2m: Overriding V2M MSI_TYPER (base:64, num:256) [ 0.000000] GICv2m: Node v2m: range[0xe1180000:0xe1180fff],SPI[64:320]
Quoted 7 lines
Quoted 5 lines
So, during setting up v2m in hypervisor, I also call
route_irq_to_guest()for the all SPIs used for MSI (i.e. 64-320 on Seattle), which will force the MSIs to Dom0. However, we would need to figure out how to detach and re-route certain interrupts to a specific DomU in case of passing through PCI devices in the future.Who decide to assign the MSI n to the SPI x? DOM0 or Xen?
For v2m, each MSI is tied to a specific SPI. The range of SPIs is specified in the GIC V2M_MSI_TYPER register. In Xen, we need to make sure that these are routed to Dom0 initially since Dom0 GICv2m driver is the one handling all MSI assignments.
Quoted 3 lines
Wouldn't it be possible to route the SPI dynamically when the domain decide to use the MSI n? We would need to implement
PHYSDEVOP_map_pirqfor MSI.
Enabling MSI is done by each end-point PCI device drivers in the guest. In Linux, this would mean that when the driver tries to allocate an MSI interrupt, it would need to communicate back to Xen (possibly via hypercall as you pointed out) to get the next available SPI. It is not necessary for now. I am planning to revisit this when we try to implement pass-through support. Lemme know if you think this should be handled differently.
Cheers,
Suravee
Quoted 54 lines · Suravee Suthikulanit
Quoted 11 lines · Julien Grall
Hi Suravee,
Quoted 4 lines · Suravee Suthikulanit
Actually, that seems to be more related to the PCI pass-through devices. Isn't the Cavium guys already done that work to support their PCI device pass-through?
They were working on it, but so far there is no patch series on the ML. It would be nice to come with a common solution (i.e between GICv2m and GICv3 ITS) for MSI.
Agree, although for supporting Dom0 MSI, I don't see anything that could be shared since this would totally be two separate
drivers/interfaces.Quoted 31 lines
Quoted 31 lines
Anyways, at this point, I am able to generated Dom0 device tree with correct v2m node, and I can see Dom0 gicv2m driver probing and initializing correctly as it would on bare-metal.
# Snippet from
/sys/firmware/fdtshowing dom0 GIC nodeinterrupt-controller { compatible = "arm,gic-400", "arm,cortex-a15-gic"; #interrupt-cells = <0x3>; interrupt-controller; reg = <0x0 0xe1110000 0x0 0x1000 0x0 0xe112f000 0x0 0x2000>; phandle = <0x1>; #address-cells = <0x2>; #size-cells = <0x2>; ranges = <0x0 0x0 0x0 0xe1100000 0x0 0x100000>; v2m { compatible = "arm,gic-v2m-frame"; msi-controller; arm,msi-base-spi = <0x40>; arm,msi-num-spis = <0x100>; phandle = <0x5>; reg = <0x0 0x80000 0x0 0x1000>; }; };linux:~ # dmesg | grep v2m
[ 0.000000] GICv2m: Overriding V2M MSI_TYPER (base:64, num:256) [ 0.000000] GICv2m: Node v2m: range[0xe1180000:0xe1180fff],SPI[64:320]
Quoted 7 lines
Quoted 5 lines
So, during setting up v2m in hypervisor, I also call
route_irq_to_guest()for the all SPIs used for MSI (i.e. 64-320 on Seattle), which will force the MSIs to Dom0. However, we would need to figure out how to detach and re-route certain interrupts to a specific DomU in case of passing through PCI devices in the future.Who decide to assign the MSI n to the SPI x? DOM0 or Xen?
Quoted 8 lines
For v2m, each MSI is tied to a specific SPI. The range of SPIs is specified in the GIC
V2M_MSI_TYPERregister. In Xen, we need to make sure that these are routed to Dom0 initially since Dom0 GICv2m driver is the one handling all MSI assignments.Quoted 3 lines
Wouldn't it be possible to route the SPI dynamically when the domain decide to use the MSI n? We would need to implement
PHYSDEVOP_map_pirqfor MSI.Enabling MSI is done by each end-point PCI device drivers in the guest. In Linux, this would mean that when the driver tries to allocate an MSI interrupt, it would need to communicate back to Xen (possibly via hypercall as you pointed out) to get the next available SPI. It is not necessary for now. I am planning to revisit this when we try to implement pass-through support. Lemme know if you think this should be handled differently.
For cavium we are doing it differently. In the frontend bus we have added its. I am planning to send RFC patches soon.
Quoted 9 lines
Cheers,
Suravee
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 6 lines
Quoted 1 line
Quoted 1 line · On 18.02.15 at 21:13, <[email protected]> wrote:
Quoted 3 lines · Julien Grall
Wouldn't it be possible to route the SPI dynamically when the domain decide to use the MSI n? We would need to implement
PHYSDEVOP_map_pirqfor MSI.
Quoted 7 lines
Enabling MSI is done by each end-point PCI device drivers in the guest. In Linux, this would mean that when the driver tries to allocate an MSI interrupt, it would need to communicate back to Xen (possibly via hypercall as you pointed out) to get the next available SPI. It is not necessary for now. I am planning to revisit this when we try to implement pass-through support. Lemme know if you think this should be handled differently.
I think guest MSI setup should work at least similarly (if identically is not possible) to x86, i.e. via the vpci interface.
Jan
Quoted 12 lines · Jan Beulich
Quoted 5 lines
Quoted 1 line
Quoted 1 line · On 18.02.15 at 21:13, <[email protected]> wrote:
Quoted 3 lines · Julien Grall
Wouldn't it be possible to route the SPI dynamically when the domain decide to use the MSI n? We would need to implement
PHYSDEVOP_map_pirqfor MSI.Quoted 7 lines
Enabling MSI is done by each end-point PCI device drivers in the guest. In Linux, this would mean that when the driver tries to allocate an MSI interrupt, it would need to communicate back to Xen (possibly via hypercall as you pointed out) to get the next available SPI. It is not necessary for now. I am planning to revisit this when we try to implement pass-through support. Lemme know if you think this should be handled differently.
Quoted 2 lines
I think guest MSI setup should work at least similarly (if identically is not possible) to x86, i.e. via the vpci interface.
For cavium thunder pci passthrough, we have avoided that. Guest driver directly traps into hypervisor. No front end backend communication for MSI setup. The front-back communication is limited to reading PCI configuration space.
Quoted 7 lines
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Hi Suravee,
Quoted 6 lines · Suravee Suthikulanit
Quoted 1 line
Who decide to assign the MSI n to the SPI x? DOM0 or Xen?
For v2m, each MSI is tied to a specific SPI. The range of SPIs is specified in the GIC
V2M_MSI_TYPERregister. In Xen, we need to make sure that these are routed to Dom0 initially since Dom0 GICv2m driver is the one handling all MSI assignments.
I guess you emulate the GICv2m frame? If so, can't you trap access to the frame in order to configure the SPI?
Quoted 11 lines
Quoted 3 lines
Wouldn't it be possible to route the SPI dynamically when the domain decide to use the MSI n? We would need to implement
PHYSDEVOP_map_pirqfor MSI.Enabling MSI is done by each end-point PCI device drivers in the guest. In Linux, this would mean that when the driver tries to allocate an MSI interrupt, it would need to communicate back to Xen (possibly via hypercall as you pointed out) to get the next available SPI. It is not necessary for now. I am planning to revisit this when we try to implement pass-through support. Lemme know if you think this should be handled differently.
The pass-through support will come quickly. I think it would be better to support this case now, mainly if we have to add some code in Linux. It may take some times to upstream it.
Although, I haven't really though about how to do it in Xen.
Regards,
Signature · 1 line
Julien Grall
Quoted 7 lines
Quoted 1 line
Quoted 1 line · On 18.02.15 at 01:31, <[email protected]> wrote:
Quoted 5 lines · Suravee Suthikulanit
Ok, now that I look at the
arch/arm/physdev.c, I don't think the code for supporting any of thePHYSDEVOP_xxxis there. That's probably why Xen complains. In contrast,arch/x86/physdev.chas mostPHYSDEVOP_xxxalready supported.
Quoted 6 lines
Quoted 2 lines
My question is, are we supposed to be adding code to put the support in here?
My guess is yes, and that would mean we need to enable building
drivers/pci.cwhen building arm code, which then open up a can of worms with re-factoring MSI support code from x86 and etc.
As you said later, that's pass-through related, i.e. not immediately needed. Right now - to get rid of those messages - you'd have to simply stub out the respective physdevop-s as much as is necessary to no longer emit log messages and no longer return an error on valid input.
Jan
Quoted 22 lines · Suravee Suthikulpanit
Quoted 13 lines · Ian Campbell
This message has been archived. View the original item
<http://ausev2.amd.com/EnterpriseVault/ViewMessage.asp?VaultId=18C5C586ACEF6BE42BD5DBA644C96C94F1110000amdvault.amd.com&SavesetId=201411267046116~201410241000000000~Z~F141820FA2B2A99F4BA9EAD48EEB5211>
This series adds parsing of the DT ranges and interrupt-map properties for PCI devices, these contain the MMIOs and IRQs used by children on the bus. This replaces the specific mapping on xgene which should also mean that it works on xgene devices other than mustang which use a different PCI root controller (the xgene has several, we only map the first).I've tested on Mustang and on a FastModel with virtio-pci based rootfs.
This is *not* for 4.5.
Quoted 7 lines
Ian.
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
Quoted 5 lines
Ian,
I have tested this on Seattle w/ the ToT Xen-4.6 unstable branch (commit
001324547356af86875fad5003f679571a6b8f1c), and I have PCI working in Dom0. Are you planning on committing this series at some point?
At some point, yes.
I originally had some doubts that this was the correct approach, but I
a) can't remember what they were and b) think I've convinced myself this is the right way to go afterall...
Since this is now in your way I'll bump revisiting it up my stack a bit.
Ian.