Re: [PATCH v5 3/3] xen: add helpers to allocate unpopulated memory

[Jürgen Groß <jgross@xxxxxxxx>]

On 01.09.20 16:45, Roger Pau Monné wrote:
> On Tue, Sep 01, 2020 at 10:33:26AM +0200, Roger Pau Monne wrote:
> > +static int fill_list(unsigned int nr_pages)
> > +{
> > +       struct dev_pagemap *pgmap;
> > +       void *vaddr;
> > +       unsigned int i, alloc_pages = round_up(nr_pages, PAGES_PER_SECTION);
> > +       int nid, ret;
> > +
> > +       pgmap = kzalloc(sizeof(*pgmap), GFP_KERNEL);
> > +       if (!pgmap)
> > +               return -ENOMEM;
> > +
> > +       pgmap->type = MEMORY_DEVICE_GENERIC;
> > +       pgmap->res.name = "Xen scratch";
> > +       pgmap->res.flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> > +
> > +       ret = allocate_resource(&iomem_resource, &pgmap->res,
> > +                               alloc_pages * PAGE_SIZE, 0, -1,
> > +                               PAGES_PER_SECTION * PAGE_SIZE, NULL, NULL);
> > +       if (ret < 0) {
> > +               pr_err("Cannot allocate new IOMEM resource\n");
> > +               kfree(pgmap);
> > +               return ret;
> > +       }
> > +
> > +       nid = memory_add_physaddr_to_nid(pgmap->res.start);
> I think this is not needed ...
>
> > +
> > +#ifdef CONFIG_XEN_HAVE_PVMMU
> > +        /*
> > +         * memremap will build page tables for the new memory so
> > +         * the p2m must contain invalid entries so the correct
> > +         * non-present PTEs will be written.
> > +         *
> > +         * If a failure occurs, the original (identity) p2m entries
> > +         * are not restored since this region is now known not to
> > +         * conflict with any devices.
> > +         */
> > +       if (!xen_feature(XENFEAT_auto_translated_physmap)) {
> > +               xen_pfn_t pfn = PFN_DOWN(pgmap->res.start);
> > +
> > +               for (i = 0; i < alloc_pages; i++) {
> > +                       if (!set_phys_to_machine(pfn + i, INVALID_P2M_ENTRY)) {
> > +                               pr_warn("set_phys_to_machine() failed, no memory 
> > added\n");
> > +                               release_resource(&pgmap->res);
> > +                               kfree(pgmap);
> > +                               return -ENOMEM;
> > +                       }
> > +                }
> > +       }
> > +#endif
> > +
> > +       vaddr = memremap_pages(pgmap, nid);
> ... and NUMA_NO_NODE should be used here instead, as this memory is just
> fictitious space to map foreign memory, and shouldn't be related to
> any NUMA node.
>
> The following chunk should be folded in, or I can resend.
I can fold it in.


Juergen