[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v5 3/5] p2m: change write_p2m_entry to return an error code
On Mon, Feb 25, 2019 at 05:42:02PM +0000, George Dunlap wrote: > On 2/21/19 4:50 PM, Roger Pau Monne wrote: > > This is in preparation for also changing p2m_entry_modify to return an > > error code. > > > > No functional change intended. > > > > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> > > --- > [snip] > > diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c > > index 04e9d81cf6..254c5dfd19 100644 > > --- a/xen/arch/x86/mm/p2m-pt.c > > +++ b/xen/arch/x86/mm/p2m-pt.c > > @@ -184,6 +184,8 @@ p2m_next_level(struct p2m_domain *p2m, void **table, > > l1_pgentry_t *p2m_entry, new_entry; > > void *next; > > unsigned int flags; > > + int rc; > > + mfn_t mfn = INVALID_MFN; > > > > if ( !(p2m_entry = p2m_find_entry(*table, gfn_remainder, gfn, > > shift, max)) ) > > @@ -194,7 +196,7 @@ p2m_next_level(struct p2m_domain *p2m, void **table, > > /* PoD/paging: Not present doesn't imply empty. */ > > if ( !flags ) > > { > > - mfn_t mfn = p2m_alloc_ptp(p2m, level); > > + mfn = p2m_alloc_ptp(p2m, level); > > > > if ( mfn_eq(mfn, INVALID_MFN) ) > > return -ENOMEM; > > @@ -202,13 +204,14 @@ p2m_next_level(struct p2m_domain *p2m, void **table, > > new_entry = l1e_from_mfn(mfn, P2M_BASE_FLAGS | _PAGE_RW); > > > > p2m_add_iommu_flags(&new_entry, level, > > IOMMUF_readable|IOMMUF_writable); > > - p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, level + 1); > > + rc = p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, level + > > 1); > > + if ( rc ) > > + ASSERT_UNREACHABLE(); > > } > > else if ( flags & _PAGE_PSE ) > > { > > /* Split superpages pages into smaller ones. */ > > unsigned long pfn = l1e_get_pfn(*p2m_entry); > > - mfn_t mfn; > > l1_pgentry_t *l1_entry; > > unsigned int i; > > > > @@ -250,18 +253,37 @@ p2m_next_level(struct p2m_domain *p2m, void **table, > > { > > new_entry = l1e_from_pfn(pfn | (i << ((level - 1) * > > PAGETABLE_ORDER)), > > flags); > > - p2m->write_p2m_entry(p2m, gfn, l1_entry + i, new_entry, level); > > + rc = p2m->write_p2m_entry(p2m, gfn, l1_entry + i, new_entry, > > level); > > + if ( rc ) > > + { > > + ASSERT_UNREACHABLE(); > > + break; > > + } > > } > > > > unmap_domain_page(l1_entry); > > > > - new_entry = l1e_from_mfn(mfn, P2M_BASE_FLAGS | _PAGE_RW); > > - p2m_add_iommu_flags(&new_entry, level, > > IOMMUF_readable|IOMMUF_writable); > > - p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, level + 1); > > + if ( !rc ) > > + { > > + new_entry = l1e_from_mfn(mfn, P2M_BASE_FLAGS | _PAGE_RW); > > + p2m_add_iommu_flags(&new_entry, level, > > + IOMMUF_readable|IOMMUF_writable); > > + rc = p2m->write_p2m_entry(p2m, gfn, p2m_entry, new_entry, > > + level + 1); > > + if ( rc ) > > + ASSERT_UNREACHABLE(); > > + } > > } > > else > > ASSERT(flags & _PAGE_PRESENT); > > > > + if ( rc ) > > + { > > + ASSERT(mfn_valid(mfn)); > > + p2m_free_ptp(p2m, mfn_to_page(mfn)); > > + return rc; > > + } > > + > > I think the idiomatic way to deal with this would be to have a label at > the end that everything jumps to something like the attached? That way > you don't have to spend mental effort making sure that nothing happens > between the error and the clean-up call. Jan explicitly asked to not use a label. I can change it, but I would like to have consensus first. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |