|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH V3 02/10] Introduce HostPCIDevice to access a pci device on the host.
> +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;
So the decleration is 'unsigned long' but you return -1 here.
Should the decleration be 'signed long' ?
Or perhaps return the value as parameter and return zero for success
and <= 0 for failure?
> + }
> + 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;
Seems like this could be a 'bool'?
> +}
> +
> +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_get_byte(HostPCIDevice *d, int pos)
> +{
> + uint8_t buf;
> + host_pci_config_read(d, pos, &buf, 1);
Not checking the return value?
> + return buf;
> +}
> +uint16_t host_pci_get_word(HostPCIDevice *d, int pos)
> +{
> + uint16_t buf;
> + host_pci_config_read(d, pos, &buf, 2);
Here as well?
> + return le16_to_cpu(buf);
So if we can't read those buffers, won't that mean we end up with
garbage in buf? As we haven't actually written anything to it?
Perhaps we should do:
if (host_pci..() < 0)
return 0;
... normal case?
> +}
> +uint32_t host_pci_get_long(HostPCIDevice *d, int pos)
> +{
> + uint32_t buf;
> + host_pci_config_read(d, pos, &buf, 4);
> + return le32_to_cpu(buf);
> +}
> +int host_pci_get_block(HostPCIDevice *d, int pos, uint8_t *buf, int len)
> +{
> + return host_pci_config_read(d, pos, buf, len);
Oh, so that is called.. Hm, not much chance of returning an error there is.
Can we propage the errors in case there is some fundamental failure
when reading/writting the data stream? Say the PCI device gets
unplugged by the user.. won't pread return -EXIO?
> +}
> +
> +int host_pci_set_byte(HostPCIDevice *d, int pos, uint8_t data)
> +{
> + return host_pci_config_write(d, pos, &data, 1);
> +}
> +int host_pci_set_word(HostPCIDevice *d, int pos, uint16_t data)
> +{
> + data = cpu_to_le16(data);
> + return host_pci_config_write(d, pos, &data, 2);
> +}
> +int host_pci_set_long(HostPCIDevice *d, int pos, uint32_t data)
> +{
> + data = cpu_to_le32(data);
> + return host_pci_config_write(d, pos, &data, 4);
> +}
> +int host_pci_set_block(HostPCIDevice *d, int pos, uint8_t *buf, int len)
> +{
> + return host_pci_config_write(d, pos, buf, len);
> +}
> +
> +uint32_t host_pci_find_ext_cap_offset(HostPCIDevice *d, uint32_t cap)
> +{
> + uint32_t header = 0;
> + int max_cap = PCI_MAX_EXT_CAP;
> + int pos = PCI_CONFIG_SPACE_SIZE;
> +
> + do {
> + header = host_pci_get_long(d, pos);
> + /*
> + * If we have no capabilities, this is indicated by cap ID,
> + * cap version and next pointer all being 0.
> + */
> + if (header == 0) {
> + break;
> + }
> +
> + if (PCI_EXT_CAP_ID(header) == cap) {
> + return pos;
> + }
> +
> + pos = PCI_EXT_CAP_NEXT(header);
> + if (pos < PCI_CONFIG_SPACE_SIZE) {
> + break;
> + }
> +
> + max_cap--;
> + } while (max_cap > 0);
> +
> + return 0;
> +}
> +
> +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) != 0) {
> + 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;
> +}
> +
> +void host_pci_device_put(HostPCIDevice *d)
> +{
> + if (d->config_fd >= 0) {
> + close(d->config_fd);
> + }
> + g_free(d);
> +}
> diff --git a/hw/host-pci-device.h b/hw/host-pci-device.h
> new file mode 100644
> index 0000000..d79ba48
> --- /dev/null
> +++ b/hw/host-pci-device.h
> @@ -0,0 +1,75 @@
> +#ifndef HW_HOST_PCI_DEVICE
> +# define HW_HOST_PCI_DEVICE
> +
> +#include "pci.h"
> +
> +/*
> + * from linux/ioport.h
> + * IO resources have these defined flags.
> + */
> +#define IORESOURCE_BITS 0x000000ff /* Bus-specific bits */
> +
> +#define IORESOURCE_TYPE_BITS 0x00000f00 /* Resource type */
> +#define IORESOURCE_IO 0x00000100
> +#define IORESOURCE_MEM 0x00000200
> +#define IORESOURCE_IRQ 0x00000400
> +#define IORESOURCE_DMA 0x00000800
> +
> +#define IORESOURCE_PREFETCH 0x00001000 /* No side effects */
> +#define IORESOURCE_READONLY 0x00002000
> +#define IORESOURCE_CACHEABLE 0x00004000
> +#define IORESOURCE_RANGELENGTH 0x00008000
> +#define IORESOURCE_SHADOWABLE 0x00010000
> +
> +#define IORESOURCE_SIZEALIGN 0x00020000 /* size indicates alignment
> */
> +#define IORESOURCE_STARTALIGN 0x00040000 /* start field is alignment
> */
> +
> +#define IORESOURCE_MEM_64 0x00100000
> +
> + /* Userland may not map this resource */
> +#define IORESOURCE_EXCLUSIVE 0x08000000
> +#define IORESOURCE_DISABLED 0x10000000
> +#define IORESOURCE_UNSET 0x20000000
> +#define IORESOURCE_AUTO 0x40000000
> + /* Driver has marked this resource busy */
> +#define IORESOURCE_BUSY 0x80000000
> +
> +
> +typedef struct HostPCIIORegion {
> + unsigned long flags;
> + pcibus_t base_addr;
> + pcibus_t size;
> +} HostPCIIORegion;
> +
> +typedef struct HostPCIDevice {
> + uint16_t domain;
> + uint8_t bus;
> + uint8_t dev;
> + uint8_t func;
> +
> + uint16_t vendor_id;
> + uint16_t device_id;
> +
> + HostPCIIORegion io_regions[PCI_NUM_REGIONS - 1];
> + HostPCIIORegion rom;
> +
> + bool is_virtfn;
> +
> + int config_fd;
> +} HostPCIDevice;
> +
> +HostPCIDevice *host_pci_device_get(uint8_t bus, uint8_t dev, uint8_t func);
> +void host_pci_device_put(HostPCIDevice *pci_dev);
> +
> +uint8_t host_pci_get_byte(HostPCIDevice *d, int pos);
> +uint16_t host_pci_get_word(HostPCIDevice *d, int pos);
> +uint32_t host_pci_get_long(HostPCIDevice *d, int pos);
> +int host_pci_get_block(HostPCIDevice *d, int pos, uint8_t *buf, int len);
> +int host_pci_set_byte(HostPCIDevice *d, int pos, uint8_t data);
> +int host_pci_set_word(HostPCIDevice *d, int pos, uint16_t data);
> +int host_pci_set_long(HostPCIDevice *d, int pos, uint32_t data);
> +int host_pci_set_block(HostPCIDevice *d, int pos, uint8_t *buf, int len);
> +
> +uint32_t host_pci_find_ext_cap_offset(HostPCIDevice *s, uint32_t cap);
> +
> +#endif /* !HW_HOST_PCI_DEVICE */
> --
> Anthony PERARD
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |