2021-12-27 16:44:54

by Niklas Schnelle

Subject: [RFC 30/32] /dev/port: don't compile file operations without CONFIG_DEVPORT

In the future inb() and friends will not be available when compiling
with CONFIG_HAS_IOPORT=n so we must only try to access them here if
CONFIG_DEVPORT is set which depends on HAS_IOPORT.

Co-developed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Niklas Schnelle <[email protected]>
---
drivers/char/mem.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index cc296f0823bd..c1373617153f 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -402,6 +402,7 @@ static int mmap_mem(struct file *file, struct vm_area_struct *vma)
return 0;
}

+#ifdef CONFIG_DEVPORT
static ssize_t read_port(struct file *file, char __user *buf,
size_t count, loff_t *ppos)
{
@@ -443,6 +444,7 @@ static ssize_t write_port(struct file *file, const char __user *buf,
*ppos = i;
return tmp-buf;
}
+#endif

static ssize_t read_null(struct file *file, char __user *buf,
size_t count, loff_t *ppos)
@@ -665,12 +667,14 @@ static const struct file_operations null_fops = {
.splice_write = splice_write_null,
};

-static const struct file_operations __maybe_unused port_fops = {
+#ifdef CONFIG_DEVPORT
+static const struct file_operations port_fops = {
.llseek = memory_lseek,
.read = read_port,
.write = write_port,
.open = open_port,
};
+#endif

static const struct file_operations zero_fops = {
.llseek = zero_lseek,
--
2.32.0

2021-12-28 08:17:47

by Greg Kroah-Hartman

Subject: Re: [RFC 30/32] /dev/port: don't compile file operations without CONFIG_DEVPORT

On Mon, Dec 27, 2021 at 05:43:15PM +0100, Niklas Schnelle wrote:
> In the future inb() and friends will not be available when compiling
> with CONFIG_HAS_IOPORT=n so we must only try to access them here if
> CONFIG_DEVPORT is set which depends on HAS_IOPORT.
>
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Niklas Schnelle <[email protected]>
> ---
> drivers/char/mem.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index cc296f0823bd..c1373617153f 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -402,6 +402,7 @@ static int mmap_mem(struct file *file, struct vm_area_struct *vma)
> return 0;
> }
>
> +#ifdef CONFIG_DEVPORT
> static ssize_t read_port(struct file *file, char __user *buf,
> size_t count, loff_t *ppos)
> {
> @@ -443,6 +444,7 @@ static ssize_t write_port(struct file *file, const char __user *buf,
> *ppos = i;
> return tmp-buf;
> }
> +#endif
>
> static ssize_t read_null(struct file *file, char __user *buf,
> size_t count, loff_t *ppos)
> @@ -665,12 +667,14 @@ static const struct file_operations null_fops = {
> .splice_write = splice_write_null,
> };
>
> -static const struct file_operations __maybe_unused port_fops = {
> +#ifdef CONFIG_DEVPORT
> +static const struct file_operations port_fops = {
> .llseek = memory_lseek,
> .read = read_port,
> .write = write_port,
> .open = open_port,
> };
> +#endif

Why is this #ifdef needed if it is already __maybe_unused?

In looking closer, this change could be taken now as the use of this
variable already is behind this same #ifdef statement, right?

thanks,

greg k-h

2021-12-29 10:26:04

by Niklas Schnelle

Subject: Re: [RFC 30/32] /dev/port: don't compile file operations without CONFIG_DEVPORT

On Tue, 2021-12-28 at 09:17 +0100, Greg Kroah-Hartman wrote:
> On Mon, Dec 27, 2021 at 05:43:15PM +0100, Niklas Schnelle wrote:
> > In the future inb() and friends will not be available when compiling
> > with CONFIG_HAS_IOPORT=n so we must only try to access them here if
> > CONFIG_DEVPORT is set which depends on HAS_IOPORT.
> >
> > Co-developed-by: Arnd Bergmann <[email protected]>
> > Signed-off-by: Arnd Bergmann <[email protected]>
> > Signed-off-by: Niklas Schnelle <[email protected]>
> > ---
> > drivers/char/mem.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> > index cc296f0823bd..c1373617153f 100644
> > --- a/drivers/char/mem.c
> > +++ b/drivers/char/mem.c
> > @@ -402,6 +402,7 @@ static int mmap_mem(struct file *file, struct vm_area_struct *vma)
> > return 0;
> > }
> >
> > +#ifdef CONFIG_DEVPORT
> > static ssize_t read_port(struct file *file, char __user *buf,
> > size_t count, loff_t *ppos)
> > {
> > @@ -443,6 +444,7 @@ static ssize_t write_port(struct file *file, const char __user *buf,
> > *ppos = i;
> > return tmp-buf;
> > }
> > +#endif
> >
> > static ssize_t read_null(struct file *file, char __user *buf,
> > size_t count, loff_t *ppos)
> > @@ -665,12 +667,14 @@ static const struct file_operations null_fops = {
> > .splice_write = splice_write_null,
> > };
> >
> > -static const struct file_operations __maybe_unused port_fops = {
> > +#ifdef CONFIG_DEVPORT
> > +static const struct file_operations port_fops = {
> > .llseek = memory_lseek,
> > .read = read_port,
> > .write = write_port,
> > .open = open_port,
> > };
> > +#endif
>
> Why is this #ifdef needed if it is already __maybe_unused?

Because read_port() calls inb() and write_port() calls outb() they
wouldn't compile once these are no longer defined. Then however the
read_port/write_port symbols in the struct initialization above
couldn't be resolved.

>
> In looking closer, this change could be taken now as the use of this
> variable already is behind this same #ifdef statement, right?

Yes

>
> thanks,
>
> greg k-h

2021-12-29 10:38:17

by Greg Kroah-Hartman

Subject: Re: [RFC 30/32] /dev/port: don't compile file operations without CONFIG_DEVPORT

On Wed, Dec 29, 2021 at 11:25:12AM +0100, Niklas Schnelle wrote:
> On Tue, 2021-12-28 at 09:17 +0100, Greg Kroah-Hartman wrote:
> > On Mon, Dec 27, 2021 at 05:43:15PM +0100, Niklas Schnelle wrote:
> > > In the future inb() and friends will not be available when compiling
> > > with CONFIG_HAS_IOPORT=n so we must only try to access them here if
> > > CONFIG_DEVPORT is set which depends on HAS_IOPORT.
> > >
> > > Co-developed-by: Arnd Bergmann <[email protected]>
> > > Signed-off-by: Arnd Bergmann <[email protected]>
> > > Signed-off-by: Niklas Schnelle <[email protected]>
> > > ---
> > > drivers/char/mem.c | 6 +++++-
> > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> > > index cc296f0823bd..c1373617153f 100644
> > > --- a/drivers/char/mem.c
> > > +++ b/drivers/char/mem.c
> > > @@ -402,6 +402,7 @@ static int mmap_mem(struct file *file, struct vm_area_struct *vma)
> > > return 0;
> > > }
> > >
> > > +#ifdef CONFIG_DEVPORT
> > > static ssize_t read_port(struct file *file, char __user *buf,
> > > size_t count, loff_t *ppos)
> > > {
> > > @@ -443,6 +444,7 @@ static ssize_t write_port(struct file *file, const char __user *buf,
> > > *ppos = i;
> > > return tmp-buf;
> > > }
> > > +#endif
> > >
> > > static ssize_t read_null(struct file *file, char __user *buf,
> > > size_t count, loff_t *ppos)
> > > @@ -665,12 +667,14 @@ static const struct file_operations null_fops = {
> > > .splice_write = splice_write_null,
> > > };
> > >
> > > -static const struct file_operations __maybe_unused port_fops = {
> > > +#ifdef CONFIG_DEVPORT
> > > +static const struct file_operations port_fops = {
> > > .llseek = memory_lseek,
> > > .read = read_port,
> > > .write = write_port,
> > > .open = open_port,
> > > };
> > > +#endif
> >
> > Why is this #ifdef needed if it is already __maybe_unused?
>
> Because read_port() calls inb() and write_port() calls outb() they
> wouldn't compile once these are no longer defined. Then however the
> read_port/write_port symbols in the struct initialization above
> couldn't be resolved.
>
> >
> > In looking closer, this change could be taken now as the use of this
> > variable already is behind this same #ifdef statement, right?
>
> Yes

Great, feel free to send this individually, not as a RFC patch, and I
will be glad to queue it up.

thanks,

greg k-h

2021-12-30 16:19:29

by Arnd Bergmann

Subject: Re: [RFC 30/32] /dev/port: don't compile file operations without CONFIG_DEVPORT

On Wed, Dec 29, 2021 at 5:38 AM Greg Kroah-Hartman
<[email protected]> wrote:
> > > > -static const struct file_operations __maybe_unused port_fops = {
> > > > +#ifdef CONFIG_DEVPORT
> > > > +static const struct file_operations port_fops = {
> > > > .llseek = memory_lseek,
> > > > .read = read_port,
> > > > .write = write_port,
> > > > .open = open_port,
> > > > };
> > > > +#endif
> > >
> > > Why is this #ifdef needed if it is already __maybe_unused?
> >
> > Because read_port() calls inb() and write_port() calls outb() they
> > wouldn't compile once these are no longer defined. Then however the
> > read_port/write_port symbols in the struct initialization above
> > couldn't be resolved.
> >
> > >
> > > In looking closer, this change could be taken now as the use of this
> > > variable already is behind this same #ifdef statement, right?
> >
> > Yes
>
> Great, feel free to send this individually, not as a RFC patch, and I
> will be glad to queue it up.

I think this patch should contain the 'depends on HAS_IOPORT' that
is currently added in a different patch (char: impi, tpm: depend on
HAS_IOPORT).

However, we can't merge that version until HAS_IOPORT is actually
added to the kernel.

Arnd