From mboxrd@z Thu Jan  1 00:00:00 1970
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH v3 12/16] x86/VPMU: Handle PMU interrupts
	for PV guests
Date: Thu, 16 Jan 2014 09:58:25 -0500
Message-ID: <52D7F391.60500@oracle.com>
References: <1389036295-3877-1-git-send-email-boris.ostrovsky@oracle.com>
	<1389036295-3877-13-git-send-email-boris.ostrovsky@oracle.com>
	<52D7B158020000780011427E@nat28.tlf.novell.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>
Received: from mail6.bemta3.messagelabs.com ([195.245.230.39])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <boris.ostrovsky@oracle.com>) id 1W3oOS-0005iL-3s
	for xen-devel@lists.xenproject.org; Thu, 16 Jan 2014 14:58:04 +0000
In-Reply-To: <52D7B158020000780011427E@nat28.tlf.novell.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: Jan Beulich <JBeulich@suse.com>
Cc: keir@xen.org, suravee.suthikulpanit@amd.com, andrew.cooper3@citrix.com, eddie.dong@intel.com, dietmar.hahn@ts.fujitsu.com, jun.nakajima@intel.com, xen-devel <xen-devel@lists.xenproject.org>
List-Id: xen-devel@lists.xenproject.org

On 01/16/2014 04:15 AM, Jan Beulich wrote:
>> @@ -82,7 +87,23 @@ int vpmu_do_wrmsr(unsigned int msr, uint64_t msr_content)
>> ...
>> +        {
>> +            vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
>> +            (void)vpmu->arch_vpmu_ops->arch_vpmu_save(current);
> What's this cast good for?

These routines return an int that indicates whether this was full 
context save or just that the counters have been stopped. Depending on 
where the routines are called from we may ignore the return value 
(having VPMU_CONTEXT_SAVE forces "full save").

I did it so that compiler won't complain about ignoring return value (I 
don't think it does now but I imagine some version still may).

>> +            vpmu_reset(vpmu, VPMU_CONTEXT_SAVE | VPMU_CONTEXT_LOADED);
>> +        }
>> +        return val;
>> +    }
>>       return 0;
>>   }
>>   
...
>> +    /* dom0 will handle this interrupt */
>> +    if ( v->domain->domain_id >= DOMID_FIRST_RESERVED )
>> +    {
>> +        if ( smp_processor_id() >= dom0->max_vcpus )
>> +            return 0;
>> +        v = dom0->vcpu[smp_processor_id()];
>> +    }
> Ugly new uses of "dom0". And the correlation between
> smp_processor_id() and dom0->max_vcpus doesn't look sane
> either.

Yes, this needs to be fixed. Maybe

     v = dom0->vcpu[smp_processor_id() % dom0->max_vcpus];

And drop the check. (And the same change in pmu_softnmi() in a later patch).

Not sure what to do about dom0. What else can I use instead? I need to 
choose dom0's vcpu.


Thanks.
-boris