| Message ID | 20190808231242.26424-3-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/6] xen/arm: introduce handle_interrupts | expand |
Stefano Stabellini writes: > Scan the user provided dtb fragment at boot. For each device node, map > memory to guests, and route interrupts and setup the iommu. > > The iommu is setup by passing the node of the device to assign on the > host device tree. The path is specified in the device tree fragment as > the "xen,path" string property. > > The memory region to remap is specified by the "xen,reg" property. > (Perhaps it might be possible to use "range" instead of "xen,regs". This > is something to investigate.) > > The interrupts are taken from the host device tree corresponding node. > To map the interrupt call handle_interrupts, which is shared with the > existing dom0 path. > > Add a interrupt-parent property automatically to the guest device tree > when the interrupt-parent should be the GIC. Copy over the interrupt > property from the host device tree node. > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > --- > Changes in v3: > - improve commit message > - remove superfluous cast > - merge code with the copy code > - add interrup-parent > - demove depth > 2 check > - reuse code from handle_interrupts > - copy interrupts from host dt > > Changes in v2: > - rename "path" to "xen,path" > - grammar fix > - use gaddr_to_gfn and maddr_to_mfn > - remove depth <= 2 limitation in scanning the dtb fragment > - introduce and parse xen,reg > - code style > - support more than one interrupt per device > - specify only the GIC is supported > --- > xen/arch/arm/domain_build.c | 66 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 70bcdc449d..0057a509d1 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -1712,6 +1712,9 @@ static int __init handle_properties(struct domain *d, void *fdt, const void *pfd > { > int propoff, nameoff, r; > const struct fdt_property *prop; > + struct dt_device_node *node; > + const __be32 *cell; > + int i, len; > > for ( propoff = fdt_first_property_offset(pfdt, nodeoff); > propoff >= 0; > @@ -1726,6 +1729,69 @@ static int __init handle_properties(struct domain *d, void *fdt, const void *pfd > prop->data, fdt32_to_cpu(prop->len)); > if ( r ) > return r; > + > + if ( strcmp("xen,reg", fdt_string(pfdt, nameoff)) == 0 ) IIRC, in your other patch series you used dt_*_cmp function there. Should it be dt_prop_cmp() in this case? > + { > + paddr_t mstart, size, gstart; > + cell = (const __be32 *)prop->data; > + len = fdt32_to_cpu(prop->len) / > + ((address_cells*2 + size_cells) * sizeof (u32)); Coding style: address_cells * 2. Also "sizeof(u32)". > + > + for ( i = 0; i < len; i++ ) > + { > + mstart = dt_next_cell(address_cells, &cell); > + size = dt_next_cell(size_cells, &cell); > + gstart = dt_next_cell(address_cells, &cell); > + > + r = guest_physmap_add_entry(d, gaddr_to_gfn(gstart), > + maddr_to_mfn(mstart), > + get_order_from_bytes(size), > + p2m_mmio_direct_dev); > + if ( r < 0 ) > + { > + dprintk(XENLOG_ERR, > + "Failed to map %"PRIpaddr" to the guest at%"PRIpaddr"\n", > + mstart, gstart); > + return -EFAULT; > + } > + } > + } > + > + if ( strcmp("xen,path", fdt_string(pfdt, nameoff)) == 0 ) The same question about dt_prop_cmp. > + { > + node = dt_find_node_by_path(prop->data); > + if ( node != NULL ) > + r = iommu_assign_dt_device(d, node); > + else > + { > + dprintk(XENLOG_ERR, "Couldn't find node %s in host_dt!\n", > + (char *)prop->data); > + return -EINVAL; > + } > + > + r = handle_interrupts(d, node, true); > + if ( r < 0 ) > + return r; > + if ( r > 0 ) nitpicking: how about if ( r == 0 ) continue; ? This will save one level of nesting. Also, now I can see why handle_interrupts() returns either <0, 0 or 1. But this was not clear in the patch #1. Additionally, need_mapping is always true in this case. So actually you can check only for (r < 0) > + { > + unsigned int intlen; > + const u32* intspec; > + > + /* generate interrupt-parent to point to the virtual GIC */ > + r = fdt_property_u32(fdt, "interrupt-parent", GUEST_PHANDLE_GIC); > + if ( r ) > + return r; > + > + /* copy interrupts/interrupts-extended from the host DT node */ I can't see "interrupts-extended" handling in the code below. > + intspec = dt_get_property(node, "interrupts", &intlen); > + if ( intspec == NULL ) > + return -EFAULT; > + > + r = fdt_property(fdt, "interrupts", intspec, intlen); > + if ( r ) > + return r; > + } > + } > } > > /* FDT_ERR_NOTFOUND => There is no more properties for this node */ -- Volodymyr Babchuk at EPAM
Hi Stefano, On 8/9/19 12:12 AM, Stefano Stabellini wrote: > Scan the user provided dtb fragment at boot. For each device node, map > memory to guests, and route interrupts and setup the iommu. > > The iommu is setup by passing the node of the device to assign on the > host device tree. The path is specified in the device tree fragment as > the "xen,path" string property. > > The memory region to remap is specified by the "xen,reg" property. > (Perhaps it might be possible to use "range" instead of "xen,regs". This s/xen,regs/xen,reg/ But I don't see how you could use range here... This is for translation address between two address-space. > is something to investigate.) > > The interrupts are taken from the host device tree corresponding node. > To map the interrupt call handle_interrupts, which is shared with the > existing dom0 path. > > Add a interrupt-parent property automatically to the guest device tree > when the interrupt-parent should be the GIC. Copy over the interrupt > property from the host device tree node. > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > --- > Changes in v3: > - improve commit message > - remove superfluous cast > - merge code with the copy code > - add interrup-parent > - demove depth > 2 check > - reuse code from handle_interrupts > - copy interrupts from host dt > > Changes in v2: > - rename "path" to "xen,path" > - grammar fix > - use gaddr_to_gfn and maddr_to_mfn > - remove depth <= 2 limitation in scanning the dtb fragment > - introduce and parse xen,reg > - code style > - support more than one interrupt per device > - specify only the GIC is supported > --- > xen/arch/arm/domain_build.c | 66 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 66 insertions(+) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 70bcdc449d..0057a509d1 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -1712,6 +1712,9 @@ static int __init handle_properties(struct domain *d, void *fdt, const void *pfd > { > int propoff, nameoff, r; > const struct fdt_property *prop; > + struct dt_device_node *node; > + const __be32 *cell; > + int i, len; Again any variable that can't be negative should be unsigned. > > for ( propoff = fdt_first_property_offset(pfdt, nodeoff); > propoff >= 0; > @@ -1726,6 +1729,69 @@ static int __init handle_properties(struct domain *d, void *fdt, const void *pfd > prop->data, fdt32_to_cpu(prop->len)); > if ( r ) > return r; > + > + if ( strcmp("xen,reg", fdt_string(pfdt, nameoff)) == 0 ) This should be dt_prop_cmp. But this property should not be part of the guest DTB afterwards. Lastly, a bit of documentation would be nice. > + { > + paddr_t mstart, size, gstart; > + cell = (const __be32 *)prop->data; > + len = fdt32_to_cpu(prop->len) / > + ((address_cells*2 + size_cells) * sizeof (u32)); > + > + for ( i = 0; i < len; i++ ) > + { > + mstart = dt_next_cell(address_cells, &cell); > + size = dt_next_cell(size_cells, &cell); Please use device_get_reg here. > + gstart = dt_next_cell(address_cells, &cell); > + > + r = guest_physmap_add_entry(d, gaddr_to_gfn(gstart), > + maddr_to_mfn(mstart), > + get_order_from_bytes(size), > + p2m_mmio_direct_dev); The indentation is wrong. > + if ( r < 0 ) > + { > + dprintk(XENLOG_ERR, > + "Failed to map %"PRIpaddr" to the guest at%"PRIpaddr"\n", > + mstart, gstart); > + return -EFAULT; > + } > + } > + } > + > + if ( strcmp("xen,path", fdt_string(pfdt, nameoff)) == 0 ) Same remark as for "xen,reg". > + { > + node = dt_find_node_by_path(prop->data); > + if ( node != NULL ) > + r = iommu_assign_dt_device(d, node); > + else > + { > + dprintk(XENLOG_ERR, "Couldn't find node %s in host_dt!\n", > + (char *)prop->data); > + return -EINVAL; > + } > + > + r = handle_interrupts(d, node, true); > + if ( r < 0 ) > + return r; > + if ( r > 0 ) > + { > + unsigned int intlen; > + const u32* intspec; I don't think the code below is correct. For a first, your implementation of handle_interrupts is not right (see my comments on the patch where you added the function). Then you may be here even if no interrupts property. So the code below will fail to copy those nodes. > + > + /* generate interrupt-parent to point to the virtual GIC */ > + r = fdt_property_u32(fdt, "interrupt-parent", GUEST_PHANDLE_GIC); From your implementation of handle_interrupts(), there are no promise you would be here with just a GIC interrupts. You may also need to copy any interrupts property for node that does not belong to the GIC. > + if ( r ) > + return r; > + > + /* copy interrupts/interrupts-extended from the host DT node */ > + intspec = dt_get_property(node, "interrupts", &intlen); > + if ( intspec == NULL ) > + return -EFAULT; You don't handle interrupts-extended nor interrupt-mapping. > + > + r = fdt_property(fdt, "interrupts", intspec, intlen); > + if ( r ) > + return r; > + } > + } > } > > /* FDT_ERR_NOTFOUND => There is no more properties for this node */ > Cheers,
On Fri, 9 Aug 2019, Julien Grall wrote: > Hi Stefano, > > On 8/9/19 12:12 AM, Stefano Stabellini wrote: > > Scan the user provided dtb fragment at boot. For each device node, map > > memory to guests, and route interrupts and setup the iommu. > > > > The iommu is setup by passing the node of the device to assign on the > > host device tree. The path is specified in the device tree fragment as > > the "xen,path" string property. > > > > The memory region to remap is specified by the "xen,reg" property. > > (Perhaps it might be possible to use "range" instead of "xen,regs". This > > s/xen,regs/xen,reg/ > > But I don't see how you could use range here... This is for translation > address between two address-space. I'll remove the comment > > is something to investigate.) > > > > The interrupts are taken from the host device tree corresponding node. > > To map the interrupt call handle_interrupts, which is shared with the > > existing dom0 path. > > > > Add a interrupt-parent property automatically to the guest device tree > > when the interrupt-parent should be the GIC. Copy over the interrupt > > property from the host device tree node. > > > > Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> > > --- > > Changes in v3: > > - improve commit message > > - remove superfluous cast > > - merge code with the copy code > > - add interrup-parent > > - demove depth > 2 check > > - reuse code from handle_interrupts > > - copy interrupts from host dt > > > > Changes in v2: > > - rename "path" to "xen,path" > > - grammar fix > > - use gaddr_to_gfn and maddr_to_mfn > > - remove depth <= 2 limitation in scanning the dtb fragment > > - introduce and parse xen,reg > > - code style > > - support more than one interrupt per device > > - specify only the GIC is supported > > --- > > xen/arch/arm/domain_build.c | 66 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 66 insertions(+) > > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > > index 70bcdc449d..0057a509d1 100644 > > --- a/xen/arch/arm/domain_build.c > > +++ b/xen/arch/arm/domain_build.c > > @@ -1712,6 +1712,9 @@ static int __init handle_properties(struct domain *d, > > void *fdt, const void *pfd > > { > > int propoff, nameoff, r; > > const struct fdt_property *prop; > > + struct dt_device_node *node; > > + const __be32 *cell; > > + int i, len; > > Again any variable that can't be negative should be unsigned. OK > > for ( propoff = fdt_first_property_offset(pfdt, nodeoff); > > propoff >= 0; > > @@ -1726,6 +1729,69 @@ static int __init handle_properties(struct domain *d, > > void *fdt, const void *pfd > > prop->data, fdt32_to_cpu(prop->len)); > > if ( r ) > > return r; > > + > > + if ( strcmp("xen,reg", fdt_string(pfdt, nameoff)) == 0 ) > > This should be dt_prop_cmp. But this property should not be part of the guest > DTB afterwards. Good point! > Lastly, a bit of documentation would be nice. Do you mean an in-code comment, or a document somewhere? > > + { > > + paddr_t mstart, size, gstart; > > + cell = (const __be32 *)prop->data; > > + len = fdt32_to_cpu(prop->len) / > > + ((address_cells*2 + size_cells) * sizeof (u32)); > > + > > + for ( i = 0; i < len; i++ ) > > + { > > + mstart = dt_next_cell(address_cells, &cell); > > + size = dt_next_cell(size_cells, &cell); > > Please use device_get_reg here. OK (device_tree_get_reg) > > + gstart = dt_next_cell(address_cells, &cell); > > + > > + r = guest_physmap_add_entry(d, gaddr_to_gfn(gstart), > > + maddr_to_mfn(mstart), > > + get_order_from_bytes(size), > > + p2m_mmio_direct_dev); > > The indentation is wrong. I'll fix > > + if ( r < 0 ) > > + { > > + dprintk(XENLOG_ERR, > > + "Failed to map %"PRIpaddr" to the guest > > at%"PRIpaddr"\n", > > + mstart, gstart); > > + return -EFAULT; > > + } > > + } > > + } > > + > > + if ( strcmp("xen,path", fdt_string(pfdt, nameoff)) == 0 ) > > Same remark as for "xen,reg". OK > > + { > > + node = dt_find_node_by_path(prop->data); > > + if ( node != NULL ) > > + r = iommu_assign_dt_device(d, node); > > + else > > + { > > + dprintk(XENLOG_ERR, "Couldn't find node %s in host_dt!\n", > > + (char *)prop->data); > > + return -EINVAL; > > + } > > + > > + r = handle_interrupts(d, node, true); > > + if ( r < 0 ) > > + return r; > > + if ( r > 0 ) > > + { > > + unsigned int intlen; > > + const u32* intspec; > > I don't think the code below is correct. For a first, your implementation of > handle_interrupts is not right (see my comments on the patch where you added > the function). Then you may be here even if no interrupts property. So the > code below will fail to copy those nodes. I don't get the last sentence: "Then you may be here even if no interrupts property. So the code below will fail to copy those nodes." Why the code would fail to copy those nodes? Which nodes? > > + > > + /* generate interrupt-parent to point to the virtual GIC */ > > + r = fdt_property_u32(fdt, "interrupt-parent", > > GUEST_PHANDLE_GIC); > From your implementation of handle_interrupts(), there are no promise you > would be here with just a GIC interrupts. You may also need to copy any > interrupts property for node that does not belong to the GIC. Let's say that we have a mix of GIC and non-GIC interrupts at the node with xen,path and xen,reg. Let's also say that we are in a regular interrupt-parent/interrupts configuration (no interrupts-extended, see below). Is it possible without interrupts-extended? How would it look like? > > + if ( r ) > > + return r; > > + > > + /* copy interrupts/interrupts-extended from the host DT > > node */ > > + intspec = dt_get_property(node, "interrupts", &intlen); > > + if ( intspec == NULL ) > > + return -EFAULT; > > You don't handle interrupts-extended nor interrupt-mapping. I was wondering what to do about that. For now, I added a note in the last patch of the series, the one adding info to the doc. Already in this v3 series: "For GIC interrupts, only the interrupts property is currently supported, not the newer interrupts-extended property." > > + > > + r = fdt_property(fdt, "interrupts", intspec, intlen); > > + if ( r ) > > + return r; > > + } > > + } > > } > > /* FDT_ERR_NOTFOUND => There is no more properties for this node */
Hi Stefano, On 20/08/2019 00:20, Stefano Stabellini wrote: > On Fri, 9 Aug 2019, Julien Grall wrote: >> Hi Stefano, >> >> On 8/9/19 12:12 AM, Stefano Stabellini wrote: >>> Scan the user provided dtb fragment at boot. For each device node, map >>> memory to guests, and route interrupts and setup the iommu. >>> >>> The iommu is setup by passing the node of the device to assign on the >>> host device tree. The path is specified in the device tree fragment as >>> the "xen,path" string property. >>> >>> The memory region to remap is specified by the "xen,reg" property. >>> (Perhaps it might be possible to use "range" instead of "xen,regs". This >> >> s/xen,regs/xen,reg/ >> >> But I don't see how you could use range here... This is for translation >> address between two address-space. > > I'll remove the comment > > >>> is something to investigate.) >>> >>> The interrupts are taken from the host device tree corresponding node. >>> To map the interrupt call handle_interrupts, which is shared with the >>> existing dom0 path. >>> >>> Add a interrupt-parent property automatically to the guest device tree >>> when the interrupt-parent should be the GIC. Copy over the interrupt >>> property from the host device tree node. >>> >>> Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> >>> --- >>> Changes in v3: >>> - improve commit message >>> - remove superfluous cast >>> - merge code with the copy code >>> - add interrup-parent >>> - demove depth > 2 check >>> - reuse code from handle_interrupts >>> - copy interrupts from host dt >>> >>> Changes in v2: >>> - rename "path" to "xen,path" >>> - grammar fix >>> - use gaddr_to_gfn and maddr_to_mfn >>> - remove depth <= 2 limitation in scanning the dtb fragment >>> - introduce and parse xen,reg >>> - code style >>> - support more than one interrupt per device >>> - specify only the GIC is supported >>> --- >>> xen/arch/arm/domain_build.c | 66 +++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 66 insertions(+) >>> >>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >>> index 70bcdc449d..0057a509d1 100644 >>> --- a/xen/arch/arm/domain_build.c >>> +++ b/xen/arch/arm/domain_build.c >>> @@ -1712,6 +1712,9 @@ static int __init handle_properties(struct domain *d, >>> void *fdt, const void *pfd >>> { >>> int propoff, nameoff, r; >>> const struct fdt_property *prop; >>> + struct dt_device_node *node; >>> + const __be32 *cell; >>> + int i, len; >> >> Again any variable that can't be negative should be unsigned. > > OK > > >>> for ( propoff = fdt_first_property_offset(pfdt, nodeoff); >>> propoff >= 0; >>> @@ -1726,6 +1729,69 @@ static int __init handle_properties(struct domain *d, >>> void *fdt, const void *pfd >>> prop->data, fdt32_to_cpu(prop->len)); >>> if ( r ) >>> return r; >>> + >>> + if ( strcmp("xen,reg", fdt_string(pfdt, nameoff)) == 0 ) >> >> This should be dt_prop_cmp. But this property should not be part of the guest >> DTB afterwards. > > Good point! > > >> Lastly, a bit of documentation would be nice. > > Do you mean an in-code comment, or a document somewhere? in-code comment. This is not a new request from my side, the code should be either self-explained or contain in-code comment to understand it. Have a look at write_properties() for example. > > >>> + { >>> + paddr_t mstart, size, gstart; >>> + cell = (const __be32 *)prop->data; >>> + len = fdt32_to_cpu(prop->len) / >>> + ((address_cells*2 + size_cells) * sizeof (u32)); >>> + >>> + for ( i = 0; i < len; i++ ) >>> + { >>> + mstart = dt_next_cell(address_cells, &cell); >>> + size = dt_next_cell(size_cells, &cell); >> >> Please use device_get_reg here. > > OK (device_tree_get_reg) > > >>> + gstart = dt_next_cell(address_cells, &cell); >>> + >>> + r = guest_physmap_add_entry(d, gaddr_to_gfn(gstart), >>> + maddr_to_mfn(mstart), >>> + get_order_from_bytes(size), >>> + p2m_mmio_direct_dev); >> >> The indentation is wrong. > > I'll fix > > >>> + if ( r < 0 ) >>> + { >>> + dprintk(XENLOG_ERR, >>> + "Failed to map %"PRIpaddr" to the guest >>> at%"PRIpaddr"\n", >>> + mstart, gstart); >>> + return -EFAULT; >>> + } >>> + } >>> + } >>> + >>> + if ( strcmp("xen,path", fdt_string(pfdt, nameoff)) == 0 ) >> >> Same remark as for "xen,reg". > > OK > > >>> + { >>> + node = dt_find_node_by_path(prop->data); >>> + if ( node != NULL ) >>> + r = iommu_assign_dt_device(d, node); >>> + else >>> + { >>> + dprintk(XENLOG_ERR, "Couldn't find node %s in host_dt!\n", >>> + (char *)prop->data); >>> + return -EINVAL; >>> + } >>> + >>> + r = handle_interrupts(d, node, true); >>> + if ( r < 0 ) >>> + return r; >>> + if ( r > 0 ) >>> + { >>> + unsigned int intlen; >>> + const u32* intspec; >> >> I don't think the code below is correct. For a first, your implementation of >> handle_interrupts is not right (see my comments on the patch where you added >> the function). Then you may be here even if no interrupts property. So the >> code below will fail to copy those nodes. > > I don't get the last sentence: "Then you may be here even if no > interrupts property. So the code below will fail to copy those nodes." > Why the code would fail to copy those nodes? Which nodes? Your assumption here is if handle_interrupts return a strictly positive integer (i.e some interrupts have been routed), then the "interrupts" property exists. But this is definitely not correct, Xen is able to handle "interrupts-extended" so if you have a node using that property and using GIC interrupts. Then you would be executing this path. So if the property "interrupts-extended" is used, then the code below will fail as the property "interrupts" is not present. > > >>> + >>> + /* generate interrupt-parent to point to the virtual GIC */ >>> + r = fdt_property_u32(fdt, "interrupt-parent", >>> GUEST_PHANDLE_GIC); >> From your implementation of handle_interrupts(), there are no promise you >> would be here with just a GIC interrupts. You may also need to copy any >> interrupts property for node that does not belong to the GIC. > > Let's say that we have a mix of GIC and non-GIC interrupts at the node > with xen,path and xen,reg. Let's also say that we are in a regular > interrupt-parent/interrupts configuration (no interrupts-extended, see > below). Is it possible without interrupts-extended? How would it look > like? It is not possible to have a node using a mix of GIC and non-GIC interrupts without the property "interrupts-extended". However, this is not my point here. My point is the presence of xen,path does not mean the node will be using GIC interrupts. So blindly setting "interrupt-parent" to point to the GIC node is wrong. > > >>> + if ( r ) >>> + return r; >>> + >>> + /* copy interrupts/interrupts-extended from the host DT >>> node */ >>> + intspec = dt_get_property(node, "interrupts", &intlen); >>> + if ( intspec == NULL ) >>> + return -EFAULT; >> >> You don't handle interrupts-extended nor interrupt-mapping. > > I was wondering what to do about that. For now, I added a note in the > last patch of the series, the one adding info to the doc. Already in > this v3 series: My point here is your in-code comment does not match the code. But... > > "For GIC interrupts, only the interrupts property is currently > supported, not the newer interrupts-extended property." ... I don't think this series should be merged with "interrupts-extended" not handled. I don't think this is right to request the user to fix its device-tree in order to test passthrough dom0less. The property is also not very difficult to handle as this is copying the property over. Regarding the property "interrupt-map" (and interrupt-map-mask), it is used for bus (i.e PCI...) so I think it may not be critical yet. However, it made me realized that the node you are trying to passthrough may be under a bus. That bus may have an "interrupt-map" so the property "interrupts" will not contain directly GIC interrupts. You can relate this to the way memory region are translated. So this means to copying over "interrupts" does not look to be a good solution. Cheers,
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 70bcdc449d..0057a509d1 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1712,6 +1712,9 @@ static int __init handle_properties(struct domain *d, void *fdt, const void *pfd { int propoff, nameoff, r; const struct fdt_property *prop; + struct dt_device_node *node; + const __be32 *cell; + int i, len; for ( propoff = fdt_first_property_offset(pfdt, nodeoff); propoff >= 0; @@ -1726,6 +1729,69 @@ static int __init handle_properties(struct domain *d, void *fdt, const void *pfd prop->data, fdt32_to_cpu(prop->len)); if ( r ) return r; + + if ( strcmp("xen,reg", fdt_string(pfdt, nameoff)) == 0 ) + { + paddr_t mstart, size, gstart; + cell = (const __be32 *)prop->data; + len = fdt32_to_cpu(prop->len) / + ((address_cells*2 + size_cells) * sizeof (u32)); + + for ( i = 0; i < len; i++ ) + { + mstart = dt_next_cell(address_cells, &cell); + size = dt_next_cell(size_cells, &cell); + gstart = dt_next_cell(address_cells, &cell); + + r = guest_physmap_add_entry(d, gaddr_to_gfn(gstart), + maddr_to_mfn(mstart), + get_order_from_bytes(size), + p2m_mmio_direct_dev); + if ( r < 0 ) + { + dprintk(XENLOG_ERR, + "Failed to map %"PRIpaddr" to the guest at%"PRIpaddr"\n", + mstart, gstart); + return -EFAULT; + } + } + } + + if ( strcmp("xen,path", fdt_string(pfdt, nameoff)) == 0 ) + { + node = dt_find_node_by_path(prop->data); + if ( node != NULL ) + r = iommu_assign_dt_device(d, node); + else + { + dprintk(XENLOG_ERR, "Couldn't find node %s in host_dt!\n", + (char *)prop->data); + return -EINVAL; + } + + r = handle_interrupts(d, node, true); + if ( r < 0 ) + return r; + if ( r > 0 ) + { + unsigned int intlen; + const u32* intspec; + + /* generate interrupt-parent to point to the virtual GIC */ + r = fdt_property_u32(fdt, "interrupt-parent", GUEST_PHANDLE_GIC); + if ( r ) + return r; + + /* copy interrupts/interrupts-extended from the host DT node */ + intspec = dt_get_property(node, "interrupts", &intlen); + if ( intspec == NULL ) + return -EFAULT; + + r = fdt_property(fdt, "interrupts", intspec, intlen); + if ( r ) + return r; + } + } } /* FDT_ERR_NOTFOUND => There is no more properties for this node */
Scan the user provided dtb fragment at boot. For each device node, map memory to guests, and route interrupts and setup the iommu. The iommu is setup by passing the node of the device to assign on the host device tree. The path is specified in the device tree fragment as the "xen,path" string property. The memory region to remap is specified by the "xen,reg" property. (Perhaps it might be possible to use "range" instead of "xen,regs". This is something to investigate.) The interrupts are taken from the host device tree corresponding node. To map the interrupt call handle_interrupts, which is shared with the existing dom0 path. Add a interrupt-parent property automatically to the guest device tree when the interrupt-parent should be the GIC. Copy over the interrupt property from the host device tree node. Signed-off-by: Stefano Stabellini <stefanos@xilinx.com> --- Changes in v3: - improve commit message - remove superfluous cast - merge code with the copy code - add interrup-parent - demove depth > 2 check - reuse code from handle_interrupts - copy interrupts from host dt Changes in v2: - rename "path" to "xen,path" - grammar fix - use gaddr_to_gfn and maddr_to_mfn - remove depth <= 2 limitation in scanning the dtb fragment - introduce and parse xen,reg - code style - support more than one interrupt per device - specify only the GIC is supported --- xen/arch/arm/domain_build.c | 66 +++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+)