Re: [PATCH 0/7] kvmtool: Cleanup kernel loading

[Will Deacon]

On Wed, Nov 18, 2015 at 10:29:30AM +, Andre Przywara wrote:
> On 02/11/15 14:58, Will Deacon wrote:
> > On Fri, Oct 30, 2015 at 06:26:53PM +, Andre Przywara wrote:
> >> this series cleans up kvmtool's kernel loading functionality a bit.
> >> It has been broken out of a previous series I sent [1] and contains
> >> just the cleanup and bug fix parts, which should be less controversial
> >> and thus easier to merge ;-)
> >> I will resend the pipe loading part later on as a separate series.
> >>
> >> The first patch properly abstracts kernel loading to move
> >> responsibility into each architecture's code. It removes quite some
> >> ugly code from the generic kvm.c file.
> >> The later patches address the naive usage of read(2) to, well, read
> >> data from files. Doing this without coping with the subtleties of
> >> the UNIX read semantics (returning with less or none data read is not
> >> an error) can provoke hard to debug failures.
> >> So these patches make use of the existing and one new wrapper function
> >> to make sure we read everything we actually wanted to.
> >> The last patch moves the ARM kernel loading code into the proper
> >> location to be in line with the other architectures.
> >>
> >> Please have a look and give some comments!
> > 
> > Looks good to me, but I'd like to see some comments from some mips/ppc/x86
> > people on the changes you're making over there.
> 
> Sounds reasonable, but no answers yet.
> 
> Can you take at least patch 1 and 2 meanwhile, preferably 6 and 7 (the
> ARM parts) also if you are OK with it?
> I have other patches that depend on 1/7 and 2/7, so having them upstream
> would help me and reduce further dependency churn.
> I am happy to resend the remaining patches for further discussion later.

We let them sit on the list for a while with no comments, so I just pushed
out your series. If a bug report shows up, then we can always revert the
offending patch if there's no quick fix.

Will
--
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 0/7] kvmtool: Cleanup kernel loading

[Will Deacon]

On Wed, Nov 18, 2015 at 10:29:30AM +, Andre Przywara wrote:
> On 02/11/15 14:58, Will Deacon wrote:
> > On Fri, Oct 30, 2015 at 06:26:53PM +, Andre Przywara wrote:
> >> this series cleans up kvmtool's kernel loading functionality a bit.
> >> It has been broken out of a previous series I sent [1] and contains
> >> just the cleanup and bug fix parts, which should be less controversial
> >> and thus easier to merge ;-)
> >> I will resend the pipe loading part later on as a separate series.
> >>
> >> The first patch properly abstracts kernel loading to move
> >> responsibility into each architecture's code. It removes quite some
> >> ugly code from the generic kvm.c file.
> >> The later patches address the naive usage of read(2) to, well, read
> >> data from files. Doing this without coping with the subtleties of
> >> the UNIX read semantics (returning with less or none data read is not
> >> an error) can provoke hard to debug failures.
> >> So these patches make use of the existing and one new wrapper function
> >> to make sure we read everything we actually wanted to.
> >> The last patch moves the ARM kernel loading code into the proper
> >> location to be in line with the other architectures.
> >>
> >> Please have a look and give some comments!
> > 
> > Looks good to me, but I'd like to see some comments from some mips/ppc/x86
> > people on the changes you're making over there.
> 
> Sounds reasonable, but no answers yet.
> 
> Can you take at least patch 1 and 2 meanwhile, preferably 6 and 7 (the
> ARM parts) also if you are OK with it?
> I have other patches that depend on 1/7 and 2/7, so having them upstream
> would help me and reduce further dependency churn.
> I am happy to resend the remaining patches for further discussion later.

We let them sit on the list for a while with no comments, so I just pushed
out your series. If a bug report shows up, then we can always revert the
offending patch if there's no quick fix.

Will
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" 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] [PATCH v3 1/3] target-i386: add pkeys support for cpuid handling

[Eduardo Habkost]

On Wed, Nov 18, 2015 at 10:20:15AM +0800, Huaitong Han wrote:
[...]
> @@ -408,6 +420,13 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] 
> = {
>  .cpuid_reg = R_EBX,
>  .tcg_features = TCG_7_0_EBX_FEATURES,
>  },
> +[FEAT_7_0_ECX] = {
> +.feat_names = cpuid_7_0_ecx_feature_name,
> +.cpuid_eax = 7,
> +.cpuid_needs_ecx = true, .cpuid_ecx = 0,
> +.cpuid_reg = R_ECX,
> +.tcg_features = TCG_7_0_ECX_FEATURES,
> +},

The patch looks good, but when we add the feature names to
cpuid_7_0_ecx_feature_name, QEMU will consider them as
migratable, but they are truly migratable only after we add the
ext_save_areas entry.

We can fix this by moving cpuid_7_0_ecx_feature_name to patch 2/3
(or to a separate patch, to be applied after 2/3).

-- 
Eduardo
--
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] kvmtool: add support for modern virtio-pci

[Sasha Levin]

On 11/18/2015 03:22 AM, Gerd Hoffmann wrote:
> /me goes undust the kvmtool patches for seabios.
> 
> (see https://www.kraxel.org/cgit/seabios/commit/?h=kvmtool,
> build with CONFIG_KVMTOOL=y + CONFIG_DEBUG_LEVEL=9)
> 
> nilsson kraxel ~# ~kraxel/projects/kvmtool/lkvm run --name seabios
> --firmware /home/kraxel/projects/seabios/out-bios-kvmtool/bios.bin
> --disk /vmdisk/cloud/persistent/Fedora-Cloud-Base-22-20150521.x86_64.raw
>   # lkvm run -k /boot/vmlinuz-3.10.0-324.el7.x86_64 -m 448 -c 4 --name
> seabios

Thanks for testing! I didn't even thing about seabios as a testing target.

I tried to do what you described, and built seabios with:

$ cat .config | grep 'KVMTOOL\|DEBUG'
CONFIG_KVMTOOL=y
CONFIG_DEBUG_LEVEL=9

But when booting, it just hangs on:

$ ./lkvm run --firmware ~/seabios/out/bios.bin -d dummy
  # lkvm run -k /boot/vmlinuz-4.2.0-17-generic -m 448 -c 4 --name guest-12566


And same result with --virtio-legacy...

What did I miss?


Thanks,
Sasha
--
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 1/1] KVM: PPC: Increase memslots to 320

[Thomas Huth]

On 04/11/15 10:03, Thomas Huth wrote:
> Only using 32 memslots for KVM on powerpc is way too low, you can
> nowadays hit this limit quite fast by adding a couple of PCI devices
> and/or pluggable memory DIMMs to the guest.
> x86 already increased the limit to 512 in total, to satisfy 256
> pluggable DIMM slots, 3 private slots and 253 slots for other things
> like PCI devices. On powerpc, we only have 32 pluggable DIMMs in
> QEMU, not 256, so we likely do not as much slots as on x86. Thus
> setting the slot limit to 320 sounds like a good value for the
> time being (until we have some code in the future to resize the
> memslot array dynamically).
> And while we're at it, also remove the KVM_MEM_SLOTS_NUM definition
> from the powerpc-specific header since this gets defined in the
> generic kvm_host.h header anyway.
> 
> Signed-off-by: Thomas Huth 
> ---
>  arch/powerpc/include/asm/kvm_host.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/kvm_host.h 
> b/arch/powerpc/include/asm/kvm_host.h
> index 887c259..89d387a 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -38,8 +38,7 @@
>  
>  #define KVM_MAX_VCPUSNR_CPUS
>  #define KVM_MAX_VCORES   NR_CPUS
> -#define KVM_USER_MEM_SLOTS 32
> -#define KVM_MEM_SLOTS_NUM KVM_USER_MEM_SLOTS
> +#define KVM_USER_MEM_SLOTS 320
>  
>  #ifdef CONFIG_KVM_MMIO
>  #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
> 

Ping! ... any comments on this one?

 Thomas

--
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] [PATCH v3 0/3] target-i386: add memory protection-key support

[Eduardo Habkost]

On Wed, Nov 18, 2015 at 10:20:14AM +0800, Huaitong Han wrote:
> Changes in v3:
> *Fix cpuid_7_0_ecx_feature_name error.
> 
> Changes in v2:
> *Fix memcpy error for xsave state.
> *Fix TCG_7_0_ECX_FEATURES to 0.
> *Make subjects more readable.
> 
> The protection-key feature provides an additional mechanism by which IA-32e
> paging controls access to usermode addresses.
> 
> Hardware support for protection keys for user pages is enumerated with CPUID
> feature flag CPUID.7.0.ECX[3]:PKU. Software support is CPUID.7.0.ECX[4]:OSPKE
> with the setting of CR4.PKE(bit 22).
> 
> The PKRU register is XSAVE-managed state CPUID.D.0.EAX[9], the size of XSAVE
> state component for PKRU is 8 bytes, the offset is 0xa80.

Is every CPU supporting PKU guaranteed to have
CPUID.(EAX=0DH,ECX=9):EBX = 0xa80? Where is the PKRU state
offset/layout documented?

> 
> The specification of Protection Keys can be found at SDM (4.6.2, volume 3)
> http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf.
> 
[...]

-- 
Eduardo
--
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 1/3] x86/cpu: Unify CPU family, model, stepping calculation

[Borislav Petkov]

On Wed, Nov 18, 2015 at 12:35:24PM +0100, Paolo Bonzini wrote:
> Yes, exactly.  I'm suggesting that the same applies to x86_vendor().  I
> also prefer x86_cpuid_* to x86_*_cpuid because, once you add two
> functions in the same family it's nice that they share a prefix.

Ok, makes sense:

---
commit 804437270083a1aabf87f65f65b32e2ae23121b6
Author: Borislav Petkov 
Date:   Sat Nov 14 10:54:22 2015 +0100

x86/cpu: Unify CPU family, model, stepping calculation

Add generic functions which calc family, model and stepping from the
CPUID_1.EAX leaf and stick them into the library we have.

Rename those which do call CPUID with the prefix "x86_cpuid" as
suggested by Paolo Bonzini.

No functionality change.

Signed-off-by: Borislav Petkov 

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index bf2caa1dedc5..678637ad7476 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -36,4 +36,7 @@ extern int _debug_hotplug_cpu(int cpu, int action);
 
 int mwait_usable(const struct cpuinfo_x86 *);
 
+unsigned int x86_family(unsigned int sig);
+unsigned int x86_model(unsigned int sig);
+unsigned int x86_stepping(unsigned int sig);
 #endif /* _ASM_X86_CPU_H */
diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index 34e62b1dcfce..1e1b07a5a738 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -1,6 +1,7 @@
 #ifndef _ASM_X86_MICROCODE_H
 #define _ASM_X86_MICROCODE_H
 
+#include 
 #include 
 
 #define native_rdmsr(msr, val1, val2)  \
@@ -95,14 +96,14 @@ static inline void __exit exit_amd_microcode(void) {}
 
 /*
  * In early loading microcode phase on BSP, boot_cpu_data is not set up yet.
- * x86_vendor() gets vendor id for BSP.
+ * x86_cpuid_vendor() gets vendor id for BSP.
  *
  * In 32 bit AP case, accessing boot_cpu_data needs linear address. To simplify
- * coding, we still use x86_vendor() to get vendor id for AP.
+ * coding, we still use x86_cpuid_vendor() to get vendor id for AP.
  *
- * x86_vendor() gets vendor information directly from CPUID.
+ * x86_cpuid_vendor() gets vendor information directly from CPUID.
  */
-static inline int x86_vendor(void)
+static inline int x86_cpuid_vendor(void)
 {
u32 eax = 0x;
u32 ebx, ecx = 0, edx;
@@ -118,40 +119,14 @@ static inline int x86_vendor(void)
return X86_VENDOR_UNKNOWN;
 }
 
-static inline unsigned int __x86_family(unsigned int sig)
-{
-   unsigned int x86;
-
-   x86 = (sig >> 8) & 0xf;
-
-   if (x86 == 0xf)
-   x86 += (sig >> 20) & 0xff;
-
-   return x86;
-}
-
-static inline unsigned int x86_family(void)
+static inline unsigned int x86_cpuid_family(void)
 {
u32 eax = 0x0001;
u32 ebx, ecx = 0, edx;
 
native_cpuid(, , , );
 
-   return __x86_family(eax);
-}
-
-static inline unsigned int x86_model(unsigned int sig)
-{
-   unsigned int x86, model;
-
-   x86 = __x86_family(sig);
-
-   model = (sig >> 4) & 0xf;
-
-   if (x86 == 0x6 || x86 == 0xf)
-   model += ((sig >> 16) & 0xf) << 4;
-
-   return model;
+   return x86_family(eax);
 }
 
 #ifdef CONFIG_MICROCODE
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 4ddd780aeac9..c311b51efe15 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -582,14 +582,9 @@ void cpu_detect(struct cpuinfo_x86 *c)
u32 junk, tfms, cap0, misc;
 
cpuid(0x0001, , , , );
-   c->x86 = (tfms >> 8) & 0xf;
-   c->x86_model = (tfms >> 4) & 0xf;
-   c->x86_mask = tfms & 0xf;
-
-   if (c->x86 == 0xf)
-   c->x86 += (tfms >> 20) & 0xff;
-   if (c->x86 >= 0x6)
-   c->x86_model += ((tfms >> 16) & 0xf) << 4;
+   c->x86  = x86_family(tfms);
+   c->x86_model= x86_model(tfms);
+   c->x86_mask = x86_stepping(tfms);
 
if (cap0 & (1<<19)) {
c->x86_clflush_size = ((misc >> 8) & 0xff) * 8;
diff --git a/arch/x86/kernel/cpu/microcode/core.c 
b/arch/x86/kernel/cpu/microcode/core.c
index 7fc27f1cca58..3aaffb601c91 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -129,8 +129,8 @@ void __init load_ucode_bsp(void)
if (!have_cpuid_p())
return;
 
-   vendor = x86_vendor();
-   family = x86_family();
+   vendor = x86_cpuid_vendor();
+   family = x86_cpuid_family();
 
switch (vendor) {
case X86_VENDOR_INTEL:
@@ -165,8 +165,8 @@ void load_ucode_ap(void)
if (!have_cpuid_p())
return;
 
-   vendor = x86_vendor();
-   family = x86_family();
+   vendor = x86_cpuid_vendor();
+   family = x86_cpuid_family();
 
switch (vendor) {
case 

Re: [RFC] kvmtool: add support for modern virtio-pci

[Gerd Hoffmann]

  Hi,

> Thanks for testing! I didn't even thing about seabios as a testing target.

Not surprising, support isn't upstream, ran into a bunch of issues[1][2]
last time I tried to combine the two, ran into some issues and nobody
seemed to care, so the seabios patches where just sitting in a branch in
my repo ...

> $ cat .config | grep 'KVMTOOL\|DEBUG'
> CONFIG_KVMTOOL=y
> CONFIG_DEBUG_LEVEL=9

Hmm, 'CONFIG_KVMTOOL=y > .config; make olddefconfig' should give you a
working configuration.

Setting 'CONFIG_DEBUG_LEVEL=9' is useful for trouble-shooting as it
makes the virtio drivers more verbose, but not mandatory to have.

Serial line support is needed to get output:

CONFIG_DEBUG_SERIAL=y
CONFIG_DEBUG_SERIAL_PORT=0x3f8

Also I think rom size must be 128k b/c kvmtool expects it to be that
way:

CONFIG_ROM_SIZE=128

But those are the defaults, and after "make olddefconfig" you should
already have them ...

cheers,
  Gerd

[1] kernel doesn't find pci (can be worked around by tweaking kernel
command line in boot loader config).
[2] kernel virtio drivers fail to initialize (probably device reset
not working properly).


--
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 v8 0/5] implement vNVDIMM

[Eduardo Habkost]

On Wed, Nov 18, 2015 at 09:59:34AM +0800, Xiao Guangrong wrote:
> 
> Ping...
> 
> Do you have any comment on this patchset? Could it be applied to somewhere
> if it is okay for you?

I have no additional comments, as the memory-backend patches I
was reviewing are not included in this version. I didn't take the
time to review the TYPE_NVDIMM and ACPI changes.

-- 
Eduardo
--
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 v8 0/5] implement vNVDIMM

[Michael S. Tsirkin]

On Wed, Nov 18, 2015 at 05:18:17PM -0200, Eduardo Habkost wrote:
> On Wed, Nov 18, 2015 at 09:59:34AM +0800, Xiao Guangrong wrote:
> > 
> > Ping...
> > 
> > Do you have any comment on this patchset? Could it be applied to somewhere
> > if it is okay for you?
> 
> I have no additional comments, as the memory-backend patches I
> was reviewing are not included in this version. I didn't take the
> time to review the TYPE_NVDIMM and ACPI changes.

No, I don't think the way guest memory is allocated here is ok.  I'm
sorry, I'm busy with 2.5 now, and this is clearly not 2.5 material.
Once that's out, I'll post some suggestions.

Thanks!

> -- 
> Eduardo
--
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 v3 7/7] KVM, pkeys: disable PKU feature without ept

[Paolo Bonzini]

On 18/11/2015 06:44, Huaitong Han wrote:
> This patch disables CPUID:PKU without ept, becase pkeys is not supported
> with softmmu.

Sure, but _what_ makes it impossible to support pkeys with shadow pages?

Is it enough to add the pkey bits to the role (and then to
kvm_get_mmu_page, mmu_set_spte, set_spte) or are there fundamental problems?

The trick to handle !CR0.WP in FNAME(page_fault) (search for
"walker.pte_access &= ~ACC_USER_MASK"; it's documented in
Documentation/virtual/kvm/mmu.txt as well) should work for PKRU.

If you just want me to change it to "is not yet implemented for shadow
paging", I can do that.

Thanks,

Paolo

> Signed-off-by: Huaitong Han 
--
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] KVM: powerpc: kvmppc_visible_gpa can be boolean

[Paolo Bonzini]

On 16/11/2015 04:10, Yaowei Bai wrote:
> In another patch kvm_is_visible_gfn is maken return bool due to this
> function only returns zero or one as its return value, let's also make
> kvmppc_visible_gpa return bool to keep consistent.
> 
> No functional change.
> 
> Signed-off-by: Yaowei Bai 
> ---
>  arch/powerpc/kvm/book3s_pr.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> index 64891b0..70fb08d 100644
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c
> @@ -512,7 +512,7 @@ static void kvmppc_patch_dcbz(struct kvm_vcpu *vcpu, 
> struct kvmppc_pte *pte)
>   put_page(hpage);
>  }
>  
> -static int kvmppc_visible_gpa(struct kvm_vcpu *vcpu, gpa_t gpa)
> +static bool kvmppc_visible_gpa(struct kvm_vcpu *vcpu, gpa_t gpa)
>  {
>   ulong mp_pa = vcpu->arch.magic_page_pa;
>  
> @@ -521,7 +521,7 @@ static int kvmppc_visible_gpa(struct kvm_vcpu *vcpu, 
> gpa_t gpa)
>  
>   gpa &= ~0xFFFULL;
>   if (unlikely(mp_pa) && unlikely((mp_pa & KVM_PAM) == (gpa & KVM_PAM))) {
> - return 1;
> + return true;
>   }
>  
>   return kvm_is_visible_gfn(vcpu->kvm, gpa >> PAGE_SHIFT);
> 

Applied all three patches to kvm/queue, 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: [PATCH 0/7] kvmtool: Cleanup kernel loading

[Andre Przywara]

Hi Will,

On 02/11/15 14:58, Will Deacon wrote:
> On Fri, Oct 30, 2015 at 06:26:53PM +, Andre Przywara wrote:
>> Hi,
> 
> Hello Andre,
> 
>> this series cleans up kvmtool's kernel loading functionality a bit.
>> It has been broken out of a previous series I sent [1] and contains
>> just the cleanup and bug fix parts, which should be less controversial
>> and thus easier to merge ;-)
>> I will resend the pipe loading part later on as a separate series.
>>
>> The first patch properly abstracts kernel loading to move
>> responsibility into each architecture's code. It removes quite some
>> ugly code from the generic kvm.c file.
>> The later patches address the naive usage of read(2) to, well, read
>> data from files. Doing this without coping with the subtleties of
>> the UNIX read semantics (returning with less or none data read is not
>> an error) can provoke hard to debug failures.
>> So these patches make use of the existing and one new wrapper function
>> to make sure we read everything we actually wanted to.
>> The last patch moves the ARM kernel loading code into the proper
>> location to be in line with the other architectures.
>>
>> Please have a look and give some comments!
> 
> Looks good to me, but I'd like to see some comments from some mips/ppc/x86
> people on the changes you're making over there.

Sounds reasonable, but no answers yet.

Can you take at least patch 1 and 2 meanwhile, preferably 6 and 7 (the
ARM parts) also if you are OK with it?
I have other patches that depend on 1/7 and 2/7, so having them upstream
would help me and reduce further dependency churn.
I am happy to resend the remaining patches for further discussion later.

Cheers,
Andre.
--
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] KVM-async_pf: Delete an unnecessary check before the function call "kmem_cache_destroy"

[Paolo Bonzini]

On 15/11/2015 10:45, SF Markus Elfring wrote:
> From: Markus Elfring 
> Date: Sun, 15 Nov 2015 10:40:36 +0100
> 
> The kmem_cache_destroy() function tests whether its argument is NULL
> and then returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring 
> ---
>  virt/kvm/async_pf.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c
> index 77d42be..3531599 100644
> --- a/virt/kvm/async_pf.c
> +++ b/virt/kvm/async_pf.c
> @@ -57,8 +57,7 @@ int kvm_async_pf_init(void)
>  
>  void kvm_async_pf_deinit(void)
>  {
> - if (async_pf_cache)
> - kmem_cache_destroy(async_pf_cache);
> + kmem_cache_destroy(async_pf_cache);
>   async_pf_cache = NULL;
>  }
>  
> 

Applied to kvm/queue, 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: [PATCH] KVM: powerpc: kvmppc_visible_gpa can be boolean

[Paolo Bonzini]

On 16/11/2015 04:10, Yaowei Bai wrote:
> In another patch kvm_is_visible_gfn is maken return bool due to this
> function only returns zero or one as its return value, let's also make
> kvmppc_visible_gpa return bool to keep consistent.
> 
> No functional change.
> 
> Signed-off-by: Yaowei Bai 
> ---
>  arch/powerpc/kvm/book3s_pr.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> index 64891b0..70fb08d 100644
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c
> @@ -512,7 +512,7 @@ static void kvmppc_patch_dcbz(struct kvm_vcpu *vcpu, 
> struct kvmppc_pte *pte)
>   put_page(hpage);
>  }
>  
> -static int kvmppc_visible_gpa(struct kvm_vcpu *vcpu, gpa_t gpa)
> +static bool kvmppc_visible_gpa(struct kvm_vcpu *vcpu, gpa_t gpa)
>  {
>   ulong mp_pa = vcpu->arch.magic_page_pa;
>  
> @@ -521,7 +521,7 @@ static int kvmppc_visible_gpa(struct kvm_vcpu *vcpu, 
> gpa_t gpa)
>  
>   gpa &= ~0xFFFULL;
>   if (unlikely(mp_pa) && unlikely((mp_pa & KVM_PAM) == (gpa & KVM_PAM))) {
> - return 1;
> + return true;
>   }
>  
>   return kvm_is_visible_gfn(vcpu->kvm, gpa >> PAGE_SHIFT);
> 

Applied all three patches to kvm/queue, thanks.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" 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 1/3] x86/cpu: Unify CPU family, model, stepping calculation

[Paolo Bonzini]

On 18/11/2015 12:28, Borislav Petkov wrote:
>> On 14/11/2015 11:37, Borislav Petkov wrote:
>>> > > vendor = x86_vendor();
>>> > > -   family = x86_family();
>>> > > +   family = x86_family_cpuid();
>> > 
>> > What about renaming x86_vendor() so that this looks like
>> > 
>> > -  vendor = x86_vendor();
>> > -  family = x86_family();
>> > +  vendor = x86_cpuid_vendor();
>> > +  family = x86_cpuid_family();
> 
> The idea is that x86_family_cpuid() gives the family *after* having
> executed CPUID while x86_family() only computes the family from a
> supplied CPUID_1_EAX. I.e., the last saves us the CPUID call.

Yes, exactly.  I'm suggesting that the same applies to x86_vendor().  I
also prefer x86_cpuid_* to x86_*_cpuid because, once you add two
functions in the same family it's nice that they share a prefix.

Paolo

> Hmm, maybe I should make that more clear ...
--
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 1/3] x86/cpu: Unify CPU family, model, stepping calculation

[Borislav Petkov]

On Wed, Nov 18, 2015 at 12:10:08PM +0100, Paolo Bonzini wrote:
> On 14/11/2015 11:37, Borislav Petkov wrote:
> > vendor = x86_vendor();
> > -   family = x86_family();
> > +   family = x86_family_cpuid();
> 
> What about renaming x86_vendor() so that this looks like
> 
> - vendor = x86_vendor();
> - family = x86_family();
> + vendor = x86_cpuid_vendor();
> + family = x86_cpuid_family();

The idea is that x86_family_cpuid() gives the family *after* having
executed CPUID while x86_family() only computes the family from a
supplied CPUID_1_EAX. I.e., the last saves us the CPUID call.

Hmm, maybe I should make that more clear ...

-- 
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 1/2] KVM: Remove unnecessary debugfs dentry references

[Christian Borntraeger]

From: Janosch Frank 

KVM creates debugfs files to export VM statistics to userland. To be
able to remove them on kvm exit it tracks the files' dentries.

Since their parent directory is also tracked and since each parent
direntry knows its children we can easily remove them by using
debugfs_remove_recursive(kvm_debugfs_dir). Therefore we don't
need the extra tracking in the kvm_stats_debugfs_item anymore.

Signed-off-by: Janosch Frank 
Reviewed-by: Sascha Silbe 
Acked-by: Christian Borntraeger 
Signed-off-by: Christian Borntraeger 
---
 include/linux/kvm_host.h |  1 -
 virt/kvm/kvm_main.c  | 18 --
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 7821137..6981bc6 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1009,7 +1009,6 @@ struct kvm_stats_debugfs_item {
const char *name;
int offset;
enum kvm_stat_kind kind;
-   struct dentry *dentry;
 };
 extern struct kvm_stats_debugfs_item debugfs_entries[];
 extern struct dentry *kvm_debugfs_dir;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 244c9b4..726bb51 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3447,10 +3447,9 @@ static int kvm_init_debug(void)
goto out;
 
for (p = debugfs_entries; p->name; ++p) {
-   p->dentry = debugfs_create_file(p->name, 0444, kvm_debugfs_dir,
-   (void *)(long)p->offset,
-   stat_fops[p->kind]);
-   if (p->dentry == NULL)
+   if (!debugfs_create_file(p->name, 0444, kvm_debugfs_dir,
+(void *)(long)p->offset,
+stat_fops[p->kind]))
goto out_dir;
}
 
@@ -3462,15 +3461,6 @@ out:
return r;
 }
 
-static void kvm_exit_debug(void)
-{
-   struct kvm_stats_debugfs_item *p;
-
-   for (p = debugfs_entries; p->name; ++p)
-   debugfs_remove(p->dentry);
-   debugfs_remove(kvm_debugfs_dir);
-}
-
 static int kvm_suspend(void)
 {
if (kvm_usage_count)
@@ -3628,7 +3618,7 @@ EXPORT_SYMBOL_GPL(kvm_init);
 
 void kvm_exit(void)
 {
-   kvm_exit_debug();
+   debugfs_remove_recursive(kvm_debugfs_dir);
misc_deregister(_dev);
kmem_cache_destroy(kvm_vcpu_cache);
kvm_async_pf_deinit();
-- 
2.3.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 0/2] kvm: provide kvm stat per process

[Christian Borntraeger]

Paolo,

I plan to submit these patches via my next s390 pull request.
Basic idea is: we already have all kvm_stats per cpu and per
vm, lets provide some additional debugfs folders to get
kvm_stats also per VM to detect cases where only one guest
goes wild. (no per vCPU stats yet. Do we want those as well?)

Can you review and ack/nack? (Or as an alternative take the
patches yourself)


Janosch Frank (2):
  KVM: Remove unnecessary debugfs dentry references
  KVM: Create debugfs dir and stat files for each VM

 include/linux/kvm_host.h |   8 +-
 virt/kvm/kvm_main.c  | 206 ++-
 2 files changed, 191 insertions(+), 23 deletions(-)

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

Re: [RFC PATCH 1/3] x86/cpu: Unify CPU family, model, stepping calculation

[Paolo Bonzini]

On 14/11/2015 11:37, Borislav Petkov wrote:
>   vendor = x86_vendor();
> - family = x86_family();
> + family = x86_family_cpuid();

What about renaming x86_vendor() so that this looks like

-   vendor = x86_vendor();
-   family = x86_family();
+   vendor = x86_cpuid_vendor();
+   family = x86_cpuid_family();

Otherwise looks good.

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

[PATCH 2/2] KVM: Create debugfs dir and stat files for each VM

[Christian Borntraeger]

From: Janosch Frank 

KVM statistics for VMs (no. of exits, halts and other special
instructions) are currently only available in a summarized manner for
all VMs. They are exported to userland through files in the kvm
debugfs directory and used for performance monitoring, as well as VM
problem detection with helper tools like kvm_stat. If a VM has
problems and therefore creates a large number of exits, one can not
easily find out which one it is, as there is no VM specific data.

This patch adds a kvm debugfs subdirectory for each VM on
kvm_create_vm(). The subdirectories are named by the VM pid and
contain the same type of exported statistics that are already in the
kvm debugfs directory, but the exported data is now VM specific.

Signed-off-by: Janosch Frank 
Reviewed-by: Pierre Morel 
Acked-by: Christian Borntraeger 
Signed-off-by: Christian Borntraeger 
---
 include/linux/kvm_host.h |   7 ++
 virt/kvm/kvm_main.c  | 188 +--
 2 files changed, 187 insertions(+), 8 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 6981bc6..23071bc 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -425,6 +425,8 @@ struct kvm {
 #endif
long tlbs_dirty;
struct list_head devices;
+   struct dentry *debugfs_dentry;
+   struct kvm_stat_data **debugfs_data;
 };
 
 #define kvm_err(fmt, ...) \
@@ -1005,6 +1007,11 @@ enum kvm_stat_kind {
KVM_STAT_VCPU,
 };
 
+struct kvm_stat_data {
+   int offset;
+   struct kvm *kvm;
+};
+
 struct kvm_stats_debugfs_item {
const char *name;
int offset;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 726bb51..1ee2f73 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -63,6 +63,9 @@
 #define CREATE_TRACE_POINTS
 #include 
 
+/* Worst case buffer size needed for holding an integer. */
+#define ITOA_MAX_LEN 12
+
 MODULE_AUTHOR("Qumranet");
 MODULE_LICENSE("GPL");
 
@@ -100,6 +103,9 @@ static __read_mostly struct preempt_ops kvm_preempt_ops;
 struct dentry *kvm_debugfs_dir;
 EXPORT_SYMBOL_GPL(kvm_debugfs_dir);
 
+static u64 kvm_debugfs_num_entries;
+static const struct file_operations *stat_fops_per_vm[];
+
 static long kvm_vcpu_ioctl(struct file *file, unsigned int ioctl,
   unsigned long arg);
 #ifdef CONFIG_KVM_COMPAT
@@ -539,6 +545,71 @@ static void kvm_free_memslots(struct kvm *kvm, struct 
kvm_memslots *slots)
kvfree(slots);
 }
 
+static int kvm_destroy_vm_debugfs(struct kvm *kvm)
+{
+   u64 i;
+   struct kvm_stat_data **stat_data = kvm->debugfs_data;
+
+   for (i = 0; i < kvm_debugfs_num_entries; i++)
+   kfree(stat_data[i]);
+
+   kfree(kvm->debugfs_data);
+
+   return 0;
+}
+
+static int kvm_create_vm_debugfs(struct kvm *kvm)
+{
+   int r = 0, i = 0;
+   char dir_name[ITOA_MAX_LEN];
+   struct kvm_stat_data *stat_data;
+   struct kvm_stats_debugfs_item *p;
+
+   if (!kvm)
+   return -EINVAL;
+
+   snprintf(dir_name, sizeof(dir_name), "%d", current->pid);
+   kvm->debugfs_dentry = debugfs_create_dir(dir_name, kvm_debugfs_dir);
+   if (!kvm->debugfs_dentry)
+   goto out_err;
+
+   kvm->debugfs_data = kmalloc(sizeof(*kvm->debugfs_data) *
+   kvm_debugfs_num_entries, GFP_KERNEL);
+   if (!kvm->debugfs_data)
+   return -ENOMEM;
+
+   for (p = debugfs_entries; p->name; p++, i++) {
+   stat_data = kzalloc(sizeof(*stat_data), GFP_KERNEL);
+   if (!stat_data) {
+   r = -ENOMEM;
+   goto out_err_clean;
+   }
+
+   stat_data->offset = p->offset;
+   stat_data->kvm = kvm;
+   if (!debugfs_create_file(p->name, 0444,
+kvm->debugfs_dentry,
+stat_data,
+stat_fops_per_vm[p->kind])) {
+   r = -EEXIST;
+   goto out_err_clean;
+   }
+   kvm->debugfs_data[i] = stat_data;
+   }
+
+   return r;
+
+out_err_clean:
+   debugfs_remove_recursive(kvm->debugfs_dentry);
+   kfree(stat_data);
+   for (i--; i >= 0; i--)
+   kfree(kvm->debugfs_data[i]);
+
+   kfree(kvm->debugfs_data);
+out_err:
+   return r;
+}
+
 static struct kvm *kvm_create_vm(unsigned long type)
 {
int r, i;
@@ -597,6 +668,10 @@ static struct kvm *kvm_create_vm(unsigned long type)
list_add(>vm_list, _list);
spin_unlock(_lock);
 
+   r = kvm_create_vm_debugfs(kvm);
+   if (r)
+   goto out_err;
+
preempt_notifier_inc();
 
return kvm;
@@ -646,6 +721,7 @@ static void kvm_destroy_vm(struct 

KVM call for November 22th

[Juan Quintela]

Hi

Please, send any topic that you are interested in covering.

At the end of Monday I will send an email with the agenda or the
cancellation of the call, so hurry up.

After discussions on the QEMU Summit, we are going to have always open a
KVM call where you can add topics.

 Call details:

By popular demand, a google calendar public entry with it

  
https://www.google.com/calendar/embed?src=dG9iMXRqcXAzN3Y4ZXZwNzRoMHE4a3BqcXNAZ3JvdXAuY2FsZW5kYXIuZ29vZ2xlLmNvbQ

(Let me know if you have any problems with the calendar entry.  I just
gave up about getting right at the same time CEST, CET, EDT and DST).

If you need phone number details,  contact me privately

Thanks, Juan.

--
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 0/7] kvmtool: Cleanup kernel loading

[Andre Przywara]

Hi Will,

On 02/11/15 14:58, Will Deacon wrote:
> On Fri, Oct 30, 2015 at 06:26:53PM +, Andre Przywara wrote:
>> Hi,
> 
> Hello Andre,
> 
>> this series cleans up kvmtool's kernel loading functionality a bit.
>> It has been broken out of a previous series I sent [1] and contains
>> just the cleanup and bug fix parts, which should be less controversial
>> and thus easier to merge ;-)
>> I will resend the pipe loading part later on as a separate series.
>>
>> The first patch properly abstracts kernel loading to move
>> responsibility into each architecture's code. It removes quite some
>> ugly code from the generic kvm.c file.
>> The later patches address the naive usage of read(2) to, well, read
>> data from files. Doing this without coping with the subtleties of
>> the UNIX read semantics (returning with less or none data read is not
>> an error) can provoke hard to debug failures.
>> So these patches make use of the existing and one new wrapper function
>> to make sure we read everything we actually wanted to.
>> The last patch moves the ARM kernel loading code into the proper
>> location to be in line with the other architectures.
>>
>> Please have a look and give some comments!
> 
> Looks good to me, but I'd like to see some comments from some mips/ppc/x86
> people on the changes you're making over there.

Sounds reasonable, but no answers yet.

Can you take at least patch 1 and 2 meanwhile, preferably 6 and 7 (the
ARM parts) also if you are OK with it?
I have other patches that depend on 1/7 and 2/7, so having them upstream
would help me and reduce further dependency churn.
I am happy to resend the remaining patches for further discussion later.

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

Re: [PATCH v3 0/7] KVM, pkeys: add memory protection-key support

[Paolo Bonzini]

On 18/11/2015 06:43, Huaitong Han wrote:
> Changes in v3:
> *Add comments for patch that disable PKU feature without ept.
> 
> Changes in v2:
> *Add pku.c for kvm-unit-tests.
> *Optimize permission_fault codes for patch4.
> *Delete is_long_mode and PK for patch5.
> *Squash cpuid and cr4 patches.
> 
> The protection-key feature provides an additional mechanism by which IA-32e
> paging controls access to usermode addresses.
> 
> Hardware support for protection keys for user pages is enumerated with CPUID
> feature flag CPUID.7.0.ECX[3]:PKU. Software support is CPUID.7.0.ECX[4]:OSPKE
> with the setting of CR4.PKE(bit 22).
> 
> When CR4.PKE = 1, every linear address is associated with the 4-bit protection
> key located in bits 62:59 of the paging-structure entry that mapped the page
> containing the linear address. The PKRU register determines, for each
> protection key, whether user-mode addresses with that protection key may be
> read or written.
> 
> The PKRU register (protection key rights for user pages) is a 32-bit register
> with the following format: for each i (0 ≤ i ≤ 15), PKRU[2i] is the
> access-disable bit for protection key i (ADi); PKRU[2i+1] is the write-disable
> bit for protection key i (WDi).
> 
> Software can use the RDPKRU and WRPKRU instructions with ECX = 0 to read and
> write PKRU. In addition, the PKRU register is XSAVE-managed state and can thus
> be read and written by instructions in the XSAVE feature set.
> 
> PFEC.PK (bit 5) is defined as protection key violations.
> 
> The specification of Protection Keys can be found at SDM (4.6.2, volume 3)
> http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf.
> 
> The kernel native patchset have not yet been merged to upstream, you can found
> at git://git.kernel.org/pub/scm/linux/kernel/git/daveh/x86-pkeys.git 
> pkeys-v007.
> 
> Huaitong Han (7):
>   KVM, pkeys: expose CPUID/CR4 to guest
>   KVM, pkeys: disable pkeys for guests in non-paging mode
>   KVM, pkeys: update memeory permission bitmask for pkeys
>   KVM, pkeys: add pkeys support for permission_fault logic
>   KVM, pkeys: Add pkeys support for gva_to_gpa funcions
>   KVM, pkeys: add pkeys support for xsave state
>   KVM, pkeys: disable PKU feature without ept
> 
>  arch/x86/include/asm/kvm_host.h | 11 +---
>  arch/x86/kvm/cpuid.c| 24 --
>  arch/x86/kvm/cpuid.h|  8 ++
>  arch/x86/kvm/mmu.c  | 32 +--
>  arch/x86/kvm/mmu.h  | 56 
> +
>  arch/x86/kvm/paging_tmpl.h  | 18 ++---
>  arch/x86/kvm/vmx.c  | 10 
>  arch/x86/kvm/x86.c  | 27 ++--
>  arch/x86/kvm/x86.h  |  3 ++-
>  9 files changed, 161 insertions(+), 28 deletions(-)
> 

Looks good, but it will have to wait for kernel PKU support.

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: [kvm-unit-test RFC] x86: Memory instructions test case

[Paolo Bonzini]

On 04/11/2015 22:21, Eduardo Habkost wrote:
> Quickly hacked test case for memory instructions (clflush, mfence,
> sfence, lfence, clflushopt, clwb, pcommit), that simply checks for #UD
> exceptions.
> 
> This was useful to test TCG handling of those instructions.
> 
> The "fake clwb" part will probably break once a new instruction use
> those opcodes.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  config/config-x86-common.mak |  2 +
>  config/config-x86_64.mak |  2 +-
>  x86/memory.c | 88 
> 
>  3 files changed, 91 insertions(+), 1 deletion(-)
>  create mode 100644 x86/memory.c
> 
> diff --git a/config/config-x86-common.mak b/config/config-x86-common.mak
> index c2f9908..b89684d 100644
> --- a/config/config-x86-common.mak
> +++ b/config/config-x86-common.mak
> @@ -108,6 +108,8 @@ $(TEST_DIR)/vmx.elf: $(cstart.o) $(TEST_DIR)/vmx.o 
> $(TEST_DIR)/vmx_tests.o
>  
>  $(TEST_DIR)/debug.elf: $(cstart.o) $(TEST_DIR)/debug.o
>  
> +$(TEST_DIR)/memory.elf: $(cstart.o) $(TEST_DIR)/memory.o
> +
>  arch_clean:
>   $(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
>   $(TEST_DIR)/.*.d lib/x86/.*.d
> diff --git a/config/config-x86_64.mak b/config/config-x86_64.mak
> index 7d4eb34..ec4bded 100644
> --- a/config/config-x86_64.mak
> +++ b/config/config-x86_64.mak
> @@ -7,7 +7,7 @@ tests = $(TEST_DIR)/access.flat $(TEST_DIR)/apic.flat \
> $(TEST_DIR)/emulator.flat $(TEST_DIR)/idt_test.flat \
> $(TEST_DIR)/xsave.flat $(TEST_DIR)/rmap_chain.flat \
> $(TEST_DIR)/pcid.flat $(TEST_DIR)/debug.flat \
> -   $(TEST_DIR)/ioapic.flat
> +   $(TEST_DIR)/ioapic.flat $(TEST_DIR)/memory.flat
>  tests += $(TEST_DIR)/svm.flat
>  tests += $(TEST_DIR)/vmx.flat
>  tests += $(TEST_DIR)/tscdeadline_latency.flat
> diff --git a/x86/memory.c b/x86/memory.c
> new file mode 100644
> index 000..cd1eb46
> --- /dev/null
> +++ b/x86/memory.c
> @@ -0,0 +1,88 @@
> +/*
> + * Test for x86 cache and memory instructions
> + *
> + * Copyright (c) 2015 Red Hat Inc
> + *
> + * Authors:
> + *  Eduardo Habkost 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.
> + */
> +
> +#include "libcflat.h"
> +#include "desc.h"
> +#include "processor.h"
> +
> +static long target;
> +static volatile int ud;
> +static volatile int isize;
> +
> +static void handle_ud(struct ex_regs *regs)
> +{
> + ud = 1;
> + regs->rip += isize;
> +}
> +
> +int main(int ac, char **av)
> +{
> + struct cpuid cpuid7, cpuid1;
> + int xfail;
> +
> + setup_idt();
> + handle_exception(UD_VECTOR, handle_ud);
> +
> + cpuid1 = cpuid(1);
> + cpuid7 = cpuid_indexed(7, 0);
> +
> + /* 3-byte instructions: */
> + isize = 3;
> +
> + xfail = !(cpuid1.d & (1U << 19)); /* CLFLUSH */
> + ud = 0;
> + asm volatile("clflush (%0)" : : "b" ());
> + report_xfail("clflush", xfail, ud == 0);
> +
> + xfail = !(cpuid1.d & (1U << 25)); /* SSE */
> + ud = 0;
> + asm volatile("sfence");
> + report_xfail("sfence", xfail, ud == 0);
> +
> + xfail = !(cpuid1.d & (1U << 26)); /* SSE2 */
> + ud = 0;
> + asm volatile("lfence");
> + report_xfail("lfence", xfail, ud == 0);
> +
> + ud = 0;
> + asm volatile("mfence");
> + report_xfail("mfence", xfail, ud == 0);
> +
> + /* 4-byte instructions: */
> + isize = 4;
> +
> + xfail = !(cpuid7.b & (1U << 23)); /* CLFLUSHOPT */
> + ud = 0;
> + /* clflushopt (%rbx): */
> + asm volatile(".byte 0x66, 0x0f, 0xae, 0x3b" : : "b" ());
> + report_xfail("clflushopt", xfail, ud == 0);
> +
> + xfail = !(cpuid7.b & (1U << 24)); /* CLWB */
> + ud = 0;
> + /* clwb (%rbx): */
> + asm volatile(".byte 0x66, 0x0f, 0xae, 0x33" : : "b" ());
> + report_xfail("clwb", xfail, ud == 0);
> +
> + ud = 0;
> + /* clwb requires a memory operand, the following is NOT a valid
> +  * CLWB instruction (modrm == 0xF0).
> +  */
> + asm volatile(".byte 0x66, 0x0f, 0xae, 0xf0");
> + report("fake clwb", ud);
> +
> + xfail = !(cpuid7.b & (1U << 22)); /* PCOMMIT */
> + ud = 0;
> + /* pcommit: */
> + asm volatile(".byte 0x66, 0x0f, 0xae, 0xf8");
> + report_xfail("pcommit", xfail, ud == 0);
> +
> + return report_summary();
> +}
> 

Applied, 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: [RFC PATCH 2/3] kvm: Add accessors for guest CPU's family, model, stepping

[Paolo Bonzini]

On 14/11/2015 11:37, Borislav Petkov wrote:
> From: Borislav Petkov 
> 
> Those give the family, model and stepping of the guest vcpu.
> 
> Signed-off-by: Borislav Petkov 
> Cc: Paolo Bonzini 
> ---
>  arch/x86/kvm/cpuid.h | 34 ++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> index 06332cb7e7d1..5d47e0d95ef1 100644
> --- a/arch/x86/kvm/cpuid.h
> +++ b/arch/x86/kvm/cpuid.h
> @@ -2,6 +2,7 @@
>  #define ARCH_X86_KVM_CPUID_H
>  
>  #include "x86.h"
> +#include 
>  
>  int kvm_update_cpuid(struct kvm_vcpu *vcpu);
>  struct kvm_cpuid_entry2 *kvm_find_cpuid_entry(struct kvm_vcpu *vcpu,
> @@ -170,4 +171,37 @@ static inline bool guest_cpuid_has_nrips(struct kvm_vcpu 
> *vcpu)
>  }
>  #undef BIT_NRIPS
>  
> +static inline int guest_cpuid_family(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_cpuid_entry2 *best;
> +
> + best = kvm_find_cpuid_entry(vcpu, 0x1, 0);
> + if (!best)
> + return -1;
> +
> + return x86_family(best->eax);
> +}
> +
> +static inline int guest_cpuid_model(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_cpuid_entry2 *best;
> +
> + best = kvm_find_cpuid_entry(vcpu, 0x1, 0);
> + if (!best)
> + return -1;
> +
> + return x86_model(best->eax);
> +}
> +
> +static inline int guest_cpuid_stepping(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_cpuid_entry2 *best;
> +
> + best = kvm_find_cpuid_entry(vcpu, 0x1, 0);
> + if (!best)
> + return -1;
> +
> + return x86_stepping(best->eax);
> +}
> +
>  #endif
> 

Reviewed-by: Paolo Bonzini 
--
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 3/3] x86/cpu/amd, kvm: Satisfy guest kernel reads of IC_CFG MSR

[Paolo Bonzini]

On 14/11/2015 11:37, Borislav Petkov wrote:
> From: Borislav Petkov 
> 
> The kernel accesses IC_CFG MSR (0xc0011021) on AMD because it checks
> whether the way access filter is enabled on some F15h models, and, if
> so, disables it.
> 
> kvm doesn't handle that MSR access and complains about it, which can
> get really noisy in dmesg when one starts kvm guests all the time for
> testing. And it is useless anyway - guest kernel shouldn't be doing such
> changes anyway so tell it that that filter is disabled.
> 
> Signed-off-by: Borislav Petkov 
> Cc: Paolo Bonzini 
> ---
>  arch/x86/include/asm/msr-index.h |  1 +
>  arch/x86/kernel/cpu/amd.c|  4 ++--
>  arch/x86/kvm/svm.c   | 17 +
>  3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/msr-index.h 
> b/arch/x86/include/asm/msr-index.h
> index 9f3905697f12..5384485f8569 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -322,6 +322,7 @@
>  #define MSR_F15H_PERF_CTR0xc0010201
>  #define MSR_F15H_NB_PERF_CTL 0xc0010240
>  #define MSR_F15H_NB_PERF_CTR 0xc0010241
> +#define MSR_F15H_IC_CFG  0xc0011021
>  
>  /* Fam 10h MSRs */
>  #define MSR_FAM10H_MMIO_CONF_BASE0xc0010058
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 4a70fc6d400a..1d76dcdf7e55 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -665,9 +665,9 @@ static void init_amd_bd(struct cpuinfo_x86 *c)
>* Disable it on the affected CPUs.
>*/
>   if ((c->x86_model >= 0x02) && (c->x86_model < 0x20)) {
> - if (!rdmsrl_safe(0xc0011021, ) && !(value & 0x1E)) {
> + if (!rdmsrl_safe(MSR_F15H_IC_CFG, ) && !(value & 0x1E)) {
>   value |= 0x1E;
> - wrmsrl_safe(0xc0011021, value);
> + wrmsrl_safe(MSR_F15H_IC_CFG, value);
>   }
>   }
>  }
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 83a1c643f9a5..58b64c17c4a8 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -3053,6 +3053,23 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct 
> msr_data *msr_info)
>   case MSR_IA32_UCODE_REV:
>   msr_info->data = 0x0165;
>   break;
> + case MSR_F15H_IC_CFG: {
> +
> + int family, model;
> +
> + family = guest_cpuid_family(vcpu);
> + model  = guest_cpuid_model(vcpu);
> +
> + if (family < 0 || model < 0)
> + return kvm_get_msr_common(vcpu, msr_info);
> +
> + msr_info->data = 0;
> +
> + if (family == 0x15 &&
> + (model >= 0x2 && model < 0x20))
> + msr_info->data = 0x1E;
> + }
> + break;
>   default:
>   return kvm_get_msr_common(vcpu, msr_info);
>   }
> 

Reviewed-by: Paolo Bonzini 
--
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 07/10] KVM: x86: MMU: Encapsulate the type of rmap-chain head in a new struct

[Paolo Bonzini]

On 18/11/2015 04:21, Xiao Guangrong wrote:
> 
> 
> On 11/12/2015 07:55 PM, Takuya Yoshikawa wrote:
>> @@ -1720,7 +1724,7 @@ static struct kvm_mmu_page
>> *kvm_mmu_alloc_page(struct kvm_vcpu *vcpu,
>>* this feature. See the comments in kvm_zap_obsolete_pages().
>>*/
>>   list_add(>link, >kvm->arch.active_mmu_pages);
>> -sp->parent_ptes = 0;
>> +sp->parent_ptes.val = 0;
> 
> The sp is allocated from kmem_cache_zalloc() so explicitly initialize it
> to zero is not needed.

Right, but it should be a separate patch.

Takuya, since you are going to send another version of this series, can
you also:

1) move this patch either to the beginning or to the end

2) include "KVM: x86: always set accessed bit in shadow PTEs", also near
the beginning of the series?

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: [RFC] kvmtool: add support for modern virtio-pci

[Gerd Hoffmann]

On Mi, 2015-11-18 at 00:11 -0500, Sasha Levin wrote:
> This is a first go at adding support for the modern (based on the 1.0 virtio
> spec) virtio-pci implementation.

> To sum it up: this is a lightly tested version for feedback about the design
> and to weed out major bugs people notice. Feedback is very welcome!

/me goes undust the kvmtool patches for seabios.

(see https://www.kraxel.org/cgit/seabios/commit/?h=kvmtool,
build with CONFIG_KVMTOOL=y + CONFIG_DEBUG_LEVEL=9)

nilsson kraxel ~# ~kraxel/projects/kvmtool/lkvm run --name seabios
--firmware /home/kraxel/projects/seabios/out-bios-kvmtool/bios.bin
--disk /vmdisk/cloud/persistent/Fedora-Cloud-Base-22-20150521.x86_64.raw
  # lkvm run -k /boot/vmlinuz-3.10.0-324.el7.x86_64 -m 448 -c 4 --name
seabios
Changing serial settings was 0/0 now 3/0
SeaBIOS (version rel-1.9.0-7-g532b527)
BUILD: gcc: (GCC) 4.8.5 20150623 (Red Hat 4.8.5-4) binutils: version
2.23.52.0.1-55.el7 20130226
kvmtool: probed 448 MB RAM.
Add to e820 map:  1c00 1
malloc preinit
Add to e820 map: 000a 0005 -1
Add to e820 map: 000f 0001 2
Add to e820 map: 1bfc 0004 2
phys_alloc zone=0x000f78e8 size=14464 align=10 ret=1bfbc6f0
(detail=0x1bfbc6c0)
Relocating init from 0x000f40a0 to 0x1bfbc6f0 (size 14464)
malloc init
init ivt
init bda
Add to e820 map: 0009fc00 0400 2
init bios32
init PNPBIOS table
init keyboard
init mouse
init pic
math cp init
PCI probe
phys_alloc zone=0x1bfbff38 size=32 align=10 ret=1bfbc640
(detail=0x1bfbc610)
PCI device 00:00.0 (vd=1af4:1000 c=0200)
phys_alloc zone=0x1bfbff38 size=32 align=10 ret=1bfbc5f0
(detail=0x1bfbc5c0)
PCI device 00:01.0 (vd=1af4:1001 c=0180)
Found 2 PCI devices (max PCI bus is 00)
tsc calibrate start=71959316 end=71968721 diff=9405
CPU Mhz=5
init timer
init lpt
Found 2 lpt ports
init serial
Found 4 serial ports
init virtio-blk
found virtio-blk at 0:1
phys_alloc zone=0x1bfbff40 size=80 align=10 ret=f78d0
(detail=0x1bfbc590)
pci dev 0:1 virtio cap at 0x4c type 1 bar 0 at 0xd2000800 off +0x
[mmio]
pci dev 0:1 virtio cap at 0x5c type 2 bar 0 at 0xd2000800 off +0x0080
[mmio]
pci dev 0:1 virtio cap at 0x70 type 3 bar 0 at 0xd2000800 off +0x0100
[mmio]
pci dev 0:1 virtio cap at 0x80 type 4 bar 0 at 0xd2000800 off +0x0180
[mmio]
pci dev 0:1 using modern (1.0) virtio mode
vp write  d2000814 (1) <- 0x0
Segmentation fault

With '--virtio-legacy' added seabios manages to load the kernel from
disk.

cheers,
  Gerd


--
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 v3 7/7] KVM, pkeys: disable PKU feature without ept

[Han, Huaitong]

On Wed, 2015-11-18 at 10:06 +0100, Paolo Bonzini wrote:
> 
> On 18/11/2015 06:44, Huaitong Han wrote:
> > This patch disables CPUID:PKU without ept, becase pkeys is not
> > supported
> > with softmmu.
> 
> Sure, but _what_ makes it impossible to support pkeys with shadow
> pages?
> 
> Is it enough to add the pkey bits to the role (and then to
> kvm_get_mmu_page, mmu_set_spte, set_spte) or are there fundamental
> problems?
> 
> The trick to handle !CR0.WP in FNAME(page_fault) (search for
> "walker.pte_access &= ~ACC_USER_MASK"; it's documented in
> Documentation/virtual/kvm/mmu.txt as well) should work for PKRU.
> 
> If you just want me to change it to "is not yet implemented for
> shadow
> paging", I can do that.

Yes, the comments should be described as "becase pkeys is not yet
implemented for shadow paging."

Thanks
Huaitong.

> 
> Thanks,
> 
> Paolo
> 
> > Signed-off-by: Huaitong Han 
> > N�r��yb�X��ǧv�^�)޺{.n�+h����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

Re: [PATCH 02/10] KVM: x86: MMU: Add helper function to clear a bit in unsync child bitmap

[Xiao Guangrong]

On 11/19/2015 08:59 AM, Takuya Yoshikawa wrote:

On 2015/11/18 11:44, Xiao Guangrong wrote:


On 11/12/2015 07:50 PM, Takuya Yoshikawa wrote:

+if (!ret) {
+clear_unsync_child_bit(sp, i);
+continue;
+} else if (ret > 0) {
  nr_unsync_leaf += ret;


Just a single line here, braces are unnecessary.


-else
+} else
  return ret;


I know we can eliminate the braces, but that does not mean
we should do so: there seems to be no consensus about this
style issue and checkpatch accepts both ways.

Actually, some people prefer to put braces when one of the
if/else-if/else cases has multiple lines.  You can see
some examples in kernel/sched/core.c: see hrtick_start(),
sched_fork(), free_sched_domain().

In our case, I thought putting braces would align the else-if
and else and make the code look a bit nicer, but I know this
may be just a matter of personal feeling.

In short, unless the maintainer, Paolo for this file, has any
preference, both ways will be accepted.


The reason why i pointed this out is that it is the style documented
in Documentation/CodingStyle:
| Do not unnecessarily use braces where a single statement will do.
|
|if (condition)
|action();
|

Actually, Ingo Molnar hated this braces-style too much and blamed
many developers who used this style (include me, that why i was
nervous to see this style :( ).

If this style is commonly accepted now, it is worth making a patch
to update Documentation/CodingStyle.

--
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 02/10] KVM: x86: MMU: Add helper function to clear a bit in unsync child bitmap

[Takuya Yoshikawa]

On 2015/11/18 11:44, Xiao Guangrong wrote:


On 11/12/2015 07:50 PM, Takuya Yoshikawa wrote:

+if (!ret) {
+clear_unsync_child_bit(sp, i);
+continue;
+} else if (ret > 0) {
  nr_unsync_leaf += ret;


Just a single line here, braces are unnecessary.


-else
+} else
  return ret;


I know we can eliminate the braces, but that does not mean
we should do so: there seems to be no consensus about this
style issue and checkpatch accepts both ways.

Actually, some people prefer to put braces when one of the
if/else-if/else cases has multiple lines.  You can see
some examples in kernel/sched/core.c: see hrtick_start(),
sched_fork(), free_sched_domain().

In our case, I thought putting braces would align the else-if
and else and make the code look a bit nicer, but I know this
may be just a matter of personal feeling.

In short, unless the maintainer, Paolo for this file, has any
preference, both ways will be accepted.

  Takuya

--
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 v8 0/5] implement vNVDIMM

[Xiao Guangrong]

On 11/19/2015 04:44 AM, Michael S. Tsirkin wrote:

On Wed, Nov 18, 2015 at 05:18:17PM -0200, Eduardo Habkost wrote:

On Wed, Nov 18, 2015 at 09:59:34AM +0800, Xiao Guangrong wrote:


Ping...

Do you have any comment on this patchset? Could it be applied to somewhere
if it is okay for you?


I have no additional comments, as the memory-backend patches I
was reviewing are not included in this version. I didn't take the
time to review the TYPE_NVDIMM and ACPI changes.


No, I don't think the way guest memory is allocated here is ok.  I'm


Since the DSM memory/ACPI memory was not included in this patchset, i really
do not understand what is "guest memory is allocated" exactly stands for...


sorry, I'm busy with 2.5 now, and this is clearly not 2.5 material.


I still see some pull requests were send our for 2.5 merge window today and
yesterday ...

This patchset is the simplest version we can figure out to implement basic
functionality for vNVDIMM and only minor change is needed for other code.
It would be nice and really appreciate if it can go to 2.5.


Once that's out, I'll post some suggestions.


Look forward to you suggestions.

Thanks for your time, Michael and Eduardo!
--
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 07/10] KVM: x86: MMU: Encapsulate the type of rmap-chain head in a new struct

[Takuya Yoshikawa]

On 2015/11/18 18:09, Paolo Bonzini wrote:


On 18/11/2015 04:21, Xiao Guangrong wrote:



On 11/12/2015 07:55 PM, Takuya Yoshikawa wrote:

@@ -1720,7 +1724,7 @@ static struct kvm_mmu_page
*kvm_mmu_alloc_page(struct kvm_vcpu *vcpu,
* this feature. See the comments in kvm_zap_obsolete_pages().
*/
   list_add(>link, >kvm->arch.active_mmu_pages);
-sp->parent_ptes = 0;
+sp->parent_ptes.val = 0;


The sp is allocated from kmem_cache_zalloc() so explicitly initialize it
to zero is not needed.


Right, but it should be a separate patch.

Takuya, since you are going to send another version of this series, can
you also:


Yes, I'm preparing to do so.


1) move this patch either to the beginning or to the end

2) include "KVM: x86: always set accessed bit in shadow PTEs", also near
the beginning of the series?


Commit 1c9a5e19b1af8a2c ("KVM: x86: MMU: always set accessed bit
in shadow PTEs") will be the first.

Then, the ordering will become something like this:

02: Encapsulate the type of rmap-chain head in a new struct
03: Remove unused parameter of __direct_map()
04: Add helper function to clear a bit in unsync child bitmap
05: Make mmu_set_spte() return emulate value
06: Remove is_rmap_spte() and use is_shadow_present_pte()

These five seem to be easy ones for you to apply: since patch 02
touches many places, it should go first to become the base of the
following work.

07: Consolidate BUG_ON checks for reverse-mapped sptes

I will change the WARN_ON to BUG_ON.  // Marcelo's comment

08: Move initialization of parent_ptes out from kvm_mmu_alloc_page()

In this patch, I will delete "sp->parent_ptes.val = 0;" line since
this is the problem of kvm_mmu_alloc_page(), though not a new one.
  // Xiao's comment

09: Use for_each_rmap_spte macro instead of pte_list_walk()

There is some confusion between us: Paolo and I agreed that the
patch keeps the original way and calls mark_unsync() the same way
as before, but there are still comments from Marcelo and Xiao and
those comments seem to explain the code differently.

I will check again, but I may not change this one and the following
two patches in the next version.  If we can eliminate some of the
mark_unsync() calls, that will be kind of an optimization which this
series does not intend to achieve.

Anyway, by moving the non-trivial two patches (09 and 10) here,
reviewing will become easier and you can apply the other patches
separately.

10: Move parent_pte handling from kvm_mmu_get_page()
to link_shadow_page()
11: Remove unused parameter parent_pte from kvm_mmu_get_page()

Thanks,
  Takuya

--
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] kvmtool: add support for modern virtio-pci

[Sasha Levin]

On 11/18/2015 11:00 PM, Sasha Levin wrote:
> Anyways, I debugged it for a bit a found that seabios attempts to write to
> the notification BAR, I look further tomorrow to narrow it down and fix it.

Err, *read*, obviously.

I've never implemented that because the kernel doesn't try to do that (it 
doesn't
make much sense, I think...).


Thanks,
Sasha
--
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

[RESEND PATCH] vfio: Drop owner assignment from platform_driver

[Krzysztof Kozlowski]

platform_driver does not need to set an owner because
platform_driver_register() will set it.

Signed-off-by: Krzysztof Kozlowski 
Acked-by: Baptiste Reynal 

---

The coccinelle script which generated the patch was sent here:
http://www.spinics.net/lists/kernel/msg2029903.html
---
 drivers/vfio/platform/vfio_platform.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/vfio/platform/vfio_platform.c 
b/drivers/vfio/platform/vfio_platform.c
index f1625dcfbb23..b1cc3a768784 100644
--- a/drivers/vfio/platform/vfio_platform.c
+++ b/drivers/vfio/platform/vfio_platform.c
@@ -92,7 +92,6 @@ static struct platform_driver vfio_platform_driver = {
.remove = vfio_platform_remove,
.driver = {
.name   = "vfio-platform",
-   .owner  = THIS_MODULE,
},
 };
 
-- 
1.9.1

--
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] kvmtool: add support for modern virtio-pci

[Sasha Levin]

On 11/18/2015 12:52 PM, Gerd Hoffmann wrote:
>   Hi,
> 
>> Thanks for testing! I didn't even thing about seabios as a testing target.
> 
> Not surprising, support isn't upstream, ran into a bunch of issues[1][2]
> last time I tried to combine the two, ran into some issues and nobody
> seemed to care, so the seabios patches where just sitting in a branch in
> my repo ...
> 
>> $ cat .config | grep 'KVMTOOL\|DEBUG'
>> CONFIG_KVMTOOL=y
>> CONFIG_DEBUG_LEVEL=9
> 
> Hmm, 'CONFIG_KVMTOOL=y > .config; make olddefconfig' should give you a
> working configuration.
> 
> Setting 'CONFIG_DEBUG_LEVEL=9' is useful for trouble-shooting as it
> makes the virtio drivers more verbose, but not mandatory to have.
> 
> Serial line support is needed to get output:
> 
> CONFIG_DEBUG_SERIAL=y
> CONFIG_DEBUG_SERIAL_PORT=0x3f8
> 
> Also I think rom size must be 128k b/c kvmtool expects it to be that
> way:
> 
> CONFIG_ROM_SIZE=128
> 
> But those are the defaults, and after "make olddefconfig" you should
> already have them ...

It was the ROM_SIZE one as it seems, it was set to 0 here.

Anyways, I debugged it for a bit a found that seabios attempts to write to
the notification BAR, I look further tomorrow to narrow it down and fix it.


Thanks,
Sasha

--
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 02/10] KVM: x86: MMU: Add helper function to clear a bit in unsync child bitmap

[Takuya Yoshikawa]

On 2015/11/19 11:46, Xiao Guangrong wrote:


Actually, some people prefer to put braces when one of the
if/else-if/else cases has multiple lines.  You can see
some examples in kernel/sched/core.c: see hrtick_start(),
sched_fork(), free_sched_domain().

In our case, I thought putting braces would align the else-if
and else and make the code look a bit nicer, but I know this
may be just a matter of personal feeling.

In short, unless the maintainer, Paolo for this file, has any
preference, both ways will be accepted.


The reason why i pointed this out is that it is the style documented
in Documentation/CodingStyle:
| Do not unnecessarily use braces where a single statement will do.
|
|if (condition)
|action();
|


Ah, this is a different thing.  For this case, there is a consensus
and checkpatch will complain if we don't obey the rule.

What I explained was:

  if (condition) {
 line1;
 line2;  // multiple lines
  } else if {
 single-line-statement;  -- (*1)
  } else
 single-line-statement;  -- (*2)

For (*1) and (*2), especially for (*1), some people put braces.


Actually, Ingo Molnar hated this braces-style too much and blamed
many developers who used this style (include me, that why i was
nervous to see this style :( ).


I think he likes the coding style of kernel/sched/core.c very much,
as you know.  Actually that is one reason why I took it as an example.

Let's just choose the way which Paolo prefers for this time, I don't
know which is better.

Thank you,
  Takuya


--
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] [PATCH v3 1/3] target-i386: add pkeys support for cpuid handling

[Han, Huaitong]

On Wed, 2015-11-18 at 13:58 -0200, Eduardo Habkost wrote:
> On Wed, Nov 18, 2015 at 10:20:15AM +0800, Huaitong Han wrote:
> [...]
> > @@ -408,6 +420,13 @@ static FeatureWordInfo
> > feature_word_info[FEATURE_WORDS] = {
> >  .cpuid_reg = R_EBX,
> >  .tcg_features = TCG_7_0_EBX_FEATURES,
> >  },
> > +[FEAT_7_0_ECX] = {
> > +.feat_names = cpuid_7_0_ecx_feature_name,
> > +.cpuid_eax = 7,
> > +.cpuid_needs_ecx = true, .cpuid_ecx = 0,
> > +.cpuid_reg = R_ECX,
> > +.tcg_features = TCG_7_0_ECX_FEATURES,
> > +},
> 
> The patch looks good, but when we add the feature names to
> cpuid_7_0_ecx_feature_name, QEMU will consider them as
> migratable, but they are truly migratable only after we add the
> ext_save_areas entry.

> We can fix this by moving cpuid_7_0_ecx_feature_name to patch 2/3
> (or to a separate patch, to be applied after 2/3).

I understand it has always been that QEMU considers the feature of
 cpuid_7_0_ecx_feature_name as migratable. If the feature is
 unmigratable, it will been added to unmigratable_flags.

A series of patches do complete a full function, moving
 cpuid_7_0_ecx_feature_name to 2/3 patch may make 2/3 patch look
better, but make 1/3 patch look somewhat incomplete.

Maybe it is a solution that adding the feature to unmigratable_flags in
1/3 patch, and deleting unmigratable_flags in 2/3 patch, but I think it
is pointless.

Re: [RFC] kvmtool: add support for modern virtio-pci

[Gerd Hoffmann]

On Mi, 2015-11-18 at 23:01 -0500, Sasha Levin wrote:
> On 11/18/2015 11:00 PM, Sasha Levin wrote:
> > Anyways, I debugged it for a bit a found that seabios attempts to write to
> > the notification BAR, I look further tomorrow to narrow it down and fix it.
> 
> Err, *read*, obviously.
> 
> I've never implemented that because the kernel doesn't try to do that (it 
> doesn't
> make much sense, I think...).

It doesn't make sense indeed (kvmtool still shouldn't segfault though),
and on a quick look I can't spot a place in seabios doing that ...

It's reading ISR, as part of device reset, to make sure any pending
interrupts are cleared.

cheers,
  Gerd


--
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] [PATCH v3 0/3] target-i386: add memory protection-key support

[Paolo Bonzini]

On 18/11/2015 18:51, Eduardo Habkost wrote:
> Is every CPU supporting PKU guaranteed to have
> CPUID.(EAX=0DH,ECX=9):EBX = 0xa80?

We asked Intel a while ago when reorganizing XSAVE support in KVM and
QEMU.  Unfortunately, Intel is not listing the offsets anymore in the
documentation, but they confirmed at the time that offsets won't change.

http://lists.xen.org/archives/html/xen-devel/2013-09/msg00484.html is
pretty clear in this respect:

>> [adding H. Peter Anvin... the context is whether the layout of the
>> XSAVE/XRSTOR area is fixed, including the offset of each separate
>> Ext_SAVE_Area].
> 
> It is.
> 
>> So please Intel, pretty please do not modify the XSAVE offsets, and
>> clarify this as soon as possible.
> 
> They will not change.
> 
> -hpa


This of course doesn't mean that the 0xa80 is correct; it only means
that if it is correct, it will always stay correct. :)

It makes sense that 0xa80 is correct, since ECX=8 is a supervisor state
(thus never saved by XSAVE/XSAVEOPT).

The fact that standard format does not account for supervisor states
means that supporting supervisor states in KVM might encounter a few
small complications.  In particular we might have to modify KVM and QEMU
to support compacted format in KVM_GET/SET_XSAVE.  For now the only
supervisor state is just a bunch of MSRs, so it would not be necessary
to include it in KVM_GET/SET_XSAVE.  Problem averted for now, then.

> Where is the PKRU state offset/layout documented?

Volume 1 of the SDM, section 13.5.7 ("PKRU State") documents the layout.
 (Careful: chapter 13 of volume 1 is "Managing State Using the XSAVE
Feature Set"; chapter 13 of volume 3 is "System Programming for
Instruction Set Extensions and Processor Extended States", and also
deals with XSAVE).

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