[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC 4/4] xen: arch-specific hooks for domain_soft_reset()
"Jan Beulich" <JBeulich@xxxxxxxx> writes: >>>> On 03.06.15 at 15:35, <vkuznets@xxxxxxxxxx> wrote: >> +int arch_domain_soft_reset(struct domain *d) >> +{ >> + struct page_info *page = virt_to_page(d->shared_info), *new_page; >> + int ret = 0; >> + struct domain *owner; >> + unsigned long mfn, mfn_new, gfn; >> + p2m_type_t __p2mt; > > No need for leading underscores here. > >> + if ( is_hvm_domain(d) ) >> + { > > Isn't the shared_info manipulation also HVM specific? Or at least > paging_mode_translate()-specific? xenmem_add_to_physmap_one() > bails on !paging_mode_translate()... But then I'm not really > understanding the issue you try to address there anyway, and > hence I may be overlooking something: Why does allocating a new > page help with a subsequent XENMAPSPACE_shared_info add-to- > physmap request? When an HVM domain starts it selects one of its pages and sacrifices it in favor of the shared_info page which belongs to xen heap, XENMAPSPACE_shared_info maps this page instead of the sacrificed one which is being freed. After kexec the domain can select another page it is willing to sacrifice and ask to map shared_info there but if we just remap it from its previous location we get a hole. So here I'm trying to unmap the shared_info page from domain's address space replacing it with an empty page. The other possible solution was suggested by Tim: instead of sucha a swap reassign the currently-used shared_info page to the domain and allocate a new shared_info page in xen heap. I haven't tried this approach. >> + return ret; >> + >> + /* >> + * shared_info page needs to be replaced with a new page, otherwise we >> + * will get a hole if the domain does XENMAPSPACE_shared_info >> + */ >> + >> + owner = page_get_owner_and_reference(page); >> + if ( owner != d ) >> + { >> + put_page(page); >> + return 0; > > Isn't this an error? > It probably means the shared_info page was never mapped to the domain's space, no need to replace it. >> --- a/xen/common/domain.c >> +++ b/xen/common/domain.c >> @@ -1467,6 +1467,8 @@ int domain_soft_reset(struct domain *d) >> for_each_vcpu ( d, v ) >> unmap_vcpu_info(v); >> >> + ret = arch_domain_soft_reset(d); >> + >> vcpu_unpause: >> for_each_vcpu ( d, v ) >> if ( v != current ) > > Similar question as on an earlier patch - is it really correct to unpause > the vCPU-s again after a possible failure from arch_domain_soft_reset()? Not sure what's the best here: unpausing and hoping the domain can cope (e.g. in kdump case we don't need much) or destroying the domain as we can't recover from all possible failures. -- Vitaly _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |