From: Paolo Bonzini <pbonzini@redhat.com>
To: Andrew Honig <ahonig@google.com>
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: Allow cross page reads and writes from cached translations
Date: Fri, 29 Mar 2013 09:34:08 +0100 [thread overview]
Message-ID: <51555200.9050207@redhat.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1303282041300.2353@ahonig-virtual-machine>
Il 29/03/2013 04:44, Andrew Honig ha scritto:
>
> This patch adds support for kvm_gfn_to_hva_cache_init functions for
> reads and writes that will cross a page. If the range falls within
> the same memslot, then this will be a fast operation. If the range
> is split between two memslots, then the slower kvm_read_guest and
> kvm_write_guest are used.
>
> Tested: Test against kvm_clock unit tests.
>
> Signed-off-by: Andrew Honig <ahonig@google.com>
> ---
> arch/x86/kvm/lapic.c | 2 +-
> arch/x86/kvm/x86.c | 12 +++++-------
> include/linux/kvm_host.h | 2 +-
> include/linux/kvm_types.h | 2 ++
> virt/kvm/kvm_main.c | 43 +++++++++++++++++++++++++++++++++----------
> 5 files changed, 42 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 02b51dd..f77df1c 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1857,7 +1857,7 @@ int kvm_lapic_enable_pv_eoi(struct kvm_vcpu *vcpu, u64 data)
> if (!pv_eoi_enabled(vcpu))
> return 0;
> return kvm_gfn_to_hva_cache_init(vcpu->kvm, &vcpu->arch.pv_eoi.data,
> - addr);
> + addr, sizeof(u8));
> }
>
> void kvm_lapic_init(void)
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index f19ac0a..0fd5ad8 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1823,7 +1823,7 @@ static int kvm_pv_enable_async_pf(struct kvm_vcpu *vcpu, u64 data)
> return 0;
> }
>
> - if (kvm_gfn_to_hva_cache_init(vcpu->kvm, &vcpu->arch.apf.data, gpa))
> + if (kvm_gfn_to_hva_cache_init(vcpu->kvm, &vcpu->arch.apf.data, gpa, 4))
> return 1;
Please use sizeof(u32).
> vcpu->arch.apf.send_user_only = !(data & KVM_ASYNC_PF_SEND_ALWAYS);
> @@ -1952,12 +1952,9 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>
> gpa_offset = data & ~(PAGE_MASK | 1);
>
> - /* Check that the address is 32-byte aligned. */
> - if (gpa_offset & (sizeof(struct pvclock_vcpu_time_info) - 1))
> - break;
> -
> if (kvm_gfn_to_hva_cache_init(vcpu->kvm,
> - &vcpu->arch.pv_time, data & ~1ULL))
> + &vcpu->arch.pv_time, data & ~1ULL,
> + sizeof(struct pvclock_vcpu_time_info)))
> vcpu->arch.pv_time_enabled = false;
> else
> vcpu->arch.pv_time_enabled = true;
> @@ -1977,7 +1974,8 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> return 1;
>
> if (kvm_gfn_to_hva_cache_init(vcpu->kvm, &vcpu->arch.st.stime,
> - data & KVM_STEAL_VALID_BITS))
> + data & KVM_STEAL_VALID_BITS,
> + sizeof(struct kvm_steal_time)))
> return 1;
>
> vcpu->arch.st.msr_val = data;
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index cad77fe..c139582 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -518,7 +518,7 @@ int kvm_write_guest(struct kvm *kvm, gpa_t gpa, const void *data,
> int kvm_write_guest_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
> void *data, unsigned long len);
> int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
> - gpa_t gpa);
> + gpa_t gpa, unsigned long len);
> int kvm_clear_guest_page(struct kvm *kvm, gfn_t gfn, int offset, int len);
> int kvm_clear_guest(struct kvm *kvm, gpa_t gpa, unsigned long len);
> struct kvm_memory_slot *gfn_to_memslot(struct kvm *kvm, gfn_t gfn);
> diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
> index fa7cc72..bd420d6 100644
> --- a/include/linux/kvm_types.h
> +++ b/include/linux/kvm_types.h
> @@ -71,6 +71,8 @@ struct gfn_to_hva_cache {
> u64 generation;
> gpa_t gpa;
> unsigned long hva;
> + unsigned long len;
> + int slow;
You can just use memslot = NULL instead of the new member "slow".
> struct kvm_memory_slot *memslot;
> };
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index adc68fe..dcd3e57 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1541,21 +1541,38 @@ int kvm_write_guest(struct kvm *kvm, gpa_t gpa, const void *data,
> }
>
> int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
> - gpa_t gpa)
> + gpa_t gpa, unsigned long len)
> {
> struct kvm_memslots *slots = kvm_memslots(kvm);
> int offset = offset_in_page(gpa);
> - gfn_t gfn = gpa >> PAGE_SHIFT;
> + gfn_t start_gfn = gpa >> PAGE_SHIFT;
> + gfn_t end_gfn = (gpa + len - 1) >> PAGE_SHIFT;
> + gfn_t nr_pages_needed = end_gfn - start_gfn + 1;
> + gfn_t nr_pages_avail;
>
> ghc->gpa = gpa;
> + ghc->slow = 0;
> ghc->generation = slots->generation;
> - ghc->memslot = gfn_to_memslot(kvm, gfn);
> - ghc->hva = gfn_to_hva_many(ghc->memslot, gfn, NULL);
> - if (!kvm_is_error_hva(ghc->hva))
> + ghc->len = len;
> + ghc->memslot = gfn_to_memslot(kvm, start_gfn);
> + ghc->hva = gfn_to_hva_many(ghc->memslot, start_gfn, &nr_pages_avail);
> + if (!kvm_is_error_hva(ghc->hva) && nr_pages_avail >= nr_pages_needed) {
> ghc->hva += offset;
> - else
> - return -EFAULT;
> -
> + } else {
> + ghc->slow = 1;
> + /*
> + * If the requested region crosses two memslots, we still
> + * verify that the entire region is valid here.
> + */
> + while (start_gfn <= end_gfn) {
> + ghc->memslot = gfn_to_memslot(kvm, start_gfn);
> + ghc->hva = gfn_to_hva_many(ghc->memslot, start_gfn,
> + &nr_pages_avail);
> + if (kvm_is_error_hva(ghc->hva))
> + return -EFAULT;
> + start_gfn += nr_pages_avail;
> + }
> + }
> return 0;
> }
> EXPORT_SYMBOL_GPL(kvm_gfn_to_hva_cache_init);
> @@ -1567,7 +1584,10 @@ int kvm_write_guest_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
> int r;
I think you want a
BUG_ON(len > ghc->len);
here and in kvm_read_guest_cached.
Thanks,
Paolo
> if (slots->generation != ghc->generation)
> - kvm_gfn_to_hva_cache_init(kvm, ghc, ghc->gpa);
> + kvm_gfn_to_hva_cache_init(kvm, ghc, ghc->gpa, ghc->len);
> +
> + if (unlikely(ghc->slow))
> + return kvm_write_guest(kvm, ghc->gpa, data, len);
>
> if (kvm_is_error_hva(ghc->hva))
> return -EFAULT;
> @@ -1588,7 +1608,10 @@ int kvm_read_guest_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
> int r;
>
> if (slots->generation != ghc->generation)
> - kvm_gfn_to_hva_cache_init(kvm, ghc, ghc->gpa);
> + kvm_gfn_to_hva_cache_init(kvm, ghc, ghc->gpa, ghc->len);
> +
> + if (unlikely(ghc->slow))
> + return kvm_read_guest(kvm, ghc->gpa, data, len);
>
> if (kvm_is_error_hva(ghc->hva))
> return -EFAULT;
>
prev parent reply other threads:[~2013-03-29 8:34 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-29 3:44 [PATCH] KVM: Allow cross page reads and writes from cached translations Andrew Honig
2013-03-29 8:34 ` Paolo Bonzini [this message]
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=51555200.9050207@redhat.com \
--to=pbonzini@redhat.com \
--cc=ahonig@google.com \
--cc=kvm@vger.kernel.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.