[PATCH 4/4] KVM: Use common function for VCPU lookup by id
From: David Hildenbrand <d...@linux.vnet.ibm.com>
Let's reuse the new common function for VPCU lookup by id.
Reviewed-by: Christian Borntraeger <borntrae...@de.ibm.com>
Reviewed-by: Dominik Dingel <din...@linux.vnet.ibm.com>
Signed-off-by: David Hildenbrand <d...@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntrae...@de.ibm.com>
[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
[PATCH 2/4] KVM: s390: fix wrong lookup of VCPUs by array index
From: David Hildenbrand <d...@linux.vnet.ibm.com>
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 <borntrae...@de.ibm.com>
Signed-off-by: David Hildenbrand <d...@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntrae...@de.ibm.com>
[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 1/4] KVM: Provide function for VCPU lookup by id
From: David Hildenbrand <d...@linux.vnet.ibm.com>
Let's provide a function to lookup a VCPU by id.
Reviewed-by: Christian Borntraeger <borntrae...@de.ibm.com>
Reviewed-by: Dominik Dingel <din...@linux.vnet.ibm.com>
Signed-off-by: David Hildenbrand <d...@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntrae...@de.ibm.com>
[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 <d...@linux.vnet.ibm.com> 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 <borntrae...@de.ibm.com> Reviewed-by: Dominik Dingel <din...@linux.vnet.ibm.com> Reviewed-by: Christian Borntraeger <borntrae...@de.ibm.com> Signed-off-by: David Hildenbrand <d...@linux.vnet.ibm.com> Signed-off-by: Christian Borntraeger <borntrae...@de.ibm.com> --- 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
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 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 <d...@linux.vnet.ibm.com>
>
> 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 <borntrae...@de.ibm.com>
> Signed-off-by: David Hildenbrand <d...@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntrae...@de.ibm.com>
> [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: [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: KVM memory slots limit on powerpc
Am 04.09.2015 um 11:35 schrieb Thomas Huth:
>
> Hi all,
>
> now that we get memory hotplugging for the spapr machine on qemu-ppc,
> too, it seems like we easily can hit the amount of KVM-internal memory
> slots now ("#define KVM_USER_MEM_SLOTS 32" in
> arch/powerpc/include/asm/kvm_host.h). For example, start
> qemu-system-ppc64 with a couple of "-device secondary-vga" and "-m
> 4G,slots=32,maxmem=40G" and then try to hot-plug all 32 DIMMs ... and
> you'll see that it aborts way earlier already.
>
> The x86 code already increased the amount of KVM_USER_MEM_SLOTS to 509
> already (+3 internal slots = 512) ... maybe we should now increase the
> amount of slots on powerpc, too? Since we don't use internal slots on
> POWER, would 512 be a good value? Or would less be sufficient, too?
When you are at it, the s390 value should also be increased I guess.
Christian
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-ppc] KVM memory slots limit on powerpc
Am 04.09.2015 um 12:04 schrieb Alexander Graf:
>
>
> On 04.09.15 11:59, Christian Borntraeger wrote:
>> Am 04.09.2015 um 11:35 schrieb Thomas Huth:
>>>
>>> Hi all,
>>>
>>> now that we get memory hotplugging for the spapr machine on qemu-ppc,
>>> too, it seems like we easily can hit the amount of KVM-internal memory
>>> slots now ("#define KVM_USER_MEM_SLOTS 32" in
>>> arch/powerpc/include/asm/kvm_host.h). For example, start
>>> qemu-system-ppc64 with a couple of "-device secondary-vga" and "-m
>>> 4G,slots=32,maxmem=40G" and then try to hot-plug all 32 DIMMs ... and
>>> you'll see that it aborts way earlier already.
>>>
>>> The x86 code already increased the amount of KVM_USER_MEM_SLOTS to 509
>>> already (+3 internal slots = 512) ... maybe we should now increase the
>>> amount of slots on powerpc, too? Since we don't use internal slots on
>>> POWER, would 512 be a good value? Or would less be sufficient, too?
>>
>> When you are at it, the s390 value should also be increased I guess.
>
> That constant defines the array size for the memslot array in struct kvm
> which in turn again gets allocated by kzalloc, so it's pinned kernel
> memory that is physically contiguous. Doing big allocations can turn
> into problems during runtime.
>
> So maybe there is another way? Can we extend the memslot array size
> dynamically somehow? Allocate it separately? How much memory does the
> memslot array use up with 512 entries?
Maybe some rcu protected scheme that doubles the amount of memslots for
each overrun? Yes, that would be good and even reduce the footprint for
systems with only a small number of memslots.
Christian
--
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
[PATCHv2 0/2] KVM: micro-optimization and interrupt disabling
This rework allows to avoid some cycles by not disabling interrupts
twice.
Christian Borntraeger (2):
KVM: provide irq_unsafe kvm_guest_{enter|exit}
KVM: arm/mips/x86/power use __kvm_guest_{enter|exit}
arch/arm/kvm/arm.c | 4 ++--
arch/mips/kvm/mips.c | 4 ++--
arch/powerpc/kvm/powerpc.c | 2 +-
arch/s390/kvm/kvm-s390.c | 10 ++
arch/x86/kvm/x86.c | 2 +-
include/linux/kvm_host.h | 27 ++-
6 files changed, 30 insertions(+), 19 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
Re: [PATCH 1/2] KVM: provide irq_unsafe kvm_guest_{enter|exit}
Am 30.04.2015 um 14:02 schrieb Christian Borntraeger:
Am 30.04.2015 um 14:01 schrieb Christian Borntraeger:
Am 30.04.2015 um 13:50 schrieb Paolo Bonzini:
On 30/04/2015 13:43, Christian Borntraeger wrote:
+/* must be called with irqs disabled */
+static inline void __kvm_guest_enter(void)
{
- unsigned long flags;
-
- BUG_ON(preemptible());
Please keep the BUG_ON() in kvm_guest_enter. Otherwise looks good, thanks!
Ah, you mean have the BUG_ON in the non underscore version? Yes, makes sense.
Hmmm, too quick.
the BUG_ON was there to make sure that rcu_virt_note_context_switch is safe.
The reworked code pulls the rcu_virt_note_context_switch within the irq_save
section so we no longer need this BUG_ON, no?
Christian
--
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/2] KVM: provide irq_unsafe kvm_guest_{enter|exit}
Am 30.04.2015 um 13:50 schrieb Paolo Bonzini:
On 30/04/2015 13:43, Christian Borntraeger wrote:
+/* must be called with irqs disabled */
+static inline void __kvm_guest_enter(void)
{
-unsigned long flags;
-
-BUG_ON(preemptible());
Please keep the BUG_ON() in kvm_guest_enter. Otherwise looks good, thanks!
would be
BUG_ON(!irqs_disabled())
then?
--
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/2] KVM: provide irq_unsafe kvm_guest_{enter|exit}
Am 30.04.2015 um 14:01 schrieb Christian Borntraeger:
Am 30.04.2015 um 13:50 schrieb Paolo Bonzini:
On 30/04/2015 13:43, Christian Borntraeger wrote:
+/* must be called with irqs disabled */
+static inline void __kvm_guest_enter(void)
{
- unsigned long flags;
-
- BUG_ON(preemptible());
Please keep the BUG_ON() in kvm_guest_enter. Otherwise looks good, thanks!
Ah, you mean have the BUG_ON in the non underscore version? Yes, makes sense.
--
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] KVM: booke: use __kvm_guest_exit
Am 30.04.2015 um 14:40 schrieb Paolo Bonzini: Signed-off-by: Paolo Bonzini pbonz...@redhat.com Reviewed-by: Christian Borntraeger borntrae...@de.ibm.com but no way to test it --- arch/powerpc/kvm/booke.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 6c1316a15a27..3872ab31c80a 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -1004,10 +1004,10 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu, break; } - local_irq_enable(); - trace_kvm_exit(exit_nr, vcpu); - kvm_guest_exit(); + __kvm_guest_exit(); + + local_irq_enable(); run-exit_reason = KVM_EXIT_UNKNOWN; run-ready_for_interrupt_injection = 1; -- 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/RFC 2/2] KVM: push down irq_save from kvm_guest_exit
Am 28.04.2015 um 13:37 schrieb Paolo Bonzini:
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -891,7 +891,9 @@ int kvmppc_handle_exit_pr(struct kvm_run *run, struct
kvm_vcpu *vcpu,
/* We get here with MSR.EE=1 */
+local_irq_disable();
trace_kvm_exit(exit_nr, vcpu);
+local_irq_enable();
kvm_guest_exit();
This looks wrong.
Yes it is.
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -773,11 +773,7 @@ static inline void kvm_guest_enter(void)
static inline void kvm_guest_exit(void)
{
-unsigned long flags;
-
-local_irq_save(flags);
Why no WARN_ON here?
Yes,WARN_ON makes sense.
Hmm, on the other hand the irqs_disabled call might be somewhat expensive again
(would boil down on s390 to call stnsm (store and and system mask) once for
query and once for setting.
so...
I think the patches are sensible, especially since you can add warnings.
This kind of code definitely knows if it has interrupts disabled or enabled
Alternatively, the irq-disabled versions could be called
__kvm_guest_{enter,exit}. Then you can use those directly when it makes
sense.
..having a special __kvm_guest_{enter,exit} without the WARN_ON might be even
the cheapest way. In that way I could leave everything besides s390 alone and
arch maintainers can do a followup patch if appropriate.
--
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/RFC 0/2] KVM: micro-optimization and interrupt disabling
I was able to get rid of some nanoseconds for a guest exit loop on s390. I did my best to not break other architectures but review and comments on the general approach is welcome. Downside is that the existing irq_save things will just work no matter what the callers have done, the new code must do the right thing in the callers. Is that approach acceptible? Does anybody else see some measurable difference for guest exits? Christian Borntraeger (2): KVM: Push down irq_save to architectures before kvm_guest_enter KVM: push down irq_save from kvm_guest_exit arch/powerpc/kvm/book3s_hv.c | 4 arch/powerpc/kvm/book3s_pr.c | 2 ++ arch/powerpc/kvm/booke.c | 4 ++-- arch/s390/kvm/kvm-s390.c | 6 -- arch/x86/kvm/x86.c | 2 ++ include/linux/kvm_host.h | 18 -- 6 files changed, 22 insertions(+), 14 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/RFC 2/2] KVM: push down irq_save from kvm_guest_exit
Some architectures already have irq disabled when calling
kvm_guest_exit. Push down the disabling into the architectures
to avoid double disabling. This also allows to replace
irq_save with irq_disable which might be cheaper.
arm and mips already have interrupts disabled. s390/power/x86
need adoptions.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
arch/powerpc/kvm/book3s_hv.c | 2 ++
arch/powerpc/kvm/book3s_pr.c | 2 ++
arch/powerpc/kvm/booke.c | 4 ++--
arch/s390/kvm/kvm-s390.c | 2 ++
arch/x86/kvm/x86.c | 2 ++
include/linux/kvm_host.h | 4
6 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index a5f392d..63b35b1 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1811,7 +1811,9 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc)
/* make sure updates to secondary vcpu structs are visible now */
smp_mb();
+ local_irq_disable();
kvm_guest_exit();
+ local_irq_enable();
preempt_enable();
cond_resched();
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index f573839..eafcb8c 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -891,7 +891,9 @@ int kvmppc_handle_exit_pr(struct kvm_run *run, struct
kvm_vcpu *vcpu,
/* We get here with MSR.EE=1 */
+ local_irq_disable();
trace_kvm_exit(exit_nr, vcpu);
+ local_irq_enable();
kvm_guest_exit();
switch (exit_nr) {
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 6c1316a..f1d6af3 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -1004,11 +1004,11 @@ int kvmppc_handle_exit(struct kvm_run *run, struct
kvm_vcpu *vcpu,
break;
}
- local_irq_enable();
-
trace_kvm_exit(exit_nr, vcpu);
kvm_guest_exit();
+ local_irq_enable();
+
run-exit_reason = KVM_EXIT_UNKNOWN;
run-ready_for_interrupt_injection = 1;
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 9f4c954..0aa2534 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2015,7 +2015,9 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
local_irq_enable();
exit_reason = sie64a(vcpu-arch.sie_block,
vcpu-run-s.regs.gprs);
+ local_irq_disable();
kvm_guest_exit();
+ local_irq_enable();
vcpu-srcu_idx = srcu_read_lock(vcpu-kvm-srcu);
rc = vcpu_post_run(vcpu, exit_reason);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 32bf19e..a5fbd45 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6351,7 +6351,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
*/
barrier();
+ local_irq_disable();
kvm_guest_exit();
+ local_irq_enable();
preempt_enable();
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index a34bf6ed..fe699fb 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -773,11 +773,7 @@ static inline void kvm_guest_enter(void)
static inline void kvm_guest_exit(void)
{
- unsigned long flags;
-
- local_irq_save(flags);
guest_exit();
- local_irq_restore(flags);
}
/*
--
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/RFC 1/2] KVM: Push down irq_save to architectures before kvm_guest_enter
local_irq_disable can be cheaper than local_irq_save, especially
when done only once instead of twice. We can push down the
local_irq_save (and replace it with local_irq_disable) to
save some cycles.
x86, mips and arm already disable the interrupts before calling
kvm_guest_enter. Here we save one local_irq_save/restore pair.
power and s390 are reworked to disable the interrupts before calling
kvm_guest_enter. s390 saves a preempt_disable/enable pair but also
saves some cycles as local_irq_disable/enable can be cheaper than
local_irq_save/restore on some machines.
power should be almost a no-op change (interrupts are disabled
slighty longer).
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
arch/powerpc/kvm/book3s_hv.c | 2 ++
arch/s390/kvm/kvm-s390.c | 4 ++--
include/linux/kvm_host.h | 14 --
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index de74756..a5f392d 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1779,7 +1779,9 @@ static void kvmppc_run_core(struct kvmppc_vcore *vc)
spin_unlock(vc-lock);
+ local_irq_disable();
kvm_guest_enter();
+ local_irq_enable();
srcu_idx = srcu_read_lock(vc-kvm-srcu);
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 46f37df..9f4c954 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2010,9 +2010,9 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
* As PF_VCPU will be used in fault handler, between
* guest_enter and guest_exit should be no uaccess.
*/
- preempt_disable();
+ local_irq_disable();
kvm_guest_enter();
- preempt_enable();
+ local_irq_enable();
exit_reason = sie64a(vcpu-arch.sie_block,
vcpu-run-s.regs.gprs);
kvm_guest_exit();
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index d12b210..a34bf6ed 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -751,13 +751,15 @@ static inline void kvm_iommu_unmap_pages(struct kvm *kvm,
static inline void kvm_guest_enter(void)
{
- unsigned long flags;
-
- BUG_ON(preemptible());
-
- local_irq_save(flags);
+ /*
+* guest_enter needs disabled irqs and rcu_virt_note_context_switch
+* wants disabled preemption. Ensure that the caller has disabled
+* irqs for kvm_guest_enter. Please note: Some architectures (e.g.
+* s390) will reenable irqs before entering the guest, but this is
+* ok. We just need a stable CPU for the accounting itself.
+*/
+ WARN_ON(!irqs_disabled());
guest_enter();
- local_irq_restore(flags);
/* KVM does not hold any references to rcu protected data when it
* switches CPU into a guest mode. In fact switching to a guest mode
--
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 1/8] ppc/kvm: Replace ACCESS_ONCE with READ_ONCE
Am 16.01.2015 um 00:09 schrieb Michael Ellerman:
On Thu, 2015-01-15 at 09:58 +0100, Christian Borntraeger wrote:
ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
Change the ppc/kvm code to replace ACCESS_ONCE with READ_ONCE.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
arch/powerpc/kvm/book3s_hv_rm_xics.c | 8
arch/powerpc/kvm/book3s_xics.c | 16
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rm_xics.c
b/arch/powerpc/kvm/book3s_hv_rm_xics.c
index 7b066f6..7c22997 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_xics.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_xics.c
@@ -152,7 +152,7 @@ static void icp_rm_down_cppr(struct kvmppc_xics *xics,
struct kvmppc_icp *icp,
* in virtual mode.
*/
do {
-old_state = new_state = ACCESS_ONCE(icp-state);
+old_state = new_state = READ_ONCE(icp-state);
These are all icp-state.
Which is a union, but it's only the size of unsigned long. So in practice
there
shouldn't be a bug here right?
This bug was that gcc lost the volatile tag when propagating aggregates to
scalar types.
So in theory a union could be affected. See the original problem
( http://marc.info/?i=54611D86.4040306%40de.ibm.com )
which happened on
union ipte_control {
unsigned long val;
struct {
unsigned long k : 1;
unsigned long kh : 31;
unsigned long kg : 32;
};
};
Christian
--
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 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE-READ_ONCE
Am 15.01.2015 um 20:38 schrieb Oleg Nesterov:
On 01/15, Christian Borntraeger wrote:
--- a/arch/x86/include/asm/spinlock.h
+++ b/arch/x86/include/asm/spinlock.h
@@ -186,7 +186,7 @@ static inline void arch_spin_unlock_wait(arch_spinlock_t
*lock)
__ticket_t head = ACCESS_ONCE(lock-tickets.head);
for (;;) {
-struct __raw_tickets tmp = ACCESS_ONCE(lock-tickets);
+struct __raw_tickets tmp = READ_ONCE(lock-tickets);
Agreed, but what about another ACCESS_ONCE() above?
Oleg.
tickets.head is a scalar type, so ACCESS_ONCE does work fine with gcc 4.6/4.7.
My goal was to convert all accesses on non-scalar types as until
kernel: tighten rules for ACCESS ONCE is merged because anything else would be
a Whac-a-mole like adventure (I learned that during the last round in next: all
conversions in this series fix up changes made during this merge window)
We probably going to do a bigger bunch of bulk conversion later on when
kernel: tighten rules for ACCESS ONCE prevents new problems.
Makes sense?
Christian
--
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 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE-READ_ONCE
Am 15.01.2015 um 21:01 schrieb Oleg Nesterov:
On 01/15, Christian Borntraeger wrote:
Am 15.01.2015 um 20:38 schrieb Oleg Nesterov:
On 01/15, Christian Borntraeger wrote:
--- a/arch/x86/include/asm/spinlock.h
+++ b/arch/x86/include/asm/spinlock.h
@@ -186,7 +186,7 @@ static inline void
arch_spin_unlock_wait(arch_spinlock_t *lock)
__ticket_t head = ACCESS_ONCE(lock-tickets.head);
for (;;) {
- struct __raw_tickets tmp = ACCESS_ONCE(lock-tickets);
+ struct __raw_tickets tmp = READ_ONCE(lock-tickets);
Agreed, but what about another ACCESS_ONCE() above?
Oleg.
tickets.head is a scalar type, so ACCESS_ONCE does work fine with gcc
4.6/4.7.
My goal was to convert all accesses on non-scalar types
I understand, but READ_ONCE(lock-tickets.head) looks better anyway and
arch_spin_lock() already use READ_ONCE() for this.
So why we should keep the last ACCESS_ONCE() in spinlock.h ? Just to make
another cosmetic cleanup which touches the same function later?
OK, I will change that one as well.
--
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/8] current ACCESS_ONCE patch queue
Folks, fyi, this is my current patch queue for the next merge window. It does contain a patch that will disallow ACCESS_ONCE on non-scalar types. The tree is part of linux-next and can be found at git://git.kernel.org/pub/scm/linux/kernel/git/borntraeger/linux.git linux-next Christian Borntraeger (7): ppc/kvm: Replace ACCESS_ONCE with READ_ONCE ppc/hugetlbfs: Replace ACCESS_ONCE with READ_ONCE x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE x86/spinlock: Leftover conversion ACCESS_ONCE-READ_ONCE mm/gup: Replace ACCESS_ONCE with READ_ONCE kernel: tighten rules for ACCESS ONCE kernel: Fix sparse warning for ACCESS_ONCE Guenter Roeck (1): next: sh: Fix compile error arch/powerpc/kvm/book3s_hv_rm_xics.c | 8 arch/powerpc/kvm/book3s_xics.c | 16 arch/powerpc/mm/hugetlbpage.c| 4 ++-- arch/sh/mm/gup.c | 2 +- arch/x86/include/asm/spinlock.h | 2 +- arch/x86/xen/p2m.c | 2 +- include/linux/compiler.h | 21 - mm/gup.c | 2 +- 8 files changed, 34 insertions(+), 23 deletions(-) -- 1.9.3 -- 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: [Xen-devel] [PATCH 3/8] x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE
Am 15.01.2015 um 11:43 schrieb David Vrabel: On 15/01/15 08:58, Christian Borntraeger wrote: ACCESS_ONCE does not work reliably on non-scalar types. For example gcc 4.6 and 4.7 might remove the volatile tag for such accesses during the SRA (scalar replacement of aggregates) step (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145) Change the p2m code to replace ACCESS_ONCE with READ_ONCE. Acked-by: David Vrabel david.vra...@citrix.com Thanks Let me know if you want me to merge this via the Xen tree. With your Acked-by, I can easily carry this in my tree. We can then ensure that this patch is merged before the ACCESS_ONCE is made stricter - making bisecting easier. Christian -- 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 1/8] ppc/kvm: Replace ACCESS_ONCE with READ_ONCE
ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
Change the ppc/kvm code to replace ACCESS_ONCE with READ_ONCE.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
arch/powerpc/kvm/book3s_hv_rm_xics.c | 8
arch/powerpc/kvm/book3s_xics.c | 16
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rm_xics.c
b/arch/powerpc/kvm/book3s_hv_rm_xics.c
index 7b066f6..7c22997 100644
--- a/arch/powerpc/kvm/book3s_hv_rm_xics.c
+++ b/arch/powerpc/kvm/book3s_hv_rm_xics.c
@@ -152,7 +152,7 @@ static void icp_rm_down_cppr(struct kvmppc_xics *xics,
struct kvmppc_icp *icp,
* in virtual mode.
*/
do {
- old_state = new_state = ACCESS_ONCE(icp-state);
+ old_state = new_state = READ_ONCE(icp-state);
/* Down_CPPR */
new_state.cppr = new_cppr;
@@ -211,7 +211,7 @@ unsigned long kvmppc_rm_h_xirr(struct kvm_vcpu *vcpu)
* pending priority
*/
do {
- old_state = new_state = ACCESS_ONCE(icp-state);
+ old_state = new_state = READ_ONCE(icp-state);
xirr = old_state.xisr | (((u32)old_state.cppr) 24);
if (!old_state.xisr)
@@ -277,7 +277,7 @@ int kvmppc_rm_h_ipi(struct kvm_vcpu *vcpu, unsigned long
server,
* whenever the MFRR is made less favored.
*/
do {
- old_state = new_state = ACCESS_ONCE(icp-state);
+ old_state = new_state = READ_ONCE(icp-state);
/* Set_MFRR */
new_state.mfrr = mfrr;
@@ -352,7 +352,7 @@ int kvmppc_rm_h_cppr(struct kvm_vcpu *vcpu, unsigned long
cppr)
icp_rm_clr_vcpu_irq(icp-vcpu);
do {
- old_state = new_state = ACCESS_ONCE(icp-state);
+ old_state = new_state = READ_ONCE(icp-state);
reject = 0;
new_state.cppr = cppr;
diff --git a/arch/powerpc/kvm/book3s_xics.c b/arch/powerpc/kvm/book3s_xics.c
index 807351f..a4a8d9f 100644
--- a/arch/powerpc/kvm/book3s_xics.c
+++ b/arch/powerpc/kvm/book3s_xics.c
@@ -327,7 +327,7 @@ static bool icp_try_to_deliver(struct kvmppc_icp *icp, u32
irq, u8 priority,
icp-server_num);
do {
- old_state = new_state = ACCESS_ONCE(icp-state);
+ old_state = new_state = READ_ONCE(icp-state);
*reject = 0;
@@ -512,7 +512,7 @@ static void icp_down_cppr(struct kvmppc_xics *xics, struct
kvmppc_icp *icp,
* in virtual mode.
*/
do {
- old_state = new_state = ACCESS_ONCE(icp-state);
+ old_state = new_state = READ_ONCE(icp-state);
/* Down_CPPR */
new_state.cppr = new_cppr;
@@ -567,7 +567,7 @@ static noinline unsigned long kvmppc_h_xirr(struct kvm_vcpu
*vcpu)
* pending priority
*/
do {
- old_state = new_state = ACCESS_ONCE(icp-state);
+ old_state = new_state = READ_ONCE(icp-state);
xirr = old_state.xisr | (((u32)old_state.cppr) 24);
if (!old_state.xisr)
@@ -634,7 +634,7 @@ static noinline int kvmppc_h_ipi(struct kvm_vcpu *vcpu,
unsigned long server,
* whenever the MFRR is made less favored.
*/
do {
- old_state = new_state = ACCESS_ONCE(icp-state);
+ old_state = new_state = READ_ONCE(icp-state);
/* Set_MFRR */
new_state.mfrr = mfrr;
@@ -679,7 +679,7 @@ static int kvmppc_h_ipoll(struct kvm_vcpu *vcpu, unsigned
long server)
if (!icp)
return H_PARAMETER;
}
- state = ACCESS_ONCE(icp-state);
+ state = READ_ONCE(icp-state);
kvmppc_set_gpr(vcpu, 4, ((u32)state.cppr 24) | state.xisr);
kvmppc_set_gpr(vcpu, 5, state.mfrr);
return H_SUCCESS;
@@ -721,7 +721,7 @@ static noinline void kvmppc_h_cppr(struct kvm_vcpu *vcpu,
unsigned long cppr)
BOOK3S_INTERRUPT_EXTERNAL_LEVEL);
do {
- old_state = new_state = ACCESS_ONCE(icp-state);
+ old_state = new_state = READ_ONCE(icp-state);
reject = 0;
new_state.cppr = cppr;
@@ -885,7 +885,7 @@ static int xics_debug_show(struct seq_file *m, void
*private)
if (!icp)
continue;
- state.raw = ACCESS_ONCE(icp-state.raw);
+ state.raw = READ_ONCE(icp-state.raw);
seq_printf(m, cpu server %#lx XIRR:%#x PPRI:%#x CPPR:%#x
MFRR:%#x OUT:%d NR:%d\n,
icp-server_num, state.xisr,
state.pending_pri, state.cppr, state.mfrr
[PATCH 3/8] x86/xen/p2m: Replace ACCESS_ONCE with READ_ONCE
ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
Change the p2m code to replace ACCESS_ONCE with READ_ONCE.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
arch/x86/xen/p2m.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index edbc7a6..cb71016 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -554,7 +554,7 @@ static bool alloc_p2m(unsigned long pfn)
mid_mfn = NULL;
}
- p2m_pfn = pte_pfn(ACCESS_ONCE(*ptep));
+ p2m_pfn = pte_pfn(READ_ONCE(*ptep));
if (p2m_pfn == PFN_DOWN(__pa(p2m_identity)) ||
p2m_pfn == PFN_DOWN(__pa(p2m_missing))) {
/* p2m leaf page is missing */
--
1.9.3
--
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 8/8] kernel: Fix sparse warning for ACCESS_ONCE
Commit a91ed664749c (kernel: tighten rules for ACCESS ONCE) results in
sparse warnings like Using plain integer as NULL pointer - Let's add a
type cast to the dummy assignment.
To avoid warnings lik sparse: warning: cast to restricted __hc32 we also
use __force on that cast.
Fixes: a91ed664749c (kernel: tighten rules for ACCESS ONCE)
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
include/linux/compiler.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 5e186bf..7bebf05 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -461,7 +461,7 @@ static __always_inline void __assign_once_size(volatile
void *p, void *res, int
* If possible use READ_ONCE/ASSIGN_ONCE instead.
*/
#define __ACCESS_ONCE(x) ({ \
-__maybe_unused typeof(x) __var = 0; \
+__maybe_unused typeof(x) __var = (__force typeof(x)) 0; \
(volatile typeof(x) *)(x); })
#define ACCESS_ONCE(x) (*__ACCESS_ONCE(x))
--
1.9.3
--
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 2/8] ppc/hugetlbfs: Replace ACCESS_ONCE with READ_ONCE
ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
Change the ppc/hugetlbfs code to replace ACCESS_ONCE with READ_ONCE.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
arch/powerpc/mm/hugetlbpage.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 5ff4e07..620d0ec 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -978,7 +978,7 @@ pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned
long ea, unsigned *shift
*/
pdshift = PUD_SHIFT;
pudp = pud_offset(pgd, ea);
- pud = ACCESS_ONCE(*pudp);
+ pud = READ_ONCE(*pudp);
if (pud_none(pud))
return NULL;
@@ -990,7 +990,7 @@ pte_t *find_linux_pte_or_hugepte(pgd_t *pgdir, unsigned
long ea, unsigned *shift
else {
pdshift = PMD_SHIFT;
pmdp = pmd_offset(pud, ea);
- pmd = ACCESS_ONCE(*pmdp);
+ pmd = READ_ONCE(*pmdp);
/*
* A hugepage collapse is captured by pmd_none, because
* it mark the pmd none and do a hpte invalidate.
--
1.9.3
--
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 5/8] mm/gup: Replace ACCESS_ONCE with READ_ONCE
ACCESS_ONCE does not work reliably on non-scalar types. For
example gcc 4.6 and 4.7 might remove the volatile tag for such
accesses during the SRA (scalar replacement of aggregates) step
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145)
Fixup gup_pmd_range.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
mm/gup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/gup.c b/mm/gup.c
index a900759..bed30efa 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -926,7 +926,7 @@ static int gup_pmd_range(pud_t pud, unsigned long addr,
unsigned long end,
pmdp = pmd_offset(pud, addr);
do {
- pmd_t pmd = ACCESS_ONCE(*pmdp);
+ pmd_t pmd = READ_ONCE(*pmdp);
next = pmd_addr_end(addr, end);
if (pmd_none(pmd) || pmd_trans_splitting(pmd))
--
1.9.3
--
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 6/8] kernel: tighten rules for ACCESS ONCE
Now that all non-scalar users of ACCESS_ONCE have been converted
to READ_ONCE or ASSIGN once, lets tighten ACCESS_ONCE to only
work on scalar types.
This variant was proposed by Alexei Starovoitov.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
Reviewed-by: Paul E. McKenney paul...@linux.vnet.ibm.com
---
include/linux/compiler.h | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index a1c81f8..5e186bf 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -447,12 +447,23 @@ static __always_inline void __assign_once_size(volatile
void *p, void *res, int
* to make the compiler aware of ordering is to put the two invocations of
* ACCESS_ONCE() in different C statements.
*
- * This macro does absolutely -nothing- to prevent the CPU from reordering,
- * merging, or refetching absolutely anything at any time. Its main intended
- * use is to mediate communication between process-level code and irq/NMI
- * handlers, all running on the same CPU.
+ * ACCESS_ONCE will only work on scalar types. For union types, ACCESS_ONCE
+ * on a union member will work as long as the size of the member matches the
+ * size of the union and the size is smaller than word size.
+ *
+ * The major use cases of ACCESS_ONCE used to be (1) Mediating communication
+ * between process-level code and irq/NMI handlers, all running on the same
CPU,
+ * and (2) Ensuring that the compiler does not fold, spindle, or otherwise
+ * mutilate accesses that either do not require ordering or that interact
+ * with an explicit memory barrier or atomic instruction that provides the
+ * required ordering.
+ *
+ * If possible use READ_ONCE/ASSIGN_ONCE instead.
*/
-#define ACCESS_ONCE(x) (*(volatile typeof(x) *)(x))
+#define __ACCESS_ONCE(x) ({ \
+__maybe_unused typeof(x) __var = 0; \
+ (volatile typeof(x) *)(x); })
+#define ACCESS_ONCE(x) (*__ACCESS_ONCE(x))
/* Ignore/forbid kprobes attach on very low level functions marked by this
attribute: */
#ifdef CONFIG_KPROBES
--
1.9.3
--
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 7/8] next: sh: Fix compile error
From: Guenter Roeck li...@roeck-us.net
Commit a91ed664749c (kernel: tighten rules for ACCESS ONCE) results in a
compile failure for sh builds with CONFIG_X2TLB enabled.
arch/sh/mm/gup.c: In function 'gup_get_pte':
arch/sh/mm/gup.c:20:2: error: invalid initializer
make[1]: *** [arch/sh/mm/gup.o] Error 1
Replace ACCESS_ONCE with READ_ONCE to fix the problem.
Fixes: a91ed664749c (kernel: tighten rules for ACCESS ONCE)
Cc: Paul E. McKenney paul...@linux.vnet.ibm.com
Cc: Christian Borntraeger borntrae...@de.ibm.com
Signed-off-by: Guenter Roeck li...@roeck-us.net
Reviewed-by: Paul E. McKenney paul...@linux.vnet.ibm.com
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
---
arch/sh/mm/gup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/sh/mm/gup.c b/arch/sh/mm/gup.c
index 37458f3..e113bb4 100644
--- a/arch/sh/mm/gup.c
+++ b/arch/sh/mm/gup.c
@@ -17,7 +17,7 @@
static inline pte_t gup_get_pte(pte_t *ptep)
{
#ifndef CONFIG_X2TLB
- return ACCESS_ONCE(*ptep);
+ return READ_ONCE(*ptep);
#else
/*
* With get_user_pages_fast, we walk down the pagetables without
--
1.9.3
--
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 4/8] x86/spinlock: Leftover conversion ACCESS_ONCE-READ_ONCE
commit 78bff1c8684f (x86/ticketlock: Fix spin_unlock_wait() livelock)
introduced another ACCESS_ONCE case in x86 spinlock.h.
Change that as well.
Signed-off-by: Christian Borntraeger borntrae...@de.ibm.com
Cc: Oleg Nesterov o...@redhat.com
---
arch/x86/include/asm/spinlock.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/spinlock.h b/arch/x86/include/asm/spinlock.h
index 625660f..9264f0f 100644
--- a/arch/x86/include/asm/spinlock.h
+++ b/arch/x86/include/asm/spinlock.h
@@ -186,7 +186,7 @@ static inline void arch_spin_unlock_wait(arch_spinlock_t
*lock)
__ticket_t head = ACCESS_ONCE(lock-tickets.head);
for (;;) {
- struct __raw_tickets tmp = ACCESS_ONCE(lock-tickets);
+ struct __raw_tickets tmp = READ_ONCE(lock-tickets);
/*
* We need to check unlocked in a loop, tmp.head == head
* can be false positive because of overflow.
--
1.9.3
--
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 v12 0/6] arm/KVM: dirty page logging support for ARMv7 (3.17.0-rc1)
Am 23.10.2014 00:34, schrieb Mario Smarduch: This patch series introduces dirty page logging for ARMv7 and adds some degree of generic dirty logging support for x86, armv7 and later armv8. I implemented Alex's suggestion after he took a look at the patches at kvm forum to simplify the generic/arch split - leaving mips, powerpc, s390, (ia64 although broken) unchanged. x86/armv7 now share some dirty logging code. armv8 dirty log patches have been posted and tested but for time being armv8 is non-generic as well. I briefly spoke to most of you at kvm forum, and this is the patch series I was referring to. Implementation changed from previous version (patches 1 2), those who acked previous revision, please review again. Last 4 patches (ARM) have been rebased for newer kernel, with no signifcant changes. Testing: - Generally live migration + checksumming of source/destination memory regions is used validate correctness. - qemu machvirt, VExpress - Exynos 5440, FastModels - lmbench + dirty guest memory cycling. - ARMv8 Foundation Model/kvmtool - Due to slight overlap in 2nd stage handlers did a basic bringup using qemu. - x86_64 qemu default machine model, tested migration on HP Z620, tested convergence for several dirty page rates See https://github.com/mjsmar/arm-dirtylog-tests - Dirtlogtest-setup.pdf for ARMv7 - https://github.com/mjsmar/arm-dirtylog-tests/tree/master/v7 - README The patch affects armv7,armv8, mips, ia64, powerpc, s390, x86_64. Patch series has been compiled for affected architectures: - x86_64 - defconfig - ia64 - ia64-linux-gcc4.6.3 - defconfig, ia64 Kconfig defines BROKEN worked around that to make sure new changes don't break build. Eventually build breaks due to other reasons. - mips - mips64-linux-gcc4.6.3 - malta_kvm_defconfig - ppc - powerpc64-linux-gcc4.6.3 - pseries_defconfig - s390 - s390x-linux-gcc4.6.3 - defconfig - armv8 - aarch64-linux-gnu-gcc4.8.1 - defconfig ARMv7 Dirty page logging implementation overivew- - initially write protects VM RAM memory region - 2nd stage page tables - add support to read dirty page log and again write protect the dirty pages - second stage page table for next pass. - second stage huge page are dissolved into small page tables to keep track of dirty pages at page granularity. Tracking at huge page granularity limits migration to an almost idle system. Small page size logging supports higher memory dirty rates. - In the event migration is canceled, normal behavior is resumed huge pages are rebuilt over time. Changes since v11: - Implemented Alex's comments to simplify generic layer. Changes since v10: - addressed wanghaibin comments - addressed Christoffers comments Changes since v9: - Split patches into generic and architecture specific variants for TLB Flushing and dirty log read (patches 1,2 3,4,5,6) - rebased to 3.16.0-rc1 - Applied Christoffers comments. Mario Smarduch (6): KVM: Add architecture-defined TLB flush support KVM: Add generic support for dirty page logging arm: KVM: Add ARMv7 API to flush TLBs arm: KVM: Add initial dirty page locking infrastructure arm: KVM: dirty log read write protect support arm: KVM: ARMv7 dirty page logging 2nd stage page fault arch/arm/include/asm/kvm_asm.h|1 + arch/arm/include/asm/kvm_host.h | 14 +++ arch/arm/include/asm/kvm_mmu.h| 20 arch/arm/include/asm/pgtable-3level.h |1 + arch/arm/kvm/Kconfig |2 + arch/arm/kvm/Makefile |1 + arch/arm/kvm/arm.c|2 + arch/arm/kvm/interrupts.S | 11 ++ arch/arm/kvm/mmu.c| 209 +++-- arch/x86/include/asm/kvm_host.h |3 - arch/x86/kvm/Kconfig |1 + arch/x86/kvm/Makefile |1 + arch/x86/kvm/x86.c| 86 -- include/linux/kvm_host.h |4 + virt/kvm/Kconfig |6 + virt/kvm/dirtylog.c | 112 ++ virt/kvm/kvm_main.c |2 + 17 files changed, 380 insertions(+), 96 deletions(-) create mode 100644 virt/kvm/dirtylog.c Patches 1-3 seem to work fine on s390. The other patches are arm-only (well cant find 5 and 6) so I guess its ok for s390. -- 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: Preparing kvm/next for first pull request of 3.15 merge window
On 25/03/14 18:29, Paolo Bonzini wrote: Hi all, I've pushed the last set of updates to kvm/next for the 3.15 merge window. I don't expect any other changes for either x86 or s390 (thanks Christian!). The pull request is already pretty beefy. I would like to know from ARM and PPC maintainers *now* (before the merge window opens) what will be in 3.15. Also, PPC guys, please make sure the pull requests will be based on commit e724f080f5dd (KVM: PPC: Book3S HV: Fix register usage when loading/saving VRSAVE, 2014-03-13). Also, the pull requests I got in the last couple of months were a bit messy, and I'm already fearing that Linus notices. In the future, I'll tag kvm-3.x-base (e.g. kvm-3.16-base) when I open kvm/next, and I will *reject* pull requests from submaintainers that include extra non-KVM commits without a very good reason. Paolo, can you clarify this statement? What are the dont do things: (I already checked one obvious answer) [ ] I include non-kvm s390 patches (with a Maintainer ack from Martin or Heiko) which are required for other patches. These patches might even go via the s390 tree as well [ ] My pull request is based on current kvm/next instead of kvm/next that was branched away after rc1 [X] My pull request is based on 3.x-rcy instead of kvm/next [ ] My pull request is based on kvm/queue instead of kvm/next [ ] other: . Or maybe: what is your preferred way of pull requests from submaintainers? Thanks Christian -- 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] KVM: Clean up vm creation and release
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 985d825..d8d1877 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -164,24 +164,18 @@ long kvm_arch_vm_ioctl(struct file *filp,
return r;
}
-struct kvm *kvm_arch_create_vm(void)
+int kvm_arch_create_vm(struct kvm *kvm)
needs to be renamed to
kvm_arch_init_vm
[...]
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -441,7 +441,19 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu);
void kvm_free_physmem(struct kvm *kvm);
-struct kvm *kvm_arch_create_vm(void);
+#ifndef __KVM_HAVE_ARCH_VM_ALLOC
+static inline struct kvm *kvm_arch_alloc_vm(void)
+{
+ return kzalloc(sizeof(struct kvm), GFP_KERNEL);
I have not checked kvm.git but on Linux head we need
#include linux/slab.h
for kzalloc.
Christian
--
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
