Date: Tue, 01 Jun 2010 12:20:35 -0500 From: Alan Cox <alc@cs.rice.edu> To: Marcel Moolenaar <xcllnt@mac.com> Cc: Marcel Moolenaar <marcel@freebsd.org>, Alan Cox <alc@cs.rice.edu>, ppc@freebsd.org Subject: Re: BookE pmap_enter() bug? Message-ID: <4C054163.1050900@cs.rice.edu> In-Reply-To: <B5C7F1F3-AD95-49C9-AC9F-02AFCB3FEF06@mac.com> References: <4C053389.1040204@cs.rice.edu> <B5C7F1F3-AD95-49C9-AC9F-02AFCB3FEF06@mac.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Marcel Moolenaar wrote:
> On Jun 1, 2010, at 9:21 AM, Alan Cox wrote:
>
>
>> I've been reviewing the various pmap_enter() implementations and came across the following bit of code in the BookE pmap_enter():
>>
>> if (prot & VM_PROT_EXECUTE) {
>> flags |= PTE_SX;
>> if (!su)
>> flags |= PTE_UX;
>>
>> /*
>> * Check existing flags for execute permissions: if we
>> * are turning execute permissions on, icache should
>> * be flushed.
>> */
>> if ((flags & (PTE_UX | PTE_SX)) == 0)
>> sync++;
>> }
>>
>>
>> This will never flush the instruction cache because the new flags, not the old flags, are being used. I suspect that the attached change should be made.
>>
>
> *snip*
>
> You're right.
>
> I'm not aware of icache incoherency problems on BookE, so this raises
> the question as to whether we actually need to worry about syncing the
> icache here. But that's ortogonal...
>
>
I'm also skeptical of the following, which is a few lines earlier:
/*
* The PTE_MODIFIED flag could be set by underlying
* TLB misses since we last read it (above), possibly
* other CPUs could update it so we check in the PTE
* directly rather than rely on that saved local flags
* copy.
*/
if (PTE_ISMODIFIED(pte))
vm_page_dirty(m);
I don't see how this completely addresses the race. Why can't another
processor update the PTE immediately after this test and before the PTE
is finally updated a few lines later?
Alan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C054163.1050900>