Re: [Xen-devel] [PATCH v2 01/20] x86: make hvm_{get/set}_param accessible

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.12.2019 14:10, Tamas K Lengyel wrote:
> On Fri, Dec 27, 2019 at 1:04 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>
>> (re-sending, as I still don't see the mail having appeared on the list)
>>
>> On 23.12.2019 15:55, Tamas K Lengyel wrote:
>>> On Mon, Dec 23, 2019 at 2:37 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>>
>>>> On 20.12.2019 18:32, Andrew Cooper wrote:
>>>>> On 20/12/2019 17:27, Tamas K Lengyel wrote:
>>>>>> On Fri, Dec 20, 2019 at 9:47 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>>>>> On 18.12.2019 20:40, Tamas K Lengyel wrote:
>>>>>>>> Currently the hvm parameters are only accessible via the HVMOP 
>>>>>>>> hypercalls. By
>>>>>>>> exposing hvm_{get/set}_param it will be possible for VM forking to 
>>>>>>>> copy the
>>>>>>>> parameters directly into the clone domain.
>>>>>>> Having peeked ahead at patch 17, where this gets used, I wonder why
>>>>>>> you want a pair of one-by-one functions, rather than a copy-all one.
>>>>>>> This then wouldn't require exposure of the functions you touch here.
>>>>>> Well, provided there is no such function in existence today it was
>>>>>> just easier to use what's already available. I still wouldn't want to
>>>>>> implement a one-shot function like that because this same code-path is
>>>>>> shared by the save-restore operations on the toolstack side, so at
>>>>>> least I have a reasonable assumption that it won't break on me in the
>>>>>> future.
>>>>>
>>>>> In particular, a number of the set operations are distinctly
>>>>> non-trivial.
>>>>
>>>> How is trivial or not related to there being one function doing
>>>> the looping wanted here vs the looping being done by the caller
>>>> around the two per-entity calls?
>>>
>>> I don't really get why would it matter where the looping is being
>>> done? Even if I were to add a single function to do this, it would do
>>> the same looping and just call the now internally kept get/set params
>>> functions.
>>
>> The difference (to me) is what level of control gets exposed outside
>> of the file. For example I also dislike external code doing this
>> somewhat odd (but necessary as per your communication with Andrew)
>> checking against zero values. Such are implementation details which
>> would better not be scatter around.
> 
> But you do realize that both of these functions are already exposed
> via hypercalls? So it's OK to call them from the toolstack but not
> from other parts of Xen itself? I don't see much reason there.

Right now we have exactly one path each allowing this get/set. Adding
a 2nd (from outside of hvm.c) opens the door for possible races
between the various (for now just two) possible call sites. Noticing
a possible problem when adding new code is imo quite a bit more
likely if everything lives centralized in one place. IOW "exposure"
here isn't meant so much in the sense of what entity in the system
gets to drive the data, but which entities within Xen may play with
it.

Jan

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