[PATCH 1/3] arm64: spin-table: handle unmapped cpu-release-addrs

[mark.rutland@arm.com (Mark Rutland)]

On Tue, Jul 29, 2014 at 05:13:07PM +0100, Arnd Bergmann wrote:
> On Tuesday 29 July 2014 17:03:03 Mark Rutland wrote:
> > On Tue, Jul 29, 2014 at 04:20:46PM +0100, Arnd Bergmann wrote:
> > > On Tuesday 29 July 2014 11:15:45 Mark Salter wrote:
> > > > > -
> > > > > -     __flush_dcache_area(release_addr, sizeof(release_addr[0]));
> > > > > +     writeq_relaxed(__pa(secondary_holding_pen), release_addr);
> > > > > +     __flush_dcache_area(release_addr, sizeof(*release_addr));
> > > > 
> > > >        __flush_dcache_area((__force void *)release_addr, ... 
> > > > 
> > > > to avoid sparse warning.
> > 
> > Presumably we'd get this for the write_relaxed too?
> 
> writeq_relaxed() actually expects an __iomem  pointer

Ah. I was being thick and thought this was about the underlying type of
release_addr (le64 * vs void *) rather than the __iomem annotation.

> > > 
> > > I think it would be cleaner to drop the __iomem annotation and use vmap()
> > > rather than ioremap(). That requires having a 'struct page' though, which
> > > I'm not sure you have.
> > 
> > As far as I am aware, we'd only have a struct page for memory falling in
> > the linear map, so for the cases this patch is actually required we
> > wouldn't have a struct page.
> > 
> > So it looks like I should just make release_addr a void __iomem *. Then
> > this line can just be:
> 
> You mean make it a 'void *' instead of 'void __iomem *', right?
> 
> > __flush_dcache_area(release_addr, 8);
> > 
> > Where we could replace 8 with sizeof(u64), sizeof(__le64), etc if 8 is
> > too magic.
> > 
> > How does that sound?
> 
> Not sure where you're getting at. Using a regular pointer sounds fine,
> but then you have to cast the result of ioremap and do a manual
> cpu_to_le64 conversion on the assignment.
> 
> Keeping the iomem annotation will also work,a nd then we only need
> the cast in the __flush_dcache_area call.

Sorry, I'd misunderstood the problem and my suggestion was nonsense
deriving from that.

Having the (__force void *) cast in the call to __flush_dcache_area
sounds like the right solution to me.

Thanks,
Mark.