date:20151119

From: David Hildenbrand 

Let's reuse the new common function for VPCU lookup by id.

Reviewed-by: Christian Borntraeger 
Reviewed-by: Dominik Dingel 
Signed-off-by: David Hildenbrand 
Signed-off-by: Christian Borntraeger 
[split out the new function into a separate patch]
---
 arch/powerpc/kvm/book3s_hv.c | 10 ++
 arch/s390/kvm/diag.c | 11 +++
 virt/kvm/kvm_main.c  | 12 +---
 3 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 54b45b7..904b3b0 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -308,16 +308,10 @@ static void kvmppc_dump_regs(struct kvm_vcpu *vcpu)
 
 static struct kvm_vcpu *kvmppc_find_vcpu(struct kvm *kvm, int id)
 {
-   int r;
-   struct kvm_vcpu *v, *ret = NULL;
+   struct kvm_vcpu *ret;
 
mutex_lock(>lock);
-   kvm_for_each_vcpu(r, v, kvm) {
-   if (v->vcpu_id == id) {
-   ret = v;
-   break;
-   }
-   }
+   ret = kvm_lookup_vcpu(kvm, id);
mutex_unlock(>lock);
return ret;
 }
diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
index 5fbfb88..aaa7cc0 100644
--- a/arch/s390/kvm/diag.c
+++ b/arch/s390/kvm/diag.c
@@ -155,10 +155,8 @@ static int __diag_time_slice_end(struct kvm_vcpu *vcpu)
 
 static int __diag_time_slice_end_directed(struct kvm_vcpu *vcpu)
 {
-   struct kvm *kvm = vcpu->kvm;
struct kvm_vcpu *tcpu;
int tid;
-   int i;
 
tid = vcpu->run->s.regs.gprs[(vcpu->arch.sie_block->ipa & 0xf0) >> 4];
vcpu->stat.diagnose_9c++;
@@ -167,12 +165,9 @@ static int __diag_time_slice_end_directed(struct kvm_vcpu 
*vcpu)
if (tid == vcpu->vcpu_id)
return 0;
 
-   kvm_for_each_vcpu(i, tcpu, kvm)
-   if (tcpu->vcpu_id == tid) {
-   kvm_vcpu_yield_to(tcpu);
-   break;
-   }
-
+   tcpu = kvm_lookup_vcpu(vcpu->kvm, tid);
+   if (tcpu)
+   kvm_vcpu_yield_to(tcpu);
return 0;
 }
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 484079e..244c9b4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2257,7 +2257,7 @@ static int create_vcpu_fd(struct kvm_vcpu *vcpu)
 static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
 {
int r;
-   struct kvm_vcpu *vcpu, *v;
+   struct kvm_vcpu *vcpu;
 
if (id >= KVM_MAX_VCPUS)
return -EINVAL;
@@ -2281,12 +2281,10 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, 
u32 id)
r = -EINVAL;
goto unlock_vcpu_destroy;
}
-
-   kvm_for_each_vcpu(r, v, kvm)
-   if (v->vcpu_id == id) {
-   r = -EEXIST;
-   goto unlock_vcpu_destroy;
-   }
+   if (kvm_lookup_vcpu(kvm, id)) {
+   r = -EEXIST;
+   goto unlock_vcpu_destroy;
+   }
 
BUG_ON(kvm->vcpus[atomic_read(>online_vcpus)]);
 
-- 
2.3.0

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

Re: [PATCH v8 0/5] implement vNVDIMM

On Thu, Nov 19, 2015 at 10:39:05AM +0800, Xiao Guangrong wrote:
> 
> 
> On 11/19/2015 04:44 AM, Michael S. Tsirkin wrote:
> >On Wed, Nov 18, 2015 at 05:18:17PM -0200, Eduardo Habkost wrote:
> >>On Wed, Nov 18, 2015 at 09:59:34AM +0800, Xiao Guangrong wrote:
> >>>
> >>>Ping...
> >>>
> >>>Do you have any comment on this patchset? Could it be applied to somewhere
> >>>if it is okay for you?
> >>
> >>I have no additional comments, as the memory-backend patches I
> >>was reviewing are not included in this version. I didn't take the
> >>time to review the TYPE_NVDIMM and ACPI changes.
> >
> >No, I don't think the way guest memory is allocated here is ok.  I'm
> 
> Since the DSM memory/ACPI memory was not included in this patchset, i really
> do not understand what is "guest memory is allocated" exactly stands for...

I might even be confusing this with another patchset.
Let's have this discussion when I have the time to review
and respond properly.

> >sorry, I'm busy with 2.5 now, and this is clearly not 2.5 material.
> 
> I still see some pull requests were send our for 2.5 merge window today and
> yesterday ...
> 
> This patchset is the simplest version we can figure out to implement basic
> functionality for vNVDIMM and only minor change is needed for other code.
> It would be nice and really appreciate if it can go to 2.5.

Sorry, no way, we are in a bugfix only mode for 2.5.

> >Once that's out, I'll post some suggestions.
> 
> Look forward to you suggestions.
> 
> Thanks for your time, Michael and Eduardo!
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] KVM: s390: fix wrong lookup of VCPUs by array index

From: David Hildenbrand 

For now, VCPUs were always created sequentially with incrementing
VCPU ids. Therefore, the index in the VCPUs array matched the id.

As sequential creation might change with cpu hotplug, let's use
the correct lookup function to find a VCPU by id, not array index.

Reviewed-by: Christian Borntraeger 
Signed-off-by: David Hildenbrand 
Signed-off-by: Christian Borntraeger 
[split stable/non-stable parts]
Cc: sta...@vger.kernel.org # c3853a8: KVM: Provide function for VCPU lookup by 
id
---
 arch/s390/kvm/sigp.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/s390/kvm/sigp.c b/arch/s390/kvm/sigp.c
index da690b6..081dbd0 100644
--- a/arch/s390/kvm/sigp.c
+++ b/arch/s390/kvm/sigp.c
@@ -291,12 +291,8 @@ static int handle_sigp_dst(struct kvm_vcpu *vcpu, u8 
order_code,
   u16 cpu_addr, u32 parameter, u64 *status_reg)
 {
int rc;
-   struct kvm_vcpu *dst_vcpu;
+   struct kvm_vcpu *dst_vcpu = kvm_lookup_vcpu(vcpu->kvm, cpu_addr);
 
-   if (cpu_addr >= KVM_MAX_VCPUS)
-   return SIGP_CC_NOT_OPERATIONAL;
-
-   dst_vcpu = kvm_get_vcpu(vcpu->kvm, cpu_addr);
if (!dst_vcpu)
return SIGP_CC_NOT_OPERATIONAL;
 
@@ -478,7 +474,7 @@ int kvm_s390_handle_sigp_pei(struct kvm_vcpu *vcpu)
trace_kvm_s390_handle_sigp_pei(vcpu, order_code, cpu_addr);
 
if (order_code == SIGP_EXTERNAL_CALL) {
-   dest_vcpu = kvm_get_vcpu(vcpu->kvm, cpu_addr);
+   dest_vcpu = kvm_lookup_vcpu(vcpu->kvm, cpu_addr);
BUG_ON(dest_vcpu == NULL);
 
kvm_s390_vcpu_wakeup(dest_vcpu);
-- 
2.3.0

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

[PATCH 3/4] KVM: use heuristic for fast VCPU lookup by id

From: David Hildenbrand 

Usually, VCPU ids match the array index. So let's try a fast
lookup first before falling back to the slow iteration.

Suggested-by: Christian Borntraeger 
Reviewed-by: Dominik Dingel 
Reviewed-by: Christian Borntraeger 
Signed-off-by: David Hildenbrand 
Signed-off-by: Christian Borntraeger 
---
 include/linux/kvm_host.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index b9f345f..7821137 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -465,6 +465,11 @@ static inline struct kvm_vcpu *kvm_lookup_vcpu(struct kvm 
*kvm, int id)
struct kvm_vcpu *vcpu;
int i;
 
+   if (id < 0 || id >= KVM_MAX_VCPUS)
+   return NULL;
+   vcpu = kvm_get_vcpu(kvm, id);
+   if (vcpu && vcpu->vcpu_id == id)
+   return vcpu;
kvm_for_each_vcpu(i, vcpu, kvm)
if (vcpu->vcpu_id == id)
return vcpu;
-- 
2.3.0

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

[PATCH 1/4] KVM: Provide function for VCPU lookup by id

From: David Hildenbrand 

Let's provide a function to lookup a VCPU by id.

Reviewed-by: Christian Borntraeger 
Reviewed-by: Dominik Dingel 
Signed-off-by: David Hildenbrand 
Signed-off-by: Christian Borntraeger 
[split patch from refactoring patch]
---
 include/linux/kvm_host.h | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 5706a21..b9f345f 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -460,6 +460,17 @@ static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm 
*kvm, int i)
 (vcpup = kvm_get_vcpu(kvm, idx)) != NULL; \
 idx++)
 
+static inline struct kvm_vcpu *kvm_lookup_vcpu(struct kvm *kvm, int id)
+{
+   struct kvm_vcpu *vcpu;
+   int i;
+
+   kvm_for_each_vcpu(i, vcpu, kvm)
+   if (vcpu->vcpu_id == id)
+   return vcpu;
+   return NULL;
+}
+
 #define kvm_for_each_memslot(memslot, slots)   \
for (memslot = >memslots[0]; \
  memslot < slots->memslots + KVM_MEM_SLOTS_NUM && memslot->npages;\
-- 
2.3.0

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

[PATCH 2/4] KVM: s390: fix wrong lookup of VCPUs by array index

From: David Hildenbrand 

For now, VCPUs were always created sequentially with incrementing
VCPU ids. Therefore, the index in the VCPUs array matched the id.

As sequential creation might change with cpu hotplug, let's use
the correct lookup function to find a VCPU by id, not array index.

Reviewed-by: Christian Borntraeger 
Signed-off-by: David Hildenbrand 
Signed-off-by: Christian Borntraeger 
[split stable/non-stable parts]
Cc: sta...@vger.kernel.org # c3853a8: KVM: Provide function for VCPU lookup by 
id
---
 arch/s390/kvm/sigp.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/s390/kvm/sigp.c b/arch/s390/kvm/sigp.c
index da690b6..081dbd0 100644
--- a/arch/s390/kvm/sigp.c
+++ b/arch/s390/kvm/sigp.c
@@ -291,12 +291,8 @@ static int handle_sigp_dst(struct kvm_vcpu *vcpu, u8 
order_code,
   u16 cpu_addr, u32 parameter, u64 *status_reg)
 {
int rc;
-   struct kvm_vcpu *dst_vcpu;
+   struct kvm_vcpu *dst_vcpu = kvm_lookup_vcpu(vcpu->kvm, cpu_addr);
 
-   if (cpu_addr >= KVM_MAX_VCPUS)
-   return SIGP_CC_NOT_OPERATIONAL;
-
-   dst_vcpu = kvm_get_vcpu(vcpu->kvm, cpu_addr);
if (!dst_vcpu)
return SIGP_CC_NOT_OPERATIONAL;
 
@@ -478,7 +474,7 @@ int kvm_s390_handle_sigp_pei(struct kvm_vcpu *vcpu)
trace_kvm_s390_handle_sigp_pei(vcpu, order_code, cpu_addr);
 
if (order_code == SIGP_EXTERNAL_CALL) {
-   dest_vcpu = kvm_get_vcpu(vcpu->kvm, cpu_addr);
+   dest_vcpu = kvm_lookup_vcpu(vcpu->kvm, cpu_addr);
BUG_ON(dest_vcpu == NULL);
 
kvm_s390_vcpu_wakeup(dest_vcpu);
-- 
2.3.0

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

[PATCH 1/4] KVM: Provide function for VCPU lookup by id

From: David Hildenbrand 

Let's provide a function to lookup a VCPU by id.

Reviewed-by: Christian Borntraeger 
Reviewed-by: Dominik Dingel 
Signed-off-by: David Hildenbrand 
Signed-off-by: Christian Borntraeger 
[split patch from refactoring patch]
---
 include/linux/kvm_host.h | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 5706a21..b9f345f 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -460,6 +460,17 @@ static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm 
*kvm, int i)
 (vcpup = kvm_get_vcpu(kvm, idx)) != NULL; \
 idx++)
 
+static inline struct kvm_vcpu *kvm_lookup_vcpu(struct kvm *kvm, int id)
+{
+   struct kvm_vcpu *vcpu;
+   int i;
+
+   kvm_for_each_vcpu(i, vcpu, kvm)
+   if (vcpu->vcpu_id == id)
+   return vcpu;
+   return NULL;
+}
+
 #define kvm_for_each_memslot(memslot, slots)   \
for (memslot = >memslots[0]; \
  memslot < slots->memslots + KVM_MEM_SLOTS_NUM && memslot->npages;\
-- 
2.3.0

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

[PATCH 0/4] Preview: vcpu lookup by id changes

Paolo,

some more patches for review before I add them in my next pull request.
Can you review the common code changes and Ack/Nack?

Alex, Paul,
can you review the power changes and Ack/Nack? 

As I want to have patch 2 in 4.4, I splitted up Davids patch into smaller
chunks. David, can you double check that I did not mess up?
So I want to send patch 1 and patch 2 for 4.4. Patch 3 and Patch 4
are for kvm/next (4.5), but need patch 1 as well. 
So I will probably also add all patches into kvm/next by setting up
the git branches in a way that during 4.5 merge window kvm/next will
have my branch that will go via kvm/master as common ancestor.

Ok?

Christian

David Hildenbrand (4):
  KVM: Provide function for VCPU lookup by id
  KVM: s390: fix wrong lookup of VCPUs by array index
  KVM: use heuristic for fast VCPU lookup by id
  KVM: Use common function for VCPU lookup by id

 arch/powerpc/kvm/book3s_hv.c | 10 ++
 arch/s390/kvm/diag.c | 11 +++
 arch/s390/kvm/sigp.c |  8 ++--
 include/linux/kvm_host.h | 16 
 virt/kvm/kvm_main.c  | 12 +---
 5 files changed, 28 insertions(+), 29 deletions(-)

-- 
2.3.0

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

[PATCH 3/4] KVM: use heuristic for fast VCPU lookup by id

From: David Hildenbrand 

Usually, VCPU ids match the array index. So let's try a fast
lookup first before falling back to the slow iteration.

Suggested-by: Christian Borntraeger 
Reviewed-by: Dominik Dingel 
Reviewed-by: Christian Borntraeger 
Signed-off-by: David Hildenbrand 
Signed-off-by: Christian Borntraeger 
---
 include/linux/kvm_host.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index b9f345f..7821137 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -465,6 +465,11 @@ static inline struct kvm_vcpu *kvm_lookup_vcpu(struct kvm 
*kvm, int id)
struct kvm_vcpu *vcpu;
int i;
 
+   if (id < 0 || id >= KVM_MAX_VCPUS)
+   return NULL;
+   vcpu = kvm_get_vcpu(kvm, id);
+   if (vcpu && vcpu->vcpu_id == id)
+   return vcpu;
kvm_for_each_vcpu(i, vcpu, kvm)
if (vcpu->vcpu_id == id)
return vcpu;
-- 
2.3.0

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

[PATCH 0/4] Preview: vcpu lookup by id changes

Paolo,

some more patches for review before I add them in my next pull request.
Can you review the common code changes and Ack/Nack?

Alex, Paul,
can you review the power changes and Ack/Nack? 

As I want to have patch 2 in 4.4, I splitted up Davids patch into smaller
chunks. David, can you double check that I did not mess up?
So I want to send patch 1 and patch 2 for 4.4. Patch 3 and Patch 4
are for kvm/next (4.5), but need patch 1 as well. 
So I will probably also add all patches into kvm/next by setting up
the git branches in a way that during 4.5 merge window kvm/next will
have my branch that will go via kvm/master as common ancestor.

Ok?

Christian

David Hildenbrand (4):
  KVM: Provide function for VCPU lookup by id
  KVM: s390: fix wrong lookup of VCPUs by array index
  KVM: use heuristic for fast VCPU lookup by id
  KVM: Use common function for VCPU lookup by id

 arch/powerpc/kvm/book3s_hv.c | 10 ++
 arch/s390/kvm/diag.c | 11 +++
 arch/s390/kvm/sigp.c |  8 ++--
 include/linux/kvm_host.h | 16 
 virt/kvm/kvm_main.c  | 12 +---
 5 files changed, 28 insertions(+), 29 deletions(-)

-- 
2.3.0

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

[PATCH 4/4] KVM: Use common function for VCPU lookup by id

From: David Hildenbrand 

Let's reuse the new common function for VPCU lookup by id.

Reviewed-by: Christian Borntraeger 
Reviewed-by: Dominik Dingel 
Signed-off-by: David Hildenbrand 
Signed-off-by: Christian Borntraeger 
[split out the new function into a separate patch]
---
 arch/powerpc/kvm/book3s_hv.c | 10 ++
 arch/s390/kvm/diag.c | 11 +++
 virt/kvm/kvm_main.c  | 12 +---
 3 files changed, 10 insertions(+), 23 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 54b45b7..904b3b0 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -308,16 +308,10 @@ static void kvmppc_dump_regs(struct kvm_vcpu *vcpu)
 
 static struct kvm_vcpu *kvmppc_find_vcpu(struct kvm *kvm, int id)
 {
-   int r;
-   struct kvm_vcpu *v, *ret = NULL;
+   struct kvm_vcpu *ret;
 
mutex_lock(>lock);
-   kvm_for_each_vcpu(r, v, kvm) {
-   if (v->vcpu_id == id) {
-   ret = v;
-   break;
-   }
-   }
+   ret = kvm_lookup_vcpu(kvm, id);
mutex_unlock(>lock);
return ret;
 }
diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
index 5fbfb88..aaa7cc0 100644
--- a/arch/s390/kvm/diag.c
+++ b/arch/s390/kvm/diag.c
@@ -155,10 +155,8 @@ static int __diag_time_slice_end(struct kvm_vcpu *vcpu)
 
 static int __diag_time_slice_end_directed(struct kvm_vcpu *vcpu)
 {
-   struct kvm *kvm = vcpu->kvm;
struct kvm_vcpu *tcpu;
int tid;
-   int i;
 
tid = vcpu->run->s.regs.gprs[(vcpu->arch.sie_block->ipa & 0xf0) >> 4];
vcpu->stat.diagnose_9c++;
@@ -167,12 +165,9 @@ static int __diag_time_slice_end_directed(struct kvm_vcpu 
*vcpu)
if (tid == vcpu->vcpu_id)
return 0;
 
-   kvm_for_each_vcpu(i, tcpu, kvm)
-   if (tcpu->vcpu_id == tid) {
-   kvm_vcpu_yield_to(tcpu);
-   break;
-   }
-
+   tcpu = kvm_lookup_vcpu(vcpu->kvm, tid);
+   if (tcpu)
+   kvm_vcpu_yield_to(tcpu);
return 0;
 }
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 484079e..244c9b4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2257,7 +2257,7 @@ static int create_vcpu_fd(struct kvm_vcpu *vcpu)
 static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
 {
int r;
-   struct kvm_vcpu *vcpu, *v;
+   struct kvm_vcpu *vcpu;
 
if (id >= KVM_MAX_VCPUS)
return -EINVAL;
@@ -2281,12 +2281,10 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, 
u32 id)
r = -EINVAL;
goto unlock_vcpu_destroy;
}
-
-   kvm_for_each_vcpu(r, v, kvm)
-   if (v->vcpu_id == id) {
-   r = -EEXIST;
-   goto unlock_vcpu_destroy;
-   }
+   if (kvm_lookup_vcpu(kvm, id)) {
+   r = -EEXIST;
+   goto unlock_vcpu_destroy;
+   }
 
BUG_ON(kvm->vcpus[atomic_read(>online_vcpus)]);
 
-- 
2.3.0

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

Re: [PATCH 0/4] Preview: vcpu lookup by id changes



On 19/11/2015 09:37, Christian Borntraeger wrote:
> 
> some more patches for review before I add them in my next pull request.
> Can you review the common code changes and Ack/Nack?
> 
> Alex, Paul,
> can you review the power changes and Ack/Nack? 
> 
> As I want to have patch 2 in 4.4, I splitted up Davids patch into smaller
> chunks. David, can you double check that I did not mess up?
> So I want to send patch 1 and patch 2 for 4.4. Patch 3 and Patch 4
> are for kvm/next (4.5), but need patch 1 as well. 
> So I will probably also add all patches into kvm/next by setting up
> the git branches in a way that during 4.5 merge window kvm/next will
> have my branch that will go via kvm/master as common ancestor.
> 
> Ok?

Patches and plan both look good!

I can apply patch 1 and 2 now to kvm/master.  By the time kvm/next forks
from Linus's tree (sometime next week, since kvm/queue is already
largish) they will be in.

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

Re: [PATCH 0/4] Preview: vcpu lookup by id changes



On 19/11/2015 09:37, Christian Borntraeger wrote:
> 
> some more patches for review before I add them in my next pull request.
> Can you review the common code changes and Ack/Nack?
> 
> Alex, Paul,
> can you review the power changes and Ack/Nack? 
> 
> As I want to have patch 2 in 4.4, I splitted up Davids patch into smaller
> chunks. David, can you double check that I did not mess up?
> So I want to send patch 1 and patch 2 for 4.4. Patch 3 and Patch 4
> are for kvm/next (4.5), but need patch 1 as well. 
> So I will probably also add all patches into kvm/next by setting up
> the git branches in a way that during 4.5 merge window kvm/next will
> have my branch that will go via kvm/master as common ancestor.
> 
> Ok?

Patches and plan both look good!

I can apply patch 1 and 2 now to kvm/master.  By the time kvm/next forks
from Linus's tree (sometime next week, since kvm/queue is already
largish) they will be in.

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

Re: [PATCH 0/4] Preview: vcpu lookup by id changes

On 11/19/2015 10:38 AM, Paolo Bonzini wrote:
> 
> 
> On 19/11/2015 09:37, Christian Borntraeger wrote:
>>
>> some more patches for review before I add them in my next pull request.
>> Can you review the common code changes and Ack/Nack?
>>
>> Alex, Paul,
>> can you review the power changes and Ack/Nack? 
>>
>> As I want to have patch 2 in 4.4, I splitted up Davids patch into smaller
>> chunks. David, can you double check that I did not mess up?
>> So I want to send patch 1 and patch 2 for 4.4. Patch 3 and Patch 4
>> are for kvm/next (4.5), but need patch 1 as well. 
>> So I will probably also add all patches into kvm/next by setting up
>> the git branches in a way that during 4.5 merge window kvm/next will
>> have my branch that will go via kvm/master as common ancestor.
>>
>> Ok?
> 
> Patches and plan both look good!
> 
> I can apply patch 1 and 2 now to kvm/master.  By the time kvm/next forks
> from Linus's tree (sometime next week, since kvm/queue is already
> largish) they will be in.

I have 2or 3 more patches for 4.4 and I will prepare a pull request today. So
either take them or wait for my request. Whatever works best for you.



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

Re: [PATCH 0/4] Preview: vcpu lookup by id changes

On 11/19/2015 10:38 AM, Paolo Bonzini wrote:
> 
> 
> On 19/11/2015 09:37, Christian Borntraeger wrote:
>>
>> some more patches for review before I add them in my next pull request.
>> Can you review the common code changes and Ack/Nack?
>>
>> Alex, Paul,
>> can you review the power changes and Ack/Nack? 
>>
>> As I want to have patch 2 in 4.4, I splitted up Davids patch into smaller
>> chunks. David, can you double check that I did not mess up?
>> So I want to send patch 1 and patch 2 for 4.4. Patch 3 and Patch 4
>> are for kvm/next (4.5), but need patch 1 as well. 
>> So I will probably also add all patches into kvm/next by setting up
>> the git branches in a way that during 4.5 merge window kvm/next will
>> have my branch that will go via kvm/master as common ancestor.
>>
>> Ok?
> 
> Patches and plan both look good!
> 
> I can apply patch 1 and 2 now to kvm/master.  By the time kvm/next forks
> from Linus's tree (sometime next week, since kvm/queue is already
> largish) they will be in.

I have 2or 3 more patches for 4.4 and I will prepare a pull request today. So
either take them or wait for my request. Whatever works best for you.



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

Re: [PATCH 0/4] Preview: vcpu lookup by id changes



On 19/11/2015 10:51, Christian Borntraeger wrote:
> > I can apply patch 1 and 2 now to kvm/master.  By the time kvm/next forks
> > from Linus's tree (sometime next week, since kvm/queue is already
> > largish) they will be in.
> 
> I have 2or 3 more patches for 4.4 and I will prepare a pull request today. So
> either take them or wait for my request. Whatever works best for you.

I'll wait then.  ARM folks also have a couple patches for me. :)

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

Re: [PATCH 0/4] Preview: vcpu lookup by id changes



On 19/11/2015 10:51, Christian Borntraeger wrote:
> > I can apply patch 1 and 2 now to kvm/master.  By the time kvm/next forks
> > from Linus's tree (sometime next week, since kvm/queue is already
> > largish) they will be in.
> 
> I have 2or 3 more patches for 4.4 and I will prepare a pull request today. So
> either take them or wait for my request. Whatever works best for you.

I'll wait then.  ARM folks also have a couple patches for me. :)

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

Re: [PATCH 2/4] KVM: s390: fix wrong lookup of VCPUs by array index

Sigh.
Seems that my mail script got confused by the # after stable. So please
strip down the cc list.


On 11/19/2015 09:37 AM, Christian Borntraeger wrote:
> From: David Hildenbrand 
> 
> For now, VCPUs were always created sequentially with incrementing
> VCPU ids. Therefore, the index in the VCPUs array matched the id.
> 
> As sequential creation might change with cpu hotplug, let's use
> the correct lookup function to find a VCPU by id, not array index.
> 
> Reviewed-by: Christian Borntraeger 
> Signed-off-by: David Hildenbrand 
> Signed-off-by: Christian Borntraeger 
> [split stable/non-stable parts]
> Cc: sta...@vger.kernel.org # c3853a8: KVM: Provide function for VCPU lookup 
> by id
> ---
>  arch/s390/kvm/sigp.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/s390/kvm/sigp.c b/arch/s390/kvm/sigp.c
> index da690b6..081dbd0 100644
> --- a/arch/s390/kvm/sigp.c
> +++ b/arch/s390/kvm/sigp.c
> @@ -291,12 +291,8 @@ static int handle_sigp_dst(struct kvm_vcpu *vcpu, u8 
> order_code,
>  u16 cpu_addr, u32 parameter, u64 *status_reg)
>  {
>   int rc;
> - struct kvm_vcpu *dst_vcpu;
> + struct kvm_vcpu *dst_vcpu = kvm_lookup_vcpu(vcpu->kvm, cpu_addr);
> 
> - if (cpu_addr >= KVM_MAX_VCPUS)
> - return SIGP_CC_NOT_OPERATIONAL;
> -
> - dst_vcpu = kvm_get_vcpu(vcpu->kvm, cpu_addr);
>   if (!dst_vcpu)
>   return SIGP_CC_NOT_OPERATIONAL;
> 
> @@ -478,7 +474,7 @@ int kvm_s390_handle_sigp_pei(struct kvm_vcpu *vcpu)
>   trace_kvm_s390_handle_sigp_pei(vcpu, order_code, cpu_addr);
> 
>   if (order_code == SIGP_EXTERNAL_CALL) {
> - dest_vcpu = kvm_get_vcpu(vcpu->kvm, cpu_addr);
> + dest_vcpu = kvm_lookup_vcpu(vcpu->kvm, cpu_addr);
>   BUG_ON(dest_vcpu == NULL);
> 
>   kvm_s390_vcpu_wakeup(dest_vcpu);
> 

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

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

2015-11-19 Thread Borislav Petkov

On Thu, Nov 19, 2015 at 10:34:07AM +0100, Paolo Bonzini wrote:
> Let me know if I should include this patch via the KVM tree.  Do you
> want it in 4.4?

Nah, no need.

I'll send the whole pile with your Reviewed-by's to Ingo so that they
all go together. I'll run them some more on my boxes first though...

Thanks.

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] KVM: s390: fix wrong lookup of VCPUs by array index

Sigh.
Seems that my mail script got confused by the # after stable. So please
strip down the cc list.


On 11/19/2015 09:37 AM, Christian Borntraeger wrote:
> From: David Hildenbrand 
> 
> For now, VCPUs were always created sequentially with incrementing
> VCPU ids. Therefore, the index in the VCPUs array matched the id.
> 
> As sequential creation might change with cpu hotplug, let's use
> the correct lookup function to find a VCPU by id, not array index.
> 
> Reviewed-by: Christian Borntraeger 
> Signed-off-by: David Hildenbrand 
> Signed-off-by: Christian Borntraeger 
> [split stable/non-stable parts]
> Cc: sta...@vger.kernel.org # c3853a8: KVM: Provide function for VCPU lookup 
> by id
> ---
>  arch/s390/kvm/sigp.c | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/s390/kvm/sigp.c b/arch/s390/kvm/sigp.c
> index da690b6..081dbd0 100644
> --- a/arch/s390/kvm/sigp.c
> +++ b/arch/s390/kvm/sigp.c
> @@ -291,12 +291,8 @@ static int handle_sigp_dst(struct kvm_vcpu *vcpu, u8 
> order_code,
>  u16 cpu_addr, u32 parameter, u64 *status_reg)
>  {
>   int rc;
> - struct kvm_vcpu *dst_vcpu;
> + struct kvm_vcpu *dst_vcpu = kvm_lookup_vcpu(vcpu->kvm, cpu_addr);
> 
> - if (cpu_addr >= KVM_MAX_VCPUS)
> - return SIGP_CC_NOT_OPERATIONAL;
> -
> - dst_vcpu = kvm_get_vcpu(vcpu->kvm, cpu_addr);
>   if (!dst_vcpu)
>   return SIGP_CC_NOT_OPERATIONAL;
> 
> @@ -478,7 +474,7 @@ int kvm_s390_handle_sigp_pei(struct kvm_vcpu *vcpu)
>   trace_kvm_s390_handle_sigp_pei(vcpu, order_code, cpu_addr);
> 
>   if (order_code == SIGP_EXTERNAL_CALL) {
> - dest_vcpu = kvm_get_vcpu(vcpu->kvm, cpu_addr);
> + dest_vcpu = kvm_lookup_vcpu(vcpu->kvm, cpu_addr);
>   BUG_ON(dest_vcpu == NULL);
> 
>   kvm_s390_vcpu_wakeup(dest_vcpu);
> 

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

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



On 18/11/2015 19:50, Borislav Petkov wrote:
> On Wed, Nov 18, 2015 at 12:35:24PM +0100, Paolo Bonzini wrote:
>> Yes, exactly.  I'm suggesting that the same applies to x86_vendor().  I
>> also prefer x86_cpuid_* to x86_*_cpuid because, once you add two
>> functions in the same family it's nice that they share a prefix.
> 
> Ok, makes sense:
> 
> ---
> commit 804437270083a1aabf87f65f65b32e2ae23121b6
> Author: Borislav Petkov 
> Date:   Sat Nov 14 10:54:22 2015 +0100
> 
> x86/cpu: Unify CPU family, model, stepping calculation
> 
> Add generic functions which calc family, model and stepping from the
> CPUID_1.EAX leaf and stick them into the library we have.
> 
> Rename those which do call CPUID with the prefix "x86_cpuid" as
> suggested by Paolo Bonzini.
> 
> No functionality change.
> 
> Signed-off-by: Borislav Petkov 

Reviewed-by: Paolo Bonzini 

Let me know if I should include this patch via the KVM tree.  Do you
want it in 4.4?

Paolo

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

Re: [patch 1/2] KVM: fix error handling in kvm_create_vm_debugfs()

2015-11-19 Thread Dan Carpenter

No problem.  Fold away.

regards,
dan carpenter

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

Re: [PATCH 9/9] vfio-pci: constify pci_error_handlers structures

2015-11-19 Thread Alex Williamson

On Sat, 2015-11-14 at 11:07 +0100, Julia Lawall wrote:
> This pci_error_handlers structure is never modified, like all the other
> pci_error_handlers structures, so declare it as const.
> 
> Done with the help of Coccinelle.
> 
> Signed-off-by: Julia Lawall 
> 
> ---
> There are no dependencies between these patches.
> 
>  drivers/vfio/pci/vfio_pci.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 32b88bd..2760a7b 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -1035,7 +1035,7 @@ static pci_ers_result_t 
> vfio_pci_aer_err_detected(struct pci_dev *pdev,
>   return PCI_ERS_RESULT_CAN_RECOVER;
>  }
>  
> -static struct pci_error_handlers vfio_err_handlers = {
> +static const struct pci_error_handlers vfio_err_handlers = {
>   .error_detected = vfio_pci_aer_err_detected,
>  };
>  
> 

Thank you!  I'll queue this one in my tree.

Alex

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

Re: [PATCH v3 0/3] virtio DMA API core stuff

2015-11-19 Thread David Woodhouse

On Thu, 2015-11-19 at 13:59 -0800, Andy Lutomirski wrote:
> 
> >
> > So thinking hard about it, I don't see any real drawbacks to making this
> > conditional on a new feature bit, that Xen can then set..
> 
> Can you elaborate?  If I run QEMU, hosting Xen, hosting Linux, and the
> virtio device is provided by QEMU, then how does Xen set the bit?
> Similarly, how would Xen set the bit for a real physical device?

Right. This is *not* a fundamental characteristic of the device. This
is all about how your *particular* hypervisor (in the set of turtles-
all-the-way-down) happened to expose the thing to you.

This is why it lives in the DMAR table, in the Intel world, which
*tells* you which devices are behind which IOMMU (and which are not).
And why I keep repeating myself that it has nothing to do with the
actual device or the virtio drivers.

I understand that POWER and other platforms don't currently have a
clean way to indicate that certain device don't have translation. And I
understand that we may end up with a *quirk* which ensures that the DMA
API does the right thing (i.e. nothing) in certain cases.

But we should *NOT* be involving the virtio device drivers in that
quirk, in any way. And putting a feature bit in the virtio device
itself doesn't seem at all sane either.

Bear in mind that qemu-system-x86_64 currently has the *same* problem
with assigned physical devices. It's claiming they're translated, and
they're not.

-- 
dwmw2

smime.p7s
Description: S/MIME cryptographic signature

[PATCH] vfio: platform: remove needless stack usage

2015-11-19 Thread Kees Cook

request_module already takes format strings, so no need to duplicate
the effort.

Signed-off-by: Kees Cook 
---
 drivers/vfio/platform/vfio_platform_common.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/vfio/platform/vfio_platform_common.c 
b/drivers/vfio/platform/vfio_platform_common.c
index a1c50d630792..418cdd9ba3f4 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -51,13 +51,10 @@ static vfio_platform_reset_fn_t 
vfio_platform_lookup_reset(const char *compat,
 
 static void vfio_platform_get_reset(struct vfio_platform_device *vdev)
 {
-   char modname[256];
-
vdev->reset = vfio_platform_lookup_reset(vdev->compat,
>reset_module);
if (!vdev->reset) {
-   snprintf(modname, 256, "vfio-reset:%s", vdev->compat);
-   request_module(modname);
+   request_module("vfio-reset:%s", vdev->compat);
vdev->reset = vfio_platform_lookup_reset(vdev->compat,
 >reset_module);
}
-- 
1.9.1


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

linux-next: manual merge of the kvms390 tree with the kvm tree

2015-11-19 Thread Stephen Rothwell

Hi all,

Today's linux-next merge of the kvms390 tree got a conflict in:

  include/linux/kvm_host.h
  arch/s390/kvm/interrupt.c
  arch/s390/kvm/sigp.c

between commits:

  db27a7a37aa0 ("KVM: Provide function for VCPU lookup by id")
  b85de33a1a34 ("KVM: s390: avoid memory overwrites on emergency signal 
injection")
  152e9f65d66f ("KVM: s390: fix wrong lookup of VCPUs by array index")

from the kvm tree and commit:

  a8ab7a5fd13b ("KVM: Provide function for VCPU lookup by id")
  e896d1d08efc ("KVM: s390: avoid memory overwrites on emergency signal 
injection")
  32d7bd98e799 ("KVM: s390: fix wrong lookup of VCPUs by array index")

from the kvms390 tree.

These patches used different names for the added function.  I used the
name from the kvm tree (kvm_get_vcpu_by_id).

This also required the following merge patch:

From: Stephen Rothwell 
Date: Fri, 20 Nov 2015 12:42:17 +1100
Subject: [PATCH] KVM: s390: update for kvm_lookup_vcpu name change

Signed-off-by: Stephen Rothwell 
---
 arch/powerpc/kvm/book3s_hv.c | 2 +-
 arch/s390/kvm/diag.c | 2 +-
 virt/kvm/kvm_main.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 904b3b0afb48..a29da44cdc6c 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -311,7 +311,7 @@ static struct kvm_vcpu *kvmppc_find_vcpu(struct kvm *kvm, 
int id)
struct kvm_vcpu *ret;
 
mutex_lock(>lock);
-   ret = kvm_lookup_vcpu(kvm, id);
+   ret = kvm_get_vcpu_by_id(kvm, id);
mutex_unlock(>lock);
return ret;
 }
diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
index aaa7cc041335..05f7de9869a9 100644
--- a/arch/s390/kvm/diag.c
+++ b/arch/s390/kvm/diag.c
@@ -165,7 +165,7 @@ static int __diag_time_slice_end_directed(struct kvm_vcpu 
*vcpu)
if (tid == vcpu->vcpu_id)
return 0;
 
-   tcpu = kvm_lookup_vcpu(vcpu->kvm, tid);
+   tcpu = kvm_get_vcpu_by_id(vcpu->kvm, tid);
if (tcpu)
kvm_vcpu_yield_to(tcpu);
return 0;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1ee2f738bf61..1f0f8c4ca345 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2358,7 +2358,7 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 
id)
r = -EINVAL;
goto unlock_vcpu_destroy;
}
-   if (kvm_lookup_vcpu(kvm, id)) {
+   if (kvm_get_vcpu_by_id(kvm, id)) {
r = -EEXIST;
goto unlock_vcpu_destroy;
}
-- 
2.6.2

This should probably all be cleaned up in the kvms390 tree.

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

Re: [PATCH v3 0/3] virtio DMA API core stuff

2015-11-19 Thread Andy Lutomirski

On Nov 19, 2015 5:45 AM, "Michael S. Tsirkin"  wrote:
>
> On Tue, Oct 27, 2015 at 11:38:57PM -0700, Andy Lutomirski wrote:
> > This switches virtio to use the DMA API unconditionally.  I'm sure
> > it breaks things, but it seems to work on x86 using virtio-pci, with
> > and without Xen, and using both the modern 1.0 variant and the
> > legacy variant.
>
> So thinking hard about it, I don't see any real drawbacks to making this
> conditional on a new feature bit, that Xen can then set..

Can you elaborate?  If I run QEMU, hosting Xen, hosting Linux, and the
virtio device is provided by QEMU, then how does Xen set the bit?
Similarly, how would Xen set the bit for a real physical device?


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

Re: [RESEND PATCH] vfio: Drop owner assignment from platform_driver

2015-11-19 Thread Alex Williamson

On Thu, 2015-11-19 at 13:00 +0900, Krzysztof Kozlowski wrote:
> platform_driver does not need to set an owner because
> platform_driver_register() will set it.
> 
> Signed-off-by: Krzysztof Kozlowski 
> Acked-by: Baptiste Reynal 
> 
> ---

Oops, sorry I dropped it.  Since it's a fix, I'll queue it for 4.4.
Thanks,

Alex


> 
> The coccinelle script which generated the patch was sent here:
> http://www.spinics.net/lists/kernel/msg2029903.html
> ---
>  drivers/vfio/platform/vfio_platform.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/vfio/platform/vfio_platform.c 
> b/drivers/vfio/platform/vfio_platform.c
> index f1625dcfbb23..b1cc3a768784 100644
> --- a/drivers/vfio/platform/vfio_platform.c
> +++ b/drivers/vfio/platform/vfio_platform.c
> @@ -92,7 +92,6 @@ static struct platform_driver vfio_platform_driver = {
>   .remove = vfio_platform_remove,
>   .driver = {
>   .name   = "vfio-platform",
> - .owner  = THIS_MODULE,
>   },
>  };
>  



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

[patch 2/2] KVM: fix vm_stat_get()

2015-11-19 Thread Dan Carpenter

The indenting suggests missing curly braces.

Fixes: 7805f53a85ec ('KVM: Create debugfs dir and stat files for each VM')
Signed-off-by: Dan Carpenter 

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f62621f..4740e54 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3576,10 +3576,11 @@ static int vm_stat_get(void *_offset, u64 *val)
 
*val = 0;
spin_lock(_lock);
-   list_for_each_entry(kvm, _list, vm_list)
+   list_for_each_entry(kvm, _list, vm_list) {
stat_tmp.kvm = kvm;
vm_stat_get_per_vm((void *)_tmp, _val);
*val += tmp_val;
+   }
spin_unlock(_lock);
return 0;
 }
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] KVM: Provide function for VCPU lookup by id

2015-11-19 Thread Thomas Huth

On 19/11/15 09:37, Christian Borntraeger wrote:
> From: David Hildenbrand 
> 
> Let's provide a function to lookup a VCPU by id.
> 
> Reviewed-by: Christian Borntraeger 
> Reviewed-by: Dominik Dingel 
> Signed-off-by: David Hildenbrand 
> Signed-off-by: Christian Borntraeger 
> [split patch from refactoring patch]
> ---
>  include/linux/kvm_host.h | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 5706a21..b9f345f 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -460,6 +460,17 @@ static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm 
> *kvm, int i)
>(vcpup = kvm_get_vcpu(kvm, idx)) != NULL; \
>idx++)
>  
> +static inline struct kvm_vcpu *kvm_lookup_vcpu(struct kvm *kvm, int id)
> +{
> + struct kvm_vcpu *vcpu;
> + int i;
> +
> + kvm_for_each_vcpu(i, vcpu, kvm)
> + if (vcpu->vcpu_id == id)
> + return vcpu;
> + return NULL;
> +}
> +

Any chance that you name this function differently? Otherwise we've got
two functions that sound very similar and also have similar prototypes:

- kvm_get_vcpu(struct kvm *kvm, int i)
- kvm_lookup_vcpu(struct kvm *kvm, int id)

I'm pretty sure this will cause confusion in the future!
==> Could you maybe name the new function something like
"kvm_lookup_vcpu_by_id" or "kvm_get_vcpu_by_id" instead?

Also a short comment before the function would be nice to make the
reader aware of the difference (e.g. when hot-plugging) between the
vcpu-id and array-id.

Otherwise: +1 for finally having a proper common function for this!

 Thomas

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

Re: [PATCH 1/4] KVM: Provide function for VCPU lookup by id

2015-11-19 Thread Thomas Huth

On 19/11/15 09:37, Christian Borntraeger wrote:
> From: David Hildenbrand 
> 
> Let's provide a function to lookup a VCPU by id.
> 
> Reviewed-by: Christian Borntraeger 
> Reviewed-by: Dominik Dingel 
> Signed-off-by: David Hildenbrand 
> Signed-off-by: Christian Borntraeger 
> [split patch from refactoring patch]
> ---
>  include/linux/kvm_host.h | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 5706a21..b9f345f 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -460,6 +460,17 @@ static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm 
> *kvm, int i)
>(vcpup = kvm_get_vcpu(kvm, idx)) != NULL; \
>idx++)
>  
> +static inline struct kvm_vcpu *kvm_lookup_vcpu(struct kvm *kvm, int id)
> +{
> + struct kvm_vcpu *vcpu;
> + int i;
> +
> + kvm_for_each_vcpu(i, vcpu, kvm)
> + if (vcpu->vcpu_id == id)
> + return vcpu;
> + return NULL;
> +}
> +

Any chance that you name this function differently? Otherwise we've got
two functions that sound very similar and also have similar prototypes:

- kvm_get_vcpu(struct kvm *kvm, int i)
- kvm_lookup_vcpu(struct kvm *kvm, int id)

I'm pretty sure this will cause confusion in the future!
==> Could you maybe name the new function something like
"kvm_lookup_vcpu_by_id" or "kvm_get_vcpu_by_id" instead?

Also a short comment before the function would be nice to make the
reader aware of the difference (e.g. when hot-plugging) between the
vcpu-id and array-id.

Otherwise: +1 for finally having a proper common function for this!

 Thomas

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

[patch 1/2] KVM: fix error handling in kvm_create_vm_debugfs()

2015-11-19 Thread Dan Carpenter

The "goto out_err" is buggy because we forgot to set the return code.

The other issue is that if the kmalloc() fails, we should remove the
debugfs directory before returning.

Fixes: 7805f53a85ec ('KVM: Create debugfs dir and stat files for each VM')
Signed-off-by: Dan Carpenter 

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1ee2f73..f62621f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -571,12 +571,14 @@ static int kvm_create_vm_debugfs(struct kvm *kvm)
snprintf(dir_name, sizeof(dir_name), "%d", current->pid);
kvm->debugfs_dentry = debugfs_create_dir(dir_name, kvm_debugfs_dir);
if (!kvm->debugfs_dentry)
-   goto out_err;
+   return -ENOMEM;
 
kvm->debugfs_data = kmalloc(sizeof(*kvm->debugfs_data) *
kvm_debugfs_num_entries, GFP_KERNEL);
-   if (!kvm->debugfs_data)
-   return -ENOMEM;
+   if (!kvm->debugfs_data) {
+   r = -ENOMEM;
+   goto err_remove_dir;
+   }
 
for (p = debugfs_entries; p->name; p++, i++) {
stat_data = kzalloc(sizeof(*stat_data), GFP_KERNEL);
@@ -600,13 +602,15 @@ static int kvm_create_vm_debugfs(struct kvm *kvm)
return r;
 
 out_err_clean:
-   debugfs_remove_recursive(kvm->debugfs_dentry);
kfree(stat_data);
for (i--; i >= 0; i--)
kfree(kvm->debugfs_data[i]);
 
kfree(kvm->debugfs_data);
-out_err:
+
+err_remove_dir:
+   debugfs_remove_recursive(kvm->debugfs_dentry);
+
return r;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Qemu-devel] [PATCH v3 1/3] target-i386: add pkeys support for cpuid handling



On 19/11/2015 07:36, Han, Huaitong wrote:
> I understand it has always been that QEMU considers the feature of
>  cpuid_7_0_ecx_feature_name as migratable. If the feature is
>  unmigratable, it will been added to unmigratable_flags.
> 
> A series of patches do complete a full function, moving
>  cpuid_7_0_ecx_feature_name to 2/3 patch may make 2/3 patch look
> better, but make 1/3 patch look somewhat incomplete.
> 
> Maybe it is a solution that adding the feature to unmigratable_flags in
> 1/3 patch, and deleting unmigratable_flags in 2/3 patch, but I think it
> is pointless.

Or just squash everything together.  After all we're talking of

 4 files changed, 55 insertions(+), 1 deletion(-)

It's not a large patch.

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

Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest

2015-11-19 Thread Wanpeng Li

2015-11-19 19:05 GMT+08:00 Paolo Bonzini :
>
>
> On 19/11/2015 11:45, Wanpeng li wrote:
>> Intel cpu doesn't support syscall/sysret in non 64-bit mode which
>> is different from AMD. Expose syscall/sysret to intel 32-bit guest
>> just makes no sense and leads to #UD which will confuse the users.
>>
>> This patch disable expose syscall/sysret to intel 32-bit guest.
>>
>> Signed-off-by: Wanpeng li 
>> ---
>>  arch/x86/kvm/cpuid.c | 16 
>>  arch/x86/kvm/x86.c   |  1 +
>>  2 files changed, 17 insertions(+)
>>
>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>> index 6525e92..f8ddca6 100644
>> --- a/arch/x86/kvm/cpuid.c
>> +++ b/arch/x86/kvm/cpuid.c
>> @@ -62,6 +62,7 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
>>  {
>>   struct kvm_cpuid_entry2 *best;
>>   struct kvm_lapic *apic = vcpu->arch.apic;
>> + bool vendor_intel = false;
>>
>>   best = kvm_find_cpuid_entry(vcpu, 1, 0);
>>   if (!best)
>> @@ -114,6 +115,21 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
>>   vcpu->arch.maxphyaddr = cpuid_query_maxphyaddr(vcpu);
>>
>>   kvm_pmu_refresh(vcpu);
>> +
>> + /* Don't expose syscall/sysret to intel non 64-bit mode guest */
>> + best = kvm_find_cpuid_entry(vcpu, 0x, 0x);
>> + if (best && best->ebx == 0x756e6547 && best->ecx == 0x6c65746e &&
>> + best->edx == 0x49656e69)
>> + vendor_intel = true;
>
> You can use X86EMUL_CPUID_VENDOR_GenuineIntel_ebx/ecx/edx instead here.

Ok.

>
>> + best = kvm_find_cpuid_entry(vcpu, 0x8001, 0);
>> + if (best && vendor_intel) {
>> + if (!is_long_mode(vcpu))
>> + best->edx &= ~F(SYSCALL);
>> + else
>> + best->edx |= F(SYSCALL);
>
> This is not correct.  As far as I know, the SYSCALL bit is always
> present in CPUID, even if the machine is running in 32-bit mode; CPUID
> documentation (SDM Volume 2) explicitly documents bit 11 as "Bit 11:
> SYSCALL/SYSRET available in 64-bit mode".

No, I try a 32-bit linux host kernel, cpuid tool shows that SYSCALL
bit is not set.

>
> So there are two possibilities here:
>
> 1) Clear F(SYSCALL) in kvm_update_cpuid, like you are doing here but
> only if F(LM) is already clear (in addition to the vendor being Intel).

Good point. :-)

>
> 2) do nothing, declare this a userspace bug.  Linux knows that only AMD
> machines support SYSCALL in 32-bit mode (there is a separate feature bit
> X86_FEATURE_SYSCALL32 that is only set by arch/x86/kernel/cpu/amd.c).
> Are you fixing an issue with Windows?  If so, with what QEMU command

No.

> line?  But if it's not an issue with a real guest, doing nothing would
> be my favorite option...
>
> In any case, set_efer doesn't need to call kvm_update_cpuid.

Btw, I also try a 32-bit linux guest in vmware on my macbook, it also
doesn't expose
syscall/sysret.

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

Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest



On 19/11/2015 13:01, Wanpeng Li wrote:
> > This is not correct.  As far as I know, the SYSCALL bit is always
> > present in CPUID, even if the machine is running in 32-bit mode; CPUID
> > documentation (SDM Volume 2) explicitly documents bit 11 as "Bit 11:
> > SYSCALL/SYSRET available in 64-bit mode".
> 
> No, I try a 32-bit linux host kernel, cpuid tool shows that SYSCALL
> bit is not set.

Ok, let me try...

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

Re: [PATCH] KVM: x86: don't expose syscall/sysret to intel 32-bit guest

On 19/11/2015 11:45, Wanpeng li wrote:
> Intel cpu doesn't support syscall/sysret in non 64-bit mode which 
> is different from AMD. Expose syscall/sysret to intel 32-bit guest 
> just makes no sense and leads to #UD which will confuse the users. 
> 
> This patch disable expose syscall/sysret to intel 32-bit guest.
> 
> Signed-off-by: Wanpeng li 
> ---
>  arch/x86/kvm/cpuid.c | 16 
>  arch/x86/kvm/x86.c   |  1 +
>  2 files changed, 17 insertions(+)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 6525e92..f8ddca6 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -62,6 +62,7 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
>  {
>   struct kvm_cpuid_entry2 *best;
>   struct kvm_lapic *apic = vcpu->arch.apic;
> + bool vendor_intel = false;
>  
>   best = kvm_find_cpuid_entry(vcpu, 1, 0);
>   if (!best)
> @@ -114,6 +115,21 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
>   vcpu->arch.maxphyaddr = cpuid_query_maxphyaddr(vcpu);
>  
>   kvm_pmu_refresh(vcpu);
> +
> + /* Don't expose syscall/sysret to intel non 64-bit mode guest */
> + best = kvm_find_cpuid_entry(vcpu, 0x, 0x);
> + if (best && best->ebx == 0x756e6547 && best->ecx == 0x6c65746e &&
> + best->edx == 0x49656e69)
> + vendor_intel = true;

You can use X86EMUL_CPUID_VENDOR_GenuineIntel_ebx/ecx/edx instead here.

> + best = kvm_find_cpuid_entry(vcpu, 0x8001, 0);
> + if (best && vendor_intel) {
> + if (!is_long_mode(vcpu))
> + best->edx &= ~F(SYSCALL);
> + else
> + best->edx |= F(SYSCALL);

This is not correct.  As far as I know, the SYSCALL bit is always
present in CPUID, even if the machine is running in 32-bit mode; CPUID
documentation (SDM Volume 2) explicitly documents bit 11 as "Bit 11:
SYSCALL/SYSRET available in 64-bit mode".

So there are two possibilities here:

1) Clear F(SYSCALL) in kvm_update_cpuid, like you are doing here but
only if F(LM) is already clear (in addition to the vendor being Intel).

2) do nothing, declare this a userspace bug.  Linux knows that only AMD
machines support SYSCALL in 32-bit mode (there is a separate feature bit
X86_FEATURE_SYSCALL32 that is only set by arch/x86/kernel/cpu/amd.c).
Are you fixing an issue with Windows?  If so, with what QEMU command
line?  But if it's not an issue with a real guest, doing nothing would
be my favorite option...

In any case, set_efer doesn't need to call kvm_update_cpuid.

Thanks,

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

[PATCH v4 05/13] VFIO: platform: add vfio_platform_set_forwarded

This function allows to change the forwarded mode and selects
the IRQ handler accordingly.

Signed-off-by: Eric Auger 

---
v3 -> v3:
- renamed vfio_platform_set_automasked into
  vfio_platform_set_forwarded
- do not change VFIO_IRQ_INFO_AUTOMASKED setting when turning
  forwarding on/off

v1 -> v2:
- set forwarded flag
---
 drivers/vfio/platform/vfio_platform_irq.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/vfio/platform/vfio_platform_irq.c 
b/drivers/vfio/platform/vfio_platform_irq.c
index c58e0ad..257200b 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -186,6 +186,23 @@ static irqreturn_t vfio_handler(int irq, void *dev_id)
return ret;
 }
 
+static int vfio_platform_set_forwarded(struct vfio_platform_irq *irq,
+  bool forwarded)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(>lock, flags);
+   irq->forwarded = forwarded;
+
+   if (!forwarded && (irq->flags & VFIO_IRQ_INFO_AUTOMASKED))
+   irq->handler = vfio_automasked_irq_handler;
+   else
+   irq->handler = vfio_irq_handler;
+
+   spin_unlock_irqrestore(>lock, flags);
+   return 0;
+}
+
 static void vfio_platform_irq_bypass_stop(struct irq_bypass_producer *prod)
 {
 }
-- 
1.9.1

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

Re: [PATCH v3 0/3] virtio DMA API core stuff

2015-11-19 Thread Benjamin Herrenschmidt

On Thu, 2015-11-19 at 23:38 +, David Woodhouse wrote:
> 
> I understand that POWER and other platforms don't currently have a
> clean way to indicate that certain device don't have translation. And I
> understand that we may end up with a *quirk* which ensures that the DMA
> API does the right thing (i.e. nothing) in certain cases.
> 
> But we should *NOT* be involving the virtio device drivers in that
> quirk, in any way. And putting a feature bit in the virtio device
> itself doesn't seem at all sane either.
> 
> Bear in mind that qemu-system-x86_64 currently has the *same* problem
> with assigned physical devices. It's claiming they're translated, and
> they're not.

It's not that clear but yeah ... as I mentioned, I can't find a
way to do that quirk that won't break when we want to actually use
the iommu... 

Ben.

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

Re: [PATCH] KVM: x86: emulate: correct page fault error code for NoWrite instructions

2015-11-19 Thread Wanpeng Li

2015-11-20 10:52 GMT+08:00 Wanpeng Li :
> Hi Paolo,
> 2015-02-09 17:03 GMT+08:00 Paolo Bonzini :
>> NoWrite instructions (e.g. cmp or test) never set the "write access"
>> bit in the error code, even if one of the operands is treated as a
>> destination.
>
> Sorry to reply to an old commit, btw, could you point out where in SDM
> describe above?

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

Re: [PATCH] KVM: x86: emulate: correct page fault error code for NoWrite instructions

2015-11-19 Thread Wanpeng Li

Hi Paolo,
2015-02-09 17:03 GMT+08:00 Paolo Bonzini :
> NoWrite instructions (e.g. cmp or test) never set the "write access"
> bit in the error code, even if one of the operands is treated as a
> destination.

Sorry to reply to an old commit, btw, could you point out where in SDM
describe above?

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

Re: [PATCH] virtio_ring: Shadow available ring flags & index

2015-11-19 Thread Xie, Huawei

On 11/18/2015 12:28 PM, Venkatesh Srinivas wrote:
> On Tue, Nov 17, 2015 at 08:08:18PM -0800, Venkatesh Srinivas wrote:
>> On Mon, Nov 16, 2015 at 7:46 PM, Xie, Huawei  wrote:
>>
>>> On 11/14/2015 7:41 AM, Venkatesh Srinivas wrote:
 On Wed, Nov 11, 2015 at 02:34:33PM +0200, Michael S. Tsirkin wrote:
> On Tue, Nov 10, 2015 at 04:21:07PM -0800, Venkatesh Srinivas wrote:
>> Improves cacheline transfer flow of available ring header.
>>
>> Virtqueues are implemented as a pair of rings, one producer->consumer
>> avail ring and one consumer->producer used ring; preceding the
>> avail ring in memory are two contiguous u16 fields -- avail->flags
>> and avail->idx. A producer posts work by writing to avail->idx and
>> a consumer reads avail->idx.
>>
>> The flags and idx fields only need to be written by a producer CPU
>> and only read by a consumer CPU; when the producer and consumer are
>> running on different CPUs and the virtio_ring code is structured to
>> only have source writes/sink reads, we can continuously transfer the
>> avail header cacheline between 'M' states between cores. This flow
>> optimizes core -> core bandwidth on certain CPUs.
>>
>> (see: "Software Optimization Guide for AMD Family 15h Processors",
>> Section 11.6; similar language appears in the 10h guide and should
>> apply to CPUs w/ exclusive caches, using LLC as a transfer cache)
>>
>> Unfortunately the existing virtio_ring code issued reads to the
>> avail->idx and read-modify-writes to avail->flags on the producer.
>>
>> This change shadows the flags and index fields in producer memory;
>> the vring code now reads from the shadows and only ever writes to
>> avail->flags and avail->idx, allowing the cacheline to transfer
>> core -> core optimally.
> Sounds logical, I'll apply this after a  bit of testing
> of my own, thanks!
 Thanks!
>>> Venkatesh:
>>> Is it that your patch only applies to CPUs w/ exclusive caches?
>> No --- it applies when the inter-cache coherence flow is optimized by
>> 'M' -> 'M' transfers and when producer reads might interfere w/
>> consumer prefetchw/reads. The AMD Optimization guides have specific
>> language on this subject, but other platforms may benefit.
>> (see Intel #'s below)
For core2core case(not HT paire), after consumer reads that M cache line
for avail_idx, is that line still in the producer core's L1 data cache
with state changing from M->O state?
>>
>>> Do you have perf data on Intel CPUs?
>> Good idea -- I ran some tests on a couple of Intel platforms:
>>
>> (these are perf data from sample runs; for each I ran many runs, the
>>  numbers were pretty stable except for Haswell-EP cross-socket)
>>
>> One-socket Intel Xeon W3690 ("Westmere"), 3.46 GHz; core turbo disabled
>> ===
>> (note -- w/ core turbo disabled, performance is _very_ stable; variance of
>>  < 0.5% run-to-run; figure of merit is "seconds elapsed" here)
>>
>> * Producer / consumer bound to Hyperthread pairs:
>>
>>  Performance counter stats for './vring_bench_noshadow 10':
>>
>>  343,425,166,916 L1-dcache-loads
>>   21,393,148 L1-dcache-load-misses #0.01% of all L1-dcache hits
>>   61,709,640,363 L1-dcache-stores
>>5,745,690 L1-dcache-store-misses
>>   10,186,932,553 L1-dcache-prefetches
>>1,491 L1-dcache-prefetch-misses
>>121.335699344 seconds time elapsed
>>
>>  Performance counter stats for './vring_bench_shadow 10':
>>
>>  334,766,413,861 L1-dcache-loads
>>   15,787,778 L1-dcache-load-misses #0.00% of all L1-dcache hits
>>   62,735,792,799 L1-dcache-stores
>>3,252,113 L1-dcache-store-misses
>>9,018,273,596 L1-dcache-prefetches
>>  819 L1-dcache-prefetch-misses
>>121.206339656 seconds time elapsed
>>
>> Effectively Performance-neutral.
>>
>> * Producer / consumer bound to separate cores, same socket:
>>
>>  Performance counter stats for './vring_bench_noshadow 10':
>>
>>399,943,384,509 L1-dcache-loads
>>  8,868,334,693 L1-dcache-load-misses #2.22% of all L1-dcache hits
>> 62,721,376,685 L1-dcache-stores
>>  2,786,806,982 L1-dcache-store-misses
>> 10,915,046,967 L1-dcache-prefetches
>>328,508 L1-dcache-prefetch-misses
>>  146.585969976 seconds time elapsed
>>
>>  Performance counter stats for './vring_bench_shadow 10':
>>
>>425,123,067,750 L1-dcache-loads 
>>  6,689,318,709 L1-dcache-load-misses #1.57% of all L1-dcache hits
>> 62,747,525,005 L1-dcache-stores 
>>  2,496,274,505 L1-dcache-store-misses
>>  8,627,873,397 L1-dcache-prefetches
>>146,729 L1-dcache-prefetch-misses
>>  142.657327765 seconds time elapsed
>>
>> 2.6% reduction in runtime; note that L1-dcache-load-misses reduced
>> dramatically, 2 Billion(!) L1d misses saved.

Re: [PATCH v3 0/3] virtio DMA API core stuff

On Fri, Nov 20, 2015 at 08:56:46AM +0200, Michael S. Tsirkin wrote:
> On Thu, Nov 19, 2015 at 01:59:05PM -0800, Andy Lutomirski wrote:
> > On Nov 19, 2015 5:45 AM, "Michael S. Tsirkin"  wrote:
> > >
> > > On Tue, Oct 27, 2015 at 11:38:57PM -0700, Andy Lutomirski wrote:
> > > > This switches virtio to use the DMA API unconditionally.  I'm sure
> > > > it breaks things, but it seems to work on x86 using virtio-pci, with
> > > > and without Xen, and using both the modern 1.0 variant and the
> > > > legacy variant.
> > >
> > > So thinking hard about it, I don't see any real drawbacks to making this
> > > conditional on a new feature bit, that Xen can then set..
> > 
> > Can you elaborate?  If I run QEMU, hosting Xen, hosting Linux, and the
> > virtio device is provided by QEMU, then how does Xen set the bit?
> 
> You would run QEMU with the appropriate flag. E.g.
> -global virtio-pci,use_platform_dma=on

Or Xen code within QEMU can tweak this global internally
so users don't need to care.

> > Similarly, how would Xen set the bit for a real physical device?
> > 
> > 
> > --Andy
> 
> There's no need to set bits for physical devices I think: from security
> point of view, using them from a VM isn't very different from using them
> from host.
> 
> 
> 
> -- 
> MST
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/3] virtio DMA API core stuff

On Thu, Nov 19, 2015 at 01:59:05PM -0800, Andy Lutomirski wrote:
> On Nov 19, 2015 5:45 AM, "Michael S. Tsirkin"  wrote:
> >
> > On Tue, Oct 27, 2015 at 11:38:57PM -0700, Andy Lutomirski wrote:
> > > This switches virtio to use the DMA API unconditionally.  I'm sure
> > > it breaks things, but it seems to work on x86 using virtio-pci, with
> > > and without Xen, and using both the modern 1.0 variant and the
> > > legacy variant.
> >
> > So thinking hard about it, I don't see any real drawbacks to making this
> > conditional on a new feature bit, that Xen can then set..
> 
> Can you elaborate?  If I run QEMU, hosting Xen, hosting Linux, and the
> virtio device is provided by QEMU, then how does Xen set the bit?

You would run QEMU with the appropriate flag. E.g.
-global virtio-pci,use_platform_dma=on

> Similarly, how would Xen set the bit for a real physical device?
> 
> 
> --Andy

There's no need to set bits for physical devices I think: from security
point of view, using them from a VM isn't very different from using them
from host.



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

Re: [PATCH] KVM: x86: emulate: correct page fault error code for NoWrite instructions

2015-11-19 Thread Nadav Amit

Wanpeng Li  wrote:

> 2015-11-20 10:52 GMT+08:00 Wanpeng Li :
>> Hi Paolo,
>> 2015-02-09 17:03 GMT+08:00 Paolo Bonzini :
>>> NoWrite instructions (e.g. cmp or test) never set the "write access"
>>> bit in the error code, even if one of the operands is treated as a
>>> destination.
>> 
>> Sorry to reply to an old commit, btw, could you point out where in SDM
>> describe above?
> 
> I see.

I don’t understand whether you still need my help, so to clarify: on a
page-fault the error code should indicate whether the access was due to a
write access. Previously KVM marked it as “write access” for instructions
such as test and cmp that do not perform write.

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

Re: [PATCH v3 0/3] virtio DMA API core stuff

On Tue, Oct 27, 2015 at 11:38:57PM -0700, Andy Lutomirski wrote:
> This switches virtio to use the DMA API unconditionally.  I'm sure
> it breaks things, but it seems to work on x86 using virtio-pci, with
> and without Xen, and using both the modern 1.0 variant and the
> legacy variant.

So thinking hard about it, I don't see any real drawbacks to making this
conditional on a new feature bit, that Xen can then set.

As a bonus, host can distinguish between old and new guests using the
feature bit, even though making driver *control* whether IOMMU is
bypassed makes userspace drivers unsafe, so might not be a good idea.

A tiny bit more code but not by much, and we clearly won't
be breaking anything that's not already broken,
and we will be able to drop the extra code later
if we think it's a good idea.

I'll run this by the virtio TC on OASIS next week so we
can reserve a feature bit.

> Changes from v2:
>  - Fix really embarrassing bug.  This version actually works.
> 
> Changes from v1:
>  - Fix an endian conversion error causing a BUG to hit.
>  - Fix a DMA ordering issue (swiotlb=force works now).
>  - Minor cleanups.
> 
> Andy Lutomirski (3):
>   virtio_net: Stop doing DMA from the stack
>   virtio_ring: Support DMA APIs
>   virtio_pci: Use the DMA API
> 
>  drivers/net/virtio_net.c   |  53 +++
>  drivers/virtio/Kconfig |   2 +-
>  drivers/virtio/virtio_pci_common.h |   3 +-
>  drivers/virtio/virtio_pci_legacy.c |  19 +++-
>  drivers/virtio/virtio_pci_modern.c |  34 +--
>  drivers/virtio/virtio_ring.c   | 187 
> ++---
>  tools/virtio/linux/dma-mapping.h   |  17 
>  7 files changed, 246 insertions(+), 69 deletions(-)
>  create mode 100644 tools/virtio/linux/dma-mapping.h
> 
> -- 
> 2.4.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 09/13] KVM: arm/arm64: vgic: support irqfd injection of a mapped IRQ

Up to now irqfd injection was only possible for unmapped IRQ. This
patch adds support for injecting mapped interrupts.

Signed-off-by: Eric Auger 
---
 virt/kvm/arm/vgic.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index dba8eb6..e96f79e 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2476,13 +2476,21 @@ int kvm_irq_map_chip_pin(struct kvm *kvm, unsigned 
irqchip, unsigned pin)
 int kvm_set_irq(struct kvm *kvm, int irq_source_id,
u32 irq, int level, bool line_status)
 {
+   struct vgic_dist *dist = >arch.vgic;
unsigned int spi = irq + VGIC_NR_PRIVATE_IRQS;
+   struct irq_phys_map *map;
+   int vcpu_id;
 
trace_kvm_set_irq(irq, level, irq_source_id);
 
BUG_ON(!vgic_initialized(kvm));
 
-   return kvm_vgic_inject_irq(kvm, 0, spi, level);
+   vcpu_id = dist->irq_spi_cpu[irq];
+   map = vgic_irq_map_search(kvm_get_vcpu(kvm, vcpu_id), spi);
+   if (!map)
+   return kvm_vgic_inject_irq(kvm, vcpu_id, spi, level);
+   else
+   return kvm_vgic_inject_mapped_irq(kvm, vcpu_id, map, level);
 }
 
 /* MSI not implemented yet */
-- 
1.9.1

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

Re: [patch 1/2] KVM: fix error handling in kvm_create_vm_debugfs()

On 11/19/2015 01:40 PM, Dan Carpenter wrote:
> The "goto out_err" is buggy because we forgot to set the return code.
> 
> The other issue is that if the kmalloc() fails, we should remove the
> debugfs directory before returning.
> 
> Fixes: 7805f53a85ec ('KVM: Create debugfs dir and stat files for each VM')
> Signed-off-by: Dan Carpenter 

Dan

this patch is not yet part of the kvm repo. It was posted for review
and I added it on the kvms390 linux next branch. So I assume you have gotten
that patch via next?


Given that there might be more potential feedback and
given that I will respin my next branch (I have no consumers, so my next
branch is rebased from time to time), and then the patch will go
from my tree via Paolos kvm tree into Linus tree, I would prefer to fold in
your patches and adopt the commit message to give you some credit.
ok with you?

Christian
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 1ee2f73..f62621f 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -571,12 +571,14 @@ static int kvm_create_vm_debugfs(struct kvm *kvm)
>   snprintf(dir_name, sizeof(dir_name), "%d", current->pid);
>   kvm->debugfs_dentry = debugfs_create_dir(dir_name, kvm_debugfs_dir);
>   if (!kvm->debugfs_dentry)
> - goto out_err;
> + return -ENOMEM;
> 
>   kvm->debugfs_data = kmalloc(sizeof(*kvm->debugfs_data) *
>   kvm_debugfs_num_entries, GFP_KERNEL);
> - if (!kvm->debugfs_data)
> - return -ENOMEM;
> + if (!kvm->debugfs_data) {
> + r = -ENOMEM;
> + goto err_remove_dir;
> + }
> 
>   for (p = debugfs_entries; p->name; p++, i++) {
>   stat_data = kzalloc(sizeof(*stat_data), GFP_KERNEL);
> @@ -600,13 +602,15 @@ static int kvm_create_vm_debugfs(struct kvm *kvm)
>   return r;
> 
>  out_err_clean:
> - debugfs_remove_recursive(kvm->debugfs_dentry);
>   kfree(stat_data);
>   for (i--; i >= 0; i--)
>   kfree(kvm->debugfs_data[i]);
> 
>   kfree(kvm->debugfs_data);
> -out_err:
> +
> +err_remove_dir:
> + debugfs_remove_recursive(kvm->debugfs_dentry);
> +
>   return r;
>  }
> 

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

[PATCH] KVM: arm/arm64: vgic: leave the LR active state on GICD_ICENABLERn access

Currently on clear-enable MMIO we retire the corresponding LR whatever
its state. More precisely we do not sync ACTIVE state but we erase the LR
state.

In case of a forwarded IRQ, the physical IRQ source is also erased meaning
the physical IRQ will never be deactivated. In case of a non forwarded
IRQ, the LR can be reused (since the state was reset) and the guest can
deactivate an IRQ that is not marked in the LR anymore.

This patch adds a parameter to vgic_retire_lr that makes possible to select
the type of the LR that must be retired.  unqueue will retire/sync all LR's
while disable will leave the active LR's.

Signed-off-by: Eric Auger 
---
 virt/kvm/arm/vgic.c | 45 +++--
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 5335383..bc30d93 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -105,7 +105,7 @@
 #include "vgic.h"
 
 static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu);
-static void vgic_retire_lr(int lr_nr, struct kvm_vcpu *vcpu);
+static void vgic_retire_lr(int lr_nr, struct kvm_vcpu *vcpu, unsigned state);
 static struct vgic_lr vgic_get_lr(const struct kvm_vcpu *vcpu, int lr);
 static void vgic_set_lr(struct kvm_vcpu *vcpu, int lr, struct vgic_lr lr_desc);
 static u64 vgic_get_elrsr(struct kvm_vcpu *vcpu);
@@ -713,18 +713,10 @@ void vgic_unqueue_irqs(struct kvm_vcpu *vcpu)
add_sgi_source(vcpu, lr.irq, lr.source);
 
/*
-* If the LR holds an active (10) or a pending and active (11)
-* interrupt then move the active state to the
-* distributor tracking bit.
+* retire pending, active, active and pending LR's and
+* sync their state back to the distributor
 */
-   if (lr.state & LR_STATE_ACTIVE)
-   vgic_irq_set_active(vcpu, lr.irq);
-
-   /*
-* Reestablish the pending state on the distributor and the
-* CPU interface and mark the LR as free for other use.
-*/
-   vgic_retire_lr(i, vcpu);
+   vgic_retire_lr(i, vcpu, LR_STATE_ACTIVE | LR_STATE_PENDING);
 
/* Finally update the VGIC state. */
vgic_update_state(vcpu->kvm);
@@ -1077,22 +1069,25 @@ static inline void vgic_enable(struct kvm_vcpu *vcpu)
vgic_ops->enable(vcpu);
 }
 
-static void vgic_retire_lr(int lr_nr, struct kvm_vcpu *vcpu)
+static void vgic_retire_lr(int lr_nr, struct kvm_vcpu *vcpu, unsigned state)
 {
struct vgic_lr vlr = vgic_get_lr(vcpu, lr_nr);
 
-   vgic_irq_clear_queued(vcpu, vlr.irq);
+   if (vlr.state & LR_STATE_ACTIVE & state) {
+   vgic_irq_set_active(vcpu, vlr.irq);
+   vlr.state &= ~LR_STATE_ACTIVE;
+   }
 
-   /*
-* We must transfer the pending state back to the distributor before
-* retiring the LR, otherwise we may loose edge-triggered interrupts.
-*/
-   if (vlr.state & LR_STATE_PENDING) {
+   if (vlr.state & LR_STATE_PENDING & state) {
vgic_dist_irq_set_pending(vcpu, vlr.irq);
-   vlr.hwirq = 0;
+   vlr.state &= ~LR_STATE_PENDING;
}
 
-   vlr.state = 0;
+   if (!(vlr.state & LR_STATE_MASK)) {
+   vlr.hwirq = 0;
+   vlr.state = 0;
+   vgic_irq_clear_queued(vcpu, vlr.irq);
+   }
vgic_set_lr(vcpu, lr_nr, vlr);
 }
 
@@ -1114,8 +1109,14 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu 
*vcpu)
for_each_clear_bit(lr, elrsr_ptr, vgic->nr_lr) {
struct vgic_lr vlr = vgic_get_lr(vcpu, lr);
 
+   /*
+* retire pending only LR's and sync their state
+* back to the distributor. Active LR's cannot be
+* retired since the guest will attempt to deactivate
+* the IRQ.
+*/
if (!vgic_irq_is_enabled(vcpu, vlr.irq))
-   vgic_retire_lr(lr, vcpu);
+   vgic_retire_lr(lr, vcpu, LR_STATE_PENDING);
}
 }
 
-- 
1.9.1

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

[PATCH v4 07/13] VFIO: platform: add irq bypass producer management

This patch populates the IRQ bypass callacks:
- stop/start producer simply consist in disabling/enabling the host irq
- add/del consumer: basically set the automasked flag to false/true

Signed-off-by: Eric Auger 

---
v3 -> v4:
- use vfio_platform_set_forwarded in place of vfio_platform_set_automasked
- use vfio_platform_irq_is_active and its returned value
- reword comments related to auto/explicit VFIO masking

v2 -> v3:
- vfio_platform_irq_bypass_add_consumer now returns an error in case
  the IRQ is recognized as active
- active boolean not set anymore
- do not VFIO mask the IRQ anymore on unforward

v1 -> v2:
- device handle caching in vfio_platform_device is introduced in a
  separate patch
- use container_of
---
 drivers/vfio/platform/vfio_platform_irq.c | 28 +++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/platform/vfio_platform_irq.c 
b/drivers/vfio/platform/vfio_platform_irq.c
index cb7d5e9..e486c32 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -224,23 +224,49 @@ out:
 
 static void vfio_platform_irq_bypass_stop(struct irq_bypass_producer *prod)
 {
+   disable_irq(prod->irq);
 }
 
 static void vfio_platform_irq_bypass_start(struct irq_bypass_producer *prod)
 {
+   enable_irq(prod->irq);
 }
 
 static int vfio_platform_irq_bypass_add_consumer(
struct irq_bypass_producer *prod,
struct irq_bypass_consumer *cons)
 {
-   return 0;
+   struct vfio_platform_irq *irq =
+   container_of(prod, struct vfio_platform_irq, producer);
+   int ret;
+
+   /*
+* if the IRQ is active at irqchip level or masked by VFIO
+* this means the host IRQ may be under injection and
+* it is not safe to change the forwarding state at that time.
+* It is not possible to discriminate VFIO explicit masking
+* from VFIO auto-masking, leading to possible false detection of
+* IRQ activity. It is up to the user-space to avoid VFIO explicit
+* masking situation. In case of failure, no retry is attempted and
+* bypass setup simply fails.
+*/
+   ret = vfio_platform_irq_is_active(irq);
+   if (ret < 0)
+   return ret;
+   else if (ret == 1)
+   return -EAGAIN;
+
+   return vfio_platform_set_forwarded(irq, true);
 }
 
 static void vfio_platform_irq_bypass_del_consumer(
struct irq_bypass_producer *prod,
struct irq_bypass_consumer *cons)
 {
+   struct vfio_platform_irq *irq =
+   container_of(prod, struct vfio_platform_irq, producer);
+
+   vfio_platform_set_forwarded(irq, false);
 }
 
 static int vfio_set_trigger(struct vfio_platform_device *vdev, int index,
-- 
1.9.1

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

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

2015-11-19 Thread Gerd Hoffmann

  Hi,

> That was indeed the ISR field. Fixing that makes seabios reach the same point 
> as
> legacy virtio before failing.
> 
> I don't see the original correspondence about seabios failures you've 
> reported, if
> you want to forward them over we can look at it further.

It was a few months back, when I posted the seabios patches for kvmtool
to both seabios and kvm lists.

Issue #1 is that kvmtool adds a bunch of kernel command line options,
not only for stuff like rootfs configuration, but also to force specific
things the kernel fails to autodetect (or to speedup boot by
shortcutting hardware probing).  Among them is "pci=conf1", without that
the kernel doesn't find a pci bus and therefore also doesn't find the
virtio-{blk,net} devices.

So, when booting with seabios and let grub or another boot loader load
the kernel from the guest disk image those kernel arguments are not
there.  Of course you can boot the image with qemu, add "pci=conf1" to
grub.cfg (maybe others are required too, don't remember exactly), then
try again with kvmtool.  That gets the boot one step further and leads
to ...

Issue #2: virtio kernel drivers fail initialize the virtio devices.
I suspect virtio device reset is not implemented properly and because of
that the state of the device as left by seabios confuses the kernel
driver.  Didn't check that in detail though.

cheers,
  Gerd


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

[PATCH v4 12/13] KVM: arm/arm64: vgic: implement clear pending for non shared mapped IRQ

This patch implements the clear of a pending non shared mapped IRQ.
In case of an edge IRQ, we deactivate the physical IRQ that will never
be deactivated by the guest. In case of a level sensitive IRQ we check
the level of the input signal. If it is asserted we leave the virtual
IRQ pending. In the opposite, we remove the pending state and deactivate
the IRQ.

Signed-off-by: Eric Auger 
---
 virt/kvm/arm/vgic.c | 34 --
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 98ae15f..4be5972 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -550,20 +550,50 @@ bool vgic_handle_clear_pending_reg(struct kvm *kvm,
   phys_addr_t offset, int vcpu_id)
 {
u32 *level_active;
-   u32 *reg, orig;
+   u32 *reg, orig, cleared;
int mode = ACCESS_READ_VALUE | ACCESS_WRITE_CLEARBIT;
+   unsigned int i;
struct vgic_dist *dist = >arch.vgic;
+   struct kvm_vcpu *vcpu = kvm_get_vcpu(kvm, vcpu_id);
 
reg = vgic_bitmap_get_reg(>irq_pending, vcpu_id, offset);
orig = *reg;
vgic_reg_access(mmio, reg, offset, mode);
if (mmio->is_write) {
-   /* Re-set level triggered level-active interrupts */
level_active = vgic_bitmap_get_reg(>irq_level,
  vcpu_id, offset);
+   cleared = orig ^ *reg;
+
+   /* Re-set level triggered level-active interrupts */
reg = vgic_bitmap_get_reg(>irq_pending, vcpu_id, offset);
*reg |= *level_active;
 
+   for (i = 0; i < 32; i++) {
+   struct irq_phys_map *map;
+   bool phys_pending;
+   unsigned int irq_num;
+
+   if (!(cleared && (1 << i)))
+   continue;
+   irq_num = (offset * 8) + i;
+   map = vgic_irq_map_search(vcpu, irq_num);
+   if (!map || (map && map->shared))
+   continue;
+   /* check whether the signal is asserted */
+   irq_get_irqchip_state(map->irq,
+ IRQCHIP_STATE_PENDING,
+ _pending);
+   if (!vgic_irq_is_edge(vcpu, irq_num) && phys_pending) {
+   vgic_dist_irq_set_pending(vcpu, irq_num);
+   continue;
+   }
+
+   vgic_dist_irq_clear_pending(vcpu, irq_num);
+   irq_set_irqchip_state(map->irq,
+ IRQCHIP_STATE_ACTIVE,
+ false);
+   }
+
/* Ignore writes to SGIs */
if (offset < 2) {
*reg &= ~0x;
-- 
1.9.1

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

[PATCH v4 13/13] KVM: arm/arm64: implement IRQ bypass consumer functions

Implement IRQ bypass callbacks for arm/arm64 IRQ forwarding:
- kvm_arch_irq_bypass_add_producer: perform VGIC/irqchip
  settings for forwarding
- kvm_arch_irq_bypass_del_producer: same for inverse operation
- kvm_arch_irq_bypass_stop: halt guest execution
- kvm_arch_irq_bypass_start: resume guest execution

and set CONFIG_HAVE_KVM_IRQ_BYPASS for arm/arm64.

Signed-off-by: Eric Auger 

---
v3 -> v4:
- kvm_set/unset_forward now takes the virtual IRQ ID as a parameter

v2 -> v3:
- kvm_arch_irq_bypass_resume renamed into kvm_arch_irq_bypass_start
- kvm_vgic_unset_forward does not take the active bool param anymore
- kvm_arch_irq_bypass_add_producer now returns an error value
- remove kvm_arch_irq_bypass_update

v1 -> v2:
- struct kvm_kernel_irqfd is retrieved with container_of
- function names changed
---
 arch/arm/kvm/Kconfig   |  1 +
 arch/arm/kvm/arm.c | 37 +
 arch/arm64/kvm/Kconfig |  1 +
 3 files changed, 39 insertions(+)

diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
index 73d7201..4c9a95e 100644
--- a/arch/arm/kvm/Kconfig
+++ b/arch/arm/kvm/Kconfig
@@ -34,6 +34,7 @@ config KVM
select HAVE_KVM_EVENTFD
select HAVE_KVM_IRQFD
select IRQ_BYPASS_MANAGER
+   select HAVE_KVM_IRQ_BYPASS
depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
---help---
  Support hosting virtualized guest machines.
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index eab83b2..99517bd 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -27,6 +27,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 #define CREATE_TRACE_POINTS
@@ -1165,6 +1167,41 @@ struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, 
unsigned long mpidr)
return NULL;
 }
 
+int kvm_arch_irq_bypass_add_producer(struct irq_bypass_consumer *cons,
+ struct irq_bypass_producer *prod)
+{
+   struct kvm_kernel_irqfd *irqfd =
+   container_of(cons, struct kvm_kernel_irqfd, consumer);
+
+   return kvm_vgic_set_forward(irqfd->kvm, prod->irq,
+   irqfd->gsi + VGIC_NR_PRIVATE_IRQS);
+}
+void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons,
+ struct irq_bypass_producer *prod)
+{
+   struct kvm_kernel_irqfd *irqfd =
+   container_of(cons, struct kvm_kernel_irqfd, consumer);
+
+   kvm_vgic_unset_forward(irqfd->kvm, prod->irq,
+  irqfd->gsi + VGIC_NR_PRIVATE_IRQS);
+}
+
+void kvm_arch_irq_bypass_stop(struct irq_bypass_consumer *cons)
+{
+   struct kvm_kernel_irqfd *irqfd =
+   container_of(cons, struct kvm_kernel_irqfd, consumer);
+
+   kvm_arm_halt_guest(irqfd->kvm);
+}
+
+void kvm_arch_irq_bypass_start(struct irq_bypass_consumer *cons)
+{
+   struct kvm_kernel_irqfd *irqfd =
+   container_of(cons, struct kvm_kernel_irqfd, consumer);
+
+   kvm_arm_resume_guest(irqfd->kvm);
+}
+
 /**
  * Initialize Hyp-mode and memory mappings on all CPUs.
  */
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index 64f460d..f6829af 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -38,6 +38,7 @@ config KVM
select HAVE_KVM_IRQFD
select KVM_ARM_VGIC_V3
select IRQ_BYPASS_MANAGER
+   select HAVE_KVM_IRQ_BYPASS
---help---
  Support hosting virtualized guest machines.
  We don't support KVM with 16K page tables yet, due to the multiple
-- 
1.9.1

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

Re: [Qemu-devel] [PATCH v3 1/3] target-i386: add pkeys support for cpuid handling

2015-11-19 Thread Eduardo Habkost

On Thu, Nov 19, 2015 at 12:10:49PM +0100, Paolo Bonzini wrote:
> 
> 
> On 19/11/2015 07:36, Han, Huaitong wrote:
> > I understand it has always been that QEMU considers the feature of
> >  cpuid_7_0_ecx_feature_name as migratable. If the feature is
> >  unmigratable, it will been added to unmigratable_flags.
> > 
> > A series of patches do complete a full function, moving
> >  cpuid_7_0_ecx_feature_name to 2/3 patch may make 2/3 patch look
> > better, but make 1/3 patch look somewhat incomplete.
> > 
> > Maybe it is a solution that adding the feature to unmigratable_flags in
> > 1/3 patch, and deleting unmigratable_flags in 2/3 patch, but I think it
> > is pointless.
> 
> Or just squash everything together.  After all we're talking of
> 
>  4 files changed, 55 insertions(+), 1 deletion(-)
> 
> It's not a large patch.

It makes sense. Adding the state to X86CPU (2/3) is useful only
if we migrate it (3/3), and adding the feature names (1/3) is
useful only if we can handle the new state.

I will squash everything together when applying, in case there's
no new version.

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

[PATCH v4 11/13] KVM: arm/arm64: vgic: implement clear active for non shared mapped IRQ

When disabling a non shared mapped IRQs, we must manually deactivate
the corresponding physical IRQ on top of removing the active state
from the distributor.

Signed-off-by: Eric Auger 
---
 virt/kvm/arm/vgic.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index bd500b4..98ae15f 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -604,14 +604,34 @@ bool vgic_handle_clear_active_reg(struct kvm *kvm,
  struct kvm_exit_mmio *mmio,
  phys_addr_t offset, int vcpu_id)
 {
-   u32 *reg;
+   u32 *reg, orig, cleared;
struct vgic_dist *dist = >arch.vgic;
+   struct kvm_vcpu *vcpu = kvm_get_vcpu(kvm, vcpu_id);
+   unsigned int i;
 
reg = vgic_bitmap_get_reg(>irq_active, vcpu_id, offset);
+   orig = *reg;
vgic_reg_access(mmio, reg, offset,
ACCESS_READ_VALUE | ACCESS_WRITE_CLEARBIT);
 
if (mmio->is_write) {
+   cleared = orig ^ *reg;
+
+   for (i = 0; i < 32; i++) {
+   struct irq_phys_map *map;
+   unsigned int irq_num;
+
+   if (!(cleared && (1 << i)))
+   continue;
+   irq_num = (offset * 8) + i;
+   map = vgic_irq_map_search(vcpu, irq_num);
+   if (!map || (map && map->shared))
+   continue;
+   irq_set_irqchip_state(map->irq,
+ IRQCHIP_STATE_ACTIVE,
+ false);
+   }
+
vgic_update_state(kvm);
return true;
}
-- 
1.9.1

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

[PATCH v4 04/13] VFIO: platform: single handler using function pointer

A single handler now is registered whatever the use case: automasked
or not. A function pointer is set according to the wished behavior
and the handler calls this function.

The irq lock is taken/released in the root handler. eventfd_signal can
be called in regions not allowed to sleep.

Signed-off-by: Eric Auger 

---

v4: creation
---
 drivers/vfio/platform/vfio_platform_irq.c | 21 +++--
 drivers/vfio/platform/vfio_platform_private.h |  1 +
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/vfio/platform/vfio_platform_irq.c 
b/drivers/vfio/platform/vfio_platform_irq.c
index d4462fd..c58e0ad 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -148,11 +148,8 @@ static int vfio_platform_set_irq_unmask(struct 
vfio_platform_device *vdev,
 static irqreturn_t vfio_automasked_irq_handler(int irq, void *dev_id)
 {
struct vfio_platform_irq *irq_ctx = dev_id;
-   unsigned long flags;
int ret = IRQ_NONE;
 
-   spin_lock_irqsave(_ctx->lock, flags);
-
if (!irq_ctx->masked) {
ret = IRQ_HANDLED;
 
@@ -161,8 +158,6 @@ static irqreturn_t vfio_automasked_irq_handler(int irq, 
void *dev_id)
irq_ctx->masked = true;
}
 
-   spin_unlock_irqrestore(_ctx->lock, flags);
-
if (ret == IRQ_HANDLED)
eventfd_signal(irq_ctx->trigger, 1);
 
@@ -178,6 +173,19 @@ static irqreturn_t vfio_irq_handler(int irq, void *dev_id)
return IRQ_HANDLED;
 }
 
+static irqreturn_t vfio_handler(int irq, void *dev_id)
+{
+   struct vfio_platform_irq *irq_ctx = dev_id;
+   unsigned long flags;
+   irqreturn_t ret;
+
+   spin_lock_irqsave(_ctx->lock, flags);
+   ret = irq_ctx->handler(irq, dev_id);
+   spin_unlock_irqrestore(_ctx->lock, flags);
+
+   return ret;
+}
+
 static void vfio_platform_irq_bypass_stop(struct irq_bypass_producer *prod)
 {
 }
@@ -230,9 +238,10 @@ static int vfio_set_trigger(struct vfio_platform_device 
*vdev, int index,
}
 
irq->trigger = trigger;
+   irq->handler = handler;
 
irq_set_status_flags(irq->hwirq, IRQ_NOAUTOEN);
-   ret = request_irq(irq->hwirq, handler, 0, irq->name, irq);
+   ret = request_irq(irq->hwirq, vfio_handler, 0, irq->name, irq);
if (ret) {
kfree(irq->name);
eventfd_ctx_put(trigger);
diff --git a/drivers/vfio/platform/vfio_platform_private.h 
b/drivers/vfio/platform/vfio_platform_private.h
index 55e8043..8a8c050 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -40,6 +40,7 @@ struct vfio_platform_irq {
struct virqfd   *mask;
struct irq_bypass_producer producer;
boolforwarded;
+   irqreturn_t (*handler)(int irq, void *dev_id);
 };
 
 struct vfio_platform_region {
-- 
1.9.1

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

[PATCH v4 10/13] KVM: arm/arm64: vgic: forwarding control

Implements kvm_vgic_[set|unset]_forward.

Handle low-level VGIC programming: physical IRQ/guest IRQ mapping,
list register cleanup, VGIC state machine. Also interacts with
the irqchip.

Signed-off-by: Eric Auger 

---
v3 -> v4:
- use the target vCPU in set/unset_forward
- rebase after removal of vgic_irq_lr_map
- clarify set/unset_forward comments,
- host_irq renamed into irq and virt_irq (SPI ID) now used in
  place of SPI index (former guest_irq)
- replaced BUG_ON by WARN_ON
- simplify unset_forward implementation by using vgic_unqueue_irqs
- handle edge mapped unshared IRQs

v2 -> v3:
- on unforward, we do not compute & output the active state anymore.
  This means if the unforward happens while the physical IRQ is
  active, we will not VFIO mask the IRQ while deactiving it. If a
  new physical IRQ hits, the corresponding virtual IRQ might not
  be injected (hence lost) due to VGIC state machine.

bypass rfc v2:
- use irq_set_vcpu_affinity API
- use irq_set_irqchip_state instead of chip->irq_eoi

bypass rfc:
- rename kvm_arch_{set|unset}_forward into
  kvm_vgic_{set|unset}_forward. Remove __KVM_HAVE_ARCH_HALT_GUEST.
  The function is bound to be called by ARM code only.

v4 -> v5:
- fix arm64 compilation issues, ie. also defines
  __KVM_HAVE_ARCH_HALT_GUEST for arm64

v3 -> v4:
- code originally located in kvm_vfio_arm.c
- kvm_arch_vfio_{set|unset}_forward renamed into
  kvm_arch_{set|unset}_forward
- split into 2 functions (set/unset) since unset does not fail anymore
- unset can be invoked at whatever time. Extra care is taken to handle
  transition in VGIC state machine, LR cleanup, ...

v2 -> v3:
- renaming of kvm_arch_set_fwd_state into kvm_arch_vfio_set_forward
- takes a bool arg instead of kvm_fwd_irq_action enum
- removal of KVM_VFIO_IRQ_CLEANUP
- platform device check now happens here
- more precise errors returned
- irq_eoi handled externally to this patch (VGIC)
- correct enable_irq bug done twice
- reword the commit message
- correct check of platform_bus_type
- use raw_spin_lock_irqsave and check the validity of the handler
---
 include/kvm/arm_vgic.h |   5 +++
 virt/kvm/arm/vgic.c| 118 +
 2 files changed, 123 insertions(+)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 9bf6a30..8090ab4 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -368,4 +368,9 @@ static inline int vgic_v3_probe(struct device_node 
*vgic_node,
 }
 #endif
 
+int kvm_vgic_set_forward(struct kvm *kvm, unsigned int irq,
+unsigned int virt_irq);
+void kvm_vgic_unset_forward(struct kvm *kvm, unsigned int irq,
+   unsigned int virt_irq);
+
 #endif
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index e96f79e..bd500b4 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2500,3 +2500,121 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
 {
return 0;
 }
+
+/**
+ * kvm_vgic_set_forward - Set IRQ forwarding
+ *
+ * @kvm: handle to the VM
+ * @irq: the host linux IRQ
+ * @virt_irq: the guest SPI ID
+ *
+ * This function is supposed to be called only if the IRQ
+ * is not in progress: ie. not active at GIC level and not
+ * currently under injection in the guest. The physical IRQ must
+ * also be disabled and all vCPUs must have been exited and
+ * prevented from being re-entered.
+ */
+int kvm_vgic_set_forward(struct kvm *kvm, unsigned int irq,
+unsigned int virt_irq)
+{
+   struct vgic_dist *dist = >arch.vgic;
+   struct kvm_vcpu *vcpu;
+   struct irq_phys_map *map;
+   int cpu_id;
+
+   kvm_debug("%s irq=%d virt_irq=%d\n", __func__, irq, virt_irq);
+
+   cpu_id = dist->irq_spi_cpu[virt_irq - VGIC_NR_PRIVATE_IRQS];
+   vcpu = kvm_get_vcpu(kvm, cpu_id);
+   if (!vcpu)
+   return 0;
+   /*
+* let's tell the irqchip driver that after this function
+* returns, a new occurrence of that physical irq will be handled
+* as a forwarded IRQ, ie. the host will only perform priority
+* drop but will not deactivate the physical IRQ: guest will
+*/
+   irq_set_vcpu_affinity(irq, vcpu);
+
+   /*
+* let's program the vgic so that after this function returns
+* any subsequent virt_irq injection will be considered as
+* forwarded and LR will be programmed with HWbit set
+*/
+   map = kvm_vgic_map_phys_irq(vcpu, virt_irq, irq, false);
+
+   return !map;
+}
+
+/**
+ * kvm_vgic_unset_forward - Unset IRQ forwarding
+ *
+ * @kvm: handle to the VM
+ * @irq: host Linux IRQ number
+ * @virt_irq: virtual SPI ID
+ *
+ * This function must be called when the host irq is disabled
+ * and all vCPUs have been exited and prevented from being re-entered.
+ */
+void kvm_vgic_unset_forward(struct kvm *kvm,
+ unsigned int irq,
+ unsigned int virt_irq)
+{
+

[PATCH v4 08/13] KVM: arm/arm64: vgic: adapt state machine for non shared mapped interrupts

From: Marc Zyngier 

So far, the only user of the mapped interrupt facility was the timer:
the physical distributor active state needed to be context-switched
for each vcpu, as the device is shared across all vcpus.

This patch allows to indicate whether a mapped IRQ originates from a
device shared between several VMs (typically the architected timer) or
from a device assigned to a single VM.

A new "shared" flag is added to irq_phys_map and passed to the
mapping function.

the VGIC state machine is adapted to support the non shared mapped IRQs:
- only can be sampled when it is pending
- when queueing the IRQ (programming the LR), the pending state is
  removed as for edge sensitive IRQs
- queued state is not modelled. Level state is not modelled
- its injection with high level always is valid since steming from the HW.

Signed-off-by: Eric Auger 
Signed-off-by: Marc Zyngier 

---

v3 -> v4:
- reword the patch title
- rebase on [PATCH v2 0/8] Rework architected timer and forwarded IRQs
  handling
- renamed shared_hw into non_shared_mapped_irq;
- handle edge-sensitive unshared mapped IRQs
- vgic_validate_injection rejects mapped unshared edge with
  level == 0
---
 include/kvm/arm_vgic.h|  3 ++-
 virt/kvm/arm/arch_timer.c |  2 +-
 virt/kvm/arm/vgic.c   | 37 -
 3 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 9c747cb..9bf6a30 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -158,6 +158,7 @@ struct irq_phys_map {
u32 virt_irq;
u32 phys_irq;
u32 irq;
+   boolshared;
 };
 
 struct irq_phys_map_entry {
@@ -344,7 +345,7 @@ void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg);
 int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu);
 int kvm_vgic_vcpu_active_irq(struct kvm_vcpu *vcpu);
 struct irq_phys_map *kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu,
-  int virt_irq, int irq);
+  int virt_irq, int irq, bool shared);
 int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, struct irq_phys_map *map);
 
 #define irqchip_in_kernel(k)   (!!((k)->arch.vgic.in_kernel))
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 21a0ab2..9eea751 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -289,7 +289,7 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu,
 * Tell the VGIC that the virtual interrupt is tied to a
 * physical interrupt. We do that once per VCPU.
 */
-   map = kvm_vgic_map_phys_irq(vcpu, irq->irq, host_vtimer_irq);
+   map = kvm_vgic_map_phys_irq(vcpu, irq->irq, host_vtimer_irq, true);
if (WARN_ON(IS_ERR(map)))
return PTR_ERR(map);
 
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index bc30d93..dba8eb6 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -410,7 +410,11 @@ void vgic_cpu_irq_clear(struct kvm_vcpu *vcpu, int irq)
 
 static bool vgic_can_sample_irq(struct kvm_vcpu *vcpu, int irq)
 {
-   return !vgic_irq_is_queued(vcpu, irq);
+   struct irq_phys_map *map = vgic_irq_map_search(vcpu, irq);
+   bool non_shared_mapped_irq = map && !map->shared;
+
+   return !vgic_irq_is_queued(vcpu, irq) ||
+   (non_shared_mapped_irq && vgic_dist_irq_is_pending(vcpu, irq));
 }
 
 /**
@@ -1205,11 +1209,14 @@ bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 
sgi_source_id, int irq)
 
 static bool vgic_queue_hwirq(struct kvm_vcpu *vcpu, int irq)
 {
+   struct irq_phys_map *map = vgic_irq_map_search(vcpu, irq);
+   bool non_shared_mapped_irq = map && !map->shared;
+
if (!vgic_can_sample_irq(vcpu, irq))
return true; /* level interrupt, already queued */
 
if (vgic_queue_irq(vcpu, 0, irq)) {
-   if (vgic_irq_is_edge(vcpu, irq)) {
+   if (vgic_irq_is_edge(vcpu, irq) || non_shared_mapped_irq) {
vgic_dist_irq_clear_pending(vcpu, irq);
vgic_cpu_irq_clear(vcpu, irq);
} else {
@@ -1292,6 +1299,8 @@ static int process_queued_irq(struct kvm_vcpu *vcpu,
   int lr, struct vgic_lr vlr)
 {
int pending = 0;
+   struct irq_phys_map *map = vgic_irq_map_search(vcpu, vlr.irq);
+   bool non_shared_mapped_irq = map && !map->shared;
 
/*
 * If the IRQ was EOIed (called from vgic_process_maintenance) or it
@@ -1312,8 +1321,7 @@ static int process_queued_irq(struct kvm_vcpu *vcpu,
vgic_irq_clear_queued(vcpu, vlr.irq);
 
/* Any additional pending interrupt? */
-   if (vgic_irq_is_edge(vcpu, vlr.irq)) {
-   BUG_ON(!(vlr.state & LR_HW));
+   if (vgic_irq_is_edge(vcpu, vlr.irq) || non_shared_mapped_irq) {

[PATCH v4 06/13] VFIO: platform: add vfio_platform_irq_is_active

This function returns whether the IRQ is active at irqchip level or
VFIO masked. If either is true, the IRQ is considered active.
Currently there is no way to differentiate userspace masked IRQ from
automasked IRQ. There might be false detection of activity. However
it is currently acceptable to have false detection.

Signed-off-by: Eric Auger 

---

v3 -> v4:
- rename vfio_platform_is_active into vfio_platform_irq_is_active
- remove BUG_ON and return the error if irq_get_irqchip_state fails
---
 drivers/vfio/platform/vfio_platform_irq.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/vfio/platform/vfio_platform_irq.c 
b/drivers/vfio/platform/vfio_platform_irq.c
index 257200b..cb7d5e9 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -203,6 +203,25 @@ static int vfio_platform_set_forwarded(struct 
vfio_platform_irq *irq,
return 0;
 }
 
+static int vfio_platform_irq_is_active(struct vfio_platform_irq *irq)
+{
+   unsigned long flags;
+   bool active;
+   int ret;
+
+   spin_lock_irqsave(>lock, flags);
+
+   ret = irq_get_irqchip_state(irq->hwirq, IRQCHIP_STATE_ACTIVE, );
+   if (ret)
+   goto out;
+
+   ret = active || irq->masked;
+
+out:
+   spin_unlock_irqrestore(>lock, flags);
+   return ret;
+}
+
 static void vfio_platform_irq_bypass_stop(struct irq_bypass_producer *prod)
 {
 }
-- 
1.9.1

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

[PATCH v4 01/13] KVM: arm/arm64: select IRQ_BYPASS_MANAGER

Select IRQ_BYPASS_MANAGER when CONFIG_KVM is set

Signed-off-by: Eric Auger 
---
 arch/arm/kvm/Kconfig   | 2 ++
 arch/arm64/kvm/Kconfig | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/arch/arm/kvm/Kconfig b/arch/arm/kvm/Kconfig
index 95a0005..73d7201 100644
--- a/arch/arm/kvm/Kconfig
+++ b/arch/arm/kvm/Kconfig
@@ -3,6 +3,7 @@
 #
 
 source "virt/kvm/Kconfig"
+source "virt/lib/Kconfig"
 
 menuconfig VIRTUALIZATION
bool "Virtualization"
@@ -32,6 +33,7 @@ config KVM
select KVM_VFIO
select HAVE_KVM_EVENTFD
select HAVE_KVM_IRQFD
+   select IRQ_BYPASS_MANAGER
depends on ARM_VIRT_EXT && ARM_LPAE && ARM_ARCH_TIMER
---help---
  Support hosting virtualized guest machines.
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index a5272c0..64f460d 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -3,6 +3,7 @@
 #
 
 source "virt/kvm/Kconfig"
+source "virt/lib/Kconfig"
 
 menuconfig VIRTUALIZATION
bool "Virtualization"
@@ -36,6 +37,7 @@ config KVM
select HAVE_KVM_EVENTFD
select HAVE_KVM_IRQFD
select KVM_ARM_VGIC_V3
+   select IRQ_BYPASS_MANAGER
---help---
  Support hosting virtualized guest machines.
  We don't support KVM with 16K page tables yet, due to the multiple
-- 
1.9.1

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

[PATCH v4 02/13] VFIO: platform: registration of a dummy IRQ bypass producer

Register a dummy producer with void callbacks

Signed-off-by: Eric Auger 

---
v3 -> v3:
- replace WARN_ON by pr_info() in case irq_bypass_register_producer
  fails

v2 -> v3:
- rename vfio_platform_irq_bypass_resume into *_start
---
 drivers/vfio/platform/vfio_platform_irq.c | 34 +++
 drivers/vfio/platform/vfio_platform_private.h |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/drivers/vfio/platform/vfio_platform_irq.c 
b/drivers/vfio/platform/vfio_platform_irq.c
index 46d4750..ab7658a 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "vfio_platform_private.h"
 
@@ -177,6 +178,27 @@ static irqreturn_t vfio_irq_handler(int irq, void *dev_id)
return IRQ_HANDLED;
 }
 
+static void vfio_platform_irq_bypass_stop(struct irq_bypass_producer *prod)
+{
+}
+
+static void vfio_platform_irq_bypass_start(struct irq_bypass_producer *prod)
+{
+}
+
+static int vfio_platform_irq_bypass_add_consumer(
+   struct irq_bypass_producer *prod,
+   struct irq_bypass_consumer *cons)
+{
+   return 0;
+}
+
+static void vfio_platform_irq_bypass_del_consumer(
+   struct irq_bypass_producer *prod,
+   struct irq_bypass_consumer *cons)
+{
+}
+
 static int vfio_set_trigger(struct vfio_platform_device *vdev, int index,
int fd, irq_handler_t handler)
 {
@@ -187,6 +209,7 @@ static int vfio_set_trigger(struct vfio_platform_device 
*vdev, int index,
if (irq->trigger) {
irq_clear_status_flags(irq->hwirq, IRQ_NOAUTOEN);
free_irq(irq->hwirq, irq);
+   irq_bypass_unregister_producer(>producer);
kfree(irq->name);
eventfd_ctx_put(irq->trigger);
irq->trigger = NULL;
@@ -217,6 +240,17 @@ static int vfio_set_trigger(struct vfio_platform_device 
*vdev, int index,
return ret;
}
 
+   irq->producer.token = (void *)trigger;
+   irq->producer.irq = irq->hwirq;
+   irq->producer.add_consumer = vfio_platform_irq_bypass_add_consumer;
+   irq->producer.del_consumer = vfio_platform_irq_bypass_del_consumer;
+   irq->producer.stop = vfio_platform_irq_bypass_stop;
+   irq->producer.start = vfio_platform_irq_bypass_start;
+   ret = irq_bypass_register_producer(>producer);
+   if (ret)
+   pr_info("irq bypass producer (token %p) registration fails: 
%d\n",
+   irq->producer.token, ret);
+
if (!irq->masked)
enable_irq(irq->hwirq);
 
diff --git a/drivers/vfio/platform/vfio_platform_private.h 
b/drivers/vfio/platform/vfio_platform_private.h
index 42816dd..979c191 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -17,6 +17,7 @@
 
 #include 
 #include 
+#include 
 
 #define VFIO_PLATFORM_OFFSET_SHIFT   40
 #define VFIO_PLATFORM_OFFSET_MASK (((u64)(1) << VFIO_PLATFORM_OFFSET_SHIFT) - 
1)
@@ -37,6 +38,7 @@ struct vfio_platform_irq {
spinlock_t  lock;
struct virqfd   *unmask;
struct virqfd   *mask;
+   struct irq_bypass_producer producer;
 };
 
 struct vfio_platform_region {
-- 
1.9.1

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

[PATCH v4 03/13] VFIO: platform: test forwarded state when selecting the IRQ handler

Add a new forwarded flag in vfio_platform_irq.  In case the IRQ
is forwarded, the VFIO platform IRQ handler does not need to
disable the IRQ anymore.

When setting the IRQ handler we now also test the forwarded state. In
case the IRQ is forwarded we select the vfio_irq_handler.

Signed-off-by: Eric Auger 

---

v2:
- add a new forwarded flag and do not use irqd_irq_forwarded anymore
---
 drivers/vfio/platform/vfio_platform_irq.c | 3 ++-
 drivers/vfio/platform/vfio_platform_private.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/platform/vfio_platform_irq.c 
b/drivers/vfio/platform/vfio_platform_irq.c
index ab7658a..d4462fd 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -265,7 +265,8 @@ static int vfio_platform_set_irq_trigger(struct 
vfio_platform_device *vdev,
struct vfio_platform_irq *irq = >irqs[index];
irq_handler_t handler;
 
-   if (vdev->irqs[index].flags & VFIO_IRQ_INFO_AUTOMASKED)
+   if (vdev->irqs[index].flags & VFIO_IRQ_INFO_AUTOMASKED &&
+   !irq->forwarded)
handler = vfio_automasked_irq_handler;
else
handler = vfio_irq_handler;
diff --git a/drivers/vfio/platform/vfio_platform_private.h 
b/drivers/vfio/platform/vfio_platform_private.h
index 979c191..55e8043 100644
--- a/drivers/vfio/platform/vfio_platform_private.h
+++ b/drivers/vfio/platform/vfio_platform_private.h
@@ -39,6 +39,7 @@ struct vfio_platform_irq {
struct virqfd   *unmask;
struct virqfd   *mask;
struct irq_bypass_producer producer;
+   boolforwarded;
 };
 
 struct vfio_platform_region {
-- 
1.9.1

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

[PATCH v4 00/13] ARM IRQ forward control based on IRQ bypass manager

This series allows to optimize the deactivation of virtual interrupts
associated to a vfio platform device IRQ. Let's call this optimization:
ARM IRQ forwarding.

Without that optimization the deactivation of the physical IRQ is done
by the host and the deactivation of the virtual IRQ, by the guest,
is trapped by KVM with a maintenance IRQ. With ARM IRQ forwarding, the
physical IRQ is automatically deactivated by the GIC interrupt controller
when the guest deactivates the virtual IRQ.

The series enables the auto-negotiation of this optimization, on ARM.
The auto-negotiation is coordinated by the IRQ bypass manager. The VFIO
platform driver and KVM/irqfd cooperate to turn that feature on.

The series implements the setup of IRQ forwarding and adapt the existing
VGIC state machine so that it works.

When the IRQ is forwarded, the VFIO platform driver does not need to
mask the physical IRQ anymore before signaling the eventfd. Indeed
genirq lowers the running priority, enabling other physical IRQ to hit
except that one.

The injection still is based on irqfd triggering. The only impact on
irqfd process is the resamplefd is not triggered anymore for level
sensitive IRQs since deactivation is not trapped by KVM.

More backgroung on ARM IRQ forwarding in the text below and at
http://www.linux-kvm.org/images/a/a8/01x04-ARMdevice.pdf.

This was tested on:
- Calxeda Midway, assigning the xgmac main IRQ.
- AMD Seattle, where all XGBE interrupts are assigned:
- 2 level sensitive and 4 DMA edge sensitive IRQs

applies on top of:
KVM: arm/arm64: leave the LR active state on GICD_ICENABLERn access

All those pieces can be found at:
https://git.linaro.org/people/eric.auger/linux.git/shortlog/refs/heads/v4.4-rc1-forward-v4

History:

v3 -> v4:
- dependencies now are fixed (IRQ bypass manager, guest synchronous
halt/resume, VGIC architected timer)
- only applies on top of:
KVM: arm/arm64: leave the LR active state on GICD_ICENABLERn access
- Rebase on vgic series related to forwarded shared interrupts, mainly
[PATCH v2 0/8] Rework architected timer and forwarded IRQs handling &
[PATCH 0/2] arm/arm64: KVM: Fix architected timer issues
- took into account Alex's comment on VFIO part:
- vfio_platform_set_forwarded replacing vfio_platform_set_automasked
- automasked flag not updated
- vfio_platform_is_active renamed into vfio_platform_irq_is_active
- handle returned value for vfio_platform_irq_is_active
- remove WARN_ON on irq_bypass_register_producer failure
- took into account Christoffer's comments on vgic part
- clarify comments
- clarify edge-sensitive handling and validate_injection of edge
mapped unshared IRQ with level == 0
- use actual vcpu_id instead of VCPU 0
- revisit unset_forward implement using unqueue
- moved KVM: arm/arm64: select IRQ_BYPASS_MANAGER in this series
- add support for clear active and clear pending

v2 (RFC) -> v3(PATCH):
- all dependencies now have a patch status
- we dropped the producer active boolean exchanged between the
VFIO producer and irqfd arm consumer. As a consequence, on
unforward, if the IRQ is active, this latter is deactivated
without VFIO-masking it. So we do not exactly come back to the
exact state where we would be in unforwarded state. A new
physical IRQ can hit while the previous virtual IRQ is under
treatment. Its injection in the guest may be rejected thanks
to the VGIC state machine. This IRQ will be lost but I don't
think this is a severe issue. In case no new IRQ hits, the
guest deactivation of the virtual IRQ will trigger the resamplefd
which will VFIO unmask the non-masked IRQ. This also has no
consequence.
- VFIO platform driver consumer_add now can fail. It rejects the
transition for forwarding state in case the IRQ is active
- the series is rebased on new irq_vcpu_affinity series
- no dependency anymore on "chip/vgic adaptations for forwarded irq"
which was partially integrated into Marc's series. A fix is still
needed through.
- Guest synchronous halt/resume patch re-integrated into this series
- integrate a new patch file coming mixing
[PATCH v4 11/11] KVM: arm/arm64: vgic: Allow HW interrupts for
non-shared devices &
[RFC v2 2/4] KVM: arm: vgic: fix state machine for forwarded IRQ

v1 -> v2:
- irq bypass manager and irqfd consumer moved in a separate patch
- kvm_arm_[halt,resume]_guest moved in a separate patch
- remove VFIO external functions since we do not need them anymore
- apply container_of strategy advised by Paolo. Only active field
remains and discussions will tell whether we get rid of it.
- renamed kvm_arch functions

- kvm-vfio v6 -> RFC v1 based on IRQ bypass manager
see previous history in https://lkml.org/lkml/2015/4/13/353).

Best Regards

Eric

Eric Auger (12):
KVM: arm/arm64: select IRQ_BYPASS_MANAGER
VFIO: platform: registration of a dummy IRQ bypass producer
VFIO: platform: test forwarded state when selecting the IRQ handler
VFIO: platform:

Re: [GIT PULL 0/5] KVM: s390: Fixes for 4.4



On 19/11/2015 15:09, Christian Borntraeger wrote:
> Paolo,
> 
> The following changes since commit 8005c49d9aea74d382f474ce11afbbc7d7130bec:
> 
>   Linux 4.4-rc1 (2015-11-15 17:00:27 -0800)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git  
> tags/kvm-s390-master-4.4-1
> 
> for you to fetch changes up to 152e9f65d66f0a3891efc3869440becc0e7ff53f:
> 
>   KVM: s390: fix wrong lookup of VCPUs by array index (2015-11-19 14:47:43 
> +0100)
> 
> 
> KVM: s390: Fixes for 4.4
> 
> 1. disallow changing the SIMD mode when CPUs have been created.
>it allowed userspace to corrupt kernel memory
> 2. Fix vCPU lookup. Until now the vCPU number equals the vCPU id. Some
>kernel code places relied on that. This might
>a: cause guest failures
>b: allow userspace to corrupt kernel memory
> 3. Fencing of the PFMF instruction should use the guest facilities
>and not the host facilities.
> 
> 
> David Hildenbrand (4):
>   KVM: s390: enable SIMD only when no VCPUs were created
>   KVM: Provide function for VCPU lookup by id
>   KVM: s390: avoid memory overwrites on emergency signal injection
>   KVM: s390: fix wrong lookup of VCPUs by array index
> 
> Heiko Carstens (1):
>   KVM: s390: fix pfmf intercept handler
> 
>  arch/s390/kvm/interrupt.c |  7 +--
>  arch/s390/kvm/kvm-s390.c  |  6 +-
>  arch/s390/kvm/priv.c  |  2 +-
>  arch/s390/kvm/sigp.c  |  8 ++--
>  include/linux/kvm_host.h  | 11 +++
>  5 files changed, 24 insertions(+), 10 deletions(-)
> 

Pulled, thanks (for now to kvm/next, since that's what Stephen Rothwell
tests daily).

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

Re: [PATCH 1/4] KVM: Provide function for VCPU lookup by id

> 
> Any chance that you name this function differently? Otherwise we've got
> two functions that sound very similar and also have similar prototypes:
> 
> - kvm_get_vcpu(struct kvm *kvm, int i)
> - kvm_lookup_vcpu(struct kvm *kvm, int id)
> 
> I'm pretty sure this will cause confusion in the future!
> ==> Could you maybe name the new function something like
> "kvm_lookup_vcpu_by_id" or "kvm_get_vcpu_by_id" instead?

Had that in a previous version but decided to name it that way ... hmm ...
other opinions?

> 
> Also a short comment before the function would be nice to make the
> reader aware of the difference (e.g. when hot-plugging) between the
> vcpu-id and array-id.

Also not sure as the header directly includes the (for my eyes ;) ) easy code.
I would agree for a pure prototype. Other opinions?

> 
> Otherwise: +1 for finally having a proper common function for this!
> 

Thanks for having a look Thomas!

David

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

Re: [PATCH 1/4] KVM: Provide function for VCPU lookup by id

> 
> Any chance that you name this function differently? Otherwise we've got
> two functions that sound very similar and also have similar prototypes:
> 
> - kvm_get_vcpu(struct kvm *kvm, int i)
> - kvm_lookup_vcpu(struct kvm *kvm, int id)
> 
> I'm pretty sure this will cause confusion in the future!
> ==> Could you maybe name the new function something like
> "kvm_lookup_vcpu_by_id" or "kvm_get_vcpu_by_id" instead?

Had that in a previous version but decided to name it that way ... hmm ...
other opinions?

> 
> Also a short comment before the function would be nice to make the
> reader aware of the difference (e.g. when hot-plugging) between the
> vcpu-id and array-id.

Also not sure as the header directly includes the (for my eyes ;) ) easy code.
I would agree for a pure prototype. Other opinions?

> 
> Otherwise: +1 for finally having a proper common function for this!
> 

Thanks for having a look Thomas!

David

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

Re: [PATCH 1/4] KVM: Provide function for VCPU lookup by id



On 19/11/2015 13:55, David Hildenbrand wrote:
>> > 
>> > I'm pretty sure this will cause confusion in the future!
>> > ==> Could you maybe name the new function something like
>> > "kvm_lookup_vcpu_by_id" or "kvm_get_vcpu_by_id" instead?
> Had that in a previous version but decided to name it that way ... hmm ...
> other opinions?

Having _by_id at the end of the name makes sense indeed.

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

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

2015-11-19 Thread Sasha Levin

On 11/19/2015 02:21 AM, Gerd Hoffmann wrote:
> On Mi, 2015-11-18 at 23:01 -0500, Sasha Levin wrote:
>> On 11/18/2015 11:00 PM, Sasha Levin wrote:
>>> Anyways, I debugged it for a bit a found that seabios attempts to write to
>>> the notification BAR, I look further tomorrow to narrow it down and fix it.
>>
>> Err, *read*, obviously.
>>
>> I've never implemented that because the kernel doesn't try to do that (it 
>> doesn't
>> make much sense, I think...).
> 
> It doesn't make sense indeed (kvmtool still shouldn't segfault though),
> and on a quick look I can't spot a place in seabios doing that ...
> 
> It's reading ISR, as part of device reset, to make sure any pending
> interrupts are cleared.

That was indeed the ISR field. Fixing that makes seabios reach the same point as
legacy virtio before failing.

I don't see the original correspondence about seabios failures you've reported, 
if
you want to forward them over we can look at it further.


Thanks,
Sasha

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

Re: [PATCH 1/4] KVM: Provide function for VCPU lookup by id



On 19/11/2015 13:55, David Hildenbrand wrote:
>> > 
>> > I'm pretty sure this will cause confusion in the future!
>> > ==> Could you maybe name the new function something like
>> > "kvm_lookup_vcpu_by_id" or "kvm_get_vcpu_by_id" instead?
> Had that in a previous version but decided to name it that way ... hmm ...
> other opinions?

Having _by_id at the end of the name makes sense indeed.

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

Re: [PATCH 1/4] KVM: Provide function for VCPU lookup by id

On 11/19/2015 02:13 PM, Paolo Bonzini wrote:
> 
> 
> On 19/11/2015 13:55, David Hildenbrand wrote:

 I'm pretty sure this will cause confusion in the future!
 ==> Could you maybe name the new function something like
 "kvm_lookup_vcpu_by_id" or "kvm_get_vcpu_by_id" instead?
>> Had that in a previous version but decided to name it that way ... hmm ...
>> other opinions?
> 
> Having _by_id at the end of the name makes sense indeed.

David,
I will fix up the function name myself to kvm_get_vcpu_by_id. Ok?





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

Re: [PATCH 1/4] KVM: Provide function for VCPU lookup by id

On 11/19/2015 02:13 PM, Paolo Bonzini wrote:
> 
> 
> On 19/11/2015 13:55, David Hildenbrand wrote:

 I'm pretty sure this will cause confusion in the future!
 ==> Could you maybe name the new function something like
 "kvm_lookup_vcpu_by_id" or "kvm_get_vcpu_by_id" instead?
>> Had that in a previous version but decided to name it that way ... hmm ...
>> other opinions?
> 
> Having _by_id at the end of the name makes sense indeed.

David,
I will fix up the function name myself to kvm_get_vcpu_by_id. Ok?





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

Re: [PATCH 1/4] KVM: Provide function for VCPU lookup by id

> On 11/19/2015 02:13 PM, Paolo Bonzini wrote:
> > 
> > 
> > On 19/11/2015 13:55, David Hildenbrand wrote:
> 
>  I'm pretty sure this will cause confusion in the future!
>  ==> Could you maybe name the new function something like
>  "kvm_lookup_vcpu_by_id" or "kvm_get_vcpu_by_id" instead?
> >> Had that in a previous version but decided to name it that way ... hmm ...
> >> other opinions?
> > 
> > Having _by_id at the end of the name makes sense indeed.
> 
> David,
> I will fix up the function name myself to kvm_get_vcpu_by_id. Ok?

Sure, thanks a lot!

David

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

Re: [PATCH 1/4] KVM: Provide function for VCPU lookup by id

> On 11/19/2015 02:13 PM, Paolo Bonzini wrote:
> > 
> > 
> > On 19/11/2015 13:55, David Hildenbrand wrote:
> 
>  I'm pretty sure this will cause confusion in the future!
>  ==> Could you maybe name the new function something like
>  "kvm_lookup_vcpu_by_id" or "kvm_get_vcpu_by_id" instead?
> >> Had that in a previous version but decided to name it that way ... hmm ...
> >> other opinions?
> > 
> > Having _by_id at the end of the name makes sense indeed.
> 
> David,
> I will fix up the function name myself to kvm_get_vcpu_by_id. Ok?

Sure, thanks a lot!

David

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

[GIT PULL 3/5] KVM: Provide function for VCPU lookup by id

From: David Hildenbrand 

Let's provide a function to lookup a VCPU by id.

Reviewed-by: Christian Borntraeger 
Reviewed-by: Dominik Dingel 
Signed-off-by: David Hildenbrand 
Signed-off-by: Christian Borntraeger 
[split patch from refactoring patch]
---
 include/linux/kvm_host.h | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 5706a21..c923350 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -460,6 +460,17 @@ static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm 
*kvm, int i)
 (vcpup = kvm_get_vcpu(kvm, idx)) != NULL; \
 idx++)
 
+static inline struct kvm_vcpu *kvm_get_vcpu_by_id(struct kvm *kvm, int id)
+{
+   struct kvm_vcpu *vcpu;
+   int i;
+
+   kvm_for_each_vcpu(i, vcpu, kvm)
+   if (vcpu->vcpu_id == id)
+   return vcpu;
+   return NULL;
+}
+
 #define kvm_for_each_memslot(memslot, slots)   \
for (memslot = >memslots[0]; \
  memslot < slots->memslots + KVM_MEM_SLOTS_NUM && memslot->npages;\
-- 
2.3.0

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

[GIT PULL 5/5] KVM: s390: fix wrong lookup of VCPUs by array index

From: David Hildenbrand 

For now, VCPUs were always created sequentially with incrementing
VCPU ids. Therefore, the index in the VCPUs array matched the id.

As sequential creation might change with cpu hotplug, let's use
the correct lookup function to find a VCPU by id, not array index.

Let's also use kvm_lookup_vcpu() for validation of the sending VCPU
on external call injection.

Reviewed-by: Christian Borntraeger 
Signed-off-by: David Hildenbrand 
Signed-off-by: Christian Borntraeger 
Cc: sta...@vger.kernel.org # db27a7a KVM: Provide function for VCPU lookup by id
---
 arch/s390/kvm/interrupt.c | 3 +--
 arch/s390/kvm/sigp.c  | 8 ++--
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 7242c2d..6a75352 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -1030,8 +1030,7 @@ static int __inject_extcall(struct kvm_vcpu *vcpu, struct 
kvm_s390_irq *irq)
   src_id, 0);
 
/* sending vcpu invalid */
-   if (src_id >= KVM_MAX_VCPUS ||
-   kvm_get_vcpu(vcpu->kvm, src_id) == NULL)
+   if (kvm_get_vcpu_by_id(vcpu->kvm, src_id) == NULL)
return -EINVAL;
 
if (sclp.has_sigpif)
diff --git a/arch/s390/kvm/sigp.c b/arch/s390/kvm/sigp.c
index da690b6..77c22d6 100644
--- a/arch/s390/kvm/sigp.c
+++ b/arch/s390/kvm/sigp.c
@@ -291,12 +291,8 @@ static int handle_sigp_dst(struct kvm_vcpu *vcpu, u8 
order_code,
   u16 cpu_addr, u32 parameter, u64 *status_reg)
 {
int rc;
-   struct kvm_vcpu *dst_vcpu;
+   struct kvm_vcpu *dst_vcpu = kvm_get_vcpu_by_id(vcpu->kvm, cpu_addr);
 
-   if (cpu_addr >= KVM_MAX_VCPUS)
-   return SIGP_CC_NOT_OPERATIONAL;
-
-   dst_vcpu = kvm_get_vcpu(vcpu->kvm, cpu_addr);
if (!dst_vcpu)
return SIGP_CC_NOT_OPERATIONAL;
 
@@ -478,7 +474,7 @@ int kvm_s390_handle_sigp_pei(struct kvm_vcpu *vcpu)
trace_kvm_s390_handle_sigp_pei(vcpu, order_code, cpu_addr);
 
if (order_code == SIGP_EXTERNAL_CALL) {
-   dest_vcpu = kvm_get_vcpu(vcpu->kvm, cpu_addr);
+   dest_vcpu = kvm_get_vcpu_by_id(vcpu->kvm, cpu_addr);
BUG_ON(dest_vcpu == NULL);
 
kvm_s390_vcpu_wakeup(dest_vcpu);
-- 
2.3.0

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

[GIT PULL 4/5] KVM: s390: avoid memory overwrites on emergency signal injection

From: David Hildenbrand 

Commit 383d0b050106 ("KVM: s390: handle pending local interrupts via
bitmap") introduced a possible memory overwrite from user space.

User space could pass an invalid emergency signal code (sending VCPU)
and therefore exceed the bitmap. Let's take care of this case and
check that the id is in the valid range.

Reviewed-by: Dominik Dingel 
Cc: sta...@vger.kernel.org # v3.19+ db27a7a KVM: Provide function for VCPU 
lookup by id
Signed-off-by: David Hildenbrand 
Signed-off-by: Christian Borntraeger 
---
 arch/s390/kvm/interrupt.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 373e323..7242c2d 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -1110,6 +1110,10 @@ static int __inject_sigp_emergency(struct kvm_vcpu *vcpu,
trace_kvm_s390_inject_vcpu(vcpu->vcpu_id, KVM_S390_INT_EMERGENCY,
   irq->u.emerg.code, 0);
 
+   /* sending vcpu invalid */
+   if (kvm_get_vcpu_by_id(vcpu->kvm, irq->u.emerg.code) == NULL)
+   return -EINVAL;
+
set_bit(irq->u.emerg.code, li->sigp_emerg_pending);
set_bit(IRQ_PEND_EXT_EMERGENCY, >pending_irqs);
atomic_or(CPUSTAT_EXT_INT, li->cpuflags);
-- 
2.3.0

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

[GIT PULL 1/5] KVM: s390: enable SIMD only when no VCPUs were created

From: David Hildenbrand 

We should never allow to enable/disable any facilities for the guest
when other VCPUs were already created.

kvm_arch_vcpu_(load|put) relies on SIMD not changing during runtime.
If somebody would create and run VCPUs and then decides to enable
SIMD, undefined behaviour could be possible (e.g. vector save area
not being set up).

Acked-by: Christian Borntraeger 
Acked-by: Cornelia Huck 
Signed-off-by: David Hildenbrand 
Signed-off-by: Christian Borntraeger 
Cc: sta...@vger.kernel.org # 4.1+
---
 arch/s390/kvm/kvm-s390.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 8fe2f1c..8465892 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -342,12 +342,16 @@ static int kvm_vm_ioctl_enable_cap(struct kvm *kvm, 
struct kvm_enable_cap *cap)
r = 0;
break;
case KVM_CAP_S390_VECTOR_REGISTERS:
-   if (MACHINE_HAS_VX) {
+   mutex_lock(>lock);
+   if (atomic_read(>online_vcpus)) {
+   r = -EBUSY;
+   } else if (MACHINE_HAS_VX) {
set_kvm_facility(kvm->arch.model.fac->mask, 129);
set_kvm_facility(kvm->arch.model.fac->list, 129);
r = 0;
} else
r = -EINVAL;
+   mutex_unlock(>lock);
VM_EVENT(kvm, 3, "ENABLE: CAP_S390_VECTOR_REGISTERS %s",
 r ? "(not available)" : "(success)");
break;
-- 
2.3.0

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

[GIT PULL 2/5] KVM: s390: fix pfmf intercept handler

From: Heiko Carstens 

The pfmf intercept handler should check if the EDAT 1 facility
is installed in the guest, not if it is installed in the host.

Signed-off-by: Heiko Carstens 
Signed-off-by: Christian Borntraeger 
---
 arch/s390/kvm/priv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index 77191b8..d76b51c 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -660,7 +660,7 @@ static int handle_pfmf(struct kvm_vcpu *vcpu)
 
kvm_s390_get_regs_rre(vcpu, , );
 
-   if (!MACHINE_HAS_PFMF)
+   if (!test_kvm_facility(vcpu->kvm, 8))
return kvm_s390_inject_program_int(vcpu, PGM_OPERATION);
 
if (vcpu->arch.sie_block->gpsw.mask & PSW_MASK_PSTATE)
-- 
2.3.0

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

[GIT PULL 0/5] KVM: s390: Fixes for 4.4