Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <439f929f-9d15-c33c-b40d-88dd06cebd85@linux.ibm.com>
Date:   Tue, 8 Mar 2022 10:39:09 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
Reply-To: jjherne@linux.ibm.com
Subject: Re: [PATCH v18 08/18] s390/vfio-ap: allow assignment of unavailable
 AP queues to mdev device
Content-Language: en-US
To:     Tony Krowiak <akrowiak@linux.ibm.com>, linux-s390@vger.kernel.org,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Cc:     freude@linux.ibm.com, borntraeger@de.ibm.com, cohuck@redhat.com,
        mjrosato@linux.ibm.com, pasic@linux.ibm.com,
        alex.williamson@redhat.com, kwankhede@nvidia.com,
        fiuczy@linux.ibm.com
References: <20220215005040.52697-1-akrowiak@linux.ibm.com>
 <20220215005040.52697-9-akrowiak@linux.ibm.com>
 <97681738-50a1-976d-9f0f-be326eab7202@linux.ibm.com>
 <9ac3908e-06da-6276-d1df-94898918fc5b@linux.ibm.com>
From:   "Jason J. Herne" <jjherne@linux.ibm.com>
Organization: IBM
In-Reply-To: <9ac3908e-06da-6276-d1df-94898918fc5b@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 3/7/22 07:31, Tony Krowiak wrote:
>>> +         * If the input apm and aqm belong to the matrix_mdev's matrix,
>>> +         * then move on to the next.
>>> +         */
>>> +        if (mdev_apm == matrix_mdev->matrix.apm &&
>>> +            mdev_aqm == matrix_mdev->matrix.aqm)
>>>               continue;
>>
>> We may have a problem here. This check seems like it exists to stop you from
>> comparing an mdev's apm/aqm with itself. Obviously comparing an mdev's newly
>> updated apm/aqm with itself would cause a false positive sharing check, right?
>> If this is the case, I think the comment should be changed to reflect that.
> 
> You are correct, this check is performed to prevent comparing an mdev to
> itself, I'll improve the comment.
> 
>>
>> Aside from the comment, what stops this particular series of if statements from
>> allowing us to configure a second mdev with the exact same apm/aqm values as an
>> existing mdev? If we do, then this check's continue will short circuit the rest
>> of the function thereby allowing that 2nd mdev even though it should be a
>> sharing violation.
> 
> I don't see how this is possible.

You are correct. I missed the fact that you are comparing pointers here, and not
values. Apologies. Now that I understand the code, I agree that this is fine as is.


-- 
-- Jason J. Herne (jjherne@linux.ibm.com)