Messages in this thread | | | From | Peter Gonda <> | Date | Mon, 10 May 2021 12:23:27 -0600 | Subject | Re: [PATCH Part2 RFC v2 16/37] crypto: ccp: Handle the legacy TMR allocation when SNP is enabled |
| |
> + > +static int snp_set_rmptable_state(unsigned long paddr, int npages, > + struct rmpupdate *val, bool locked, bool need_reclaim) > +{ > + unsigned long pfn = __sme_clr(paddr) >> PAGE_SHIFT; > + unsigned long pfn_end = pfn + npages; > + int rc; > + > + while (pfn < pfn_end) { > + if (need_reclaim) > + if (snp_reclaim_page(pfn_to_page(pfn), locked)) > + return -EFAULT; > + > + rc = rmpupdate(pfn_to_page(pfn), val); > + if (rc) > + return rc;
This functional can return an error but have partially converted some of the npages requested by the caller. Should this function return the number of affected pages or something to allow the caller to know if some pages need to be reverted? Or should the function attempt to do that itself?
> + > + pfn++; > + } > + > + return 0; > +}
> + > +static void __snp_free_firmware_pages(struct page *page, int order) > +{ > + struct rmpupdate val = {}; > + unsigned long paddr; > + > + if (!page) > + return; > + > + paddr = __pa((unsigned long)page_address(page)); > + > + if (snp_set_rmptable_state(paddr, 1 << order, &val, false, true)) > + return;
We now have leaked the given pages right? Should some warning be logged or should we track these leaked pages and maybe try and free them with a kworker?
> + > + __free_pages(page, order); > +} > +
| |