Re: [Xen-devel] [PATCH v3 3/3] xen/arm: introduce XENFEAT_grant_map_identity

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 24 Jul 2014, Julien Grall wrote:
> Hi Stefano,
> 
> On 07/24/2014 02:31 PM, Stefano Stabellini wrote:
> > diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> > index 7e83353..dacbe38 100644
> > --- a/xen/common/kernel.c
> > +++ b/xen/common/kernel.c
> > @@ -15,6 +15,7 @@
> >  #include <xen/guest_access.h>
> >  #include <xen/hypercall.h>
> >  #include <asm/current.h>
> > +#include <asm/grant_table.h>
> >  #include <public/nmi.h>
> >  #include <public/version.h>
> >  
> > @@ -325,6 +326,8 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) 
> > arg)
> >                  break;
> >              }
> >  #endif
> > +            if ( gnttab_need_identity_mapping(d) )
> 
> Actually even platform the IOMMU support needs to have this flags on.
> With this solution you break platform where not every DMA-capable device
> are behind an SMMU.

I see.


> > +                fi.submap |= 1U << XENFEAT_grant_map_identity;
> >              break;
> >          default:
> >              return -EINVAL;
> > diff --git a/xen/include/asm-arm/grant_table.h 
> > b/xen/include/asm-arm/grant_table.h
> > index eac8a70..6f7ccd9 100644
> > --- a/xen/include/asm-arm/grant_table.h
> > +++ b/xen/include/asm-arm/grant_table.h
> > @@ -36,6 +36,9 @@ static inline int replace_grant_supported(void)
> >  #define gnttab_need_iommu_mapping(d)                    \
> >      (is_domain_direct_mapped(d) && need_iommu(d))
> >  
> > +#define gnttab_need_identity_mapping(d)                    \
> > +    (is_domain_direct_mapped(d) && !need_iommu(d))
> > +
> 
> Why didn't you drop the need_iommu(d) in is_domain_direct_mapped?

I guess you mean why didn't you drop the need_iommu(d) in
gnttab_need_iommu_mapping?

Because how can you need an iommu mapping if an iommu is not present?


> Hence the name is confusing. You may think that we don't need identity
> mapping when IOMMU is used while it's actually the case, but we use a
> different function.

Yes, you are right. The whole thing is a bit confusing, I am trying to
come up with a naming scheme that is as clear as possible.
What if we simply define:

#define gnttab_need_identity_mapping(d) (is_domain_direct_mapped(d))

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel