From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrey Smetanin Subject: Re: [PATCH v4 5/5] kvm/x86: Hyper-V kvm exit Date: Mon, 21 Dec 2015 15:59:49 +0300 Message-ID: <5677F7C5.5040404@virtuozzo.com> References: <1447158995-21919-1-git-send-email-asmetanin@virtuozzo.com> <1447158995-21919-6-git-send-email-asmetanin@virtuozzo.com> <01cc01d139a7$7baad550$73007ff0$@samsung.com> <56742DF7.9000902@redhat.com> <20151218183911.GF3050@rkaganb.sw.ru> Reply-To: Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit To: Roman Kagan , Peter Hornyack , Paolo Bonzini , Pavel Fedin , kvm list , "Denis V. Lunev" , Gleb Natapov , Return-path: Received: from relay.parallels.com ([195.214.232.42]:42568 "EHLO relay.parallels.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751123AbbLUNAR (ORCPT ); Mon, 21 Dec 2015 08:00:17 -0500 In-Reply-To: <20151218183911.GF3050@rkaganb.sw.ru> Sender: kvm-owner@vger.kernel.org List-ID: On 12/18/2015 09:39 PM, Roman Kagan wrote: > On Fri, Dec 18, 2015 at 10:10:11AM -0800, Peter Hornyack wrote: >> On Fri, Dec 18, 2015 at 8:01 AM, Paolo Bonzini wrote: >>> On 18/12/2015 16:19, Pavel Fedin wrote: >>>> As far as i understand this code, KVM_EXIT_HYPERV is called when one >>>> of three MSRs are accessed. But, shouldn't we have implemented >>>> instead something more generic, like KVM_EXIT_REG_IO, which would >>>> work similar to KVM_EXIT_PIO or KVM_EXIT_MMIO, but carry register >>>> code and value? >>> >>> Yes, we considered that. There were actually patches for this as well. >>> However, in this case the register is still emulated in the kernel, and >>> userspace just gets informed of the new value. >> >> On brief inspection of Andrey's patch (I have not been following >> closely) it looks like the kvm_hyperv_exit struct that's returned to >> userspace contains more data (control, evt_page, and msg_page fields) >> than simply the value of the MSR, so would the desired SynIC exit fit >> into a general-purpose exit for MSR emulation? > > Frankly I'm struggling trying to recall why we implemented it this way. > Actually all three fields are the values of respective MSRs and I don't > see any necessity to pass all three at the same time when any of them > gets updated. The patch for QEMU adds an exit handler which processes > the fields individually, so I have a strong suspicion that union was > meant here rather than struct. > > I hope Andrey will help to shed some light on that when he's back in the > office on Monday; meanwhile I think this peculiarity can be ignored. Hello! We have implemented Hyper-V related Vcpu exit not only for Hyper-V SynIC MSR's changes but also to provide future interface to transfer guest VMBus hypercalls parameters into QEMU. Yes, we can use KVM_EXIT_REG_IO/MSR_IO for Hyper-V SynIC MSRS's changes and can even use only one MSR value . So union inside struct kvm_hyperv_exit is excessive. But we still need Vcpu exit to handle VMBus hypercalls by QEMU to emulate VMBus devices inside QEMU. And currently we are going to extend struct kvm_hyperv_exit to store Hyper-V VMBus hypercall parameters. SynIC MSR's changes could be replaced by KVM_EXIT_REG_IO/MSR_IO but could we replace Hyper-V VMBus hypercall and it's parameters by KVM_EXIT_REG_IO/MSR_IO too? > > Roman. >