diff mbox series

ioapic: kvm: Skip route updates for masked pins

Message ID a84b7e03-f9a8-b577-be27-4d93d1caa1c9@siemens.com (mailing list archive)
State New, archived
Headers show
Series ioapic: kvm: Skip route updates for masked pins | expand

Commit Message

Jan Kiszka June 2, 2019, 11:42 a.m. UTC
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>
---
 hw/intc/ioapic.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Peter Xu June 2, 2019, 12:10 p.m. UTC | #1
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,
Michael S. Tsirkin June 3, 2019, 12:36 a.m. UTC | #2
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
Jan Kiszka June 3, 2019, 6:30 a.m. UTC | #3
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
Jan Kiszka July 21, 2019, 8:58 a.m. UTC | #4
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
Michael S. Tsirkin July 21, 2019, 10:04 a.m. UTC | #5
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.
Paolo Bonzini July 21, 2019, 4:55 p.m. UTC | #6
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 mbox series

Patch

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);
     }