Re: [Xen-devel] [PATCH 8/9] x86/vmx: Support removing MSRs from the host/guest load/save lists

2018-05-24 Thread Andrew Cooper
On 24/05/18 16:42, Roger Pau Monné wrote:
> On Tue, May 22, 2018 at 12:20:45PM +0100, Andrew Cooper wrote:
>> Up until this point, the MSR load/save lists have only ever accumulated
>> content.  Introduce vmx_del_msr() as a companion to vmx_add_msr().
>>
>> Signed-off-by: Andrew Cooper 
>> ---
>> CC: Jan Beulich 
>> CC: Jun Nakajima 
>> CC: Kevin Tian 
>> CC: Wei Liu 
>> CC: Roger Pau Monné 
>> ---
>>  xen/arch/x86/hvm/vmx/vmcs.c| 68 
>> ++
>>  xen/include/asm-x86/hvm/vmx/vmcs.h |  1 +
>>  2 files changed, 69 insertions(+)
>>
>> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
>> index 7bf19a0..e1a8f95 100644
>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>> @@ -1465,6 +1465,74 @@ int vmx_add_msr(struct vcpu *v, uint32_t msr, 
>> uint64_t val,
>>  return rc;
>>  }
>>  
>> +int vmx_del_msr(struct vcpu *v, uint32_t msr, enum vmx_msr_list_type type)
>> +{
>> +struct arch_vmx_struct *arch_vmx = &v->arch.hvm_vmx;
>> +struct vmx_msr_entry *start = NULL, *ent, *end;
>> +unsigned int substart, subend, total;
>> +
>> +ASSERT(v == current || !vcpu_runnable(v));
>> +
>> +switch ( type )
>> +{
>> +case VMX_MSR_HOST:
>> +start= arch_vmx->host_msr_area;
>> +substart = 0;
>> +subend   = arch_vmx->host_msr_count;
>> +total= subend;
>> +break;
>> +
>> +case VMX_MSR_GUEST:
>> +start= arch_vmx->msr_area;
>> +substart = 0;
>> +subend   = arch_vmx->msr_save_count;
>> +total= arch_vmx->msr_load_count;
>> +break;
>> +
>> +case VMX_MSR_GUEST_LOADONLY:
>> +start= arch_vmx->msr_area;
>> +substart = arch_vmx->msr_save_count;
>> +subend   = arch_vmx->msr_load_count;
>> +total= subend;
>> +break;
>> +
>> +default:
>> +ASSERT_UNREACHABLE();
>> +}
> The above chunk is already in vmx_find_msr and vmx_add_msr, maybe a
> static helper that sets start/substart/subend/total would be helpful
> here?

Its actually more than that.  They are identical identical until after
the locate_msr_entry() call.  The problem is that I can't find a clean
way of collecting the logic which is also readable.

I'd also like to fold together the add side, but that is further
complicated by the memory allocation logic.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 8/9] x86/vmx: Support removing MSRs from the host/guest load/save lists

2018-05-24 Thread Roger Pau Monné
On Tue, May 22, 2018 at 12:20:45PM +0100, Andrew Cooper wrote:
> Up until this point, the MSR load/save lists have only ever accumulated
> content.  Introduce vmx_del_msr() as a companion to vmx_add_msr().
> 
> Signed-off-by: Andrew Cooper 
> ---
> CC: Jan Beulich 
> CC: Jun Nakajima 
> CC: Kevin Tian 
> CC: Wei Liu 
> CC: Roger Pau Monné 
> ---
>  xen/arch/x86/hvm/vmx/vmcs.c| 68 
> ++
>  xen/include/asm-x86/hvm/vmx/vmcs.h |  1 +
>  2 files changed, 69 insertions(+)
> 
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index 7bf19a0..e1a8f95 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -1465,6 +1465,74 @@ int vmx_add_msr(struct vcpu *v, uint32_t msr, uint64_t 
> val,
>  return rc;
>  }
>  
> +int vmx_del_msr(struct vcpu *v, uint32_t msr, enum vmx_msr_list_type type)
> +{
> +struct arch_vmx_struct *arch_vmx = &v->arch.hvm_vmx;
> +struct vmx_msr_entry *start = NULL, *ent, *end;
> +unsigned int substart, subend, total;
> +
> +ASSERT(v == current || !vcpu_runnable(v));
> +
> +switch ( type )
> +{
> +case VMX_MSR_HOST:
> +start= arch_vmx->host_msr_area;
> +substart = 0;
> +subend   = arch_vmx->host_msr_count;
> +total= subend;
> +break;
> +
> +case VMX_MSR_GUEST:
> +start= arch_vmx->msr_area;
> +substart = 0;
> +subend   = arch_vmx->msr_save_count;
> +total= arch_vmx->msr_load_count;
> +break;
> +
> +case VMX_MSR_GUEST_LOADONLY:
> +start= arch_vmx->msr_area;
> +substart = arch_vmx->msr_save_count;
> +subend   = arch_vmx->msr_load_count;
> +total= subend;
> +break;
> +
> +default:
> +ASSERT_UNREACHABLE();
> +}

The above chunk is already in vmx_find_msr and vmx_add_msr, maybe a
static helper that sets start/substart/subend/total would be helpful
here?

Roger.

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel