Re: [PATCH 6/8] hvf: Use OS provided vcpu kick function

From: Alexander Graf <agraf@csgraf.de>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Richard Henderson <richard.henderson@linaro.org>,
	qemu-devel@nongnu.org, Cameron Esfahani <dirty@apple.com>,
	Roman Bolshakov <r.bolshakov@yadro.com>,
	qemu-arm@nongnu.org, Claudio Fontana <cfontana@suse.de>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH 6/8] hvf: Use OS provided vcpu kick function
Date: Mon, 30 Nov 2020 03:42:09 +0100	[thread overview]
Message-ID: <2281f481-d64a-1750-0bd2-e7f52b26da51@csgraf.de> (raw)
In-Reply-To: <20201126221808.GU2271382@habkost.net>

On 26.11.20 23:18, Eduardo Habkost wrote:
> On Thu, Nov 26, 2020 at 10:50:15PM +0100, Alexander Graf wrote:
>> When kicking another vCPU, we get an OS function that explicitly does that for us
>> on Apple Silicon. That works better than the current signaling logic, let's make
>> use of it there.
>>
>> Signed-off-by: Alexander Graf <agraf@csgraf.de>
>> ---
>>   accel/hvf/hvf-cpus.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/accel/hvf/hvf-cpus.c b/accel/hvf/hvf-cpus.c
>> index b9f674478d..74a272d2e8 100644
>> --- a/accel/hvf/hvf-cpus.c
>> +++ b/accel/hvf/hvf-cpus.c
>> @@ -418,8 +418,20 @@ static void hvf_start_vcpu_thread(CPUState *cpu)
>>                          cpu, QEMU_THREAD_JOINABLE);
>>   }
>>   
>> +#ifdef __aarch64__
>> +static void hvf_kick_vcpu_thread(CPUState *cpu)
>> +{
>> +    if (!qemu_cpu_is_self(cpu)) {
>> +        hv_vcpus_exit(&cpu->hvf_fd, 1);
>> +    }
>> +}
>> +#endif
>> +
>>   static const CpusAccel hvf_cpus = {
>>       .create_vcpu_thread = hvf_start_vcpu_thread,
>> +#ifdef __aarch64__
>> +    .kick_vcpu_thread = hvf_kick_vcpu_thread,
>> +#endif
> Interesting.  We have considered the possibility of adding
> arch-specific TYPE_ACCEL subclasses when discussing Claudio's,
> series.  Here we have another arch-specific hack that could be
> avoided if we had a TYPE_ARM_HVF_ACCEL QOM class.

I don't think that's necessary in this case. I don't see how you could 
ever have aarch64 and x86 HVF backends compiled into the same binary. 
The header files even have a lot of #ifdef's.

Either way, I've changed it to a weak function in v2. That way it's a 
bit easier to read.

Alex