Re: [Xen-devel] [PATCH v3 13/24] xen/arm: Implement hypercall PHYSDEVOP_{, un}map_pirq

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 13 Jan 2015, Julien Grall wrote:
> The physdev sub-hypercalls PHYSDEVOP_{,map}_pirq allow the toolstack to
> assign/deassign a physical IRQ to the guest (via the config options "irqs"
> for xl). The x86 version is using them with PIRQ (IRQ bound to an event
> channel). As ARM doesn't have a such concept, we could reuse it to bound
> a physical IRQ to a virtual IRQ.
> 
> For now, we allow only SPIs to be mapped to the guest.
> The type MAP_PIRQ_TYPE_GSI is used for this purpose.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> 
> ---
>     I'm not sure it's the best solution to reuse hypercalls for a
>     different purpose. If x86 plan to have a such concept (i.e binding a
>     physical IRQ to a virtual IRQ), we could introduce new hypercalls.
>     Any thoughs?

I think it is OK, as long as we write down very clearly what we are
doing.


>     TODO: This patch is lacking of support of vIRQ != IRQ. I plan to
>     handle it correctly on the next version.

Why do you say that? From the code in this patch it looks like it
supports vIRQ != IRQ already.


>     Changes in v3:
>         - Functions to allocate/release/reserved a VIRQ has been moved
>         in a separate patch

That might be a good idea, but then you need to move that patch before
this one, otherwise it won't compile. As is it would break the build.


>         - Make clear that only MAP_PIRQ_GSI is only supported for now
> 
>     Changes in v2:
>         - Add PHYSDEVOP_unmap_pirq
>         - Rework commit message
>         - Add functions to allocate/release a VIRQ
>         - is_routable_irq has been renamed into is_assignable_irq
> ---
>  xen/arch/arm/physdev.c | 136 
> ++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 134 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
> index 61b4a18..0cf9bbd 100644
> --- a/xen/arch/arm/physdev.c
> +++ b/xen/arch/arm/physdev.c
> @@ -8,13 +8,145 @@
>  #include <xen/types.h>
>  #include <xen/lib.h>
>  #include <xen/errno.h>
> +#include <xen/iocap.h>
> +#include <xen/guest_access.h>
> +#include <xsm/xsm.h>
> +#include <asm/current.h>
>  #include <asm/hypercall.h>
> +#include <public/physdev.h>
>  
> +static int physdev_map_pirq(domid_t domid, int type, int index, int *pirq_p)
> +{
> +    struct domain *d;
> +    int ret;
> +    int irq = index;
> +    int virq;
> +
> +    d = rcu_lock_domain_by_any_id(domid);
> +    if ( d == NULL )
> +        return -ESRCH;
> +
> +    ret = xsm_map_domain_pirq(XSM_TARGET, d);
> +    if ( ret )
> +        goto free_domain;
> +
> +    /* For now we only suport GSI */
> +    if ( type != MAP_PIRQ_TYPE_GSI )
> +    {
> +        ret = -EINVAL;
> +        dprintk(XENLOG_G_ERR,
> +                "dom%u: wrong map_pirq type 0x%x, only MAP_PIRQ_TYPE_GSI is 
> supported.\n",
> +                d->domain_id, type);
> +        goto free_domain;
> +    }
> +
> +    if ( !is_assignable_irq(irq) )
> +    {
> +        ret = -EINVAL;
> +        dprintk(XENLOG_G_ERR, "IRQ%u is not routable to a guest\n", irq);
> +        goto free_domain;
> +    }
> +
> +    ret = -EPERM;
> +    if ( !irq_access_permitted(current->domain, irq) )
> +        goto free_domain;
> +
> +    if ( *pirq_p < 0 )
> +    {



> +        BUG_ON(irq < 16);   /* is_assignable_irq already denies SGIs */
> +        virq = vgic_allocate_virq(d, (irq >= 32));
> +
> +        ret = -ENOSPC;
> +        if ( virq < 0 )
> +            goto free_domain;
> +    }
> +    else
> +    {
> +        ret = -EBUSY;
> +        virq = *pirq_p;
> +
> +        if ( !vgic_reserve_virq(d, virq) )
> +            goto free_domain;
> +    }
> +
> +    gdprintk(XENLOG_DEBUG, "irq = %u virq = %u\n", irq, virq);
> +
> +    ret = route_irq_to_guest(d, virq, irq, "routed IRQ");
> +
> +    if ( !ret )
> +        *pirq_p = virq;
> +    else
> +        vgic_free_virq(d, virq);
> +
> +free_domain:
> +    rcu_unlock_domain(d);
> +
> +    return ret;
> +}
> +
> +int physdev_unmap_pirq(domid_t domid, int pirq)
> +{
> +    struct domain *d;
> +    int ret;
> +
> +    d = rcu_lock_domain_by_any_id(domid);
> +    if ( d == NULL )
> +        return -ESRCH;
> +
> +    ret = xsm_unmap_domain_pirq(XSM_TARGET, d);
> +    if ( ret )
> +        goto free_domain;
> +
> +    ret = release_guest_irq(d, pirq);
> +    if ( ret )
> +        goto free_domain;
> +
> +    vgic_free_virq(d, pirq);
> +
> +free_domain:
> +    rcu_unlock_domain(d);
> +
> +    return ret;
> +}
>  
>  int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
> -    printk("%s %d cmd=%d: not implemented yet\n", __func__, __LINE__, cmd);
> -    return -ENOSYS;
> +    int ret;
> +
> +    switch ( cmd )
> +    {
> +    case PHYSDEVOP_map_pirq:
> +        {
> +            physdev_map_pirq_t map;
> +
> +            ret = -EFAULT;
> +            if ( copy_from_guest(&map, arg, 1) != 0 )
> +                break;
> +
> +            ret = physdev_map_pirq(map.domid, map.type, map.index, 
> &map.pirq);
> +
> +            if ( __copy_to_guest(arg, &map, 1) )
> +                ret = -EFAULT;
> +        }
> +        break;
> +
> +    case PHYSDEVOP_unmap_pirq:
> +        {
> +            physdev_unmap_pirq_t unmap;
> +
> +            ret = -EFAULT;
> +            if ( copy_from_guest(&unmap, arg, 1) != 0 )
> +                break;
> +
> +            ret = physdev_unmap_pirq(unmap.domid, unmap.pirq);
> +        }
> +
> +    default:
> +        ret = -ENOSYS;
> +        break;
> +    }
> +
> +    return ret;
>  }
>  
>  /*
> -- 
> 2.1.4
> 

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