From mboxrd@z Thu Jan  1 00:00:00 1970
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH v11 for-xen-4.5 16/20] x86/VPMU: Handle PMU
 interrupts for PV guests
Date: Tue, 23 Sep 2014 13:36:41 -0400
Message-ID: <5421AFA9.1010106@oracle.com>
References: <1411430281-6132-1-git-send-email-boris.ostrovsky@oracle.com>
	<1411430281-6132-17-git-send-email-boris.ostrovsky@oracle.com>
	<20140923171819.GP3007@laptop.dumpdata.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <20140923171819.GP3007@laptop.dumpdata.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: kevin.tian@intel.com, keir@xen.org, jbeulich@suse.com, jun.nakajima@intel.com, andrew.cooper3@citrix.com, tim@xen.org, dietmar.hahn@ts.fujitsu.com, xen-devel@lists.xen.org, Aravind.Gopalakrishnan@amd.com, suravee.suthikulpanit@amd.com
List-Id: xen-devel@lists.xenproject.org

On 09/23/2014 01:18 PM, Konrad Rzeszutek Wilk wrote:
>
>   int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content, uint64_t supported)
>   {
> -    struct vpmu_struct *vpmu = vcpu_vpmu(current);
> +    struct vcpu *curr = current;
> +    struct vpmu_struct *vpmu = vcpu_vpmu(curr);
>   
>       if ( !(vpmu_mode & (XENPMU_MODE_SELF | XENPMU_MODE_HV)) )
>           return 0;
>   
>       if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_wrmsr )
> -        return vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content, supported);
> +    {
> +        int ret = vpmu->arch_vpmu_ops->do_wrmsr(msr, msr_content, supported);
> +
> +        /*
> +         * We may have received a PMU interrupt during WRMSR handling
> +         * and since do_wrmsr may load VPMU context we should save
> +         * (and unload) it again.
> +         */
> +        if ( !is_hvm_domain(curr->domain) &&
> +             vpmu->xenpmu_data && (vpmu->xenpmu_data->pmu_flags & PMU_CACHED) )
> +        {
> +            vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
> +            vpmu->arch_vpmu_ops->arch_vpmu_save(curr);
> +            vpmu_reset(vpmu, VPMU_CONTEXT_SAVE | VPMU_CONTEXT_LOADED);
> +        }
> +        return ret;
> +    }
>       return 0;
>   }
>   
>   int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content)
>   {
> -    struct vpmu_struct *vpmu = vcpu_vpmu(current);
> +    struct vcpu *curr = current;
> +    struct vpmu_struct *vpmu = vcpu_vpmu(curr);
>   
>       if ( !(vpmu_mode & (XENPMU_MODE_SELF | XENPMU_MODE_HV)) )
>           return 0;
>   
>       if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->do_rdmsr )
> -        return vpmu->arch_vpmu_ops->do_rdmsr(msr, msr_content);
> +    {
> You have a nice comment in the above code. Could you replicate it
> here or just point the reader of the code to the reasoning?

The next patch will merge these two routines and the comment will be 
part of the new routine that combines them.

-boris

>
>> +        int ret = vpmu->arch_vpmu_ops->do_rdmsr(msr, msr_content);
>> +
>> +        if ( !is_hvm_domain(curr->domain) &&
>> +             vpmu->xenpmu_data && (vpmu->xenpmu_data->pmu_flags & PMU_CACHED) )
>> +        {
>> +            vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
>> +            vpmu->arch_vpmu_ops->arch_vpmu_save(curr);
>> +            vpmu_reset(vpmu, VPMU_CONTEXT_SAVE | VPMU_CONTEXT_LOADED);
>> +        }
>> +        return ret;
>> +    }
>>       return 0;
>>   }
>>