Re: [PATCH v2] x86: svm: use kvm_register_write()/read()

2015-02-20 Thread Borislav Petkov
On Fri, Feb 20, 2015 at 04:02:10PM -0600, Joel Schopp wrote:
> From: David Kaplan 
> 
> KVM has nice wrappers to access the register values, clean up a few places
> that should use them but currently do not.
> 
> Signed-off-by: David Kaplan 
> [forward port and testing]
> Signed-off-by: Joel Schopp 

Looks good.

Acked-by: Borislav Petkov 

> ---
>  arch/x86/kvm/svm.c |   19 +--
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index d319e0c..a7d88e4 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c

...

> @@ -3142,8 +3142,8 @@ static int rdmsr_interception(struct vcpu_svm *svm)
>   } else {
>   trace_kvm_msr_read(ecx, data);
>  
> - svm->vcpu.arch.regs[VCPU_REGS_RAX] = data & 0x;
> - svm->vcpu.arch.regs[VCPU_REGS_RDX] = data >> 32;
> + kvm_register_write(&svm->vcpu, VCPU_REGS_RAX, data & 
> 0x);
> + kvm_register_write(&svm->vcpu, VCPU_REGS_RDX, data >> 32);

Right, kvm is missing kvm_write_edx_eax() in addition to the read
variant. Someone might want to do a patch and such, ^^hint hint^^...

:-)

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--
--
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


[PATCH] x86: svm: don't intercept CR0 TS or MP bit write

2015-02-20 Thread Joel Schopp
From: David Kaplan 

Reduce the number of exits by avoiding exiting when the guest writes TS or MP
bits of CR0.  INTERCEPT_CR0_WRITE intercepts all writes to CR0 including TS and
MP bits. It intercepts these even if INTERCEPT_SELECTIVE_CR0 is set.  What we
should be doing is setting INTERCEPT_SELECTIVE_CR0 and not setting
INTERCEPT_CR0_WRITE.

Signed-off-by: David Kaplan 
[added remove of clr_cr_intercept in init_vmcb, fixed check in handle_exit,
added emulation on interception back in, forward ported, tested]
Signed-off-by: Joel Schopp 
---
 arch/x86/kvm/svm.c |   13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index d319e0c..55822e5 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1093,7 +1093,6 @@ static void init_vmcb(struct vcpu_svm *svm)
set_cr_intercept(svm, INTERCEPT_CR0_READ);
set_cr_intercept(svm, INTERCEPT_CR3_READ);
set_cr_intercept(svm, INTERCEPT_CR4_READ);
-   set_cr_intercept(svm, INTERCEPT_CR0_WRITE);
set_cr_intercept(svm, INTERCEPT_CR3_WRITE);
set_cr_intercept(svm, INTERCEPT_CR4_WRITE);
set_cr_intercept(svm, INTERCEPT_CR8_WRITE);
@@ -1539,10 +1538,8 @@ static void update_cr0_intercept(struct vcpu_svm *svm)
 
if (gcr0 == *hcr0 && svm->vcpu.fpu_active) {
clr_cr_intercept(svm, INTERCEPT_CR0_READ);
-   clr_cr_intercept(svm, INTERCEPT_CR0_WRITE);
} else {
set_cr_intercept(svm, INTERCEPT_CR0_READ);
-   set_cr_intercept(svm, INTERCEPT_CR0_WRITE);
}
 }
 
@@ -2940,7 +2937,11 @@ static int cr_interception(struct vcpu_svm *svm)
return emulate_on_interception(svm);
 
reg = svm->vmcb->control.exit_info_1 & SVM_EXITINFO_REG_MASK;
-   cr = svm->vmcb->control.exit_code - SVM_EXIT_READ_CR0;
+
+   if (svm->vmcb->control.exit_code == SVM_EXIT_CR0_SEL_WRITE)
+  cr = 16;
+   else
+  cr = svm->vmcb->control.exit_code - SVM_EXIT_READ_CR0;
 
err = 0;
if (cr >= 16) { /* mov to cr */
@@ -3325,7 +3326,7 @@ static int (*const svm_exit_handlers[])(struct vcpu_svm 
*svm) = {
[SVM_EXIT_READ_CR3] = cr_interception,
[SVM_EXIT_READ_CR4] = cr_interception,
[SVM_EXIT_READ_CR8] = cr_interception,
-   [SVM_EXIT_CR0_SEL_WRITE]= emulate_on_interception,
+   [SVM_EXIT_CR0_SEL_WRITE]= cr_interception,
[SVM_EXIT_WRITE_CR0]= cr_interception,
[SVM_EXIT_WRITE_CR3]= cr_interception,
[SVM_EXIT_WRITE_CR4]= cr_interception,
@@ -3502,7 +3503,7 @@ static int handle_exit(struct kvm_vcpu *vcpu)
struct kvm_run *kvm_run = vcpu->run;
u32 exit_code = svm->vmcb->control.exit_code;
 
-   if (!is_cr_intercept(svm, INTERCEPT_CR0_WRITE))
+   if (!is_cr_intercept(svm, INTERCEPT_SELECTIVE_CR0))
vcpu->arch.cr0 = svm->vmcb->save.cr0;
if (npt_enabled)
vcpu->arch.cr3 = svm->vmcb->save.cr3;

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


[PATCH v2] x86: svm: use kvm_register_write()/read()

2015-02-20 Thread Joel Schopp
From: David Kaplan 

KVM has nice wrappers to access the register values, clean up a few places
that should use them but currently do not.

Signed-off-by: David Kaplan 
[forward port and testing]
Signed-off-by: Joel Schopp 
---
 arch/x86/kvm/svm.c |   19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index d319e0c..a7d88e4 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2757,11 +2757,11 @@ static int invlpga_interception(struct vcpu_svm *svm)
 {
struct kvm_vcpu *vcpu = &svm->vcpu;
 
-   trace_kvm_invlpga(svm->vmcb->save.rip, vcpu->arch.regs[VCPU_REGS_RCX],
- vcpu->arch.regs[VCPU_REGS_RAX]);
+   trace_kvm_invlpga(svm->vmcb->save.rip, kvm_register_read(&svm->vcpu, 
VCPU_REGS_RCX),
+ kvm_register_read(&svm->vcpu, VCPU_REGS_RAX));
 
/* Let's treat INVLPGA the same as INVLPG (can be optimized!) */
-   kvm_mmu_invlpg(vcpu, vcpu->arch.regs[VCPU_REGS_RAX]);
+   kvm_mmu_invlpg(vcpu, kvm_register_read(&svm->vcpu, VCPU_REGS_RAX));
 
svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
skip_emulated_instruction(&svm->vcpu);
@@ -2770,7 +2770,7 @@ static int invlpga_interception(struct vcpu_svm *svm)
 
 static int skinit_interception(struct vcpu_svm *svm)
 {
-   trace_kvm_skinit(svm->vmcb->save.rip, 
svm->vcpu.arch.regs[VCPU_REGS_RAX]);
+   trace_kvm_skinit(svm->vmcb->save.rip, kvm_register_read(&svm->vcpu, 
VCPU_REGS_RAX));
 
kvm_queue_exception(&svm->vcpu, UD_VECTOR);
return 1;
@@ -3133,7 +3133,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, unsigned 
ecx, u64 *data)
 
 static int rdmsr_interception(struct vcpu_svm *svm)
 {
-   u32 ecx = svm->vcpu.arch.regs[VCPU_REGS_RCX];
+   u32 ecx = kvm_register_read(&svm->vcpu, VCPU_REGS_RCX);
u64 data;
 
if (svm_get_msr(&svm->vcpu, ecx, &data)) {
@@ -3142,8 +3142,8 @@ static int rdmsr_interception(struct vcpu_svm *svm)
} else {
trace_kvm_msr_read(ecx, data);
 
-   svm->vcpu.arch.regs[VCPU_REGS_RAX] = data & 0x;
-   svm->vcpu.arch.regs[VCPU_REGS_RDX] = data >> 32;
+   kvm_register_write(&svm->vcpu, VCPU_REGS_RAX, data & 
0x);
+   kvm_register_write(&svm->vcpu, VCPU_REGS_RDX, data >> 32);
svm->next_rip = kvm_rip_read(&svm->vcpu) + 2;
skip_emulated_instruction(&svm->vcpu);
}
@@ -3246,9 +3246,8 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct 
msr_data *msr)
 static int wrmsr_interception(struct vcpu_svm *svm)
 {
struct msr_data msr;
-   u32 ecx = svm->vcpu.arch.regs[VCPU_REGS_RCX];
-   u64 data = (svm->vcpu.arch.regs[VCPU_REGS_RAX] & -1u)
-   | ((u64)(svm->vcpu.arch.regs[VCPU_REGS_RDX] & -1u) << 32);
+   u32 ecx = kvm_register_read(&svm->vcpu, VCPU_REGS_RCX);
+   u64 data = kvm_read_edx_eax(&svm->vcpu);
 
msr.data = data;
msr.index = ecx;

--
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] x86: svm: use kvm_register_write()/read()

2015-02-20 Thread Joel Schopp

On 02/20/2015 02:54 PM, Borislav Petkov wrote:
> On Fri, Feb 20, 2015 at 12:39:40PM -0600, Joel Schopp wrote:
>> KVM has nice wrappers to access the register values, clean up a few places
>> that should use them but currently do not.
>>
>> Signed-off-by:David Kaplan 
>> Signed-off-by:Joel Schopp 
> This SOB chain looks strange. If David is the author, you want to have
> him in From: at the beginning of the patch.
>
> Stuff you did ontop should be in []-braces before your SOB, like this:
Will resend with From: line and braces for clarification.

>
> Signed-off-by: David Kaplan 
> [ Did this and that to patch. ]
> Signed-off-by: Joel Schopp 
>

--
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] x86: svm: use kvm_register_write()/read()

2015-02-20 Thread Borislav Petkov
On Fri, Feb 20, 2015 at 12:39:40PM -0600, Joel Schopp wrote:
> KVM has nice wrappers to access the register values, clean up a few places
> that should use them but currently do not.
> 
> Signed-off-by:David Kaplan 
> Signed-off-by:Joel Schopp 

This SOB chain looks strange. If David is the author, you want to have
him in From: at the beginning of the patch.

Stuff you did ontop should be in []-braces before your SOB, like this:

Signed-off-by: David Kaplan 
[ Did this and that to patch. ]
Signed-off-by: Joel Schopp 

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--
--
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: [Qemu-devel] [RFC PATCH v2 04/15] cpu-model/s390: Introduce S390 CPU models

2015-02-20 Thread Alexander Graf


On 20.02.15 20:43, Michael Mueller wrote:
> On Fri, 20 Feb 2015 18:50:20 +0100
> Alexander Graf  wrote:
> 
>>
>>
>>
>>> Am 20.02.2015 um 18:37 schrieb Michael Mueller :
>>>
>>> On Fri, 20 Feb 2015 17:57:52 +0100
>>> Alexander Graf  wrote:
>>>
 Because all CPUs we have in our list only expose 128 bits?
>>>
>>> Here a STFLE result on a EC12 GA2, already more than 128 bits... Is that 
>>> model on the list?
>>
>> If that model has 3 elements, yes, the array should span 3.
>>
>> I hope it's in the list. Every model wecare about should be, no?
>>
> 
> On my list? Yes!
> 
>>>
>>> [mimu@p57lp59 s390xfac]$ ./s390xfac -b
>>> fac[0] = 0xfbfbfcfff840
>>> fac[1] = 0xffde
>>> fac[2] = 0x1800

> I want to have this independent from a future machine of the z/Arch. The 
> kernel stores the
> full facility set, KVM does and there is no good reason for QEMU not to 
> do. If other
> accelerators decide to just implement 64 or 128 bits of facilities that's 
> ok...  

 So you want to support CPUs that are not part of the list?
>>>
>>> The architecture at least defines more than 2 or 3. Do you want me to limit 
>>> it to an arbitrary
>>> size?. Only in QEMU or also in the KVM interface?
>>
>> Only internally in QEMU. The kvm interface should definitely be as big as 
>> the spec allows!
> 
> Right, now we're on the same page again. That can be taken in consideration. 
> ... Although it's
> just and optimization. :-)

Yeah. You could also consider using the QEMU built-in bitmap type and
functions and just convert from there. That would give you native
support for bit values > 64.


Alex
--
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: [nVMX] With 3.20.0-0.rc0.git5.1 on L0, booting L2 guest results in L1 *rebooting*

2015-02-20 Thread Kashyap Chamarthy
On Fri, Feb 20, 2015 at 05:14:15PM +0100, Radim Krčmář wrote:
> 2015-02-19 23:28+0100, Kashyap Chamarthy:
> > On Thu, Feb 19, 2015 at 10:10:11PM +0100, Kashyap Chamarthy wrote:
> > > On Thu, Feb 19, 2015 at 05:02:22PM +0100, Radim Krčmář wrote:

[. . .]

> > Then, I did another test:
> > 
> >   - Rebooted into Kernel 3.20.0-0.rc0.git5.1.fc23.x86_64 on physical
> > host (L0).
> >   - In L1, checked out the KVM tree, applied your patch and built
> > Kernel[*] from the current KVM tree and booted into the newly built
> > one, here too, I'm thrown into a dracut shell
> 
> Weird, but considering that boot fails on L0 as well, I think it that
> basing off a different commit could help ...

What I missed to do was to build initramfs:

$ cd /boot
$ dracut initramfs-3.19.0+.img 3.19.0+ --force

Then I can boot. However, networking was hosed due to this bug[1] in
`dhclient` (Andrea Arcangeli said it's fixed for him in newest Kernels,
but unfortunately it's still not fixed for me as I noted in the bug.

Anyway, for the nVMX bug in question, I actually built a Fedora scratch
Kernel build[2], with your fix, which was successful[3]. I will test
with it once I get the networking fixed on the physical machine,
hopefully, early next week.

> > [*] Exactly, I built it this way:
> > 
> >   # Clone the tree
> >   $ git://git.kernel.org/pub/scm/virt/kvm/kvm.git
> > 
> >   # Make a new branch:
> >   $ git checkout -b nvmx_test
> >   $ git describe
> >   warning: tag 'for-linus' is really 'kvm-3.19-1' here
> >   for-linus-14459-g49776d5
> 
> Hm, it should say v3.19 -- does it stay the same if you do
> `git fetch && git checkout origin/master`?
> 
> If it still does, please try to apply it on top of `git checkout v3.18`.
> (The one that one failed too.)
> 
> >   # Make a config file
> >   $ make defconfig
> 
> It would be safer to copy the fedora config (from /boot) to .config and
> do `make olddefconfig`.

That's actually what I did on my later compiles.

For now, as noted above, will test with the Fedora Kernel scratch build
I made.

  [1] https://bugzilla.redhat.com/show_bug.cgi?id=1194809 --  `dhclient`
  crashes on boot
  [2] http://koji.fedoraproject.org/koji/taskinfo?taskID=9004708
  [3] https://kojipkgs.fedoraproject.org//work/tasks/4708/9004708/build.log

-- 
/kashyap
--
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: [Qemu-devel] [RFC PATCH v2 04/15] cpu-model/s390: Introduce S390 CPU models

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 18:50:20 +0100
Alexander Graf  wrote:

> 
> 
> 
> > Am 20.02.2015 um 18:37 schrieb Michael Mueller :
> > 
> > On Fri, 20 Feb 2015 17:57:52 +0100
> > Alexander Graf  wrote:
> > 
> >> Because all CPUs we have in our list only expose 128 bits?
> > 
> > Here a STFLE result on a EC12 GA2, already more than 128 bits... Is that 
> > model on the list?
> 
> If that model has 3 elements, yes, the array should span 3.
> 
> I hope it's in the list. Every model wecare about should be, no?
> 

On my list? Yes!

> > 
> > [mimu@p57lp59 s390xfac]$ ./s390xfac -b
> > fac[0] = 0xfbfbfcfff840
> > fac[1] = 0xffde
> > fac[2] = 0x1800
> >> 
> >>> I want to have this independent from a future machine of the z/Arch. The 
> >>> kernel stores the
> >>> full facility set, KVM does and there is no good reason for QEMU not to 
> >>> do. If other
> >>> accelerators decide to just implement 64 or 128 bits of facilities that's 
> >>> ok...  
> >> 
> >> So you want to support CPUs that are not part of the list?
> > 
> > The architecture at least defines more than 2 or 3. Do you want me to limit 
> > it to an arbitrary
> > size?. Only in QEMU or also in the KVM interface?
> 
> Only internally in QEMU. The kvm interface should definitely be as big as the 
> spec allows!

Right, now we're on the same page again. That can be taken in consideration. 
... Although it's
just and optimization. :-)

Michael

> 
> Alex
> 
> > 
> > Thanks
> > Michael
> > 
> 

--
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: [Qemu-devel] [RFC PATCH v2 10/15] cpu-model/s390: Add cpu class initialization routines

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 20:21:45 +0100
Alexander Graf  wrote:

> 
> 
> 
> > Am 20.02.2015 um 19:59 schrieb Michael Mueller :
> > 
> > On Fri, 20 Feb 2015 10:11:55 -0800
> > Richard Henderson  wrote:
> > 
> >>> +static inline uint64_t big_endian_bit(unsigned long nr)
> >>> +{
> >>> +return 1ul << (BITS_PER_LONG - (nr % BITS_PER_LONG));
> >>> +};  
> >> 
> >> This is buggy.  NR=0 should map to 63, not 64.
> > 
> > I'm sure I was asked to replace my constant 64 and 63 with that defines and 
> > at the end I
> > messed it up... :-(
> > 
> >> 
> >>> +return !!(*ptr & big_endian_bit(nr));  
> >> 
> >> Personally I dislike !! as an idiom.  Given that big_endian_bit isn't used
> >> anywhere else, can we integrate it and change this to
> >> 
> >> static inline int test_facility(unsigned long nr, uint64_t *fac_list)
> >> {
> >>  unsigned long word = nr / BITS_PER_LONG;
> >>  unsigned long be_bit = 63 - (nr % BITS_PER_LONG);
> >>  return (fac_list[word] >> be_bit) & 1;
> >> }
> > 
> > Yes, I just use it in this context. I will integrate your version.
> > 
> > BTW I changed the whole facility defining code to be generated by an 
> > external helper at
> > compile time. That is more simple and safe to change. I will send it with 
> > v3. See attachment
> > for an example of the generated header file.
> 
> Please make sure to use ULL with constants and uint64_t on variables. Long is 
> almost always
> wrong in QEMU.

yep

> 
> Alex
> 
> > 
> > Thanks,
> > Michael
> > 
> > 
> 

--
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: [Qemu-devel] [RFC PATCH v2 13/15] cpu-model/s390: Add processor property routines

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 18:00:19 +0100
Alexander Graf  wrote:

> > So above s390_set/get_processor_props() the code is accelerator 
> > independent.  
> 
> Any particular reason you can't do it like PPC?

That seems to be a short question... and when I started one year ago, I 
oriented myself on
the PPC version and I'm also willing to revisit it but I can't give you a quick 
answer different
from no currently to that.

There are no PVRs for s390x CPUs and thus I came up with "pseudo PVRs":

/*
 * bits 0-7   : CMOS generation
 * bits 8-9   : reserved
 * bits 10-11 : machine class 0=unknown 1=EC 2=BC
 * bits 12-15 : GA
 * bits 16-31 : machine type
 *
 * note: bits are named according to s390
 *   architecture specific endienness
 */
enum {
CPU_S390_2064_GA1 = 0x07112064,
CPU_S390_2064_GA2 = 0x07122064,
CPU_S390_2064_GA3 = 0x07132064,
CPU_S390_2066_GA1 = 0x07212066,
CPU_S390_2084_GA1 = 0x08112084,
CPU_S390_2084_GA2 = 0x08122084,
CPU_S390_2084_GA3 = 0x08132084,
CPU_S390_2084_GA4 = 0x08142084,
CPU_S390_2084_GA5 = 0x08152084,
CPU_S390_2086_GA1 = 0x08212086,
CPU_S390_2086_GA2 = 0x08222086,
CPU_S390_2086_GA3 = 0x08232086,
CPU_S390_2094_GA1 = 0x09112094,
CPU_S390_2094_GA2 = 0x09122094,
CPU_S390_2094_GA3 = 0x09132094,
CPU_S390_2096_GA1 = 0x09212096,
CPU_S390_2096_GA2 = 0x09222096,
CPU_S390_2097_GA1 = 0x0a112097,
CPU_S390_2097_GA2 = 0x0a122097,
CPU_S390_2097_GA3 = 0x0a132097,
CPU_S390_2098_GA1 = 0x0a212098,
CPU_S390_2098_GA2 = 0x0a222098,
CPU_S390_2817_GA1 = 0x0b112817,
CPU_S390_2817_GA2 = 0x0b122817,
CPU_S390_2818_GA1 = 0x0b212818,
CPU_S390_2827_GA1 = 0x0c112827,
CPU_S390_2827_GA2 = 0x0c122827,
CPU_S390_2828_GA1 = 0x0c212828,
CPU_S390_2964_GA1 = 0x0d112964,
};

And initially I had a version that was limiting the accelerator to be able to 
implement just them
with all their properties encapsulated in the a accelerator as well. After 
identifying the real
processor related attributes defining the model, I changed the interface such 
that KVM or
other accelerators give hints what it is able to support in dependency of the 
current code
version and the hosting machine and let QEMU decide how to set these attributes
(cpuid,ibc,fac_list). Thus I think the implementation is now quite open and 
easily adoptable also
for TCG and possibly others as well. Eventually the integration and also some 
trigger points of
my code are to adjust. So coming back to your question, the answer is still no 
for the whole item
but eventually yes if you have limited it to the s390_set/get_processor_props() 
triggers. But I
have to look into it first again. I will do that when I'm back on Tuesday 
morning.

Thanks and have a nice WE
Michael

--
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: [Qemu-devel] [RFC PATCH v2 10/15] cpu-model/s390: Add cpu class initialization routines

2015-02-20 Thread Alexander Graf



> Am 20.02.2015 um 19:59 schrieb Michael Mueller :
> 
> On Fri, 20 Feb 2015 10:11:55 -0800
> Richard Henderson  wrote:
> 
>>> +static inline uint64_t big_endian_bit(unsigned long nr)
>>> +{
>>> +return 1ul << (BITS_PER_LONG - (nr % BITS_PER_LONG));
>>> +};  
>> 
>> This is buggy.  NR=0 should map to 63, not 64.
> 
> I'm sure I was asked to replace my constant 64 and 63 with that defines and 
> at the end I messed
> it up... :-(
> 
>> 
>>> +return !!(*ptr & big_endian_bit(nr));  
>> 
>> Personally I dislike !! as an idiom.  Given that big_endian_bit isn't used
>> anywhere else, can we integrate it and change this to
>> 
>> static inline int test_facility(unsigned long nr, uint64_t *fac_list)
>> {
>>  unsigned long word = nr / BITS_PER_LONG;
>>  unsigned long be_bit = 63 - (nr % BITS_PER_LONG);
>>  return (fac_list[word] >> be_bit) & 1;
>> }
> 
> Yes, I just use it in this context. I will integrate your version.
> 
> BTW I changed the whole facility defining code to be generated by an external 
> helper at compile
> time. That is more simple and safe to change. I will send it with v3. See 
> attachment for an
> example of the generated header file.

Please make sure to use ULL with constants and uint64_t on variables. Long is 
almost always wrong in QEMU.

Alex

> 
> Thanks,
> Michael
> 
> 
--
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: [Qemu-devel] [RFC PATCH v2 10/15] cpu-model/s390: Add cpu class initialization routines

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 10:11:55 -0800
Richard Henderson  wrote:

> > +static inline uint64_t big_endian_bit(unsigned long nr)
> > +{
> > +return 1ul << (BITS_PER_LONG - (nr % BITS_PER_LONG));
> > +};  
> 
> This is buggy.  NR=0 should map to 63, not 64.

I'm sure I was asked to replace my constant 64 and 63 with that defines and at 
the end I messed
it up... :-(

> 
> > +return !!(*ptr & big_endian_bit(nr));  
> 
> Personally I dislike !! as an idiom.  Given that big_endian_bit isn't used
> anywhere else, can we integrate it and change this to
> 
> static inline int test_facility(unsigned long nr, uint64_t *fac_list)
> {
>   unsigned long word = nr / BITS_PER_LONG;
>   unsigned long be_bit = 63 - (nr % BITS_PER_LONG);
>   return (fac_list[word] >> be_bit) & 1;
> }

Yes, I just use it in this context. I will integrate your version.

BTW I changed the whole facility defining code to be generated by an external 
helper at compile
time. That is more simple and safe to change. I will send it with v3. See 
attachment for an
example of the generated header file.

Thanks,
Michael

/*
 * AUTOMATICALLY GENERATED, DO NOT MODIFY HERE, EDIT
 * SOURCE FILE "target-s390x/tools/gen-facilities.c" INSTEAD.
 *
 * Copyright 2014, 2015 IBM Corp.
 *
 * This work is licensed under the terms of the GNU GPL, version 2 or (at
 * your option) any later version. See the COPYING file in the top-level
 * directory.
 */

#ifndef TARGET_S390X_GEN_FACILITIES_H
#define TARGET_S390X_GEN_FACILITIES_H

/* S390 CPU facility defines per CPU model */
#define FAC_LIST_CPU_S390_2064_GA1 0xe000
#define FAC_LIST_CPU_S390_2064_GA2 0xe0008000
#define FAC_LIST_CPU_S390_2064_GA3 0xe0008000
#define FAC_LIST_CPU_S390_2066_GA1 0xe0008000
#define FAC_LIST_CPU_S390_2084_GA1 0xf000f800
#define FAC_LIST_CPU_S390_2084_GA2 0xf800f800
#define FAC_LIST_CPU_S390_2084_GA3 0xfa00fa00
#define FAC_LIST_CPU_S390_2086_GA1 0xfa00fa00
#define FAC_LIST_CPU_S390_2084_GA4 0xfa00fa00
#define FAC_LIST_CPU_S390_2086_GA2 0xfa00fa00
#define FAC_LIST_CPU_S390_2084_GA5 0xfa00fa08
#define FAC_LIST_CPU_S390_2086_GA3 0xfa00fa08
#define FAC_LIST_CPU_S390_2094_GA1 0xfb00ffcb
#define FAC_LIST_CPU_S390_2094_GA2 0xfb40ffdb8060
#define FAC_LIST_CPU_S390_2094_GA3 0xfb40ffdb8068
#define FAC_LIST_CPU_S390_2096_GA1 0xfb40ffdb8068
#define FAC_LIST_CPU_S390_2096_GA2 0xfb40ffdb8068
#define FAC_LIST_CPU_S390_2097_GA1 0xfbf0fffbf078
#define FAC_LIST_CPU_S390_2097_GA2 0xfbf0fffbf078,0x5800
#define FAC_LIST_CPU_S390_2098_GA1 0xfbf0fffbf078,0x5800
#define FAC_LIST_CPU_S390_2097_GA3 0xfbf0fffbf0f8,0x5800
#define FAC_LIST_CPU_S390_2098_GA2 0xfbf0fffbf0f8,0x5800
#define FAC_LIST_CPU_S390_2817_GA1 0xfbf0fffbfcfe,0x1810
#define FAC_LIST_CPU_S390_2817_GA2 0xfbf6fffbfcff,0x381c
#define FAC_LIST_CPU_S390_2818_GA1 0xfbf6fffbfcff,0x381c
#define FAC_LIST_CPU_S390_2827_GA1 0xfbf6fffbfcfff800,0x3dde
#define FAC_LIST_CPU_S390_2827_GA2 0xfbf6fffbfcfff800,0x3dde
#define FAC_LIST_CPU_S390_2828_GA1 0xfbf6fffbfcfff800,0x3dde
#define FAC_LIST_CPU_S390_2964_GA1 0xfbf6fffbfcfff800,0x3dde,0x4000

/* QEMU facility mask defines */
#define FAC_LIST_CPU_S390_MASK_QEMU 0x,0x0580

/* Maximum size of generated facility list defines */
#define FAC_LIST_CPU_S390_SIZE_UINT64 3

#endif


[PATCH] x86: svm: use kvm_register_write()/read()

2015-02-20 Thread Joel Schopp
KVM has nice wrappers to access the register values, clean up a few places
that should use them but currently do not.

Signed-off-by:David Kaplan 
Signed-off-by:Joel Schopp 
---
 arch/x86/kvm/svm.c |   19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index d319e0c..a7d88e4 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2757,11 +2757,11 @@ static int invlpga_interception(struct vcpu_svm *svm)
 {
struct kvm_vcpu *vcpu = &svm->vcpu;
 
-   trace_kvm_invlpga(svm->vmcb->save.rip, vcpu->arch.regs[VCPU_REGS_RCX],
- vcpu->arch.regs[VCPU_REGS_RAX]);
+   trace_kvm_invlpga(svm->vmcb->save.rip, kvm_register_read(&svm->vcpu, 
VCPU_REGS_RCX),
+ kvm_register_read(&svm->vcpu, VCPU_REGS_RAX));
 
/* Let's treat INVLPGA the same as INVLPG (can be optimized!) */
-   kvm_mmu_invlpg(vcpu, vcpu->arch.regs[VCPU_REGS_RAX]);
+   kvm_mmu_invlpg(vcpu, kvm_register_read(&svm->vcpu, VCPU_REGS_RAX));
 
svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
skip_emulated_instruction(&svm->vcpu);
@@ -2770,7 +2770,7 @@ static int invlpga_interception(struct vcpu_svm *svm)
 
 static int skinit_interception(struct vcpu_svm *svm)
 {
-   trace_kvm_skinit(svm->vmcb->save.rip, 
svm->vcpu.arch.regs[VCPU_REGS_RAX]);
+   trace_kvm_skinit(svm->vmcb->save.rip, kvm_register_read(&svm->vcpu, 
VCPU_REGS_RAX));
 
kvm_queue_exception(&svm->vcpu, UD_VECTOR);
return 1;
@@ -3133,7 +3133,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, unsigned 
ecx, u64 *data)
 
 static int rdmsr_interception(struct vcpu_svm *svm)
 {
-   u32 ecx = svm->vcpu.arch.regs[VCPU_REGS_RCX];
+   u32 ecx = kvm_register_read(&svm->vcpu, VCPU_REGS_RCX);
u64 data;
 
if (svm_get_msr(&svm->vcpu, ecx, &data)) {
@@ -3142,8 +3142,8 @@ static int rdmsr_interception(struct vcpu_svm *svm)
} else {
trace_kvm_msr_read(ecx, data);
 
-   svm->vcpu.arch.regs[VCPU_REGS_RAX] = data & 0x;
-   svm->vcpu.arch.regs[VCPU_REGS_RDX] = data >> 32;
+   kvm_register_write(&svm->vcpu, VCPU_REGS_RAX, data & 
0x);
+   kvm_register_write(&svm->vcpu, VCPU_REGS_RDX, data >> 32);
svm->next_rip = kvm_rip_read(&svm->vcpu) + 2;
skip_emulated_instruction(&svm->vcpu);
}
@@ -3246,9 +3246,8 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct 
msr_data *msr)
 static int wrmsr_interception(struct vcpu_svm *svm)
 {
struct msr_data msr;
-   u32 ecx = svm->vcpu.arch.regs[VCPU_REGS_RCX];
-   u64 data = (svm->vcpu.arch.regs[VCPU_REGS_RAX] & -1u)
-   | ((u64)(svm->vcpu.arch.regs[VCPU_REGS_RDX] & -1u) << 32);
+   u32 ecx = kvm_register_read(&svm->vcpu, VCPU_REGS_RCX);
+   u64 data = kvm_read_edx_eax(&svm->vcpu);
 
msr.data = data;
msr.index = ecx;

--
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: [Qemu-devel] [RFC PATCH v2 09/15] cpu-model/s390: Add KVM VM attribute interface routines

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 17:59:14 +0100
Alexander Graf  wrote:

> But please give a nutshell explanation on what exactly you're patching
> at all here.

Please don't ask in riddles... :-)

S390ProcessorProps are attributes that represent cpu model related properties 
of the processor. 

typedef struct S390ProcessorProps {
uint64_t cpuid;
uint16_t ibc;
uint8_t  pad[6];
uint64_t fac_list[S390_ARCH_FAC_LIST_SIZE_UINT64];
} S390ProcessorProps;

S390MachineProps are attributes that represent cpu model related properties 
that a specific host
offers.

fac_list_mask are the facilities that are supported by the accelerator code and 
the hosting
machine in case of KVM it is kvm_s390_fac_list_mask & STFLE, fac_list is STFLE 
only

typedef struct S390MachineProps {
uint64_t cpuid;
uint32_t ibc_range;
uint8_t  pad[4];
uint64_t fac_list_mask[S390_ARCH_FAC_LIST_SIZE_UINT64];
uint64_t fac_list[S390_ARCH_FAC_LIST_SIZE_UINT64];
} S390MachineProps;

The various S390CPUModel classes represent all well defined cpu models (those 
which have been
implemented as real machines). Not all of these models are executable on a 
given host. Only the
most current machine implementation is able to run all models. The list of 
"runnable" cpu models
is calculated by "matching" the S390MachineProps with the S390CPUModel classes 
and the
qemu_s390_fac_list_mask[] (indicates the facilities that are implemented by 
QEMU itself.)

The qemu cpu_model is translated to the respective S390CPUModel class and its 
processor
properties (S390ProcessorProps) are used with the s390_set_proceccor_props() 
call to communicate
them to the accelerator what specific cpuid,ibc,fac_list shall be used.

Michael

--
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: [RFC PATCH v2 10/15] cpu-model/s390: Add cpu class initialization routines

2015-02-20 Thread Richard Henderson
On 02/17/2015 06:24 AM, Michael Mueller wrote:
> +static inline uint64_t big_endian_bit(unsigned long nr)
> +{
> +return 1ul << (BITS_PER_LONG - (nr % BITS_PER_LONG));
> +};

This is buggy.  NR=0 should map to 63, not 64.

> +return !!(*ptr & big_endian_bit(nr));

Personally I dislike !! as an idiom.  Given that big_endian_bit isn't used
anywhere else, can we integrate it and change this to

static inline int test_facility(unsigned long nr, uint64_t *fac_list)
{
  unsigned long word = nr / BITS_PER_LONG;
  unsigned long be_bit = 63 - (nr % BITS_PER_LONG);
  return (fac_list[word] >> be_bit) & 1;
}


r~
--
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: [Qemu-devel] [RFC PATCH v2 04/15] cpu-model/s390: Introduce S390 CPU models

2015-02-20 Thread Alexander Graf



> Am 20.02.2015 um 18:37 schrieb Michael Mueller :
> 
> On Fri, 20 Feb 2015 17:57:52 +0100
> Alexander Graf  wrote:
> 
>> Because all CPUs we have in our list only expose 128 bits?
> 
> Here a STFLE result on a EC12 GA2, already more than 128 bits... Is that 
> model on the list?

If that model has 3 elements, yes, the array should span 3.

I hope it's in the list. Every model wecare about should be, no?

> 
> [mimu@p57lp59 s390xfac]$ ./s390xfac -b
> fac[0] = 0xfbfbfcfff840
> fac[1] = 0xffde
> fac[2] = 0x1800
>> 
>>> I want to have this independent from a future machine of the z/Arch. The 
>>> kernel stores the
>>> full facility set, KVM does and there is no good reason for QEMU not to do. 
>>> If other
>>> accelerators decide to just implement 64 or 128 bits of facilities that's 
>>> ok...  
>> 
>> So you want to support CPUs that are not part of the list?
> 
> The architecture at least defines more than 2 or 3. Do you want me to limit 
> it to an arbitrary
> size?. Only in QEMU or also in the KVM interface?

Only internally in QEMU. The kvm interface should definitely be as big as the 
spec allows!

Alex

> 
> Thanks
> Michael
> 
--
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: [Qemu-devel] [RFC PATCH v2 04/15] cpu-model/s390: Introduce S390 CPU models

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 17:57:52 +0100
Alexander Graf  wrote:

> Because all CPUs we have in our list only expose 128 bits?

Here a STFLE result on a EC12 GA2, already more than 128 bits... Is that model 
on the list?

[mimu@p57lp59 s390xfac]$ ./s390xfac -b
fac[0] = 0xfbfbfcfff840
fac[1] = 0xffde
fac[2] = 0x1800
> 
> > I want to have this independent from a future machine of the z/Arch. The 
> > kernel stores the
> > full facility set, KVM does and there is no good reason for QEMU not to do. 
> > If other
> > accelerators decide to just implement 64 or 128 bits of facilities that's 
> > ok...  
> 
> So you want to support CPUs that are not part of the list?

The architecture at least defines more than 2 or 3. Do you want me to limit it 
to an arbitrary
size?. Only in QEMU or also in the KVM interface?

Thanks
Michael

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


Marcelo handling KVM tree for a few weeks

2015-02-20 Thread Paolo Bonzini
KVM folks, Linus,

my second son should be getting out of his current cozy location very
soon, after which I'll be working part time for about a month.

During this period, I will not have enough time to devote to KVM
maintainership, and Marcelo will therefore take care of the kernel KVM
tree from next Monday until roughly the end of March.  I will likely
handle the next merge window, but just to be safe, either I or Marcelo
will write again when things get back to normal on my side.

Thanks,

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


Re: [Qemu-devel] [RFC PATCH v2 13/15] cpu-model/s390: Add processor property routines

2015-02-20 Thread Alexander Graf


On 20.02.15 16:32, Michael Mueller wrote:
> On Fri, 20 Feb 2015 15:03:30 +0100
> Alexander Graf  wrote:
> 
>>>
>>> - s390_get_proceccor_props()
>>> - s390_set_proceccor_props()
>>>
>>> They can be used to request or retrieve processor related information from 
>>> an accelerator.
>>> That information comprises the cpu identifier, the ICB value and the 
>>> facility lists.
>>>
>>> Signed-off-by: Michael Mueller   
>>
>> Hrm, I still seem to miss the point of this interface. What do you need
>> it for?
> 
> These functions make the internal s390 cpu model API independent from a 
> specific accelerator:  
> 
> int s390_set_processor_props(S390ProcessorProps *prop)
> {
> if (kvm_enabled()) {
> return kvm_s390_set_processor_props(prop);
> }
> return -ENOSYS;
> }
> 
> It's called by:
> 
> s390_select_cpu_model(const char *model)
> 
> which is itself called by:
> 
> S390CPU *cpu_s390x_init(const char *cpu_model)
> {
> S390CPU *cpu;
> 
> cpu = S390_CPU(object_new(s390_select_cpu_model(cpu_model)));
> 
> object_property_set_bool(OBJECT(cpu), true, "realized", NULL);
> 
> return cpu;
> }
> 
> So above s390_set/get_processor_props() the code is accelerator independent.

Any particular reason you can't do it like PPC?


Alex
--
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: [Qemu-devel] [RFC PATCH v2 09/15] cpu-model/s390: Add KVM VM attribute interface routines

2015-02-20 Thread Alexander Graf


On 20.02.15 16:18, Michael Mueller wrote:
> On Fri, 20 Feb 2015 14:59:20 +0100
> Alexander Graf  wrote:
> 
>>> +typedef struct S390ProcessorProps {
>>> +uint64_t cpuid;
>>> +uint16_t ibc;
>>> +uint8_t  pad[6];
>>> +uint64_t fac_list[S390_ARCH_FAC_LIST_SIZE_UINT64];
>>> +} S390ProcessorProps;
>>> +
>>> +typedef struct S390MachineProps {
>>> +uint64_t cpuid;
>>> +uint32_t ibc_range;
>>> +uint8_t  pad[4];
>>> +uint64_t fac_list_mask[S390_ARCH_FAC_LIST_SIZE_UINT64];
>>> +uint64_t fac_list[S390_ARCH_FAC_LIST_SIZE_UINT64];
>>> +} S390MachineProps;  
>>
>> What are those structs there for? To convert between a kvm facing
>> interface to an internal interface?
> 
> Yes, that's their current use, but if the interface structs: 
> 
> +struct kvm_s390_vm_cpu_processor {
> +   __u64 cpuid;
> +   __u16 ibc;
> +   __u8  pad[6];
> +   __u64 fac_list[256];
> +};
> +
> +/* kvm S390 machine related attributes are r/o */
> +#define KVM_S390_VM_CPU_MACHINE1
> +struct kvm_s390_vm_cpu_machine {
> +   __u64 cpuid;
> +   __u32 ibc_range;
> +   __u8  pad[4];
> +   __u64 fac_mask[256];
> +   __u64 fac_list[256];
> +};
> 
> are visible here, I'll reuse them... But stop, that will not work in the 
> --disable-kvm case... I need them!

I meant it the other way around - do KVM specific patching of the cpu
types from kvm.c.

But please give a nutshell explanation on what exactly you're patching
at all here.


Alex
--
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: [Qemu-devel] [RFC PATCH v2 04/15] cpu-model/s390: Introduce S390 CPU models

2015-02-20 Thread Alexander Graf


On 20.02.15 16:49, Michael Mueller wrote:
> On Fri, 20 Feb 2015 16:22:20 +0100
> Alexander Graf  wrote:
> 

 Just make this uint64_t fac_list[2]. That way we don't have to track any
 messy allocations.  
>>>
>>> It will be something like "uint64_t 
>>> fac_list[S390_CPU_FAC_LIST_SIZE_UINT64]" and in total 2KB
>>> not just 16 bytes but I will change it.   
>>
>> Why? Do we actually need that many? This is a qemu internal struct.
> 
> How do you know that 2 is a good size?

Because all CPUs we have in our list only expose 128 bits?

> I want to have this independent from a future machine of the z/Arch. The 
> kernel stores the full
> facility set, KVM does and there is no good reason for QEMU not to do. If 
> other accelerators
> decide to just implement 64 or 128 bits of facilities that's ok...

So you want to support CPUs that are not part of the list?


Alex
--
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: [Qemu-devel] [RFC PATCH v2 10/15] cpu-model/s390: Add cpu class initialization routines

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 17:34:28 +0100
Andreas Färber  wrote:

> Please note that QEMU uses gtk-doc style, where the description goes
> between arguments and Returns:, and the function name gets a ':'.
> There's also fancy syntax like #CPUClass, %true, etc.

On my TODOs...

Thanks,
Michael

--
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: [Qemu-devel] [RFC PATCH v2 13/15] cpu-model/s390: Add processor property routines

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 17:28:14 +0100
Andreas Färber  wrote:

Andreas,

> Sorry for my ignorance, but what is proc actually needed for? For
> initializing the class, there's .class_init (and cc->fac_list apparently
> is initialized here). If you need to pass info to KVM, you can do so in

yes, it is communication to the accelerator to prepare its local cpu model 
related data
structures which are used to initialize a vcpu (e.g. the facility list beside 
others) 

> DeviceClass::realize when the vCPU actually goes "live". A

I will look what "goes live" in detail means here, it should at least be before
kvm_arch_vcpu_setup() gets triggered on accelerator side.

> string-to-string (or string-to-ObjectClass) translation function seems
> like a weird point in time to take action with global effect.
> 
> Anyway, please implement the generic callback, then you can still call
> it from your own helper functions if needed.

Thanks a lot!
Michael

--
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: [Qemu-devel] [RFC PATCH v2 10/15] cpu-model/s390: Add cpu class initialization routines

2015-02-20 Thread Andreas Färber
Am 20.02.2015 um 17:12 schrieb Michael Mueller:
> On Fri, 20 Feb 2015 08:02:42 -0800
> Richard Henderson  wrote:
> 
>>> +/**
>>> + * s390_test_facility - test if given facility bit is set facility list
>>> + *  of given cpu class
>>> + * @class: address of cpu class to test
>>> + * @nr: bit number to test
>>> + *
>>> + * Returns: true in case it is set
>>> + *  false in case it is not set
>>> + */

Please note that QEMU uses gtk-doc style, where the description goes
between arguments and Returns:, and the function name gets a ':'.
There's also fancy syntax like #CPUClass, %true, etc.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
--
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: [Qemu-devel] [RFC PATCH v2 13/15] cpu-model/s390: Add processor property routines

2015-02-20 Thread Andreas Färber
Am 20.02.2015 um 17:04 schrieb Michael Mueller:
> On Fri, 20 Feb 2015 16:41:49 +0100
> Andreas Färber  wrote:
> 
>> Can't you just implement the class-level name-to-ObjectClass callback
>> that other CPUs have grown for the above use case?
> 
> If it fulfills the requirements sure. Please point me to an example,

Take a look at include/qom/cpu.h CPUClass::class_by_name and git-grep
the existing targets - most implement it already. It's a generic hook to
be used from everywhere rather than a local function specific to the
legacy init function. Apart from the error handling it should be
straight-forward.

> sounds that
> s390_select_cpu_model() is doing something similar to that, just that it 
> hooks in
> the s390_set_processor_props() call.
> 
> const char *s390_select_cpu_model(const char *model)
> {
> S390ProcessorProps proc;
> const char *typename;
> S390CPUClass *cc;
> 
> /* return already selected cpu typename */
> typename = s390_cpu_typename();
> if (typename) {
> goto out;
> }
> 
> /* return standard cpu typename when cpu models are unavailable */
> typename = TYPE_S390_CPU;
> if (!s390_cpu_classes_initialized() || !model) {
> goto out;
> }
> cc = S390_CPU_CLASS(s390_cpu_class_by_name(model));
> if (!cc) {
> goto out;
> }
> proc.cpuid = cpuid(cc->proc);
> proc.ibc = cc->proc->ibc;
> memcpy(proc.fac_list, cc->fac_list, S390_ARCH_FAC_LIST_SIZE_BYTE);
> if (s390_set_processor_props(&proc)) {
> goto out;
> }

Sorry for my ignorance, but what is proc actually needed for? For
initializing the class, there's .class_init (and cc->fac_list apparently
is initialized here). If you need to pass info to KVM, you can do so in
DeviceClass::realize when the vCPU actually goes "live". A
string-to-string (or string-to-ObjectClass) translation function seems
like a weird point in time to take action with global effect.

Anyway, please implement the generic callback, then you can still call
it from your own helper functions if needed.

Regards,
Andreas

> 
> /* return requested cpu typename in success case */
> typename = object_class_get_name((ObjectClass *) cc);
> out:
> selected_cpu_typename = typename;
> trace_select_cpu_model(model, typename);
> return typename;
> }

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
--
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: [Qemu-devel] [RFC PATCH v2 10/15] cpu-model/s390: Add cpu class initialization routines

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 17:12:49 +0100
Michael Mueller  wrote:

> Good spot, it's not being used yet. It's planned to be used with a patch that 
> implements zPCI
> related instructions on QEMU side. Maybe you have seen the discussion from 
> Frank Blaschka in
> this e-mail list in regard to that.

I will factor it out.

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


[GIT PULL] VFIO updates for 3.20-rc1

2015-02-20 Thread Alex Williamson
Hi Linus,

The following changes since commit e36f014edff70fc02b3d3d79cead1d58f289332e:

  Linux 3.19-rc7 (2015-02-01 20:07:21 -0800)

are available in the git repository at:

  git://github.com/awilliam/linux-vfio.git tags/vfio-v3.20-rc1

for you to fetch changes up to 6140a8f5623820cec7f56c63444b9551d8d35775:

  vfio-pci: Add device request interface (2015-02-10 12:38:14 -0700)


VFIO updates for v3.20-rc1
 - IOMMU updates based on trace analysis (Alex Williamson)
 - VFIO device request interface (Alex Williamson)


Alex Williamson (8):
  vfio/type1: DMA unmap chunking
  vfio/type1: Chunk contiguous reserved/invalid page mappings
  vfio/type1: Add conditional rescheduling
  vfio: Add device tracking during unbind
  vfio: Tie IOMMU group reference to vfio group
  vfio: Add and use device request op for vfio bus drivers
  vfio-pci: Generalize setup of simple eventfds
  vfio-pci: Add device request interface

 drivers/vfio/pci/vfio_pci.c |  21 ++-
 drivers/vfio/pci/vfio_pci_intrs.c   |  60 +-
 drivers/vfio/pci/vfio_pci_private.h |   1 +
 drivers/vfio/vfio.c | 119 +++-
 drivers/vfio/vfio_iommu_type1.c |  80 
 include/linux/vfio.h|   2 +
 include/uapi/linux/vfio.h   |   1 +
 7 files changed, 242 insertions(+), 42 deletions(-)


--
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: [nVMX] With 3.20.0-0.rc0.git5.1 on L0, booting L2 guest results in L1 *rebooting*

2015-02-20 Thread Radim Krčmář
2015-02-19 23:28+0100, Kashyap Chamarthy:
> On Thu, Feb 19, 2015 at 10:10:11PM +0100, Kashyap Chamarthy wrote:
> > On Thu, Feb 19, 2015 at 05:02:22PM +0100, Radim Krčmář wrote:
> 
> [. . .]
> 
> > > Can you try if the following patch works?
> > 
> > Sure, will test a Kernel built with the below patch and report back.
> 
> Hmm, I'm stuck with a meta issue.
> 
> I checked out the KVM tree[1] on L0, applied your patch and built[*] the
> Kernel, and booted into it. Boot fails and drops into a dracut shell on
> because:
> 
>  . . .
>  dracut-initqueue[3045]: Warning: Cancelling resume operation. Device not 
> found.
>  [ TIME ] Timed out waiting for device
>  dev-ma...per910\x2d\x2d02\x2droot.device.
>  [DEPEND] Dependency failed for /sysroot.
>  [DEPEND] Dependency failed for Initrd Root File SyWarning:
>  /dev/disk/by-uuid/4ccddb2d-4d63-4fce-b4d4-9b2f119a30cc does not exist
>  . . .
> 
> I saved the report from /run/initramfs/rdsosreport.txt here[2].
> 
> 
> Then, I did another test:
> 
>   - Rebooted into Kernel 3.20.0-0.rc0.git5.1.fc23.x86_64 on physical
> host (L0).
>   - In L1, checked out the KVM tree, applied your patch and built
> Kernel[*] from the current KVM tree and booted into the newly built
> one, here too, I'm thrown into a dracut shell

Weird, but considering that boot fails on L0 as well, I think it that
basing off a different commit could help ...

> [1] git://git.kernel.org/pub/scm/virt/kvm/kvm.git
> [2] https://kashyapc.fedorapeople.org/temp/kernel-boot-failure.txt
> 
> [*] Exactly, I built it this way:
> 
>   # Clone the tree
>   $ git://git.kernel.org/pub/scm/virt/kvm/kvm.git
> 
>   # Make a new branch:
>   $ git checkout -b nvmx_test
>   $ git describe
>   warning: tag 'for-linus' is really 'kvm-3.19-1' here
>   for-linus-14459-g49776d5

Hm, it should say v3.19 -- does it stay the same if you do
`git fetch && git checkout origin/master`?

If it still does, please try to apply it on top of `git checkout v3.18`.
(The one that one failed too.)

>   # Make a config file
>   $ make defconfig

It would be safer to copy the fedora config (from /boot) to .config and
do `make olddefconfig`.

>   # Compile
>   $ make -j4 && make bzImage && make modules
>   
>   # Install
>   $ sudo -i
>   $ make modules_install && make install
> 
> -- 
> /kashyap
--
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: [Qemu-devel] [RFC PATCH v2 10/15] cpu-model/s390: Add cpu class initialization routines

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 08:02:42 -0800
Richard Henderson  wrote:

> > +/**
> > + * s390_test_facility - test if given facility bit is set facility list
> > + *  of given cpu class
> > + * @class: address of cpu class to test
> > + * @nr: bit number to test
> > + *
> > + * Returns: true in case it is set
> > + *  false in case it is not set
> > + */
> > +bool s390_test_facility(S390CPUClass *cc, unsigned long nr)
> > +{
> > +if (cc) {
> > +return test_facility(nr, cc->fac_list) ? true : false;
> > +}
> > +return false;
> > +}  
> 
> Where do you see this being used?

Good spot, it's not being used yet. It's planned to be used with a patch that 
implements zPCI
related instructions on QEMU side. Maybe you have seen the discussion from 
Frank Blaschka in this
e-mail list in regard to that.

Michael

--
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: [Qemu-devel] [RFC PATCH v2 13/15] cpu-model/s390: Add processor property routines

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 16:41:49 +0100
Andreas Färber  wrote:

> Also a general comment: cpu-model/ is not an existing directory nor one
> you add, so please use "target-s390x: Add foo to S390CPU" or so.

I will address this with v3, thanks a lot for the hint, I never saw this as 
directories though...

Michael

--
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: [Qemu-devel] [RFC PATCH v2 13/15] cpu-model/s390: Add processor property routines

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 16:41:49 +0100
Andreas Färber  wrote:

> Can't you just implement the class-level name-to-ObjectClass callback
> that other CPUs have grown for the above use case?

If it fulfills the requirements sure. Please point me to an example, sounds that
s390_select_cpu_model() is doing something similar to that, just that it hooks 
in
the s390_set_processor_props() call.

const char *s390_select_cpu_model(const char *model)
{
S390ProcessorProps proc;
const char *typename;
S390CPUClass *cc;

/* return already selected cpu typename */
typename = s390_cpu_typename();
if (typename) {
goto out;
}

/* return standard cpu typename when cpu models are unavailable */
typename = TYPE_S390_CPU;
if (!s390_cpu_classes_initialized() || !model) {
goto out;
}
cc = S390_CPU_CLASS(s390_cpu_class_by_name(model));
if (!cc) {
goto out;
}
proc.cpuid = cpuid(cc->proc);
proc.ibc = cc->proc->ibc;
memcpy(proc.fac_list, cc->fac_list, S390_ARCH_FAC_LIST_SIZE_BYTE);
if (s390_set_processor_props(&proc)) {
goto out;
}

/* return requested cpu typename in success case */
typename = object_class_get_name((ObjectClass *) cc);
out:
selected_cpu_typename = typename;
trace_select_cpu_model(model, typename);
return typename;
}


> 
> Also a general comment: cpu-model/ is not an existing directory nor one
> you add, so please use "target-s390x: Add foo to S390CPU" or so.

--
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: [RFC PATCH v2 10/15] cpu-model/s390: Add cpu class initialization routines

2015-02-20 Thread Richard Henderson
On 02/17/2015 06:24 AM, Michael Mueller wrote:
> +/**
> + * s390_test_facility - test if given facility bit is set facility list
> + *  of given cpu class
> + * @class: address of cpu class to test
> + * @nr: bit number to test
> + *
> + * Returns: true in case it is set
> + *  false in case it is not set
> + */
> +bool s390_test_facility(S390CPUClass *cc, unsigned long nr)
> +{
> +if (cc) {
> +return test_facility(nr, cc->fac_list) ? true : false;
> +}
> +return false;
> +}

Where do you see this being used?


r~
--
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: [Qemu-devel] [RFC PATCH v2 04/15] cpu-model/s390: Introduce S390 CPU models

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 16:22:20 +0100
Alexander Graf  wrote:

> >> 
> >> Just make this uint64_t fac_list[2]. That way we don't have to track any
> >> messy allocations.  
> > 
> > It will be something like "uint64_t 
> > fac_list[S390_CPU_FAC_LIST_SIZE_UINT64]" and in total 2KB
> > not just 16 bytes but I will change it.   
> 
> Why? Do we actually need that many? This is a qemu internal struct.

How do you know that 2 is a good size?

I want to have this independent from a future machine of the z/Arch. The kernel 
stores the full
facility set, KVM does and there is no good reason for QEMU not to do. If other 
accelerators
decide to just implement 64 or 128 bits of facilities that's ok...

Michael

--
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: [Qemu-devel] [RFC PATCH v2 13/15] cpu-model/s390: Add processor property routines

2015-02-20 Thread Andreas Färber
Am 20.02.2015 um 16:32 schrieb Michael Mueller:
> On Fri, 20 Feb 2015 15:03:30 +0100
> Alexander Graf  wrote:
> 
>>>
>>> - s390_get_proceccor_props()
>>> - s390_set_proceccor_props()
>>>
>>> They can be used to request or retrieve processor related information from 
>>> an accelerator.
>>> That information comprises the cpu identifier, the ICB value and the 
>>> facility lists.
>>>
>>> Signed-off-by: Michael Mueller   
>>
>> Hrm, I still seem to miss the point of this interface. What do you need
>> it for?
> 
> These functions make the internal s390 cpu model API independent from a 
> specific accelerator:  
> 
> int s390_set_processor_props(S390ProcessorProps *prop)
> {
> if (kvm_enabled()) {
> return kvm_s390_set_processor_props(prop);
> }
> return -ENOSYS;
> }
> 
> It's called by:
> 
> s390_select_cpu_model(const char *model)
> 
> which is itself called by:
> 
> S390CPU *cpu_s390x_init(const char *cpu_model)
> {
> S390CPU *cpu;
> 
> cpu = S390_CPU(object_new(s390_select_cpu_model(cpu_model)));
> 
> object_property_set_bool(OBJECT(cpu), true, "realized", NULL);
> 
> return cpu;
> }
> 
> So above s390_set/get_processor_props() the code is accelerator independent.

Can't you just implement the class-level name-to-ObjectClass callback
that other CPUs have grown for the above use case?

Also a general comment: cpu-model/ is not an existing directory nor one
you add, so please use "target-s390x: Add foo to S390CPU" or so.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
--
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: [RFC/RFT PATCH 0/3] arm64: KVM: work around incoherency with uncached guest mappings

2015-02-20 Thread Andrew Jones
On Fri, Feb 20, 2015 at 02:37:25PM +, Ard Biesheuvel wrote:
> On 20 February 2015 at 14:29, Andrew Jones  wrote:
> > So looks like the 3 orders of magnitude greater number of traps
> > (only to el2) don't impact kernel compiles.
> >
> 
> OK, good! That was what I was hoping for, obviously.
> 
> > Then I thought I'd be able to quick measure the number of cycles
> > a trap to el2 takes with this kvm-unit-tests test
> >
> > int main(void)
> > {
> > unsigned long start, end;
> > unsigned int sctlr;
> >
> > asm volatile(
> > "   mrs %0, sctlr_el1\n"
> > "   msr pmcr_el0, %1\n"
> > : "=&r" (sctlr) : "r" (5));
> >
> > asm volatile(
> > "   mrs %0, pmccntr_el0\n"
> > "   msr sctlr_el1, %2\n"
> > "   mrs %1, pmccntr_el0\n"
> > : "=&r" (start), "=&r" (end) : "r" (sctlr));
> >
> > printf("%llx\n", end - start);
> > return 0;
> > }
> >
> > after applying this patch to kvm
> >
> > diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> > index bb91b6fc63861..5de39d740aa58 100644
> > --- a/arch/arm64/kvm/hyp.S
> > +++ b/arch/arm64/kvm/hyp.S
> > @@ -770,7 +770,7 @@
> >
> > mrs x2, mdcr_el2
> > and x2, x2, #MDCR_EL2_HPMN_MASK
> > -   orr x2, x2, #(MDCR_EL2_TPM | MDCR_EL2_TPMCR)
> > +// orr x2, x2, #(MDCR_EL2_TPM | MDCR_EL2_TPMCR)
> > orr x2, x2, #(MDCR_EL2_TDRA | MDCR_EL2_TDOSA)
> >
> > // Check for KVM_ARM64_DEBUG_DIRTY, and set debug to trap
> >
> > But I get zero for the cycle count. Not sure what I'm missing.
> >
> 
> No clue tbh. Does the counter work as expected in the host?
>

Guess not. I dropped the test into a module_init and inserted
it on the host. Always get zero for pmccntr_el0 reads. Or, if
I set it to something non-zero with a write, then I always get
that back - no increments. pmcr_el0 looks OK... I had forgotten
to set bit 31 of pmcntenset_el0, but doing that still doesn't
help. Anyway, I assume the problem is me. I'll keep looking to
see what I'm missing.

drew
--
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: [Qemu-devel] [RFC PATCH v2 13/15] cpu-model/s390: Add processor property routines

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 15:03:30 +0100
Alexander Graf  wrote:

> > 
> > - s390_get_proceccor_props()
> > - s390_set_proceccor_props()
> > 
> > They can be used to request or retrieve processor related information from 
> > an accelerator.
> > That information comprises the cpu identifier, the ICB value and the 
> > facility lists.
> > 
> > Signed-off-by: Michael Mueller   
> 
> Hrm, I still seem to miss the point of this interface. What do you need
> it for?

These functions make the internal s390 cpu model API independent from a 
specific accelerator:  

int s390_set_processor_props(S390ProcessorProps *prop)
{
if (kvm_enabled()) {
return kvm_s390_set_processor_props(prop);
}
return -ENOSYS;
}

It's called by:

s390_select_cpu_model(const char *model)

which is itself called by:

S390CPU *cpu_s390x_init(const char *cpu_model)
{
S390CPU *cpu;

cpu = S390_CPU(object_new(s390_select_cpu_model(cpu_model)));

object_property_set_bool(OBJECT(cpu), true, "realized", NULL);

return cpu;
}

So above s390_set/get_processor_props() the code is accelerator independent.

Michael





--
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: [Qemu-devel] [RFC PATCH v2 04/15] cpu-model/s390: Introduce S390 CPU models

2015-02-20 Thread Alexander Graf



> Am 20.02.2015 um 16:00 schrieb Michael Mueller :
> 
> On Fri, 20 Feb 2015 14:54:23 +0100
> Alexander Graf  wrote:
> 
>>> 
>>> +/* machine related properties */
>>> +typedef struct S390CPUMachineProps {
>>> +uint16_t class;  /* machine class */
>>> +uint16_t ga; /* availability number of machine */
>>> +uint16_t order;  /* order of availability */
>>> +} S390CPUMachineProps;
>>> +
>>> +/* processor related properties */
>>> +typedef struct S390CPUProcessorProps {
>>> +uint16_t gen;/* S390 CMOS generation */
>>> +uint16_t ver;/* version of processor */
>>> +uint32_t id; /* processor identification*/
>>> +uint16_t type;   /* machine type */
>>> +uint16_t ibc;/* IBC value */
>>> +uint64_t *fac_list;  /* list of facilities */  
>> 
>> Just make this uint64_t fac_list[2]. That way we don't have to track any
>> messy allocations.
> 
> It will be something like "uint64_t fac_list[S390_CPU_FAC_LIST_SIZE_UINT64]" 
> and in total 2KB not
> just 16 bytes but I will change it. 

Why? Do we actually need that many? This is a qemu internal struct.

Alex--
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: [Qemu-devel] [RFC PATCH v2 09/15] cpu-model/s390: Add KVM VM attribute interface routines

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 14:59:20 +0100
Alexander Graf  wrote:

> > +typedef struct S390ProcessorProps {
> > +uint64_t cpuid;
> > +uint16_t ibc;
> > +uint8_t  pad[6];
> > +uint64_t fac_list[S390_ARCH_FAC_LIST_SIZE_UINT64];
> > +} S390ProcessorProps;
> > +
> > +typedef struct S390MachineProps {
> > +uint64_t cpuid;
> > +uint32_t ibc_range;
> > +uint8_t  pad[4];
> > +uint64_t fac_list_mask[S390_ARCH_FAC_LIST_SIZE_UINT64];
> > +uint64_t fac_list[S390_ARCH_FAC_LIST_SIZE_UINT64];
> > +} S390MachineProps;  
> 
> What are those structs there for? To convert between a kvm facing
> interface to an internal interface?

Yes, that's their current use, but if the interface structs: 

+struct kvm_s390_vm_cpu_processor {
+   __u64 cpuid;
+   __u16 ibc;
+   __u8  pad[6];
+   __u64 fac_list[256];
+};
+
+/* kvm S390 machine related attributes are r/o */
+#define KVM_S390_VM_CPU_MACHINE1
+struct kvm_s390_vm_cpu_machine {
+   __u64 cpuid;
+   __u32 ibc_range;
+   __u8  pad[4];
+   __u64 fac_mask[256];
+   __u64 fac_list[256];
+};

are visible here, I'll reuse them... But stop, that will not work in the 
--disable-kvm case... I need them!
> 
> I don't think they're necessary. The internal layout is visible from the
> KVM code. Just either spawn the class straight from the kvm file or if
> you consider that ugly, pass the values of that struct that you need as
> function parameters to a function in cpu-models.c.

--
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: [Qemu-devel] [RFC PATCH v2 04/15] cpu-model/s390: Introduce S390 CPU models

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 14:55:32 +0100
Alexander Graf  wrote:

> >  /**
> >   * S390CPUClass:
> >   * @parent_realize: The parent class' realize handler.
> > @@ -52,6 +69,11 @@ typedef struct S390CPUClass {
> >  void (*load_normal)(CPUState *cpu);
> >  void (*cpu_reset)(CPUState *cpu);
> >  void (*initial_cpu_reset)(CPUState *cpu);
> > +bool is_active[ACCEL_ID_MAX]; /* model enabled for given host and 
> > accel */
> > +bool is_host[ACCEL_ID_MAX];   /* model markes host for given accel */
> > +uint64_t *fac_list;   /* active facility list */
> > +S390CPUMachineProps   *mach;  /* machine specific properties */
> > +S390CPUProcessorProps *proc;  /* processor specific properties */  
> 
> Sorry, same here. Just put the structs straight into the class struct.

Yep, consistent.

--
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: [Qemu-devel] [RFC PATCH v2 04/15] cpu-model/s390: Introduce S390 CPU models

2015-02-20 Thread Michael Mueller
On Fri, 20 Feb 2015 14:54:23 +0100
Alexander Graf  wrote:

> >  
> > +/* machine related properties */
> > +typedef struct S390CPUMachineProps {
> > +uint16_t class;  /* machine class */
> > +uint16_t ga; /* availability number of machine */
> > +uint16_t order;  /* order of availability */
> > +} S390CPUMachineProps;
> > +
> > +/* processor related properties */
> > +typedef struct S390CPUProcessorProps {
> > +uint16_t gen;/* S390 CMOS generation */
> > +uint16_t ver;/* version of processor */
> > +uint32_t id; /* processor identification*/
> > +uint16_t type;   /* machine type */
> > +uint16_t ibc;/* IBC value */
> > +uint64_t *fac_list;  /* list of facilities */  
> 
> Just make this uint64_t fac_list[2]. That way we don't have to track any
> messy allocations.

It will be something like "uint64_t fac_list[S390_CPU_FAC_LIST_SIZE_UINT64]" 
and in total 2KB not
just 16 bytes but I will change it. 



--
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: [RFC/RFT PATCH 0/3] arm64: KVM: work around incoherency with uncached guest mappings

2015-02-20 Thread Ard Biesheuvel
On 20 February 2015 at 14:29, Andrew Jones  wrote:
> On Thu, Feb 19, 2015 at 06:57:24PM +0100, Paolo Bonzini wrote:
>>
>>
>> On 19/02/2015 18:55, Andrew Jones wrote:
>> >> > > (I don't have an exact number for how many times it went to EL1 
>> >> > > because
>> >> > >  access_mair() doesn't have a trace point.)
>> >> > > (I got the 62873 number by testing a 3rd kernel build that only had 
>> >> > > patch
>> >> > >  3/3 applied to the base, and counting kvm_toggle_cache events.)
>> >> > > (The number 50 is the number of kvm_toggle_cache events *without* 3/3
>> >> > >  applied.)
>> >> > >
>> >> > > I consider this bad news because, even considering it only goes to 
>> >> > > EL2,
>> >> > > it goes a ton more than it used to. I realize patch 3/3 isn't the 
>> >> > > final
>> >> > > plan for enabling traps though.
>>
>> If a full guest boots, can you try timing a kernel compile?
>>
>
> Guests boot. I used an 8 vcpu, 14G memory guest; compiled the kernel 4
> times inside the guest for each host kernel; base and mair. I dropped
> the time from the first run of each set, and captured the other 3.
> Command line used below. Time is from the
>   Elapsed (wall clock) time (h:mm:ss or m:ss):
> output of /usr/bin/time - the host's wall clock.
>
>   /usr/bin/time --verbose ssh $VM 'cd kernel && make -s clean && make -s -j8'
>
> Results:
> base: 3:06.11 3:07.00 3:10.93
> mair: 3:08.47 3:06.75 3:04.76
>
> So looks like the 3 orders of magnitude greater number of traps
> (only to el2) don't impact kernel compiles.
>

OK, good! That was what I was hoping for, obviously.

> Then I thought I'd be able to quick measure the number of cycles
> a trap to el2 takes with this kvm-unit-tests test
>
> int main(void)
> {
> unsigned long start, end;
> unsigned int sctlr;
>
> asm volatile(
> "   mrs %0, sctlr_el1\n"
> "   msr pmcr_el0, %1\n"
> : "=&r" (sctlr) : "r" (5));
>
> asm volatile(
> "   mrs %0, pmccntr_el0\n"
> "   msr sctlr_el1, %2\n"
> "   mrs %1, pmccntr_el0\n"
> : "=&r" (start), "=&r" (end) : "r" (sctlr));
>
> printf("%llx\n", end - start);
> return 0;
> }
>
> after applying this patch to kvm
>
> diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
> index bb91b6fc63861..5de39d740aa58 100644
> --- a/arch/arm64/kvm/hyp.S
> +++ b/arch/arm64/kvm/hyp.S
> @@ -770,7 +770,7 @@
>
> mrs x2, mdcr_el2
> and x2, x2, #MDCR_EL2_HPMN_MASK
> -   orr x2, x2, #(MDCR_EL2_TPM | MDCR_EL2_TPMCR)
> +// orr x2, x2, #(MDCR_EL2_TPM | MDCR_EL2_TPMCR)
> orr x2, x2, #(MDCR_EL2_TDRA | MDCR_EL2_TDOSA)
>
> // Check for KVM_ARM64_DEBUG_DIRTY, and set debug to trap
>
> But I get zero for the cycle count. Not sure what I'm missing.
>

No clue tbh. Does the counter work as expected in the host?

-- 
Ard.
--
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: [RFC/RFT PATCH 0/3] arm64: KVM: work around incoherency with uncached guest mappings

2015-02-20 Thread Andrew Jones
On Thu, Feb 19, 2015 at 06:57:24PM +0100, Paolo Bonzini wrote:
> 
> 
> On 19/02/2015 18:55, Andrew Jones wrote:
> >> > > (I don't have an exact number for how many times it went to EL1 because
> >> > >  access_mair() doesn't have a trace point.)
> >> > > (I got the 62873 number by testing a 3rd kernel build that only had 
> >> > > patch
> >> > >  3/3 applied to the base, and counting kvm_toggle_cache events.)
> >> > > (The number 50 is the number of kvm_toggle_cache events *without* 3/3
> >> > >  applied.)
> >> > >
> >> > > I consider this bad news because, even considering it only goes to EL2,
> >> > > it goes a ton more than it used to. I realize patch 3/3 isn't the final
> >> > > plan for enabling traps though.
> 
> If a full guest boots, can you try timing a kernel compile?
>

Guests boot. I used an 8 vcpu, 14G memory guest; compiled the kernel 4
times inside the guest for each host kernel; base and mair. I dropped
the time from the first run of each set, and captured the other 3.
Command line used below. Time is from the
  Elapsed (wall clock) time (h:mm:ss or m:ss):
output of /usr/bin/time - the host's wall clock.

  /usr/bin/time --verbose ssh $VM 'cd kernel && make -s clean && make -s -j8'

Results:
base: 3:06.11 3:07.00 3:10.93
mair: 3:08.47 3:06.75 3:04.76

So looks like the 3 orders of magnitude greater number of traps
(only to el2) don't impact kernel compiles.

Then I thought I'd be able to quick measure the number of cycles
a trap to el2 takes with this kvm-unit-tests test

int main(void)
{
unsigned long start, end;
unsigned int sctlr;

asm volatile(
"   mrs %0, sctlr_el1\n"
"   msr pmcr_el0, %1\n"
: "=&r" (sctlr) : "r" (5));

asm volatile(
"   mrs %0, pmccntr_el0\n"
"   msr sctlr_el1, %2\n"
"   mrs %1, pmccntr_el0\n"
: "=&r" (start), "=&r" (end) : "r" (sctlr));

printf("%llx\n", end - start);
return 0;
}

after applying this patch to kvm

diff --git a/arch/arm64/kvm/hyp.S b/arch/arm64/kvm/hyp.S
index bb91b6fc63861..5de39d740aa58 100644
--- a/arch/arm64/kvm/hyp.S
+++ b/arch/arm64/kvm/hyp.S
@@ -770,7 +770,7 @@
 
mrs x2, mdcr_el2
and x2, x2, #MDCR_EL2_HPMN_MASK
-   orr x2, x2, #(MDCR_EL2_TPM | MDCR_EL2_TPMCR)
+// orr x2, x2, #(MDCR_EL2_TPM | MDCR_EL2_TPMCR)
orr x2, x2, #(MDCR_EL2_TDRA | MDCR_EL2_TDOSA)
 
// Check for KVM_ARM64_DEBUG_DIRTY, and set debug to trap

But I get zero for the cycle count. Not sure what I'm missing.

drew
--
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: [RFC PATCH v2 13/15] cpu-model/s390: Add processor property routines

2015-02-20 Thread Alexander Graf


On 17.02.15 15:24, Michael Mueller wrote:
> This patch implements the functions:
> 
> - s390_get_proceccor_props()
> - s390_set_proceccor_props()
> 
> They can be used to request or retrieve processor related information from an 
> accelerator.
> That information comprises the cpu identifier, the ICB value and the facility 
> lists.
> 
> Signed-off-by: Michael Mueller 

Hrm, I still seem to miss the point of this interface. What do you need
it for?


Alex
--
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: [RFC PATCH v2 09/15] cpu-model/s390: Add KVM VM attribute interface routines

2015-02-20 Thread Alexander Graf


On 17.02.15 15:24, Michael Mueller wrote:
> The patch implements routines to set and retrieve processor configuration
> data and to retrieve machine configuration data. The machine related data
> is used together with the cpu model facility lists to determine the list of
> supported cpu models of this host. The above mentioned routines have QEMU
> trace point instrumentation.
> 
> Signed-off-by: Michael Mueller 
> ---
>  target-s390x/cpu-models.h |  39 ++
>  target-s390x/kvm.c| 102 
> ++
>  trace-events  |   3 ++
>  3 files changed, 144 insertions(+)
> 
> diff --git a/target-s390x/cpu-models.h b/target-s390x/cpu-models.h
> index 623a7b2..76b3456 100644
> --- a/target-s390x/cpu-models.h
> +++ b/target-s390x/cpu-models.h
> @@ -45,6 +45,45 @@ typedef struct S390CPUAlias {
>  char *model;
>  } S390CPUAlias;
>  
> +typedef struct S390ProcessorProps {
> +uint64_t cpuid;
> +uint16_t ibc;
> +uint8_t  pad[6];
> +uint64_t fac_list[S390_ARCH_FAC_LIST_SIZE_UINT64];
> +} S390ProcessorProps;
> +
> +typedef struct S390MachineProps {
> +uint64_t cpuid;
> +uint32_t ibc_range;
> +uint8_t  pad[4];
> +uint64_t fac_list_mask[S390_ARCH_FAC_LIST_SIZE_UINT64];
> +uint64_t fac_list[S390_ARCH_FAC_LIST_SIZE_UINT64];
> +} S390MachineProps;

What are those structs there for? To convert between a kvm facing
interface to an internal interface?

I don't think they're necessary. The internal layout is visible from the
KVM code. Just either spawn the class straight from the kvm file or if
you consider that ugly, pass the values of that struct that you need as
function parameters to a function in cpu-models.c.


Alex
--
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: [RFC PATCH v2 04/15] cpu-model/s390: Introduce S390 CPU models

2015-02-20 Thread Alexander Graf


On 17.02.15 15:24, Michael Mueller wrote:
> This patch implements the static part of the s390 cpu class definitions.
> It defines s390 cpu models by means of virtual cpu ids (enum) which contain
> information on the cpu generation, the machine class, the GA number and
> the machine type. The cpu id is used to instantiate a cpu class per cpu
> model.
> 
> In addition the patch introduces the QMP enumeration AccelId. It is used
> to index certain cpu model poperties per accelerator.
> 
> Furthermore it extends the existing S390CPUClass by model related properties.
> 
> Signed-off-by: Michael Mueller 
> Reviewed-by: Thomas Huth 
> ---
>  qapi-schema.json   | 11 +++
>  target-s390x/Makefile.objs |  1 +
>  target-s390x/cpu-models.c  | 79 
> ++
>  target-s390x/cpu-models.h  | 71 +
>  target-s390x/cpu-qom.h | 22 +
>  target-s390x/cpu.c |  2 ++
>  6 files changed, 186 insertions(+)
>  create mode 100644 target-s390x/cpu-models.c
>  create mode 100644 target-s390x/cpu-models.h
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index e16f8eb..4d237c8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2473,6 +2473,17 @@
>  ##
>  { 'command': 'query-machines', 'returns': ['MachineInfo'] }
>  
> +
> +##
> +# @AccelId
> +#
> +# Defines accelerator ids
> +#
> +# Since: 2.3.0
> +##
> +{ 'enum': 'AccelId',
> +  'data': ['qtest', 'tcg', 'kvm', 'xen'  ] }
> +
>  ##
>  # @CpuDefinitionInfo:
>  #
> diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
> index 2c57494..9f55140 100644
> --- a/target-s390x/Makefile.objs
> +++ b/target-s390x/Makefile.objs
> @@ -1,5 +1,6 @@
>  obj-y += translate.o helper.o cpu.o interrupt.o
>  obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
>  obj-y += gdbstub.o
> +obj-y += cpu-models.o
>  obj-$(CONFIG_SOFTMMU) += machine.o ioinst.o arch_dump.o
>  obj-$(CONFIG_KVM) += kvm.o
> diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
> new file mode 100644
> index 000..4841553
> --- /dev/null
> +++ b/target-s390x/cpu-models.c
> @@ -0,0 +1,79 @@
> +/*
> + * CPU models for s390
> + *
> + * Copyright 2014,2015 IBM Corp.
> + *
> + * Author(s): Michael Mueller 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +
> +#include "qemu-common.h"
> +#include "cpu-models.h"
> +
> +#define S390_PROC_DEF(_name, _cpu_id, _desc)\
> +static void \
> +glue(_cpu_id, _cpu_class_init)  \
> +(ObjectClass *oc, void *data)   \
> +{   \
> +DeviceClass *dc = DEVICE_CLASS(oc); \
> +S390CPUClass *cc = S390_CPU_CLASS(oc);  \
> +\
> +cc->is_active[ACCEL_ID_KVM] = true; \
> +cc->mach= g_malloc0(sizeof(S390CPUMachineProps));   \
> +cc->mach->ga= cpu_ga(_cpu_id);  \
> +cc->mach->class = cpu_class(_cpu_id);   \
> +cc->mach->order = cpu_order(_cpu_id);   \
> +cc->proc= g_malloc0(sizeof(S390CPUProcessorProps)); \
> +cc->proc->gen   = cpu_generation(_cpu_id);  \
> +cc->proc->ver   = S390_DEF_VERSION; \
> +cc->proc->id= S390_DEF_ID;  \
> +cc->proc->type  = cpu_type(_cpu_id);\
> +cc->proc->ibc   = S390_DEF_IBC; \
> +dc->desc= _desc;\
> +}   \
> +static const TypeInfo   \
> +glue(_cpu_id, _cpu_type_info) = {   \
> +.name   = _name "-" TYPE_S390_CPU,  \
> +.parent = TYPE_S390_CPU,\
> +.class_init = glue(_cpu_id, _cpu_class_init),   \
> +};  \
> +static void \
> +glue(_cpu_id, _cpu_register_types)(void)\
> +{   \
> +type_register_static(   \
> +&glue(_cpu_id, _cpu_type_info));  

Re: [RFC PATCH v2 04/15] cpu-model/s390: Introduce S390 CPU models

2015-02-20 Thread Alexander Graf


On 17.02.15 15:24, Michael Mueller wrote:
> This patch implements the static part of the s390 cpu class definitions.
> It defines s390 cpu models by means of virtual cpu ids (enum) which contain
> information on the cpu generation, the machine class, the GA number and
> the machine type. The cpu id is used to instantiate a cpu class per cpu
> model.
> 
> In addition the patch introduces the QMP enumeration AccelId. It is used
> to index certain cpu model poperties per accelerator.
> 
> Furthermore it extends the existing S390CPUClass by model related properties.
> 
> Signed-off-by: Michael Mueller 
> Reviewed-by: Thomas Huth 
> ---
>  qapi-schema.json   | 11 +++
>  target-s390x/Makefile.objs |  1 +
>  target-s390x/cpu-models.c  | 79 
> ++
>  target-s390x/cpu-models.h  | 71 +
>  target-s390x/cpu-qom.h | 22 +
>  target-s390x/cpu.c |  2 ++
>  6 files changed, 186 insertions(+)
>  create mode 100644 target-s390x/cpu-models.c
>  create mode 100644 target-s390x/cpu-models.h
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index e16f8eb..4d237c8 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2473,6 +2473,17 @@
>  ##
>  { 'command': 'query-machines', 'returns': ['MachineInfo'] }
>  
> +
> +##
> +# @AccelId
> +#
> +# Defines accelerator ids
> +#
> +# Since: 2.3.0
> +##
> +{ 'enum': 'AccelId',
> +  'data': ['qtest', 'tcg', 'kvm', 'xen'  ] }
> +
>  ##
>  # @CpuDefinitionInfo:
>  #
> diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
> index 2c57494..9f55140 100644
> --- a/target-s390x/Makefile.objs
> +++ b/target-s390x/Makefile.objs
> @@ -1,5 +1,6 @@
>  obj-y += translate.o helper.o cpu.o interrupt.o
>  obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
>  obj-y += gdbstub.o
> +obj-y += cpu-models.o
>  obj-$(CONFIG_SOFTMMU) += machine.o ioinst.o arch_dump.o
>  obj-$(CONFIG_KVM) += kvm.o
> diff --git a/target-s390x/cpu-models.c b/target-s390x/cpu-models.c
> new file mode 100644
> index 000..4841553
> --- /dev/null
> +++ b/target-s390x/cpu-models.c
> @@ -0,0 +1,79 @@
> +/*
> + * CPU models for s390
> + *
> + * Copyright 2014,2015 IBM Corp.
> + *
> + * Author(s): Michael Mueller 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +
> +#include "qemu-common.h"
> +#include "cpu-models.h"
> +
> +#define S390_PROC_DEF(_name, _cpu_id, _desc)\
> +static void \
> +glue(_cpu_id, _cpu_class_init)  \
> +(ObjectClass *oc, void *data)   \
> +{   \
> +DeviceClass *dc = DEVICE_CLASS(oc); \
> +S390CPUClass *cc = S390_CPU_CLASS(oc);  \
> +\
> +cc->is_active[ACCEL_ID_KVM] = true; \
> +cc->mach= g_malloc0(sizeof(S390CPUMachineProps));   \
> +cc->mach->ga= cpu_ga(_cpu_id);  \
> +cc->mach->class = cpu_class(_cpu_id);   \
> +cc->mach->order = cpu_order(_cpu_id);   \
> +cc->proc= g_malloc0(sizeof(S390CPUProcessorProps)); \
> +cc->proc->gen   = cpu_generation(_cpu_id);  \
> +cc->proc->ver   = S390_DEF_VERSION; \
> +cc->proc->id= S390_DEF_ID;  \
> +cc->proc->type  = cpu_type(_cpu_id);\
> +cc->proc->ibc   = S390_DEF_IBC; \
> +dc->desc= _desc;\
> +}   \
> +static const TypeInfo   \
> +glue(_cpu_id, _cpu_type_info) = {   \
> +.name   = _name "-" TYPE_S390_CPU,  \
> +.parent = TYPE_S390_CPU,\
> +.class_init = glue(_cpu_id, _cpu_class_init),   \
> +};  \
> +static void \
> +glue(_cpu_id, _cpu_register_types)(void)\
> +{   \
> +type_register_static(   \
> +&glue(_cpu_id, _cpu_type_info));  

[PATCH 1/2] white space formatting in kvm_main.c

2015-02-20 Thread Kevin Mulvey
Better alignment of loop using tabs rather than spaces, this
makes checkpatch.pl happier.

Signed-off-by: Kevin Mulvey 
---
 virt/kvm/kvm_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index a109370..36ab89d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1742,7 +1742,7 @@ int kvm_clear_guest(struct kvm *kvm, gpa_t gpa, unsigned 
long len)
int offset = offset_in_page(gpa);
int ret;
 
-while ((seg = next_segment(len, offset)) != 0) {
+   while ((seg = next_segment(len, offset)) != 0) {
ret = kvm_clear_guest_page(kvm, gfn, offset, seg);
if (ret < 0)
return ret;
-- 
2.1.0

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


[PATCH 2/2] [PATCH] fix checkpatch.pl errors in kvm/irqchip.c

2015-02-20 Thread Kevin Mulvey
Fix whitespace around while

Signed-off-by: Kevin Mulvey 
---
 virt/kvm/irqchip.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/virt/kvm/irqchip.c b/virt/kvm/irqchip.c
index 7f256f3..1d56a90 100644
--- a/virt/kvm/irqchip.c
+++ b/virt/kvm/irqchip.c
@@ -105,7 +105,7 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id, u32 
irq, int level,
i = kvm_irq_map_gsi(kvm, irq_set, irq);
srcu_read_unlock(&kvm->irq_srcu, idx);
 
-   while(i--) {
+   while (i--) {
int r;
r = irq_set[i].set(&irq_set[i], kvm, irq_source_id, level,
   line_status);
-- 
2.1.0

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


[PATCH 3/3] vhost_net: fix virtio_net header endianness

2015-02-20 Thread Greg Kurz
Without this patch, packets are being silently dropped by the tap backend.

Signed-off-by: Greg Kurz 
---
 drivers/vhost/net.c |   32 ++--
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index afa06d2..2923eee 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -288,6 +288,16 @@ static void vhost_zerocopy_callback(struct ubuf_info 
*ubuf, bool success)
rcu_read_unlock_bh();
 }
 
+static void fix_virtio_net_hdr(struct vhost_virtqueue *vq)
+{
+   struct virtio_net_hdr *hdr = vq->iov[0].iov_base;
+
+   hdr->hdr_len = vhost16_to_cpu(vq, hdr->hdr_len);
+   hdr->gso_size = vhost16_to_cpu(vq, hdr->gso_size);
+   hdr->csum_start  = vhost16_to_cpu(vq, hdr->csum_start);
+   hdr->csum_offset = vhost16_to_cpu(vq, hdr->csum_offset);
+}
+
 /* Expects to be always run from workqueue - which acts as
  * read-size critical section for our kind of RCU. */
 static void handle_tx(struct vhost_net *net)
@@ -352,6 +362,10 @@ static void handle_tx(struct vhost_net *net)
   "out %d, int %d\n", out, in);
break;
}
+
+   if (!hdr_size)
+   fix_virtio_net_hdr(vq);
+
/* Skip header. TODO: support TSO. */
len = iov_length(vq->iov, out);
iov_iter_init(&msg.msg_iter, WRITE, vq->iov, out, len);
@@ -609,12 +623,18 @@ static void handle_rx(struct vhost_net *net)
continue;
}
/* Supply virtio_net_hdr if VHOST_NET_F_VIRTIO_NET_HDR */
-   if (unlikely(vhost_hlen) &&
-   copy_to_iter(&hdr, sizeof(hdr), &fixup) != sizeof(hdr)) {
-   vq_err(vq, "Unable to write vnet_hdr at addr %p\n",
-  vq->iov->iov_base);
-   break;
-   }
+   if (unlikely(vhost_hlen)) {
+   size_t len = copy_to_iter(&hdr, sizeof(hdr), &fixup);
+
+   if (len != sizeof(hdr)) {
+   vq_err(vq,
+  "Unable to write vnet_hdr at addr %p\n",
+  vq->iov->iov_base);
+   break;
+   }
+   } else
+   fix_virtio_net_hdr(vq);
+
/* TODO: Should check and handle checksum. */
 
num_buffers = cpu_to_vhost16(vq, headcount);

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


[PATCH 2/3] vhost: add support for legacy virtio

2015-02-20 Thread Greg Kurz
Signed-off-by: Greg Kurz 
---
 drivers/vhost/vhost.h |   20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

Michael,

The vhost_is_little_endian() helper adds unconditionnal overhead to fixed
endian architectures: that is all architectures except arm and ppc64. This
was addressed in QEMU with a TARGET_IS_BIENDIAN macro. Please give an
advice about how to address this in the vhost code.

Thanks.

diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index ce2c68e..21e7d6a 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -176,34 +176,42 @@ static inline bool vhost_has_feature(struct 
vhost_virtqueue *vq, int bit)
return vq->acked_features & (1ULL << bit);
 }
 
+static inline bool vhost_is_little_endian(struct vhost_virtqueue *vq)
+{
+   if (vhost_has_feature(vq, VIRTIO_F_VERSION_1))
+   return true;
+   else
+   return !vq->legacy_big_endian;
+}
+
 /* Memory accessors */
 static inline u16 vhost16_to_cpu(struct vhost_virtqueue *vq, __virtio16 val)
 {
-   return __virtio16_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), 
val);
+   return __virtio16_to_cpu(vhost_is_little_endian(vq), val);
 }
 
 static inline __virtio16 cpu_to_vhost16(struct vhost_virtqueue *vq, u16 val)
 {
-   return __cpu_to_virtio16(vhost_has_feature(vq, VIRTIO_F_VERSION_1), 
val);
+   return __cpu_to_virtio16(vhost_is_little_endian(vq), val);
 }
 
 static inline u32 vhost32_to_cpu(struct vhost_virtqueue *vq, __virtio32 val)
 {
-   return __virtio32_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), 
val);
+   return __virtio32_to_cpu(vhost_is_little_endian(vq), val);
 }
 
 static inline __virtio32 cpu_to_vhost32(struct vhost_virtqueue *vq, u32 val)
 {
-   return __cpu_to_virtio32(vhost_has_feature(vq, VIRTIO_F_VERSION_1), 
val);
+   return __cpu_to_virtio32(vhost_is_little_endian(vq), val);
 }
 
 static inline u64 vhost64_to_cpu(struct vhost_virtqueue *vq, __virtio64 val)
 {
-   return __virtio64_to_cpu(vhost_has_feature(vq, VIRTIO_F_VERSION_1), 
val);
+   return __virtio64_to_cpu(vhost_is_little_endian(vq), val);
 }
 
 static inline __virtio64 cpu_to_vhost64(struct vhost_virtqueue *vq, u64 val)
 {
-   return __cpu_to_virtio64(vhost_has_feature(vq, VIRTIO_F_VERSION_1), 
val);
+   return __cpu_to_virtio64(vhost_is_little_endian(vq), val);
 }
 #endif

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


[PATCH 0/3] vhost_net: support for cross endian guests

2015-02-20 Thread Greg Kurz
Hi,

This patchset allows vhost_net to be used with legacy virtio
when guest and host have a different endianness. It is based
on previous work by Cédric Le Goater:

https://www.mail-archive.com/kvm-ppc@vger.kernel.org/msg09848.html

As suggested by MST:
- the API now asks for a specific format (big endian) instead of the hint
  whether byteswap is needed or not (patch 1)
- rebased on top of the virtio-1 accessors (patch 2)

Patch 3 is a separate fix: I think it is also valid for virtio-1.

Please comment.

---

Greg Kurz (3):
  vhost: add VHOST_VRING_F_LEGACY_BIG_ENDIAN flag
  vhost: add support for legacy virtio
  vhost_net: fix virtio_net header endianness


 drivers/vhost/net.c|   32 ++--
 drivers/vhost/vhost.c  |6 +-
 drivers/vhost/vhost.h  |   23 +--
 include/uapi/linux/vhost.h |2 ++
 4 files changed, 50 insertions(+), 13 deletions(-)

--
Greg

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


[PATCH 1/3] vhost: add VHOST_VRING_F_LEGACY_BIG_ENDIAN flag

2015-02-20 Thread Greg Kurz
The VHOST_VRING_F_LEGACY_BIG_ENDIAN flag informs the kernel that the
associated device is big endian. Of course, this only makes sense for
legacy virtio devices since modern virtio devices are always little
endian.

It will be used by the vhost memory accessors to byteswap vring data when
we have a legacy device, in case host and guest endianness differ.

Signed-off-by: Greg Kurz 
---
 drivers/vhost/vhost.c  |6 +-
 drivers/vhost/vhost.h  |3 +++
 include/uapi/linux/vhost.h |2 ++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 2ee2826..dad3c37 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -199,6 +199,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->call = NULL;
vq->log_ctx = NULL;
vq->memory = NULL;
+   vq->legacy_big_endian = false;
 }
 
 static int vhost_worker(void *data)
@@ -701,7 +702,8 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void 
__user *argp)
r = -EFAULT;
break;
}
-   if (a.flags & ~(0x1 << VHOST_VRING_F_LOG)) {
+   if (a.flags & ~(0x1 << VHOST_VRING_F_LOG|
+   0x1 << VHOST_VRING_F_LEGACY_BIG_ENDIAN)) {
r = -EOPNOTSUPP;
break;
}
@@ -751,6 +753,8 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void 
__user *argp)
vq->avail = (void __user *)(unsigned long)a.avail_user_addr;
vq->log_addr = a.log_guest_addr;
vq->used = (void __user *)(unsigned long)a.used_user_addr;
+   vq->legacy_big_endian =
+   !!(a.flags & (0x1 << VHOST_VRING_F_LEGACY_BIG_ENDIAN));
break;
case VHOST_SET_VRING_KICK:
if (copy_from_user(&f, argp, sizeof f)) {
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 8c1c792..ce2c68e 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -106,6 +106,9 @@ struct vhost_virtqueue {
/* Log write descriptors */
void __user *log_base;
struct vhost_log *log;
+
+   /* We need to know the device endianness with legacy virtio. */
+   bool legacy_big_endian;
 };
 
 struct vhost_dev {
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index bb6a5b4..0bf4491 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -34,6 +34,8 @@ struct vhost_vring_addr {
/* Flag values: */
/* Whether log address is valid. If set enables logging. */
 #define VHOST_VRING_F_LOG 0
+   /* Whether we have a big-endian legacy virtio device. */
+#define VHOST_VRING_F_LEGACY_BIG_ENDIAN 1
 
/* Start of array of descriptors (virtually contiguous) */
__u64 desc_user_addr;

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