On 26.08.2022 14:51, Carlo Nonato wrote:
> --- a/xen/common/vmap.c
> +++ b/xen/common/vmap.c
> @@ -8,6 +8,9 @@
> #include <xen/types.h>
> #include <xen/vmap.h>
> #include <asm/page.h>
> +#ifdef CONFIG_CACHE_COLORING
> +#include <asm/coloring.h>
> +#endif
Even independent of my earlier question towards more code becoming common,
I think there will want to be a xen/coloring.h which takes care of this
abstraction, requiring such #ifdef in just a single place.
> @@ -218,6 +221,28 @@ void *__vmap(const mfn_t *mfn, unsigned int granularity,
> return va;
> }
>
> +#ifdef CONFIG_CACHE_COLORING
> +void * __vmap_colored(const mfn_t *mfn, unsigned int nr, unsigned int align,
> + unsigned int flags, enum vmap_region type)
Please no new functions with double underscores as prefix. Only static
symbol names may start with an underscore, and then also only with a
single one.
> +{
> + void *va = vm_alloc(nr, align, type);
> + unsigned long cur = (unsigned long)va;
> + paddr_t pa = mfn_to_maddr(*mfn);
> +
> + for ( ; va && nr-- ; cur += PAGE_SIZE )
> + {
> + pa = next_xen_colored(pa);
This may alter the address, yet the caller expects that the original
address be mapped. I must be missing something?
> + if ( map_pages_to_xen(cur, maddr_to_mfn(pa), 1, flags) )
> + {
> + vunmap(va);
> + return NULL;
> + }
> + pa += PAGE_SIZE;
> + }
> + return va;
> +}
Afaics you only consume the first slot of *mfn. What about the other
(nr - 1) ones? And compared to __vmap() there's no "granularity"
parameter, which is what controls the mapping of multiple contiguous
pages.
> --- a/xen/include/xen/vmap.h
> +++ b/xen/include/xen/vmap.h
> @@ -14,6 +14,10 @@ void vm_init_type(enum vmap_region type, void *start, void
> *end);
>
> void *__vmap(const mfn_t *mfn, unsigned int granularity, unsigned int nr,
> unsigned int align, unsigned int flags, enum vmap_region);
> +#ifdef CONFIG_CACHE_COLORING
> +void *__vmap_colored(const mfn_t *mfn, unsigned int nr, unsigned int align,
> + unsigned int flags, enum vmap_region);
> +#endif
I don't think such a declaration really needs putting inside #ifdef.
Jan
