Re: [PATCH v7 08/12] KVM: arm64: Add KVM_CAP_ARM_EAGER_SPLIT_CHUNK_SIZE

[Ricardo Koller <ricarkol@google.com>]

On Mon, Apr 24, 2023 at 07:14:21PM +0800, Gavin Shan wrote:
> On 4/24/23 4:27 AM, Ricardo Koller wrote:
> > On Mon, Apr 17, 2023 at 03:04:47PM +0800, Gavin Shan wrote:
> > > On 4/9/23 2:29 PM, Ricardo Koller wrote:
> > > > Add a capability for userspace to specify the eager split chunk size.
> > > > The chunk size specifies how many pages to break at a time, using a
> > > > single allocation. Bigger the chunk size, more pages need to be
> > > > allocated ahead of time.
> > > > 
> > > > Suggested-by: Oliver Upton <oliver.upton@linux.dev>
> > > > Signed-off-by: Ricardo Koller <ricarkol@google.com>
> > > > ---
> > > >    Documentation/virt/kvm/api.rst       | 28 ++++++++++++++++++++++++++
> > > >    arch/arm64/include/asm/kvm_host.h    | 15 ++++++++++++++
> > > >    arch/arm64/include/asm/kvm_pgtable.h | 18 +++++++++++++++++
> > > >    arch/arm64/kvm/arm.c                 | 30 ++++++++++++++++++++++++++++
> > > >    arch/arm64/kvm/mmu.c                 |  3 +++
> > > >    include/uapi/linux/kvm.h             |  2 ++
> > > >    6 files changed, 96 insertions(+)
> > > > 
> > > 
> > > With the following comments addressed:
> > > 
> > > Reviewed-by: Gavin Shan <gshan@redhat.com>
> > > 
> 
> [...]
> 
> > > 
> > > > +static inline bool kvm_is_block_size_supported(u64 size)
> > > > +{
> > > > +	bool is_power_of_two = !((size) & ((size)-1));
> > > > +
> > > > +	return is_power_of_two && (size & kvm_supported_block_sizes());
> > > > +}
> > > > +
> > > 
> > > IS_ALIGNED() maybe used here.
> > 
> > I've been trying to reuse some bitmap related function in the kernel,
> > like IS_ALIGNED(), but can't find anything. Or at least it doesn't occur
> > to me how.
> > 
> > kvm_is_block_size_supported() returns true if @size matches only one of
> > the bits set in kvm_supported_block_sizes(). For example, given these
> > supported sizes: 10000100001000.
> > 
> > kvm_is_block_size_supported(100000000) => true
> > kvm_is_block_size_supported(1100) => false
> > 
> 
> I was actually thinking of @is_power_of_two is replaced by IS_ALIGNED(),
> For example:
> 
> static inline bool kvm_is_block_size_supported(u64 size)
> {
>     return IS_ALIGNED(size, size) && (size & kvm_supported_block_sizes());
> }
> 
> IS_ALIGNED() is defined in include/linux/align.h, as below. It's almost
> similar to '((size) & ((size)-1))'
> 
> #define IS_ALIGNED(x, a)                (((x) & ((typeof(x))(a) - 1)) == 0)

Ah! you are right, yes, will use this instead.

Thanks,
Ricardo

> 
> Thanks,
> Gavin
>