[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/4] x86/altp2m: p2m_altp2m_get_or_propagate() should honor present page order


  • To: Andrew Cooper <amc96@xxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 2 Dec 2021 10:55:18 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=qS13c7112W8LQjv6N90bO86hzLmviVvmGeT7WYs0FmI=; b=MOEx4HIFd/TQOnKrfn7uGfwt0NUQaCjGJa4azVD3NF6TggiEBfE1lFTYNZzeCGfFGLbpP6xQvI7J6h/Q3pkq0t30/ONOKJC2pcmb62aKHAOeRwkDOPtckyyLtXbhj791tGv4RXfyVhJehKGDxHk+upjBSzGyoQHGBzg4wnws6QrQMw/o9fla6fxwI6aSCFsch5xCJOnQAOv9D4M8xdAWVrDE+9tPTS9BY/yvErpWRClrRcG23YurBboKclG8V7oymtZzGr3e7VSeZivhkBcmk99i4kE6QItfHErKQSdToUe5g5MaIIxMbSY0d3WyDXkNNgLwuuhnnSZppcV4TpWT5Q==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hTsCB0WPvki4EZhK564qstSSRFzsuvm4cgTHkHVzG1a1ole88iCPHR1O8rzSt71QKbO55Cy1RuqnUkKzBymkpBfmrH2qEjEynTHpPvnH4VIdRlwmLuh3UKEAqHgK6GQ1++WogOTseCWxogBS/fGT71DA88hLkGUVbClPm5KQvsTFQubc50MiiX0+DQ6jB9RZlGggzHteg4y1U/PWFq9qFtUx5L/jJq5CmfVbhok4PKMAx+Vaa+gruwtHSKRvyd8EjUYwHDLIdoeYnBMYMQKMdn1rWcxwoZxUBoF8Xv2jveyw5oZL09HO80hfMZ3sVLkmz9bkvl+Twdx/6c2YgG5lnw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Tamas K Lengyel <tamas@xxxxxxxxxxxxx>, Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>, Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 02 Dec 2021 09:55:42 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 01.12.2021 13:44, Andrew Cooper wrote:
> On 01/12/2021 10:54, Jan Beulich wrote:
>> @@ -2237,11 +2243,11 @@ bool p2m_altp2m_get_or_propagate(struct
>>       * to the start of the superpage.  NB that we repupose `amfn`
>>       * here.
>>       */
>> -    mask = ~((1UL << page_order) - 1);
>> +    mask = ~((1UL << cur_order) - 1);
>>      amfn = _mfn(mfn_x(*mfn) & mask);
>>      gfn = _gfn(gfn_l & mask);
>>  
>> -    rc = p2m_set_entry(ap2m, gfn, amfn, page_order, *p2mt, *p2ma);
>> +    rc = p2m_set_entry(ap2m, gfn, amfn, cur_order, *p2mt, *p2ma);
>>      p2m_unlock(ap2m);
> 
> While I agree with the problem you've identified, this function has some
> very broken return semantics.
> 
> Logically, it is taking some hostp2m properties for gfn, and replacing
> them with ap2m properties for the same gfn.
> 
> 
> It cannot be correct to only update the caller state on the error
> paths.  At a minimum, the
> 
>     if ( paged )
>         p2m_mem_paging_populate(currd, _gfn(gfn));
> 
> path in the success case is wrong when we've adjusted gfn down.

I wonder which of the exit paths you consider to be "error" ones. The
first one returning "false" pretty clearly isn't, for example. And the
path returning "true" is after a p2m_set_entry(), which means (if that
one succeeded) that caller values are now in sync with the P2M and
hence doen't need updating afaics.

And anyway - how does what you say relate to the patch at hand? I don't
think you mean to request that I fix further problems elsewhere, right
here?

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.