[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v3 3/6] xen/arm: assign devices to boot domains
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@xxxxxxxxxx>
---
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,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
