On Fri, Jul 17, 2015 at 3:45 PM, Alex Bennée <alex.ben...@linaro.org> wrote:
>
> Alvise Rigo <a.r...@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.kokko...@huawei.com>
>> Suggested-by: Claudio Fontana <claudio.font...@huawei.com>
>> Signed-off-by: Alvise Rigo <a.r...@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
