Re: [Qemu-devel] [PATCH 3/3] Target-ppc: Remove unnecessary variable

2015-09-26 Thread Eric Blake
On 09/25/2015 02:37 AM, Shraddha Barke wrote:
> Compress lines and remove the variable.
> 

> +++ b/target-ppc/kvm.c
> @@ -1782,8 +1782,7 @@ uint32_t kvmppc_get_tbfreq(void)
>  
>  ns++;
>  
> -retval = atoi(ns);
> -return retval;
> +return atoi(ns);

atoi() is lousy; it cannot properly detect user input errors.  This
should probably be converted to use the appropriate qemu_strtol variant
instead.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


[PATCH v2 0/2] KVM/arm: enhance arvm7 vfp/simd lazy switch support

2015-09-26 Thread Mario Smarduch
Current lazy vfp/simd implementation switches hardware context only on 
guest access and again on exit to host, otherwise hardware context is
skipped.

This patch set builds on that functionality and executes a hardware context 
switch only when  vCPU is scheduled out or returns to user space.

Patches were tested on FVP sw platform. FP crunching applications summing up 
values, with outcome compared to known result were executed on several guests, 
and host. 

Changes since v1->v2:
* Fixed vfp/simd trap configuration to enable trace trapping
* Removed set_hcptr branch label
* Fixed handling of FPEXC to restore guest and host versions on vcpu_put

Mario Smarduch (2):
  add hooks for armv7 fp/simd lazy switch support
  enable armv7 fp/simd lazy switch

 arch/arm/include/asm/kvm_asm.h  |  1 +
 arch/arm/include/asm/kvm_host.h |  6 +
 arch/arm/kernel/asm-offsets.c   |  2 ++
 arch/arm/kvm/arm.c  | 17 
 arch/arm/kvm/interrupts.S   | 60 ++---
 arch/arm/kvm/interrupts_head.S  | 12 ++---
 6 files changed, 79 insertions(+), 19 deletions(-)

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


[PATCH v2 1/2] KVM/arm: add hooks for armv7 fp/simd lazy switch support

2015-09-26 Thread Mario Smarduch
This patch adds vcpu fields to track lazy state, save host FPEXC, and 
offsets to fields.

Signed-off-by: Mario Smarduch 
---
 arch/arm/include/asm/kvm_host.h | 6 ++
 arch/arm/kernel/asm-offsets.c   | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index dcba0fa..194a8ef 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -111,6 +111,12 @@ struct kvm_vcpu_arch {
/* Interrupt related fields */
u32 irq_lines;  /* IRQ and FIQ levels */
 
+   /* Track fp/simd lazy switch state */
+   u32 vfp_lazy;
+
+   /* Save host FPEXC register to restore on vcpu put */
+   u32 saved_fpexc;
+
/* Exception Information */
struct kvm_vcpu_fault_info fault;
 
diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
index 871b826..e1c3a41 100644
--- a/arch/arm/kernel/asm-offsets.c
+++ b/arch/arm/kernel/asm-offsets.c
@@ -186,6 +186,8 @@ int main(void)
   DEFINE(VCPU_CPSR,offsetof(struct kvm_vcpu, 
arch.regs.usr_regs.ARM_cpsr));
   DEFINE(VCPU_HCR, offsetof(struct kvm_vcpu, arch.hcr));
   DEFINE(VCPU_IRQ_LINES,   offsetof(struct kvm_vcpu, arch.irq_lines));
+  DEFINE(VCPU_VFP_LAZY,offsetof(struct kvm_vcpu, 
arch.vfp_lazy));
+  DEFINE(VCPU_VFP_FPEXC,   offsetof(struct kvm_vcpu, arch.saved_fpexc));
   DEFINE(VCPU_HSR, offsetof(struct kvm_vcpu, arch.fault.hsr));
   DEFINE(VCPU_HxFAR,   offsetof(struct kvm_vcpu, arch.fault.hxfar));
   DEFINE(VCPU_HPFAR,   offsetof(struct kvm_vcpu, arch.fault.hpfar));
-- 
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


[PATCH v2 2/2] KVM/arm: enable enhanced armv7 fp/simd lazy switch

2015-09-26 Thread Mario Smarduch
This patch enhances current lazy vfp/simd hardware switch. In addition to
current lazy switch, it tracks vfp/simd hardware state with a vcpu 
lazy flag. 

vcpu lazy flag is set on guest access and trap to vfp/simd hardware switch 
handler. On vm-enter if lazy flag is set skip trap enable and saving 
host fpexc. On vm-exit if flag is set skip hardware context switch
and return to host with guest context.

On vcpu_put check if vcpu lazy flag is set, and execute a hardware context 
switch to restore host.

Signed-off-by: Mario Smarduch 
---
 arch/arm/include/asm/kvm_asm.h |  1 +
 arch/arm/kvm/arm.c | 17 
 arch/arm/kvm/interrupts.S  | 60 +++---
 arch/arm/kvm/interrupts_head.S | 12 ++---
 4 files changed, 71 insertions(+), 19 deletions(-)

diff --git a/arch/arm/include/asm/kvm_asm.h b/arch/arm/include/asm/kvm_asm.h
index 194c91b..4b45d79 100644
--- a/arch/arm/include/asm/kvm_asm.h
+++ b/arch/arm/include/asm/kvm_asm.h
@@ -97,6 +97,7 @@ extern char __kvm_hyp_code_end[];
 extern void __kvm_flush_vm_context(void);
 extern void __kvm_tlb_flush_vmid_ipa(struct kvm *kvm, phys_addr_t ipa);
 extern void __kvm_tlb_flush_vmid(struct kvm *kvm);
+extern void __kvm_restore_host_vfp_state(struct kvm_vcpu *vcpu);
 
 extern int __kvm_vcpu_run(struct kvm_vcpu *vcpu);
 #endif
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index ce404a5..79f49c7 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -105,6 +105,20 @@ void kvm_arch_check_processor_compat(void *rtn)
*(int *)rtn = 0;
 }
 
+/**
+ * kvm_switch_fp_regs() - switch guest/host VFP/SIMD registers
+ * @vcpu:  pointer to vcpu structure.
+ *
+ */
+static void kvm_switch_fp_regs(struct kvm_vcpu *vcpu)
+{
+#ifdef CONFIG_ARM
+   if (vcpu->arch.vfp_lazy == 1) {
+   kvm_call_hyp(__kvm_restore_host_vfp_state, vcpu);
+   vcpu->arch.vfp_lazy = 0;
+   }
+#endif
+}
 
 /**
  * kvm_arch_init_vm - initializes a VM data structure
@@ -295,6 +309,9 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 {
+   /* Check if Guest accessed VFP registers */
+   kvm_switch_fp_regs(vcpu);
+
/*
 * The arch-generic KVM code expects the cpu field of a vcpu to be -1
 * if the vcpu is no longer assigned to a cpu.  This is used for the
diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
index 900ef6d..6d98232 100644
--- a/arch/arm/kvm/interrupts.S
+++ b/arch/arm/kvm/interrupts.S
@@ -96,6 +96,29 @@ ENTRY(__kvm_flush_vm_context)
bx  lr
 ENDPROC(__kvm_flush_vm_context)
 
+/**
+ * void __kvm_restore_host_vfp_state(struct vcpu *vcpu) - Executes a lazy
+ * fp/simd switch, saves the guest, restores host.
+ *
+ */
+ENTRY(__kvm_restore_host_vfp_state)
+#ifdef CONFIG_VFPv3
+   push{r3-r7}
+
+   add r7, r0, #VCPU_VFP_GUEST
+   store_vfp_state r7
+
+   add r7, r0, #VCPU_VFP_HOST
+   ldr r7, [r7]
+   restore_vfp_state r7
+
+   ldr r3, [vcpu, #VCPU_VFP_FPEXC]
+   VFPFMXR FPEXC, r3
+
+   pop {r3-r7}
+#endif
+   bx  lr
+ENDPROC(__kvm_restore_host_vfp_state)
 
 /
  *  Hypervisor world-switch code
@@ -119,11 +142,15 @@ ENTRY(__kvm_vcpu_run)
@ If the host kernel has not been configured with VFPv3 support,
@ then it is safer if we deny guests from using it as well.
 #ifdef CONFIG_VFPv3
+   @ r7 must be preserved until next vfp lazy check
+   vfp_inlazy_mode r7, skip_save_host_fpexc
+
@ Set FPEXC_EN so the guest doesn't trap floating point instructions
VFPFMRX r2, FPEXC   @ VMRS
-   push{r2}
+   str r2, [vcpu, #VCPU_VFP_FPEXC]
orr r2, r2, #FPEXC_EN
VFPFMXR FPEXC, r2   @ VMSR
+skip_save_host_fpexc:
 #endif
 
@ Configure Hyp-role
@@ -131,7 +158,14 @@ ENTRY(__kvm_vcpu_run)
 
@ Trap coprocessor CRx accesses
set_hstr vmentry
-   set_hcptr vmentry, (HCPTR_TTA | HCPTR_TCP(10) | HCPTR_TCP(11))
+   set_hcptr vmentry, (HCPTR_TTA)
+
+   @ check if vfp_lazy flag set
+   cmp r7, #1
+   beq skip_guest_vfp_trap
+   set_hcptr vmentry, (HCPTR_TCP(10) | HCPTR_TCP(11))
+skip_guest_vfp_trap:
+
set_hdcr vmentry
 
@ Write configured ID register into MIDR alias
@@ -170,22 +204,14 @@ __kvm_vcpu_return:
@ Don't trap coprocessor accesses for host kernel
set_hstr vmexit
set_hdcr vmexit
-   set_hcptr vmexit, (HCPTR_TTA | HCPTR_TCP(10) | HCPTR_TCP(11)), 
after_vfp_restore
+   set_hcptr vmexit, (HCPTR_TTA | HCPTR_TCP(10) | HCPTR_TCP(11))
 
 #ifdef CONFIG_VFPv3
-   @ Switch VFP/NEON hardware state to the host's
-   add r7, vcpu, #VCPU_VFP_GUEST
-   store_vfp_state r7
-   add r7, vcpu, #VCPU_VFP_HOST
-   ldr r7, [r7]
-   

Re: [PATCH] x86: Use entire page for the per-cpu GDT only if paravirt-enabled

2015-09-26 Thread Denys Vlasenko
On 09/26/2015 09:50 PM, H. Peter Anvin wrote:
> NAK.  We really should map the GDT read-only on all 64 bit systems,
> since we can't hide the address from SLDT.  Same with the IDT.

Sorry, I don't understand your point.


> On September 26, 2015 11:00:40 AM PDT, Denys Vlasenko  
> wrote:
>> We have our GDT in a page-sized per-cpu structure, gdt_page.
>>
>> On x86_64 kernel, GDT is 128 bytes - only ~3% of that page is used.
>>
>> It is page-sized because of paravirt. Hypervisors need to know when
>> GDT is changed, so they remap it read-only and handle write faults.
>> If it's not in its own page, other writes nearby will cause
>> those faults too.
>>
>> In other words, we need GDT to live in a separate page
>> only if CONFIG_HYPERVISOR_GUEST=y.
>>
>> This patch reduces GDT alignment to cacheline-aligned
>> if CONFIG_HYPERVISOR_GUEST is not set.
>>
>> Patch also renames gdt_page to cpu_gdt (mimicking naming of existing
>> cpu_tss per-cpu variable), since now it is not always a full page.

--
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: Use entire page for the per-cpu GDT only if paravirt-enabled

2015-09-26 Thread Denys Vlasenko
We have our GDT in a page-sized per-cpu structure, gdt_page.

On x86_64 kernel, GDT is 128 bytes - only ~3% of that page is used.

It is page-sized because of paravirt. Hypervisors need to know when
GDT is changed, so they remap it read-only and handle write faults.
If it's not in its own page, other writes nearby will cause
those faults too.

In other words, we need GDT to live in a separate page
only if CONFIG_HYPERVISOR_GUEST=y.

This patch reduces GDT alignment to cacheline-aligned
if CONFIG_HYPERVISOR_GUEST is not set.

Patch also renames gdt_page to cpu_gdt (mimicking naming of existing
cpu_tss per-cpu variable), since now it is not always a full page.

$ objdump -x vmlinux | grep .data..percpu | sort
Before:
(offset)(size)
  wO .data..percpu  4000 irq_stack_union
4000  wO .data..percpu  5000 exception_stacks
9000  wO .data..percpu  1000 gdt_page  <<< HERE
a000  wO .data..percpu  0008 espfix_waddr
a008  wO .data..percpu  0008 espfix_stack
...
00019398 g   .data..percpu   __per_cpu_end
After:
  wO .data..percpu  4000 irq_stack_union
4000  wO .data..percpu  5000 exception_stacks
9000  wO .data..percpu  0008 espfix_waddr
9008  wO .data..percpu  0008 espfix_stack
...
00013c80  wO .data..percpu  0040 cyc2ns
00013cc0  wO .data..percpu  22c0 cpu_tss
00015f80  wO .data..percpu  0080 cpu_gdt  <<< HERE
00016000  wO .data..percpu  0018 cpu_tlbstate
...
00018418 g   .data..percpu   __per_cpu_end

Run-tested on a 144 CPU machine.

Signed-off-by: Denys Vlasenko 
CC: Ingo Molnar 
CC: H. Peter Anvin 
CC: Konrad Rzeszutek Wilk 
CC: Boris Ostrovsky 
CC: David Vrabel 
CC: Joerg Roedel 
CC: Gleb Natapov 
CC: Paolo Bonzini 
CC: kvm@vger.kernel.org
CC: x...@kernel.org
CC: linux-ker...@vger.kernel.org
---
 arch/x86/entry/entry_32.S|  2 +-
 arch/x86/include/asm/desc.h  | 16 +++-
 arch/x86/kernel/cpu/common.c | 10 --
 arch/x86/kernel/cpu/perf_event.c |  2 +-
 arch/x86/kernel/head_32.S|  4 ++--
 arch/x86/kernel/head_64.S|  2 +-
 arch/x86/kernel/vmlinux.lds.S|  2 +-
 arch/x86/tools/relocs.c  |  2 +-
 arch/x86/xen/enlighten.c |  4 ++--
 9 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index b2909bf..bc6ae1c 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -429,7 +429,7 @@ ldt_ss:
  * compensating for the offset by changing to the ESPFIX segment with
  * a base address that matches for the difference.
  */
-#define GDT_ESPFIX_SS PER_CPU_VAR(gdt_page) + (GDT_ENTRY_ESPFIX_SS * 8)
+#define GDT_ESPFIX_SS PER_CPU_VAR(cpu_gdt) + (GDT_ENTRY_ESPFIX_SS * 8)
mov %esp, %edx  /* load kernel esp */
mov PT_OLDESP(%esp), %eax   /* load userspace esp */
mov %dx, %ax/* eax: new kernel esp */
diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
index 4e10d73..76de300 100644
--- a/arch/x86/include/asm/desc.h
+++ b/arch/x86/include/asm/desc.h
@@ -39,15 +39,21 @@ extern gate_desc idt_table[];
 extern struct desc_ptr debug_idt_descr;
 extern gate_desc debug_idt_table[];
 
-struct gdt_page {
+struct cpu_gdt {
struct desc_struct gdt[GDT_ENTRIES];
-} __attribute__((aligned(PAGE_SIZE)));
-
-DECLARE_PER_CPU_PAGE_ALIGNED(struct gdt_page, gdt_page);
+}
+#ifdef CONFIG_HYPERVISOR_GUEST
+/* Xen et al want GDT to have its own page. They remap it read-only */
+__attribute__((aligned(PAGE_SIZE)));
+DECLARE_PER_CPU_PAGE_ALIGNED(struct cpu_gdt, cpu_gdt);
+#else
+cacheline_aligned;
+DECLARE_PER_CPU_ALIGNED(struct cpu_gdt, cpu_gdt);
+#endif
 
 static inline struct desc_struct *get_cpu_gdt_table(unsigned int cpu)
 {
-   return per_cpu(gdt_page, cpu).gdt;
+   return per_cpu(cpu_gdt, cpu).gdt;
 }
 
 #ifdef CONFIG_X86_64
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index de22ea7..6b90785 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -92,7 +92,13 @@ static const struct cpu_dev default_cpu = {
 
 static const struct cpu_dev *this_cpu = _cpu;
 
-DEFINE_PER_CPU_PAGE_ALIGNED(struct gdt_page, gdt_page) = { .gdt = {
+#ifdef CONFIG_HYPERVISOR_GUEST
+/* Xen et al want GDT to have its own page. They remap it read-only */

Re: [PATCH] x86: Use entire page for the per-cpu GDT only if paravirt-enabled

2015-09-26 Thread H. Peter Anvin
NAK.  We really should map the GDT read-only on all 64 bit systems, since we 
can't hide the address from SLDT.  Same with the IDT.

On September 26, 2015 11:00:40 AM PDT, Denys Vlasenko  
wrote:
>We have our GDT in a page-sized per-cpu structure, gdt_page.
>
>On x86_64 kernel, GDT is 128 bytes - only ~3% of that page is used.
>
>It is page-sized because of paravirt. Hypervisors need to know when
>GDT is changed, so they remap it read-only and handle write faults.
>If it's not in its own page, other writes nearby will cause
>those faults too.
>
>In other words, we need GDT to live in a separate page
>only if CONFIG_HYPERVISOR_GUEST=y.
>
>This patch reduces GDT alignment to cacheline-aligned
>if CONFIG_HYPERVISOR_GUEST is not set.
>
>Patch also renames gdt_page to cpu_gdt (mimicking naming of existing
>cpu_tss per-cpu variable), since now it is not always a full page.
>
>$ objdump -x vmlinux | grep .data..percpu | sort
>Before:
>(offset)(size)
>  wO .data..percpu  4000
>irq_stack_union
>4000  wO .data..percpu  5000
>exception_stacks
>9000  wO .data..percpu  1000 gdt_page  <<<
>HERE
>  a000  wO .data..percpu  0008 espfix_waddr
>  a008  wO .data..percpu  0008 espfix_stack
>...
> 00019398 g   .data..percpu   __per_cpu_end
>After:
>  wO .data..percpu  4000
>irq_stack_union
>4000  wO .data..percpu  5000
>exception_stacks
>  9000  wO .data..percpu  0008 espfix_waddr
>  9008  wO .data..percpu  0008 espfix_stack
>...
>00013c80  wO .data..percpu  0040 cyc2ns
>00013cc0  wO .data..percpu  22c0 cpu_tss
>00015f80  wO .data..percpu  0080 cpu_gdt  <<<
>HERE
>  00016000  wO .data..percpu  0018 cpu_tlbstate
>...
> 00018418 g   .data..percpu   __per_cpu_end
>
>Run-tested on a 144 CPU machine.
>
>Signed-off-by: Denys Vlasenko 
>CC: Ingo Molnar 
>CC: H. Peter Anvin 
>CC: Konrad Rzeszutek Wilk 
>CC: Boris Ostrovsky 
>CC: David Vrabel 
>CC: Joerg Roedel 
>CC: Gleb Natapov 
>CC: Paolo Bonzini 
>CC: kvm@vger.kernel.org
>CC: x...@kernel.org
>CC: linux-ker...@vger.kernel.org
>---
> arch/x86/entry/entry_32.S|  2 +-
> arch/x86/include/asm/desc.h  | 16 +++-
> arch/x86/kernel/cpu/common.c | 10 --
> arch/x86/kernel/cpu/perf_event.c |  2 +-
> arch/x86/kernel/head_32.S|  4 ++--
> arch/x86/kernel/head_64.S|  2 +-
> arch/x86/kernel/vmlinux.lds.S|  2 +-
> arch/x86/tools/relocs.c  |  2 +-
> arch/x86/xen/enlighten.c |  4 ++--
> 9 files changed, 28 insertions(+), 16 deletions(-)
>
>diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
>index b2909bf..bc6ae1c 100644
>--- a/arch/x86/entry/entry_32.S
>+++ b/arch/x86/entry/entry_32.S
>@@ -429,7 +429,7 @@ ldt_ss:
>  * compensating for the offset by changing to the ESPFIX segment with
>  * a base address that matches for the difference.
>  */
>-#define GDT_ESPFIX_SS PER_CPU_VAR(gdt_page) + (GDT_ENTRY_ESPFIX_SS *
>8)
>+#define GDT_ESPFIX_SS PER_CPU_VAR(cpu_gdt) + (GDT_ENTRY_ESPFIX_SS * 8)
>   mov %esp, %edx  /* load kernel esp */
>   mov PT_OLDESP(%esp), %eax   /* load userspace esp */
>   mov %dx, %ax/* eax: new kernel esp */
>diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
>index 4e10d73..76de300 100644
>--- a/arch/x86/include/asm/desc.h
>+++ b/arch/x86/include/asm/desc.h
>@@ -39,15 +39,21 @@ extern gate_desc idt_table[];
> extern struct desc_ptr debug_idt_descr;
> extern gate_desc debug_idt_table[];
> 
>-struct gdt_page {
>+struct cpu_gdt {
>   struct desc_struct gdt[GDT_ENTRIES];
>-} __attribute__((aligned(PAGE_SIZE)));
>-
>-DECLARE_PER_CPU_PAGE_ALIGNED(struct gdt_page, gdt_page);
>+}
>+#ifdef CONFIG_HYPERVISOR_GUEST
>+/* Xen et al want GDT to have its own page. They remap it read-only */
>+__attribute__((aligned(PAGE_SIZE)));
>+DECLARE_PER_CPU_PAGE_ALIGNED(struct cpu_gdt, cpu_gdt);
>+#else
>+cacheline_aligned;
>+DECLARE_PER_CPU_ALIGNED(struct cpu_gdt, cpu_gdt);
>+#endif
> 
> static inline struct desc_struct *get_cpu_gdt_table(unsigned int cpu)
> {
>-  return per_cpu(gdt_page, cpu).gdt;
>+  return per_cpu(cpu_gdt, cpu).gdt;
> }
> 
> #ifdef CONFIG_X86_64
>diff --git a/arch/x86/kernel/cpu/common.c
>b/arch/x86/kernel/cpu/common.c
>index de22ea7..6b90785 100644
>--- a/arch/x86/kernel/cpu/common.c
>+++ b/arch/x86/kernel/cpu/common.c
>@@