All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
@ 2012-03-22  1:20 Hao, Xudong
  2012-03-22 10:15 ` George Dunlap
  2012-04-02 17:00 ` Ian Jackson
  0 siblings, 2 replies; 18+ messages in thread
From: Hao, Xudong @ 2012-03-22  1:20 UTC (permalink / raw)
  To: ian.jackson; +Cc: xen-devel, Kay, Allen M

<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.

Signed-off-by: Allen Kay <allen.m.kay@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>

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 ) {

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-03-22  1:20 [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback Hao, Xudong
@ 2012-03-22 10:15 ` George Dunlap
  2012-03-23  2:47   ` Hao, Xudong
  2012-04-02 17:00 ` Ian Jackson
  1 sibling, 1 reply; 18+ messages in thread
From: George Dunlap @ 2012-03-22 10:15 UTC (permalink / raw)
  To: Hao, Xudong; +Cc: Kay, Allen M, xen-devel, ian.jackson

On Thu, Mar 22, 2012 at 1:20 AM, Hao, Xudong <xudong.hao@intel.com> 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@intel.com>
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
>
> 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@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-03-22 10:15 ` George Dunlap
@ 2012-03-23  2:47   ` Hao, Xudong
  2012-03-27 14:00     ` Ian Jackson
  2012-03-27 14:41     ` George Dunlap
  0 siblings, 2 replies; 18+ messages in thread
From: Hao, Xudong @ 2012-03-23  2:47 UTC (permalink / raw)
  To: George Dunlap; +Cc: Kay, Allen M, xen-devel, ian.jackson


> -----Original Message-----
> From: dunlapg@gmail.com [mailto:dunlapg@gmail.com] On Behalf Of George
> Dunlap
> Sent: Thursday, March 22, 2012 6:16 PM
> To: Hao, Xudong
> Cc: ian.jackson@eu.citrix.com; xen-devel@lists.xensource.com; Kay, Allen M
> Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> On Thu, Mar 22, 2012 at 1:20 AM, Hao, Xudong <xudong.hao@intel.com>
> 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.
> 
George, your theory is correct, but I think it's not much suitable for this patch. This patch just add an additional checking, it return ERROR_FAIL to its call function, but it can't control the behavior by the result of upper function libxl_device_pci_add().
 
> 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?
> 
We did not see any Intel pci device need quirk in libxl till now, so maybe no plan for libxl pci-quirks support for the 4.2 release.

>  -George
> 
> >
> > Signed-off-by: Allen Kay <allen.m.kay@intel.com>
> > Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> >
> > 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@lists.xen.org
> > http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-03-23  2:47   ` Hao, Xudong
@ 2012-03-27 14:00     ` Ian Jackson
  2012-03-31  2:12       ` Hao, Xudong
  2012-03-27 14:41     ` George Dunlap
  1 sibling, 1 reply; 18+ messages in thread
From: Ian Jackson @ 2012-03-27 14:00 UTC (permalink / raw)
  To: Hao, Xudong; +Cc: George Dunlap, xen-devel, Kay, Allen M

Hao, Xudong writes ("RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback"):
> We did not see any Intel pci device need quirk in libxl till now, so maybe no plan for libxl pci-quirks support for the 4.2 release.

OK.  The rest of the patch looks sensible (although the code
formatting isn't ideal).

Ian.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-03-23  2:47   ` Hao, Xudong
  2012-03-27 14:00     ` Ian Jackson
@ 2012-03-27 14:41     ` George Dunlap
  2012-03-31  2:10       ` Hao, Xudong
  1 sibling, 1 reply; 18+ messages in thread
From: George Dunlap @ 2012-03-27 14:41 UTC (permalink / raw)
  To: Hao, Xudong; +Cc: ian.jackson, xen-devel, Kay, Allen M

On Fri, Mar 23, 2012 at 2:47 AM, Hao, Xudong <xudong.hao@intel.com> wrote:
>> 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?
>>
> We did not see any Intel pci device need quirk in libxl till now, so maybe no plan for libxl pci-quirks support for the 4.2 release.

FWIW, I did see a quirk on the following Intel NIC (don't have the
details of the warning message handy):

07:00.0 Ethernet controller [0200]: Intel Corporation 82575GB Gigabit
Network Connection [8086:10d6] (rev 02)

But given the proximity to the 4.2 release, it would make perfect
sense to wait for 4.3.  I'm going to try to implement a per-device
"permissive" feature in libxl which should tide us over.

 -George

>
>>  -George
>>
>> >
>> > Signed-off-by: Allen Kay <allen.m.kay@intel.com>
>> > Signed-off-by: Xudong Hao <xudong.hao@intel.com>
>> >
>> > 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@lists.xen.org
>> > http://lists.xen.org/xen-devel
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-03-27 14:41     ` George Dunlap
@ 2012-03-31  2:10       ` Hao, Xudong
  0 siblings, 0 replies; 18+ messages in thread
From: Hao, Xudong @ 2012-03-31  2:10 UTC (permalink / raw)
  To: George Dunlap; +Cc: ian.jackson, xen-devel, Kay, Allen M


> -----Original Message-----
> From: dunlapg@gmail.com [mailto:dunlapg@gmail.com] On Behalf Of George
> Dunlap
> Sent: Tuesday, March 27, 2012 10:42 PM
> To: Hao, Xudong
> Cc: Kay, Allen M; xen-devel@lists.xensource.com; ian.jackson@eu.citrix.com
> Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> On Fri, Mar 23, 2012 at 2:47 AM, Hao, Xudong <xudong.hao@intel.com>
> wrote:
> >> 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?
> >>
> > We did not see any Intel pci device need quirk in libxl till now, so maybe no
> plan for libxl pci-quirks support for the 4.2 release.
> 
> FWIW, I did see a quirk on the following Intel NIC (don't have the details of the
> warning message handy):
> 
> 07:00.0 Ethernet controller [0200]: Intel Corporation 82575GB Gigabit Network
> Connection [8086:10d6] (rev 02)
> 
> But given the proximity to the 4.2 release, it would make perfect sense to wait
> for 4.3.  I'm going to try to implement a per-device "permissive" feature in
> libxl which should tide us over.
>
That's great, looking forward to your new feature. 

Thanks,
-Xudong
 
>  -George
> 
> >
> >>  -George
> >>
> >> >
> >> > Signed-off-by: Allen Kay <allen.m.kay@intel.com>
> >> > Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> >> >
> >> > 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@lists.xen.org
> >> > http://lists.xen.org/xen-devel
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-03-27 14:00     ` Ian Jackson
@ 2012-03-31  2:12       ` Hao, Xudong
  0 siblings, 0 replies; 18+ messages in thread
From: Hao, Xudong @ 2012-03-31  2:12 UTC (permalink / raw)
  To: Ian Jackson; +Cc: George Dunlap, xen-devel, Kay, Allen M


> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Tuesday, March 27, 2012 10:00 PM
> To: Hao, Xudong
> Cc: George Dunlap; xen-devel@lists.xensource.com; Kay, Allen M
> Subject: RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> Hao, Xudong writes ("RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing
> through devices not owned by pciback"):
> > We did not see any Intel pci device need quirk in libxl till now, so maybe no
> plan for libxl pci-quirks support for the 4.2 release.
> 
> OK.  The rest of the patch looks sensible (although the code formatting isn't
> ideal).
>
Ian, 

Do you want me to rewrite a patch with correct code formatting, or check this patch in?
 
> Ian.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-03-22  1:20 [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback Hao, Xudong
  2012-03-22 10:15 ` George Dunlap
@ 2012-04-02 17:00 ` Ian Jackson
  2012-04-05  1:13   ` Hao, Xudong
  1 sibling, 1 reply; 18+ messages in thread
From: Ian Jackson @ 2012-04-02 17:00 UTC (permalink / raw)
  To: Hao, Xudong; +Cc: xen-devel, Kay, Allen M

Hao, Xudong writes ("[Xen-devel] [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback"):
> <Porting from Xen 4.1 tree.>
> 
> libxl: passthrough: avoid passing through devices not owned by pciback

I'm afraid this no longer applies to xen-unstable.hg tip.

Ian.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-04-02 17:00 ` Ian Jackson
@ 2012-04-05  1:13   ` Hao, Xudong
  2012-04-05  8:07     ` George Dunlap
  2012-04-05 14:41     ` Ian Jackson
  0 siblings, 2 replies; 18+ messages in thread
From: Hao, Xudong @ 2012-04-05  1:13 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Kay, Allen M


> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Tuesday, April 03, 2012 1:01 AM
> To: Hao, Xudong
> Cc: xen-devel@lists.xensource.com; Kay, Allen M
> Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> Hao, Xudong writes ("[Xen-devel] [PATCH] libxl: passthrough: avoid passing
> through devices not owned by pciback"):
> > <Porting from Xen 4.1 tree.>
> >
> > libxl: passthrough: avoid passing through devices not owned by pciback
> 
> I'm afraid this no longer applies to xen-unstable.hg tip.
> 
Reason?

If no pciback checking, one device could be assigned to guest even it's being used by dom0, is there security issue?

Thanks,
-Xudong

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-04-05  1:13   ` Hao, Xudong
@ 2012-04-05  8:07     ` George Dunlap
  2012-04-05 14:41     ` Ian Jackson
  1 sibling, 0 replies; 18+ messages in thread
From: George Dunlap @ 2012-04-05  8:07 UTC (permalink / raw)
  To: Hao, Xudong; +Cc: Kay, Allen M, xen-devel, Ian Jackson

On Thu, Apr 5, 2012 at 2:13 AM, Hao, Xudong <xudong.hao@intel.com> wrote:
>
>> -----Original Message-----
>> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
>> Sent: Tuesday, April 03, 2012 1:01 AM
>> To: Hao, Xudong
>> Cc: xen-devel@lists.xensource.com; Kay, Allen M
>> Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
>> devices not owned by pciback
>>
>> Hao, Xudong writes ("[Xen-devel] [PATCH] libxl: passthrough: avoid passing
>> through devices not owned by pciback"):
>> > <Porting from Xen 4.1 tree.>
>> >
>> > libxl: passthrough: avoid passing through devices not owned by pciback
>>
>> I'm afraid this no longer applies to xen-unstable.hg tip.
>>
> Reason?
>
> If no pciback checking, one device could be assigned to guest even it's being used by dom0, is there security issue?

I think Ian is saying that the actual patch itself doesn't apply. That is,

$ patch -p1 < passthru.diff

fails; so he's asking you to send a new version of the patch.

 -George

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-04-05  1:13   ` Hao, Xudong
  2012-04-05  8:07     ` George Dunlap
@ 2012-04-05 14:41     ` Ian Jackson
  2012-04-05 15:37       ` Hao, Xudong
  2012-04-17  1:28       ` Hao, Xudong
  1 sibling, 2 replies; 18+ messages in thread
From: Ian Jackson @ 2012-04-05 14:41 UTC (permalink / raw)
  To: Hao, Xudong; +Cc: xen-devel, Kay, Allen M

Hao, Xudong writes ("RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback"):
> 
> > -----Original Message-----
> > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > Sent: Tuesday, April 03, 2012 1:01 AM
> > To: Hao, Xudong
> > Cc: xen-devel@lists.xensource.com; Kay, Allen M
> > Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> > devices not owned by pciback
> > 
> > Hao, Xudong writes ("[Xen-devel] [PATCH] libxl: passthrough: avoid passing
> > through devices not owned by pciback"):
> > > <Porting from Xen 4.1 tree.>
> > >
> > > libxl: passthrough: avoid passing through devices not owned by pciback
> > 
> > I'm afraid this no longer applies to xen-unstable.hg tip.
> > 
> Reason?
> 
> If no pciback checking, one device could be assigned to guest even it's being used by dom0, is there security issue?

I mean that it has conflicts when I try to apply it.  You need to
refresh it.

Thanks,
Ian.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-04-05 14:41     ` Ian Jackson
@ 2012-04-05 15:37       ` Hao, Xudong
  2012-04-24 18:02         ` Ian Jackson
  2012-04-17  1:28       ` Hao, Xudong
  1 sibling, 1 reply; 18+ messages in thread
From: Hao, Xudong @ 2012-04-05 15:37 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Kay, Allen M

<Porting from xen 4.1, patch on Xen unstable 25138>

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.

Signed-off-by: Allen Kay <allen.m.kay@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>

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 ) {

Thanks,
-Xudong

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Thursday, April 05, 2012 10:42 PM
> To: Hao, Xudong
> Cc: xen-devel@lists.xensource.com; Kay, Allen M
> Subject: RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> Hao, Xudong writes ("RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing
> through devices not owned by pciback"):
> >
> > > -----Original Message-----
> > > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > > Sent: Tuesday, April 03, 2012 1:01 AM
> > > To: Hao, Xudong
> > > Cc: xen-devel@lists.xensource.com; Kay, Allen M
> > > Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing
> > > through devices not owned by pciback
> > >
> > > Hao, Xudong writes ("[Xen-devel] [PATCH] libxl: passthrough: avoid
> > > passing through devices not owned by pciback"):
> > > > <Porting from Xen 4.1 tree.>
> > > >
> > > > libxl: passthrough: avoid passing through devices not owned by
> > > > pciback
> > >
> > > I'm afraid this no longer applies to xen-unstable.hg tip.
> > >
> > Reason?
> >
> > If no pciback checking, one device could be assigned to guest even it's being
> used by dom0, is there security issue?
> 
> I mean that it has conflicts when I try to apply it.  You need to refresh it.
> 
> Thanks,
> Ian.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-04-05 14:41     ` Ian Jackson
  2012-04-05 15:37       ` Hao, Xudong
@ 2012-04-17  1:28       ` Hao, Xudong
  1 sibling, 0 replies; 18+ messages in thread
From: Hao, Xudong @ 2012-04-17  1:28 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

Hi, Ian

Any other comments for this patch?

Thanks,
-Xudong


> -----Original Message-----
> From: Hao, Xudong
> Sent: Thursday, April 05, 2012 11:37 PM
> To: 'Ian Jackson'
> Cc: xen-devel@lists.xensource.com; Kay, Allen M
> Subject: RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> <Porting from xen 4.1, patch on Xen unstable 25138>
> 
> 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.
> 
> Signed-off-by: Allen Kay <allen.m.kay@intel.com>
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> 
> 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 ) {
> 
> Thanks,
> -Xudong
> 
> > -----Original Message-----
> > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > Sent: Thursday, April 05, 2012 10:42 PM
> > To: Hao, Xudong
> > Cc: xen-devel@lists.xensource.com; Kay, Allen M
> > Subject: RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> > devices not owned by pciback
> >
> > Hao, Xudong writes ("RE: [Xen-devel] [PATCH] libxl: passthrough: avoid
> passing
> > through devices not owned by pciback"):
> > >
> > > > -----Original Message-----
> > > > From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> > > > Sent: Tuesday, April 03, 2012 1:01 AM
> > > > To: Hao, Xudong
> > > > Cc: xen-devel@lists.xensource.com; Kay, Allen M
> > > > Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing
> > > > through devices not owned by pciback
> > > >
> > > > Hao, Xudong writes ("[Xen-devel] [PATCH] libxl: passthrough: avoid
> > > > passing through devices not owned by pciback"):
> > > > > <Porting from Xen 4.1 tree.>
> > > > >
> > > > > libxl: passthrough: avoid passing through devices not owned by
> > > > > pciback
> > > >
> > > > I'm afraid this no longer applies to xen-unstable.hg tip.
> > > >
> > > Reason?
> > >
> > > If no pciback checking, one device could be assigned to guest even it's being
> > used by dom0, is there security issue?
> >
> > I mean that it has conflicts when I try to apply it.  You need to refresh it.
> >
> > Thanks,
> > Ian.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-04-05 15:37       ` Hao, Xudong
@ 2012-04-24 18:02         ` Ian Jackson
  2012-04-25  1:18           ` Hao, Xudong
  2012-04-25  2:01           ` Hao, Xudong
  0 siblings, 2 replies; 18+ messages in thread
From: Ian Jackson @ 2012-04-24 18:02 UTC (permalink / raw)
  To: Hao, Xudong; +Cc: xen-devel, Kay, Allen M

Hao, Xudong writes ("Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback"):
> <Porting from xen 4.1, patch on Xen unstable 25138>

I'm afraid that 25138 was out of date even when you posted your
revised patch, and it still doesn't apply.

Are you working with the staging tree ?

Ian.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-04-24 18:02         ` Ian Jackson
@ 2012-04-25  1:18           ` Hao, Xudong
  2012-04-25 10:16             ` Ian Jackson
  2012-04-25  2:01           ` Hao, Xudong
  1 sibling, 1 reply; 18+ messages in thread
From: Hao, Xudong @ 2012-04-25  1:18 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Kay, Allen M

I work on unstable tree.

I think the confliction is my MS outlook configuration issue, I'll re-resend a revised patch.

Thanks,
-Xudong

> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Wednesday, April 25, 2012 2:03 AM
> To: Hao, Xudong
> Cc: xen-devel@lists.xensource.com; Kay, Allen M
> Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> Hao, Xudong writes ("Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing
> through devices not owned by pciback"):
> > <Porting from xen 4.1, patch on Xen unstable 25138>
> 
> I'm afraid that 25138 was out of date even when you posted your revised patch,
> and it still doesn't apply.
> 
> Are you working with the staging tree ?
> 
> Ian.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-04-24 18:02         ` Ian Jackson
  2012-04-25  1:18           ` Hao, Xudong
@ 2012-04-25  2:01           ` Hao, Xudong
  2012-04-25 10:18             ` Ian Jackson
  1 sibling, 1 reply; 18+ messages in thread
From: Hao, Xudong @ 2012-04-25  2:01 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel, Kay, Allen M

<RESEND: Porting from xen 4.1, patch on Xen unstable 25220>

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.

Signed-off-by: Allen Kay <allen.m.kay@intel.com>
Signed-off-by: Xudong Hao <xudong.hao@intel.com>

diff -r a06e6cdeafe3 tools/libxl/libxl_pci.c
--- a/tools/libxl/libxl_pci.c	Mon Apr 16 13:05:28 2012 +0200
+++ b/tools/libxl/libxl_pci.c	Wed Apr 17 17:02:25 2013 +0800
@@ -664,6 +664,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);
@@ -674,6 +692,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 ) {


Thanks,
-Xudong


> -----Original Message-----
> From: Ian Jackson [mailto:Ian.Jackson@eu.citrix.com]
> Sent: Wednesday, April 25, 2012 2:03 AM
> To: Hao, Xudong
> Cc: xen-devel@lists.xensource.com; Kay, Allen M
> Subject: Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through
> devices not owned by pciback
> 
> Hao, Xudong writes ("Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing
> through devices not owned by pciback"):
> > <Porting from xen 4.1, patch on Xen unstable 25138>
> 
> I'm afraid that 25138 was out of date even when you posted your
> revised patch, and it still doesn't apply.
> 
> Are you working with the staging tree ?
> 
> Ian.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-04-25  1:18           ` Hao, Xudong
@ 2012-04-25 10:16             ` Ian Jackson
  0 siblings, 0 replies; 18+ messages in thread
From: Ian Jackson @ 2012-04-25 10:16 UTC (permalink / raw)
  To: Hao, Xudong; +Cc: xen-devel, Kay, Allen M

Hao, Xudong writes ("RE: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback"):
> I work on unstable tree.

xen-unstable.hg comes in two variants.  See the text under
  http://wiki.xen.org/wiki/Submitting_Xen_Patches#After_your_patch_is_committed

> I think the confliction is my MS outlook configuration issue, I'll re-resend a revised patch.

I think it's because you used an out of date tip.  You based your
patch on 25138, according to your email.  But at the time you sent
your email, the most recent changeset was something later than 25156,
and 25156 changed libcl_pci.c.

But I will try your repost.

Ian.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback
  2012-04-25  2:01           ` Hao, Xudong
@ 2012-04-25 10:18             ` Ian Jackson
  0 siblings, 0 replies; 18+ messages in thread
From: Ian Jackson @ 2012-04-25 10:18 UTC (permalink / raw)
  To: Hao, Xudong; +Cc: xen-devel, Kay, Allen M

Hao, Xudong writes ("Re: [Xen-devel] [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback"):
> libxl: passthrough: avoid passing through devices not owned by pciback

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2012-04-25 10:18 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-22  1:20 [PATCH] libxl: passthrough: avoid passing through devices not owned by pciback Hao, Xudong
2012-03-22 10:15 ` George Dunlap
2012-03-23  2:47   ` Hao, Xudong
2012-03-27 14:00     ` Ian Jackson
2012-03-31  2:12       ` Hao, Xudong
2012-03-27 14:41     ` George Dunlap
2012-03-31  2:10       ` Hao, Xudong
2012-04-02 17:00 ` Ian Jackson
2012-04-05  1:13   ` Hao, Xudong
2012-04-05  8:07     ` George Dunlap
2012-04-05 14:41     ` Ian Jackson
2012-04-05 15:37       ` Hao, Xudong
2012-04-24 18:02         ` Ian Jackson
2012-04-25  1:18           ` Hao, Xudong
2012-04-25 10:16             ` Ian Jackson
2012-04-25  2:01           ` Hao, Xudong
2012-04-25 10:18             ` Ian Jackson
2012-04-17  1:28       ` Hao, Xudong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.