From: Auger Eric <eric.auger@redhat.com>
To: Zenghui Yu <yuzenghui@huawei.com>,
maz@kernel.org, james.morse@arm.com,
julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com
Cc: wanghaibin.wang@huawei.com, kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] KVM: arm/arm64: vgic: Don't rely on the wrong pending table
Date: Tue, 29 Oct 2019 13:17:00 +0100 [thread overview]
Message-ID: <5e4d1a2f-7107-efe3-9dde-626662e31ac5@redhat.com> (raw)
In-Reply-To: <20191029071919.177-4-yuzenghui@huawei.com>
Hi Zenghui, Marc,
On 10/29/19 8:19 AM, Zenghui Yu wrote:
> It's possible that two LPIs locate in the same "byte_offset" but target
> two different vcpus, where their pending status are indicated by two
> different pending tables. In such a scenario, using last_byte_offset
> optimization will lead KVM relying on the wrong pending table entry.
> Let us use last_ptr instead, which can be treated as a byte index into
> a pending table and also, can be vcpu specific.
>
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> ---
>
> If this patch has done the right thing, we can even add the:
>
> Fixes: 280771252c1b ("KVM: arm64: vgic-v3: KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES")
>
> But to be honest, I'm not clear about what has this patch actually fixed.
> Pending tables should contain all zeros before we flush vgic_irq's pending
> status into guest's RAM (thinking that guest should never write anything
> into it). So the pending table entry we've read from the guest memory
> seems always be zero. And we will always do the right thing even if we
> rely on the wrong pending table entry.
>
> I think I must have some misunderstanding here... Please fix me.
>
> virt/kvm/arm/vgic/vgic-v3.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
> index 5ef93e5041e1..7cd2e2f81513 100644
> --- a/virt/kvm/arm/vgic/vgic-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-v3.c
> @@ -363,8 +363,8 @@ int vgic_v3_lpi_sync_pending_status(struct kvm *kvm, struct vgic_irq *irq)
> int vgic_v3_save_pending_tables(struct kvm *kvm)
> {
> struct vgic_dist *dist = &kvm->arch.vgic;
> - int last_byte_offset = -1;
> struct vgic_irq *irq;
> + gpa_t last_ptr = -1;
> int ret;
> u8 val;
>
> @@ -384,11 +384,11 @@ int vgic_v3_save_pending_tables(struct kvm *kvm)
> bit_nr = irq->intid % BITS_PER_BYTE;
> ptr = pendbase + byte_offset;
>
> - if (byte_offset != last_byte_offset) {
> + if (ptr != last_ptr) {
> ret = kvm_read_guest_lock(kvm, ptr, &val, 1);
> if (ret)
> return ret;
> - last_byte_offset = byte_offset;
> + last_ptr = ptr;
> }
>
> stored = val & (1U << bit_nr);
>
Acked-by: Eric Auger <eric.auger@redhat.com>
Thanks for fixing this.
Eric
next prev parent reply other threads:[~2019-10-29 12:17 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-29 7:19 [PATCH 0/3] KVM: arm/arm64: vgic: Some cleanups and fixes Zenghui Yu
2019-10-29 7:19 ` [PATCH 1/3] KVM: arm/arm64: vgic: Remove the declaration of kvm_send_userspace_msi() Zenghui Yu
2019-10-29 12:29 ` Auger Eric
2019-10-29 7:19 ` [PATCH 2/3] KVM: arm/arm64: vgic: Fix some comments typo Zenghui Yu
2019-10-29 9:04 ` Marc Zyngier
2019-10-29 12:45 ` Zenghui Yu
2019-10-29 13:22 ` Marc Zyngier
2019-10-29 7:19 ` [PATCH 3/3] KVM: arm/arm64: vgic: Don't rely on the wrong pending table Zenghui Yu
2019-10-29 9:23 ` Marc Zyngier
2019-10-29 12:27 ` Zenghui Yu
2019-10-29 12:49 ` Auger Eric
2019-10-29 13:31 ` Zenghui Yu
2019-10-29 22:52 ` Auger Eric
2019-10-29 12:17 ` Auger Eric [this message]
2019-10-29 12:30 ` Zenghui Yu
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=5e4d1a2f-7107-efe3-9dde-626662e31ac5@redhat.com \
--to=eric.auger@redhat.com \
--cc=james.morse@arm.com \
--cc=julien.thierry.kdev@gmail.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maz@kernel.org \
--cc=suzuki.poulose@arm.com \
--cc=wanghaibin.wang@huawei.com \
--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 NNTP newsgroup(s).