| Message ID | 20220304174701.1453977-13-marco.solieri@minervasys.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Arm cache coloring | expand |
Hi, On 04/03/2022 17:46, Marco Solieri wrote: > From: Luca Miccio <lucmiccio@gmail.com> > > Initialize cache coloring configuration during domain creation. If no > colors assignment is provided by the user, use the default one. > The default configuration is the one assigned to Dom0. The latter is > configured as a standard domain with default configuration. > > Signed-off-by: Luca Miccio <lucmiccio@gmail.com> > Signed-off-by: Marco Solieri <marco.solieri@minervasys.tech> > --- > xen/arch/arm/domain.c | 53 +++++++++++++++++++++++++++++++++++++ > xen/arch/arm/domain_build.c | 5 +++- > 2 files changed, 57 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 8110c1df86..33471b3c58 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -38,6 +38,7 @@ > #include <asm/vfp.h> > #include <asm/vgic.h> > #include <asm/vtimer.h> > +#include <asm/coloring.h> > > #include "vpci.h" > #include "vuart.h" > @@ -782,6 +783,58 @@ int arch_domain_create(struct domain *d, > if ( (rc = domain_vpci_init(d)) != 0 ) > goto fail; > > + d->max_colors = 0; NIT: d is always zeroed when allocated. So it is not necessary to initialize the field again. > +#ifdef CONFIG_COLORING Please move this code in a separate helper. The new helper could be defined in coloring.c. Furthermore, I would initialize the coloring information earlier in arch_domain_create(). This could be useful if we want to allocate internal structure from a color assigned to the domain. > + /* Setup domain colors */ > + if ( !config->arch.colors.max_colors ) > + { > + if ( !is_hardware_domain(d) ) > + printk(XENLOG_INFO "Color configuration not found for dom%u, using default\n", This message and the other below wants to be ratelimited. I would use XENLOG_G_{INFO, ERROR}. Please use %pd instead of dom%u. This remark is valid for all the other use below. > + d->domain_id); This would need to be changed to 'd'. > + d->colors = setup_default_colors(&d->max_colors); Looking at setup_default_colors(), it using "dom0_col_num". This implies we are using the dom0 color. Shouldn't we return an error if d is not the hardware domain? Also, AFAICT, you allocate the memory but never free it. > + if ( !d->colors ) > + { > + rc = -ENOMEM; > + printk(XENLOG_ERR "Color array allocation failed for dom%u\n", > + d->domain_id); > + goto fail; > + } > + } > + else > + { > + int i, k; > + > + d->colors = xzalloc_array(uint32_t, config->arch.colors.max_colors); Same here. > + if ( !d->colors ) > + { > + rc = -ENOMEM; > + printk(XENLOG_ERR "Failed to alloc colors for dom%u\n", > + d->domain_id); > + goto fail; > + } > + > + d->max_colors = config->arch.colors.max_colors; > + for ( i = 0, k = 0; > + k < d->max_colors && i < sizeof(config->arch.colors.colors) * 8; > + i++ ) > + { > + if ( config->arch.colors.colors[i / 32] & (1 << (i % 32)) ) > + d->colors[k++] = i; > + } > + } > + > + printk("Dom%u colors: [ ", d->domain_id); > + for ( int i = 0; i < d->max_colors; i++ ) > + printk("%u ", d->colors[i]); > + printk("]\n"); You will be able to get the same information using the debug-key. So I am not convinced this is warrant to print here. The more the configuration should always be the same as what the user requested. > + > + if ( !check_domain_colors(d) ) > + { > + rc = -EINVAL; > + printk(XENLOG_ERR "Failed to check colors for dom%u\n", d->domain_id); > + goto fail; > + } > +#endif > return 0; > > fail: > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 8be01678de..9630d00066 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -3344,7 +3344,10 @@ void __init create_dom0(void) > printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n"); > dom0_cfg.arch.tee_type = tee_get_type(); > dom0_cfg.max_vcpus = dom0_max_vcpus(); > - > +#ifdef CONFIG_COLORING > + /* Colors are set after domain_create */ Do you instead mean 'by'? > + dom0_cfg.arch.colors.max_colors = 0; > +#endif > if ( iommu_enabled ) > dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu; > Cheers,
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 8110c1df86..33471b3c58 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -38,6 +38,7 @@ #include <asm/vfp.h> #include <asm/vgic.h> #include <asm/vtimer.h> +#include <asm/coloring.h> #include "vpci.h" #include "vuart.h" @@ -782,6 +783,58 @@ int arch_domain_create(struct domain *d, if ( (rc = domain_vpci_init(d)) != 0 ) goto fail; + d->max_colors = 0; +#ifdef CONFIG_COLORING + /* Setup domain colors */ + if ( !config->arch.colors.max_colors ) + { + if ( !is_hardware_domain(d) ) + printk(XENLOG_INFO "Color configuration not found for dom%u, using default\n", + d->domain_id); + d->colors = setup_default_colors(&d->max_colors); + if ( !d->colors ) + { + rc = -ENOMEM; + printk(XENLOG_ERR "Color array allocation failed for dom%u\n", + d->domain_id); + goto fail; + } + } + else + { + int i, k; + + d->colors = xzalloc_array(uint32_t, config->arch.colors.max_colors); + if ( !d->colors ) + { + rc = -ENOMEM; + printk(XENLOG_ERR "Failed to alloc colors for dom%u\n", + d->domain_id); + goto fail; + } + + d->max_colors = config->arch.colors.max_colors; + for ( i = 0, k = 0; + k < d->max_colors && i < sizeof(config->arch.colors.colors) * 8; + i++ ) + { + if ( config->arch.colors.colors[i / 32] & (1 << (i % 32)) ) + d->colors[k++] = i; + } + } + + printk("Dom%u colors: [ ", d->domain_id); + for ( int i = 0; i < d->max_colors; i++ ) + printk("%u ", d->colors[i]); + printk("]\n"); + + if ( !check_domain_colors(d) ) + { + rc = -EINVAL; + printk(XENLOG_ERR "Failed to check colors for dom%u\n", d->domain_id); + goto fail; + } +#endif return 0; fail: diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 8be01678de..9630d00066 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -3344,7 +3344,10 @@ void __init create_dom0(void) printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n"); dom0_cfg.arch.tee_type = tee_get_type(); dom0_cfg.max_vcpus = dom0_max_vcpus(); - +#ifdef CONFIG_COLORING + /* Colors are set after domain_create */ + dom0_cfg.arch.colors.max_colors = 0; +#endif if ( iommu_enabled ) dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;