Re: [RFC v3 06/19] vfio-user: Define type vfio_user_pci_dev_info

2021-12-06 Thread John Johnson


> On Nov 19, 2021, at 2:42 PM, Alex Williamson  
> wrote:
> 
> On Mon,  8 Nov 2021 16:46:34 -0800
> John Johnson  wrote:
> 
>> New class for vfio-user with its class and instance
>> constructors and destructors, and its pci ops.
>> 
>> Signed-off-by: Elena Ufimtseva 
>> Signed-off-by: John G Johnson 
>> Signed-off-by: Jagannathan Raman 
>> ---
>> hw/vfio/pci.h   |  9 ++
>> hw/vfio/pci.c   | 97 
>> +
>> hw/vfio/Kconfig | 10 ++
>> 3 files changed, 116 insertions(+)
>> 
>> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
>> index bbc78aa..08ac647 100644
>> --- a/hw/vfio/pci.h
>> +++ b/hw/vfio/pci.h
>> @@ -187,6 +187,15 @@ struct VFIOKernPCIDevice {
>> VFIOPCIDevice device;
>> };
>> 
>> +#define TYPE_VFIO_USER_PCI "vfio-user-pci"
>> +OBJECT_DECLARE_SIMPLE_TYPE(VFIOUserPCIDevice, VFIO_USER_PCI)
>> +
>> +struct VFIOUserPCIDevice {
>> +VFIOPCIDevice device;
>> +char *sock_name;
>> +bool secure_dma; /* disable shared mem for DMA */
>> +};
>> +
>> /* Use uin32_t for vendor & device so PCI_ANY_ID expands and cannot match hw 
>> */
>> static inline bool vfio_pci_is(VFIOPCIDevice *vdev, uint32_t vendor, 
>> uint32_t device)
>> {
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 6e2ce35..fa3e028 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -19,6 +19,7 @@
>>  */
>> 
>> #include "qemu/osdep.h"
>> +#include CONFIG_DEVICES
>> #include 
>> #include 
>> 
>> @@ -3438,3 +3439,99 @@ struct VFIOValidOps vfio_pci_valid_ops = {
>> .validate_get_region_info = vfio_pci_valid_region_info,
>> .validate_get_irq_info = vfio_pci_valid_irq_info,
>> };
>> +
>> +
>> +#ifdef CONFIG_VFIO_USER_PCI
>> +
>> +/*
>> + * vfio-user routines.
>> + */
>> +
>> +/*
>> + * Emulated devices don't use host hot reset
>> + */
>> +static int vfio_user_pci_no_reset(VFIODevice *vbasedev)
>> +{
>> +error_printf("vfio-user - no hot reset\n");
>> +return 0;
>> +}
>> +
>> +static void vfio_user_pci_not_needed(VFIODevice *vbasedev)
>> +{
>> +vbasedev->needs_reset = false;
>> +}
> 
> Seems like we should make some of these optional rather than stubbing
> dummy functions.
> 

ok

>> +
>> +static VFIODeviceOps vfio_user_pci_ops = {
>> +.vfio_compute_needs_reset = vfio_user_pci_not_needed,
>> +.vfio_hot_reset_multi = vfio_user_pci_no_reset,
>> +.vfio_eoi = vfio_intx_eoi,
>> +.vfio_get_object = vfio_pci_get_object,
>> +.vfio_save_config = vfio_pci_save_config,
>> +.vfio_load_config = vfio_pci_load_config,
>> +};
>> +
>> +static void vfio_user_pci_realize(PCIDevice *pdev, Error **errp)
>> +{
>> +ERRP_GUARD();
>> +VFIOUserPCIDevice *udev = VFIO_USER_PCI(pdev);
>> +VFIOPCIDevice *vdev = VFIO_PCI_BASE(pdev);
>> +VFIODevice *vbasedev = >vbasedev;
>> +
>> +/*
>> + * TODO: make option parser understand SocketAddress
>> + * and use that instead of having scalar options
>> + * for each socket type.
>> + */
>> +if (!udev->sock_name) {
>> +error_setg(errp, "No socket specified");
>> +error_append_hint(errp, "Use -device 
>> vfio-user-pci,socket=\n");
>> +return;
>> +}
>> +
>> +vbasedev->name = g_strdup_printf("VFIO user <%s>", udev->sock_name);
>> +vbasedev->dev = DEVICE(vdev);
>> +vbasedev->fd = -1;
>> +vbasedev->type = VFIO_DEVICE_TYPE_PCI;
>> +vbasedev->no_mmap = false;
> 
> Why hard coded rather than a property?  This is a useful debugging
> feature to be able to trap all device accesses.  The device should work
> either way.
> 

As I said in the earlier property reply, I thought these were
HW workarounds.



>> +vbasedev->ops = _user_pci_ops;
>> +vbasedev->valid_ops = _pci_valid_ops;
>> +
>> +}
>> +
>> +static void vfio_user_instance_finalize(Object *obj)
>> +{
>> +}
>> +
>> +static Property vfio_user_pci_dev_properties[] = {
>> +DEFINE_PROP_STRING("socket", VFIOUserPCIDevice, sock_name),
>> +DEFINE_PROP_BOOL("secure-dma", VFIOUserPCIDevice, secure_dma, false),
> 
> Add this when it means something.  Thanks,
> 

This can be moved to the path that uses ’secure_dma”

JJ




Re: [RFC v3 06/19] vfio-user: Define type vfio_user_pci_dev_info

2021-11-19 Thread Alex Williamson
On Mon,  8 Nov 2021 16:46:34 -0800
John Johnson  wrote:

> New class for vfio-user with its class and instance
> constructors and destructors, and its pci ops.
> 
> Signed-off-by: Elena Ufimtseva 
> Signed-off-by: John G Johnson 
> Signed-off-by: Jagannathan Raman 
> ---
>  hw/vfio/pci.h   |  9 ++
>  hw/vfio/pci.c   | 97 
> +
>  hw/vfio/Kconfig | 10 ++
>  3 files changed, 116 insertions(+)
> 
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index bbc78aa..08ac647 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -187,6 +187,15 @@ struct VFIOKernPCIDevice {
>  VFIOPCIDevice device;
>  };
>  
> +#define TYPE_VFIO_USER_PCI "vfio-user-pci"
> +OBJECT_DECLARE_SIMPLE_TYPE(VFIOUserPCIDevice, VFIO_USER_PCI)
> +
> +struct VFIOUserPCIDevice {
> +VFIOPCIDevice device;
> +char *sock_name;
> +bool secure_dma; /* disable shared mem for DMA */
> +};
> +
>  /* Use uin32_t for vendor & device so PCI_ANY_ID expands and cannot match hw 
> */
>  static inline bool vfio_pci_is(VFIOPCIDevice *vdev, uint32_t vendor, 
> uint32_t device)
>  {
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 6e2ce35..fa3e028 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -19,6 +19,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include CONFIG_DEVICES
>  #include 
>  #include 
>  
> @@ -3438,3 +3439,99 @@ struct VFIOValidOps vfio_pci_valid_ops = {
>  .validate_get_region_info = vfio_pci_valid_region_info,
>  .validate_get_irq_info = vfio_pci_valid_irq_info,
>  };
> +
> +
> +#ifdef CONFIG_VFIO_USER_PCI
> +
> +/*
> + * vfio-user routines.
> + */
> +
> +/*
> + * Emulated devices don't use host hot reset
> + */
> +static int vfio_user_pci_no_reset(VFIODevice *vbasedev)
> +{
> +error_printf("vfio-user - no hot reset\n");
> +return 0;
> +}
> +
> +static void vfio_user_pci_not_needed(VFIODevice *vbasedev)
> +{
> +vbasedev->needs_reset = false;
> +}

Seems like we should make some of these optional rather than stubbing
dummy functions.

> +
> +static VFIODeviceOps vfio_user_pci_ops = {
> +.vfio_compute_needs_reset = vfio_user_pci_not_needed,
> +.vfio_hot_reset_multi = vfio_user_pci_no_reset,
> +.vfio_eoi = vfio_intx_eoi,
> +.vfio_get_object = vfio_pci_get_object,
> +.vfio_save_config = vfio_pci_save_config,
> +.vfio_load_config = vfio_pci_load_config,
> +};
> +
> +static void vfio_user_pci_realize(PCIDevice *pdev, Error **errp)
> +{
> +ERRP_GUARD();
> +VFIOUserPCIDevice *udev = VFIO_USER_PCI(pdev);
> +VFIOPCIDevice *vdev = VFIO_PCI_BASE(pdev);
> +VFIODevice *vbasedev = >vbasedev;
> +
> +/*
> + * TODO: make option parser understand SocketAddress
> + * and use that instead of having scalar options
> + * for each socket type.
> + */
> +if (!udev->sock_name) {
> +error_setg(errp, "No socket specified");
> +error_append_hint(errp, "Use -device vfio-user-pci,socket=\n");
> +return;
> +}
> +
> +vbasedev->name = g_strdup_printf("VFIO user <%s>", udev->sock_name);
> +vbasedev->dev = DEVICE(vdev);
> +vbasedev->fd = -1;
> +vbasedev->type = VFIO_DEVICE_TYPE_PCI;
> +vbasedev->no_mmap = false;

Why hard coded rather than a property?  This is a useful debugging
feature to be able to trap all device accesses.  The device should work
either way.

> +vbasedev->ops = _user_pci_ops;
> +vbasedev->valid_ops = _pci_valid_ops;
> +
> +}
> +
> +static void vfio_user_instance_finalize(Object *obj)
> +{
> +}
> +
> +static Property vfio_user_pci_dev_properties[] = {
> +DEFINE_PROP_STRING("socket", VFIOUserPCIDevice, sock_name),
> +DEFINE_PROP_BOOL("secure-dma", VFIOUserPCIDevice, secure_dma, false),

Add this when it means something.  Thanks,

Alex

> +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(_user_pci_dev_info);
> +}
> +
> +type_init(register_vfio_user_dev_type)
> +
> +#endif /* VFIO_USER_PCI */
> diff --git a/hw/vfio/Kconfig b/hw/vfio/Kconfig
> index 7cdba05..301894e 100644
> --- a/hw/vfio/Kconfig
> +++ b/hw/vfio/Kconfig
> @@ -2,6 +2,10 @@ config VFIO
>  bool
>  depends on LINUX
>  
> +config VFIO_USER
> +bool
> +depends on VFIO
> +
>  config VFIO_PCI
>  

[RFC v3 06/19] vfio-user: Define type vfio_user_pci_dev_info

2021-11-08 Thread John Johnson
New class for vfio-user with its class and instance
constructors and destructors, and its pci ops.

Signed-off-by: Elena Ufimtseva 
Signed-off-by: John G Johnson 
Signed-off-by: Jagannathan Raman 
---
 hw/vfio/pci.h   |  9 ++
 hw/vfio/pci.c   | 97 +
 hw/vfio/Kconfig | 10 ++
 3 files changed, 116 insertions(+)

diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index bbc78aa..08ac647 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -187,6 +187,15 @@ struct VFIOKernPCIDevice {
 VFIOPCIDevice device;
 };
 
+#define TYPE_VFIO_USER_PCI "vfio-user-pci"
+OBJECT_DECLARE_SIMPLE_TYPE(VFIOUserPCIDevice, VFIO_USER_PCI)
+
+struct VFIOUserPCIDevice {
+VFIOPCIDevice device;
+char *sock_name;
+bool secure_dma; /* disable shared mem for DMA */
+};
+
 /* Use uin32_t for vendor & device so PCI_ANY_ID expands and cannot match hw */
 static inline bool vfio_pci_is(VFIOPCIDevice *vdev, uint32_t vendor, uint32_t 
device)
 {
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 6e2ce35..fa3e028 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -19,6 +19,7 @@
  */
 
 #include "qemu/osdep.h"
+#include CONFIG_DEVICES
 #include 
 #include 
 
@@ -3438,3 +3439,99 @@ struct VFIOValidOps vfio_pci_valid_ops = {
 .validate_get_region_info = vfio_pci_valid_region_info,
 .validate_get_irq_info = vfio_pci_valid_irq_info,
 };
+
+
+#ifdef CONFIG_VFIO_USER_PCI
+
+/*
+ * vfio-user routines.
+ */
+
+/*
+ * Emulated devices don't use host hot reset
+ */
+static int vfio_user_pci_no_reset(VFIODevice *vbasedev)
+{
+error_printf("vfio-user - no hot reset\n");
+return 0;
+}
+
+static void vfio_user_pci_not_needed(VFIODevice *vbasedev)
+{
+vbasedev->needs_reset = false;
+}
+
+static VFIODeviceOps vfio_user_pci_ops = {
+.vfio_compute_needs_reset = vfio_user_pci_not_needed,
+.vfio_hot_reset_multi = vfio_user_pci_no_reset,
+.vfio_eoi = vfio_intx_eoi,
+.vfio_get_object = vfio_pci_get_object,
+.vfio_save_config = vfio_pci_save_config,
+.vfio_load_config = vfio_pci_load_config,
+};
+
+static void vfio_user_pci_realize(PCIDevice *pdev, Error **errp)
+{
+ERRP_GUARD();
+VFIOUserPCIDevice *udev = VFIO_USER_PCI(pdev);
+VFIOPCIDevice *vdev = VFIO_PCI_BASE(pdev);
+VFIODevice *vbasedev = >vbasedev;
+
+/*
+ * TODO: make option parser understand SocketAddress
+ * and use that instead of having scalar options
+ * for each socket type.
+ */
+if (!udev->sock_name) {
+error_setg(errp, "No socket specified");
+error_append_hint(errp, "Use -device vfio-user-pci,socket=\n");
+return;
+}
+
+vbasedev->name = g_strdup_printf("VFIO user <%s>", udev->sock_name);
+vbasedev->dev = DEVICE(vdev);
+vbasedev->fd = -1;
+vbasedev->type = VFIO_DEVICE_TYPE_PCI;
+vbasedev->no_mmap = false;
+vbasedev->ops = _user_pci_ops;
+vbasedev->valid_ops = _pci_valid_ops;
+
+}
+
+static void vfio_user_instance_finalize(Object *obj)
+{
+}
+
+static Property vfio_user_pci_dev_properties[] = {
+DEFINE_PROP_STRING("socket", VFIOUserPCIDevice, sock_name),
+DEFINE_PROP_BOOL("secure-dma", VFIOUserPCIDevice, secure_dma, false),
+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(_user_pci_dev_info);
+}
+
+type_init(register_vfio_user_dev_type)
+
+#endif /* VFIO_USER_PCI */
diff --git a/hw/vfio/Kconfig b/hw/vfio/Kconfig
index 7cdba05..301894e 100644
--- a/hw/vfio/Kconfig
+++ b/hw/vfio/Kconfig
@@ -2,6 +2,10 @@ config VFIO
 bool
 depends on LINUX
 
+config VFIO_USER
+bool
+depends on VFIO
+
 config VFIO_PCI
 bool
 default y
@@ -9,6 +13,12 @@ config VFIO_PCI
 select EDID
 depends on LINUX && PCI
 
+config VFIO_USER_PCI
+bool
+default y
+select VFIO_USER
+depends on VFIO_PCI
+
 config VFIO_CCW
 bool
 default y
-- 
1.8.3.1