Mailing List Archive

At 08:28 +0000 on 02 Dec (1322814505), Jan Beulich wrote:
> >>> On 29.11.11 at 21:21, Andres Lagar-Cavilla <andres@lagarcavilla.org> wrote:
> > @@ -559,13 +560,15 @@ int set_p2m_entry(struct p2m_domain *p2m
> > extern void p2m_pt_init(struct p2m_domain *p2m);
> >
> > /* Debugging and auditing of the P2M code? */
> > -#define P2M_AUDIT 0
> > +#define P2M_AUDIT 1
> > #define P2M_DEBUGGING 0
> >
> > #if P2M_AUDIT
>
> Was this change of the default really intended?

I think so - now that the ausdit code is not on any critical paths there's
no reason not to have it available by default.

> It uncovered a problem
> in 22356:cbb6b4b17024 (the code meanwhile moved elsewhere): In a
> 32-bit build,
>
> if ( gfn == 0x55555555 || gfn == 0x5555555555555555 )
>
> causes (at least with some compiler versions, didn't check tonight's
> stage tester results yet) a compiler warning

I don't think these debug values are serving any purpose. How about we
just get rid of them?

x86/mm: remove 0x55 debug pattern from M2P table

It's not really any more useful than explicitly setting new M2P entries
to the invalid value.

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r 60d4e257d04b tools/libxc/xc_domain.c
--- a/tools/libxc/xc_domain.c Fri Dec 02 09:05:26 2011 +0100
+++ b/tools/libxc/xc_domain.c Fri Dec 02 10:19:18 2011 +0000
@@ -1474,8 +1474,7 @@ int xc_domain_debug_control(xc_interface

int xc_domain_p2m_audit(xc_interface *xch,
uint32_t domid,
- uint64_t *orphans_debug,
- uint64_t *orphans_invalid,
+ uint64_t *orphans,
uint64_t *m2p_bad,
uint64_t *p2m_bad)
{
@@ -1486,10 +1485,9 @@ int xc_domain_p2m_audit(xc_interface *xc
domctl.domain = domid;
rc = do_domctl(xch, &domctl);

- *orphans_debug = domctl.u.audit_p2m.orphans_debug;
- *orphans_invalid = domctl.u.audit_p2m.orphans_invalid;
- *m2p_bad = domctl.u.audit_p2m.m2p_bad;
- *p2m_bad = domctl.u.audit_p2m.p2m_bad;
+ *orphans = domctl.u.audit_p2m.orphans;
+ *m2p_bad = domctl.u.audit_p2m.m2p_bad;
+ *p2m_bad = domctl.u.audit_p2m.p2m_bad;

return rc;
}
diff -r 60d4e257d04b tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h Fri Dec 02 09:05:26 2011 +0100
+++ b/tools/libxc/xenctrl.h Fri Dec 02 10:19:18 2011 +0000
@@ -718,9 +718,7 @@ int xc_domain_setdebugging(xc_interface
* @parm xch a handle to an open hypervisor interface
* @parm domid the domain id whose top level p2m we
* want to audit
- * @parm orphans_debug count of m2p entries for valid
- * domain pages containing a debug value
- * @parm orphans_invalid count of m2p entries for valid
+ * @parm orphans count of m2p entries for valid
* domain pages containing an invalid value
* @parm m2p_bad count of m2p entries mismatching the
* associated p2m entry for this domain
@@ -732,9 +730,8 @@ int xc_domain_setdebugging(xc_interface
* -EFAULT: could not copy results back to guest
*/
int xc_domain_p2m_audit(xc_interface *xch,
- uint32_t domid,
- uint64_t *orphans_debug,
- uint64_t *orphans_invalid,
+ uint32_t domid,
+ uint64_t *orphans,
uint64_t *m2p_bad,
uint64_t *p2m_bad);

diff -r 60d4e257d04b xen/arch/x86/domctl.c
--- a/xen/arch/x86/domctl.c Fri Dec 02 09:05:26 2011 +0100
+++ b/xen/arch/x86/domctl.c Fri Dec 02 10:19:18 2011 +0000
@@ -1459,8 +1459,7 @@ long arch_do_domctl(
break;

audit_p2m(d,
- &domctl->u.audit_p2m.orphans_debug,
- &domctl->u.audit_p2m.orphans_invalid,
+ &domctl->u.audit_p2m.orphans,
&domctl->u.audit_p2m.m2p_bad,
&domctl->u.audit_p2m.p2m_bad);
rcu_unlock_domain(d);
diff -r 60d4e257d04b xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c Fri Dec 02 09:05:26 2011 +0100
+++ b/xen/arch/x86/mm/p2m.c Fri Dec 02 10:19:18 2011 +0000
@@ -307,13 +307,7 @@ int p2m_alloc_table(struct p2m_domain *p
/* Pages should not be shared that early */
ASSERT(gfn != SHARED_M2P_ENTRY);
page_count++;
- if (
-#ifdef __x86_64__
- (gfn != 0x5555555555555555L)
-#else
- (gfn != 0x55555555L)
-#endif
- && gfn != INVALID_M2P_ENTRY
+ if ( gfn != INVALID_M2P_ENTRY
&& !set_p2m_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2m_ram_rw, p2m->default_access) )
goto error_unlock;
}
@@ -513,14 +507,7 @@ guest_physmap_add_entry(struct domain *d
if ( page_get_owner(mfn_to_page(_mfn(mfn + i))) != d )
continue;
ogfn = mfn_to_gfn(d, _mfn(mfn+i));
- if (
-#ifdef __x86_64__
- (ogfn != 0x5555555555555555L)
-#else
- (ogfn != 0x55555555L)
-#endif
- && (ogfn != INVALID_M2P_ENTRY)
- && (ogfn != gfn + i) )
+ if ( (ogfn != INVALID_M2P_ENTRY) && (ogfn != gfn + i) )
{
/* This machine frame is already mapped at another physical
* address */
@@ -1447,8 +1434,7 @@ unsigned long paging_gva_to_gfn(struct v

#if P2M_AUDIT
void audit_p2m(struct domain *d,
- uint64_t *orphans_debug,
- uint64_t *orphans_invalid,
+ uint64_t *orphans,
uint64_t *m2p_bad,
uint64_t *p2m_bad)
{
@@ -1456,7 +1442,7 @@ void audit_p2m(struct domain *d,
struct domain *od;
unsigned long mfn, gfn;
mfn_t p2mfn;
- unsigned long orphans_d = 0, orphans_i = 0, mpbad = 0, pmbad = 0;
+ unsigned long orphans_count = 0, mpbad = 0, pmbad = 0;
p2m_access_t p2ma;
p2m_type_t type;
struct p2m_domain *p2m = p2m_get_hostp2m(d);
@@ -1492,20 +1478,12 @@ void audit_p2m(struct domain *d,
gfn = get_gpfn_from_mfn(mfn);
if ( gfn == INVALID_M2P_ENTRY )
{
- orphans_i++;
+ orphans_count++;
P2M_PRINTK("orphaned guest page: mfn=%#lx has invalid gfn\n",
mfn);
continue;
}

- if ( gfn == 0x55555555 || gfn == 0x5555555555555555 )
- {
- orphans_d++;
- P2M_PRINTK("orphaned guest page: mfn=%#lx has debug gfn\n",
- mfn);
- continue;
- }
-
if ( gfn == SHARED_M2P_ENTRY )
{
P2M_PRINTK("shared mfn (%lx) on domain page list!\n",
@@ -1538,9 +1516,8 @@ void audit_p2m(struct domain *d,
p2m_unlock(p2m);

P2M_PRINTK("p2m audit complete\n");
- if ( orphans_i | orphans_d | mpbad | pmbad )
- P2M_PRINTK("p2m audit found %lu orphans (%lu inval %lu debug)\n",
- orphans_i + orphans_d, orphans_i, orphans_d);
+ if ( orphans_count | mpbad | pmbad )
+ P2M_PRINTK("p2m audit found %lu orphans\n", orphans);
if ( mpbad | pmbad )
{
P2M_PRINTK("p2m audit found %lu odd p2m, %lu bad m2p entries\n",
@@ -1549,10 +1526,9 @@ void audit_p2m(struct domain *d,
}

out_p2m_audit:
- *orphans_debug = (uint64_t) orphans_d;
- *orphans_invalid = (uint64_t) orphans_i;
- *m2p_bad = (uint64_t) mpbad;
- *p2m_bad = (uint64_t) pmbad;
+ *orphans = (uint64_t) orphans_count;
+ *m2p_bad = (uint64_t) mpbad;
+ *p2m_bad = (uint64_t) pmbad;
}
#endif /* P2M_AUDIT */

diff -r 60d4e257d04b xen/arch/x86/x86_32/mm.c
--- a/xen/arch/x86/x86_32/mm.c Fri Dec 02 09:05:26 2011 +0100
+++ b/xen/arch/x86/x86_32/mm.c Fri Dec 02 10:19:18 2011 +0000
@@ -114,8 +114,8 @@ void __init paging_init(void)
l2e_write(&idle_pg_table_l2[l2_linear_offset(RO_MPT_VIRT_START) + i],
l2e_from_page(
pg, (__PAGE_HYPERVISOR | _PAGE_PSE) & ~_PAGE_RW));
- /* Fill with an obvious debug pattern. */
- memset((void *)(RDWR_MPT_VIRT_START + (i << L2_PAGETABLE_SHIFT)), 0x55,
+ /* Fill with INVALID_M2P_ENTRY. */
+ memset((void *)(RDWR_MPT_VIRT_START + (i << L2_PAGETABLE_SHIFT)), 0xFF,
1UL << L2_PAGETABLE_SHIFT);
}
#undef CNT
diff -r 60d4e257d04b xen/arch/x86/x86_64/mm.c
--- a/xen/arch/x86/x86_64/mm.c Fri Dec 02 09:05:26 2011 +0100
+++ b/xen/arch/x86/x86_64/mm.c Fri Dec 02 10:19:18 2011 +0000
@@ -495,7 +495,8 @@ static int setup_compat_m2p_table(struct
PAGE_HYPERVISOR);
if ( err )
break;
- memset((void *)rwva, 0x55, 1UL << L2_PAGETABLE_SHIFT);
+ /* Fill with INVALID_M2P_ENTRY. */
+ memset((void *)rwva, 0xFF, 1UL << L2_PAGETABLE_SHIFT);
/* NB. Cannot be GLOBAL as the ptes get copied into per-VM space. */
l2e_write(&l2_ro_mpt[l2_table_offset(va)], l2e_from_page(l1_pg, _PAGE_PSE|_PAGE_PRESENT));
}
@@ -569,8 +570,9 @@ static int setup_m2p_table(struct mem_ho
PAGE_HYPERVISOR);
if ( ret )
goto error;
+ /* Fill with INVALID_M2P_ENTRY. */
memset((void *)(RDWR_MPT_VIRT_START + i * sizeof(unsigned long)),
- 0x55, 1UL << L2_PAGETABLE_SHIFT);
+ 0xFF, 1UL << L2_PAGETABLE_SHIFT);

ASSERT(!(l3e_get_flags(l3_ro_mpt[l3_table_offset(va)]) &
_PAGE_PSE));
@@ -727,8 +729,9 @@ void __init paging_init(void)
page_to_mfn(l1_pg),
1UL << PAGETABLE_ORDER,
PAGE_HYPERVISOR);
+ /* Fill with INVALID_M2P_ENTRY. */
memset((void *)(RDWR_MPT_VIRT_START + (i << L2_PAGETABLE_SHIFT)),
- 0x55, 1UL << L2_PAGETABLE_SHIFT);
+ 0xFF, 1UL << L2_PAGETABLE_SHIFT);
}
if ( !((unsigned long)l2_ro_mpt & ~PAGE_MASK) )
{
diff -r 60d4e257d04b xen/include/asm-x86/p2m.h
--- a/xen/include/asm-x86/p2m.h Fri Dec 02 09:05:26 2011 +0100
+++ b/xen/include/asm-x86/p2m.h Fri Dec 02 10:19:18 2011 +0000
@@ -566,10 +566,9 @@ extern void p2m_pt_init(struct p2m_domai

#if P2M_AUDIT
extern void audit_p2m(struct domain *d,
- uint64_t *orphans_debug,
- uint64_t *orphans_invalid,
- uint64_t *m2p_bad,
- uint64_t *p2m_bad);
+ uint64_t *orphans,
+ uint64_t *m2p_bad,
+ uint64_t *p2m_bad);
#endif /* P2M_AUDIT */

/* Printouts */
diff -r 60d4e257d04b xen/include/public/domctl.h
--- a/xen/include/public/domctl.h Fri Dec 02 09:05:26 2011 +0100
+++ b/xen/include/public/domctl.h Fri Dec 02 10:19:18 2011 +0000
@@ -806,8 +806,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_mem_s

struct xen_domctl_audit_p2m {
/* OUT error counts */
- uint64_t orphans_debug;
- uint64_t orphans_invalid;
+ uint64_t orphans;
uint64_t m2p_bad;
uint64_t p2m_bad;
};