From: Fuad Tabba <tabba@google.com>
To: Marc Zyngier <maz@kernel.org>
Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
will@kernel.org, qperret@google.com, seanjc@google.com,
alexandru.elisei@arm.com, catalin.marinas@arm.com,
philmd@linaro.org, james.morse@arm.com, suzuki.poulose@arm.com,
oliver.upton@linux.dev, mark.rutland@arm.com,
broonie@kernel.org, joey.gouly@arm.com, rananta@google.com,
yuzenghui@huawei.com
Subject: Re: [PATCH v2 4/7] KVM: arm64: Store the maximum sve vector length at hyp
Date: Wed, 22 May 2024 15:36:13 +0100 [thread overview]
Message-ID: <CA+EHjTySE1V+2q2wKOuop00P0vSjpG8DVvWiw5NWNo0+ESWHXg@mail.gmail.com> (raw)
In-Reply-To: <86r0dun9nm.wl-maz@kernel.org>
Hi Marc,
On Tue, May 21, 2024 at 10:21 PM Marc Zyngier <maz@kernel.org> wrote:
>
> On Tue, 21 May 2024 17:37:17 +0100,
> Fuad Tabba <tabba@google.com> wrote:
> >
> > In subsequent patches, hyp needs to know the maximum sve vector
> > length for the host, without needing the trust the host for that.
> > This is used when allocating memory for the host sve state in the
> > following patch, as well as for allocating and restricting guest
> > sve state in a future patch series.
> >
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> > arch/arm64/include/asm/kvm_host.h | 1 +
> > arch/arm64/include/asm/kvm_hyp.h | 1 +
> > arch/arm64/kvm/arm.c | 1 +
> > arch/arm64/kvm/hyp/nvhe/pkvm.c | 2 ++
> > arch/arm64/kvm/reset.c | 2 ++
> > 5 files changed, 7 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 8170c04fde91..0a5fceb20f3a 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -76,6 +76,7 @@ static inline enum kvm_mode kvm_get_mode(void) { return KVM_MODE_NONE; };
> > DECLARE_STATIC_KEY_FALSE(userspace_irqchip_in_use);
> >
> > extern unsigned int __ro_after_init kvm_sve_max_vl;
> > +extern unsigned int __ro_after_init kvm_host_sve_max_vl;
> > int __init kvm_arm_init_sve(void);
> >
> > u32 __attribute_const__ kvm_target_cpu(void);
> > diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
> > index 2ab23589339a..d313adf53bef 100644
> > --- a/arch/arm64/include/asm/kvm_hyp.h
> > +++ b/arch/arm64/include/asm/kvm_hyp.h
> > @@ -143,5 +143,6 @@ extern u64 kvm_nvhe_sym(id_aa64smfr0_el1_sys_val);
> >
> > extern unsigned long kvm_nvhe_sym(__icache_flags);
> > extern unsigned int kvm_nvhe_sym(kvm_arm_vmid_bits);
> > +extern unsigned int kvm_nvhe_sym(kvm_host_sve_max_vl);
> >
> > #endif /* __ARM64_KVM_HYP_H__ */
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index 9996a989b52e..9e565ea3d645 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -2378,6 +2378,7 @@ static void kvm_hyp_init_symbols(void)
> > kvm_nvhe_sym(id_aa64smfr0_el1_sys_val) = read_sanitised_ftr_reg(SYS_ID_AA64SMFR0_EL1);
> > kvm_nvhe_sym(__icache_flags) = __icache_flags;
> > kvm_nvhe_sym(kvm_arm_vmid_bits) = kvm_arm_vmid_bits;
> > + kvm_nvhe_sym(kvm_host_sve_max_vl) = kvm_host_sve_max_vl;
> > }
> >
> > static int __init kvm_hyp_init_protection(u32 hyp_va_bits)
> > diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > index 16aa4875ddb8..25e9a94f6d76 100644
> > --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> > @@ -18,6 +18,8 @@ unsigned long __icache_flags;
> > /* Used by kvm_get_vttbr(). */
> > unsigned int kvm_arm_vmid_bits;
> >
> > +unsigned int kvm_host_sve_max_vl;
> > +
> > /*
> > * Set trap register values based on features in ID_AA64PFR0.
> > */
> > diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> > index 1b7b58cb121f..e818727900ec 100644
> > --- a/arch/arm64/kvm/reset.c
> > +++ b/arch/arm64/kvm/reset.c
> > @@ -46,11 +46,13 @@ static u32 __ro_after_init kvm_ipa_limit;
> > PSR_AA32_I_BIT | PSR_AA32_F_BIT)
> >
> > unsigned int __ro_after_init kvm_sve_max_vl;
> > +unsigned int __ro_after_init kvm_host_sve_max_vl;
> >
> > int __init kvm_arm_init_sve(void)
> > {
> > if (system_supports_sve()) {
> > kvm_sve_max_vl = sve_max_virtualisable_vl();
> > + kvm_host_sve_max_vl = sve_max_vl();
>
> Why the intermediate storage?
It's not needed for this patch, but rather for the one that allocates
the memory for the host sve state, to be able to reuse the same
functions at el1 and hyp, e.g., pkvm_host_sve_state_size() . I'll move
it to where it's used or find a way of getting rid of it altogether.
> Setting kvm_nvhe_sym(kvm_host_sve_max_vl) to sve_max_vl() should be
> enough. But it doesn't mean that all the CPUs support the same max VL,
> and I wonder whether we end-up with the wrong in-memory layout in this
> case...
Looking at how the value behind sve_max_vl() is calculated
(vl_info[ARM64_VEC_SVE].max_vl;): it seems to store the maximum VL in
common to all CPUs, which in turn becomes a limit to the usable VLs in
the system as a whole.
So for example PR_SVE_SET_VL (sve_set_current_vl()) eventually comes
down to find_supported_vector_length(), which limits the VL set to the
lengths supported in vl_info[ARM64_VEC_SVE], which are system-wide
rather than per cpu.
So if my understanding is correct, I don't think we need to allocate
more memory than sve_max_vl() to store the host state. It shouldn't be
a problem in terms of layout either, as long as we consistently use it
for save and restore. Similar to how kvm_arch_vcpu_put_fp() always
uses the vcpu_sve_max_vq(). It does need to be documented, as you
mention in a later patch.
Thanks,
/fuad
>
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
next prev parent reply other threads:[~2024-05-22 14:36 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-21 16:37 [PATCH v2 0/7] KVM: arm64: Fix handling of host fpsimd/sve state in protected mode Fuad Tabba
2024-05-21 16:37 ` [PATCH v2 1/7] KVM: arm64: Reintroduce __sve_save_state Fuad Tabba
2024-05-21 16:37 ` [PATCH v2 2/7] KVM: arm64: Abstract set/clear of CPTR_EL2 bits behind helper Fuad Tabba
2024-05-21 21:08 ` Marc Zyngier
2024-05-22 13:48 ` Fuad Tabba
2024-05-28 7:58 ` Marc Zyngier
2024-05-21 16:37 ` [PATCH v2 3/7] KVM: arm64: Specialize handling of host fpsimd state on trap Fuad Tabba
2024-05-21 16:37 ` [PATCH v2 4/7] KVM: arm64: Store the maximum sve vector length at hyp Fuad Tabba
2024-05-21 21:21 ` Marc Zyngier
2024-05-22 14:36 ` Fuad Tabba [this message]
2024-05-21 16:37 ` [PATCH v2 5/7] KVM: arm64: Allocate memory at hyp for host sve state in pKVM Fuad Tabba
2024-05-21 21:44 ` Marc Zyngier
2024-05-22 14:37 ` Fuad Tabba
2024-05-28 8:16 ` Marc Zyngier
2024-05-21 16:37 ` [PATCH v2 6/7] KVM: arm64: Eagerly restore host fpsimd/sve " Fuad Tabba
2024-05-21 22:52 ` Marc Zyngier
2024-05-22 14:48 ` Fuad Tabba
2024-05-28 8:21 ` Marc Zyngier
2024-05-21 16:37 ` [PATCH v2 7/7] KVM: arm64: Consolidate initializing the host data's fpsimd_state/sve " Fuad Tabba
2024-05-21 22:55 ` Marc Zyngier
2024-05-22 14:49 ` Fuad Tabba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CA+EHjTySE1V+2q2wKOuop00P0vSjpG8DVvWiw5NWNo0+ESWHXg@mail.gmail.com \
--to=tabba@google.com \
--cc=alexandru.elisei@arm.com \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=james.morse@arm.com \
--cc=joey.gouly@arm.com \
--cc=kvmarm@lists.linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=maz@kernel.org \
--cc=oliver.upton@linux.dev \
--cc=philmd@linaro.org \
--cc=qperret@google.com \
--cc=rananta@google.com \
--cc=seanjc@google.com \
--cc=suzuki.poulose@arm.com \
--cc=will@kernel.org \
--cc=yuzenghui@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).