Re: [Xen-devel] [RFC][v2][PATCH 08/14] tools: extend xc_assign_device() to support rdm reservation policy

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

On 2015/6/3 0:36, Wei Liu wrote:
> On Fri, May 22, 2015 at 05:35:08PM +0800, Tiejun Chen wrote:
> > This patch passes rdm reservation policy to xc_assign_device() so the policy
> > is checked when assigning devices to a VM.
> >
> > Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>
> > ---
> >   tools/libxc/include/xenctrl.h       |  3 ++-
> >   tools/libxc/xc_domain.c             |  4 +++-
> >   tools/libxl/libxl_pci.c             | 11 ++++++++++-
> >   tools/libxl/xl_cmdimpl.c            | 23 +++++++++++++++++++----
> >   tools/libxl/xl_cmdtable.c           |  2 +-
> Where is document for the new options you added to xl pci commands?
Looks I'm missing to describe something specific to pci-attach?

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 4eb929d..2ebfd54 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1368,10 +1368,15 @@ it will also attempt to re-bind the device to 
its original driver, making it
 usable by Domain 0 again.  If the device is not bound to pciback, it will
 return success.

-=item B<pci-attach> I<domain-id> I<BDF>
+=item B<pci-attach> I<domain-id> I<BDF> I<rdm policy>

 Hot-plug a new pass-through pci device to the specified domain.
 B<BDF> is the PCI Bus/Device/Function of the physical device to 
pass-through.
+B<rdm policy> is about how to handle conflict between reserving 
reserved device
+memory and guest address space. "strict" means an unsolved conflict 
leads to
+immediate VM crash, while "relaxed" allows VM moving forward with a warning
+message thrown out. Here "strict" is default.
+

 =item B<pci-detach> [I<-f>] I<domain-id> I<BDF>


> BTW you might want to consider rearrange patches in this series so that
Yes, this is really what I intend to do.

> you keep the tree bisectable.
Overall, I can separate this series as several parts,

#1. Introduce our policy configuration on tools side
#2. Interact with Hypervisor to get rdm info
#3. Implement our policy with rdm info on tool side
#4. Make hvmloader to align our policy

If you already see something obviously wrong, let me know.

> >   tools/ocaml/libs/xc/xenctrl_stubs.c | 18 ++++++++++++++----
> >   tools/python/xen/lowlevel/xc/xc.c   | 29 +++++++++++++++++++----------
> >   xen/drivers/passthrough/pci.c       |  3 ++-
> >   8 files changed, 70 insertions(+), 23 deletions(-)
> >
> > diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> > index 5f84a62..2a447b9 100644
> > --- a/tools/libxc/include/xenctrl.h
> > +++ b/tools/libxc/include/xenctrl.h
> > @@ -2078,7 +2078,8 @@ int xc_hvm_destroy_ioreq_server(xc_interface *xch,
> >   /* HVM guest pass-through */
> >   int xc_assign_device(xc_interface *xch,
> >                        uint32_t domid,
> > -                     uint32_t machine_sbdf);
> > +                     uint32_t machine_sbdf,
> > +                     uint32_t flag);
> >
> >   int xc_get_device_group(xc_interface *xch,
> >                        uint32_t domid,
> > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> > index c17a5a8..9761e5a 100644
> > --- a/tools/libxc/xc_domain.c
> > +++ b/tools/libxc/xc_domain.c
> > @@ -1704,7 +1704,8 @@ int xc_domain_setdebugging(xc_interface *xch,
> >   int xc_assign_device(
> >       xc_interface *xch,
> >       uint32_t domid,
> > -    uint32_t machine_sbdf)
> > +    uint32_t machine_sbdf,
> > +    uint32_t flag)
> >   {
> >       DECLARE_DOMCTL;
> >
> > @@ -1712,6 +1713,7 @@ int xc_assign_device(
> >       domctl.domain = domid;
> >       domctl.u.assign_device.dev = XEN_DOMCTL_DEV_PCI;
> >       domctl.u.assign_device.u.pci.machine_sbdf = machine_sbdf;
> > +    domctl.u.assign_device.flag = flag;
> >
> >       return do_domctl(xch, &domctl);
> >   }
> > diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> > index 07e84f2..ac70edc 100644
> > --- a/tools/libxl/libxl_pci.c
> > +++ b/tools/libxl/libxl_pci.c
> > @@ -894,6 +894,7 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, 
> > libxl_device_pci *pcidev, i
> >       FILE *f;
> >       unsigned long long start, end, flags, size;
> >       int irq, i, rc, hvm = 0;
> > +    uint32_t flag;
> >
> >       if (type == LIBXL_DOMAIN_TYPE_INVALID)
> >           return ERROR_FAIL;
> > @@ -987,7 +988,15 @@ static int do_pci_add(libxl__gc *gc, uint32_t domid, 
> > libxl_device_pci *pcidev, i
> >
> >   out:
> >       if (!libxl_is_stubdom(ctx, domid, NULL)) {
> > -        rc = xc_assign_device(ctx->xch, domid, pcidev_encode_bdf(pcidev));
> > +        if (pcidev->rdm_reserve == LIBXL_RDM_RESERVE_FLAG_RELAXED) {
> > +            flag = XEN_DOMCTL_DEV_RDM_RELAXED;
> > +        } else if (pcidev->rdm_reserve == LIBXL_RDM_RESERVE_FLAG_STRICT) {
> > +            flag = XEN_DOMCTL_DEV_RDM_STRICT;
> > +        } else {
> > +            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "unkwon rdm check flag.");
> unknown
>
> Couldn't continue reviewing because I don't know the expected behaviour.
> But the changes look mostly mechanical.
I want to make this assignment failed so return ERROR_FAIL


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