|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 1/1] Introduce VCPUOP_reset_vcpu_info
On 19/08/14 11:04, Vitaly Kuznetsov wrote:
> When an SMP guest performs kexec/kdump it tries issuing
> VCPUOP_register_vcpu_info.
> This fails due to vcpu_info already being registered. Introduce new vcpu
> operation
> to reset vcpu_info to its default state.
>
> Based on the original patch by Konrad Rzeszutek Wilk.
>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
> ---
> Changes from RFCv2:
> - modify unmap_vcpu_info() to match both needs [Jan Beulich]
> - support VCPUOP_reset_vcpu_info for current VCPU [Jan Beulich]
> - improve description in public/vcpu.h [Jan Beulich]
>
> Changes from RFCv1:
> - Don't use unsuitable unmap_vcpu_info(), rewrite [Jan Beulich]
> - Require FIFO ABI being in 2-level mode
> - Describe limitations in include/public/vcpu.h [Jan Beulich]
> ---
> xen/arch/x86/hvm/hvm.c | 1 +
> xen/common/domain.c | 73
> +++++++++++++++++++++++++++++++++++++++++------
> xen/include/public/vcpu.h | 19 ++++++++++++
> 3 files changed, 85 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 216c3f2..7917272 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3332,6 +3332,7 @@ static long hvm_vcpu_op(
> case VCPUOP_set_singleshot_timer:
> case VCPUOP_stop_singleshot_timer:
> case VCPUOP_register_vcpu_info:
> + case VCPUOP_reset_vcpu_info:
> case VCPUOP_register_vcpu_time_memory_area:
> rc = do_vcpu_op(cmd, vcpuid, arg);
> break;
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 44e5cbe..9e10e1e 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -956,25 +956,52 @@ int map_vcpu_info(struct vcpu *v, unsigned long gfn,
> unsigned offset)
> }
>
> /*
> - * Unmap the vcpu info page if the guest decided to place it somewhere
> - * else. This is only used from arch_domain_destroy, so there's no
> - * need to do anything clever.
> + * Unmap the vcpu info page if the guest decided to place it somewhere else.
> + * There's no need to do anything clever when the domain is dying (when
> called
> + * from arch_domain_destroy) and we need to prepare for rebinding when it is
> not
> + * (in case VCPUOP_reset_vcpu_info was called).
> */
> void unmap_vcpu_info(struct vcpu *v)
> {
> + struct domain *d = v->domain;
> unsigned long mfn;
> + vcpu_info_t *old_info;
> + unsigned int i;
> +
> + mfn = v->vcpu_info_mfn;
> + old_info = v->vcpu_info;
>
> - if ( v->vcpu_info_mfn == INVALID_MFN )
> + if ( mfn == INVALID_MFN )
> return;
>
> - mfn = v->vcpu_info_mfn;
> + if ( (!d->is_dying) && (v->vcpu_id < XEN_LEGACY_MAX_VCPUS) )
> + {
> + memcpy(&shared_info(d, vcpu_info[v->vcpu_id]), v->vcpu_info,
> + sizeof(vcpu_info_t));
> + v->vcpu_info = (vcpu_info_t *)&shared_info(d, vcpu_info[v->vcpu_id]);
> + }
> + else
> + v->vcpu_info = &dummy_vcpu_info;
> unmap_domain_page_global((void *)
> - ((unsigned long)v->vcpu_info & PAGE_MASK));
> + ((unsigned long)old_info & PAGE_MASK));
>
> - v->vcpu_info = &dummy_vcpu_info;
> + put_page_and_type(mfn_to_page(mfn));
> v->vcpu_info_mfn = INVALID_MFN;
>
> - put_page_and_type(mfn_to_page(mfn));
> + if ( (!d->is_dying) && (v->vcpu_id < XEN_LEGACY_MAX_VCPUS) &&
> + (test_bit(_VPF_down, &v->pause_flags)) )
> + {
> + /* Make sure vcpu_info was set */
> + smp_wmb();
> +
> + /*
> + * Mark everything as being pending for all offline VCPUs to make
> sure
> + * nothing gets lost when this VCPU goes online again.
> + */
> + vcpu_info(v, evtchn_upcall_pending) = 1;
> + for ( i = 0; i < BITS_PER_EVTCHN_WORD(d); i++ )
> + set_bit(i, &vcpu_info(v, evtchn_pending_sel));
> + }
> }
>
> long do_vcpu_op(int cmd, int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
> @@ -1116,6 +1143,36 @@ long do_vcpu_op(int cmd, int vcpuid,
> XEN_GUEST_HANDLE_PARAM(void) arg)
> break;
> }
>
> + case VCPUOP_reset_vcpu_info:
> + {
> +
> + if ( (v != current) && !test_bit(_VPF_down, &v->pause_flags) )
> + {
> + printk(XENLOG_G_WARNING "%pv: VCPU is up or not current,"
> + "refusing to reset vcpu_info!\n", v);
> + return -EBUSY;
> + }
> +
> + if ( v->evtchn_fifo )
> + {
> + printk(XENLOG_G_WARNING "%pv: FIFO evtchn ABI is being used,"
> + "refusing to reset vcpu_info!\n", v);
> + return -EBUSY;
> + }
These two checks should be in unmap_vcpu_info() similar to
map_vcpu_info(), and unmap_vcpu_info() should return an int. I am not
sure whether the printks are really needed; a buggy/malicious guest is
very capable of causing logspam with them.
> +
> + if ( v->vcpu_info_mfn == INVALID_MFN )
> + {
> + rc = 0;
> + break;
> + }
This check is not safe without the domain lock held. Drop it entirely,
as the head of unmap_vcpu_info() already does it.
~Andrew
> +
> + domain_lock(d);
> + unmap_vcpu_info(v);
> + domain_unlock(d);
> + rc = 0;
> + break;
> + }
> +
> case VCPUOP_register_runstate_memory_area:
> {
> struct vcpu_register_runstate_memory_area area;
> diff --git a/xen/include/public/vcpu.h b/xen/include/public/vcpu.h
> index e888daf..c4d166c 100644
> --- a/xen/include/public/vcpu.h
> +++ b/xen/include/public/vcpu.h
> @@ -227,6 +227,25 @@ struct vcpu_register_time_memory_area {
> typedef struct vcpu_register_time_memory_area
> vcpu_register_time_memory_area_t;
> DEFINE_XEN_GUEST_HANDLE(vcpu_register_time_memory_area_t);
>
> +/*
> + * Reset all of the vcpu_info information from their previous location
> + * to the default one used at bootup. The following prerequisites should be
> met:
> + * 1. Domain should be using 2-level event channel ABI. In case another
> + * non-default ABI is in use the domain is supposed to switch back to
> + * 2-level ABI with EVTCHNOP_reset.
> + * 2. The operation is unsupported for non-current online VCPUs. If
> performed
> + * during shutdown/kexec/... a guest domain is supposed to behave in the
> + * following sequence:
> + * - Choose special 'shutdown' VCPU, bring all other VCPUs down;
> + * - Issue VCPUOP_reset_vcpu_info for all offline VCPUs;
> + * - Issue VCPUOP_reset_vcpu_info for the 'shutdown' VCPU.
> + *
> + * After the call vcpu_info is reset to its default state: first
> + * XEN_LEGACY_MAX_VCPUS VCPUs will get it switched to shared_info and all
> other
> + * VCPUs will get dummy_vcpu_info.
> + */
> +#define VCPUOP_reset_vcpu_info 14
> +
> #endif /* __XEN_PUBLIC_VCPU_H__ */
>
> /*
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |