Patchwork [RFC,V1,01/11] Introduce HostPCIDevice to access a pci device on the host.

login
register
mail settings
Submitter Anthony PERARD
Date Oct. 4, 2011, 2:51 p.m.
Message ID <1317739882-4809-2-git-send-email-anthony.perard@citrix.com>
Download mbox | patch
Permalink /patch/117635/
State New
Headers show

Comments

Anthony PERARD - Oct. 4, 2011, 2:51 p.m.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 hw/host-pci-device.c |  192 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/host-pci-device.h |   36 +++++++++
 2 files changed, 228 insertions(+), 0 deletions(-)
 create mode 100644 hw/host-pci-device.c
 create mode 100644 hw/host-pci-device.h
Jan Kiszka - Oct. 4, 2011, 6:21 p.m.
This wasn't run through checkpatch.pl, I bet.

On 2011-10-04 16:51, Anthony PERARD wrote:
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  hw/host-pci-device.c |  192 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/host-pci-device.h |   36 +++++++++
>  2 files changed, 228 insertions(+), 0 deletions(-)
>  create mode 100644 hw/host-pci-device.c
>  create mode 100644 hw/host-pci-device.h
> 
> diff --git a/hw/host-pci-device.c b/hw/host-pci-device.c
> new file mode 100644
> index 0000000..b3f2899
> --- /dev/null
> +++ b/hw/host-pci-device.c
> @@ -0,0 +1,192 @@
> +#include "qemu-common.h"
> +#include "host-pci-device.h"
> +
> +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;
> +    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 -1;
> +    }
> +
> +    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);
> +            break;
> +        }
> +        if (start) {
> +            size = end - start + 1;
> +        } else {
> +            size = 0;
> +        }
> +
> +        flags &= 0xf;

No magic numbers please.

It also looks a bit strange to me: It's the resource type encoded in the
second byte? Aren't you interested in it?

> +
> +        if (i < PCI_ROM_SLOT) {
> +            d->base_addr[i] = start | flags;
> +            d->size[i] = size;
> +        } else {
> +            d->rom_base_addr = start | flags;
> +            d->rom_size = size;
> +        }
> +    }
> +
> +    fclose(f);
> +    return 0;
> +}
> +
> +static unsigned long get_value(HostPCIDevice *d, const char *name)
> +{
> +    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;
> +    }
> +    if (fscanf(f, "%lx\n", &value) != 1) {
> +        fprintf(stderr, "Error: Syntax error in %s\n", path);
> +        value = -1;
> +    }
> +    fclose(f);
> +    return value;
> +}
> +
> +static int 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)

[ We will also need the reverse: pass in open file descriptors that
HostPCIDevice should use. Can be added later. ]

> +{
> +    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;
> +
> +    res = pread(fd, buf, len, pos);
> +    if (res < 0) {
> +        fprintf(stderr, "host_pci_config: read failed: %s (fd: %i)\n",
> +                strerror(errno), fd);
> +        return -1;
> +    }
> +    return res;
> +}
> +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;
> +
> +    res = pwrite(fd, buf, len, pos);
> +    if (res < 0) {
> +        fprintf(stderr, "host_pci_config: write failed: %s\n",
> +                strerror(errno));
> +        return -1;
> +    }
> +    return res;
> +}
> +
> +uint8_t host_pci_read_byte(HostPCIDevice *d, int pos)
> +{
> +  uint8_t buf;
> +  host_pci_config_read(d, pos, &buf, 1);
> +  return buf;
> +}
> +uint16_t host_pci_read_word(HostPCIDevice *d, int pos)
> +{
> +  uint16_t buf;
> +  host_pci_config_read(d, pos, &buf, 2);
> +  return le16_to_cpu(buf);
> +}
> +uint32_t host_pci_read_long(HostPCIDevice *d, int pos)
> +{
> +  uint32_t buf;
> +  host_pci_config_read(d, pos, &buf, 4);
> +  return le32_to_cpu(buf);
> +}
> +int host_pci_read_block(HostPCIDevice *d, int pos, uint8_t *buf, int len)
> +{
> +  return host_pci_config_read(d, pos, buf, len);
> +}
> +
> +int host_pci_write_byte(HostPCIDevice *d, int pos, uint8_t data)
> +{
> +  return host_pci_config_write(d, pos, &data, 1);
> +}
> +int host_pci_write_word(HostPCIDevice *d, int pos, uint16_t data)
> +{
> +  return host_pci_config_write(d, pos, &data, 2);

You adjust endianess on read, but not on write.

> +}
> +int host_pci_write_long(HostPCIDevice *d, int pos, uint32_t data)
> +{
> +  return host_pci_config_write(d, pos, &data, 4);
> +}
> +int host_pci_write_block(HostPCIDevice *d, int pos, uint8_t *buf, int len)
> +{
> +  return host_pci_config_write(d, pos, buf, len);
> +}
> +
> +HostPCIDevice *host_pci_device_get(uint8_t bus, uint8_t dev, uint8_t func)
> +{
> +    HostPCIDevice *d = NULL;
> +
> +    d = g_new0(HostPCIDevice, 1);
> +
> +    d->config_fd = -1;
> +    d->domain = 0;
> +    d->bus = bus;
> +    d->dev = dev;
> +    d->func = func;
> +
> +    if (host_pci_config_fd(d) == -1)
> +        goto error;
> +    if (get_resource(d) == -1)
> +        goto error;
> +
> +    d->vendor_id = get_value(d, "vendor");
> +    d->device_id = get_value(d, "device");
> +    d->is_virtfn = pci_dev_is_virtfn(d);
> +
> +    return d;
> +error:
> +    if (d->config_fd >= 0)
> +        close(d->config_fd);
> +    g_free(d);
> +    return NULL;
> +}
> diff --git a/hw/host-pci-device.h b/hw/host-pci-device.h
> new file mode 100644
> index 0000000..0137507
> --- /dev/null
> +++ b/hw/host-pci-device.h
> @@ -0,0 +1,36 @@
> +#ifndef HW_HOST_PCI_DEVICE
> +#  define HW_HOST_PCI_DEVICE
> +
> +#include "pci.h"
> +
> +typedef struct HostPCIDevice {
> +    uint16_t domain;
> +    uint8_t bus;
> +    uint8_t dev;
> +    uint8_t func;
> +
> +    uint16_t vendor_id;
> +    uint16_t device_id;
> +
> +    pcibus_t base_addr[PCI_NUM_REGIONS - 1];
> +    pcibus_t size[PCI_NUM_REGIONS - 1];
> +    pcibus_t rom_base_addr;
> +    pcibus_t rom_size;

Regions deserve their own type IMHO. In KVM we have

typedef struct {
    int type;           /* Memory or port I/O */
    int valid;
    uint32_t base_addr;
    uint32_t size;    /* size of the region */
    int resource_fd;
} PCIRegion;

Should probably become HostPCIIORegion (vs. virtual PCIIORegion), and
our field types need some cleanups.

> +
> +    bool is_virtfn;
> +
> +    int config_fd;
> +} HostPCIDevice;
> +
> +HostPCIDevice *host_pci_device_get(uint8_t bus, uint8_t dev, uint8_t func);

And what about some host_pci_device_put when we're done with it?

> +
> +uint8_t host_pci_read_byte(HostPCIDevice *d, int pos);
> +uint16_t host_pci_read_word(HostPCIDevice *d, int pos);
> +uint32_t host_pci_read_long(HostPCIDevice *d, int pos);
> +int host_pci_read_block(HostPCIDevice *d, int pos, uint8_t *buf, int len);
> +int host_pci_write_byte(HostPCIDevice *d, int pos, uint8_t data);
> +int host_pci_write_word(HostPCIDevice *d, int pos, uint16_t data);
> +int host_pci_write_long(HostPCIDevice *d, int pos, uint32_t data);
> +int host_pci_write_block(HostPCIDevice *d, int pos, uint8_t *buf, int len);

I think these should be analogous to our pci layer:
host_pci_get/set_byte/word/long/quad.

Looks like it's generally useful for KVM as well.

Jan
Stefano Stabellini - Oct. 4, 2011, 6:36 p.m.
On Tue, 4 Oct 2011, Anthony PERARD wrote:
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  hw/host-pci-device.c |  192 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/host-pci-device.h |   36 +++++++++
>  2 files changed, 228 insertions(+), 0 deletions(-)
>  create mode 100644 hw/host-pci-device.c
>  create mode 100644 hw/host-pci-device.h
> 
> diff --git a/hw/host-pci-device.c b/hw/host-pci-device.c
> new file mode 100644
> index 0000000..b3f2899
> --- /dev/null
> +++ b/hw/host-pci-device.c
> @@ -0,0 +1,192 @@
> +#include "qemu-common.h"
> +#include "host-pci-device.h"
> +
> +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);
> +}

This is all very Linux specific so at the very least it should only be
compiled if CONFIG_LINUX.


> +static int get_resource(HostPCIDevice *d)
> +{
> +    int i;
> +    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 -1;
> +    }
> +
> +    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);
> +            break;
> +        }

you should return error in this case


> +        if (start) {
> +            size = end - start + 1;
> +        } else {
> +            size = 0;
> +        }
> +
> +        flags &= 0xf;

please #define what mask is this 0xf


> +        if (i < PCI_ROM_SLOT) {
> +            d->base_addr[i] = start | flags;
> +            d->size[i] = size;
> +        } else {
> +            d->rom_base_addr = start | flags;
> +            d->rom_size = size;
> +        }
> +    }
> +
> +    fclose(f);
> +    return 0;
> +}
> +
> +static unsigned long get_value(HostPCIDevice *d, const char *name)
> +{
> +    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;
> +    }
> +    if (fscanf(f, "%lx\n", &value) != 1) {
> +        fprintf(stderr, "Error: Syntax error in %s\n", path);
> +        value = -1;
> +    }
> +    fclose(f);
> +    return value;
> +}
> +
> +static int 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;
> +
> +    res = pread(fd, buf, len, pos);
> +    if (res < 0) {
> +        fprintf(stderr, "host_pci_config: read failed: %s (fd: %i)\n",
> +                strerror(errno), fd);
> +        return -1;
> +    }
> +    return res;
> +}
> +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;
> +
> +    res = pwrite(fd, buf, len, pos);
> +    if (res < 0) {
> +        fprintf(stderr, "host_pci_config: write failed: %s\n",
> +                strerror(errno));
> +        return -1;
> +    }
> +    return res;
> +}
> +
> +uint8_t host_pci_read_byte(HostPCIDevice *d, int pos)
> +{
> +  uint8_t buf;
> +  host_pci_config_read(d, pos, &buf, 1);
> +  return buf;
> +}
> +uint16_t host_pci_read_word(HostPCIDevice *d, int pos)
> +{
> +  uint16_t buf;
> +  host_pci_config_read(d, pos, &buf, 2);
> +  return le16_to_cpu(buf);
> +}
> +uint32_t host_pci_read_long(HostPCIDevice *d, int pos)
> +{
> +  uint32_t buf;
> +  host_pci_config_read(d, pos, &buf, 4);
> +  return le32_to_cpu(buf);
> +}
> +int host_pci_read_block(HostPCIDevice *d, int pos, uint8_t *buf, int len)
> +{
> +  return host_pci_config_read(d, pos, buf, len);
> +}
> +
> +int host_pci_write_byte(HostPCIDevice *d, int pos, uint8_t data)
> +{
> +  return host_pci_config_write(d, pos, &data, 1);
> +}
> +int host_pci_write_word(HostPCIDevice *d, int pos, uint16_t data)
> +{
> +  return host_pci_config_write(d, pos, &data, 2);
> +}
> +int host_pci_write_long(HostPCIDevice *d, int pos, uint32_t data)
> +{
> +  return host_pci_config_write(d, pos, &data, 4);
> +}
> +int host_pci_write_block(HostPCIDevice *d, int pos, uint8_t *buf, int len)
> +{
> +  return host_pci_config_write(d, pos, buf, len);
> +}
> +
> +HostPCIDevice *host_pci_device_get(uint8_t bus, uint8_t dev, uint8_t func)
> +{
> +    HostPCIDevice *d = NULL;
> +
> +    d = g_new0(HostPCIDevice, 1);
> +
> +    d->config_fd = -1;
> +    d->domain = 0;
> +    d->bus = bus;
> +    d->dev = dev;
> +    d->func = func;
> +
> +    if (host_pci_config_fd(d) == -1)
> +        goto error;
> +    if (get_resource(d) == -1)
> +        goto error;
> +
> +    d->vendor_id = get_value(d, "vendor");
> +    d->device_id = get_value(d, "device");
> +    d->is_virtfn = pci_dev_is_virtfn(d);
> +
> +    return d;
> +error:
> +    if (d->config_fd >= 0)
> +        close(d->config_fd);
> +    g_free(d);
> +    return NULL;
> +}
> diff --git a/hw/host-pci-device.h b/hw/host-pci-device.h
> new file mode 100644
> index 0000000..0137507
> --- /dev/null
> +++ b/hw/host-pci-device.h
> @@ -0,0 +1,36 @@
> +#ifndef HW_HOST_PCI_DEVICE
> +#  define HW_HOST_PCI_DEVICE
> +
> +#include "pci.h"
> +
> +typedef struct HostPCIDevice {
> +    uint16_t domain;
> +    uint8_t bus;
> +    uint8_t dev;
> +    uint8_t func;
> +
> +    uint16_t vendor_id;
> +    uint16_t device_id;
> +
> +    pcibus_t base_addr[PCI_NUM_REGIONS - 1];
> +    pcibus_t size[PCI_NUM_REGIONS - 1];
> +    pcibus_t rom_base_addr;
> +    pcibus_t rom_size;
> +
> +    bool is_virtfn;
> +
> +    int config_fd;
> +} HostPCIDevice;
> +
> +HostPCIDevice *host_pci_device_get(uint8_t bus, uint8_t dev, uint8_t func);
> +
> +uint8_t host_pci_read_byte(HostPCIDevice *d, int pos);
> +uint16_t host_pci_read_word(HostPCIDevice *d, int pos);
> +uint32_t host_pci_read_long(HostPCIDevice *d, int pos);
> +int host_pci_read_block(HostPCIDevice *d, int pos, uint8_t *buf, int len);
> +int host_pci_write_byte(HostPCIDevice *d, int pos, uint8_t data);
> +int host_pci_write_word(HostPCIDevice *d, int pos, uint16_t data);
> +int host_pci_write_long(HostPCIDevice *d, int pos, uint32_t data);
> +int host_pci_write_block(HostPCIDevice *d, int pos, uint8_t *buf, int len);
> +
> +#endif /* !HW_HOST_PCI_DEVICE */
Anthony PERARD - Oct. 12, 2011, 4:56 p.m.
On Tue, Oct 4, 2011 at 19:21, Jan Kiszka <jan.kiszka@web.de> wrote:
> This wasn't run through checkpatch.pl, I bet.
>
> On 2011-10-04 16:51, Anthony PERARD wrote:
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
>>  hw/host-pci-device.c |  192 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  hw/host-pci-device.h |   36 +++++++++
>>  2 files changed, 228 insertions(+), 0 deletions(-)
>>  create mode 100644 hw/host-pci-device.c
>>  create mode 100644 hw/host-pci-device.h
>>
>> diff --git a/hw/host-pci-device.c b/hw/host-pci-device.c
>> new file mode 100644
>> index 0000000..b3f2899
>> --- /dev/null
>> +++ b/hw/host-pci-device.c
>> @@ -0,0 +1,192 @@
>> +#include "qemu-common.h"
>> +#include "host-pci-device.h"
>> +
>> +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;
>> +    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 -1;
>> +    }
>> +
>> +    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);
>> +            break;
>> +        }
>> +        if (start) {
>> +            size = end - start + 1;
>> +        } else {
>> +            size = 0;
>> +        }
>> +
>> +        flags &= 0xf;
>
> No magic numbers please.
>
> It also looks a bit strange to me: It's the resource type encoded in the
> second byte? Aren't you interested in it?

Actually, we are interessted to have the BAR with the different flags
(IO/MEM, prefetch, size) like the value in the config space. Because
the base_address value will be given to the VM (by the function
pt_bar_reg_read). But I can later merge the values (start | (flags &
~pci_base_address_io/mem_mask)), and have the right value to give
back.

So here, I will keep seperate the base address, and the flags.

>> +
>> +        if (i < PCI_ROM_SLOT) {
>> +            d->base_addr[i] = start | flags;
>> +            d->size[i] = size;
>> +        } else {
>> +            d->rom_base_addr = start | flags;
>> +            d->rom_size = size;
>> +        }
>> +    }
>> +
>> +    fclose(f);
>> +    return 0;
>> +}
>> +
>> +static unsigned long get_value(HostPCIDevice *d, const char *name)
>> +{
>> +    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;
>> +    }
>> +    if (fscanf(f, "%lx\n", &value) != 1) {
>> +        fprintf(stderr, "Error: Syntax error in %s\n", path);
>> +        value = -1;
>> +    }
>> +    fclose(f);
>> +    return value;
>> +}
>> +
>> +static int 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)
>
> [ We will also need the reverse: pass in open file descriptors that
> HostPCIDevice should use. Can be added later. ]
>
>> +{
>> +    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;
>> +
>> +    res = pread(fd, buf, len, pos);
>> +    if (res < 0) {
>> +        fprintf(stderr, "host_pci_config: read failed: %s (fd: %i)\n",
>> +                strerror(errno), fd);
>> +        return -1;
>> +    }
>> +    return res;
>> +}
>> +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;
>> +
>> +    res = pwrite(fd, buf, len, pos);
>> +    if (res < 0) {
>> +        fprintf(stderr, "host_pci_config: write failed: %s\n",
>> +                strerror(errno));
>> +        return -1;
>> +    }
>> +    return res;
>> +}
>> +
>> +uint8_t host_pci_read_byte(HostPCIDevice *d, int pos)
>> +{
>> +  uint8_t buf;
>> +  host_pci_config_read(d, pos, &buf, 1);
>> +  return buf;
>> +}
>> +uint16_t host_pci_read_word(HostPCIDevice *d, int pos)
>> +{
>> +  uint16_t buf;
>> +  host_pci_config_read(d, pos, &buf, 2);
>> +  return le16_to_cpu(buf);
>> +}
>> +uint32_t host_pci_read_long(HostPCIDevice *d, int pos)
>> +{
>> +  uint32_t buf;
>> +  host_pci_config_read(d, pos, &buf, 4);
>> +  return le32_to_cpu(buf);
>> +}
>> +int host_pci_read_block(HostPCIDevice *d, int pos, uint8_t *buf, int len)
>> +{
>> +  return host_pci_config_read(d, pos, buf, len);
>> +}
>> +
>> +int host_pci_write_byte(HostPCIDevice *d, int pos, uint8_t data)
>> +{
>> +  return host_pci_config_write(d, pos, &data, 1);
>> +}
>> +int host_pci_write_word(HostPCIDevice *d, int pos, uint16_t data)
>> +{
>> +  return host_pci_config_write(d, pos, &data, 2);
>
> You adjust endianess on read, but not on write.

Will fix that.

>> +}
>> +int host_pci_write_long(HostPCIDevice *d, int pos, uint32_t data)
>> +{
>> +  return host_pci_config_write(d, pos, &data, 4);
>> +}
>> +int host_pci_write_block(HostPCIDevice *d, int pos, uint8_t *buf, int len)
>> +{
>> +  return host_pci_config_write(d, pos, buf, len);
>> +}
>> +
>> +HostPCIDevice *host_pci_device_get(uint8_t bus, uint8_t dev, uint8_t func)
>> +{
>> +    HostPCIDevice *d = NULL;
>> +
>> +    d = g_new0(HostPCIDevice, 1);
>> +
>> +    d->config_fd = -1;
>> +    d->domain = 0;
>> +    d->bus = bus;
>> +    d->dev = dev;
>> +    d->func = func;
>> +
>> +    if (host_pci_config_fd(d) == -1)
>> +        goto error;
>> +    if (get_resource(d) == -1)
>> +        goto error;
>> +
>> +    d->vendor_id = get_value(d, "vendor");
>> +    d->device_id = get_value(d, "device");
>> +    d->is_virtfn = pci_dev_is_virtfn(d);
>> +
>> +    return d;
>> +error:
>> +    if (d->config_fd >= 0)
>> +        close(d->config_fd);
>> +    g_free(d);
>> +    return NULL;
>> +}
>> diff --git a/hw/host-pci-device.h b/hw/host-pci-device.h
>> new file mode 100644
>> index 0000000..0137507
>> --- /dev/null
>> +++ b/hw/host-pci-device.h
>> @@ -0,0 +1,36 @@
>> +#ifndef HW_HOST_PCI_DEVICE
>> +#  define HW_HOST_PCI_DEVICE
>> +
>> +#include "pci.h"
>> +
>> +typedef struct HostPCIDevice {
>> +    uint16_t domain;
>> +    uint8_t bus;
>> +    uint8_t dev;
>> +    uint8_t func;
>> +
>> +    uint16_t vendor_id;
>> +    uint16_t device_id;
>> +
>> +    pcibus_t base_addr[PCI_NUM_REGIONS - 1];
>> +    pcibus_t size[PCI_NUM_REGIONS - 1];
>> +    pcibus_t rom_base_addr;
>> +    pcibus_t rom_size;
>
> Regions deserve their own type IMHO. In KVM we have
>
> typedef struct {
>    int type;           /* Memory or port I/O */
>    int valid;
>    uint32_t base_addr;
>    uint32_t size;    /* size of the region */
>    int resource_fd;
> } PCIRegion;
>
> Should probably become HostPCIIORegion (vs. virtual PCIIORegion), and
> our field types need some cleanups.

I will do that, but I think to have only base_addr, size and flags.
flags will be the flags given by the sysfs resource file. Is that OK ?

>> +
>> +    bool is_virtfn;
>> +
>> +    int config_fd;
>> +} HostPCIDevice;
>> +
>> +HostPCIDevice *host_pci_device_get(uint8_t bus, uint8_t dev, uint8_t func);
>
> And what about some host_pci_device_put when we're done with it?

Will do it.

>> +
>> +uint8_t host_pci_read_byte(HostPCIDevice *d, int pos);
>> +uint16_t host_pci_read_word(HostPCIDevice *d, int pos);
>> +uint32_t host_pci_read_long(HostPCIDevice *d, int pos);
>> +int host_pci_read_block(HostPCIDevice *d, int pos, uint8_t *buf, int len);
>> +int host_pci_write_byte(HostPCIDevice *d, int pos, uint8_t data);
>> +int host_pci_write_word(HostPCIDevice *d, int pos, uint16_t data);
>> +int host_pci_write_long(HostPCIDevice *d, int pos, uint32_t data);
>> +int host_pci_write_block(HostPCIDevice *d, int pos, uint8_t *buf, int len);
>
> I think these should be analogous to our pci layer:
> host_pci_get/set_byte/word/long/quad.

Yes, I will change that.

> Looks like it's generally useful for KVM as well.

Thanks,

Patch

diff --git a/hw/host-pci-device.c b/hw/host-pci-device.c
new file mode 100644
index 0000000..b3f2899
--- /dev/null
+++ b/hw/host-pci-device.c
@@ -0,0 +1,192 @@ 
+#include "qemu-common.h"
+#include "host-pci-device.h"
+
+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;
+    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 -1;
+    }
+
+    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);
+            break;
+        }
+        if (start) {
+            size = end - start + 1;
+        } else {
+            size = 0;
+        }
+
+        flags &= 0xf;
+
+        if (i < PCI_ROM_SLOT) {
+            d->base_addr[i] = start | flags;
+            d->size[i] = size;
+        } else {
+            d->rom_base_addr = start | flags;
+            d->rom_size = size;
+        }
+    }
+
+    fclose(f);
+    return 0;
+}
+
+static unsigned long get_value(HostPCIDevice *d, const char *name)
+{
+    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;
+    }
+    if (fscanf(f, "%lx\n", &value) != 1) {
+        fprintf(stderr, "Error: Syntax error in %s\n", path);
+        value = -1;
+    }
+    fclose(f);
+    return value;
+}
+
+static int 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;
+
+    res = pread(fd, buf, len, pos);
+    if (res < 0) {
+        fprintf(stderr, "host_pci_config: read failed: %s (fd: %i)\n",
+                strerror(errno), fd);
+        return -1;
+    }
+    return res;
+}
+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;
+
+    res = pwrite(fd, buf, len, pos);
+    if (res < 0) {
+        fprintf(stderr, "host_pci_config: write failed: %s\n",
+                strerror(errno));
+        return -1;
+    }
+    return res;
+}
+
+uint8_t host_pci_read_byte(HostPCIDevice *d, int pos)
+{
+  uint8_t buf;
+  host_pci_config_read(d, pos, &buf, 1);
+  return buf;
+}
+uint16_t host_pci_read_word(HostPCIDevice *d, int pos)
+{
+  uint16_t buf;
+  host_pci_config_read(d, pos, &buf, 2);
+  return le16_to_cpu(buf);
+}
+uint32_t host_pci_read_long(HostPCIDevice *d, int pos)
+{
+  uint32_t buf;
+  host_pci_config_read(d, pos, &buf, 4);
+  return le32_to_cpu(buf);
+}
+int host_pci_read_block(HostPCIDevice *d, int pos, uint8_t *buf, int len)
+{
+  return host_pci_config_read(d, pos, buf, len);
+}
+
+int host_pci_write_byte(HostPCIDevice *d, int pos, uint8_t data)
+{
+  return host_pci_config_write(d, pos, &data, 1);
+}
+int host_pci_write_word(HostPCIDevice *d, int pos, uint16_t data)
+{
+  return host_pci_config_write(d, pos, &data, 2);
+}
+int host_pci_write_long(HostPCIDevice *d, int pos, uint32_t data)
+{
+  return host_pci_config_write(d, pos, &data, 4);
+}
+int host_pci_write_block(HostPCIDevice *d, int pos, uint8_t *buf, int len)
+{
+  return host_pci_config_write(d, pos, buf, len);
+}
+
+HostPCIDevice *host_pci_device_get(uint8_t bus, uint8_t dev, uint8_t func)
+{
+    HostPCIDevice *d = NULL;
+
+    d = g_new0(HostPCIDevice, 1);
+
+    d->config_fd = -1;
+    d->domain = 0;
+    d->bus = bus;
+    d->dev = dev;
+    d->func = func;
+
+    if (host_pci_config_fd(d) == -1)
+        goto error;
+    if (get_resource(d) == -1)
+        goto error;
+
+    d->vendor_id = get_value(d, "vendor");
+    d->device_id = get_value(d, "device");
+    d->is_virtfn = pci_dev_is_virtfn(d);
+
+    return d;
+error:
+    if (d->config_fd >= 0)
+        close(d->config_fd);
+    g_free(d);
+    return NULL;
+}
diff --git a/hw/host-pci-device.h b/hw/host-pci-device.h
new file mode 100644
index 0000000..0137507
--- /dev/null
+++ b/hw/host-pci-device.h
@@ -0,0 +1,36 @@ 
+#ifndef HW_HOST_PCI_DEVICE
+#  define HW_HOST_PCI_DEVICE
+
+#include "pci.h"
+
+typedef struct HostPCIDevice {
+    uint16_t domain;
+    uint8_t bus;
+    uint8_t dev;
+    uint8_t func;
+
+    uint16_t vendor_id;
+    uint16_t device_id;
+
+    pcibus_t base_addr[PCI_NUM_REGIONS - 1];
+    pcibus_t size[PCI_NUM_REGIONS - 1];
+    pcibus_t rom_base_addr;
+    pcibus_t rom_size;
+
+    bool is_virtfn;
+
+    int config_fd;
+} HostPCIDevice;
+
+HostPCIDevice *host_pci_device_get(uint8_t bus, uint8_t dev, uint8_t func);
+
+uint8_t host_pci_read_byte(HostPCIDevice *d, int pos);
+uint16_t host_pci_read_word(HostPCIDevice *d, int pos);
+uint32_t host_pci_read_long(HostPCIDevice *d, int pos);
+int host_pci_read_block(HostPCIDevice *d, int pos, uint8_t *buf, int len);
+int host_pci_write_byte(HostPCIDevice *d, int pos, uint8_t data);
+int host_pci_write_word(HostPCIDevice *d, int pos, uint16_t data);
+int host_pci_write_long(HostPCIDevice *d, int pos, uint32_t data);
+int host_pci_write_block(HostPCIDevice *d, int pos, uint8_t *buf, int len);
+
+#endif /* !HW_HOST_PCI_DEVICE */