Re: [Xen-devel] [PATCH 3/4] x86/domctl: Remove PV MSR parts of XEN_DOMCTL_[gs]et_ext_vcpucontext

[Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>]

On Wed, 2014-06-04 at 18:26 +0100, Andrew Cooper wrote:
> The PV MSR functionality is now implemented as a separate set of domctls.
> 
> This is a revert of parts of c/s65e3554908
>   "x86/PV: support data breakpoint extension registers"
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: Keir Fraser <keir@xxxxxxx>
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> ---
>  xen/arch/x86/domctl.c       |   78 
> +------------------------------------------
>  xen/include/public/domctl.h |   19 -----------
>  2 files changed, 1 insertion(+), 96 deletions(-)
> 
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index c0c2d1b..8f5b287 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -818,7 +818,6 @@ long arch_do_domctl(
>      {
>          struct xen_domctl_ext_vcpucontext *evc;
>          struct vcpu *v;
> -        struct xen_domctl_ext_vcpu_msr msr;
>  
>          evc = &domctl->u.ext_vcpucontext;
>  
> @@ -864,42 +863,7 @@ long arch_do_domctl(
>              evc->vmce.mci_ctl2_bank0 = v->arch.vmce.bank[0].mci_ctl2;
>              evc->vmce.mci_ctl2_bank1 = v->arch.vmce.bank[1].mci_ctl2;
>  
> -            i = ret = 0;
> -            if ( boot_cpu_has(X86_FEATURE_DBEXT) )
> -            {
> -                unsigned int j;
> -
> -                if ( v->arch.pv_vcpu.dr_mask[0] )
> -                {
> -                    if ( i < evc->msr_count && !ret )
> -                    {
> -                        msr.index = MSR_AMD64_DR0_ADDRESS_MASK;
> -                        msr.reserved = 0;
> -                        msr.value = v->arch.pv_vcpu.dr_mask[0];
> -                        if ( copy_to_guest_offset(evc->msrs, i, &msr, 1) )
> -                            ret = -EFAULT;
> -                    }
> -                    ++i;
> -                }
> -                for ( j = 0; j < 3; ++j )
> -                {
> -                    if ( !v->arch.pv_vcpu.dr_mask[1 + j] )
> -                        continue;
> -                    if ( i < evc->msr_count && !ret )
> -                    {
> -                        msr.index = MSR_AMD64_DR1_ADDRESS_MASK + j;
> -                        msr.reserved = 0;
> -                        msr.value = v->arch.pv_vcpu.dr_mask[1 + j];
> -                        if ( copy_to_guest_offset(evc->msrs, i, &msr, 1) )
> -                            ret = -EFAULT;
> -                    }
> -                    ++i;
> -                }
> -            }
> -            if ( i > evc->msr_count && !ret )
> -                ret = -ENOBUFS;
> -            evc->msr_count = i;
> -
> +            ret = 0;
>              vcpu_unpause(v);
>              copyback = 1;
>          }
> @@ -954,49 +918,9 @@ long arch_do_domctl(
>  
>                  ret = vmce_restore_vcpu(v, &vmce);
>              }
> -            else if ( evc->size > offsetof(typeof(*evc), vmce) )
> -                ret = -EINVAL;
>              else
>                  ret = 0;
>  
> -            if ( ret || evc->size <= offsetof(typeof(*evc), msrs) )
> -                /* nothing */;
> -            else if ( evc->size < offsetof(typeof(*evc), msrs) +
> -                                  sizeof(evc->msrs) )
> -                ret = -EINVAL;
> -            else
> -            {
> -                for ( i = 0; i < evc->msr_count; ++i )
> -                {
> -                    ret = -EFAULT;
> -                    if ( copy_from_guest_offset(&msr, evc->msrs, i, 1) )
> -                        break;
> -                    ret = -EINVAL;
> -                    if ( msr.reserved )
> -                        break;
> -                    switch ( msr.index )
> -                    {
> -                    case MSR_AMD64_DR0_ADDRESS_MASK:
> -                        if ( !boot_cpu_has(X86_FEATURE_DBEXT) ||
> -                             (msr.value >> 32) )
> -                            break;
> -                        v->arch.pv_vcpu.dr_mask[0] = msr.value;
> -                        continue;
> -                    case MSR_AMD64_DR1_ADDRESS_MASK ...
> -                         MSR_AMD64_DR3_ADDRESS_MASK:
> -                        if ( !boot_cpu_has(X86_FEATURE_DBEXT) ||
> -                             (msr.value >> 32) )
> -                            break;
> -                        msr.index -= MSR_AMD64_DR1_ADDRESS_MASK - 1;
> -                        v->arch.pv_vcpu.dr_mask[msr.index] = msr.value;
> -                        continue;
> -                    }
> -                    break;
> -                }
> -                if ( i == evc->msr_count )
> -                    ret = 0;
> -            }
> -
>              domain_unpause(d);
>          }
>      }
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 7a13e25..4321a64 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -543,16 +543,6 @@ typedef struct xen_domctl_pin_mem_cacheattr 
> xen_domctl_pin_mem_cacheattr_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_pin_mem_cacheattr_t);
>  
> 
> -#if defined(__i386__) || defined(__x86_64__)
> -struct xen_domctl_ext_vcpu_msr {
> -    uint32_t         index;
> -    uint32_t         reserved;
> -    uint64_aligned_t value;
> -};
> -typedef struct xen_domctl_ext_vcpu_msr xen_domctl_ext_vcpu_msr_t;
> -DEFINE_XEN_GUEST_HANDLE(xen_domctl_ext_vcpu_msr_t);
> -#endif
> -
>  /* XEN_DOMCTL_set_ext_vcpucontext */
>  /* XEN_DOMCTL_get_ext_vcpucontext */
>  struct xen_domctl_ext_vcpucontext {
> @@ -572,14 +562,6 @@ struct xen_domctl_ext_vcpucontext {
>      uint16_t         sysenter_callback_cs;
>      uint8_t          syscall32_disables_events;
>      uint8_t          sysenter_disables_events;
> -    /*
> -     * When, for the "get" version, msr_count is too small to cover all MSRs
> -     * the hypervisor needs to be saved, the call will return -ENOBUFS and
> -     * set msr_count to the required (minimum) value. Furthermore, for both
> -     * "get" and "set", that field as well as the msrs one only get looked at
> -     * if the size field above covers the structure up to the entire msrs 
> one.
> -     */
> -    uint16_t         msr_count;
>  #if defined(__GNUC__)
>      union {
>          uint64_aligned_t mcg_cap;
> @@ -588,7 +570,6 @@ struct xen_domctl_ext_vcpucontext {
>  #else
>      struct hvm_vmce_vcpu vmce;
>  #endif
> -    XEN_GUEST_HANDLE_64(xen_domctl_ext_vcpu_msr_t) msrs;
>  #endif
>  };
>  typedef struct xen_domctl_ext_vcpucontext xen_domctl_ext_vcpucontext_t;

This is an ABI change, you should bump XEN_DOMCTL_INTERFACE_VERSION too
(if not already done, if done there should be a comment in the header
stating when is bumped).

Frediano



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel