Xen devel - Sep 2009 - [PATCH] x86 hvm: freeze PIT/LAPIC timer emulation while its IRQ is masked

Hi,

I''ve found that modern windows OS never use the PIT timer,
and neither cpu#0''s LAPIC timer after boot.
Despite that, xen emulates them busily. It''s inefficient.

Note: this patch ignores the IRQ mask of legacy i8259 since
rombios frequently modifies it.

Thanks,
Kouya

Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Although I can see the sense here, the patch makes me a bit uneasy. Also I
think neater code would result from simply making vpt.c''s handling of
periodic timers less stupid. By this I mean that calling set_timer() from
the timer handler is not really necessary -- you already know an interrupt
is now pending and the vcpu is kicked. May as well not set_timer() until the
pending interrupt(s) are delivered. That would then effectively get your
optimisation "for free", wouldn''t it?

 -- Keir

On 10/09/2009 06:47, "Kouya Shimura" <kouya@jp.fujitsu.com>
wrote:
> Hi,
> 
> I''ve found that modern windows OS never use the PIT timer,
> and neither cpu#0''s LAPIC timer after boot.
> Despite that, xen emulates them busily. It''s inefficient.
> 
> Note: this patch ignores the IRQ mask of legacy i8259 since
> rombios frequently modifies it.
> 
> Thanks,
> Kouya
> 
> Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Well, this patch has two aims.  One is to stop the xen''s timer by
stop_timer(). And another is to simplify pt_update_irq() which
searches the earliest platform timer.

To answer the latter aim, this patch gets rid of unused (masked
by IRQ) platform timer from vcpu->arch.hvm_vcpu.tm_list.
Your suggestion satisfies the first but the latter, I think.
Besides, pt_update_irq() is more critical since it is called on
every VM_EXIT.

Although I understand your uneasiness, is there any other way?

Thanks,
Kouya

Keir Fraser writes:
> Although I can see the sense here, the patch makes me a bit uneasy. Also I
> think neater code would result from simply making vpt.c''s handling
of
> periodic timers less stupid. By this I mean that calling set_timer() from
> the timer handler is not really necessary -- you already know an interrupt
> is now pending and the vcpu is kicked. May as well not set_timer() until
the
> pending interrupt(s) are delivered. That would then effectively get your
> optimisation "for free", wouldn''t it?
> 
>  -- Keir
> 
> On 10/09/2009 06:47, "Kouya Shimura" <kouya@jp.fujitsu.com>
wrote:
> 
> > Hi,
> > 
> > I''ve found that modern windows OS never use the PIT timer,
> > and neither cpu#0''s LAPIC timer after boot.
> > Despite that, xen emulates them busily. It''s inefficient.
> > 
> > Note: this patch ignores the IRQ mask of legacy i8259 since
> > rombios frequently modifies it.
> > 
> > Thanks,
> > Kouya
> > 
> > Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>
> > 
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Do you get a measurable performance win from this patch?

Not using pt_irq_masked() seems odd. Is it to avoid 859 IRQ mask checks, and
if so why are you doing that? Why do we care that rombios twiddles the 8259
masks a bunch during boot?

 -- Keir

On 10/09/2009 09:22, "Kouya Shimura" <kouya@jp.fujitsu.com>
wrote:
> Well, this patch has two aims.  One is to stop the xen''s timer by
> stop_timer(). And another is to simplify pt_update_irq() which
> searches the earliest platform timer.
> 
> To answer the latter aim, this patch gets rid of unused (masked
> by IRQ) platform timer from vcpu->arch.hvm_vcpu.tm_list.
> Your suggestion satisfies the first but the latter, I think.
> Besides, pt_update_irq() is more critical since it is called on
> every VM_EXIT.
> 
> Although I understand your uneasiness, is there any other way?
> 
> Thanks,
> Kouya
> 
> Keir Fraser writes:
>> Although I can see the sense here, the patch makes me a bit uneasy.
Also I
>> think neater code would result from simply making vpt.c''s
handling of
>> periodic timers less stupid. By this I mean that calling set_timer()
from
>> the timer handler is not really necessary -- you already know an
interrupt
>> is now pending and the vcpu is kicked. May as well not set_timer()
until the
>> pending interrupt(s) are delivered. That would then effectively get
your
>> optimisation "for free", wouldn''t it?
>> 
>>  -- Keir
>> 
>> On 10/09/2009 06:47, "Kouya Shimura"
<kouya@jp.fujitsu.com> wrote:
>> 
>>> Hi,
>>> 
>>> I''ve found that modern windows OS never use the PIT timer,
>>> and neither cpu#0''s LAPIC timer after boot.
>>> Despite that, xen emulates them busily. It''s inefficient.
>>> 
>>> Note: this patch ignores the IRQ mask of legacy i8259 since
>>> rombios frequently modifies it.
>>> 
>>> Thanks,
>>> Kouya
>>> 
>>> Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>
>>> 
>> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Keir Fraser writes:
> Do you get a measurable performance win from this patch?
Sorry, I don''t measure yet. Probably a little bit win.
> Not using pt_irq_masked() seems odd. Is it to avoid 859 IRQ mask checks,
and
> if so why are you doing that? Why do we care that rombios twiddles the 8259
> masks a bunch during boot?
I guessed that the cost of freezing/thawing a timer with the 8259 mask
is higher than timer emulation.  And the PIT timer is actually active
while rombios masks the 8259.

If using pt_irq_masked() is desirable, I''ll remake it.

Thanks,
Kouya
> 
>  -- Keir
> 
> On 10/09/2009 09:22, "Kouya Shimura" <kouya@jp.fujitsu.com>
wrote:
> 
> > Well, this patch has two aims.  One is to stop the xen''s
timer by
> > stop_timer(). And another is to simplify pt_update_irq() which
> > searches the earliest platform timer.
> > 
> > To answer the latter aim, this patch gets rid of unused (masked
> > by IRQ) platform timer from vcpu->arch.hvm_vcpu.tm_list.
> > Your suggestion satisfies the first but the latter, I think.
> > Besides, pt_update_irq() is more critical since it is called on
> > every VM_EXIT.
> > 
> > Although I understand your uneasiness, is there any other way?
> > 
> > Thanks,
> > Kouya
> > 
> > Keir Fraser writes:
> >> Although I can see the sense here, the patch makes me a bit
uneasy. Also I
> >> think neater code would result from simply making vpt.c''s
handling of
> >> periodic timers less stupid. By this I mean that calling
set_timer() from
> >> the timer handler is not really necessary -- you already know an
interrupt
> >> is now pending and the vcpu is kicked. May as well not set_timer()
until the
> >> pending interrupt(s) are delivered. That would then effectively
get your
> >> optimisation "for free", wouldn''t it?
> >> 
> >>  -- Keir
> >> 
> >> On 10/09/2009 06:47, "Kouya Shimura"
<kouya@jp.fujitsu.com> wrote:
> >> 
> >>> Hi,
> >>> 
> >>> I''ve found that modern windows OS never use the PIT
timer,
> >>> and neither cpu#0''s LAPIC timer after boot.
> >>> Despite that, xen emulates them busily. It''s
inefficient.
> >>> 
> >>> Note: this patch ignores the IRQ mask of legacy i8259 since
> >>> rombios frequently modifies it.
> >>> 
> >>> Thanks,
> >>> Kouya
> >>> 
> >>> Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>
> >>> 
> >> 
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 10/09/2009 10:54, "Kouya Shimura" <kouya@jp.fujitsu.com>
wrote:
>> Not using pt_irq_masked() seems odd. Is it to avoid 859 IRQ mask
checks, and
>> if so why are you doing that? Why do we care that rombios twiddles the
8259
>> masks a bunch during boot?
> 
> I guessed that the cost of freezing/thawing a timer with the 8259 mask
> is higher than timer emulation.  And the PIT timer is actually active
> while rombios masks the 8259.
> 
> If using pt_irq_masked() is desirable, I''ll remake it.
If your patch works without even any theoretical lost of correctness, then
that makes it more attractive, obviously.

Some evidence that the cost saving is at all measurable would also be good.
Premature optimisation being evil, etc. ;-)

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Thu, Sep 10, 2009 at 02:47:22PM +0900, Kouya Shimura wrote:
Content-Description: message body text
> Hi,
> 
> I''ve found that modern windows OS never use the PIT timer,
What about older Linux versions? Like RHEL3?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Konrad Rzeszutek Wilk writes:
> On Thu, Sep 10, 2009 at 02:47:22PM +0900, Kouya Shimura wrote:
> Content-Description: message body text
> > Hi,
> > 
> > I''ve found that modern windows OS never use the PIT timer,
> 
> What about older Linux versions? Like RHEL3?
older Linux uses the PIT timer. Although I don''t know about RHEL3,
RHEL5 uses the PIT timer actually.

Any way, my patch doesn''t give a benefit to Linux.  For example,
linux-2.6.28 (ubuntu 9.04) stops the PIT timer by itself.

Thanks,
Kouya

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

I remade the patch and measured the performance win. Attached is
a benchmark program which I wrote. It is complied by cygwin''s gcc
by -O2 and runs on Windows XP(32bit). And my cpu is 
Intel Core2 Quad Q9450@2.66GHz.

The result is that my patch saves 32 cycles(TSC) per one VM_EXIT(cpuid).
(2696 tsc => 2664 tsc)

The patch is split to two. I''ll post them in another mail.

Thanks,
Kouya

Keir Fraser writes:
> On 10/09/2009 10:54, "Kouya Shimura" <kouya@jp.fujitsu.com>
wrote:
> 
> >> Not using pt_irq_masked() seems odd. Is it to avoid 859 IRQ mask
checks, and
> >> if so why are you doing that? Why do we care that rombios twiddles
the 8259
> >> masks a bunch during boot?
> > 
> > I guessed that the cost of freezing/thawing a timer with the 8259 mask
> > is higher than timer emulation.  And the PIT timer is actually active
> > while rombios masks the 8259.
> > 
> > If using pt_irq_masked() is desirable, I''ll remake it.
> 
> If your patch works without even any theoretical lost of correctness, then
> that makes it more attractive, obviously.
> 
> Some evidence that the cost saving is at all measurable would also be good.
> Premature optimisation being evil, etc. ;-)
> 
>  -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Modern Windows OS (ex XP,2003,2008) never use the PIT timer,
and neither cpu#0''s LAPIC timer after boot.
Despite that, xen emulates them busily. It''s inefficient.

With this patch, setting a timer is defered while its IRQ is masked.

The reasons why pt_timer_fn() simply calls vcpu_kick() are:
- checking by pt_irq_masked() is duplicated. pt_update_irq() also does.
- pt_timer_fn() is likely called on the same processor
  as pt->vcpu->processor. Hence vcpu_kick() hardly send IPI.

Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

This patch gets rid of a timer which IRQ is masked from vcpu''s timer
list.
It reduces the overhead of VM EXIT and context switch of vm.

Also fixes a potential bug.
(1) VCPU#0: mask the IRQ of a timer. (ex. vioapic.redir[2].mask=1)
(2) VCPU#1: pt_timer_fn() is invoked by expiration of the timer.
(3) VCPU#1: pt_update_irq() is called but does nothing by pt_irq_masked()=1.
(4) VCPU#1: sleep by halt.
(5) VCPU#0: unmask the IRQ of the timer.
After that, no one wakes up the VCPU#1.

IRQ of ISA is masked by:
 - PIC''s IMR
 - IOAPIC''s redir[0]
 - IOAPIC''s redir[N].mask
 - LAPIC''s LVT0
 - LAPIC enabled/disabled

IRQ of LAPIC timer is masked by:
 - LAPIC''s LVTT
 - LAPIC disabled

When above stuffs are changed, the corresponding vcpu is kicked and
suspended timer emulation is resumed.

In addition, a small bug fix in pt_adjust_global_vcpu_target().

Signed-off-by: Kouya Shimura <kouya@jp.fujitsu.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 16/09/2009 07:46, "Kouya Shimura" <kouya@jp.fujitsu.com>
wrote:
> I remade the patch and measured the performance win. Attached is
> a benchmark program which I wrote. It is complied by cygwin''s gcc
> by -O2 and runs on Windows XP(32bit). And my cpu is
> Intel Core2 Quad Q9450@2.66GHz.
> 
> The result is that my patch saves 32 cycles(TSC) per one VM_EXIT(cpuid).
> (2696 tsc => 2664 tsc)
> 
> The patch is split to two. I''ll post them in another mail.
That''s really not enough of a win to bother with, is it.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Looks the little win doesn''t deserve the increased complexity in code.

BTW, recent Intel CPUs run much faster with respect to VMEntry/VMExit and
VMREAD/VMWRITE, so I don''t think the SW optimizatin is appearling here.
:-)

Thanks,
-- Dexuan



-----Original Message-----
From: xen-devel-bounces@lists.xensource.com
[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser
Sent: 2009?9?16? 15:41
To: Kouya Shimura
Cc: xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] [PATCH] x86 hvm: freeze PIT/LAPIC timer emulation while
its IRQ is masked

On 16/09/2009 07:46, "Kouya Shimura" <kouya@jp.fujitsu.com>
wrote:
> I remade the patch and measured the performance win. Attached is
> a benchmark program which I wrote. It is complied by cygwin''s gcc
> by -O2 and runs on Windows XP(32bit). And my cpu is
> Intel Core2 Quad Q9450@2.66GHz.
> 
> The result is that my patch saves 32 cycles(TSC) per one VM_EXIT(cpuid).
> (2696 tsc => 2664 tsc)
> 
> The patch is split to two. I''ll post them in another mail.
That''s really not enough of a win to bother with, is it.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 16/09/2009 09:00, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> Looks the little win doesn''t deserve the increased complexity in
code.
> 
> BTW, recent Intel CPUs run much faster with respect to VMEntry/VMExit and
> VMREAD/VMWRITE, so I don''t think the SW optimizatin is appearling
here. :-)
Makes it more appealing doesn''t it? As this path will be a greater
proportion of an overall vmexit/vmenter cycle.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Cui, Dexuan writes:
> Looks the little win doesn''t deserve the increased complexity in
code.
Agreed. I don''t strongly push my patches. 
But this win is given by 2/2 patch and it fixes a certain bug.
Besides, 1/2 patch simplifies the code, I think.
> BTW, recent Intel CPUs run much faster with respect to VMEntry/VMExit and
VMREAD/VMWRITE, so I don''t think the SW optimizatin is appearling here.
:-)
I have a "Mottainai"(http://en.wikipedia.org/wiki/Mottainai) spirit.
:-)

Thanks,
Kouya
> 
> Thanks,
> -- Dexuan
> 
> 
> 
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com
[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser
> Sent: 2009?9?16? 15:41
> To: Kouya Shimura
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] x86 hvm: freeze PIT/LAPIC timer emulation
while its IRQ is masked
> 
> On 16/09/2009 07:46, "Kouya Shimura" <kouya@jp.fujitsu.com>
wrote:
> 
> > I remade the patch and measured the performance win. Attached is
> > a benchmark program which I wrote. It is complied by cygwin''s
gcc
> > by -O2 and runs on Windows XP(32bit). And my cpu is
> > Intel Core2 Quad Q9450@2.66GHz.
> > 
> > The result is that my patch saves 32 cycles(TSC) per one
VM_EXIT(cpuid).
> > (2696 tsc => 2664 tsc)
> > 
> > The patch is split to two. I''ll post them in another mail.
> 
> That''s really not enough of a win to bother with, is it.
> 
>  -- Keir
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

I liked both patches when I looked at them, so I checked them in and
we''ll
see how well they work!

 -- Keir

On 16/09/2009 09:28, "Kouya Shimura" <kouya@jp.fujitsu.com>
wrote:
> Cui, Dexuan writes:
>> Looks the little win doesn''t deserve the increased complexity
in code.
> 
> Agreed. I don''t strongly push my patches.
> But this win is given by 2/2 patch and it fixes a certain bug.
> Besides, 1/2 patch simplifies the code, I think.
> 
>> BTW, recent Intel CPUs run much faster with respect to VMEntry/VMExit
and
>> VMREAD/VMWRITE, so I don''t think the SW optimizatin is
appearling here. :-)
> 
> I have a "Mottainai"(http://en.wikipedia.org/wiki/Mottainai)
spirit. :-)
> 
> Thanks,
> Kouya
> 
>> 
>> Thanks,
>> -- Dexuan
>> 
>> 
>> 
>> -----Original Message-----
>> From: xen-devel-bounces@lists.xensource.com
>> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser
>> Sent: 2009?9?16? 15:41
>> To: Kouya Shimura
>> Cc: xen-devel@lists.xensource.com
>> Subject: Re: [Xen-devel] [PATCH] x86 hvm: freeze PIT/LAPIC timer
emulation
>> while its IRQ is masked
>> 
>> On 16/09/2009 07:46, "Kouya Shimura"
<kouya@jp.fujitsu.com> wrote:
>> 
>>> I remade the patch and measured the performance win. Attached is
>>> a benchmark program which I wrote. It is complied by
cygwin''s gcc
>>> by -O2 and runs on Windows XP(32bit). And my cpu is
>>> Intel Core2 Quad Q9450@2.66GHz.
>>> 
>>> The result is that my patch saves 32 cycles(TSC) per one
VM_EXIT(cpuid).
>>> (2696 tsc => 2664 tsc)
>>> 
>>> The patch is split to two. I''ll post them in another mail.
>> 
>> That''s really not enough of a win to bother with, is it.
>> 
>>  -- Keir
>> 
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

I''m not sure if the number of the saved cycles is the same on recent
Intel CPUs, so the proportion may not be greater.  :-)

Thanks,
-- Dexuan

-----Original Message-----
From: Keir Fraser [mailto:keir.fraser@eu.citrix.com] 
Sent: 2009?9?16? 16:21
To: Cui, Dexuan; Kouya Shimura
Cc: xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] [PATCH] x86 hvm: freeze PIT/LAPIC timer emulation while
its IRQ is masked

On 16/09/2009 09:00, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> Looks the little win doesn''t deserve the increased complexity in
code.
> 
> BTW, recent Intel CPUs run much faster with respect to VMEntry/VMExit and
> VMREAD/VMWRITE, so I don''t think the SW optimizatin is appearling
here. :-)
Makes it more appealing doesn''t it? As this path will be a greater
proportion of an overall vmexit/vmenter cycle.

 -- Keir
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Perhaps. Anyway, the patches fixed bugs and generally cleaned up some logic,
so I think they are good for reasons other than the few saved cycles on
vmenter.

 -- Keir

On 16/09/2009 09:40, "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> I''m not sure if the number of the saved cycles is the same on
recent Intel
> CPUs, so the proportion may not be greater.  :-)
> 
> Thanks,
> -- Dexuan
> 
> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
> Sent: 2009?9?16? 16:21
> To: Cui, Dexuan; Kouya Shimura
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] x86 hvm: freeze PIT/LAPIC timer emulation
> while its IRQ is masked
> 
> On 16/09/2009 09:00, "Cui, Dexuan" <dexuan.cui@intel.com>
wrote:
> 
>> Looks the little win doesn''t deserve the increased complexity
in code.
>> 
>> BTW, recent Intel CPUs run much faster with respect to VMEntry/VMExit
and
>> VMREAD/VMWRITE, so I don''t think the SW optimizatin is
appearling here. :-)
> 
> Makes it more appealing doesn''t it? As this path will be a greater
> proportion of an overall vmexit/vmenter cycle.
> 
>  -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel