[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Xen-devel] [PATCH V4 05/10] Introduce HostPCIDevice to
From: |
Konrad Rzeszutek Wilk |
Subject: |
Re: [Qemu-devel] [Xen-devel] [PATCH V4 05/10] Introduce HostPCIDevice to access a pci device on the host. |
Date: |
Fri, 18 Nov 2011 10:06:22 -0500 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Thu, Nov 17, 2011 at 03:17:08PM +0000, Anthony PERARD wrote:
> Signed-off-by: Anthony PERARD <address@hidden>
> ---
> Makefile.target | 1 +
> hw/host-pci-device.c | 279
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> hw/host-pci-device.h | 75 ++++++++++++++
> 3 files changed, 355 insertions(+), 0 deletions(-)
> create mode 100644 hw/host-pci-device.c
> create mode 100644 hw/host-pci-device.h
>
> diff --git a/Makefile.target b/Makefile.target
> index 2e881ce..e527c1b 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -220,6 +220,7 @@ obj-$(CONFIG_NO_XEN) += xen-stub.o
> obj-i386-$(CONFIG_XEN) += xen_platform.o
>
> # Xen PCI Passthrough
> +obj-i386-$(CONFIG_XEN_PCI_PASSTHROUGH) += host-pci-device.o
>
> # Inter-VM PCI shared memory
> CONFIG_IVSHMEM =
> diff --git a/hw/host-pci-device.c b/hw/host-pci-device.c
> new file mode 100644
> index 0000000..06f7761
> --- /dev/null
> +++ b/hw/host-pci-device.c
> @@ -0,0 +1,279 @@
> +/*
> + * Copyright (C) 2011 Citrix Ltd.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu-common.h"
> +#include "host-pci-device.h"
> +
> +#define PCI_MAX_EXT_CAP \
> + ((PCIE_CONFIG_SPACE_SIZE - PCI_CONFIG_SPACE_SIZE) / (PCI_CAP_SIZEOF + 4))
> +
> +enum error_code {
> + ERROR_SYNTAX = 1,
> +};
> +
> +static int path_to(const HostPCIDevice *d,
> + const char *name, char *buf, ssize_t size)
> +{
> + return snprintf(buf, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%x/%s",
> + d->domain, d->bus, d->dev, d->func, name);
> +}
> +
> +static int get_resource(HostPCIDevice *d)
> +{
> + int i, rc = 0;
> + FILE *f;
> + char path[PATH_MAX];
> + unsigned long long start, end, flags, size;
> +
> + path_to(d, "resource", path, sizeof (path));
> + f = fopen(path, "r");
> + if (!f) {
> + fprintf(stderr, "Error: Can't open %s: %s\n", path, strerror(errno));
> + return -errno;
> + }
> +
> + for (i = 0; i < PCI_NUM_REGIONS; i++) {
> + if (fscanf(f, "%llx %llx %llx", &start, &end, &flags) != 3) {
> + fprintf(stderr, "Error: Syntax error in %s\n", path);
> + rc = ERROR_SYNTAX;
> + break;
> + }
> + if (start) {
> + size = end - start + 1;
> + } else {
> + size = 0;
> + }
> +
> + if (i < PCI_ROM_SLOT) {
> + d->io_regions[i].base_addr = start;
> + d->io_regions[i].size = size;
> + d->io_regions[i].flags = flags;
> + } else {
> + d->rom.base_addr = start;
> + d->rom.size = size;
> + d->rom.flags = flags;
> + }
> + }
> +
> + fclose(f);
> + return rc;
> +}
> +
> +static int get_value(HostPCIDevice *d, const char *name, unsigned long
> *pvalue)
Perhaps 'get_hex_value'? You are just doing %lx, not %ld, so it has
to have to an exact format to get the contents right.
> +{
> + char path[PATH_MAX];
> + FILE *f;
> + unsigned long value;
> +
> + path_to(d, name, path, sizeof (path));
> + f = fopen(path, "r");
> + if (!f) {
> + fprintf(stderr, "Error: Can't open %s: %s\n", path, strerror(errno));
> + return -1;
So the get_resource can return 0, 1, or -errno. This one can return
0, or -1. Would it make sense to duplicate the -errno mechanism
that you employed in get_resources to have uniformity?
> + }
> + if (fscanf(f, "%lx\n", &value) != 1) {
> + fprintf(stderr, "Error: Syntax error in %s\n", path);
> + return -1;
> + }
> + fclose(f);
> + *pvalue = value;
> + return 0;
> +}
> +
> +static bool pci_dev_is_virtfn(HostPCIDevice *d)
> +{
> + int rc;
> + char path[PATH_MAX];
> + struct stat buf;
> +
> + path_to(d, "physfn", path, sizeof (path));
> + rc = !stat(path, &buf);
> +
> + return rc;
> +}
> +
> +static int host_pci_config_fd(HostPCIDevice *d)
> +{
> + char path[PATH_MAX];
> +
> + if (d->config_fd < 0) {
> + path_to(d, "config", path, sizeof (path));
> + d->config_fd = open(path, O_RDWR);
> + if (d->config_fd < 0) {
> + fprintf(stderr, "HostPCIDevice: Can not open '%s': %s\n",
> + path, strerror(errno));
> + }
> + }
> + return d->config_fd;
> +}
> +static int host_pci_config_read(HostPCIDevice *d, int pos, void *buf, int
> len)
> +{
> + int fd = host_pci_config_fd(d);
> + int res = 0;
> +
> +again:
> + res = pread(fd, buf, len, pos);
> + if (res != len) {
> + if (res < 0 && (errno == EINTR || errno == EAGAIN)) {
> + goto again;
> + }
> + fprintf(stderr, "host_pci_config: read failed: %s (fd: %i)\n",
Well, the comment says 'host_pci_config', but that is not the name
of the function. Perhaps replace it with '%s' and use __func__
as one of the parameters?
> + strerror(errno), fd);
> + return -errno;
> + }
> + return 0;
> +}
> +static int host_pci_config_write(HostPCIDevice *d,
> + int pos, const void *buf, int len)
> +{
> + int fd = host_pci_config_fd(d);
> + int res = 0;
> +
> +again:
> + res = pwrite(fd, buf, len, pos);
> + if (res != len) {
> + if (res < 0 && (errno == EINTR || errno == EAGAIN)) {
> + goto again;
> + }
> + fprintf(stderr, "host_pci_config: write failed: %s\n",
> + strerror(errno));
> + return -errno;
> + }
> + return 0;
> +}
> +
> +int host_pci_get_byte(HostPCIDevice *d, int pos, uint8_t *p)
> +{
> + uint8_t buf;
> + if (host_pci_config_read(d, pos, &buf, 1)) {
> + return -1;
Would it make sense to use the nice enum you decleraed at the
top of the file? Or not?
- [Qemu-devel] [PATCH V4 00/10] Xen PCI Passthrough, Anthony PERARD, 2011/11/17
- [Qemu-devel] [PATCH V4 01/10] pci_ids: Add INTEL_82599_VF id., Anthony PERARD, 2011/11/17
- [Qemu-devel] [PATCH V4 02/10] pci_regs: Fix value of PCI_EXP_TYPE_RC_EC., Anthony PERARD, 2011/11/17
- [Qemu-devel] [PATCH V4 03/10] pci_regs: Add PCI_EXP_TYPE_PCIE_BRIDGE, Anthony PERARD, 2011/11/17
- [Qemu-devel] [PATCH V4 05/10] Introduce HostPCIDevice to access a pci device on the host., Anthony PERARD, 2011/11/17
- Re: [Qemu-devel] [Xen-devel] [PATCH V4 05/10] Introduce HostPCIDevice to access a pci device on the host.,
Konrad Rzeszutek Wilk <=
- [Qemu-devel] [PATCH V4 04/10] configure: Introduce --enable-xen-pci-passthrough., Anthony PERARD, 2011/11/17
- [Qemu-devel] [PATCH V4 06/10] pci.c: Add pci_check_bar_overlap, Anthony PERARD, 2011/11/17
- [Qemu-devel] [PATCH V4 07/10] Introduce Xen PCI Passthrough, qdevice (1/3), Anthony PERARD, 2011/11/17
- [Qemu-devel] [PATCH V4 08/10] Introduce Xen PCI Passthrough, PCI config space helpers (2/3), Anthony PERARD, 2011/11/17
- [Qemu-devel] [PATCH V4 09/10] Introduce apic-msidef.h, Anthony PERARD, 2011/11/17
- [Qemu-devel] [PATCH V4 10/10] Introduce Xen PCI Passthrough, MSI (3/3), Anthony PERARD, 2011/11/17
- Re: [Qemu-devel] [PATCH V4 00/10] Xen PCI Passthrough, Stefano Stabellini, 2011/11/18