Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1202904 > unrolled thread

[PATCH v3 00/20] xen/arm64: Add support for 64KB page

Started byJulien Grall <julien.grall@citrix.com>
First post2015-08-07 18:50 +0200
Last post2015-08-20 17:20 +0200
Articles 20 on this page of 54 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v3 00/20] xen/arm64: Add support for 64KB page Julien Grall <julien.grall@citrix.com> - 2015-08-07 18:50 +0200
    [PATCH v3 09/20] xen/biomerge: Don't allow biovec to be merge when Linux is not using 4KB page Julien Grall <julien.grall@citrix.com> - 2015-08-07 18:50 +0200
      Re: [PATCH v3 09/20] xen/biomerge: Don't allow biovec to be merge  when Linux is not using 4KB page Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 13:00 +0200
        Re: [Xen-devel] [PATCH v3 09/20] xen/biomerge: Don't allow biovec  to be merge when Linux is not using 4KB page Julien Grall <julien.grall@citrix.com> - 2015-08-10 13:30 +0200
          Re: [Xen-devel] [PATCH v3 09/20] xen/biomerge: Don't allow biovec  to be merge when Linux is not using 4KB page Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 13:30 +0200
            Re: [Xen-devel] [PATCH v3 09/20] xen/biomerge: Don't allow biovec  to be merge when Linux is not using 4KB page Julien Grall <julien.grall@citrix.com> - 2015-08-10 13:40 +0200
              Re: [Xen-devel] [PATCH v3 09/20] xen/biomerge: Don't allow biovec  to be merge when Linux is not using 4KB page David Vrabel <david.vrabel@citrix.com> - 2015-08-10 14:50 +0200
    [PATCH v3 07/20] block/xen-blkfront: Store a page rather a pfn in the grant structure Julien Grall <julien.grall@citrix.com> - 2015-08-07 18:50 +0200
    [PATCH v3 06/20] block/xen-blkfront: Split blkif_queue_request in 2 Julien Grall <julien.grall@citrix.com> - 2015-08-07 18:50 +0200
    [PATCH v3 03/20] xen: Add Xen specific page definition Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:00 +0200
      Re: [PATCH v3 03/20] xen: Add Xen specific page definition Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 12:50 +0200
      Re: [Xen-devel] [PATCH v3 03/20] xen: Add Xen specific page definition David Vrabel <david.vrabel@citrix.com> - 2015-08-20 11:50 +0200
    [PATCH v3 01/20] net/xen-netback: xenvif_gop_frag_copy: move GSO check out of the loop Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:00 +0200
      Re: [PATCH v3 01/20] net/xen-netback: xenvif_gop_frag_copy: move GSO  check out of the loop Wei Liu <wei.liu2@citrix.com> - 2015-08-08 16:00 +0200
    [PATCH v3 14/20] xen/grant-table: Make it running on 64KB granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
    Re: [Xen-devel] [PATCH v3 00/20] xen/arm64: Add support for 64KB  page Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
    [PATCH v3 10/20] xen/xenbus: Use Xen page definition Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
    [PATCH v3 17/20] net/xen-netfront: Make it running on 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
      Re: [Xen-devel] [PATCH v3 17/20] net/xen-netfront: Make it running  on 64KB page granularity David Vrabel <david.vrabel@citrix.com> - 2015-08-20 12:10 +0200
    [PATCH v3 11/20] tty/hvc: xen: Use xen page definition Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
      Re: [Xen-devel] [PATCH v3 11/20] tty/hvc: xen: Use xen page definition David Vrabel <david.vrabel@citrix.com> - 2015-08-20 12:00 +0200
        Re: [Xen-devel] [PATCH v3 11/20] tty/hvc: xen: Use xen page definition Julien Grall <julien.grall@citrix.com> - 2015-08-28 17:10 +0200
    [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
      Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity  is the same for Xen and Linux Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 13:20 +0200
        Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity  is the same for Xen and Linux Julien Grall <julien.grall@citrix.com> - 2015-08-10 13:40 +0200
          Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity  is the same for Xen and Linux Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 15:00 +0200
            Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity  is the same for Xen and Linux Julien Grall <julien.grall@citrix.com> - 2015-08-10 15:40 +0200
      Re: [Xen-devel] [PATCH v3 12/20] xen/balloon: Don't rely on the page  granularity is the same for Xen and Linux David Vrabel <david.vrabel@citrix.com> - 2015-08-20 12:10 +0200
        Re: [Xen-devel] [PATCH v3 12/20] xen/balloon: Don't rely on the page  granularity is the same for Xen and Linux Julien Grall <julien.grall@citrix.com> - 2015-08-28 17:20 +0200
    [PATCH v3 13/20] xen/events: fifo: Make it running on 64KB granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
    [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
      Re: [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page  granularity Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 14:10 +0200
        Re: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux  64KB page granularity David Vrabel <david.vrabel@citrix.com> - 2015-08-10 14:20 +0200
          Re: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux  64KB page granularity Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 15:00 +0200
            Re: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux  64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-10 15:30 +0200
        Re: [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page  granularity Julien Grall <julien.grall@citrix.com> - 2015-09-01 19:20 +0200
      Re: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux  64KB page granularity David Vrabel <david.vrabel@citrix.com> - 2015-08-20 12:10 +0200
    [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
      Re: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page  granularity Wei Liu <wei.liu2@citrix.com> - 2015-08-08 17:00 +0200
        Re: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page  granularity Julien Grall <julien.grall@citrix.com> - 2015-08-10 12:00 +0200
          Re: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page  granularity Wei Liu <wei.liu2@citrix.com> - 2015-08-10 13:40 +0200
            Re: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page  granularity Julien Grall <julien.grall@citrix.com> - 2015-08-10 14:10 +0200
    [PATCH v3 15/20] block/xen-blkfront: Make it running on 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
      Re: [PATCH v3 15/20] block/xen-blkfront: Make it running on 64KB  page granularity Roger Pau Monné <roger.pau@citrix.com> - 2015-08-20 10:20 +0200
        Re: [PATCH v3 15/20] block/xen-blkfront: Make it running on 64KB  page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-28 17:40 +0200
    [PATCH v3 16/20] block/xen-blkback: Make it running on 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
      Re: [PATCH v3 16/20] block/xen-blkback: Make it running on 64KB page  granularity Roger Pau Monné <roger.pau@citrix.com> - 2015-08-20 10:20 +0200
    [PATCH v3 20/20] arm/xen: Add support for 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
      Re: [PATCH v3 20/20] arm/xen: Add support for 64KB page  granularity Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 15:00 +0200
    Re: [Xen-devel] [PATCH v3 00/20] xen/arm64: Add support for 64KB  page Julien Grall <julien.grall@citrix.com> - 2015-08-20 02:50 +0200
      Re: [Xen-devel] [PATCH v3 00/20] xen/arm64: Add support for 64KB  page Roger Pau Monné <roger.pau@citrix.com> - 2015-08-20 10:20 +0200
      Re: [Xen-devel] [PATCH v3 00/20] xen/arm64: Add support for 64KB  page David Vrabel <david.vrabel@citrix.com> - 2015-08-20 12:20 +0200
        Re: [Xen-devel] [PATCH v3 00/20] xen/arm64: Add support for 64KB  page Julien Grall <julien.grall@citrix.com> - 2015-08-20 17:10 +0200
          Re: [Xen-devel] [PATCH v3 00/20] xen/arm64: Add support for 64KB  page David Vrabel <david.vrabel@citrix.com> - 2015-08-20 17:20 +0200

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →


#1210368 — Re: [Xen-devel] [PATCH v3 11/20] tty/hvc: xen: Use xen page definition

FromDavid Vrabel <david.vrabel@citrix.com>
Date2015-08-20 12:00 +0200
SubjectRe: [Xen-devel] [PATCH v3 11/20] tty/hvc: xen: Use xen page definition
Message-ID<pZuND-1gR-59@gated-at.bofh.it>
In reply to#1202943
On 07/08/15 17:46, Julien Grall wrote:
> The console ring is always based on the page granularity of Xen.
[...]
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -230,7 +230,7 @@ static int xen_hvm_console_init(void)
>  	if (r < 0 || v == 0)
>  		goto err;
>  	gfn = v;
> -	info->intf = xen_remap(gfn << PAGE_SHIFT, PAGE_SIZE);
> +	info->intf = xen_remap(gfn << XEN_PAGE_SHIFT, PAGE_SIZE);

You need XEN_PAGE_SIZE here I think...

>  	if (info->intf == NULL)
>  		goto err;
>  	info->vtermno = HVC_COOKIE;
> @@ -472,7 +472,7 @@ static int xencons_resume(struct xenbus_device *dev)
>  	struct xencons_info *info = dev_get_drvdata(&dev->dev);
>  
>  	xencons_disconnect_backend(info);
> -	memset(info->intf, 0, PAGE_SIZE);
> +	memset(info->intf, 0, XEN_PAGE_SIZE);

...particularly since you use it here.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1215386 — Re: [Xen-devel] [PATCH v3 11/20] tty/hvc: xen: Use xen page definition

FromJulien Grall <julien.grall@citrix.com>
Date2015-08-28 17:10 +0200
SubjectRe: [Xen-devel] [PATCH v3 11/20] tty/hvc: xen: Use xen page definition
Message-ID<q2trY-6Qn-13@gated-at.bofh.it>
In reply to#1210368
Hi David,

On 20/08/15 10:55, David Vrabel wrote:
> On 07/08/15 17:46, Julien Grall wrote:
>> The console ring is always based on the page granularity of Xen.
> [...]
>> --- a/drivers/tty/hvc/hvc_xen.c
>> +++ b/drivers/tty/hvc/hvc_xen.c
>> @@ -230,7 +230,7 @@ static int xen_hvm_console_init(void)
>>  	if (r < 0 || v == 0)
>>  		goto err;
>>  	gfn = v;
>> -	info->intf = xen_remap(gfn << PAGE_SHIFT, PAGE_SIZE);
>> +	info->intf = xen_remap(gfn << XEN_PAGE_SHIFT, PAGE_SIZE);
> 
> You need XEN_PAGE_SIZE here I think...

Right, I did the mistake while rebase on my s/mfn/gfn/ series. I will
fix it in the next version.

Regards,

-- 
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1202944 — [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux

FromJulien Grall <julien.grall@citrix.com>
Date2015-08-07 19:20 +0200
Subject[PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux
Message-ID<pUTtg-61C-9@gated-at.bofh.it>
In reply to#1202904
For ARM64 guests, Linux is able to support either 64K or 4K page
granularity. Although, the hypercall interface is always based on 4K
page granularity.

With 64K page granularity, a single page will be spread over multiple
Xen frame.

To avoid splitting the page into 4K frame, take advantage of the
extent_order field to directly allocate/free chunk of the Linux page
size.

Note that PVMMU is only used for PV guest (which is x86) and the page
granularity is always 4KB. Some BUILD_BUG_ON has been added to ensure
that because the code has not been modified.

Signed-off-by: Julien Grall <julien.grall@citrix.com>

---
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>

    Changes in v3:
        - Fix errors reported by checkpatch.pl
        - s/mfn/gfn/ based on the new naming
        - Rather than splitting the page into 4KB chunk, use the
        extent_order field to allocate directly a Linux page size. This
        is avoid lots of code for no benefits.

    Changes in v2:
        - Use xen_apply_to_page to split a page in 4K chunk
        - It's not necessary to have a smaller frame list. Re-use
        PAGE_SIZE
        - Convert reserve_additional_memory to use XEN_... macro
---
 drivers/xen/balloon.c | 47 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 9734649..c8739c8 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -70,6 +70,9 @@
 #include <xen/features.h>
 #include <xen/page.h>
 
+/* Use one extent per PAGE_SIZE to avoid break down into multiple frame */
+#define EXTENT_ORDER (fls(XEN_PFN_PER_PAGE) - 1)
+
 /*
  * balloon_process() state:
  *
@@ -230,6 +233,11 @@ static enum bp_state reserve_additional_memory(long credit)
 	nid = memory_add_physaddr_to_nid(hotplug_start_paddr);
 
 #ifdef CONFIG_XEN_HAVE_PVMMU
+	/* We don't support PV MMU when Linux and Xen is using
+	 * different page granularity.
+	 */
+	BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
+
         /*
          * add_memory() will build page tables for the new memory so
          * the p2m must contain invalid entries so the correct
@@ -326,11 +334,11 @@ static enum bp_state reserve_additional_memory(long credit)
 static enum bp_state increase_reservation(unsigned long nr_pages)
 {
 	int rc;
-	unsigned long  pfn, i;
+	unsigned long i;
 	struct page   *page;
 	struct xen_memory_reservation reservation = {
 		.address_bits = 0,
-		.extent_order = 0,
+		.extent_order = EXTENT_ORDER,
 		.domid        = DOMID_SELF
 	};
 
@@ -352,7 +360,11 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
 			nr_pages = i;
 			break;
 		}
-		frame_list[i] = page_to_pfn(page);
+
+		/* XENMEM_populate_physmap requires a PFN based on Xen
+		 * granularity.
+		 */
+		frame_list[i] = xen_page_to_pfn(page);
 		page = balloon_next_page(page);
 	}
 
@@ -366,10 +378,15 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
 		page = balloon_retrieve(false);
 		BUG_ON(page == NULL);
 
-		pfn = page_to_pfn(page);
-
 #ifdef CONFIG_XEN_HAVE_PVMMU
+		/* We don't support PV MMU when Linux and Xen is using
+		 * different page granularity.
+		 */
+		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
+
 		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
+			unsigned long pfn = page_to_pfn(page);
+
 			set_phys_to_machine(pfn, frame_list[i]);
 
 			/* Link back into the page tables if not highmem. */
@@ -396,14 +413,15 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
 static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
 {
 	enum bp_state state = BP_DONE;
-	unsigned long  pfn, i;
+	unsigned long i;
 	struct page   *page;
 	int ret;
 	struct xen_memory_reservation reservation = {
 		.address_bits = 0,
-		.extent_order = 0,
+		.extent_order = EXTENT_ORDER,
 		.domid        = DOMID_SELF
 	};
+	static struct page *pages[ARRAY_SIZE(frame_list)];
 
 #ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
 	if (balloon_stats.hotplug_pages) {
@@ -426,7 +444,9 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
 		}
 		scrub_page(page);
 
-		frame_list[i] = page_to_pfn(page);
+		/* XENMEM_decrease_reservation requires a GFN */
+		frame_list[i] = xen_page_to_gfn(page);
+		pages[i] = page;
 	}
 
 	/*
@@ -440,12 +460,17 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
 
 	/* Update direct mapping, invalidate P2M, and add to balloon. */
 	for (i = 0; i < nr_pages; i++) {
-		pfn = frame_list[i];
-		frame_list[i] = pfn_to_gfn(pfn);
-		page = pfn_to_page(pfn);
+		page = pages[i];
 
 #ifdef CONFIG_XEN_HAVE_PVMMU
+		/* We don't support PV MMU when Linux and Xen is using
+		 * different page granularity.
+		 */
+		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
+
 		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
+			unsigned long pfn = page_to_pfn(page);
+
 			if (!PageHighMem(page)) {
 				ret = HYPERVISOR_update_va_mapping(
 						(unsigned long)__va(pfn << PAGE_SHIFT),
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1204174 — Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux

FromStefano Stabellini <stefano.stabellini@eu.citrix.com>
Date2015-08-10 13:20 +0200
SubjectRe: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux
Message-ID<pVThw-4Hc-13@gated-at.bofh.it>
In reply to#1202944
On Fri, 7 Aug 2015, Julien Grall wrote:
> For ARM64 guests, Linux is able to support either 64K or 4K page
> granularity. Although, the hypercall interface is always based on 4K
> page granularity.
> 
> With 64K page granularity, a single page will be spread over multiple
> Xen frame.
> 
> To avoid splitting the page into 4K frame, take advantage of the
> extent_order field to directly allocate/free chunk of the Linux page
> size.
> 
> Note that PVMMU is only used for PV guest (which is x86) and the page
> granularity is always 4KB. Some BUILD_BUG_ON has been added to ensure
> that because the code has not been modified.
> 
> Signed-off-by: Julien Grall <julien.grall@citrix.com>

This is much better than the previous version. Good idea using the
extent_order field.

I only have a minor comment below.


> ---
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> 
>     Changes in v3:
>         - Fix errors reported by checkpatch.pl
>         - s/mfn/gfn/ based on the new naming
>         - Rather than splitting the page into 4KB chunk, use the
>         extent_order field to allocate directly a Linux page size. This
>         is avoid lots of code for no benefits.
> 
>     Changes in v2:
>         - Use xen_apply_to_page to split a page in 4K chunk
>         - It's not necessary to have a smaller frame list. Re-use
>         PAGE_SIZE
>         - Convert reserve_additional_memory to use XEN_... macro
> ---
>  drivers/xen/balloon.c | 47 ++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 36 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 9734649..c8739c8 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -70,6 +70,9 @@
>  #include <xen/features.h>
>  #include <xen/page.h>
>  
> +/* Use one extent per PAGE_SIZE to avoid break down into multiple frame */
> +#define EXTENT_ORDER (fls(XEN_PFN_PER_PAGE) - 1)
> +
>  /*
>   * balloon_process() state:
>   *
> @@ -230,6 +233,11 @@ static enum bp_state reserve_additional_memory(long credit)
>  	nid = memory_add_physaddr_to_nid(hotplug_start_paddr);
>  
>  #ifdef CONFIG_XEN_HAVE_PVMMU
> +	/* We don't support PV MMU when Linux and Xen is using
> +	 * different page granularity.
> +	 */
> +	BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
> +
>          /*
>           * add_memory() will build page tables for the new memory so
>           * the p2m must contain invalid entries so the correct
> @@ -326,11 +334,11 @@ static enum bp_state reserve_additional_memory(long credit)
>  static enum bp_state increase_reservation(unsigned long nr_pages)
>  {
>  	int rc;
> -	unsigned long  pfn, i;
> +	unsigned long i;
>  	struct page   *page;
>  	struct xen_memory_reservation reservation = {
>  		.address_bits = 0,
> -		.extent_order = 0,
> +		.extent_order = EXTENT_ORDER,
>  		.domid        = DOMID_SELF
>  	};
>  
> @@ -352,7 +360,11 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
>  			nr_pages = i;
>  			break;
>  		}
> -		frame_list[i] = page_to_pfn(page);
> +
> +		/* XENMEM_populate_physmap requires a PFN based on Xen
> +		 * granularity.
> +		 */
> +		frame_list[i] = xen_page_to_pfn(page);
>  		page = balloon_next_page(page);
>  	}
>  
> @@ -366,10 +378,15 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
>  		page = balloon_retrieve(false);
>  		BUG_ON(page == NULL);
>  
> -		pfn = page_to_pfn(page);
> -
>  #ifdef CONFIG_XEN_HAVE_PVMMU
> +		/* We don't support PV MMU when Linux and Xen is using
> +		 * different page granularity.
> +		 */
> +		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
> +
>  		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
> +			unsigned long pfn = page_to_pfn(page);
> +
>  			set_phys_to_machine(pfn, frame_list[i]);
>  
>  			/* Link back into the page tables if not highmem. */
> @@ -396,14 +413,15 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
>  static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>  {
>  	enum bp_state state = BP_DONE;
> -	unsigned long  pfn, i;
> +	unsigned long i;
>  	struct page   *page;
>  	int ret;
>  	struct xen_memory_reservation reservation = {
>  		.address_bits = 0,
> -		.extent_order = 0,
> +		.extent_order = EXTENT_ORDER,
>  		.domid        = DOMID_SELF
>  	};
> +	static struct page *pages[ARRAY_SIZE(frame_list)];

This array can be rather large: I would try to avoid it, see below.


>  #ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
>  	if (balloon_stats.hotplug_pages) {
> @@ -426,7 +444,9 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>  		}
>  		scrub_page(page);
>  
> -		frame_list[i] = page_to_pfn(page);
> +		/* XENMEM_decrease_reservation requires a GFN */
> +		frame_list[i] = xen_page_to_gfn(page);
> +		pages[i] = page;
>  	}
>  
>  	/*
> @@ -440,12 +460,17 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>  
>  	/* Update direct mapping, invalidate P2M, and add to balloon. */
>  	for (i = 0; i < nr_pages; i++) {
> -		pfn = frame_list[i];
> -		frame_list[i] = pfn_to_gfn(pfn);
> -		page = pfn_to_page(pfn);
> +		page = pages[i];
>  
>  #ifdef CONFIG_XEN_HAVE_PVMMU
> +		/* We don't support PV MMU when Linux and Xen is using
> +		 * different page granularity.
> +		 */
> +		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
> +
>  		if (!xen_feature(XENFEAT_auto_translated_physmap)) {
> +			unsigned long pfn = page_to_pfn(page);

I would simply and avoid introducing a new array:
    pfn = (frame_list[i] << XEN_PAGE_SHIFT) >> PAGE_SHIFT;
    page = pfn_to_page(pfn);
    
    
    
>  			if (!PageHighMem(page)) {
>  				ret = HYPERVISOR_update_va_mapping(
>  						(unsigned long)__va(pfn << PAGE_SHIFT),
> -- 
> 2.1.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1204183 — Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux

FromJulien Grall <julien.grall@citrix.com>
Date2015-08-10 13:40 +0200
SubjectRe: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux
Message-ID<pVTAS-53A-25@gated-at.bofh.it>
In reply to#1204174
Hi Stefano,

On 10/08/15 12:18, Stefano Stabellini wrote:
>>  			/* Link back into the page tables if not highmem. */
>> @@ -396,14 +413,15 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
>>  static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>>  {
>>  	enum bp_state state = BP_DONE;
>> -	unsigned long  pfn, i;
>> +	unsigned long i;
>>  	struct page   *page;
>>  	int ret;
>>  	struct xen_memory_reservation reservation = {
>>  		.address_bits = 0,
>> -		.extent_order = 0,
>> +		.extent_order = EXTENT_ORDER,
>>  		.domid        = DOMID_SELF
>>  	};
>> +	static struct page *pages[ARRAY_SIZE(frame_list)];
> 
> This array can be rather large: I would try to avoid it, see below.

[..]

> 
> I would simply and avoid introducing a new array:
>     pfn = (frame_list[i] << XEN_PAGE_SHIFT) >> PAGE_SHIFT;
>     page = pfn_to_page(pfn);

Which won't work because the frame_list contains a gfn and not a pfn.
We need to translate back the gfn into a pfn and the into a page.

The cost of the translation may be big and I wanted to avoid anymore
XEN_PAGE_SHIFT in the code. In general we should avoid to deal with 4KB
PFN when it's not necessary, it make the code more confusing to read.

If your only concern is the size of the array, we could decrease the
number of frames by batch. Or allocation the variable once a boot time.

Regards,

-- 
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1204233 — Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux

FromStefano Stabellini <stefano.stabellini@eu.citrix.com>
Date2015-08-10 15:00 +0200
SubjectRe: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux
Message-ID<pVUQj-6Mt-23@gated-at.bofh.it>
In reply to#1204183
On Mon, 10 Aug 2015, Julien Grall wrote:
> Hi Stefano,
> 
> On 10/08/15 12:18, Stefano Stabellini wrote:
> >>  			/* Link back into the page tables if not highmem. */
> >> @@ -396,14 +413,15 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
> >>  static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
> >>  {
> >>  	enum bp_state state = BP_DONE;
> >> -	unsigned long  pfn, i;
> >> +	unsigned long i;
> >>  	struct page   *page;
> >>  	int ret;
> >>  	struct xen_memory_reservation reservation = {
> >>  		.address_bits = 0,
> >> -		.extent_order = 0,
> >> +		.extent_order = EXTENT_ORDER,
> >>  		.domid        = DOMID_SELF
> >>  	};
> >> +	static struct page *pages[ARRAY_SIZE(frame_list)];
> > 
> > This array can be rather large: I would try to avoid it, see below.
> 
> [..]
> 
> > 
> > I would simply and avoid introducing a new array:
> >     pfn = (frame_list[i] << XEN_PAGE_SHIFT) >> PAGE_SHIFT;
> >     page = pfn_to_page(pfn);
> 
> Which won't work because the frame_list contains a gfn and not a pfn.
> We need to translate back the gfn into a pfn and the into a page.
> 
> The cost of the translation may be big and I wanted to avoid anymore
> XEN_PAGE_SHIFT in the code. In general we should avoid to deal with 4KB
> PFN when it's not necessary, it make the code more confusing to read.

That is true


> If your only concern is the size of the array, we could decrease the
> number of frames by batch. Or allocation the variable once a boot time.

Yes, that is my only concern. Allocating only nr_pages new struct page*
would be good enough I guess.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1204277 — Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux

FromJulien Grall <julien.grall@citrix.com>
Date2015-08-10 15:40 +0200
SubjectRe: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux
Message-ID<pVVt0-7LE-23@gated-at.bofh.it>
In reply to#1204233
On 10/08/15 13:55, Stefano Stabellini wrote:
>> If your only concern is the size of the array, we could decrease the
>> number of frames by batch. Or allocation the variable once a boot time.
> 
> Yes, that is my only concern. Allocating only nr_pages new struct page*
> would be good enough I guess.

That would be even worst. We shouldn't allocate the array at every call,
but at boot time.

Note that frame_list is already a static variable use 64KB when 64KB
page is used. I guess this will be unlikely to remove that much frame in
a single batch. But I will keep this optimization for later.

Anyway, I'm wondering if we could re-use the lru field to link the page
when allocate them and retrieve in the second loop in order to avoid the
pages array.

Regards,

-- 
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1210376 — Re: [Xen-devel] [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux

FromDavid Vrabel <david.vrabel@citrix.com>
Date2015-08-20 12:10 +0200
SubjectRe: [Xen-devel] [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux
Message-ID<pZuXg-1Hx-17@gated-at.bofh.it>
In reply to#1202944
On 07/08/15 17:46, Julien Grall wrote:
> For ARM64 guests, Linux is able to support either 64K or 4K page
> granularity. Although, the hypercall interface is always based on 4K
> page granularity.
> 
> With 64K page granularity, a single page will be spread over multiple
> Xen frame.
> 
> To avoid splitting the page into 4K frame, take advantage of the
> extent_order field to directly allocate/free chunk of the Linux page
> size.
> 
> Note that PVMMU is only used for PV guest (which is x86) and the page
> granularity is always 4KB. Some BUILD_BUG_ON has been added to ensure
> that because the code has not been modified.
[...]
>  #ifdef CONFIG_XEN_HAVE_PVMMU
> +		/* We don't support PV MMU when Linux and Xen is using
> +		 * different page granularity.
> +		 */
> +		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);

You don't need this BUILD_BUG_ON() twice.

Otherwise,

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1215403 — Re: [Xen-devel] [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux

FromJulien Grall <julien.grall@citrix.com>
Date2015-08-28 17:20 +0200
SubjectRe: [Xen-devel] [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux
Message-ID<q2tBF-71T-37@gated-at.bofh.it>
In reply to#1210376
On 20/08/15 10:59, David Vrabel wrote:
> On 07/08/15 17:46, Julien Grall wrote:
>> For ARM64 guests, Linux is able to support either 64K or 4K page
>> granularity. Although, the hypercall interface is always based on 4K
>> page granularity.
>>
>> With 64K page granularity, a single page will be spread over multiple
>> Xen frame.
>>
>> To avoid splitting the page into 4K frame, take advantage of the
>> extent_order field to directly allocate/free chunk of the Linux page
>> size.
>>
>> Note that PVMMU is only used for PV guest (which is x86) and the page
>> granularity is always 4KB. Some BUILD_BUG_ON has been added to ensure
>> that because the code has not been modified.
> [...]
>>  #ifdef CONFIG_XEN_HAVE_PVMMU
>> +		/* We don't support PV MMU when Linux and Xen is using
>> +		 * different page granularity.
>> +		 */
>> +		BUILD_BUG_ON(XEN_PAGE_SIZE != PAGE_SIZE);
> 
> You don't need this BUILD_BUG_ON() twice.

I put twice the BUILD_BUG_ON so if we ever decide to drop one of the
#ifdef CONFIG_XEN_HAVE_PVMMU, the check will still be present.

So I'd like to keep it.

> Otherwise,
> 
> Reviewed-by: David Vrabel <david.vrabel@citrix.com>

Based on the discussion with Stefano I rework a bit the balloon code to
re-use lru field (see [1]), so I won't retain your reviewed-by on the
next series.

Regards,

[1] http://lists.xen.org/archives/html/xen-devel/2015-08/msg00781.html

-- 
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1202945 — [PATCH v3 13/20] xen/events: fifo: Make it running on 64KB granularity

FromJulien Grall <julien.grall@citrix.com>
Date2015-08-07 19:20 +0200
Subject[PATCH v3 13/20] xen/events: fifo: Make it running on 64KB granularity
Message-ID<pUTtg-61C-17@gated-at.bofh.it>
In reply to#1202904
Only use the first 4KB of the page to store the events channel info. It
means that we will waste 60KB every time we allocate page for:
     * control block: a page is allocating per CPU
     * event array: a page is allocating everytime we need to expand it

I think we can reduce the memory waste for the 2 areas by:

    * control block: sharing between multiple vCPUs. Although it will
    require some bookkeeping in order to not free the page when the CPU
    goes offline and the other CPUs sharing the page still there

    * event array: always extend the array event by 64K (i.e 16 4K
    chunk). That would require more care when we fail to expand the
    event channel.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@citrix.com>

---
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>

    Note I haven't updated the suggestion to reduce the memory waste
    after David's email [1]. I can do it if necessary.

    Changes in v3:
        - Add David and Stefano's reviewed-by

    [1] http://lists.xen.org/archives/html/xen-devel/2015-07/msg04596.html
---
 drivers/xen/events/events_base.c | 2 +-
 drivers/xen/events/events_fifo.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index c49bb7a..00dd923 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -40,11 +40,11 @@
 #include <asm/idle.h>
 #include <asm/io_apic.h>
 #include <asm/xen/pci.h>
-#include <xen/page.h>
 #endif
 #include <asm/sync_bitops.h>
 #include <asm/xen/hypercall.h>
 #include <asm/xen/hypervisor.h>
+#include <xen/page.h>
 
 #include <xen/xen.h>
 #include <xen/hvm.h>
diff --git a/drivers/xen/events/events_fifo.c b/drivers/xen/events/events_fifo.c
index 1d4baf5..e3e9e3d 100644
--- a/drivers/xen/events/events_fifo.c
+++ b/drivers/xen/events/events_fifo.c
@@ -54,7 +54,7 @@
 
 #include "events_internal.h"
 
-#define EVENT_WORDS_PER_PAGE (PAGE_SIZE / sizeof(event_word_t))
+#define EVENT_WORDS_PER_PAGE (XEN_PAGE_SIZE / sizeof(event_word_t))
 #define MAX_EVENT_ARRAY_PAGES (EVTCHN_FIFO_NR_CHANNELS / EVENT_WORDS_PER_PAGE)
 
 struct evtchn_fifo_queue {
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1202946 — [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity

FromJulien Grall <julien.grall@citrix.com>
Date2015-08-07 19:20 +0200
Subject[PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity
Message-ID<pUTtg-61C-15@gated-at.bofh.it>
In reply to#1202904
The hypercall interface (as well as the toolstack) is always using 4KB
page granularity. When the toolstack is asking for mapping a series of
guest PFN in a batch, it expects to have the page map contiguously in
its virtual memory.

When Linux is using 64KB page granularity, the privcmd driver will have
to map multiple Xen PFN in a single Linux page.

Note that this solution works on page granularity which is a multiple of
4KB.

Signed-off-by: Julien Grall <julien.grall@citrix.com>

---
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>

    I kept the hypercall arguments in remap_data to avoid allocating them on
    the stack every time that remap_pte_fn is called.
    I will keep like that unless someone is strongly disagree.

    Changes in v3:
        - The function to split a Linux page in mutiple Xen page has
        been moved internally. It was the only use (not used anymore in
        the balloon) and it's not quite clear what should be the common
        interface. Differ the question until someone need to use it.
        - s/nr_pfn/numgfns/ to make clear that we are dealing with GFN
        - Use DIV_ROUND_UP rather round_up and fix the usage in
        xen_xlate_unmap_gfn_range

    Changes in v2:
        - Use xen_apply_to_page
---
 drivers/xen/privcmd.c   |   8 ++--
 drivers/xen/xlate_mmu.c | 124 ++++++++++++++++++++++++++++++++----------------
 2 files changed, 89 insertions(+), 43 deletions(-)

diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index c6deb87..c8798ee 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -446,7 +446,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
 		return -EINVAL;
 	}
 
-	nr_pages = m.num;
+	nr_pages = DIV_ROUND_UP(m.num, XEN_PFN_PER_PAGE);
 	if ((m.num <= 0) || (nr_pages > (LONG_MAX >> PAGE_SHIFT)))
 		return -EINVAL;
 
@@ -494,7 +494,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
 			goto out_unlock;
 		}
 		if (xen_feature(XENFEAT_auto_translated_physmap)) {
-			ret = alloc_empty_pages(vma, m.num);
+			ret = alloc_empty_pages(vma, nr_pages);
 			if (ret < 0)
 				goto out_unlock;
 		} else
@@ -518,6 +518,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
 	state.global_error  = 0;
 	state.version       = version;
 
+	BUILD_BUG_ON(((PAGE_SIZE / sizeof(xen_pfn_t)) % XEN_PFN_PER_PAGE) != 0);
 	/* mmap_batch_fn guarantees ret == 0 */
 	BUG_ON(traverse_pages_block(m.num, sizeof(xen_pfn_t),
 				    &pagelist, mmap_batch_fn, &state));
@@ -582,12 +583,13 @@ static void privcmd_close(struct vm_area_struct *vma)
 {
 	struct page **pages = vma->vm_private_data;
 	int numpgs = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+	int numgfns = (vma->vm_end - vma->vm_start) >> XEN_PAGE_SHIFT;
 	int rc;
 
 	if (!xen_feature(XENFEAT_auto_translated_physmap) || !numpgs || !pages)
 		return;
 
-	rc = xen_unmap_domain_gfn_range(vma, numpgs, pages);
+	rc = xen_unmap_domain_gfn_range(vma, numgfns, pages);
 	if (rc == 0)
 		free_xenballooned_pages(numpgs, pages);
 	else
diff --git a/drivers/xen/xlate_mmu.c b/drivers/xen/xlate_mmu.c
index cff2387..a1d3904 100644
--- a/drivers/xen/xlate_mmu.c
+++ b/drivers/xen/xlate_mmu.c
@@ -38,31 +38,28 @@
 #include <xen/interface/xen.h>
 #include <xen/interface/memory.h>
 
-/* map fgfn of domid to lpfn in the current domain */
-static int map_foreign_page(unsigned long lpfn, unsigned long fgfn,
-			    unsigned int domid)
-{
-	int rc;
-	struct xen_add_to_physmap_range xatp = {
-		.domid = DOMID_SELF,
-		.foreign_domid = domid,
-		.size = 1,
-		.space = XENMAPSPACE_gmfn_foreign,
-	};
-	xen_ulong_t idx = fgfn;
-	xen_pfn_t gpfn = lpfn;
-	int err = 0;
+typedef void (*xen_gfn_fn_t)(unsigned long gfn, void *data);
 
-	set_xen_guest_handle(xatp.idxs, &idx);
-	set_xen_guest_handle(xatp.gpfns, &gpfn);
-	set_xen_guest_handle(xatp.errs, &err);
+/* Break down the pages in 4KB chunk and call fn for each gfn */
+static void xen_for_each_gfn(struct page **pages, unsigned nr_gfn,
+			     xen_gfn_fn_t fn, void *data)
+{
+	unsigned long xen_pfn = 0;
+	struct page *page;
+	int i;
 
-	rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
-	return rc < 0 ? rc : err;
+	for (i = 0; i < nr_gfn; i++) {
+		if ((i % XEN_PFN_PER_PAGE) == 0) {
+			page = pages[i / XEN_PFN_PER_PAGE];
+			xen_pfn = xen_page_to_pfn(page);
+		}
+		fn(pfn_to_gfn(xen_pfn++), data);
+	}
 }
 
 struct remap_data {
 	xen_pfn_t *fgfn; /* foreign domain's gfn */
+	int nr_fgfn; /* Number of foreign gfn left to map */
 	pgprot_t prot;
 	domid_t  domid;
 	struct vm_area_struct *vma;
@@ -71,24 +68,71 @@ struct remap_data {
 	struct xen_remap_gfn_info *info;
 	int *err_ptr;
 	int mapped;
+
+	/* Hypercall parameters */
+	int h_errs[XEN_PFN_PER_PAGE];
+	xen_ulong_t h_idxs[XEN_PFN_PER_PAGE];
+	xen_pfn_t h_gpfns[XEN_PFN_PER_PAGE];
+
+	int h_iter;	/* Iterator */
 };
 
+static void setup_hparams(unsigned long gfn, void *data)
+{
+	struct remap_data *info = data;
+
+	info->h_idxs[info->h_iter] = *info->fgfn;
+	info->h_gpfns[info->h_iter] = gfn;
+	info->h_errs[info->h_iter] = 0;
+
+	info->h_iter++;
+	info->fgfn++;
+}
+
 static int remap_pte_fn(pte_t *ptep, pgtable_t token, unsigned long addr,
 			void *data)
 {
 	struct remap_data *info = data;
 	struct page *page = info->pages[info->index++];
-	unsigned long pfn = page_to_pfn(page);
-	pte_t pte = pte_mkspecial(pfn_pte(pfn, info->prot));
-	int rc;
+	pte_t pte = pte_mkspecial(pfn_pte(page_to_pfn(page), info->prot));
+	int rc, nr_gfn;
+	uint32_t i;
+	struct xen_add_to_physmap_range xatp = {
+		.domid = DOMID_SELF,
+		.foreign_domid = info->domid,
+		.space = XENMAPSPACE_gmfn_foreign,
+	};
 
-	rc = map_foreign_page(pfn, *info->fgfn, info->domid);
-	*info->err_ptr++ = rc;
-	if (!rc) {
-		set_pte_at(info->vma->vm_mm, addr, ptep, pte);
-		info->mapped++;
+	nr_gfn = min_t(typeof(info->nr_fgfn), XEN_PFN_PER_PAGE, info->nr_fgfn);
+	info->nr_fgfn -= nr_gfn;
+
+	info->h_iter = 0;
+	xen_for_each_gfn(&page, nr_gfn, setup_hparams, info);
+	BUG_ON(info->h_iter != nr_gfn);
+
+	set_xen_guest_handle(xatp.idxs, info->h_idxs);
+	set_xen_guest_handle(xatp.gpfns, info->h_gpfns);
+	set_xen_guest_handle(xatp.errs, info->h_errs);
+	xatp.size = nr_gfn;
+
+	rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
+
+	/* info->err_ptr expect to have one error status per Xen PFN */
+	for (i = 0; i < nr_gfn; i++) {
+		int err = (rc < 0) ? rc : info->h_errs[i];
+
+		*(info->err_ptr++) = err;
+		if (!err)
+			info->mapped++;
 	}
-	info->fgfn++;
+
+	/*
+	 * Note: The hypercall will return 0 in most of the case if even if
+	 * all the fgmfn are not mapped. We still have to update the pte
+	 * as the userspace may decide to continue.
+	 */
+	if (!rc)
+		set_pte_at(info->vma->vm_mm, addr, ptep, pte);
 
 	return 0;
 }
@@ -102,13 +146,14 @@ int xen_xlate_remap_gfn_array(struct vm_area_struct *vma,
 {
 	int err;
 	struct remap_data data;
-	unsigned long range = nr << PAGE_SHIFT;
+	unsigned long range = DIV_ROUND_UP(nr, XEN_PFN_PER_PAGE) << PAGE_SHIFT;
 
 	/* Kept here for the purpose of making sure code doesn't break
 	   x86 PVOPS */
 	BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_IO)) == (VM_PFNMAP | VM_IO)));
 
 	data.fgfn = gfn;
+	data.nr_fgfn = nr;
 	data.prot  = prot;
 	data.domid = domid;
 	data.vma   = vma;
@@ -123,21 +168,20 @@ int xen_xlate_remap_gfn_array(struct vm_area_struct *vma,
 }
 EXPORT_SYMBOL_GPL(xen_xlate_remap_gfn_array);
 
-int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma,
-			      int nr, struct page **pages)
+static void unmap_gfn(unsigned long gfn, void *data)
 {
-	int i;
+	struct xen_remove_from_physmap xrp;
 
-	for (i = 0; i < nr; i++) {
-		struct xen_remove_from_physmap xrp;
-		unsigned long pfn;
+	xrp.domid = DOMID_SELF;
+	xrp.gpfn = gfn;
+	(void)HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrp);
+}
 
-		pfn = page_to_pfn(pages[i]);
+int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma,
+			      int nr, struct page **pages)
+{
+	xen_for_each_gfn(pages, nr, unmap_gfn, NULL);
 
-		xrp.domid = DOMID_SELF;
-		xrp.gpfn = pfn;
-		(void)HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrp);
-	}
 	return 0;
 }
 EXPORT_SYMBOL_GPL(xen_xlate_unmap_gfn_range);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1204204 — Re: [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity

FromStefano Stabellini <stefano.stabellini@eu.citrix.com>
Date2015-08-10 14:10 +0200
SubjectRe: [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity
Message-ID<pVU3U-5R8-9@gated-at.bofh.it>
In reply to#1202946
On Fri, 7 Aug 2015, Julien Grall wrote:
> The hypercall interface (as well as the toolstack) is always using 4KB
> page granularity. When the toolstack is asking for mapping a series of
> guest PFN in a batch, it expects to have the page map contiguously in
> its virtual memory.
> 
> When Linux is using 64KB page granularity, the privcmd driver will have
> to map multiple Xen PFN in a single Linux page.
> 
> Note that this solution works on page granularity which is a multiple of
> 4KB.
> 
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
> 
> ---
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> 
>     I kept the hypercall arguments in remap_data to avoid allocating them on
>     the stack every time that remap_pte_fn is called.
>     I will keep like that unless someone is strongly disagree.
> 
>     Changes in v3:
>         - The function to split a Linux page in mutiple Xen page has
>         been moved internally. It was the only use (not used anymore in
>         the balloon) and it's not quite clear what should be the common
>         interface. Differ the question until someone need to use it.
>         - s/nr_pfn/numgfns/ to make clear that we are dealing with GFN
>         - Use DIV_ROUND_UP rather round_up and fix the usage in
>         xen_xlate_unmap_gfn_range
> 
>     Changes in v2:
>         - Use xen_apply_to_page
> ---
>  drivers/xen/privcmd.c   |   8 ++--
>  drivers/xen/xlate_mmu.c | 124 ++++++++++++++++++++++++++++++++----------------
>  2 files changed, 89 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> index c6deb87..c8798ee 100644
> --- a/drivers/xen/privcmd.c
> +++ b/drivers/xen/privcmd.c
> @@ -446,7 +446,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
>  		return -EINVAL;
>  	}
>  
> -	nr_pages = m.num;
> +	nr_pages = DIV_ROUND_UP(m.num, XEN_PFN_PER_PAGE);
>  	if ((m.num <= 0) || (nr_pages > (LONG_MAX >> PAGE_SHIFT)))
>  		return -EINVAL;
>  
> @@ -494,7 +494,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
>  			goto out_unlock;
>  		}
>  		if (xen_feature(XENFEAT_auto_translated_physmap)) {
> -			ret = alloc_empty_pages(vma, m.num);
> +			ret = alloc_empty_pages(vma, nr_pages);
>  			if (ret < 0)
>  				goto out_unlock;
>  		} else
> @@ -518,6 +518,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
>  	state.global_error  = 0;
>  	state.version       = version;
>  
> +	BUILD_BUG_ON(((PAGE_SIZE / sizeof(xen_pfn_t)) % XEN_PFN_PER_PAGE) != 0);
>  	/* mmap_batch_fn guarantees ret == 0 */
>  	BUG_ON(traverse_pages_block(m.num, sizeof(xen_pfn_t),
>  				    &pagelist, mmap_batch_fn, &state));
> @@ -582,12 +583,13 @@ static void privcmd_close(struct vm_area_struct *vma)
>  {
>  	struct page **pages = vma->vm_private_data;
>  	int numpgs = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> +	int numgfns = (vma->vm_end - vma->vm_start) >> XEN_PAGE_SHIFT;
>  	int rc;
>  
>  	if (!xen_feature(XENFEAT_auto_translated_physmap) || !numpgs || !pages)
>  		return;
>  
> -	rc = xen_unmap_domain_gfn_range(vma, numpgs, pages);
> +	rc = xen_unmap_domain_gfn_range(vma, numgfns, pages);
>  	if (rc == 0)
>  		free_xenballooned_pages(numpgs, pages);
>  	else
> diff --git a/drivers/xen/xlate_mmu.c b/drivers/xen/xlate_mmu.c
> index cff2387..a1d3904 100644
> --- a/drivers/xen/xlate_mmu.c
> +++ b/drivers/xen/xlate_mmu.c
> @@ -38,31 +38,28 @@
>  #include <xen/interface/xen.h>
>  #include <xen/interface/memory.h>
>  
> -/* map fgfn of domid to lpfn in the current domain */
> -static int map_foreign_page(unsigned long lpfn, unsigned long fgfn,
> -			    unsigned int domid)
> -{
> -	int rc;
> -	struct xen_add_to_physmap_range xatp = {
> -		.domid = DOMID_SELF,
> -		.foreign_domid = domid,
> -		.size = 1,
> -		.space = XENMAPSPACE_gmfn_foreign,
> -	};
> -	xen_ulong_t idx = fgfn;
> -	xen_pfn_t gpfn = lpfn;
> -	int err = 0;
> +typedef void (*xen_gfn_fn_t)(unsigned long gfn, void *data);
>  
> -	set_xen_guest_handle(xatp.idxs, &idx);
> -	set_xen_guest_handle(xatp.gpfns, &gpfn);
> -	set_xen_guest_handle(xatp.errs, &err);
> +/* Break down the pages in 4KB chunk and call fn for each gfn */
> +static void xen_for_each_gfn(struct page **pages, unsigned nr_gfn,
> +			     xen_gfn_fn_t fn, void *data)
> +{
> +	unsigned long xen_pfn = 0;
> +	struct page *page;
> +	int i;
>  
> -	rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
> -	return rc < 0 ? rc : err;
> +	for (i = 0; i < nr_gfn; i++) {
> +		if ((i % XEN_PFN_PER_PAGE) == 0) {
> +			page = pages[i / XEN_PFN_PER_PAGE];

If this function is going to be called very frequently you might want to
consider using a shift instead.

    page = pages[i >> 4];

With an appropriate macro of course.


> +			xen_pfn = xen_page_to_pfn(page);
> +		}
> +		fn(pfn_to_gfn(xen_pfn++), data);

What is the purpose of incrementing xen_pfn here?


> +	}
>  }
>  
>  struct remap_data {
>  	xen_pfn_t *fgfn; /* foreign domain's gfn */
> +	int nr_fgfn; /* Number of foreign gfn left to map */
>  	pgprot_t prot;
>  	domid_t  domid;
>  	struct vm_area_struct *vma;
> @@ -71,24 +68,71 @@ struct remap_data {
>  	struct xen_remap_gfn_info *info;
>  	int *err_ptr;
>  	int mapped;
> +
> +	/* Hypercall parameters */
> +	int h_errs[XEN_PFN_PER_PAGE];
> +	xen_ulong_t h_idxs[XEN_PFN_PER_PAGE];
> +	xen_pfn_t h_gpfns[XEN_PFN_PER_PAGE];
> +
> +	int h_iter;	/* Iterator */
>  };
>  
> +static void setup_hparams(unsigned long gfn, void *data)
> +{
> +	struct remap_data *info = data;
> +
> +	info->h_idxs[info->h_iter] = *info->fgfn;
> +	info->h_gpfns[info->h_iter] = gfn;
> +	info->h_errs[info->h_iter] = 0;
> +
> +	info->h_iter++;
> +	info->fgfn++;
> +}
> +
>  static int remap_pte_fn(pte_t *ptep, pgtable_t token, unsigned long addr,
>  			void *data)
>  {
>  	struct remap_data *info = data;
>  	struct page *page = info->pages[info->index++];
> -	unsigned long pfn = page_to_pfn(page);
> -	pte_t pte = pte_mkspecial(pfn_pte(pfn, info->prot));
> -	int rc;
> +	pte_t pte = pte_mkspecial(pfn_pte(page_to_pfn(page), info->prot));
> +	int rc, nr_gfn;
> +	uint32_t i;
> +	struct xen_add_to_physmap_range xatp = {
> +		.domid = DOMID_SELF,
> +		.foreign_domid = info->domid,
> +		.space = XENMAPSPACE_gmfn_foreign,
> +	};
>  
> -	rc = map_foreign_page(pfn, *info->fgfn, info->domid);
> -	*info->err_ptr++ = rc;
> -	if (!rc) {
> -		set_pte_at(info->vma->vm_mm, addr, ptep, pte);
> -		info->mapped++;
> +	nr_gfn = min_t(typeof(info->nr_fgfn), XEN_PFN_PER_PAGE, info->nr_fgfn);
> +	info->nr_fgfn -= nr_gfn;
> +
> +	info->h_iter = 0;
> +	xen_for_each_gfn(&page, nr_gfn, setup_hparams, info);
> +	BUG_ON(info->h_iter != nr_gfn);
> +
> +	set_xen_guest_handle(xatp.idxs, info->h_idxs);
> +	set_xen_guest_handle(xatp.gpfns, info->h_gpfns);
> +	set_xen_guest_handle(xatp.errs, info->h_errs);
> +	xatp.size = nr_gfn;
> +
> +	rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
> +
> +	/* info->err_ptr expect to have one error status per Xen PFN */
> +	for (i = 0; i < nr_gfn; i++) {
> +		int err = (rc < 0) ? rc : info->h_errs[i];
> +
> +		*(info->err_ptr++) = err;
> +		if (!err)
> +			info->mapped++;
>  	}
> -	info->fgfn++;
> +
> +	/*
> +	 * Note: The hypercall will return 0 in most of the case if even if
> +	 * all the fgmfn are not mapped. We still have to update the pte
> +	 * as the userspace may decide to continue.
> +	 */
> +	if (!rc)
> +		set_pte_at(info->vma->vm_mm, addr, ptep, pte);
>  
>  	return 0;
>  }
> @@ -102,13 +146,14 @@ int xen_xlate_remap_gfn_array(struct vm_area_struct *vma,
>  {
>  	int err;
>  	struct remap_data data;
> -	unsigned long range = nr << PAGE_SHIFT;
> +	unsigned long range = DIV_ROUND_UP(nr, XEN_PFN_PER_PAGE) << PAGE_SHIFT;
>  
>  	/* Kept here for the purpose of making sure code doesn't break
>  	   x86 PVOPS */
>  	BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_IO)) == (VM_PFNMAP | VM_IO)));
>  
>  	data.fgfn = gfn;
> +	data.nr_fgfn = nr;
>  	data.prot  = prot;
>  	data.domid = domid;
>  	data.vma   = vma;
> @@ -123,21 +168,20 @@ int xen_xlate_remap_gfn_array(struct vm_area_struct *vma,
>  }
>  EXPORT_SYMBOL_GPL(xen_xlate_remap_gfn_array);
>  
> -int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma,
> -			      int nr, struct page **pages)
> +static void unmap_gfn(unsigned long gfn, void *data)
>  {
> -	int i;
> +	struct xen_remove_from_physmap xrp;
>  
> -	for (i = 0; i < nr; i++) {
> -		struct xen_remove_from_physmap xrp;
> -		unsigned long pfn;
> +	xrp.domid = DOMID_SELF;
> +	xrp.gpfn = gfn;
> +	(void)HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrp);
> +}
>  
> -		pfn = page_to_pfn(pages[i]);
> +int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma,
> +			      int nr, struct page **pages)
> +{
> +	xen_for_each_gfn(pages, nr, unmap_gfn, NULL);
>  
> -		xrp.domid = DOMID_SELF;
> -		xrp.gpfn = pfn;
> -		(void)HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrp);
> -	}
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(xen_xlate_unmap_gfn_range);
> -- 
> 2.1.4
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1204212 — Re: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity

FromDavid Vrabel <david.vrabel@citrix.com>
Date2015-08-10 14:20 +0200
SubjectRe: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity
Message-ID<pVUdA-62u-25@gated-at.bofh.it>
In reply to#1204204
On 10/08/15 13:03, Stefano Stabellini wrote:
> On Fri, 7 Aug 2015, Julien Grall wrote:
>> -	rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
>> -	return rc < 0 ? rc : err;
>> +	for (i = 0; i < nr_gfn; i++) {
>> +		if ((i % XEN_PFN_PER_PAGE) == 0) {
>> +			page = pages[i / XEN_PFN_PER_PAGE];
> 
> If this function is going to be called very frequently you might want to
> consider using a shift instead.
> 
>     page = pages[i >> 4];
> 
> With an appropriate macro of course.

This change isn't necessary.  Compilers already turn divides into
suitable shifts.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1204232 — Re: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity

FromStefano Stabellini <stefano.stabellini@eu.citrix.com>
Date2015-08-10 15:00 +0200
SubjectRe: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity
Message-ID<pVUQi-6Mt-5@gated-at.bofh.it>
In reply to#1204212
On Mon, 10 Aug 2015, David Vrabel wrote:
> On 10/08/15 13:03, Stefano Stabellini wrote:
> > On Fri, 7 Aug 2015, Julien Grall wrote:
> >> -	rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
> >> -	return rc < 0 ? rc : err;
> >> +	for (i = 0; i < nr_gfn; i++) {
> >> +		if ((i % XEN_PFN_PER_PAGE) == 0) {
> >> +			page = pages[i / XEN_PFN_PER_PAGE];
> > 
> > If this function is going to be called very frequently you might want to
> > consider using a shift instead.
> > 
> >     page = pages[i >> 4];
> > 
> > With an appropriate macro of course.
> 
> This change isn't necessary.  Compilers already turn divides into
> suitable shifts.

The ARM compiler I used last time I tested this did not, but that was 1
or 2 years ago. In any case to be clear this change is not required.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1204265 — Re: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity

FromJulien Grall <julien.grall@citrix.com>
Date2015-08-10 15:30 +0200
SubjectRe: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity
Message-ID<pVVjl-7A8-33@gated-at.bofh.it>
In reply to#1204232
Hi Stefano,

On 10/08/15 13:57, Stefano Stabellini wrote:
> On Mon, 10 Aug 2015, David Vrabel wrote:
>> On 10/08/15 13:03, Stefano Stabellini wrote:
>>> On Fri, 7 Aug 2015, Julien Grall wrote:
>>>> -	rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
>>>> -	return rc < 0 ? rc : err;
>>>> +	for (i = 0; i < nr_gfn; i++) {
>>>> +		if ((i % XEN_PFN_PER_PAGE) == 0) {
>>>> +			page = pages[i / XEN_PFN_PER_PAGE];
>>>
>>> If this function is going to be called very frequently you might want to
>>> consider using a shift instead.
>>>
>>>     page = pages[i >> 4];
>>>
>>> With an appropriate macro of course.
>>
>> This change isn't necessary.  Compilers already turn divides into
>> suitable shifts.
> 
> The ARM compiler I used last time I tested this did not, but that was 1
> or 2 years ago. In any case to be clear this change is not required.

I gave a try on the compiler used by Debian Jessy (gcc 4.9.2). It turns
divides into suitable shifts.

Anyway, if it may happen that older ARM compiler doesn't do this change,
I sure we would have to modify many other places in order to make the
code efficient.

Regards,

-- 
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1216977 — Re: [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity

FromJulien Grall <julien.grall@citrix.com>
Date2015-09-01 19:20 +0200
SubjectRe: [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity
Message-ID<q3XnY-4ya-1@gated-at.bofh.it>
In reply to#1204204
Hi Stefano,

On 10/08/15 13:03, Stefano Stabellini wrote:
>> +			xen_pfn = xen_page_to_pfn(page);
>> +		}
>> +		fn(pfn_to_gfn(xen_pfn++), data);
> 
> What is the purpose of incrementing xen_pfn here?

Because the Linux page is split into multiple xen_pfn, so we want to get
the next xen_pfn for the next iteration.

Regards,

-- 
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1210388 — Re: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity

FromDavid Vrabel <david.vrabel@citrix.com>
Date2015-08-20 12:10 +0200
SubjectRe: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity
Message-ID<pZuXh-1Hx-53@gated-at.bofh.it>
In reply to#1202946
On 07/08/15 17:46, Julien Grall wrote:
> The hypercall interface (as well as the toolstack) is always using 4KB
> page granularity. When the toolstack is asking for mapping a series of
> guest PFN in a batch, it expects to have the page map contiguously in
> its virtual memory.
> 
> When Linux is using 64KB page granularity, the privcmd driver will have
> to map multiple Xen PFN in a single Linux page.
> 
> Note that this solution works on page granularity which is a multiple of
> 4KB.

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1202947 — [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity

FromJulien Grall <julien.grall@citrix.com>
Date2015-08-07 19:20 +0200
Subject[PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity
Message-ID<pUTtg-61C-21@gated-at.bofh.it>
In reply to#1202904
The PV network protocol is using 4KB page granularity. The goal of this
patch is to allow a Linux using 64KB page granularity working as a
network backend on a non-modified Xen.

It's only necessary to adapt the ring size and break skb data in small
chunk of 4KB. The rest of the code is relying on the grant table code.

Signed-off-by: Julien Grall <julien.grall@citrix.com>

---
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: netdev@vger.kernel.org

Improvement such as support of 64KB grant is not taken into
consideration in this patch because we have the requirement to run a
Linux using 64KB pages on a non-modified Xen.

    Changes in v3:
        - Fix errors reported by checkpatch.pl
        - s/mfn/gfn/ based on the new naming
        - gnttab_foreach_grant has been renamed to gnttab_forach_grant_in_range
        - The grant callback doesn't allow anymore to use less data. An
        helpers has been added in netback to handle this.

    Changes in v2:
        - Correctly set MAX_GRANT_COPY_OPS and XEN_NETBK_RX_SLOTS_MAX
        - Don't use XEN_PAGE_SIZE in handle_frag_list as we coalesce
        fragment into a new skb
        - Use gnntab_foreach_grant to split a Linux page into grant
---
 drivers/net/xen-netback/common.h  |  15 ++--
 drivers/net/xen-netback/netback.c | 153 ++++++++++++++++++++++++--------------
 2 files changed, 107 insertions(+), 61 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 8a495b3..bb68211 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -44,6 +44,7 @@
 #include <xen/interface/grant_table.h>
 #include <xen/grant_table.h>
 #include <xen/xenbus.h>
+#include <xen/page.h>
 #include <linux/debugfs.h>
 
 typedef unsigned int pending_ring_idx_t;
@@ -64,8 +65,8 @@ struct pending_tx_info {
 	struct ubuf_info callback_struct;
 };
 
-#define XEN_NETIF_TX_RING_SIZE __CONST_RING_SIZE(xen_netif_tx, PAGE_SIZE)
-#define XEN_NETIF_RX_RING_SIZE __CONST_RING_SIZE(xen_netif_rx, PAGE_SIZE)
+#define XEN_NETIF_TX_RING_SIZE __CONST_RING_SIZE(xen_netif_tx, XEN_PAGE_SIZE)
+#define XEN_NETIF_RX_RING_SIZE __CONST_RING_SIZE(xen_netif_rx, XEN_PAGE_SIZE)
 
 struct xenvif_rx_meta {
 	int id;
@@ -80,16 +81,18 @@ struct xenvif_rx_meta {
 /* Discriminate from any valid pending_idx value. */
 #define INVALID_PENDING_IDX 0xFFFF
 
-#define MAX_BUFFER_OFFSET PAGE_SIZE
+#define MAX_BUFFER_OFFSET XEN_PAGE_SIZE
 
 #define MAX_PENDING_REQS XEN_NETIF_TX_RING_SIZE
 
+#define MAX_XEN_SKB_FRAGS (65536 / XEN_PAGE_SIZE + 1)
+
 /* It's possible for an skb to have a maximal number of frags
  * but still be less than MAX_BUFFER_OFFSET in size. Thus the
- * worst-case number of copy operations is MAX_SKB_FRAGS per
+ * worst-case number of copy operations is MAX_XEN_SKB_FRAGS per
  * ring slot.
  */
-#define MAX_GRANT_COPY_OPS (MAX_SKB_FRAGS * XEN_NETIF_RX_RING_SIZE)
+#define MAX_GRANT_COPY_OPS (MAX_XEN_SKB_FRAGS * XEN_NETIF_RX_RING_SIZE)
 
 #define NETBACK_INVALID_HANDLE -1
 
@@ -203,7 +206,7 @@ struct xenvif_queue { /* Per-queue data for xenvif */
 /* Maximum number of Rx slots a to-guest packet may use, including the
  * slot needed for GSO meta-data.
  */
-#define XEN_NETBK_RX_SLOTS_MAX (MAX_SKB_FRAGS + 1)
+#define XEN_NETBK_RX_SLOTS_MAX ((MAX_XEN_SKB_FRAGS + 1))
 
 enum state_bit_shift {
 	/* This bit marks that the vif is connected */
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 66f1780..c32a9f2 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -263,6 +263,80 @@ static struct xenvif_rx_meta *get_next_rx_buffer(struct xenvif_queue *queue,
 	return meta;
 }
 
+struct gop_frag_copy {
+	struct xenvif_queue *queue;
+	struct netrx_pending_operations *npo;
+	struct xenvif_rx_meta *meta;
+	int head;
+	int gso_type;
+
+	struct page *page;
+};
+
+static void xenvif_setup_copy_gop(unsigned long gfn,
+				  unsigned int offset,
+				  unsigned int *len,
+				  struct gop_frag_copy *info)
+{
+	struct gnttab_copy *copy_gop;
+	struct xen_page_foreign *foreign;
+	/* Convenient aliases */
+	struct xenvif_queue *queue = info->queue;
+	struct netrx_pending_operations *npo = info->npo;
+	struct page *page = info->page;
+
+	BUG_ON(npo->copy_off > MAX_BUFFER_OFFSET);
+
+	if (npo->copy_off == MAX_BUFFER_OFFSET)
+		info->meta = get_next_rx_buffer(queue, npo);
+
+	if (npo->copy_off + *len > MAX_BUFFER_OFFSET)
+		*len = MAX_BUFFER_OFFSET - npo->copy_off;
+
+	copy_gop = npo->copy + npo->copy_prod++;
+	copy_gop->flags = GNTCOPY_dest_gref;
+	copy_gop->len = *len;
+
+	foreign = xen_page_foreign(page);
+	if (foreign) {
+		copy_gop->source.domid = foreign->domid;
+		copy_gop->source.u.ref = foreign->gref;
+		copy_gop->flags |= GNTCOPY_source_gref;
+	} else {
+		copy_gop->source.domid = DOMID_SELF;
+		copy_gop->source.u.gmfn = gfn;
+	}
+	copy_gop->source.offset = offset;
+
+	copy_gop->dest.domid = queue->vif->domid;
+	copy_gop->dest.offset = npo->copy_off;
+	copy_gop->dest.u.ref = npo->copy_gref;
+
+	npo->copy_off += *len;
+	info->meta->size += *len;
+
+	/* Leave a gap for the GSO descriptor. */
+	if (info->head && ((1 << info->gso_type) & queue->vif->gso_mask))
+		queue->rx.req_cons++;
+
+	info->head = 0; /* There must be something in this buffer now */
+}
+
+static void xenvif_gop_frag_copy_grant(unsigned long gfn,
+				       unsigned offset,
+				       unsigned int len,
+				       void *data)
+{
+	unsigned int bytes;
+
+	while (len) {
+		bytes = len;
+		xenvif_setup_copy_gop(gfn, offset, &bytes, data);
+		offset += bytes;
+		len -= bytes;
+	}
+}
+
 /*
  * Set up the grant operations for this fragment. If it's a flipping
  * interface, we also set up the unmap request from here.
@@ -272,83 +346,52 @@ static void xenvif_gop_frag_copy(struct xenvif_queue *queue, struct sk_buff *skb
 				 struct page *page, unsigned long size,
 				 unsigned long offset, int *head)
 {
-	struct gnttab_copy *copy_gop;
-	struct xenvif_rx_meta *meta;
+	struct gop_frag_copy info = {
+		.queue = queue,
+		.npo = npo,
+		.head = *head,
+		.gso_type = XEN_NETIF_GSO_TYPE_NONE,
+	};
 	unsigned long bytes;
-	int gso_type = XEN_NETIF_GSO_TYPE_NONE;
 
 	if (skb_is_gso(skb)) {
 		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
-			gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
+			info.gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
 		else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
-			gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
+			info.gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
 	}
 
 	/* Data must not cross a page boundary. */
 	BUG_ON(size + offset > PAGE_SIZE<<compound_order(page));
 
-	meta = npo->meta + npo->meta_prod - 1;
+	info.meta = npo->meta + npo->meta_prod - 1;
 
 	/* Skip unused frames from start of page */
 	page += offset >> PAGE_SHIFT;
 	offset &= ~PAGE_MASK;
 
 	while (size > 0) {
-		struct xen_page_foreign *foreign;
-
 		BUG_ON(offset >= PAGE_SIZE);
-		BUG_ON(npo->copy_off > MAX_BUFFER_OFFSET);
-
-		if (npo->copy_off == MAX_BUFFER_OFFSET)
-			meta = get_next_rx_buffer(queue, npo);
 
 		bytes = PAGE_SIZE - offset;
 		if (bytes > size)
 			bytes = size;
 
-		if (npo->copy_off + bytes > MAX_BUFFER_OFFSET)
-			bytes = MAX_BUFFER_OFFSET - npo->copy_off;
-
-		copy_gop = npo->copy + npo->copy_prod++;
-		copy_gop->flags = GNTCOPY_dest_gref;
-		copy_gop->len = bytes;
-
-		foreign = xen_page_foreign(page);
-		if (foreign) {
-			copy_gop->source.domid = foreign->domid;
-			copy_gop->source.u.ref = foreign->gref;
-			copy_gop->flags |= GNTCOPY_source_gref;
-		} else {
-			copy_gop->source.domid = DOMID_SELF;
-			copy_gop->source.u.gmfn =
-				virt_to_gfn(page_address(page));
-		}
-		copy_gop->source.offset = offset;
-
-		copy_gop->dest.domid = queue->vif->domid;
-		copy_gop->dest.offset = npo->copy_off;
-		copy_gop->dest.u.ref = npo->copy_gref;
-
-		npo->copy_off += bytes;
-		meta->size += bytes;
-
-		offset += bytes;
+		info.page = page;
+		gnttab_foreach_grant_in_range(page, offset, bytes,
+					      xenvif_gop_frag_copy_grant,
+					      &info);
 		size -= bytes;
+		offset = 0;
 
-		/* Next frame */
-		if (offset == PAGE_SIZE && size) {
+		/* Next page */
+		if (size) {
 			BUG_ON(!PageCompound(page));
 			page++;
-			offset = 0;
 		}
-
-		/* Leave a gap for the GSO descriptor. */
-		if (*head && ((1 << gso_type) & queue->vif->gso_mask))
-			queue->rx.req_cons++;
-
-		*head = 0; /* There must be something in this buffer now. */
-
 	}
+
+	*head = info.head;
 }
 
 /*
@@ -747,7 +790,7 @@ static int xenvif_count_requests(struct xenvif_queue *queue,
 		first->size -= txp->size;
 		slots++;
 
-		if (unlikely((txp->offset + txp->size) > PAGE_SIZE)) {
+		if (unlikely((txp->offset + txp->size) > XEN_PAGE_SIZE)) {
 			netdev_err(queue->vif->dev, "Cross page boundary, txp->offset: %u, size: %u\n",
 				 txp->offset, txp->size);
 			xenvif_fatal_tx_err(queue->vif);
@@ -1241,11 +1284,11 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
 		}
 
 		/* No crossing a page as the payload mustn't fragment. */
-		if (unlikely((txreq.offset + txreq.size) > PAGE_SIZE)) {
+		if (unlikely((txreq.offset + txreq.size) > XEN_PAGE_SIZE)) {
 			netdev_err(queue->vif->dev,
 				   "txreq.offset: %u, size: %u, end: %lu\n",
 				   txreq.offset, txreq.size,
-				   (unsigned long)(txreq.offset&~PAGE_MASK) + txreq.size);
+				   (unsigned long)(txreq.offset&~XEN_PAGE_MASK) + txreq.size);
 			xenvif_fatal_tx_err(queue->vif);
 			break;
 		}
@@ -1287,7 +1330,7 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
 			virt_to_gfn(skb->data);
 		queue->tx_copy_ops[*copy_ops].dest.domid = DOMID_SELF;
 		queue->tx_copy_ops[*copy_ops].dest.offset =
-			offset_in_page(skb->data);
+			offset_in_page(skb->data) & ~XEN_PAGE_MASK;
 
 		queue->tx_copy_ops[*copy_ops].len = data_len;
 		queue->tx_copy_ops[*copy_ops].flags = GNTCOPY_source_gref;
@@ -1780,7 +1823,7 @@ int xenvif_map_frontend_rings(struct xenvif_queue *queue,
 		goto err;
 
 	txs = (struct xen_netif_tx_sring *)addr;
-	BACK_RING_INIT(&queue->tx, txs, PAGE_SIZE);
+	BACK_RING_INIT(&queue->tx, txs, XEN_PAGE_SIZE);
 
 	err = xenbus_map_ring_valloc(xenvif_to_xenbus_device(queue->vif),
 				     &rx_ring_ref, 1, &addr);
@@ -1788,7 +1831,7 @@ int xenvif_map_frontend_rings(struct xenvif_queue *queue,
 		goto err;
 
 	rxs = (struct xen_netif_rx_sring *)addr;
-	BACK_RING_INIT(&queue->rx, rxs, PAGE_SIZE);
+	BACK_RING_INIT(&queue->rx, rxs, XEN_PAGE_SIZE);
 
 	return 0;
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1203278 — Re: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity

FromWei Liu <wei.liu2@citrix.com>
Date2015-08-08 17:00 +0200
SubjectRe: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity
Message-ID<pVdLk-1Rl-5@gated-at.bofh.it>
In reply to#1202947
On Fri, Aug 07, 2015 at 05:46:57PM +0100, Julien Grall wrote:
> The PV network protocol is using 4KB page granularity. The goal of this
> patch is to allow a Linux using 64KB page granularity working as a
> network backend on a non-modified Xen.
> 
> It's only necessary to adapt the ring size and break skb data in small
> chunk of 4KB. The rest of the code is relying on the grant table code.
> 
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
> 
> ---
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: netdev@vger.kernel.org
> 
[...]
> +#define XEN_NETIF_TX_RING_SIZE __CONST_RING_SIZE(xen_netif_tx, XEN_PAGE_SIZE)
> +#define XEN_NETIF_RX_RING_SIZE __CONST_RING_SIZE(xen_netif_rx, XEN_PAGE_SIZE)
>  
>  struct xenvif_rx_meta {
>  	int id;
> @@ -80,16 +81,18 @@ struct xenvif_rx_meta {
>  /* Discriminate from any valid pending_idx value. */
>  #define INVALID_PENDING_IDX 0xFFFF
>  
> -#define MAX_BUFFER_OFFSET PAGE_SIZE
> +#define MAX_BUFFER_OFFSET XEN_PAGE_SIZE
>  
>  #define MAX_PENDING_REQS XEN_NETIF_TX_RING_SIZE
>  
> +#define MAX_XEN_SKB_FRAGS (65536 / XEN_PAGE_SIZE + 1)
> +

It might be clearer if you add a comment saying the maximum number of
frags is derived from the page size of the grant page, which happens to
be XEN_PAGE_SIZE at the moment. 

In the future we need to figure out the page size of grant page in a
dynamic way. We shall cross the bridge when we get there.

>  /* It's possible for an skb to have a maximal number of frags
>   * but still be less than MAX_BUFFER_OFFSET in size. Thus the
> - * worst-case number of copy operations is MAX_SKB_FRAGS per
> + * worst-case number of copy operations is MAX_XEN_SKB_FRAGS per
>   * ring slot.
>   */
> -#define MAX_GRANT_COPY_OPS (MAX_SKB_FRAGS * XEN_NETIF_RX_RING_SIZE)
> +#define MAX_GRANT_COPY_OPS (MAX_XEN_SKB_FRAGS * XEN_NETIF_RX_RING_SIZE)
>  
>  #define NETBACK_INVALID_HANDLE -1
>  
> @@ -203,7 +206,7 @@ struct xenvif_queue { /* Per-queue data for xenvif */
>  /* Maximum number of Rx slots a to-guest packet may use, including the
>   * slot needed for GSO meta-data.
>   */
> -#define XEN_NETBK_RX_SLOTS_MAX (MAX_SKB_FRAGS + 1)
> +#define XEN_NETBK_RX_SLOTS_MAX ((MAX_XEN_SKB_FRAGS + 1))
>  
>  enum state_bit_shift {
>  	/* This bit marks that the vif is connected */
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 66f1780..c32a9f2 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -263,6 +263,80 @@ static struct xenvif_rx_meta *get_next_rx_buffer(struct xenvif_queue *queue,
>  	return meta;
>  }
>  
[...]
>   * Set up the grant operations for this fragment. If it's a flipping
>   * interface, we also set up the unmap request from here.
> @@ -272,83 +346,52 @@ static void xenvif_gop_frag_copy(struct xenvif_queue *queue, struct sk_buff *skb
>  				 struct page *page, unsigned long size,
>  				 unsigned long offset, int *head)
>  {
> -	struct gnttab_copy *copy_gop;
> -	struct xenvif_rx_meta *meta;
> +	struct gop_frag_copy info = {
> +		.queue = queue,
> +		.npo = npo,
> +		.head = *head,
> +		.gso_type = XEN_NETIF_GSO_TYPE_NONE,
> +	};
>  	unsigned long bytes;
> -	int gso_type = XEN_NETIF_GSO_TYPE_NONE;
>  
>  	if (skb_is_gso(skb)) {
>  		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
> -			gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
> +			info.gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
>  		else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
> -			gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
> +			info.gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
>  	}
>  
>  	/* Data must not cross a page boundary. */
>  	BUG_ON(size + offset > PAGE_SIZE<<compound_order(page));
>  
> -	meta = npo->meta + npo->meta_prod - 1;
> +	info.meta = npo->meta + npo->meta_prod - 1;
>  
>  	/* Skip unused frames from start of page */
>  	page += offset >> PAGE_SHIFT;
>  	offset &= ~PAGE_MASK;
>  
>  	while (size > 0) {
> -		struct xen_page_foreign *foreign;
> -
>  		BUG_ON(offset >= PAGE_SIZE);
> -		BUG_ON(npo->copy_off > MAX_BUFFER_OFFSET);
> -
> -		if (npo->copy_off == MAX_BUFFER_OFFSET)
> -			meta = get_next_rx_buffer(queue, npo);
>  
>  		bytes = PAGE_SIZE - offset;
>  		if (bytes > size)
>  			bytes = size;
>  
> -		if (npo->copy_off + bytes > MAX_BUFFER_OFFSET)
> -			bytes = MAX_BUFFER_OFFSET - npo->copy_off;
> -
> -		copy_gop = npo->copy + npo->copy_prod++;
> -		copy_gop->flags = GNTCOPY_dest_gref;
> -		copy_gop->len = bytes;
> -
> -		foreign = xen_page_foreign(page);
> -		if (foreign) {
> -			copy_gop->source.domid = foreign->domid;
> -			copy_gop->source.u.ref = foreign->gref;
> -			copy_gop->flags |= GNTCOPY_source_gref;
> -		} else {
> -			copy_gop->source.domid = DOMID_SELF;
> -			copy_gop->source.u.gmfn =
> -				virt_to_gfn(page_address(page));
> -		}
> -		copy_gop->source.offset = offset;
> -
> -		copy_gop->dest.domid = queue->vif->domid;
> -		copy_gop->dest.offset = npo->copy_off;
> -		copy_gop->dest.u.ref = npo->copy_gref;
> -
> -		npo->copy_off += bytes;
> -		meta->size += bytes;
> -
> -		offset += bytes;
> +		info.page = page;
> +		gnttab_foreach_grant_in_range(page, offset, bytes,
> +					      xenvif_gop_frag_copy_grant,
> +					      &info);

Looks like I need to at least wait until the API is settle before giving
my ack.

>  		size -= bytes;
> +		offset = 0;

This looks wrong. Should be offset += bytes.

>  
> -		/* Next frame */
> -		if (offset == PAGE_SIZE && size) {
> +		/* Next page */
> +		if (size) {
>  			BUG_ON(!PageCompound(page));
>  			page++;
> -			offset = 0;

And this should not be deleted, I think.

What is the reason for changing offset calculation? I think there is
still compound page when using 64K page.

>  		}
> -
> -		/* Leave a gap for the GSO descriptor. */
> -		if (*head && ((1 << gso_type) & queue->vif->gso_mask))
> -			queue->rx.req_cons++;
> -
> -		*head = 0; /* There must be something in this buffer now. */
> -
>  	}
> +
> +	*head = info.head;
>  }
>  

The reset looks OK.

Wei.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1203967 — Re: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity

FromJulien Grall <julien.grall@citrix.com>
Date2015-08-10 12:00 +0200
SubjectRe: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity
Message-ID<pVS28-2wD-49@gated-at.bofh.it>
In reply to#1203278
Hi Wei,

On 08/08/2015 15:55, Wei Liu wrote:
>>   struct xenvif_rx_meta {
>>   	int id;
>> @@ -80,16 +81,18 @@ struct xenvif_rx_meta {
>>   /* Discriminate from any valid pending_idx value. */
>>   #define INVALID_PENDING_IDX 0xFFFF
>>
>> -#define MAX_BUFFER_OFFSET PAGE_SIZE
>> +#define MAX_BUFFER_OFFSET XEN_PAGE_SIZE
>>
>>   #define MAX_PENDING_REQS XEN_NETIF_TX_RING_SIZE
>>
>> +#define MAX_XEN_SKB_FRAGS (65536 / XEN_PAGE_SIZE + 1)
>> +
>
> It might be clearer if you add a comment saying the maximum number of
> frags is derived from the page size of the grant page, which happens to
> be XEN_PAGE_SIZE at the moment.

Will do.


> In the future we need to figure out the page size of grant page in a
> dynamic way. We shall cross the bridge when we get there.

Right, there is few other places where we would need to do that too (see 
MAX_BUFFER_OFFSET for instance).


[..]

>> +		info.page = page;
>> +		gnttab_foreach_grant_in_range(page, offset, bytes,
>> +					      xenvif_gop_frag_copy_grant,
>> +					      &info);
>
> Looks like I need to at least wait until the API is settle before giving
> my ack.
>
>>   		size -= bytes;
>> +		offset = 0;
>
> This looks wrong. Should be offset += bytes.

With the new implementation of the loop, each iteration will be on a 
different page.
So only the first page has an offset different than zero.

>
>>
>> -		/* Next frame */
>> -		if (offset == PAGE_SIZE && size) {
>> +		/* Next page */
>> +		if (size) {
>>   			BUG_ON(!PageCompound(page));
>>   			page++;
>> -			offset = 0;
>
> And this should not be deleted, I think.
>
> What is the reason for changing offset calculation? I think there is
> still compound page when using 64K page.

The compound pages are still working ... gnttab_foreach_grant_in_range 
is called once per page. So the offset can be reset to 0 every time. No 
need to add code which would make the result less clear.

We only need to know if the size is not 0 to get the next page.

The patch may not be clear enough to see it's working so I've copied the 
result loop below:

         while (size > 0) {
                 BUG_ON(offset >= PAGE_SIZE);

                 bytes = PAGE_SIZE - offset;
                 if (bytes > size)
                         bytes = size;

                 info.page = page;
                 gnttab_foreach_grant_in_range(page, offset, bytes,
                                              xenvif_gop_frag_copy_grant,
                                               &info);
                 size -= bytes;
                 offset = 0;

                 /* Next page */
                 if (size) {
                         BUG_ON(!PageCompound(page));
                         page++;
                 }
         }

Regards,

-- 
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel


csiph-web