RFC: extend iommu_ops with domain attributes API

RFC: extend iommu_ops with domain attributes API

[Stuart Yoder]

As we work on mapping the Freescale IOMMU (called PAMU) into the existing
Linux iommu infrastructure, one issue is that we have additional domain
attributes that need to be set.   This was discussed briefly a month ago
or so and I believe there was a need for a similar mechanism by IBM.

We are proposing a couple of APIs to be added to iommu_ops to
get/set domain attributes:

   int domain_set_attr(struct iommu_domain *domain, int attr_type, void *data);
   int domain_get_attr(struct iommu_domain *domain, int attr_type, void *data);

A couple of the attributes I'm considering PAMU specific with a generic
enable attribute:

   enum iommu_attr_type {
       IOMMU_ATTR_PAMU_GEOMETRY,       // the PAMU geometry for the domain
       IOMMU_ATTR_PAMU_STASH,          // stash characteristics for a domain
       IOMMU_ATTR_ENABLE
   };

   The data for each attribute value is defined as:

   IOMMU_ATTR_PAMU_GEOMETRY - data is a struct:

      struct dma_geometry_attr {
          u64 iova;          // iova of the DMA domain
          u32 size;          // must be a power of 2 and be greater
                             // or equal than 4KB.
          u32 subwin_cnt;    // power of 2 between 1 and 256
      };

   IOMMU_ATTR_PAMU_STASH - data is a struct:

      struct dma_stash_attr {
          u32 vcpu;      // cpu number
          u32 cache;     // cache to stash to: 1=L1,2=L2,3=L3
      }

   IOMMU_ATTR_ENABLE - data is a u32:

       0=domain is disabled
       1=domain is enabled

Would like any feedback you have.

Thanks,
Stuart

Re: RFC: extend iommu_ops with domain attributes API

[Joerg Roedel]

On Tue, Jan 10, 2012 at 10:22:36AM -0600, Stuart Yoder wrote:
> As we work on mapping the Freescale IOMMU (called PAMU) into the existing
> Linux iommu infrastructure, one issue is that we have additional domain
> attributes that need to be set.   This was discussed briefly a month ago
> or so and I believe there was a need for a similar mechanism by IBM.
> 
> We are proposing a couple of APIs to be added to iommu_ops to
> get/set domain attributes:
> 
>    int domain_set_attr(struct iommu_domain *domain, int attr_type, void *data);
>    int domain_get_attr(struct iommu_domain *domain, int attr_type, void *data);

Yes, something similar to that interface is required to support
GART-like IOMMUs too.
I prefer to split the attr-type into generic ones supported by many
IOMMU drivers and implementation specific ones required by your PAMU for
example.

> A couple of the attributes I'm considering PAMU specific with a generic
> enable attribute:
> 
>    enum iommu_attr_type {
>        IOMMU_ATTR_PAMU_GEOMETRY,       // the PAMU geometry for the domain

This should be a generic attribute. It makes sense for nearly all IOMMUs
to export geometry information.

>        IOMMU_ATTR_PAMU_STASH,          // stash characteristics for a domain

That one is fine.

>        IOMMU_ATTR_ENABLE

I do not get the need of this one. Can you explain why you need to
enable/disable a domain? What happens on the hardware side when you do
that?


	Joerg

Re: RFC: extend iommu_ops with domain attributes API

[Scott Wood]

On Thu, Jan 12, 2012 at 03:47:47PM +0100, Joerg Roedel wrote:
> On Tue, Jan 10, 2012 at 10:22:36AM -0600, Stuart Yoder wrote:
> > A couple of the attributes I'm considering PAMU specific with a generic
> > enable attribute:
> > 
> >    enum iommu_attr_type {
> >        IOMMU_ATTR_PAMU_GEOMETRY,       // the PAMU geometry for the domain
> 
> This should be a generic attribute. It makes sense for nearly all IOMMUs
> to export geometry information.

The type of geometry information expected is not generic, though.

> >        IOMMU_ATTR_ENABLE
> 
> I do not get the need of this one. Can you explain why you need to
> enable/disable a domain? What happens on the hardware side when you do
> that?

When assigning to a KVM guest (or userspace driver) a device that we
can't generically reset or disable bus-mastering, we don't want DMA to go
through until the guest has reset or otherwise quiesced the device, and
told the HV that it's ready for DMA to work.  When we reset the guest, we
stop DMA before reloading the OS image, and again enable it when the
guest driver says it's OK.

While it's possible to accomplish this by only having mappings when it's
OK for DMA to work, this is undesireable because when configuring static
mappings for a guest, we want to know during guest configuration if
the mappings are invalid, rather than at some later point during guest
execution.  PAMU has significant restrictions on what sort of mappings
can be done, especially if we're trying to map a guest's entire memory. 

At the hardware level we can accomplish the enable/disable by flipping a
bit; we'd like for it to be as simple for the IOMMU API user.  Even
ignoring the issue of delayed errors, it's more complicated to have to
remember mappings and reissue them when the guest says OK, and tear them
down on reset.

If this isn't wanted as a generic IOMMU API feature, it could be a
PAMU implementation attribute.

-Scott

Re: RFC: extend iommu_ops with domain attributes API

[Stuart Yoder]

On Thu, Jan 12, 2012 at 10:41 AM, Scott Wood <scottwood@freescale.com> wrote:
> On Thu, Jan 12, 2012 at 03:47:47PM +0100, Joerg Roedel wrote:
>> On Tue, Jan 10, 2012 at 10:22:36AM -0600, Stuart Yoder wrote:
>> > A couple of the attributes I'm considering PAMU specific with a generic
>> > enable attribute:
>> >
>> >    enum iommu_attr_type {
>> >        IOMMU_ATTR_PAMU_GEOMETRY,       // the PAMU geometry for the domain
>>
>> This should be a generic attribute. It makes sense for nearly all IOMMUs
>> to export geometry information.

Is the generic geometry simply iova and size (I'm not familiar with the other
iommus out there)?    Given that the subwindow count is PAMU specific,
I'm wondering
whether we want that grouped with the general geometry fields or split
out as a separate attribute.

Stuart

Re: RFC: extend iommu_ops with domain attributes API

[Alex Williamson]

On Tue, 2012-01-10 at 10:22 -0600, Stuart Yoder wrote:
> As we work on mapping the Freescale IOMMU (called PAMU) into the existing
> Linux iommu infrastructure, one issue is that we have additional domain
> attributes that need to be set.   This was discussed briefly a month ago
> or so and I believe there was a need for a similar mechanism by IBM.
> 
> We are proposing a couple of APIs to be added to iommu_ops to
> get/set domain attributes:
> 
>    int domain_set_attr(struct iommu_domain *domain, int attr_type, void *data);
>    int domain_get_attr(struct iommu_domain *domain, int attr_type, void *data);
> 
> A couple of the attributes I'm considering PAMU specific with a generic
> enable attribute:
> 
>    enum iommu_attr_type {
>        IOMMU_ATTR_PAMU_GEOMETRY,       // the PAMU geometry for the domain
>        IOMMU_ATTR_PAMU_STASH,          // stash characteristics for a domain
>        IOMMU_ATTR_ENABLE
>    };
> 
>    The data for each attribute value is defined as:
> 
>    IOMMU_ATTR_PAMU_GEOMETRY - data is a struct:
> 
>       struct dma_geometry_attr {
>           u64 iova;          // iova of the DMA domain
>           u32 size;          // must be a power of 2 and be greater
>                              // or equal than 4KB.
>           u32 subwin_cnt;    // power of 2 between 1 and 256

Agree with Joerg's comments.  size/subwin count should probably both be
u64 if generalized and the restrictions would be iommu dependent.  Is
this also where we should expose a bitmap of available mapping sizes?
Thanks,

Alex