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

Re: [Xen-devel] [PATCH 1/8] viridian: add init hooks



> -----Original Message-----
> From: Andrew Cooper
> Sent: 02 January 2019 16:08
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Jan Beulich <jbeulich@xxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Roger
> Pau Monne <roger.pau@xxxxxxxxxx>
> Subject: Re: [PATCH 1/8] viridian: add init hooks
> 
> On 20/12/2018 16:33, Paul Durrant wrote:
> > This patch adds domain and vcpu init hooks for viridian features. The
> init
> > hooks do not yet do anything; they will be added to by subsequent
> patches.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> 
> Please can we start by fixing the current, broken, initialisation and
> destruction logic ?
> 
> To get rid of DOMCTL_setmaxvcpus, we need the *_destroy() logic to be
> fully idempotent.  Also, viridian_domain_deinit() should not call into
> viridian_vcpu_deinit(), and viridian_vcpu_deinit() shouldn't be faking
> up a write to the assist page.

The deinit calling logic is not changed by this patch... the domain deinit 
function has always called into the vcpu deinit function. I can try dropping it 
if you want.
The zero write to the assist page MSR is there to cause the mapping to be 
freed, which is necessary to avoid a leak. It is a somewhat hacky way of doing 
it though, I agree, and it is changed by a later patch in the series.

> 
> AFAICT, the deinit path is all entirely pointless at the moment and can
> be deleted.
> 

It is not required at this stage, but it will be required later and I generally 
prefer to introduce mirrored pairs of hooks in the same patch rather than 
separate patches in the same series.

  Paul

 ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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