Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Thu, Mar 22, 2012 at 1:20 AM, Hao, Xudong <xudong.hao@xxxxxxxxx> wrote:
> <Porting from Xen 4.1 tree.>
>
> libxl: passthrough: avoid passing through devices not owned by pciback
>
> This patch makes sure the passthrough device belongs to pciback before allow 
> them passthrough to the guest.  There are still many other checks missing.
>
> xm terminates the guest startup process when this type of condition is found. 
>  This patch just allows the guest to continue to boot but with no device 
> passthrough.

Thanks for the patch.  My one comment is that I think that xm's
behavior is the correct one.  If I tell the computer "Do X with
feature Y", I want the computer to either do X with feature Y, or to
say, "I'm sorry, feature Y is not available."  Then I can figure out
if I want X without Y, or whether I want to fix things so Y is
available.  Saying, "Y is not available so I'll just to X" is rarely
the right thing.

On a different (but related) note, it appears that libxl doesn't have
the pci-quirks functionality that xend has.  Do you know if this is
still needed, if it's in the pvops tree and needs to be upstreamed?
Is anyone at Intel working on libxl pci-quirks support for the 4.2
release?

 -George

>
> Signed-off-by: Allen Kay <allen.m.kay@xxxxxxxxx>
> Signed-off-by: Xudong Hao <xudong.hao@xxxxxxxxx>
>
> diff -r 4e1d091d10d8 tools/libxl/libxl_pci.c
> --- a/tools/libxl/libxl_pci.c   Fri Mar 16 15:24:25 2012 +0000
> +++ b/tools/libxl/libxl_pci.c   Thu Mar 22 00:43:14 2012 +0800
> @@ -779,6 +779,24 @@ int libxl_device_pci_add(libxl_ctx *ctx,
>     return rc;
>  }
>
> +static int libxl_pcidev_assignable(libxl_ctx *ctx, libxl_device_pci
> +*pcidev) {
> +    libxl_device_pci *pcidevs;
> +    int num, i;
> +
> +    pcidevs = libxl_device_pci_list_assignable(ctx, &num);
> +    for (i = 0; i < num; i++) {
> +        if (pcidevs[i].domain == pcidev->domain &&
> +            pcidevs[i].bus == pcidev->bus &&
> +            pcidevs[i].dev == pcidev->dev &&
> +            pcidevs[i].func == pcidev->func)
> +        {
> +            return 1;
> +        }
> +    }
> +    return 0;
> +}
> +
>  int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, libxl_device_pci 
> *pcidev, int starting)  {
>     libxl_ctx *ctx = libxl__gc_owner(gc); @@ -789,6 +807,13 @@ int 
> libxl__device_pci_add(libxl__gc *gc,
>
>     rc = libxl__device_pci_setdefault(gc, pcidev);
>     if (rc) goto out;
> +
> +    if (!libxl_pcidev_assignable(ctx, pcidev)) {
> +        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "PCI device %x:%x:%x.%x is not 
> assignable",
> +                   pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func);
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
>
>     rc = get_all_assigned_devices(gc, &assigned, &num_assigned);
>     if ( rc ) {
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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