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