From: "Arnd Bergmann" <arnd@arndb.de>
To: "Michael Ellerman" <mpe@ellerman.id.au>,
"Arnd Bergmann" <arnd@kernel.org>
Cc: "Nathan Chancellor" <nathan@kernel.org>,
"Anders Roxell" <anders.roxell@linaro.org>,
"Kees Cook" <keescook@chromium.org>,
"Baoquan He" <bhe@redhat.com>,
"Niklas Schnelle" <schnelle@linux.ibm.com>,
"Naresh Kamboju" <naresh.kamboju@linaro.org>,
clang-built-linux <llvm@lists.linux.dev>,
"Nick Desaulniers" <ndesaulniers@google.com>,
linux-kernel@vger.kernel.org,
"Nicholas Piggin" <npiggin@gmail.com>,
"Aneesh Kumar K . V" <aneesh.kumar@kernel.org>,
"Jeff Xu" <jeffxu@chromium.org>,
"Mike Rapoport" <rppt@kernel.org>,
"Justin Stitt" <justinstitt@google.com>,
"Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
"Dan Carpenter" <dan.carpenter@linaro.org>,
"Bill Wendling" <morbo@google.com>
Subject: Re: [PATCH] powerpc: drop port I/O helpers for CONFIG_HAS_IOPORT=n
Date: Fri, 19 Apr 2024 07:58:40 +0200 [thread overview]
Message-ID: <16334340-d409-4be4-a89d-981baa7a4b82@app.fastmail.com> (raw)
In-Reply-To: <87wmou9bqd.fsf@mail.lhotse>
On Fri, Apr 19, 2024, at 07:12, Michael Ellerman wrote:
> Michael Ellerman <mpe@ellerman.id.au> writes:
>> "Arnd Bergmann" <arnd@arndb.de> writes:
>>>
>>> I had included this at first, but then I still ran into
>>> the same warnings because it ends up pulling in the
>>> generic outsb() etc from include/asm-generic/io.h
>>> that relies on setting a non-NULL PCI_IOBASE.
>>
>> Yes you're right. The above fixes the gcc build, but not clang.
>>
>> So I think I'll just cherry pick f0a816fb12da ("/dev/port: don't compile
>> file operations without CONFIG_DEVPORT") into my next and then apply
>> this. But will see if there's any other build failures over night.
>
> That didn't work. Still lots of drivers in my tree (based on rc2) which
> use inb/outb etc, and barf on the empty #define inb.
Right, the patches from Niklas only went into linux-next so far,
and a few are missing (including the 8250 one I think), so -rc2
at the moment regresses, but that doesn't have the warning either.
The idea of my patch was to both fix the current linux-next
build regression and have something that works in the long
run, I didn't expect it to work by itself. Sorry that wasn't
clear from my description.
> So I think this patch needs to wait until all the CONFIG_HAS_IOPORT
> checks have been merged for various drivers.
>
> For now the below fixes the clang warning. AFAICS it's safe because any
> code using inb() etc. with CONFIG_PCI=n is currently just doing a plain
> load from virtual address ~zero which should fault anyway.
If the port number is high enough, the current code might end
up referencing a user space address, depending on mmap_min_addr,
which defaults to 4096.
Using POISON_POINTER_DELTA is clearly an improvement over that.
> diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
> index 08c550ed49be..1cd6eb6c8101 100644
> --- a/arch/powerpc/include/asm/io.h
> +++ b/arch/powerpc/include/asm/io.h
> @@ -37,7 +37,7 @@ extern struct pci_dev *isa_bridge_pcidev;
> * define properly based on the platform
> */
> #ifndef CONFIG_PCI
> -#define _IO_BASE 0
> +#define _IO_BASE POISON_POINTER_DELTA
> #define _ISA_MEM_BASE 0
> #define PCI_DRAM_OFFSET 0
> #elif defined(CONFIG_PPC32)
You may need to double-check, but I think for ppc64 we
can just unconditionally set _IO_BASE to ISA_IO_BASE
regardless of CONFIG_PCI. 3d5134ee8341 ("[POWERPC]
Rewrite IO allocation & mapping on powerpc64") ensured
that the I/O space is only ever mapped to this virtual
address, and the same method is used with the
asm-generic/io.h implementation on arm/arm64/loongarch/
m68k/riscv/xtensa. Using this would be both safer and
more efficient than the current version.
It should also not cause any regressions ;-)
Unfortunately, ppc32 never got that cleanup, so
POISON_POINTER_DELTA is probably still best until Niklas's
series is merged. You could set _ISA_MEM_BASE to the same
here for good measure.
[another side note: the non-zero PCI_DRAM_OFFSET looks
unnecessary as well now, apparently this was meant for
ibm cpc710 and ppc440 platforms that are no longer
supported.]
Arnd
WARNING: multiple messages have this Message-ID (diff)
From: "Arnd Bergmann" <arnd@arndb.de>
To: "Michael Ellerman" <mpe@ellerman.id.au>,
"Arnd Bergmann" <arnd@kernel.org>
Cc: "Aneesh Kumar K . V" <aneesh.kumar@kernel.org>,
Anders Roxell <anders.roxell@linaro.org>,
Kees Cook <keescook@chromium.org>, Baoquan He <bhe@redhat.com>,
Niklas Schnelle <schnelle@linux.ibm.com>,
Naresh Kamboju <naresh.kamboju@linaro.org>,
clang-built-linux <llvm@lists.linux.dev>,
Nick Desaulniers <ndesaulniers@google.com>,
linux-kernel@vger.kernel.org, Nicholas Piggin <npiggin@gmail.com>,
Nathan Chancellor <nathan@kernel.org>,
Jeff Xu <jeffxu@chromium.org>,
Dan Carpenter <dan.carpenter@linaro.org>,
Justin Stitt <justinstitt@google.com>,
"Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
Mike Rapoport <rppt@kernel.org>, Bill Wendling <morbo@google.com>
Subject: Re: [PATCH] powerpc: drop port I/O helpers for CONFIG_HAS_IOPORT=n
Date: Fri, 19 Apr 2024 07:58:40 +0200 [thread overview]
Message-ID: <16334340-d409-4be4-a89d-981baa7a4b82@app.fastmail.com> (raw)
In-Reply-To: <87wmou9bqd.fsf@mail.lhotse>
On Fri, Apr 19, 2024, at 07:12, Michael Ellerman wrote:
> Michael Ellerman <mpe@ellerman.id.au> writes:
>> "Arnd Bergmann" <arnd@arndb.de> writes:
>>>
>>> I had included this at first, but then I still ran into
>>> the same warnings because it ends up pulling in the
>>> generic outsb() etc from include/asm-generic/io.h
>>> that relies on setting a non-NULL PCI_IOBASE.
>>
>> Yes you're right. The above fixes the gcc build, but not clang.
>>
>> So I think I'll just cherry pick f0a816fb12da ("/dev/port: don't compile
>> file operations without CONFIG_DEVPORT") into my next and then apply
>> this. But will see if there's any other build failures over night.
>
> That didn't work. Still lots of drivers in my tree (based on rc2) which
> use inb/outb etc, and barf on the empty #define inb.
Right, the patches from Niklas only went into linux-next so far,
and a few are missing (including the 8250 one I think), so -rc2
at the moment regresses, but that doesn't have the warning either.
The idea of my patch was to both fix the current linux-next
build regression and have something that works in the long
run, I didn't expect it to work by itself. Sorry that wasn't
clear from my description.
> So I think this patch needs to wait until all the CONFIG_HAS_IOPORT
> checks have been merged for various drivers.
>
> For now the below fixes the clang warning. AFAICS it's safe because any
> code using inb() etc. with CONFIG_PCI=n is currently just doing a plain
> load from virtual address ~zero which should fault anyway.
If the port number is high enough, the current code might end
up referencing a user space address, depending on mmap_min_addr,
which defaults to 4096.
Using POISON_POINTER_DELTA is clearly an improvement over that.
> diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
> index 08c550ed49be..1cd6eb6c8101 100644
> --- a/arch/powerpc/include/asm/io.h
> +++ b/arch/powerpc/include/asm/io.h
> @@ -37,7 +37,7 @@ extern struct pci_dev *isa_bridge_pcidev;
> * define properly based on the platform
> */
> #ifndef CONFIG_PCI
> -#define _IO_BASE 0
> +#define _IO_BASE POISON_POINTER_DELTA
> #define _ISA_MEM_BASE 0
> #define PCI_DRAM_OFFSET 0
> #elif defined(CONFIG_PPC32)
You may need to double-check, but I think for ppc64 we
can just unconditionally set _IO_BASE to ISA_IO_BASE
regardless of CONFIG_PCI. 3d5134ee8341 ("[POWERPC]
Rewrite IO allocation & mapping on powerpc64") ensured
that the I/O space is only ever mapped to this virtual
address, and the same method is used with the
asm-generic/io.h implementation on arm/arm64/loongarch/
m68k/riscv/xtensa. Using this would be both safer and
more efficient than the current version.
It should also not cause any regressions ;-)
Unfortunately, ppc32 never got that cleanup, so
POISON_POINTER_DELTA is probably still best until Niklas's
series is merged. You could set _ISA_MEM_BASE to the same
here for good measure.
[another side note: the non-zero PCI_DRAM_OFFSET looks
unnecessary as well now, apparently this was meant for
ibm cpc710 and ppc440 platforms that are no longer
supported.]
Arnd
next prev parent reply other threads:[~2024-04-19 5:59 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-16 15:33 [PATCH] powerpc: drop port I/O helpers for CONFIG_HAS_IOPORT=n Arnd Bergmann
2024-04-16 15:33 ` Arnd Bergmann
2024-04-18 6:26 ` Michael Ellerman
2024-04-18 6:26 ` Michael Ellerman
2024-04-18 6:33 ` Michael Ellerman
2024-04-18 6:33 ` Michael Ellerman
2024-04-18 6:59 ` Arnd Bergmann
2024-04-18 6:59 ` Arnd Bergmann
2024-04-18 13:01 ` Michael Ellerman
2024-04-18 13:01 ` Michael Ellerman
2024-04-19 5:12 ` Michael Ellerman
2024-04-19 5:12 ` Michael Ellerman
2024-04-19 5:58 ` Arnd Bergmann [this message]
2024-04-19 5:58 ` Arnd Bergmann
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=16334340-d409-4be4-a89d-981baa7a4b82@app.fastmail.com \
--to=arnd@arndb.de \
--cc=anders.roxell@linaro.org \
--cc=aneesh.kumar@kernel.org \
--cc=arnd@kernel.org \
--cc=bhe@redhat.com \
--cc=dan.carpenter@linaro.org \
--cc=jeffxu@chromium.org \
--cc=justinstitt@google.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=llvm@lists.linux.dev \
--cc=morbo@google.com \
--cc=mpe@ellerman.id.au \
--cc=naresh.kamboju@linaro.org \
--cc=nathan@kernel.org \
--cc=naveen.n.rao@linux.ibm.com \
--cc=ndesaulniers@google.com \
--cc=npiggin@gmail.com \
--cc=rppt@kernel.org \
--cc=schnelle@linux.ibm.com \
/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.