linux-kernel - Re: [PATCH V2 4/5] virtio-pci: implement synchronize

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 7 Apr 2022 14:38:34 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>,
        Cornelia Huck <cohuck@...hat.com>
Cc:     "Paul E. McKenney" <paulmck@...nel.org>, peterz@...radead.org,
        maz@...nel.org, linux-kernel@...r.kernel.org,
        virtualization@...ts.linux-foundation.org, tglx@...utronix.de,
        Halil Pasic <pasic@...ux.ibm.com>
Subject: Re: [PATCH V2 4/5] virtio-pci: implement synchronize_vqs()


在 2022/4/6 下午11:31, Michael S. Tsirkin 写道:
> On Wed, Apr 06, 2022 at 03:04:32PM +0200, Cornelia Huck wrote:
>> On Wed, Apr 06 2022, "Michael S. Tsirkin" <mst@...hat.com> wrote:
>>
>>> On Wed, Apr 06, 2022 at 04:35:37PM +0800, Jason Wang wrote:
>>>> This patch implements PCI version of synchronize_vqs().
>>>>
>>>> Cc: Thomas Gleixner <tglx@...utronix.de>
>>>> Cc: Peter Zijlstra <peterz@...radead.org>
>>>> Cc: "Paul E. McKenney" <paulmck@...nel.org>
>>>> Cc: Marc Zyngier <maz@...nel.org>
>>>> Signed-off-by: Jason Wang <jasowang@...hat.com>
>>> Please add implementations at least for ccw and mmio.
>> I'm not sure what (if anything) can/should be done for ccw...
>>
>>>> ---
>>>>   drivers/virtio/virtio_pci_common.c | 14 ++++++++++++++
>>>>   drivers/virtio/virtio_pci_common.h |  2 ++
>>>>   drivers/virtio/virtio_pci_legacy.c |  1 +
>>>>   drivers/virtio/virtio_pci_modern.c |  2 ++
>>>>   4 files changed, 19 insertions(+)
>>>>
>>>> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
>>>> index d724f676608b..b78c8bc93a97 100644
>>>> --- a/drivers/virtio/virtio_pci_common.c
>>>> +++ b/drivers/virtio/virtio_pci_common.c
>>>> @@ -37,6 +37,20 @@ void vp_synchronize_vectors(struct virtio_device *vdev)
>>>>   		synchronize_irq(pci_irq_vector(vp_dev->pci_dev, i));
>>>>   }
>>>>   
>>>> +void vp_synchronize_vqs(struct virtio_device *vdev)
>>>> +{
>>>> +	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
>>>> +	int i;
>>>> +
>>>> +	if (vp_dev->intx_enabled) {
>>>> +		synchronize_irq(vp_dev->pci_dev->irq);
>>>> +		return;
>>>> +	}
>>>> +
>>>> +	for (i = 0; i < vp_dev->msix_vectors; ++i)
>>>> +		synchronize_irq(pci_irq_vector(vp_dev->pci_dev, i));
>>>> +}
>>>> +
>> ...given that this seems to synchronize threaded interrupt handlers?
> No, any handlers at all. The point is to make sure any memory changes
> made prior to this op are visible to callbacks.
>
> Jason, maybe add that to the documentation?


Sure.


>
>> Halil, do you think ccw needs to do anything? (AFAICS, we only have one
>> 'irq' for channel devices anyway, and the handler just calls the
>> relevant callbacks directly.)
> Then you need to synchronize with that.


Have a quick glance at the codes, it looks to me we can synchronize with 
the IO_INTERRUPT. (Assuming all callbacks are triggered via 
ccw_device_irq()).

Thanks


>
>>>>   /* the notify function used when creating a virt queue */
>>>>   bool vp_notify(struct virtqueue *vq)
>>>>   {