Re: [Qemu-devel] [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack
On Wed, Nov 25, 2015 at 06:20:20PM +0300, Andrey Smetanin wrote: > The SynIC message protocol mandates that the message slot is claimed > by atomically setting message type to something other than HVMSG_NONE. > If another message is to be delivered while the slot is still busy, > message pending flag is asserted to indicate to the guest that the > hypervisor wants to be notified when the slot is released. > > To make sure the protocol works regardless of where the message > sources are (kernel or userspace), clear the pending flag on SINT ACK > notification, and let the message sources compete for the slot again. > > Signed-off-by: Andrey Smetanin > CC: Gleb Natapov > CC: Paolo Bonzini > CC: "K. Y. Srinivasan" > CC: Haiyang Zhang > CC: Vitaly Kuznetsov > CC: Roman Kagan > CC: Denis V. Lunev > CC: qemu-devel@nongnu.org Reviewed-by: Roman Kagan Roman.
Re: [Qemu-devel] [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack
On 26/11/2015 16:53, Andrey Smetanin wrote: >> Then the patches look good, I think. With a testcase I can try them out >> and hopefully merge them for Linux 4.5 / QEMU 2.6. > > Thank you! > > We already have a working Hyper-V SynIC timers kvm-unit-tests test case. > We are going to send appropriate patches seria into kvm-unit-tests git. > > But kvm-unit-tests master now broken: >> make > objcopy -O elf32-i386 x86/memory.elf x86/memory.flat > make: *** No rule to make target 'x86/pku.o', needed by 'x86/pku.elf'. > Stop. > > The problem is in latest commit 3da70799dd3cf1169c4668b4a3fd6f598528b8b9. > > The commit adds 'pku' test case building, but not added any pku.c > implementation file. > > [root@asm-pc kvm-unit-tests]# ls -al x86/pku.c > ls: cannot access x86/pku.c: No such file or directory > > > Could you please fix it ? Done, sorry. Paolo
Re: [Qemu-devel] [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack
On 11/26/2015 05:43 PM, Paolo Bonzini wrote:
On 26/11/2015 10:06, Andrey Smetanin wrote:
On 11/25/2015 08:14 PM, Paolo Bonzini wrote:
On 25/11/2015 17:55, Andrey Smetanin wrote:
+gpa = synic->msg_page & PAGE_MASK;
+page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
+if (is_error_page(page)) {
+vcpu_err(vcpu, "Hyper-V SynIC can't get msg page, gpa
0x%llx\n",
+ gpa);
+return;
+}
+msg_page = kmap_atomic(page);
But the message page is not being pinned, is it?
Actually I don't know anything about pinning.
Is it pinning against page swapping ?
Yes. Unless the page is pinned, kmap_atomic can fail.
kmap_atomic() can't fail for a valid page struct. Does
kvm_vcpu_gfn_to_page() can provide invalid page(swapped page) struct
which may pass is_error_page(page) check but can leads to incorrect
behavior inside kmap_atomic()?
No, you're right. Nevermind, I was confused because I thought you
needed kmap_atomic rather than kmap. Here using kmap_atomic is just an
optimization, so it's okay. (If you needed kmap_atomic, the problem
would have been that kvm_vcpu_gfn_to_page() can sleep).
In patch 7/7 you're also not in atomic context, so kvm_vcpu_gfn_to_page
is okay.
Shouldn't have reviewed the patch when tired. :)
Then the patches look good, I think. With a testcase I can try them out
and hopefully merge them for Linux 4.5 / QEMU 2.6.
Thank you!
We already have a working Hyper-V SynIC timers kvm-unit-tests test case.
We are going to send appropriate patches seria into kvm-unit-tests git.
But kvm-unit-tests master now broken:
> make
objcopy -O elf32-i386 x86/memory.elf x86/memory.flat
make: *** No rule to make target 'x86/pku.o', needed by 'x86/pku.elf'.
Stop.
The problem is in latest commit 3da70799dd3cf1169c4668b4a3fd6f598528b8b9.
The commit adds 'pku' test case building, but not added any pku.c
implementation file.
[root@asm-pc kvm-unit-tests]# ls -al x86/pku.c
ls: cannot access x86/pku.c: No such file or directory
Could you please fix it ?
Paolo
Re: [Qemu-devel] [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack
On 26/11/2015 10:06, Andrey Smetanin wrote:
>
>
> On 11/25/2015 08:14 PM, Paolo Bonzini wrote:
>>
>>
>> On 25/11/2015 17:55, Andrey Smetanin wrote:
+gpa = synic->msg_page & PAGE_MASK;
+page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
+if (is_error_page(page)) {
+vcpu_err(vcpu, "Hyper-V SynIC can't get msg page, gpa
0x%llx\n",
+ gpa);
+return;
+}
+msg_page = kmap_atomic(page);
>>>
>>> But the message page is not being pinned, is it?
>>>
>>> Actually I don't know anything about pinning.
>>> Is it pinning against page swapping ?
>>
>> Yes. Unless the page is pinned, kmap_atomic can fail.
> kmap_atomic() can't fail for a valid page struct. Does
> kvm_vcpu_gfn_to_page() can provide invalid page(swapped page) struct
> which may pass is_error_page(page) check but can leads to incorrect
> behavior inside kmap_atomic()?
No, you're right. Nevermind, I was confused because I thought you
needed kmap_atomic rather than kmap. Here using kmap_atomic is just an
optimization, so it's okay. (If you needed kmap_atomic, the problem
would have been that kvm_vcpu_gfn_to_page() can sleep).
In patch 7/7 you're also not in atomic context, so kvm_vcpu_gfn_to_page
is okay.
Shouldn't have reviewed the patch when tired. :)
Then the patches look good, I think. With a testcase I can try them out
and hopefully merge them for Linux 4.5 / QEMU 2.6.
Paolo
Re: [Qemu-devel] [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack
On 11/25/2015 08:14 PM, Paolo Bonzini wrote:
On 25/11/2015 17:55, Andrey Smetanin wrote:
+gpa = synic->msg_page & PAGE_MASK;
+page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
+if (is_error_page(page)) {
+vcpu_err(vcpu, "Hyper-V SynIC can't get msg page, gpa 0x%llx\n",
+ gpa);
+return;
+}
+msg_page = kmap_atomic(page);
But the message page is not being pinned, is it?
Actually I don't know anything about pinning.
Is it pinning against page swapping ?
Yes. Unless the page is pinned, kmap_atomic can fail.
kmap_atomic() can't fail for a valid page struct. Does
kvm_vcpu_gfn_to_page() can provide invalid page(swapped page) struct
which may pass is_error_page(page) check but can leads to incorrect
behavior inside kmap_atomic()?
However, I don't think that kvm_hv_notify_acked_sint is called from
atomic context. It is only called from apic_set_eoi. Could you just
use kvm_vcpu_write_guest_page?
In this case I can use kvm_vcpu_write_guest_page(), but in the 'PATCH v1
7/7' I do the same page mapping method to sync_cmpxchg() at guest
message page address to exclusively acquire message page slot(see
synic_deliver_msg()). So we need some method to map and access
atomically memory of guest page in KVM. Does any method to pin and map
guest page in kernel exists? Or should we use mlock() for this page in
QEMU part ?
By the way, do you need to do this also in kvm_get_apic_interrupt, for
auto EOI interrupts?
No we don't need this because in case of auto EOI interrupts, if
->msg_pending was set, host will receive HV_X64_MSR_EOM write request
which calls kvm_hv_notify_acked_sint().
Thanks,
Paolo
Could you please clarify and provide an API to use in this case ?
Re: [Qemu-devel] [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack
On 25/11/2015 17:55, Andrey Smetanin wrote:
>>
>> +gpa = synic->msg_page & PAGE_MASK;
>> +page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
>> +if (is_error_page(page)) {
>> +vcpu_err(vcpu, "Hyper-V SynIC can't get msg page, gpa 0x%llx\n",
>> + gpa);
>> +return;
>> +}
>> +msg_page = kmap_atomic(page);
>
> But the message page is not being pinned, is it?
>
> Actually I don't know anything about pinning.
> Is it pinning against page swapping ?
Yes. Unless the page is pinned, kmap_atomic can fail.
However, I don't think that kvm_hv_notify_acked_sint is called from
atomic context. It is only called from apic_set_eoi. Could you just
use kvm_vcpu_write_guest_page?
By the way, do you need to do this also in kvm_get_apic_interrupt, for
auto EOI interrupts?
Thanks,
Paolo
> Could you please clarify and provide an API to use in this case ?
Re: [Qemu-devel] [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack
On 11/25/2015 07:52 PM, Paolo Bonzini wrote:
On 25/11/2015 16:20, Andrey Smetanin wrote:
+static void synic_clear_sint_msg_pending(struct kvm_vcpu_hv_synic *synic,
+ u32 sint)
+{
+ struct kvm_vcpu *vcpu = synic_to_vcpu(synic);
+ struct page *page;
+ gpa_t gpa;
+ struct hv_message *msg;
+ struct hv_message_page *msg_page;
+
+ gpa = synic->msg_page & PAGE_MASK;
+ page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
+ if (is_error_page(page)) {
+ vcpu_err(vcpu, "Hyper-V SynIC can't get msg page, gpa 0x%llx\n",
+gpa);
+ return;
+ }
+ msg_page = kmap_atomic(page);
But the message page is not being pinned, is it?
Actually I don't know anything about pinning.
Is it pinning against page swapping ?
Could you please clarify and provide an API to use in this case ?
Paolo
+ msg = &msg_page->sint_message[sint];
+ msg->header.message_flags.msg_pending = 0;
+
+ kunmap_atomic(msg_page);
+ kvm_release_page_dirty(page);
+ kvm_vcpu_mark_page_dirty(vcpu, gpa >> PAGE_SHIFT);
+}
+
Re: [Qemu-devel] [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack
On 25/11/2015 16:20, Andrey Smetanin wrote:
> +static void synic_clear_sint_msg_pending(struct kvm_vcpu_hv_synic *synic,
> + u32 sint)
> +{
> + struct kvm_vcpu *vcpu = synic_to_vcpu(synic);
> + struct page *page;
> + gpa_t gpa;
> + struct hv_message *msg;
> + struct hv_message_page *msg_page;
> +
> + gpa = synic->msg_page & PAGE_MASK;
> + page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
> + if (is_error_page(page)) {
> + vcpu_err(vcpu, "Hyper-V SynIC can't get msg page, gpa 0x%llx\n",
> + gpa);
> + return;
> + }
> + msg_page = kmap_atomic(page);
But the message page is not being pinned, is it?
Paolo
> + msg = &msg_page->sint_message[sint];
> + msg->header.message_flags.msg_pending = 0;
> +
> + kunmap_atomic(msg_page);
> + kvm_release_page_dirty(page);
> + kvm_vcpu_mark_page_dirty(vcpu, gpa >> PAGE_SHIFT);
> +}
> +
[Qemu-devel] [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack
The SynIC message protocol mandates that the message slot is claimed
by atomically setting message type to something other than HVMSG_NONE.
If another message is to be delivered while the slot is still busy,
message pending flag is asserted to indicate to the guest that the
hypervisor wants to be notified when the slot is released.
To make sure the protocol works regardless of where the message
sources are (kernel or userspace), clear the pending flag on SINT ACK
notification, and let the message sources compete for the slot again.
Signed-off-by: Andrey Smetanin
CC: Gleb Natapov
CC: Paolo Bonzini
CC: "K. Y. Srinivasan"
CC: Haiyang Zhang
CC: Vitaly Kuznetsov
CC: Roman Kagan
CC: Denis V. Lunev
CC: qemu-devel@nongnu.org
---
arch/x86/kvm/hyperv.c| 31 +++
include/linux/kvm_host.h | 2 ++
2 files changed, 33 insertions(+)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 9958926..6412b6b 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -27,6 +27,7 @@
#include "hyperv.h"
#include
+#include
#include
#include
@@ -116,13 +117,43 @@ static struct kvm_vcpu_hv_synic *synic_get(struct kvm
*kvm, u32 vcpu_id)
return (synic->active) ? synic : NULL;
}
+static void synic_clear_sint_msg_pending(struct kvm_vcpu_hv_synic *synic,
+ u32 sint)
+{
+ struct kvm_vcpu *vcpu = synic_to_vcpu(synic);
+ struct page *page;
+ gpa_t gpa;
+ struct hv_message *msg;
+ struct hv_message_page *msg_page;
+
+ gpa = synic->msg_page & PAGE_MASK;
+ page = kvm_vcpu_gfn_to_page(vcpu, gpa >> PAGE_SHIFT);
+ if (is_error_page(page)) {
+ vcpu_err(vcpu, "Hyper-V SynIC can't get msg page, gpa 0x%llx\n",
+gpa);
+ return;
+ }
+ msg_page = kmap_atomic(page);
+
+ msg = &msg_page->sint_message[sint];
+ msg->header.message_flags.msg_pending = 0;
+
+ kunmap_atomic(msg_page);
+ kvm_release_page_dirty(page);
+ kvm_vcpu_mark_page_dirty(vcpu, gpa >> PAGE_SHIFT);
+}
+
static void kvm_hv_notify_acked_sint(struct kvm_vcpu *vcpu, u32 sint)
{
struct kvm *kvm = vcpu->kvm;
+ struct kvm_vcpu_hv_synic *synic = vcpu_to_synic(vcpu);
int gsi, idx;
vcpu_debug(vcpu, "Hyper-V SynIC acked sint %d\n", sint);
+ if (synic->msg_page & HV_SYNIC_SIMP_ENABLE)
+ synic_clear_sint_msg_pending(synic, sint);
+
idx = srcu_read_lock(&kvm->irq_srcu);
gsi = atomic_read(&vcpu_to_synic(vcpu)->sint_to_gsi[sint]);
if (gsi != -1)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 2911919..9b64c8c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -450,6 +450,8 @@ struct kvm {
#define vcpu_debug(vcpu, fmt, ...) \
kvm_debug("vcpu%i " fmt, (vcpu)->vcpu_id, ## __VA_ARGS__)
+#define vcpu_err(vcpu, fmt, ...) \
+ kvm_err("vcpu%i " fmt, (vcpu)->vcpu_id, ## __VA_ARGS__)
static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm *kvm, int i)
{
--
2.4.3
