All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: James Morse <james.morse@arm.com>
Cc: Julien Grall <julien@xen.org>,
	kvm@vger.kernel.org, Andre Przywara <Andre.Przywara@arm.com>,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 4/6] KVM: arm: vgic-v2: Only use the virtual state when userspace accesses pending bits
Date: Wed, 22 Apr 2020 17:02:30 +0100	[thread overview]
Message-ID: <299b5f1307cff29944e5f89e307b2015@kernel.org> (raw)
In-Reply-To: <5aa2acf8-e775-325c-0340-fa000a4e3513@arm.com>

Hi James,

On 2020-04-22 16:55, James Morse wrote:
> Hi Marc,
> 
> On 20/04/2020 11:03, Marc Zyngier wrote:
>> On Fri, 17 Apr 2020 17:48:34 +0100
>> James Morse <james.morse@arm.com> wrote:

[...]

>>> (but if user-space never actually does this, then we should do the 
>>> simplest thing)
> 
> Adding printk() to this combined patch and using 'loadvm' on the qemu
> console, I see Qemu
> writing '0xffffffff' into cpending to clear all 16 SGIs. I guess it is
> 'resetting' the
> in-kernel state to replace it with the state read from disk.
> 
> 
>> A third way would be to align on what GICv3 does, which is that 
>> ISPENDR
>> is used for both setting and clearing in one go. Given that the 
>> current
>> state it broken (and has been for some time now), I'm tempted to adopt
>> the same behaviour...
> 
>> What do you think?
> 
> I think Qemu is expecting the bank of cpending writes to clear
> whatever the kernel has
> stored, so that it can replay the new state. Ignoring the cpending
> writes means the kernel
> keeps an interrupt pending if nothing else in that 64bit group was
> set. Its not what Qemu
> expects, it looks like we'd get away with it, but I don't think we 
> should do it!
> 
> I think we should let user-space write to those WI registers, and
> clearing the SGIs should clear all sources of SGI...

I'd be happy with that. Let me rework the patch, and I'll post the 
series again
shortly.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...
_______________________________________________
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: James Morse <james.morse@arm.com>
Cc: Julien Grall <julien@xen.org>,
	kvm@vger.kernel.org, Suzuki K Poulose <suzuki.poulose@arm.com>,
	Andre Przywara <Andre.Przywara@arm.com>,
	Eric Auger <eric.auger@redhat.com>,
	Julien Thierry <julien.thierry.kdev@gmail.com>,
	Zenghui Yu <yuzenghui@huawei.com>,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 4/6] KVM: arm: vgic-v2: Only use the virtual state when userspace accesses pending bits
Date: Wed, 22 Apr 2020 17:02:30 +0100	[thread overview]
Message-ID: <299b5f1307cff29944e5f89e307b2015@kernel.org> (raw)
In-Reply-To: <5aa2acf8-e775-325c-0340-fa000a4e3513@arm.com>

Hi James,

On 2020-04-22 16:55, James Morse wrote:
> Hi Marc,
> 
> On 20/04/2020 11:03, Marc Zyngier wrote:
>> On Fri, 17 Apr 2020 17:48:34 +0100
>> James Morse <james.morse@arm.com> wrote:

[...]

>>> (but if user-space never actually does this, then we should do the 
>>> simplest thing)
> 
> Adding printk() to this combined patch and using 'loadvm' on the qemu
> console, I see Qemu
> writing '0xffffffff' into cpending to clear all 16 SGIs. I guess it is
> 'resetting' the
> in-kernel state to replace it with the state read from disk.
> 
> 
>> A third way would be to align on what GICv3 does, which is that 
>> ISPENDR
>> is used for both setting and clearing in one go. Given that the 
>> current
>> state it broken (and has been for some time now), I'm tempted to adopt
>> the same behaviour...
> 
>> What do you think?
> 
> I think Qemu is expecting the bank of cpending writes to clear
> whatever the kernel has
> stored, so that it can replay the new state. Ignoring the cpending
> writes means the kernel
> keeps an interrupt pending if nothing else in that 64bit group was
> set. Its not what Qemu
> expects, it looks like we'd get away with it, but I don't think we 
> should do it!
> 
> I think we should let user-space write to those WI registers, and
> clearing the SGIs should clear all sources of SGI...

I'd be happy with that. Let me rework the patch, and I'll post the 
series again
shortly.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
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: James Morse <james.morse@arm.com>
Cc: linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org,
	Zenghui Yu <yuzenghui@huawei.com>,
	Eric Auger <eric.auger@redhat.com>,
	Andre Przywara <Andre.Przywara@arm.com>,
	Julien Grall <julien@xen.org>,
	Julien Thierry <julien.thierry.kdev@gmail.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>
Subject: Re: [PATCH v2 4/6] KVM: arm: vgic-v2: Only use the virtual state when userspace accesses pending bits
Date: Wed, 22 Apr 2020 17:02:30 +0100	[thread overview]
Message-ID: <299b5f1307cff29944e5f89e307b2015@kernel.org> (raw)
In-Reply-To: <5aa2acf8-e775-325c-0340-fa000a4e3513@arm.com>

Hi James,

On 2020-04-22 16:55, James Morse wrote:
> Hi Marc,
> 
> On 20/04/2020 11:03, Marc Zyngier wrote:
>> On Fri, 17 Apr 2020 17:48:34 +0100
>> James Morse <james.morse@arm.com> wrote:

[...]

>>> (but if user-space never actually does this, then we should do the 
>>> simplest thing)
> 
> Adding printk() to this combined patch and using 'loadvm' on the qemu
> console, I see Qemu
> writing '0xffffffff' into cpending to clear all 16 SGIs. I guess it is
> 'resetting' the
> in-kernel state to replace it with the state read from disk.
> 
> 
>> A third way would be to align on what GICv3 does, which is that 
>> ISPENDR
>> is used for both setting and clearing in one go. Given that the 
>> current
>> state it broken (and has been for some time now), I'm tempted to adopt
>> the same behaviour...
> 
>> What do you think?
> 
> I think Qemu is expecting the bank of cpending writes to clear
> whatever the kernel has
> stored, so that it can replay the new state. Ignoring the cpending
> writes means the kernel
> keeps an interrupt pending if nothing else in that 64bit group was
> set. Its not what Qemu
> expects, it looks like we'd get away with it, but I don't think we 
> should do it!
> 
> I think we should let user-space write to those WI registers, and
> clearing the SGIs should clear all sources of SGI...

I'd be happy with that. Let me rework the patch, and I'll post the 
series again
shortly.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

  reply	other threads:[~2020-04-22 16:02 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-17  8:33 [PATCH v2 0/6] KVM: arm: vgic fixes for 5.7 Marc Zyngier
2020-04-17  8:33 ` Marc Zyngier
2020-04-17  8:33 ` Marc Zyngier
2020-04-17  8:33 ` [PATCH v2 1/6] KVM: arm: vgic: Fix limit condition when writing to GICD_I[CS]ACTIVER Marc Zyngier
2020-04-17  8:33   ` Marc Zyngier
2020-04-17  8:33   ` Marc Zyngier
2020-04-17  8:33 ` [PATCH v2 2/6] KVM: arm: vgic: Synchronize the whole guest on GIC{D, R}_I{S, C}ACTIVER read Marc Zyngier
2020-04-17  8:33   ` [PATCH v2 2/6] KVM: arm: vgic: Synchronize the whole guest on GIC{D,R}_I{S,C}ACTIVER read Marc Zyngier
2020-04-17  8:33   ` [PATCH v2 2/6] KVM: arm: vgic: Synchronize the whole guest on GIC{D, R}_I{S, C}ACTIVER read Marc Zyngier
2020-04-17 16:26   ` [PATCH v2 2/6] KVM: arm: vgic: Synchronize the whole guest on GIC{D,R}_I{S,C}ACTIVER read André Przywara
2020-04-17 16:26     ` André Przywara
2020-04-17 16:26     ` André Przywara
2020-04-17  8:33 ` [PATCH v2 3/6] KVM: arm: vgic: Only use the virtual state when userspace accesses enable bits Marc Zyngier
2020-04-17  8:33   ` Marc Zyngier
2020-04-17  8:33   ` Marc Zyngier
2020-04-17 11:17   ` James Morse
2020-04-17 11:17     ` James Morse
2020-04-17 11:17     ` James Morse
2020-04-17  8:33 ` [PATCH v2 4/6] KVM: arm: vgic-v2: Only use the virtual state when userspace accesses pending bits Marc Zyngier
2020-04-17  8:33   ` Marc Zyngier
2020-04-17  8:33   ` Marc Zyngier
2020-04-17 11:22   ` James Morse
2020-04-17 11:22     ` James Morse
2020-04-17 11:22     ` James Morse
2020-04-17 12:41     ` Marc Zyngier
2020-04-17 12:41       ` Marc Zyngier
2020-04-17 12:41       ` Marc Zyngier
2020-04-17 16:48       ` James Morse
2020-04-17 16:48         ` James Morse
2020-04-17 16:48         ` James Morse
2020-04-20 10:03         ` Marc Zyngier
2020-04-20 10:03           ` Marc Zyngier
2020-04-20 10:03           ` Marc Zyngier
2020-04-22 15:55           ` James Morse
2020-04-22 15:55             ` James Morse
2020-04-22 15:55             ` James Morse
2020-04-22 16:02             ` Marc Zyngier [this message]
2020-04-22 16:02               ` Marc Zyngier
2020-04-22 16:02               ` Marc Zyngier
2020-04-17  8:33 ` [PATCH v2 5/6] KVM: arm64: vgic-v3: Retire all pending LPIs on vcpu destroy Marc Zyngier
2020-04-17  8:33   ` Marc Zyngier
2020-04-17  8:33   ` Marc Zyngier
2020-04-17  8:33 ` [PATCH v2 6/6] KVM: arm64: vgic-its: Fix memory leak on the error path of vgic_add_lpi() Marc Zyngier
2020-04-17  8:33   ` Marc Zyngier
2020-04-17  8:33   ` Marc Zyngier

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=299b5f1307cff29944e5f89e307b2015@kernel.org \
    --to=maz@kernel.org \
    --cc=Andre.Przywara@arm.com \
    --cc=james.morse@arm.com \
    --cc=julien@xen.org \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.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.