Message-ID: <25a5ee1a-b00f-bfcb-2273-8b5aa3927dcb@linux.ibm.com>
Date:   Tue, 11 Jan 2022 16:27:37 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.0
Subject: Re: [PATCH v17 08/15] s390/vfio-ap: keep track of active guests
Content-Language: en-US
To:     Halil Pasic <pasic@linux.ibm.com>
Cc:     linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
        kvm@vger.kernel.org, jjherne@linux.ibm.com, freude@linux.ibm.com,
        borntraeger@de.ibm.com, cohuck@redhat.com, mjrosato@linux.ibm.com,
        alex.williamson@redhat.com, kwankhede@nvidia.com,
        fiuczy@linux.ibm.com
References: <20211021152332.70455-1-akrowiak@linux.ibm.com>
 <20211021152332.70455-9-akrowiak@linux.ibm.com>
 <20211230030419.2f3e5bda.pasic@linux.ibm.com>
From:   Tony Krowiak <akrowiak@linux.ibm.com>
In-Reply-To: <20211230030419.2f3e5bda.pasic@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 12/29/21 21:04, Halil Pasic wrote:
> On Thu, 21 Oct 2021 11:23:25 -0400
> Tony Krowiak <akrowiak@linux.ibm.com> wrote:
>
>> The reason a lockdep splat can occur has to do with the fact that the
>> kvm->lock has to be taken before the vcpu->lock; so, for example, when a
>> secure execution guest is started, you may end up with the following
>> scenario:
>>
>>          Interception of PQAP(AQIC) instruction executed on the guest:
>>          ------------------------------------------------------------
>>          handle_pqap:                    matrix_dev->lock
>>          kvm_vcpu_ioctl:                 vcpu_mutex
>>
>>          Start of secure execution guest:
>>          -------------------------------
>>          kvm_s390_cpus_to_pv:            vcpu->mutex
>>          kvm_arch_vm_ioctl:              kvm->lock
>>
>>          Queue is unbound from vfio_ap device driver:
>>          -------------------------------------------
>>                                          kvm->lock
>>          vfio_ap_mdev_remove_queue:      matrix_dev->lock
> The way you describe your scenario is a little ambiguous. It
> seems you choose a stack-trace like description, in a sense that for
> example for PQAP: first vcpu->mutex is taken and then matrix_dev->lock
> but you write the latter first and the former second. I think it is more
> usual to describe such stuff a a sequence of event in a sense that
> if A precedes B in the text (from the top towards the bottom), then
> execution of a A precedes the execution of B in time.

I wrote it the way it is displayed in the lockdep splat trace.
I'd be happy to re-arrange it if you'd prefer.

>
> Also you are inconsistent with vcpu_mutex vs vcpu->mutex.
>
> I can't say I understand the need for this yet. I have been starring
> at the end result for a while. Let me see if I can come up with an
> alternate proposal for some things.

Go for it, and may the force be with you.

>
> Regards,
> Halil
>
>