Re: [Xen-devel] [PATCH v3 38/39] ARM: new VGIC: Allocate two pages for struct vcpu

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andre,

On 03/21/2018 04:32 PM, Andre Przywara wrote:
> At the moment we allocate exactly one page for struct vcpu on ARM, also
> have a check in place to prevent it growing beyond 4KB.
> As the struct includes the state of all 32 private (per-VCPU) interrupts,
> we are at 3840 bytes on arm64 at the moment already. Growing the per-IRQ
> VGIC structure even slightly makes the VCPU quickly exceed the 4K limit.
> The new VGIC will need more space per virtual IRQ. I spent a few hours
> trying to trim this down, but couldn't get it below 4KB, even with the
> nasty hacks piling up to save some bytes here and there.
> It turns out that beyond efficiency, maybe, there is no real technical
> reason this struct has to fit in one page, so lifting the limit to two
> pages seems like the most pragmatic solution.
> Restrict this to compiling with the new VGIC and for ARM64 only.
>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
> ---
> Changelog v2 ... v3:
> - rework alloc_vcpu_struct() to avoid nasty #ifdef
>
> Changelog v1 ... v2:
> - confine change to new VGIC and ARM64 only
>
>   xen/arch/arm/domain.c | 25 +++++++++++++++++++++----
>   1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 9688e62f78..23bda3f7db 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -505,19 +505,36 @@ void dump_pageframe_info(struct domain *d)
>   
>   }
>   
> +/*
> + * The new VGIC has a bigger per-IRQ structure, so we need more than one
> + * page on ARM64. Cowardly increase the limit in this case.
> + */
> +#if defined(CONFIG_NEW_VGIC) && defined(CONFIG_ARM_64)
> +#define PAGES_PER_VCPU  2
> +#else
> +#define PAGES_PER_VCPU  1
> +#endif
> +
>   struct vcpu *alloc_vcpu_struct(void)
>   {
>       struct vcpu *v;
> -    BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE);
> -    v = alloc_xenheap_pages(0, 0);
> +
> +    BUILD_BUG_ON(sizeof(*v) > PAGES_PER_VCPU * PAGE_SIZE);
> +    v = alloc_xenheap_pages(get_order_from_pages(PAGES_PER_VCPU), 0);
I was suggesting to use get_order_from_pages(sizeof (...)) so if we end 
up to be smaller, you don't lose a page for nothing. But I am ok with 
that too and can revisit later. So:
Acked-by: Julien Grall <julien.grall@xxxxxxx>

>       if ( v != NULL )
> -        clear_page(v);
> +    {
> +        unsigned int i;
> +
> +        for ( i = 0; i < PAGES_PER_VCPU; i++ )
> +            clear_page((void *)v + i * PAGE_SIZE);
> +    }
> +
>       return v;
>   }
>   
>   void free_vcpu_struct(struct vcpu *v)
>   {
> -    free_xenheap_page(v);
> +    free_xenheap_pages(v, get_order_from_pages(PAGES_PER_VCPU));
>   }
>   
>   int vcpu_initialise(struct vcpu *v)
Cheers,

--
Julien Grall

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