All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	David Laight <David.Laight@ACULAB.COM>,
	Michael Ellerman <mpe@ellerman.id.au>,
	"msuchanek@suse.de" <msuchanek@suse.de>,
	Paul Mackerras <paulus@samba.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH v4 11/23] powerpc/syscall: Rename syscall_64.c into syscall.c
Date: Tue, 02 Feb 2021 16:38:31 +1000	[thread overview]
Message-ID: <1612247170.ea0f766ml4.astroid@bobo.none> (raw)
In-Reply-To: <0cf90825-da89-6464-98d4-dc7490bff557@csgroup.eu>

Excerpts from Christophe Leroy's message of February 2, 2021 4:15 pm:
> 
> 
> Le 28/01/2021 à 00:50, Nicholas Piggin a écrit :
>> Excerpts from David Laight's message of January 26, 2021 8:28 pm:
>>> From: Nicholas Piggin
>>>> Sent: 26 January 2021 10:21
>>>>
>>>> Excerpts from Christophe Leroy's message of January 26, 2021 12:48 am:
>>>>> syscall_64.c will be reused almost as is for PPC32.
>>>>>
>>>>> Rename it syscall.c
>>>>
>>>> Could you rename it to interrupt.c instead? A system call is an
>>>> interrupt, and the file now also has code to return from other
>>>> interrupts as well, and it matches the new asm/interrupt.h from
>>>> the interrupts series.
>>>
>>> Hmmm....
>>>
>>> That might make it harder for someone looking for the system call
>>> entry code to find it.
>> 
>> It's very grep'able.
>> 
>>> In some sense interrupts are the simpler case.
>>>
>>> Especially when comparing with other architectures which have
>>> special instructions for syscall entry.
>> 
>> powerpc does have a special instruction for syscall, and it causes a
>> system call interrupt.
>> 
>> I'm not sure about other architectures, but for powerpc its more
>> sensible to call it interrupt.c than syscall.c.
> 
> Many other architectures have a syscall.c but for a different purpose: it contains arch specific 
> system calls. We have that in powerpc as well, it is called syscalls.c
> 
> So to avoid confusion, I'll rename it. But I think "interrupt" is maybe not the right name. An 
> interrupt most of the time refers to IRQ.

That depends what you mean by interrupt and IRQ.

Linux kind of considers any asynchronous maskable interrupt an irq 
(local_irq_disable()). But if you say irq it's more likely to mean
a device interrupt, and "interrupt" usually refres to the asynch
ones.

But Linux doesn't really assign names to synchronous interrupts in
core code. It doesn't say they aren't interrupts, it just doesn't
really have a convention for them at all.

Other architectures e.g., x86 also have things like interrupt
descriptor table for synchronous interrupts as well. That's where
I got the interrupt wrappers code from actually.

So it's really fine to use the proper arch-specific names for things
in arch code. I'm trying to slowly change names from exception to
interrupt.

> For me system call is not an interrupt in the way it 
> doesn't unexpectedly interrupt a program flow. In powerpc manuals it is generally called exceptions, 
> no I'm more inclined to call it exception.c

Actually that's backwards. Powerpc manuals (at least the one I look at) 
calls them all interrupts including system calls, and also the system
call interrupt is actually the only one that doesn't appear to be
associated with an exception.

Also there is no distinction about expecte/unexpected -- a data storage 
interrupt is expected if you access a location without the right access 
permissions for example, but it is still an interrupt.

These handlers very specifically deal with the change to execution flow
(i.e., the interrupt), they do *not* deal with the exception which may 
be associated with it (that is the job of the handler).

And on the other hand you can deal with exceptions in some cases without
taking an interrupt at all. For example if you had MSR[EE]=0 you could
change the decrementer or execute msgclr or change HMER SPR etc to clear
various exceptions without ever taking the interrupt.

Thanks,
Nick

WARNING: multiple messages have this Message-ID (diff)
From: Nicholas Piggin <npiggin@gmail.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	David Laight <David.Laight@ACULAB.COM>,
	Michael Ellerman <mpe@ellerman.id.au>,
	"msuchanek@suse.de" <msuchanek@suse.de>,
	Paul Mackerras <paulus@samba.org>
Cc: "linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 11/23] powerpc/syscall: Rename syscall_64.c into syscall.c
Date: Tue, 02 Feb 2021 16:38:31 +1000	[thread overview]
Message-ID: <1612247170.ea0f766ml4.astroid@bobo.none> (raw)
In-Reply-To: <0cf90825-da89-6464-98d4-dc7490bff557@csgroup.eu>

Excerpts from Christophe Leroy's message of February 2, 2021 4:15 pm:
> 
> 
> Le 28/01/2021 à 00:50, Nicholas Piggin a écrit :
>> Excerpts from David Laight's message of January 26, 2021 8:28 pm:
>>> From: Nicholas Piggin
>>>> Sent: 26 January 2021 10:21
>>>>
>>>> Excerpts from Christophe Leroy's message of January 26, 2021 12:48 am:
>>>>> syscall_64.c will be reused almost as is for PPC32.
>>>>>
>>>>> Rename it syscall.c
>>>>
>>>> Could you rename it to interrupt.c instead? A system call is an
>>>> interrupt, and the file now also has code to return from other
>>>> interrupts as well, and it matches the new asm/interrupt.h from
>>>> the interrupts series.
>>>
>>> Hmmm....
>>>
>>> That might make it harder for someone looking for the system call
>>> entry code to find it.
>> 
>> It's very grep'able.
>> 
>>> In some sense interrupts are the simpler case.
>>>
>>> Especially when comparing with other architectures which have
>>> special instructions for syscall entry.
>> 
>> powerpc does have a special instruction for syscall, and it causes a
>> system call interrupt.
>> 
>> I'm not sure about other architectures, but for powerpc its more
>> sensible to call it interrupt.c than syscall.c.
> 
> Many other architectures have a syscall.c but for a different purpose: it contains arch specific 
> system calls. We have that in powerpc as well, it is called syscalls.c
> 
> So to avoid confusion, I'll rename it. But I think "interrupt" is maybe not the right name. An 
> interrupt most of the time refers to IRQ.

That depends what you mean by interrupt and IRQ.

Linux kind of considers any asynchronous maskable interrupt an irq 
(local_irq_disable()). But if you say irq it's more likely to mean
a device interrupt, and "interrupt" usually refres to the asynch
ones.

But Linux doesn't really assign names to synchronous interrupts in
core code. It doesn't say they aren't interrupts, it just doesn't
really have a convention for them at all.

Other architectures e.g., x86 also have things like interrupt
descriptor table for synchronous interrupts as well. That's where
I got the interrupt wrappers code from actually.

So it's really fine to use the proper arch-specific names for things
in arch code. I'm trying to slowly change names from exception to
interrupt.

> For me system call is not an interrupt in the way it 
> doesn't unexpectedly interrupt a program flow. In powerpc manuals it is generally called exceptions, 
> no I'm more inclined to call it exception.c

Actually that's backwards. Powerpc manuals (at least the one I look at) 
calls them all interrupts including system calls, and also the system
call interrupt is actually the only one that doesn't appear to be
associated with an exception.

Also there is no distinction about expecte/unexpected -- a data storage 
interrupt is expected if you access a location without the right access 
permissions for example, but it is still an interrupt.

These handlers very specifically deal with the change to execution flow
(i.e., the interrupt), they do *not* deal with the exception which may 
be associated with it (that is the job of the handler).

And on the other hand you can deal with exceptions in some cases without
taking an interrupt at all. For example if you had MSR[EE]=0 you could
change the decrementer or execute msgclr or change HMER SPR etc to clear
various exceptions without ever taking the interrupt.

Thanks,
Nick

  reply	other threads:[~2021-02-02  6:39 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-25 14:48 [PATCH v4 00/23] powerpc/32: Implement C syscall entry/exit Christophe Leroy
2021-01-25 14:48 ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 01/23] powerpc/32s: Add missing call to kuep_lock on syscall entry Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 02/23] powerpc/32: Always enable data translation " Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 03/23] powerpc/32: On syscall entry, enable instruction translation at the same time as data Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 04/23] powerpc/32: Reorder instructions to avoid using CTR in syscall entry Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 05/23] powerpc/64s: Make kuap_check_amr() and kuap_get_and_check_amr() generic Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 06/23] powerpc/32s: Create C version of kuap_user/kernel_restore() and friends Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 07/23] powerpc/8xx: " Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 08/23] powerpc/irq: Add helper to set regs->softe Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 09/23] powerpc/irq: Rework helpers that manipulate MSR[EE/RI] Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 10/23] powerpc/irq: Add stub irq_soft_mask_return() for PPC32 Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 11/23] powerpc/syscall: Rename syscall_64.c into syscall.c Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-26 10:21   ` Nicholas Piggin
2021-01-26 10:21     ` Nicholas Piggin
2021-01-26 10:28     ` David Laight
2021-01-27 23:50       ` Nicholas Piggin
2021-01-27 23:50         ` Nicholas Piggin
2021-02-02  6:15         ` Christophe Leroy
2021-02-02  6:15           ` Christophe Leroy
2021-02-02  6:38           ` Nicholas Piggin [this message]
2021-02-02  6:38             ` Nicholas Piggin
2021-02-02  6:58             ` Christophe Leroy
2021-02-02  6:58               ` Christophe Leroy
2021-02-02 20:10             ` Segher Boessenkool
2021-02-02 20:10               ` Segher Boessenkool
2021-02-08 17:47     ` Christophe Leroy
2021-02-08 17:47       ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 12/23] powerpc/syscall: Make syscall.c buildable on PPC32 Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 13/23] powerpc/syscall: Use is_compat_task() Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 14/23] powerpc/syscall: Save r3 in regs->orig_r3 Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-26 10:18   ` Nicholas Piggin
2021-01-26 10:18     ` Nicholas Piggin
2021-02-08 17:47     ` Christophe Leroy
2021-02-08 17:47       ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 15/23] powerpc/syscall: Change condition to check MSR_RI Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 16/23] powerpc/32: Always save non volatile GPRs at syscall entry Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 17/23] powerpc/syscall: implement system call entry/exit logic in C for PPC32 Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 18/23] powerpc/32: Remove verification of MSR_PR on syscall in the ASM entry Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 19/23] powerpc/syscall: Avoid stack frame in likely part of system_call_exception() Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-26 10:14   ` Nicholas Piggin
2021-01-26 10:14     ` Nicholas Piggin
2021-01-25 14:48 ` [PATCH v4 20/23] powerpc/syscall: Do not check unsupported scv vector on PPC32 Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-26 10:16   ` Nicholas Piggin
2021-01-26 10:16     ` Nicholas Piggin
2021-02-08 17:45     ` Christophe Leroy
2021-02-08 17:45       ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 21/23] powerpc/syscall: Remove FULL_REGS verification in system_call_exception Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 22/23] powerpc/syscall: Optimise checks in beginning of system_call_exception() Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy
2021-01-25 14:48 ` [PATCH v4 23/23] powerpc/syscall: Avoid storing 'current' in another pointer Christophe Leroy
2021-01-25 14:48   ` Christophe Leroy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1612247170.ea0f766ml4.astroid@bobo.none \
    --to=npiggin@gmail.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=benh@kernel.crashing.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=msuchanek@suse.de \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.