Re: [PATCH RFC 04/19] vfio-user: Define type vfio_user_pci_dev_info

[John Johnson <john.g.johnson@oracle.com>]

> On Jul 28, 2021, at 3:16 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
> On Sun, Jul 18, 2021 at 11:27:43PM -0700, Elena Ufimtseva wrote:
>> From: John G Johnson <john.g.johnson@oracle.com>
>> 
>> New class for vfio-user with its class and instance
>> constructors and destructors.
>> 
>> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
>> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
>> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
>> ---
>> hw/vfio/pci.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 49 insertions(+)
>> 
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index bea95efc33..554b562769 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -42,6 +42,7 @@
>> #include "qapi/error.h"
>> #include "migration/blocker.h"
>> #include "migration/qemu-file.h"
>> +#include "hw/vfio/user.h"
>> 
>> #define TYPE_VFIO_PCI_NOHOTPLUG "vfio-pci-nohotplug"
>> 
>> @@ -3326,3 +3327,51 @@ static void register_vfio_pci_dev_type(void)
>> }
>> 
>> type_init(register_vfio_pci_dev_type)
>> +
>> +static void vfio_user_pci_realize(PCIDevice *pdev, Error **errp)
>> +{
>> +    ERRP_GUARD();
>> +    VFIOUserPCIDevice *udev = VFIO_USER_PCI(pdev);
>> +
>> +    if (!udev->sock_name) {
>> +        error_setg(errp, "No socket specified");
>> +        error_append_hint(errp, "Use -device vfio-user-pci,socket=<name>\n");
>> +        return;
>> +    }
>> +}
>> +
>> +static void vfio_user_instance_finalize(Object *obj)
>> +{
>> +}
>> +
>> +static Property vfio_user_pci_dev_properties[] = {
>> +    DEFINE_PROP_STRING("socket", VFIOUserPCIDevice, sock_name),
> 
> Please use SocketAddress so that alternative socket connection details
> can be supported without inventing custom syntax for vfio-user-pci. For
> example, file descriptor passing should be possible.
> 
> I think this requires a bit of command-line parsing work, so don't worry
> about it for now, but please add a TODO comment. When the -device
> vfio-user-pci syntax is finalized (i.e. when the code is merged and the
> device name doesn't start with the experimental x- prefix), then it
> needs to be solved.
> 

	What do you want the options to look like at the endgame?  I’d
rather work backward from that than have several different flavors of
options as new socket options are added.  I did look at -chardev socket,
and it was confusing enough that I went for the simple string.



>> +    DEFINE_PROP_BOOL("secure-dma", VFIOUserPCIDevice, secure, false),
> 
> I'm not sure what "secure-dma" means and the "secure" variable name is
> even more inscrutable. Does this mean don't share memory so that each
> DMA access is checked individually?
> 

	Yes.  Do you have another name you’d prefer? “no-shared-mem”?

						JJ



>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void vfio_user_pci_dev_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    PCIDeviceClass *pdc = PCI_DEVICE_CLASS(klass);
>> +
>> +    device_class_set_props(dc, vfio_user_pci_dev_properties);
>> +    dc->desc = "VFIO over socket PCI device assignment";
>> +    pdc->realize = vfio_user_pci_realize;
>> +}
>> +
>> +static const TypeInfo vfio_user_pci_dev_info = {
>> +    .name = TYPE_VFIO_USER_PCI,
>> +    .parent = TYPE_VFIO_PCI_BASE,
>> +    .instance_size = sizeof(VFIOUserPCIDevice),
>> +    .class_init = vfio_user_pci_dev_class_init,
>> +    .instance_init = vfio_instance_init,
>> +    .instance_finalize = vfio_user_instance_finalize,
>> +};
>> +
>> +static void register_vfio_user_dev_type(void)
>> +{
>> +    type_register_static(&vfio_user_pci_dev_info);
>> +}
>> +
>> +type_init(register_vfio_user_dev_type)
>> -- 
>> 2.25.1
>>