RE: ** POTENTIAL FRAUD ALERT - RED HAT ** [PATCH v2 01/10] Drivers: hv: vmbus: Move Hyper-V page allocator to arch neutral code

[Michael Kelley <mikelley@microsoft.com>]

From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Tuesday, March 2, 2021 4:57 AM
> 
> Michael Kelley <mikelley@microsoft.com> writes:
> 
> > The Hyper-V page allocator functions are implemented in an architecture
> > neutral way.  Move them into the architecture neutral VMbus module so
> > a separate implementation for ARM64 is not needed.
> >
> > No functional change.
> >
> > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> > Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
> > ---
> >  arch/x86/hyperv/hv_init.c       | 22 ----------------------
> >  arch/x86/include/asm/mshyperv.h |  5 -----
> >  drivers/hv/hv.c                 | 36 ++++++++++++++++++++++++++++++++++++
> >  include/asm-generic/mshyperv.h  |  4 ++++
> >  4 files changed, 40 insertions(+), 27 deletions(-)
> >

[snip]

> >
> >  /*
> > + * Functions for allocating and freeing memory with size and
> > + * alignment HV_HYP_PAGE_SIZE. These functions are needed because
> > + * the guest page size may not be the same as the Hyper-V page
> > + * size. We depend upon kmalloc() aligning power-of-two size
> > + * allocations to the allocation size boundary, so that the
> > + * allocated memory appears to Hyper-V as a page of the size
> > + * it expects.
> > + */
> > +
> > +void *hv_alloc_hyperv_page(void)
> > +{
> > +	BUILD_BUG_ON(PAGE_SIZE <  HV_HYP_PAGE_SIZE);
> > +
> > +	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > +		return (void *)__get_free_page(GFP_KERNEL);
> > +	else
> > +		return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> 
> PAGE_SIZE and HV_HYP_PAGE_SIZE are known compile-time and in case this
> won't change in the future we can probably write this as
> 
> #if PAGE_SIZE == HV_HYP_PAGE_SIZE
>        return (void *)__get_free_page(GFP_KERNEL);
> #else
>        return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> #endif
> 
> (not sure if the output is going to be any different with e.g. gcc's '-O2')
> 

I looked at the generated code, and the compiler does the right
thing on both x86/x64 and on ARM64.  I'd rather leave the code
as is so that both legs of the 'if' statement get checked by the
compiler regardless of whether PAGE_SIZE == HV_HYP_PAGE_SIZE.

Michael

> > +}
> > +
> > +void *hv_alloc_hyperv_zeroed_page(void)
> > +{
> > +	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > +		return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> > +	else
> > +		return kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> > +}
> > +
> > +void hv_free_hyperv_page(unsigned long addr)
> > +{
> > +	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > +		free_page(addr);
> > +	else
> > +		kfree((void *)addr);
> > +}
> > +
> > +/*