[SLOF] [PATCH v1 07/27] virtio: add virtio 1.0 capability parsing

Nikunj A Dadhania nikunj at linux.vnet.ibm.com
Thu Jan 14 21:45:23 AEDT 2016


Alexey Kardashevskiy <aik at ozlabs.ru> writes:

> On 01/13/2016 10:16 PM, Nikunj A Dadhania wrote:
>> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
>> ---
>>   lib/libvirtio/virtio.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   lib/libvirtio/virtio.h |   3 +-
>>   2 files changed, 137 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/libvirtio/virtio.c b/lib/libvirtio/virtio.c
>> index 7e0d6a3..bcf050b 100644
>> --- a/lib/libvirtio/virtio.c
>> +++ b/lib/libvirtio/virtio.c
>> @@ -14,6 +14,7 @@
>>   #include <stdbool.h>
>>   #include <stdlib.h>
>>   #include <string.h>
>> +#include <stddef.h>
>>   #include <cpu.h>
>>   #include <cache.h>
>>   #include <byteorder.h>
>> @@ -31,6 +32,140 @@
>>   #define VIRTIOHDR_ISR_STATUS		19
>>   #define VIRTIOHDR_DEVICE_CONFIG 	20
>>
>> +/* PCI defines */
>> +#define PCI_BASE_ADDR_SPACE_IO	0x01
>> +#define PCI_BASE_ADDR_SPACE_MEM	0x00
>> +#define PCI_BASE_ADDR_MEM_MASK	(~0x0fUL)
>> +#define PCI_BASE_ADDR_IO_MASK	(~0x03UL)
>> +
>> +#define PCI_BASE_ADDR_REG_0	0x10
>> +#define PCI_CONFIG_CAP_REG	0x34
>> +
>> +#define PCI_CAP_ID_VNDR		0x9
>> +
>> +#define VIRTIO_PCI_CAP_COMMON 1
>> +#define VIRTIO_PCI_CAP_NOTIFY 2
>> +#define VIRTIO_PCI_CAP_ISR    3
>> +#define VIRTIO_PCI_CAP_DEVICE 4
>> +#define VIRTIO_PCI_CAP_PCI    5
>> +
>> +#define VIRTIO_CAP_VNDR     0
>> +#define VIRTIO_CAP_NEXT     1
>> +#define VIRTIO_CAP_LEN      2
>> +#define VIRTIO_CAP_CFG_TYPE 3
>> +#define VIRTIO_CAP_BAR      4
>> +#define VIRTIO_CAP_OFFSET   8
>> +#define VIRTIO_CAP_LENGTH  12
>
>
> VIRTIO_CAP_LEN and VIRTIO_CAP_LENGTH - how are they different? I could not 
> figure out from the code as they are not used at all.

VIRTIO_CAP_LEN -    Capability length
VIRTIO_CAP_LENGTH - Length of the virtio structure pointed by
"addr[bar]+ offset"

>From the virtio 1.0 spec

struct virtio_pci_cap {
u8 cap_vndr; /* Generic PCI field: PCI_CAP_ID_VNDR */
u8 cap_next; /* Generic PCI field: next ptr. */
u8 cap_len;  /* Generic PCI field: capability length */
u8 cfg_type; /* Identifies the structure. */ 
u8 bar;      /* Where to find it. */
u8 padding[3]; /* Pad to full dword. */
le32 offset;   /* Offset within the bar. */
le32 length; /* Length of the structure, in bytes. */
};

>
>
>> +
>> +#define le64 uint64_t
>> +#define le32 uint32_t
>> +#define le16 uint16_t
>
>
> I'd use:
>
> typedef __u16 __bitwise __le16;
> typedef __u16 __bitwise __be16;
> typedef __u32 __bitwise __le32;
> typedef __u32 __bitwise __be32;
> typedef __u64 __bitwise __le64;
> typedef __u64 __bitwise __be64;
>
> as include/uapi/linux/types.h does in the kernel.

Sure. I can take that.

>
>> +
>> +struct virtio_dev_common {
>> +	le32 dev_features_sel;
>> +	le32 dev_features;
>> +	le32 drv_features_sel;
>> +	le32 drv_features;
>> +	le16 msix_config;
>> +	le16 num_queues;
>> +	uint8_t dev_status;
>> +	uint8_t cfg_generation;
>> +
>> +	le16 q_select;
>> +	le16 q_size;
>> +	le16 q_msix_vec;
>> +	le16 q_enable;
>> +	le16 q_notify_off;
>> +	le64 q_desc;
>> +	le64 q_avail;
>> +	le64 q_used;
>> +} __attribute__ ((packed));
>> +
>> +static void virtio_process_cap(struct virtio_device *dev, uint8_t cap_ptr)
>> +{
>> +	struct virtio_cap *cap;
>> +	uint8_t cfg_type, bar;
>> +	uint32_t offset;
>> +	uint64_t addr = 0;
>> +
>> +	cfg_type = SLOF_pci_config_read8(cap_ptr + VIRTIO_CAP_CFG_TYPE);
>> +	bar = SLOF_pci_config_read8(cap_ptr + VIRTIO_CAP_BAR);
>> +	offset = SLOF_pci_config_read32(cap_ptr + VIRTIO_CAP_OFFSET);
>> +
>> +	switch(cfg_type) {
>> +	case VIRTIO_PCI_CAP_COMMON:
>> +		cap = &dev->common;
>> +		break;
>> +	case VIRTIO_PCI_CAP_NOTIFY:
>> +		cap = &dev->notify;
>> +		dev->notify_off_mul = SLOF_pci_config_read32(cap_ptr + sizeof(struct virtio_cap));
>> +		break;
>> +	case VIRTIO_PCI_CAP_ISR:
>> +		cap = &dev->isr;
>> +		break;
>> +	case VIRTIO_PCI_CAP_DEVICE:
>> +		cap = &dev->device;
>> +		break;
>> +	default:
>> +		cap = NULL;
>> +	}
>> +
>> +	if (cap && cfg_type) {
>> +		cap->bar = bar;
>> +		addr = SLOF_pci_config_read32(PCI_BASE_ADDR_REG_0 + 4 * cap->bar);
>> +		if (addr & PCI_BASE_ADDR_SPACE_IO) {
>> +			cap->is_io = 1;
>> +			addr = addr & PCI_BASE_ADDR_IO_MASK;
>> +		} else if (addr & 4) {
>> +			addr = SLOF_pci_config_read32(PCI_BASE_ADDR_REG_0 + 4 * cap->bar) |
>> +				(SLOF_pci_config_read32(PCI_BASE_ADDR_REG_0 + 4 * (cap->bar + 1)) << 32);
>> +			addr = (uint64_t)SLOF_translate_my_address((void *)addr);
>> +			addr = addr & PCI_BASE_ADDR_MEM_MASK;
>> +		}
>> +		else {
>> +			cap->is_io = 0;
>> +			addr = addr & PCI_BASE_ADDR_MEM_MASK;
>> +		}
>> +		cap->addr = (void *)addr + offset;
>> +		cap->cap_id = cfg_type;
>> +	}
>> +	return;
>> +}
>> +/**
>> + * Setup virtio device details, gets called from SLOF routines
>> + * Determines legacy or modern device.
>> + */
>> +void virtio_setup_device(struct virtio_device *dev)
>
>
> s/virtio_setup_device/virtio_parse_capabilities/ ? This function does not 
> set up anything...w

Sure.

>
>
>> +{
>> +	uint8_t cap_ptr, cap_vndr;
>> +	uint64_t addr = 0;
>> +
>> +	cap_ptr = SLOF_pci_config_read8(PCI_CONFIG_CAP_REG);
>> +	while (cap_ptr != 0)
>> +	{
>> +		cap_vndr = SLOF_pci_config_read8(cap_ptr + VIRTIO_CAP_VNDR);
>> +		if (cap_vndr == PCI_CAP_ID_VNDR)
>> +			virtio_process_cap(dev, cap_ptr);
>> +		cap_ptr = SLOF_pci_config_read8(cap_ptr+VIRTIO_CAP_NEXT);
>> +	}
>> +	if (dev->common.cap_id && dev->notify.cap_id && dev->isr.cap_id && dev->device.cap_id)
>> +		dev->is_modern = 1;
>> +	else {
>> +		dev->is_modern = 0;
>> +		dev->legacy.cap_id = 0;
>> +		dev->legacy.bar = 0;
>> +		addr = SLOF_pci_config_read32(PCI_BASE_ADDR_REG_0);
>> +		if (addr & PCI_BASE_ADDR_SPACE_IO) {
>> +			dev->legacy.is_io = 1;
>> +			addr = addr & PCI_BASE_ADDR_IO_MASK;
>> +		} else {
>> +			dev->legacy.is_io = 0;
>> +			addr = addr & PCI_BASE_ADDR_MEM_MASK;
>> +		}
>> +		dev->legacy.addr = (void *)addr;
>> +	}
>> +	return;
>> +}
>>
>>   /**
>>    * Calculate ring size according to queue size number
>> diff --git a/lib/libvirtio/virtio.h b/lib/libvirtio/virtio.h
>> index 56e044b..eee3144 100644
>> --- a/lib/libvirtio/virtio.h
>> +++ b/lib/libvirtio/virtio.h
>> @@ -74,7 +74,7 @@ struct vring_used {
>>
>>   /* Structure shared with SLOF and is 16bytes */
>>   struct virtio_cap {
>> -	uint64_t addr;
>> +	void *addr;
>>   	uint8_t bar;
>>   	uint8_t is_io;
>>   	uint8_t cap_id;
>> @@ -116,6 +116,7 @@ extern void virtio_fill_desc(struct vring_desc *desc,
>>                                uint16_t flags, uint16_t next);
>>   extern int virtio_queue_init_vq(struct virtio_device *dev, struct vqs *vq, unsigned int id);
>>
>> +extern void virtio_setup_device(struct virtio_device *dev);
>>   extern void virtio_reset_device(struct virtio_device *dev);
>>   extern void virtio_queue_notify(struct virtio_device *dev, int queue);
>>   extern void virtio_set_status(struct virtio_device *dev, int status);
>>
>
>
> The whole patch seems to belong to 18/27.

why ?

Regards
Nikunj



More information about the SLOF mailing list