RE: [PATCH v4 5/5] kvm/x86: Hyper-V kvm exit

[Pavel Fedin]

 Hello!

> 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.

 Hm... Hypercalls, you say?
 We already have KVM_EXIT_HYPERCALL. Documentation says it's currently unused. 
Is it a leftover from ia64 KVM? Could we reuse it for
the purpose?

> but could we replace Hyper-V VMBus hypercall and it's parameters
> by KVM_EXIT_REG_IO/MSR_IO too?

 It depends. Can i read about these hypercalls somewhere? Is there any 
documentation?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/5] kvm/x86: Hyper-V kvm exit

[Andrey Smetanin]

On 12/21/2015 04:28 PM, Pavel Fedin wrote:

  Hello!


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.


  Hm... Hypercalls, you say?
  We already have KVM_EXIT_HYPERCALL. Documentation says it's currently unused. 
Is it a leftover from ia64 KVM? Could we reuse it for
the purpose?


but could we replace Hyper-V VMBus hypercall and it's parameters
by KVM_EXIT_REG_IO/MSR_IO too?


  It depends. Can i read about these hypercalls somewhere? Is there any 
documentation?
I don't know about a documentation, but you can look at the code of 
Hyper-V hypercall handling inside KVM:


https://github.com/torvalds/linux/blob/master/arch/x86/kvm/hyperv.c#L346

The code simply decodes hypercall parameters from vcpu registers then 
handle hypercall code in switch and encode return code inside vcpu 
registers. Probably encode and decode of hypercall parameters/return 
code can be done in QEMU so we need only some exit with parameter that 
this is Hyper-V hypercall and probably KVM_EXIT_HYPERCALL is good for it.


But KVM_EXIT_HYPERCALL is not used inside KVM/QEMU so requires
implementation.



Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia



--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/5] kvm/x86: Hyper-V kvm exit

[Andrey Smetanin]

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.


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v4 5/5] kvm/x86: Hyper-V kvm exit

[Pavel Fedin]

 Hello!

> >   It depends. Can i read about these hypercalls somewhere? Is there any 
> > documentation?
> I don't know about a documentation, but you can look at the code of
> Hyper-V hypercall handling inside KVM:
> 
> https://github.com/torvalds/linux/blob/master/arch/x86/kvm/hyperv.c#L346

 Aha, i see, so vmmcall CPU instruction is employed. Well, i believe this very 
well fits into the sematics of KVM_EXIT_HYPERCALL,
because it's a true hypercall.

> The code simply decodes hypercall parameters from vcpu registers then
> handle hypercall code in switch and encode return code inside vcpu
> registers. Probably encode and decode of hypercall parameters/return
> code can be done in QEMU so we need only some exit with parameter that
> this is Hyper-V hypercall and probably KVM_EXIT_HYPERCALL is good for it.

 Or you could even reuse the whole structure, it has all you need:

__u64 nr;   /* Reserved for x86, other 
architectures can use it, for example ARM "hvc #nr" */
__u64 args[6];  /* rax, rbx, rcx, rdx, rdi, rsi */
__u64 ret;
__u32 longmode; /* longmode; other architectures (like 
ARM64) can also make sense of it */

 Or you could put in struct kvm_regs instead of args and ret, and allow the 
userspace to manipulate it.

> But KVM_EXIT_HYPERCALL is not used inside KVM/QEMU so requires
> implementation.

 I guess your hypercalls to be introduced using KVM_EXIT_HYPERV are also not 
used inside qemu so require implementation :)

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v4 5/5] kvm/x86: Hyper-V kvm exit

[Pavel Fedin]

 Hello!

 Replying to everything in one message.

> > 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.

 Ah, i missed them, what a pity. There are lots of patches, i don't review them 
all. Actually i have noticed the change only after
it appeared in linux-next.

>  However, in this case the register is still emulated in the kernel, and
> userspace just gets informed of the new value.

 I see, but this doesn't change the semantic. All we need to do is to tell the 
userland that "register has been written".
Additionally to this we could do whatever we want, including caching the data 
in kernel, using it in kernel, and processing reads in
kernel.

> If we do get that, we will just rename KVM_EXIT_HYPERV to
> KVM_EXIT_MSR_ACCESS, and KVM_EXIT_HYPERV_SYNIC to
> KVM_EXIT_MSR_HYPERV_SYNIC, and struct kvm_hyperv_exit to kvm_msr_exit.

 Actually, i see this in more generic way, something like:

/* KVM_EXIT_REG_ACCESS */
struct {
__u64 reg;
__u64 data;
__u8  is_write;
} mmio;
 
 'data' and 'is_write' are self-explanatory, 'reg' would be generalized 
register code, the same as used for KVM_(GET|SET)_ONE_REG:
 - for ARM64: ARM64_SYS_REG(op0, op1, crn, crm, op2) - see
http://lxr.free-electrons.com/source/arch/arm64/include/uapi/asm/kvm.h#L189
 - for x86  : to be defined (i know, we don't use ..._ONE_REG operations here 
yet), like X86_MSR_REG(id), where the macro itself is:

#define X86_MSR_REG(id) (KVM_REG_X86 | KVM_REG_X86_MSR | 
KVM_REG_SIZE_U64 | (id))

 - for other architectures: to be defined in a similar way, once needed.

> 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?

 I have looked at the code too, and these three fields are nothing more than 
values of respective MSR's:

case HV_X64_MSR_SCONTROL:
synic->control = data;
if (!host)
synic_exit(synic, msr);
break;



case HV_X64_MSR_SIEFP:
if (data & HV_SYNIC_SIEFP_ENABLE)
if (kvm_clear_guest(vcpu->kvm,
data & PAGE_MASK, PAGE_SIZE)) {
ret = 1;
break;
}
synic->evt_page = data;
if (!host)
synic_exit(synic, msr);
break;
case HV_X64_MSR_SIMP:
if (data & HV_SYNIC_SIMP_ENABLE)
if (kvm_clear_guest(vcpu->kvm,
data & PAGE_MASK, PAGE_SIZE)) {
ret = 1;
break;
}
synic->msg_page = data;
if (!host)
synic_exit(synic, msr);
break;

 So, every time one of these thee MSRs is written, we get a vmexit with values 
of all three registers, and that's all. We could
easily have 'synic_exit(synic, msr, data)' in all three cases, and i think the 
userspace could easily deal with proposed
KVM_EXIT_REG_ACCESS, just cache these values internally if needed.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/5] kvm/x86: Hyper-V kvm exit

[Denis V. Lunev]

On 12/18/2015 06:19 PM, Pavel Fedin wrote:

  Hello!

  I realize that it's perhaps too late, because patches are already on 
Linux-next, but i have one concern... May be it's not too
late...

  I dislike implementing architecture-dependent exit code where we could 
implement an architecture-independent one.

  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?

  This would allow us to solve the same task which we have done here, but this 
solution would be reusable for other devices and other
archirectures. What if in future we have more system registers to emulate in 
userspace?

  I write this because at one point i suggested similar thing for ARM64 (but i 
never actually wrote it), to emulate physical CP15
timer. And it would require exactly the same capability - process some trapped 
system register accesses in userspace.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia



we have discussed this AFAIR. HyperV guest implementation
is available in Linux kernel and thus technically we can have
this stuff on any platform.

Den
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/5] kvm/x86: Hyper-V kvm exit

[Paolo Bonzini]

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.

> This would allow us to solve the same task which we have done here,
> but this solution would be reusable for other devices and other 
> archirectures. What if in future we have more system registers to
> emulate in userspace?

If we do get that, we will just rename KVM_EXIT_HYPERV to
KVM_EXIT_MSR_ACCESS, and KVM_EXIT_HYPERV_SYNIC to
KVM_EXIT_MSR_HYPERV_SYNIC, and struct kvm_hyperv_exit to kvm_msr_exit.

Actually, we can do it now.  What do you guys think?  Peter?  I might
even be convinced to document the capability (in Documentation/ and
uapi/) even if the upstream kernel doesn't provide it.  We still have a
lot of time until 4.5 is out, it can be done after the merge window even.

Paolo

> I write this because at one point i suggested similar thing for ARM64
> (but i never actually wrote it), to emulate physical CP15 timer. And
> it would require exactly the same capability - process some trapped
> system register accesses in userspace.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH v4 5/5] kvm/x86: Hyper-V kvm exit

[Pavel Fedin]

 Hello!

 I realize that it's perhaps too late, because patches are already on 
Linux-next, but i have one concern... May be it's not too
late...

 I dislike implementing architecture-dependent exit code where we could 
implement an architecture-independent one.

 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?

 This would allow us to solve the same task which we have done here, but this 
solution would be reusable for other devices and other
archirectures. What if in future we have more system registers to emulate in 
userspace?

 I write this because at one point i suggested similar thing for ARM64 (but i 
never actually wrote it), to emulate physical CP15
timer. And it would require exactly the same capability - process some trapped 
system register accesses in userspace.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/5] kvm/x86: Hyper-V kvm exit

[Peter Hornyack]

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?

>> This would allow us to solve the same task which we have done here,
>> but this solution would be reusable for other devices and other
>> archirectures. What if in future we have more system registers to
>> emulate in userspace?
>
> If we do get that, we will just rename KVM_EXIT_HYPERV to
> KVM_EXIT_MSR_ACCESS, and KVM_EXIT_HYPERV_SYNIC to
> KVM_EXIT_MSR_HYPERV_SYNIC, and struct kvm_hyperv_exit to kvm_msr_exit.
>
> Actually, we can do it now.  What do you guys think?  Peter?  I might
> even be convinced to document the capability (in Documentation/ and
> uapi/) even if the upstream kernel doesn't provide it.  We still have a
> lot of time until 4.5 is out, it can be done after the merge window even.

I will update and re-send that patchset for discussion.

>
> Paolo
>
>> I write this because at one point i suggested similar thing for ARM64
>> (but i never actually wrote it), to emulate physical CP15 timer. And
>> it would require exactly the same capability - process some trapped
>> system register accesses in userspace.

Peter
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/5] kvm/x86: Hyper-V kvm exit

['Roman Kagan']

On Fri, Dec 18, 2015 at 06:19:25PM +0300, Pavel Fedin wrote:
>  I dislike implementing architecture-dependent exit code where we could 
> implement an architecture-independent one.
> 
>  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?
> 
>  This would allow us to solve the same task which we have done here, but this 
> solution would be reusable for other devices and other
> archirectures. What if in future we have more system registers to emulate in 
> userspace?
> 
>  I write this because at one point i suggested similar thing for ARM64 (but i 
> never actually wrote it), to emulate physical CP15
> timer. And it would require exactly the same capability - process some 
> trapped system register accesses in userspace.

Note that, as Paolo pointed out, in case of HyperV the value of the
register is of interest not only to the userspace but to the KVM, too.
Otherwise I don't see off hand why a generic infrastructure wouldn't fit
in our usecase.

Roman.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/5] kvm/x86: Hyper-V kvm exit

['Roman Kagan']

On Fri, Dec 18, 2015 at 05:01:59PM +0100, 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.
> 
> > This would allow us to solve the same task which we have done here,
> > but this solution would be reusable for other devices and other 
> > archirectures. What if in future we have more system registers to
> > emulate in userspace?
> 
> If we do get that, we will just rename KVM_EXIT_HYPERV to
> KVM_EXIT_MSR_ACCESS, and KVM_EXIT_HYPERV_SYNIC to
> KVM_EXIT_MSR_HYPERV_SYNIC, and struct kvm_hyperv_exit to kvm_msr_exit.

A generic implemenation will probably just convey (msr#, value) pair,
and KVM_EXIT_MSR_HYPERV_SYNIC wouldn't be needed at all.

I don't immediately see why it wouldn't work for us; we'd have reused
the infrastructure if it existed when we started our work.  I didn't see
Peter's patches yet; maybe we can come up with an interim solution to
fit in the merge window but expose a sufficiently generic API.

Roman.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/5] kvm/x86: Hyper-V kvm exit

[Roman Kagan]

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.

Roman.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 5/5] kvm/x86: Hyper-V kvm exit

[Paolo Bonzini]

On 18/12/2015 19:10, Peter Hornyack wrote:
> 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?

This would be a special case that is used even if the hyperv MSRs are
emulated in the kernel.  Other exit subcodes than
KVM_EXIT_(MSR_)HYPERV_SYNIC would be used for your usecase of for MSR
emulation in userspace.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html