From: Paolo Bonzini <pbonzini@redhat.com>
To: Anup Patel <anup@brainfault.org>
Cc: Anup Patel <Anup.Patel@wdc.com>,
Palmer Dabbelt <palmer@sifive.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Radim K <rkrcmar@redhat.com>,
Daniel Lezcano <daniel.lezcano@linaro.org>,
Thomas Gleixner <tglx@linutronix.de>,
Atish Patra <Atish.Patra@wdc.com>,
Alistair Francis <Alistair.Francis@wdc.com>,
Damien Le Moal <Damien.LeMoal@wdc.com>,
Christoph Hellwig <hch@infradead.org>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"linux-riscv@lists.infradead.org"
<linux-riscv@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH 05/16] RISC-V: KVM: Implement VCPU interrupts and requests handling
Date: Tue, 30 Jul 2019 14:12:15 +0200 [thread overview]
Message-ID: <66c4e468-7a69-31e7-778b-228908f0e737@redhat.com> (raw)
In-Reply-To: <CAAhSdy3JZVEEnPnssALaxvCsyznF=rt=7-d5J_OgQEJv6cPhxQ@mail.gmail.com>
On 30/07/19 14:00, Anup Patel wrote:
> On Tue, Jul 30, 2019 at 4:47 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> First, something that is not clear to me: how do you deal with a guest
>> writing 1 to VSIP.SSIP? I think that could lead to lost interrupts if
>> you have the following sequence
>>
>> 1) guest writes 1 to VSIP.SSIP
>>
>> 2) guest leaves VS-mode
>>
>> 3) host syncs VSIP
>>
>> 4) user mode triggers interrupt
>>
>> 5) host reenters guest
>>
>> 6) host moves irqs_pending to VSIP and clears VSIP.SSIP in the process
>
> This reasoning also apply to M-mode firmware (OpenSBI) providing timer
> and IPI services to HS-mode software. We had some discussion around
> it in a different context.
> (Refer, https://github.com/riscv/opensbi/issues/128)
>
> The thing is SIP CSR is supposed to be read-only for any S-mode SW. This
> means HS-mode/VS-mode SW modifications to SIP CSR should have no
> effect.
Is it? The privileged specification says
Interprocessor interrupts are sent to other harts by implementation-
specific means, which will ultimately cause the SSIP bit to be set in
the recipient hart’s sip register.
All bits besides SSIP in the sip register are read-only.
Meaning that sending an IPI to self by writing 1 to sip.SSIP is
well-defined. The same should be true of vsip.SSIP while in VS mode.
> Do you still an issue here?
Do you see any issues in the pseudocode I sent? It gets away with the
spinlock and request so it may be a good idea anyway. :)
Paolo
> Regards,
> Anup
>
>>
>> Perhaps irqs_pending needs to be split in two fields, irqs_pending and
>> irqs_pending_mask, and then you can do this:
>>
>> /*
>> * irqs_pending and irqs_pending_mask have multiple-producer/single-
>> * consumer semantics; therefore bits can be set in the mask without
>> * a lock, but clearing the bits requires vcpu_lock. Furthermore,
>> * consumers should never write to irqs_pending, and should not
>> * use bits of irqs_pending that weren't 1 in the mask.
>> */
>>
>> int kvm_riscv_vcpu_set_interrupt(struct kvm_vcpu *vcpu, unsigned int irq)
>> {
>> ...
>> set_bit(irq, &vcpu->arch.irqs_pending);
>> smp_mb__before_atomic();
>> set_bit(irq, &vcpu->arch.irqs_pending_mask);
>> kvm_vcpu_kick(vcpu);
>> }
>>
>> int kvm_riscv_vcpu_unset_interrupt(struct kvm_vcpu *vcpu, unsigned int irq)
>> {
>> ...
>> clear_bit(irq, &vcpu->arch.irqs_pending);
>> smp_mb__before_atomic();
>> set_bit(irq, &vcpu->arch.irqs_pending_mask);
>> }
>>
>> static void kvm_riscv_reset_vcpu(struct kvm_vcpu *vcpu)
>> {
>> ...
>> WRITE_ONCE(vcpu->arch.irqs_pending_mask, 0);
>> }
>>
>> and kvm_riscv_vcpu_flush_interrupts can leave aside VSIP bits that
>> aren't in vcpu->arch.irqs_pending_mask:
>>
>> if (atomic_read(&vcpu->arch.irqs_pending_mask)) {
>> u32 mask, val;
>>
>> mask = xchg_acquire(&vcpu->arch.irqs_pending_mask, 0);
>> val = READ_ONCE(vcpu->arch.irqs_pending) & mask;
>>
>> vcpu->arch.guest_csr.vsip &= ~mask;
>> vcpu->arch.guest_csr.vsip |= val;
>> csr_write(CSR_VSIP, vsip);
>> }
>>
>> Also, the getter of CSR_VSIP should call
>> kvm_riscv_vcpu_flush_interrupts, while the setter should clear
>> irqs_pending_mask.
>>
>> On 29/07/19 13:56, Anup Patel wrote:
>>> + kvm_make_request(KVM_REQ_IRQ_PENDING, vcpu);
>>> + kvm_vcpu_kick(vcpu);
>>
>> The request is not needed as long as kvm_riscv_vcpu_flush_interrupts is
>> called *after* smp_store_mb(vcpu->mode, IN_GUEST_MODE) in
>> kvm_arch_vcpu_ioctl_run. This is the "request-less vCPU kick" pattern
>> in Documentation/virtual/kvm/vcpu-requests.rst. The smp_store_mb then
>> orders the write of IN_GUEST_MODE before the read of irqs_pending (or
>> irqs_pending_mask in my proposal above); in the producers, there is a
>> dual memory barrier in kvm_vcpu_exiting_guest_mode(), ordering the write
>> of irqs_pending(_mask) before the read of vcpu->mode.
>>
>> Similar to other VS* CSRs, I'd rather have a ONE_REG interface for VSIE
>> and VSIP from the beginning as well. Note that the VSIP setter would
>> clear irqs_pending_mask, while the getter would call
>> kvm_riscv_vcpu_flush_interrupts before reading. It's up to userspace to
>> ensure that no interrupt injections happen between the calls to the
>> getter and the setter.
>>
>> Paolo
>>
>>> + csr_write(CSR_VSIP, vcpu->arch.irqs_pending);
>>> + vcpu->arch.guest_csr.vsip = vcpu->arch.irqs_pending;
>>> + }
>>
next prev parent reply other threads:[~2019-07-30 12:12 UTC|newest]
Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-29 11:56 [RFC PATCH 00/16] KVM RISC-V Support Anup Patel
2019-07-29 11:56 ` [RFC PATCH 01/16] KVM: RISC-V: Add KVM_REG_RISCV for ONE_REG interface Anup Patel
2019-07-29 11:56 ` [RFC PATCH 02/16] RISC-V: Add hypervisor extension related CSR defines Anup Patel
2019-07-29 11:56 ` [RFC PATCH 03/16] RISC-V: Add initial skeletal KVM support Anup Patel
2019-07-30 9:23 ` Paolo Bonzini
2019-07-30 11:04 ` Anup Patel
2019-07-30 9:25 ` Paolo Bonzini
2019-07-30 11:03 ` Anup Patel
2019-07-29 11:56 ` [RFC PATCH 04/16] RISC-V: KVM: Implement VCPU create, init and destroy functions Anup Patel
2019-07-30 8:48 ` Paolo Bonzini
2019-07-30 10:16 ` Paolo Bonzini
2019-07-30 11:45 ` Anup Patel
2019-07-30 11:47 ` Paolo Bonzini
2019-07-29 11:56 ` [RFC PATCH 05/16] RISC-V: KVM: Implement VCPU interrupts and requests handling Anup Patel
2019-07-30 11:17 ` Paolo Bonzini
2019-07-30 12:00 ` Anup Patel
2019-07-30 12:12 ` Paolo Bonzini [this message]
2019-07-30 12:45 ` Anup Patel
2019-07-30 13:18 ` Paolo Bonzini
2019-07-30 13:35 ` Anup Patel
2019-07-30 14:08 ` Paolo Bonzini
2019-08-02 3:59 ` Anup Patel
2019-07-29 11:56 ` [RFC PATCH 06/16] RISC-V: KVM: Implement KVM_GET_ONE_REG/KVM_SET_ONE_REG ioctls Anup Patel
2019-07-30 8:43 ` Paolo Bonzini
2019-07-30 9:35 ` Paolo Bonzini
2019-07-30 12:08 ` Anup Patel
2019-07-30 12:10 ` Paolo Bonzini
2019-07-30 12:16 ` Anup Patel
2019-07-29 11:57 ` [RFC PATCH 07/16] RISC-V: KVM: Implement VCPU world-switch Anup Patel
2019-07-30 9:34 ` Paolo Bonzini
2019-07-30 12:51 ` Anup Patel
2019-07-29 11:57 ` [RFC PATCH 08/16] RISC-V: KVM: Handle MMIO exits for VCPU Anup Patel
2019-07-30 11:20 ` Paolo Bonzini
2019-07-31 7:23 ` Anup Patel
2019-07-29 11:57 ` [RFC PATCH 09/16] RISC-V: KVM: Handle WFI " Anup Patel
2019-07-29 11:57 ` [RFC PATCH 10/16] RISC-V: KVM: Implement VMID allocator Anup Patel
2019-07-30 8:59 ` Paolo Bonzini
2019-07-29 11:57 ` [RFC PATCH 11/16] RISC-V: KVM: Implement stage2 page table programming Anup Patel
2019-07-30 9:00 ` Paolo Bonzini
2019-07-30 12:14 ` Anup Patel
2019-07-29 11:57 ` [RFC PATCH 12/16] RISC-V: KVM: Implement MMU notifiers Anup Patel
2019-07-29 11:57 ` [RFC PATCH 13/16] RISC-V: KVM: Add timer functionality Anup Patel
2019-07-29 14:40 ` Andreas Schwab
2019-07-29 18:02 ` Atish Patra
2019-07-30 6:51 ` Andreas Schwab
2019-07-30 7:00 ` Atish Patra
2019-07-30 11:26 ` Paolo Bonzini
2019-07-31 1:55 ` Atish Patra
2019-07-31 6:58 ` Paolo Bonzini
2019-07-31 7:18 ` Anup Patel
2019-07-29 11:57 ` [RFC PATCH 14/16] RISC-V: KVM: FP lazy save/restore Anup Patel
2019-07-29 11:57 ` [RFC PATCH 15/16] RISC-V: KVM: Add SBI v0.1 support Anup Patel
2019-07-29 19:40 ` Paolo Bonzini
2019-07-29 19:51 ` Atish Patra
2019-07-29 20:08 ` Paolo Bonzini
2019-07-29 21:08 ` Atish Patra
2019-07-30 9:26 ` Paolo Bonzini
2019-07-29 11:58 ` [RFC PATCH 16/16] RISC-V: Enable VIRTIO drivers in RV64 and RV32 defconfig Anup Patel
2019-07-29 21:47 ` [RFC PATCH 00/16] KVM RISC-V Support Paolo Bonzini
2019-07-30 5:26 ` Anup Patel
2019-07-30 11:33 ` Paolo Bonzini
2019-07-30 13:50 ` Anup Patel
2019-07-30 14:02 ` Paolo Bonzini
2019-07-30 6:53 ` Andreas Schwab
2019-07-30 7:25 ` Anup Patel
2019-07-30 7:42 ` Andreas Schwab
2019-07-30 7:36 ` Anup Patel
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=66c4e468-7a69-31e7-778b-228908f0e737@redhat.com \
--to=pbonzini@redhat.com \
--cc=Alistair.Francis@wdc.com \
--cc=Anup.Patel@wdc.com \
--cc=Atish.Patra@wdc.com \
--cc=Damien.LeMoal@wdc.com \
--cc=anup@brainfault.org \
--cc=daniel.lezcano@linaro.org \
--cc=hch@infradead.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@sifive.com \
--cc=paul.walmsley@sifive.com \
--cc=rkrcmar@redhat.com \
--cc=tglx@linutronix.de \
/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).