From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <JBeulich@suse.com>
Subject: Re: [PATCH v5 2/6] x86: allow reading MSR_IA32_TSC with
 XENPF_resource_op
Date: Fri, 23 Jan 2015 08:58:32 +0000
Message-ID: <54C21B48020000780005894A@mail.emea.novell.com>
References: <1421839164-26037-1-git-send-email-chao.p.peng@linux.intel.com>
	<1421839164-26037-3-git-send-email-chao.p.peng@linux.intel.com>
	<54C0EAFF0200007800058100@mail.emea.novell.com>
	<20150122125314.GD28428@pengc-linux.bj.intel.com>
	<54C10B05020000780005824E@mail.emea.novell.com>
	<54C10320.8090107@citrix.com>
	<54C112B102000078000582FC@mail.emea.novell.com>
	<20150123084206.GE28428@pengc-linux.bj.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <20150123084206.GE28428@pengc-linux.bj.intel.com>
Content-Disposition: inline
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: Chao Peng <chao.p.peng@linux.intel.com>
Cc: wei.liu2@citrix.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, Andrew Cooper <andrew.cooper3@citrix.com>, Ian.Jackson@eu.citrix.com, xen-devel@lists.xen.org, will.auld@intel.com, keir@xen.org
List-Id: xen-devel@lists.xenproject.org

>>> On 23.01.15 at 09:42, <chao.p.peng@linux.intel.com> wrote:
> Except that MSR_IA32_TSC will looks heterogeneous. But since we already
> treat it as a special case, I have no problem here. And if we do so,
> even the first patch to add irq_disable ability is not needed. Sounds to
> me that the MSR_IA32_TSC read should always imply irq disabled. Codes
> would be like this:

Looks reasonable, minus some formatting issues and ...

>         case XEN_RESOURCE_OP_MSR_READ:
>             if ( unlikely(entry->idx == MSR_IA32_TSC) )
>             { 
>                 entry->val = get_s_time_fixed(tsc);
>                 ret = 0;
>             }   
>             else
>             {
>                 bool_t  read_tsc = 0;
>                 if ( i < ra->nr_done - 1 ) 
>                 {   
>                     xenpf_resource_entry_t *next = ra->entries + i + 1;
>                     if ( unlikely(next->idx == MSR_IA32_TSC) )

... this simply being entry[1].idx.

Jan

>                         read_tsc = 1;
>                 }
> 
>                 if ( unlikely(read_tsc) )
>                     local_irq_save(irqflags);
> 
>                 ret = rdmsr_safe(entry->idx, entry->val);
> 
>                 if ( unlikely(read_tsc) )
>                 {
>                     rdtscll(tsc);
>                     local_irq_restore(irqflags);
>                 }
>             }
>             break;
>         case XEN_RESOURCE_OP_MSR_WRITE:
> 
> Chao