Re: [Xen-devel] [PATCH v2 1/2] xenconsoled: use grant references instead of map_foreign_range

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 11 Jan 2013, Daniel De Graaf wrote:
> On 01/11/2013 02:08 PM, Stefano Stabellini wrote:
> > On Thu, 10 Jan 2013, Daniel De Graaf wrote:
> >> Grant references for the xenstore and xenconsole shared pages exist, but
> >> currently only xenstore uses these references.  Change the xenconsole
> >> daemon to prefer using the grant reference over map_foreign_range when
> >> mapping the shared console ring.
> >>
> >> This allows xenconsoled to be run in a domain other than dom0 if set up
> >> correctly - for libxl, the xenstore path /tool/xenconsoled/domid
> >> specifies the domain containing xenconsoled.
> > 
> > Unfortunately at the moment xc_dom_gnttab_init doesn't work with an
> > autotranslated dom0, that is the only type of dom0 that you get on ARM.
> > As a consequence this patch would break xenconsoled on ARM. I expect the
> > same to happen with PVH as well.
> >
> > I also have the same issue with xenstored and I was thinking about
> > writing a patch to make it gracefully fallback to xc_map_foreign_range
> > if xc_gnttab_map_grant_ref fails.
> 
> This patch should already do this, unlike xenstored where it only tries
> xc_map_foreign_range when the grant table handle failed to open.

Yes, you are right, sorry for noticing it only now.


> > The reason why xc_dom_gnttab_init is broken is that it uses
> > xc_map_foreign_range to map the grant table page of the foreign domain,
> > that is not a regular gpfn page, therefore it fails.
> > Specifically on ARM it hits the case XENMAPSPACE_gmfn_foreign in
> > xenmem_add_to_physmap_one, then it fails the p2m lookup and returns
> > EINVAL.
> 
> I'm unsure why xc_map_foreign_range is being used to map a grant table
> page: the only use of this function in xenconsoled is as a fallback for
> when xc_gnttab_map_grant_ref fails (which sounds like the case on PVH/ARM),
> but in this case the code should act the same as before this patch.

It didn't explain myself very well. Give a look at xc_dom_gnttab_seed:
right after xc_dom_gnttab_setup, it calls xc_map_foreign_range on it,
but the page returned by xc_dom_gnttab_setup is not going to be part of
the guest p2m, therefore xc_map_foreign_range will fail.
However xc_dom_gnttab_hvm_seed should work just fine.


> > Do you feel up for making xc_dom_gnttab_init work for an autotranslated
> > dom0?
> > Otherwise I think that for the moment is best to modify this patch to
> > gracefully fallback to xc_map_foreign_range. Then we need a couple of
> > other patches to do the same with xenstored and libxl.
> 
> xenstored also should fall back gracefully if the grant table fails to open
> (a simple way test this on x86 is to fail to load the xen-gntdev module
> before starting xenstored).  I haven't tested this recently, but it looks
> correct from a glance.

Right, but in my case xenstored opens the grant table correctly. However
xc_gnttab_map_grant_ref would still fail and at that point xenstored is
unable to fall back to xc_map_foreign_range.


> I don't think libxl does any grant table mapping: it seeds the guest's grant
> table with some entries for xenstore and xenconsole, but never maps them.

libxl doesn't but libxc does in xc_dom_gnttab_seed.

In any case I managed to solve the problem on ARM, calling
xc_dom_gnttab_hvm_seed instead of xc_dom_gnttab_seed, and implementing
gnttab_create_shared_page and gnttab_shared_gmfn correctly in the
hypervisor. I am now able to use the grant table mapping for the
xenstore page. I hope that the same strategy is going to work correctly
for the console page in xenconsoled.

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