[Linux-Xtensa] [PULL 0/25] xtensa improvements and fixes for 3.17

Max Filippov jcmvbkbc at gmail.com
Thu Aug 14 09:04:48 UTC 2014


Hi Chris,

On Thu, Aug 14, 2014 at 9:23 AM, czankel <chris at zankel.net> wrote:
> Hi Max,
>
> Great to finally get support for highmen and cache aliasing :-)
>
> Just a couple of questions:
>
> ----
>
> commit af177dd2992f2860e85465538574e5d8cc0b5f87
>
>     xtensa: simplify addition of new core variants
>
> Instead of:
>  $(strip $(subst $(QUOTE),,$(CONFIG_XTENSA_VARIANT_NAME)))
> you could also use
>  $(patsubst "%",%,$(CONFIG_XTENSA_VARIANT_NAME))
> which eliminates the use of the extra commented line to close the quote.

Looks better, I've updated the patch, thanks!

> ----
>
> commit 850781f435e7db821461e754608995b140ecc42a
>
>     xtensa: replace IOCTL code definitions with constants
>
> Is there a reason we have to keep the direction and size bits there?
> Looking at other architectures and the generic implementation, I would
> assume it's not used or gets stripped somehow.
> (Does strace, etc. depend on them)?

AFAICS it is used as is in switch statements in functions tty_mode_ioctl
and tty_ioctl, so although these bits are not semantically checked, they
are compared anyway.

> In that case, maybe we could just use the generic header file (unless we
> have some different mapping).

Unfortunately we have a number of IOCTLs with values different from those
in the generic header.

> ----
>
>
> commit 10812568df81566c15732779f62408cef79bb559
>
>     xtensa: implement clear_user_highpage and copy_user_highpage
>
> Would we need some different mapping depending on the CPU index for SMP?
> (You added something there in the other patch)

No, I believe we don't need it, because
- mappings used by these functions exist only in TLB, so different CPUs won't
  collide updating same entries in a page table;
- mappings are used with preemption disabled, so these functions cannot migrate
  to other CPU while in progress;
- mappings are always initialized on entry and invalidated on exit from these
  functions, so that even if used from hard or soft IRQ these
functions won't use
  existing mappings and won't let interrupted context use mapping set up in IRQ.

> ----
>
> commit 62cad517f7c0e981e5d21f6e05b10403888a677e
>
>     xtensa: support aliasing cache in kmap
>
> There was an old comment to use __always_inline in header files.
> (https://www.kernel.org/doc/local/inline.html)

These inlines are indeed only advisory, actual inlining/non-inlining doesn't
affect code correctness. So I believe plain 'inline' fits perfectly here.

> ----
>
> commit 59a6c8fcf234581b860f76a5b1b84bdf469ae0f8
>
>     xtensa: support highmem in aliasing cache flushing code
>
> Does the previous patch above handle cache colors for high-mem pages by the
> temporary mapping, or do we need this patch to keep them in different
> arrays?

They are independent: previous patch provides hooks that force kmap to
map pages at slots inside PKMAP area with the same color as page's
physical address. This patch replaces

__flush_invalidate_dcache_page((long)page_address(page))

calls, that need page to be mapped for page_address(page) to work, with

__flush_invalidate_dcache_page_alias(virt, phys);

calls, that establish temporary mapping in TLBTEMP area.

> ----
>
> commit 61e1e9e953501ce271d359ff976f0b5eacb8aada
>
>     xtensa: move invalid unaligned instruction handler closer to its users
>
> I vaguely remember that there were some problems when the density option was
> disabled. Did you have a chance if the distances are still within the limits
> for such processor configurations?

Yes, I've successfully built it for cores with and without density instructions.

-- 
Thanks.
-- Max


More information about the linux-xtensa mailing list