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

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

[steven.smith]

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.

Re: [PATCH 0 of 5] Add support for mapping grant references into HVM guests (+ some cleanup)

[Isaku Yamahata]

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

Re: [PATCH 0 of 5] Add support for mapping grant references into HVM guests (+ some cleanup)

[Steven Smith]

[-- Attachment #1.1: Type: text/plain, Size: 1622 bytes --]

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

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH 0 of 5] Add support for mapping grant references into HVM guests (+ some cleanup)

[Isaku Yamahata]

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

Re: [PATCH 0 of 5] Add support for mapping grant references into HVM guests (+ some cleanup)

[Steven Smith]

[-- Attachment #1.1: Type: text/plain, Size: 2387 bytes --]

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

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH 0 of 5] Add support for mapping grant references into HVM guests (+ some cleanup)

[Isaku Yamahata]

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

Re: [PATCH 0 of 5] Add support for mapping grant references into HVM guests (+ some cleanup)

[Justin T. Gibbs]

Was any consensus reached on how to cleanly allow an HVM guest to map
grant references?  I'm in need of this feature for a FreeBSD x86_64 HVM
driver domain.  FreeBSD x86_64 does not support full PV, and I need
the ability to run backends in these domains for my application.  The
patches from this thread would likely work for me, but like the author
of them, I'd rather use a publicly supported Xen interface than to
have a locally maintained solution.

--
Justin

Re: [PATCH 0 of 5] Add support for mapping grant references into HVM guests (+ some cleanup)

[Keir Fraser]

Support is already in the tree and will be part of Xen 4.0. See xen-unstable
changeset 19932:c0cb307d927f.

 -- Keir

On 18/12/2009 15:26, "Justin T. Gibbs" <gibbs@scsiguy.com> wrote:

> Was any consensus reached on how to cleanly allow an HVM guest to map
> grant references?  I'm in need of this feature for a FreeBSD x86_64 HVM
> driver domain.  FreeBSD x86_64 does not support full PV, and I need
> the ability to run backends in these domains for my application.  The
> patches from this thread would likely work for me, but like the author
> of them, I'd rather use a publicly supported Xen interface than to
> have a locally maintained solution.
> 
> --
> Justin
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 0 of 5] Add support for mapping grant references into HVM guests (+ some cleanup)

[Justin T. Gibbs]

On 12/18/2009 9:05 AM, Keir Fraser wrote:
> Support is already in the tree and will be part of Xen 4.0. See xen-unstable
> changeset 19932:c0cb307d927f.
>
>   -- Keir
>    

I have successfully integrated the 4.0 change into my 3.4.1 test 
system.  The changes works as expected
so long as the I/O is serviced via a copy from memory owned by the 
device domain.  However, iommu
mappings are not updated for HVM grant table updates so it is not 
possible to use DMA.  I am experimenting
with changes to update the iommu for p2m_grant_map* types but wonder is 
some overarching
architectural consideration I'm missing that would explain why the iommu 
was not updated in the original
changes?

--
Justin