Re: [PATCH 02/12] xen/arm: add cache coloring initialization for domains

[Julien Grall <julien@xxxxxxx>]

On 07/11/2022 13:44, Carlo Nonato wrote:
> Hi Julien,
Hi Carlo,

> On Tue, Oct 25, 2022 at 1:15 PM Julien Grall <julien@xxxxxxx> wrote:
> > On 25/10/2022 11:53, Carlo Nonato wrote:
> > > Hi Julien,
> > >
> > > On Fri, Oct 21, 2022 at 8:02 PM Julien Grall <julien@xxxxxxx> wrote:
> > > > Hi Carlo,
> > > >
> > > > On 26/08/2022 13:51, Carlo Nonato wrote:
> > > > > This commit adds array pointers to domains as well as to the hypercall
> > > > > and configuration structure employed in domain creation. The latter is used
> > > > > both by the toolstack and by Xen itself to pass configuration data to the
> > > > > domain creation function, so the XEN_GUEST_HANDLE macro must be adopted to be
> > > > > able to access guest memory in the first case. This implies special care for
> > > > > the copy of the configuration data into the domain data, meaning that a
> > > > > discrimination variable for the two possible code paths (coming from Xen or
> > > > > from the toolstack) is needed.
> > > > So this means that a toolstack could set from_guest. I know the
> > > > toolstack is trusted... However, we should try to limit when the trust
> > > > when this is possible.
> > > >
> > > > In this case, I would consider to modify the prototype of
> > > > domain_create() to pass internal information.
> > > Doing as you said isn't a bit too invasive? I should also change all the call
> > > sites of domain_create() and this means x86 too.
> > Yes there will be a few calls to modify. But this is better than hacking
> > the hypercall interface to cater for internal use.
> >
> > > Isn't there an easier way to spot a guest address? Maybe just looking at the
> > > address value...
> > HVM/Arm guest have a separate address space. So it is not possible to
> > differentiate between guest vs hypervisor address.
> >
> > > Or maybe adding an internal flag to the do_domctl() path.
> > IIUC, this flag would indicate whether the XEN_GUEST_HANDLE() is an
> > hypervisor or guest address. Is that correct?
> >
> > If so, I dislike it. I am not sure what the other maintainers think, but
> > personally updating domain_create() is my preferred way.
> Sorry to bother you again on this topic, but I thought of a way to get rid of
> the "from_guest" field which I hope is simple enough to convince you.
> I can call copy_from_guest() *only* in domctl.c, overwriting the colors
> pointer with a new, Xen allocated, array.
> This lets me simplify the logic in domain_coloring_init() since all the arrays
> coming to it via the domainconfig struct are allocated in Xen memory only.
> It's still a bit of a hack since I'm using the XEN_GUEST_HANDLE as a normal
> Xen pointer, but it's by far less hacky than before and doesn't have the trust
> problem.
You don't have the trust problem but you are still mixing guest handle 
and xen pointer. I continue dislike this because this a gross hack that 
may save you some effort today but will be a nightmare to 
review/use/maintain (the developer will have to remember whether the 
field contain a guest address or xen address).
Cheers,

--
Julien Grall