[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [V8 PATCH 5/8] pvh dom0: Add and remove foreign pages
>>> On 08.04.14 at 03:11, <mukesh.rathor@xxxxxxxxxx> wrote: > On Mon, 07 Apr 2014 07:57:56 +0100 > "Jan Beulich" <JBeulich@xxxxxxxx> wrote: >> In the end I think we'll want to make them all report proper error >> codes... > > Ok, how about the following patch then? If it's OK, I'd like to submit > independently. Yes, I just now stumbled across this oddity too. Looks generally like what we want - a couple of comments below. > @@ -696,7 +695,7 @@ long arch_do_domctl( > > if ( paging_mode_translate(d) ) > for ( i = 0; i < nr_mfns; i++ ) > - add |= !clear_mmio_p2m_entry(d, gfn + i); > + add |= !!clear_mmio_p2m_entry(d, gfn + i); No need for the double !!, and in fact ... > ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1); > if ( !ret && add ) > ret = -EIO; ... you should be propagating the error code here anyway rather than making one up. > @@ -124,15 +124,16 @@ nestedhap_fix_p2m(struct vcpu *v, struct p2m_domain > *p2m, > gfn = (L2_gpa >> PAGE_SHIFT) & mask; > mfn = _mfn((L0_gpa >> PAGE_SHIFT) & mask); > > - rv = set_p2m_entry(p2m, gfn, mfn, page_order, p2mt, p2ma); > + rc = set_p2m_entry(p2m, gfn, mfn, page_order, p2mt, p2ma); > } > > p2m_unlock(p2m); > > - if (rv == 0) { > + if ( rc ) > + { > gdprintk(XENLOG_ERR, > - "failed to set entry for %#"PRIx64" -> %#"PRIx64"\n", > - L2_gpa, L0_gpa); > + "failed to set entry for %#"PRIx64" -> %#"PRIx64" rc:%d\n", > + L2_gpa, L0_gpa, rc); Together with the other coding style cleanup you should also get rid of the hard tabs here. > @@ -769,26 +769,28 @@ set_mmio_p2m_entry(struct domain *d, unsigned long gfn, > mfn_t mfn) > } > > P2M_DEBUG("set mmio %lx %lx\n", gfn, mfn_x(mfn)); > - rc = set_p2m_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2m_mmio_direct, > p2m->default_access); > + rc = set_p2m_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2m_mmio_direct, > + p2m->default_access); > gfn_unlock(p2m, gfn, 0); > - if ( 0 == rc ) > + if ( rc ) > gdprintk(XENLOG_ERR, > - "set_mmio_p2m_entry: set_p2m_entry failed! mfn=%08lx\n", > - mfn_x(get_gfn_query_unlocked(p2m->domain, gfn, &ot))); > + "set_mmio_p2m_entry: set_p2m_entry failed! mfn=%08lx rc:%d\n", > + mfn_x(get_gfn_query_unlocked(p2m->domain, gfn, &ot)), rc); Again, if you already alter this and do minor cleanup at once, this should become printk(XENLOG_G_ERR "d%d: ...", ...); or the "set_mmio_p2m_entry:" prefix should be dropped. > @@ -835,12 +839,13 @@ set_shared_p2m_entry(struct domain *d, unsigned long > gfn, mfn_t mfn) > set_gpfn_from_mfn(mfn_x(omfn), INVALID_M2P_ENTRY); > > P2M_DEBUG("set shared %lx %lx\n", gfn, mfn_x(mfn)); > - rc = set_p2m_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2m_ram_shared, > p2m->default_access); > + rc = set_p2m_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2m_ram_shared, > + p2m->default_access); > gfn_unlock(p2m, gfn, 0); > - if ( 0 == rc ) > + if ( rc ) > gdprintk(XENLOG_ERR, > - "set_shared_p2m_entry: set_p2m_entry failed! mfn=%08lx\n", > - mfn_x(get_gfn_query_unlocked(p2m->domain, gfn, &ot))); > + "set_shared_p2m_entry: set_p2m_entry failed! mfn=%08lx rc:%d\n", > + mfn_x(get_gfn_query_unlocked(p2m->domain, gfn, &ot)), rc); Similarly here. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |