Alex Williamson <alex.williamson@xxxxxxxxxx> wrote: >On Mon, 2013-03-18 at 18:20 +0100, Bjørn Mork wrote: >> Alex Williamson <alex.williamson@xxxxxxxxxx> 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? > >The expectation is that the user doesn't mess with the device through >pci-sysfs while it's running. This is really no different than config >space or MMIO space in that respect. But it is. That's the problem. As a user I expect to be able to run e.g "grep . /sys/devices/whatever/*" with no ill effects. This holds for config space or MMIO space. It does not for any reset-on-read register. > You can use setpci to break your >PCI card while it's used by the driver today. The difference is that >MMIO spaces side-step the issue by only allowing mmap and config space >is known not to have read side-effects. Yes. And that is why there is no problem exporting those. This difference is fundamental. >> 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? > >Never underestimate how broken hardware can be, True :) > though in this case >reading a device register seems to be causing a system hang/reset. I understand that it does so if the ahci driver is bound to the device while reading the registers, but does it also hang the system with no bound driver? How does it do that? By killing the bus? >> 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? > >That doesn't really solve anything though. Let's pretend the resource >files only work while the device is bound to pci-stub. Now what >happens >when you run this udevadm command as admin while it's in use by the >userspace driver? All we've done is limit the scope of the problem. Assuming that the system hangs without driver help and that this brokenness is widespread. I don't think any of those assumptions hold. Do they? >> > 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. > >Yes, if these are dead registers, let's blacklist and move along. I >suspect though that these registers probably work fine if you access >them according to the device programming model, so blacklisting just >prevents full use through something like KVM device assignment. Well, if the device is that broken then I think it will require the kernel to police the device programming. I don't see how you can leave a bomb like that because it might be useful in a rare and very theoretical case. Easier to just blacklist it... Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
