Re: [PATCH 2/3] ACPI, APEI: Add RAM mapping support to ACPI

[Huang Ying <ying.huang@intel.com>]

Hi, Bjorn,

Sorry for late.  Just return from Chinese new year holiday.

On Sat, 2012-01-21 at 08:04 -0700, Bjorn Helgaas wrote:
[snip]
> > +
> > +static void __iomem *acpi_map(acpi_physical_address pg_off, unsigned long pg_sz)
> > +{
> > +       unsigned long pfn;
> > +
> > +       pfn = pg_off >> PAGE_SHIFT;
> > +       if (should_use_kmap(pfn)) {
> > +               if (pg_sz > PAGE_SIZE)
> > +                       return NULL;
> > +               return (void __iomem __force *)kmap(pfn_to_page(pfn));
> > +       } else
> > +               return acpi_os_ioremap(pg_off, pg_sz);
> 
> This implies that ioremap() works differently on ia64 than on x86.
> Apparently one can ioremap() RAM on x86, but not on ia64.  Why is this
> different?  Shouldn't we instead fix ioremap() on ia64 so it works the
> same as on x86?

If my understanding were correct, ioremap can not work for RAM on x86.
So we need to use kmap for RAM.  And on IA64, ioremap works for RAM and
will take care of cache attributes while kmap will not.  So ioremap is
used on IA64, while kmap is used on x86.

> I looked at the ia64 ioremap(), and I can't see the reason it fails
> for RAM.  Huang, do you remember the details from 76da3fb3575?
> 
> > +}
> > +
> > +static void acpi_unmap(acpi_physical_address pg_off, void __iomem *vaddr)
> > +{
> > +       unsigned long pfn;
> > +
> > +       pfn = pg_off >> PAGE_SHIFT;
> > +       if (page_is_ram(pfn))
> > +               kunmap(pfn_to_page(pfn));
> > +       else
> > +               iounmap(vaddr);
> 
> I hope we can resolve the ioremap() question so we don't need this
> patch at all.  But if we do need this, I don't like the asymmetry here
> -- on x86 RAM, I think we use ioremap() and kunmap(), which seems
> wrong.  We should be able to use ioremap() and iounmap().

Yes.   The asymmetry here is not good.  We should use should_use_kmap()
instead of page_is_ram directly here.

Best Regards,
Huang Ying