| Message ID | 20250316192445.2376484-7-luca.fancellu@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | MPU mm subsystem skeleton | expand |
On 16.03.2025 20:24, Luca Fancellu wrote: > --- a/xen/common/Kconfig > +++ b/xen/common/Kconfig > @@ -74,6 +74,9 @@ config HAS_KEXEC > config HAS_LLC_COLORING > bool > > +config HAS_PAGING_MEMPOOL > + bool Imo this is too little of a change outside of Arm-specific code here. Just go grep for "paging_mempool" to see what pretty obviously wants to fall under that new control. There may be more stuff. The stubs for arch_{get,set}_paging_mempool_size() likely also want to live in a header, perhaps even one in asm-generic/. Jan
Hi Jan, > On 17 Mar 2025, at 08:41, Jan Beulich <jbeulich@suse.com> wrote: > > On 16.03.2025 20:24, Luca Fancellu wrote: >> --- a/xen/common/Kconfig >> +++ b/xen/common/Kconfig >> @@ -74,6 +74,9 @@ config HAS_KEXEC >> config HAS_LLC_COLORING >> bool >> >> +config HAS_PAGING_MEMPOOL >> + bool > > Imo this is too little of a change outside of Arm-specific code here. Just > go grep for "paging_mempool" to see what pretty obviously wants to fall > under that new control. There may be more stuff. The stubs for > arch_{get,set}_paging_mempool_size() likely also want to live in a header, > perhaps even one in asm-generic/. I’m wondering, since we already have arch_{get,set}_paging_mempool_size architecture specific, I believe this config needs to be ARCH_PAGING_MEMPOOL and needs to provide stubs inside include/xen/domain.h in a similar way of how ARCH_MAP_DOMAIN_PAGE is done. What do you think? Cheers, Luca
On 17.03.2025 10:29, Luca Fancellu wrote: >> On 17 Mar 2025, at 08:41, Jan Beulich <jbeulich@suse.com> wrote: >> On 16.03.2025 20:24, Luca Fancellu wrote: >>> --- a/xen/common/Kconfig >>> +++ b/xen/common/Kconfig >>> @@ -74,6 +74,9 @@ config HAS_KEXEC >>> config HAS_LLC_COLORING >>> bool >>> >>> +config HAS_PAGING_MEMPOOL >>> + bool >> >> Imo this is too little of a change outside of Arm-specific code here. Just >> go grep for "paging_mempool" to see what pretty obviously wants to fall >> under that new control. There may be more stuff. The stubs for >> arch_{get,set}_paging_mempool_size() likely also want to live in a header, >> perhaps even one in asm-generic/. > > I’m wondering, since we already have arch_{get,set}_paging_mempool_size > architecture specific, I believe this config needs to be ARCH_PAGING_MEMPOOL > and needs to provide stubs inside include/xen/domain.h in a similar way of how > ARCH_MAP_DOMAIN_PAGE is done. > > What do you think? Imo both routes are possible. I'm not sure we have a clear preference when looking at the code base as a whole. Jan
On 16/03/2025 20:24, Luca Fancellu wrote: > > > From: Penny Zheng <Penny.Zheng@arm.com> > > ARM MPU system doesn't need to use paging memory pool, as MPU memory > mapping table at most takes only one 4KB page, which is enough to > manage the maximum 255 MPU memory regions, for all EL2 stage 1 > translation and EL1 stage 2 translation. > > Introduce HAS_PAGING_MEMPOOL Kconfig common symbol, selected for Arm > MMU systems. I think it should be selected by other arches that implement the stubs. Anyway, Jan added a comment and I do agree with him. Just a few remarks. > > Wrap the code inside 'construct_domU' that deal with p2m paging > allocation in a new function 'domu_p2m_set_allocation', protected > by HAS_PAGING_MEMPOOL, this is done in this way to prevent polluting > the former function with #ifdefs and improve readability > > Introduce arch_{get,set}_paging_mempool_size implementation for MPU > system. > > Remove 'struct paging_domain' from Arm 'struct arch_domain' when the > field is not required. > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > Signed-off-by: Wei Chen <wei.chen@arm.com> > Signed-off-by: Luca Fancellu <luca.fancellu@arm.com> > --- > v2 changes: > - make Kconfig HAS_PAGING_MEMPOOL common > - protect also "xen,domain-p2m-mem-mb" reading with HAS_PAGING_MEMPOOL > - do not define p2m_teardown{_allocation} in this patch > - change commit message > --- > xen/arch/arm/Kconfig | 1 + > xen/arch/arm/dom0less-build.c | 48 ++++++++++++++++++++++++------- > xen/arch/arm/include/asm/domain.h | 2 ++ > xen/arch/arm/mpu/Makefile | 1 + > xen/arch/arm/mpu/p2m.c | 25 ++++++++++++++++ > xen/common/Kconfig | 3 ++ > 6 files changed, 69 insertions(+), 11 deletions(-) > create mode 100644 xen/arch/arm/mpu/p2m.c > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig > index 5ac6ec0212d2..6b4bcf12683e 100644 > --- a/xen/arch/arm/Kconfig > +++ b/xen/arch/arm/Kconfig > @@ -76,6 +76,7 @@ choice > config MMU > bool "MMU" > select HAS_LLC_COLORING if !NUMA && ARM_64 > + select HAS_PAGING_MEMPOOL > select HAS_PMAP > select HAS_VMAP > select HAS_PASSTHROUGH > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c > index 573b0d25ae41..a65bbbb05301 100644 > --- a/xen/arch/arm/dom0less-build.c > +++ b/xen/arch/arm/dom0less-build.c > @@ -673,6 +673,8 @@ static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo) > return -EINVAL; > } > > +#ifdef CONFIG_HAS_PAGING_MEMPOOL > + > static unsigned long __init domain_p2m_pages(unsigned long maxmem_kb, > unsigned int smp_cpus) > { > @@ -688,6 +690,8 @@ static unsigned long __init domain_p2m_pages(unsigned long maxmem_kb, > return DIV_ROUND_UP(memkb, 1024) << (20 - PAGE_SHIFT); > } > > +#endif > + > static int __init alloc_xenstore_evtchn(struct domain *d) > { > evtchn_alloc_unbound_t alloc; > @@ -841,6 +845,38 @@ static void __init domain_vcpu_affinity(struct domain *d, > } > } > > +#ifdef CONFIG_HAS_PAGING_MEMPOOL Why can't this be moved to upper CONFIG_HAS_PAGING_MEMPOOL block to avoid excessive ifdefery? > + > +static int __init domu_p2m_set_allocation(struct domain *d, u64 mem, s/domu/domain/ to match naming of other functions. uint64_t. Linux types shall no longer be used. > + const struct dt_device_node *node) > +{ > + unsigned long p2m_pages; > + u32 p2m_mem_mb; uint32_t ~Michal
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index 5ac6ec0212d2..6b4bcf12683e 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -76,6 +76,7 @@ choice config MMU bool "MMU" select HAS_LLC_COLORING if !NUMA && ARM_64 + select HAS_PAGING_MEMPOOL select HAS_PMAP select HAS_VMAP select HAS_PASSTHROUGH diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c index 573b0d25ae41..a65bbbb05301 100644 --- a/xen/arch/arm/dom0less-build.c +++ b/xen/arch/arm/dom0less-build.c @@ -673,6 +673,8 @@ static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo) return -EINVAL; } +#ifdef CONFIG_HAS_PAGING_MEMPOOL + static unsigned long __init domain_p2m_pages(unsigned long maxmem_kb, unsigned int smp_cpus) { @@ -688,6 +690,8 @@ static unsigned long __init domain_p2m_pages(unsigned long maxmem_kb, return DIV_ROUND_UP(memkb, 1024) << (20 - PAGE_SHIFT); } +#endif + static int __init alloc_xenstore_evtchn(struct domain *d) { evtchn_alloc_unbound_t alloc; @@ -841,6 +845,38 @@ static void __init domain_vcpu_affinity(struct domain *d, } } +#ifdef CONFIG_HAS_PAGING_MEMPOOL + +static int __init domu_p2m_set_allocation(struct domain *d, u64 mem, + const struct dt_device_node *node) +{ + unsigned long p2m_pages; + u32 p2m_mem_mb; + int rc; + + rc = dt_property_read_u32(node, "xen,domain-p2m-mem-mb", &p2m_mem_mb); + /* If xen,domain-p2m-mem-mb is not specified, use the default value. */ + p2m_pages = rc ? + p2m_mem_mb << (20 - PAGE_SHIFT) : + domain_p2m_pages(mem, d->max_vcpus); + + spin_lock(&d->arch.paging.lock); + rc = p2m_set_allocation(d, p2m_pages, NULL); + spin_unlock(&d->arch.paging.lock); + + return rc; +} + +#else /* !CONFIG_HAS_PAGING_MEMPOOL */ + +static inline int domu_p2m_set_allocation(struct domain *d, u64 mem, + const struct dt_device_node *node) +{ + return 0; +} + +#endif /* CONFIG_HAS_PAGING_MEMPOOL */ + static int __init construct_domU(struct domain *d, const struct dt_device_node *node) { @@ -848,8 +884,6 @@ static int __init construct_domU(struct domain *d, const char *dom0less_enhanced; int rc; u64 mem; - u32 p2m_mem_mb; - unsigned long p2m_pages; rc = dt_property_read_u64(node, "memory", &mem); if ( !rc ) @@ -859,15 +893,7 @@ static int __init construct_domU(struct domain *d, } kinfo.unassigned_mem = (paddr_t)mem * SZ_1K; - rc = dt_property_read_u32(node, "xen,domain-p2m-mem-mb", &p2m_mem_mb); - /* If xen,domain-p2m-mem-mb is not specified, use the default value. */ - p2m_pages = rc ? - p2m_mem_mb << (20 - PAGE_SHIFT) : - domain_p2m_pages(mem, d->max_vcpus); - - spin_lock(&d->arch.paging.lock); - rc = p2m_set_allocation(d, p2m_pages, NULL); - spin_unlock(&d->arch.paging.lock); + rc = domu_p2m_set_allocation(d, mem, node); if ( rc != 0 ) return rc; diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h index 50b6a4b00982..fadec7d8fa9e 100644 --- a/xen/arch/arm/include/asm/domain.h +++ b/xen/arch/arm/include/asm/domain.h @@ -75,7 +75,9 @@ struct arch_domain struct hvm_domain hvm; +#ifdef CONFIG_HAS_PAGING_MEMPOOL struct paging_domain paging; +#endif struct vmmio vmmio; diff --git a/xen/arch/arm/mpu/Makefile b/xen/arch/arm/mpu/Makefile index b18cec483671..5424ab6239c8 100644 --- a/xen/arch/arm/mpu/Makefile +++ b/xen/arch/arm/mpu/Makefile @@ -1 +1,2 @@ obj-y += mm.o +obj-y += p2m.o diff --git a/xen/arch/arm/mpu/p2m.c b/xen/arch/arm/mpu/p2m.c new file mode 100644 index 000000000000..71f9cdbf2fdb --- /dev/null +++ b/xen/arch/arm/mpu/p2m.c @@ -0,0 +1,25 @@ + +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <xen/domain.h> +#include <xen/errno.h> +#include <xen/types.h> + +int arch_get_paging_mempool_size(struct domain *d, uint64_t *size) +{ + return -EOPNOTSUPP; +} + +int arch_set_paging_mempool_size(struct domain *d, uint64_t size) +{ + return -EOPNOTSUPP; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/common/Kconfig b/xen/common/Kconfig index 6166327f4d14..a3bdf546d02f 100644 --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -74,6 +74,9 @@ config HAS_KEXEC config HAS_LLC_COLORING bool +config HAS_PAGING_MEMPOOL + bool + config HAS_PIRQ bool