Re: [Xen-devel] I cannot get any message from domU by console / pv_ops domU kernel crashes with xen_create_contiguous_region failed

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2009-12-22 at 16:59 +0000, Konrad Rzeszutek Wilk wrote:
> On Tue, Dec 22, 2009 at 11:19:19AM -0500, Konrad Rzeszutek Wilk wrote:
> > On Tue, Dec 22, 2009 at 04:09:36PM +0000, Ian Campbell wrote:
> > > On Tue, 2009-12-22 at 15:47 +0000, Konrad Rzeszutek Wilk wrote:
> > > > > I thought it used to be that you could only (successfully) make 
> > > > > order>0
> > > > > increase_reservation or mem_exchange hypercalls if you had I/O
> > > > > privileges? Has this changed?
> > > > 
> > > > I am looking at the 3.4 code I am not seeing any I/O privileges check.
> > > > 
> > > > I did not even know that those existed actually - could you give me an 
> > > > idea
> > > > when was the last time you saw it?
> > > 
> > > In xen-unstable multipage_allocation_permitted is called from
> > > memory_exchange() and increase_reservation() and is defined as 
> > > #define multipage_allocation_permitted(d, order)        \
> > >     (((order) <= 9) || /* allow 2MB superpages */       \
> > >      !rangeset_is_empty((d)->iomem_caps) ||             \
> > >      !rangeset_is_empty((d)->arch.ioport_caps))
> > > 
> > > The ((order) <= 9) || is new and isn't present in the 3.4 tree,
> > > previously you would have had to add a passthrough device to cause one
> > > of the other rangesets to become non-empty. 
> > 
> > AAh, and since the exchange of memory is done in small chunks we pass
> > underneath the radar even if did not have the passthrough device set.
> > 
> > Ian, thanks for finding the culprit. Let me roll out a patch that
> > will do what was done in the past (ie, turn SWIOTLB for DomU only
> > when iommu=soft was passed in) as a fix.
> 
> Jeremy,
> 
> Please consider this patch to the PV-OPS tree. 
> 
> [xen/swiotlb] Enable Xen-SWIOTLB only if running in privileged domain or if 
> in non-privileged with iommu=soft.
> 
> Previous to this patch we would unconditionally enable Xen-SWIOTLB
> if running in PV context. That does not work with Xen 3.4.2 as it has a
> security check to disable exchanging of MFNs if no PCI devices have been
> passed through. In 4.0 there is an additional check to allow 2MB super-pages
> - we accidentally circumvented that by exchanging pages under the 2MB chunk 
> size
> even without any PCI devices passed through.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> 
> diff --git a/arch/x86/xen/pci-swiotlb.c b/arch/x86/xen/pci-swiotlb.c
> index ecdbfe2..7fdfccc 100644
> --- a/arch/x86/xen/pci-swiotlb.c
> +++ b/arch/x86/xen/pci-swiotlb.c
> @@ -960,7 +960,7 @@ xen_swiotlb_fixup(void *buf, size_t size, unsigned long 
> nslabs)
>                               dma_bits);
>               } while (rc && dma_bits++ < max_dma_bits);
>               if (rc)
> -                     panic(KERN_ERR "xen_create_contiguous_region failed\n");
> +                     panic(KERN_ERR "xen_create_contiguous_region failed: 
> rc: %d\n", rc);
>  
>               i += slabs;
>       } while(i < nslabs);
> @@ -984,7 +984,16 @@ static struct dma_map_ops xen_swiotlb_dma_ops = {
>  
>  void __init xen_swiotlb_init(void)
>  {
> -     if (xen_domain()) {
> +        int use_swiotlb = 0;
> +
> +        if (xen_initial_domain())
> +                use_swiotlb = 1;
> +
> +        /* For PV guest, only if iommu=soft is passed in. */
> +        if (xen_pv_domain() && !xen_initial_domain() && swiotlb)
> +               use_swiotlb = 1;
> + 
> +     if (use_swiotlb) {

How about just 
          if (xen_pv_domain() && (xen_initial_domain() || swiotlb))
Or depending on how/where swiotlb gets set (i.e. if it is set IFF
xen_pv_domain) simply:
          if (xen_initial_domain() || swiotlb)

Ian.


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