[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 14/19] xen/arm: add Persistent Map (PMAP) infrastructure
On 21.02.2022 11:22, Julien Grall wrote: > --- a/xen/common/Makefile > +++ b/xen/common/Makefile > @@ -28,6 +28,7 @@ obj-y += multicall.o > obj-y += notifier.o > obj-y += page_alloc.o > obj-$(CONFIG_HAS_PDX) += pdx.o > +obj-bin-$(CONFIG_HAS_PMAP) += pmap.init.o > obj-$(CONFIG_PERF_COUNTERS) += perfc.o > obj-y += preempt.o > obj-y += random.o Nit: Please move the insertion one line further down. > --- /dev/null > +++ b/xen/common/pmap.c > @@ -0,0 +1,79 @@ > +#include <xen/bitops.h> > +#include <xen/init.h> > +#include <xen/pmap.h> > + > +#include <asm/pmap.h> > +#include <asm/fixmap.h> > + > +/* > + * Simple mapping infrastructure to map / unmap pages in fixed map. > + * This is used to set up the page table for mapcache, which is used > + * by map domain page infrastructure. Is this comment stale from its original x86 purpose? > + * This structure is not protected by any locks, so it must not be used after > + * smp bring-up. > + */ > + > +/* Bitmap to track which slot is used */ > +static unsigned long __initdata inuse; I guess this wants to use DECLARE_BITMAP(), for ... > +void *__init pmap_map(mfn_t mfn) > +{ > + unsigned long flags; > + unsigned int idx; > + unsigned int slot; > + > + BUILD_BUG_ON(sizeof(inuse) * BITS_PER_BYTE < NUM_FIX_PMAP); > + > + ASSERT(system_state < SYS_STATE_smp_boot); > + > + local_irq_save(flags); > + > + idx = find_first_zero_bit(&inuse, NUM_FIX_PMAP); ... this to be correct irrespective of how large NUM_FIX_PMAP is? I think that's preferable over the BUILD_BUG_ON(). > + if ( idx == NUM_FIX_PMAP ) > + panic("Out of PMAP slots\n"); > + > + __set_bit(idx, &inuse); > + > + slot = idx + FIXMAP_PMAP_BEGIN; > + ASSERT(slot >= FIXMAP_PMAP_BEGIN && slot <= FIXMAP_PMAP_END); > + > + /* > + * We cannot use set_fixmap() here. We use PMAP when there is no direct > map, > + * so map_pages_to_xen() called by set_fixmap() needs to map pages on > + * demand, which then calls pmap() again, resulting in a loop. Modify the > + * PTEs directly instead. The same is true for pmap_unmap(). > + */ > + arch_pmap_map(slot, mfn); I'm less certain here, but like above I'm under the impression that this comment may no longer be accurate. > + local_irq_restore(flags); What is this IRQ save/restore intended to protect against, when use of this function is limited to pre-SMP boot anyway? Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |