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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 3 Oct 2012, Mukesh Rathor wrote:
> On Tue, 2 Oct 2012 18:36:19 -0700
> Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> 
> > On Wed, 26 Sep 2012 12:33:39 +0100
> > Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> 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.

Did you add the newly allocated shared_info page to dom0's p2m?
If not, the page doesn't actually belong to the domain (from the p2m
POV), so get_gpfn_from_mfn might not behave correctly.


> 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.

We can leave page special as it is for now with very little
consequences, because it is not part of the interface with Xen. However
this is part of the interface, so whatever we choose here is going to be
hard to change later on.

I think it would be good if we could either make dom0 use a pfn or domU
use mfn, whatever is easier for you.

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