public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Avi Kivity <avi@redhat.com>
Cc: kvm@vger.kernel.org, Alexander Graf <agraf@suse.de>,
	kvm-ppc@vger.kernel.org, Scott Wood <scottwood@freescale.com>
Subject: Question about removing memslots
Date: Wed, 28 Mar 2012 18:24:46 +1100	[thread overview]
Message-ID: <1332919486.2882.88.camel@pasglop> (raw)

So I was chasing a bug today when I realized that some "drivers" in qemu
do interesting things with memory regions.

More specifically, cirrus emulation constantly flips the linear
framebuffer between being mapped into the guest and being emulated MMIO
(the latter for the purpose of image blits).

This made me ponder ... whenever a memslot is "removed" like that (in
the case for example where cirrus turns the fb into emulation), we need
to ensure that any cached translation that involve those GPAs are
flushed out of whatever caching (HW or SW) is done by the KVM arch
code...

So I started looking and the only thing I can find (let me know if I
missed something) is kvm_arch_flush_shadow(). Is that it ? Because it
doesn't take the memslot going away as an argument, so it doesn't know
-what- to flush...

Now I see that x86 just seems to flush everything, which is quite heavy
handed considering how often cirrus does it, but maybe it doesn't have a
choice (lack of reverse mapping from GPA ?).

I also noticed that powerpc ... doesn't do anything :-) Ooops....

So all translations still present in the TLB will remain there, all
translations present in the MMU hash table as well, etc...

Now, in order to implement that properly and efficiently, we would need
to at least get the GPA (if not the whole memslot).

Do you have any objection (provided I didn't completely misunderstand
something which is quite possible) to us adding that argument to
kvm_arch_flush_shadow() ? We can easily put in a small patch adding that
as an unused argument, and later get the proper implementation for
powerpc.

Another thing I noticed while at it is that my version of
__kvm_set_memory_region() appears to call kvm_iommu_map_pages() whenever
adding a memslot ... but never does the opposite unmapping when that
memory slot is removed.... isn't that potentially an issue ?

Cheers,
Ben.

             reply	other threads:[~2012-03-28  7:24 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-28  7:24 Benjamin Herrenschmidt [this message]
2012-03-28  9:37 ` Question about removing memslots Avi Kivity
2012-03-28  9:59   ` Benjamin Herrenschmidt
2012-03-28 10:05     ` Avi Kivity
2012-03-28 10:17       ` Benjamin Herrenschmidt
2012-03-28 10:51         ` Avi Kivity
2012-03-28 21:04           ` Benjamin Herrenschmidt
2012-03-29  9:36             ` Avi Kivity
2012-03-29 11:46               ` Benjamin Herrenschmidt
2012-03-29 13:49               ` Gerd Hoffmann
2012-03-29  5:15   ` Takuya Yoshikawa
2012-03-29  9:44     ` Avi Kivity
2012-03-29 15:21       ` Takuya Yoshikawa
2012-03-29 15:26         ` Avi Kivity
2012-03-29 15:35           ` Takuya Yoshikawa

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=1332919486.2882.88.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=agraf@suse.de \
    --cc=avi@redhat.com \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=scottwood@freescale.com \
    /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