Re: [PATCH v2 2/7] dma: Introduce dma_aligned_pow2_mask()

[Peter Xu]

On Thu, Feb 25, 2021 at 10:14:30AM +0100, Eric Auger wrote:
> @@ -296,4 +296,7 @@ uint64_t dma_buf_write(uint8_t *ptr, int32_t len, 
> QEMUSGList *sg);
>  void dma_acct_start(BlockBackend *blk, BlockAcctCookie *cookie,
>                      QEMUSGList *sg, enum BlockAcctType type);
>  
> +uint64_t dma_aligned_pow2_mask(uint64_t start, uint64_t end,
> +                               int max_addr_bits);
> +
>  #endif
> diff --git a/softmmu/dma-helpers.c b/softmmu/dma-helpers.c
> index 29001b5459..7d766a5e89 100644
> --- a/softmmu/dma-helpers.c
> +++ b/softmmu/dma-helpers.c
> @@ -330,3 +330,29 @@ void dma_acct_start(BlockBackend *blk, BlockAcctCookie 
> *cookie,
>  {
>      block_acct_start(blk_get_stats(blk), cookie, sg->size, type);
>  }
> +
> +uint64_t dma_aligned_pow2_mask(uint64_t start, uint64_t end, int 
> max_addr_bits)

Nitpick: If to make it a common function, shall we comment the interface to
avoid misusing it? E.g., start/end seem to be inclusive. Also, max_addr_bits
should be <=64 (and we can also assert in the code, maybe?).

> +{
> +    uint64_t max_mask = UINT64_MAX, addr_mask = end - start;
> +    uint64_t alignment_mask, size_mask;
> +
> +    if (max_addr_bits != 64) {
> +        max_mask = (1ULL << max_addr_bits) - 1;
> +    }
> +
> +    alignment_mask = start ? (start & -start) - 1 : max_mask;
> +    alignment_mask = MIN(alignment_mask, max_mask);
> +    size_mask = MIN(addr_mask, max_mask);
> +
> +    if (alignment_mask <= size_mask) {
> +        /* Increase the alignment of start */
> +        return alignment_mask;
> +    } else {
> +        /* Find the largest page mask from size */
> +        if (addr_mask == UINT64_MAX) {
> +            return UINT64_MAX;
> +        }
> +        return (1ULL << (63 - clz64(addr_mask + 1))) - 1;
> +    }
> +}
> +

In all cases, the code looks right to me:

Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks,

-- 
Peter Xu