Re: [Xen-devel] [PATCH v2 44/45] ARM: new VGIC: Allocate two pages for struct vcpu

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andre,

Title: Please update it.

On 03/15/2018 08:30 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 v1 ... v2:
> - confine change to new VGIC and ARM64 only
>
>   xen/arch/arm/domain.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 18b915d2e9..3fba05bda5 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -508,10 +508,25 @@ void dump_pageframe_info(struct domain *d)
>   struct vcpu *alloc_vcpu_struct(void)
>   {
>       struct vcpu *v;
> +
> +    /*
> +     * 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)
> +    BUILD_BUG_ON(sizeof(*v) > 2 * PAGE_SIZE);
> +    v = alloc_xenheap_pages(1, 0);
> +    if ( v != NULL ) {
> +        clear_page(v);
> +        clear_page((void *)v + PAGE_SIZE);
> +    }
> +#else
>       BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE);
>       v = alloc_xenheap_pages(0, 0);
>       if ( v != NULL )
>           clear_page(v);
> +#endif.
All this logic can just be simplified using get_order_* helpers. This 
will avoid #ifdef like that in alloc_vcpu_struct which is really really 
ugly.

Cheers,

--
Julien Grall

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