| Message ID | a84b7e03-f9a8-b577-be27-4d93d1caa1c9@siemens.com |
|---|---|
| State | New |
| Headers | show |
| Series | ioapic: kvm: Skip route updates for masked pins | expand |
On Sun, Jun 02, 2019 at 01:42:13PM +0200, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Masked entries will not generate interrupt messages, thus do no need to > be routed by KVM. This is a cosmetic cleanup, just avoiding warnings of > the kind > > qemu-system-x86_64: vtd_irte_get: detected non-present IRTE (index=0, high=0xff00, low=0x100) > > if the masked entry happens to reference a non-present IRTE. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Reviewed-by: Peter Xu <peterx@redhat.com> Thanks, Jan. The "non-cosmetic" part of clearing of those entries (e.g. including when the entries were not setup correctly rather than masked) was never really implemented and that task has been on my todo list for quite a while but with a very low priority (low enough to sink...). I hope I didn't overlook its importance since AFAICT general OSs should hardly trigger those paths and so far I don't see it a very big issue. Regards,
On Sun, Jun 02, 2019 at 01:42:13PM +0200, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Masked entries will not generate interrupt messages, thus do no need to > be routed by KVM. This is a cosmetic cleanup, just avoiding warnings of > the kind > > qemu-system-x86_64: vtd_irte_get: detected non-present IRTE (index=0, high=0xff00, low=0x100) > > if the masked entry happens to reference a non-present IRTE. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/intc/ioapic.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c > index 7074489fdf..2fb288a22d 100644 > --- a/hw/intc/ioapic.c > +++ b/hw/intc/ioapic.c > @@ -197,9 +197,11 @@ static void ioapic_update_kvm_routes(IOAPICCommonState *s) > MSIMessage msg; > struct ioapic_entry_info info; > ioapic_entry_parse(s->ioredtbl[i], &info); > - msg.address = info.addr; > - msg.data = info.data; > - kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL); > + if (!info.masked) { > + msg.address = info.addr; > + msg.data = info.data; > + kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL); > + } > } > kvm_irqchip_commit_routes(kvm_state); > } > -- > 2.16.4
On 02.06.19 14:10, Peter Xu wrote: > On Sun, Jun 02, 2019 at 01:42:13PM +0200, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> Masked entries will not generate interrupt messages, thus do no need to >> be routed by KVM. This is a cosmetic cleanup, just avoiding warnings of >> the kind >> >> qemu-system-x86_64: vtd_irte_get: detected non-present IRTE (index=0, high=0xff00, low=0x100) >> >> if the masked entry happens to reference a non-present IRTE. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > Reviewed-by: Peter Xu <peterx@redhat.com> > > Thanks, Jan. > > The "non-cosmetic" part of clearing of those entries (e.g. including > when the entries were not setup correctly rather than masked) was > never really implemented and that task has been on my todo list for > quite a while but with a very low priority (low enough to sink...). I > hope I didn't overlook its importance since AFAICT general OSs should > hardly trigger those paths and so far I don't see it a very big issue. I triggered the message during the handover phase from Linux to Jailhouse. That involves reprogramming both IOAPIC and IRTEs - likely unusual sequences, I just didn't find invalid ones. Thanks, Jan
On 03.06.19 02:36, Michael S. Tsirkin wrote: > On Sun, Jun 02, 2019 at 01:42:13PM +0200, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> Masked entries will not generate interrupt messages, thus do no need to >> be routed by KVM. This is a cosmetic cleanup, just avoiding warnings of >> the kind >> >> qemu-system-x86_64: vtd_irte_get: detected non-present IRTE (index=0, high=0xff00, low=0x100) >> >> if the masked entry happens to reference a non-present IRTE. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > >> --- >> hw/intc/ioapic.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c >> index 7074489fdf..2fb288a22d 100644 >> --- a/hw/intc/ioapic.c >> +++ b/hw/intc/ioapic.c >> @@ -197,9 +197,11 @@ static void ioapic_update_kvm_routes(IOAPICCommonState *s) >> MSIMessage msg; >> struct ioapic_entry_info info; >> ioapic_entry_parse(s->ioredtbl[i], &info); >> - msg.address = info.addr; >> - msg.data = info.data; >> - kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL); >> + if (!info.masked) { >> + msg.address = info.addr; >> + msg.data = info.data; >> + kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL); >> + } >> } >> kvm_irqchip_commit_routes(kvm_state); >> } >> -- >> 2.16.4 > > Ping. Or is this queued for 4.2? Jan
On Sun, Jul 21, 2019 at 10:58:42AM +0200, Jan Kiszka wrote: > On 03.06.19 02:36, Michael S. Tsirkin wrote: > > On Sun, Jun 02, 2019 at 01:42:13PM +0200, Jan Kiszka wrote: > >> From: Jan Kiszka <jan.kiszka@siemens.com> > >> > >> Masked entries will not generate interrupt messages, thus do no need to > >> be routed by KVM. This is a cosmetic cleanup, just avoiding warnings of > >> the kind > >> > >> qemu-system-x86_64: vtd_irte_get: detected non-present IRTE (index=0, high=0xff00, low=0x100) > >> > >> if the masked entry happens to reference a non-present IRTE. > >> > >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > > > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > > > >> --- > >> hw/intc/ioapic.c | 8 +++++--- > >> 1 file changed, 5 insertions(+), 3 deletions(-) > >> > >> diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c > >> index 7074489fdf..2fb288a22d 100644 > >> --- a/hw/intc/ioapic.c > >> +++ b/hw/intc/ioapic.c > >> @@ -197,9 +197,11 @@ static void ioapic_update_kvm_routes(IOAPICCommonState *s) > >> MSIMessage msg; > >> struct ioapic_entry_info info; > >> ioapic_entry_parse(s->ioredtbl[i], &info); > >> - msg.address = info.addr; > >> - msg.data = info.data; > >> - kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL); > >> + if (!info.masked) { > >> + msg.address = info.addr; > >> + msg.data = info.data; > >> + kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL); > >> + } > >> } > >> kvm_irqchip_commit_routes(kvm_state); > >> } > >> -- > >> 2.16.4 > > > > > > Ping. Or is this queued for 4.2? > > Jan Paolo was queueing ioapic things recently. I can take this if he doesn't respond until I pack up my next pull req.
On 21/07/19 12:04, Michael S. Tsirkin wrote: > On Sun, Jul 21, 2019 at 10:58:42AM +0200, Jan Kiszka wrote: >> On 03.06.19 02:36, Michael S. Tsirkin wrote: >>> On Sun, Jun 02, 2019 at 01:42:13PM +0200, Jan Kiszka wrote: >>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>> >>>> Masked entries will not generate interrupt messages, thus do no need to >>>> be routed by KVM. This is a cosmetic cleanup, just avoiding warnings of >>>> the kind >>>> >>>> qemu-system-x86_64: vtd_irte_get: detected non-present IRTE (index=0, high=0xff00, low=0x100) >>>> >>>> if the masked entry happens to reference a non-present IRTE. >>>> >>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >>> >>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> >>> >>>> --- >>>> hw/intc/ioapic.c | 8 +++++--- >>>> 1 file changed, 5 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c >>>> index 7074489fdf..2fb288a22d 100644 >>>> --- a/hw/intc/ioapic.c >>>> +++ b/hw/intc/ioapic.c >>>> @@ -197,9 +197,11 @@ static void ioapic_update_kvm_routes(IOAPICCommonState *s) >>>> MSIMessage msg; >>>> struct ioapic_entry_info info; >>>> ioapic_entry_parse(s->ioredtbl[i], &info); >>>> - msg.address = info.addr; >>>> - msg.data = info.data; >>>> - kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL); >>>> + if (!info.masked) { >>>> + msg.address = info.addr; >>>> + msg.data = info.data; >>>> + kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL); >>>> + } >>>> } >>>> kvm_irqchip_commit_routes(kvm_state); >>>> } >>>> -- >>>> 2.16.4 >>> >>> >> >> Ping. Or is this queued for 4.2? >> >> Jan > > Paolo was queueing ioapic things recently. I can take this if > he doesn't respond until I pack up my next pull req. > I've already sent a pull request and had missed this patch, so please go ahead. Paolo
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c index 7074489fdf..2fb288a22d 100644 --- a/hw/intc/ioapic.c +++ b/hw/intc/ioapic.c @@ -197,9 +197,11 @@ static void ioapic_update_kvm_routes(IOAPICCommonState *s) MSIMessage msg; struct ioapic_entry_info info; ioapic_entry_parse(s->ioredtbl[i], &info); - msg.address = info.addr; - msg.data = info.data; - kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL); + if (!info.masked) { + msg.address = info.addr; + msg.data = info.data; + kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL); + } } kvm_irqchip_commit_routes(kvm_state); }