Re: [PATCH v1 2/6] xen/riscv: introduce things necessary for p2m initialization

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 5/9/25 6:14 PM, Andrew Cooper wrote:

> On 09/05/2025 4:57 pm, Oleksii Kurochko wrote:
> > diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c
> > new file mode 100644
> > index 0000000000..ad4beef8f9
> > --- /dev/null
> > +++ b/xen/arch/riscv/p2m.c
> > @@ -0,0 +1,168 @@
> > +#include <xen/domain_page.h>
> > +#include <xen/iommu.h>
> > +#include <xen/lib.h>
> > +#include <xen/mm.h>
> > +#include <xen/pfn.h>
> > +#include <xen/rwlock.h>
> > +#include <xen/sched.h>
> > +#include <xen/spinlock.h>
> > +
> > +#include <asm/page.h>
> > +#include <asm/p2m.h>
> > +
> > +/*
> > + * Force a synchronous P2M TLB flush.
> > + *
> > + * Must be called with the p2m lock held.
> > + *
> > + * TODO: add support of flushing TLB connected to VMID.
> > + */
> > +static void p2m_force_tlb_flush_sync(struct p2m_domain *p2m)
> > +{
> > +    ASSERT(p2m_is_write_locked(p2m));
> > +
> > +    /*
> > +     * TODO: shouldn't be this flush done for each physical CPU?
> > +     *       If yes, then SBI call sbi_remote_hfence_gvma() could
> > +     *       be used for that.
> > +     */
> > +#if defined(__riscv_hh) || defined(__riscv_h)
> > +    asm volatile ( "hfence.gvma" ::: "memory" );
> > +#else
> > +    asm volatile ( ".insn r 0x73, 0x0, 0x31, x0, x0, x0" ::: "memory" );
> > +#endif
> >       
> TLB flushing needs to happen for each pCPU which potentially has cached
> a mapping.
>
> In other arches, this is tracked by d->dirty_cpumask which is the bitmap
> of pCPUs where this domain is scheduled.
>     
I could only find usage of d->dirty_cpumask in x86 and common code (grant
tables) for flushing the TLB. However, it seems that d->dirty_cpumask is
not set anywhere for ARM. Is it sufficient to set a bit in d->dirty_cpumask
in startup_cpu_idle_loop()?
In addition, it’s also necessary to set and clear bits in d->dirty_cpumask
during context_switch, correct? Set it before switching from the previous
domain, and clear it after switching to the new domain?
Also, when a bit is set in d->dirty_cpumask, the v->processor value is also
stored in v->dirty_cpu. Is this needed to track which processor is
currently being used for the vCPU?
> CPUs need to flush their TLBs before removing themselves from
> d->dirty_cpumask, which is typically done during context switch, but it
> means that to flush the P2M, you only need to IPI a subset of CPUs.
>     
I can't find where the P2M flush happens for x86/ARM. Could you please point me
to where it is handled?

Also, I found guest_flush_tlb_mask() for x86. I assume that it is x86 specific
and generally it is enough to have only flush_tlb_mask(), right?

Thanks in advance for the answers.
~ Oleksii