Re: [PATCH kvmtool v3] aarch64: Add stolen time support

[Marc Zyngier <maz@kernel.org>]

On 2022-02-22 13:58, Alexandru Elisei wrote:
> Hi,
> 
> On Tue, Feb 22, 2022 at 10:08:30AM +0000, Sebastian Ene wrote:
>> This patch adds support for stolen time by sharing a memory region
>> with the guest which will be used by the hypervisor to store the 
>> stolen
>> time information. The exact format of the structure stored by the
>> hypervisor is described in the ARM DEN0057A document.
>> 
>> Signed-off-by: Sebastian Ene <sebastianene@google.com>
>> ---
>>  Changelog since v2:
>>  - Moved the AARCH64_PVTIME_* definitions from arm-common/kvm-arch.h 
>> to
>>    arm64/pvtime.c as pvtime is only available for arm64.
>> 
>>  Changelog since v1:
>>  - Removed the pvtime.h header file and moved the definitions to 
>> kvm-cpu-arch.h
>>    Verified if the stolen time capability is supported before 
>> allocating
>>    and mapping the memory.
>> 
>>  Makefile                               |  1 +
>>  arm/aarch64/arm-cpu.c                  |  1 +
>>  arm/aarch64/include/kvm/kvm-cpu-arch.h |  1 +
>>  arm/aarch64/pvtime.c                   | 89 
>> ++++++++++++++++++++++++++
>>  arm/kvm-cpu.c                          | 14 ++--
>>  5 files changed, 99 insertions(+), 7 deletions(-)
>>  create mode 100644 arm/aarch64/pvtime.c
>> 
>> diff --git a/Makefile b/Makefile
>> index f251147..e9121dc 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -182,6 +182,7 @@ ifeq ($(ARCH), arm64)
>>  	OBJS		+= arm/aarch64/arm-cpu.o
>>  	OBJS		+= arm/aarch64/kvm-cpu.o
>>  	OBJS		+= arm/aarch64/kvm.o
>> +	OBJS		+= arm/aarch64/pvtime.o
>>  	ARCH_INCLUDE	:= $(HDRS_ARM_COMMON)
>>  	ARCH_INCLUDE	+= -Iarm/aarch64/include
>> 
>> diff --git a/arm/aarch64/arm-cpu.c b/arm/aarch64/arm-cpu.c
>> index d7572b7..326fb20 100644
>> --- a/arm/aarch64/arm-cpu.c
>> +++ b/arm/aarch64/arm-cpu.c
>> @@ -22,6 +22,7 @@ static void generate_fdt_nodes(void *fdt, struct kvm 
>> *kvm)
>>  static int arm_cpu__vcpu_init(struct kvm_cpu *vcpu)
>>  {
>>  	vcpu->generate_fdt_nodes = generate_fdt_nodes;
>> +	kvm_cpu__setup_pvtime(vcpu);
>>  	return 0;
>>  }
>> 
>> diff --git a/arm/aarch64/include/kvm/kvm-cpu-arch.h 
>> b/arm/aarch64/include/kvm/kvm-cpu-arch.h
>> index 8dfb82e..b57d6e6 100644
>> --- a/arm/aarch64/include/kvm/kvm-cpu-arch.h
>> +++ b/arm/aarch64/include/kvm/kvm-cpu-arch.h
>> @@ -19,5 +19,6 @@
>> 
>>  void kvm_cpu__select_features(struct kvm *kvm, struct kvm_vcpu_init 
>> *init);
>>  int kvm_cpu__configure_features(struct kvm_cpu *vcpu);
>> +void kvm_cpu__setup_pvtime(struct kvm_cpu *vcpu);
>> 
>>  #endif /* KVM__KVM_CPU_ARCH_H */
>> diff --git a/arm/aarch64/pvtime.c b/arm/aarch64/pvtime.c
>> new file mode 100644
>> index 0000000..247e4f3
>> --- /dev/null
>> +++ b/arm/aarch64/pvtime.c
>> @@ -0,0 +1,89 @@
>> +#include "kvm/kvm.h"
>> +#include "kvm/kvm-cpu.h"
>> +#include "kvm/util.h"
>> +
>> +#include <linux/byteorder.h>
>> +#include <linux/types.h>
>> +
>> +#define AARCH64_PVTIME_IPA_MAX_SIZE	SZ_64K
>> +#define AARCH64_PVTIME_IPA_START	(ARM_MEMORY_AREA - \
>> +					 AARCH64_PVTIME_IPA_MAX_SIZE)
> 
> This doesn't change the fact that it overlaps with KVM_PCI_MMIO_AREA, 
> which is
> exposed to the guest in the DTB (see my reply to v2).

Yup, this is a bit of a problem, and overlapping regions are
a big no-no. Why can't the pvtime region be dynamically placed
after the RAM (after checking that there is enough space to
register it in the IPA space)?

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...