Re: [PATCH v10 25/41] virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
On Tue, 28 Jun 2022 14:12:13 +0800, Jason Wang wrote:
> On Tue, Jun 28, 2022 at 2:10 PM Michael S. Tsirkin wrote:
> >
> > On Tue, Jun 28, 2022 at 02:07:28PM +0800, Jason Wang wrote:
> > > On Tue, Jun 28, 2022 at 1:46 PM Michael S. Tsirkin
> > > wrote:
> > > >
> > > > On Tue, Jun 28, 2022 at 11:50:37AM +0800, Jason Wang wrote:
> > > > > On Mon, Jun 27, 2022 at 7:53 PM Michael S. Tsirkin
> > > > > wrote:
> > > > > >
> > > > > > On Mon, Jun 27, 2022 at 04:14:20PM +0800, Jason Wang wrote:
> > > > > > > On Mon, Jun 27, 2022 at 3:58 PM Michael S. Tsirkin
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Mon, Jun 27, 2022 at 03:45:30PM +0800, Jason Wang wrote:
> > > > > > > > > On Mon, Jun 27, 2022 at 2:39 PM Michael S. Tsirkin
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > On Mon, Jun 27, 2022 at 10:30:42AM +0800, Jason Wang wrote:
> > > > > > > > > > > On Fri, Jun 24, 2022 at 2:59 PM Michael S. Tsirkin
> > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On Fri, Jun 24, 2022 at 10:56:05AM +0800, Xuan Zhuo
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > > Add queue_notify_data in struct
> > > > > > > > > > > > > virtio_pci_common_cfg, which comes from
> > > > > > > > > > > > > here
> > > > > > > > > > > > > https://github.com/oasis-tcs/virtio-spec/issues/89
> > > > > > > > > > > > >
> > > > > > > > > > > > > For not breaks uABI, add a new struct
> > > > > > > > > > > > > virtio_pci_common_cfg_notify.
> > > > > > > > > > > >
> > > > > > > > > > > > What exactly is meant by not breaking uABI?
> > > > > > > > > > > > Users are supposed to be prepared for struct size to
> > > > > > > > > > > > change ... no?
> > > > > > > > > > >
> > > > > > > > > > > Not sure, any doc for this?
> > > > > > > > > > >
> > > > > > > > > > > Thanks
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Well we have this:
> > > > > > > > > >
> > > > > > > > > > The drivers SHOULD only map part of configuration
> > > > > > > > > > structure
> > > > > > > > > > large enough for device operation. The drivers
> > > > > > > > > > MUST handle
> > > > > > > > > > an unexpectedly large \field{length}, but MAY check
> > > > > > > > > > that \field{length}
> > > > > > > > > > is large enough for device operation.
> > > > > > > > >
> > > > > > > > > Yes, but that's the device/driver interface. What's done here
> > > > > > > > > is the
> > > > > > > > > userspace/kernel.
> > > > > > > > >
> > > > > > > > > Userspace may break if it uses e.g sizeof(struct
> > > > > > > > > virtio_pci_common_cfg)?
> > > > > > > > >
> > > > > > > > > Thanks
> > > > > > > >
> > > > > > > > Hmm I guess there's risk... but then how are we going to
> > > > > > > > maintain this
> > > > > > > > going forward? Add a new struct on any change?
> > > > > > >
> > > > > > > This is the way we have used it for the past 5 or more years. I
> > > > > > > don't
> > > > > > > see why this must be handled in the vq reset feature.
> > > > > > >
> > > > > > > >Can we at least
> > > > > > > > prevent this going forward somehow?
> > > > > > >
> > > > > > > Like have some padding?
> > > > > > >
> > > > > > > Thanks
> > > > > >
> > > > > > Maybe - this is what QEMU does ...
> > > > >
> > > > > Do you want this to be addressed in this series (it's already very
> > > > > huge anyhow)?
> > > > >
> > > > > Thanks
> > > >
> > > > Let's come up with a solution at least. QEMU does not seem to need the
> > > > struct.
> > >
> > > If we want to implement it in Qemu we need that:
> > >
> > > https://github.com/fengidri/qemu/commit/39b79335cb55144d11a3b01f93d46cc73342c6bb
> > >
> > > > Let's just put
> > > > it in virtio_pci_modern.h for now then?
> > >
> > > Does this mean userspace needs to define the struct by their own
> > > instead of depending on the uapi in the future?
> > >
> > > Thanks
> >
> >
> > $ git grep 'struct virtio_pci_common_cfg'
> > include/standard-headers/linux/virtio_pci.h:struct virtio_pci_common_cfg {
> > tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> > virtio_pci_common_cfg,
> > tests/qtest/libqos/virtio-pci-modern.c:
> > offsetof(struct virtio_pci_common_cfg, device_feature));
> > tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> > virtio_pci_common_cfg,
> > tests/qtest/libqos/virtio-pci-modern.c:
> > offsetof(struct virtio_pci_common_cfg, device_feature));
> > tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> > virtio_pci_common_cfg,
> > tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> > virtio_pci_common_cfg,
> > tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> > virtio_pci_common_cfg,
> > tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> > virtio_pci_common_cfg,
> > tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> >
Re: [PATCH v10 25/41] virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
On Tue, Jun 28, 2022 at 2:10 PM Michael S. Tsirkin wrote:
>
> On Tue, Jun 28, 2022 at 02:07:28PM +0800, Jason Wang wrote:
> > On Tue, Jun 28, 2022 at 1:46 PM Michael S. Tsirkin wrote:
> > >
> > > On Tue, Jun 28, 2022 at 11:50:37AM +0800, Jason Wang wrote:
> > > > On Mon, Jun 27, 2022 at 7:53 PM Michael S. Tsirkin
> > > > wrote:
> > > > >
> > > > > On Mon, Jun 27, 2022 at 04:14:20PM +0800, Jason Wang wrote:
> > > > > > On Mon, Jun 27, 2022 at 3:58 PM Michael S. Tsirkin
> > > > > > wrote:
> > > > > > >
> > > > > > > On Mon, Jun 27, 2022 at 03:45:30PM +0800, Jason Wang wrote:
> > > > > > > > On Mon, Jun 27, 2022 at 2:39 PM Michael S. Tsirkin
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Mon, Jun 27, 2022 at 10:30:42AM +0800, Jason Wang wrote:
> > > > > > > > > > On Fri, Jun 24, 2022 at 2:59 PM Michael S. Tsirkin
> > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Fri, Jun 24, 2022 at 10:56:05AM +0800, Xuan Zhuo wrote:
> > > > > > > > > > > > Add queue_notify_data in struct virtio_pci_common_cfg,
> > > > > > > > > > > > which comes from
> > > > > > > > > > > > here https://github.com/oasis-tcs/virtio-spec/issues/89
> > > > > > > > > > > >
> > > > > > > > > > > > For not breaks uABI, add a new struct
> > > > > > > > > > > > virtio_pci_common_cfg_notify.
> > > > > > > > > > >
> > > > > > > > > > > What exactly is meant by not breaking uABI?
> > > > > > > > > > > Users are supposed to be prepared for struct size to
> > > > > > > > > > > change ... no?
> > > > > > > > > >
> > > > > > > > > > Not sure, any doc for this?
> > > > > > > > > >
> > > > > > > > > > Thanks
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Well we have this:
> > > > > > > > >
> > > > > > > > > The drivers SHOULD only map part of configuration
> > > > > > > > > structure
> > > > > > > > > large enough for device operation. The drivers MUST
> > > > > > > > > handle
> > > > > > > > > an unexpectedly large \field{length}, but MAY check
> > > > > > > > > that \field{length}
> > > > > > > > > is large enough for device operation.
> > > > > > > >
> > > > > > > > Yes, but that's the device/driver interface. What's done here
> > > > > > > > is the
> > > > > > > > userspace/kernel.
> > > > > > > >
> > > > > > > > Userspace may break if it uses e.g sizeof(struct
> > > > > > > > virtio_pci_common_cfg)?
> > > > > > > >
> > > > > > > > Thanks
> > > > > > >
> > > > > > > Hmm I guess there's risk... but then how are we going to maintain
> > > > > > > this
> > > > > > > going forward? Add a new struct on any change?
> > > > > >
> > > > > > This is the way we have used it for the past 5 or more years. I
> > > > > > don't
> > > > > > see why this must be handled in the vq reset feature.
> > > > > >
> > > > > > >Can we at least
> > > > > > > prevent this going forward somehow?
> > > > > >
> > > > > > Like have some padding?
> > > > > >
> > > > > > Thanks
> > > > >
> > > > > Maybe - this is what QEMU does ...
> > > >
> > > > Do you want this to be addressed in this series (it's already very huge
> > > > anyhow)?
> > > >
> > > > Thanks
> > >
> > > Let's come up with a solution at least. QEMU does not seem to need the
> > > struct.
> >
> > If we want to implement it in Qemu we need that:
> >
> > https://github.com/fengidri/qemu/commit/39b79335cb55144d11a3b01f93d46cc73342c6bb
> >
> > > Let's just put
> > > it in virtio_pci_modern.h for now then?
> >
> > Does this mean userspace needs to define the struct by their own
> > instead of depending on the uapi in the future?
> >
> > Thanks
>
>
> $ git grep 'struct virtio_pci_common_cfg'
> include/standard-headers/linux/virtio_pci.h:struct virtio_pci_common_cfg {
> tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> virtio_pci_common_cfg,
> tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> virtio_pci_common_cfg, device_feature));
> tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> virtio_pci_common_cfg,
> tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> virtio_pci_common_cfg, device_feature));
> tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> virtio_pci_common_cfg,
> tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> virtio_pci_common_cfg,
> tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> virtio_pci_common_cfg,
> tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> virtio_pci_common_cfg,
> tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> virtio_pci_common_cfg,
> tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> virtio_pci_common_cfg, guest_feature));
> tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
> virtio_pci_common_cfg,
> tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
>
Re: [PATCH v10 25/41] virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
On Tue, Jun 28, 2022 at 02:07:28PM +0800, Jason Wang wrote:
> On Tue, Jun 28, 2022 at 1:46 PM Michael S. Tsirkin wrote:
> >
> > On Tue, Jun 28, 2022 at 11:50:37AM +0800, Jason Wang wrote:
> > > On Mon, Jun 27, 2022 at 7:53 PM Michael S. Tsirkin
> > > wrote:
> > > >
> > > > On Mon, Jun 27, 2022 at 04:14:20PM +0800, Jason Wang wrote:
> > > > > On Mon, Jun 27, 2022 at 3:58 PM Michael S. Tsirkin
> > > > > wrote:
> > > > > >
> > > > > > On Mon, Jun 27, 2022 at 03:45:30PM +0800, Jason Wang wrote:
> > > > > > > On Mon, Jun 27, 2022 at 2:39 PM Michael S. Tsirkin
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Mon, Jun 27, 2022 at 10:30:42AM +0800, Jason Wang wrote:
> > > > > > > > > On Fri, Jun 24, 2022 at 2:59 PM Michael S. Tsirkin
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > On Fri, Jun 24, 2022 at 10:56:05AM +0800, Xuan Zhuo wrote:
> > > > > > > > > > > Add queue_notify_data in struct virtio_pci_common_cfg,
> > > > > > > > > > > which comes from
> > > > > > > > > > > here https://github.com/oasis-tcs/virtio-spec/issues/89
> > > > > > > > > > >
> > > > > > > > > > > For not breaks uABI, add a new struct
> > > > > > > > > > > virtio_pci_common_cfg_notify.
> > > > > > > > > >
> > > > > > > > > > What exactly is meant by not breaking uABI?
> > > > > > > > > > Users are supposed to be prepared for struct size to change
> > > > > > > > > > ... no?
> > > > > > > > >
> > > > > > > > > Not sure, any doc for this?
> > > > > > > > >
> > > > > > > > > Thanks
> > > > > > > >
> > > > > > > >
> > > > > > > > Well we have this:
> > > > > > > >
> > > > > > > > The drivers SHOULD only map part of configuration
> > > > > > > > structure
> > > > > > > > large enough for device operation. The drivers MUST
> > > > > > > > handle
> > > > > > > > an unexpectedly large \field{length}, but MAY check
> > > > > > > > that \field{length}
> > > > > > > > is large enough for device operation.
> > > > > > >
> > > > > > > Yes, but that's the device/driver interface. What's done here is
> > > > > > > the
> > > > > > > userspace/kernel.
> > > > > > >
> > > > > > > Userspace may break if it uses e.g sizeof(struct
> > > > > > > virtio_pci_common_cfg)?
> > > > > > >
> > > > > > > Thanks
> > > > > >
> > > > > > Hmm I guess there's risk... but then how are we going to maintain
> > > > > > this
> > > > > > going forward? Add a new struct on any change?
> > > > >
> > > > > This is the way we have used it for the past 5 or more years. I don't
> > > > > see why this must be handled in the vq reset feature.
> > > > >
> > > > > >Can we at least
> > > > > > prevent this going forward somehow?
> > > > >
> > > > > Like have some padding?
> > > > >
> > > > > Thanks
> > > >
> > > > Maybe - this is what QEMU does ...
> > >
> > > Do you want this to be addressed in this series (it's already very huge
> > > anyhow)?
> > >
> > > Thanks
> >
> > Let's come up with a solution at least. QEMU does not seem to need the
> > struct.
>
> If we want to implement it in Qemu we need that:
>
> https://github.com/fengidri/qemu/commit/39b79335cb55144d11a3b01f93d46cc73342c6bb
>
> > Let's just put
> > it in virtio_pci_modern.h for now then?
>
> Does this mean userspace needs to define the struct by their own
> instead of depending on the uapi in the future?
>
> Thanks
$ git grep 'struct virtio_pci_common_cfg'
include/standard-headers/linux/virtio_pci.h:struct virtio_pci_common_cfg {
tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
virtio_pci_common_cfg,
tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
virtio_pci_common_cfg, device_feature));
tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
virtio_pci_common_cfg,
tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
virtio_pci_common_cfg, device_feature));
tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
virtio_pci_common_cfg,
tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
virtio_pci_common_cfg,
tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
virtio_pci_common_cfg,
tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
virtio_pci_common_cfg,
tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
virtio_pci_common_cfg,
tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
virtio_pci_common_cfg, guest_feature));
tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
virtio_pci_common_cfg,
tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
virtio_pci_common_cfg, guest_feature));
tests/qtest/libqos/virtio-pci-modern.c: offsetof(struct
virtio_pci_common_cfg,
tests/qtest/libqos/virtio-pci-modern.c:
offsetof(struct virtio_pci_common_cfg,
tests/qtest/libqos/virtio-pci-modern.c:
Re: [PATCH v10 25/41] virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
On Tue, Jun 28, 2022 at 1:46 PM Michael S. Tsirkin wrote:
>
> On Tue, Jun 28, 2022 at 11:50:37AM +0800, Jason Wang wrote:
> > On Mon, Jun 27, 2022 at 7:53 PM Michael S. Tsirkin wrote:
> > >
> > > On Mon, Jun 27, 2022 at 04:14:20PM +0800, Jason Wang wrote:
> > > > On Mon, Jun 27, 2022 at 3:58 PM Michael S. Tsirkin
> > > > wrote:
> > > > >
> > > > > On Mon, Jun 27, 2022 at 03:45:30PM +0800, Jason Wang wrote:
> > > > > > On Mon, Jun 27, 2022 at 2:39 PM Michael S. Tsirkin
> > > > > > wrote:
> > > > > > >
> > > > > > > On Mon, Jun 27, 2022 at 10:30:42AM +0800, Jason Wang wrote:
> > > > > > > > On Fri, Jun 24, 2022 at 2:59 PM Michael S. Tsirkin
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Fri, Jun 24, 2022 at 10:56:05AM +0800, Xuan Zhuo wrote:
> > > > > > > > > > Add queue_notify_data in struct virtio_pci_common_cfg,
> > > > > > > > > > which comes from
> > > > > > > > > > here https://github.com/oasis-tcs/virtio-spec/issues/89
> > > > > > > > > >
> > > > > > > > > > For not breaks uABI, add a new struct
> > > > > > > > > > virtio_pci_common_cfg_notify.
> > > > > > > > >
> > > > > > > > > What exactly is meant by not breaking uABI?
> > > > > > > > > Users are supposed to be prepared for struct size to change
> > > > > > > > > ... no?
> > > > > > > >
> > > > > > > > Not sure, any doc for this?
> > > > > > > >
> > > > > > > > Thanks
> > > > > > >
> > > > > > >
> > > > > > > Well we have this:
> > > > > > >
> > > > > > > The drivers SHOULD only map part of configuration
> > > > > > > structure
> > > > > > > large enough for device operation. The drivers MUST
> > > > > > > handle
> > > > > > > an unexpectedly large \field{length}, but MAY check that
> > > > > > > \field{length}
> > > > > > > is large enough for device operation.
> > > > > >
> > > > > > Yes, but that's the device/driver interface. What's done here is the
> > > > > > userspace/kernel.
> > > > > >
> > > > > > Userspace may break if it uses e.g sizeof(struct
> > > > > > virtio_pci_common_cfg)?
> > > > > >
> > > > > > Thanks
> > > > >
> > > > > Hmm I guess there's risk... but then how are we going to maintain this
> > > > > going forward? Add a new struct on any change?
> > > >
> > > > This is the way we have used it for the past 5 or more years. I don't
> > > > see why this must be handled in the vq reset feature.
> > > >
> > > > >Can we at least
> > > > > prevent this going forward somehow?
> > > >
> > > > Like have some padding?
> > > >
> > > > Thanks
> > >
> > > Maybe - this is what QEMU does ...
> >
> > Do you want this to be addressed in this series (it's already very huge
> > anyhow)?
> >
> > Thanks
>
> Let's come up with a solution at least. QEMU does not seem to need the struct.
If we want to implement it in Qemu we need that:
https://github.com/fengidri/qemu/commit/39b79335cb55144d11a3b01f93d46cc73342c6bb
> Let's just put
> it in virtio_pci_modern.h for now then?
Does this mean userspace needs to define the struct by their own
instead of depending on the uapi in the future?
Thanks
>
> > >
> > > > >
> > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > Since I want to add queue_reset after queue_notify_data, I
> > > > > > > > > > submitted
> > > > > > > > > > this patch first.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Xuan Zhuo
> > > > > > > > > > Acked-by: Jason Wang
> > > > > > > > > > ---
> > > > > > > > > > include/uapi/linux/virtio_pci.h | 7 +++
> > > > > > > > > > 1 file changed, 7 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git a/include/uapi/linux/virtio_pci.h
> > > > > > > > > > b/include/uapi/linux/virtio_pci.h
> > > > > > > > > > index 3a86f36d7e3d..22bec9bd0dfc 100644
> > > > > > > > > > --- a/include/uapi/linux/virtio_pci.h
> > > > > > > > > > +++ b/include/uapi/linux/virtio_pci.h
> > > > > > > > > > @@ -166,6 +166,13 @@ struct virtio_pci_common_cfg {
> > > > > > > > > > __le32 queue_used_hi; /* read-write */
> > > > > > > > > > };
> > > > > > > > > >
> > > > > > > > > > +struct virtio_pci_common_cfg_notify {
> > > > > > > > > > + struct virtio_pci_common_cfg cfg;
> > > > > > > > > > +
> > > > > > > > > > + __le16 queue_notify_data; /* read-write */
> > > > > > > > > > + __le16 padding;
> > > > > > > > > > +};
> > > > > > > > > > +
> > > > > > > > > > /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> > > > > > > > > > struct virtio_pci_cfg_cap {
> > > > > > > > > > struct virtio_pci_cap cap;
> > > > > > > > > > --
> > > > > > > > > > 2.31.0
> > > > > > > > >
> > > > > > >
> > > > >
> > >
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v10 25/41] virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
On Tue, Jun 28, 2022 at 11:50:37AM +0800, Jason Wang wrote:
> On Mon, Jun 27, 2022 at 7:53 PM Michael S. Tsirkin wrote:
> >
> > On Mon, Jun 27, 2022 at 04:14:20PM +0800, Jason Wang wrote:
> > > On Mon, Jun 27, 2022 at 3:58 PM Michael S. Tsirkin
> > > wrote:
> > > >
> > > > On Mon, Jun 27, 2022 at 03:45:30PM +0800, Jason Wang wrote:
> > > > > On Mon, Jun 27, 2022 at 2:39 PM Michael S. Tsirkin
> > > > > wrote:
> > > > > >
> > > > > > On Mon, Jun 27, 2022 at 10:30:42AM +0800, Jason Wang wrote:
> > > > > > > On Fri, Jun 24, 2022 at 2:59 PM Michael S. Tsirkin
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Fri, Jun 24, 2022 at 10:56:05AM +0800, Xuan Zhuo wrote:
> > > > > > > > > Add queue_notify_data in struct virtio_pci_common_cfg, which
> > > > > > > > > comes from
> > > > > > > > > here https://github.com/oasis-tcs/virtio-spec/issues/89
> > > > > > > > >
> > > > > > > > > For not breaks uABI, add a new struct
> > > > > > > > > virtio_pci_common_cfg_notify.
> > > > > > > >
> > > > > > > > What exactly is meant by not breaking uABI?
> > > > > > > > Users are supposed to be prepared for struct size to change ...
> > > > > > > > no?
> > > > > > >
> > > > > > > Not sure, any doc for this?
> > > > > > >
> > > > > > > Thanks
> > > > > >
> > > > > >
> > > > > > Well we have this:
> > > > > >
> > > > > > The drivers SHOULD only map part of configuration structure
> > > > > > large enough for device operation. The drivers MUST handle
> > > > > > an unexpectedly large \field{length}, but MAY check that
> > > > > > \field{length}
> > > > > > is large enough for device operation.
> > > > >
> > > > > Yes, but that's the device/driver interface. What's done here is the
> > > > > userspace/kernel.
> > > > >
> > > > > Userspace may break if it uses e.g sizeof(struct
> > > > > virtio_pci_common_cfg)?
> > > > >
> > > > > Thanks
> > > >
> > > > Hmm I guess there's risk... but then how are we going to maintain this
> > > > going forward? Add a new struct on any change?
> > >
> > > This is the way we have used it for the past 5 or more years. I don't
> > > see why this must be handled in the vq reset feature.
> > >
> > > >Can we at least
> > > > prevent this going forward somehow?
> > >
> > > Like have some padding?
> > >
> > > Thanks
> >
> > Maybe - this is what QEMU does ...
>
> Do you want this to be addressed in this series (it's already very huge
> anyhow)?
>
> Thanks
Let's come up with a solution at least. QEMU does not seem to need the struct.
Let's just put
it in virtio_pci_modern.h for now then?
> >
> > > >
> > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > > Since I want to add queue_reset after queue_notify_data, I
> > > > > > > > > submitted
> > > > > > > > > this patch first.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Xuan Zhuo
> > > > > > > > > Acked-by: Jason Wang
> > > > > > > > > ---
> > > > > > > > > include/uapi/linux/virtio_pci.h | 7 +++
> > > > > > > > > 1 file changed, 7 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git a/include/uapi/linux/virtio_pci.h
> > > > > > > > > b/include/uapi/linux/virtio_pci.h
> > > > > > > > > index 3a86f36d7e3d..22bec9bd0dfc 100644
> > > > > > > > > --- a/include/uapi/linux/virtio_pci.h
> > > > > > > > > +++ b/include/uapi/linux/virtio_pci.h
> > > > > > > > > @@ -166,6 +166,13 @@ struct virtio_pci_common_cfg {
> > > > > > > > > __le32 queue_used_hi; /* read-write */
> > > > > > > > > };
> > > > > > > > >
> > > > > > > > > +struct virtio_pci_common_cfg_notify {
> > > > > > > > > + struct virtio_pci_common_cfg cfg;
> > > > > > > > > +
> > > > > > > > > + __le16 queue_notify_data; /* read-write */
> > > > > > > > > + __le16 padding;
> > > > > > > > > +};
> > > > > > > > > +
> > > > > > > > > /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> > > > > > > > > struct virtio_pci_cfg_cap {
> > > > > > > > > struct virtio_pci_cap cap;
> > > > > > > > > --
> > > > > > > > > 2.31.0
> > > > > > > >
> > > > > >
> > > >
> >
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v10 25/41] virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
On Mon, Jun 27, 2022 at 7:53 PM Michael S. Tsirkin wrote:
>
> On Mon, Jun 27, 2022 at 04:14:20PM +0800, Jason Wang wrote:
> > On Mon, Jun 27, 2022 at 3:58 PM Michael S. Tsirkin wrote:
> > >
> > > On Mon, Jun 27, 2022 at 03:45:30PM +0800, Jason Wang wrote:
> > > > On Mon, Jun 27, 2022 at 2:39 PM Michael S. Tsirkin
> > > > wrote:
> > > > >
> > > > > On Mon, Jun 27, 2022 at 10:30:42AM +0800, Jason Wang wrote:
> > > > > > On Fri, Jun 24, 2022 at 2:59 PM Michael S. Tsirkin
> > > > > > wrote:
> > > > > > >
> > > > > > > On Fri, Jun 24, 2022 at 10:56:05AM +0800, Xuan Zhuo wrote:
> > > > > > > > Add queue_notify_data in struct virtio_pci_common_cfg, which
> > > > > > > > comes from
> > > > > > > > here https://github.com/oasis-tcs/virtio-spec/issues/89
> > > > > > > >
> > > > > > > > For not breaks uABI, add a new struct
> > > > > > > > virtio_pci_common_cfg_notify.
> > > > > > >
> > > > > > > What exactly is meant by not breaking uABI?
> > > > > > > Users are supposed to be prepared for struct size to change ...
> > > > > > > no?
> > > > > >
> > > > > > Not sure, any doc for this?
> > > > > >
> > > > > > Thanks
> > > > >
> > > > >
> > > > > Well we have this:
> > > > >
> > > > > The drivers SHOULD only map part of configuration structure
> > > > > large enough for device operation. The drivers MUST handle
> > > > > an unexpectedly large \field{length}, but MAY check that
> > > > > \field{length}
> > > > > is large enough for device operation.
> > > >
> > > > Yes, but that's the device/driver interface. What's done here is the
> > > > userspace/kernel.
> > > >
> > > > Userspace may break if it uses e.g sizeof(struct virtio_pci_common_cfg)?
> > > >
> > > > Thanks
> > >
> > > Hmm I guess there's risk... but then how are we going to maintain this
> > > going forward? Add a new struct on any change?
> >
> > This is the way we have used it for the past 5 or more years. I don't
> > see why this must be handled in the vq reset feature.
> >
> > >Can we at least
> > > prevent this going forward somehow?
> >
> > Like have some padding?
> >
> > Thanks
>
> Maybe - this is what QEMU does ...
Do you want this to be addressed in this series (it's already very huge anyhow)?
Thanks
>
> > >
> > >
> > > > >
> > > > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > Since I want to add queue_reset after queue_notify_data, I
> > > > > > > > submitted
> > > > > > > > this patch first.
> > > > > > > >
> > > > > > > > Signed-off-by: Xuan Zhuo
> > > > > > > > Acked-by: Jason Wang
> > > > > > > > ---
> > > > > > > > include/uapi/linux/virtio_pci.h | 7 +++
> > > > > > > > 1 file changed, 7 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/include/uapi/linux/virtio_pci.h
> > > > > > > > b/include/uapi/linux/virtio_pci.h
> > > > > > > > index 3a86f36d7e3d..22bec9bd0dfc 100644
> > > > > > > > --- a/include/uapi/linux/virtio_pci.h
> > > > > > > > +++ b/include/uapi/linux/virtio_pci.h
> > > > > > > > @@ -166,6 +166,13 @@ struct virtio_pci_common_cfg {
> > > > > > > > __le32 queue_used_hi; /* read-write */
> > > > > > > > };
> > > > > > > >
> > > > > > > > +struct virtio_pci_common_cfg_notify {
> > > > > > > > + struct virtio_pci_common_cfg cfg;
> > > > > > > > +
> > > > > > > > + __le16 queue_notify_data; /* read-write */
> > > > > > > > + __le16 padding;
> > > > > > > > +};
> > > > > > > > +
> > > > > > > > /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> > > > > > > > struct virtio_pci_cfg_cap {
> > > > > > > > struct virtio_pci_cap cap;
> > > > > > > > --
> > > > > > > > 2.31.0
> > > > > > >
> > > > >
> > >
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v10 25/41] virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
On Mon, Jun 27, 2022 at 04:14:20PM +0800, Jason Wang wrote:
> On Mon, Jun 27, 2022 at 3:58 PM Michael S. Tsirkin wrote:
> >
> > On Mon, Jun 27, 2022 at 03:45:30PM +0800, Jason Wang wrote:
> > > On Mon, Jun 27, 2022 at 2:39 PM Michael S. Tsirkin
> > > wrote:
> > > >
> > > > On Mon, Jun 27, 2022 at 10:30:42AM +0800, Jason Wang wrote:
> > > > > On Fri, Jun 24, 2022 at 2:59 PM Michael S. Tsirkin
> > > > > wrote:
> > > > > >
> > > > > > On Fri, Jun 24, 2022 at 10:56:05AM +0800, Xuan Zhuo wrote:
> > > > > > > Add queue_notify_data in struct virtio_pci_common_cfg, which
> > > > > > > comes from
> > > > > > > here https://github.com/oasis-tcs/virtio-spec/issues/89
> > > > > > >
> > > > > > > For not breaks uABI, add a new struct
> > > > > > > virtio_pci_common_cfg_notify.
> > > > > >
> > > > > > What exactly is meant by not breaking uABI?
> > > > > > Users are supposed to be prepared for struct size to change ... no?
> > > > >
> > > > > Not sure, any doc for this?
> > > > >
> > > > > Thanks
> > > >
> > > >
> > > > Well we have this:
> > > >
> > > > The drivers SHOULD only map part of configuration structure
> > > > large enough for device operation. The drivers MUST handle
> > > > an unexpectedly large \field{length}, but MAY check that
> > > > \field{length}
> > > > is large enough for device operation.
> > >
> > > Yes, but that's the device/driver interface. What's done here is the
> > > userspace/kernel.
> > >
> > > Userspace may break if it uses e.g sizeof(struct virtio_pci_common_cfg)?
> > >
> > > Thanks
> >
> > Hmm I guess there's risk... but then how are we going to maintain this
> > going forward? Add a new struct on any change?
>
> This is the way we have used it for the past 5 or more years. I don't
> see why this must be handled in the vq reset feature.
>
> >Can we at least
> > prevent this going forward somehow?
>
> Like have some padding?
>
> Thanks
Maybe - this is what QEMU does ...
> >
> >
> > > >
> > > >
> > > >
> > > > >
> > > > > >
> > > > > >
> > > > > > > Since I want to add queue_reset after queue_notify_data, I
> > > > > > > submitted
> > > > > > > this patch first.
> > > > > > >
> > > > > > > Signed-off-by: Xuan Zhuo
> > > > > > > Acked-by: Jason Wang
> > > > > > > ---
> > > > > > > include/uapi/linux/virtio_pci.h | 7 +++
> > > > > > > 1 file changed, 7 insertions(+)
> > > > > > >
> > > > > > > diff --git a/include/uapi/linux/virtio_pci.h
> > > > > > > b/include/uapi/linux/virtio_pci.h
> > > > > > > index 3a86f36d7e3d..22bec9bd0dfc 100644
> > > > > > > --- a/include/uapi/linux/virtio_pci.h
> > > > > > > +++ b/include/uapi/linux/virtio_pci.h
> > > > > > > @@ -166,6 +166,13 @@ struct virtio_pci_common_cfg {
> > > > > > > __le32 queue_used_hi; /* read-write */
> > > > > > > };
> > > > > > >
> > > > > > > +struct virtio_pci_common_cfg_notify {
> > > > > > > + struct virtio_pci_common_cfg cfg;
> > > > > > > +
> > > > > > > + __le16 queue_notify_data; /* read-write */
> > > > > > > + __le16 padding;
> > > > > > > +};
> > > > > > > +
> > > > > > > /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> > > > > > > struct virtio_pci_cfg_cap {
> > > > > > > struct virtio_pci_cap cap;
> > > > > > > --
> > > > > > > 2.31.0
> > > > > >
> > > >
> >
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v10 25/41] virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
On Mon, Jun 27, 2022 at 3:58 PM Michael S. Tsirkin wrote:
>
> On Mon, Jun 27, 2022 at 03:45:30PM +0800, Jason Wang wrote:
> > On Mon, Jun 27, 2022 at 2:39 PM Michael S. Tsirkin wrote:
> > >
> > > On Mon, Jun 27, 2022 at 10:30:42AM +0800, Jason Wang wrote:
> > > > On Fri, Jun 24, 2022 at 2:59 PM Michael S. Tsirkin
> > > > wrote:
> > > > >
> > > > > On Fri, Jun 24, 2022 at 10:56:05AM +0800, Xuan Zhuo wrote:
> > > > > > Add queue_notify_data in struct virtio_pci_common_cfg, which comes
> > > > > > from
> > > > > > here https://github.com/oasis-tcs/virtio-spec/issues/89
> > > > > >
> > > > > > For not breaks uABI, add a new struct virtio_pci_common_cfg_notify.
> > > > >
> > > > > What exactly is meant by not breaking uABI?
> > > > > Users are supposed to be prepared for struct size to change ... no?
> > > >
> > > > Not sure, any doc for this?
> > > >
> > > > Thanks
> > >
> > >
> > > Well we have this:
> > >
> > > The drivers SHOULD only map part of configuration structure
> > > large enough for device operation. The drivers MUST handle
> > > an unexpectedly large \field{length}, but MAY check that
> > > \field{length}
> > > is large enough for device operation.
> >
> > Yes, but that's the device/driver interface. What's done here is the
> > userspace/kernel.
> >
> > Userspace may break if it uses e.g sizeof(struct virtio_pci_common_cfg)?
> >
> > Thanks
>
> Hmm I guess there's risk... but then how are we going to maintain this
> going forward? Add a new struct on any change?
This is the way we have used it for the past 5 or more years. I don't
see why this must be handled in the vq reset feature.
>Can we at least
> prevent this going forward somehow?
Like have some padding?
Thanks
>
>
> > >
> > >
> > >
> > > >
> > > > >
> > > > >
> > > > > > Since I want to add queue_reset after queue_notify_data, I submitted
> > > > > > this patch first.
> > > > > >
> > > > > > Signed-off-by: Xuan Zhuo
> > > > > > Acked-by: Jason Wang
> > > > > > ---
> > > > > > include/uapi/linux/virtio_pci.h | 7 +++
> > > > > > 1 file changed, 7 insertions(+)
> > > > > >
> > > > > > diff --git a/include/uapi/linux/virtio_pci.h
> > > > > > b/include/uapi/linux/virtio_pci.h
> > > > > > index 3a86f36d7e3d..22bec9bd0dfc 100644
> > > > > > --- a/include/uapi/linux/virtio_pci.h
> > > > > > +++ b/include/uapi/linux/virtio_pci.h
> > > > > > @@ -166,6 +166,13 @@ struct virtio_pci_common_cfg {
> > > > > > __le32 queue_used_hi; /* read-write */
> > > > > > };
> > > > > >
> > > > > > +struct virtio_pci_common_cfg_notify {
> > > > > > + struct virtio_pci_common_cfg cfg;
> > > > > > +
> > > > > > + __le16 queue_notify_data; /* read-write */
> > > > > > + __le16 padding;
> > > > > > +};
> > > > > > +
> > > > > > /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> > > > > > struct virtio_pci_cfg_cap {
> > > > > > struct virtio_pci_cap cap;
> > > > > > --
> > > > > > 2.31.0
> > > > >
> > >
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v10 25/41] virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
On Mon, Jun 27, 2022 at 03:45:30PM +0800, Jason Wang wrote:
> On Mon, Jun 27, 2022 at 2:39 PM Michael S. Tsirkin wrote:
> >
> > On Mon, Jun 27, 2022 at 10:30:42AM +0800, Jason Wang wrote:
> > > On Fri, Jun 24, 2022 at 2:59 PM Michael S. Tsirkin
> > > wrote:
> > > >
> > > > On Fri, Jun 24, 2022 at 10:56:05AM +0800, Xuan Zhuo wrote:
> > > > > Add queue_notify_data in struct virtio_pci_common_cfg, which comes
> > > > > from
> > > > > here https://github.com/oasis-tcs/virtio-spec/issues/89
> > > > >
> > > > > For not breaks uABI, add a new struct virtio_pci_common_cfg_notify.
> > > >
> > > > What exactly is meant by not breaking uABI?
> > > > Users are supposed to be prepared for struct size to change ... no?
> > >
> > > Not sure, any doc for this?
> > >
> > > Thanks
> >
> >
> > Well we have this:
> >
> > The drivers SHOULD only map part of configuration structure
> > large enough for device operation. The drivers MUST handle
> > an unexpectedly large \field{length}, but MAY check that
> > \field{length}
> > is large enough for device operation.
>
> Yes, but that's the device/driver interface. What's done here is the
> userspace/kernel.
>
> Userspace may break if it uses e.g sizeof(struct virtio_pci_common_cfg)?
>
> Thanks
Hmm I guess there's risk... but then how are we going to maintain this
going forward? Add a new struct on any change? Can we at least
prevent this going forward somehow?
> >
> >
> >
> > >
> > > >
> > > >
> > > > > Since I want to add queue_reset after queue_notify_data, I submitted
> > > > > this patch first.
> > > > >
> > > > > Signed-off-by: Xuan Zhuo
> > > > > Acked-by: Jason Wang
> > > > > ---
> > > > > include/uapi/linux/virtio_pci.h | 7 +++
> > > > > 1 file changed, 7 insertions(+)
> > > > >
> > > > > diff --git a/include/uapi/linux/virtio_pci.h
> > > > > b/include/uapi/linux/virtio_pci.h
> > > > > index 3a86f36d7e3d..22bec9bd0dfc 100644
> > > > > --- a/include/uapi/linux/virtio_pci.h
> > > > > +++ b/include/uapi/linux/virtio_pci.h
> > > > > @@ -166,6 +166,13 @@ struct virtio_pci_common_cfg {
> > > > > __le32 queue_used_hi; /* read-write */
> > > > > };
> > > > >
> > > > > +struct virtio_pci_common_cfg_notify {
> > > > > + struct virtio_pci_common_cfg cfg;
> > > > > +
> > > > > + __le16 queue_notify_data; /* read-write */
> > > > > + __le16 padding;
> > > > > +};
> > > > > +
> > > > > /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> > > > > struct virtio_pci_cfg_cap {
> > > > > struct virtio_pci_cap cap;
> > > > > --
> > > > > 2.31.0
> > > >
> >
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v10 25/41] virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
On Mon, Jun 27, 2022 at 2:39 PM Michael S. Tsirkin wrote:
>
> On Mon, Jun 27, 2022 at 10:30:42AM +0800, Jason Wang wrote:
> > On Fri, Jun 24, 2022 at 2:59 PM Michael S. Tsirkin wrote:
> > >
> > > On Fri, Jun 24, 2022 at 10:56:05AM +0800, Xuan Zhuo wrote:
> > > > Add queue_notify_data in struct virtio_pci_common_cfg, which comes from
> > > > here https://github.com/oasis-tcs/virtio-spec/issues/89
> > > >
> > > > For not breaks uABI, add a new struct virtio_pci_common_cfg_notify.
> > >
> > > What exactly is meant by not breaking uABI?
> > > Users are supposed to be prepared for struct size to change ... no?
> >
> > Not sure, any doc for this?
> >
> > Thanks
>
>
> Well we have this:
>
> The drivers SHOULD only map part of configuration structure
> large enough for device operation. The drivers MUST handle
> an unexpectedly large \field{length}, but MAY check that
> \field{length}
> is large enough for device operation.
Yes, but that's the device/driver interface. What's done here is the
userspace/kernel.
Userspace may break if it uses e.g sizeof(struct virtio_pci_common_cfg)?
Thanks
>
>
>
> >
> > >
> > >
> > > > Since I want to add queue_reset after queue_notify_data, I submitted
> > > > this patch first.
> > > >
> > > > Signed-off-by: Xuan Zhuo
> > > > Acked-by: Jason Wang
> > > > ---
> > > > include/uapi/linux/virtio_pci.h | 7 +++
> > > > 1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/include/uapi/linux/virtio_pci.h
> > > > b/include/uapi/linux/virtio_pci.h
> > > > index 3a86f36d7e3d..22bec9bd0dfc 100644
> > > > --- a/include/uapi/linux/virtio_pci.h
> > > > +++ b/include/uapi/linux/virtio_pci.h
> > > > @@ -166,6 +166,13 @@ struct virtio_pci_common_cfg {
> > > > __le32 queue_used_hi; /* read-write */
> > > > };
> > > >
> > > > +struct virtio_pci_common_cfg_notify {
> > > > + struct virtio_pci_common_cfg cfg;
> > > > +
> > > > + __le16 queue_notify_data; /* read-write */
> > > > + __le16 padding;
> > > > +};
> > > > +
> > > > /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> > > > struct virtio_pci_cfg_cap {
> > > > struct virtio_pci_cap cap;
> > > > --
> > > > 2.31.0
> > >
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v10 25/41] virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
On Mon, Jun 27, 2022 at 10:30:42AM +0800, Jason Wang wrote:
> On Fri, Jun 24, 2022 at 2:59 PM Michael S. Tsirkin wrote:
> >
> > On Fri, Jun 24, 2022 at 10:56:05AM +0800, Xuan Zhuo wrote:
> > > Add queue_notify_data in struct virtio_pci_common_cfg, which comes from
> > > here https://github.com/oasis-tcs/virtio-spec/issues/89
> > >
> > > For not breaks uABI, add a new struct virtio_pci_common_cfg_notify.
> >
> > What exactly is meant by not breaking uABI?
> > Users are supposed to be prepared for struct size to change ... no?
>
> Not sure, any doc for this?
>
> Thanks
Well we have this:
The drivers SHOULD only map part of configuration structure
large enough for device operation. The drivers MUST handle
an unexpectedly large \field{length}, but MAY check that \field{length}
is large enough for device operation.
>
> >
> >
> > > Since I want to add queue_reset after queue_notify_data, I submitted
> > > this patch first.
> > >
> > > Signed-off-by: Xuan Zhuo
> > > Acked-by: Jason Wang
> > > ---
> > > include/uapi/linux/virtio_pci.h | 7 +++
> > > 1 file changed, 7 insertions(+)
> > >
> > > diff --git a/include/uapi/linux/virtio_pci.h
> > > b/include/uapi/linux/virtio_pci.h
> > > index 3a86f36d7e3d..22bec9bd0dfc 100644
> > > --- a/include/uapi/linux/virtio_pci.h
> > > +++ b/include/uapi/linux/virtio_pci.h
> > > @@ -166,6 +166,13 @@ struct virtio_pci_common_cfg {
> > > __le32 queue_used_hi; /* read-write */
> > > };
> > >
> > > +struct virtio_pci_common_cfg_notify {
> > > + struct virtio_pci_common_cfg cfg;
> > > +
> > > + __le16 queue_notify_data; /* read-write */
> > > + __le16 padding;
> > > +};
> > > +
> > > /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> > > struct virtio_pci_cfg_cap {
> > > struct virtio_pci_cap cap;
> > > --
> > > 2.31.0
> >
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v10 25/41] virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
On Fri, Jun 24, 2022 at 2:59 PM Michael S. Tsirkin wrote:
>
> On Fri, Jun 24, 2022 at 10:56:05AM +0800, Xuan Zhuo wrote:
> > Add queue_notify_data in struct virtio_pci_common_cfg, which comes from
> > here https://github.com/oasis-tcs/virtio-spec/issues/89
> >
> > For not breaks uABI, add a new struct virtio_pci_common_cfg_notify.
>
> What exactly is meant by not breaking uABI?
> Users are supposed to be prepared for struct size to change ... no?
Not sure, any doc for this?
Thanks
>
>
> > Since I want to add queue_reset after queue_notify_data, I submitted
> > this patch first.
> >
> > Signed-off-by: Xuan Zhuo
> > Acked-by: Jason Wang
> > ---
> > include/uapi/linux/virtio_pci.h | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/include/uapi/linux/virtio_pci.h
> > b/include/uapi/linux/virtio_pci.h
> > index 3a86f36d7e3d..22bec9bd0dfc 100644
> > --- a/include/uapi/linux/virtio_pci.h
> > +++ b/include/uapi/linux/virtio_pci.h
> > @@ -166,6 +166,13 @@ struct virtio_pci_common_cfg {
> > __le32 queue_used_hi; /* read-write */
> > };
> >
> > +struct virtio_pci_common_cfg_notify {
> > + struct virtio_pci_common_cfg cfg;
> > +
> > + __le16 queue_notify_data; /* read-write */
> > + __le16 padding;
> > +};
> > +
> > /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> > struct virtio_pci_cfg_cap {
> > struct virtio_pci_cap cap;
> > --
> > 2.31.0
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v10 25/41] virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
On Fri, 24 Jun 2022 02:59:39 -0400, "Michael S. Tsirkin"
wrote:
> On Fri, Jun 24, 2022 at 10:56:05AM +0800, Xuan Zhuo wrote:
> > Add queue_notify_data in struct virtio_pci_common_cfg, which comes from
> > here https://github.com/oasis-tcs/virtio-spec/issues/89
> >
> > For not breaks uABI, add a new struct virtio_pci_common_cfg_notify.
>
> What exactly is meant by not breaking uABI?
> Users are supposed to be prepared for struct size to change ... no?
This was a previous discussion with Jason Wang, who was concerned about
affecting some existing programs.
https://lore.kernel.org/all/CACGkMEshTp8vSP9=pkj82y8+ddqfu9tfak1eghmzlvxue-o...@mail.gmail.com/
Thanks.
>
>
> > Since I want to add queue_reset after queue_notify_data, I submitted
> > this patch first.
> >
> > Signed-off-by: Xuan Zhuo
> > Acked-by: Jason Wang
> > ---
> > include/uapi/linux/virtio_pci.h | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/include/uapi/linux/virtio_pci.h
> > b/include/uapi/linux/virtio_pci.h
> > index 3a86f36d7e3d..22bec9bd0dfc 100644
> > --- a/include/uapi/linux/virtio_pci.h
> > +++ b/include/uapi/linux/virtio_pci.h
> > @@ -166,6 +166,13 @@ struct virtio_pci_common_cfg {
> > __le32 queue_used_hi; /* read-write */
> > };
> >
> > +struct virtio_pci_common_cfg_notify {
> > + struct virtio_pci_common_cfg cfg;
> > +
> > + __le16 queue_notify_data; /* read-write */
> > + __le16 padding;
> > +};
> > +
> > /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> > struct virtio_pci_cfg_cap {
> > struct virtio_pci_cap cap;
> > --
> > 2.31.0
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v10 25/41] virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
On Fri, Jun 24, 2022 at 10:56:05AM +0800, Xuan Zhuo wrote:
> Add queue_notify_data in struct virtio_pci_common_cfg, which comes from
> here https://github.com/oasis-tcs/virtio-spec/issues/89
>
> For not breaks uABI, add a new struct virtio_pci_common_cfg_notify.
What exactly is meant by not breaking uABI?
Users are supposed to be prepared for struct size to change ... no?
> Since I want to add queue_reset after queue_notify_data, I submitted
> this patch first.
>
> Signed-off-by: Xuan Zhuo
> Acked-by: Jason Wang
> ---
> include/uapi/linux/virtio_pci.h | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
> index 3a86f36d7e3d..22bec9bd0dfc 100644
> --- a/include/uapi/linux/virtio_pci.h
> +++ b/include/uapi/linux/virtio_pci.h
> @@ -166,6 +166,13 @@ struct virtio_pci_common_cfg {
> __le32 queue_used_hi; /* read-write */
> };
>
> +struct virtio_pci_common_cfg_notify {
> + struct virtio_pci_common_cfg cfg;
> +
> + __le16 queue_notify_data; /* read-write */
> + __le16 padding;
> +};
> +
> /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> struct virtio_pci_cfg_cap {
> struct virtio_pci_cap cap;
> --
> 2.31.0
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
