Re: [PATCH v4 1/8] public / x86: Introduce __HYPERCALL_dm_op...

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Andrew Cooper [mailto:amc96@hermes.cam.ac.uk] On Behalf Of
> Andrew Cooper
> Sent: 23 January 2017 09:15
> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org
> Cc: Ian Jackson <Ian.Jackson@citrix.com>; Jennifer Herbert
> <jennifer.herbert@citrix.com>; Daniel De Graaf <dgdegra@tycho.nsa.gov>;
> Wei Liu <wei.liu2@citrix.com>; Jan Beulich <jbeulich@suse.com>
> Subject: Re: [PATCH v4 1/8] public / x86: Introduce __HYPERCALL_dm_op...
> 
> On 20/01/2017 15:02, Paul Durrant wrote:
> >
> >>> +    if ( !rc &&
> >>> +         !copy_buf_to_guest(bufs, nr_bufs, 0, &op, sizeof(op)) )
> >> Do all ops need a copyback?  If they do, this is fine.  If not, it would
> >> be better to have a copyback boolean which subops set as necessary.
> > I can restrict copy-back using a boolean set for sub-ops that have 'out'
> params, or when there needs to be a continuation but I didn't really think it
> was worth the extra complexity.
> 
> Extraneous writebacks to PV guests are fairly cheep, but is is certainly
> not the case for HVM guests.  A writeback to HVM requires a least one
> guest pagetable walk (which itself most likely includes an EPT/NPT walk).
> 
> From a correctness point of view, it is reasonable for an implementation
> which expects a hypercall datastructure to be read only, to put said
> structure in read-only memory.  The PKRU feature in particular makes it
> very easy to set something up, then switch it from RW to RO for use.
> Such an implementation should have the hypercall fail with a spurious
> -EFAULT after it has otherwise completed successfully.

<sigh> Ok, I'll re-work the code yet again.

  Paul

> 
> ~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel