All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Avi Kivity <avi@cloudius-systems.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2 5/7] exec: memory radix tree page level?compression
Date: Sun, 17 Nov 2013 17:37:55 +0200	[thread overview]
Message-ID: <20131117153755.GB28008@redhat.com> (raw)
In-Reply-To: <5284EF62.8070108@cloudius-systems.com>

On Thu, Nov 14, 2013 at 05:42:26PM +0200, Avi Kivity wrote:
> On 11/14/2013 05:37 PM, Michael S. Tsirkin wrote:
> >On Thu, Nov 14, 2013 at 04:56:43PM +0200, Avi Kivity wrote:
> >>On 11/14/2013 04:40 PM, Michael S. Tsirkin wrote:
> >>>On Thu, Nov 14, 2013 at 08:54:11AM +0000, Avi Kivity wrote:
> >>>>Michael S. Tsirkin <mst <at> redhat.com> writes:
> >>>>
> >>>>>At the moment, memory radix tree is already variable width, but it can
> >>>>>only skip the low bits of address.
> >>>>>
> >>>>>This is efficient if we have huge memory regions but inefficient if we
> >>>>>are only using a tiny portion of the address space.
> >>>>>
> >>>>>After we have built up the map, detect
> >>>>>configurations where a single L2 entry is valid.
> >>>>>
> >>>>>We then speed up the lookup by skipping one or more levels.
> >>>>>In case any levels were skipped, we might end up in a valid section
> >>>>>instead of erroring out. We handle this by checking that
> >>>>>the address is in range of the resulting section.
> >>>>>
> >>>>I think this is overkill.  It can be done in a simpler way as follows:
> >>>>
> >>>>
> >>>>phys_page_find(RadixTree* tr, hwaddr index, ...)
> >>>>{
> >>>>    if (index & rt->invalid_index_mask) {
> >>>>        // not found
> >>>>    }
> >>>>    lp = rt->root;
> >>>>    for (i = rt->nb_levels - 1; i >= 0 && !lp.is_leaf; --i) {
> >>>>      ...
> >>>>
> >>>>This exploits the fact the lower portion of the address space is always
> >>>>filled, at least in the cases that matter to us.
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>Basically skip unused high bits?
> >>>Sure.
> >>>In fact I think both optimizations can be combined.
> >>Not much value in combining them -- the variable level tree check
> >>will be dominated by the level skip logic.
> >>
> >>IMO however skipping intermediate levels will be too rare to justify
> >>the complexity and the doubling of the page table size -- it can
> >>only happen in iommu setups that place memory in very high
> >>addresses.  These ought to be rare.
> >>
> >Well maybe not very high address, but you can have a device with
> >a 64 bit bar and this will add back levels (though it would not
> >be slower than it is currently).
> >
> >I agree the simplicity is attractive.
> >
> >However I really would like some logic that can handle > 1 leaf
> >somehow.
> >
> >Specifically both tricks break if we add a full 64 bit io region
> >in order to return -1 for reads on master abort.
> 
> That can be achieved by directing a missed lookup to a catch-all region.
> 
> It would be best to do this completely internally to the memory
> code, without API changes.

You mean e.g. add a catch-all region to the address space?

  reply	other threads:[~2013-11-17 15:35 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-13 19:23 [Qemu-devel] [PATCH v2 0/7] making address spaces 64 bit wide Michael S. Tsirkin
2013-11-13 19:23 ` [Qemu-devel] [PATCH v2 1/7] split definitions for exec.c and translate-all.c radix trees Michael S. Tsirkin
2013-11-13 19:23 ` [Qemu-devel] [PATCH v2 2/7] exec: replace leaf with skip Michael S. Tsirkin
2013-11-13 19:23 ` [Qemu-devel] [PATCH v2 3/7] exec: extend skip field to 6 bit, page entry to 32 bit Michael S. Tsirkin
2013-11-13 19:23 ` [Qemu-devel] [PATCH v2 4/7] exec: pass hw address to phys_page_find Michael S. Tsirkin
2013-11-13 19:23 ` [Qemu-devel] [PATCH v2 5/7] exec: memory radix tree page level compression Michael S. Tsirkin
2013-11-14  8:54   ` Avi Kivity
2013-11-14 14:40     ` [Qemu-devel] [PATCH v2 5/7] exec: memory radix tree page level?compression Michael S. Tsirkin
2013-11-14 14:56       ` Avi Kivity
2013-11-14 15:37         ` Michael S. Tsirkin
2013-11-14 15:42           ` Avi Kivity
2013-11-17 15:37             ` Michael S. Tsirkin [this message]
2013-11-17 16:12               ` Avi Kivity
2013-11-13 19:24 ` [Qemu-devel] [PATCH v2 6/7] exec: make address spaces 64-bit wide Michael S. Tsirkin
2013-11-13 19:24 ` [Qemu-devel] [PATCH v2 7/7] exec: reduce L2_PAGE_SIZE Michael S. Tsirkin

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=20131117153755.GB28008@redhat.com \
    --to=mst@redhat.com \
    --cc=avi@cloudius-systems.com \
    --cc=qemu-devel@nongnu.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.