* Re: [PATCH] KVM: use slowpath for cross page cached accesses
2015-04-08 12:16 ` [PATCH] KVM: use slowpath for cross page cached accesses Radim Krčmář
@ 2015-04-08 12:23 ` Paolo Bonzini
2015-04-09 0:18 ` Wanpeng Li
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2015-04-08 12:23 UTC (permalink / raw)
To: Radim Krčmář; +Cc: linux-kernel, kvm
On 08/04/2015 14:16, Radim Krčmář wrote:
> 2015-04-08 12:43+0200, Paolo Bonzini:
>> On 08/04/2015 11:26, Radim Krčmář wrote:
>>> Only cross-memslot writes have NULL memslot.
>>
>> The power of wrong comments...
>>
>> Considering how kvm_gfn_to_hva_cache_init is used (one 1-byte field, two
>> 4-byte fields, one 28-bytes struct that is 32-bytes aligned, one
>> 32-bytes field that is in practice cacheline-aligned), I wonder if we
>> should just use ghc->memslot = NULL for cross page writes. This would
>> bypass the bug you are fixing here, and avoid worries about partial writes.
>
> Good idea, and it could make those comments right :)
> (Though in general, I prefer less constraints on APIs ...)
It doesn't put constraints, it still handles cross page writes right
(just slower). copy_to_user in some sense is the API that constrains us
to do this.
> Partial writes would be a pain; copy_to_user API does not define which
> bytes were not written. I think the write can't fail mid-page, which
> makes our implementation ok
No, writes can't fail mid-page (I guess in atomic context it's
theoretically possible, but we're equipped to handle the failure in that
case).
Patch applied, thanks!
Paolo
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] KVM: use slowpath for cross page cached accesses
2015-04-08 12:16 ` [PATCH] KVM: use slowpath for cross page cached accesses Radim Krčmář
2015-04-08 12:23 ` Paolo Bonzini
@ 2015-04-09 0:18 ` Wanpeng Li
2015-05-02 12:23 ` Patch "KVM: use slowpath for cross page cached accesses" has been added to the 4.0-stable tree gregkh
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Wanpeng Li @ 2015-04-09 0:18 UTC (permalink / raw)
To: Radim Krčmář; +Cc: Paolo Bonzini, linux-kernel, kvm
On Wed, Apr 08, 2015 at 02:16:48PM +0200, Radim Krčmář wrote:
>2015-04-08 12:43+0200, Paolo Bonzini:
>> On 08/04/2015 11:26, Radim Krčmář wrote:
>>> 2015-04-08 10:49+0200, Paolo Bonzini:
>>>> On 07/04/2015 22:34, Radim Krčmář wrote:
>>>>> We dirtied only one page because writes originally couldn't span more.
>>>>> Use improved syntax for '>> PAGE_SHIFT' while at it.
>>>>>
>>>>> Fixes: 8f964525a121 ("KVM: Allow cross page reads and writes from cached translations.")
>>>>> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
>>>>
>>>> Cross-page reads and writes should never get here; they have
>>>> ghc->memslot set to NULL and go through the slow path in kvm_write_guest.
>>>
>>> Only cross-memslot writes have NULL memslot.
>>
>> The power of wrong comments...
>>
>> Considering how kvm_gfn_to_hva_cache_init is used (one 1-byte field, two
>> 4-byte fields, one 28-bytes struct that is 32-bytes aligned, one
>> 32-bytes field that is in practice cacheline-aligned), I wonder if we
>> should just use ghc->memslot = NULL for cross page writes. This would
>> bypass the bug you are fixing here, and avoid worries about partial writes.
>
>Good idea, and it could make those comments right :)
>(Though in general, I prefer less constraints on APIs ...)
>
>Partial writes would be a pain; copy_to_user API does not define which
>bytes were not written. I think the write can't fail mid-page, which
>makes our implementation ok, but I still worry a bit about it.
>
>Anyway, here's the patch:
>
>---8<---
>kvm_write_guest_cached() does not mark all written pages as dirty and
>code comments in kvm_gfn_to_hva_cache_init() talk about NULL memslot
>with cross page accesses. Fix all the easy way.
>
>The check is '<= 1' to have the same result for 'len = 0' cache anywhere
>in the page. (nr_pages_needed is 0 on page boundary.)
>
>Fixes: 8f964525a121 ("KVM: Allow cross page reads and writes from cached translations.")
>Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
Reviewed-by: Wanpeng Li <wanpeng.li@linux.intel.com>
>---
> virt/kvm/kvm_main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>index aadef264bed1..f3dc641f9640 100644
>--- a/virt/kvm/kvm_main.c
>+++ b/virt/kvm/kvm_main.c
>@@ -1637,8 +1637,8 @@ int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
> ghc->generation = slots->generation;
> 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 = gfn_to_hva_many(ghc->memslot, start_gfn, NULL);
>+ if (!kvm_is_error_hva(ghc->hva) && nr_pages_needed <= 1) {
> ghc->hva += offset;
> } else {
> /*
>--
>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/
^ permalink raw reply [flat|nested] 11+ messages in thread* Patch "KVM: use slowpath for cross page cached accesses" has been added to the 4.0-stable tree
2015-04-08 12:16 ` [PATCH] KVM: use slowpath for cross page cached accesses Radim Krčmář
2015-04-08 12:23 ` Paolo Bonzini
2015-04-09 0:18 ` Wanpeng Li
@ 2015-05-02 12:23 ` gregkh
2015-05-02 12:25 ` Patch "KVM: use slowpath for cross page cached accesses" has been added to the 3.19-stable tree gregkh
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: gregkh @ 2015-05-02 12:23 UTC (permalink / raw)
To: rkrcmar, gregkh, pbonzini, wanpeng.li; +Cc: stable, stable-commits
This is a note to let you know that I've just added the patch titled
KVM: use slowpath for cross page cached accesses
to the 4.0-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
kvm-use-slowpath-for-cross-page-cached-accesses.patch
and it can be found in the queue-4.0 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
>From ca3f0874723fad81d0c701b63ae3a17a408d5f25 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= <rkrcmar@redhat.com>
Date: Wed, 8 Apr 2015 14:16:48 +0200
Subject: KVM: use slowpath for cross page cached accesses
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= <rkrcmar@redhat.com>
commit ca3f0874723fad81d0c701b63ae3a17a408d5f25 upstream.
kvm_write_guest_cached() does not mark all written pages as dirty and
code comments in kvm_gfn_to_hva_cache_init() talk about NULL memslot
with cross page accesses. Fix all the easy way.
The check is '<= 1' to have the same result for 'len = 0' cache anywhere
in the page. (nr_pages_needed is 0 on page boundary.)
Fixes: 8f964525a121 ("KVM: Allow cross page reads and writes from cached translations.")
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
Message-Id: <20150408121648.GA3519@potion.brq.redhat.com>
Reviewed-by: Wanpeng Li <wanpeng.li@linux.intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
virt/kvm/kvm_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1653,8 +1653,8 @@ int kvm_gfn_to_hva_cache_init(struct kvm
ghc->generation = slots->generation;
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 = gfn_to_hva_many(ghc->memslot, start_gfn, NULL);
+ if (!kvm_is_error_hva(ghc->hva) && nr_pages_needed <= 1) {
ghc->hva += offset;
} else {
/*
Patches currently in stable-queue which might be from rkrcmar@redhat.com are
queue-4.0/kvm-use-slowpath-for-cross-page-cached-accesses.patch
queue-4.0/x86-vdso-fix-pvclock-races-with-task-migration.patch
^ permalink raw reply [flat|nested] 11+ messages in thread* Patch "KVM: use slowpath for cross page cached accesses" has been added to the 3.19-stable tree
2015-04-08 12:16 ` [PATCH] KVM: use slowpath for cross page cached accesses Radim Krčmář
` (2 preceding siblings ...)
2015-05-02 12:23 ` Patch "KVM: use slowpath for cross page cached accesses" has been added to the 4.0-stable tree gregkh
@ 2015-05-02 12:25 ` gregkh
2015-05-02 12:28 ` Patch "KVM: use slowpath for cross page cached accesses" has been added to the 3.10-stable tree gregkh
2015-05-02 12:29 ` Patch "KVM: use slowpath for cross page cached accesses" has been added to the 3.14-stable tree gregkh
5 siblings, 0 replies; 11+ messages in thread
From: gregkh @ 2015-05-02 12:25 UTC (permalink / raw)
To: rkrcmar, gregkh, pbonzini, wanpeng.li; +Cc: stable, stable-commits
This is a note to let you know that I've just added the patch titled
KVM: use slowpath for cross page cached accesses
to the 3.19-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
kvm-use-slowpath-for-cross-page-cached-accesses.patch
and it can be found in the queue-3.19 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
>From ca3f0874723fad81d0c701b63ae3a17a408d5f25 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= <rkrcmar@redhat.com>
Date: Wed, 8 Apr 2015 14:16:48 +0200
Subject: KVM: use slowpath for cross page cached accesses
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= <rkrcmar@redhat.com>
commit ca3f0874723fad81d0c701b63ae3a17a408d5f25 upstream.
kvm_write_guest_cached() does not mark all written pages as dirty and
code comments in kvm_gfn_to_hva_cache_init() talk about NULL memslot
with cross page accesses. Fix all the easy way.
The check is '<= 1' to have the same result for 'len = 0' cache anywhere
in the page. (nr_pages_needed is 0 on page boundary.)
Fixes: 8f964525a121 ("KVM: Allow cross page reads and writes from cached translations.")
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
Message-Id: <20150408121648.GA3519@potion.brq.redhat.com>
Reviewed-by: Wanpeng Li <wanpeng.li@linux.intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
virt/kvm/kvm_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1608,8 +1608,8 @@ int kvm_gfn_to_hva_cache_init(struct kvm
ghc->generation = slots->generation;
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 = gfn_to_hva_many(ghc->memslot, start_gfn, NULL);
+ if (!kvm_is_error_hva(ghc->hva) && nr_pages_needed <= 1) {
ghc->hva += offset;
} else {
/*
Patches currently in stable-queue which might be from rkrcmar@redhat.com are
queue-3.19/kvm-use-slowpath-for-cross-page-cached-accesses.patch
^ permalink raw reply [flat|nested] 11+ messages in thread* Patch "KVM: use slowpath for cross page cached accesses" has been added to the 3.10-stable tree
2015-04-08 12:16 ` [PATCH] KVM: use slowpath for cross page cached accesses Radim Krčmář
` (3 preceding siblings ...)
2015-05-02 12:25 ` Patch "KVM: use slowpath for cross page cached accesses" has been added to the 3.19-stable tree gregkh
@ 2015-05-02 12:28 ` gregkh
2015-05-02 12:29 ` Patch "KVM: use slowpath for cross page cached accesses" has been added to the 3.14-stable tree gregkh
5 siblings, 0 replies; 11+ messages in thread
From: gregkh @ 2015-05-02 12:28 UTC (permalink / raw)
To: rkrcmar, gregkh, pbonzini, wanpeng.li; +Cc: stable, stable-commits
This is a note to let you know that I've just added the patch titled
KVM: use slowpath for cross page cached accesses
to the 3.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
kvm-use-slowpath-for-cross-page-cached-accesses.patch
and it can be found in the queue-3.10 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
>From ca3f0874723fad81d0c701b63ae3a17a408d5f25 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= <rkrcmar@redhat.com>
Date: Wed, 8 Apr 2015 14:16:48 +0200
Subject: KVM: use slowpath for cross page cached accesses
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= <rkrcmar@redhat.com>
commit ca3f0874723fad81d0c701b63ae3a17a408d5f25 upstream.
kvm_write_guest_cached() does not mark all written pages as dirty and
code comments in kvm_gfn_to_hva_cache_init() talk about NULL memslot
with cross page accesses. Fix all the easy way.
The check is '<= 1' to have the same result for 'len = 0' cache anywhere
in the page. (nr_pages_needed is 0 on page boundary.)
Fixes: 8f964525a121 ("KVM: Allow cross page reads and writes from cached translations.")
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
Message-Id: <20150408121648.GA3519@potion.brq.redhat.com>
Reviewed-by: Wanpeng Li <wanpeng.li@linux.intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
virt/kvm/kvm_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1549,8 +1549,8 @@ int kvm_gfn_to_hva_cache_init(struct kvm
ghc->generation = slots->generation;
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 = gfn_to_hva_many(ghc->memslot, start_gfn, NULL);
+ if (!kvm_is_error_hva(ghc->hva) && nr_pages_needed <= 1) {
ghc->hva += offset;
} else {
/*
Patches currently in stable-queue which might be from rkrcmar@redhat.com are
queue-3.10/kvm-use-slowpath-for-cross-page-cached-accesses.patch
^ permalink raw reply [flat|nested] 11+ messages in thread* Patch "KVM: use slowpath for cross page cached accesses" has been added to the 3.14-stable tree
2015-04-08 12:16 ` [PATCH] KVM: use slowpath for cross page cached accesses Radim Krčmář
` (4 preceding siblings ...)
2015-05-02 12:28 ` Patch "KVM: use slowpath for cross page cached accesses" has been added to the 3.10-stable tree gregkh
@ 2015-05-02 12:29 ` gregkh
5 siblings, 0 replies; 11+ messages in thread
From: gregkh @ 2015-05-02 12:29 UTC (permalink / raw)
To: rkrcmar, gregkh, pbonzini, wanpeng.li; +Cc: stable, stable-commits
This is a note to let you know that I've just added the patch titled
KVM: use slowpath for cross page cached accesses
to the 3.14-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
kvm-use-slowpath-for-cross-page-cached-accesses.patch
and it can be found in the queue-3.14 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
>From ca3f0874723fad81d0c701b63ae3a17a408d5f25 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= <rkrcmar@redhat.com>
Date: Wed, 8 Apr 2015 14:16:48 +0200
Subject: KVM: use slowpath for cross page cached accesses
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= <rkrcmar@redhat.com>
commit ca3f0874723fad81d0c701b63ae3a17a408d5f25 upstream.
kvm_write_guest_cached() does not mark all written pages as dirty and
code comments in kvm_gfn_to_hva_cache_init() talk about NULL memslot
with cross page accesses. Fix all the easy way.
The check is '<= 1' to have the same result for 'len = 0' cache anywhere
in the page. (nr_pages_needed is 0 on page boundary.)
Fixes: 8f964525a121 ("KVM: Allow cross page reads and writes from cached translations.")
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
Message-Id: <20150408121648.GA3519@potion.brq.redhat.com>
Reviewed-by: Wanpeng Li <wanpeng.li@linux.intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
virt/kvm/kvm_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1549,8 +1549,8 @@ int kvm_gfn_to_hva_cache_init(struct kvm
ghc->generation = slots->generation;
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 = gfn_to_hva_many(ghc->memslot, start_gfn, NULL);
+ if (!kvm_is_error_hva(ghc->hva) && nr_pages_needed <= 1) {
ghc->hva += offset;
} else {
/*
Patches currently in stable-queue which might be from rkrcmar@redhat.com are
queue-3.14/kvm-use-slowpath-for-cross-page-cached-accesses.patch
^ permalink raw reply [flat|nested] 11+ messages in thread