Re: [PATCH v4 08/11] xen/arm: use colored allocator for p2m page tables

[Carlo Nonato <carlo.nonato@xxxxxxxxxxxxxxx>]

Hi Julien,

On Thu, Jan 26, 2023 at 11:25 AM Julien Grall <julien@xxxxxxx> wrote:
>
> Hi Carlo,
>
> On 23/01/2023 15:47, Carlo Nonato wrote:
> > Cache colored domains can benefit from having p2m page tables allocated
> > with the same coloring schema so that isolation can be achieved also for
> > those kind of memory accesses.
> > In order to do that, the domain struct is passed to the allocator and the
> > MEMF_no_owner flag is used.
> >
> > Signed-off-by: Carlo Nonato <carlo.nonato@xxxxxxxxxxxxxxx>
> > Signed-off-by: Marco Solieri <marco.solieri@xxxxxxxxxxxxxxx>
> > ---
> > v4:
> > - fixed p2m page allocation using MEMF_no_owner memflag
> > ---
> >   xen/arch/arm/p2m.c | 11 +++++++++--
> >   1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > index 948f199d84..f9faeb61af 100644
> > --- a/xen/arch/arm/p2m.c
> > +++ b/xen/arch/arm/p2m.c
> > @@ -4,6 +4,7 @@
> >   #include <xen/iocap.h>
> >   #include <xen/ioreq.h>
> >   #include <xen/lib.h>
> > +#include <xen/llc_coloring.h>
> >   #include <xen/sched.h>
> >   #include <xen/softirq.h>
> >
> > @@ -56,7 +57,10 @@ static struct page_info *p2m_alloc_page(struct domain *d)
> >        */
> >       if ( is_hardware_domain(d) )
> >       {
> > -        pg = alloc_domheap_page(NULL, 0);
> > +        if ( is_domain_llc_colored(d) )
> > +            pg = alloc_domheap_page(d, MEMF_no_owner);
> > +        else
> > +            pg = alloc_domheap_page(NULL, 0);
> I don't think we need to special case a colored domain here.You could
> simply always pass the domain/MEMF_no_owner and let the function decide
> what to do.
>
> This approach would also be useful when NUMA will be supported on Arm
> (the series is still under review).

Ok, nice. This was pointed out also by Jan in the previous revision.

> >           if ( pg == NULL )
> >               printk(XENLOG_G_ERR "Failed to allocate P2M pages for 
> > hwdom.\n");
> >       }
> > @@ -105,7 +109,10 @@ int p2m_set_allocation(struct domain *d, unsigned long 
> > pages, bool *preempted)
> >           if ( d->arch.paging.p2m_total_pages < pages )
> >           {
> >               /* Need to allocate more memory from domheap */
> > -            pg = alloc_domheap_page(NULL, 0);
> > +            if ( is_domain_llc_colored(d) )
> > +                pg = alloc_domheap_page(d, MEMF_no_owner);
> > +            else
> > +                pg = alloc_domheap_page(NULL, 0);
>
> Ditto.
>
> >               if ( pg == NULL )
> >               {
> >                   printk(XENLOG_ERR "Failed to allocate P2M pages.\n");
>
> Cheers,
>
> --
> Julien Grall