|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 4/6] libxl: Add wrappers for new commands and add AER error handler
On 2017-06-30 11:18:15 +0100, Wei Liu wrote:
> On Tue, Jun 27, 2017 at 12:14:56PM -0500, Venu Busireddy wrote:
> > libxl: Add wrappers for new commands and add AER error handler
>
> Extraneous line.
My mistake. Will remove it.
> >
> > Add wrappers for the newly introduced commands "pci-assignable-hide",
> > "pci-assignable-unhide", and "pci-assignable-list-hidden".
> >
> > Implement the callback function to handle unrecoverable AER errors.
> >
> > Signed-off-by: Venu Busireddy <venu.busireddy@xxxxxxxxxx>
> > ---
> > tools/libxl/libxl.h | 3 +
> > tools/libxl/libxl_event.h | 2 +
> > tools/libxl/libxl_pci.c | 150
> > ++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 155 insertions(+)
> >
> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> > index cf8687a..5a5bd14 100644
> > --- a/tools/libxl/libxl.h
> > +++ b/tools/libxl/libxl.h
> > @@ -1944,6 +1944,9 @@ int libxl_device_events_handler(libxl_ctx *ctx,
> > int libxl_device_pci_assignable_add(libxl_ctx *ctx, libxl_device_pci
> > *pcidev, int rebind);
> > int libxl_device_pci_assignable_remove(libxl_ctx *ctx, libxl_device_pci
> > *pcidev, int rebind);
> > libxl_device_pci *libxl_device_pci_assignable_list(libxl_ctx *ctx, int
> > *num);
> > +int libxl_device_pci_assignable_hide(libxl_ctx *ctx, libxl_device_pci
> > *pcidev);
> > +int libxl_device_pci_assignable_unhide(libxl_ctx *ctx, libxl_device_pci
> > *pcidev);
> > +int libxl_device_pci_assignable_is_hidden(libxl_ctx *ctx, libxl_device_pci
> > *pcidev);
> >
> > /* CPUID handling */
> > int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char*
> > str);
> > diff --git a/tools/libxl/libxl_event.h b/tools/libxl/libxl_event.h
> > index 1ea789e..4c78798 100644
> > --- a/tools/libxl/libxl_event.h
> > +++ b/tools/libxl/libxl_event.h
> > @@ -178,6 +178,8 @@ void libxl_event_register_callbacks(libxl_ctx *ctx,
> > typedef struct libxl__evgen_domain_death libxl_evgen_domain_death;
> > int libxl_evenable_domain_death(libxl_ctx *ctx, uint32_t domid,
> > libxl_ev_user, libxl_evgen_domain_death
> > **evgen_out);
> > +int libxl_reg_aer_events_handler(libxl_ctx *, uint32_t)
> > LIBXL_EXTERNAL_CALLERS_ONLY;
> > +void libxl_unreg_aer_events_handler(libxl_ctx *, uint32_t);
> > void libxl_evdisable_domain_death(libxl_ctx *ctx,
> > libxl_evgen_domain_death*);
> > /* Arranges for the generation of DOMAIN_SHUTDOWN and DOMAIN_DEATH
> > * events. A domain which is destroyed before it shuts down
> > diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> > index b14df16..e6996e5 100644
> > --- a/tools/libxl/libxl_pci.c
> > +++ b/tools/libxl/libxl_pci.c
> > @@ -874,6 +874,42 @@ int libxl_device_pci_assignable_add(libxl_ctx *ctx,
> > libxl_device_pci *pcidev,
> > return rc;
> > }
> >
> > +int libxl_device_pci_assignable_hide(libxl_ctx *ctx, libxl_device_pci
> > *pcidev)
> > +{
> > + GC_INIT(ctx);
> > + int rc;
>
> int r;
>
> > +
> > + rc = xc_hide_device(ctx->xch, pcidev_encode_bdf(pcidev));
>
> r = xc_...
>
> > + if (rc < 0)
>
> if (r < 0) { LOG(...); rc = ERROR_???; }
>
> > + LOGD(ERROR, 0, "xc_hide_device failed");
> > +
> > + GC_FREE;
> > + return rc;
> > +}
> > +
> [...]
Will make these changes, if deemed necessary, depending on what we agree
on the final solution. This code may go away if our approach changes.
Please see [1] and [2] below.
> >
> > int libxl_device_pci_assignable_remove(libxl_ctx *ctx, libxl_device_pci
> > *pcidev,
> > int rebind)
> > @@ -1292,6 +1328,120 @@ out:
> > return rc;
> > }
> >
> > +static void domain_destroy_callback(libxl__egc *egc,
> > + libxl__domain_destroy_state *dds,
> > + int rc)
> > +{
> > + STATE_AO_GC(dds->ao);
> > +
> > + if (rc)
> > + LOGD(ERROR, dds->domid, "Destruction of domain failed, rc = %d",
> > rc);
> > +
> > + libxl__nested_ao_free(ao);
> > +}
> > +
> > +
> > +typedef struct {
> > + uint32_t domid;
> > + libxl__ao *ao;
> > + libxl__ev_xswatch watch;
> > +} libxl_aer_watch;
> > +static libxl_aer_watch aer_watch;
> > +
> > +static void aer_backend_watch_callback(libxl__egc *egc,
> > + libxl__ev_xswatch *watch,
> > + const char *watch_path,
> > + const char *event_path)
> > +{
> > + libxl_aer_watch *l_aer_watch = CONTAINER_OF(watch, *l_aer_watch,
> > watch);
> > + libxl__ao *nested_ao = libxl__nested_ao_create(l_aer_watch->ao);
> > + STATE_AO_GC(nested_ao);
> > + libxl_ctx *ctx = libxl__gc_owner(gc);
> > + uint32_t domid = l_aer_watch->domid;
> > + uint32_t seg, bus, dev, fn;
> > + int rc;
> > + char *p, *path, *dst_path;
> > + const char *aerFailedSBDF;
> > + struct xs_permissions rwperm[1];
> > + libxl__domain_destroy_state *dds;
> > + GCNEW(dds);
> > +
> > + /* Extract the backend directory. */
> > + path = libxl__strdup(gc, event_path);
> > + p = strrchr(path, '/');
> > + if (p == NULL)
> > + goto skip;
> > + if (strcmp(p, "/aerFailedSBDF") != 0)
> > + goto skip;
> > + /* Truncate the string so it points to the backend directory. */
> > + *p = '\0';
> > +
> > + /* Fetch the value of the failed PCI device. */
> > + rc = libxl__xs_read_checked(gc, XBT_NULL,
> > + GCSPRINTF("%s/aerFailedSBDF", path), &aerFailedSBDF);
> > + if (rc || !aerFailedSBDF)
> > + goto skip;
> > + sscanf(aerFailedSBDF, "%x:%x:%x.%x", &seg, &bus, &dev, &fn);
> > +
> > + libxl_unreg_aer_events_handler(ctx, domid);
>
> You need to be careful about calling a public API. In this case I think
> either calling libxl__ev_xswatch_deregister directly or having something
> like libxl__unregister_aer_events_handler(libxl__gc *gc, ...).
Will switch to using libxl__ev_xswatch_deregister().
> > +
> > + dds->ao = nested_ao;
> > + dds->domid = domid;
> > + dds->callback = domain_destroy_callback;
> > + libxl__domain_destroy(egc, dds);
> > +
> > + rc = xc_hide_device(ctx->xch, seg << 16 | bus << 8 | dev << 3 | fn);
>
> Please provide a helper function or macro to return sbdf form instead of
> open coding.
Will do.
> > + if (rc)
> > + LOGD(ERROR, domid, " xc_hide_device() failed, rc = %d", rc);
> > +
> > + rwperm[0].id = 0;
> > + rwperm[0].perms = XS_PERM_READ | XS_PERM_WRITE;
> > + dst_path = GCSPRINTF("/local/domain/0/backend/pci/0/0/%s",
> > "aerFailedPCIs");
> > + rc = libxl__xs_mknod(gc, XBT_NULL, dst_path, rwperm, 1);
> > + if (rc) {
> > + LOGD(ERROR, domid, " libxl__xs_mknod() failed, rc = %d", rc);
> > + goto skip;
> > + }
> > +
> > + rc = libxl__xs_write_checked(gc, XBT_NULL, dst_path, aerFailedSBDF);
> > + if (rc)
> > + LOGD(ERROR, domid, " libxl__xs_write_checked() failed, rc = %d",
> > rc);
> > +
> > +skip:
> > + return;
> > +}
> > +
> > +/* Handler of events for device driver domains */
> > +int libxl_reg_aer_events_handler(libxl_ctx *ctx, uint32_t domid)
> > +{
> > + AO_CREATE(ctx, 0, 0);
> > + int rc;
> > + char *be_path;
> > +
> > + /*
> > + * We use absolute paths because we want xswatch to also return
> > + * absolute paths that can be parsed by libxl__parse_backend_path.
> > + */
> > + aer_watch.ao = ao;
> > + aer_watch.domid = domid;
> > + be_path = GCSPRINTF("/local/domain/0/backend/pci/%u/0/aerFailedSBDF",
> > domid);
>
> Again, what about driver domain?
Please see [1] and [2] below:
>
> > + rc = libxl__ev_xswatch_register(gc, &aer_watch.watch,
> > + aer_backend_watch_callback, be_path);
> > + if (rc)
> > + return AO_CREATE_FAIL(rc);
> > +
> > + return AO_INPROGRESS;
> > +}
> > +
> > +/* Handler of events for device driver domains */
> > +void libxl_unreg_aer_events_handler(libxl_ctx *ctx, uint32_t domid)
>
> unregister
Will get rid of this wrapper.
> > +{
> > + GC_INIT(ctx);
> > +
> > + libxl__ev_xswatch_deregister(gc, &aer_watch.watch);
> >
>
> GC_FREE
>
> > + return;
> > +}
> > +
> > static void libxl__add_pcidevs(libxl__egc *egc, libxl__ao *ao, uint32_t
> > domid,
> > libxl_domain_config *d_config,
> > libxl__multidev *multidev)
> >
[1] https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg00552.html
[2] https://lists.xenproject.org/archives/html/xen-devel/2017-07/msg00555.html
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |