Re: [Xen-devel] [PATCH v7b 5/8] xen/arm: assign devices to boot domains

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 02/10/2019 23:48, Stefano Stabellini wrote:
> On Wed, 2 Oct 2019, Stefano Stabellini wrote:
> > On Wed, 2 Oct 2019, Julien Grall wrote:
> > > Hi Stefano,
> > >
> > > On 10/1/19 12:30 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 memory region to remap is specified by the "xen,reg" property.
> > > >
> > > > 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 interrupts are remapped based on the information from the
> > > > corresponding node on the host device tree. Call
> > > > handle_device_interrupts to remap interrupts. Interrupts related device
> > > > tree properties are copied from the device tree fragment, same as all
> > > > the other properties.
> > > >
> > > > Require both xen,reg and xen,path to be present, unless
> > > > xen,force-assign-without-iommu is also set. In that case, tolerate a
> > > > missing xen,path, also tolerate iommu setup failure for the passthrough
> > > > device.
> > > >
> > > > Also set add the new flag XEN_DOMCTL_CDF_iommu so that dom0less domU
> > > > can use the IOMMU if a partial dtb is specified.
> > > The patch looks good a few comments below.
> > Thanks
> >
> >
> > > [...]
> > >
> > > >    xen/arch/arm/domain_build.c | 133 ++++++++++++++++++++++++++++++++++--
> > > >    1 file changed, 129 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > > > index 84b65b8f25..47f9bb31df 100644
> > > > --- a/xen/arch/arm/domain_build.c
> > > > +++ b/xen/arch/arm/domain_build.c
> > > > @@ -1714,6 +1714,88 @@ static int __init make_vpl011_uart_node(struct
> > > > kernel_info *kinfo)
> > > >    }
> > > >    #endif
> > > >    +/*
> > > > + * Scan device tree properties for passthrough specific information.
> > > > + * Returns < 0 on error
> > > > + *         0 on success
> > > > + */
> > > > +static int __init handle_passthrough_prop(struct kernel_info *kinfo,
> > > > +                                          const struct fdt_property
> > > > *xen_reg,
> > > > +                                          const struct fdt_property
> > > > *xen_path,
> > > > +                                          bool xen_force,
> > > > +                                          uint32_t address_cells, uint32_t
> > > > size_cells)
> > > > +{
> > > > +    const __be32 *cell;
> > > > +    unsigned int i, len;
> > > > +    struct dt_device_node *node;
> > > > +    int res;
> > > > +    paddr_t mstart, size, gstart;
> > > > +
> > > > +    /* xen,reg specifies where to map the MMIO region */
> > > > +    cell = (const __be32 *)xen_reg->data;
> > > > +    len = fdt32_to_cpu(xen_reg->len) /
> > > > +          ((address_cells * 2 + size_cells) * sizeof(uint32_t));
> > > Coding style again. I was kind of expecting you configured your editor
> > > properly after the last discussion...
> > Actually I fail to see the coding style issue on this one. Is it still
> > an alignment issue you are talking about?
> Is it because you would like it to look like this?
>
>      len = fdt32_to_cpu(xen_reg->len) / ((address_cells * 2 + size_cells) *
>                                          sizeof(uint32_t));
No, I completely misread the line and thought the division was part of the 
fdt32_to_cpu argument. Apologies for the noise :(.
This is fixable on commit.

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel