From: Marc Zyngier <maz@kernel.org>
To: Reiji Watanabe <reijiw@google.com>
Cc: kvm@vger.kernel.org, Will Deacon <will@kernel.org>,
Peter Shier <pshier@google.com>,
Paolo Bonzini <pbonzini@redhat.com>,
kvmarm@lists.cs.columbia.edu,
Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2 1/2] KVM: arm64: mixed-width check should be skipped for uninitialized vCPUs
Date: Wed, 09 Feb 2022 12:04:37 +0000 [thread overview]
Message-ID: <875ypo5jqi.wl-maz@kernel.org> (raw)
In-Reply-To: <CAAeT=FwjcgTM0hKSERfVMYDvYWqdC+Deqd=x2xT=-Zymb6SLtA@mail.gmail.com>
Hi Reiji,
On Wed, 09 Feb 2022 05:32:36 +0000,
Reiji Watanabe <reijiw@google.com> wrote:
>
> Hi Marc,
>
> On Tue, Feb 8, 2022 at 6:41 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > In [1], I suggested another approach that didn't require extra state,
> > and moved the existing checks under the kvm lock. What was wrong with
> > that approach?
>
> With that approach, even for a vcpu that has a broken set of features,
> which leads kvm_reset_vcpu() to fail for the vcpu, the vcpu->arch.features
> are checked by other vCPUs' vcpu_allowed_register_width() until the
> vcpu->arch.target is set to -1.
> Due to this, I would think some or possibly all vCPUs' kvm_reset_vcpu()
> may or may not fail (e.g. if userspace tries to configure vCPU#0 with
> 32bit EL1, and vCPU#1 and #2 with 64 bit EL1, KVM_ARM_VCPU_INIT
> for either vCPU#0, or both vCPU#1 and #2 should fail. But, with that
> approach, it doesn't always work that way. Instead, KVM_ARM_VCPU_INIT
> for all vCPUs could fail or KVM_ARM_VCPU_INIT for vCPU#0 and #1 could
> fail while the one for CPU#2 works).
> Also, even after the first KVM_RUN for vCPUs are already done,
> (the first) KVM_ARM_VCPU_INIT for another vCPU could cause the
> kvm_reset_vcpu() for those vCPUs to fail.
>
> I would think those behaviors are odd, and I wanted to avoid them.
OK, fair enough. But then you need to remove most of the uses of
KVM_ARM_VCPU_EL1_32BIT so that it is only used as a userspace
interface and maybe not carried as part of the vcpu feature flag
anymore.
Also, we really should turn all these various bits in the kvm struct
into a set of flags. I have a patch posted there[1] for this, feel
free to pick it up.
Thanks,
M.
[1] https://lore.kernel.org/r/20211004174849.2831548-2-maz@kernel.org
--
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Reiji Watanabe <reijiw@google.com>
Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
James Morse <james.morse@arm.com>,
Alexandru Elisei <alexandru.elisei@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Will Deacon <will@kernel.org>, Peter Shier <pshier@google.com>,
Ricardo Koller <ricarkol@google.com>,
Oliver Upton <oupton@google.com>,
Jing Zhang <jingzhangos@google.com>,
Raghavendra Rao Anata <rananta@google.com>
Subject: Re: [PATCH v2 1/2] KVM: arm64: mixed-width check should be skipped for uninitialized vCPUs
Date: Wed, 09 Feb 2022 12:04:37 +0000 [thread overview]
Message-ID: <875ypo5jqi.wl-maz@kernel.org> (raw)
In-Reply-To: <CAAeT=FwjcgTM0hKSERfVMYDvYWqdC+Deqd=x2xT=-Zymb6SLtA@mail.gmail.com>
Hi Reiji,
On Wed, 09 Feb 2022 05:32:36 +0000,
Reiji Watanabe <reijiw@google.com> wrote:
>
> Hi Marc,
>
> On Tue, Feb 8, 2022 at 6:41 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > In [1], I suggested another approach that didn't require extra state,
> > and moved the existing checks under the kvm lock. What was wrong with
> > that approach?
>
> With that approach, even for a vcpu that has a broken set of features,
> which leads kvm_reset_vcpu() to fail for the vcpu, the vcpu->arch.features
> are checked by other vCPUs' vcpu_allowed_register_width() until the
> vcpu->arch.target is set to -1.
> Due to this, I would think some or possibly all vCPUs' kvm_reset_vcpu()
> may or may not fail (e.g. if userspace tries to configure vCPU#0 with
> 32bit EL1, and vCPU#1 and #2 with 64 bit EL1, KVM_ARM_VCPU_INIT
> for either vCPU#0, or both vCPU#1 and #2 should fail. But, with that
> approach, it doesn't always work that way. Instead, KVM_ARM_VCPU_INIT
> for all vCPUs could fail or KVM_ARM_VCPU_INIT for vCPU#0 and #1 could
> fail while the one for CPU#2 works).
> Also, even after the first KVM_RUN for vCPUs are already done,
> (the first) KVM_ARM_VCPU_INIT for another vCPU could cause the
> kvm_reset_vcpu() for those vCPUs to fail.
>
> I would think those behaviors are odd, and I wanted to avoid them.
OK, fair enough. But then you need to remove most of the uses of
KVM_ARM_VCPU_EL1_32BIT so that it is only used as a userspace
interface and maybe not carried as part of the vcpu feature flag
anymore.
Also, we really should turn all these various bits in the kvm struct
into a set of flags. I have a patch posted there[1] for this, feel
free to pick it up.
Thanks,
M.
[1] https://lore.kernel.org/r/20211004174849.2831548-2-maz@kernel.org
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Reiji Watanabe <reijiw@google.com>
Cc: kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
James Morse <james.morse@arm.com>,
Alexandru Elisei <alexandru.elisei@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Will Deacon <will@kernel.org>, Peter Shier <pshier@google.com>,
Ricardo Koller <ricarkol@google.com>,
Oliver Upton <oupton@google.com>,
Jing Zhang <jingzhangos@google.com>,
Raghavendra Rao Anata <rananta@google.com>
Subject: Re: [PATCH v2 1/2] KVM: arm64: mixed-width check should be skipped for uninitialized vCPUs
Date: Wed, 09 Feb 2022 12:04:37 +0000 [thread overview]
Message-ID: <875ypo5jqi.wl-maz@kernel.org> (raw)
In-Reply-To: <CAAeT=FwjcgTM0hKSERfVMYDvYWqdC+Deqd=x2xT=-Zymb6SLtA@mail.gmail.com>
Hi Reiji,
On Wed, 09 Feb 2022 05:32:36 +0000,
Reiji Watanabe <reijiw@google.com> wrote:
>
> Hi Marc,
>
> On Tue, Feb 8, 2022 at 6:41 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > In [1], I suggested another approach that didn't require extra state,
> > and moved the existing checks under the kvm lock. What was wrong with
> > that approach?
>
> With that approach, even for a vcpu that has a broken set of features,
> which leads kvm_reset_vcpu() to fail for the vcpu, the vcpu->arch.features
> are checked by other vCPUs' vcpu_allowed_register_width() until the
> vcpu->arch.target is set to -1.
> Due to this, I would think some or possibly all vCPUs' kvm_reset_vcpu()
> may or may not fail (e.g. if userspace tries to configure vCPU#0 with
> 32bit EL1, and vCPU#1 and #2 with 64 bit EL1, KVM_ARM_VCPU_INIT
> for either vCPU#0, or both vCPU#1 and #2 should fail. But, with that
> approach, it doesn't always work that way. Instead, KVM_ARM_VCPU_INIT
> for all vCPUs could fail or KVM_ARM_VCPU_INIT for vCPU#0 and #1 could
> fail while the one for CPU#2 works).
> Also, even after the first KVM_RUN for vCPUs are already done,
> (the first) KVM_ARM_VCPU_INIT for another vCPU could cause the
> kvm_reset_vcpu() for those vCPUs to fail.
>
> I would think those behaviors are odd, and I wanted to avoid them.
OK, fair enough. But then you need to remove most of the uses of
KVM_ARM_VCPU_EL1_32BIT so that it is only used as a userspace
interface and maybe not carried as part of the vcpu feature flag
anymore.
Also, we really should turn all these various bits in the kvm struct
into a set of flags. I have a patch posted there[1] for this, feel
free to pick it up.
Thanks,
M.
[1] https://lore.kernel.org/r/20211004174849.2831548-2-maz@kernel.org
--
Without deviation from the norm, progress is not possible.
next prev parent reply other threads:[~2022-02-09 12:04 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-18 4:19 [PATCH v2 1/2] KVM: arm64: mixed-width check should be skipped for uninitialized vCPUs Reiji Watanabe
2022-01-18 4:19 ` Reiji Watanabe
2022-01-18 4:19 ` Reiji Watanabe
2022-01-18 4:19 ` [PATCH v2 2/2] KVM: arm64: selftests: Introduce vcpu_width_config Reiji Watanabe
2022-01-18 4:19 ` Reiji Watanabe
2022-01-18 4:19 ` Reiji Watanabe
2022-02-08 14:41 ` [PATCH v2 1/2] KVM: arm64: mixed-width check should be skipped for uninitialized vCPUs Marc Zyngier
2022-02-08 14:41 ` Marc Zyngier
2022-02-08 14:41 ` Marc Zyngier
2022-02-09 5:32 ` Reiji Watanabe
2022-02-09 5:32 ` Reiji Watanabe
2022-02-09 5:32 ` Reiji Watanabe
2022-02-09 12:04 ` Marc Zyngier [this message]
2022-02-09 12:04 ` Marc Zyngier
2022-02-09 12:04 ` Marc Zyngier
2022-02-10 5:31 ` Reiji Watanabe
2022-02-10 5:31 ` Reiji Watanabe
2022-02-10 5:31 ` Reiji Watanabe
2022-02-10 10:31 ` Marc Zyngier
2022-02-10 10:31 ` Marc Zyngier
2022-02-10 10:31 ` Marc Zyngier
2022-02-11 5:04 ` Reiji Watanabe
2022-02-11 5:04 ` Reiji Watanabe
2022-02-11 5:04 ` Reiji Watanabe
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=875ypo5jqi.wl-maz@kernel.org \
--to=maz@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=pbonzini@redhat.com \
--cc=pshier@google.com \
--cc=reijiw@google.com \
--cc=will@kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.