linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: "Weitao Wang\(BJ-RD\)" <WeitaoWang@zhaoxin.com>,
	Greg KH <gregkh@linuxfoundation.org>,
	WeitaoWang-oc <WeitaoWang-oc@zhaoxin.com>,
	"mathias.nyman@linux.intel.com" <mathias.nyman@linux.intel.com>,
	"ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
	"vkoul@kernel.org" <vkoul@kernel.org>,
	"hslester96@gmail.com" <hslester96@gmail.com>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Carsten_Schmid@mentor.com" <Carsten_Schmid@mentor.com>,
	"efremov@linux.com" <efremov@linux.com>,
	"Tony W. Wang\(XA-RD\)" <TonyWWang@zhaoxin.com>,
	"Cobe Chen\(BJ-RD\)" <CobeChen@zhaoxin.com>,
	"Tim Guo\(BJ-RD\)" <TimGuo@zhaoxin.com>,
	"wwt8723@163.com" <wwt8723@163.com>
Subject: Re: [PATCH] USB:Fix kernel NULL pointer when unbind UHCI form vfio-pci
Date: Thu, 23 Jul 2020 12:38:35 -0400	[thread overview]
Message-ID: <20200723163835.GA1357775@rowland.harvard.edu> (raw)
In-Reply-To: <20200723101735.3222c289@w520.home>

On Thu, Jul 23, 2020 at 10:17:35AM -0600, Alex Williamson wrote:
> The IOMMU grouping restriction does solve the hardware issue, so long
> as one driver doesn't blindly assume the driver private data for
> another device and modify it.

Correction: The IOMMU grouping restriction solves the hardware issue for 
vfio-pci.  It won't necessarily help if some other driver comes along 
and wants to bind to this hardware.

>   I do agree that your solution would
> work, requiring all devices are unbound before any can be bound, but it
> also seems difficult to manage.  The issue is largely unique to USB
> AFAIK.  On the other hand, drivers coordinating with each other to
> register their _private_ data as share-able within a set of drivers
> seems like a much more direct and explicit interaction between the
> drivers.  Thanks,

Yes, that makes sense.  But it would have to be implemented in the 
driver core, not in particular subsystems like USB or PCI.  And it might 
be seen as overkill, given that only UHCI/OHCI/EHCI devices require this 
sort of sharing AFAIK.

Also, when you think about it, what form would such coordination among 
drivers take?  From your description, it sounds like the drivers would 
agree to avoid accessing each other's private data if the proper 
registration wasn't in place.

On the other hand, a stronger and perhaps more robust approach would be 
to enforce the condition that non-cooperating drivers are never bound to 
devices in the same group at the same time.  That's basically what I'm 
proposing here -- the question is whether the enforcement should be 
instituted in the kernel or should merely be part of a standard protocol 
followed by userspace drivers.

Given that it's currently needed in only one place, it seems reasonable 
to leave this as a "gentlemen's agreement" in userspace for the time 
being instead of adding it to the kernel.

Alan Stern

  reply	other threads:[~2020-07-23 16:38 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-22 11:57 [PATCH] USB:Fix kernel NULL pointer when unbind UHCI form vfio-pci WeitaoWangoc
2020-07-22 12:44 ` Greg KH
2020-07-22 14:59   ` Alan Stern
2020-07-23  2:59     ` 答复: " Weitao Wang(BJ-RD)
2020-07-23  4:18       ` Alex Williamson
2020-07-23 15:38         ` Alan Stern
2020-07-23 16:17           ` Alex Williamson
2020-07-23 16:38             ` Alan Stern [this message]
2020-07-24 12:57               ` 答复: " WeitaoWang-oc
2020-07-24 19:17                 ` Alex Williamson
2020-07-28  5:53                   ` 答复: " WeitaoWang-oc
2020-08-19  3:23               ` TimGuo-oc
2020-07-23  5:25       ` Greg KH
2020-07-23  2:58   ` Weitao Wang(BJ-RD)
2020-07-23  3:53 ` Alex Williamson
2020-07-23  8:36   ` 答复: " WeitaoWang-oc
2020-07-23  8:41     ` gregkh

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=20200723163835.GA1357775@rowland.harvard.edu \
    --to=stern@rowland.harvard.edu \
    --cc=Carsten_Schmid@mentor.com \
    --cc=CobeChen@zhaoxin.com \
    --cc=TimGuo@zhaoxin.com \
    --cc=TonyWWang@zhaoxin.com \
    --cc=WeitaoWang-oc@zhaoxin.com \
    --cc=WeitaoWang@zhaoxin.com \
    --cc=alex.williamson@redhat.com \
    --cc=efremov@linux.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hslester96@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@linux.intel.com \
    --cc=ulf.hansson@linaro.org \
    --cc=vkoul@kernel.org \
    --cc=wwt8723@163.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).