On Wed, 10 Jan 2018 11:28:59 +0200 Marcel Apfelbaum <mar...@redhat.com> wrote:
> On 09/01/2018 14:51, Cornelia Huck wrote: > > On Tue, 9 Jan 2018 13:08:33 +0200 > > Yuval Shaia <yuval.sh...@oracle.com> wrote: > > > >> On Tue, Jan 09, 2018 at 11:39:11AM +0100, Cornelia Huck wrote: > >>> On Sun, 7 Jan 2018 14:32:23 +0200 > >>> Marcel Apfelbaum <mar...@redhat.com> wrote: > > > >>>> diff --git a/hw/rdma/vmw/pvrdma_dev_api.h b/hw/rdma/vmw/pvrdma_dev_api.h > >>>> new file mode 100644 > >>>> index 0000000000..bf1986a976 > >>>> --- /dev/null > >>>> +++ b/hw/rdma/vmw/pvrdma_dev_api.h > >>>> @@ -0,0 +1,602 @@ > >>>> +/* > >>>> + * QEMU VMWARE paravirtual RDMA device definitions > >>>> + * > >>>> + * Copyright (C) 2018 Oracle > >>>> + * Copyright (C) 2018 Red Hat Inc > >>>> + * > >>>> + * Authors: > >>>> + * Yuval Shaia <yuval.sh...@oracle.com> > >>>> + * Marcel Apfelbaum <mar...@redhat.com> > >>>> + * > >>>> + * This work is licensed under the terms of the GNU GPL, version 2. > >>>> + * See the COPYING file in the top-level directory. > >>>> + * > >>>> + */ > >>>> + > >>>> +#ifndef PVRDMA_DEV_API_H > >>>> +#define PVRDMA_DEV_API_H > >>>> + > >>>> +/* > >>>> + * Following is an interface definition for PVRDMA device as provided by > >>>> + * VMWARE. > >>>> + * See original copyright from Linux kernel v4.14.5 header file > >>>> + * drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h > >>> > >>> Could that file be exported as UAPI in the kernel and added to the > >>> linux-headers script? > >> > >> We took this approach as apposed to kernel-headers with the following on > >> our mind: > >> (1) This is the convention used in vmxnet3. > >> (2) vmw_pvrdma was introduced only lately, taking the kernel-headers > >> approach will force specific kernel on a host in order to compile QEMU. > > > > qemu will get the kernel headers once from the upstream kernel and then > > will be able to be built everywhere. > > > >> (3) To support VMWare's pvrdma device we took a snapshot of existing > >> driver/device settings and breezed there. This is driver/device API and we > >> can't allow our self to chase VMWare's tail whenever they are changing the > >> API. Just consider a case where they will change for example the ARM bit. > > > > But as want to enable the existing device driver, you'll want to be > > able to produce a compatible device anyway, don't you? Also, wouldn't > > VMWare break older kernels if they suddenly changed the api? > > > > I think it was a missunderstanding here :) > > We don't actually need the VMware headers here in order to compile the device. > Requesting the pvrdma headers to be present really limits the hosts we could > compile QEMU without an actual benefit. > > The headers describe the data structures passed by the Guest driver to the > PVRDMA device. > Having them on the host should not be a requirement, we could simply define > our own > structs but it would be harder to maintain. (diffing with linux headers to > see what's new) > > Note we don't care about ABI changes, we emulate a PCI device, not > a para-virt one, the driver has the contract to pass the same data > definitions always. > If the data changes, there are 2 options: > 1. Pass another "version nr" on the command channel (DSR) and we declare as > not supported > by this device and fail gracefully. > 2. Use another device id. OK, thanks for the clarification, that makes sense. > > We looked for what are the options for *not requiring* the headers to be > present > and we saw two in QEMU: > 1. Adding it to include/standard-headers/linux/ > But it seemed to much, only our device need them. > 2. Adding it to the hw/rdma/vwm directory > Please look at: (hw/net/vmxnet3.h) > > The second option seemed better, but we could go with "1." > if I got it right and you think is cleaner. > But again, the first option kind of requires updating the > linux headers always, but we want quite the opposite, to not > be connected to the changes. > I hope is more clear now. OK, that makes sense as well, if you intend this to be a one-time effort. "Copy a header from the Linux kernel" just raised kind of a flag for me. > > > > [Also, is there a canonical reference for this API?] > > > > Linux Kernel PVRDMA driver implementation... :) > > >> > >> Just IMHO. > >> > > Thanks a lot for your inputs, much appreciated! NP. I'd like to do more, as this is interesting stuff, but you know how it goes...