[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 7/7] xen/memory: Fix mapping grant tables with XENMEM_acquire_resource
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- Date: Fri, 29 Jan 2021 18:18:04 +0000
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=AOQqzVNBjI5KMWusL5Z9o0Gp7YdRFB4swm2EAp3FDhc=; b=PhNROhD1HHj7lQ3LBJUMFNTODp8wccc+5CNenvuAhWixoZ1B5eMITCvnnwBB0uhg7QDladOkFQ1MDPJIbypY85Gx0X//BRrejhpe4JuFxm7UwhaTz47e2i/5Rr7C3uWDoFbNq+y4IDYQm2/5xpccQOsPIY7MKYNgpXjqkp3c/jOPyv/A1MpBW6XoYnozPq8VgXCOOKHrWsMkl0ZG45eQ4OOEzIGDs5d4w4hu0M+nlIXMYRaaZvax95fhxerrEIM+wZA1d7/yEtGcBf4AnIxMJ28felUnq9aNoXqvxSMQxcq3/xG66thlb4d8Qc10+03p67eMoHCjFhcB+u+dc6GDfQ==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=mrGhoWcpeeKfEMDj6sN5rRjAjMVZHvtGCyyy02YgrbF5sWzO9pRGSpjCDRHnvcNq53gvgiNaHirp1NB1fLUxmn0b1A6LXG2GFnGwkQGH4r2ckKRE6syPsarXBDT8aIb027Mnn4xzTdk1h//yUkyk/ACks0eEhaamSUEILQ2HurHgvnPyeRpzz+Txiqwy/emIg+60NUVeytQjMwaf6S5XXdUk4L37PTYRFrG2aZXGWAKOCAIPTCucpdTr49191m6THZM5LDchsw0o7GpgEM0BwvvZSgznbKCv6BGZrhAKvnT6gHI1OtAL8ukNXt088WAEM+HNxhSki9BZZ7heF/FSSw==
- Authentication-results: esa5.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
- Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Julien Grall <julien@xxxxxxx>, Paul Durrant <paul@xxxxxxx>, Michał Leszczyński <michal.leszczynski@xxxxxxx>, "Hubert Jasudowicz" <hubert.jasudowicz@xxxxxxx>, Tamas K Lengyel <tamas@xxxxxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Fri, 29 Jan 2021 18:19:25 +0000
- Ironport-sdr: d5ixRG3lYDwjtk3xDwaI++5ns/Ax0XwRSkZ6lwhnel1w7MEVTdBBw+PVRLrFQlJSj4KZEk+xkG 4SO18esMifCvahAtY8lp+sioVnE7YjwpODBYKLjYmQCh9dqUK7nVcXrDdpKgjQ0OElMH1zsShp 7CLAMTMpf4SiEqcvHJXmNy75b19kwFlV7TmVyfEKUqO+3QKw3cYhAYFT8FufWMfp4NKMbfVNd+ V6ayBpzFqz843IcZyBg6MeyuVjASWw0mpMu60l+r04RWQd986XyOPqDB6Y5Oa0CDoQON6HJnSn as0=
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 29/01/2021 09:46, Jan Beulich wrote:
> On 29.01.2021 00:44, Andrew Cooper wrote:
>> On 15/01/2021 16:12, Jan Beulich wrote:
>>> On 12.01.2021 20:48, Andrew Cooper wrote:
>>>> --- a/xen/arch/x86/mm.c
>>>> +++ b/xen/arch/x86/mm.c
>>>> @@ -4628,7 +4628,6 @@ int arch_acquire_resource(struct domain *d, unsigned
>>>> int type,
>>>> if ( id != (unsigned int)ioservid )
>>>> break;
>>>>
>>>> - rc = 0;
>>>> for ( i = 0; i < nr_frames; i++ )
>>>> {
>>>> mfn_t mfn;
>>> How "good" are our chances that older gcc won't recognize that
>>> without this initialization ...
>>>
>>>> @@ -4639,6 +4638,9 @@ int arch_acquire_resource(struct domain *d, unsigned
>>>> int type,
>>>>
>>>> mfn_list[i] = mfn_x(mfn);
>>>> }
>>>> + if ( i == nr_frames )
>>>> + /* Success. Passed nr_frames back to the caller. */
>>>> + rc = nr_frames;
>>> ... rc will nevertheless not remain uninitialized when nr_frames
>>> is zero?
>> I don't anticipate this function getting complicated enough for us to
>> need to rely on tricks like that to spot bugs.
>>
>> AFAICT, it would take a rather larger diffstat to make it "uninitialised
>> clean" again.
> Well, we'll see how it goes then. We still allow rather ancient
> gcc, and I will eventually run into a build issue here once
> having rebased accordingly, if such old gcc won't cope.
But those problematic compilers have problems spotting that a variable
is actually initialised on all paths.
This case is opposite. It is unconditionally initialised to -EINVAL
(just outside of the context above), which breaks some "the compiler
would warn us about error paths" logic that we try to use.
~Andrew
|
