Re: [PATCH] Fix left over EFI cache mapping problems

[Andi Kleen <andi@firstfloor.org>]

On Thu, Feb 14, 2008 at 02:08:16PM -0800, Arjan van de Ven wrote:
> On Thu, 14 Feb 2008 22:42:41 +0100
> Andi Kleen <andi@firstfloor.org> wrote:
> 
> > On Thu, Feb 14, 2008 at 07:38:19PM +0100, Ingo Molnar wrote:
> > > 
> > > * Andi Kleen <andi@firstfloor.org> wrote:
> > > 
> > > > > this is indeed a bug (we change the attributes for a larger
> > > > > area than needed), but your fix is unclean. Find below a
> > > > > cleaner solution.
> > > > 
> > > > You're still ignoring the other problem of set_memory_uc() not 
> > > > handling fixmap and ioremap correctly. [...]
> > > 
> > > No, we did not ignore it, and yes, you are wrong.
> > > 
> > > One thing that you miss is that the 64-bit EFI runtime has to be
> > > marked uncacheable only if it the EFI image attribute signals an
> > > uncacheable area:
> > > 
> > >                 if (!(md->attribute & EFI_MEMORY_WB))
> > >                         set_memory_uc(md->virt_addr, md->num_pages);
> > > 
> > > and Linux EFI does not support device EFI runtimes. So your
> > > observation, 
> > 
> > Sorry I didn't get that (you were a bit terse). 
> > 
> > You're saying the EFI BIOSes will never set that flag ?
> > 
> > I'm reading page 123+ of UEFI 2.1 which describes GetMemoryMap() 
> > and these flags and I see nothing to that effect. I admit I didn't
> > read the full EFI bible so far so there are certainly EFI
> > aspects I don't understand.
> > 
> > Can you please clarify why EFI would not set that flag on Linux?
> 
> because it will only normally 

normally or guaranteed? 

e.g. possible scenario: one of the EFI runtime services needs
access to some device to do something (e.g. for rebooting
or putting information into the battery backed RAM). Wouldn't they likely 
provide a UC mapping to that device's memory using this flag? 

> get set on EFI code that lives in device memory.

Ok but what prevents the firmware from passing such memory areas?
I assume it doesn't know that Linux doesn't need such mappings, does it?

Anyways if all the questions above get firmly answered with no then the code 
setting mappings UC should be definitely removed.  I would welcome such
a patch.

-Andi