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

[Henry Wang <Henry.Wang@xxxxxxx>]

Hi Andrew and Stefano,

Thanks for pushing things forward!

> -----Original Message-----
> From: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Subject: Re: [PATCH 3/4] xen/arm, libxl: Revert XEN_DOMCTL_shadow_op;
> use p2m mempool hypercalls
> 
> 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.

Sorry - today it took me a little bit longer to get the office, so hopefully
I still jumped into discussion on time.

About this series, I don't have strong objection to taking all 4 patches, so
if this series can have proper review/agreements by this weekend, feel free
to add my release-ack for the patches.

However, if we cannot sort out all 4 patches, I think at least patch #4 should
go into 4.17 (with a commit message adjustment). The patch #4 already has
proper tags from Arm maintainer and me.

Kind regards,
Henry