linux-kernel - Re: [PATCH V4 6/9] virtio-ccw: 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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 11 May 2022 11:13:06 +0200
From:   Cornelia Huck <cohuck@...hat.com>
To:     Jason Wang <jasowang@...hat.com>
Cc:     "Michael S. Tsirkin" <mst@...hat.com>,
        virtualization <virtualization@...ts.linux-foundation.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Marc Zyngier <maz@...nel.org>,
        Halil Pasic <pasic@...ux.ibm.com>,
        eperezma <eperezma@...hat.com>, Cindy Lu <lulu@...hat.com>,
        Stefano Garzarella <sgarzare@...hat.com>,
        Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
        Vineeth Vijayan <vneethv@...ux.ibm.com>,
        Peter Oberparleiter <oberpar@...ux.ibm.com>
Subject: Re: [PATCH V4 6/9] virtio-ccw: implement synchronize_cbs()

On Wed, May 11 2022, Jason Wang <jasowang@...hat.com> wrote:

> On Wed, May 11, 2022 at 4:17 PM Cornelia Huck <cohuck@...hat.com> wrote:
>>
>> On Wed, May 11 2022, Jason Wang <jasowang@...hat.com> wrote:
>>
>> > On Tue, May 10, 2022 at 7:28 PM Michael S. Tsirkin <mst@...hat.com> wrote:
>> >>
>> >> On Sat, May 07, 2022 at 03:19:51PM +0800, Jason Wang wrote:
>> >> > @@ -1106,6 +1130,7 @@ static void virtio_ccw_int_handler(struct ccw_device *cdev,
>> >> >                       vcdev->err = -EIO;
>> >> >       }
>> >> >       virtio_ccw_check_activity(vcdev, activity);
>> >> > +     read_lock_irqsave(&vcdev->irq_lock, flags);
>> >> >       for_each_set_bit(i, indicators(vcdev),
>> >> >                        sizeof(*indicators(vcdev)) * BITS_PER_BYTE) {
>> >> >               /* The bit clear must happen before the vring kick. */
>> >>
>> >> Cornelia sent a lockdep trace on this.
>> >>
>> >> Basically I think this gets the irqsave/restore logic wrong.
>> >> It attempts to disable irqs in the handler (which is an interrupt
>> >> anyway).
>> >
>> > The reason I use irqsave/restore is that it can be called from process
>> > context (if I was not wrong), e.g from io_subchannel_quiesce().
>>
>> io_subchannel_quiesce() should disable interrupts, though? Otherwise, it
>> would be a bug.
>
> Right, it was protected by a spin_lock_irq(), but I can see other
> cdev->handler() in e.g device_fsm.c, the irq status is not obvious, do
> they have the same assumption which IRQ is disabled?

Yes, that should be the case for any invocations via the fsm as well.

It's been some time since I've worked on that part of the code, though,
so let's cc: the s390 cio maintainers so that they can speak up if I'm
wrong.