Re: [Xen-devel] [PATCH v2 1/4] amd-iommu: add flush iommu_ops

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.12.18 at 18:40, <paul.durrant@xxxxxxxxxx> wrote:
> +static unsigned long flush_count(unsigned long dfn, unsigned int page_count,
> +                                 unsigned int order)
> +{
> +    unsigned long start = dfn / (1u << order);
> +    unsigned long end = DIV_ROUND_UP(dfn + page_count, (1u << order));

Luckily this in not in generic code, so the anomaly at the upper address
space end is not going to surface, and in particular not cause ...

> +    ASSERT(end > start);

... this to trigger. I therefore nevertheless wonder whether it
would't be better to use

    unsigned long start = dfn >> order;
    unsigned long end = (dfn + page_count - 1) >> order) + 1;

instead.

> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -52,6 +52,11 @@ static inline bool_t dfn_eq(dfn_t x, dfn_t y)
>      return dfn_x(x) == dfn_x(y);
>  }
>  
> +static inline bool_t dfn_lt(dfn_t x, dfn_t y)
> +{
> +    return dfn_x(x) < dfn_x(y);
> +}

The revision log says this is gone ...

With it really gone, and irrespective of the other comment
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Of course one or both adjustments could be easily done while
committing, provided you agree and provided there's no other
need for a v3.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel