Xen devel - Apr 2007 - [PATCH-REWORKED] Added xen_is_contiguous_region

Continuation of the following thread
http://lists.xensource.com/archives/html/xen-devel/2007-04/msg00135.html

Find the patch attached.

I haven''t compiled the changes for IA64.  Consider changes as
indicative and
make suitable amends as you deem necessary.

Sorry for the trouble.

Jambunathan K.



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

What are you trying to do? There¹s no reasoning behind this patch, nor any
sign off.

 -- Keir

On 13/4/07 20:25, "Jambunathan K" <kjambunathan@gmail.com>
wrote:
> Continuation of the following thread
> http://lists.xensource.com/archives/html/xen-devel/2007-04/msg00135.html
> 
> Find the patch attached.
> 
> I haven''t compiled the changes for IA64.  Consider changes as
indicative and
> make suitable amends as you deem necessary.
> 
> Sorry for the trouble.



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

On 4/14/07, Keir Fraser <Keir.Fraser@cl.cam.ac.uk>
wrote:
>
>  What are you trying to do? There''s no reasoning behind this
patch, nor
> any sign off.
>
>  -- Keir
>

As indicated in the therad (*
http://lists.xensource.com/archives/html/xen-devel/2007-04/msg00135.html*)
our driver has a 35-bit mask and maintains it''s own bounce pools. Our
experience with native linux is that this arrangement scales well as against
using the shared system pool.

pci_map_single as it stands today sneaks a swiotlb buffer underneath us and
we end up implicitly falling back upon the shared pool.

Using the new xen_is_contiguous_region() would enable our driver to fall
back upon it''s own pool.

This is my first patch to Xen. If you approve of the patch, I will resubmit
the changes as required by you.

Jambunathan K.


On 13/4/07 20:25, "Jambunathan K" <kjambunathan@gmail.com>
wrote:
>
> Continuation of the following thread
> *http://lists.xensource.com/archives/html/xen-devel/2007-04/msg00135.html
> *
> Find the patch attached.
>
> I haven''t compiled the changes for IA64.  Consider changes as
indicative
> and make suitable amends as you deem necessary.
>
> Sorry for the trouble.
>
>
>

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

On 13/4/07 20:56, "Jambunathan K" <kjambunathan@gmail.com>
wrote:
> Using the new xen_is_contiguous_region() would enable our driver to fall
back
> upon it''s own pool.
> 
> This is my first patch to Xen. If you approve of the patch, I will resubmit
> the changes as required by you.
You can check whether a memory region straddles a page boundary yourself
without needing an external helper function. Also you should have a pretty
good idea whether a piece of memory you allocated will pass the
contiguous_bitmap test: if you called pci_alloc_consistent(), or
xen_create_contiguous_region(), then it will; if you simply kmalloc()ed or
get_free_pages()ed a region then it will not. If you are getting buffers
passed to you from elsewhere in the kernel, I guess it depends what kind of
device you¹re talking about, but it¹s pretty likely to be the case that if
you are passed page-straddling buffers that you are going to need to use a
bounce buffer.

But anyway ‹ I thought the issue was your DMA limit of 32GB? Why should
page-straddling buffers be a problem at all: your hardware probably supports
scatter-gather DMA, right?

 -- Keir



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

Could someone please explain the following
What is the purpose of get_page_and_type(page, domain,
PGT_writable_page) and why is it needed when doing a grant copy in
__gnttab_copy()?
Why can''t we just use get_page() to prevent the page from being freed
while we are doing the copy?
Thanks
 
Renato
 


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

> Could someone please explain the following
> What is the purpose of get_page_and_type(page, domain,
> PGT_writable_page) and why is it needed when doing a grant copy in
> __gnttab_copy()?
> Why can''t we just use get_page() to prevent the page from being
freed
> while we are doing the copy?
You don''t want guests receiving data into pages containing pagetables,
hence we make sure we can get the page as a writeable page.

Ian
 

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

>  You can check whether a memory region straddles a page boundary yourself
> without needing an external helper function. Also you should have a pretty
> good idea whether a piece of memory you allocated will pass the
> contiguous_bitmap test:
Agreed.

But I had seen a crash in DomU (with no swiotlb) when we try to pci_map_single
a alloc_skb buffer.

In Xen-3.0.4, alloc_skb in all probability allocs a contig buffer but
it wouldn''t
guarantee me the same. With Xen-3.0.5, with bit-width based allocator
we are much
more likely to succeed compared to system-wide functions that cater to least
common denominator.
> But anyway — I thought the issue was your DMA limit of 32GB? Why should
> page-straddling buffers be a problem at all: your hardware probably
supports
> scatter-gather DMA, right?
We don''t do scatter-gather DMA as yet.

Jambunathan K.

On 4/14/07, Keir Fraser <Keir.Fraser@cl.cam.ac.uk>
wrote:
>
>  On 13/4/07 20:56, "Jambunathan K" <kjambunathan@gmail.com>
wrote:
>
>
> Using the new xen_is_contiguous_region() would enable our driver to fall
back upon it''s own pool.
>
>  This is my first patch to Xen. If you approve of the patch, I will
resubmit the changes as required by you.
>
>  You can check whether a memory region straddles a page boundary yourself
without needing an external helper function. Also you should have a pretty good
idea whether a piece of memory you allocated will pass the contiguous_bitmap
test: if you called pci_alloc_consistent(), or xen_create_contiguous_region(),
then it will; if you simply kmalloc()ed or get_free_pages()ed a region then it
will not. If you are getting buffers passed to you from elsewhere in the kernel,
I guess it depends what kind of device you''re talking about, but
it''s pretty likely to be the case that if you are passed
page-straddling buffers that you are going to need to use a bounce buffer.
>
>  But anyway — I thought the issue was your DMA limit of 32GB? Why should
page-straddling buffers be a problem at all: your hardware probably supports
scatter-gather DMA, right?
>
>   -- Keir
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 14/4/07 05:56, "Jambunathan K" <kjambunathan@gmail.com>
wrote:
> But I had seen a crash in DomU (with no swiotlb) when we try to
pci_map_single
> a alloc_skb buffer.
> 
> In Xen-3.0.4, alloc_skb in all probability allocs a contig buffer but
> it wouldn''t
> guarantee me the same. With Xen-3.0.5, with bit-width based allocator
> we are much
> more likely to succeed compared to system-wide functions that cater to
least
> common denominator.
We stripped out all our own custom alloc_skb code in 3.0.5. Hence
alloc_skb() now will not return contiguous buffers.

 -- Keir




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

Ah, OK. Thanks for clarifying!
I assume this could only happen if the guest is buggy, right? Normally
the guest should not do a grant_copy into its pagetables or issue a
grant for its page tables. But in case it does it is only corrupting
itself right? I assume only the page owner could use the page for page
tables. Is it fair to say that the get_page_and_type() is only
protecting a guest to harm itself? In that case would it be acceptable
to transfer this enforcement to the guest itself if this gives a boost
in performance?

Thanks

Renato
> -----Original Message-----
> From: Ian Pratt [mailto:Ian.Pratt@cl.cam.ac.uk] 
> Sent: Friday, April 13, 2007 8:15 PM
> To: Santos, Jose Renato G; xen-devel@lists.xensource.com
> Cc: ian.pratt@cl.cam.ac.uk
> Subject: RE: [Xen-devel] question on get_page_and_type()
> 
> 
> > Could someone please explain the following What is the purpose of 
> > get_page_and_type(page, domain,
> > PGT_writable_page) and why is it needed when doing a grant copy in 
> > __gnttab_copy()?
> > Why can''t we just use get_page() to prevent the page from 
> being freed 
> > while we are doing the copy?
> 
> You don''t want guests receiving data into pages containing 
> pagetables, hence we make sure we can get the page as a 
> writeable page.
> 
> Ian
>  
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> Ah, OK. Thanks for clarifying!
> I assume this could only happen if the guest is buggy, right? Normally
> the guest should not do a grant_copy into its pagetables or issue a
> grant for its page tables. But in case it does it is only corrupting
> itself right? I assume only the page owner could use the page for page
> tables. Is it fair to say that the get_page_and_type() is only
> protecting a guest to harm itself? In that case would it be acceptable
> to transfer this enforcement to the guest itself if this gives a boost
> in performance?
No, for direct pagetable mode guests it''s necessary for correctness,
otherwise a guest can corrupt it''s pagetables and access memory
belonging to other guests or the hypervisor, or even tripple fault the
machine.

Ian

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

Yes, of course. Thanks, Ian.
I am not thinking straight (Tired, working late to finish the slides for
the summit). 
Regards
Renato 
> -----Original Message-----
> From: Ian Pratt [mailto:Ian.Pratt@cl.cam.ac.uk] 
> Sent: Saturday, April 14, 2007 9:20 PM
> To: Santos, Jose Renato G; Ian Pratt; xen-devel@lists.xensource.com
> Subject: RE: [Xen-devel] question on get_page_and_type()
> 
> > Ah, OK. Thanks for clarifying!
> > I assume this could only happen if the guest is buggy, 
> right? Normally 
> > the guest should not do a grant_copy into its pagetables or issue a 
> > grant for its page tables. But in case it does it is only 
> corrupting 
> > itself right? I assume only the page owner could use the 
> page for page 
> > tables. Is it fair to say that the get_page_and_type() is only 
> > protecting a guest to harm itself? In that case would it be 
> acceptable 
> > to transfer this enforcement to the guest itself if this 
> gives a boost 
> > in performance?
> 
> No, for direct pagetable mode guests it''s necessary for 
> correctness, otherwise a guest can corrupt it''s pagetables 
> and access memory belonging to other guests or the 
> hypervisor, or even tripple fault the machine.
> 
> Ian
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel