[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 07/22] x86/pv: domheap pages should be mapped while relocating initrd


  • To: Julien Grall <julien@xxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 22 Dec 2022 12:18:49 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=aP9W60VpTyxZH8ArdFJJbj0zoSjt/ZG7rFAEM0rVdxY=; b=nrox0zGJcNDIVBbrtryRJ4+ClUQz43bGq5zdEkBrNBnwyIQvK8opvCjDPjzqPdHilWq+dYyKOe2rC98TN4ahBZGhsRSjeNuYZ0lP5xvK1VhnI//OVaEC446/3n3tcgOlohXoJhPNpT4IFpNkqZJlx9xxm0vm0g5UOr90SpT4ppcylNAAC6Pb2/ozv/MzmctzfC0h4+c9lYk6ElGoS4wSpqvN5/KgWBgVOlPBBX+Q3J4Kq1x+O/gFHDUmA/xrOcvWKjduzHHytTTCFazh1xqEtZzo3vb1NrETjKltvWuiIbeTLEIVPXyqOmyjfuv/F2lNk6eWVuZ7xSIFJeRcIqGTqQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=E9hsV7XtxLn1zbhT7xnusjA2GcLUpwzx7JYK2z2JtCByXutYhajVZw9Ef8IC/K3GGC1RxA/2XcauecFcY1H3qc7C11ne6Ln93B0Z645W3nX1MtPAdkIOIUf0eGc8ZZPpGFePpdloAALtPUV9LGXZZBEQ+7VuormdbSCk3xgGKAJBUxXd8YuRqnUA0vyGg74DPVunqX2o2B8Z+qkiRaNaXjiwsilTJGGiuEru0Kes3HNa6p9ndPpVBQSphXoT3ZHSqSUpLlCwQ4CzuGnpaHnjm1W0RVjTo2AmQb6AujZQEzrbKlDOc7A8z5mCk01f4exkt2dco+NoaCsc/FHBpwD84g==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Wei Liu <wei.liu2@xxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Wei Wang <wawei@xxxxxxxxx>, Julien Grall <jgrall@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Thu, 22 Dec 2022 11:18:58 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 16.12.2022 12:48, Julien Grall wrote:
> From: Wei Liu <wei.liu2@xxxxxxxxxx>
> 
> Xen shouldn't use domheap page as if they were xenheap pages. Map and
> unmap pages accordingly.
> 
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> Signed-off-by: Wei Wang <wawei@xxxxxxxxx>
> Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> 
> ----
> 
>     Changes since Hongyan's version:
>         * Add missing newline after the variable declaration
> ---
>  xen/arch/x86/pv/dom0_build.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
> index a62f0fa2ef29..c837b2d96f89 100644
> --- a/xen/arch/x86/pv/dom0_build.c
> +++ b/xen/arch/x86/pv/dom0_build.c
> @@ -611,18 +611,32 @@ int __init dom0_construct_pv(struct domain *d,
>          if ( d->arch.physaddr_bitsize &&
>               ((mfn + count - 1) >> (d->arch.physaddr_bitsize - PAGE_SHIFT)) )
>          {
> +            unsigned long nr_pages;
> +            unsigned long len = initrd_len;
> +
>              order = get_order_from_pages(count);
>              page = alloc_domheap_pages(d, order, MEMF_no_scrub);
>              if ( !page )
>                  panic("Not enough RAM for domain 0 initrd\n");
> +
> +            nr_pages = 1UL << order;

I don't think this needs establishing here and ...

>              for ( count = -count; order--; )
>                  if ( count & (1UL << order) )
>                  {
>                      free_domheap_pages(page, order);
>                      page += 1UL << order;
> +                    nr_pages -= 1UL << order;

... updating here. Doing so just once ...

>                  }
> -            memcpy(page_to_virt(page), mfn_to_virt(initrd->mod_start),
> -                   initrd_len);
> +

... here ought to suffice, assuming this 2nd variable is needed at all
(alongside "len").

> +            for ( i = 0; i < nr_pages; i++, len -= PAGE_SIZE )
> +            {
> +                void *p = __map_domain_page(page + i);
> +
> +                memcpy(p, mfn_to_virt(initrd_mfn + i),
> +                       min(len, (unsigned long)PAGE_SIZE));
> +                unmap_domain_page(p);
> +            }

You're half open-coding copy_domain_page() without saying anywhere why
the remaining mfn_to_virt() is okay to keep. If you used
copy_domain_page(), no such remark would need adding in the description.

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.