[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 03/23] libxl: Make sure devices added by pci-attach are reflected in the config
On 12/3/20 3:17 PM, Paul Durrant wrote: >> -----Original Message----- >> From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx> >> Sent: 01 December 2020 13:12 >> To: Paul Durrant <paul@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx >> Cc: Paul Durrant <pdurrant@xxxxxxxxxx>; Ian Jackson <iwj@xxxxxxxxxxxxxx>; >> Wei Liu <wl@xxxxxxx>; >> Anthony PERARD <anthony.perard@xxxxxxxxxx> >> Subject: Re: [PATCH v4 03/23] libxl: Make sure devices added by pci-attach >> are reflected in the config >> >> Hi, Paul! >> >> On 11/24/20 10:01 AM, Paul Durrant wrote: >>> From: Paul Durrant <pdurrant@xxxxxxxxxx> >>> >>> Currently libxl__device_pci_add_xenstore() is broken in that does not >>> update the domain's configuration for the first device added (which causes >>> creation of the overall backend area in xenstore). This can be easily >>> observed >>> by running 'xl list -l' after adding a single device: the device will be >>> missing. >>> >>> This patch fixes the problem and adds a DEBUG log line to allow easy >>> verification that the domain configuration is being modified. Also, the use >>> of libxl__device_generic_add() is dropped as it leads to a confusing >>> situation >>> where only partial backend information is written under the xenstore >>> '/libxl' path. For LIBXL__DEVICE_KIND_PCI devices the only definitive >>> information in xenstore is under '/local/domain/0/backend' (the '0' being >>> hard-coded). >>> >>> NOTE: This patch includes a whitespace in add_pcis_done(). >>> >>> Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx> >>> --- >>> Cc: Ian Jackson <iwj@xxxxxxxxxxxxxx> >>> Cc: Wei Liu <wl@xxxxxxx> >>> Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx> >>> >>> v2: >>> - Avoid having two completely different ways of adding devices into >>> xenstore >>> >>> v3: >>> - Revert some changes form v2 as there is confusion over use of the libxl >>> and backend xenstore paths which needs to be fixed >>> --- >>> tools/libs/light/libxl_pci.c | 87 >>> +++++++++++++++++++++++--------------------- >>> 1 file changed, 45 insertions(+), 42 deletions(-) >>> >>> diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c >>> index 9d44b28f0a..da01c77ba2 100644 >>> --- a/tools/libs/light/libxl_pci.c >>> +++ b/tools/libs/light/libxl_pci.c >>> @@ -79,39 +79,55 @@ static void libxl__device_from_pci(libxl__gc *gc, >>> uint32_t domid, >>> device->kind = LIBXL__DEVICE_KIND_PCI; >>> } >>> >>> -static int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid, >>> - const libxl_device_pci *pci, >>> - int num) >>> +static void libxl__create_pci_backend(libxl__gc *gc, xs_transaction_t t, >>> + uint32_t domid, const >>> libxl_device_pci *pci) >>> { >>> - flexarray_t *front = NULL; >>> - flexarray_t *back = NULL; >>> - libxl__device device; >>> - int i; >>> + libxl_ctx *ctx = libxl__gc_owner(gc); >>> + flexarray_t *front, *back; >>> + char *fe_path, *be_path; >>> + struct xs_permissions fe_perms[2], be_perms[2]; >>> + >>> + LOGD(DEBUG, domid, "Creating pci backend"); >>> >>> front = flexarray_make(gc, 16, 1); >>> back = flexarray_make(gc, 16, 1); >>> >>> - LOGD(DEBUG, domid, "Creating pci backend"); >>> - >>> - /* add pci device */ >>> - libxl__device_from_pci(gc, domid, pci, &device); >>> + fe_path = libxl__domain_device_frontend_path(gc, domid, 0, >>> + LIBXL__DEVICE_KIND_PCI); >>> + be_path = libxl__domain_device_backend_path(gc, 0, domid, 0, >>> + LIBXL__DEVICE_KIND_PCI); >>> >>> + flexarray_append_pair(back, "frontend", fe_path); >>> flexarray_append_pair(back, "frontend-id", GCSPRINTF("%d", domid)); >>> - flexarray_append_pair(back, "online", "1"); >>> + flexarray_append_pair(back, "online", GCSPRINTF("%d", 1)); >>> flexarray_append_pair(back, "state", GCSPRINTF("%d", >>> XenbusStateInitialising)); >>> flexarray_append_pair(back, "domain", libxl__domid_to_name(gc, >>> domid)); >>> >>> - for (i = 0; i < num; i++, pci++) >>> - libxl_create_pci_backend_device(gc, back, i, pci); >>> + be_perms[0].id = 0; >> There was a discussion [1] on PCI on ARM and one of the question was that it >> is possible >> >> that we have the pci backend running in a late hardware domain/driver >> domain, which may >> >> not be Domain-0. Do you think we can avoid using 0 here and get some clue of >> the domain >> >> from "*backend=domain-id"? If not set it will return Domain-0's ID and won't >> break anything* > Not sure what you're asking for since... My bad, please ignore Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx> Thank you, Oleksandr > >> *Thank you,* >> >> *Oleksandr >> * >> >>> + be_perms[0].perms = XS_PERM_NONE; >>> + be_perms[1].id = domid; >>> + be_perms[1].perms = XS_PERM_READ; >>> + >>> + xs_rm(ctx->xsh, t, be_path); >>> + xs_mkdir(ctx->xsh, t, be_path); >>> + xs_set_permissions(ctx->xsh, t, be_path, be_perms, >>> + ARRAY_SIZE(be_perms)); >>> + libxl__xs_writev(gc, t, be_path, libxl__xs_kvs_of_flexarray(gc, back)); >>> >>> - flexarray_append_pair(back, "num_devs", GCSPRINTF("%d", num)); >>> + flexarray_append_pair(front, "backend", be_path); >>> flexarray_append_pair(front, "backend-id", GCSPRINTF("%d", 0)); > ... backend-id is written here. > > Paul > >
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |