Messages in this thread | | | Date | Fri, 27 Jan 2012 11:26:08 +0100 | From | Joerg Roedel <> | Subject | Re: [PATCH 2/6] iommu/amd: Implement DOMAIN_ATTR_GEOMETRY attribute |
| |
Hi Ohad,
On Fri, Jan 27, 2012 at 09:08:44AM +0200, Ohad Ben-Cohen wrote: > On Thu, Jan 26, 2012 at 8:40 PM, Joerg Roedel <joerg.roedel@amd.com> wrote: > > Implement the attribute itself and add the code for the > > AMD IOMMU driver. > > It's somewhat non-intuitive to have the generic attribute code bundled > with the AMD implementation in the same patch (took me some time to > find it here :). > > Maybe split this to two patches ?
Yes, that is not ideal. I'll probably split it or move the generic part to the first patch.
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > > index ef54718..3d0b0bf 100644 > > --- a/drivers/iommu/iommu.c > > +++ b/drivers/iommu/iommu.c > > @@ -345,10 +345,23 @@ EXPORT_SYMBOL_GPL(iommu_device_group); > > int iommu_domain_get_attr(struct iommu_domain *domain, > > enum iommu_attr attr, void *data) > > { > > - if (!domain->ops->domain_get_attr) > > - return -EINVAL; > > + struct iommu_domain_geometry *geometry; > > + int ret = 0; > > + > > + switch (attr) { > > + case DOMAIN_ATTR_GEOMETRY: > > + geometry = data; > > + *geometry = domain->geometry; > > This is not type-safe and will surely bite someone some day. > > Sure, we can't have type safety in a multi-purpose generic interface, > but I wonder whether this is the right interface for getting the > geometry ? I.e. why not have a dedicated API for this ?
I fell also uncomfortable with the missing type-safety of this interface. But the alternative is to have dedicated functions for set/get each attribute. Well, it depends on how many attributes we have in the end, but given that the PAMU guys already have need for a number of hardware specific attributes it is likely that having individual functions makes the api too complex in the end.
But probably we can replace the 'void *data' with a 'union domain_attr'? This will give us some type-safety.
Joerg
-- AMD Operating System Research Center
Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |