Re: [Xen-devel] [PATCH v3 12/13] xen/arm: Add the property "protected-devices" in the hypervisor node

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-03-11 at 15:49 +0000, Julien Grall wrote:
> DOM0 is using the swiotlb to bounce DMA. With the IOMMU support in Xen,
> protected devices should not use it.
> 
> Only Xen is abled to know if an IOMMU protects the device. The new property
> "protected-devices" is a list of device phandles protected by an IOMMU.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> 
> ---
>     This patch *MUST NOT* be applied until we agreed on a device binding
>     the device tree folks. DOM0 can run safely with swiotlb on protected
>     devices while LVM is not used for guest disk.

LVM works these days I think.

> 
>     Changes in v2:
>         - Patch added
> ---
>  xen/arch/arm/domain_build.c |   51 
> ++++++++++++++++++++++++++++++++++++++-----
>  xen/arch/arm/kernel.h       |    3 +++
>  2 files changed, 48 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 2438aa0..565784a 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -324,19 +324,22 @@ static int make_memory_node(const struct domain *d,
>      return res;
>  }
>  
> -static int make_hypervisor_node(struct domain *d,
> -                                void *fdt, const struct dt_device_node 
> *parent)
> +static int make_hypervisor_node(struct domain *d, struct kernel_info *kinfo,
> +                                const struct dt_device_node *parent)
>  {
>      const char compat[] =
>          "xen,xen-"__stringify(XEN_VERSION)"."__stringify(XEN_SUBVERSION)"\0"
>          "xen,xen";
>      __be32 reg[4];
>      gic_interrupt_t intr;
> -    __be32 *cells;
> +    __be32 *cells, *_cells;
>      int res;
>      int addrcells = dt_n_addr_cells(parent);
>      int sizecells = dt_n_size_cells(parent);
>      paddr_t gnttab_start, gnttab_size;
> +    const struct dt_device_node *dev;
> +    struct hvm_iommu *hd = domain_hvm_iommu(d);
> +    void *fdt = kinfo->fdt;
>  
>      DPRINT("Create hypervisor node\n");
>  
> @@ -384,6 +387,39 @@ static int make_hypervisor_node(struct domain *d,
>      if ( res )
>          return res;
>  
> +    if ( kinfo->num_dev_protected )
> +    {
> +        /* Don't need to take dtdevs_lock here */

Why not? Please explain in the comment.

> +        cells = xmalloc_array(__be32, kinfo->num_dev_protected *
> +                              dt_size_to_cells(sizeof(dt_phandle)));
> +        if ( !cells )
> +            return -FDT_ERR_XEN(ENOMEM);
> +
> +        _cells = cells;

Odd numbers of leading _ are reserved for the compiler IIRC. Even
numbers are reserved for the libc/environment which is why we can get
away with such names in hypervisor context.

But lets just skirt the whole issue and pick a name which doesn't use a
leading _. cells_iter or c or something.

Is there no interface to say "make an fdt_property(name, size)"
returning the data to be filled in?

Ian.


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