From: Andrey Smetanin <asmetanin@virtuozzo.com>
To: Paolo Bonzini <pbonzini@redhat.com>, <kvm@vger.kernel.org>
Cc: Gleb Natapov <gleb@kernel.org>,
"K. Y. Srinivasan" <kys@microsoft.com>,
Haiyang Zhang <haiyangz@microsoft.com>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
Roman Kagan <rkagan@virtuozzo.com>,
"Denis V. Lunev" <den@openvz.org>, <qemu-devel@nongnu.org>
Subject: Re: [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack
Date: Thu, 26 Nov 2015 18:53:36 +0300 [thread overview]
Message-ID: <56572B00.6070208@virtuozzo.com> (raw)
In-Reply-To: <56571A7F.6080504@redhat.com>
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
>
WARNING: multiple messages have this Message-ID (diff)
From: Andrey Smetanin <asmetanin@virtuozzo.com>
To: Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org
Cc: Gleb Natapov <gleb@kernel.org>,
Haiyang Zhang <haiyangz@microsoft.com>,
qemu-devel@nongnu.org, Roman Kagan <rkagan@virtuozzo.com>,
"Denis V. Lunev" <den@openvz.org>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
"K. Y. Srinivasan" <kys@microsoft.com>
Subject: Re: [Qemu-devel] [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack
Date: Thu, 26 Nov 2015 18:53:36 +0300 [thread overview]
Message-ID: <56572B00.6070208@virtuozzo.com> (raw)
In-Reply-To: <56571A7F.6080504@redhat.com>
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
>
next prev parent reply other threads:[~2015-11-26 15:54 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-25 15:20 [PATCH v1 0/7] KVM: Hyper-V SynIC timers Andrey Smetanin
2015-11-25 15:20 ` [Qemu-devel] " Andrey Smetanin
2015-11-25 15:20 ` [PATCH v1 1/7] drivers/hv: Move HV_SYNIC_STIMER_COUNT into Hyper-V UAPI x86 header Andrey Smetanin
2015-11-25 15:20 ` [Qemu-devel] " Andrey Smetanin
2015-11-27 17:27 ` KY Srinivasan
2015-11-27 17:27 ` [Qemu-devel] " KY Srinivasan
2015-11-25 15:20 ` [PATCH v1 2/7] drivers/hv: Move struct hv_message into UAPI Hyper-V " Andrey Smetanin
2015-11-25 15:20 ` [Qemu-devel] " Andrey Smetanin
2015-11-27 9:34 ` Paolo Bonzini
2015-11-27 9:34 ` [Qemu-devel] " Paolo Bonzini
2015-11-27 11:21 ` Andrey Smetanin
2015-11-27 11:21 ` [Qemu-devel] " Andrey Smetanin
2015-11-27 17:34 ` KY Srinivasan
2015-11-27 17:34 ` [Qemu-devel] " KY Srinivasan
2015-11-25 15:20 ` [PATCH v1 3/7] kvm/x86: Rearrange func's declarations inside Hyper-V header Andrey Smetanin
2015-11-25 15:20 ` [Qemu-devel] " Andrey Smetanin
2015-11-25 15:20 ` [PATCH v1 4/7] kvm/x86: Added Hyper-V vcpu_to_hv_vcpu()/hv_vcpu_to_vcpu() helpers Andrey Smetanin
2015-11-25 15:20 ` [Qemu-devel] " Andrey Smetanin
2015-11-25 15:20 ` [PATCH v1 5/7] kvm/x86: Hyper-V internal helper to read MSR HV_X64_MSR_TIME_REF_COUNT Andrey Smetanin
2015-11-25 15:20 ` [Qemu-devel] " Andrey Smetanin
2015-11-25 15:20 ` [PATCH v1 6/7] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack Andrey Smetanin
2015-11-25 15:20 ` [Qemu-devel] " Andrey Smetanin
2015-11-25 16:52 ` Paolo Bonzini
2015-11-25 16:52 ` [Qemu-devel] " Paolo Bonzini
2015-11-25 16:55 ` Andrey Smetanin
2015-11-25 16:55 ` [Qemu-devel] " Andrey Smetanin
2015-11-25 17:14 ` Paolo Bonzini
2015-11-25 17:14 ` [Qemu-devel] " Paolo Bonzini
2015-11-26 9:06 ` Andrey Smetanin
2015-11-26 9:06 ` [Qemu-devel] " Andrey Smetanin
2015-11-26 14:43 ` Paolo Bonzini
2015-11-26 14:43 ` [Qemu-devel] " Paolo Bonzini
2015-11-26 15:53 ` Andrey Smetanin [this message]
2015-11-26 15:53 ` Andrey Smetanin
2015-11-26 15:56 ` Paolo Bonzini
2015-11-26 15:56 ` [Qemu-devel] " Paolo Bonzini
2015-11-27 8:16 ` Roman Kagan
2015-11-27 8:16 ` [Qemu-devel] " Roman Kagan
2015-11-25 15:20 ` [PATCH v1 7/7] kvm/x86: Hyper-V SynIC timers Andrey Smetanin
2015-11-25 15:20 ` [Qemu-devel] " Andrey Smetanin
2015-11-27 8:12 ` Roman Kagan
2015-11-27 8:12 ` [Qemu-devel] " Roman Kagan
2015-11-27 10:49 ` Paolo Bonzini
2015-11-27 10:49 ` [Qemu-devel] " Paolo Bonzini
2015-11-27 11:24 ` Andrey Smetanin
2015-11-27 11:24 ` [Qemu-devel] " Andrey Smetanin
2015-11-30 12:17 ` Roman Kagan
2015-11-30 12:17 ` [Qemu-devel] " Roman Kagan
2015-11-26 5:28 ` [PATCH v1 0/7] KVM: " Wanpeng Li
2015-11-26 5:28 ` [Qemu-devel] " Wanpeng Li
2015-11-26 8:34 ` Andrey Smetanin
2015-11-26 8:34 ` [Qemu-devel] " Andrey Smetanin
2015-11-26 9:03 ` Wanpeng Li
2015-11-26 9:03 ` [Qemu-devel] " Wanpeng Li
2015-12-01 10:12 ` Wanpeng Li
2015-12-01 10:12 ` [Qemu-devel] " Wanpeng Li
2015-12-01 10:28 ` Denis V. Lunev
2015-12-01 10:28 ` [Qemu-devel] " Denis V. Lunev
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=56572B00.6070208@virtuozzo.com \
--to=asmetanin@virtuozzo.com \
--cc=den@openvz.org \
--cc=gleb@kernel.org \
--cc=haiyangz@microsoft.com \
--cc=kvm@vger.kernel.org \
--cc=kys@microsoft.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=rkagan@virtuozzo.com \
--cc=vkuznets@redhat.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 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.