On Wed, Feb 03, 2021, Will Deacon wrote:
> On Fri, Jan 08, 2021 at 12:15:14PM +0000, Quentin Perret wrote:
...
> > +static inline unsigned long hyp_s1_pgtable_size(void)
> > +{
...
> > + res += nr_pages << PAGE_SHIFT;
> > + }
> > +
> > + /* Allow 1 GiB for private mappings */
> > + nr_pages = (1 << 30) >> PAGE_SHIFT;
>
> SZ_1G >> PAGE_SHIFT
Where does the 1gb magic number come from? IIUC, this is calculating the number
of pages needed for the hypervisor's Stage-1 page tables. The amount of memory
needed for those page tables should be easily calculated, and assuming huge
pages can be used, should be far less the 1gb.
> > + nr_pages = __hyp_pgtable_max_pages(nr_pages);
> > + res += nr_pages << PAGE_SHIFT;
> > +
> > + return res;
...
> > +void __init kvm_hyp_reserve(void)
> > +{
> > + u64 nr_pages, prev;
> > +
> > + if (!is_hyp_mode_available() || is_kernel_in_hyp_mode())
> > + return;
> > +
> > + if (kvm_get_mode() != KVM_MODE_PROTECTED)
> > + return;
> > +
> > + if (kvm_nvhe_sym(hyp_memblock_nr) < 0) {
> > + kvm_err("Failed to register hyp memblocks\n");
> > + return;
> > + }
> > +
> > + sort_memblock_regions();
> > +
> > + /*
> > + * We don't know the number of possible CPUs yet, so allocate for the
> > + * worst case.
> > + */
> > + hyp_mem_size += NR_CPUS << PAGE_SHIFT;
Is this for per-cpu stack?
If so, what guarantees a single page is sufficient? Mostly a curiosity question,
since it looks like this is an existing assumption by init_hyp_mode(). Shouldn't
the required stack size be defined in bytes and converted to pages, or is there a
guarantee that 64kb pages will be used?
> There was a recent patch bumping NR_CPUs to 512, so this would be 32MB
> with 64k pages. Is it possible to return memory to the host later on once
> we have a better handle on the number of CPUs in the system?
Does kvm_hyp_reserve() really need to be called during bootmem_init()? What
prevents doing the reservation during init_hyp_mode()? If the problem is that
pKVM needs a single contiguous chunk of memory, then it might be worth solving
_that_ problem, e.g. letting the host donate memory in N-byte chunks instead of
requiring a single huge blob of memory.
> > + hyp_mem_size += hyp_s1_pgtable_size();
