All of lore.kernel.org
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: Paul Brook <paul@codesourcery.com>
Cc: qemu-devel@nongnu.org, Blue Swirl <blauwirbel@gmail.com>,
	ohmura.kei@lab.ntt.co.jp,
	Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>,
	kvm@vger.kernel.org
Subject: Re: [Qemu-devel] Re: [PATCH 2/6] qemu-kvm: Modify and introduce wrapper functions to access phys_ram_dirty.
Date: Wed, 17 Mar 2010 18:28:03 +0200	[thread overview]
Message-ID: <4BA10313.7050307@redhat.com> (raw)
In-Reply-To: <201003171606.10804.paul@codesourcery.com>

On 03/17/2010 06:06 PM, Paul Brook wrote:
>> On 03/16/2010 10:10 PM, Blue Swirl wrote:
>>      
>>>>    Yes, and is what tlb_protect_code() does and it's called from
>>>> tb_alloc_page() which is what's code when a TB is created.
>>>>          
>>> Just a tangential note: a long time ago, I tried to disable self
>>> modifying code detection for Sparc. On most RISC architectures, SMC
>>> needs explicit flushing so in theory we need not track code memory
>>> writes. However, during exceptions the translator needs to access the
>>> original unmodified code that was used to generate the TB. But maybe
>>> there are other ways to avoid SMC tracking, on x86 it's still needed
>>>        
>> On x86 you're supposed to execute a serializing instruction (one of
>> INVD, INVEPT, INVLPG, INVVPID, LGDT, LIDT, LLDT, LTR, MOV (to control
>> register, with the exception of MOV CR8), MOV (to debug register),
>> WBINVD, WRMSR, CPUID, IRET, and RSM) before running modified code.
>>      
> Last time I checked, a jump instruction was sufficient to ensure coherency
> withing a core.  Serializing instructions are only required for coherency
> between cores on SMP systems.
>    

Yeah, the docs say either a jump or a serializing instruction is needed.

> QEMU effectively has a very large physically tagged icache[1] with very
> expensive cache loads.  AFAIK The only practical way to maintain that cache on
> x86 targets is to do write snooping via dirty bits. On targets that mandate
> explicit icache invalidation we might be able to get away with this, however I
> doubt it actually gains you anything - a correctly written guest is going to
> invalidate at least as much as we get from dirty tracking, and we still need
> to provide correct behaviour when executing with cache disabled.
>    

Agreed.

>    
>>> but I suppose SMC is pretty rare.
>>>        
>> Every time you demand load a code page from disk, you're running self
>> modifying code (though it usually doesn't exist in the tlb, so there's
>> no previous version that can cause trouble).
>>      
> I think you're confusing TLB flushes with TB flushes.
>    

No - my thinking was page fault, load page, invlpg, continue.  But the 
invlpg is unneeded, and "continue" has to include a jump anyway.

-- 
error compiling committee.c: too many arguments to function


WARNING: multiple messages have this Message-ID (diff)
From: Avi Kivity <avi@redhat.com>
To: Paul Brook <paul@codesourcery.com>
Cc: Blue Swirl <blauwirbel@gmail.com>,
	ohmura.kei@lab.ntt.co.jp, qemu-devel@nongnu.org,
	kvm@vger.kernel.org,
	Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>
Subject: Re: [Qemu-devel] Re: [PATCH 2/6] qemu-kvm: Modify and introduce wrapper functions to access phys_ram_dirty.
Date: Wed, 17 Mar 2010 18:28:03 +0200	[thread overview]
Message-ID: <4BA10313.7050307@redhat.com> (raw)
In-Reply-To: <201003171606.10804.paul@codesourcery.com>

On 03/17/2010 06:06 PM, Paul Brook wrote:
>> On 03/16/2010 10:10 PM, Blue Swirl wrote:
>>      
>>>>    Yes, and is what tlb_protect_code() does and it's called from
>>>> tb_alloc_page() which is what's code when a TB is created.
>>>>          
>>> Just a tangential note: a long time ago, I tried to disable self
>>> modifying code detection for Sparc. On most RISC architectures, SMC
>>> needs explicit flushing so in theory we need not track code memory
>>> writes. However, during exceptions the translator needs to access the
>>> original unmodified code that was used to generate the TB. But maybe
>>> there are other ways to avoid SMC tracking, on x86 it's still needed
>>>        
>> On x86 you're supposed to execute a serializing instruction (one of
>> INVD, INVEPT, INVLPG, INVVPID, LGDT, LIDT, LLDT, LTR, MOV (to control
>> register, with the exception of MOV CR8), MOV (to debug register),
>> WBINVD, WRMSR, CPUID, IRET, and RSM) before running modified code.
>>      
> Last time I checked, a jump instruction was sufficient to ensure coherency
> withing a core.  Serializing instructions are only required for coherency
> between cores on SMP systems.
>    

Yeah, the docs say either a jump or a serializing instruction is needed.

> QEMU effectively has a very large physically tagged icache[1] with very
> expensive cache loads.  AFAIK The only practical way to maintain that cache on
> x86 targets is to do write snooping via dirty bits. On targets that mandate
> explicit icache invalidation we might be able to get away with this, however I
> doubt it actually gains you anything - a correctly written guest is going to
> invalidate at least as much as we get from dirty tracking, and we still need
> to provide correct behaviour when executing with cache disabled.
>    

Agreed.

>    
>>> but I suppose SMC is pretty rare.
>>>        
>> Every time you demand load a code page from disk, you're running self
>> modifying code (though it usually doesn't exist in the tlb, so there's
>> no previous version that can cause trouble).
>>      
> I think you're confusing TLB flushes with TB flushes.
>    

No - my thinking was page fault, load page, invlpg, continue.  But the 
invlpg is unneeded, and "continue" has to include a jump anyway.

-- 
error compiling committee.c: too many arguments to function

  reply	other threads:[~2010-03-17 16:28 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-16 10:53 [PATCH 0/6] qemu-kvm: Introduce bit-based phys_ram_dirty, and bit-based dirty page checker Yoshiaki Tamura
2010-03-16 10:53 ` [Qemu-devel] " Yoshiaki Tamura
2010-03-16 10:53 ` [PATCH 1/6] qemu-kvm: Introduce bit-based phys_ram_dirty for VGA, CODE and MIGRATION Yoshiaki Tamura
2010-03-16 10:53   ` [Qemu-devel] " Yoshiaki Tamura
2010-03-16 12:26   ` Avi Kivity
2010-03-16 12:26     ` [Qemu-devel] " Avi Kivity
2010-03-16 13:01     ` Yoshiaki Tamura
2010-03-16 13:01       ` [Qemu-devel] " Yoshiaki Tamura
2010-03-16 13:04       ` Avi Kivity
2010-03-16 13:04         ` [Qemu-devel] " Avi Kivity
2010-03-16 10:53 ` [PATCH 2/6] qemu-kvm: Modify and introduce wrapper functions to access phys_ram_dirty Yoshiaki Tamura
2010-03-16 10:53   ` [Qemu-devel] " Yoshiaki Tamura
2010-03-16 12:45   ` Avi Kivity
2010-03-16 12:45     ` [Qemu-devel] " Avi Kivity
2010-03-16 13:17     ` Yoshiaki Tamura
2010-03-16 13:17       ` [Qemu-devel] " Yoshiaki Tamura
2010-03-16 13:29       ` Avi Kivity
2010-03-16 13:29         ` [Qemu-devel] " Avi Kivity
2010-03-16 13:49         ` Yoshiaki Tamura
2010-03-16 13:49           ` [Qemu-devel] " Yoshiaki Tamura
2010-03-16 13:51         ` Anthony Liguori
2010-03-16 13:51           ` [Qemu-devel] " Anthony Liguori
2010-03-16 13:57           ` Avi Kivity
2010-03-16 13:57             ` [Qemu-devel] " Avi Kivity
2010-03-16 14:50             ` Anthony Liguori
2010-03-16 14:50               ` [Qemu-devel] " Anthony Liguori
2010-03-16 20:10               ` Blue Swirl
2010-03-16 20:10                 ` Blue Swirl
2010-03-16 22:31                 ` Richard Henderson
2010-03-16 22:31                   ` [Qemu-devel] " Richard Henderson
2010-03-17  0:05                   ` Paul Brook
2010-03-17  0:05                     ` Paul Brook
2010-03-17  4:07                 ` Avi Kivity
2010-03-17  4:07                   ` Avi Kivity
2010-03-17 16:06                   ` Paul Brook
2010-03-17 16:06                     ` Paul Brook
2010-03-17 16:28                     ` Avi Kivity [this message]
2010-03-17 16:28                       ` Avi Kivity
2010-03-16 13:35     ` Anthony Liguori
2010-03-16 13:35       ` [Qemu-devel] " Anthony Liguori
2010-03-16 22:50       ` Yoshiaki Tamura
2010-03-16 22:50         ` [Qemu-devel] " Yoshiaki Tamura
2010-03-16 10:53 ` [PATCH 3/6] qemu-kvm: Replace direct phys_ram_dirty access with wrapper functions Yoshiaki Tamura
2010-03-16 10:53   ` [Qemu-devel] " Yoshiaki Tamura
2010-03-16 10:53 ` [PATCH 4/6] qemu-kvm: Introduce cpu_physical_memory_get_dirty_range() Yoshiaki Tamura
2010-03-16 10:53   ` [Qemu-devel] " Yoshiaki Tamura
2010-03-16 12:47   ` Avi Kivity
2010-03-16 12:47     ` [Qemu-devel] " Avi Kivity
2010-03-16 10:53 ` [PATCH 5/6] qemu-kvm: Use cpu_physical_memory_set_dirty_range() to update phys_ram_dirty Yoshiaki Tamura
2010-03-16 10:53   ` [Qemu-devel] " Yoshiaki Tamura
2010-03-16 10:53 ` [PATCH 6/6] qemu-kvm: Use cpu_physical_memory_get_dirty_range() to check multiple dirty pages Yoshiaki Tamura
2010-03-16 10:53   ` [Qemu-devel] " Yoshiaki Tamura
2010-03-16 13:11 ` [PATCH 0/6] qemu-kvm: Introduce bit-based phys_ram_dirty, and bit-based dirty page checker Avi Kivity
2010-03-16 13:11   ` [Qemu-devel] " Avi Kivity
2010-03-16 13:41   ` Yoshiaki Tamura
2010-03-16 13:41     ` [Qemu-devel] " Yoshiaki Tamura

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BA10313.7050307@redhat.com \
    --to=avi@redhat.com \
    --cc=blauwirbel@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=ohmura.kei@lab.ntt.co.jp \
    --cc=paul@codesourcery.com \
    --cc=qemu-devel@nongnu.org \
    --cc=tamura.yoshiaki@lab.ntt.co.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.