As it was suggested in [1] it would be better to allocate one page for struct vcpu for all arch-es. To do that it is needed to align Arm code to allocate one page (as there is a case(when CONFIG_NEW_VGIC=y) when Arm64 will require to allocate two pages). As a result, the following patches for Arm have been introduced: - [PATCH v2 1/4] xen/arm: optimize the size of struct vcpu - [PATCH v2 2/4] xen/arm: drop MAX_PAGES_PER_VCPU This patches are dependency for: - [PATCH v2 3/4] xen: move alloc/free_vcpu_struct() to common code Also, as a part of this patch series another clean up is done which makes {alloc,free}_domain_struct() static. [1] https://lore.kernel.org/xen-devel/f8a9be3a-a0c6-496a-806f-40760dca5aee@suse.com/T/#m275dfcbdccef0461fa9a8acef072403f18091768 CI: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2228022495 --- Changes in v3: - Come up with a different way to optimize struct vcpu for Arm. - Merge patches "[PATCH v2 2/4] xen/arm: drop MAX_PAGES_PER_VCPU]" and "[PATCH v2 4/4] xen/common: make {alloc,free}_domain_struct() static" together. - Clean up vcpu_vgic_free() a little bit. --- Changes in v2: - Introduce new patches for Arm: - [PATCH v2 1/4] xen/arm: optimize the size of struct vcpu - [PATCH v2 2/4] xen/arm: drop MAX_PAGES_PER_VCPU to allocate one page for struct vcpu in common code for all the arch-es. - Introduce patch to clean up xen/domain.h a little bit: - [PATCH v2 4/4] xen/common: make {alloc,free}_domain_struct() static - Address the comments from v1: - [PATCH v2 3/4] xen: move alloc/free_vcpu_struct() to common code --- Oleksii Kurochko (4): xen/arm: vcpu_vgic_free() updates xen/arm: optimize the size of struct vcpu xen: move alloc/free_vcpu_struct() to common code xen/common: make {alloc,free}_domain_struct() static xen/arch/arm/domain.c | 32 --------------- xen/arch/arm/include/asm/new_vgic.h | 2 +- xen/arch/arm/include/asm/vgic.h | 2 +- xen/arch/arm/vgic.c | 5 +-- xen/arch/arm/vgic/vgic-init.c | 9 ++++- xen/arch/ppc/stubs.c | 10 ----- xen/arch/riscv/stubs.c | 10 ----- xen/arch/x86/domain.c | 24 ----------- xen/arch/x86/include/asm/domain.h | 12 ++++++ xen/common/domain.c | 62 +++++++++++++++++++---------- xen/include/xen/domain.h | 8 ---- 11 files changed, 64 insertions(+), 112 deletions(-) -- 2.52.0 Use XFREE() instead of xfree() so that vcpu_vgic_free() can be idempotent. With XFREE(), vgic_vcpu->private_irqs is set to NULL, so calling vcpu_vgic_free() a second time is not an issue. Update the prototype of vcpu_vgic_free() to return void to satisfy MISRA Rule 17.7, since the return value of vcpu_vgic_free() is not used by any callers. Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Change in v3: - New patch. --- xen/arch/arm/include/asm/vgic.h | 2 +- xen/arch/arm/vgic.c | 5 ++--- xen/arch/arm/vgic/vgic-init.c | 4 +--- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/xen/arch/arm/include/asm/vgic.h b/xen/arch/arm/include/asm/vgic.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/arm/include/asm/vgic.h +++ b/xen/arch/arm/include/asm/vgic.h @@ -XXX,XX +XXX,XX @@ int domain_vgic_register(struct domain *d, unsigned int *mmio_count); int domain_vgic_init(struct domain *d, unsigned int nr_spis); void domain_vgic_free(struct domain *d); int vcpu_vgic_init(struct vcpu *v); -int vcpu_vgic_free(struct vcpu *v); +void vcpu_vgic_free(struct vcpu *v); void vgic_inject_irq(struct domain *d, struct vcpu *v, unsigned int virq, bool level); diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/arm/vgic.c +++ b/xen/arch/arm/vgic.c @@ -XXX,XX +XXX,XX @@ int vcpu_vgic_init(struct vcpu *v) return 0; } -int vcpu_vgic_free(struct vcpu *v) +void vcpu_vgic_free(struct vcpu *v) { - xfree(v->arch.vgic.private_irqs); - return 0; + XFREE(v->arch.vgic.private_irqs); } struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int virq) diff --git a/xen/arch/arm/vgic/vgic-init.c b/xen/arch/arm/vgic/vgic-init.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/arm/vgic/vgic-init.c +++ b/xen/arch/arm/vgic/vgic-init.c @@ -XXX,XX +XXX,XX @@ void domain_vgic_free(struct domain *d) dist->nr_spis = 0; } -int vcpu_vgic_free(struct vcpu *v) +void vcpu_vgic_free(struct vcpu *v) { struct vgic_cpu *vgic_cpu = &v->arch.vgic; INIT_LIST_HEAD(&vgic_cpu->ap_list_head); - - return 0; } /* -- 2.52.0 When CONFIG_NEW_VGIC=y and CONFIG_ARM_64=y, the size of struct vcpu exceeds one page, which requires allocating two pages and led to the introduction of MAX_PAGES_PER_VCPU. To remove the need for MAX_PAGES_PER_VCPU in a follow-up patch, the vgic member of NEW_VGIC's struct vgic_vcpu member private_irq is changed to a pointer to struct vgic_irq. As a result, the size of struct vcpu for Arm64 is reduced to 2176 bytes, compared to 3840 bytes (without these changes and with CONFIG_ARM_64=y) and 4736 bytes (without these changes and with both CONFIG_ARM_64=y and CONFIG_NEW_VGIC=y). Since the private_irqs member is now a pointer, vcpu_vgic_init() and vcpu_vgic_free() are updated to allocate and free private_irqs instance. Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in v3: - Make private_irqs member as pointer to vgic_irq in struct vgic_cpu of new_vgic instead of vgic member of arch_vcpu. --- Changes in v2: - New patch. --- xen/arch/arm/include/asm/new_vgic.h | 2 +- xen/arch/arm/vgic/vgic-init.c | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/xen/arch/arm/include/asm/new_vgic.h b/xen/arch/arm/include/asm/new_vgic.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/arm/include/asm/new_vgic.h +++ b/xen/arch/arm/include/asm/new_vgic.h @@ -XXX,XX +XXX,XX @@ struct vgic_dist { }; struct vgic_cpu { - struct vgic_irq private_irqs[VGIC_NR_PRIVATE_IRQS]; + struct vgic_irq *private_irqs; struct list_head ap_list_head; spinlock_t ap_list_lock; /* Protects the ap_list */ diff --git a/xen/arch/arm/vgic/vgic-init.c b/xen/arch/arm/vgic/vgic-init.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/arm/vgic/vgic-init.c +++ b/xen/arch/arm/vgic/vgic-init.c @@ -XXX,XX +XXX,XX @@ int vcpu_vgic_init(struct vcpu *v) { int ret = 0; + v->arch.vgic.private_irqs = + xzalloc_array(struct vgic_irq, VGIC_NR_PRIVATE_IRQS); + if ( !v->arch.vgic.private_irqs ) + return -ENOMEM; + vgic_vcpu_early_init(v); if ( gic_hw_version() == GIC_V2 ) @@ -XXX,XX +XXX,XX @@ void vcpu_vgic_free(struct vcpu *v) struct vgic_cpu *vgic_cpu = &v->arch.vgic; INIT_LIST_HEAD(&vgic_cpu->ap_list_head); + + XFREE(v->arch.vgic.private_irqs); } /* -- 2.52.0 alloc_vcpu_struct() and free_vcpu_struct() contain little architecture-specific logic and are suitable for sharing across architectures. Move both helpers to common code. To support the remaining architectural differences, introduce arch_vcpu_struct_memflags(), allowing architectures to override the memory flags passed to alloc_xenheap_pages(). This is currently needed by x86, which may require MEMF_bits(32) for HVM guests using shadow paging. The ARM implementation of alloc/free_vcpu_struct() is removed and replaced by the common version. Stub implementations are also dropped from PPC and RISC-V. Now that the size of struct vcpu for Arm64 is smaller than PAGE_SIZE, MAX_PAGES_PER_VCPU is no longer needed and is removed. Finally, make alloc_vcpu_struct() and free_vcpu_struct() static to common/domain.c, as they are no longer used outside common code. No functional changes. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in v3: - Make from function arch_vcpu_struct_memflags() a macros in asm/domain.h. - Drop forward declaration of arch_vcpu_struct_memflags() in asm/domain.h. - Update defintion of arch_vcpu_stuct_memflags() in alloc_vcpu_struct(). --- Changes in v2: - Rework alloc/free_vcpu_struct() to work with only one page. - Return back the comment about the restriction inside x86's arch_vcpu_struct_memflags(). - Drop MAX_PAGES_PER_VCPU. --- xen/arch/arm/domain.c | 32 ------------------------------- xen/arch/ppc/stubs.c | 10 ---------- xen/arch/riscv/stubs.c | 10 ---------- xen/arch/x86/domain.c | 24 ----------------------- xen/arch/x86/include/asm/domain.h | 12 ++++++++++++ xen/common/domain.c | 20 +++++++++++++++++++ xen/include/xen/domain.h | 4 ---- 7 files changed, 32 insertions(+), 80 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -XXX,XX +XXX,XX @@ 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 MAX_PAGES_PER_VCPU 2 -#else -#define MAX_PAGES_PER_VCPU 1 -#endif - -struct vcpu *alloc_vcpu_struct(const struct domain *d) -{ - struct vcpu *v; - - BUILD_BUG_ON(sizeof(*v) > MAX_PAGES_PER_VCPU * PAGE_SIZE); - v = alloc_xenheap_pages(get_order_from_bytes(sizeof(*v)), 0); - if ( v != NULL ) - { - unsigned int i; - - for ( i = 0; i < DIV_ROUND_UP(sizeof(*v), PAGE_SIZE); i++ ) - clear_page((void *)v + i * PAGE_SIZE); - } - - return v; -} - -void free_vcpu_struct(struct vcpu *v) -{ - free_xenheap_pages(v, get_order_from_bytes(sizeof(*v))); -} - int arch_vcpu_create(struct vcpu *v) { int rc = 0; diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/ppc/stubs.c +++ b/xen/arch/ppc/stubs.c @@ -XXX,XX +XXX,XX @@ void dump_pageframe_info(struct domain *d) BUG_ON("unimplemented"); } -void free_vcpu_struct(struct vcpu *v) -{ - BUG_ON("unimplemented"); -} - int arch_vcpu_create(struct vcpu *v) { BUG_ON("unimplemented"); @@ -XXX,XX +XXX,XX @@ void vcpu_kick(struct vcpu *v) BUG_ON("unimplemented"); } -struct vcpu *alloc_vcpu_struct(const struct domain *d) -{ - BUG_ON("unimplemented"); -} - unsigned long hypercall_create_continuation(unsigned int op, const char *format, ...) { diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/riscv/stubs.c +++ b/xen/arch/riscv/stubs.c @@ -XXX,XX +XXX,XX @@ void dump_pageframe_info(struct domain *d) BUG_ON("unimplemented"); } -void free_vcpu_struct(struct vcpu *v) -{ - BUG_ON("unimplemented"); -} - int arch_vcpu_create(struct vcpu *v) { BUG_ON("unimplemented"); @@ -XXX,XX +XXX,XX @@ void vcpu_kick(struct vcpu *v) BUG_ON("unimplemented"); } -struct vcpu *alloc_vcpu_struct(const struct domain *d) -{ - BUG_ON("unimplemented"); -} - unsigned long hypercall_create_continuation(unsigned int op, const char *format, ...) { diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -XXX,XX +XXX,XX @@ unsigned int arch_domain_struct_memflags(void) return MEMF_bits(bits); } -struct vcpu *alloc_vcpu_struct(const struct domain *d) -{ - struct vcpu *v; - /* - * This structure contains embedded PAE PDPTEs, used when an HVM guest - * runs on shadow pagetables outside of 64-bit mode. In this case the CPU - * may require that the shadow CR3 points below 4GB, and hence the whole - * structure must satisfy this restriction. Thus we specify MEMF_bits(32). - */ - unsigned int memflags = - (is_hvm_domain(d) && paging_mode_shadow(d)) ? MEMF_bits(32) : 0; - - BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE); - v = alloc_xenheap_pages(0, memflags); - if ( v != NULL ) - clear_page(v); - return v; -} - -void free_vcpu_struct(struct vcpu *v) -{ - free_xenheap_page(v); -} - /* Initialise various registers to their architectural INIT/RESET state. */ void arch_vcpu_regs_init(struct vcpu *v) { diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/include/asm/domain.h +++ b/xen/arch/x86/include/asm/domain.h @@ -XXX,XX +XXX,XX @@ unsigned int arch_domain_struct_memflags(void); #define arch_domain_struct_memflags arch_domain_struct_memflags +/* + * This structure contains embedded PAE PDPTEs, used when an HVM guest + * runs on shadow pagetables outside of 64-bit mode. In this case the CPU + * may require that the shadow CR3 points below 4GB, and hence the whole + * structure must satisfy this restriction. Thus we specify MEMF_bits(32). + */ +#define arch_vcpu_struct_memflags(d) ({ \ + const struct domain *d_ = (d); \ + \ + (is_hvm_domain(d_) && paging_mode_shadow(d_) ? MEMF_bits(32) : 0); \ +}) + #define has_32bit_shinfo(d) ((d)->arch.has_32bit_shinfo) /* diff --git a/xen/common/domain.c b/xen/common/domain.c index XXXXXXX..XXXXXXX 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -XXX,XX +XXX,XX @@ static int vcpu_teardown(struct vcpu *v) return 0; } +static struct vcpu *alloc_vcpu_struct(const struct domain *d) +{ +#ifndef arch_vcpu_struct_memflags +# define arch_vcpu_struct_memflags(d) ((void)(d), 0) +#endif + struct vcpu *v; + + BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE); + v = alloc_xenheap_pages(0, arch_vcpu_struct_memflags(d)); + if ( v ) + clear_page(v); + + return v; +} + +static void free_vcpu_struct(struct vcpu *v) +{ + free_xenheap_page(v); +} + /* * Destoy a vcpu once all references to it have been dropped. Used either * from domain_destroy()'s RCU path, or from the vcpu_create() error path diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h index XXXXXXX..XXXXXXX 100644 --- a/xen/include/xen/domain.h +++ b/xen/include/xen/domain.h @@ -XXX,XX +XXX,XX @@ void domid_free(domid_t domid); struct domain *alloc_domain_struct(void); void free_domain_struct(struct domain *d); -/* Allocate/free a VCPU structure. */ -struct vcpu *alloc_vcpu_struct(const struct domain *d); -void free_vcpu_struct(struct vcpu *v); - /* Allocate/free a PIRQ structure. */ #ifndef alloc_pirq_struct struct pirq *alloc_pirq_struct(struct domain *d); -- 2.52.0 As {alloc,free}_domain_struct() are used only within domain.c, they can be declared static and their declarations removed from xen/domain.h. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> --- Changes in v3: - Move alloc_domain_struct() and free_domain_struct() to not have forward declaration. - Add Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>. --- Changes in v2: - New patch. --- xen/common/domain.c | 42 ++++++++++++++++++++-------------------- xen/include/xen/domain.h | 4 ---- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/xen/common/domain.c b/xen/common/domain.c index XXXXXXX..XXXXXXX 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -XXX,XX +XXX,XX @@ static int domain_teardown(struct domain *d) return 0; } +static struct domain *alloc_domain_struct(void) +{ +#ifndef arch_domain_struct_memflags +# define arch_domain_struct_memflags() 0 +#endif + + struct domain *d = alloc_xenheap_pages(0, arch_domain_struct_memflags()); + + BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE); + + if ( d ) + clear_page(d); + + return d; +} + +static void free_domain_struct(struct domain *d) +{ + free_xenheap_page(d); +} + /* * Destroy a domain once all references to it have been dropped. Used either * from the RCU path, or from the domain_create() error path before the domain @@ -XXX,XX +XXX,XX @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config) return arch_sanitise_domain_config(config); } -struct domain *alloc_domain_struct(void) -{ -#ifndef arch_domain_struct_memflags -# define arch_domain_struct_memflags() 0 -#endif - - struct domain *d = alloc_xenheap_pages(0, arch_domain_struct_memflags()); - - BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE); - - if ( d ) - clear_page(d); - - return d; -} - -void free_domain_struct(struct domain *d) -{ - free_xenheap_page(d); -} - struct domain *domain_create(domid_t domid, struct xen_domctl_createdomain *config, unsigned int flags) diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h index XXXXXXX..XXXXXXX 100644 --- a/xen/include/xen/domain.h +++ b/xen/include/xen/domain.h @@ -XXX,XX +XXX,XX @@ void domid_free(domid_t domid); * Arch-specifics. */ -/* Allocate/free a domain structure. */ -struct domain *alloc_domain_struct(void); -void free_domain_struct(struct domain *d); - /* Allocate/free a PIRQ structure. */ #ifndef alloc_pirq_struct struct pirq *alloc_pirq_struct(struct domain *d); -- 2.52.0 As it was suggested in [1] it would be better to allocate one page for struct vcpu for all arch-es. To do that it is needed to align Arm code to allocate one page (as there is a case(when CONFIG_NEW_VGIC=y) when Arm64 will require to allocate two pages). As a result, the following patches for Arm have been introduced: - [PATCH v2 1/4] xen/arm: optimize the size of struct vcpu - [PATCH v2 2/4] xen/arm: drop MAX_PAGES_PER_VCPU This patches are dependency for: - [PATCH v2 3/4] xen: move alloc/free_vcpu_struct() to common code Also, as a part of this patch series another clean up is done which makes {alloc,free}_domain_struct() static. [1] https://lore.kernel.org/xen-devel/f8a9be3a-a0c6-496a-806f-40760dca5aee@suse.com/T/#m275dfcbdccef0461fa9a8acef072403f18091768 CI: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2246917084 --- Changes in v5: - Address the comments recieved for v4. - Patch "xen/arm: vcpu_vgic_free() updates" has been merged to staging. --- Changes in v4: - Address the comments recieved for v3. --- Changes in v3: - Come up with a different way to optimize struct vcpu for Arm. - Merge patches "[PATCH v2 2/4] xen/arm: drop MAX_PAGES_PER_VCPU]" and "[PATCH v2 4/4] xen/common: make {alloc,free}_domain_struct() static" together. - Clean up vcpu_vgic_free() a little bit. --- Changes in v2: - Introduce new patches for Arm: - [PATCH v2 1/4] xen/arm: optimize the size of struct vcpu - [PATCH v2 2/4] xen/arm: drop MAX_PAGES_PER_VCPU to allocate one page for struct vcpu in common code for all the arch-es. - Introduce patch to clean up xen/domain.h a little bit: - [PATCH v2 4/4] xen/common: make {alloc,free}_domain_struct() static - Address the comments from v1: - [PATCH v2 3/4] xen: move alloc/free_vcpu_struct() to common code --- Oleksii Kurochko (3): xen/arm: optimize the size of struct vcpu xen: move alloc/free_vcpu_struct() to common code xen/common: make {alloc,free}_domain_struct() static xen/arch/arm/domain.c | 32 --------------- xen/arch/arm/include/asm/new_vgic.h | 2 +- xen/arch/arm/vgic/vgic-init.c | 7 ++++ xen/arch/ppc/stubs.c | 10 ----- xen/arch/riscv/stubs.c | 10 ----- xen/arch/x86/domain.c | 24 ----------- xen/arch/x86/include/asm/domain.h | 12 ++++++ xen/common/domain.c | 62 +++++++++++++++++++---------- xen/include/xen/domain.h | 8 ---- 9 files changed, 61 insertions(+), 106 deletions(-) -- 2.52.0 When CONFIG_NEW_VGIC=y and CONFIG_ARM_64=y, the size of struct vcpu exceeds one page, which requires allocating two pages and led to the introduction of MAX_PAGES_PER_VCPU. To remove the need for MAX_PAGES_PER_VCPU, the vgic member of NEW_VGIC's struct vgic_cpu member private_irqs is changed to a pointer to struct vgic_irq. As a result, the size of struct vcpu for Arm64 is reduced to 2176 bytes in the case when CONFIG_ARM_64=y and CONFIG_NEW_VGIC=y, compared to 3840 bytes (without these changes and with CONFIG_ARM_64=y) and 4736 bytes (without these changes and with both CONFIG_ARM_64=y and CONFIG_NEW_VGIC=y). Note that all numbers are based on defconfig with the mentioned options enabled or disabled as specified. Since the private_irqs member is now a pointer, vcpu_vgic_init() and vcpu_vgic_free() are updated to allocate and free private_irqs instance. As struct vcpu now fits into one page, drop MAX_PAGES_PER_VCPU. Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> --- Michal gave his: Acked-by: Michal Orzel <michal.orzel@amd.com> But wrote to MAX_PAGES_PER_VCPU in this patch, so probably he would like to look at how it was done. --- Change in v5: - Correct the commit message: - s/vgic_vcpu/vgic_cpu/ - s/private_irq/private_irqs/ - Drop MAX_PAGES_PER_VCPU. --- Change in v4: - Add Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>. --- Changes in v3: - Make private_irqs member as pointer to vgic_irq in struct vgic_cpu of new_vgic instead of vgic member of arch_vcpu. --- Changes in v2: - New patch. --- xen/arch/arm/domain.c | 23 ++++------------------- xen/arch/arm/include/asm/new_vgic.h | 2 +- xen/arch/arm/vgic/vgic-init.c | 7 +++++++ 3 files changed, 12 insertions(+), 20 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -XXX,XX +XXX,XX @@ 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 MAX_PAGES_PER_VCPU 2 -#else -#define MAX_PAGES_PER_VCPU 1 -#endif - struct vcpu *alloc_vcpu_struct(const struct domain *d) { struct vcpu *v; - BUILD_BUG_ON(sizeof(*v) > MAX_PAGES_PER_VCPU * PAGE_SIZE); - v = alloc_xenheap_pages(get_order_from_bytes(sizeof(*v)), 0); + BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE); + v = alloc_xenheap_page(); if ( v != NULL ) - { - unsigned int i; - - for ( i = 0; i < DIV_ROUND_UP(sizeof(*v), PAGE_SIZE); i++ ) - clear_page((void *)v + i * PAGE_SIZE); - } + clear_page(v); return v; } void free_vcpu_struct(struct vcpu *v) { - free_xenheap_pages(v, get_order_from_bytes(sizeof(*v))); + free_xenheap_page(v); } int arch_vcpu_create(struct vcpu *v) diff --git a/xen/arch/arm/include/asm/new_vgic.h b/xen/arch/arm/include/asm/new_vgic.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/arm/include/asm/new_vgic.h +++ b/xen/arch/arm/include/asm/new_vgic.h @@ -XXX,XX +XXX,XX @@ struct vgic_dist { }; struct vgic_cpu { - struct vgic_irq private_irqs[VGIC_NR_PRIVATE_IRQS]; + struct vgic_irq *private_irqs; struct list_head ap_list_head; spinlock_t ap_list_lock; /* Protects the ap_list */ diff --git a/xen/arch/arm/vgic/vgic-init.c b/xen/arch/arm/vgic/vgic-init.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/arm/vgic/vgic-init.c +++ b/xen/arch/arm/vgic/vgic-init.c @@ -XXX,XX +XXX,XX @@ int vcpu_vgic_init(struct vcpu *v) { int ret = 0; + v->arch.vgic.private_irqs = + xzalloc_array(struct vgic_irq, VGIC_NR_PRIVATE_IRQS); + if ( !v->arch.vgic.private_irqs ) + return -ENOMEM; + vgic_vcpu_early_init(v); if ( gic_hw_version() == GIC_V2 ) @@ -XXX,XX +XXX,XX @@ void vcpu_vgic_free(struct vcpu *v) struct vgic_cpu *vgic_cpu = &v->arch.vgic; INIT_LIST_HEAD(&vgic_cpu->ap_list_head); + + XFREE(v->arch.vgic.private_irqs); } /* -- 2.52.0 alloc_vcpu_struct() and free_vcpu_struct() contain little architecture-specific logic and are suitable for sharing across architectures. Move both helpers to common code. To support the remaining architectural differences, introduce arch_vcpu_struct_memflags(), allowing architectures to override the memory flags passed to alloc_xenheap_pages(). This is currently needed by x86, which may require MEMF_bits(32) for HVM guests using shadow paging. The ARM implementation of alloc/free_vcpu_struct() is removed and replaced by the common version. Stub implementations are also dropped from PPC and RISC-V. Now that the size of struct vcpu for Arm64 is smaller than PAGE_SIZE, MAX_PAGES_PER_VCPU is no longer needed and is removed. Finally, make alloc_vcpu_struct() and free_vcpu_struct() static to common/domain.c, as they are no longer used outside common code. No functional changes. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> Acked-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> --- Changes in v5: - Nothing changed. Only rebase. --- Changes in v4: - Move implementation of alloc_vcpu_struct() and free_vcpu_struct() ahead of vmtrace_free_buffer(). - Add Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>. - Add Acked-by: Jan Beulich <jbeulich@suse.com>. --- Changes in v3: - Make from function arch_vcpu_struct_memflags() a macros in asm/domain.h. - Drop forward declaration of arch_vcpu_struct_memflags() in asm/domain.h. - Update defintion of arch_vcpu_stuct_memflags() in alloc_vcpu_struct(). --- Changes in v2: - Rework alloc/free_vcpu_struct() to work with only one page. - Return back the comment about the restriction inside x86's arch_vcpu_struct_memflags(). - Drop MAX_PAGES_PER_VCPU. --- xen/arch/arm/domain.c | 17 ----------------- xen/arch/ppc/stubs.c | 10 ---------- xen/arch/riscv/stubs.c | 10 ---------- xen/arch/x86/domain.c | 24 ------------------------ xen/arch/x86/include/asm/domain.h | 12 ++++++++++++ xen/common/domain.c | 20 ++++++++++++++++++++ xen/include/xen/domain.h | 4 ---- 7 files changed, 32 insertions(+), 65 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -XXX,XX +XXX,XX @@ void dump_pageframe_info(struct domain *d) } -struct vcpu *alloc_vcpu_struct(const struct domain *d) -{ - struct vcpu *v; - - BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE); - v = alloc_xenheap_page(); - if ( v != NULL ) - clear_page(v); - - return v; -} - -void free_vcpu_struct(struct vcpu *v) -{ - free_xenheap_page(v); -} - int arch_vcpu_create(struct vcpu *v) { int rc = 0; diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/ppc/stubs.c +++ b/xen/arch/ppc/stubs.c @@ -XXX,XX +XXX,XX @@ void dump_pageframe_info(struct domain *d) BUG_ON("unimplemented"); } -void free_vcpu_struct(struct vcpu *v) -{ - BUG_ON("unimplemented"); -} - int arch_vcpu_create(struct vcpu *v) { BUG_ON("unimplemented"); @@ -XXX,XX +XXX,XX @@ void vcpu_kick(struct vcpu *v) BUG_ON("unimplemented"); } -struct vcpu *alloc_vcpu_struct(const struct domain *d) -{ - BUG_ON("unimplemented"); -} - unsigned long hypercall_create_continuation(unsigned int op, const char *format, ...) { diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/riscv/stubs.c +++ b/xen/arch/riscv/stubs.c @@ -XXX,XX +XXX,XX @@ void dump_pageframe_info(struct domain *d) BUG_ON("unimplemented"); } -void free_vcpu_struct(struct vcpu *v) -{ - BUG_ON("unimplemented"); -} - int arch_vcpu_create(struct vcpu *v) { BUG_ON("unimplemented"); @@ -XXX,XX +XXX,XX @@ void vcpu_kick(struct vcpu *v) BUG_ON("unimplemented"); } -struct vcpu *alloc_vcpu_struct(const struct domain *d) -{ - BUG_ON("unimplemented"); -} - unsigned long hypercall_create_continuation(unsigned int op, const char *format, ...) { diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -XXX,XX +XXX,XX @@ unsigned int arch_domain_struct_memflags(void) return MEMF_bits(bits); } -struct vcpu *alloc_vcpu_struct(const struct domain *d) -{ - struct vcpu *v; - /* - * This structure contains embedded PAE PDPTEs, used when an HVM guest - * runs on shadow pagetables outside of 64-bit mode. In this case the CPU - * may require that the shadow CR3 points below 4GB, and hence the whole - * structure must satisfy this restriction. Thus we specify MEMF_bits(32). - */ - unsigned int memflags = - (is_hvm_domain(d) && paging_mode_shadow(d)) ? MEMF_bits(32) : 0; - - BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE); - v = alloc_xenheap_pages(0, memflags); - if ( v != NULL ) - clear_page(v); - return v; -} - -void free_vcpu_struct(struct vcpu *v) -{ - free_xenheap_page(v); -} - /* Initialise various registers to their architectural INIT/RESET state. */ void arch_vcpu_regs_init(struct vcpu *v) { diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/include/asm/domain.h +++ b/xen/arch/x86/include/asm/domain.h @@ -XXX,XX +XXX,XX @@ unsigned int arch_domain_struct_memflags(void); #define arch_domain_struct_memflags arch_domain_struct_memflags +/* + * This structure contains embedded PAE PDPTEs, used when an HVM guest + * runs on shadow pagetables outside of 64-bit mode. In this case the CPU + * may require that the shadow CR3 points below 4GB, and hence the whole + * structure must satisfy this restriction. Thus we specify MEMF_bits(32). + */ +#define arch_vcpu_struct_memflags(d) ({ \ + const struct domain *d_ = (d); \ + \ + (is_hvm_domain(d_) && paging_mode_shadow(d_) ? MEMF_bits(32) : 0); \ +}) + #define has_32bit_shinfo(d) ((d)->arch.has_32bit_shinfo) /* diff --git a/xen/common/domain.c b/xen/common/domain.c index XXXXXXX..XXXXXXX 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -XXX,XX +XXX,XX @@ static void vcpu_info_reset(struct vcpu *v) : &dummy_vcpu_info); } +static struct vcpu *alloc_vcpu_struct(const struct domain *d) +{ +#ifndef arch_vcpu_struct_memflags +# define arch_vcpu_struct_memflags(d) ((void)(d), 0) +#endif + struct vcpu *v; + + BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE); + v = alloc_xenheap_pages(0, arch_vcpu_struct_memflags(d)); + if ( v ) + clear_page(v); + + return v; +} + +static void free_vcpu_struct(struct vcpu *v) +{ + free_xenheap_page(v); +} + static void vmtrace_free_buffer(struct vcpu *v) { const struct domain *d = v->domain; diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h index XXXXXXX..XXXXXXX 100644 --- a/xen/include/xen/domain.h +++ b/xen/include/xen/domain.h @@ -XXX,XX +XXX,XX @@ void domid_free(domid_t domid); struct domain *alloc_domain_struct(void); void free_domain_struct(struct domain *d); -/* Allocate/free a VCPU structure. */ -struct vcpu *alloc_vcpu_struct(const struct domain *d); -void free_vcpu_struct(struct vcpu *v); - /* Allocate/free a PIRQ structure. */ #ifndef alloc_pirq_struct struct pirq *alloc_pirq_struct(struct domain *d); -- 2.52.0 As {alloc,free}_domain_struct() are used only within domain.c, they can be declared static and their declarations removed from xen/domain.h. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> --- Changes in v5: - Nothing changed. Only rebase. --- Changes in v4: - Move implementation of alloc_domain_struct() and free_domain_struct() ahead of alloc_vcpu_struct(). --- Changes in v3: - Move alloc_domain_struct() and free_domain_struct() to not have forward declaration. - Add Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>. --- Changes in v2: - New patch. --- xen/common/domain.c | 42 ++++++++++++++++++++-------------------- xen/include/xen/domain.h | 4 ---- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/xen/common/domain.c b/xen/common/domain.c index XXXXXXX..XXXXXXX 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -XXX,XX +XXX,XX @@ static void vcpu_info_reset(struct vcpu *v) : &dummy_vcpu_info); } +static struct domain *alloc_domain_struct(void) +{ +#ifndef arch_domain_struct_memflags +# define arch_domain_struct_memflags() 0 +#endif + + struct domain *d = alloc_xenheap_pages(0, arch_domain_struct_memflags()); + + BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE); + + if ( d ) + clear_page(d); + + return d; +} + +static void free_domain_struct(struct domain *d) +{ + free_xenheap_page(d); +} + static struct vcpu *alloc_vcpu_struct(const struct domain *d) { #ifndef arch_vcpu_struct_memflags @@ -XXX,XX +XXX,XX @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config) return arch_sanitise_domain_config(config); } -struct domain *alloc_domain_struct(void) -{ -#ifndef arch_domain_struct_memflags -# define arch_domain_struct_memflags() 0 -#endif - - struct domain *d = alloc_xenheap_pages(0, arch_domain_struct_memflags()); - - BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE); - - if ( d ) - clear_page(d); - - return d; -} - -void free_domain_struct(struct domain *d) -{ - free_xenheap_page(d); -} - struct domain *domain_create(domid_t domid, struct xen_domctl_createdomain *config, unsigned int flags) diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h index XXXXXXX..XXXXXXX 100644 --- a/xen/include/xen/domain.h +++ b/xen/include/xen/domain.h @@ -XXX,XX +XXX,XX @@ void domid_free(domid_t domid); * Arch-specifics. */ -/* Allocate/free a domain structure. */ -struct domain *alloc_domain_struct(void); -void free_domain_struct(struct domain *d); - /* Allocate/free a PIRQ structure. */ #ifndef alloc_pirq_struct struct pirq *alloc_pirq_struct(struct domain *d); -- 2.52.0