Re: [PATCH 3/4] xen/arm, libxl: Revert XEN_DOMCTL_shadow_op; use p2m mempool hypercalls

From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <Andrew.Cooper3@citrix.com>
Cc: Xen-devel <xen-devel@lists.xenproject.org>,
	Xen Security Team <security@xen.org>,
	Roger Pau Monne <roger.pau@citrix.com>, Wei Liu <wl@xen.org>,
	Julien Grall <julien@xen.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Henry Wang <Henry.Wang@arm.com>,
	Anthony Perard <anthony.perard@citrix.com>,
	Stefano Stabellini <sstabellini@kernel.org>
Subject: Re: [PATCH 3/4] xen/arm, libxl: Revert XEN_DOMCTL_shadow_op; use p2m mempool hypercalls
Date: Thu, 17 Nov 2022 09:18:25 +0100	[thread overview]
Message-ID: <0e2e79fc-efe0-501b-e80e-c98748c9babe@suse.com> (raw)
In-Reply-To: <269f4e70-07c2-5cf6-f42e-6f98e2ce70dd@citrix.com>

On 17.11.2022 00:41, Andrew Cooper wrote:
> On 16/11/2022 08:30, Jan Beulich wrote:
>> On 16.11.2022 03:00, Stefano Stabellini wrote:
>>> On Wed, 16 Nov 2022, Andrew Cooper wrote:
>>>> On 16/11/2022 01:37, Stefano Stabellini wrote:
>>>>> On Wed, 26 Oct 2022, Andrew Cooper wrote:
>>>>>> This reverts most of commit cf2a68d2ffbc3ce95e01449d46180bddb10d24a0, and bits
>>>>>> of cbea5a1149ca7fd4b7cdbfa3ec2e4f109b601ff7.
>>>>>>
>>>>>> First of all, with ARM borrowing x86's implementation, the logic to set the
>>>>>> pool size should have been common, not duplicated.  Introduce
>>>>>> libxl__domain_set_p2m_pool_size() as a shared implementation, and use it from
>>>>>> the ARM and x86 paths.  It is left as an exercise to the reader to judge how
>>>>>> libxl/xl can reasonably function without the ability to query the pool size...
>>>>>>
>>>>>> Remove ARM's p2m_domctl() infrastructure now the functioanlity has been
>>>>>> replaced with a working and unit tested interface.
>>>>>>
>>>>>> This is part of XSA-409 / CVE-2022-33747.
>>>>> Genuine question: I can see this patch removes the implementation of
>>>>> XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION on ARM. It also switches libxl (both
>>>>> ARM and x86) to the new hypercall.
>>>>>
>>>>> Why keep the old hypercall (XEN_DOMCTL_shadow_op and
>>>>> XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION) implementation on x86 (not on ARM)?
>>>>>
>>>>> Is that because it was only recently implemented? And not actually
>>>>> present in any past Xen release?
>>>>>
>>>>> If so, please add a note about this in the commit message. Also, if that
>>>>> is the case, I think this patch series should go in 4.17. If it is too
>>>>> late to get it in before the release, then we should backport it to 4.17
>>>>> as soon as possible. That's because ideally we want to keep the
>>>>> hypercall interface changes down to a minimum.
>>>> On ARM, the hypercall has existed for a little over 4 weeks, and isn't
>>>> in any released version of Xen (yet).
>>>>
>>>> On x86, the hypercall has existed for more than a decade, and has known
>>>> out-of-tree users.  It needs to be deprecated properly, which in this
>>>> case means "phased out in the 4.18 cycle once known callers have been
>>>> adapted to the new hypercall".
>>> Understoon. Then I am in favor of getting all 4 patches in 4.17, either
>>> before the release or via backports.
>> Removing something from the domctl interface generally requires bumping
>> the interface version, so some extra care may need applying if such an
>> interface change was to be backported to any stable branch.
> 
> To be clear, I have no plans to remove the x86 "older" interface in this
> patch series.  It will definitely break out of tree users.
> 
> In the 4.18 timeframe, we can see about retiring the older hypercalls,
> but as a non-backportable change.

Sure, but I was referring to the (pretty new) Arm incarnation thereof.

Jan