|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC 09/19] xen/dts: Add hypercalls to retrieve device node information
On Mon, 16 Jun 2014, Julien Grall wrote:
> DOM0 doesn't provide a generic way to get information about a device tree
> node. If we want to do it in userspace, we will have to duplicate the
> MMIO/IRQ translation from Xen. Therefore, we can let the hypervisor
> doing the job for us and get nearly all the informations.
>
> This new physdev operation will let the toolstack get the IRQ/MMIO regions
> and the compatible string. Most the device node can be described with only
> theses 3 items. If we need to add a specific properties, then we will have
> to implement it in userspace (some idea was to use a configuration file
> describing the additional properties).
>
> The hypercall is divided in 4 parts:
> - GET_INFO: get the numbers of IRQ/MMIO and the size of the
> compatible string;
> - GET_IRQ: get the IRQ by index. If the IRQ is not routable (i.e not
> an SPIs), the errno will be set to -EINVAL;
> - GET_MMIO: get the MMIO range by index. If the base and the size of
> is not page-aligned, the errno will be set to -EINVAL;
> - GET_COMPAT: get the compatible string
>
> All the information will be accessible if the device is not used by Xen
> and protected by an IOMMU.
>
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
>
I know that we talked about this face to face already, but this troubles
me: is it really so uncommon for a device tree node corresponding to a
device to have a key-value pair that is critical for the initialization
of the device?
The ACPI on ARM people are discussing how to introduce these key-value
pairs in ACPI too, so I wonder if we can really dismiss them so easily
for device assignment.
Could Xen discard everything that it knows cannot be passed to the guest
(information on clocks and phandles for example), but return to the
toolstack other harmless key-value pairs, such as device specific
configurations? Maybe we could introduce PHYSDEVOP_DTDEV_GET_KEYVALUE.
> I'm wondering if we can let the toolstack retrieve device information for
> every device not used by Xen. This would allow embedded guys using passthrough
> "easily" when their devices are not under an IOMMU.
> ---
> tools/libxc/xc_physdev.c | 129
> +++++++++++++++++++++++++++++++++++++++++
> tools/libxc/xenctrl.h | 36 ++++++++++++
> xen/arch/arm/physdev.c | 16 +++++
> xen/common/device_tree.c | 112 +++++++++++++++++++++++++++++++++++
> xen/include/public/physdev.h | 40 +++++++++++++
> xen/include/xen/device_tree.h | 3 +
> 6 files changed, 336 insertions(+)
>
> diff --git a/tools/libxc/xc_physdev.c b/tools/libxc/xc_physdev.c
> index cf02d85..405fe78 100644
> --- a/tools/libxc/xc_physdev.c
> +++ b/tools/libxc/xc_physdev.c
> @@ -108,3 +108,132 @@ int xc_physdev_unmap_pirq(xc_interface *xch,
> return rc;
> }
>
> +int xc_physdev_dtdev_getinfo(xc_interface *xch,
> + char *path,
> + xc_dtdev_info_t *info)
> +{
> + int rc;
> + size_t size = strlen(path);
> + struct physdev_dtdev_op op;
> + DECLARE_HYPERCALL_BOUNCE(path, size, XC_HYPERCALL_BUFFER_BOUNCE_IN);
> +
> + if ( xc_hypercall_bounce_pre(xch, path) )
> + return -1;
> +
> + op.op = PHYSDEVOP_DTDEV_GET_INFO;
> + op.plen = size;
> + set_xen_guest_handle(op.path, path);
> +
> + rc = do_physdev_op(xch, PHYSDEVOP_dtdev_op, &op, sizeof(op));
> +
> + xc_hypercall_bounce_post(xch, path);
> +
> + if ( !rc )
> + {
> + info->num_irqs = op.u.info.num_irqs;
> + info->num_mmios = op.u.info.num_mmios;
> + info->compat_len = op.u.info.compat_len;
> + }
> +
> + return rc;
> +}
> +
> +int xc_physdev_dtdev_getirq(xc_interface *xch,
> + char *path,
> + uint32_t index,
> + xc_dtdev_irq_t *irq)
> +{
> + int rc;
> + size_t size = strlen(path);
> + struct physdev_dtdev_op op;
> + DECLARE_HYPERCALL_BOUNCE(path, size, XC_HYPERCALL_BUFFER_BOUNCE_IN);
> +
> + if ( xc_hypercall_bounce_pre(xch, path) )
> + return -1;
> +
> + op.op = PHYSDEVOP_DTDEV_GET_IRQ;
> + op.plen = size;
> + op.index = index;
> + set_xen_guest_handle(op.path, path);
> +
> + rc = do_physdev_op(xch, PHYSDEVOP_dtdev_op, &op, sizeof(op));
> +
> + xc_hypercall_bounce_post(xch, path);
> +
> + if ( !rc )
> + {
> + irq->irq = op.u.irq.irq;
> + irq->type = op.u.irq.type;
> + }
> +
> + return rc;
> +}
> +
> +int xc_physdev_dtdev_getmmio(xc_interface *xch,
> + char *path,
> + uint32_t index,
> + xc_dtdev_mmio_t *mmio)
> +{
> + int rc;
> + size_t size = strlen(path);
> + struct physdev_dtdev_op op;
> + DECLARE_HYPERCALL_BOUNCE(path, size, XC_HYPERCALL_BUFFER_BOUNCE_IN);
> +
> + if ( xc_hypercall_bounce_pre(xch, path) )
> + return -1;
> +
> + op.op = PHYSDEVOP_DTDEV_GET_MMIO;
> + op.plen = size;
> + op.index = index;
> + set_xen_guest_handle(op.path, path);
> +
> + rc = do_physdev_op(xch, PHYSDEVOP_dtdev_op, &op, sizeof(op));
> +
> + xc_hypercall_bounce_post(xch, path);
> +
> + if ( !rc )
> + {
> + mmio->mfn = op.u.mmio.mfn;
> + mmio->nr_mfn = op.u.mmio.nr_mfn;
> + }
> +
> + return rc;
> +}
> +
> +int xc_physdev_dtdev_getcompat(xc_interface *xch,
> + char *path,
> + char *compat,
> + uint32_t *clen)
> +{
> + int rc;
> + size_t size = strlen(path);
> + struct physdev_dtdev_op op;
> + DECLARE_HYPERCALL_BOUNCE(path, size, XC_HYPERCALL_BUFFER_BOUNCE_IN);
> + DECLARE_HYPERCALL_BOUNCE(compat, *clen, XC_HYPERCALL_BUFFER_BOUNCE_OUT);
> +
> + if ( xc_hypercall_bounce_pre(xch, path) )
> + return -1;
> +
> + rc = -1;
> + if ( xc_hypercall_bounce_pre(xch, compat) )
> + goto out;
> +
> + op.op = PHYSDEVOP_DTDEV_GET_COMPAT;
> + op.plen = size;
> + set_xen_guest_handle(op.path, path);
> +
> + op.u.compat.clen = *clen;
> + set_xen_guest_handle(op.u.compat.compat, compat);
> +
> + rc = do_physdev_op(xch, PHYSDEVOP_dtdev_op, &op, sizeof(op));
> +
> + if ( !rc )
> + *clen = op.u.compat.clen;
> +
> + xc_hypercall_bounce_post(xch, compat);
> +
> +out:
> + xc_hypercall_bounce_post(xch, path);
> +
> + return rc;
> +}
> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> index b55d857..5ad2d65 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -1143,6 +1143,42 @@ int xc_physdev_pci_access_modify(xc_interface *xch,
> int func,
> int enable);
>
> +typedef struct xc_dtdev_info {
> + uint32_t num_irqs;
> + uint32_t num_mmios;
> + uint32_t compat_len;
> +} xc_dtdev_info_t;
> +
> +int xc_physdev_dtdev_getinfo(xc_interface *xch,
> + char *path,
> + xc_dtdev_info_t *info);
> +
> +typedef struct xc_dtdev_irq {
> + uint32_t irq;
> + /* TODO: Maybe an enum here? */
> + uint32_t type;
> +} xc_dtdev_irq_t;
> +
> +int xc_physdev_dtdev_getirq(xc_interface *xch,
> + char *path,
> + uint32_t index,
> + xc_dtdev_irq_t *irq);
> +
> +typedef struct xc_dtdev_mmio {
> + uint64_t mfn;
> + uint64_t nr_mfn;
> +} xc_dtdev_mmio_t;
> +
> +int xc_physdev_dtdev_getmmio(xc_interface *xch,
> + char *path,
> + uint32_t index,
> + xc_dtdev_mmio_t *mmio);
> +
> +int xc_physdev_dtdev_getcompat(xc_interface *xch,
> + char *path,
> + char *compat,
> + uint32_t *clen);
> +
> int xc_readconsolering(xc_interface *xch,
> char *buffer,
> unsigned int *pnr_chars,
> diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
> index d17589c..11c5b59 100644
> --- a/xen/arch/arm/physdev.c
> +++ b/xen/arch/arm/physdev.c
> @@ -82,6 +82,22 @@ int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void)
> arg)
> ret = -EFAULT;
> }
> break;
> +
> + case PHYSDEVOP_dtdev_op:
> + {
> + physdev_dtdev_op_t info;
> +
> + ret = -EFAULT;
> + if ( copy_from_guest(&info, arg, 1) != 0 )
> + break;
> +
> + /* TODO: Add xsm */
> + ret = dt_do_physdev_op(&info);
> +
> + if ( __copy_to_guest(arg, &info, 1) )
> + ret = -EFAULT;
> + }
> + break;
> default:
> ret = -ENOSYS;
> break;
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index fd95307..482ff8f 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -24,6 +24,7 @@
> #include <xen/cpumask.h>
> #include <xen/ctype.h>
> #include <xen/lib.h>
> +#include <xen/irq.h>
>
> struct dt_early_info __initdata early_info;
> const void *device_tree_flattened;
> @@ -2021,6 +2022,117 @@ void __init dt_unflatten_host_device_tree(void)
> dt_alias_scan();
> }
>
> +
> +/* TODO: I think we need a bit of caching in each device node to get the
> + * information in constant time.
> + * For now we need to translate IRQs/MMIOs every time
> + */
> +int dt_do_physdev_op(physdev_dtdev_op_t *info)
> +{
> + struct dt_device_node *dev;
> + int ret;
> +
> + ret = dt_find_node_by_gpath(info->path, info->plen, &dev);
> + if ( ret )
> + return ret;
> +
> + /* Only allow access to protected device and not used by Xen */
> + if ( !dt_device_is_protected(dev) || dt_device_used_by(dev) == DOMID_XEN
> )
> + return -EACCES;
> +
> + switch ( info->op )
> + {
> + case PHYSDEVOP_DTDEV_GET_INFO:
> + {
> + const struct dt_property *compat;
> +
> + compat = dt_find_property(dev, "compatible", NULL);
> + /* Hopefully, this case should never happen, print error
> + * if it occurs
> + */
> + if ( !compat )
> + {
> + dprintk(XENLOG_G_ERR, "Unable to find compatible node for
> %s\n",
> + dt_node_full_name(dev));
> + return -EBADFD;
> + }
> +
> + info->u.info.num_irqs = dt_number_of_irq(dev);
> + info->u.info.num_mmios = dt_number_of_address(dev);
> + info->u.info.compat_len = compat->length;
> + }
> + break;
> +
> + case PHYSDEVOP_DTDEV_GET_IRQ:
> + {
> + struct dt_irq irq;
> +
> + ret = dt_device_get_irq(dev, info->index, &irq);
> + if ( ret )
> + return ret;
> +
> + /* Check if Xen is able to route the IRQ to the guest */
> + if ( !is_routable_irq(irq.irq) )
> + return -EINVAL;
> +
> + info->u.irq.irq = irq.irq;
> + /* TODO: Translate the type into an exportable value */
> + info->u.irq.type = irq.type;
> + }
> + break;
> +
> + case PHYSDEVOP_DTDEV_GET_MMIO:
> + {
> + uint64_t addr, size;
> +
> + ret = dt_device_get_address(dev, info->index, &addr, &size);
> + if ( ret )
> + return ret;
> +
> + /* Make sure the address and the size are page aligned.
> + * If not, we may passthrough MMIO regions which may belong
> + * to another device. Deny it!
> + */
> + if ( (addr & (PAGE_SIZE - 1)) || (size & (PAGE_SIZE - 1)) )
> + {
> + dprintk(XENLOG_ERR, "%s: contain non-page aligned range:"
> + " addr = 0x%"PRIx64" size = 0x%"PRIx64"\n",
> + dt_node_full_name(dev), addr, size);
> + return -EINVAL;
> + }
> +
> + info->u.mmio.mfn = paddr_to_pfn(addr);
> + info->u.mmio.nr_mfn = paddr_to_pfn(size);
> + }
> + break;
> +
> + case PHYSDEVOP_DTDEV_GET_COMPAT:
> + {
> + const struct dt_property *compat;
> +
> + compat = dt_find_property(dev, "compatible", NULL);
> + if ( !compat || !compat->length )
> + return -ENOENT;
> +
> + if ( info->u.compat.clen < compat->length )
> + return -ENOSPC;
> +
> + if ( copy_to_guest(info->u.compat.compat, compat->value,
> + compat->length) != 0 )
> + return -EFAULT;
> +
> + info->u.compat.clen = compat->length;
> + }
> + break;
> +
> + default:
> + return -ENOSYS;
> + }
> +
> +
> + return 0;
> +}
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
> index d547928..23cf673 100644
> --- a/xen/include/public/physdev.h
> +++ b/xen/include/public/physdev.h
> @@ -337,6 +337,46 @@ struct physdev_dbgp_op {
> typedef struct physdev_dbgp_op physdev_dbgp_op_t;
> DEFINE_XEN_GUEST_HANDLE(physdev_dbgp_op_t);
>
> +/* Retrieve informations about a device node */
> +#define PHYSDEVOP_dtdev_op 32
> +
> +struct physdev_dtdev_op {
> + /* IN */
> + uint32_t plen; /* Length of the path */
> + XEN_GUEST_HANDLE(char) path; /* Path to the device tree node */
> +#define PHYSDEVOP_DTDEV_GET_INFO 0
> +#define PHYSDEVOP_DTDEV_GET_IRQ 1
> +#define PHYSDEVOP_DTDEV_GET_MMIO 2
> +#define PHYSDEVOP_DTDEV_GET_COMPAT 3
> + uint8_t op;
> + uint32_t pad0:24;
> + uint32_t index; /* Index for the IRQ/MMIO to retrieve */
> + /* OUT */
> + union {
> + struct {
> + uint32_t num_irqs; /* Number of IRQs */
> + uint32_t num_mmios; /* Number of MMIOs */
> + uint32_t compat_len; /* Length of the compatible string */
> + } info;
> + struct {
> + /* TODO: Do we need to handle MSI-X? */
> + uint32_t irq; /* IRQ number */
> + /* TODO: Describe with defines the IRQ type */
> + uint32_t type; /* IRQ type (i.e edge, level...) */
> + } irq;
> + struct {
> + uint64_t mfn;
> + uint64_t nr_mfn;
> + } mmio;
> + struct {
> + uint32_t clen; /* IN: Size of buffer. OUT: Size copied
> */
> + XEN_GUEST_HANDLE_64(char) compat;
> + } compat;
> + } u;
> +};
> +typedef struct physdev_dtdev_op physdev_dtdev_op_t;
> +DEFINE_XEN_GUEST_HANDLE(physdev_dtdev_op_t);
> +
> /*
> * Notify that some PIRQ-bound event channels have been unmasked.
> * ** This command is obsolete since interface version 0x00030202 and is **
> diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> index bb33e54..3d5101c 100644
> --- a/xen/include/xen/device_tree.h
> +++ b/xen/include/xen/device_tree.h
> @@ -12,6 +12,7 @@
>
> #include <asm/byteorder.h>
> #include <public/xen.h>
> +#include <public/physdev.h>
> #include <xen/init.h>
> #include <xen/string.h>
> #include <xen/types.h>
> @@ -711,6 +712,8 @@ int dt_parse_phandle_with_args(const struct
> dt_device_node *np,
> const char *cells_name, int index,
> struct dt_phandle_args *out_args);
>
> +int dt_do_physdev_op(physdev_dtdev_op_t *info);
> +
> #endif /* __XEN_DEVICE_TREE_H */
>
> /*
> --
> 1.7.10.4
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |