Re: [PATCH 6/8] xen/debug: WARN_ON when 1-1 but no _PAGE_IOMAP flag set.

Previous thread: [PATCH 8/8] xen/mmu: Set _PAGE_IOMAP if PFN is in identity P2M. by Konrad Rzeszutek Wilk on Thursday, December 30, 2010 - 12:48 pm. (1 message)

Next thread: [PATCH 7/8] xen/mmu: Introduce IDENTITY_FRAME_BIT by Konrad Rzeszutek Wilk on Thursday, December 30, 2010 - 12:48 pm. (3 messages)
From: Konrad Rzeszutek Wilk
Date: Thursday, December 30, 2010 - 12:48 pm

Only enabled if XEN_DEBUG_FS is enabled. We print a warning
when:

 pfn_to_mfn(pfn) == pfn, but no VM_IO (_PAGE_IOMAP) flag
 pfn_to_mfn(pfn) != pfn, and VM_IO flag is set.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/xen/mmu.c |   37 +++++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index e9dfdd6..d98bd43 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -944,6 +944,40 @@ pte_t xen_make_pte(pteval_t pte)
 }
 PV_CALLEE_SAVE_REGS_THUNK(xen_make_pte);
 
+#ifdef CONFIG_XEN_DEBUG_FS
+pte_t xen_make_pte_debug(pteval_t pte)
+{
+	phys_addr_t addr = (pte & PTE_PFN_MASK);
+	phys_addr_t other_addr;
+	bool io_page = false;
+	pte_t _pte;
+
+	if (pte & _PAGE_IOMAP)
+		io_page = true;
+
+	_pte = xen_make_pte(pte);
+
+	if (!addr)
+		return _pte;
+
+	if (io_page &&
+	    (xen_initial_domain() || addr >= ISA_END_ADDRESS)) {
+		other_addr = pfn_to_mfn(addr >> PAGE_SHIFT) << PAGE_SHIFT;
+		WARN(addr != other_addr,
+			"0x%lx is using VM_IO, but it is 0x%lx!\n",
+			(unsigned long)addr, (unsigned long)other_addr);
+	} else {
+		other_addr = (_pte.pte & PTE_PFN_MASK);
+		WARN((addr == other_addr) && (!io_page),
+			"0x%lx is missing VM_IO!\n",
+			(unsigned long)addr);
+	}
+
+	return _pte;
+}
+PV_CALLEE_SAVE_REGS_THUNK(xen_make_pte_debug);
+#endif
+
 pgd_t xen_make_pgd(pgdval_t pgd)
 {
 	pgd = pte_pfn_to_mfn(pgd);
@@ -2354,6 +2388,9 @@ __init void xen_ident_map_ISA(void)
 
 static __init void xen_post_allocator_init(void)
 {
+#ifdef CONFIG_XEN_DEBUG_FS
+	pv_mmu_ops.make_pte = PV_CALLEE_SAVE(xen_make_pte_debug);
+#endif
 	pv_mmu_ops.set_pte = xen_set_pte;
 	pv_mmu_ops.set_pmd = xen_set_pmd;
 	pv_mmu_ops.set_pud = xen_set_pud;
-- 
1.7.1

--

From: Ian Campbell
Date: Tuesday, January 4, 2011 - 10:24 am

Bit of an odd configuration option to use. Perhaps co-opt
CONFIG_PARAVIRT_DEBUG instead?

Or maybe we need a new XEN_DEBUG option? or just make it a developer
only EXTRA_CFLAGS +=-DDEBUG thing?

Is this only temporary until the need for _PAGE_IOMAP is removed anyway?

Ian.


--

From: Konrad Rzeszutek Wilk
Date: Tuesday, January 4, 2011 - 11:46 am

I was thinking to leave it as a way to weed out bugs, but I could as well
just leave it in my "debug" branch and not propose it upstream.

I am not sure how to remove the _PAGE_IOMAP fully. We need some _flag_ to
signal 'xen_pte_val' that the PTE should not be looked up in the M2P.

Otherwise, for identity mappings, the value is 0xfffff.. (or 0x55555.. sometimes)
and the PTE ends up being messed up. Instead there is a check to see if
_PAGE_IOMAP is part of the PTE, and if so, no M2P lookup is done.

I guess we could do the M2P irregardless and just see if it is 0xfffff... and
--

From: Ian Campbell
Date: Tuesday, January 4, 2011 - 12:20 pm

Perhaps this ties in with the m2p overlay which Stefano+Jeremy have been
working on to deal with granted foreign pages? I/O pages are a bit like
foreign memory (if you squint enough)...

Ian.

--

Previous thread: [PATCH 8/8] xen/mmu: Set _PAGE_IOMAP if PFN is in identity P2M. by Konrad Rzeszutek Wilk on Thursday, December 30, 2010 - 12:48 pm. (1 message)

Next thread: [PATCH 7/8] xen/mmu: Introduce IDENTITY_FRAME_BIT by Konrad Rzeszutek Wilk on Thursday, December 30, 2010 - 12:48 pm. (3 messages)