Re: [PATCH v5 02/13] xen/arm: add cache coloring initialization

[Julien Grall <julien@xxxxxxx>]

On 11/01/2024 10:17, Carlo Nonato wrote:
> Hi Julien,
Hi Carlo,

> > > > +bool __init llc_coloring_init(void)
> > > > +{
> > > > +    if ( !llc_way_size && !(llc_way_size = get_llc_way_size()) )
> > > > +    {
> > > > +        printk(XENLOG_ERR
> > > > +               "Probed LLC way size is 0 and no custom value provided\n");
> > > > +        return false;
> > > > +    }
> > > > +
> > > > +    /*
> > > > +     * The maximum number of colors must be a power of 2 in order to correctly
> > > > +     * map them to bits of an address, so also the LLC way size must be so.
> > > > +     */
> > > > +    if ( llc_way_size & (llc_way_size - 1) )
> > > > +    {
> > > > +        printk(XENLOG_WARNING "LLC way size (%u) isn't a power of 2.\n",
> > > > +               llc_way_size);
> > > > +        llc_way_size = 1U << flsl(llc_way_size);
> > > > +        printk(XENLOG_WARNING
> > > > +               "Using %u instead. Performances will be suboptimal\n",
> > > > +               llc_way_size);
> > > > +    }
> > > > +
> > > > +    nr_colors = llc_way_size >> PAGE_SHIFT;
> > > > +
> > > > +    if ( nr_colors < 2 || nr_colors > CONFIG_NR_LLC_COLORS )
> > > I didn't find any documentation explaining why we need at least two
> > > colors. I guess you want to make sure that there is a color for Xen and
> > > domain. But I wonder what could wrong with just one color (other than
> > > been pointless)?
> > Yes, it would just be pointless. I'll change it to 1.
> Just wanted to correct myself here. Having just a single color introduces a
> clear sharing of the cache between Xen and domains. So it's not just
> pointless, but also inefficient. I would discourage such a configuration, so I
> plan to better describe this with a range in the Kconfig option (see
> discussion in #1).
I understand this could be inneficient. But you are also allowing the 
user to not specify the color configuration (at least for dom0less 
domain). So the colors would end up to shared with everyone (including Xen).
I don't particularly mind which way you want to go, but I think we need 
some coherency. If we want to avoid innefficiency, then we should 
prevent all the setups.
Cheers,

--
Julien Grall