Rohit Seth wrote:
>>and
>>it's only interested when it's executable i.e. "lazy_mmu_prot_update"
>>is a name concealing some overdesign.
>
>
> You are right that ia64 is only interested in whne the execute permissions
> kick in (and FWIW ia64 used to use update_mmu_cache API to do what it is now
> doing lazy_mmu_prot_update). Though the idea was to design an API that any
> arch can use to know when ever there is change in protections on a mapping.
What I think what we should do is audit flush_icache_page coverage, and
convert ia64 to use that (because it needs this to happen _before_ the
pte is set).
All we should need to do is add a pte argument to flush_icache, and it
should be possible to do what ia64 wants, and we can remove
lazy_mmu_prot_update (or at least rename it to something like
flush_icache_page_chprot and move it to the normal flush_icache_page
position above set_pte if not all architectures want their
flush_icache_page called at protection change time).
--
SUSE Labs, Novell Inc.
On Tue, 2007-05-01 at 21:52 +1000, Nick Piggin wrote:
> Rohit Seth wrote:
>
> >>and
> >>it's only interested when it's executable i.e. "lazy_mmu_prot_update"
> >>is a name concealing some overdesign.
> >
> >
> > You are right that ia64 is only interested in whne the execute permissions
> > kick in (and FWIW ia64 used to use update_mmu_cache API to do what it is now
> > doing lazy_mmu_prot_update). Though the idea was to design an API that any
> > arch can use to know when ever there is change in protections on a mapping.
>
> What I think what we should do is audit flush_icache_page coverage, and
> convert ia64 to use that (because it needs this to happen _before_ the
> pte is set).
>
That doesn't address the underlying requirement that arch specific code
should be told of change in protections. For ia64, you are right that
it equates to flushing icache in some cases, but this API is more
generic.
> All we should need to do is add a pte argument to flush_icache,
I'm sure this is doable. Though it is more of design issue of whether
that is the right way to do it. I understand this extra API is
difficult at this time because of one single consumer.
-rohit
Rohit Seth wrote:
> On Tue, 2007-05-01 at 21:52 +1000, Nick Piggin wrote:
>
>>Rohit Seth wrote:
>>
>>
>>>>and
>>>>it's only interested when it's executable i.e. "lazy_mmu_prot_update"
>>>>is a name concealing some overdesign.
>>>
>>>
>>>You are right that ia64 is only interested in whne the execute permissions
>>>kick in (and FWIW ia64 used to use update_mmu_cache API to do what it is now
>>>doing lazy_mmu_prot_update). Though the idea was to design an API that any
>>>arch can use to know when ever there is change in protections on a mapping.
>>
>>What I think what we should do is audit flush_icache_page coverage, and
>>convert ia64 to use that (because it needs this to happen _before_ the
>>pte is set).
>>
>
>
> That doesn't address the underlying requirement that arch specific code
> should be told of change in protections.
But you would add the flush_icache_page_chprot to address that.
> For ia64, you are right that
> it equates to flushing icache in some cases, but this API is more
> generic.
And it is also broken, because it needs to be done before the set_pte
for ia64. It needs to be done where flush_icache_page is done. And on
ia64 it flushes the icache. So I don't understand why ia64 would not
use flush_icache_page but add something completely new "because it is
more generic".
>>All we should need to do is add a pte argument to flush_icache,
>
>
> I'm sure this is doable. Though it is more of design issue of whether
> that is the right way to do it. I understand this extra API is
> difficult at this time because of one single consumer.
There is just no point in adding something else if you already have
a hook that seems to do what ia64 wants (or could, with a small amount
of work).
--
SUSE Labs, Novell Inc.