Re: [Xen-devel] [PATCH v2 2/2] xen/arm: introduce XENFEAT_grant_map_identity

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.07.14 at 19:19, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> Changes in v2:
> - rename XENFEAT_grant_map_11 to XENFEAT_grant_map_identity;
> - remove superfluous ifdef CONFIG_ARM in xen/common/kernel.c;

Looks like you forgot to add an is_domain_direct_mapped() stub for
x86, breaking the build there?

> @@ -727,7 +727,7 @@ __gnttab_map_grant_ref(
>  
>      double_gt_lock(lgt, rgt);
>  
> -    if ( gnttab_need_iommu_mapping(ld) )
> +    if ( gnttab_need_iommu_mapping(ld) || is_domain_direct_mapped(ld) )

This is bogus: On x86 the right part is always false, while on
ARM the right part is already part of gnttab_need_iommu_mapping().
IOW no need to change anything here afaict.

> @@ -738,13 +738,23 @@ __gnttab_map_grant_ref(
>               !(old_pin & (GNTPIN_hstw_mask|GNTPIN_devw_mask)) )
>          {
>              if ( wrc == 0 )
> -                err = iommu_map_page(ld, frame, frame,
> -                                     IOMMUF_readable|IOMMUF_writable);
> +            {
> +                if ( gnttab_need_iommu_mapping(ld) )
> +                    err = iommu_map_page(ld, frame, frame,
> +                            IOMMUF_readable|IOMMUF_writable);
> +                else
> +                    err = arch_grant_map_page_identity(ld, frame, true);

Irrespective of the above this is inefficient too: If you used
!is_domain_direct_mapped() as the conditional here, the compiler
would be able to eliminate the arch_grant_map_page_identity()
path on x86, making it unnecessary to have the stub as an inline
function (a simple declaration without definition would then
suffice as long as we don't ever build with -Od, which we already
depend upon elsewhere).

Jan


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