xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Julien Grall <julien.grall@arm.com>
To: xen-devel@lists.xenproject.org
Cc: sstabellini@kernel.org, "Wei Liu" <wei.liu2@citrix.com>,
	"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
	"George Dunlap" <George.Dunlap@eu.citrix.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	"Tim Deegan" <tim@xen.org>, "Jan Beulich" <jbeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [PATCH for-4.12 v2 17/17] xen/arm: Track page accessed between batch of Set/Way operations
Date: Thu, 6 Dec 2018 12:21:12 +0000	[thread overview]
Message-ID: <25eb1beb-27e5-7057-550e-58e3c69e074e@arm.com> (raw)
In-Reply-To: <20181204202651.8836-18-julien.grall@arm.com>

Hi,

On 12/4/18 8:26 PM, Julien Grall wrote:
> At the moment, the implementation of Set/Way operations will go through
> all the entries of the guest P2M and flush them. However, this is very
> expensive and may render unusable a guest OS using them.
> 
> For instance, Linux 32-bit will use Set/Way operations during secondary
> CPU bring-up. As the implementation is really expensive, it may be possible
> to hit the CPU bring-up timeout.
> 
> To limit the Set/Way impact, we track what pages has been of the guest
> has been accessed between batch of Set/Way operations. This is done
> using bit[0] (aka valid bit) of the P2M entry.
> 
> This patch adds a new per-arch helper is introduced to perform actions just
> before the guest is first unpaused. This will be used to invalidate the
> P2M to track access from the start of the guest.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
> 
> While we can spread d->creation_finished all over the code, the per-arch
> helper to perform actions just before the guest is first unpaused can
> bring a lot of benefit for both architecture. For instance, on Arm, the
> flush to the instruction cache could be delayed until the domain is
> first run. This would improve greatly the performance of creating guest.
> 
> I am still doing the benchmark whether having a command line option is
> worth it. I will provide numbers as soon as I have them.

I remembered Stefano suggested to look at the impact on the boot. This 
is a bit tricky to do as there are many kernel configurations existing 
and all the mappings may not have been touched during the boot.

Instead I wrote a tiny guest [1] that will zero roughly 1GB of memory. 
Because the toolstack will always try to allocate with the biggest 
mapping, I had to hack a bit the toolstack to be able to test with 
different mapping size (but not a mix). The guest has only one vCPU with 
a dedicated pCPU.
	- 1GB: 0.03% slower when starting with valid bit unset
	- 2MB: 0.04% faster when starting with valid bit unset
         - 4KB: ~3% slower when starting with valid bit unset

The performance using 1GB and 2MB mapping is pretty much insignificant 
because the number of traps is very limited (resp. 1 and 513). With 4KB 
mapping, there are a much significant drop because you have more traps 
(~262700) as the P2M contains more entries.

However, having many 4KB mappings in the P2M is pretty unlikely as the 
toolstack will always try to get bigger mapping. In real world, you 
should only have 4KB mappings when you guest has not memory aligned with 
a bigger mapping. If you end up to have many 4KB mappings, then you are 
already going to have a performance impact in long run because of the 
TLB pressure.

Overall, I would not recommend to introduce a command line option until 
we figured out a use case where the trap will be a slow down.

Cheers,

[1]

.text
     b       _start                  /* branch to kernel start, magic */
     .long   0                       /* reserved */
     .quad   0x0                     /* Image load offset from start of 
RAM */
     .quad   0x0                     /* XXX: Effective Image size */
     .quad   2                       /* kernel flags: LE, 4K page size */
     .quad   0                       /* reserved */
     .quad   0                       /* reserved */
     .quad   0                       /* reserved */
     .byte   0x41                    /* Magic number, "ARM\x64" */
     .byte   0x52
     .byte   0x4d
     .byte   0x64
     .long   0                       /* reserved */

_start:
     isb
     mrs     x0, CNTPCT_EL0
     isb

     adrp    x2, _end
     ldr     x3, =(0x40000000 + (1 << 30))
1:  str     xzr, [x2], #8
     cmp     x2, x3
     b.lo    1b

     isb
     mrs     x1, CNTPCT_EL0
     isb
     hvc     #0xffff
1:  b       1b

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  parent reply	other threads:[~2018-12-06 12:21 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-04 20:26 [PATCH for-4.12 v2 00/17] xen/arm: Implement Set/Way operations Julien Grall
2018-12-04 20:26 ` [PATCH for-4.12 v2 01/17] xen/arm: Introduce helpers to clear/flags flags in HCR_EL2 Julien Grall
2018-12-04 20:26 ` [PATCH for-4.12 v2 02/17] xen/arm: traps: Move the implementation of GUEST_BUG_ON in traps.h Julien Grall
2018-12-04 20:26 ` [PATCH for-4.12 v2 03/17] xen/arm: p2m: Clean-up headers included and order them alphabetically Julien Grall
2018-12-04 23:47   ` Stefano Stabellini
2018-12-04 20:26 ` [PATCH for-4.12 v2 04/17] xen/arm: p2m: Introduce p2m_is_valid and use it Julien Grall
2018-12-04 23:50   ` Stefano Stabellini
2018-12-05  9:46     ` Julien Grall
2018-12-06 22:02       ` Stefano Stabellini
2018-12-07 10:14         ` Julien Grall
2018-12-04 20:26 ` [PATCH for-4.12 v2 05/17] xen/arm: p2m: Handle translation fault in get_page_from_gva Julien Grall
2018-12-04 23:59   ` Stefano Stabellini
2018-12-05 10:03     ` Julien Grall
2018-12-06 22:04       ` Stefano Stabellini
2018-12-07 10:16         ` Julien Grall
2018-12-07 16:56           ` Stefano Stabellini
2018-12-04 20:26 ` [PATCH for-4.12 v2 06/17] xen/arm: p2m: Introduce a function to resolve translation fault Julien Grall
2018-12-06 22:33   ` Stefano Stabellini
2018-12-04 20:26 ` [PATCH for-4.12 v2 07/17] xen/arm: vcpreg: Add wrappers to handle co-proc access trapped by HCR_EL2.TVM Julien Grall
2018-12-06 22:33   ` Stefano Stabellini
2018-12-04 20:26 ` [PATCH for-4.12 v2 08/17] xen/arm: vsysreg: Add wrapper to handle sysreg " Julien Grall
2018-12-04 20:26 ` [PATCH for-4.12 v2 09/17] xen/arm: Rework p2m_cache_flush to take a range [begin, end) Julien Grall
2018-12-04 20:26 ` [PATCH for-4.12 v2 10/17] xen/arm: p2m: Allow to flush cache on any RAM region Julien Grall
2018-12-04 20:26 ` [PATCH for-4.12 v2 11/17] xen/arm: p2m: Extend p2m_get_entry to return the value of bit[0] (valid bit) Julien Grall
2018-12-04 20:35   ` Razvan Cojocaru
2018-12-06 22:32     ` Stefano Stabellini
2018-12-07 10:17     ` Julien Grall
2018-12-04 20:26 ` [PATCH for-4.12 v2 12/17] xen/arm: traps: Rework leave_hypervisor_tail Julien Grall
2018-12-06 23:08   ` Stefano Stabellini
2018-12-04 20:26 ` [PATCH for-4.12 v2 13/17] xen/arm: p2m: Rework p2m_cache_flush_range Julien Grall
2018-12-06 23:53   ` Stefano Stabellini
2018-12-07 10:18     ` Julien Grall
2018-12-04 20:26 ` [PATCH for-4.12 v2 14/17] xen/arm: domctl: Use typesafe gfn in XEN_DOMCTL_cacheflush Julien Grall
2018-12-06 23:13   ` Stefano Stabellini
2018-12-04 20:26 ` [PATCH for-4.12 v2 15/17] xen/arm: p2m: Add support for preemption in p2m_cache_flush_range Julien Grall
2018-12-06 23:32   ` Stefano Stabellini
2018-12-07 11:15     ` Julien Grall
2018-12-07 22:11       ` Stefano Stabellini
2018-12-11 16:11         ` Julien Grall
2018-12-04 20:26 ` [PATCH for-4.12 v2 16/17] xen/arm: Implement Set/Way operations Julien Grall
2018-12-06 23:32   ` Stefano Stabellini
2018-12-07 13:22     ` Julien Grall
2018-12-07 21:29       ` Stefano Stabellini
2018-12-12 15:33         ` Julien Grall
2018-12-12 17:25           ` Stefano Stabellini
2018-12-12 17:49             ` Dario Faggioli
2018-12-04 20:26 ` [PATCH for-4.12 v2 17/17] xen/arm: Track page accessed between batch of " Julien Grall
2018-12-05  8:37   ` Jan Beulich
2018-12-07 13:24     ` Julien Grall
2018-12-06 12:21   ` Julien Grall [this message]
2018-12-07 21:52     ` Stefano Stabellini
2018-12-07 21:43   ` Stefano Stabellini
2018-12-11 16:22     ` Julien Grall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=25eb1beb-27e5-7057-550e-58e3c69e074e@arm.com \
    --to=julien.grall@arm.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=konrad.wilk@oracle.com \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).