Re: [PATCH] virtio_pci: move structure to a header

2023-10-31 Thread Xuan Zhuo 
On Tue, 31 Oct 2023 12:19:54 -0400, "Michael S. Tsirkin"  
wrote:
> These are guest/host interfaces so belong in the header
> where e.g. qemu will know to find them.
> Note: we added a new structure as opposed to extending existing one
> because someone might be relying on the size of the existing structure
> staying unchanged.  Add a warning to avoid using sizeof.
>
> Signed-off-by: Michael S. Tsirkin 

Reviewed-by: Xuan Zhuo 

> ---
>  drivers/virtio/virtio_pci_modern_dev.c |  7 ---
>  include/linux/virtio_pci_modern.h  |  7 ---
>  include/uapi/linux/virtio_pci.h| 11 +++
>  3 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_modern_dev.c 
> b/drivers/virtio/virtio_pci_modern_dev.c
> index e2a1fe7bb66c..7de8b1ebabac 100644
> --- a/drivers/virtio/virtio_pci_modern_dev.c
> +++ b/drivers/virtio/virtio_pci_modern_dev.c
> @@ -294,9 +294,10 @@ int vp_modern_probe(struct virtio_pci_modern_device 
> *mdev)
>
>   err = -EINVAL;
>   mdev->common = vp_modern_map_capability(mdev, common,
> -   sizeof(struct virtio_pci_common_cfg), 4,
> -   0, sizeof(struct 
> virtio_pci_modern_common_cfg),
> -   &mdev->common_len, NULL);
> +   sizeof(struct virtio_pci_common_cfg), 4, 0,
> +   offsetofend(struct virtio_pci_modern_common_cfg,
> +   queue_reset),
> +   &mdev->common_len, NULL);
>   if (!mdev->common)
>   goto err_map_common;
>   mdev->isr = vp_modern_map_capability(mdev, isr, sizeof(u8), 1,
> diff --git a/include/linux/virtio_pci_modern.h 
> b/include/linux/virtio_pci_modern.h
> index d0f2797420f7..a09e13a577a9 100644
> --- a/include/linux/virtio_pci_modern.h
> +++ b/include/linux/virtio_pci_modern.h
> @@ -5,13 +5,6 @@
>  #include 
>  #include 
>
> -struct virtio_pci_modern_common_cfg {
> - struct virtio_pci_common_cfg cfg;
> -
> - __le16 queue_notify_data;   /* read-write */
> - __le16 queue_reset; /* read-write */
> -};
> -
>  /**
>   * struct virtio_pci_modern_device - info for modern PCI virtio
>   * @pci_dev: Ptr to the PCI device struct
> diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
> index f703afc7ad31..44f4dd2add18 100644
> --- a/include/uapi/linux/virtio_pci.h
> +++ b/include/uapi/linux/virtio_pci.h
> @@ -166,6 +166,17 @@ struct virtio_pci_common_cfg {
>   __le32 queue_used_hi;   /* read-write */
>  };
>
> +/*
> + * Warning: do not use sizeof on this: use offsetofend for
> + * specific fields you need.
> + */
> +struct virtio_pci_modern_common_cfg {
> + struct virtio_pci_common_cfg cfg;
> +
> + __le16 queue_notify_data;   /* read-write */
> + __le16 queue_reset; /* read-write */
> +};
> +
>  /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
>  struct virtio_pci_cfg_cap {
>   struct virtio_pci_cap cap;
> --
> MST
>
> ___
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_pci: move structure to a header

2023-10-31 Thread Jason Wang 
On Wed, Nov 1, 2023 at 12:20 AM Michael S. Tsirkin  wrote:
>
> These are guest/host interfaces so belong in the header
> where e.g. qemu will know to find them.
> Note: we added a new structure as opposed to extending existing one
> because someone might be relying on the size of the existing structure
> staying unchanged.  Add a warning to avoid using sizeof.
>
> Signed-off-by: Michael S. Tsirkin 

Acked-by: Jason Wang 

Thanks

> ---
>  drivers/virtio/virtio_pci_modern_dev.c |  7 ---
>  include/linux/virtio_pci_modern.h  |  7 ---
>  include/uapi/linux/virtio_pci.h| 11 +++
>  3 files changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_modern_dev.c 
> b/drivers/virtio/virtio_pci_modern_dev.c
> index e2a1fe7bb66c..7de8b1ebabac 100644
> --- a/drivers/virtio/virtio_pci_modern_dev.c
> +++ b/drivers/virtio/virtio_pci_modern_dev.c
> @@ -294,9 +294,10 @@ int vp_modern_probe(struct virtio_pci_modern_device 
> *mdev)
>
> err = -EINVAL;
> mdev->common = vp_modern_map_capability(mdev, common,
> - sizeof(struct virtio_pci_common_cfg), 4,
> - 0, sizeof(struct 
> virtio_pci_modern_common_cfg),
> - &mdev->common_len, NULL);
> + sizeof(struct virtio_pci_common_cfg), 4, 0,
> + offsetofend(struct virtio_pci_modern_common_cfg,
> + queue_reset),
> + &mdev->common_len, NULL);
> if (!mdev->common)
> goto err_map_common;
> mdev->isr = vp_modern_map_capability(mdev, isr, sizeof(u8), 1,
> diff --git a/include/linux/virtio_pci_modern.h 
> b/include/linux/virtio_pci_modern.h
> index d0f2797420f7..a09e13a577a9 100644
> --- a/include/linux/virtio_pci_modern.h
> +++ b/include/linux/virtio_pci_modern.h
> @@ -5,13 +5,6 @@
>  #include 
>  #include 
>
> -struct virtio_pci_modern_common_cfg {
> -   struct virtio_pci_common_cfg cfg;
> -
> -   __le16 queue_notify_data;   /* read-write */
> -   __le16 queue_reset; /* read-write */
> -};
> -
>  /**
>   * struct virtio_pci_modern_device - info for modern PCI virtio
>   * @pci_dev:   Ptr to the PCI device struct
> diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
> index f703afc7ad31..44f4dd2add18 100644
> --- a/include/uapi/linux/virtio_pci.h
> +++ b/include/uapi/linux/virtio_pci.h
> @@ -166,6 +166,17 @@ struct virtio_pci_common_cfg {
> __le32 queue_used_hi;   /* read-write */
>  };
>
> +/*
> + * Warning: do not use sizeof on this: use offsetofend for
> + * specific fields you need.
> + */
> +struct virtio_pci_modern_common_cfg {
> +   struct virtio_pci_common_cfg cfg;
> +
> +   __le16 queue_notify_data;   /* read-write */
> +   __le16 queue_reset; /* read-write */
> +};
> +
>  /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
>  struct virtio_pci_cfg_cap {
> struct virtio_pci_cap cap;
> --
> MST
>

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH] virtio_pci: move structure to a header

2023-10-31 Thread Michael S. Tsirkin 
These are guest/host interfaces so belong in the header
where e.g. qemu will know to find them.
Note: we added a new structure as opposed to extending existing one
because someone might be relying on the size of the existing structure
staying unchanged.  Add a warning to avoid using sizeof.

Signed-off-by: Michael S. Tsirkin 
---
 drivers/virtio/virtio_pci_modern_dev.c |  7 ---
 include/linux/virtio_pci_modern.h  |  7 ---
 include/uapi/linux/virtio_pci.h| 11 +++
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/virtio/virtio_pci_modern_dev.c 
b/drivers/virtio/virtio_pci_modern_dev.c
index e2a1fe7bb66c..7de8b1ebabac 100644
--- a/drivers/virtio/virtio_pci_modern_dev.c
+++ b/drivers/virtio/virtio_pci_modern_dev.c
@@ -294,9 +294,10 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev)
 
err = -EINVAL;
mdev->common = vp_modern_map_capability(mdev, common,
- sizeof(struct virtio_pci_common_cfg), 4,
- 0, sizeof(struct 
virtio_pci_modern_common_cfg),
- &mdev->common_len, NULL);
+ sizeof(struct virtio_pci_common_cfg), 4, 0,
+ offsetofend(struct virtio_pci_modern_common_cfg,
+ queue_reset),
+ &mdev->common_len, NULL);
if (!mdev->common)
goto err_map_common;
mdev->isr = vp_modern_map_capability(mdev, isr, sizeof(u8), 1,
diff --git a/include/linux/virtio_pci_modern.h 
b/include/linux/virtio_pci_modern.h
index d0f2797420f7..a09e13a577a9 100644
--- a/include/linux/virtio_pci_modern.h
+++ b/include/linux/virtio_pci_modern.h
@@ -5,13 +5,6 @@
 #include 
 #include 
 
-struct virtio_pci_modern_common_cfg {
-   struct virtio_pci_common_cfg cfg;
-
-   __le16 queue_notify_data;   /* read-write */
-   __le16 queue_reset; /* read-write */
-};
-
 /**
  * struct virtio_pci_modern_device - info for modern PCI virtio
  * @pci_dev:   Ptr to the PCI device struct
diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index f703afc7ad31..44f4dd2add18 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -166,6 +166,17 @@ struct virtio_pci_common_cfg {
__le32 queue_used_hi;   /* read-write */
 };
 
+/*
+ * Warning: do not use sizeof on this: use offsetofend for
+ * specific fields you need.
+ */
+struct virtio_pci_modern_common_cfg {
+   struct virtio_pci_common_cfg cfg;
+
+   __le16 queue_notify_data;   /* read-write */
+   __le16 queue_reset; /* read-write */
+};
+
 /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
 struct virtio_pci_cfg_cap {
struct virtio_pci_cap cap;
-- 
MST

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization