[PATCH] PCI/sysfs: Use correct variable for the legacy_mem sysfs object

[PATCH] PCI/sysfs: Use correct variable for the legacy_mem sysfs object

[Krzysztof Wilczyński]

Two legacy PCI sysfs objects "legacy_io" and "legacy_mem" were updated
to use an unified address space in the commit 636b21b50152 ("PCI: Revoke
mappings like devmem").  This allows for revocations to be managed from
a single place when drivers want to take over and mmap() a /dev/mem
range.

Following the update, both of the sysfs objects should leverage the
iomem_get_mapping() function to get an appropriate address range, but
only the "legacy_io" has been correctly updated - the second attribute
seems to be using a wrong variable to pass the iomem_get_mapping()
function to.

Thus, correct the variable name used so that the "legacy_mem" sysfs
object would also correctly call the iomem_get_mapping() function.

Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/pci-sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 5d63df7c1820..7bbf2673c7f2 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -978,7 +978,7 @@ void pci_create_legacy_files(struct pci_bus *b)
 	b->legacy_mem->size = 1024*1024;
 	b->legacy_mem->attr.mode = 0600;
 	b->legacy_mem->mmap = pci_mmap_legacy_mem;
-	b->legacy_io->mapping = iomem_get_mapping();
+	b->legacy_mem->mapping = iomem_get_mapping();
 	pci_adjust_legacy_attr(b, pci_mmap_mem);
 	error = device_create_bin_file(&b->dev, b->legacy_mem);
 	if (error)
-- 
2.32.0

Re: [PATCH] PCI/sysfs: Use correct variable for the legacy_mem sysfs object

[Bjorn Helgaas]

[+to Greg, please update sysfs_defferred_iomem_get_mapping-5.15]

On Thu, Aug 12, 2021 at 01:21:44PM +0000, Krzysztof Wilczyński wrote:
> Two legacy PCI sysfs objects "legacy_io" and "legacy_mem" were updated
> to use an unified address space in the commit 636b21b50152 ("PCI: Revoke
> mappings like devmem").  This allows for revocations to be managed from
> a single place when drivers want to take over and mmap() a /dev/mem
> range.
> 
> Following the update, both of the sysfs objects should leverage the
> iomem_get_mapping() function to get an appropriate address range, but
> only the "legacy_io" has been correctly updated - the second attribute
> seems to be using a wrong variable to pass the iomem_get_mapping()
> function to.
> 
> Thus, correct the variable name used so that the "legacy_mem" sysfs
> object would also correctly call the iomem_get_mapping() function.
> 
> Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

Ouch.  This needs to be applied to any -stable trees that contain
636b21b50152, which was merged in v5.12.

It *also* needs to be applied with the obvious updates to Greg's
sysfs_defferred_iomem_get_mapping-5.15 branch because it was changed
by f06aff924f97 ("sysfs: Rename struct bin_attribute member to
f_mapping") [1]

[1] https://git.kernel.org/cgit/linux/kernel/git/gregkh/driver-core.git/commit/?id=f06aff924f97

> ---
>  drivers/pci/pci-sysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 5d63df7c1820..7bbf2673c7f2 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -978,7 +978,7 @@ void pci_create_legacy_files(struct pci_bus *b)
>  	b->legacy_mem->size = 1024*1024;
>  	b->legacy_mem->attr.mode = 0600;
>  	b->legacy_mem->mmap = pci_mmap_legacy_mem;
> -	b->legacy_io->mapping = iomem_get_mapping();
> +	b->legacy_mem->mapping = iomem_get_mapping();
>  	pci_adjust_legacy_attr(b, pci_mmap_mem);
>  	error = device_create_bin_file(&b->dev, b->legacy_mem);
>  	if (error)
> -- 
> 2.32.0
> 

Re: [PATCH] PCI/sysfs: Use correct variable for the legacy_mem sysfs object

[Bjorn Helgaas]

On Thu, Aug 12, 2021 at 11:17:12AM -0500, Bjorn Helgaas wrote:
> [+to Greg, please update sysfs_defferred_iomem_get_mapping-5.15]

Actually, Greg, totally up to you, but if nobody else is depending on
the sysfs_defferred_iomem_get_mapping-5.15 branch, another possibility
would be for you to drop that branch and for me to merge the two
patches on it + Krzysztof's fix below + (hopefully) Krzysztof's PCI
static attribute work.

That would make my v5.15 pull request easier because I simple-mindedly
base all my branches on -rc1 while your branch is based on -rc3.  But
again, up to you.  I put those two patches on a local branch:

  94b34bc04c25 ("sysfs: Rename struct bin_attribute member to f_mapping")
  97e9dada53f1 ("sysfs: Invoke iomem_get_mapping() from the sysfs open callback")

in case that seems better to you.

> On Thu, Aug 12, 2021 at 01:21:44PM +0000, Krzysztof Wilczyński wrote:
> > Two legacy PCI sysfs objects "legacy_io" and "legacy_mem" were updated
> > to use an unified address space in the commit 636b21b50152 ("PCI: Revoke
> > mappings like devmem").  This allows for revocations to be managed from
> > a single place when drivers want to take over and mmap() a /dev/mem
> > range.
> > 
> > Following the update, both of the sysfs objects should leverage the
> > iomem_get_mapping() function to get an appropriate address range, but
> > only the "legacy_io" has been correctly updated - the second attribute
> > seems to be using a wrong variable to pass the iomem_get_mapping()
> > function to.
> > 
> > Thus, correct the variable name used so that the "legacy_mem" sysfs
> > object would also correctly call the iomem_get_mapping() function.
> > 
> > Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
> > Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
> 
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> Ouch.  This needs to be applied to any -stable trees that contain
> 636b21b50152, which was merged in v5.12.
> 
> It *also* needs to be applied with the obvious updates to Greg's
> sysfs_defferred_iomem_get_mapping-5.15 branch because it was changed
> by f06aff924f97 ("sysfs: Rename struct bin_attribute member to
> f_mapping") [1]
> 
> [1] https://git.kernel.org/cgit/linux/kernel/git/gregkh/driver-core.git/commit/?id=f06aff924f97
> 
> > ---
> >  drivers/pci/pci-sysfs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> > index 5d63df7c1820..7bbf2673c7f2 100644
> > --- a/drivers/pci/pci-sysfs.c
> > +++ b/drivers/pci/pci-sysfs.c
> > @@ -978,7 +978,7 @@ void pci_create_legacy_files(struct pci_bus *b)
> >  	b->legacy_mem->size = 1024*1024;
> >  	b->legacy_mem->attr.mode = 0600;
> >  	b->legacy_mem->mmap = pci_mmap_legacy_mem;
> > -	b->legacy_io->mapping = iomem_get_mapping();
> > +	b->legacy_mem->mapping = iomem_get_mapping();
> >  	pci_adjust_legacy_attr(b, pci_mmap_mem);
> >  	error = device_create_bin_file(&b->dev, b->legacy_mem);
> >  	if (error)
> > -- 
> > 2.32.0
> > 

Re: [PATCH] PCI/sysfs: Use correct variable for the legacy_mem sysfs object

[Greg Kroah-Hartman]

On Thu, Aug 12, 2021 at 12:14:50PM -0500, Bjorn Helgaas wrote:
> On Thu, Aug 12, 2021 at 11:17:12AM -0500, Bjorn Helgaas wrote:
> > [+to Greg, please update sysfs_defferred_iomem_get_mapping-5.15]
> 
> Actually, Greg, totally up to you, but if nobody else is depending on
> the sysfs_defferred_iomem_get_mapping-5.15 branch, another possibility
> would be for you to drop that branch and for me to merge the two
> patches on it + Krzysztof's fix below + (hopefully) Krzysztof's PCI
> static attribute work.

I can not "drop" the branch as it is already merged into my
driver-core-next branch that I can not rebase.  I could revert it, but
is that really needed?

> That would make my v5.15 pull request easier because I simple-mindedly
> base all my branches on -rc1 while your branch is based on -rc3.  But
> again, up to you.  I put those two patches on a local branch:
> 
>   94b34bc04c25 ("sysfs: Rename struct bin_attribute member to f_mapping")
>   97e9dada53f1 ("sysfs: Invoke iomem_get_mapping() from the sysfs open callback")
> 
> in case that seems better to you.

I do not see what I can do with that here as it's already in my tree :(

confused,

greg k-h

Re: [PATCH] PCI/sysfs: Use correct variable for the legacy_mem sysfs object

[Bjorn Helgaas]

On Fri, Aug 13, 2021 at 10:19:25AM +0200, Greg Kroah-Hartman wrote:
> On Thu, Aug 12, 2021 at 12:14:50PM -0500, Bjorn Helgaas wrote:
> > On Thu, Aug 12, 2021 at 11:17:12AM -0500, Bjorn Helgaas wrote:
> > > [+to Greg, please update sysfs_defferred_iomem_get_mapping-5.15]
> > 
> > Actually, Greg, totally up to you, but if nobody else is depending on
> > the sysfs_defferred_iomem_get_mapping-5.15 branch, another possibility
> > would be for you to drop that branch and for me to merge the two
> > patches on it + Krzysztof's fix below + (hopefully) Krzysztof's PCI
> > static attribute work.
> 
> I can not "drop" the branch as it is already merged into my
> driver-core-next branch that I can not rebase.  I could revert it, but
> is that really needed?

Nope, I don't think a revert is warranted.  I'll take care of getting
Krzysztof's fix into v5.15.

Bjorn

Re: [PATCH] PCI/sysfs: Use correct variable for the legacy_mem sysfs object

[Greg Kroah-Hartman]

On Fri, Aug 13, 2021 at 09:29:01AM -0500, Bjorn Helgaas wrote:
> On Fri, Aug 13, 2021 at 10:19:25AM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Aug 12, 2021 at 12:14:50PM -0500, Bjorn Helgaas wrote:
> > > On Thu, Aug 12, 2021 at 11:17:12AM -0500, Bjorn Helgaas wrote:
> > > > [+to Greg, please update sysfs_defferred_iomem_get_mapping-5.15]
> > > 
> > > Actually, Greg, totally up to you, but if nobody else is depending on
> > > the sysfs_defferred_iomem_get_mapping-5.15 branch, another possibility
> > > would be for you to drop that branch and for me to merge the two
> > > patches on it + Krzysztof's fix below + (hopefully) Krzysztof's PCI
> > > static attribute work.
> > 
> > I can not "drop" the branch as it is already merged into my
> > driver-core-next branch that I can not rebase.  I could revert it, but
> > is that really needed?
> 
> Nope, I don't think a revert is warranted.  I'll take care of getting
> Krzysztof's fix into v5.15.

Thanks, I'll handle the merge issues after that happens in my tree.

greg k-h

Re: [PATCH] PCI/sysfs: Use correct variable for the legacy_mem sysfs object

[Daniel Vetter]

On Thu, Aug 12, 2021 at 6:17 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> [+to Greg, please update sysfs_defferred_iomem_get_mapping-5.15]
>
> On Thu, Aug 12, 2021 at 01:21:44PM +0000, Krzysztof Wilczyński wrote:
> > Two legacy PCI sysfs objects "legacy_io" and "legacy_mem" were updated
> > to use an unified address space in the commit 636b21b50152 ("PCI: Revoke
> > mappings like devmem").  This allows for revocations to be managed from
> > a single place when drivers want to take over and mmap() a /dev/mem
> > range.
> >
> > Following the update, both of the sysfs objects should leverage the
> > iomem_get_mapping() function to get an appropriate address range, but
> > only the "legacy_io" has been correctly updated - the second attribute
> > seems to be using a wrong variable to pass the iomem_get_mapping()
> > function to.
> >
> > Thus, correct the variable name used so that the "legacy_mem" sysfs
> > object would also correctly call the iomem_get_mapping() function.
> >
> > Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
> > Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>
> Ouch.  This needs to be applied to any -stable trees that contain
> 636b21b50152, which was merged in v5.12.

Ouch indeed. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>
> It *also* needs to be applied with the obvious updates to Greg's
> sysfs_defferred_iomem_get_mapping-5.15 branch because it was changed
> by f06aff924f97 ("sysfs: Rename struct bin_attribute member to
> f_mapping") [1]
>
> [1] https://git.kernel.org/cgit/linux/kernel/git/gregkh/driver-core.git/commit/?id=f06aff924f97
>
> > ---
> >  drivers/pci/pci-sysfs.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> > index 5d63df7c1820..7bbf2673c7f2 100644
> > --- a/drivers/pci/pci-sysfs.c
> > +++ b/drivers/pci/pci-sysfs.c
> > @@ -978,7 +978,7 @@ void pci_create_legacy_files(struct pci_bus *b)
> >       b->legacy_mem->size = 1024*1024;
> >       b->legacy_mem->attr.mode = 0600;
> >       b->legacy_mem->mmap = pci_mmap_legacy_mem;
> > -     b->legacy_io->mapping = iomem_get_mapping();
> > +     b->legacy_mem->mapping = iomem_get_mapping();
> >       pci_adjust_legacy_attr(b, pci_mmap_mem);
> >       error = device_create_bin_file(&b->dev, b->legacy_mem);
> >       if (error)
> > --
> > 2.32.0
> >



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] PCI/sysfs: Use correct variable for the legacy_mem sysfs object

[Bjorn Helgaas]

On Thu, Aug 12, 2021 at 01:21:44PM +0000, Krzysztof Wilczyński wrote:
> Two legacy PCI sysfs objects "legacy_io" and "legacy_mem" were updated
> to use an unified address space in the commit 636b21b50152 ("PCI: Revoke
> mappings like devmem").  This allows for revocations to be managed from
> a single place when drivers want to take over and mmap() a /dev/mem
> range.
> 
> Following the update, both of the sysfs objects should leverage the
> iomem_get_mapping() function to get an appropriate address range, but
> only the "legacy_io" has been correctly updated - the second attribute
> seems to be using a wrong variable to pass the iomem_get_mapping()
> function to.
> 
> Thus, correct the variable name used so that the "legacy_mem" sysfs
> object would also correctly call the iomem_get_mapping() function.
> 
> Fixes: 636b21b50152 ("PCI: Revoke mappings like devmem")
> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>

Applied to for-linus for v5.14, thanks!

> ---
>  drivers/pci/pci-sysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 5d63df7c1820..7bbf2673c7f2 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -978,7 +978,7 @@ void pci_create_legacy_files(struct pci_bus *b)
>  	b->legacy_mem->size = 1024*1024;
>  	b->legacy_mem->attr.mode = 0600;
>  	b->legacy_mem->mmap = pci_mmap_legacy_mem;
> -	b->legacy_io->mapping = iomem_get_mapping();
> +	b->legacy_mem->mapping = iomem_get_mapping();
>  	pci_adjust_legacy_attr(b, pci_mmap_mem);
>  	error = device_create_bin_file(&b->dev, b->legacy_mem);
>  	if (error)
> -- 
> 2.32.0
>