Re: [PATCH v3 06/10] hvf: Add Apple Silicon support

[Alexander Graf <agraf@csgraf.de>]

On 03.12.20 06:21, Roman Bolshakov wrote:
> On Wed, Dec 02, 2020 at 08:04:04PM +0100, Alexander Graf wrote:
>> With Apple Silicon available to the masses, it's a good time to add support
>> for driving its virtualization extensions from QEMU.
>>
>> This patch adds all necessary architecture specific code to get basic VMs
>> working. It's still pretty raw, but definitely functional.
>>
> That's very cool, Alex!
>
>> [...]
>> diff --git a/accel/hvf/hvf-cpus.c b/accel/hvf/hvf-cpus.c
>> index a423f629d5..e613c22ad0 100644
>> --- a/accel/hvf/hvf-cpus.c
>> +++ b/accel/hvf/hvf-cpus.c
>> @@ -60,6 +60,10 @@
>>   
>>   #include <Hypervisor/Hypervisor.h>
>>   
> On an older laptop with 10.15 I've noticed this causes a build failure.
> Here's layout of Hypervisor.framework on 10.15:
>
>   Hypervisor.framework find .
>   .
>   ./Versions
>   ./Versions/A
>   ./Versions/A/Hypervisor.tbd
>   ./Versions/A/Headers
>   ./Versions/A/Headers/hv_arch_vmx.h
>   ./Versions/A/Headers/hv_error.h
>   ./Versions/A/Headers/hv_types.h
>   ./Versions/A/Headers/hv.h
>   ./Versions/A/Headers/hv_arch_x86.h
>   ./Versions/A/Headers/hv_vmx.h
>   ./Versions/Current
>   ./module.map
>   ./Hypervisor.tbd
>   ./Headers
>
> The issue also exists in another patch in the series:
>    "hvf: Move common code out"


Ugh, I'll try and see if I can dig out a 10.15 machine somewhere.


>
>> +#ifdef __aarch64__
>> +#define HV_VM_DEFAULT NULL
>> +#endif
>> +
> I don't see if it's used anywhere.


It's used in hv_vm_create() as argument. The enum that contains it is 
hidden behind an #ifdef __x86_64__ in HVF though.


>
>>   /* Memory slots */
>>   
>>   struct mac_slot {
>> [...]
>>
> Side question. I have very little knowledge of ARM but it seems much
> leaner compared to x86 trap/emulation layer. Is it a consequence of
> load/store architecture and it's expected to be that small on ARM?


It's multiple things coming together. Early in the virtualization days 
of KVM on ARM, we decided that KVM would only support MMIO for 
instructions that hardware predecodes on exception. That seems to have 
trickled down into basically all relevant OSs these days, so you can run 
Linux, *BSD and Windows just fine without handling any instructions that 
are not already predecoded for you.

This in turn means you don't need an instruction emulator, which is most 
of the complexity on the x86 hvf code.


> I have only noticed MMIO, system registers (access to them apparently
> leads to a trap), kick and PSCI traps (which sounds somewhat similar to
> Intel MPSpec/APIC) and no system instruction traps (except WFI in the
> next patch).


System Registers are a bit tricky. Some of them lead to traps (as you've 
seen), others do not and instead read/write shadow registers directly. 
For those, we need to do the register sync.

But yes, there is little to handle for an ARM guest. I was positively 
surprised as well. To be fair though, most of the complexity in KVM ARM 
code comes from vGIC (not available on M1), debug registers (not handled 
here yet), PMU multiplexing (not handled) and stage2 page table 
maintenance (done by HVF). We just wiggle ourselves out of those.


Alex