From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id E0BB12737EF for ; Mon, 4 Aug 2025 14:45:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1754318706; cv=none; b=nz7cFOZbV1O1FptmA4SIdFkzi/po0OoUyOZ3CpR5k80Ze6BLtoa5O2tj3LG3vhPF0murE0Vh86MuO24+twBSHKzJwScyAujwO6BKHDjoypTnP1WubcDTdsGrMDjeSGhwEdw9K/vpoNdr3pzuVLffvhAFC33yNuKPFfu/IO6ARTQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1754318706; c=relaxed/simple; bh=7uuuSHXLG2akh0J5M63x8UNL3dsZ5ISwWooQgHd3C24=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=bey6YfrM6K3FPpqmGi2c2qTROH7LwbP01KgL2/1Bi+orODNJLFS3oXGpnPnDiaicpjUoFNDQUHzUDiooL+2ht2odgWo4oVja3eoewhQ/AL74NDD2NUPCCSLA/MHaSh7a/Gp5IajmrYx135UVbsv1P6qaxUC5DowUjPUP3cnV2MA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3F011150C; Mon, 4 Aug 2025 07:44:56 -0700 (PDT) Received: from raptor (usa-sjc-mx-foss1.foss.arm.com [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 40EB53F738; Mon, 4 Aug 2025 07:45:03 -0700 (PDT) Date: Mon, 4 Aug 2025 15:45:00 +0100 From: Alexandru Elisei To: Andre Przywara Cc: Will Deacon , Julien Thierry , Marc Zyngier , kvm@vger.kernel.org, kvmarm@lists.linux.dev Subject: Re: [PATCH kvmtool v3 4/6] arm64: add counter offset control Message-ID: References: <20250729095745.3148294-1-andre.przywara@arm.com> <20250729095745.3148294-5-andre.przywara@arm.com> Precedence: bulk X-Mailing-List: kvm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250729095745.3148294-5-andre.przywara@arm.com> Hi Andre, You might want to capitalize the first letter of the subject line (add->Add). On Tue, Jul 29, 2025 at 10:57:43AM +0100, Andre Przywara wrote: > From: Marc Zyngier > > KVM allows the offsetting of the global counter in order to help with > migration of a VM. This offset applies cumulatively with the offsets > provided by the architecture. > > Although kvmtool doesn't provide a way to migrate a VM, controlling > this offset is useful to test the timer subsystem. > > Add the command line option --counter-offset to allow setting this value > when creating a VM. Out of curiosity, how is this related to nested virtualization? > > Signed-off-by: Marc Zyngier > Signed-off-by: Andre Przywara > --- > arm64/include/kvm/kvm-config-arch.h | 3 +++ > arm64/kvm.c | 17 +++++++++++++++++ > 2 files changed, 20 insertions(+) > > diff --git a/arm64/include/kvm/kvm-config-arch.h b/arm64/include/kvm/kvm-config-arch.h > index a1dac28e6..44c43367b 100644 > --- a/arm64/include/kvm/kvm-config-arch.h > +++ b/arm64/include/kvm/kvm-config-arch.h > @@ -14,6 +14,7 @@ struct kvm_config_arch { > u64 kaslr_seed; > enum irqchip_type irqchip; > u64 fw_addr; > + u64 counter_offset; > unsigned int sve_max_vq; > bool no_pvtime; > }; > @@ -59,6 +60,8 @@ int sve_vl_parser(const struct option *opt, const char *arg, int unset); > irqchip_parser, NULL), \ > OPT_U64('\0', "firmware-address", &(cfg)->fw_addr, \ > "Address where firmware should be loaded"), \ > + OPT_U64('\0', "counter-offset", &(cfg)->counter_offset, \ > + "Specify the counter offset, defaulting to 0"), \ I'm having a hard time parsing this - if it's zero, then kvmtool leaves it unset, how is the default value 0? Maybe you want to say that if left unset, the counters behaves as if the global offset is zero. > OPT_BOOLEAN('\0', "nested", &(cfg)->nested_virt, \ > "Start VCPUs in EL2 (for nested virt)"), > > diff --git a/arm64/kvm.c b/arm64/kvm.c > index 23b4dab1f..6e971dd78 100644 > --- a/arm64/kvm.c > +++ b/arm64/kvm.c > @@ -119,6 +119,22 @@ static void kvm__arch_enable_mte(struct kvm *kvm) > pr_debug("MTE capability enabled"); > } > > +static void kvm__arch_set_counter_offset(struct kvm *kvm) > +{ > + struct kvm_arm_counter_offset offset = { > + .counter_offset = kvm->cfg.arch.counter_offset, > + }; > + > + if (!kvm->cfg.arch.counter_offset) > + return; > + > + if (!kvm__supports_extension(kvm, KVM_CAP_COUNTER_OFFSET)) > + die("No support for global counter offset"); What happens when the user sets --counter-offset 0 and KVM doesn't support the capability? Looks to me like instead of getting an error, kvmtool is happy to proceed without actually setting the counter offset to 0. User might then be fooled into thinking that KVM supports KVM_CAP_COUNTER_OFFSET, and when the same user does --counter-offset x, they will get an error saying that there's no support for it in KVM. I would be extremely confused by that. If this is something that you want to address, you can do it similar to ram_addr: initialize the offset to something unreasonable before parsing the command line parameters, and then bail early in kvm__arch_set_counter_offset(). Thanks, Alex > + > + if (ioctl(kvm->vm_fd, KVM_ARM_SET_COUNTER_OFFSET, &offset)) > + die_perror("KVM_ARM_SET_COUNTER_OFFSET"); > +} > + > void kvm__arch_init(struct kvm *kvm) > { > /* Create the virtual GIC. */ > @@ -126,6 +142,7 @@ void kvm__arch_init(struct kvm *kvm) > die("Failed to create virtual GIC"); > > kvm__arch_enable_mte(kvm); > + kvm__arch_set_counter_offset(kvm); > } > > static u64 kvm__arch_get_payload_region_size(struct kvm *kvm) > -- > 2.25.1 >