Re: [PATCH v1 3/8]: PVH startup changes (enlighten.c)

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Tue, 2 Oct 2012 18:36:19 -0700
Mukesh Rathor <mukesh.rathor@oracle.com> wrote:

> On Wed, 26 Sep 2012 12:33:39 +0100
> Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote:
> 
> > > Wish it was simple. But for PV and PVH, domU, it's already setup
> > > the shared page. All we need to do is __va(shared_info). But for
> > > HVM domUs and PVH dom0, we need to hcall with pfn to get it
> > > remapped.
> > 
> > For PVH domU is already setup as a pfn only because in
> > tools/libxc/xc_dom_x86.c:alloc_magic_pages we have this code:
> > 
> >     if ( xc_dom_feature_translated(dom) )
> >         dom->shared_info_pfn = xc_dom_alloc_page(dom, "shared
> > info");
> > 
> > and in tools/libxc/xc_dom_x86.c:start_info_x86_64 we have:
> > 
> >     xen_pfn_t shinfo =
> >         xc_dom_feature_translated(dom) ? dom->shared_info_pfn :
> > dom-> shared_info_mfn;
> > 
> > if we simply get rid of the two "if xc_dom_feature_translated(dom)"
> > wouldn't we get an mfn for PVH domU too? AFAICT all the other cases
> > would remain unmodified, but PVH domU would start getting an mfn for
> > shared_info.
> > 
> > > Changing the
> > > tool to map pfn, would result in unnecessary hcall for all PV and
> > > PVH domUs. It's only two lines of code, so lets just leave it.
> > > I'll make the comment better.
> > 
> > Yes, there would be one more unnecessary hypercall but we would get
> > rid of 4 "if". I think is a good trade off.
> 
> Well, not really unfortunately! There are two fields in the library 
> shared_info_mfn and shared_info_pfn in struct xc_dom_image. For
> xlated, pfn is set, otherwise, mfn is set. If I set mfn for auto 
> xlated also, I end up changing/adding  more code in the library where
> it tries to map the shared page. Moreover, it's better to stick with
> the library assumption that for auto xlated, pfn is set, otherwise,
> mfn is set.
> 
> Hope that makes sense. I tried it, btw.
> 

I'm not able to make a quick change to xen to just put pfn for dom0
also. get_gpfn_from_mfn() is crashing. So for now, how about, lets go
with what I've got. I'll make a note, and when I do xen patch, I'll
figure it out, and would be a very small linux patch. I want to get
linux patch in soon before the window closes.

thanks
Mukesh