Re: [Xen-devel] workaround for dom0 crash due to QEMU using O_DIRECT

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 16 Jul 2013, Diana Crisan wrote:
> Hello,
> 
> I have tested the above patch against xen 4.3 with O_DIRECT *not* enabled and
> this patch makes dom0 crash when opening a file on nfs. Please see below my
> findings and a trace from the crashed dom0.
> 
> Environment:
> Linux 3.10 custom build with the patch that can be found below.
> O_DIRECT disabled
> 
> Actions perfomed:
> mount an nfs storage
> xl create xl.conf (which refers to a disk located in the nfs storage)
> 
> Findings: dom0 crashes before the guest fully boots up.

I can't really explain why it crashes for you without even using
O_DIRECT. I am testing it with and without O_DIRECT and with or without
NFS and it works OK for me. Also all those NMIs are suspicious.


Unfortunately I found out that the patch I posted is not complete
because unmapping the grants and restoring the old mappings is not a
single atomic operation at the moment.  The real issue is that the grant
unmap operation doesn't restore the original mapping automatically. We
do have a GNTTABOP_unmap_and_replace operation but it's not implemented
on x86 if GNTMAP_contains_pte.

I cannot see any solutions other than implementing a new grant table
hypercall or maybe force the usage of multicall.
For this test patch I have taken the second approach.



diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index 95fb2aa..f70aa46 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -968,7 +968,9 @@ int m2p_remove_override(struct page *page,
                if (!PageHighMem(page)) {
                        struct multicall_space mcs;
                        struct gnttab_unmap_grant_ref *unmap_op;
+                       struct mmu_update *u;
 
+                       WARN_ON(paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE);
                        /*
                         * It might be that we queued all the m2p grant table
                         * hypercalls in a multicall, then m2p_remove_override
@@ -989,8 +991,9 @@ int m2p_remove_override(struct page *page,
                                return -1;
                        }
 
-                       mcs = xen_mc_entry(
-                                       sizeof(struct gnttab_unmap_grant_ref));
+                       xen_mc_batch();
+
+                       mcs = __xen_mc_entry(sizeof(*unmap_op));
                        unmap_op = mcs.args;
                        unmap_op->host_addr = kmap_op->host_addr;
                        unmap_op->handle = kmap_op->handle;
@@ -999,10 +1002,15 @@ int m2p_remove_override(struct page *page,
                        MULTI_grant_table_op(mcs.mc,
                                        GNTTABOP_unmap_grant_ref, unmap_op, 1);
 
+                       mcs = __xen_mc_entry(sizeof(*u));
+                       u = mcs.args;
+                       u->ptr = virt_to_machine(ptep).maddr | 
MMU_NORMAL_PT_UPDATE;
+                       u->val = pte_val_ma(pfn_pte(pfn, PAGE_KERNEL));
+
+                       MULTI_mmu_update(mcs.mc, mcs.args, 1, NULL, DOMID_SELF);
+
                        xen_mc_issue(PARAVIRT_LAZY_MMU);
 
-                       set_pte_at(&init_mm, address, ptep,
-                                       pfn_pte(pfn, PAGE_KERNEL));
                        __flush_tlb_single(address);
                        kmap_op->host_addr = 0;
                }
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 930fb68..ef9bc91 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -88,6 +88,7 @@ EXPORT_SYMBOL_GPL(balloon_stats);
 
 /* We increase/decrease in batches which fit in a page */
 static xen_pfn_t frame_list[PAGE_SIZE / sizeof(unsigned long)];
+struct page* trade_page;
 
 #ifdef CONFIG_HIGHMEM
 #define inc_totalhigh_pages() (totalhigh_pages++)
@@ -423,7 +424,7 @@ static enum bp_state decrease_reservation(unsigned long 
nr_pages, gfp_t gfp)
                if (xen_pv_domain() && !PageHighMem(page)) {
                        ret = HYPERVISOR_update_va_mapping(
                                (unsigned long)__va(pfn << PAGE_SHIFT),
-                               __pte_ma(0), 0);
+                               pfn_pte(page_to_pfn(trade_page), 
PAGE_KERNEL_RO), 0);
                        BUG_ON(ret);
                }
 #endif
@@ -436,7 +437,7 @@ static enum bp_state decrease_reservation(unsigned long 
nr_pages, gfp_t gfp)
        /* No more mappings: invalidate P2M and add to balloon. */
        for (i = 0; i < nr_pages; i++) {
                pfn = mfn_to_pfn(frame_list[i]);
-               __set_phys_to_machine(pfn, INVALID_P2M_ENTRY);
+               __set_phys_to_machine(pfn, pfn_to_mfn(page_to_pfn(trade_page)));
                balloon_append(pfn_to_page(pfn));
        }
 
@@ -591,6 +592,10 @@ static int __init balloon_init(void)
        if (!xen_domain())
                return -ENODEV;
 
+       trade_page = alloc_page(GFP_KERNEL);
+       if (trade_page == NULL)
+               return -ENOMEM;
+
        pr_info("xen/balloon: Initialising balloon driver.\n");
 
        balloon_stats.current_pages = xen_pv_domain()

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel