Michael S. Tsirkin
2021-Sep-11 23:54 UTC
[PATCH v4 11/15] pci: Add pci_iomap_shared{,_range}
On Fri, Sep 10, 2021 at 09:34:45AM -0700, Andi Kleen wrote:> > > that's why > > > an extra level of defense of ioremap opt-in is useful. > > OK even assuming this, why is pci_iomap opt-in useful? > > That never happens before probe - there's simply no pci_device then. > > > Hmm, yes that's true. I guess we can make it default to opt-in for > pci_iomap. > > It only really matters for device less ioremaps.OK. And same thing for other things with device, such as devm_platform_ioremap_resource. If we agree on all that, this will basically remove virtio changes from the picture ;) -- MST
Michael S. Tsirkin
2021-Sep-13 05:53 UTC
[PATCH v4 11/15] pci: Add pci_iomap_shared{,_range}
On Sat, Sep 11, 2021 at 07:54:43PM -0400, Michael S. Tsirkin wrote:> On Fri, Sep 10, 2021 at 09:34:45AM -0700, Andi Kleen wrote: > > > > that's why > > > > an extra level of defense of ioremap opt-in is useful. > > > OK even assuming this, why is pci_iomap opt-in useful? > > > That never happens before probe - there's simply no pci_device then. > > > > > > Hmm, yes that's true. I guess we can make it default to opt-in for > > pci_iomap. > > > > It only really matters for device less ioremaps. > > OK. And same thing for other things with device, such as > devm_platform_ioremap_resource. > If we agree on all that, this will basically remove virtio > changes from the picture ;)Something else that was pointed out to me: fs->window_kaddr = devm_memremap_pages(&vdev->dev, pgmap); if (IS_ERR(fs->window_kaddr)) return PTR_ERR(fs->window_kaddr); looks like if we forget to set the shared flag then it will corrupt the DAX data?> -- > MST >
>> Hmm, yes that's true. I guess we can make it default to opt-in for >> pci_iomap. >> >> It only really matters for device less ioremaps. > OK. And same thing for other things with device, such as > devm_platform_ioremap_resource. > If we agree on all that, this will basically remove virtio > changes from the picture ;)Hi we revisited this now. One problem with removing the ioremap opt-in is that it's still possible for drivers to get at devices without going through probe. For example they can walk the PCI device list. Some drivers do that for various reasons. So if we remove the opt-in we would need to audit and possibly fix all that, which would be potentially a lot of churn. That's why I think it's better to keep the opt-in. -Andi