Re: [Qemu-devel] RFC [v2]: vfio / device assignment -- layout of device fd files

[David Gibson <dwg@au1.ibm.com>]

On Mon, Sep 26, 2011 at 12:34:52PM -0600, Alex Williamson wrote:
> On Mon, 2011-09-26 at 12:04 +0200, Alexander Graf wrote:
> > Am 26.09.2011 um 09:51 schrieb David Gibson <david@gibson.dropbear.id.au>:
[snip]
> > Also, if you can come up with an interface that does not have variable
> > length descriptors but is still able to export all the required
> > generic information, please send a proposal to the list :)
> > 
> 
> Hi,
> 
> The other obvious possibility is a pure ioctl interface.  To match what
> this proposal is trying to describe, plus the runtime interfaces, we'd
> need something like:

Right, this also seems a reasonable possibility to me, depending on
the details.

> /* :0 - PCI devices, :1 - Devices path device, 63:2 - reserved */
> #define VFIO_DEVICE_GET_FLAGS			_IOR(, , u64)
> 
> 
> /* Return number of mmio/iop/config regions.
>  * For PCI this is always 8 (BAR0-5 + ROM + Config) */
> #define VFIO_DEVICE_GET_NUM_REGIONS		_IOR(, , int)
> 
> /* Return length for region index (may be zero) */
> #define VFIO_DEVICE_GET_REGION_LEN		_IOWR(, , u64)
> 
> /* Return flags for region index
>  * :0 - mmap'able, :1 - read-only, 63:2 - reserved */
> #define VFIO_DEVICE_GET_REGION_FLAGS		_IOR(, , u64)
> 
> /* Return file offset for region index */
> #define VFIO_DEVICE_GET_REGION_OFFSET		_IOWR(, , u64)

The above 3 can be be folded into one "getregioninfo" call.

> /* Return physical address for region index - not implemented for PCI */
> #define VFIO_DEVICE_GET_REGION_PHYS_ADDR	_IOWR(, , u64)
> 
> 
> 
> /* Return number of IRQs (Not including MSI/MSI-X for PCI) */
> #define VFIO_DEVICE_GET_NUM_IRQ			_IOR(, , int)
> 
> /* Set IRQ eventfd for IRQ index, arg[0] = index, arg[1] = fd */
> #define VFIO_DEVICE_SET_IRQ_EVENTFD		_IOW(, , int)
> 
> /* Unmask IRQ index */
> #define VFIO_DEVICE_UNMASK_IRQ			_IOW(, , int)
> 
> /* Set unmask eventfd for index, arg[0] = index, arg[1] = fd */
> #define VFIO_DEVICE_SET_UNMASK_IRQ_EVENTFD	_IOW(, , int)
> 
> 
> /* Return the device tree path for type/index into the user
>  * allocated buffer */
> struct dtpath {
> 	u32	type; (0 = region, 1 = IRQ)
> 	u32	index;
> 	u32	buf_len;
> 	char	*buf;
> };
> #define VFIO_DEVICE_GET_DTPATH			_IOWR(, , struct dtpath)
> 
> /* Return the device tree index for type/index */
> struct dtindex {
> 	u32	type; (0 = region, 1 = IRQ)
> 	u32	index;
> 	u32	prop_type;
> 	u32	prop_index;
> };
> #define VFIO_DEVICE_GET_DTINDEX			_IOWR(, , struct dtindex)

I think those need some work, but that doesn't impinge on the core
semantics.

> /* Reset the device */
> #define VFIO_DEVICE_RESET			_IO(, ,)
> 
> 
> /* PCI MSI setup, arg[0] = #, arg[1-n] = eventfds */
> #define VFIO_DEVICE_PCI_SET_MSI_EVENTFDS	_IOW(, , int)
> #define VFIO_DEVICE_PCI_SET_MSIX_EVENTFDS	_IOW(, , int)

Why does this need seperate controls, rather than just treating MSIs
as interrupts beyond the first for PCI devices?

> Hope that covers it.  Something I prefer about this interface is that
> everything can easily be generated on the fly, whereas reading out a
> table from the device means we really need to have that table somewhere
> in kernel memory to easily support reading random offsets.
> Thoughts?

I certainly prefer it to the previous proposal.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson