| Message ID | 20220304174701.1453977-12-marco.solieri@minervasys.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Arm cache coloring | expand |
On 04.03.2022 18:46, Marco Solieri wrote: > --- a/xen/include/public/arch-arm.h > +++ b/xen/include/public/arch-arm.h > @@ -303,6 +303,12 @@ struct vcpu_guest_context { > typedef struct vcpu_guest_context vcpu_guest_context_t; > DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t); > > +#define MAX_COLORS_CELLS 4 > +struct color_guest_config { > + uint32_t max_colors; > + uint32_t colors[MAX_COLORS_CELLS]; > +}; > + > /* > * struct xen_arch_domainconfig's ABI is covered by > * XEN_DOMCTL_INTERFACE_VERSION. > @@ -335,6 +341,8 @@ struct xen_arch_domainconfig { > * > */ > uint32_t clock_frequency; > + /* IN */ > + struct color_guest_config colors; > }; > #endif /* __XEN__ || __XEN_TOOLS__ */ > Please no new additions to the public interface without proper XEN_ / xen_ name prefixes on anything going in some global name space. (Personally I also wonder whether a separate struct is warranted, but I'm not a maintainer here, so I've got little say.) Jan
Hi, On 04/03/2022 17:46, Marco Solieri wrote: > From: Luca Miccio <lucmiccio@gmail.com> > > During domU creation process the colors selection has to be passed to > the Xen hypercall. > This is generally done using what Xen calls GUEST_HANDLE_PARAMS. In this > case a simple bitmask for the coloring configuration suffices. > Currently the maximum amount of supported colors is 128. > Add a new parameter that allows us to pass both the colors bitmask > and the number of elements in it. > > Signed-off-by: Luca Miccio <lucmiccio@gmail.com> > Signed-off-by: Marco Solieri <marco.solieri@minervasys.tech> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com> > --- > xen/arch/arm/include/asm/coloring.h | 2 -- > xen/include/public/arch-arm.h | 8 ++++++++ I would prefer if the structure is defined in the same patch that will use it. This would make easier to figure out if the structure is indeed suitable. > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/include/asm/coloring.h b/xen/arch/arm/include/asm/coloring.h > index fdd46448d7..1f7e0dde79 100644 > --- a/xen/arch/arm/include/asm/coloring.h > +++ b/xen/arch/arm/include/asm/coloring.h > @@ -23,8 +23,6 @@ > #ifndef __ASM_ARM_COLORING_H__ > #define __ASM_ARM_COLORING_H__ > > -#define MAX_COLORS_CELLS 4 > - In general, we should avoid moving code that was introduced within the same series. In this case, I am not convinced we should use a static array to communicate the information between the toolstack and Xen. This would make more difficult for the user to tweak update the number of colors. Instead, I think it should be better to expose to the toolstack the number of color supported and allocate a dynamic array. > #ifdef CONFIG_COLORING > #include <xen/sched.h> > > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h > index 94b31511dd..627cc42164 100644 > --- a/xen/include/public/arch-arm.h > +++ b/xen/include/public/arch-arm.h > @@ -303,6 +303,12 @@ struct vcpu_guest_context { > typedef struct vcpu_guest_context vcpu_guest_context_t; > DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t); > > +#define MAX_COLORS_CELLS 4 > +struct color_guest_config { > + uint32_t max_colors; > + uint32_t colors[MAX_COLORS_CELLS]; > +}; This looks like an open-coded version of xenctl_bitmap. Can you have a look to use it? I would expect this will reduce how much code you introduced in the next patch. > + > /* > * struct xen_arch_domainconfig's ABI is covered by > * XEN_DOMCTL_INTERFACE_VERSION. > @@ -335,6 +341,8 @@ struct xen_arch_domainconfig { > * > */ > uint32_t clock_frequency; > + /* IN */ > + struct color_guest_config colors; > }; > #endif /* __XEN__ || __XEN_TOOLS__ */ >
diff --git a/xen/arch/arm/include/asm/coloring.h b/xen/arch/arm/include/asm/coloring.h index fdd46448d7..1f7e0dde79 100644 --- a/xen/arch/arm/include/asm/coloring.h +++ b/xen/arch/arm/include/asm/coloring.h @@ -23,8 +23,6 @@ #ifndef __ASM_ARM_COLORING_H__ #define __ASM_ARM_COLORING_H__ -#define MAX_COLORS_CELLS 4 - #ifdef CONFIG_COLORING #include <xen/sched.h> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h index 94b31511dd..627cc42164 100644 --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -303,6 +303,12 @@ struct vcpu_guest_context { typedef struct vcpu_guest_context vcpu_guest_context_t; DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t); +#define MAX_COLORS_CELLS 4 +struct color_guest_config { + uint32_t max_colors; + uint32_t colors[MAX_COLORS_CELLS]; +}; + /* * struct xen_arch_domainconfig's ABI is covered by * XEN_DOMCTL_INTERFACE_VERSION. @@ -335,6 +341,8 @@ struct xen_arch_domainconfig { * */ uint32_t clock_frequency; + /* IN */ + struct color_guest_config colors; }; #endif /* __XEN__ || __XEN_TOOLS__ */