* Re: KVM: Warn if mark_page_dirty() is called without an active vCPU
[not found] <e8f40b8765f2feefb653d8a67e487818f66581aa.camel@infradead.org>
@ 2022-01-13 12:06 ` Christian Borntraeger
2022-01-13 12:14 ` Paolo Bonzini
0 siblings, 1 reply; 13+ messages in thread
From: Christian Borntraeger @ 2022-01-13 12:06 UTC (permalink / raw)
To: dwmw2
Cc: butterflyhuangxx, kvm, linux-kernel, pbonzini, seanjc,
Cornelia Huck, Christian Borntraeger, Janosch Frank,
David Hildenbrand, linux-s390, Thomas Huth, Claudio Imbrenda
From: Christian Borntraeger <borntraeger@de.ibm.com>
Quick heads-up.
The new warnon triggers on s390. Here we write to the guest from an
irqfd worker. Since we do not use dirty_ring yet this might be an over-indication.
Still have to look into that.
[ 1801.980777] WARNING: CPU: 12 PID: 117600 at arch/s390/kvm/../../../virt/kvm/kvm_main.c:3166 mark_page_dirty_in_slot+0xa0/0xb0 [kvm]
[ 1801.980839] Modules linked in: xt_CHECKSUM(E) xt_MASQUERADE(E) xt_conntrack(E) ipt_REJECT(E) xt_tcpudp(E) nft_compat(E) nf_nat_tftp(E) nft_objref(E) vhost_vsock(E) vmw_vsock_virtio_transport_common(E) vsock(E) vhost(E) vhost_iotlb(E) nf_conntrack_tftp(E) crc32_generic(E) algif_hash(E) af_alg(E) paes_s390(E) dm_crypt(E) encrypted_keys(E) loop(E) lcs(E) ctcm(E) fsm(E) kvm(E) nft_fib_inet(E) nft_fib_ipv4(E) nft_fib_ipv6(E) nft_fib(E) nft_reject_inet(E) nf_reject_ipv4(E) nf_reject_ipv6(E) nft_reject(E) nft_ct(E) nft_chain_nat(E) nf_nat(E) nf_conntrack(E) nf_defrag_ipv6(E) nf_defrag_ipv4(E) ip_set(E) nf_tables(E) nfnetlink(E) sunrpc(E) dm_service_time(E) dm_multipath(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) zfcp(E) scsi_transport_fc(E) ism(E) smc(E) ib_core(E) eadm_sch(E) vfio_ccw(E) mdev(E) vfio_iommu_type1(E) vfio(E) sch_fq_codel(E) configfs(E) ip_tables(E) x_tables(E) ghash_s39 [...truncated...]
[ 1801.980915] sha1_s390(E) sha_common(E) pkey(E) zcrypt(E) rng_core(E) autofs4(E) [last unloaded: vfio_ap]
[ 1801.980931] CPU: 12 PID: 117600 Comm: kworker/12:0 Tainted: G E 5.17.0-20220113.rc0.git0.32ce2abb03cf.300.fc35.s390x+next #1
[ 1801.980935] Hardware name: IBM 2964 NC9 712 (LPAR)
[ 1801.980938] Workqueue: events irqfd_inject [kvm]
[ 1801.980959] Krnl PSW : 0704e00180000000 000003ff805f0f5c (mark_page_dirty_in_slot+0xa4/0xb0 [kvm])
[ 1801.980981] R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
[ 1801.980985] Krnl GPRS: 000003ff298e9040 000000017754a660 0000000000000000 0000000000000000
[ 1801.980988] 000000003fefcc36 ffffffffffffff68 0000000000000000 0000000177871500
[ 1801.980990] 00000001d1918000 000000003fefcc36 00000001d1918000 0000000000000000
[ 1801.980993] 00000001375b0000 00000001d191a838 000003ff805f0ee6 0000038000babb48
[ 1801.981003] Krnl Code: 000003ff805f0f4c: eb9ff0a00004 lmg %r9,%r15,160(%r15)
000003ff805f0f52: c0f400018c61 brcl 15,000003ff80622814
#000003ff805f0f58: af000000 mc 0,0
>000003ff805f0f5c: eb9ff0a00004 lmg %r9,%r15,160(%r15)
000003ff805f0f62: c0f400018c59 brcl 15,000003ff80622814
000003ff805f0f68: c004ffe37b10 brcl 0,000003ff80260588
000003ff805f0f6e: ec360033007c cgij %r3,0,6,000003ff805f0fd4
000003ff805f0f74: e31020100012 lt %r1,16(%r2)
[ 1801.981057] Call Trace:
[ 1801.981060] [<000003ff805f0f5c>] mark_page_dirty_in_slot+0xa4/0xb0 [kvm]
[ 1801.981083] [<000003ff8060e9fe>] adapter_indicators_set+0xde/0x268 [kvm]
[ 1801.981104] [<000003ff80613c24>] set_adapter_int+0x64/0xd8 [kvm]
[ 1801.981124] [<000003ff805fb9aa>] kvm_set_irq+0xc2/0x130 [kvm]
[ 1801.981144] [<000003ff805f8d86>] irqfd_inject+0x76/0xa0 [kvm]
[ 1801.981164] [<0000000175e56906>] process_one_work+0x1fe/0x470
[ 1801.981173] [<0000000175e570a4>] worker_thread+0x64/0x498
[ 1801.981176] [<0000000175e5ef2c>] kthread+0x10c/0x110
[ 1801.981180] [<0000000175de73c8>] __ret_from_fork+0x40/0x58
[ 1801.981185] [<000000017698440a>] ret_from_fork+0xa/0x40
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: KVM: Warn if mark_page_dirty() is called without an active vCPU
2022-01-13 12:06 ` KVM: Warn if mark_page_dirty() is called without an active vCPU Christian Borntraeger
@ 2022-01-13 12:14 ` Paolo Bonzini
2022-01-13 12:29 ` [PATCH] KVM: avoid warning on s390 in mark_page_dirty Christian Borntraeger
2022-01-13 12:30 ` KVM: Warn if mark_page_dirty() is called without an active vCPU David Woodhouse
0 siblings, 2 replies; 13+ messages in thread
From: Paolo Bonzini @ 2022-01-13 12:14 UTC (permalink / raw)
To: Christian Borntraeger, dwmw2
Cc: butterflyhuangxx, kvm, linux-kernel, seanjc, Cornelia Huck,
Janosch Frank, David Hildenbrand, linux-s390, Thomas Huth,
Claudio Imbrenda
On 1/13/22 13:06, Christian Borntraeger wrote:
> From: Christian Borntraeger<borntraeger@de.ibm.com>
>
> Quick heads-up.
> The new warnon triggers on s390. Here we write to the guest from an
> irqfd worker. Since we do not use dirty_ring yet this might be an over-indication.
> Still have to look into that.
Yes, it's okay to add an #ifdef around the warning.
Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] KVM: avoid warning on s390 in mark_page_dirty
2022-01-13 12:14 ` Paolo Bonzini
@ 2022-01-13 12:29 ` Christian Borntraeger
2022-01-13 12:31 ` David Woodhouse
2022-01-18 8:37 ` Christian Borntraeger
2022-01-13 12:30 ` KVM: Warn if mark_page_dirty() is called without an active vCPU David Woodhouse
1 sibling, 2 replies; 13+ messages in thread
From: Christian Borntraeger @ 2022-01-13 12:29 UTC (permalink / raw)
To: dwmw2, pbonzini
Cc: butterflyhuangxx, kvm, linux-kernel, seanjc, Cornelia Huck,
Christian Borntraeger, Janosch Frank, David Hildenbrand,
linux-s390, Thomas Huth, Claudio Imbrenda
Avoid warnings on s390 like
[ 1801.980931] CPU: 12 PID: 117600 Comm: kworker/12:0 Tainted: G E 5.17.0-20220113.rc0.git0.32ce2abb03cf.300.fc35.s390x+next #1
[ 1801.980938] Workqueue: events irqfd_inject [kvm]
[...]
[ 1801.981057] Call Trace:
[ 1801.981060] [<000003ff805f0f5c>] mark_page_dirty_in_slot+0xa4/0xb0 [kvm]
[ 1801.981083] [<000003ff8060e9fe>] adapter_indicators_set+0xde/0x268 [kvm]
[ 1801.981104] [<000003ff80613c24>] set_adapter_int+0x64/0xd8 [kvm]
[ 1801.981124] [<000003ff805fb9aa>] kvm_set_irq+0xc2/0x130 [kvm]
[ 1801.981144] [<000003ff805f8d86>] irqfd_inject+0x76/0xa0 [kvm]
[ 1801.981164] [<0000000175e56906>] process_one_work+0x1fe/0x470
[ 1801.981173] [<0000000175e570a4>] worker_thread+0x64/0x498
[ 1801.981176] [<0000000175e5ef2c>] kthread+0x10c/0x110
[ 1801.981180] [<0000000175de73c8>] __ret_from_fork+0x40/0x58
[ 1801.981185] [<000000017698440a>] ret_from_fork+0xa/0x40
when writing to a guest from an irqfd worker as long as we do not have
the dirty ring.
Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
---
virt/kvm/kvm_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 504158f0e131..1a682d3e106d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3163,8 +3163,10 @@ void mark_page_dirty_in_slot(struct kvm *kvm,
{
struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
+#ifdef CONFIG_HAVE_KVM_DIRTY_RING
if (WARN_ON_ONCE(!vcpu) || WARN_ON_ONCE(vcpu->kvm != kvm))
return;
+#endif
if (memslot && kvm_slot_dirty_track_enabled(memslot)) {
unsigned long rel_gfn = gfn - memslot->base_gfn;
--
2.33.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: KVM: Warn if mark_page_dirty() is called without an active vCPU
2022-01-13 12:14 ` Paolo Bonzini
2022-01-13 12:29 ` [PATCH] KVM: avoid warning on s390 in mark_page_dirty Christian Borntraeger
@ 2022-01-13 12:30 ` David Woodhouse
2022-01-13 12:51 ` Christian Borntraeger
2022-01-13 14:36 ` Paolo Bonzini
1 sibling, 2 replies; 13+ messages in thread
From: David Woodhouse @ 2022-01-13 12:30 UTC (permalink / raw)
To: Paolo Bonzini, Christian Borntraeger
Cc: butterflyhuangxx, kvm, linux-kernel, seanjc, Cornelia Huck,
Janosch Frank, David Hildenbrand, linux-s390, Thomas Huth,
Claudio Imbrenda
[-- Attachment #1: Type: text/plain, Size: 1544 bytes --]
On Thu, 2022-01-13 at 13:14 +0100, Paolo Bonzini wrote:
> On 1/13/22 13:06, Christian Borntraeger wrote:
> > From: Christian Borntraeger<
> > borntraeger@de.ibm.com
> > >
> >
> > Quick heads-up.
> > The new warnon triggers on s390. Here we write to the guest from an
> > irqfd worker. Since we do not use dirty_ring yet this might be an
> > over-indication.
> > Still have to look into that.
>
> Yes, it's okay to add an #ifdef around the warning.
That would be #ifndef CONFIG_HAVE_KVM_DIRTY_RING, yes?
I already found it hard to write down the rules around how
kvm_vcpu_write_guest() doesn't use the vCPU it's passed, and how both
it and kvm_write_guest() need to be invoked on a pCPU which currently
owns *a* vCPU belonging to the same KVM... if we add "unless you're on
an architecture that doesn't support dirty ring logging", you may have
to pass me a bucket.
Are you proposing that as an officially documented part of the already
horrid API, or a temporary measure :)
Btw, that get_map_page() in arch/s390/kvm/interrupt.c looks like it has
the same use-after-free problem that kvm_map_gfn() used to have. It
probably wants converting to the new gfn_to_pfn_cache.
Take a look at how I resolve the same issue for delivering Xen event
channel interrupts.
Although I gave myself a free pass on the dirty marking in that case,
by declaring that the shinfo page doesn't get marked dirty; it should
be considered *always* dirty. You might have less fun declaring that
retrospectively in your case.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] KVM: avoid warning on s390 in mark_page_dirty
2022-01-13 12:29 ` [PATCH] KVM: avoid warning on s390 in mark_page_dirty Christian Borntraeger
@ 2022-01-13 12:31 ` David Woodhouse
2022-01-18 8:37 ` Christian Borntraeger
1 sibling, 0 replies; 13+ messages in thread
From: David Woodhouse @ 2022-01-13 12:31 UTC (permalink / raw)
To: Christian Borntraeger, pbonzini
Cc: butterflyhuangxx, kvm, linux-kernel, seanjc, Cornelia Huck,
Janosch Frank, David Hildenbrand, linux-s390, Thomas Huth,
Claudio Imbrenda
[-- Attachment #1: Type: text/plain, Size: 1301 bytes --]
On Thu, 2022-01-13 at 13:29 +0100, Christian Borntraeger wrote:
> Avoid warnings on s390 like
> [ 1801.980931] CPU: 12 PID: 117600 Comm: kworker/12:0 Tainted: G E 5.17.0-20220113.rc0.git0.32ce2abb03cf.300.fc35.s390x+next #1
> [ 1801.980938] Workqueue: events irqfd_inject [kvm]
> [...]
> [ 1801.981057] Call Trace:
> [ 1801.981060] [<000003ff805f0f5c>] mark_page_dirty_in_slot+0xa4/0xb0 [kvm]
> [ 1801.981083] [<000003ff8060e9fe>] adapter_indicators_set+0xde/0x268 [kvm]
> [ 1801.981104] [<000003ff80613c24>] set_adapter_int+0x64/0xd8 [kvm]
> [ 1801.981124] [<000003ff805fb9aa>] kvm_set_irq+0xc2/0x130 [kvm]
> [ 1801.981144] [<000003ff805f8d86>] irqfd_inject+0x76/0xa0 [kvm]
> [ 1801.981164] [<0000000175e56906>] process_one_work+0x1fe/0x470
> [ 1801.981173] [<0000000175e570a4>] worker_thread+0x64/0x498
> [ 1801.981176] [<0000000175e5ef2c>] kthread+0x10c/0x110
> [ 1801.981180] [<0000000175de73c8>] __ret_from_fork+0x40/0x58
> [ 1801.981185] [<000000017698440a>] ret_from_fork+0xa/0x40
>
> when writing to a guest from an irqfd worker as long as we do not have
> the dirty ring.
>
> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Reluctantly-acked-by: David Woodhouse <dwmw@amazon.co.uk>
... with a bucket nearby just in case.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: KVM: Warn if mark_page_dirty() is called without an active vCPU
2022-01-13 12:30 ` KVM: Warn if mark_page_dirty() is called without an active vCPU David Woodhouse
@ 2022-01-13 12:51 ` Christian Borntraeger
2022-01-13 13:22 ` David Woodhouse
2022-01-13 14:36 ` Paolo Bonzini
1 sibling, 1 reply; 13+ messages in thread
From: Christian Borntraeger @ 2022-01-13 12:51 UTC (permalink / raw)
To: David Woodhouse, Paolo Bonzini
Cc: butterflyhuangxx, kvm, linux-kernel, seanjc, Cornelia Huck,
Janosch Frank, David Hildenbrand, linux-s390, Thomas Huth,
Claudio Imbrenda
Am 13.01.22 um 13:30 schrieb David Woodhouse:
> On Thu, 2022-01-13 at 13:14 +0100, Paolo Bonzini wrote:
>> On 1/13/22 13:06, Christian Borntraeger wrote:
>>> From: Christian Borntraeger<
>>> borntraeger@de.ibm.com
>>>>
>>>
>>> Quick heads-up.
>>> The new warnon triggers on s390. Here we write to the guest from an
>>> irqfd worker. Since we do not use dirty_ring yet this might be an
>>> over-indication.
>>> Still have to look into that.
>>
>> Yes, it's okay to add an #ifdef around the warning.
>
> That would be #ifndef CONFIG_HAVE_KVM_DIRTY_RING, yes?
>
> I already found it hard to write down the rules around how
> kvm_vcpu_write_guest() doesn't use the vCPU it's passed, and how both
> it and kvm_write_guest() need to be invoked on a pCPU which currently
> owns *a* vCPU belonging to the same KVM... if we add "unless you're on
> an architecture that doesn't support dirty ring logging", you may have
> to pass me a bucket.
>
> Are you proposing that as an officially documented part of the already
> horrid API, or a temporary measure :)
>
> Btw, that get_map_page() in arch/s390/kvm/interrupt.c looks like it has
> the same use-after-free problem that kvm_map_gfn() used to have. It
> probably wants converting to the new gfn_to_pfn_cache.
>
> Take a look at how I resolve the same issue for delivering Xen event
> channel interrupts.
Do you have a commit ID for your Xen event channel fix?
>
> Although I gave myself a free pass on the dirty marking in that case,
> by declaring that the shinfo page doesn't get marked dirty; it should
> be considered *always* dirty. You might have less fun declaring that
> retrospectively in your case.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: KVM: Warn if mark_page_dirty() is called without an active vCPU
2022-01-13 12:51 ` Christian Borntraeger
@ 2022-01-13 13:22 ` David Woodhouse
2022-01-13 15:09 ` Christian Borntraeger
0 siblings, 1 reply; 13+ messages in thread
From: David Woodhouse @ 2022-01-13 13:22 UTC (permalink / raw)
To: Christian Borntraeger, Paolo Bonzini
Cc: butterflyhuangxx, kvm, linux-kernel, seanjc, Cornelia Huck,
Janosch Frank, David Hildenbrand, linux-s390, Thomas Huth,
Claudio Imbrenda
[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]
On Thu, 2022-01-13 at 13:51 +0100, Christian Borntraeger wrote:
> > Btw, that get_map_page() in arch/s390/kvm/interrupt.c looks like it has
> > the same use-after-free problem that kvm_map_gfn() used to have. It
> > probably wants converting to the new gfn_to_pfn_cache.
> >
> > Take a look at how I resolve the same issue for delivering Xen event
> > channel interrupts.
>
> Do you have a commit ID for your Xen event channel fix?
14243b387137 ("KVM: x86/xen: Add KVM_IRQ_ROUTING_XEN_EVTCHN and event
channel delivery") and the commits reworking the gfn_to_pfn_cache which
lead up to it.
Questions: In your kvm_set_routing_entry() where it calls
gmap_translate() to turn the summary_addr and ind_addr from guest
addresses to userspace virtual addresses, what protects those
translations? If the gmap changes before kvm_set_routing_entry() even
returns, what ensures that the IRQ gets retranslated?
And later in adapter_indicators_set() where you take that userspace
virtual address and pass it to your get_map_page() function, the same
question: what if userspace does a concurrent mmap() and changes the
physical page that the userspace address points to?
In the latter case, at least it does look like you don't support
external memory accessed only by a PFN without having a corresponding
struct page. So at least you don't end up accessing a page that can now
belong to *another* guest, because the original underlying page is
locked. You're probably OK in that case, so it's just the gmap changing
that we need to focus on?
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: KVM: Warn if mark_page_dirty() is called without an active vCPU
2022-01-13 12:30 ` KVM: Warn if mark_page_dirty() is called without an active vCPU David Woodhouse
2022-01-13 12:51 ` Christian Borntraeger
@ 2022-01-13 14:36 ` Paolo Bonzini
1 sibling, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2022-01-13 14:36 UTC (permalink / raw)
To: David Woodhouse, Christian Borntraeger
Cc: butterflyhuangxx, kvm, linux-kernel, seanjc, Cornelia Huck,
Janosch Frank, David Hildenbrand, linux-s390, Thomas Huth,
Claudio Imbrenda
On 1/13/22 13:30, David Woodhouse wrote:
> Are you proposing that as an officially documented part of the already
> horrid API, or a temporary measure:)
Hopefully temporary, but honestly you never know how these things go.
Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: KVM: Warn if mark_page_dirty() is called without an active vCPU
2022-01-13 13:22 ` David Woodhouse
@ 2022-01-13 15:09 ` Christian Borntraeger
0 siblings, 0 replies; 13+ messages in thread
From: Christian Borntraeger @ 2022-01-13 15:09 UTC (permalink / raw)
To: David Woodhouse, Paolo Bonzini
Cc: butterflyhuangxx, kvm, linux-kernel, seanjc, Cornelia Huck,
Janosch Frank, David Hildenbrand, linux-s390, Thomas Huth,
Claudio Imbrenda
Am 13.01.22 um 14:22 schrieb David Woodhouse:
> On Thu, 2022-01-13 at 13:51 +0100, Christian Borntraeger wrote:
>>> Btw, that get_map_page() in arch/s390/kvm/interrupt.c looks like it has
>>> the same use-after-free problem that kvm_map_gfn() used to have. It
>>> probably wants converting to the new gfn_to_pfn_cache.
>>>
>>> Take a look at how I resolve the same issue for delivering Xen event
>>> channel interrupts.
>>
>> Do you have a commit ID for your Xen event channel fix?
>
> 14243b387137 ("KVM: x86/xen: Add KVM_IRQ_ROUTING_XEN_EVTCHN and event
> channel delivery") and the commits reworking the gfn_to_pfn_cache which
> lead up to it.
>
> Questions: In your kvm_set_routing_entry() where it calls
> gmap_translate() to turn the summary_addr and ind_addr from guest
> addresses to userspace virtual addresses, what protects those
> translations? If the gmap changes before kvm_set_routing_entry() even
> returns, what ensures that the IRQ gets retranslated?
In the end the gmap translated between guest physical and host virtual, just
like the kvm memslots. This is done in kvm_arch_commit_memory_region. The gmap
is a method where we share the last level page table between the guest mapping
and the user mapping. That is why our memslots have to be on 1 MB boundary (our
page tables have 256 4k entries). So instead of gmap we could have used the
memslots as well for translation.
I have trouble seeing a kernel integrity issue: worst case is that we point to
a different address in the userspace mapping if qemu would change the memslots
maybe even to an invalid one. But then the access should fail (for invalid) or
you get a self-inflicted bit flips on your own address space if you play such
games.
Well, one thing: if QEMU changes memslots, it might need to redo the irqfd
registration as well as we do not follow these changes. Maybe this is something
that we could improve as future QEMUs could do more changes regarding memslots.
>
> And later in adapter_indicators_set() where you take that userspace
> virtual address and pass it to your get_map_page() function, the same
> question: what if userspace does a concurrent mmap() and changes the
> physical page that the userspace address points to?
>
> In the latter case, at least it does look like you don't support
> external memory accessed only by a PFN without having a corresponding
> struct page. So at least you don't end up accessing a page that can now
> belong to *another* guest, because the original underlying page is
> locked. You're probably OK in that case, so it's just the gmap changing
> that we need to focus on?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] KVM: avoid warning on s390 in mark_page_dirty
2022-01-13 12:29 ` [PATCH] KVM: avoid warning on s390 in mark_page_dirty Christian Borntraeger
2022-01-13 12:31 ` David Woodhouse
@ 2022-01-18 8:37 ` Christian Borntraeger
2022-01-18 8:44 ` Paolo Bonzini
1 sibling, 1 reply; 13+ messages in thread
From: Christian Borntraeger @ 2022-01-18 8:37 UTC (permalink / raw)
To: dwmw2, pbonzini
Cc: butterflyhuangxx, kvm, linux-kernel, seanjc, Cornelia Huck,
Janosch Frank, David Hildenbrand, linux-s390, Thomas Huth,
Claudio Imbrenda
Am 13.01.22 um 13:29 schrieb Christian Borntraeger:
> Avoid warnings on s390 like
> [ 1801.980931] CPU: 12 PID: 117600 Comm: kworker/12:0 Tainted: G E 5.17.0-20220113.rc0.git0.32ce2abb03cf.300.fc35.s390x+next #1
> [ 1801.980938] Workqueue: events irqfd_inject [kvm]
> [...]
> [ 1801.981057] Call Trace:
> [ 1801.981060] [<000003ff805f0f5c>] mark_page_dirty_in_slot+0xa4/0xb0 [kvm]
> [ 1801.981083] [<000003ff8060e9fe>] adapter_indicators_set+0xde/0x268 [kvm]
> [ 1801.981104] [<000003ff80613c24>] set_adapter_int+0x64/0xd8 [kvm]
> [ 1801.981124] [<000003ff805fb9aa>] kvm_set_irq+0xc2/0x130 [kvm]
> [ 1801.981144] [<000003ff805f8d86>] irqfd_inject+0x76/0xa0 [kvm]
> [ 1801.981164] [<0000000175e56906>] process_one_work+0x1fe/0x470
> [ 1801.981173] [<0000000175e570a4>] worker_thread+0x64/0x498
> [ 1801.981176] [<0000000175e5ef2c>] kthread+0x10c/0x110
> [ 1801.981180] [<0000000175de73c8>] __ret_from_fork+0x40/0x58
> [ 1801.981185] [<000000017698440a>] ret_from_fork+0xa/0x40
>
> when writing to a guest from an irqfd worker as long as we do not have
> the dirty ring.
>
> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> ---
> virt/kvm/kvm_main.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 504158f0e131..1a682d3e106d 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3163,8 +3163,10 @@ void mark_page_dirty_in_slot(struct kvm *kvm,
> {
> struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
>
> +#ifdef CONFIG_HAVE_KVM_DIRTY_RING
> if (WARN_ON_ONCE(!vcpu) || WARN_ON_ONCE(vcpu->kvm != kvm))
> return;
> +#endif
>
> if (memslot && kvm_slot_dirty_track_enabled(memslot)) {
> unsigned long rel_gfn = gfn - memslot->base_gfn;
Paolo, are you going to pick this for next for the time being?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] KVM: avoid warning on s390 in mark_page_dirty
2022-01-18 8:37 ` Christian Borntraeger
@ 2022-01-18 8:44 ` Paolo Bonzini
2022-01-18 8:53 ` Christian Borntraeger
0 siblings, 1 reply; 13+ messages in thread
From: Paolo Bonzini @ 2022-01-18 8:44 UTC (permalink / raw)
To: Christian Borntraeger, dwmw2
Cc: butterflyhuangxx, kvm, linux-kernel, seanjc, Cornelia Huck,
Janosch Frank, David Hildenbrand, linux-s390, Thomas Huth,
Claudio Imbrenda
On 1/18/22 09:37, Christian Borntraeger wrote:
> Am 13.01.22 um 13:29 schrieb Christian Borntraeger:
>> Avoid warnings on s390 like
>> [ 1801.980931] CPU: 12 PID: 117600 Comm: kworker/12:0 Tainted:
>> G E
>> 5.17.0-20220113.rc0.git0.32ce2abb03cf.300.fc35.s390x+next #1
>> [ 1801.980938] Workqueue: events irqfd_inject [kvm]
>> [...]
>> [ 1801.981057] Call Trace:
>> [ 1801.981060] [<000003ff805f0f5c>] mark_page_dirty_in_slot+0xa4/0xb0
>> [kvm]
>> [ 1801.981083] [<000003ff8060e9fe>] adapter_indicators_set+0xde/0x268
>> [kvm]
>> [ 1801.981104] [<000003ff80613c24>] set_adapter_int+0x64/0xd8 [kvm]
>> [ 1801.981124] [<000003ff805fb9aa>] kvm_set_irq+0xc2/0x130 [kvm]
>> [ 1801.981144] [<000003ff805f8d86>] irqfd_inject+0x76/0xa0 [kvm]
>> [ 1801.981164] [<0000000175e56906>] process_one_work+0x1fe/0x470
>> [ 1801.981173] [<0000000175e570a4>] worker_thread+0x64/0x498
>> [ 1801.981176] [<0000000175e5ef2c>] kthread+0x10c/0x110
>> [ 1801.981180] [<0000000175de73c8>] __ret_from_fork+0x40/0x58
>> [ 1801.981185] [<000000017698440a>] ret_from_fork+0xa/0x40
>>
>> when writing to a guest from an irqfd worker as long as we do not have
>> the dirty ring.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
>> ---
>> virt/kvm/kvm_main.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index 504158f0e131..1a682d3e106d 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -3163,8 +3163,10 @@ void mark_page_dirty_in_slot(struct kvm *kvm,
>> {
>> struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
>> +#ifdef CONFIG_HAVE_KVM_DIRTY_RING
>> if (WARN_ON_ONCE(!vcpu) || WARN_ON_ONCE(vcpu->kvm != kvm))
>> return;
>> +#endif
>> if (memslot && kvm_slot_dirty_track_enabled(memslot)) {
>> unsigned long rel_gfn = gfn - memslot->base_gfn;
>
> Paolo, are you going to pick this for next for the time being?
>
Yep, done now.
Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] KVM: avoid warning on s390 in mark_page_dirty
2022-01-18 8:44 ` Paolo Bonzini
@ 2022-01-18 8:53 ` Christian Borntraeger
2022-01-18 11:44 ` Paolo Bonzini
0 siblings, 1 reply; 13+ messages in thread
From: Christian Borntraeger @ 2022-01-18 8:53 UTC (permalink / raw)
To: Paolo Bonzini, dwmw2
Cc: butterflyhuangxx, kvm, linux-kernel, seanjc, Cornelia Huck,
Janosch Frank, David Hildenbrand, linux-s390, Thomas Huth,
Claudio Imbrenda
Am 18.01.22 um 09:44 schrieb Paolo Bonzini:
> On 1/18/22 09:37, Christian Borntraeger wrote:
>> Am 13.01.22 um 13:29 schrieb Christian Borntraeger:
>>> Avoid warnings on s390 like
>>> [ 1801.980931] CPU: 12 PID: 117600 Comm: kworker/12:0 Tainted: G E 5.17.0-20220113.rc0.git0.32ce2abb03cf.300.fc35.s390x+next #1
>>> [ 1801.980938] Workqueue: events irqfd_inject [kvm]
>>> [...]
>>> [ 1801.981057] Call Trace:
>>> [ 1801.981060] [<000003ff805f0f5c>] mark_page_dirty_in_slot+0xa4/0xb0 [kvm]
>>> [ 1801.981083] [<000003ff8060e9fe>] adapter_indicators_set+0xde/0x268 [kvm]
>>> [ 1801.981104] [<000003ff80613c24>] set_adapter_int+0x64/0xd8 [kvm]
>>> [ 1801.981124] [<000003ff805fb9aa>] kvm_set_irq+0xc2/0x130 [kvm]
>>> [ 1801.981144] [<000003ff805f8d86>] irqfd_inject+0x76/0xa0 [kvm]
>>> [ 1801.981164] [<0000000175e56906>] process_one_work+0x1fe/0x470
>>> [ 1801.981173] [<0000000175e570a4>] worker_thread+0x64/0x498
>>> [ 1801.981176] [<0000000175e5ef2c>] kthread+0x10c/0x110
>>> [ 1801.981180] [<0000000175de73c8>] __ret_from_fork+0x40/0x58
>>> [ 1801.981185] [<000000017698440a>] ret_from_fork+0xa/0x40
>>>
>>> when writing to a guest from an irqfd worker as long as we do not have
>>> the dirty ring.
>>>
>>> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
>>> ---
>>> virt/kvm/kvm_main.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>>> index 504158f0e131..1a682d3e106d 100644
>>> --- a/virt/kvm/kvm_main.c
>>> +++ b/virt/kvm/kvm_main.c
>>> @@ -3163,8 +3163,10 @@ void mark_page_dirty_in_slot(struct kvm *kvm,
>>> {
>>> struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
>>> +#ifdef CONFIG_HAVE_KVM_DIRTY_RING
>>> if (WARN_ON_ONCE(!vcpu) || WARN_ON_ONCE(vcpu->kvm != kvm))
>>> return;
>>> +#endif
>>> if (memslot && kvm_slot_dirty_track_enabled(memslot)) {
>>> unsigned long rel_gfn = gfn - memslot->base_gfn;
>>
>> Paolo, are you going to pick this for next for the time being?
>>
>
> Yep, done now.
>
> Paolo
Thanks. I just realized that Davids patch meanwhile landed in Linus tree. So better
take this via master and not next.
Maybe also add
Fixes: 2efd61a608b0 ("KVM: Warn if mark_page_dirty() is called without an active vCPU")
in case the patch is picked for stable
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] KVM: avoid warning on s390 in mark_page_dirty
2022-01-18 8:53 ` Christian Borntraeger
@ 2022-01-18 11:44 ` Paolo Bonzini
0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2022-01-18 11:44 UTC (permalink / raw)
To: Christian Borntraeger, dwmw2
Cc: butterflyhuangxx, kvm, linux-kernel, seanjc, Cornelia Huck,
Janosch Frank, David Hildenbrand, linux-s390, Thomas Huth,
Claudio Imbrenda
On 1/18/22 09:53, Christian Borntraeger wrote:
>>>
>>> Paolo, are you going to pick this for next for the time being?
>>>
>>
>> Yep, done now.
>>
>> Paolo
>
> Thanks. I just realized that Davids patch meanwhile landed in Linus
> tree. So better
> take this via master and not next.
Yeah, I understood what you meant. :) In fact, "master" right now is
still on 5.16 (for patches that are destined to stable, but have
conflicts merge window changes; those are pushed to master and merged
from there into next). There will be another pull request this week and
it will have this patch.
> Maybe also add
> Fixes: 2efd61a608b0 ("KVM: Warn if mark_page_dirty() is called without
> an active vCPU")
> in case the patch is picked for stable
Ok, will do.
Paolo
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-01-18 11:44 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <e8f40b8765f2feefb653d8a67e487818f66581aa.camel@infradead.org>
2022-01-13 12:06 ` KVM: Warn if mark_page_dirty() is called without an active vCPU Christian Borntraeger
2022-01-13 12:14 ` Paolo Bonzini
2022-01-13 12:29 ` [PATCH] KVM: avoid warning on s390 in mark_page_dirty Christian Borntraeger
2022-01-13 12:31 ` David Woodhouse
2022-01-18 8:37 ` Christian Borntraeger
2022-01-18 8:44 ` Paolo Bonzini
2022-01-18 8:53 ` Christian Borntraeger
2022-01-18 11:44 ` Paolo Bonzini
2022-01-13 12:30 ` KVM: Warn if mark_page_dirty() is called without an active vCPU David Woodhouse
2022-01-13 12:51 ` Christian Borntraeger
2022-01-13 13:22 ` David Woodhouse
2022-01-13 15:09 ` Christian Borntraeger
2022-01-13 14:36 ` Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox