| Message ID | 20220629065656.54420-26-xuanzhuo@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | virtio pci support VIRTIO_F_RING_RESET | expand |
On Wed, Jun 29, 2022 at 02:56:41PM +0800, Xuan Zhuo wrote: > This patch allows the new introduced > __virtqueue_break()/__virtqueue_unbreak() to break/unbreak the > virtqueue. > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> I wonder how this interacts with the hardening patches. Jason? > --- > drivers/virtio/virtio_ring.c | 24 ++++++++++++++++++++++++ > include/linux/virtio.h | 3 +++ > 2 files changed, 27 insertions(+) > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index 5ec43607cc15..7b02be7fce67 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -2744,6 +2744,30 @@ unsigned int virtqueue_get_vring_size(struct virtqueue *_vq) > } > EXPORT_SYMBOL_GPL(virtqueue_get_vring_size); > > +/* > + * This function should only be called by the core, not directly by the driver. > + */ > +void __virtqueue_break(struct virtqueue *_vq) > +{ > + struct vring_virtqueue *vq = to_vvq(_vq); > + > + /* Pairs with READ_ONCE() in virtqueue_is_broken(). */ > + WRITE_ONCE(vq->broken, true); > +} > +EXPORT_SYMBOL_GPL(__virtqueue_break); > + > +/* > + * This function should only be called by the core, not directly by the driver. > + */ > +void __virtqueue_unbreak(struct virtqueue *_vq) > +{ > + struct vring_virtqueue *vq = to_vvq(_vq); > + > + /* Pairs with READ_ONCE() in virtqueue_is_broken(). */ > + WRITE_ONCE(vq->broken, false); > +} I don't think these "Pairs" comments have any value. > +EXPORT_SYMBOL_GPL(__virtqueue_unbreak); > + > bool virtqueue_is_broken(struct virtqueue *_vq) > { > struct vring_virtqueue *vq = to_vvq(_vq); > diff --git a/include/linux/virtio.h b/include/linux/virtio.h > index 1272566adec6..dc474a0d48d1 100644 > --- a/include/linux/virtio.h > +++ b/include/linux/virtio.h > @@ -138,6 +138,9 @@ bool is_virtio_device(struct device *dev); > void virtio_break_device(struct virtio_device *dev); > void __virtio_unbreak_device(struct virtio_device *dev); > > +void __virtqueue_break(struct virtqueue *_vq); > +void __virtqueue_unbreak(struct virtqueue *_vq); > + > void virtio_config_changed(struct virtio_device *dev); > #ifdef CONFIG_PM_SLEEP > int virtio_device_freeze(struct virtio_device *dev); > -- > 2.31.0
在 2022/7/1 14:31, Michael S. Tsirkin 写道: > On Wed, Jun 29, 2022 at 02:56:41PM +0800, Xuan Zhuo wrote: >> This patch allows the new introduced >> __virtqueue_break()/__virtqueue_unbreak() to break/unbreak the >> virtqueue. >> >> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > I wonder how this interacts with the hardening patches. > Jason? Consider we've marked it as broken, I think we don't need to care about the hardening in this series. Just make it work without hardening. And I will handle vq reset when rework the IRQ hardening. Thanks > >> --- >> drivers/virtio/virtio_ring.c | 24 ++++++++++++++++++++++++ >> include/linux/virtio.h | 3 +++ >> 2 files changed, 27 insertions(+) >> >> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c >> index 5ec43607cc15..7b02be7fce67 100644 >> --- a/drivers/virtio/virtio_ring.c >> +++ b/drivers/virtio/virtio_ring.c >> @@ -2744,6 +2744,30 @@ unsigned int virtqueue_get_vring_size(struct virtqueue *_vq) >> } >> EXPORT_SYMBOL_GPL(virtqueue_get_vring_size); >> >> +/* >> + * This function should only be called by the core, not directly by the driver. >> + */ >> +void __virtqueue_break(struct virtqueue *_vq) >> +{ >> + struct vring_virtqueue *vq = to_vvq(_vq); >> + >> + /* Pairs with READ_ONCE() in virtqueue_is_broken(). */ >> + WRITE_ONCE(vq->broken, true); >> +} >> +EXPORT_SYMBOL_GPL(__virtqueue_break); >> + >> +/* >> + * This function should only be called by the core, not directly by the driver. >> + */ >> +void __virtqueue_unbreak(struct virtqueue *_vq) >> +{ >> + struct vring_virtqueue *vq = to_vvq(_vq); >> + >> + /* Pairs with READ_ONCE() in virtqueue_is_broken(). */ >> + WRITE_ONCE(vq->broken, false); >> +} > I don't think these "Pairs" comments have any value. > > >> +EXPORT_SYMBOL_GPL(__virtqueue_unbreak); >> + >> bool virtqueue_is_broken(struct virtqueue *_vq) >> { >> struct vring_virtqueue *vq = to_vvq(_vq); >> diff --git a/include/linux/virtio.h b/include/linux/virtio.h >> index 1272566adec6..dc474a0d48d1 100644 >> --- a/include/linux/virtio.h >> +++ b/include/linux/virtio.h >> @@ -138,6 +138,9 @@ bool is_virtio_device(struct device *dev); >> void virtio_break_device(struct virtio_device *dev); >> void __virtio_unbreak_device(struct virtio_device *dev); >> >> +void __virtqueue_break(struct virtqueue *_vq); >> +void __virtqueue_unbreak(struct virtqueue *_vq); >> + >> void virtio_config_changed(struct virtio_device *dev); >> #ifdef CONFIG_PM_SLEEP >> int virtio_device_freeze(struct virtio_device *dev); >> -- >> 2.31.0
在 2022/7/1 17:36, Jason Wang 写道: > > 在 2022/7/1 14:31, Michael S. Tsirkin 写道: >> On Wed, Jun 29, 2022 at 02:56:41PM +0800, Xuan Zhuo wrote: >>> This patch allows the new introduced >>> __virtqueue_break()/__virtqueue_unbreak() to break/unbreak the >>> virtqueue. >>> >>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> >> I wonder how this interacts with the hardening patches. >> Jason? > > > Consider we've marked it as broken, I think we don't need to care > about the hardening in this series. Just make it work without hardening. > > And I will handle vq reset when rework the IRQ hardening. > > Thanks Rethink of this, I think Xuan's code should be fine. We know we will have another rework. Thanks > > >> >>> --- >>> drivers/virtio/virtio_ring.c | 24 ++++++++++++++++++++++++ >>> include/linux/virtio.h | 3 +++ >>> 2 files changed, 27 insertions(+) >>> >>> diff --git a/drivers/virtio/virtio_ring.c >>> b/drivers/virtio/virtio_ring.c >>> index 5ec43607cc15..7b02be7fce67 100644 >>> --- a/drivers/virtio/virtio_ring.c >>> +++ b/drivers/virtio/virtio_ring.c >>> @@ -2744,6 +2744,30 @@ unsigned int virtqueue_get_vring_size(struct >>> virtqueue *_vq) >>> } >>> EXPORT_SYMBOL_GPL(virtqueue_get_vring_size); >>> +/* >>> + * This function should only be called by the core, not directly by >>> the driver. >>> + */ >>> +void __virtqueue_break(struct virtqueue *_vq) >>> +{ >>> + struct vring_virtqueue *vq = to_vvq(_vq); >>> + >>> + /* Pairs with READ_ONCE() in virtqueue_is_broken(). */ >>> + WRITE_ONCE(vq->broken, true); >>> +} >>> +EXPORT_SYMBOL_GPL(__virtqueue_break); >>> + >>> +/* >>> + * This function should only be called by the core, not directly by >>> the driver. >>> + */ >>> +void __virtqueue_unbreak(struct virtqueue *_vq) >>> +{ >>> + struct vring_virtqueue *vq = to_vvq(_vq); >>> + >>> + /* Pairs with READ_ONCE() in virtqueue_is_broken(). */ >>> + WRITE_ONCE(vq->broken, false); >>> +} >> I don't think these "Pairs" comments have any value. >> >> >>> +EXPORT_SYMBOL_GPL(__virtqueue_unbreak); >>> + >>> bool virtqueue_is_broken(struct virtqueue *_vq) >>> { >>> struct vring_virtqueue *vq = to_vvq(_vq); >>> diff --git a/include/linux/virtio.h b/include/linux/virtio.h >>> index 1272566adec6..dc474a0d48d1 100644 >>> --- a/include/linux/virtio.h >>> +++ b/include/linux/virtio.h >>> @@ -138,6 +138,9 @@ bool is_virtio_device(struct device *dev); >>> void virtio_break_device(struct virtio_device *dev); >>> void __virtio_unbreak_device(struct virtio_device *dev); >>> +void __virtqueue_break(struct virtqueue *_vq); >>> +void __virtqueue_unbreak(struct virtqueue *_vq); >>> + >>> void virtio_config_changed(struct virtio_device *dev); >>> #ifdef CONFIG_PM_SLEEP >>> int virtio_device_freeze(struct virtio_device *dev); >>> -- >>> 2.31.0
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 5ec43607cc15..7b02be7fce67 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -2744,6 +2744,30 @@ unsigned int virtqueue_get_vring_size(struct virtqueue *_vq) } EXPORT_SYMBOL_GPL(virtqueue_get_vring_size); +/* + * This function should only be called by the core, not directly by the driver. + */ +void __virtqueue_break(struct virtqueue *_vq) +{ + struct vring_virtqueue *vq = to_vvq(_vq); + + /* Pairs with READ_ONCE() in virtqueue_is_broken(). */ + WRITE_ONCE(vq->broken, true); +} +EXPORT_SYMBOL_GPL(__virtqueue_break); + +/* + * This function should only be called by the core, not directly by the driver. + */ +void __virtqueue_unbreak(struct virtqueue *_vq) +{ + struct vring_virtqueue *vq = to_vvq(_vq); + + /* Pairs with READ_ONCE() in virtqueue_is_broken(). */ + WRITE_ONCE(vq->broken, false); +} +EXPORT_SYMBOL_GPL(__virtqueue_unbreak); + bool virtqueue_is_broken(struct virtqueue *_vq) { struct vring_virtqueue *vq = to_vvq(_vq); diff --git a/include/linux/virtio.h b/include/linux/virtio.h index 1272566adec6..dc474a0d48d1 100644 --- a/include/linux/virtio.h +++ b/include/linux/virtio.h @@ -138,6 +138,9 @@ bool is_virtio_device(struct device *dev); void virtio_break_device(struct virtio_device *dev); void __virtio_unbreak_device(struct virtio_device *dev); +void __virtqueue_break(struct virtqueue *_vq); +void __virtqueue_unbreak(struct virtqueue *_vq); + void virtio_config_changed(struct virtio_device *dev); #ifdef CONFIG_PM_SLEEP int virtio_device_freeze(struct virtio_device *dev);
This patch allows the new introduced __virtqueue_break()/__virtqueue_unbreak() to break/unbreak the virtqueue. Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> --- drivers/virtio/virtio_ring.c | 24 ++++++++++++++++++++++++ include/linux/virtio.h | 3 +++ 2 files changed, 27 insertions(+)