| Message ID | 20211117064359.2362060-2-reijiw@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Make CPU ID registers writable by userspace | expand |
On Wed, 17 Nov 2021 06:43:31 +0000, Reiji Watanabe <reijiw@google.com> wrote: > > Introduce 'has_reset_once' flag in kvm_vcpu_arch, which indicates > if the vCPU reset has been done once, for later use. It would be nice if you could at least hint at what this flag is going to be used for, as being able to reset a vcpu as often as userspace wants it is part of the KVM ABI. > > Signed-off-by: Reiji Watanabe <reijiw@google.com> > Reviewed-by: Oliver Upton <oupton@google.com> > --- > arch/arm64/include/asm/kvm_host.h | 2 ++ > arch/arm64/kvm/reset.c | 4 ++++ > 2 files changed, 6 insertions(+) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 2a5f7f38006f..edbe2cb21947 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -385,6 +385,7 @@ struct kvm_vcpu_arch { > u64 last_steal; > gpa_t base; > } steal; > + bool has_reset_once; Why can't this be a new flag (part of the vcpu->arch.flags set) rather than a discrete bool? Thanks, M.
On Sun, Nov 21, 2021 at 4:36 AM Marc Zyngier <maz@kernel.org> wrote: > > On Wed, 17 Nov 2021 06:43:31 +0000, > Reiji Watanabe <reijiw@google.com> wrote: > > > > Introduce 'has_reset_once' flag in kvm_vcpu_arch, which indicates > > if the vCPU reset has been done once, for later use. > > It would be nice if you could at least hint at what this flag is going > to be used for, as being able to reset a vcpu as often as userspace > wants it is part of the KVM ABI. I will update the description. > > Signed-off-by: Reiji Watanabe <reijiw@google.com> > > Reviewed-by: Oliver Upton <oupton@google.com> > > --- > > arch/arm64/include/asm/kvm_host.h | 2 ++ > > arch/arm64/kvm/reset.c | 4 ++++ > > 2 files changed, 6 insertions(+) > > > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > > index 2a5f7f38006f..edbe2cb21947 100644 > > --- a/arch/arm64/include/asm/kvm_host.h > > +++ b/arch/arm64/include/asm/kvm_host.h > > @@ -385,6 +385,7 @@ struct kvm_vcpu_arch { > > u64 last_steal; > > gpa_t base; > > } steal; > > + bool has_reset_once; > > Why can't this be a new flag (part of the vcpu->arch.flags set) rather > than a discrete bool? Thank you for the suggestion ! I will fix it to use vcpu->arch.flags. Regards, Reiji
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 2a5f7f38006f..edbe2cb21947 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -385,6 +385,7 @@ struct kvm_vcpu_arch { u64 last_steal; gpa_t base; } steal; + bool has_reset_once; }; /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */ @@ -450,6 +451,7 @@ struct kvm_vcpu_arch { #define vcpu_has_sve(vcpu) (system_supports_sve() && \ ((vcpu)->arch.flags & KVM_ARM64_GUEST_HAS_SVE)) +#define vcpu_has_reset_once(vcpu) ((vcpu)->arch.has_reset_once) #ifdef CONFIG_ARM64_PTR_AUTH #define vcpu_has_ptrauth(vcpu) \ diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c index 426bd7fbc3fd..c3a91ab370fa 100644 --- a/arch/arm64/kvm/reset.c +++ b/arch/arm64/kvm/reset.c @@ -305,6 +305,10 @@ int kvm_reset_vcpu(struct kvm_vcpu *vcpu) if (loaded) kvm_arch_vcpu_load(vcpu, smp_processor_id()); preempt_enable(); + + if (!ret && !vcpu->arch.has_reset_once) + vcpu->arch.has_reset_once = true; + return ret; }