| Message ID | CAAgyjC9ttHQxodPsVcrNKx2_2T9FTy_E6wZf_u3QbqGGs82P_w@mail.gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | memory access to mmaped pci sysfs file, does not fail when the pci device is removed. | expand |
[+cc Daniel (author of 74b30195395c), Greg] On Tue, Jan 30, 2024 at 08:50:10AM +0530, aravind wrote: > Hi, > I am facing an issue in v5.15 kernel due to " [PATCH v7 12/17] PCI: > Revoke mappings like devmem "related changes. > Whenever a PCI device (4f:00.0)is removed while being accessed from > user space (mmaped (sys/bus/pci/device/....4f:00.0/resource0)), no sig > bus error is raised. in earlier kernel v5.2, a sig bus error used to > get generated for this scenario. > In v5.15 5 kernel , value 0xffffffff is returned when the device is > plugged out or it is reset. > if the device is removed through "echo 1 > > /sys/bus/pci/devices/..4f:00.0/remove") command. user space code is > still able to access device memory no fault is generated in this case. > not sure if this is expected behavior. as the file which is mapped is > removed .(/sys/bus/pci/.../resource0) > > After making the below change in v5.15 , I am able to get fault for > above scenarios. (device removal or unplug/reset.) > Please let me know if this is a new feature introduced to handle > mmaped memory access holes ? and allow to work inspite of sysfs files > removal. > > > diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c > index d019d6ac6ad0..5f9b59ba8320 100644 > --- a/fs/sysfs/file.c > +++ b/fs/sysfs/file.c > @@ -251,7 +251,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = { > .read = sysfs_kf_bin_read, > .write = sysfs_kf_bin_write, > .mmap = sysfs_kf_bin_mmap, > - .open = sysfs_kf_bin_open, > +// .open = sysfs_kf_bin_open, > }; If the change above makes the difference, I guess the change might be related to https://git.kernel.org/linus/74b30195395c ("sysfs: Support zapping of binary attr mmaps"), which appeared in v5.12. I agree that SIGBUS when accessing MMIO space of a device that has been removed sounds like a better experience than reading 0xffffffff. I don't know enough about the VM side of this to know just how 74b30195395c makes this difference. Maybe Daniel will chime in. Bjorn
Hi, Thanks for your response. Adding few more points. The patch mentioned above is making a difference. as part of open. iomem file inode is being used for address space than sysfs file. For this reason mapping is still intact in spite of removing sysfs nodes.(/sys/bus/pci/device/.../resource0). To support "CONFIG_IO_STRICT_DEVMEM", revoking iomem like it is done for /dev/mem, it is being done for sysfs path as well. which is a way to map BAR to the user space and access it. [PATCH 42/65] resource: Move devmem revoke code to resource framework PCI: Revoke mappings like devmem : https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.15.y&id=636b21b50152d4e203223ee337aca1cb3c1bfe53 Regards Aravind SK On Wed, Jan 31, 2024 at 12:58 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > [+cc Daniel (author of 74b30195395c), Greg] > > On Tue, Jan 30, 2024 at 08:50:10AM +0530, aravind wrote: > > Hi, > > I am facing an issue in v5.15 kernel due to " [PATCH v7 12/17] PCI: > > Revoke mappings like devmem "related changes. > > Whenever a PCI device (4f:00.0)is removed while being accessed from > > user space (mmaped (sys/bus/pci/device/....4f:00.0/resource0)), no sig > > bus error is raised. in earlier kernel v5.2, a sig bus error used to > > get generated for this scenario. > > In v5.15 5 kernel , value 0xffffffff is returned when the device is > > plugged out or it is reset. > > if the device is removed through "echo 1 > > > /sys/bus/pci/devices/..4f:00.0/remove") command. user space code is > > still able to access device memory no fault is generated in this case. > > not sure if this is expected behavior. as the file which is mapped is > > removed .(/sys/bus/pci/.../resource0) > > > > After making the below change in v5.15 , I am able to get fault for > > above scenarios. (device removal or unplug/reset.) > > Please let me know if this is a new feature introduced to handle > > mmaped memory access holes ? and allow to work inspite of sysfs files > > removal. > > > > > > diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c > > index d019d6ac6ad0..5f9b59ba8320 100644 > > --- a/fs/sysfs/file.c > > +++ b/fs/sysfs/file.c > > @@ -251,7 +251,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = { > > .read = sysfs_kf_bin_read, > > .write = sysfs_kf_bin_write, > > .mmap = sysfs_kf_bin_mmap, > > - .open = sysfs_kf_bin_open, > > +// .open = sysfs_kf_bin_open, > > }; > > If the change above makes the difference, I guess the change might be > related to https://git.kernel.org/linus/74b30195395c ("sysfs: Support > zapping of binary attr mmaps"), which appeared in v5.12. > > I agree that SIGBUS when accessing MMIO space of a device that has > been removed sounds like a better experience than reading 0xffffffff. > > I don't know enough about the VM side of this to know just how > 74b30195395c makes this difference. Maybe Daniel will chime in. > > Bjorn
Hi Daniel
Could you please comment ? Is there a way to clean up
mapping/unmap for the process when sysfs files are removed on pci
device removal?
Regards
Aravind SK
On Wed, Jan 31, 2024 at 12:58 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> [+cc Daniel (author of 74b30195395c), Greg]
>
> On Tue, Jan 30, 2024 at 08:50:10AM +0530, aravind wrote:
> > Hi,
> > I am facing an issue in v5.15 kernel due to " [PATCH v7 12/17] PCI:
> > Revoke mappings like devmem "related changes.
> > Whenever a PCI device (4f:00.0)is removed while being accessed from
> > user space (mmaped (sys/bus/pci/device/....4f:00.0/resource0)), no sig
> > bus error is raised. in earlier kernel v5.2, a sig bus error used to
> > get generated for this scenario.
> > In v5.15 5 kernel , value 0xffffffff is returned when the device is
> > plugged out or it is reset.
> > if the device is removed through "echo 1 >
> > /sys/bus/pci/devices/..4f:00.0/remove") command. user space code is
> > still able to access device memory no fault is generated in this case.
> > not sure if this is expected behavior. as the file which is mapped is
> > removed .(/sys/bus/pci/.../resource0)
> >
> > After making the below change in v5.15 , I am able to get fault for
> > above scenarios. (device removal or unplug/reset.)
> > Please let me know if this is a new feature introduced to handle
> > mmaped memory access holes ? and allow to work inspite of sysfs files
> > removal.
> >
> >
> > diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> > index d019d6ac6ad0..5f9b59ba8320 100644
> > --- a/fs/sysfs/file.c
> > +++ b/fs/sysfs/file.c
> > @@ -251,7 +251,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = {
> > .read = sysfs_kf_bin_read,
> > .write = sysfs_kf_bin_write,
> > .mmap = sysfs_kf_bin_mmap,
> > - .open = sysfs_kf_bin_open,
> > +// .open = sysfs_kf_bin_open,
> > };
>
> If the change above makes the difference, I guess the change might be
> related to https://git.kernel.org/linus/74b30195395c ("sysfs: Support
> zapping of binary attr mmaps"), which appeared in v5.12.
>
> I agree that SIGBUS when accessing MMIO space of a device that has
> been removed sounds like a better experience than reading 0xffffffff.
>
> I don't know enough about the VM side of this to know just how
> 74b30195395c makes this difference. Maybe Daniel will chime in.
>
> Bjorn
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index d019d6ac6ad0..5f9b59ba8320 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -251,7 +251,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = { .read = sysfs_kf_bin_read, .write = sysfs_kf_bin_write, .mmap = sysfs_kf_bin_mmap, - .open = sysfs_kf_bin_open, +// .open = sysfs_kf_bin_open, }; Regards