All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Bjørn Mork" <bjorn@mork.no>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: Greg KH <gregkh@linuxfoundation.org>, Kay Sievers <kay@vrfy.org>,
	Myron Stowe <mstowe@redhat.com>,
	Myron Stowe <myron.stowe@redhat.com>,
	linux-hotplug@vger.kernel.org, linux-pci@vger.kernel.org,
	yuxiangl@marvell.com, yxlraid@gmail.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] udevadm-info: Don't access sysfs 'resource<N>' files
Date: Mon, 18 Mar 2013 17:20:46 +0000	[thread overview]
Message-ID: <87obeg39qp.fsf@nemi.mork.no> (raw)
In-Reply-To: <1363625463.24132.367.camel@bling.home> (Alex Williamson's message of "Mon, 18 Mar 2013 10:51:03 -0600")

Alex Williamson <alex.williamson@redhat.com> writes:

> At least for KVM the kernel fix is the addition of the vfio driver which
> gives us a non-sysfs way to do this.  If this problem was found a few
> years later and we were ready to make the switch I'd support just
> removing these resource files.  In the meantime we have userspace that
> depends on this interface, so I'm open to suggestions how to fix it.

I am puzzled by a couple of things in this discussion:

1) do you seriously mean that a userspace application (any, not just
   udevadm or qemu or whatever) should be able to read and write these
   registers while the device is owned by a driver?  How is that ever
   going to work?

2) is it really so that a device can be so fundamentally screwed up by
   reading some registers, that a later driver probe cannot properly
   reinitialize it?

I would have thought that the solution to all this was to return -EINVAL
on any attemt to read or write these files while a driver is bound to
the device.  If userspace is going to use the API, then the application
better unbind any driver first.

Or? Am I missing something here?

> If we want to blacklist this specific device, that's fine, but as others
> have pointed out it's really a class problem.  Perhaps we report 1 byte
> extra for the file length where EOF-1 is an enable byte?  Is there
> anything else in file ops that we could use to make it slightly more
> complicated than open(), read() to access the device?  Thanks,

If there really are devices which cannot handle reading at all, and
cannot be reset to a sane state by later driver initialization, then a
blacklist could be added for those devices.  This should not be a common
problem.



Bj√∏rn
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: "Bjørn Mork" <bjorn@mork.no>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: Greg KH <gregkh@linuxfoundation.org>, Kay Sievers <kay@vrfy.org>,
	Myron Stowe <mstowe@redhat.com>,
	Myron Stowe <myron.stowe@redhat.com>,
	linux-hotplug@vger.kernel.org, linux-pci@vger.kernel.org,
	yuxiangl@marvell.com, yxlraid@gmail.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] udevadm-info: Don't access sysfs 'resource<N>' files
Date: Mon, 18 Mar 2013 18:20:46 +0100	[thread overview]
Message-ID: <87obeg39qp.fsf@nemi.mork.no> (raw)
In-Reply-To: <1363625463.24132.367.camel@bling.home> (Alex Williamson's message of "Mon, 18 Mar 2013 10:51:03 -0600")

Alex Williamson <alex.williamson@redhat.com> writes:

> At least for KVM the kernel fix is the addition of the vfio driver which
> gives us a non-sysfs way to do this.  If this problem was found a few
> years later and we were ready to make the switch I'd support just
> removing these resource files.  In the meantime we have userspace that
> depends on this interface, so I'm open to suggestions how to fix it.

I am puzzled by a couple of things in this discussion:

1) do you seriously mean that a userspace application (any, not just
   udevadm or qemu or whatever) should be able to read and write these
   registers while the device is owned by a driver?  How is that ever
   going to work?

2) is it really so that a device can be so fundamentally screwed up by
   reading some registers, that a later driver probe cannot properly
   reinitialize it?

I would have thought that the solution to all this was to return -EINVAL
on any attemt to read or write these files while a driver is bound to
the device.  If userspace is going to use the API, then the application
better unbind any driver first.

Or? Am I missing something here?

> If we want to blacklist this specific device, that's fine, but as others
> have pointed out it's really a class problem.  Perhaps we report 1 byte
> extra for the file length where EOF-1 is an enable byte?  Is there
> anything else in file ops that we could use to make it slightly more
> complicated than open(), read() to access the device?  Thanks,

If there really are devices which cannot handle reading at all, and
cannot be reset to a sane state by later driver initialization, then a
blacklist could be added for those devices.  This should not be a common
problem.



Bjørn

  reply	other threads:[~2013-03-18 17:20 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-16 21:35 [PATCH] udevadm-info: Don't access sysfs entries backing device I/O port space Myron Stowe
2013-03-16 21:35 ` Myron Stowe
2013-03-16 21:35 ` [PATCH] udevadm-info: Don't access sysfs 'resource<N>' files Myron Stowe
2013-03-16 21:35   ` Myron Stowe
2013-03-16 22:11   ` Greg KH
2013-03-16 22:11     ` Greg KH
2013-03-16 22:55     ` Bjorn Helgaas
2013-03-16 22:55       ` Bjorn Helgaas
2013-03-16 23:50     ` Myron Stowe
2013-03-16 23:50       ` Myron Stowe
2013-03-17  1:03       ` Greg KH
2013-03-17  1:03         ` Greg KH
2013-03-17  4:11         ` Alex Williamson
2013-03-17  4:11           ` Alex Williamson
2013-03-17  5:36           ` Greg KH
2013-03-17  5:36             ` Greg KH
2013-03-17 13:38             ` Alex Williamson
2013-03-17 13:38               ` Alex Williamson
2013-03-17 14:00               ` Kay Sievers
2013-03-17 14:00                 ` Kay Sievers
2013-03-17 14:20                 ` Myron Stowe
2013-03-17 14:20                   ` Myron Stowe
2013-03-17 14:29                   ` Kay Sievers
2013-03-17 14:29                     ` Kay Sievers
2013-03-17 14:36                     ` Myron Stowe
2013-03-17 14:36                       ` Myron Stowe
2013-03-17 14:43                       ` Kay Sievers
2013-03-17 14:43                         ` Kay Sievers
2013-03-18 16:24                 ` Alex Williamson
2013-03-18 16:24                   ` Alex Williamson
2013-03-18 16:41                   ` Greg KH
2013-03-18 16:41                     ` Greg KH
2013-03-18 16:51                     ` Alex Williamson
2013-03-18 16:51                       ` Alex Williamson
2013-03-18 17:20                       ` Bjørn Mork [this message]
2013-03-18 17:20                         ` Bjørn Mork
2013-03-18 17:54                         ` Alex Williamson
2013-03-18 17:54                           ` Alex Williamson
2013-03-18 18:02                           ` Robert Brown
2013-03-18 18:02                             ` Robert Brown
2013-03-18 18:25                           ` Bjørn Mork
2013-03-18 18:25                             ` Bjørn Mork
2013-03-18 18:59                             ` Alex Williamson
2013-03-18 18:59                               ` Alex Williamson
2013-03-19 16:57                               ` Myron Stowe
2013-03-19 16:57                                 ` Myron Stowe
2013-03-19 17:06                                 ` Myron Stowe
2013-03-19 17:06                                   ` Myron Stowe
2013-03-17 14:33               ` Myron Stowe
2013-03-17 14:33                 ` Myron Stowe
2013-03-17 22:28                 ` Alex Williamson
2013-03-17 22:28                   ` Alex Williamson
2013-03-18 14:50                   ` Don Dutile
2013-03-18 14:50                     ` Don Dutile
2013-03-18 16:34                     ` Alex Williamson
2013-03-18 16:34                       ` Alex Williamson
2013-03-17 14:12         ` Myron Stowe
2013-03-17 14:12           ` Myron Stowe
2013-03-19  1:54         ` Robert Hancock
2013-03-19  1:54           ` Robert Hancock
2013-03-19  2:03           ` Greg KH
2013-03-19  2:03             ` Greg KH
2013-03-19  2:09             ` Robert Hancock
2013-03-19  2:09               ` Robert Hancock
2013-03-19  2:35               ` Greg KH
2013-03-19  2:35                 ` Greg KH
2013-03-19  3:08                 ` Robert Hancock
2013-03-19  3:08                   ` Robert Hancock

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=87obeg39qp.fsf@nemi.mork.no \
    --to=bjorn@mork.no \
    --cc=alex.williamson@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kay@vrfy.org \
    --cc=linux-hotplug@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mstowe@redhat.com \
    --cc=myron.stowe@redhat.com \
    --cc=yuxiangl@marvell.com \
    --cc=yxlraid@gmail.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.