Re: [PATCH v2 13/36] irqchip/gic-v4.1: Don't use the VPE proxy if RVPEID is set

2019-12-18 Thread Zenghui Yu

Hi Marc,

On 2019/12/18 22:39, Marc Zyngier wrote:

On 2019-11-01 11:05, Zenghui Yu wrote:

Hi Marc,

On 2019/10/27 22:42, Marc Zyngier wrote:

The infamous VPE proxy device isn't used with GICv4.1 because:
- we can invalidate any LPI from the DirectLPI MMIO interface
- the ITS and redistributors understand the life cycle of
   the doorbell, so we don't need to enable/disable it all
   the time
So let's escape early from the proxy related functions.
Signed-off-by: Marc Zyngier 


Reviewed-by: Zenghui Yu 


---
  drivers/irqchip/irq-gic-v3-its.c | 23 ++-
  1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c 
b/drivers/irqchip/irq-gic-v3-its.c

index 220d490d516e..999e61a9b2c3 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3069,7 +3069,7 @@ static const struct irq_domain_ops 
its_domain_ops = {

  /*
   * This is insane.
   *
- * If a GICv4 doesn't implement Direct LPIs (which is extremely
+ * If a GICv4.0 doesn't implement Direct LPIs (which is extremely
   * likely), the only way to perform an invalidate is to use a fake
   * device to issue an INV command, implying that the LPI has first
   * been mapped to some event on that device. Since this is not exactly
@@ -3077,9 +3077,18 @@ static const struct irq_domain_ops 
its_domain_ops = {

   * only issue an UNMAP if we're short on available slots.
   *
   * Broken by design(tm).
+ *
+ * GICv4.1 actually mandates that we're able to invalidate by 
writing to a
+ * MMIO register. It doesn't implement the whole of DirectLPI, but 
that's

+ * good enough. And most of the time, we don't even have to invalidate
+ * anything, so that's actually pretty good!


I can't understand the meaning of this last sentence. May I ask for an
explanation? :)


Yeah, reading this now, it feels pretty clumsy, and only remotely
connected to the patch.

What I'm trying to say here is that, although GICv4.1 doesn't have
the full spectrum of v4.0 DirectLPI (it only allows a subset of it),
this subset is more then enough for us. Here's the rational:

When a vPE exits from the hypervisor, we know whether we need to
request a doorbell or not (depending on whether we're blocking on
WFI or not). On GICv4.0, this translates into enabling the doorbell
interrupt, which generates an invalidation (costly). And whenever
we've taken a doorbell, or are scheduled again, we need to turn
the doorbell off (invalidation again).

With v4.1, we can just say *at exit time* whether we want doorbells
to be subsequently generated (see its_vpe_4_1_deschedule() and the
req_db parameter in the info structure). This is part of making
the vPE non-resident, so we have 0 overhead at this stage.


Great, and get it. Thanks for this clear explanation!


Zenghui

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 30/45] KVM: Move vcpu alloc and init invocation to common code

2019-12-18 Thread Sean Christopherson
Now that all architectures tightly couple vcpu allocation/free with the
mandatory calls to kvm_{un}init_vcpu(), move the sequences verbatim to
common KVM code.

Move both allocation and initialization in a single patch to eliminate
thrash in arch specific code.  The bisection benefits of moving the two
pieces in separate patches is marginal at best, whereas the odds of
introducing a transient arch specific bug are non-zero.

Acked-by: Christoffer Dall 
Signed-off-by: Sean Christopherson 
---
 arch/mips/kvm/mips.c   | 33 ++---
 arch/powerpc/kvm/powerpc.c | 27 ---
 arch/s390/kvm/kvm-s390.c   | 31 +--
 arch/x86/kvm/x86.c | 28 ++--
 include/linux/kvm_host.h   |  2 +-
 virt/kvm/arm/arm.c | 29 ++---
 virt/kvm/kvm_main.c| 21 ++---
 7 files changed, 38 insertions(+), 133 deletions(-)

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 8546bc6e09e7..92c9321b3f95 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -285,25 +285,14 @@ int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int 
id)
return 0;
 }
 
-struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
+int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 {
int err, size;
void *gebase, *p, *handler, *refill_start, *refill_end;
int i;
 
-   struct kvm_vcpu *vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
-
-   if (!vcpu) {
-   err = -ENOMEM;
-   goto out;
-   }
-
-   err = kvm_vcpu_init(vcpu, kvm, id);
-
-   if (err)
-   goto out_free_cpu;
-
-   kvm_debug("kvm @ %p: create cpu %d at %p\n", kvm, id, vcpu);
+   kvm_debug("kvm @ %p: create cpu %d at %p\n",
+ vcpu->kvm, vcpu->vcpu_id, vcpu);
 
/*
 * Allocate space for host mode exception handlers that handle
@@ -318,7 +307,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, 
unsigned int id)
 
if (!gebase) {
err = -ENOMEM;
-   goto out_uninit_cpu;
+   goto out;
}
kvm_debug("Allocated %d bytes for KVM Exception Handlers @ %p\n",
  ALIGN(size, PAGE_SIZE), gebase);
@@ -397,19 +386,12 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, 
unsigned int id)
vcpu->arch.last_sched_cpu = -1;
vcpu->arch.last_exec_cpu = -1;
 
-   return vcpu;
+   return 0;
 
 out_free_gebase:
kfree(gebase);
-
-out_uninit_cpu:
-   kvm_vcpu_uninit(vcpu);
-
-out_free_cpu:
-   kmem_cache_free(kvm_vcpu_cache, vcpu);
-
 out:
-   return ERR_PTR(err);
+   return err;
 }
 
 void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
@@ -421,9 +403,6 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
kvm_mmu_free_memory_caches(vcpu);
kfree(vcpu->arch.guest_ebase);
kfree(vcpu->arch.kseg0_commpage);
-
-   kvm_vcpu_uninit(vcpu);
-   kmem_cache_free(kvm_vcpu_cache, vcpu);
 }
 
 int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index e3e2b88d3d8b..fce1b4776e55 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -725,32 +725,17 @@ int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int 
id)
return 0;
 }
 
-struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
+int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 {
-   struct kvm_vcpu *vcpu;
int err;
 
-   vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
-   if (!vcpu)
-   return ERR_PTR(-ENOMEM);
-
-   err = kvm_vcpu_init(vcpu, kvm, id);
-   if (err)
-   goto free_vcpu;
-
err = kvmppc_core_vcpu_create(vcpu);
if (err)
-   goto uninit_vcpu;
+   return err;
 
vcpu->arch.wqp = >wq;
-   kvmppc_create_vcpu_debugfs(vcpu, id);
-   return vcpu;
-
-uninit_vcpu:
-   kvm_vcpu_uninit(vcpu);
-free_vcpu:
-   kmem_cache_free(kvm_vcpu_cache, vcpu);
-   return ERR_PTR(err);
+   kvmppc_create_vcpu_debugfs(vcpu, vcpu->vcpu_id);
+   return 0;
 }
 
 void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
@@ -780,10 +765,6 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
}
 
kvmppc_core_vcpu_free(vcpu);
-
-   kvm_vcpu_uninit(vcpu);
-
-   kmem_cache_free(kvm_vcpu_cache, vcpu);
 }
 
 int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 8543d338a06a..2ed76584ebd9 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2530,9 +2530,6 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
if (vcpu->kvm->arch.use_cmma)
kvm_s390_vcpu_unsetup_cmma(vcpu);
free_page((unsigned long)(vcpu->arch.sie_block));
-
-   kvm_vcpu_uninit(vcpu);

[PATCH v2 36/45] KVM: PPC: BookE: Setup vcpu during kvmppc_core_vcpu_create()

2019-12-18 Thread Sean Christopherson
Fold setup() into create() now that the two are called back-to-back by
common KVM code.  This paves the way for removing kvm_arch_vcpu_setup().
Note, BookE directly implements kvm_arch_vcpu_setup() and PPC's common
kvm_arch_vcpu_create() is responsible for its own cleanup, thus the only
cleanup required when directly invoking kvmppc_core_vcpu_setup() is to
call .vcpu_free(), which is the BookE specific portion of PPC's
kvm_arch_vcpu_destroy() by way of kvmppc_core_vcpu_free().

No functional change intended.

Signed-off-by: Sean Christopherson 
---
 arch/powerpc/kvm/booke.c | 60 ++--
 1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index dd7440e50c7a..b1b5073a22b1 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -1377,34 +1377,9 @@ static void kvmppc_set_tsr(struct kvm_vcpu *vcpu, u32 
new_tsr)
update_timer_ints(vcpu);
 }
 
-/* Initial guest state: 16MB mapping 0 -> 0, PC = 0, MSR = 0, R1 = 16MB */
 int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 {
-   int i;
-   int r;
-
-   vcpu->arch.regs.nip = 0;
-   vcpu->arch.shared->pir = vcpu->vcpu_id;
-   kvmppc_set_gpr(vcpu, 1, (16<<20) - 8); /* -8 for the callee-save LR 
slot */
-   kvmppc_set_msr(vcpu, 0);
-
-#ifndef CONFIG_KVM_BOOKE_HV
-   vcpu->arch.shadow_msr = MSR_USER | MSR_IS | MSR_DS;
-   vcpu->arch.shadow_pid = 1;
-   vcpu->arch.shared->msr = 0;
-#endif
-
-   /* Eye-catching numbers so we know if the guest takes an interrupt
-* before it's programmed its own IVPR/IVORs. */
-   vcpu->arch.ivpr = 0x;
-   for (i = 0; i < BOOKE_IRQPRIO_MAX; i++)
-   vcpu->arch.ivor[i] = 0x7700 | i * 4;
-
-   kvmppc_init_timing_stats(vcpu);
-
-   r = kvmppc_core_vcpu_setup(vcpu);
-   kvmppc_sanity_check(vcpu);
-   return r;
+   return 0;
 }
 
 int kvmppc_subarch_vcpu_init(struct kvm_vcpu *vcpu)
@@ -2116,7 +2091,38 @@ int kvmppc_core_init_vm(struct kvm *kvm)
 
 int kvmppc_core_vcpu_create(struct kvm_vcpu *vcpu)
 {
-   return kvm->arch.kvm_ops->vcpu_create(vcpu);
+   int i;
+   int r;
+
+   r = kvm->arch.kvm_ops->vcpu_create(vcpu);
+   if (r)
+   return r;
+
+   /* Initial guest state: 16MB mapping 0 -> 0, PC = 0, MSR = 0, R1 = 16MB 
*/
+   vcpu->arch.regs.nip = 0;
+   vcpu->arch.shared->pir = vcpu->vcpu_id;
+   kvmppc_set_gpr(vcpu, 1, (16<<20) - 8); /* -8 for the callee-save LR 
slot */
+   kvmppc_set_msr(vcpu, 0);
+
+#ifndef CONFIG_KVM_BOOKE_HV
+   vcpu->arch.shadow_msr = MSR_USER | MSR_IS | MSR_DS;
+   vcpu->arch.shadow_pid = 1;
+   vcpu->arch.shared->msr = 0;
+#endif
+
+   /* Eye-catching numbers so we know if the guest takes an interrupt
+* before it's programmed its own IVPR/IVORs. */
+   vcpu->arch.ivpr = 0x;
+   for (i = 0; i < BOOKE_IRQPRIO_MAX; i++)
+   vcpu->arch.ivor[i] = 0x7700 | i * 4;
+
+   kvmppc_init_timing_stats(vcpu);
+
+   r = kvmppc_core_vcpu_setup(vcpu);
+   if (r)
+   vcpu->kvm->arch.kvm_ops->vcpu_free(vcpu);
+   kvmppc_sanity_check(vcpu);
+   return r;
 }
 
 void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu)
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 31/45] KVM: Unexport kvm_vcpu_cache and kvm_vcpu_{un}init()

2019-12-18 Thread Sean Christopherson
Unexport kvm_vcpu_cache and kvm_vcpu_{un}init() and make them static
now that they are referenced only in kvm_main.c.

Acked-by: Christoffer Dall 
Signed-off-by: Sean Christopherson 
---
 include/linux/kvm_host.h | 4 
 virt/kvm/kvm_main.c  | 9 +++--
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index a9abd9e9f621..d24e6c134d15 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -157,8 +157,6 @@ static inline bool is_error_page(struct page *page)
 #define KVM_USERSPACE_IRQ_SOURCE_ID0
 #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID   1
 
-extern struct kmem_cache *kvm_vcpu_cache;
-
 extern struct mutex kvm_lock;
 extern struct list_head vm_list;
 
@@ -579,8 +577,6 @@ static inline int kvm_vcpu_get_idx(struct kvm_vcpu *vcpu)
  memslot < slots->memslots + KVM_MEM_SLOTS_NUM && memslot->npages;\
memslot++)
 
-int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id);
-void kvm_vcpu_uninit(struct kvm_vcpu *vcpu);
 void kvm_vcpu_destroy(struct kvm_vcpu *vcpu);
 
 void vcpu_load(struct kvm_vcpu *vcpu);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1afffb0da7cc..fd8168b8c0e4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -104,8 +104,7 @@ static cpumask_var_t cpus_hardware_enabled;
 static int kvm_usage_count;
 static atomic_t hardware_enable_failed;
 
-struct kmem_cache *kvm_vcpu_cache;
-EXPORT_SYMBOL_GPL(kvm_vcpu_cache);
+static struct kmem_cache *kvm_vcpu_cache;
 
 static __read_mostly struct preempt_ops kvm_preempt_ops;
 
@@ -322,7 +321,7 @@ void kvm_reload_remote_mmus(struct kvm *kvm)
kvm_make_all_cpus_request(kvm, KVM_REQ_MMU_RELOAD);
 }
 
-int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id)
+static int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id)
 {
struct page *page;
int r;
@@ -360,9 +359,8 @@ int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, 
unsigned id)
 fail:
return r;
 }
-EXPORT_SYMBOL_GPL(kvm_vcpu_init);
 
-void kvm_vcpu_uninit(struct kvm_vcpu *vcpu)
+static void kvm_vcpu_uninit(struct kvm_vcpu *vcpu)
 {
/*
 * no need for rcu_read_lock as VCPU_RUN is the only place that
@@ -373,7 +371,6 @@ void kvm_vcpu_uninit(struct kvm_vcpu *vcpu)
kvm_arch_vcpu_uninit(vcpu);
free_page((unsigned long)vcpu->run);
 }
-EXPORT_SYMBOL_GPL(kvm_vcpu_uninit);
 
 void kvm_vcpu_destroy(struct kvm_vcpu *vcpu)
 {
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 45/45] KVM: Move vcpu->run page allocation out of kvm_vcpu_init()

2019-12-18 Thread Sean Christopherson
Open code the allocation and freeing of the vcpu->run page in
kvm_vm_ioctl_create_vcpu() and kvm_vcpu_destroy() respectively.  Doing
so allows kvm_vcpu_init() to be a pure init function and eliminates
kvm_vcpu_uninit() entirely.

Signed-off-by: Sean Christopherson 
---
 virt/kvm/kvm_main.c | 34 +-
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 6912d81ca32d..220dce38cf79 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -321,10 +321,8 @@ void kvm_reload_remote_mmus(struct kvm *kvm)
kvm_make_all_cpus_request(kvm, KVM_REQ_MMU_RELOAD);
 }
 
-static int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id)
+static void kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id)
 {
-   struct page *page;
-
mutex_init(>mutex);
vcpu->cpu = -1;
vcpu->kvm = kvm;
@@ -336,23 +334,11 @@ static int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct 
kvm *kvm, unsigned id)
vcpu->pre_pcpu = -1;
INIT_LIST_HEAD(>blocked_vcpu_list);
 
-   page = alloc_page(GFP_KERNEL | __GFP_ZERO);
-   if (!page)
-   return -ENOMEM;
-   vcpu->run = page_address(page);
-
kvm_vcpu_set_in_spin_loop(vcpu, false);
kvm_vcpu_set_dy_eligible(vcpu, false);
vcpu->preempted = false;
vcpu->ready = false;
preempt_notifier_init(>preempt_notifier, _preempt_ops);
-
-   return 0;
-}
-
-static void kvm_vcpu_uninit(struct kvm_vcpu *vcpu)
-{
-   free_page((unsigned long)vcpu->run);
 }
 
 void kvm_vcpu_destroy(struct kvm_vcpu *vcpu)
@@ -366,7 +352,7 @@ void kvm_vcpu_destroy(struct kvm_vcpu *vcpu)
 */
put_pid(rcu_dereference_protected(vcpu->pid, 1));
 
-   kvm_vcpu_uninit(vcpu);
+   free_page((unsigned long)vcpu->run);
kmem_cache_free(kvm_vcpu_cache, vcpu);
 }
 EXPORT_SYMBOL_GPL(kvm_vcpu_destroy);
@@ -2711,6 +2697,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 
id)
 {
int r;
struct kvm_vcpu *vcpu;
+   struct page *page;
 
if (id >= KVM_MAX_VCPU_ID)
return -EINVAL;
@@ -2734,13 +2721,18 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, 
u32 id)
goto vcpu_decrement;
}
 
-   r = kvm_vcpu_init(vcpu, kvm, id);
-   if (r)
+   page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+   if (!page) {
+   r = -ENOMEM;
goto vcpu_free;
+   }
+   vcpu->run = page_address(page);
+
+   kvm_vcpu_init(vcpu, kvm, id);
 
r = kvm_arch_vcpu_create(vcpu);
if (r)
-   goto vcpu_uninit;
+   goto vcpu_free_run_page;
 
kvm_create_vcpu_debugfs(vcpu);
 
@@ -2778,8 +2770,8 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 
id)
mutex_unlock(>lock);
debugfs_remove_recursive(vcpu->debugfs_dentry);
kvm_arch_vcpu_destroy(vcpu);
-vcpu_uninit:
-   kvm_vcpu_uninit(vcpu);
+vcpu_free_run_page:
+   free_page((unsigned long)vcpu->run);
 vcpu_free:
kmem_cache_free(kvm_vcpu_cache, vcpu);
 vcpu_decrement:
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 35/45] KVM: s390: Manually invoke vcpu setup during kvm_arch_vcpu_create()

2019-12-18 Thread Sean Christopherson
Rename kvm_arch_vcpu_setup() to kvm_s390_vcpu_setup() and manually call
the new function during kvm_arch_vcpu_create().  Define an empty
kvm_arch_vcpu_setup() as it's still required for compilation.  This
is effectively a nop as kvm_arch_vcpu_create() and kvm_arch_vcpu_setup()
are called back-to-back by common KVM code.  Obsoleting
kvm_arch_vcpu_setup() paves the way for its removal.

Note, gmap_remove() is now called if setup fails, as s390 was previously
freeing it via kvm_arch_vcpu_destroy(), which is called by common KVM
code if kvm_arch_vcpu_setup() fails.

No functional change intended.

Signed-off-by: Sean Christopherson 
---
 arch/s390/kvm/kvm-s390.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 2ed76584ebd9..5cd92c9fc050 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2932,6 +2932,11 @@ static void kvm_s390_vcpu_setup_model(struct kvm_vcpu 
*vcpu)
 }
 
 int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
+{
+   return 0;
+}
+
+static int kvm_s390_vcpu_setup(struct kvm_vcpu *vcpu)
 {
int rc = 0;
 
@@ -3070,8 +3075,14 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 vcpu->arch.sie_block);
trace_kvm_s390_create_vcpu(id, vcpu, vcpu->arch.sie_block);
 
+   rc = kvm_s390_vcpu_setup(vcpu);
+   if (rc)
+   goto out_ucontrol_uninit;
return 0;
 
+out_ucontrol_uninit:
+   if (kvm_is_ucontrol(vcpu->kvm))
+   gmap_remove(vcpu->arch.gmap);
 out_free_sie_block:
free_page((unsigned long)(vcpu->arch.sie_block));
return rc;
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 37/45] KVM: Drop kvm_arch_vcpu_setup()

2019-12-18 Thread Sean Christopherson
Remove kvm_arch_vcpu_setup() now that all arch specific implementations
are nops.

Acked-by: Christoffer Dall 
Signed-off-by: Sean Christopherson 
---
 arch/arm/kvm/guest.c  | 5 -
 arch/arm64/kvm/guest.c| 5 -
 arch/mips/kvm/mips.c  | 5 -
 arch/powerpc/kvm/book3s.c | 5 -
 arch/powerpc/kvm/booke.c  | 5 -
 arch/s390/kvm/kvm-s390.c  | 5 -
 arch/x86/kvm/x86.c| 5 -
 include/linux/kvm_host.h  | 1 -
 virt/kvm/kvm_main.c   | 5 -
 9 files changed, 41 deletions(-)

diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
index 0e6f23504c26..9f7ae0d8690f 100644
--- a/arch/arm/kvm/guest.c
+++ b/arch/arm/kvm/guest.c
@@ -34,11 +34,6 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
{ NULL }
 };
 
-int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
-{
-   return 0;
-}
-
 static u64 core_reg_offset_from_id(u64 id)
 {
return id & ~(KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_REG_ARM_CORE);
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 2fff06114a8f..2bd92301d32f 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -47,11 +47,6 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
{ NULL }
 };
 
-int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
-{
-   return 0;
-}
-
 static bool core_reg_offset_is_vreg(u64 off)
 {
return off >= KVM_REG_ARM_CORE_REG(fp_regs.vregs) &&
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index b3a4435af66b..06366e2415a6 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -1244,11 +1244,6 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
return 0;
 }
 
-int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
-{
-   return 0;
-}
-
 static void kvm_mips_set_c0_status(void)
 {
u32 status = read_c0_status();
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 5ad20fc0c6a1..0d393f1a835b 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -471,11 +471,6 @@ int kvmppc_load_last_inst(struct kvm_vcpu *vcpu,
 }
 EXPORT_SYMBOL_GPL(kvmppc_load_last_inst);
 
-int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
-{
-   return 0;
-}
-
 int kvmppc_subarch_vcpu_init(struct kvm_vcpu *vcpu)
 {
return 0;
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index b1b5073a22b1..35b781775d3d 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -1377,11 +1377,6 @@ static void kvmppc_set_tsr(struct kvm_vcpu *vcpu, u32 
new_tsr)
update_timer_ints(vcpu);
 }
 
-int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
-{
-   return 0;
-}
-
 int kvmppc_subarch_vcpu_init(struct kvm_vcpu *vcpu)
 {
/* setup watchdog timer once */
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 5cd92c9fc050..bcdd542b96a2 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2931,11 +2931,6 @@ static void kvm_s390_vcpu_setup_model(struct kvm_vcpu 
*vcpu)
vcpu->arch.sie_block->fac = (u32)(u64) model->fac_list;
 }
 
-int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
-{
-   return 0;
-}
-
 static int kvm_s390_vcpu_setup(struct kvm_vcpu *vcpu)
 {
int rc = 0;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0ab96dc3dd08..20f8ac086824 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9116,11 +9116,6 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
return 0;
 }
 
-int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
-{
-   return 0;
-}
-
 void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
 {
struct msr_data msr;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index d24e6c134d15..792b122e44fa 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -873,7 +873,6 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu);
 int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int id);
 int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu);
-int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu);
 void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu);
 void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu);
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 876cf3dd2c97..f76b2155dfee 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2753,10 +2753,6 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 
id)
if (r)
goto vcpu_uninit;
 
-   r = kvm_arch_vcpu_setup(vcpu);
-   if (r)
-   goto vcpu_destroy;
-
kvm_create_vcpu_debugfs(vcpu);
 
mutex_lock(>lock);
@@ -2792,7 +2788,6 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 
id)
 unlock_vcpu_destroy:
mutex_unlock(>lock);
debugfs_remove_recursive(vcpu->debugfs_dentry);
-vcpu_destroy:
kvm_arch_vcpu_destroy(vcpu);
 vcpu_uninit:
kvm_vcpu_uninit(vcpu);
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu

[PATCH v2 40/45] KVM: ARM: Move all vcpu init code into kvm_arch_vcpu_create()

2019-12-18 Thread Sean Christopherson
Fold init() into create() now that the two are called back-to-back by
common KVM code (kvm_vcpu_init() calls kvm_arch_vcpu_init() as its last
action, and kvm_vm_ioctl_create_vcpu() calls kvm_arch_vcpu_create()
immediately thereafter).  This paves the way for removing
kvm_arch_vcpu_{un}init() entirely.

Note, there is no associated unwinding in kvm_arch_vcpu_uninit() that
needs to be relocated (to kvm_arch_vcpu_destroy()).

No functional change intended.

Signed-off-by: Sean Christopherson 
---
 virt/kvm/arm/arm.c | 34 --
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 35eab584fb57..1d87c194a6d3 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -292,6 +292,25 @@ int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int 
id)
 
 int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 {
+   int err;
+
+   /* Force users to call KVM_ARM_VCPU_INIT */
+   vcpu->arch.target = -1;
+   bitmap_zero(vcpu->arch.features, KVM_VCPU_MAX_FEATURES);
+
+   /* Set up the timer */
+   kvm_timer_vcpu_init(vcpu);
+
+   kvm_pmu_vcpu_init(vcpu);
+
+   kvm_arm_reset_debug_ptr(vcpu);
+
+   kvm_arm_pvtime_vcpu_init(>arch);
+
+   err = kvm_vgic_vcpu_init(vcpu);
+   if (err)
+   return err;
+
return create_hyp_mappings(vcpu, vcpu + 1, PAGE_HYP);
 }
 
@@ -341,20 +360,7 @@ void kvm_arch_vcpu_unblocking(struct kvm_vcpu *vcpu)
 
 int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 {
-   /* Force users to call KVM_ARM_VCPU_INIT */
-   vcpu->arch.target = -1;
-   bitmap_zero(vcpu->arch.features, KVM_VCPU_MAX_FEATURES);
-
-   /* Set up the timer */
-   kvm_timer_vcpu_init(vcpu);
-
-   kvm_pmu_vcpu_init(vcpu);
-
-   kvm_arm_reset_debug_ptr(vcpu);
-
-   kvm_arm_pvtime_vcpu_init(>arch);
-
-   return kvm_vgic_vcpu_init(vcpu);
+   return 0;
 }
 
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 43/45] KVM: Drop kvm_arch_vcpu_init() and kvm_arch_vcpu_uninit()

2019-12-18 Thread Sean Christopherson
Remove kvm_arch_vcpu_init() and kvm_arch_vcpu_uninit() now that all
arch specific implementations are nops.

Acked-by: Christoffer Dall 
Signed-off-by: Sean Christopherson 
---
 arch/arm/include/asm/kvm_host.h   |  1 -
 arch/arm64/include/asm/kvm_host.h |  1 -
 arch/arm64/kvm/reset.c|  5 -
 arch/mips/kvm/mips.c  | 10 --
 arch/powerpc/kvm/powerpc.c| 10 --
 arch/s390/include/asm/kvm_host.h  |  1 -
 arch/s390/kvm/kvm-s390.c  |  5 -
 arch/x86/kvm/x86.c| 10 --
 include/linux/kvm_host.h  |  3 ---
 virt/kvm/arm/arm.c|  5 -
 virt/kvm/kvm_main.c   | 16 ++--
 11 files changed, 2 insertions(+), 65 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index de81dd897a30..e26cad6d11b3 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -363,7 +363,6 @@ struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, 
unsigned long mpidr);
 static inline bool kvm_arch_requires_vhe(void) { return false; }
 static inline void kvm_arch_hardware_unsetup(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
-static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
 static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
 static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
 static inline void kvm_arm_vcpu_destroy(struct kvm_vcpu *vcpu) {}
diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index b1de2b147729..7aef2e4114d8 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -54,7 +54,6 @@ int kvm_arm_init_sve(void);
 int __attribute_const__ kvm_target_cpu(void);
 int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
 void kvm_arm_vcpu_destroy(struct kvm_vcpu *vcpu);
-void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu);
 int kvm_arch_vm_ioctl_check_extension(struct kvm *kvm, long ext);
 void __extended_idmap_trampoline(phys_addr_t boot_pgd, phys_addr_t 
idmap_start);
 
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index ff3512a0ca97..30b7ea680f66 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -204,11 +204,6 @@ bool kvm_arm_vcpu_is_finalized(struct kvm_vcpu *vcpu)
return true;
 }
 
-void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
-{
-
-}
-
 void kvm_arm_vcpu_destroy(struct kvm_vcpu *vcpu)
 {
kfree(vcpu->arch.sve_state);
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 879a7cbd5b54..2606f3f02b54 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -1230,16 +1230,6 @@ static enum hrtimer_restart 
kvm_mips_comparecount_wakeup(struct hrtimer *timer)
return kvm_mips_count_timeout(vcpu);
 }
 
-int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
-{
-   return 0;
-}
-
-void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
-{
-
-}
-
 int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
  struct kvm_translation *tr)
 {
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 91cf94d4191e..80f2d1a3ca63 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -801,16 +801,6 @@ static enum hrtimer_restart 
kvmppc_decrementer_wakeup(struct hrtimer *timer)
return HRTIMER_NORESTART;
 }
 
-int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
-{
-   return 0;
-}
-
-void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
-{
-
-}
-
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 {
 #ifdef CONFIG_BOOKE
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 02f4c21c57f6..11ecc4071a29 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -914,7 +914,6 @@ extern int kvm_s390_gisc_unregister(struct kvm *kvm, u32 
gisc);
 
 static inline void kvm_arch_hardware_disable(void) {}
 static inline void kvm_arch_sync_events(struct kvm *kvm) {}
-static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
 static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
 static inline void kvm_arch_free_memslot(struct kvm *kvm,
struct kvm_memory_slot *free, struct kvm_memory_slot *dont) {}
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index bcdd542b96a2..c059b86aacd4 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2700,11 +2700,6 @@ static int sca_can_add_vcpu(struct kvm *kvm, unsigned 
int id)
return rc == 0 && id < KVM_S390_ESCA_CPU_SLOTS;
 }
 
-int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
-{
-   return 0;
-}
-
 /* needs disabled preemption to protect from TOD sync and vcpu_load/put */
 static void __start_cpu_timer_accounting(struct kvm_vcpu *vcpu)
 {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 24671f487645..292f1e53a758 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9485,16 +9485,6 @@ bool kvm_vcpu_is_bsp(struct 

[PATCH v2 42/45] KVM: arm64: Free sve_state via arm specific hook

2019-12-18 Thread Sean Christopherson
Add an arm specific hook to free the arm64-only sve_state.  Doing so
eliminates the last functional code from kvm_arch_vcpu_uninit() across
all architectures and paves the way for removing kvm_arch_vcpu_init()
and kvm_arch_vcpu_uninit() entirely.

Signed-off-by: Sean Christopherson 
---
 arch/arm/include/asm/kvm_host.h   | 1 +
 arch/arm64/include/asm/kvm_host.h | 1 +
 arch/arm64/kvm/reset.c| 5 +
 virt/kvm/arm/arm.c| 2 ++
 4 files changed, 9 insertions(+)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 556cd818eccf..de81dd897a30 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -366,6 +366,7 @@ static inline void kvm_arch_sync_events(struct kvm *kvm) {}
 static inline void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu) {}
 static inline void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu) {}
 static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
+static inline void kvm_arm_vcpu_destroy(struct kvm_vcpu *vcpu) {}
 
 static inline void kvm_arm_init_debug(void) {}
 static inline void kvm_arm_setup_debug(struct kvm_vcpu *vcpu) {}
diff --git a/arch/arm64/include/asm/kvm_host.h 
b/arch/arm64/include/asm/kvm_host.h
index b36dae9ee5f9..b1de2b147729 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -53,6 +53,7 @@ int kvm_arm_init_sve(void);
 
 int __attribute_const__ kvm_target_cpu(void);
 int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
+void kvm_arm_vcpu_destroy(struct kvm_vcpu *vcpu);
 void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu);
 int kvm_arch_vm_ioctl_check_extension(struct kvm *kvm, long ext);
 void __extended_idmap_trampoline(phys_addr_t boot_pgd, phys_addr_t 
idmap_start);
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index f4a8ae918827..ff3512a0ca97 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -205,6 +205,11 @@ bool kvm_arm_vcpu_is_finalized(struct kvm_vcpu *vcpu)
 }
 
 void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
+{
+
+}
+
+void kvm_arm_vcpu_destroy(struct kvm_vcpu *vcpu)
 {
kfree(vcpu->arch.sve_state);
 }
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 1d87c194a6d3..f6c641136501 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -326,6 +326,8 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
kvm_mmu_free_memory_caches(vcpu);
kvm_timer_vcpu_terminate(vcpu);
kvm_pmu_vcpu_destroy(vcpu);
+
+   kvm_arm_vcpu_destroy(vcpu);
 }
 
 int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 34/45] KVM: MIPS: Move .vcpu_setup() call to kvm_arch_vcpu_create()

2019-12-18 Thread Sean Christopherson
Fold setup() into create() now that the two are called back-to-back by
common KVM code.  This paves the way for removing kvm_arch_vcpu_setup().
Note, there is no unwind function associated with kvm_arch_vcpu_setup(),
i.e. no teardown path that also needs to be moved.

No functional change intended.

Signed-off-by: Sean Christopherson 
---
 arch/mips/kvm/mips.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 92c9321b3f95..b3a4435af66b 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -386,8 +386,15 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
vcpu->arch.last_sched_cpu = -1;
vcpu->arch.last_exec_cpu = -1;
 
+   /* Initial guest state */
+   err = kvm_mips_callbacks->vcpu_setup(vcpu);
+   if (err)
+   goto out_free_commpage;
+
return 0;
 
+out_free_commpage:
+   kfree(vcpu->arch.kseg0_commpage);
 out_free_gebase:
kfree(gebase);
 out:
@@ -1237,10 +1244,9 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
return 0;
 }
 
-/* Initial guest state */
 int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
 {
-   return kvm_mips_callbacks->vcpu_setup(vcpu);
+   return 0;
 }
 
 static void kvm_mips_set_c0_status(void)
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 25/45] KVM: s390: Move guts of kvm_arch_vcpu_init() into kvm_arch_vcpu_create()

2019-12-18 Thread Sean Christopherson
Move all of kvm_arch_vcpu_init(), which is invoked at the very end of
kvm_vcpu_init(), into kvm_arch_vcpu_create() in preparation of moving
the call to kvm_vcpu_init().  Moving kvm_vcpu_init() is itself a
preparatory step for moving allocation and initialization to common KVM
code.

No functional change inteded.

Signed-off-by: Sean Christopherson 
---
 arch/s390/kvm/kvm-s390.c | 62 ++--
 1 file changed, 34 insertions(+), 28 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 57c6838dff37..0049b621e56a 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2705,34 +2705,6 @@ static int sca_can_add_vcpu(struct kvm *kvm, unsigned 
int id)
 
 int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 {
-   vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID;
-   kvm_clear_async_pf_completion_queue(vcpu);
-   vcpu->run->kvm_valid_regs = KVM_SYNC_PREFIX |
-   KVM_SYNC_GPRS |
-   KVM_SYNC_ACRS |
-   KVM_SYNC_CRS |
-   KVM_SYNC_ARCH0 |
-   KVM_SYNC_PFAULT;
-   kvm_s390_set_prefix(vcpu, 0);
-   if (test_kvm_facility(vcpu->kvm, 64))
-   vcpu->run->kvm_valid_regs |= KVM_SYNC_RICCB;
-   if (test_kvm_facility(vcpu->kvm, 82))
-   vcpu->run->kvm_valid_regs |= KVM_SYNC_BPBC;
-   if (test_kvm_facility(vcpu->kvm, 133))
-   vcpu->run->kvm_valid_regs |= KVM_SYNC_GSCB;
-   if (test_kvm_facility(vcpu->kvm, 156))
-   vcpu->run->kvm_valid_regs |= KVM_SYNC_ETOKEN;
-   /* fprs can be synchronized via vrs, even if the guest has no vx. With
-* MACHINE_HAS_VX, (load|store)_fpu_regs() will work with vrs format.
-*/
-   if (MACHINE_HAS_VX)
-   vcpu->run->kvm_valid_regs |= KVM_SYNC_VRS;
-   else
-   vcpu->run->kvm_valid_regs |= KVM_SYNC_FPRS;
-
-   if (kvm_is_ucontrol(vcpu->kvm))
-   return __kvm_ucontrol_vcpu_init(vcpu);
-
return 0;
 }
 
@@ -3077,11 +3049,45 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
rc = kvm_vcpu_init(vcpu, kvm, id);
if (rc)
goto out_free_sie_block;
+
+   vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID;
+   kvm_clear_async_pf_completion_queue(vcpu);
+   vcpu->run->kvm_valid_regs = KVM_SYNC_PREFIX |
+   KVM_SYNC_GPRS |
+   KVM_SYNC_ACRS |
+   KVM_SYNC_CRS |
+   KVM_SYNC_ARCH0 |
+   KVM_SYNC_PFAULT;
+   kvm_s390_set_prefix(vcpu, 0);
+   if (test_kvm_facility(vcpu->kvm, 64))
+   vcpu->run->kvm_valid_regs |= KVM_SYNC_RICCB;
+   if (test_kvm_facility(vcpu->kvm, 82))
+   vcpu->run->kvm_valid_regs |= KVM_SYNC_BPBC;
+   if (test_kvm_facility(vcpu->kvm, 133))
+   vcpu->run->kvm_valid_regs |= KVM_SYNC_GSCB;
+   if (test_kvm_facility(vcpu->kvm, 156))
+   vcpu->run->kvm_valid_regs |= KVM_SYNC_ETOKEN;
+   /* fprs can be synchronized via vrs, even if the guest has no vx. With
+* MACHINE_HAS_VX, (load|store)_fpu_regs() will work with vrs format.
+*/
+   if (MACHINE_HAS_VX)
+   vcpu->run->kvm_valid_regs |= KVM_SYNC_VRS;
+   else
+   vcpu->run->kvm_valid_regs |= KVM_SYNC_FPRS;
+
+   if (kvm_is_ucontrol(vcpu->kvm)) {
+   rc = __kvm_ucontrol_vcpu_init(vcpu);
+   if (rc)
+   goto out_uninit_vcpu;
+   }
+
VM_EVENT(kvm, 3, "create cpu %d at 0x%pK, sie block at 0x%pK", id, vcpu,
 vcpu->arch.sie_block);
trace_kvm_s390_create_vcpu(id, vcpu, vcpu->arch.sie_block);
 
return vcpu;
+out_uninit_vcpu:
+   kvm_vcpu_uninit(vcpu);
 out_free_sie_block:
free_page((unsigned long)(vcpu->arch.sie_block));
 out_free_cpu:
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 29/45] KVM: Introduce kvm_vcpu_destroy()

2019-12-18 Thread Sean Christopherson
Add kvm_vcpu_destroy() and wire up all architectures to call the common
function instead of their arch specific implementation.  The common
destruction function will be used by future patches to move allocation
and initialization of vCPUs to common KVM code, i.e. to free resources
that are allocated by arch agnostic code.

No functional change intended.

Acked-by: Christoffer Dall 
Signed-off-by: Sean Christopherson 
---
 arch/mips/kvm/mips.c   | 2 +-
 arch/powerpc/kvm/powerpc.c | 2 +-
 arch/s390/kvm/kvm-s390.c   | 2 +-
 arch/x86/kvm/x86.c | 2 +-
 include/linux/kvm_host.h   | 1 +
 virt/kvm/arm/arm.c | 2 +-
 virt/kvm/kvm_main.c| 6 ++
 7 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 73360e021259..8546bc6e09e7 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -156,7 +156,7 @@ void kvm_mips_free_vcpus(struct kvm *kvm)
struct kvm_vcpu *vcpu;
 
kvm_for_each_vcpu(i, vcpu, kvm) {
-   kvm_arch_vcpu_destroy(vcpu);
+   kvm_vcpu_destroy(vcpu);
}
 
mutex_lock(>lock);
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 998ef60ac463..e3e2b88d3d8b 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -475,7 +475,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 #endif
 
kvm_for_each_vcpu(i, vcpu, kvm)
-   kvm_arch_vcpu_destroy(vcpu);
+   kvm_vcpu_destroy(vcpu);
 
mutex_lock(>lock);
for (i = 0; i < atomic_read(>online_vcpus); i++)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 1f8ba074cbd6..8543d338a06a 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2541,7 +2541,7 @@ static void kvm_free_vcpus(struct kvm *kvm)
struct kvm_vcpu *vcpu;
 
kvm_for_each_vcpu(i, vcpu, kvm)
-   kvm_arch_vcpu_destroy(vcpu);
+   kvm_vcpu_destroy(vcpu);
 
mutex_lock(>lock);
for (i = 0; i < atomic_read(>online_vcpus); i++)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f21169515910..c808329290e1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9590,7 +9590,7 @@ static void kvm_free_vcpus(struct kvm *kvm)
kvm_unload_vcpu_mmu(vcpu);
}
kvm_for_each_vcpu(i, vcpu, kvm)
-   kvm_arch_vcpu_destroy(vcpu);
+   kvm_vcpu_destroy(vcpu);
 
mutex_lock(>lock);
for (i = 0; i < atomic_read(>online_vcpus); i++)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 7c57f1a5d7b3..9e3371f9a4cd 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -581,6 +581,7 @@ static inline int kvm_vcpu_get_idx(struct kvm_vcpu *vcpu)
 
 int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id);
 void kvm_vcpu_uninit(struct kvm_vcpu *vcpu);
+void kvm_vcpu_destroy(struct kvm_vcpu *vcpu);
 
 void vcpu_load(struct kvm_vcpu *vcpu);
 void vcpu_put(struct kvm_vcpu *vcpu);
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 3cdd1c3be2c7..f569cbb2f123 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -194,7 +194,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 
for (i = 0; i < KVM_MAX_VCPUS; ++i) {
if (kvm->vcpus[i]) {
-   kvm_arch_vcpu_destroy(kvm->vcpus[i]);
+   kvm_vcpu_destroy(kvm->vcpus[i]);
kvm->vcpus[i] = NULL;
}
}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 281da692c774..c2766a1108b9 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -375,6 +375,12 @@ void kvm_vcpu_uninit(struct kvm_vcpu *vcpu)
 }
 EXPORT_SYMBOL_GPL(kvm_vcpu_uninit);
 
+void kvm_vcpu_destroy(struct kvm_vcpu *vcpu)
+{
+   kvm_arch_vcpu_destroy(vcpu);
+}
+EXPORT_SYMBOL_GPL(kvm_vcpu_destroy);
+
 #if defined(CONFIG_MMU_NOTIFIER) && defined(KVM_ARCH_WANT_MMU_NOTIFIER)
 static inline struct kvm *mmu_notifier_to_kvm(struct mmu_notifier *mn)
 {
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 28/45] KVM: x86: Invoke kvm_vcpu_uninit() immediately prior to freeing vcpu

2019-12-18 Thread Sean Christopherson
Move the call to kvm_vcpu_uninit() in kvm_arch_vcpu_destroy() down a few
lines so that it is invoked immediately prior to freeing the vCPU.  This
paves the way for moving the uninit and free sequence to common KVM code
without an associated functional change.

Signed-off-by: Sean Christopherson 
---
 arch/x86/kvm/x86.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c8b875aefeee..f21169515910 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9170,11 +9170,11 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 
kvm_x86_ops->vcpu_free(vcpu);
 
-   kvm_vcpu_uninit(vcpu);
-
free_cpumask_var(vcpu->arch.wbinvd_dirty_mask);
kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
+
+   kvm_vcpu_uninit(vcpu);
kmem_cache_free(kvm_vcpu_cache, vcpu);
 }
 
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 23/45] KVM: Remove kvm_arch_vcpu_free() declaration

2019-12-18 Thread Sean Christopherson
Remove KVM's declaration of kvm_arch_vcpu_free() now that the function
is gone from all architectures (several architectures were relying on
the forward declaration).

Acked-by: Christoffer Dall 
Signed-off-by: Sean Christopherson 
---
 include/linux/kvm_host.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 339de08e5fa2..a3f7ab881846 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -872,7 +872,6 @@ void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu);
 
 void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu);
 
-void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu);
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu);
 struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id);
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 17/45] KVM: MIPS: Drop kvm_arch_vcpu_free()

2019-12-18 Thread Sean Christopherson
Remove the superfluous kvm_arch_vcpu_free() as it is no longer called
from commmon KVM code.  Note, kvm_arch_vcpu_destroy() *is* called from
common code, i.e. choosing which function to whack is not completely
arbitrary.

Signed-off-by: Sean Christopherson 
---
 arch/mips/kvm/mips.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 5f985773417c..d72bceb10439 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -156,7 +156,7 @@ void kvm_mips_free_vcpus(struct kvm *kvm)
struct kvm_vcpu *vcpu;
 
kvm_for_each_vcpu(i, vcpu, kvm) {
-   kvm_arch_vcpu_free(vcpu);
+   kvm_arch_vcpu_destroy(vcpu);
}
 
mutex_lock(>lock);
@@ -407,7 +407,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, 
unsigned int id)
return ERR_PTR(err);
 }
 
-void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
+void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 {
hrtimer_cancel(>arch.comparecount_timer);
 
@@ -421,11 +421,6 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
kmem_cache_free(kvm_vcpu_cache, vcpu);
 }
 
-void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
-{
-   kvm_arch_vcpu_free(vcpu);
-}
-
 int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
struct kvm_guest_debug *dbg)
 {
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 44/45] KVM: Move putting of vcpu->pid to kvm_vcpu_destroy()

2019-12-18 Thread Sean Christopherson
Move the putting of vcpu->pid to kvm_vcpu_destroy().  vcpu->pid is
guaranteed to be NULL when kvm_vcpu_uninit() is called in the error path
of kvm_vm_ioctl_create_vcpu(), e.g. it is explicitly nullified by
kvm_vcpu_init() and is only changed by KVM_RUN.

No functional change intended.

Acked-by: Christoffer Dall 
Signed-off-by: Sean Christopherson 
---
 virt/kvm/kvm_main.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 21533a472ead..6912d81ca32d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -352,12 +352,6 @@ static int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm 
*kvm, unsigned id)
 
 static void kvm_vcpu_uninit(struct kvm_vcpu *vcpu)
 {
-   /*
-* no need for rcu_read_lock as VCPU_RUN is the only place that
-* will change the vcpu->pid pointer and on uninit all file
-* descriptors are already gone.
-*/
-   put_pid(rcu_dereference_protected(vcpu->pid, 1));
free_page((unsigned long)vcpu->run);
 }
 
@@ -365,6 +359,13 @@ void kvm_vcpu_destroy(struct kvm_vcpu *vcpu)
 {
kvm_arch_vcpu_destroy(vcpu);
 
+   /*
+* No need for rcu_read_lock as VCPU_RUN is the only place that changes
+* the vcpu->pid pointer, and at destruction time all file descriptors
+* are already gone.
+*/
+   put_pid(rcu_dereference_protected(vcpu->pid, 1));
+
kvm_vcpu_uninit(vcpu);
kmem_cache_free(kvm_vcpu_cache, vcpu);
 }
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 39/45] KVM: MIPS: Move all vcpu init code into kvm_arch_vcpu_create()

2019-12-18 Thread Sean Christopherson
Fold init() into create() now that the two are called back-to-back by
common KVM code (kvm_vcpu_init() calls kvm_arch_vcpu_init() as its last
action, and kvm_vm_ioctl_create_vcpu() calls kvm_arch_vcpu_create()
immediately thereafter).  Rinse and repeat for kvm_arch_vcpu_uninit()
and kvm_arch_vcpu_destroy().  This paves the way for removing
kvm_arch_vcpu_{un}init() entirely.

No functional change intended.

Signed-off-by: Sean Christopherson 
---
 arch/mips/kvm/mips.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 06366e2415a6..879a7cbd5b54 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -294,6 +294,14 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
kvm_debug("kvm @ %p: create cpu %d at %p\n",
  vcpu->kvm, vcpu->vcpu_id, vcpu);
 
+   err = kvm_mips_callbacks->vcpu_init(vcpu);
+   if (err)
+   return err;
+
+   hrtimer_init(>arch.comparecount_timer, CLOCK_MONOTONIC,
+HRTIMER_MODE_REL);
+   vcpu->arch.comparecount_timer.function = kvm_mips_comparecount_wakeup;
+
/*
 * Allocate space for host mode exception handlers that handle
 * guest mode exits
@@ -307,7 +315,7 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 
if (!gebase) {
err = -ENOMEM;
-   goto out;
+   goto out_uninit_vcpu;
}
kvm_debug("Allocated %d bytes for KVM Exception Handlers @ %p\n",
  ALIGN(size, PAGE_SIZE), gebase);
@@ -397,7 +405,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
kfree(vcpu->arch.kseg0_commpage);
 out_free_gebase:
kfree(gebase);
-out:
+out_uninit_vcpu:
+   kvm_mips_callbacks->vcpu_uninit(vcpu);
return err;
 }
 
@@ -410,6 +419,8 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
kvm_mmu_free_memory_caches(vcpu);
kfree(vcpu->arch.guest_ebase);
kfree(vcpu->arch.kseg0_commpage);
+
+   kvm_mips_callbacks->vcpu_uninit(vcpu);
 }
 
 int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
@@ -1221,21 +1232,12 @@ static enum hrtimer_restart 
kvm_mips_comparecount_wakeup(struct hrtimer *timer)
 
 int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 {
-   int err;
-
-   err = kvm_mips_callbacks->vcpu_init(vcpu);
-   if (err)
-   return err;
-
-   hrtimer_init(>arch.comparecount_timer, CLOCK_MONOTONIC,
-HRTIMER_MODE_REL);
-   vcpu->arch.comparecount_timer.function = kvm_mips_comparecount_wakeup;
return 0;
 }
 
 void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
 {
-   kvm_mips_callbacks->vcpu_uninit(vcpu);
+
 }
 
 int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 18/45] KVM: PPC: Drop kvm_arch_vcpu_free()

2019-12-18 Thread Sean Christopherson
Remove the superfluous kvm_arch_vcpu_free() as it is no longer called
from commmon KVM code.  Note, kvm_arch_vcpu_destroy() *is* called from
common code, i.e. choosing which function to whack is not completely
arbitrary.

Signed-off-by: Sean Christopherson 
---
 arch/powerpc/kvm/powerpc.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 173f57e0a1b5..a2ba708f5cec 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -475,7 +475,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 #endif
 
kvm_for_each_vcpu(i, vcpu, kvm)
-   kvm_arch_vcpu_free(vcpu);
+   kvm_arch_vcpu_destroy(vcpu);
 
mutex_lock(>lock);
for (i = 0; i < atomic_read(>online_vcpus); i++)
@@ -752,7 +752,7 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
 {
 }
 
-void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
+void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 {
/* Make sure we're not using the vcpu anymore */
hrtimer_cancel(>arch.dec_timer);
@@ -781,11 +781,6 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
kmem_cache_free(kvm_vcpu_cache, vcpu);
 }
 
-void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
-{
-   kvm_arch_vcpu_free(vcpu);
-}
-
 int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
 {
return kvmppc_core_pending_dec(vcpu);
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 32/45] KVM: Move initialization of preempt notifier to kvm_vcpu_init()

2019-12-18 Thread Sean Christopherson
Initialize the preempt notifier immediately in kvm_vcpu_init() to pave
the way for removing kvm_arch_vcpu_setup(), i.e. to allow arch specific
code to call vcpu_load() during kvm_arch_vcpu_create().

Back when preemption support was added, the location of the call to init
the preempt notifier was perfectly sane.  The overall vCPU creation flow
featured a single arch specific hook and the preempt notifer was used
immediately after its initialization (by vcpu_load()).  E.g.:

vcpu = kvm_arch_ops->vcpu_create(kvm, n);
if (IS_ERR(vcpu))
return PTR_ERR(vcpu);

preempt_notifier_init(>preempt_notifier, _preempt_ops);

vcpu_load(vcpu);
r = kvm_mmu_setup(vcpu);
vcpu_put(vcpu);
if (r < 0)
goto free_vcpu;

Today, the call to preempt_notifier_init() is sandwiched between two
arch specific calls, kvm_arch_vcpu_create() and kvm_arch_vcpu_setup(),
which needlessly forces x86 (and possibly others?) to split its vCPU
creation flow.  Init the preempt notifier prior to any arch specific
call so that each arch can independently decide how best to organize
its creation flow.

Acked-by: Christoffer Dall 
Signed-off-by: Sean Christopherson 
---
 virt/kvm/kvm_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index fd8168b8c0e4..876cf3dd2c97 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -348,6 +348,7 @@ static int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm 
*kvm, unsigned id)
kvm_vcpu_set_dy_eligible(vcpu, false);
vcpu->preempted = false;
vcpu->ready = false;
+   preempt_notifier_init(>preempt_notifier, _preempt_ops);
 
r = kvm_arch_vcpu_init(vcpu);
if (r < 0)
@@ -2752,8 +2753,6 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 
id)
if (r)
goto vcpu_uninit;
 
-   preempt_notifier_init(>preempt_notifier, _preempt_ops);
-
r = kvm_arch_vcpu_setup(vcpu);
if (r)
goto vcpu_destroy;
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 21/45] KVM: x86: Remove spurious clearing of async #PF MSR

2019-12-18 Thread Sean Christopherson
Remove a bogus clearing of apf.msr_val from kvm_arch_vcpu_destroy().

apf.msr_val is only set to a non-zero value by kvm_pv_enable_async_pf(),
which is only reachable by kvm_set_msr_common(), i.e. by writing
MSR_KVM_ASYNC_PF_EN.  KVM does not autonomously write said MSR, i.e.
can only be written via KVM_SET_MSRS or KVM_RUN.  Since KVM_SET_MSRS and
KVM_RUN are vcpu ioctls, they require a valid vcpu file descriptor.
kvm_arch_vcpu_destroy() is only called if KVM_CREATE_VCPU fails, and KVM
declares KVM_CREATE_VCPU successful once the vcpu fd is installed and
thus visible to userspace.  Ergo, apf.msr_val cannot be non-zero when
kvm_arch_vcpu_destroy() is called.

Fixes: 344d9588a9df0 ("KVM: Add PV MSR to enable asynchronous page faults 
delivery.")
Signed-off-by: Sean Christopherson 
---
 arch/x86/kvm/x86.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 19c14b786255..46626b52a4da 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9176,8 +9176,6 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
 
 void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 {
-   vcpu->arch.apf.msr_val = 0;
-
kvm_arch_vcpu_free(vcpu);
 }
 
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 38/45] KVM: x86: Move all vcpu init code into kvm_arch_vcpu_create()

2019-12-18 Thread Sean Christopherson
Fold init() into create() now that the two are called back-to-back by
common KVM code (kvm_vcpu_init() calls kvm_arch_vcpu_init() as its last
action, and kvm_vm_ioctl_create_vcpu() calls kvm_arch_vcpu_create()
immediately thereafter).  This paves the way for removing
kvm_arch_vcpu_init() entirely.

No functional change intended.

Signed-off-by: Sean Christopherson 
---
 arch/x86/kvm/x86.c | 198 +++--
 1 file changed, 100 insertions(+), 98 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 20f8ac086824..24671f487645 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9100,11 +9100,78 @@ int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned 
int id)
 
 int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 {
-   int ret;
+   struct page *page;
+   int r;
 
-   ret = kvm_x86_ops->vcpu_create(vcpu);
-   if (ret)
-   return ret;
+   vcpu->arch.emulate_ctxt.ops = _ops;
+   if (!irqchip_in_kernel(vcpu->kvm) || kvm_vcpu_is_reset_bsp(vcpu))
+   vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE;
+   else
+   vcpu->arch.mp_state = KVM_MP_STATE_UNINITIALIZED;
+
+   kvm_set_tsc_khz(vcpu, max_tsc_khz);
+
+   r = kvm_mmu_create(vcpu);
+   if (r < 0)
+   return r;
+
+   if (irqchip_in_kernel(vcpu->kvm)) {
+   vcpu->arch.apicv_active = 
kvm_x86_ops->get_enable_apicv(vcpu->kvm);
+   r = kvm_create_lapic(vcpu, lapic_timer_advance_ns);
+   if (r < 0)
+   goto fail_mmu_destroy;
+   } else
+   static_key_slow_inc(_no_apic_vcpu);
+
+   r = -ENOMEM;
+
+   page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+   if (!page)
+   goto fail_free_lapic;
+   vcpu->arch.pio_data = page_address(page);
+
+   vcpu->arch.mce_banks = kzalloc(KVM_MAX_MCE_BANKS * sizeof(u64) * 4,
+  GFP_KERNEL_ACCOUNT);
+   if (!vcpu->arch.mce_banks)
+   goto fail_free_pio_data;
+   vcpu->arch.mcg_cap = KVM_MAX_MCE_BANKS;
+
+   if (!zalloc_cpumask_var(>arch.wbinvd_dirty_mask,
+   GFP_KERNEL_ACCOUNT))
+   goto fail_free_mce_banks;
+
+   vcpu->arch.user_fpu = kmem_cache_zalloc(x86_fpu_cache,
+   GFP_KERNEL_ACCOUNT);
+   if (!vcpu->arch.user_fpu) {
+   pr_err("kvm: failed to allocate userspace's fpu\n");
+   goto free_wbinvd_dirty_mask;
+   }
+
+   vcpu->arch.guest_fpu = kmem_cache_zalloc(x86_fpu_cache,
+GFP_KERNEL_ACCOUNT);
+   if (!vcpu->arch.guest_fpu) {
+   pr_err("kvm: failed to allocate vcpu's fpu\n");
+   goto free_user_fpu;
+   }
+   fx_init(vcpu);
+
+   vcpu->arch.guest_xstate_size = XSAVE_HDR_SIZE + XSAVE_HDR_OFFSET;
+
+   vcpu->arch.maxphyaddr = cpuid_query_maxphyaddr(vcpu);
+
+   vcpu->arch.pat = MSR_IA32_CR_PAT_DEFAULT;
+
+   kvm_async_pf_hash_reset(vcpu);
+   kvm_pmu_init(vcpu);
+
+   vcpu->arch.pending_external_vector = -1;
+   vcpu->arch.preempted_in_kernel = false;
+
+   kvm_hv_vcpu_init(vcpu);
+
+   r = kvm_x86_ops->vcpu_create(vcpu);
+   if (r)
+   goto free_guest_fpu;
 
vcpu->arch.arch_capabilities = kvm_get_arch_capabilities();
vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT;
@@ -9114,6 +9181,22 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
kvm_init_mmu(vcpu, false);
vcpu_put(vcpu);
return 0;
+
+free_guest_fpu:
+   kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
+free_user_fpu:
+   kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
+free_wbinvd_dirty_mask:
+   free_cpumask_var(vcpu->arch.wbinvd_dirty_mask);
+fail_free_mce_banks:
+   kfree(vcpu->arch.mce_banks);
+fail_free_pio_data:
+   free_page((unsigned long)vcpu->arch.pio_data);
+fail_free_lapic:
+   kvm_free_lapic(vcpu);
+fail_mmu_destroy:
+   kvm_mmu_destroy(vcpu);
+   return r;
 }
 
 void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
@@ -9146,6 +9229,8 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
 
 void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 {
+   int idx;
+
kvmclock_reset(vcpu);
 
kvm_x86_ops->vcpu_free(vcpu);
@@ -9153,6 +9238,17 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
free_cpumask_var(vcpu->arch.wbinvd_dirty_mask);
kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
+
+   kvm_hv_vcpu_uninit(vcpu);
+   kvm_pmu_destroy(vcpu);
+   kfree(vcpu->arch.mce_banks);
+   kvm_free_lapic(vcpu);
+   idx = srcu_read_lock(>kvm->srcu);
+   kvm_mmu_destroy(vcpu);
+   srcu_read_unlock(>kvm->srcu, idx);
+   free_page((unsigned long)vcpu->arch.pio_data);
+   if 

[PATCH v2 41/45] KVM: PPC: Move all vcpu init code into kvm_arch_vcpu_create()

2019-12-18 Thread Sean Christopherson
Fold init() into create() now that the two are called back-to-back by
common KVM code (kvm_vcpu_init() calls kvm_arch_vcpu_init() as its last
action, and kvm_vm_ioctl_create_vcpu() calls kvm_arch_vcpu_create()
immediately thereafter).  Rinse and repeat for kvm_arch_vcpu_uninit()
and kvm_arch_vcpu_destroy().  This paves the way for removing
kvm_arch_vcpu_{un}init() entirely.

Note, calling kvmppc_mmu_destroy() if kvmppc_core_vcpu_create() fails
may or may not be necessary.  Move it along with the more obvious call
to kvmppc_subarch_vcpu_uninit() so as not to inadvertantly introduce a
functional change and/or bug.

No functional change intended.

Signed-off-by: Sean Christopherson 
---
 arch/powerpc/kvm/powerpc.c | 36 ++--
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index fce1b4776e55..91cf94d4191e 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -729,13 +729,29 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 {
int err;
 
-   err = kvmppc_core_vcpu_create(vcpu);
+   hrtimer_init(>arch.dec_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
+   vcpu->arch.dec_timer.function = kvmppc_decrementer_wakeup;
+   vcpu->arch.dec_expires = get_tb();
+
+#ifdef CONFIG_KVM_EXIT_TIMING
+   mutex_init(>arch.exit_timing_lock);
+#endif
+   err = kvmppc_subarch_vcpu_init(vcpu);
if (err)
return err;
 
+   err = kvmppc_core_vcpu_create(vcpu);
+   if (err)
+   goto out_vcpu_uninit;
+
vcpu->arch.wqp = >wq;
kvmppc_create_vcpu_debugfs(vcpu, vcpu->vcpu_id);
return 0;
+
+out_vcpu_uninit:
+   kvmppc_mmu_destroy(vcpu);
+   kvmppc_subarch_vcpu_uninit(vcpu);
+   return err;
 }
 
 void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
@@ -765,6 +781,9 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
}
 
kvmppc_core_vcpu_free(vcpu);
+
+   kvmppc_mmu_destroy(vcpu);
+   kvmppc_subarch_vcpu_uninit(vcpu);
 }
 
 int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
@@ -784,23 +803,12 @@ static enum hrtimer_restart 
kvmppc_decrementer_wakeup(struct hrtimer *timer)
 
 int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 {
-   int ret;
-
-   hrtimer_init(>arch.dec_timer, CLOCK_REALTIME, HRTIMER_MODE_ABS);
-   vcpu->arch.dec_timer.function = kvmppc_decrementer_wakeup;
-   vcpu->arch.dec_expires = get_tb();
-
-#ifdef CONFIG_KVM_EXIT_TIMING
-   mutex_init(>arch.exit_timing_lock);
-#endif
-   ret = kvmppc_subarch_vcpu_init(vcpu);
-   return ret;
+   return 0;
 }
 
 void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
 {
-   kvmppc_mmu_destroy(vcpu);
-   kvmppc_subarch_vcpu_uninit(vcpu);
+
 }
 
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 19/45] KVM: arm: Drop kvm_arch_vcpu_free()

2019-12-18 Thread Sean Christopherson
Remove the superfluous kvm_arch_vcpu_free() as it is no longer called
from commmon KVM code.  Note, kvm_arch_vcpu_destroy() *is* called from
common code, i.e. choosing which function to whack is not completely
arbitrary.

Acked-by: Christoffer Dall 
Signed-off-by: Sean Christopherson 
---
 virt/kvm/arm/arm.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 12e0280291ce..25b5e4c9b8db 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -194,7 +194,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 
for (i = 0; i < KVM_MAX_VCPUS; ++i) {
if (kvm->vcpus[i]) {
-   kvm_arch_vcpu_free(kvm->vcpus[i]);
+   kvm_arch_vcpu_destroy(kvm->vcpus[i]);
kvm->vcpus[i] = NULL;
}
}
@@ -321,7 +321,7 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
 {
 }
 
-void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
+void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 {
if (vcpu->arch.has_run_once && unlikely(!irqchip_in_kernel(vcpu->kvm)))
static_branch_dec(_irqchip_in_use);
@@ -333,11 +333,6 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
kmem_cache_free(kvm_vcpu_cache, vcpu);
 }
 
-void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
-{
-   kvm_arch_vcpu_free(vcpu);
-}
-
 int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu)
 {
return kvm_timer_is_pending(vcpu);
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 33/45] KVM: x86: Move guts of kvm_arch_vcpu_setup() into kvm_arch_vcpu_create()

2019-12-18 Thread Sean Christopherson
Fold setup() into create() now that the two are called back-to-back by
common KVM code.  This paves the way for removing kvm_arch_vcpu_setup().

Signed-off-by: Sean Christopherson 
---
 arch/x86/kvm/x86.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c04b24719b28..0ab96dc3dd08 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9100,11 +9100,12 @@ int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned 
int id)
 
 int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
 {
-   return kvm_x86_ops->vcpu_create(vcpu);
-}
+   int ret;
+
+   ret = kvm_x86_ops->vcpu_create(vcpu);
+   if (ret)
+   return ret;
 
-int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
-{
vcpu->arch.arch_capabilities = kvm_get_arch_capabilities();
vcpu->arch.msr_platform_info = MSR_PLATFORM_INFO_CPUID_FAULT;
kvm_vcpu_mtrr_init(vcpu);
@@ -9115,6 +9116,11 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
return 0;
 }
 
+int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
+{
+   return 0;
+}
+
 void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
 {
struct msr_data msr;
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 14/45] KVM: PPC: e500mc: Move reset of oldpir below call to kvm_vcpu_init()

2019-12-18 Thread Sean Christopherson
Move the initialization of oldpir so that the call to kvm_vcpu_init() is
at the top of kvmppc_core_vcpu_create_e500mc().  oldpir is only use
when loading/putting a vCPU, which currently cannot be done until after
kvm_arch_vcpu_create() completes.  Reording the call to kvm_vcpu_init()
paves the way for moving the invocation to common PPC code.

No functional change intended.

Signed-off-by: Sean Christopherson 
---
 arch/powerpc/kvm/e500mc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index aea588f73bf7..f2902e612150 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -310,13 +310,13 @@ static int kvmppc_core_vcpu_create_e500mc(struct kvm 
*kvm, struct kvm_vcpu *vcpu
BUILD_BUG_ON(offsetof(struct kvmppc_vcpu_e500, vcpu) != 0);
vcpu_e500 = to_e500(vcpu);
 
-   /* Invalid PIR value -- this LPID dosn't have valid state on any cpu */
-   vcpu->arch.oldpir = 0x;
-
err = kvm_vcpu_init(vcpu, kvm, id);
if (err)
return err;
 
+   /* Invalid PIR value -- this LPID dosn't have valid state on any cpu */
+   vcpu->arch.oldpir = 0x;
+
err = kvmppc_e500_tlb_init(vcpu_e500);
if (err)
goto uninit_vcpu;
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 11/45] KVM: PPC: e500mc: Add build-time assert that vcpu is at offset 0

2019-12-18 Thread Sean Christopherson
In preparation for moving vcpu allocation to common PPC code, add an
explicit, albeit redundant, build-time assert to ensure the vcpu member
is located at offset 0.  The assert is redundant in the sense that
kvmppc_core_vcpu_create_e500() contains a functionally identical assert.
The motiviation for adding the extra assert is to provide visual
confirmation of the correctness of moving vcpu allocation to common
code.

Signed-off-by: Sean Christopherson 
---
 arch/powerpc/kvm/e500mc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index 318e65c65999..c51f4bb086fd 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -308,6 +308,8 @@ static struct kvm_vcpu 
*kvmppc_core_vcpu_create_e500mc(struct kvm *kvm,
struct kvm_vcpu *vcpu;
int err;
 
+   BUILD_BUG_ON(offsetof(struct kvmppc_vcpu_e500, vcpu) != 0);
+
vcpu_e500 = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
if (!vcpu_e500) {
err = -ENOMEM;
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 01/45] KVM: PPC: Book3S HV: Uninit vCPU if vcore creation fails

2019-12-18 Thread Sean Christopherson
Call kvm_vcpu_uninit() if vcore creation fails to avoid leaking any
resources allocated by kvm_vcpu_init(), i.e. the vcpu->run page.

Fixes: 371fefd6f2dc4 ("KVM: PPC: Allow book3s_hv guests to use SMT processor 
modes")
Cc: sta...@vger.kernel.org
Reviewed-by: Greg Kurz 
Signed-off-by: Sean Christopherson 
---
 arch/powerpc/kvm/book3s_hv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index dc53578193ee..d07d2f5273e5 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2368,7 +2368,7 @@ static struct kvm_vcpu *kvmppc_core_vcpu_create_hv(struct 
kvm *kvm,
mutex_unlock(>lock);
 
if (!vcore)
-   goto free_vcpu;
+   goto uninit_vcpu;
 
spin_lock(>lock);
++vcore->num_threads;
@@ -2385,6 +2385,8 @@ static struct kvm_vcpu *kvmppc_core_vcpu_create_hv(struct 
kvm *kvm,
 
return vcpu;
 
+uninit_vcpu:
+   kvm_vcpu_uninit(vcpu);
 free_vcpu:
kmem_cache_free(kvm_vcpu_cache, vcpu);
 out:
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 27/45] KVM: MIPS: Invoke kvm_vcpu_uninit() immediately prior to freeing vcpu

2019-12-18 Thread Sean Christopherson
Move the call to kvm_vcpu_uninit() in kvm_arch_vcpu_destroy() down a few
lines so that it is invoked immediately prior to freeing the vCPU.  This
paves the way for moving the uninit and free sequence to common KVM code
without an associated functional change.

Signed-off-by: Sean Christopherson 
---
 arch/mips/kvm/mips.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 2e14455aec4e..73360e021259 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -416,13 +416,13 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 {
hrtimer_cancel(>arch.comparecount_timer);
 
-   kvm_vcpu_uninit(vcpu);
-
kvm_mips_dump_stats(vcpu);
 
kvm_mmu_free_memory_caches(vcpu);
kfree(vcpu->arch.guest_ebase);
kfree(vcpu->arch.kseg0_commpage);
+
+   kvm_vcpu_uninit(vcpu);
kmem_cache_free(kvm_vcpu_cache, vcpu);
 }
 
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 20/45] KVM: x86: Remove spurious kvm_mmu_unload() from vcpu destruction path

2019-12-18 Thread Sean Christopherson
x86 does not load its MMU until KVM_RUN, which cannot be invoked until
after vCPU creation succeeds.  Given that kvm_arch_vcpu_destroy() is
called if and only if vCPU creation fails, it is impossible for the MMU
to be loaded.

Note, the bogus kvm_mmu_unload() call was added during an unrelated
refactoring of vCPU allocation, i.e. was presumably added as an
opportunstic "fix" for a perceived leak.

Fixes: fb3f0f51d92d1 ("KVM: Dynamically allocate vcpus")
Signed-off-by: Sean Christopherson 
---
 arch/x86/kvm/x86.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8973037eb2fa..19c14b786255 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9178,10 +9178,6 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 {
vcpu->arch.apf.msr_val = 0;
 
-   vcpu_load(vcpu);
-   kvm_mmu_unload(vcpu);
-   vcpu_put(vcpu);
-
kvm_arch_vcpu_free(vcpu);
 }
 
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 12/45] KVM: PPC: Allocate vcpu struct in common PPC code

2019-12-18 Thread Sean Christopherson
Move allocation of all flavors of PPC vCPUs to common PPC code.  All
variants either allocate 'struct kvm_vcpu' directly, or require that
the embedded 'struct kvm_vcpu' member be located at offset 0, i.e.
guarantee that the allocation can be directly interpreted as a 'struct
kvm_vcpu' object.

Remove the message from the build-time assertion regarding placement of
the struct, as compatibility with the arch usercopy region is no longer
the sole dependent on 'struct kvm_vcpu' being at offset zero.

Signed-off-by: Sean Christopherson 
---
 arch/powerpc/include/asm/kvm_ppc.h |  7 ---
 arch/powerpc/kvm/book3s.c  |  5 +++--
 arch/powerpc/kvm/book3s_hv.c   | 20 +---
 arch/powerpc/kvm/book3s_pr.c   | 18 +-
 arch/powerpc/kvm/booke.c   |  5 +++--
 arch/powerpc/kvm/e500.c| 24 ++--
 arch/powerpc/kvm/e500mc.c  | 22 +-
 arch/powerpc/kvm/powerpc.c | 23 ++-
 8 files changed, 49 insertions(+), 75 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_ppc.h 
b/arch/powerpc/include/asm/kvm_ppc.h
index 3d2f871241a8..8f77ca5ace6f 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -119,8 +119,8 @@ extern int kvmppc_xlate(struct kvm_vcpu *vcpu, ulong eaddr,
enum xlate_instdata xlid, enum xlate_readwrite xlrw,
struct kvmppc_pte *pte);
 
-extern struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm,
-unsigned int id);
+extern int kvmppc_core_vcpu_create(struct kvm *kvm, struct kvm_vcpu *vcpu,
+  unsigned int id);
 extern void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu);
 extern int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu);
 extern int kvmppc_core_check_processor_compat(void);
@@ -274,7 +274,8 @@ struct kvmppc_ops {
void (*inject_interrupt)(struct kvm_vcpu *vcpu, int vec, u64 
srr1_flags);
void (*set_msr)(struct kvm_vcpu *vcpu, u64 msr);
int (*vcpu_run)(struct kvm_run *run, struct kvm_vcpu *vcpu);
-   struct kvm_vcpu *(*vcpu_create)(struct kvm *kvm, unsigned int id);
+   int (*vcpu_create)(struct kvm *kvm, struct kvm_vcpu *vcpu,
+  unsigned int id);
void (*vcpu_free)(struct kvm_vcpu *vcpu);
int (*check_requests)(struct kvm_vcpu *vcpu);
int (*get_dirty_log)(struct kvm *kvm, struct kvm_dirty_log *log);
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 58a59ee998e2..13385656b90d 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -789,9 +789,10 @@ void kvmppc_decrementer_func(struct kvm_vcpu *vcpu)
kvm_vcpu_kick(vcpu);
 }
 
-struct kvm_vcpu *kvmppc_core_vcpu_create(struct kvm *kvm, unsigned int id)
+int kvmppc_core_vcpu_create(struct kvm *kvm, struct kvm_vcpu *vcpu,
+   unsigned int id)
 {
-   return kvm->arch.kvm_ops->vcpu_create(kvm, id);
+   return kvm->arch.kvm_ops->vcpu_create(kvm, vcpu, id);
 }
 
 void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index d07d2f5273e5..3fb41fe24f58 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2271,22 +2271,16 @@ static void debugfs_vcpu_init(struct kvm_vcpu *vcpu, 
unsigned int id)
 }
 #endif /* CONFIG_KVM_BOOK3S_HV_EXIT_TIMING */
 
-static struct kvm_vcpu *kvmppc_core_vcpu_create_hv(struct kvm *kvm,
-  unsigned int id)
+static int kvmppc_core_vcpu_create_hv(struct kvm *kvm, struct kvm_vcpu *vcpu,
+ unsigned int id)
 {
-   struct kvm_vcpu *vcpu;
int err;
int core;
struct kvmppc_vcore *vcore;
 
-   err = -ENOMEM;
-   vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
-   if (!vcpu)
-   goto out;
-
err = kvm_vcpu_init(vcpu, kvm, id);
if (err)
-   goto free_vcpu;
+   return err;
 
vcpu->arch.shared = >arch.shregs;
 #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
@@ -2383,14 +2377,11 @@ static struct kvm_vcpu 
*kvmppc_core_vcpu_create_hv(struct kvm *kvm,
 
debugfs_vcpu_init(vcpu, id);
 
-   return vcpu;
+   return 0;
 
 uninit_vcpu:
kvm_vcpu_uninit(vcpu);
-free_vcpu:
-   kmem_cache_free(kvm_vcpu_cache, vcpu);
-out:
-   return ERR_PTR(err);
+   return err;
 }
 
 static int kvmhv_set_smt_mode(struct kvm *kvm, unsigned long smt_mode,
@@ -2445,7 +2436,6 @@ static void kvmppc_core_vcpu_free_hv(struct kvm_vcpu 
*vcpu)
unpin_vpa(vcpu->kvm, >arch.vpa);
spin_unlock(>arch.vpa_update_lock);
kvm_vcpu_uninit(vcpu);
-   kmem_cache_free(kvm_vcpu_cache, vcpu);
 }
 
 static int kvmppc_core_check_requests_hv(struct kvm_vcpu *vcpu)
diff --git a/arch/powerpc/kvm/book3s_pr.c 

[PATCH v2 22/45] KVM: x86: Drop kvm_arch_vcpu_free()

2019-12-18 Thread Sean Christopherson
Remove the superfluous kvm_arch_vcpu_free() as it is no longer called
from commmon KVM code.  Note, kvm_arch_vcpu_destroy() *is* called from
common code, i.e. choosing which function to whack is not completely
arbitrary.

Signed-off-by: Sean Christopherson 
---
 arch/x86/kvm/x86.c | 27 +++
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 46626b52a4da..cf35169733cd 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9089,20 +9089,6 @@ static void fx_init(struct kvm_vcpu *vcpu)
vcpu->arch.cr0 |= X86_CR0_ET;
 }
 
-void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
-{
-   kvmclock_reset(vcpu);
-
-   kvm_x86_ops->vcpu_free(vcpu);
-
-   kvm_vcpu_uninit(vcpu);
-
-   free_cpumask_var(vcpu->arch.wbinvd_dirty_mask);
-   kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
-   kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
-   kmem_cache_free(kvm_vcpu_cache, vcpu);
-}
-
 struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
unsigned int id)
 {
@@ -9176,7 +9162,16 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)
 
 void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 {
-   kvm_arch_vcpu_free(vcpu);
+   kvmclock_reset(vcpu);
+
+   kvm_x86_ops->vcpu_free(vcpu);
+
+   kvm_vcpu_uninit(vcpu);
+
+   free_cpumask_var(vcpu->arch.wbinvd_dirty_mask);
+   kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
+   kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
+   kmem_cache_free(kvm_vcpu_cache, vcpu);
 }
 
 void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
@@ -9591,7 +9586,7 @@ static void kvm_free_vcpus(struct kvm *kvm)
kvm_unload_vcpu_mmu(vcpu);
}
kvm_for_each_vcpu(i, vcpu, kvm)
-   kvm_arch_vcpu_free(vcpu);
+   kvm_arch_vcpu_destroy(vcpu);
 
mutex_lock(>lock);
for (i = 0; i < atomic_read(>online_vcpus); i++)
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 26/45] KVM: s390: Invoke kvm_vcpu_init() before allocating sie_page

2019-12-18 Thread Sean Christopherson
Now that s390's implementation of kvm_arch_vcpu_init() is empty, move
the call to kvm_vcpu_init() above the allocation of the sie_page.  This
paves the way for moving vcpu allocation and initialization into common
KVM code without any associated functional change.

Signed-off-by: Sean Christopherson 
---
 arch/s390/kvm/kvm-s390.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 0049b621e56a..1f8ba074cbd6 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -3027,10 +3027,16 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
if (!vcpu)
goto out;
 
+   rc = kvm_vcpu_init(vcpu, kvm, id);
+   if (rc)
+   goto out_free_cpu;
+
+   rc = -ENOMEM;
+
BUILD_BUG_ON(sizeof(struct sie_page) != 4096);
sie_page = (struct sie_page *) get_zeroed_page(GFP_KERNEL);
if (!sie_page)
-   goto out_free_cpu;
+   goto out_uninit_vcpu;
 
vcpu->arch.sie_block = _page->sie_block;
vcpu->arch.sie_block->itdba = (unsigned long) _page->itdb;
@@ -3046,10 +3052,6 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
vcpu->arch.sie_block->gd |= GISA_FORMAT1;
seqcount_init(>arch.cputm_seqcount);
 
-   rc = kvm_vcpu_init(vcpu, kvm, id);
-   if (rc)
-   goto out_free_sie_block;
-
vcpu->arch.pfault_token = KVM_S390_PFAULT_TOKEN_INVALID;
kvm_clear_async_pf_completion_queue(vcpu);
vcpu->run->kvm_valid_regs = KVM_SYNC_PREFIX |
@@ -3078,7 +3080,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
if (kvm_is_ucontrol(vcpu->kvm)) {
rc = __kvm_ucontrol_vcpu_init(vcpu);
if (rc)
-   goto out_uninit_vcpu;
+   goto out_free_sie_block;
}
 
VM_EVENT(kvm, 3, "create cpu %d at 0x%pK, sie block at 0x%pK", id, vcpu,
@@ -3086,10 +3088,10 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
trace_kvm_s390_create_vcpu(id, vcpu, vcpu->arch.sie_block);
 
return vcpu;
-out_uninit_vcpu:
-   kvm_vcpu_uninit(vcpu);
 out_free_sie_block:
free_page((unsigned long)(vcpu->arch.sie_block));
+out_uninit_vcpu:
+   kvm_vcpu_uninit(vcpu);
 out_free_cpu:
kmem_cache_free(kvm_vcpu_cache, vcpu);
 out:
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 16/45] KVM: MIPS: Use kvm_vcpu_cache to allocate vCPUs

2019-12-18 Thread Sean Christopherson
For reasons unknown, MIPS configures the vCPU allocation cache but
allocates vCPUs via kzalloc().  Allocate from the vCPU cache in
preparation for moving vCPU allocation to common KVM code.

Signed-off-by: Sean Christopherson 
---
 arch/mips/kvm/mips.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 1109924560d8..5f985773417c 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -286,7 +286,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, 
unsigned int id)
void *gebase, *p, *handler, *refill_start, *refill_end;
int i;
 
-   struct kvm_vcpu *vcpu = kzalloc(sizeof(struct kvm_vcpu), GFP_KERNEL);
+   struct kvm_vcpu *vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL);
 
if (!vcpu) {
err = -ENOMEM;
@@ -401,7 +401,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, 
unsigned int id)
kvm_vcpu_uninit(vcpu);
 
 out_free_cpu:
-   kfree(vcpu);
+   kmem_cache_free(kvm_vcpu_cache, vcpu);
 
 out:
return ERR_PTR(err);
@@ -418,7 +418,7 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
kvm_mmu_free_memory_caches(vcpu);
kfree(vcpu->arch.guest_ebase);
kfree(vcpu->arch.kseg0_commpage);
-   kfree(vcpu);
+   kmem_cache_free(kvm_vcpu_cache, vcpu);
 }
 
 void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 24/45] KVM: Add kvm_arch_vcpu_precreate() to handle pre-allocation issues

2019-12-18 Thread Sean Christopherson
Add a pre-allocation arch hook to handle checks that are currently done
by arch specific code prior to allocating the vCPU object.  This paves
the way for moving the allocation to common KVM code.

Acked-by: Christoffer Dall 
Signed-off-by: Sean Christopherson 
---
 arch/mips/kvm/mips.c   |  5 +
 arch/powerpc/kvm/powerpc.c |  5 +
 arch/s390/kvm/kvm-s390.c   | 12 
 arch/x86/kvm/x86.c | 14 +-
 include/linux/kvm_host.h   |  1 +
 virt/kvm/arm/arm.c | 21 +++--
 virt/kvm/kvm_main.c|  4 
 7 files changed, 43 insertions(+), 19 deletions(-)

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index d72bceb10439..2e14455aec4e 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -280,6 +280,11 @@ static inline void dump_handler(const char *symbol, void 
*start, void *end)
pr_debug("\tEND(%s)\n", symbol);
 }
 
+int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int id)
+{
+   return 0;
+}
+
 struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 {
int err, size;
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index a2ba708f5cec..998ef60ac463 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -720,6 +720,11 @@ void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
kvmppc_core_flush_memslot(kvm, slot);
 }
 
+int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int id)
+{
+   return 0;
+}
+
 struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 {
struct kvm_vcpu *vcpu;
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index d9e6bf3d54f0..57c6838dff37 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -3035,15 +3035,19 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
return rc;
 }
 
+int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int id)
+{
+   if (!kvm_is_ucontrol(kvm) && !sca_can_add_vcpu(kvm, id))
+   return -EINVAL;
+   return 0;
+}
+
 struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
  unsigned int id)
 {
struct kvm_vcpu *vcpu;
struct sie_page *sie_page;
-   int rc = -EINVAL;
-
-   if (!kvm_is_ucontrol(kvm) && !sca_can_add_vcpu(kvm, id))
-   goto out;
+   int rc;
 
rc = -ENOMEM;
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index cf35169733cd..c8b875aefeee 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9089,17 +9089,21 @@ static void fx_init(struct kvm_vcpu *vcpu)
vcpu->arch.cr0 |= X86_CR0_ET;
 }
 
+int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int id)
+{
+   if (kvm_check_tsc_unstable() && atomic_read(>online_vcpus) != 0)
+   pr_warn_once("kvm: SMP vm created on host with unstable TSC; "
+"guest TSC will not be reliable\n");
+
+   return 0;
+}
+
 struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
unsigned int id)
 {
struct kvm_vcpu *vcpu;
int r;
 
-   if (kvm_check_tsc_unstable() && atomic_read(>online_vcpus) != 0)
-   printk_once(KERN_WARNING
-   "kvm: SMP vm created on host with unstable TSC; "
-   "guest TSC will not be reliable\n");
-
vcpu = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL_ACCOUNT);
if (!vcpu)
return ERR_PTR(-ENOMEM);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index a3f7ab881846..7c57f1a5d7b3 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -874,6 +874,7 @@ void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu);
 
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu);
+int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int id);
 struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id);
 int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu);
 void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu);
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 25b5e4c9b8db..3cdd1c3be2c7 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -279,21 +279,22 @@ void kvm_arch_free_vm(struct kvm *kvm)
vfree(kvm);
 }
 
+int kvm_arch_vcpu_precreate(struct kvm *kvm, unsigned int id)
+{
+   if (irqchip_in_kernel(kvm) && vgic_initialized(kvm))
+   return -EBUSY;
+
+   if (id >= kvm->arch.max_vcpus)
+   return -EINVAL;
+
+   return 0;
+}
+
 struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm, unsigned int id)
 {
int err;
struct kvm_vcpu *vcpu;
 
-   if (irqchip_in_kernel(kvm) && vgic_initialized(kvm)) {
-   err = -EBUSY;
-   goto out;
-   }
-
-   if (id >= kvm->arch.max_vcpus) {
-   err = -EINVAL;
-   goto out;
-   }
-
vcpu = kmem_cache_zalloc(kvm_vcpu_cache, 

[PATCH v2 15/45] KVM: PPC: Move kvm_vcpu_init() invocation to common code

2019-12-18 Thread Sean Christopherson
Move the kvm_cpu_{un}init() calls to common PPC code as an intermediate
step towards removing kvm_cpu_{un}init() altogether.

No functional change intended.

Signed-off-by: Sean Christopherson 
---
 arch/powerpc/include/asm/kvm_ppc.h |  3 +--
 arch/powerpc/kvm/book3s.c  |  5 ++---
 arch/powerpc/kvm/book3s_hv.c   | 17 ++---
 arch/powerpc/kvm/book3s_pr.c   | 13 +++--
 arch/powerpc/kvm/booke.c   |  5 ++---
 arch/powerpc/kvm/e500.c| 16 +++-
 arch/powerpc/kvm/e500mc.c  | 12 ++--
 arch/powerpc/kvm/powerpc.c | 10 +-
 8 files changed, 28 insertions(+), 53 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_ppc.h 
b/arch/powerpc/include/asm/kvm_ppc.h
index 8f77ca5ace6f..374e4b835ff0 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -119,8 +119,7 @@ extern int kvmppc_xlate(struct kvm_vcpu *vcpu, ulong eaddr,
enum xlate_instdata xlid, enum xlate_readwrite xlrw,
struct kvmppc_pte *pte);
 
-extern int kvmppc_core_vcpu_create(struct kvm *kvm, struct kvm_vcpu *vcpu,
-  unsigned int id);
+extern int kvmppc_core_vcpu_create(struct kvm_vcpu *vcpu);
 extern void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu);
 extern int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu);
 extern int kvmppc_core_check_processor_compat(void);
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 13385656b90d..5ad20fc0c6a1 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -789,10 +789,9 @@ void kvmppc_decrementer_func(struct kvm_vcpu *vcpu)
kvm_vcpu_kick(vcpu);
 }
 
-int kvmppc_core_vcpu_create(struct kvm *kvm, struct kvm_vcpu *vcpu,
-   unsigned int id)
+int kvmppc_core_vcpu_create(struct kvm_vcpu *vcpu)
 {
-   return kvm->arch.kvm_ops->vcpu_create(kvm, vcpu, id);
+   return kvm->arch.kvm_ops->vcpu_create(vcpu);
 }
 
 void kvmppc_core_vcpu_free(struct kvm_vcpu *vcpu)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 3fb41fe24f58..d3d9a03a61fd 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2271,16 +2271,16 @@ static void debugfs_vcpu_init(struct kvm_vcpu *vcpu, 
unsigned int id)
 }
 #endif /* CONFIG_KVM_BOOK3S_HV_EXIT_TIMING */
 
-static int kvmppc_core_vcpu_create_hv(struct kvm *kvm, struct kvm_vcpu *vcpu,
- unsigned int id)
+static int kvmppc_core_vcpu_create_hv(struct kvm_vcpu *vcpu)
 {
int err;
int core;
struct kvmppc_vcore *vcore;
+   struct kvm *kvm;
+   unsigned int id;
 
-   err = kvm_vcpu_init(vcpu, kvm, id);
-   if (err)
-   return err;
+   kvm = vcpu->kvm;
+   id = vcpu->vcpu_id;
 
vcpu->arch.shared = >arch.shregs;
 #ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
@@ -2362,7 +2362,7 @@ static int kvmppc_core_vcpu_create_hv(struct kvm *kvm, 
struct kvm_vcpu *vcpu,
mutex_unlock(>lock);
 
if (!vcore)
-   goto uninit_vcpu;
+   return err;
 
spin_lock(>lock);
++vcore->num_threads;
@@ -2378,10 +2378,6 @@ static int kvmppc_core_vcpu_create_hv(struct kvm *kvm, 
struct kvm_vcpu *vcpu,
debugfs_vcpu_init(vcpu, id);
 
return 0;
-
-uninit_vcpu:
-   kvm_vcpu_uninit(vcpu);
-   return err;
 }
 
 static int kvmhv_set_smt_mode(struct kvm *kvm, unsigned long smt_mode,
@@ -2435,7 +2431,6 @@ static void kvmppc_core_vcpu_free_hv(struct kvm_vcpu 
*vcpu)
unpin_vpa(vcpu->kvm, >arch.slb_shadow);
unpin_vpa(vcpu->kvm, >arch.vpa);
spin_unlock(>arch.vpa_update_lock);
-   kvm_vcpu_uninit(vcpu);
 }
 
 static int kvmppc_core_check_requests_hv(struct kvm_vcpu *vcpu)
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 10c65d412e81..d88f708d5be3 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1744,22 +1744,17 @@ static int kvmppc_set_one_reg_pr(struct kvm_vcpu *vcpu, 
u64 id,
return r;
 }
 
-static int kvmppc_core_vcpu_create_pr(struct kvm *kvm, struct kvm_vcpu *vcpu,
- unsigned int id)
+static int kvmppc_core_vcpu_create_pr(struct kvm_vcpu *vcpu)
 {
struct kvmppc_vcpu_book3s *vcpu_book3s;
unsigned long p;
int err;
 
-   err = kvm_vcpu_init(vcpu, kvm, id);
-   if (err)
-   return err;
-
err = -ENOMEM;
 
vcpu_book3s = vzalloc(sizeof(struct kvmppc_vcpu_book3s));
if (!vcpu_book3s)
-   goto uninit_vcpu;
+   goto out;
vcpu->arch.book3s = vcpu_book3s;
 
 #ifdef CONFIG_KVM_BOOK3S_32_HANDLER
@@ -1814,8 +1809,7 @@ static int kvmppc_core_vcpu_create_pr(struct kvm *kvm, 
struct kvm_vcpu *vcpu,
 free_vcpu3s:
 #endif
vfree(vcpu_book3s);
-uninit_vcpu:
-   kvm_vcpu_uninit(vcpu);
+out:
return err;
 }
 
@@ 

[PATCH v2 06/45] KVM: SVM: Use direct vcpu pointer during vCPU create/free

2019-12-18 Thread Sean Christopherson
Capture the vcpu pointer in a local varaible and replace '>vcpu'
references with a direct reference to the pointer in anticipation of
moving bits of the code to common x86 and passing the vcpu pointer into
svm_create_vcpu(), i.e. eliminate unnecessary noise from future patches.

Signed-off-by: Sean Christopherson 
---
 arch/x86/kvm/svm.c | 30 --
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 8f1b715dfde8..1afa10cf365f 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2146,6 +2146,7 @@ static int avic_init_vcpu(struct vcpu_svm *svm)
 
 static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 {
+   struct kvm_vcpu *vcpu;
struct vcpu_svm *svm;
struct page *page;
struct page *msrpm_pages;
@@ -2161,24 +2162,25 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm 
*kvm, unsigned int id)
err = -ENOMEM;
goto out;
}
+   vcpu = >vcpu;
 
-   svm->vcpu.arch.user_fpu = kmem_cache_zalloc(x86_fpu_cache,
-GFP_KERNEL_ACCOUNT);
-   if (!svm->vcpu.arch.user_fpu) {
+   vcpu->arch.user_fpu = kmem_cache_zalloc(x86_fpu_cache,
+   GFP_KERNEL_ACCOUNT);
+   if (!vcpu->arch.user_fpu) {
printk(KERN_ERR "kvm: failed to allocate kvm userspace's 
fpu\n");
err = -ENOMEM;
goto free_partial_svm;
}
 
-   svm->vcpu.arch.guest_fpu = kmem_cache_zalloc(x86_fpu_cache,
-GFP_KERNEL_ACCOUNT);
-   if (!svm->vcpu.arch.guest_fpu) {
+   vcpu->arch.guest_fpu = kmem_cache_zalloc(x86_fpu_cache,
+GFP_KERNEL_ACCOUNT);
+   if (!vcpu->arch.guest_fpu) {
printk(KERN_ERR "kvm: failed to allocate vcpu's fpu\n");
err = -ENOMEM;
goto free_user_fpu;
}
 
-   err = kvm_vcpu_init(>vcpu, kvm, id);
+   err = kvm_vcpu_init(vcpu, kvm, id);
if (err)
goto free_svm;
 
@@ -,9 +2224,9 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, 
unsigned int id)
svm->asid_generation = 0;
init_vmcb(svm);
 
-   svm_init_osvw(>vcpu);
+   svm_init_osvw(vcpu);
 
-   return >vcpu;
+   return vcpu;
 
 free_page4:
__free_page(hsave_page);
@@ -2235,11 +2237,11 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm 
*kvm, unsigned int id)
 free_page1:
__free_page(page);
 uninit:
-   kvm_vcpu_uninit(>vcpu);
+   kvm_vcpu_uninit(vcpu);
 free_svm:
-   kmem_cache_free(x86_fpu_cache, svm->vcpu.arch.guest_fpu);
+   kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
 free_user_fpu:
-   kmem_cache_free(x86_fpu_cache, svm->vcpu.arch.user_fpu);
+   kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
 free_partial_svm:
kmem_cache_free(kvm_vcpu_cache, svm);
 out:
@@ -2270,8 +2272,8 @@ static void svm_free_vcpu(struct kvm_vcpu *vcpu)
__free_page(virt_to_page(svm->nested.hsave));
__free_pages(virt_to_page(svm->nested.msrpm), MSRPM_ALLOC_ORDER);
kvm_vcpu_uninit(vcpu);
-   kmem_cache_free(x86_fpu_cache, svm->vcpu.arch.user_fpu);
-   kmem_cache_free(x86_fpu_cache, svm->vcpu.arch.guest_fpu);
+   kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
+   kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
kmem_cache_free(kvm_vcpu_cache, svm);
 }
 
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 10/45] KVM: x86: Move kvm_vcpu_init() invocation to common code

2019-12-18 Thread Sean Christopherson
Move the kvm_cpu_{un}init() calls to common x86 code as an intermediate
step to removing kvm_cpu_{un}init() altogether.

Note, VMX'x alloc_apic_access_page() and init_rmode_identity_map() are
per-VM allocations and are intentionally kept if vCPU creation fails.
They are freed by kvm_arch_destroy_vm().

No functional change intended.

Signed-off-by: Sean Christopherson 
---
 arch/x86/include/asm/kvm_host.h |  2 +-
 arch/x86/kvm/svm.c  | 13 +++--
 arch/x86/kvm/vmx/vmx.c  | 19 ++-
 arch/x86/kvm/x86.c  | 20 +++-
 4 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index dbc2aa055942..deddd6fb198c 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1045,7 +1045,7 @@ struct kvm_x86_ops {
void (*vm_destroy)(struct kvm *kvm);
 
/* Create, but do not attach this VCPU */
-   int (*vcpu_create)(struct kvm *kvm, struct kvm_vcpu *vcpu, unsigned id);
+   int (*vcpu_create)(struct kvm_vcpu *vcpu);
void (*vcpu_free)(struct kvm_vcpu *vcpu);
void (*vcpu_reset)(struct kvm_vcpu *vcpu, bool init_event);
 
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 1a5ead23e84b..84d1a28e99d6 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2144,8 +2144,7 @@ static int avic_init_vcpu(struct vcpu_svm *svm)
return ret;
 }
 
-static int svm_create_vcpu(struct kvm *kvm, struct kvm_vcpu *vcpu,
-  unsigned int id)
+static int svm_create_vcpu(struct kvm_vcpu *vcpu)
 {
struct vcpu_svm *svm;
struct page *page;
@@ -2157,14 +2156,10 @@ static int svm_create_vcpu(struct kvm *kvm, struct 
kvm_vcpu *vcpu,
BUILD_BUG_ON(offsetof(struct vcpu_svm, vcpu) != 0);
svm = to_svm(vcpu);
 
-   err = kvm_vcpu_init(vcpu, kvm, id);
-   if (err)
-   return err;
-
err = -ENOMEM;
page = alloc_page(GFP_KERNEL_ACCOUNT);
if (!page)
-   goto uninit;
+   goto out;
 
msrpm_pages = alloc_pages(GFP_KERNEL_ACCOUNT, MSRPM_ALLOC_ORDER);
if (!msrpm_pages)
@@ -2213,8 +2208,7 @@ static int svm_create_vcpu(struct kvm *kvm, struct 
kvm_vcpu *vcpu,
__free_pages(msrpm_pages, MSRPM_ALLOC_ORDER);
 free_page1:
__free_page(page);
-uninit:
-   kvm_vcpu_uninit(vcpu);
+out:
return err;
 }
 
@@ -2241,7 +2235,6 @@ static void svm_free_vcpu(struct kvm_vcpu *vcpu)
__free_pages(virt_to_page(svm->msrpm), MSRPM_ALLOC_ORDER);
__free_page(virt_to_page(svm->nested.hsave));
__free_pages(virt_to_page(svm->nested.msrpm), MSRPM_ALLOC_ORDER);
-   kvm_vcpu_uninit(vcpu);
 }
 
 static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index fd688b16e688..eea940330dd3 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6672,11 +6672,9 @@ static void vmx_free_vcpu(struct kvm_vcpu *vcpu)
free_vpid(vmx->vpid);
nested_vmx_free_vcpu(vcpu);
free_loaded_vmcs(vmx->loaded_vmcs);
-   kvm_vcpu_uninit(vcpu);
 }
 
-static int vmx_create_vcpu(struct kvm *kvm, struct kvm_vcpu *vcpu,
-  unsigned int id)
+static int vmx_create_vcpu(struct kvm_vcpu *vcpu)
 {
struct vcpu_vmx *vmx;
unsigned long *msr_bitmap;
@@ -6685,10 +6683,6 @@ static int vmx_create_vcpu(struct kvm *kvm, struct 
kvm_vcpu *vcpu,
BUILD_BUG_ON(offsetof(struct vcpu_vmx, vcpu) != 0);
vmx = to_vmx(vcpu);
 
-   err = kvm_vcpu_init(vcpu, kvm, id);
-   if (err)
-   return err;
-
err = -ENOMEM;
 
vmx->vpid = allocate_vpid();
@@ -6702,7 +6696,7 @@ static int vmx_create_vcpu(struct kvm *kvm, struct 
kvm_vcpu *vcpu,
if (enable_pml) {
vmx->pml_pg = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);
if (!vmx->pml_pg)
-   goto uninit_vcpu;
+   goto free_vpid;
}
 
BUILD_BUG_ON(ARRAY_SIZE(vmx_msr_index) != NR_SHARED_MSRS);
@@ -6747,7 +6741,7 @@ static int vmx_create_vcpu(struct kvm *kvm, struct 
kvm_vcpu *vcpu,
vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_CS, 
MSR_TYPE_RW);
vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_ESP, 
MSR_TYPE_RW);
vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_EIP, 
MSR_TYPE_RW);
-   if (kvm_cstate_in_guest(kvm)) {
+   if (kvm_cstate_in_guest(vcpu->kvm)) {
vmx_disable_intercept_for_msr(msr_bitmap, MSR_CORE_C1_RES, 
MSR_TYPE_R);
vmx_disable_intercept_for_msr(msr_bitmap, 
MSR_CORE_C3_RESIDENCY, MSR_TYPE_R);
vmx_disable_intercept_for_msr(msr_bitmap, 
MSR_CORE_C6_RESIDENCY, MSR_TYPE_R);
@@ -6763,13 +6757,13 @@ static int vmx_create_vcpu(struct kvm *kvm, struct 
kvm_vcpu *vcpu,
vmx_vcpu_put(vcpu);

[PATCH v2 08/45] KVM: x86: Move FPU allocation to common x86 code

2019-12-18 Thread Sean Christopherson
The allocation of FPU structs is identical across VMX and SVM, move it
to common x86 code.  Somewhat arbitrarily place the allocation so that
it resides directly above the associated initialization via fx_init(),
e.g. instead of retaining its position with respect to the overall vcpu
creation flow.  Although the names names kvm_arch_vcpu_create() and
kvm_arch_vcpu_init() might suggest otherwise, x86 does not have a clean
split between 'create' and 'init'.  Allocating the struct immediately
prior to the first use arguably improves readability *now*, and will
yield even bigger improvements when kvm_arch_vcpu_init() is removed in
a future patch.

Signed-off-by: Sean Christopherson 
---
 arch/x86/kvm/svm.c | 25 +
 arch/x86/kvm/vmx/vmx.c | 25 +
 arch/x86/kvm/x86.c | 21 +
 3 files changed, 23 insertions(+), 48 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 4a979278abba..1a5ead23e84b 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2157,25 +2157,9 @@ static int svm_create_vcpu(struct kvm *kvm, struct 
kvm_vcpu *vcpu,
BUILD_BUG_ON(offsetof(struct vcpu_svm, vcpu) != 0);
svm = to_svm(vcpu);
 
-   vcpu->arch.user_fpu = kmem_cache_zalloc(x86_fpu_cache,
-   GFP_KERNEL_ACCOUNT);
-   if (!vcpu->arch.user_fpu) {
-   printk(KERN_ERR "kvm: failed to allocate kvm userspace's 
fpu\n");
-   err = -ENOMEM;
-   goto out;
-   }
-
-   vcpu->arch.guest_fpu = kmem_cache_zalloc(x86_fpu_cache,
-GFP_KERNEL_ACCOUNT);
-   if (!vcpu->arch.guest_fpu) {
-   printk(KERN_ERR "kvm: failed to allocate vcpu's fpu\n");
-   err = -ENOMEM;
-   goto free_user_fpu;
-   }
-
err = kvm_vcpu_init(vcpu, kvm, id);
if (err)
-   goto free_guest_fpu;
+   return err;
 
err = -ENOMEM;
page = alloc_page(GFP_KERNEL_ACCOUNT);
@@ -2231,11 +2215,6 @@ static int svm_create_vcpu(struct kvm *kvm, struct 
kvm_vcpu *vcpu,
__free_page(page);
 uninit:
kvm_vcpu_uninit(vcpu);
-free_guest_fpu:
-   kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
-free_user_fpu:
-   kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
-out:
return err;
 }
 
@@ -2263,8 +2242,6 @@ static void svm_free_vcpu(struct kvm_vcpu *vcpu)
__free_page(virt_to_page(svm->nested.hsave));
__free_pages(virt_to_page(svm->nested.msrpm), MSRPM_ALLOC_ORDER);
kvm_vcpu_uninit(vcpu);
-   kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
-   kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
 }
 
 static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 44166309b66c..fd688b16e688 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6673,8 +6673,6 @@ static void vmx_free_vcpu(struct kvm_vcpu *vcpu)
nested_vmx_free_vcpu(vcpu);
free_loaded_vmcs(vmx->loaded_vmcs);
kvm_vcpu_uninit(vcpu);
-   kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
-   kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
 }
 
 static int vmx_create_vcpu(struct kvm *kvm, struct kvm_vcpu *vcpu,
@@ -6687,25 +6685,9 @@ static int vmx_create_vcpu(struct kvm *kvm, struct 
kvm_vcpu *vcpu,
BUILD_BUG_ON(offsetof(struct vcpu_vmx, vcpu) != 0);
vmx = to_vmx(vcpu);
 
-   vcpu->arch.user_fpu = kmem_cache_zalloc(x86_fpu_cache,
-   GFP_KERNEL_ACCOUNT);
-   if (!vcpu->arch.user_fpu) {
-   printk(KERN_ERR "kvm: failed to allocate kvm userspace's 
fpu\n");
-   err = -ENOMEM;
-   goto out;
-   }
-
-   vcpu->arch.guest_fpu = kmem_cache_zalloc(x86_fpu_cache,
-GFP_KERNEL_ACCOUNT);
-   if (!vcpu->arch.guest_fpu) {
-   printk(KERN_ERR "kvm: failed to allocate vcpu's fpu\n");
-   err = -ENOMEM;
-   goto free_user_fpu;
-   }
-
err = kvm_vcpu_init(vcpu, kvm, id);
if (err)
-   goto free_vcpu;
+   return err;
 
err = -ENOMEM;
 
@@ -6822,11 +6804,6 @@ static int vmx_create_vcpu(struct kvm *kvm, struct 
kvm_vcpu *vcpu,
 uninit_vcpu:
kvm_vcpu_uninit(vcpu);
free_vpid(vmx->vpid);
-free_vcpu:
-   kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
-free_user_fpu:
-   kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
-out:
return err;
 }
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b7f565a43ad7..e3f122dca5f8 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9096,6 +9096,8 @@ void kvm_arch_vcpu_free(struct kvm_vcpu *vcpu)
kvm_x86_ops->vcpu_free(vcpu);
 
free_cpumask_var(vcpu->arch.wbinvd_dirty_mask);

[PATCH v2 00/45] KVM: Refactor vCPU creation

2019-12-18 Thread Sean Christopherson
The end goal of this series is to strip down the interface between common
KVM code and arch specific code so that there is precisely one arch hook
for creating a vCPU and one hook for destroying a vCPU.  In addition to
cleaning up the code base, simplifying the interface gives architectures
more freedom to organize their vCPU creation code.  Details below.

v2:
  - Rebase to commit e41a90be9659 ("KVM: x86/mmu: WARN if root_hpa is
invalid when handling a page fault").  A few minor x86 (VMX) conflicts,
and one straightforward arm conflict with commit 8564d6372a7d ("KVM:
arm64: Support stolen time reporting via shared structure") in patch
"KVM: ARM: Move all vcpu init code into kvm_arch_vcpu_create()".
  - Collect Reviews and Acks [Christoffer, Greg].
  - Fix a warning in "KVM: arm64: Free sve_state via arm specific hook"
by using a "void" return type [Christoffer].
  - Fix a bug in "KVM: PPC: Move kvm_vcpu_init() invocation to common code"
where the call in kvmppc_core_vcpu_create_e500mc() was inadvertantly
left behind.


KVM's vCPU creation code is comically messy.  kvm_vm_ioctl_create_vcpu()
calls three separate arch hooks: init(), create() and setup().  The init()
call is especially nasty as it's hidden away in a common KVM function,
kvm_init_vcpu(), that for all intents and purposes must be immediately
invoked after the vcpu object is allocated.

Not to be outdone, vCPU destruction also has three arch hooks: uninit(),
destroy() and free(), the latter of which isn't actually invoked by common
KVM code, but the hook declaration still exists because architectures are
relying on its forward declaration.

Eliminating the extra arch hooks is relatively straightforward, just
tedious.  For the most part, there is no fundamental constraint that
necessitated the proliferation of arch hooks, rather they crept in over
time, usually when x86-centric code was moved out of generic KVM and into
x86 code.

E.g. kvm_arch_vcpu_setup() was added to allow x86 to do vcpu_load(), which
can only be done after preempt_notifier initialization, but adding setup()
overlooked the fact that the preempt_notifier was only initialized after
kvm_arch_vcpu_create() because preemption support was added when x86's MMU
setup (the vcpu_load() user) was called from common KVM code.

For all intents and purposes, there is no true functional change in this
series.  The order of some allocations will change, and a few memory leaks
are fixed, but the actual functionality of a guest should be unaffected.

Patches 01-03 are bug fixes in error handling paths that were found by
inspection when refactoring the associated code.

Patches 04-43 refactor each arch implementation so that the unwanted arch
hooks can be dropped without a functional change, e.g. move code out of
kvm_arch_vcpu_setup() so that all implementations are empty, then drop the
functions and caller.

Patches 44-45 are minor clean up to eliminate kvm_vcpu_uninit().

The net result is to go from this:

vcpu = kvm_arch_vcpu_create(kvm, id);
   |
   |-> kvm_vcpu_init()
   |
   |-> kvm_arch_vcpu_init()

if (IS_ERR(vcpu)) {
r = PTR_ERR(vcpu);
goto vcpu_decrement;
}

preempt_notifier_init(>preempt_notifier, _preempt_ops);

r = kvm_arch_vcpu_setup(vcpu);
if (r)
goto vcpu_destroy;


Sean Christopherson (45):
  KVM: PPC: Book3S HV: Uninit vCPU if vcore creation fails
  KVM: PPC: Book3S PR: Free shared page if mmu initialization fails
  KVM: x86: Free wbinvd_dirty_mask if vCPU creation fails
  KVM: VMX: Allocate VPID after initializing VCPU
  KVM: VMX: Use direct vcpu pointer during vCPU create/free
  KVM: SVM: Use direct vcpu pointer during vCPU create/free
  KVM: x86: Allocate vcpu struct in common x86 code
  KVM: x86: Move FPU allocation to common x86 code
  KVM: x86: Move allocation of pio_data page down a few lines
  KVM: x86: Move kvm_vcpu_init() invocation to common code
  KVM: PPC: e500mc: Add build-time assert that vcpu is at offset 0
  KVM: PPC: Allocate vcpu struct in common PPC code
  KVM: PPC: Book3S PR: Allocate book3s and shadow vcpu after common init
  KVM: PPC: e500mc: Move reset of oldpir below call to kvm_vcpu_init()
  KVM: PPC: Move kvm_vcpu_init() invocation to common code
  KVM: MIPS: Use kvm_vcpu_cache to allocate vCPUs
  KVM: MIPS: Drop kvm_arch_vcpu_free()
  KVM: PPC: Drop kvm_arch_vcpu_free()
  KVM: arm: Drop kvm_arch_vcpu_free()
  KVM: x86: Remove spurious kvm_mmu_unload() from vcpu destruction path
  KVM: x86: Remove spurious clearing of async #PF MSR
  KVM: x86: Drop kvm_arch_vcpu_free()
  KVM: Remove kvm_arch_vcpu_free() declaration
  KVM: Add kvm_arch_vcpu_precreate() to handle pre-allocation issues
  KVM: s390: Move guts of kvm_arch_vcpu_init() into
kvm_arch_vcpu_create()
  KVM: s390: Invoke kvm_vcpu_init() before allocating sie_page
  KVM: MIPS: Invoke kvm_vcpu_uninit() immediately 

[PATCH v2 04/45] KVM: VMX: Allocate VPID after initializing VCPU

2019-12-18 Thread Sean Christopherson
Do VPID allocation after calling the common kvm_vcpu_init() as a step
towards doing vCPU allocation (via kmem_cache_zalloc()) and calling
kvm_vcpu_init() back-to-back.  Squishing allocation and initialization
together will eventually allow the sequence to be moved to arch-agnostic
creation code.

Note, the VPID is not consumed until KVM_RUN, slightly delaying its
allocation should have no real function impact.  VPID allocation was
arbitrarily placed in the original patch, commit 2384d2b326408 ("KVM:
VMX: Enable Virtual Processor Identification (VPID)").

Signed-off-by: Sean Christopherson 
---
 arch/x86/kvm/vmx/vmx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 51e3b27f90ed..2e44ef744c01 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6708,14 +6708,14 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm 
*kvm, unsigned int id)
goto free_user_fpu;
}
 
-   vmx->vpid = allocate_vpid();
-
err = kvm_vcpu_init(>vcpu, kvm, id);
if (err)
goto free_vcpu;
 
err = -ENOMEM;
 
+   vmx->vpid = allocate_vpid();
+
/*
 * If PML is turned on, failure on enabling PML just results in failure
 * of creating the vcpu, therefore we can simplify PML logic (by
@@ -6826,8 +6826,8 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, 
unsigned int id)
vmx_destroy_pml_buffer(vmx);
 uninit_vcpu:
kvm_vcpu_uninit(>vcpu);
+   free_vpid(vmx->vpid);
 free_vcpu:
-   free_vpid(vmx->vpid);
kmem_cache_free(x86_fpu_cache, vmx->vcpu.arch.guest_fpu);
 free_user_fpu:
kmem_cache_free(x86_fpu_cache, vmx->vcpu.arch.user_fpu);
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 07/45] KVM: x86: Allocate vcpu struct in common x86 code

2019-12-18 Thread Sean Christopherson
Move allocation of VMX and SVM vcpus to common x86.  Although the struct
being allocated is technically a VMX/SVM struct, it can be interpreted
directly as a 'struct kvm_vcpu' because of the pre-existing requirement
that 'struct kvm_vcpu' be located at offset zero of the arch/vendor vcpu
struct.

Remove the message from the build-time assertions regarding placement of
the struct, as compatibility with the arch usercopy region is no longer
the sole dependent on 'struct kvm_vcpu' being at offset zero.

Signed-off-by: Sean Christopherson 
---
 arch/x86/include/asm/kvm_host.h |  2 +-
 arch/x86/kvm/svm.c  | 28 +---
 arch/x86/kvm/vmx/vmx.c  | 24 
 arch/x86/kvm/x86.c  | 16 
 4 files changed, 30 insertions(+), 40 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 159a28512e4c..dbc2aa055942 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1045,7 +1045,7 @@ struct kvm_x86_ops {
void (*vm_destroy)(struct kvm *kvm);
 
/* Create, but do not attach this VCPU */
-   struct kvm_vcpu *(*vcpu_create)(struct kvm *kvm, unsigned id);
+   int (*vcpu_create)(struct kvm *kvm, struct kvm_vcpu *vcpu, unsigned id);
void (*vcpu_free)(struct kvm_vcpu *vcpu);
void (*vcpu_reset)(struct kvm_vcpu *vcpu, bool init_event);
 
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 1afa10cf365f..4a979278abba 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2144,9 +2144,9 @@ static int avic_init_vcpu(struct vcpu_svm *svm)
return ret;
 }
 
-static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
+static int svm_create_vcpu(struct kvm *kvm, struct kvm_vcpu *vcpu,
+  unsigned int id)
 {
-   struct kvm_vcpu *vcpu;
struct vcpu_svm *svm;
struct page *page;
struct page *msrpm_pages;
@@ -2154,22 +2154,15 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm 
*kvm, unsigned int id)
struct page *nested_msrpm_pages;
int err;
 
-   BUILD_BUG_ON_MSG(offsetof(struct vcpu_svm, vcpu) != 0,
-   "struct kvm_vcpu must be at offset 0 for arch usercopy region");
-
-   svm = kmem_cache_zalloc(kvm_vcpu_cache, GFP_KERNEL_ACCOUNT);
-   if (!svm) {
-   err = -ENOMEM;
-   goto out;
-   }
-   vcpu = >vcpu;
+   BUILD_BUG_ON(offsetof(struct vcpu_svm, vcpu) != 0);
+   svm = to_svm(vcpu);
 
vcpu->arch.user_fpu = kmem_cache_zalloc(x86_fpu_cache,
GFP_KERNEL_ACCOUNT);
if (!vcpu->arch.user_fpu) {
printk(KERN_ERR "kvm: failed to allocate kvm userspace's 
fpu\n");
err = -ENOMEM;
-   goto free_partial_svm;
+   goto out;
}
 
vcpu->arch.guest_fpu = kmem_cache_zalloc(x86_fpu_cache,
@@ -2182,7 +2175,7 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, 
unsigned int id)
 
err = kvm_vcpu_init(vcpu, kvm, id);
if (err)
-   goto free_svm;
+   goto free_guest_fpu;
 
err = -ENOMEM;
page = alloc_page(GFP_KERNEL_ACCOUNT);
@@ -2226,7 +2219,7 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, 
unsigned int id)
 
svm_init_osvw(vcpu);
 
-   return vcpu;
+   return 0;
 
 free_page4:
__free_page(hsave_page);
@@ -2238,14 +2231,12 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm 
*kvm, unsigned int id)
__free_page(page);
 uninit:
kvm_vcpu_uninit(vcpu);
-free_svm:
+free_guest_fpu:
kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
 free_user_fpu:
kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
-free_partial_svm:
-   kmem_cache_free(kvm_vcpu_cache, svm);
 out:
-   return ERR_PTR(err);
+   return err;
 }
 
 static void svm_clear_current_vmcb(struct vmcb *vmcb)
@@ -2274,7 +2265,6 @@ static void svm_free_vcpu(struct kvm_vcpu *vcpu)
kvm_vcpu_uninit(vcpu);
kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
-   kmem_cache_free(kvm_vcpu_cache, svm);
 }
 
 static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 5db455796965..44166309b66c 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6675,31 +6675,24 @@ static void vmx_free_vcpu(struct kvm_vcpu *vcpu)
kvm_vcpu_uninit(vcpu);
kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
-   kmem_cache_free(kvm_vcpu_cache, vmx);
 }
 
-static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
+static int vmx_create_vcpu(struct kvm *kvm, struct kvm_vcpu *vcpu,
+  unsigned int id)
 {
-   struct kvm_vcpu 

[PATCH v2 09/45] KVM: x86: Move allocation of pio_data page down a few lines

2019-12-18 Thread Sean Christopherson
Allocate the pio_data page after creating the MMU and local APIC so that
all direct memory allocations are grouped together.  This allows setting
the return value to -ENOMEM prior to starting the allocations instead of
setting it in the fail path for every allocation.

The pio_data page is only consumed when KVM_RUN is invoked, i.e. moving
its allocation has no real functional impact.

Signed-off-by: Sean Christopherson 
---
 arch/x86/kvm/x86.c | 33 +
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e3f122dca5f8..b3e367963b08 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9420,18 +9420,11 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
else
vcpu->arch.mp_state = KVM_MP_STATE_UNINITIALIZED;
 
-   page = alloc_page(GFP_KERNEL | __GFP_ZERO);
-   if (!page) {
-   r = -ENOMEM;
-   goto fail;
-   }
-   vcpu->arch.pio_data = page_address(page);
-
kvm_set_tsc_khz(vcpu, max_tsc_khz);
 
r = kvm_mmu_create(vcpu);
if (r < 0)
-   goto fail_free_pio_data;
+   return r;
 
if (irqchip_in_kernel(vcpu->kvm)) {
vcpu->arch.apicv_active = 
kvm_x86_ops->get_enable_apicv(vcpu->kvm);
@@ -9441,25 +9434,27 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
} else
static_key_slow_inc(_no_apic_vcpu);
 
+   r = -ENOMEM;
+
+   page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+   if (!page)
+   goto fail_free_lapic;
+   vcpu->arch.pio_data = page_address(page);
+
vcpu->arch.mce_banks = kzalloc(KVM_MAX_MCE_BANKS * sizeof(u64) * 4,
   GFP_KERNEL_ACCOUNT);
-   if (!vcpu->arch.mce_banks) {
-   r = -ENOMEM;
-   goto fail_free_lapic;
-   }
+   if (!vcpu->arch.mce_banks)
+   goto fail_free_pio_data;
vcpu->arch.mcg_cap = KVM_MAX_MCE_BANKS;
 
if (!zalloc_cpumask_var(>arch.wbinvd_dirty_mask,
-   GFP_KERNEL_ACCOUNT)) {
-   r = -ENOMEM;
+   GFP_KERNEL_ACCOUNT))
goto fail_free_mce_banks;
-   }
 
vcpu->arch.user_fpu = kmem_cache_zalloc(x86_fpu_cache,
GFP_KERNEL_ACCOUNT);
if (!vcpu->arch.user_fpu) {
pr_err("kvm: failed to allocate userspace's fpu\n");
-   r = -ENOMEM;
goto free_wbinvd_dirty_mask;
}
 
@@ -9467,7 +9462,6 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 GFP_KERNEL_ACCOUNT);
if (!vcpu->arch.guest_fpu) {
pr_err("kvm: failed to allocate vcpu's fpu\n");
-   r = -ENOMEM;
goto free_user_fpu;
}
fx_init(vcpu);
@@ -9494,13 +9488,12 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
free_cpumask_var(vcpu->arch.wbinvd_dirty_mask);
 fail_free_mce_banks:
kfree(vcpu->arch.mce_banks);
+fail_free_pio_data:
+   free_page((unsigned long)vcpu->arch.pio_data);
 fail_free_lapic:
kvm_free_lapic(vcpu);
 fail_mmu_destroy:
kvm_mmu_destroy(vcpu);
-fail_free_pio_data:
-   free_page((unsigned long)vcpu->arch.pio_data);
-fail:
return r;
 }
 
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 02/45] KVM: PPC: Book3S PR: Free shared page if mmu initialization fails

2019-12-18 Thread Sean Christopherson
Explicitly free the shared page if kvmppc_mmu_init() fails during
kvmppc_core_vcpu_create(), as the page is freed only in
kvmppc_core_vcpu_free(), which is not reached via kvm_vcpu_uninit().

Fixes: 96bc451a15329 ("KVM: PPC: Introduce shared page")
Cc: sta...@vger.kernel.org
Reviewed-by: Greg Kurz 
Signed-off-by: Sean Christopherson 
---
 arch/powerpc/kvm/book3s_pr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index ce4fcf76e53e..26ca62b6d773 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1806,10 +1806,12 @@ static struct kvm_vcpu 
*kvmppc_core_vcpu_create_pr(struct kvm *kvm,
 
err = kvmppc_mmu_init(vcpu);
if (err < 0)
-   goto uninit_vcpu;
+   goto free_shared_page;
 
return vcpu;
 
+free_shared_page:
+   free_page((unsigned long)vcpu->arch.shared);
 uninit_vcpu:
kvm_vcpu_uninit(vcpu);
 free_shadow_vcpu:
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 03/45] KVM: x86: Free wbinvd_dirty_mask if vCPU creation fails

2019-12-18 Thread Sean Christopherson
Free the vCPU's wbinvd_dirty_mask if vCPU creation fails after
kvm_arch_vcpu_init(), e.g. when installing the vCPU's file descriptor.
Do the freeing by calling kvm_arch_vcpu_free() instead of open coding
the freeing.  This adds a likely superfluous, but ultimately harmless,
call to kvmclock_reset(), which only clears vcpu->arch.pv_time_enabled.
Using kvm_arch_vcpu_free() allows for additional cleanup in the future.

Fixes: f5f48ee15c2ee ("KVM: VMX: Execute WBINVD to keep data consistency with 
assigned devices")
Cc: sta...@vger.kernel.org
Signed-off-by: Sean Christopherson 
---
 arch/x86/kvm/x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8bb2fb1705ff..82d41257d2a3 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9162,7 +9162,7 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
kvm_mmu_unload(vcpu);
vcpu_put(vcpu);
 
-   kvm_x86_ops->vcpu_free(vcpu);
+   kvm_arch_vcpu_free(vcpu);
 }
 
 void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 05/45] KVM: VMX: Use direct vcpu pointer during vCPU create/free

2019-12-18 Thread Sean Christopherson
Capture the vcpu pointer in a local varaible and replace '>vcpu'
references with a direct reference to the pointer in anticipation of
moving bits of the code to common x86 and passing the vcpu pointer into
vmx_create_vcpu(), i.e. eliminate unnecessary noise from future patches.

Signed-off-by: Sean Christopherson 
---
 arch/x86/kvm/vmx/vmx.c | 42 ++
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 2e44ef744c01..5db455796965 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6673,17 +6673,17 @@ static void vmx_free_vcpu(struct kvm_vcpu *vcpu)
nested_vmx_free_vcpu(vcpu);
free_loaded_vmcs(vmx->loaded_vmcs);
kvm_vcpu_uninit(vcpu);
-   kmem_cache_free(x86_fpu_cache, vmx->vcpu.arch.user_fpu);
-   kmem_cache_free(x86_fpu_cache, vmx->vcpu.arch.guest_fpu);
+   kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
+   kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
kmem_cache_free(kvm_vcpu_cache, vmx);
 }
 
 static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
 {
-   int err;
+   struct kvm_vcpu *vcpu;
struct vcpu_vmx *vmx;
unsigned long *msr_bitmap;
-   int i, cpu;
+   int i, cpu, err;
 
BUILD_BUG_ON_MSG(offsetof(struct vcpu_vmx, vcpu) != 0,
"struct kvm_vcpu must be at offset 0 for arch usercopy region");
@@ -6692,23 +6692,25 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm 
*kvm, unsigned int id)
if (!vmx)
return ERR_PTR(-ENOMEM);
 
-   vmx->vcpu.arch.user_fpu = kmem_cache_zalloc(x86_fpu_cache,
-   GFP_KERNEL_ACCOUNT);
-   if (!vmx->vcpu.arch.user_fpu) {
+   vcpu = >vcpu;
+
+   vcpu->arch.user_fpu = kmem_cache_zalloc(x86_fpu_cache,
+   GFP_KERNEL_ACCOUNT);
+   if (!vcpu->arch.user_fpu) {
printk(KERN_ERR "kvm: failed to allocate kvm userspace's 
fpu\n");
err = -ENOMEM;
goto free_partial_vcpu;
}
 
-   vmx->vcpu.arch.guest_fpu = kmem_cache_zalloc(x86_fpu_cache,
-   GFP_KERNEL_ACCOUNT);
-   if (!vmx->vcpu.arch.guest_fpu) {
+   vcpu->arch.guest_fpu = kmem_cache_zalloc(x86_fpu_cache,
+GFP_KERNEL_ACCOUNT);
+   if (!vcpu->arch.guest_fpu) {
printk(KERN_ERR "kvm: failed to allocate vcpu's fpu\n");
err = -ENOMEM;
goto free_user_fpu;
}
 
-   err = kvm_vcpu_init(>vcpu, kvm, id);
+   err = kvm_vcpu_init(vcpu, kvm, id);
if (err)
goto free_vcpu;
 
@@ -6780,12 +6782,12 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm 
*kvm, unsigned int id)
 
vmx->loaded_vmcs = >vmcs01;
cpu = get_cpu();
-   vmx_vcpu_load(>vcpu, cpu);
-   vmx->vcpu.cpu = cpu;
+   vmx_vcpu_load(vcpu, cpu);
+   vcpu->cpu = cpu;
init_vmcs(vmx);
-   vmx_vcpu_put(>vcpu);
+   vmx_vcpu_put(vcpu);
put_cpu();
-   if (cpu_need_virtualize_apic_accesses(>vcpu)) {
+   if (cpu_need_virtualize_apic_accesses(vcpu)) {
err = alloc_apic_access_page(kvm);
if (err)
goto free_vmcs;
@@ -6800,7 +6802,7 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, 
unsigned int id)
if (nested)
nested_vmx_setup_ctls_msrs(>nested.msrs,
   vmx_capability.ept,
-  kvm_vcpu_apicv_active(>vcpu));
+  kvm_vcpu_apicv_active(vcpu));
else
memset(>nested.msrs, 0, sizeof(vmx->nested.msrs));
 
@@ -6818,19 +6820,19 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm 
*kvm, unsigned int id)
 
vmx->ept_pointer = INVALID_PAGE;
 
-   return >vcpu;
+   return vcpu;
 
 free_vmcs:
free_loaded_vmcs(vmx->loaded_vmcs);
 free_pml:
vmx_destroy_pml_buffer(vmx);
 uninit_vcpu:
-   kvm_vcpu_uninit(>vcpu);
+   kvm_vcpu_uninit(vcpu);
free_vpid(vmx->vpid);
 free_vcpu:
-   kmem_cache_free(x86_fpu_cache, vmx->vcpu.arch.guest_fpu);
+   kmem_cache_free(x86_fpu_cache, vcpu->arch.guest_fpu);
 free_user_fpu:
-   kmem_cache_free(x86_fpu_cache, vmx->vcpu.arch.user_fpu);
+   kmem_cache_free(x86_fpu_cache, vcpu->arch.user_fpu);
 free_partial_vcpu:
kmem_cache_free(kvm_vcpu_cache, vmx);
return ERR_PTR(err);
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 13/45] KVM: PPC: Book3S PR: Allocate book3s and shadow vcpu after common init

2019-12-18 Thread Sean Christopherson
Call kvm_vcpu_init() in kvmppc_core_vcpu_create_pr() prior to allocating
the book3s and shadow_vcpu objects in preparation of moving said call to
common PPC code.  Although kvm_vcpu_init() has an arch callback, the
callback is empty for Book3S PR, i.e. barring unseen black magic, moving
the allocation has no real functional impact.

Signed-off-by: Sean Christopherson 
---
 arch/powerpc/kvm/book3s_pr.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index 0d7c8a7bcb7b..10c65d412e81 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1748,12 +1748,18 @@ static int kvmppc_core_vcpu_create_pr(struct kvm *kvm, 
struct kvm_vcpu *vcpu,
  unsigned int id)
 {
struct kvmppc_vcpu_book3s *vcpu_book3s;
-   int err = -ENOMEM;
unsigned long p;
+   int err;
+
+   err = kvm_vcpu_init(vcpu, kvm, id);
+   if (err)
+   return err;
+
+   err = -ENOMEM;
 
vcpu_book3s = vzalloc(sizeof(struct kvmppc_vcpu_book3s));
if (!vcpu_book3s)
-   goto out;
+   goto uninit_vcpu;
vcpu->arch.book3s = vcpu_book3s;
 
 #ifdef CONFIG_KVM_BOOK3S_32_HANDLER
@@ -1763,14 +1769,9 @@ static int kvmppc_core_vcpu_create_pr(struct kvm *kvm, 
struct kvm_vcpu *vcpu,
goto free_vcpu3s;
 #endif
 
-   err = kvm_vcpu_init(vcpu, kvm, id);
-   if (err)
-   goto free_shadow_vcpu;
-
-   err = -ENOMEM;
p = __get_free_page(GFP_KERNEL|__GFP_ZERO);
if (!p)
-   goto uninit_vcpu;
+   goto free_shadow_vcpu;
vcpu->arch.shared = (void *)p;
 #ifdef CONFIG_PPC_BOOK3S_64
/* Always start the shared struct in native endian mode */
@@ -1807,15 +1808,14 @@ static int kvmppc_core_vcpu_create_pr(struct kvm *kvm, 
struct kvm_vcpu *vcpu,
 
 free_shared_page:
free_page((unsigned long)vcpu->arch.shared);
-uninit_vcpu:
-   kvm_vcpu_uninit(vcpu);
 free_shadow_vcpu:
 #ifdef CONFIG_KVM_BOOK3S_32_HANDLER
kfree(vcpu->arch.shadow_vcpu);
 free_vcpu3s:
 #endif
vfree(vcpu_book3s);
-out:
+uninit_vcpu:
+   kvm_vcpu_uninit(vcpu);
return err;
 }
 
-- 
2.24.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v4 00/19] KVM: Dynamically size memslot arrays

2019-12-18 Thread Marc Zyngier

On 2019-12-17 20:40, Sean Christopherson wrote:
The end goal of this series is to dynamically size the memslot array 
so

that KVM allocates memory based on the number of memslots in use, as
opposed to unconditionally allocating memory for the maximum number 
of
memslots.  On x86, each memslot consumes 88 bytes, and so with 2 
address

spaces of 512 memslots, each VM consumes ~90k bytes for the memslots.
E.g. given a VM that uses a total of 30 memslots, dynamic sizing 
reduces

the memory footprint from 90k to ~2.6k bytes.

The changes required to support dynamic sizing are relatively small,
e.g. are essentially contained in patches 17/19 and 18/19.

Patches 2-16 clean up the memslot code, which has gotten quite 
crusty,
especially __kvm_set_memory_region().  The clean up is likely not 
strictly

necessary to switch to dynamic sizing, but I didn't have a remotely
reasonable level of confidence in the correctness of the dynamic 
sizing

without first doing the clean up.

The only functional change in v4 is the addition of an x86-specific 
bug
fix in x86's handling of KVM_MR_MOVE.  The bug fix is not directly 
related
to dynamically allocating memslots, but it has subtle and hidden 
conflicts
with the cleanup patches, and the fix is higher priority than 
anything

else in the series, i.e. should be merged first.

On non-x86 architectures, v3 and v4 should be functionally 
equivalent,

the only non-x86 change in v4 is the dropping of a "const" in
kvm_arch_commit_memory_region().


Gave it another go on top of 5.5-rc2 on an arm64 box, and nothing
exploded. So thumbs up from me.

Thanks,

   M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [GIT PULL] KVM/arm updates for 5.5-rc2

2019-12-18 Thread Paolo Bonzini
On 12/12/19 18:28, Marc Zyngier wrote:
> Paolo, Radim,
> 
> This is the first set of fixes for 5.5-rc2. This time around,
> a couple of MM fixes, a ONE_REG fix for an issue detected by
> GCC-10, and a handful of cleanups.
> 
> Please pull,
> 
>   M.
> 
> The following changes since commit cd7056ae34af0e9424da97bbc7d2b38246ba8a2c:
> 
>   Merge remote-tracking branch 'kvmarm/misc-5.5' into kvmarm/next (2019-11-08 
> 11:27:29 +)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git 
> tags/kvmarm-fixes-5.5-1
> 
> for you to fetch changes up to 6d674e28f642e3ff676fbae2d8d1b872814d32b6:
> 
>   KVM: arm/arm64: Properly handle faulting of device mappings (2019-12-12 
> 16:22:40 +)

Pulled, thanks.

Paolo

> 
> 
> KVM/arm fixes for .5.5, take #1
> 
> - Fix uninitialised sysreg accessor
> - Fix handling of demand-paged device mappings
> - Stop spamming the console on IMPDEF sysregs
> - Relax mappings of writable memslots
> - Assorted cleanups
> 
> 
> Jia He (1):
>   KVM: arm/arm64: Remove excessive permission check in 
> kvm_arch_prepare_memory_region
> 
> Marc Zyngier (1):
>   KVM: arm/arm64: Properly handle faulting of device mappings
> 
> Mark Rutland (2):
>   KVM: arm64: Sanely ratelimit sysreg messages
>   KVM: arm64: Don't log IMP DEF sysreg traps
> 
> Miaohe Lin (3):
>   KVM: arm/arm64: Get rid of unused arg in cpu_init_hyp_mode()
>   KVM: arm/arm64: vgic: Fix potential double free dist->spis in 
> __kvm_vgic_destroy()
>   KVM: arm/arm64: vgic: Use wrapper function to lock/unlock all vcpus in 
> kvm_vgic_create()
> 
> Will Deacon (1):
>   KVM: arm64: Ensure 'params' is initialised when looking up sys register
> 
>  arch/arm64/kvm/sys_regs.c | 25 ++---
>  arch/arm64/kvm/sys_regs.h | 17 +++--
>  virt/kvm/arm/arm.c|  4 ++--
>  virt/kvm/arm/mmu.c| 30 +-
>  virt/kvm/arm/vgic/vgic-init.c | 20 +---
>  5 files changed, 57 insertions(+), 39 deletions(-)
> 

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v4 19/19] KVM: selftests: Add test for KVM_SET_USER_MEMORY_REGION

2019-12-18 Thread Sean Christopherson
On Wed, Dec 18, 2019 at 12:39:43PM +0100, Christian Borntraeger wrote:
> 
> On 17.12.19 21:40, Sean Christopherson wrote:
> > Add a KVM selftest to test moving the base gfn of a userspace memory
> > region.  The test is primarily targeted at x86 to verify its memslot
> > metadata is correctly updated, but also provides basic functionality
> > coverage on other architectures.
> > +static void *vcpu_worker(void *data)
> > +{
> > +   struct kvm_vm *vm = data;
> > +   struct kvm_run *run;
> > +   struct ucall uc;
> > +   uint64_t cmd;
> > +
> > +   /*
> > +* Loop until the guest is done.  Re-enter the guest on all MMIO exits,
> > +* which will occur if the guest attempts to access a memslot while it
> > +* is being moved.
> > +*/
> > +   run = vcpu_state(vm, VCPU_ID);
> > +   do {
> > +   vcpu_run(vm, VCPU_ID);
> > +   } while (run->exit_reason == KVM_EXIT_MMIO);
> > +
> > +   TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
> > +   "Unexpected exit reason = %d", run->exit_reason);
> 
> 
> This will also not work for s390. Maybe just make this test x86 specific for 
> now?

Doh, that's obvious in hindsight.  I think the basic premise is also
broken on arm64 as it returns -EFAULT on is_error_noslot_pfn(pfn).  So
yeah, x86 only for now :-(
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v4 07/19] KVM: Explicitly free allocated-but-unused dirty bitmap

2019-12-18 Thread Peter Xu
On Tue, Dec 17, 2019 at 02:51:18PM -0800, Sean Christopherson wrote:
> On Tue, Dec 17, 2019 at 05:24:46PM -0500, Peter Xu wrote:
> > On Tue, Dec 17, 2019 at 12:40:29PM -0800, Sean Christopherson wrote:
> > > Explicitly free an allocated-but-unused dirty bitmap instead of relying
> > > on kvm_free_memslot() if an error occurs in __kvm_set_memory_region().
> > > There is no longer a need to abuse kvm_free_memslot() to free arch
> > > specific resources as arch specific code is now called only after the
> > > common flow is guaranteed to succeed.  Arch code can still fail, but
> > > it's responsible for its own cleanup in that case.
> > > 
> > > Eliminating the error path's abuse of kvm_free_memslot() paves the way
> > > for simplifying kvm_free_memslot(), i.e. dropping its @dont param.
> > > 
> > > Signed-off-by: Sean Christopherson 
> > > ---
> > >  virt/kvm/kvm_main.c | 7 ---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > > index d403e93e3028..6b2261a9e139 100644
> > > --- a/virt/kvm/kvm_main.c
> > > +++ b/virt/kvm/kvm_main.c
> > > @@ -1096,7 +1096,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
> > >  
> > >   slots = kvzalloc(sizeof(struct kvm_memslots), GFP_KERNEL_ACCOUNT);
> > >   if (!slots)
> > > - goto out_free;
> > > + goto out_bitmap;
> > >   memcpy(slots, __kvm_memslots(kvm, as_id), sizeof(struct kvm_memslots));
> > >  
> > >   if ((change == KVM_MR_DELETE) || (change == KVM_MR_MOVE)) {
> > > @@ -1144,8 +1144,9 @@ int __kvm_set_memory_region(struct kvm *kvm,
> > >   if (change == KVM_MR_DELETE || change == KVM_MR_MOVE)
> > >   slots = install_new_memslots(kvm, as_id, slots);
> > >   kvfree(slots);
> > > -out_free:
> > > - kvm_free_memslot(kvm, , );
> > > +out_bitmap:
> > > + if (new.dirty_bitmap && !old.dirty_bitmap)
> > > + kvm_destroy_dirty_bitmap();
> > 
> > What if both the old and new have KVM_MEM_LOG_DIRTY_PAGES set?
> > kvm_free_memslot() did cover that but I see that you explicitly
> > dropped it.  Could I ask why?  Thanks,
> 
> In that case, old.dirty_bitmap == new.dirty_bitmap, i.e. shouldn't be freed
> by this error path since doing so would result in a use-after-free via the
> old memslot.
> 
> The kvm_free_memslot() logic is the same, albeit in a very twisted way.

Yes it is. :)

> 
> In __kvm_set_memory_region(), @old and @new start with the same dirty_bitmap.
> 
>   new = old = *slot;
> 
> And @new is modified based on KVM_MEM_LOG_DIRTY_PAGES.  If LOG_DIRTY_PAGES
> is set in both @new and @old, then both the "if" and "else if" evaluate
> false, i.e. new.dirty_bitmap == old.dirty_bitmap.
> 
>   /* Allocate/free page dirty bitmap as needed */
>   if (!(new.flags & KVM_MEM_LOG_DIRTY_PAGES))
>   new.dirty_bitmap = NULL;
>   else if (!new.dirty_bitmap) {
>   r = kvm_create_dirty_bitmap();
>   if (r)
>   return r;
>   }
> 
> Subbing "@free <= @new" and "@dont <= @old" in kvm_free_memslot()
> 
>   static void kvm_free_memslot(struct kvm *kvm, struct kvm_memory_slot *free,
>  struct kvm_memory_slot *dont)
>   {
>   if (!dont || free->dirty_bitmap != dont->dirty_bitmap)
>   kvm_destroy_dirty_bitmap(free);
> 
> 
> yeids this, since @old is obviously non-NULL
> 
>   if (new.dirty_bitmap != old.dirty_bitmap)
>   kvm_destroy_dirty_bitmap();
> 
> The dirty_bitmap allocation logic guarantees that new.dirty_bitmap is
>   a) NULL (the "if" case")
>   b) != old.dirty_bitmap iff old.dirty_bitmap == NULL (the "else if" case)
>   c) == old.dirty_bitmap (the implicit "else" case).
> 
> kvm_free_memslot() frees @new.dirty_bitmap iff its != @old.dirty_bitmap,
> thus the explicit destroy only needs to check for (b).

Thanks for explaining with such a detail.

Reviewed-by: Peter Xu 

-- 
Peter Xu

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2] KVM: arm/arm64: Re-check VMA on detecting a poisoned page

2019-12-18 Thread Christoffer Dall
On Tue, Dec 17, 2019 at 12:38:09PM +, James Morse wrote:
> When we check for a poisoned page, we use the VMA to tell userspace
> about the looming disaster. But we pass a pointer to this VMA
> after having released the mmap_sem, which isn't a good idea.
> 
> Instead, stash the shift value that goes with this pfn while
> we are holding the mmap_sem.
> 
> Reported-by: Marc Zyngier 
> Signed-off-by: James Morse 
> ---
> 
> Based on Marc's patch:
> Link: lore.kernel.org/r/20191211165651.7889-3-...@kernel.org
> 
>  virt/kvm/arm/mmu.c | 20 +---
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> index 38b4c910b6c3..bb0f8d648678 100644
> --- a/virt/kvm/arm/mmu.c
> +++ b/virt/kvm/arm/mmu.c
> @@ -1591,16 +1591,8 @@ static void invalidate_icache_guest_page(kvm_pfn_t 
> pfn, unsigned long size)
>   __invalidate_icache_guest_page(pfn, size);
>  }
>  
> -static void kvm_send_hwpoison_signal(unsigned long address,
> -  struct vm_area_struct *vma)
> +static void kvm_send_hwpoison_signal(unsigned long address, short lsb)
>  {
> - short lsb;
> -
> - if (is_vm_hugetlb_page(vma))
> - lsb = huge_page_shift(hstate_vma(vma));
> - else
> - lsb = PAGE_SHIFT;
> -
>   send_sig_mceerr(BUS_MCEERR_AR, (void __user *)address, lsb, current);
>  }
>  
> @@ -1673,6 +1665,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, 
> phys_addr_t fault_ipa,
>   struct kvm *kvm = vcpu->kvm;
>   struct kvm_mmu_memory_cache *memcache = >arch.mmu_page_cache;
>   struct vm_area_struct *vma;
> + short vma_shift;
>   kvm_pfn_t pfn;
>   pgprot_t mem_type = PAGE_S2;
>   bool logging_active = memslot_is_logging(memslot);
> @@ -1696,7 +1689,12 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, 
> phys_addr_t fault_ipa,
>   return -EFAULT;
>   }
>  
> - vma_pagesize = vma_kernel_pagesize(vma);
> + if (is_vm_hugetlb_page(vma))
> + vma_shift = huge_page_shift(hstate_vma(vma));
> + else
> + vma_shift = PAGE_SHIFT;
> +
> + vma_pagesize = 1ULL << vma_shift;
>   if (logging_active ||
>   !fault_supports_stage2_huge_mapping(memslot, hva, vma_pagesize)) {
>   force_pte = true;
> @@ -1735,7 +1733,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, 
> phys_addr_t fault_ipa,
>  
>   pfn = gfn_to_pfn_prot(kvm, gfn, write_fault, );
>   if (pfn == KVM_PFN_ERR_HWPOISON) {
> - kvm_send_hwpoison_signal(hva, vma);
> + kvm_send_hwpoison_signal(hva, vma_shift);
>   return 0;
>   }
>   if (is_error_noslot_pfn(pfn))
> -- 
> 2.24.0
> 
> 
Reviewed-by: Christoffer Dall 
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH 7/7] KVM: arm/arm64: Elide CMOs when unmapping a range

2019-12-18 Thread Marc Zyngier

Hi James,

On 2019-12-18 15:07, James Morse wrote:

Hi Marc,

On 13/12/2019 18:25, Marc Zyngier wrote:

If userspace issues a munmap() on a set of pages, there is no
expectation that the pages are cleaned to the PoC.


(Pedantry: Clean and invalidate. If the guest wrote through a device
mapping, we ditch any clean+stale lines with this path, meaning 
swapout

saves the correct values)


Indeed.


So let's
not do more work than strictly necessary, and set the magic
flag that avoids CMOs in this case.


I think this assumes the pages went from anonymous->free, so no-one
cares about the contents.

If the pages are backed by a file, won't dirty pages will still get
written back before the page is free? (e.g. EFI flash 'file' mmap()ed 
in)


I believe so. Is that a problem?


What if this isn't the only mapping of the page? Can't it be swapped
out from another VMA? (tenuous example, poor man's memory mirroring?)


Swap-out wouldn't trigger this code path, as it would use a different
MMU notifier event (MMU_NOTIFY_CLEAR vs MMU_NOTIFY_UNMAP), I believe.

Thanks,

M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH 1/3] KVM: arm/arm64: Properly handle faulting of device mappings

2019-12-18 Thread Christoffer Dall
On Mon, Dec 16, 2019 at 10:31:19AM +, Marc Zyngier wrote:
> On 2019-12-13 11:14, Christoffer Dall wrote:
> > On Fri, Dec 13, 2019 at 09:28:59AM +, Marc Zyngier wrote:
> > > Hi Christoffer,
> > > 
> > > On 2019-12-13 08:29, Christoffer Dall wrote:
> > > > Hi Marc,
> > > >
> > > > On Wed, Dec 11, 2019 at 04:56:48PM +, Marc Zyngier wrote:
> > > > > A device mapping is normally always mapped at Stage-2, since
> > > there
> > > > > is very little gain in having it faulted in.
> > > >
> > > > It is actually becoming less clear to me what the real benefits of
> > > > pre-populating the stage 2 page table are, especially given that
> > > we can
> > > > provoke a situation where they're faulted in anyhow.  Do you
> > > recall if
> > > > we had any specific case that motivated us to pre-fault in the
> > > pages?
> > > 
> > > It's only a minor performance optimization that was introduced by
> > > Ard in
> > > 8eef91239e57d. Which makes sense for platform devices that have a
> > > single
> > > fixed location in memory. It makes slightly less sense for PCI,
> > > where
> > > you can move things around.
> > 
> > User space could still decide to move things around in its VA map even
> > if the device is fixed.
> > 
> > Anyway, I was thinking more if there was some sort of device, like a
> > frambuffer, which for example crosses page boundaries and where it would
> > be visible to the user that there's a sudden performance drop while
> > operating the device over page boundaries.  Anything like that?
> > 
> > > 
> > > > > Nonetheless, it is possible to end-up in a situation where the
> > > > > device
> > > > > mapping has been removed from Stage-2 (userspace munmaped the
> > > VFIO
> > > > > region, and the MMU notifier did its job), but present in a
> > > > > userspace
> > > > > mapping (userpace has mapped it back at the same address). In
> > > such
> > > > > a situation, the device mapping will be demand-paged as the
> > > guest
> > > > > performs memory accesses.
> > > > >
> > > > > This requires to be careful when dealing with mapping size,
> > > cache
> > > > > management, and to handle potential execution of a device
> > > mapping.
> > > > >
> > > > > Cc: sta...@vger.kernel.org
> > > > > Reported-by: Alexandru Elisei 
> > > > > Signed-off-by: Marc Zyngier 
> > > > > ---
> > > > >  virt/kvm/arm/mmu.c | 21 +
> > > > >  1 file changed, 17 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> > > > > index a48994af70b8..0b32a904a1bb 100644
> > > > > --- a/virt/kvm/arm/mmu.c
> > > > > +++ b/virt/kvm/arm/mmu.c
> > > > > @@ -38,6 +38,11 @@ static unsigned long io_map_base;
> > > > >  #define KVM_S2PTE_FLAG_IS_IOMAP  (1UL << 0)
> > > > >  #define KVM_S2_FLAG_LOGGING_ACTIVE   (1UL << 1)
> > > > >
> > > > > +static bool is_iomap(unsigned long flags)
> > > > > +{
> > > > > + return flags & KVM_S2PTE_FLAG_IS_IOMAP;
> > > > > +}
> > > > > +
> > > >
> > > > nit: I'm not really sure this indirection makes the code more
> > > readable,
> > > > but I guess that's a matter of taste.
> > > >
> > > > >  static bool memslot_is_logging(struct kvm_memory_slot *memslot)
> > > > >  {
> > > > >   return memslot->dirty_bitmap && !(memslot->flags &
> > > > > KVM_MEM_READONLY);
> > > > > @@ -1698,6 +1703,7 @@ static int user_mem_abort(struct kvm_vcpu
> > > > > *vcpu, phys_addr_t fault_ipa,
> > > > >
> > > > >   vma_pagesize = vma_kernel_pagesize(vma);
> > > > >   if (logging_active ||
> > > > > + (vma->vm_flags & VM_PFNMAP) ||
> > > >
> > > > WHat is actually the rationale for this?
> > > >
> > > > Why is a huge mapping not permitted to device memory?
> > > >
> > > > Are we guaranteed that VM_PFNMAP on the vma results in device
> > > mappings?
> > > > I'm not convinced this is the case, and it would be better if we
> > > can
> > > > stick to a single primitive (either kvm_is_device_pfn, or
> > > VM_PFNMAP) to
> > > > detect device mappings.
> > > 
> > > For now, I've tried to keep the two paths that deal with mapping
> > > devices
> > > (or rather, things that we interpret as devices) as close as
> > > possible.
> > > If we drop the "eager" mapping, then we're at liberty to restructure
> > > this in creative ways.
> > > 
> > > This includes potential huge mappings, but I'm not sure the rest of
> > > the
> > > kernel uses them for devices anyway (I need to find out).
> > > 
> > > > As a subsequent patch, I'd like to make sure that at the very
> > > least our
> > > > memslot prepare function follows the exact same logic for mapping
> > > device
> > > > memory as a fault-in approach does, or that we simply always fault
> > > pages
> > > > in.
> > > 
> > > As far as I can see, the two approach are now identical. Am I
> > > missing
> > > something?
> > > And yes, getting rid of the eager mapping works for me.
> > > 
> > 
> > As far as I can tell, our user_mem_abort() uses gfn_to_pfn_prot() which
> > goes doesn a long trail which 

Re: [PATCH 7/7] KVM: arm/arm64: Elide CMOs when unmapping a range

2019-12-18 Thread James Morse
Hi Marc,

On 13/12/2019 18:25, Marc Zyngier wrote:
> If userspace issues a munmap() on a set of pages, there is no
> expectation that the pages are cleaned to the PoC.

(Pedantry: Clean and invalidate. If the guest wrote through a device mapping, 
we ditch any
clean+stale lines with this path, meaning swapout saves the correct values)


> So let's
> not do more work than strictly necessary, and set the magic
> flag that avoids CMOs in this case.

I think this assumes the pages went from anonymous->free, so no-one cares about 
the contents.

If the pages are backed by a file, won't dirty pages will still get written 
back before
the page is free? (e.g. EFI flash 'file' mmap()ed in)

What if this isn't the only mapping of the page? Can't it be swapped out from 
another VMA?
(tenuous example, poor man's memory mirroring?)


Thanks,

James
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 16/36] irqchip/gic-v4.1: Add mask/unmask doorbell callbacks

2019-12-18 Thread Marc Zyngier

On 2019-11-01 11:23, Zenghui Yu wrote:

Hi Marc,

On 2019/10/27 22:42, Marc Zyngier wrote:
masking/unmasking doorbells on GICv4.1 relies on a new INVDB 
command,

which broadcasts the invalidation to all RDs.
Implement the new command as well as the masking callbacks, and plug
the whole thing into the v4.1 VPE irqchip.
Signed-off-by: Marc Zyngier 


Reviewed-by: Zenghui Yu 


---
  drivers/irqchip/irq-gic-v3-its.c   | 60 
++

  include/linux/irqchip/arm-gic-v3.h |  3 +-
  2 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c 
b/drivers/irqchip/irq-gic-v3-its.c

index dcc7227af5f1..3c34bef70bdd 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -309,6 +309,10 @@ struct its_cmd_desc {
u16 seq_num;
u16 its_list;
} its_vmovp_cmd;
+
+   struct {
+   struct its_vpe *vpe;
+   } its_invdb_cmd;
};
  };
  @@ -750,6 +754,21 @@ static struct its_vpe 
*its_build_vmovp_cmd(struct its_node *its,

return valid_vpe(its, desc->its_vmovp_cmd.vpe);
  }
  +static struct its_vpe *its_build_invdb_cmd(struct its_node *its,
+  struct its_cmd_block *cmd,
+  struct its_cmd_desc *desc)
+{
+   if (WARN_ON(!is_v4_1(its)))
+   return NULL;
+
+   its_encode_cmd(cmd, GITS_CMD_INVDB);
+   its_encode_vpeid(cmd, desc->its_invdb_cmd.vpe->vpe_id);
+
+   its_fixup_cmd(cmd);
+
+   return valid_vpe(its, desc->its_invdb_cmd.vpe);
+}
+
  static u64 its_cmd_ptr_to_offset(struct its_node *its,
 struct its_cmd_block *ptr)
  {
@@ -1117,6 +1136,14 @@ static void its_send_vinvall(struct its_node 
*its, struct its_vpe *vpe)

its_send_single_vcommand(its, its_build_vinvall_cmd, );
  }
  +static void its_send_invdb(struct its_node *its, struct its_vpe 
*vpe)

+{
+   struct its_cmd_desc desc;
+
+   desc.its_invdb_cmd.vpe = vpe;
+   its_send_single_vcommand(its, its_build_invdb_cmd, );
+}
+
  /*
   * irqchip functions - assumes MSI, mostly.
   */
@@ -3408,6 +3435,37 @@ static struct irq_chip its_vpe_irq_chip = {
.irq_set_vcpu_affinity  = its_vpe_set_vcpu_affinity,
  };
  +static void its_vpe_4_1_send_inv(struct irq_data *d)
+{
+   struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+   struct its_node *its;
+
+   /*
+* GICv4.1 wants doorbells to be invalidated using the
+* INVDB command in order to be broadcast to all RDs. Send
+* it to the first valid ITS, and let the HW do its magic.
+*/
+   list_for_each_entry(its, _nodes, entry) {
+   if (!is_v4_1(its))
+   continue;
+
+   its_send_invdb(its, vpe);
+   break;
+   }


Maybe use find_4_1_its() helper instead?


Yes, good point. I've moved it up in the series, and it is now added
to this patch.

Thanks,

M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 22/36] irqchip/gic-v4.1: Advertise support v4.1 to KVM

2019-12-18 Thread Marc Zyngier

On 2019-11-01 12:55, Zenghui Yu wrote:

Hi Marc,

On 2019/10/27 22:42, Marc Zyngier wrote:

Tell KVM that we support v4.1. Nothing uses this information so far.
Signed-off-by: Marc Zyngier 
---
  drivers/irqchip/irq-gic-v3-its.c   | 9 -
  drivers/irqchip/irq-gic-v3.c   | 1 +
  include/linux/irqchip/arm-gic-common.h | 2 ++
  3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c 
b/drivers/irqchip/irq-gic-v3-its.c

index df259e202482..6483f8051b3e 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -4580,6 +4580,7 @@ int __init its_init(struct fwnode_handle 
*handle, struct rdists *rdists,

struct device_node *of_node;
struct its_node *its;
bool has_v4 = false;
+   bool has_v4_1 = false;
int err;

gic_rdists = rdists;
@@ -4600,8 +4601,14 @@ int __init its_init(struct fwnode_handle 
*handle, struct rdists *rdists,

if (err)
return err;
  - list_for_each_entry(its, _nodes, entry)
+   list_for_each_entry(its, _nodes, entry) {
has_v4 |= is_v4(its);
+   has_v4_1 |= is_v4_1(its);
+   }
+
+   /* Don't bother with inconsistent systems */
+   if (WARN_ON(!has_v4_1 && rdists->has_rvpeid))
+   rdists->has_rvpeid = false;

if (has_v4 & rdists->has_vlpis) {
if (its_init_vpe_domain() ||
diff --git a/drivers/irqchip/irq-gic-v3.c 
b/drivers/irqchip/irq-gic-v3.c

index f0d33ac64a99..94dddfb21076 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -1758,6 +1758,7 @@ static void __init 
gic_of_setup_kvm_info(struct device_node *node)

gic_v3_kvm_info.vcpu = r;

gic_v3_kvm_info.has_v4 = gic_data.rdists.has_vlpis;
+   gic_v3_kvm_info.has_v4_1 = gic_data.rdists.has_rvpeid;


Also set gic_v3_kvm_info.has_v4_1 in gic_acpi_setup_kvm_info().


Indeed. Now fixed.

Thanks,

M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 13/36] irqchip/gic-v4.1: Don't use the VPE proxy if RVPEID is set

2019-12-18 Thread Marc Zyngier

On 2019-11-01 11:05, Zenghui Yu wrote:

Hi Marc,

On 2019/10/27 22:42, Marc Zyngier wrote:

The infamous VPE proxy device isn't used with GICv4.1 because:
- we can invalidate any LPI from the DirectLPI MMIO interface
- the ITS and redistributors understand the life cycle of
   the doorbell, so we don't need to enable/disable it all
   the time
So let's escape early from the proxy related functions.
Signed-off-by: Marc Zyngier 


Reviewed-by: Zenghui Yu 


---
  drivers/irqchip/irq-gic-v3-its.c | 23 ++-
  1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c 
b/drivers/irqchip/irq-gic-v3-its.c

index 220d490d516e..999e61a9b2c3 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3069,7 +3069,7 @@ static const struct irq_domain_ops 
its_domain_ops = {

  /*
   * This is insane.
   *
- * If a GICv4 doesn't implement Direct LPIs (which is extremely
+ * If a GICv4.0 doesn't implement Direct LPIs (which is extremely
   * likely), the only way to perform an invalidate is to use a fake
   * device to issue an INV command, implying that the LPI has first
   * been mapped to some event on that device. Since this is not 
exactly
@@ -3077,9 +3077,18 @@ static const struct irq_domain_ops 
its_domain_ops = {

   * only issue an UNMAP if we're short on available slots.
   *
   * Broken by design(tm).
+ *
+ * GICv4.1 actually mandates that we're able to invalidate by 
writing to a
+ * MMIO register. It doesn't implement the whole of DirectLPI, but 
that's
+ * good enough. And most of the time, we don't even have to 
invalidate

+ * anything, so that's actually pretty good!


I can't understand the meaning of this last sentence. May I ask for 
an

explanation? :)


Yeah, reading this now, it feels pretty clumsy, and only remotely
connected to the patch.

What I'm trying to say here is that, although GICv4.1 doesn't have
the full spectrum of v4.0 DirectLPI (it only allows a subset of it),
this subset is more then enough for us. Here's the rational:

When a vPE exits from the hypervisor, we know whether we need to
request a doorbell or not (depending on whether we're blocking on
WFI or not). On GICv4.0, this translates into enabling the doorbell
interrupt, which generates an invalidation (costly). And whenever
we've taken a doorbell, or are scheduled again, we need to turn
the doorbell off (invalidation again).

With v4.1, we can just say *at exit time* whether we want doorbells
to be subsequently generated (see its_vpe_4_1_deschedule() and the
req_db parameter in the info structure). This is part of making
the vPE non-resident, so we have 0 overhead at this stage.

I'll try and update the comment here.

Thanks,

M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v2 19/36] irqchip/gic-v4.1: Add VPE INVALL callback

2019-12-18 Thread Marc Zyngier

[Old email, doing some v3 cleanup]

On 2019-11-01 11:51, Zenghui Yu wrote:

Hi Marc,

On 2019/10/27 22:42, Marc Zyngier wrote:

GICv4.1 redistributors have a VPE-aware INVALL register. Progress!
We can now emulate a guest-requested INVALL without emiting a
VINVALL command.
Signed-off-by: Marc Zyngier 


Reviewed-by: Zenghui Yu 


---
  drivers/irqchip/irq-gic-v3-its.c   | 14 ++
  include/linux/irqchip/arm-gic-v3.h |  3 +++
  2 files changed, 17 insertions(+)
diff --git a/drivers/irqchip/irq-gic-v3-its.c 
b/drivers/irqchip/irq-gic-v3-its.c

index f7effd453729..10bd156aa042 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3511,6 +3511,19 @@ static void its_vpe_4_1_deschedule(struct 
its_vpe *vpe,

}
  }
  +static void its_vpe_4_1_invall(struct its_vpe *vpe)
+{
+   void __iomem *rdbase;
+   u64 val;
+
+   val  = GICR_INVLPIR_V;
+   val |= FIELD_PREP(GICR_INVLPIR_VPEID, vpe->vpe_id);


Can we use GICR_INVALL_V/VPEID instead, and ...


+
+   /* Target the redistributor this vPE is currently known on */
+   rdbase = per_cpu_ptr(gic_rdists->rdist, vpe->col_idx)->rd_base;
+   gic_write_lpir(val, rdbase + GICR_INVALLR);
+}
+
  static int its_vpe_4_1_set_vcpu_affinity(struct irq_data *d, void 
*vcpu_info)

  {
struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
@@ -3526,6 +3539,7 @@ static int 
its_vpe_4_1_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)

return 0;

case INVALL_VPE:
+   its_vpe_4_1_invall(vpe);
return 0;

default:
diff --git a/include/linux/irqchip/arm-gic-v3.h 
b/include/linux/irqchip/arm-gic-v3.h

index 6fd89d77b2b2..b69f60792554 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -247,6 +247,9 @@
  #define GICR_TYPER_COMMON_LPI_AFF GENMASK_ULL(25, 24)
  #define GICR_TYPER_AFFINITY   GENMASK_ULL(63, 32)
  +#define GICR_INVLPIR_VPEID   GENMASK_ULL(47, 32)
+#define GICR_INVLPIR_V GENMASK_ULL(63, 63)
+


... define them here:

#define GICR_INVALL_VPEID   GICR_INVLPIR_VPEID
#define GICR_INVALL_V   GICR_INVLPIR_V


Yes, that's a sensible things to do. I'll squash that in my rebased 
series.


Thanks,

M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v4 00/19] KVM: Dynamically size memslot arrays

2019-12-18 Thread Christian Borntraeger



On 17.12.19 21:40, Sean Christopherson wrote:
> The end goal of this series is to dynamically size the memslot array so
> that KVM allocates memory based on the number of memslots in use, as
> opposed to unconditionally allocating memory for the maximum number of
> memslots.  On x86, each memslot consumes 88 bytes, and so with 2 address
> spaces of 512 memslots, each VM consumes ~90k bytes for the memslots.
> E.g. given a VM that uses a total of 30 memslots, dynamic sizing reduces
> the memory footprint from 90k to ~2.6k bytes.
> 
> The changes required to support dynamic sizing are relatively small,
> e.g. are essentially contained in patches 17/19 and 18/19.
> 
> Patches 2-16 clean up the memslot code, which has gotten quite crusty,
> especially __kvm_set_memory_region().  The clean up is likely not strictly
> necessary to switch to dynamic sizing, but I didn't have a remotely
> reasonable level of confidence in the correctness of the dynamic sizing
> without first doing the clean up.
> 
> The only functional change in v4 is the addition of an x86-specific bug
> fix in x86's handling of KVM_MR_MOVE.  The bug fix is not directly related
> to dynamically allocating memslots, but it has subtle and hidden conflicts
> with the cleanup patches, and the fix is higher priority than anything
> else in the series, i.e. should be merged first.
> 
> On non-x86 architectures, v3 and v4 should be functionally equivalent,
> the only non-x86 change in v4 is the dropping of a "const" in
> kvm_arch_commit_memory_region().

I gave this series a quick spin and it still seems to work on s390 (minus the 
selftest).

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH v4 19/19] KVM: selftests: Add test for KVM_SET_USER_MEMORY_REGION

2019-12-18 Thread Christian Borntraeger



On 17.12.19 21:40, Sean Christopherson wrote:
> Add a KVM selftest to test moving the base gfn of a userspace memory
> region.  The test is primarily targeted at x86 to verify its memslot
> metadata is correctly updated, but also provides basic functionality
> coverage on other architectures.
> 
> Signed-off-by: Sean Christopherson 
> ---
>  tools/testing/selftests/kvm/.gitignore|   1 +
>  tools/testing/selftests/kvm/Makefile  |   3 +
>  .../testing/selftests/kvm/include/kvm_util.h  |   1 +
>  tools/testing/selftests/kvm/lib/kvm_util.c|  30 
>  .../selftests/kvm/set_memory_region_test.c| 142 ++
>  5 files changed, 177 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/set_memory_region_test.c
> 
> diff --git a/tools/testing/selftests/kvm/.gitignore 
> b/tools/testing/selftests/kvm/.gitignore
> index 30072c3f52fb..6f60ceb81440 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -17,3 +17,4 @@
>  /clear_dirty_log_test
>  /dirty_log_test
>  /kvm_create_max_vcpus
> +/set_memory_region_test
> diff --git a/tools/testing/selftests/kvm/Makefile 
> b/tools/testing/selftests/kvm/Makefile
> index 3138a916574a..01c79e02c5b7 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -29,15 +29,18 @@ TEST_GEN_PROGS_x86_64 += x86_64/xss_msr_test
>  TEST_GEN_PROGS_x86_64 += clear_dirty_log_test
>  TEST_GEN_PROGS_x86_64 += dirty_log_test
>  TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
> +TEST_GEN_PROGS_x86_64 += set_memory_region_test
> 
>  TEST_GEN_PROGS_aarch64 += clear_dirty_log_test
>  TEST_GEN_PROGS_aarch64 += dirty_log_test
>  TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
> +TEST_GEN_PROGS_aarch64 += set_memory_region_test
> 
>  TEST_GEN_PROGS_s390x = s390x/memop
>  TEST_GEN_PROGS_s390x += s390x/sync_regs_test
>  TEST_GEN_PROGS_s390x += dirty_log_test
>  TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
> +TEST_GEN_PROGS_s390x += set_memory_region_test
> 
>  TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
>  LIBKVM += $(LIBKVM_$(UNAME_M))
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h 
> b/tools/testing/selftests/kvm/include/kvm_util.h
> index 29cccaf96baf..15d3b8690ffb 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -100,6 +100,7 @@ int _vcpu_ioctl(struct kvm_vm *vm, uint32_t vcpuid, 
> unsigned long ioctl,
>   void *arg);
>  void vm_ioctl(struct kvm_vm *vm, unsigned long ioctl, void *arg);
>  void vm_mem_region_set_flags(struct kvm_vm *vm, uint32_t slot, uint32_t 
> flags);
> +void vm_mem_region_move(struct kvm_vm *vm, uint32_t slot, uint64_t new_gpa);
>  void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid);
>  vm_vaddr_t vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min,
> uint32_t data_memslot, uint32_t pgd_memslot);
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c 
> b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 41cf45416060..464a75ce9843 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -756,6 +756,36 @@ void vm_mem_region_set_flags(struct kvm_vm *vm, uint32_t 
> slot, uint32_t flags)
>   ret, errno, slot, flags);
>  }
> 
> +/*
> + * VM Memory Region Move
> + *
> + * Input Args:
> + *   vm - Virtual Machine
> + *   slot - Slot of the memory region to move
> + *   flags - Starting guest physical address
> + *
> + * Output Args: None
> + *
> + * Return: None
> + *
> + * Change the gpa of a memory region.
> + */
> +void vm_mem_region_move(struct kvm_vm *vm, uint32_t slot, uint64_t new_gpa)
> +{
> + struct userspace_mem_region *region;
> + int ret;
> +
> + region = memslot2region(vm, slot);
> +
> + region->region.guest_phys_addr = new_gpa;
> +
> + ret = ioctl(vm->fd, KVM_SET_USER_MEMORY_REGION, >region);
> +
> + TEST_ASSERT(!ret, "KVM_SET_USER_MEMORY_REGION failed\n"
> + "ret: %i errno: %i slot: %u flags: 0x%x",
> + ret, errno, slot, new_gpa);
> +}
> +
>  /*
>   * VCPU mmap Size
>   *
> diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c 
> b/tools/testing/selftests/kvm/set_memory_region_test.c
> new file mode 100644
> index ..c9603b95ccf7
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/set_memory_region_test.c
> @@ -0,0 +1,142 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE /* for program_invocation_short_name */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#define VCPU_ID 0
> +
> +/*
> + * Somewhat arbitrary location and slot, intended to not overlap anything.  
> The
> + * location and size are specifically 2mb sized/aligned so that the initial
> + * region corresponds to exactly one large page (on x86 and 

Re: [PATCH v4 19/19] KVM: selftests: Add test for KVM_SET_USER_MEMORY_REGION

2019-12-18 Thread Christian Borntraeger


On 17.12.19 21:40, Sean Christopherson wrote:
> Add a KVM selftest to test moving the base gfn of a userspace memory
> region.  The test is primarily targeted at x86 to verify its memslot
> metadata is correctly updated, but also provides basic functionality
> coverage on other architectures.

This fails to build on s390 (and probably others)

set_memory_region_test.c: In function ‘test_move_memory_region’:
set_memory_region_test.c:78:2: warning: implicit declaration of function 
‘vcpu_set_cpuid’ [-Wimplicit-function-declaration]
   78 |  vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
  |  ^~
set_memory_region_test.c:78:30: warning: implicit declaration of function 
‘kvm_get_supported_cpuid’ [-Wimplicit-function-declaration]
   78 |  vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
  |  ^~~


> 
> Signed-off-by: Sean Christopherson 
> ---
>  tools/testing/selftests/kvm/.gitignore|   1 +
>  tools/testing/selftests/kvm/Makefile  |   3 +
>  .../testing/selftests/kvm/include/kvm_util.h  |   1 +
>  tools/testing/selftests/kvm/lib/kvm_util.c|  30 
>  .../selftests/kvm/set_memory_region_test.c| 142 ++
>  5 files changed, 177 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/set_memory_region_test.c
> 
> diff --git a/tools/testing/selftests/kvm/.gitignore 
> b/tools/testing/selftests/kvm/.gitignore
> index 30072c3f52fb..6f60ceb81440 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -17,3 +17,4 @@
>  /clear_dirty_log_test
>  /dirty_log_test
>  /kvm_create_max_vcpus
> +/set_memory_region_test
> diff --git a/tools/testing/selftests/kvm/Makefile 
> b/tools/testing/selftests/kvm/Makefile
> index 3138a916574a..01c79e02c5b7 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -29,15 +29,18 @@ TEST_GEN_PROGS_x86_64 += x86_64/xss_msr_test
>  TEST_GEN_PROGS_x86_64 += clear_dirty_log_test
>  TEST_GEN_PROGS_x86_64 += dirty_log_test
>  TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
> +TEST_GEN_PROGS_x86_64 += set_memory_region_test
>  
>  TEST_GEN_PROGS_aarch64 += clear_dirty_log_test
>  TEST_GEN_PROGS_aarch64 += dirty_log_test
>  TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
> +TEST_GEN_PROGS_aarch64 += set_memory_region_test
>  
>  TEST_GEN_PROGS_s390x = s390x/memop
>  TEST_GEN_PROGS_s390x += s390x/sync_regs_test
>  TEST_GEN_PROGS_s390x += dirty_log_test
>  TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
> +TEST_GEN_PROGS_s390x += set_memory_region_test
>  
>  TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
>  LIBKVM += $(LIBKVM_$(UNAME_M))
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h 
> b/tools/testing/selftests/kvm/include/kvm_util.h
> index 29cccaf96baf..15d3b8690ffb 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -100,6 +100,7 @@ int _vcpu_ioctl(struct kvm_vm *vm, uint32_t vcpuid, 
> unsigned long ioctl,
>   void *arg);
>  void vm_ioctl(struct kvm_vm *vm, unsigned long ioctl, void *arg);
>  void vm_mem_region_set_flags(struct kvm_vm *vm, uint32_t slot, uint32_t 
> flags);
> +void vm_mem_region_move(struct kvm_vm *vm, uint32_t slot, uint64_t new_gpa);
>  void vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpuid);
>  vm_vaddr_t vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min,
> uint32_t data_memslot, uint32_t pgd_memslot);
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c 
> b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 41cf45416060..464a75ce9843 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -756,6 +756,36 @@ void vm_mem_region_set_flags(struct kvm_vm *vm, uint32_t 
> slot, uint32_t flags)
>   ret, errno, slot, flags);
>  }
>  
> +/*
> + * VM Memory Region Move
> + *
> + * Input Args:
> + *   vm - Virtual Machine
> + *   slot - Slot of the memory region to move
> + *   flags - Starting guest physical address
> + *
> + * Output Args: None
> + *
> + * Return: None
> + *
> + * Change the gpa of a memory region.
> + */
> +void vm_mem_region_move(struct kvm_vm *vm, uint32_t slot, uint64_t new_gpa)
> +{
> + struct userspace_mem_region *region;
> + int ret;
> +
> + region = memslot2region(vm, slot);
> +
> + region->region.guest_phys_addr = new_gpa;
> +
> + ret = ioctl(vm->fd, KVM_SET_USER_MEMORY_REGION, >region);
> +
> + TEST_ASSERT(!ret, "KVM_SET_USER_MEMORY_REGION failed\n"
> + "ret: %i errno: %i slot: %u flags: 0x%x",
> + ret, errno, slot, new_gpa);
> +}
> +
>  /*
>   * VCPU mmap Size
>   *
> diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c 
> b/tools/testing/selftests/kvm/set_memory_region_test.c
> new file mode 100644
> index ..c9603b95ccf7
> --- /dev/null
> +++ 

Re: [kvm-unit-tests PATCH 05/16] arm/arm64: ITS: Introspection tests

2019-12-18 Thread Auger Eric
Hi Zenghui,

On 12/18/19 4:46 AM, Zenghui Yu wrote:
> Hi Eric,
> 
> I have to admit that this is the first time I've looked into
> the kvm-unit-tests code, so only some minor comments inline :)

no problem. Thank you for looking at this.

By the way, with patch 16 I was able to test yout fix: "KVM: arm/arm64:
vgic: Don't rely on the wrong pending table". Reverting it produced an
error. I forgot to mention that.
> 
> On 2019/12/16 22:02, Eric Auger wrote:
>> Detect the presence of an ITS as part of the GICv3 init
>> routine, initialize its base address and read few registers
>> the IIDR, the TYPER to store its dimensioning parameters.
>>
>> This is our first ITS test, belonging to a new "its" group.
>>
>> Signed-off-by: Eric Auger 
> 
> [...]
> 
>> diff --git a/lib/arm/asm/gic-v3-its.h b/lib/arm/asm/gic-v3-its.h
>> new file mode 100644
>> index 000..2ce483e
>> --- /dev/null
>> +++ b/lib/arm/asm/gic-v3-its.h
>> @@ -0,0 +1,116 @@
>> +/*
>> + * All ITS* defines are lifted from include/linux/irqchip/arm-gic-v3.h
>> + *
>> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones 
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.
>> + */
>> +#ifndef _ASMARM_GIC_V3_ITS_H_
>> +#define _ASMARM_GIC_V3_ITS_H_
>> +
>> +#ifndef __ASSEMBLY__
>> +
>> +#define GITS_CTLR    0x
>> +#define GITS_IIDR    0x0004
>> +#define GITS_TYPER    0x0008
>> +#define GITS_CBASER    0x0080
>> +#define GITS_CWRITER    0x0088
>> +#define GITS_CREADR    0x0090
>> +#define GITS_BASER    0x0100
>> +
>> +#define GITS_TYPER_PLPIS    (1UL << 0)
>> +#define GITS_TYPER_IDBITS_SHIFT 8
>> +#define GITS_TYPER_DEVBITS_SHIFT    13
>> +#define GITS_TYPER_DEVBITS(r)   r) >>
>> GITS_TYPER_DEVBITS_SHIFT) & 0x1f) + 1)
>> +#define GITS_TYPER_PTA  (1UL << 19)
>> +#define GITS_TYPER_HWCOLLCNT_SHIFT  24
>> +
>> +#define GITS_CTLR_ENABLE    (1U << 0)
>> +
>> +#define GITS_CBASER_VALID   (1UL << 63)
>> +#define GITS_CBASER_SHAREABILITY_SHIFT  (10)
>> +#define GITS_CBASER_INNER_CACHEABILITY_SHIFT    (59)
>> +#define GITS_CBASER_OUTER_CACHEABILITY_SHIFT    (53)
>> +#define
>> GITS_CBASER_SHAREABILITY_MASK   \
>> +    GIC_BASER_SHAREABILITY(GITS_CBASER, SHAREABILITY_MASK)
>> +#define
>> GITS_CBASER_INNER_CACHEABILITY_MASK \
>> +    GIC_BASER_CACHEABILITY(GITS_CBASER, INNER, MASK)
>> +#define
>> GITS_CBASER_OUTER_CACHEABILITY_MASK \
>> +    GIC_BASER_CACHEABILITY(GITS_CBASER, OUTER, MASK)
>> +#define GITS_CBASER_CACHEABILITY_MASK
>> GITS_CBASER_INNER_CACHEABILITY_MASK
>> +
>> +#define
>> GITS_CBASER_InnerShareable  \
>> +    GIC_BASER_SHAREABILITY(GITS_CBASER, InnerShareable)
>> +
>> +#define GITS_CBASER_nCnB    GIC_BASER_CACHEABILITY(GITS_CBASER,
>> INNER, nCnB)
>> +#define GITS_CBASER_nC  GIC_BASER_CACHEABILITY(GITS_CBASER,
>> INNER, nC)
>> +#define GITS_CBASER_RaWt    GIC_BASER_CACHEABILITY(GITS_CBASER,
>> INNER, RaWt)
>> +#define GITS_CBASER_RaWb    GIC_BASER_CACHEABILITY(GITS_CBASER,
>> INNER, RaWt)
> 
> s/RaWt/RaWb/
OK
> 
>> +#define GITS_CBASER_WaWt    GIC_BASER_CACHEABILITY(GITS_CBASER,
>> INNER, WaWt)
>> +#define GITS_CBASER_WaWb    GIC_BASER_CACHEABILITY(GITS_CBASER,
>> INNER, WaWb)
>> +#define GITS_CBASER_RaWaWt  GIC_BASER_CACHEABILITY(GITS_CBASER,
>> INNER, RaWaWt)
>> +#define GITS_CBASER_RaWaWb  GIC_BASER_CACHEABILITY(GITS_CBASER,
>> INNER, RaWaWb)
>> +
>> +#define GITS_BASER_NR_REGS  8
>> +
>> +#define GITS_BASER_VALID    (1UL << 63)
>> +#define GITS_BASER_INDIRECT (1ULL << 62)
>> +
>> +#define GITS_BASER_INNER_CACHEABILITY_SHIFT (59)
>> +#define GITS_BASER_OUTER_CACHEABILITY_SHIFT (53)
>> +#define GITS_BASER_CACHEABILITY_MASK    0x7
>> +
>> +#define GITS_BASER_nCnB GIC_BASER_CACHEABILITY(GITS_BASER,
>> INNER, nCnB)
>> +
>> +#define GITS_BASER_TYPE_SHIFT   (56)
>> +#define GITS_BASER_TYPE(r)  (((r) >>
>> GITS_BASER_TYPE_SHIFT) & 7)
>> +#define GITS_BASER_ENTRY_SIZE_SHIFT (48)
>> +#define GITS_BASER_ENTRY_SIZE(r)    r) >>
>> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f) + 1)
>> +#define GITS_BASER_SHAREABILITY_SHIFT   (10)
>> +#define
>> GITS_BASER_InnerShareable   \
>> +    GIC_BASER_SHAREABILITY(GITS_BASER, InnerShareable)
>> +#define GITS_BASER_PAGE_SIZE_SHIFT  (8)
>> +#define GITS_BASER_PAGE_SIZE_4K (0UL <<
>> GITS_BASER_PAGE_SIZE_SHIFT)
>> +#define GITS_BASER_PAGE_SIZE_16K    (1UL <<
>> GITS_BASER_PAGE_SIZE_SHIFT)
>> +#define GITS_BASER_PAGE_SIZE_64K    (2UL <<
>> GITS_BASER_PAGE_SIZE_SHIFT)
>> +#define GITS_BASER_PAGE_SIZE_MASK   (3UL <<
>> GITS_BASER_PAGE_SIZE_SHIFT)
>> +#define GITS_BASER_PAGES_MAX    256
>> +#define