Xen devel - May 2009 - [PATCH 0 of 5] Add support for mapping grant references into HVM guests (+ some cleanup)

Add support for mapping grant references into HVM domains, by modifying
the P2M table.

The first couple of patches just tidy up the current grant table
implementation a bit, fixing a couple of bugs in the process, and
should be fairly uncontroversial.  The fourth patch in the series adds
a new HVM op to Xen which allows a HVM guest to map a remote domain''s
grant reference into its P2M table, which can then be mapped by the
pagetables in the usual way.  The final patch adds matching support to
the Linux unmodified drivers tree, allowing the new operation to
actually be used, and also adds a couple of very simple test modules.

This isn''t actually terribly useful as it stands, because there are no
realistic consumers of this interface.  I wrote it mostly for the
benefit of our closed-source Windows drivers, but that obviously
doesn''t help people on the list very much.  I''m not quite sure
what
the best way of handling this is; it''s clearly better for us for
this stuff to go into xen-unstable, rather than being maintained as an
out-of-tree patch forever, and it''d be a bit of a waste to force
anyone else who wanted this functionality to reinvent it, but it seems
odd to add an interface which has no publicly available consumers.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi. Some comments.

- Patch 4 adds a new grant hypercall, but the current
  GNTTABOP_map_grant_ref can be used, I suppose. Why not to use
  gnttab_map_grant_ref::host_addr?

- It seems that the patch 4 touches common code unnecessary.
  it would suffice to touch create_grant_host_mapping() and
  replace_grant_host_mapping() inserting something like
  "if (hvm_domain) return hvm_domain_case()".

Then, your modification will be well separated so that the
possibility for inclusion would increase.

thanks,
  
On Tue, May 19, 2009 at 12:27:02PM +0100, steven.smith@eu.citrix.com
wrote:
> Add support for mapping grant references into HVM domains, by modifying
> the P2M table.
> 
> The first couple of patches just tidy up the current grant table
> implementation a bit, fixing a couple of bugs in the process, and
> should be fairly uncontroversial.  The fourth patch in the series adds
> a new HVM op to Xen which allows a HVM guest to map a remote
domain''s
> grant reference into its P2M table, which can then be mapped by the
> pagetables in the usual way.  The final patch adds matching support to
> the Linux unmodified drivers tree, allowing the new operation to
> actually be used, and also adds a couple of very simple test modules.
> 
> This isn''t actually terribly useful as it stands, because there
are no
> realistic consumers of this interface.  I wrote it mostly for the
> benefit of our closed-source Windows drivers, but that obviously
> doesn''t help people on the list very much.  I''m not quite
sure what
> the best way of handling this is; it''s clearly better for us for
> this stuff to go into xen-unstable, rather than being maintained as an
> out-of-tree patch forever, and it''d be a bit of a waste to force
> anyone else who wanted this functionality to reinvent it, but it seems
> odd to add an interface which has no publicly available consumers.
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
> 
-- 
yamahata

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi Steven,

I understand that we are going to drop Xen 32bit, and support only Xen
64bit on this feature??

I believe that Xen 32bit is still important and desired by many
people. So please considere supporting that with your patch.

Thanks,
N


On Tue, May 19, 2009 at 8:27 PM,  <steven.smith@eu.citrix.com>
wrote:
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> I understand that we are going to drop Xen 32bit, and support only Xen
> 64bit on this feature??
That''s the plan, yes.
> I believe that Xen 32bit is still important and desired by many
> people. So please considere supporting that with your patch.
The problem is that we only have a limited number of bits available in
the P2M entries, which limits the number of distinct types of P2M
entry which we can encode.  On 32-bit Xen, that limit is eight, of
which seven are already used, and the grant mapping patch needs
another two (one for read-write maps, and one for read-only).  There
just isn''t enough room everything we want to do.

It''d be easy enough to make grant mapping work on 32-bit Xen if we
were willing to drop some other feature (populate-on-demand, say), but
I don''t think that would be an acceptable solution.  Finding somewhere
else to stash the required bits would significantly complicate things,
and, since there aren''t any particularly convincing use-cases yet for
even the 64-bit variant, I didn''t think that would be worthwhile.

Steven.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> Hi. Some comments.
> 
> - Patch 4 adds a new grant hypercall, but the current
>   GNTTABOP_map_grant_ref can be used, I suppose. Why not to use
>   gnttab_map_grant_ref::host_addr?
> 
> - It seems that the patch 4 touches common code unnecessary.
>   it would suffice to touch create_grant_host_mapping() and
>   replace_grant_host_mapping() inserting something like
>   "if (hvm_domain) return hvm_domain_case()".
> 
> Then, your modification will be well separated so that the
> possibility for inclusion would increase.
Yes, it could be done like that.  I decided against going that way,
though, because we''d end up with the same call having significantly
different semantics in PV and HVM guests, which sounds like a bad
idea.

In PV guests, the grant map hypercalls map a grant reference into the
virtual address space, by going and rewriting guest PTEs.  In an HVM
guest, the guest PTEs are all owned by the guest OS kernel, and so
it''s not a good idea for Xen to go and modify them directly (even
ignoring the nasty interactions with shadow mode).  The patch
therefore works by modifying the P2M PTEs instead, since they''re owned
by Xen and it can modify them as it wills.  Since the two operations
are different, from a guest perspective, and have different semantics,
it seemed best not to try to mosh them together.

It would have been possible to instead allocate another GNTMAP_* flag,
and use that to indicate that the guest wants the different semantics.
That would have worked fine, but it seemed a bit less clean to me than
keeping the two interfaces separate.

Steven.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tue, May 26, 2009 at 6:08 PM, Steven Smith <steven.smith@citrix.com>
wrote:
>> I understand that we are going to drop Xen 32bit, and support only Xen
>> 64bit on this feature??
> That''s the plan, yes.
>
>> I believe that Xen 32bit is still important and desired by many
>> people. So please considere supporting that with your patch.
> The problem is that we only have a limited number of bits available in
> the P2M entries, which limits the number of distinct types of P2M
> entry which we can encode.  On 32-bit Xen, that limit is eight, of
> which seven are already used, and the grant mapping patch needs
> another two (one for read-write maps, and one for read-only).  There
> just isn''t enough room everything we want to do.
>
> It''d be easy enough to make grant mapping work on 32-bit Xen if we
> were willing to drop some other feature (populate-on-demand, say), but
> I don''t think that would be an acceptable solution.  Finding
somewhere
> else to stash the required bits would significantly complicate things,
> and, since there aren''t any particularly convincing use-cases yet
for
> even the 64-bit variant, I didn''t think that would be worthwhile.
OK, I see.

I want to try your patch. However, my Dom0 is 32bit, so Xen is 32 bit,
too (I compiled Xen from source code). How can I compile Xen in 64bit
for your patch now?

If it is not possible (or easy) to do that now, could you please
provide the patch to cross compile Xen in 64bit, too?
 That would be useful to try your patch, and also for other things.

Thanks,
N

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tue, May 26, 2009 at 10:23:46AM +0100, Steven Smith
wrote:
> > Hi. Some comments.
> > 
> > - Patch 4 adds a new grant hypercall, but the current
> >   GNTTABOP_map_grant_ref can be used, I suppose. Why not to use
> >   gnttab_map_grant_ref::host_addr?
> > 
> > - It seems that the patch 4 touches common code unnecessary.
> >   it would suffice to touch create_grant_host_mapping() and
> >   replace_grant_host_mapping() inserting something like
> >   "if (hvm_domain) return hvm_domain_case()".
> > 
> > Then, your modification will be well separated so that the
> > possibility for inclusion would increase.
> Yes, it could be done like that.  I decided against going that way,
> though, because we''d end up with the same call having
significantly
> different semantics in PV and HVM guests, which sounds like a bad
> idea.
> 
> In PV guests, the grant map hypercalls map a grant reference into the
> virtual address space, by going and rewriting guest PTEs.  In an HVM
> guest, the guest PTEs are all owned by the guest OS kernel, and so
> it''s not a good idea for Xen to go and modify them directly (even
> ignoring the nasty interactions with shadow mode).  The patch
> therefore works by modifying the P2M PTEs instead, since they''re
owned
> by Xen and it can modify them as it wills.  Since the two operations
> are different, from a guest perspective, and have different semantics,
> it seemed best not to try to mosh them together.
> 
> It would have been possible to instead allocate another GNTMAP_* flag,
> and use that to indicate that the guest wants the different semantics.
> That would have worked fine, but it seemed a bit less clean to me than
> keeping the two interfaces separate.
ia64 grant table works with guest physical address as updating
ia64 p2m table. (the ia64 p2m table isn''t exactly same to x86, though.
and please note note that ia64 guest works with always
auto_translated_physmap mode enabled.)
And by using "if (xen_feature(XENFEAT_auto_translated_physmap))",
almost all pv backend/frontend works for ia64 pv guest.
So if grant table for x86 HVM domain is implemented with existing
gnttabop, x86 HVM guest can use pv backend/frontend as is.

Your concern is windows pv driver, so this doesn''t make sense to you,
though.

thanks,
-- 
yamahata

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> >> I believe that Xen 32bit is still important and desired by many
> >> people. So please considere supporting that with your patch.
> > The problem is that we only have a limited number of bits available in
> > the P2M entries, which limits the number of distinct types of P2M
> > entry which we can encode.  On 32-bit Xen, that limit is eight, of
> > which seven are already used, and the grant mapping patch needs
> > another two (one for read-write maps, and one for read-only).  There
> > just isn''t enough room everything we want to do.
> >
> > It''d be easy enough to make grant mapping work on 32-bit Xen
if we
> > were willing to drop some other feature (populate-on-demand, say), but
> > I don''t think that would be an acceptable solution.  Finding
somewhere
> > else to stash the required bits would significantly complicate things,
> > and, since there aren''t any particularly convincing use-cases
yet for
> > even the 64-bit variant, I didn''t think that would be
worthwhile.
> OK, I see.
> 
> I want to try your patch. However, my Dom0 is 32bit, so Xen is 32 bit,
> too (I compiled Xen from source code). How can I compile Xen in 64bit
> for your patch now?
> 
> If it is not possible (or easy) to do that now, could you please
> provide the patch to cross compile Xen in 64bit, too?
>  That would be useful to try your patch, and also for other things.
Err...  The easy way of building a 64-bit Xen is just to install a
64-bit Linux and do a native compile from there.  It''s possible, in
principle, to cross-compile it, but it''s not something I''ve
ever done,
and it requires a fair bit of fiddling to get right.  I''d recommend
that you install a 64-bit version of Linux somewhere and do the
compilation from there, rather than trying to set up cross toolchains,
unless you have absolutely no choice in the matter.

Steven.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> > In PV guests, the grant map hypercalls map a grant reference into the
> > virtual address space, by going and rewriting guest PTEs.  In an HVM
> > guest, the guest PTEs are all owned by the guest OS kernel, and so
> > it''s not a good idea for Xen to go and modify them directly
(even
> > ignoring the nasty interactions with shadow mode).  The patch
> > therefore works by modifying the P2M PTEs instead, since
they''re owned
> > by Xen and it can modify them as it wills.  Since the two operations
> > are different, from a guest perspective, and have different semantics,
> > it seemed best not to try to mosh them together.
> > 
> > It would have been possible to instead allocate another GNTMAP_* flag,
> > and use that to indicate that the guest wants the different semantics.
> > That would have worked fine, but it seemed a bit less clean to me than
> > keeping the two interfaces separate.
> ia64 grant table works with guest physical address as updating ia64
> p2m table. (the ia64 p2m table isn''t exactly same to x86, though.
> and please note note that ia64 guest works with always
> auto_translated_physmap mode enabled.)  And by using "if
> (xen_feature(XENFEAT_auto_translated_physmap))", almost all pv
> backend/frontend works for ia64 pv guest.
Okay, so the suggestion is that we should use map-to-VA if
auto_translated_physmap is clear, and map-via-P2M if it''s set?  Tying
together largely unrelated features like that sounds to me like
encoding implementation limitations into the interface, which doesn''t
sound like a good idea.
> So if grant table for x86 HVM domain is implemented with existing
> gnttabop, x86 HVM guest can use pv backend/frontend as is.
> 
> Your concern is windows pv driver, so this doesn''t make sense to
you,
> though.
Well, no, it''s not directly relevant to me, but at the same time being
able to move backends between PV and HVM guests easily would be pretty
useful.  Any API we can come up with will necessarily require some
changes to the backends, though, so it''s mostly a matter of balancing
the amount of porting needed now against the long-term maintenance
cost of having yet more weird special cases in our APIs.  I think that
keying off of auto_translated_physmap probably puts too much emphasis
on the short-term cost, but using an explicit GNTMAP_p2m_map flag
might be better.

Steven.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tue, May 26, 2009 at 12:46:25PM +0100, Steven Smith
wrote:
> > > In PV guests, the grant map hypercalls map a grant reference into
the
> > > virtual address space, by going and rewriting guest PTEs.  In an
HVM
> > > guest, the guest PTEs are all owned by the guest OS kernel, and
so
> > > it''s not a good idea for Xen to go and modify them
directly (even
> > > ignoring the nasty interactions with shadow mode).  The patch
> > > therefore works by modifying the P2M PTEs instead, since
they''re owned
> > > by Xen and it can modify them as it wills.  Since the two
operations
> > > are different, from a guest perspective, and have different
semantics,
> > > it seemed best not to try to mosh them together.
> > > 
> > > It would have been possible to instead allocate another GNTMAP_*
flag,
> > > and use that to indicate that the guest wants the different
semantics.
> > > That would have worked fine, but it seemed a bit less clean to me
than
> > > keeping the two interfaces separate.
> > ia64 grant table works with guest physical address as updating ia64
> > p2m table. (the ia64 p2m table isn''t exactly same to x86,
though.
> > and please note note that ia64 guest works with always
> > auto_translated_physmap mode enabled.)  And by using "if
> > (xen_feature(XENFEAT_auto_translated_physmap))", almost all pv
> > backend/frontend works for ia64 pv guest.
> Okay, so the suggestion is that we should use map-to-VA if
> auto_translated_physmap is clear, and map-via-P2M if it''s set?
Yes. That is exactly what ia64 xen is doing. (ia64 xen supports only 
auto translated mode enabled mode.)

>  Tying
> together largely unrelated features like that sounds to me like
> encoding implementation limitations into the interface, which
doesn''t
> sound like a good idea.
> 
> > So if grant table for x86 HVM domain is implemented with existing
> > gnttabop, x86 HVM guest can use pv backend/frontend as is.
> > 
> > Your concern is windows pv driver, so this doesn''t make sense
to you,
> > though.
> Well, no, it''s not directly relevant to me, but at the same time
being
> able to move backends between PV and HVM guests easily would be pretty
> useful.  Any API we can come up with will necessarily require some
> changes to the backends, though, so it''s mostly a matter of
balancing
> the amount of porting needed now against the long-term maintenance
> cost of having yet more weird special cases in our APIs.  I think that
> keying off of auto_translated_physmap probably puts too much emphasis
> on the short-term cost, but using an explicit GNTMAP_p2m_map flag
> might be better.
Yeah. I agree with the point. It''s a trade off.
auto_translated_physmap mode support was included long time before and 
uite stable. At least for linux pv drivers in linux-2.6.18-xen.hg.
In fact it was introduced before switching to linux-2.6.18-xen.hg tree.
I.e. before June 2007.

Please note that I''m not trying to prevent your new hypercall.
I''m quite fine as long as it doesn''t break existing ones.
I was curious why you came up with a new hypercall instead
of reusing existing hypercall.

thanks,
-- 
yamahata

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel