[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] common: guest_physmap_add_page()'s return value needs checking
- To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- Date: Wed, 1 Sep 2021 21:02:16 +0100
- 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=EmmyNtx9gJnqTuQ1T0C4F0cNpIZQvuM75YDFAWagy74=; b=aD2Sg7++GJQTZwTsM4iC9gsTOzwApxlxHZbZ1Yl5I0yq/11bGYSBr2M44A2QtAlAvX5AZLO5REcSK6PL+f/lC5CEGiiwuElc3i2elI2Bg0qXVkN/OrujecZJlxMSvksqxnenhyxeTDxNruDahkkXfKsWW9mTrpDKqCfkf+D80Z0DOVcjb96XaKylRnQ1Ko+hvyvRXeMzo0xebY0y6VBvZBlCLL0JxteU3nlL3CWienjDj8Mo8hhonQLUAQUUQgCrFq1KsXZjTq8ohKp7Ed+X2LjVtTRFZf6IK/D9wr7as3Lff2vWrI3Oo6tLydH5bWecjPPhuNpvLag7wZJd732mig==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=GjRT/FjPZS6CbnMueP2+d7fUFHy52bNwYgpVFML6OfLwlQkXDKtgJRNdq7KD9esG41lTSrySb87zFwgkPV1NhBvnKMssh4cZtBAgvUJnl1/DKyjOq+w/p/ubwU6tjVleRuWROxpaxYaxWwfvGsNliWhFfYS8vhEYyjEDcFDZ59ycasUEw3lVeuZ3bTtGSqCsWR+Upm1dGz62uukCwxSs9tnYphrkzRENyS0ufqzY+L3aRde0vIpkdzE7kSD08kMShi0zxoT11qIPWEoqWKHS3todBuJddZDtbztDezCsvFyMoubRy8n3u5RKxMz9NVU+HBwtX7DOuH4rGayZDhP01g==
- Authentication-results: esa4.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
- Cc: George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Delivery-date: Wed, 01 Sep 2021 20:02:47 +0000
- Ironport-hdrordr: A9a23:BTEvT6Ah5Vrc5grlHegisceALOsnbusQ8zAXPh9KJiC9I/b1qy nxppkmPH/P6Qr4WBkb6LW90dq7MAzhHPlOkPUs1NaZLXTbUQ6TQr2KgrGSuwEIdxeOkdK1kJ 0QCZSWa+eAfWSS7/yKmTVQeuxIqLLskNHK9JXjJjVWPGVXgslbnnZE422gYytLrWd9dPgE/d anl7F6T23KQwVoUi33PAhLY8Hz4/nw0L72ax8PABAqrCGIkDOT8bb/VzyVxA0XXT9jyaortT GtqX252oyT99WAjjPM3W7a6Jpb3PPn19t4HcSJzuwYMC/lhAqEbJloH5eCoDc2iuey70tCqq iDnz4Qe+BIr1/BdGC8phXgnyHmzTYV8nfnjWSVhHPyyPaJDg4SOo5kv8Z0YxHZ400vsJVXy6 RQxV+UsJJREFfpgDn9z8KgbWApqmOE5V4Z1cIDhX1WVoUTLJVLq5YEwU9TGJAcWArn9YEcFv V0Bs203ocUTbqjVQGcgoBT+q3qYpxqdS32BHTq+/blkwS+pUoJinfxn6ck7y49HPtXceg22w zGWp4Y342mAPVmNZ6UqY86ML2K41f2MGbx2VSpUBza/ZE8SgfwQqHMkcIIDcGRCdE1JcgJ6d j8uG0xjx96R6upM7zU4KF2
- Ironport-sdr: YrbDmm78MqY8HkOXyNnxTm+XUC472m1zU8p0jm4KvCFkX1zPUjZeThCktDYOs/C+ml7jLgQR1A 7yQpHbEhU5Yp9kF7fRJ87eQT3Xph+ClSDmH4Tz9kiZLf3eG4mSzIoOoWgc4w2YYyzpMJ3VPIkw bHki+XWuna1xmI6SkNUINki52x1lTnlHoCO/OS2xl1AVh0PSLdPXp9fOb3CYwhUPlGJUtil/wL Q2RrilOEq4EwW20IP7FwxC5tB0fVCBV60mGGhP4B4/NzrCX9CViUp5QjOeYG4zR5FFVkgE7cRc FxoG8OhLoShTmGIwmvPRQ3wM
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 01/09/2021 17:06, Jan Beulich wrote:
> The function may fail; it is not correct to indicate "success" in this
> case up the call stack. Mark the function must-check to prove all
> cases have been caught (and no new ones will get introduced).
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> In the grant-transfer case it is not really clear to me whether we can
> stick to setting GTF_transfer_completed in the error case. Since a guest
> may spin-wait for the flag to become set, simply not setting the flag is
> not an option either. I was wondering whether we may want to slightly
> alter (extend) the ABI and allow for a GTF_transfer_committed ->
> GTF_transfer_completed transition (i.e. clearing GTF_transfer_committed
> at the same time as setting GTF_transfer_completed).
Considering there are no production users of gnttab_transfer(), we can
do what we want. It was introduced for (IIRC) netlink2 and never got
into production, and then we clobbered it almost entirely in an XSA
several years ago by restricting steal_page() to PV guests only.
As a consequence, we can do anything which seems sensible, and does not
necessarily need to be bound by a guest spinning on the bit.
The concept of gnttab_transfer() alone is crazy from an in-guest memory
management point of view. We could alternatively save our future selves
more trouble by just Kconfig'ing it out now, deleting it in several
releases time, and fogetting about the problem as nothing will break in
practice.
~Andrew
|
