Re: [Xen-devel] [PATCH 4/5] xen: introduce XEN_GUEST_HANDLE_PARAM

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2012-08-07 at 13:35 +0100, Stefano Stabellini wrote:
> On Tue, 7 Aug 2012, Jan Beulich wrote:
> > >>> On 06.08.12 at 18:02, Stefano Stabellini 
> > >>> <stefano.stabellini@xxxxxxxxxxxxx>
> > wrote:
> > > On Mon, 6 Aug 2012, Jan Beulich wrote:
> > >> >>> On 06.08.12 at 17:47, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> > >> > On Mon, 2012-08-06 at 16:43 +0100, Jan Beulich wrote:
> > >> >> >>> On 06.08.12 at 16:12, Stefano Stabellini 
> > >> >> >>> <stefano.stabellini@xxxxxxxxxxxxx> 
> > >> > wrote:
> > >> >> > Note: this change does not make any difference on x86 and ia64.
> > >> >> > 
> > >> >> > 
> > >> >> > XEN_GUEST_HANDLE_PARAM is going to be used to distinguish guest 
> > >> >> > pointers
> > >> >> > stored in memory from guest pointers as hypercall parameters.
> > >> >> 
> > >> >> I have to admit that really dislike this, to a large part because of
> > >> >> the follow up patch that clutters the corresponding function
> > >> >> declarations even further. Plus I see no mechanism to convert
> > >> >> between the two, yet I can't see how - long term at least - you
> > >> >> could get away without such conversion.
> > >> >> 
> > >> >> Is it really a well thought through and settled upon decision to
> > >> >> make guest handles 64 bits wide even on 32-bit ARM? After all,
> > >> >> both x86 and PPC got away without doing so
> > >> > 
> > >> > Well, on x86 we have the compat XLAT layer, which is a pretty complex
> > >> > piece of code, so "got away without" is a bit strong...
> > >> 
> > >> Hmm, yes, that's a valid correction.
> > >> 
> > >> > We'd really
> > >> > rather not have to have a non-trivial compat layer on arm too by having
> > >> > the struct layouts be the same on 32/64.
> > >> 
> > >> And paying a penalty like this in the 32-bit half (if what is likely
> > >> to remain the much bigger portion for the next couple of years
> > >> can validly be called "half") is worth it? The more that the compat
> > >> layer is now reasonably mature (and should hence be easily
> > >> re-usable for ARM)?
> > > 
> > > What penalty? The only penalty is the wasted space in the structs in
> > > memory.
> > 
> > No - the caller has to zero-initialize those extra 32 bits, and the
> > hypervisor has to check for them to be zero (the latter may be
> > implicit in the 64-bit one, but certainly needs to be explicit on the
> > 32-bit side).
> 
> You are saying that on a 32 bit hypervisor we should check that the
> padding is zero? Why should we care about the value of the padding?

The point is so that we can treat them as 64 bit values in a 64 bit
hypervisor, otherwise we would need a compat layer to translate (which
is what we want to avoid).

So the 32 bit guest definitely does need to zero them, and a debug build
of the 32 bit hypervisor probably ought to reject non-zero upper halves,
otherwise the chances of the guest doing it consistently is pretty
small.


> In any case fortunately accesses to guest_handles already go via macros,
> so it should be easy to arrange if it comes down to it.



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