From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Williamson Subject: Re: [Qemu-devel] RFC [v2]: vfio / device assignment -- layout of device fd files Date: Fri, 30 Sep 2011 10:37:12 -0600 Message-ID: <1317400634.2552.47.camel@bling.home> References: <20110926075144.GT12286@yookeroo.fritz.box> <3D54B89C-A0A3-4461-A7A1-3F1E4AB79296@suse.de> <1317062095.25515.75.camel@bling.home> <20110930084647.GF4512@yookeroo.fritz.box> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: Alexander Graf , Stuart Yoder , Scott Wood , "qemu-devel@nongnu.org" , "kvm@vger.kernel.org" , "avi@redhat.com" , "anthony@codemonkey.ws" , Benjamin Herrenschmidt To: David Gibson Return-path: Received: from mx1.redhat.com ([209.132.183.28]:1877 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754141Ab1I3RKf (ORCPT ); Fri, 30 Sep 2011 13:10:35 -0400 In-Reply-To: <20110930084647.GF4512@yookeroo.fritz.box> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, 2011-09-30 at 18:46 +1000, David Gibson wrote: > 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 : > [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. Yep, and the phys addr one below. We can use a flags bit to indicate whether it's valid. > > /* 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? Well, we could say that PCI will always report 3 for VFIO_DEVICE_GET_NUM_IRQ where 0 = legacy, 1 = MSI, 2 = MSI-X. ioctls on unimplemented IRQs will fail, UNMASK* ioctls on non-level triggered interrupts will fail, and the parameter to SET_IRQ_EVENTFD becomes arg[0] = index, arg[1] = count, arg[2-n] = fd. Maybe we'd then have a GET_IRQ_INFO that takes something like: struct vfio_irq_info { int index; unsigned int count; u64 flags; #define VFIO_IRQ_INFO_FLAGS_LEVEL (1 << 0) }; count would be 0 on PCI if the type of interrupt isn't supported. Better? Thanks, Alex > > 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.