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

[Tamas K Lengyel <tamas.k.lengyel@xxxxxxxxx>]

On Fri, Dec 27, 2019 at 6:44 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> 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.

Sure, I'll move the loop to hvm.c then.

Thanks,
Tamas

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