diff mbox

[RFC,v3,08/13] exec.c: introduce a simple rendezvous support

Message ID 1436516626-8322-9-git-send-email-a.rigo@virtualopensystems.com
State New
Headers show

Commit Message

Alvise Rigo July 10, 2015, 8:23 a.m. UTC
When a vCPU is about to set a memory page as exclusive, it needs to wait
that all the running vCPUs finish to execute the current TB and to be aware
of the exact moment when that happens. For this, add a simple rendezvous
mechanism that will be used in softmmu_llsc_template.h to implement the
ldlink operation.

Suggested-by: Jani Kokkonen <jani.kokkonen@huawei.com>
Suggested-by: Claudio Fontana <claudio.fontana@huawei.com>
Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
---
 cpus.c            |  5 +++++
 exec.c            | 45 +++++++++++++++++++++++++++++++++++++++++++++
 include/qom/cpu.h | 16 ++++++++++++++++
 3 files changed, 66 insertions(+)

Comments

Alex Bennée July 17, 2015, 1:45 p.m. UTC | #1
Alvise Rigo <a.rigo@virtualopensystems.com> writes:

> When a vCPU is about to set a memory page as exclusive, it needs to wait
> that all the running vCPUs finish to execute the current TB and to be aware
> of the exact moment when that happens. For this, add a simple rendezvous
> mechanism that will be used in softmmu_llsc_template.h to implement the
> ldlink operation.
>
> Suggested-by: Jani Kokkonen <jani.kokkonen@huawei.com>
> Suggested-by: Claudio Fontana <claudio.fontana@huawei.com>
> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
> ---
>  cpus.c            |  5 +++++
>  exec.c            | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  include/qom/cpu.h | 16 ++++++++++++++++
>  3 files changed, 66 insertions(+)
>
> diff --git a/cpus.c b/cpus.c
> index aee445a..f4d938e 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1423,6 +1423,11 @@ static int tcg_cpu_exec(CPUArchState *env)
>      qemu_mutex_unlock_iothread();
>      ret = cpu_exec(env);
>      cpu->tcg_executing = 0;
> +
> +    if (unlikely(cpu->pending_rdv)) {
> +        cpu_exit_do_rendezvous(cpu);
> +    }
> +

I'll ignore this stuff for now as I assume we can all use the async work
patch of Fred's?


>      qemu_mutex_lock_iothread();
>  #ifdef CONFIG_PROFILER
>      tcg_time += profile_getclock() - ti;
> diff --git a/exec.c b/exec.c
> index 964e922..51958ed 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -746,6 +746,51 @@ void cpu_breakpoint_remove_all(CPUState *cpu, int mask)
>      }
>  }
>  
> +/* Rendezvous implementation.
> + * The corresponding definitions are in include/qom/cpu.h. */
> +CpuExitRendezvous cpu_exit_rendezvous;
> +inline void cpu_exit_init_rendezvous(void)
> +{
> +    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
> +
> +    rdv->attendees = 0;
> +}
> +
> +inline void cpu_exit_rendezvous_add_attendee(CPUState *cpu)
> +{
> +    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
> +
> +    if (!cpu->pending_rdv) {
> +        cpu->pending_rdv = 1;
> +        atomic_inc(&rdv->attendees);
> +    }
> +}
> +
> +void cpu_exit_do_rendezvous(CPUState *cpu)
> +{
> +    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
> +
> +    atomic_dec(&rdv->attendees);
> +
> +    cpu->pending_rdv = 0;
> +}
> +
> +void cpu_exit_rendezvous_wait_others(CPUState *cpu)
> +{
> +    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
> +
> +    while (rdv->attendees) {
> +        g_usleep(TCG_RDV_POLLING_PERIOD);
> +    }
> +}
> +
> +void cpu_exit_rendezvous_release(void)
> +{
> +    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
> +
> +    rdv->attendees = 0;
> +}
> +
>  /* enable or disable single step mode. EXCP_DEBUG is returned by the
>     CPU loop after each instruction */
>  void cpu_single_step(CPUState *cpu, int enabled)
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 8f3fe56..8d121b3 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -201,6 +201,20 @@ typedef struct CPUWatchpoint {
>      QTAILQ_ENTRY(CPUWatchpoint) entry;
>  } CPUWatchpoint;
>  
> +/* Rendezvous support */
> +#define TCG_RDV_POLLING_PERIOD 10
> +typedef struct CpuExitRendezvous {
> +    volatile int attendees;
> +    QemuMutex lock;
> +} CpuExitRendezvous;
> +
> +extern CpuExitRendezvous cpu_exit_rendezvous;
> +void cpu_exit_init_rendezvous(void);
> +void cpu_exit_rendezvous_add_attendee(CPUState *cpu);
> +void cpu_exit_do_rendezvous(CPUState *cpu);
> +void cpu_exit_rendezvous_wait_others(CPUState *cpu);
> +void cpu_exit_rendezvous_release(void);
> +
>  struct KVMState;
>  struct kvm_run;
>  
> @@ -291,6 +305,8 @@ struct CPUState {
>  
>      void *opaque;
>  
> +    volatile int pending_rdv;
> +

I will however echo the "hmmm" on Fred's patch about the optimistic use
of volatile here. As Peter mentioned it is a red flag and I would prefer
explicit memory consistency behaviour used and documented.


>      /* In order to avoid passing too many arguments to the MMIO helpers,
>       * we store some rarely used information in the CPU context.
>       */
Alvise Rigo July 17, 2015, 1:54 p.m. UTC | #2
On Fri, Jul 17, 2015 at 3:45 PM, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Alvise Rigo <a.rigo@virtualopensystems.com> writes:
>
>> When a vCPU is about to set a memory page as exclusive, it needs to wait
>> that all the running vCPUs finish to execute the current TB and to be aware
>> of the exact moment when that happens. For this, add a simple rendezvous
>> mechanism that will be used in softmmu_llsc_template.h to implement the
>> ldlink operation.
>>
>> Suggested-by: Jani Kokkonen <jani.kokkonen@huawei.com>
>> Suggested-by: Claudio Fontana <claudio.fontana@huawei.com>
>> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
>> ---
>>  cpus.c            |  5 +++++
>>  exec.c            | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>  include/qom/cpu.h | 16 ++++++++++++++++
>>  3 files changed, 66 insertions(+)
>>
>> diff --git a/cpus.c b/cpus.c
>> index aee445a..f4d938e 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -1423,6 +1423,11 @@ static int tcg_cpu_exec(CPUArchState *env)
>>      qemu_mutex_unlock_iothread();
>>      ret = cpu_exec(env);
>>      cpu->tcg_executing = 0;
>> +
>> +    if (unlikely(cpu->pending_rdv)) {
>> +        cpu_exit_do_rendezvous(cpu);
>> +    }
>> +
>
> I'll ignore this stuff for now as I assume we can all use the async work
> patch of Fred's?

Yes, it will be more likely based on the plain async_run_on_cpu.

>
>
>>      qemu_mutex_lock_iothread();
>>  #ifdef CONFIG_PROFILER
>>      tcg_time += profile_getclock() - ti;
>> diff --git a/exec.c b/exec.c
>> index 964e922..51958ed 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -746,6 +746,51 @@ void cpu_breakpoint_remove_all(CPUState *cpu, int mask)
>>      }
>>  }
>>
>> +/* Rendezvous implementation.
>> + * The corresponding definitions are in include/qom/cpu.h. */
>> +CpuExitRendezvous cpu_exit_rendezvous;
>> +inline void cpu_exit_init_rendezvous(void)
>> +{
>> +    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
>> +
>> +    rdv->attendees = 0;
>> +}
>> +
>> +inline void cpu_exit_rendezvous_add_attendee(CPUState *cpu)
>> +{
>> +    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
>> +
>> +    if (!cpu->pending_rdv) {
>> +        cpu->pending_rdv = 1;
>> +        atomic_inc(&rdv->attendees);
>> +    }
>> +}
>> +
>> +void cpu_exit_do_rendezvous(CPUState *cpu)
>> +{
>> +    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
>> +
>> +    atomic_dec(&rdv->attendees);
>> +
>> +    cpu->pending_rdv = 0;
>> +}
>> +
>> +void cpu_exit_rendezvous_wait_others(CPUState *cpu)
>> +{
>> +    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
>> +
>> +    while (rdv->attendees) {
>> +        g_usleep(TCG_RDV_POLLING_PERIOD);
>> +    }
>> +}
>> +
>> +void cpu_exit_rendezvous_release(void)
>> +{
>> +    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
>> +
>> +    rdv->attendees = 0;
>> +}
>> +
>>  /* enable or disable single step mode. EXCP_DEBUG is returned by the
>>     CPU loop after each instruction */
>>  void cpu_single_step(CPUState *cpu, int enabled)
>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>> index 8f3fe56..8d121b3 100644
>> --- a/include/qom/cpu.h
>> +++ b/include/qom/cpu.h
>> @@ -201,6 +201,20 @@ typedef struct CPUWatchpoint {
>>      QTAILQ_ENTRY(CPUWatchpoint) entry;
>>  } CPUWatchpoint;
>>
>> +/* Rendezvous support */
>> +#define TCG_RDV_POLLING_PERIOD 10
>> +typedef struct CpuExitRendezvous {
>> +    volatile int attendees;
>> +    QemuMutex lock;
>> +} CpuExitRendezvous;
>> +
>> +extern CpuExitRendezvous cpu_exit_rendezvous;
>> +void cpu_exit_init_rendezvous(void);
>> +void cpu_exit_rendezvous_add_attendee(CPUState *cpu);
>> +void cpu_exit_do_rendezvous(CPUState *cpu);
>> +void cpu_exit_rendezvous_wait_others(CPUState *cpu);
>> +void cpu_exit_rendezvous_release(void);
>> +
>>  struct KVMState;
>>  struct kvm_run;
>>
>> @@ -291,6 +305,8 @@ struct CPUState {
>>
>>      void *opaque;
>>
>> +    volatile int pending_rdv;
>> +
>
> I will however echo the "hmmm" on Fred's patch about the optimistic use
> of volatile here. As Peter mentioned it is a red flag and I would prefer
> explicit memory consistency behaviour used and documented.

In my local branch I'm now using atomic_set/atomic_read, basically
what is defined in qemu/atomic.h. Is this enough?

Regards,
alvise

>
>
>>      /* In order to avoid passing too many arguments to the MMIO helpers,
>>       * we store some rarely used information in the CPU context.
>>       */
>
> --
> Alex Bennée
diff mbox

Patch

diff --git a/cpus.c b/cpus.c
index aee445a..f4d938e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1423,6 +1423,11 @@  static int tcg_cpu_exec(CPUArchState *env)
     qemu_mutex_unlock_iothread();
     ret = cpu_exec(env);
     cpu->tcg_executing = 0;
+
+    if (unlikely(cpu->pending_rdv)) {
+        cpu_exit_do_rendezvous(cpu);
+    }
+
     qemu_mutex_lock_iothread();
 #ifdef CONFIG_PROFILER
     tcg_time += profile_getclock() - ti;
diff --git a/exec.c b/exec.c
index 964e922..51958ed 100644
--- a/exec.c
+++ b/exec.c
@@ -746,6 +746,51 @@  void cpu_breakpoint_remove_all(CPUState *cpu, int mask)
     }
 }
 
+/* Rendezvous implementation.
+ * The corresponding definitions are in include/qom/cpu.h. */
+CpuExitRendezvous cpu_exit_rendezvous;
+inline void cpu_exit_init_rendezvous(void)
+{
+    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
+
+    rdv->attendees = 0;
+}
+
+inline void cpu_exit_rendezvous_add_attendee(CPUState *cpu)
+{
+    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
+
+    if (!cpu->pending_rdv) {
+        cpu->pending_rdv = 1;
+        atomic_inc(&rdv->attendees);
+    }
+}
+
+void cpu_exit_do_rendezvous(CPUState *cpu)
+{
+    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
+
+    atomic_dec(&rdv->attendees);
+
+    cpu->pending_rdv = 0;
+}
+
+void cpu_exit_rendezvous_wait_others(CPUState *cpu)
+{
+    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
+
+    while (rdv->attendees) {
+        g_usleep(TCG_RDV_POLLING_PERIOD);
+    }
+}
+
+void cpu_exit_rendezvous_release(void)
+{
+    CpuExitRendezvous *rdv = &cpu_exit_rendezvous;
+
+    rdv->attendees = 0;
+}
+
 /* enable or disable single step mode. EXCP_DEBUG is returned by the
    CPU loop after each instruction */
 void cpu_single_step(CPUState *cpu, int enabled)
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 8f3fe56..8d121b3 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -201,6 +201,20 @@  typedef struct CPUWatchpoint {
     QTAILQ_ENTRY(CPUWatchpoint) entry;
 } CPUWatchpoint;
 
+/* Rendezvous support */
+#define TCG_RDV_POLLING_PERIOD 10
+typedef struct CpuExitRendezvous {
+    volatile int attendees;
+    QemuMutex lock;
+} CpuExitRendezvous;
+
+extern CpuExitRendezvous cpu_exit_rendezvous;
+void cpu_exit_init_rendezvous(void);
+void cpu_exit_rendezvous_add_attendee(CPUState *cpu);
+void cpu_exit_do_rendezvous(CPUState *cpu);
+void cpu_exit_rendezvous_wait_others(CPUState *cpu);
+void cpu_exit_rendezvous_release(void);
+
 struct KVMState;
 struct kvm_run;
 
@@ -291,6 +305,8 @@  struct CPUState {
 
     void *opaque;
 
+    volatile int pending_rdv;
+
     /* In order to avoid passing too many arguments to the MMIO helpers,
      * we store some rarely used information in the CPU context.
      */