On 09/07/2011 03:44 PM, Greg KH wrote:
On Wed, Sep 07, 2011 at 09:19:19PM +0200, Joerg Roedel wrote:
Hi Greg,
the bus_set_iommu() function will be called by the IOMMU driver. There
can be different drivers for the same bus, depending on the hardware. On
PCI for example, there can be the Intel or the AMD IOMMU driver that
implement the iommu-api and that register for that bus.
Why are you pushing this down into the driver core? What other busses
becides PCI use/need this?
If you can have a different IOMMU driver on the same bus, then wouldn't
this be a per-device thing instead of a per-bus thing?
And given the dma api takes a struct device *, it'd be more efficient
to be tied into the device structure.
Device structure would get iommu ops set by parent(bus);
if a bus (segment) doesn't provide a unique/different/layered IOMMU
then the parent bus, it inherits the parent's iommu-ops.
setting the iommu-ops in the root bus struct, seeds the iommu-ops
for the (PCI) tree.
For intel & amd IOMMUs, in early pci (bios,root?) init, you would
seed the pci root busses with appropriate IOMMU support (based on
dmar/drhd & ivrs/ivhd data structures, respectively), and
then modify the PCI code to do the inheritence (PPB code inherits
unless specific device driver for a given PPB vid-did loads a
different iommu-ops for that segment/branch).
This would enable different types of IOMMUs for different devices
(or PCI segments, or branches of PCI trees) that are designed for
different tasks -- simple IOMMUs for legacy devices; complicated, io-page-faulting
IOMMUs for plug-in, high-end devices on virtualizing servers for PCI (SRIOV) endpoints.
and as Greg indicates, is only relevant to PCI.
The catch is that dev* has to be looked at for iommu support for dma-ops.
On Wed, Sep 07, 2011 at 11:47:50AM -0700, Greg KH wrote:
+#ifdef CONFIG_IOMMU_API
+int bus_set_iommu(struct bus_type *bus, struct iommu_ops *ops)
+{
+ if (bus->iommu_ops != NULL)
+ return -EBUSY;
Busy?
Yes, it signals to the IOMMU driver that another driver has already
registered for that bus. In the previous register_iommu() interface this
was just a BUG(), but I think returning an error to the caller is
better. It can be turned back into a BUG() if it is considered better,
though.
Can you ever have more than one IOMMU driver per bus? If so, this seems
wrong (see above.)
+
+ bus->iommu_ops = ops;
+
+ /* Do IOMMU specific setup for this bus-type */
+ iommu_bus_init(bus, ops);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(bus_set_iommu);
I don't understand what this function is for, and who would call it.
It is called by the IOMMU driver.
Please provide kerneldoc that explains this.
Will do.
@@ -67,6 +68,9 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *);
* @resume: Called to bring a device on this bus out of sleep mode.
* @pm: Power management operations of this bus, callback the specific
* device driver's pm-ops.
+ * @iommu_ops IOMMU specific operations for this bus, used to attach IOMMU
+ * driver implementations to a bus and allow the driver to do
+ * bus-specific setup
So why is this just not set by the bus itself, making the above function
not needed at all?
The IOMMUs are usually devices on the bus itself, so they are
initialized after the bus is set up and the devices on it are
populated. So the function can not be called on bus initialization
because the IOMMU is not ready at this point.
Ok, that makes more sense, please state as much in the documentation.
thanks,
greg k-h
_______________________________________________
iommu mailing list
iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/iommu
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
