Re: [PATCH 04/36] xen/arm: add parsing function for cache coloring configuration

[Carlo Nonato <carlo.nonato@xxxxxxxxxxxxxxx>]

Hi Julien,

I'm Carlo, the new developer that will work on this patch set and on the 
review.
Thanks for all the comments. I'll try to answer to all the open points 
and also
ask for feedback.

On 09/03/22 20:09, Julien Grall wrote:
> > - way_size: The size of a LLC way in bytes. This value is mainly used
> >    to calculate the maximum available colors on the platform.
> We should only add command line option when they are a strong use 
> case. In documentation, you wrote that someone may want to overwrite 
> the way size for "specific needs".
> Can you explain what would be those needs?
This parameter is here mainly to support QEMU on which the automatic 
probing
of the LLC size doesn't work properly.

Also, since from this value we compute the maximum number of colors
the architecture supports, you may want to fix the way size so as to 
simulate
a different use case for debugging purposes.

Should I add those notes somewhere (doc, commit messages, etc.)?

> > A cache coloring configuration consists of a selection of colors to be
> > assigned to a VM or to the hypervisor. It is represented by a set of
> > ranges. Add a common function that parses a string with a
> > comma-separated set of hyphen-separated ranges like "0-7,15-16" and
> > returns both: the number of chosen colors, and an array containing their
> > ids.
> > Currently we support platforms with up to 128 colors.
> Is there any reason this value is hardcoded in Xen rather than part of 
> the Kconfig?
Having another parameter to configure can complicate things from
the user perspective. Also 128 is more than enough for the current ARM
processors we tested.
> > Signed-off-by: Luca Miccio <lucmiccio@xxxxxxxxx>
> > Signed-off-by: Marco Solieri <marco.solieri@xxxxxxxxxxxxxxx>
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> > ---
> >   xen/arch/arm/Kconfig                |   5 ++
> >   xen/arch/arm/Makefile               |   2 +-
> >   xen/arch/arm/coloring.c             | 131 ++++++++++++++++++++++++++++
> >   xen/arch/arm/include/asm/coloring.h |  28 ++++++
> >   4 files changed, 165 insertions(+), 1 deletion(-)
> >   create mode 100644 xen/arch/arm/coloring.c
> >   create mode 100644 xen/arch/arm/include/asm/coloring.h
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index ecfa6822e4..f0f999d172 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -97,6 +97,11 @@ config HARDEN_BRANCH_PREDICTOR
> >           If unsure, say Y.
> >   +config COLORING
> > +    bool "L2 cache coloring"
> > +    default n
> This wants to be gated with EXPERT for time-being. SUPPORT.MD woudl
> Furthermore, I think this wants to be gated with EXPERT for the 
> time-being.
> > +    depends on ARM_64
> Why is this limited to arm64?
Because arm32 isn't an "interesting" architecture where to have coloring
since there are locking primitives that provides sufficient isolation and so
the problem is not common.
On x86 instead, the functions that map memory into caches are not so 
easy to
exploit to achieve isolation.

Thanks.

- Carlo Nonato