On 03/08/17 10:36, Punit Agrawal wrote:
> Jean-Philippe Brucker <jean-philippe.brucker@xxxxxxx> writes:
>
>> Hi Punit,
>>
>> Thanks for reviewing and testing
>>
>> On 31/07/17 18:52, Punit Agrawal wrote:
>>> Hi Jean-Philippe,
>>>
>>> A couple of queries below -
>>>
>>> Jean-Philippe Brucker <jean-philippe.brucker@xxxxxxx> writes:
>>>
>>>> Assigning devices using VFIO allows the guest to have direct access to the
>>>> device, whilst filtering accesses to sensitive areas by trapping config
>>>> space accesses and mapping DMA with an IOMMU.
>>>>
>>>> This patch adds a new option to lkvm run: --vfio-group=<group_number>.
>>>> Before assigning a device to a VM, some preparation is required. As
>>>> described in Linux Documentation/vfio.txt, the device driver need to be
>>>
>>> Nitpick: "needs"
>>>
>>>> changed to vfio-pci:
>>>>
>>>> $ dev=0000:00:00.0
>>>>
>>>> $ echo $dev > /sys/bus/pci/devices/$dev/driver/unbind
>>>> $ echo vfio-pci > /sys/bus/pci/devices/$dev/driver_override
>>>> $ echo $dev > /sys/bus/pci/drivers_probe
>>>> $ readlink /sys/bus/pci/devices/$dev/iommu_group
>>>> ../../../kernel/iommu_groups/5
>>>>
>>>> Adding --vfio[-group]=5 to lkvm-run will pass the device to the guest.
>>>> Multiple groups can be passed to the guest by adding more --vfio
>>>> parameters.
>>>>
>>>> This patch only implements PCI with INTx. MSI-X routing will be added in a
>>>> subsequent patch, and at some point we might add support for passing
>>>> platform devices to guests.
>>>>
>>>> Signed-off-by: Will Deacon <will.deacon@xxxxxxx>
>>>> Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
>>>> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@xxxxxxx>
>>>> ---
>>>> Makefile | 2 +
>>>> arm/pci.c | 1 +
>>>> builtin-run.c | 5 +
>>>> include/kvm/kvm-config.h | 3 +
>>>> include/kvm/pci.h | 3 +-
>>>> include/kvm/vfio.h | 57 +++++++
>>>> vfio/core.c | 395 +++++++++++++++++++++++++++++++++++++++++++++++
>>>> vfio/pci.c | 365 +++++++++++++++++++++++++++++++++++++++++++
>>>> 8 files changed, 830 insertions(+), 1 deletion(-)
>>>> create mode 100644 include/kvm/vfio.h
>>>> create mode 100644 vfio/core.c
>>>> create mode 100644 vfio/pci.c
>>>>
>>>
>>> [...]
>>>
>>>> +static int vfio_container_init(struct kvm *kvm)
>>>> +{
>>>> + int api, i, ret, iommu_type;;
>>>> +
>>>> + /* Create a container for our IOMMU groups */
>>>> + vfio_container = open(VFIO_DEV_NODE, O_RDWR);
>>>> + if (vfio_container == -1) {
>>>> + ret = errno;
>>>> + pr_err("Failed to open %s", VFIO_DEV_NODE);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + api = ioctl(vfio_container, VFIO_GET_API_VERSION);
>>>> + if (api != VFIO_API_VERSION) {
>>>> + pr_err("Unknown VFIO API version %d", api);
>>>> + return -ENODEV;
>>>> + }
>>>
>>> We are using the VFIO_API_VERSION pulled in from linux/vfio.h. Will
>>> kvmtool be in trouble if for some reason the API version is incompatibly
>>> incremented in the future?
>>
>> There are no precedents for this, as VFIO version is still 0. Additions to
>> the API are added in a backward-compatible way (for example by adding new
>> flags in the info structures and new fields at the end). If something
>> significant enough happened and required to increase the version number,
>> then the next version would probably be incompatible, and kvmtool would
>> have to support it separately.
>
> In that case, we should use a local version number macro for comparison
> and if ever VFIO api changes kvmtool will explicitly complain instead of
> silently dealing with an unknown version.
Good idea. I think I'll go one step further and pull the UAPI headers into
include/linux/vfio.h. It seems to be what we usually do for this kind of
situation (and I'll need it anyway when working with SVM virtualization).
Thanks,
Jean
