From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH v2 0/4] Fix accessed bit tracking Date: Tue, 08 Jun 2010 10:54:39 +0300 Message-ID: <4C0DF73F.603@redhat.com> References: <1275894659-17656-1-git-send-email-avi@redhat.com> <4C0CB119.3060903@cn.fujitsu.com> <4C0CB526.9070309@redhat.com> <4C0DAC6C.8090201@cn.fujitsu.com> <4C0DD425.6050400@redhat.com> <4C0DE904.5050007@cn.fujitsu.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Lai Jiangshan , Marcelo Tosatti , kvm@vger.kernel.org To: Xiao Guangrong Return-path: Received: from mx1.redhat.com ([209.132.183.28]:7735 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751227Ab0FHHyn (ORCPT ); Tue, 8 Jun 2010 03:54:43 -0400 In-Reply-To: <4C0DE904.5050007@cn.fujitsu.com> Sender: kvm-owner@vger.kernel.org List-ID: On 06/08/2010 09:53 AM, Xiao Guangrong wrote: > > Avi Kivity wrote: > >> On 06/08/2010 05:35 AM, Xiao Guangrong wrote: >> >>> >>>> We can avoid the exchange in most cases, for example if the new spte has >>>> the accessed bit set (already in the patch set) or if the page is >>>> already marked as accessed, or if we see the old spte has the accessed >>>> bit set (so no race can occur). I'll update the patches to avoid >>>> atomics when possible. >>>> >>>> >>> Umm, the reason that we need atomics here is to avoid vcpu to update >>> spte when we read A bit >>> form it, so, perhaps we can use below way to avoid atomics completely: >>> >>> - set reserved bit in spte >>> - get A bit form spte >>> - set new spte >>> >>> the worst case is cause vcpu #PF here, but it doesn't matter since the >>> old mapping is already invalid, >>> also need a remote tlb flush later. >>> >>> >> To set the reserved bit in the spte, you need an atomic operation (well, >> unless you use a sub-word-acccess to set a reserved bit in the high 32 >> bits). >> > I think we no need atomic here, for example, we can do it like this: > > *spte |= RSVD_BIT > [ maybe need a write barrier here? ] > That can drop an A bit. If *spte starts out with A cleared, we can have cpu0 cpu1 fetch *spte (A=0) set A bit write *spte (A=0, RSVD=1) > After this sentence completed, we can ensure that the spte can not updated A bit > by vcpu, so we can get A bit safely. > You also need a remote tlb flush... >>> Yes, but atomics are "LOCK" instructions, it can stop multiple cpus >>> runing in parallel. >>> >>> >> Only if those cpus are accessing the same word you're accessing. >> >> > Oh, you are right, the LOCK only locked the memory defined by the destination operand, > but i also recall that page table access can pass LOCK instruction, below description > is form intel' spec Vol. 3 7-5: > > Locked operations are atomic with respect to all other memory operations and all externally > visible events. Only instruction fetch and page table accesses can pass locked instructions. > Locked instructions can be used to synchronize data written by one processor and read by another > processor. > But actually setting the A bit will use LOCK itself. So in the following sequence write pte (A=0) test_and_clear_bit(A, pte) access memory through pte the test_and_clear_bit can return A=1 due to speculation and the parapgraph above, but setting the A bit by the processor will happen with a bus lock, so it won't lose information. -- error compiling committee.c: too many arguments to function