Re: [PATCH RFCv2 12/15] xen/arm: add Persistent Map (PMAP) infrastructure

[Julien Grall <julien@xxxxxxx>]

Hi Stefano,

On 15/05/2021 00:25, Stefano Stabellini wrote:
> On Sun, 25 Apr 2021, Julien Grall wrote:
> > From: Wei Liu <wei.liu2@xxxxxxxxxx>
> > +extern lpae_t xen_fixmap[LPAE_ENTRIES];
> > +
> > +void *__init pmap_map(mfn_t mfn)
> > +{
> > +    unsigned long flags;
> > +    unsigned int idx;
> > +    vaddr_t linear;
> > +    unsigned int slot;
> > +    lpae_t *entry, pte;
> > +
> > +    BUILD_BUG_ON(sizeof(inuse) * BITS_PER_LONG < NUM_FIX_PMAP);
> > +
> > +    ASSERT(system_state < SYS_STATE_smp_boot);
> One small concern here is that we have been using SYS_STATE_early_boot
> to switch implementation of things like xen_map_table. Between
> SYS_STATE_early_boot and SYS_STATE_smp_boot there is SYS_STATE_boot.
>
> I guess I am wondering if instead of three potentially different mapping
> functions (<= SYS_STATE_early_boot, < SYS_STATE_smp_boot, >=
> SYS_STATE_smp_boot) we can get away with only two?
This is more flexible than the existing method to map memory when state 
== SYS_STATE_early_boot. If you look at the next patch (#13), you will 
see that there will be only two method to map memory.
[...]

> > diff --git a/xen/include/asm-arm/pmap.h b/xen/include/asm-arm/pmap.h
> > new file mode 100644
> > index 000000000000..8e1dce93f8e4
> > --- /dev/null
> > +++ b/xen/include/asm-arm/pmap.h
> > @@ -0,0 +1,10 @@
> > +#ifndef __ASM_PMAP_H__
> > +#define __ARM_PMAP_H__
> ASM/ARM
I will fix it.

> > +/* Large enough for mapping 5 levels of page tables with some headroom */
> > +#define NUM_FIX_PMAP 8
> > +
> > +void *pmap_map(mfn_t mfn);
> > +void pmap_unmap(const void *p);
> > +#endif /* __ASM_PMAP_H__ */
Cheers,

--
Julien Grall