Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 24 Jul 2020 13:17:08 -0600
From:   Alex Williamson <alex.williamson@redhat.com>
To:     WeitaoWang-oc <WeitaoWang-oc@zhaoxin.com>
Cc:     Alan Stern <stern@rowland.harvard.edu>,
        Greg KH <gregkh@linuxfoundation.org>,
        "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
Message-ID: <20200724131708.0a0f3358@x1.home>
In-Reply-To: <11a7a3e67d6c40cd9fd06cd4d6300283@zhaoxin.com>
References: <1595419068-4812-1-git-send-email-WeitaoWang-oc@zhaoxin.com>
        <20200722124414.GA3153105@kroah.com>
        <20200722145913.GB1310843@rowland.harvard.edu>
        <1bf449377e3448bc9c8bc7b64d7b7990@zhaoxin.com>
        <20200722221817.542971a2@x1.home>
        <20200723153821.GC1352396@rowland.harvard.edu>
        <20200723101735.3222c289@w520.home>
        <20200723163835.GA1357775@rowland.harvard.edu>
        <11a7a3e67d6c40cd9fd06cd4d6300283@zhaoxin.com>
Organization: Red Hat
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 24 Jul 2020 12:57:49 +0000
WeitaoWang-oc <WeitaoWang-oc@zhaoxin.com> wrote:

> On Thu, 23 Jul 2020 12:38:21 -0400, Alan wrote:
> > 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.
> > 	  
> 
> Provided that EHCI and UHCI host controller declare not support P2P and
> ACS. So, we can assign EHCI and UHCI host controller to different IOMMU 
> group separately. We assign EHCI host controller to host and assign UHCI
> host controller to VM. Then, ehci_hcd driver load/unload operation in host
> will cause the same issue as discussed

And you have an example of such a device?  I expect these do not exist,
nor should they.  It seems like it would be an improper use of ACS.
Thanks,

Alex