Re: [PATCH v6 01/11] memory: batch processing in acquire_resource()

[Julien Grall <julien@xxxxxxx>]

Hi,

On 15/07/2020 13:28, Jan Beulich wrote:
> May I ask that you drop the if() around this block of code?
>
> Also, looking at this, I wonder whether it's a good idea to use the
> "start extent" model here anyway: You could as well update the
> struct (saying that it may be clobbered in the public header) and
> copy the whole thing back to the original guest struct. This would
> then remove the pretty arbitrary "UINT_MAX >> MEMOP_EXTENT_SHIFT"
> limit you currently need to enforce. The main question is whether
> we'd consider such an adjustment to an existing interface
> acceptable; there's an at least theoretical risk that it may break
> existing callers. Then again no existing caller can sensibly have
> specified a count above 32, and when the copying back would be
> limited to just the continuation case, no such caller would be
> affected in any way afaict.
I can see two risks with this approach:
   1) There is no requirement for the 'arg' buffer to be in read-write 
memory.
   2) A guest may expect to re-use the value within the structure after 
the hypercalls.
So I think the "start extent" model is better. This is already used in 
other memory sub-hypercalls, so the risk to break existing users is more 
limited.
Cheers,

--
Julien Grall