On Mon, 2 Aug 2010, Gianni Tedesco (3P) wrote:
> tools/libxl/libxl.h | 1 +
> tools/libxl/libxl_pci.c | 118
> ++++++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 111 insertions(+), 8 deletions(-)
>
>
> # HG changeset patch
> # User Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
> # Date 1280760698 -3600
> # Branch pci-patches-v3
> # Node ID 6379343447e58cc7b83e27beba9c35e62c232d9d
> # Parent eb8ee481bc063b18b39feaa76d9b757331ed3a9f
> xl: implement multifunction device PCI pass-through
>
> Implement PCI pass-through for multi-function devices. The supported BDF
> notation is: BB:DD.* - therefore passing-through a subset of functions or
> remapping the function numbers is not supported. Largely because I don't see
> how that can ever be safe or sane (perhaps for SR-IOV cards?)
>
> Letting qemu automatically select the virtual slot is not supported since I
> don't believe that qemu-dm can actually handle that case.
>
> Signed-off-by: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
>
> diff -r eb8ee481bc06 -r 6379343447e5 tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h Mon Aug 02 15:47:38 2010 +0100
> +++ b/tools/libxl/libxl.h Mon Aug 02 15:51:38 2010 +0100
> @@ -308,6 +308,7 @@ typedef struct {
> unsigned int vdevfn;
> bool msitranslate;
> bool power_mgmt;
> + bool multifunction;
> } libxl_device_pci;
>
> enum {
> diff -r eb8ee481bc06 -r 6379343447e5 tools/libxl/libxl_pci.c
> --- a/tools/libxl/libxl_pci.c Mon Aug 02 15:47:38 2010 +0100
> +++ b/tools/libxl/libxl_pci.c Mon Aug 02 15:51:38 2010 +0100
> @@ -136,8 +136,13 @@ int libxl_device_pci_parse_bdf(libxl_ctx
> break;
> }
> *ptr = '\0';
> - if ( hex_convert(tok, &func, 0x7) )
> - goto parse_error;
> + if ( !strcmp(tok, "*") ) {
> + pcidev->multifunction = 1;
> + }else{
> + if ( hex_convert(tok, &func, 0x7) )
> + goto parse_error;
> + pcidev->multifunction = 0;
> + }
> tok = ptr + 1;
> }
> break;
Couldn't you add the func_mask somewhere in libxl_device_pci, instead of
calling pci_multifunction_check multilple times?
Would be possible to make the normal passthrough case just a subset of
the general multifunction passthrough case to simplify the code?
> @@ -531,6 +536,50 @@ int libxl_device_pci_list_assignable(lib
> return 0;
> }
>
> +static int pci_multifunction_check(libxl_ctx *ctx, libxl_device_pci *pcidev,
> unsigned int *func_mask)
> +{
a comment on top of this function to explain what it is actually checking would
be nice
> + struct dirent *de;
> + DIR *dir;
> +
> + *func_mask = 0;
> +
> + dir = opendir(SYSFS_PCI_DEV);
> + if ( NULL == dir ) {
> + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Couldn't open %s", SYSFS_PCI_DEV);
> + return -1;
> + }
> +
> + while( (de = readdir(dir)) ) {
> + unsigned dom, bus, dev, func;
> + struct stat st;
> + char *path;
> +
> + if ( sscanf(de->d_name, PCI_BDF, &dom, &bus, &dev, &func) != 4 )
> + continue;
> + if ( pcidev->domain != dom )
> + continue;
> + if ( pcidev->bus != bus )
> + continue;
> + if ( pcidev->dev != dev )
> + continue;
> +
> + path = libxl_sprintf(ctx, "%s/" PCI_BDF, SYSFS_PCIBACK_DRIVER, dom,
> bus, dev, func);
> + if ( lstat(path, &st) ) {
> + if ( errno == ENOENT )
> + XL_LOG(ctx, XL_LOG_ERROR, PCI_BDF " is not assigned to
> pciback driver",
> + dom, bus, dev, func);
> + else
> + XL_LOG_ERRNO(ctx, XL_LOG_ERROR, "Couldn't lstat %s", path);
> + closedir(dir);
> + return -1;
> + }
> + (*func_mask) |= (1 << func);
> + }
> +
> + closedir(dir);
> + return 0;
> +}
> +
> static int pci_ins_check(libxl_ctx *ctx, uint32_t domid, const char *state,
> void *priv)
> {
> char *orig_state = priv;
> @@ -679,10 +728,32 @@ int libxl_device_pci_add(libxl_ctx *ctx,
> return rc;
> }
>
> + if ( pcidev->multifunction ) {
> + unsigned int i, orig_vdev, func_mask, rc = 0;
> + orig_vdev = pcidev->vdevfn & ~7U;
> +
> + if ( pci_multifunction_check(ctx, pcidev, &func_mask) )
> + return ERROR_FAIL;
> + if ( !(pcidev->vdevfn >> 3) ) {
> + XL_LOG(ctx, XL_LOG_ERROR, "Must specify a v-slot for
> multi-function devices");
> + return ERROR_FAIL;
> + }
> +
> + for(i = 7; i < 8; --i) {
shouldn't this be for(i = 7; i >= 0; --i)?
> + if ( (1 << i) & func_mask ) {
> + pcidev->func = i;
> + pcidev->vdevfn = orig_vdev | i;
> + if ( do_pci_add(ctx, domid, pcidev) )
> + rc = ERROR_FAIL;
> + }
> + }
> + return rc;
> + }
> +
> return do_pci_add(ctx, domid, pcidev);
> }
>
> -int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci
> *pcidev)
> +static int do_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci
> *pcidev)
> {
> libxl_device_pci *assigned;
> char *path;
> @@ -711,10 +782,15 @@ int libxl_device_pci_remove(libxl_ctx *c
> libxl_xs_write(ctx, XBT_NULL, path, PCI_BDF, pcidev->domain,
> pcidev->bus, pcidev->dev, pcidev->func);
> path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/command",
> domid);
> - xs_write(ctx->xsh, XBT_NULL, path, "pci-rem", strlen("pci-rem"));
> - if (libxl_wait_for_device_model(ctx, domid, "pci-removed", NULL,
> NULL) < 0) {
> - XL_LOG(ctx, XL_LOG_ERROR, "Device Model didn't respond in time");
> - return ERROR_FAIL;
> +
> + /* Remove all functions at once atomically by only signalling
> + * device-model for function 0 */
> + if ( !pcidev->multifunction || pcidev->func == 0 ) {
> + xs_write(ctx->xsh, XBT_NULL, path, "pci-rem", strlen("pci-rem"));
> + if (libxl_wait_for_device_model(ctx, domid, "pci-removed", NULL,
> NULL) < 0) {
> + XL_LOG(ctx, XL_LOG_ERROR, "Device Model didn't respond in
> time");
> + return ERROR_FAIL;
> + }
> }
> path = libxl_sprintf(ctx, "/local/domain/0/device-model/%d/state",
> domid);
> xs_write(ctx->xsh, XBT_NULL, path, state, strlen(state));
> @@ -769,7 +845,10 @@ skip1:
> fclose(f);
> }
> out:
> - libxl_device_pci_reset(ctx, pcidev->domain, pcidev->bus, pcidev->dev,
> pcidev->func);
> + /* don't do multiple resets while some functions are still passed
> through */
> + if ( !pcidev->multifunction || pcidev->func == 0 ) {
> + libxl_device_pci_reset(ctx, pcidev->domain, pcidev->bus,
> pcidev->dev, pcidev->func);
> + }
>
> if (!libxl_is_stubdom(ctx, domid, NULL)) {
> rc = xc_deassign_device(ctx->xch, domid, pcidev->value);
> @@ -786,6 +865,29 @@ out:
> return 0;
> }
>
> +int libxl_device_pci_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_pci
> *pcidev)
> +{
> + if ( pcidev->multifunction ) {
> + unsigned int i, orig_vdev, func_mask, rc;
> + orig_vdev = pcidev->vdevfn & ~7U;
> +
> + if ( pci_multifunction_check(ctx, pcidev, &func_mask) )
> + return ERROR_FAIL;
> +
> + for(i = 7; i < 8; --i) {
same here
> + if ( (1 << i) & func_mask ) {
> + pcidev->func = i;
> + pcidev->vdevfn = orig_vdev | i;
> + if ( do_pci_remove(ctx, domid, pcidev) )
> + rc = ERROR_FAIL;
> + }
> + }
> + return rc;
> + }
> +
> + return do_pci_remove(ctx, domid, pcidev);
> +}
> +
> int libxl_device_pci_list_assigned(libxl_ctx *ctx, libxl_device_pci **list,
> uint32_t domid, int *num)
> {
> char *be_path, *num_devs, *xsdev, *xsvdevfn, *xsopts;
>
applied all the other patches in the series
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home