qemu-kvm: Clean up and enhance MSI irqchip support
cover qemu-kvm: Clean up and enhance MSI irqchip support
I'm not fully happy with the cleanups yet, but I think it is a step forward. Moreover, it adds the critical support for classic MSI in irqchip mode. That makes AHCI and other MSI-using device models available for the standard KVM mode. So I decided to roll this out.
Jan Kiszka (7):
qemu-kvm: Drop unneeded kvm_irq_routing_entry declaration
qemu-kvm: Rename kvm_msix_message to KVMMsiMessage
qemu-kvm: Refactor MSI core API of KVM
qemu-kvm: Fix and clean up msix vector use/unuse hooks
qemu-kvm: Move gsi bits from kvm_msix_vector_add to
kvm_msi_add_message
qemu-kvm: Move entry comparison into kvm_msi_update_message
qemu-kvm: Add in-kernel irqchip support for MSI
hw/msi.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/msix.c | 73 +++++++++++++-------------------------- hw/pci.h | 16 +++----- kvm.h | 20 ++++++---- qemu-kvm.c | 57 ++++++++++++++++++++----------- 5 files changed, 190 insertions(+), 87 deletions(-)
PATCH 1/7 qemu-kvm: Drop unneeded kvm_irq_routing_entry declaration
From: Jan Kiszka <[email protected]>
--- hw/pci.h | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
hw/pci.h +0 -2
@@ -6,8 +6,6 @@#include "qdev.h" -struct kvm_irq_routing_entry ; -/* PCI includes legacy ISA access. */ #include "isa.h"
Signature · 1 line
1.7.1
PATCH 2/7 qemu-kvm: Rename kvm_msix_message to KVMMsiMessage
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
This structure will be used for legacy MSI as well and will become part of the KVM API.
--- hw/msix.c | 10 +++++----- hw/pci.h | 10 ++-------- kvm.h | 7 +++++++ 3 files changed, 14 insertions(+), 13 deletions(-)
hw/msix.c +5 -5
@@ -51,7 +51,7 @@ int msix_supported;static void kvm_msix_free (PCIDevice *dev) {int vector, changed =0 ; -struct kvm_msix_message *kmm; +KVMMsiMessage *kmm;for (vector =0 ; vector < dev->msix_entries_nr; ++vector) {if (dev->msix_entry_used[vector]) {
@@ -66,7 +66,7 @@ static void kvm_msix_free(PCIDevice *dev) }static void kvm_msix_message_from_vector (PCIDevice *dev,unsigned vector, -struct kvm_msix_message *kmm) +KVMMsiMessage *kmm) {uint8_t *table_entry = dev->msix_table_page + vector *MSIX_ENTRY_SIZE ;
@@ -78,7 +78,7 @@ static void kvm_msix_message_from_vector(PCIDevice *dev, unsigned vector,static void kvm_msix_update (PCIDevice *dev,int vector,int was_masked,int is_masked) { -struct kvm_msix_message e = {}, *entry; +KVMMsiMessage e = {}, *entry;int mask_cleared = was_masked && !is_masked;/* It is only legal to change an entry when it is masked. Therefore, it is * enough to update the routing in kernel when mask is being cleared. */
@@ -114,7 +114,7 @@ static void kvm_msix_update(PCIDevice *dev, int vector,static int kvm_msix_add (PCIDevice *dev,unsigned vector) { -struct kvm_msix_message *kmm = dev->msix_irq_entries + vector; +KVMMsiMessage *kmm = dev->msix_irq_entries + vector;int r;if (!kvm_has_gsi_routing ()) {
@@ -147,7 +147,7 @@ static int kvm_msix_add(PCIDevice *dev, unsigned vector)static void kvm_msix_del (PCIDevice *dev,unsigned vector) { -struct kvm_msix_message *kmm; +KVMMsiMessage *kmm;if (dev->msix_entry_used[vector]) {return ;
hw/pci.h +2 -8
@@ -5,6 +5,7 @@#include "qobject.h" #include "qdev.h" +#include "kvm.h" /* PCI includes legacy ISA access. */ #include "isa.h"
@@ -129,13 +130,6 @@ enum {typedef int (*msix_mask_notifier_func )(PCIDevice *,unsigned vector,int masked); -struct kvm_msix_message { -uint32_t gsi; -uint32_t addr_lo; -uint32_t addr_hi; -uint32_t data; -}; -struct PCIDevice {DeviceState qdev;/* PCI config space */
@@ -208,7 +202,7 @@ struct PCIDevice {* on the rest of the region. */ target_phys_addr_t msix_page_size; -struct kvm_msix_message *msix_irq_entries; +KVMMsiMessage *msix_irq_entries;msix_mask_notifier_func msix_mask_notifier; };
kvm.h +7 -0
@@ -214,6 +214,13 @@ int kvm_set_irqfd(int gsi, int fd, bool assigned)int kvm_set_ioeventfd_pio_word (int fd,uint16_t adr,uint16_t val,bool assign); +typedef struct KVMMsiMessage { +uint32_t gsi; +uint32_t addr_lo; +uint32_t addr_hi; +uint32_t data; +}KVMMsiMessage ; +int kvm_has_gsi_routing (void );int kvm_get_irq_route_gsi (void ); int kvm_add_msix(uint32_t gsi, uint32_t addr_lo,
Signature · 1 line
1.7.1
Re: [PATCH 2/7] qemu-kvm: Rename kvm_msix_message to KVMMsiMessage
Quoted 6 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This structure will be used for legacy MSI as well and will become part of the KVM API.
Signed-off-by Jan Kiszka <[email protected]>
I'd like KVMMSIMessage better. struct kvm_msi_message would be even better but oh well.
Quoted 60 lines
--- hw/msix.c | 10 +++++----- hw/pci.h | 10 ++-------- kvm.h | 7 +++++++ 3 files changed, 14 insertions(+), 13 deletions(-)hw/msix.c +5
-5@@ -51,7 +51,7 @@ int msix_supported;static void kvm_msix_free (PCIDevice *dev) {int vector, changed =0 ; -struct kvm_msix_message *kmm; +KVMMsiMessage *kmm;for (vector =0 ; vector < dev->msix_entries_nr; ++vector) {if (dev->msix_entry_used[vector]) {@@ -66,7 +66,7 @@ static void kvm_msix_free(PCIDevice *dev) }static void kvm_msix_message_from_vector (PCIDevice *dev,unsigned vector, -struct kvm_msix_message *kmm) +KVMMsiMessage *kmm) {uint8_t *table_entry = dev->msix_table_page + vector *MSIX_ENTRY_SIZE ;@@ -78,7 +78,7 @@ static void kvm_msix_message_from_vector(PCIDevice *dev, unsigned vector,static void kvm_msix_update (PCIDevice *dev,int vector,int was_masked,int is_masked) { -struct kvm_msix_message e = {}, *entry; +KVMMsiMessage e = {}, *entry;int mask_cleared = was_masked && !is_masked;/* It is only legal to change an entry when it is masked. Therefore, it is * enough to update the routing in kernel when mask is being cleared. */ @@ -114,7 +114,7 @@ static void kvm_msix_update(PCIDevice *dev, int vector,static int kvm_msix_add (PCIDevice *dev,unsigned vector) { -struct kvm_msix_message *kmm = dev->msix_irq_entries + vector; +KVMMsiMessage *kmm = dev->msix_irq_entries + vector;int r;if (!kvm_has_gsi_routing ()) {@@ -147,7 +147,7 @@ static int kvm_msix_add(PCIDevice *dev, unsigned vector)static void kvm_msix_del (PCIDevice *dev,unsigned vector) { -struct kvm_msix_message *kmm; +KVMMsiMessage *kmm;if (dev->msix_entry_used[vector]) {return ;hw/pci.h +0
-0
hw/pci.h line 8
Quoted 4 lines
hw/pci.h +1
-0@@ -5,6 +5,7 @@#include "qobject.h" #include "qdev.h" +#include "kvm.h"
I put kvm_msix_message in pci.h to avoid having every pci device pull in kvm.h is anything wrong with that? Maybe just rename it to make it generic for msi and leave if where it is.
Quoted 46 lines
hw/pci.h +1
-8/* PCI includes legacy ISA access. */ #include "isa.h" @@ -129,13 +130,6 @@ enum {typedef int (*msix_mask_notifier_func )(PCIDevice *,unsigned vector,int masked); -struct kvm_msix_message { -uint32_t gsi; -uint32_t addr_lo; -uint32_t addr_hi; -uint32_t data; -}; -struct PCIDevice {DeviceState qdev;/* PCI config space */ @@ -208,7 +202,7 @@ struct PCIDevice {* on the rest of the region. */ target_phys_addr_t msix_page_size; -struct kvm_msix_message *msix_irq_entries; +KVMMsiMessage *msix_irq_entries;msix_mask_notifier_func msix_mask_notifier; };kvm.h +7
-0@@ -214,6 +214,13 @@ int kvm_set_irqfd(int gsi, int fd, bool assigned)int kvm_set_ioeventfd_pio_word (int fd,uint16_t adr,uint16_t val,bool assign); +typedef struct KVMMsiMessage { +uint32_t gsi; +uint32_t addr_lo; +uint32_t addr_hi; +uint32_t data; +}KVMMsiMessage ; +int kvm_has_gsi_routing (void );int kvm_get_irq_route_gsi (void ); int kvm_add_msix(uint32_t gsi, uint32_t addr_lo,Signature · 1 line
1.7.1
Re: [PATCH 2/7] qemu-kvm: Rename kvm_msix_message to KVMMsiMessage
Quoted 10 lines · Michael S. Tsirkin
Quoted 6 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This structure will be used for legacy MSI as well and will become part of the KVM API.
Signed-off-by Jan Kiszka <[email protected]>I'd like KVMMSIMessage better. struct
kvm_msi_messagewould be even better but oh well.
Yeah, QEMU naming convention is not designed for such use cases... Will change it as you prefer.
Quoted 63 lines
Quoted 63 lines
--- hw/msix.c | 10 +++++----- hw/pci.h | 10 ++-------- kvm.h | 7 +++++++ 3 files changed, 14 insertions(+), 13 deletions(-)hw/msix.c +5
-5@@ -51,7 +51,7 @@ int msix_supported;static void kvm_msix_free (PCIDevice *dev) {int vector, changed =0 ; -struct kvm_msix_message *kmm; +KVMMsiMessage *kmm;for (vector =0 ; vector < dev->msix_entries_nr; ++vector) {if (dev->msix_entry_used[vector]) {@@ -66,7 +66,7 @@ static void kvm_msix_free(PCIDevice *dev) }static void kvm_msix_message_from_vector (PCIDevice *dev,unsigned vector, -struct kvm_msix_message *kmm) +KVMMsiMessage *kmm) {uint8_t *table_entry = dev->msix_table_page + vector *MSIX_ENTRY_SIZE ;@@ -78,7 +78,7 @@ static void kvm_msix_message_from_vector(PCIDevice *dev, unsigned vector,static void kvm_msix_update (PCIDevice *dev,int vector,int was_masked,int is_masked) { -struct kvm_msix_message e = {}, *entry; +KVMMsiMessage e = {}, *entry;int mask_cleared = was_masked && !is_masked;/* It is only legal to change an entry when it is masked. Therefore, it is * enough to update the routing in kernel when mask is being cleared. */ @@ -114,7 +114,7 @@ static void kvm_msix_update(PCIDevice *dev, int vector,static int kvm_msix_add (PCIDevice *dev,unsigned vector) { -struct kvm_msix_message *kmm = dev->msix_irq_entries + vector; +KVMMsiMessage *kmm = dev->msix_irq_entries + vector;int r;if (!kvm_has_gsi_routing ()) {@@ -147,7 +147,7 @@ static int kvm_msix_add(PCIDevice *dev, unsigned vector)static void kvm_msix_del (PCIDevice *dev,unsigned vector) { -struct kvm_msix_message *kmm; +KVMMsiMessage *kmm;if (dev->msix_entry_used[vector]) {return ;hw/pci.h +0
-0@@ -5,6 +5,7 @@#include "qobject.h"
Quoted 6 lines
hw/pci.h line 8
Quoted 2 lines
hw/pci.h +1
-0#include "qdev.h" +#include "kvm.h" I put
kvm_msix_messageinpci.hto avoid having every pci device pull inkvm.his anything wrong with that? Maybe just rename it to make it generic for msi and leave if where it is.
kvm.h shall provide kvm related types, not some unrelated header. That's even more important with MSI support for non-PCI devices (aka HPET).
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [PATCH 2/7] qemu-kvm: Rename kvm_msix_message to KVMMsiMessage
Quoted 7 lines · Jan Kiszka
Quoted 4 lines
I put
kvm_msix_messageinpci.hto avoid having every pci device pull inkvm.his anything wrong with that? Maybe just rename it to make it generic for msi and leave if where it is.
kvm.hshall provide kvm related types, not some unrelated header. That's even more important with MSI support for non-PCI devices (aka HPET).
We could have an MSIMessage type that abstracts the general facility, and let it embed a KVMMsiMessage that contains just the gsi.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [PATCH 2/7] qemu-kvm: Rename kvm_msix_message to KVMMsiMessage
Quoted 11 lines · Avi Kivity
Quoted 7 lines · Jan Kiszka
Quoted 4 lines
I put
kvm_msix_messageinpci.hto avoid having every pci device pull inkvm.his anything wrong with that? Maybe just rename it to make it generic for msi and leave if where it is.
kvm.hshall provide kvm related types, not some unrelated header. That's even more important with MSI support for non-PCI devices (aka HPET).We could have an MSIMessage type that abstracts the general facility, and let it embed a KVMMsiMessage that contains just the gsi.
Yes, likely also useful for generic MSI delivery services that bypass stl_phys.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [PATCH 2/7] qemu-kvm: Rename kvm_msix_message to KVMMsiMessage
Quoted 7 lines · Jan Kiszka
Quoted 6 lines · Avi Kivity
Quoted 5 lines · Jan Kiszka
Quoted 4 lines
I put
kvm_msix_messageinpci.hto avoid having every pci device pull inkvm.his anything wrong with that? Maybe just rename it to make it generic for msi and leave if where it is.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
kvm.hshall provide kvm related types, not some unrelated header. That's even more important with MSI support for non-PCI devices (aka HPET).We could have an MSIMessage type that abstracts the general facility, and let it embed a KVMMsiMessage that contains just the gsi.
Yes, likely also useful for generic MSI delivery services that bypass
stl_phys.
Right, so you can cache the phys_page and apic lookups.
A different layer in which to accomplish this is to have a per-device tlb. So we'd have
CachedPhysicalAddress msi_fsb_tlb;
stl_phys_tlb(&msi_fsb_cache, addr, data);which caches the addr lookup in a 1-entry tlb. If we have many non-msi repeated device writes to the same address, this might be worthwhile. From a quick look at the code, I don't think we do, though.
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [PATCH 2/7] qemu-kvm: Rename kvm_msix_message to KVMMsiMessage
Quoted 23 lines · Avi Kivity
Quoted 7 lines · Jan Kiszka
Quoted 6 lines · Avi Kivity
Quoted 5 lines · Jan Kiszka
Quoted 4 lines
I put
kvm_msix_messageinpci.hto avoid having every pci device pull inkvm.his anything wrong with that? Maybe just rename it to make it generic for msi and leave if where it is.Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
kvm.hshall provide kvm related types, not some unrelated header. That's even more important with MSI support for non-PCI devices (aka HPET).We could have an MSIMessage type that abstracts the general facility, and let it embed a KVMMsiMessage that contains just the gsi.
Yes, likely also useful for generic MSI delivery services that bypass
stl_phys.Right, so you can cache the
phys_pageand apic lookups.A different layer in which to accomplish this is to have a per-device tlb. So we'd have
Quoted 7 lines
CachedPhysicalAddress msi_fsb_tlb; stl_phys_tlb(&msi_fsb_cache, addr, data);which caches the addr lookup in a 1-entry tlb. If we have many non-msi repeated device writes to the same address, this might be worthwhile.
From a quick look at the code, I don't think we do, though.
There is no caching need if the messages targets the fixed MSI address window. We can simply pass it directly to the APIC then.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [PATCH 2/7] qemu-kvm: Rename kvm_msix_message to KVMMsiMessage
In reply to Avi KivityQuoted 6 lines · Avi Kivity
Quoted 5 lines · Jan Kiszka
Quoted 4 lines
I put
kvm_msix_messageinpci.hto avoid having every pci device pull inkvm.his anything wrong with that? Maybe just rename it to make it generic for msi and leave if where it is.
Quoted 6 lines
Quoted 2 lines
kvm.hshall provide kvm related types, not some unrelated header. That's even more important with MSI support for non-PCI devices (aka HPET).We could have an MSIMessage type that abstracts the general facility, and let it embed a KVMMsiMessage that contains just the gsi.
The whole structure is only needed for kvm though: no one else seems to want an MSIMessage.
Quoted 2 lines
Signature · 1 line
error compiling committee.c: too many arguments to function
Re: [PATCH 2/7] qemu-kvm: Rename kvm_msix_message to KVMMsiMessage
In reply to Jan KiszkaQuoted 85 lines · Jan Kiszka
Quoted 10 lines · Michael S. Tsirkin
Quoted 6 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This structure will be used for legacy MSI as well and will become part of the KVM API.
Signed-off-by Jan Kiszka <[email protected]>I'd like KVMMSIMessage better. struct
kvm_msi_messagewould be even better but oh well.Yeah, QEMU naming convention is not designed for such use cases... Will change it as you prefer.
Quoted 63 lines
Quoted 63 lines
--- hw/msix.c | 10 +++++----- hw/pci.h | 10 ++-------- kvm.h | 7 +++++++ 3 files changed, 14 insertions(+), 13 deletions(-)hw/msix.c +5
-5@@ -51,7 +51,7 @@ int msix_supported;static void kvm_msix_free (PCIDevice *dev) {int vector, changed =0 ; -struct kvm_msix_message *kmm; +KVMMsiMessage *kmm;for (vector =0 ; vector < dev->msix_entries_nr; ++vector) {if (dev->msix_entry_used[vector]) {@@ -66,7 +66,7 @@ static void kvm_msix_free(PCIDevice *dev) }static void kvm_msix_message_from_vector (PCIDevice *dev,unsigned vector, -struct kvm_msix_message *kmm) +KVMMsiMessage *kmm) {uint8_t *table_entry = dev->msix_table_page + vector *MSIX_ENTRY_SIZE ;@@ -78,7 +78,7 @@ static void kvm_msix_message_from_vector(PCIDevice *dev, unsigned vector,static void kvm_msix_update (PCIDevice *dev,int vector,int was_masked,int is_masked) { -struct kvm_msix_message e = {}, *entry; +KVMMsiMessage e = {}, *entry;int mask_cleared = was_masked && !is_masked;/* It is only legal to change an entry when it is masked. Therefore, it is * enough to update the routing in kernel when mask is being cleared. */ @@ -114,7 +114,7 @@ static void kvm_msix_update(PCIDevice *dev, int vector,static int kvm_msix_add (PCIDevice *dev,unsigned vector) { -struct kvm_msix_message *kmm = dev->msix_irq_entries + vector; +KVMMsiMessage *kmm = dev->msix_irq_entries + vector;int r;if (!kvm_has_gsi_routing ()) {@@ -147,7 +147,7 @@ static int kvm_msix_add(PCIDevice *dev, unsigned vector)static void kvm_msix_del (PCIDevice *dev,unsigned vector) { -struct kvm_msix_message *kmm; +KVMMsiMessage *kmm;if (dev->msix_entry_used[vector]) {return ;hw/pci.h +0
-0@@ -5,6 +5,7 @@#include "qobject.h" Quoted 6 lines
hw/pci.h line 8
Quoted 2 lines
hw/pci.h +1
-0#include "qdev.h" +#include "kvm.h" I put
kvm_msix_messageinpci.hto avoid having every pci device pull inkvm.his anything wrong with that? Maybe just rename it to make it generic for msi and leave if where it is.
Quoted 4 lines
kvm.hshall provide kvm related types, not some unrelated header. That's even more important with MSI support for non-PCI devices (aka HPET).Jan
Generally qemu suggests forward-declaring types so all users don't need to pull them in. This should certainly work here:
declare the type in qemu-common.h, then msix.c and kvm are the only ones that need to include kvm.h
Quoted 3 lines
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
PATCH 3/7 qemu-kvm: Refactor MSI core API of KVM
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
Remove MSI-X from function names - the interface is valid for MSI as well - and use KVMMsiMessage as parameter.
--- hw/msix.c | 15 ++++----------- kvm.h | 13 +++++-------- qemu-kvm.c | 32 +++++++++++++------------------- 3 files changed, 22 insertions(+), 38 deletions(-)
hw/msix.c +4 -11
@@ -51,12 +51,10 @@ int msix_supported;static void kvm_msix_free (PCIDevice *dev) {int vector, changed =0 ; -KVMMsiMessage *kmm;for (vector =0 ; vector < dev->msix_entries_nr; ++vector) {if (dev->msix_entry_used[vector]) { - kmm = &dev->msix_irq_entries[vector]; -kvm_del_msix (kmm->gsi, kmm->addr_lo, kmm->addr_hi, kmm->data); +kvm_msi_message_del (&dev->msix_irq_entries[vector]); changed =1 ; } }
@@ -94,9 +92,7 @@ static void kvm_msix_update(PCIDevice *dev, int vector,if (memcmp (entry, &e,sizeof e) !=0 ) {int r; - r =kvm_update_msix (entry->gsi, entry->addr_lo, - entry->addr_hi, entry->data, - e.gsi, e.addr_lo, e.addr_hi, e.data); + r =kvm_msi_message_update (entry, &e);if (r) {fprintf (stderr,"%s: kvm_update_msix failed: %s\n" , __func__,strerror (-r));
@@ -131,7 +127,7 @@ static int kvm_msix_add(PCIDevice *dev, unsigned vector) } kmm->gsi = r;kvm_msix_message_from_vector (dev, vector, kmm); - r =kvm_add_msix (kmm->gsi, kmm->addr_lo, kmm->addr_hi, kmm->data); + r =kvm_msi_message_add (kmm);if (r <0 ) {fprintf (stderr,"%s: kvm_add_msix failed: %s\n" , __func__,strerror (-r));return r;
@@ -147,13 +143,10 @@ static int kvm_msix_add(PCIDevice *dev, unsigned vector)static void kvm_msix_del (PCIDevice *dev,unsigned vector) { -KVMMsiMessage *kmm; -if (dev->msix_entry_used[vector]) {return ; } - kmm = &dev->msix_irq_entries[vector]; -kvm_del_msix (kmm->gsi, kmm->addr_lo, kmm->addr_hi, kmm->data); +kvm_msi_message_del (&dev->msix_irq_entries[vector]);kvm_commit_irq_routes (); }
kvm.h +5 -8
@@ -223,14 +223,11 @@ typedef struct KVMMsiMessage {int kvm_has_gsi_routing (void );int kvm_get_irq_route_gsi (void ); -int kvm_add_msix (uint32_t gsi,uint32_t addr_lo, -uint32_t addr_hi,uint32_t data); -int kvm_del_msix (uint32_t gsi,uint32_t addr_lo, -uint32_t addr_hi,uint32_t data); -int kvm_update_msix (uint32_t old_gsi,uint32_t old_addr_lo, -uint32_t old_addr_hi,uint32_t old_data, -uint32_t new_gsi,uint32_t new_addr_lo, -uint32_t new_addr_hi,uint32_t new_data); + +int kvm_msi_message_add (KVMMsiMessage *msg); +int kvm_msi_message_del (KVMMsiMessage *msg); +int kvm_msi_message_update (KVMMsiMessage *old,KVMMsiMessage *new); +int kvm_commit_irq_routes (void );int kvm_irqchip_in_kernel (void );
qemu-kvm.c +13 -19
@@ -969,46 +969,40 @@ int kvm_get_irq_route_gsi(void)return -ENOSPC ; } -static void kvm_msix_routing_entry (struct kvm_irq_routing_entry *e, -uint32_t gsi,uint32_t addr_lo, -uint32_t addr_hi,uint32_t data) +static void kvm_msi_routing_entry (struct kvm_irq_routing_entry *e, +KVMMsiMessage *msg) { - e->gsi = gsi; + e->gsi = msg->gsi; e->type =KVM_IRQ_ROUTING_MSI ; e->flags =0 ; - e->u.msi.address_lo = addr_lo; - e->u.msi.address_hi = addr_hi; - e->u.msi.data = data; + e->u.msi.address_lo = msg->addr_lo; + e->u.msi.address_hi = msg->addr_hi; + e->u.msi.data = msg->data; } -int kvm_add_msix (uint32_t gsi,uint32_t addr_lo, -uint32_t addr_hi,uint32_t data) +int kvm_msi_message_add (KVMMsiMessage *msg) {struct kvm_irq_routing_entry e; -kvm_msix_routing_entry (&e, gsi, addr_lo, addr_hi, data); +kvm_msi_routing_entry (&e, msg);return kvm_add_routing_entry (&e); } -int kvm_del_msix (uint32_t gsi,uint32_t addr_lo, -uint32_t addr_hi,uint32_t data) +int kvm_msi_message_del (KVMMsiMessage *msg) {struct kvm_irq_routing_entry e; -kvm_msix_routing_entry (&e, gsi, addr_lo, addr_hi, data); +kvm_msi_routing_entry (&e, msg);return kvm_del_routing_entry (&e); } -int kvm_update_msix (uint32_t old_gsi,uint32_t old_addr_lo, -uint32_t old_addr_hi,uint32_t old_data, -uint32_t new_gsi,uint32_t new_addr_lo, -uint32_t new_addr_hi,uint32_t new_data) +int kvm_msi_message_update (KVMMsiMessage *old,KVMMsiMessage *new) {struct kvm_irq_routing_entry e1, e2; -kvm_msix_routing_entry (&e1, old_gsi, old_addr_lo, old_addr_hi, old_data); -kvm_msix_routing_entry (&e2, new_gsi, new_addr_lo, new_addr_hi, new_data); +kvm_msi_routing_entry (&e1, old); +kvm_msi_routing_entry (&e2, new);return kvm_update_routing_entry (&e1, &e2); }
Signature · 1 line
1.7.1
PATCH 4/7 qemu-kvm: Fix and clean up msix vector use/unuse hooks
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
Remove the premature return from msix_vector_use if the vector was already in use, this could cause usage counter imbalances. In contrast, the check for msix_entry_used on deletion was redundant. At this chance, rename the internal API to clarify what is added/deleted here.
--- hw/msix.c | 17 ++++++----------- 1 files changed, 6 insertions(+), 11 deletions(-)
hw/msix.c +6 -11
@@ -108,7 +108,7 @@ static void kvm_msix_update(PCIDevice *dev, int vector, } } -static int kvm_msix_add (PCIDevice *dev,unsigned vector) +static int kvm_msix_vector_add (PCIDevice *dev,unsigned vector) {KVMMsiMessage *kmm = dev->msix_irq_entries + vector;int r;
@@ -141,11 +141,8 @@ static int kvm_msix_add(PCIDevice *dev, unsigned vector)return 0 ; } -static void kvm_msix_del (PCIDevice *dev,unsigned vector) +static void kvm_msix_vector_del (PCIDevice *dev,unsigned vector) { -if (dev->msix_entry_used[vector]) { -return ; - }kvm_msi_message_del (&dev->msix_irq_entries[vector]);kvm_commit_irq_routes (); }
@@ -548,11 +545,9 @@ int msix_vector_use(PCIDevice *dev, unsigned vector)int ret;if (vector >= dev->msix_entries_nr)return -EINVAL ; -if (dev->msix_entry_used[vector]) { -return 0 ; - } -if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { - ret =kvm_msix_add (dev, vector); +if (kvm_enabled () &&kvm_irqchip_in_kernel () && + !dev->msix_entry_used[vector]) { + ret =kvm_msix_vector_add (dev, vector);if (ret) {return ret; }
@@ -571,7 +566,7 @@ void msix_vector_unuse(PCIDevice *dev, unsigned vector)return ; }if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { -kvm_msix_del (dev, vector); +kvm_msix_vector_del (dev, vector); }msix_clr_pending (dev, vector); }
Signature · 1 line
1.7.1
Re: [PATCH 4/7] qemu-kvm: Fix and clean up msix vector use/unuse hooks
Quoted 8 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Remove the premature return from
msix_vector_useif the vector was already in use, this could cause usage counter imbalances. In contrast, the check formsix_entry_usedon deletion was redundant. At this chance, rename the internal API to clarify what is added/deleted here.Signed-off-by Jan Kiszka <[email protected]>
Good catch!
Quoted 56 lines
--- hw/msix.c | 17 ++++++----------- 1 files changed, 6 insertions(+), 11 deletions(-)hw/msix.c +6
-11@@ -108,7 +108,7 @@ static void kvm_msix_update(PCIDevice *dev, int vector, } } -static int kvm_msix_add (PCIDevice *dev,unsigned vector) +static int kvm_msix_vector_add (PCIDevice *dev,unsigned vector) {KVMMsiMessage *kmm = dev->msix_irq_entries + vector;int r;@@ -141,11 +141,8 @@ static int kvm_msix_add(PCIDevice *dev, unsigned vector)return 0 ; } -static void kvm_msix_del (PCIDevice *dev,unsigned vector) +static void kvm_msix_vector_del (PCIDevice *dev,unsigned vector) { -if (dev->msix_entry_used[vector]) { -return ; - }kvm_msi_message_del (&dev->msix_irq_entries[vector]);kvm_commit_irq_routes (); }@@ -548,11 +545,9 @@ int msix_vector_use(PCIDevice *dev, unsigned vector)int ret;if (vector >= dev->msix_entries_nr)return -EINVAL ; -if (dev->msix_entry_used[vector]) { -return 0 ; - } -if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { - ret =kvm_msix_add (dev, vector); +if (kvm_enabled () &&kvm_irqchip_in_kernel () && + !dev->msix_entry_used[vector]) { + ret =kvm_msix_vector_add (dev, vector);if (ret) {return ret; }@@ -571,7 +566,7 @@ void msix_vector_unuse(PCIDevice *dev, unsigned vector)return ; }if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { -kvm_msix_del (dev, vector); +kvm_msix_vector_del (dev, vector); }msix_clr_pending (dev, vector); }Signature · 1 line
1.7.1
PATCH 5/7 qemu-kvm: Move gsi bits from kvm_msix_vector_add to kvm_msi_add_message
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
Testing support and allocating a GSI for an MSI message is required both for MSI and MSI-X. At this chance, drop the aging version warning.
--- hw/msix.c | 13 ------------- qemu-kvm.c | 11 +++++++++++ 2 files changed, 11 insertions(+), 13 deletions(-)
hw/msix.c +0 -13
@@ -113,19 +113,6 @@ static int kvm_msix_vector_add(PCIDevice *dev, unsigned vector)KVMMsiMessage *kmm = dev->msix_irq_entries + vector;int r; -if (!kvm_has_gsi_routing ()) { -fprintf (stderr,"Warning: no MSI-X support found. " -"At least kernel 2.6.30 is required for MSI-X support.\n" - ); -return -EOPNOTSUPP ; - } - - r =kvm_get_irq_route_gsi (); -if (r <0 ) { -fprintf (stderr,"%s: kvm_get_irq_route_gsi failed: %s\n" , __func__,strerror (-r)); -return r; - } - kmm->gsi = r;kvm_msix_message_from_vector (dev, vector, kmm); r =kvm_msi_message_add (kmm);if (r <0 ) {
qemu-kvm.c +11 -0
@@ -984,6 +984,17 @@ static void kvm_msi_routing_entry(struct kvm_irq_routing_entry *e,int kvm_msi_message_add (KVMMsiMessage *msg) {struct kvm_irq_routing_entry e; +int ret; + +if (!kvm_has_gsi_routing ()) { +return -EOPNOTSUPP ; + } + + ret =kvm_get_irq_route_gsi (); +if (ret <0 ) { +return ret; + } + msg->gsi = ret;kvm_msi_routing_entry (&e, msg);return kvm_add_routing_entry (&e);
Signature · 1 line
1.7.1
Re: [PATCH 5/7] qemu-kvm: Move gsi bits from kvm_msix_vector_add to kvm_msi_add_message
Quoted 6 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Testing support and allocating a GSI for an MSI message is required both for MSI and MSI-X. At this chance, drop the aging version warning.
Signed-off-by Jan Kiszka <[email protected]>
No objection, but I do note that this means running on an old kernel will lead to a silent failure.
stderr output is not in fact much better: I think we should check the capability in msix_init. Care coding this up?
Quoted 53 lines
--- hw/msix.c | 13 ------------- qemu-kvm.c | 11 +++++++++++ 2 files changed, 11 insertions(+), 13 deletions(-)hw/msix.c +0
-13@@ -113,19 +113,6 @@ static int kvm_msix_vector_add(PCIDevice *dev, unsigned vector)KVMMsiMessage *kmm = dev->msix_irq_entries + vector;int r; -if (!kvm_has_gsi_routing ()) { -fprintf (stderr,"Warning: no MSI-X support found. " -"At least kernel 2.6.30 is required for MSI-X support.\n" - ); -return -EOPNOTSUPP ; - } - - r =kvm_get_irq_route_gsi (); -if (r <0 ) { -fprintf (stderr,"%s: kvm_get_irq_route_gsi failed: %s\n" , __func__,strerror (-r)); -return r; - } - kmm->gsi = r;kvm_msix_message_from_vector (dev, vector, kmm); r =kvm_msi_message_add (kmm);if (r <0 ) {qemu-kvm.c +11
-0@@ -984,6 +984,17 @@ static void kvm_msi_routing_entry(struct kvm_irq_routing_entry *e,int kvm_msi_message_add (KVMMsiMessage *msg) {struct kvm_irq_routing_entry e; +int ret; + +if (!kvm_has_gsi_routing ()) { +return -EOPNOTSUPP ; + } + + ret =kvm_get_irq_route_gsi (); +if (ret <0 ) { +return ret; + } + msg->gsi = ret;kvm_msi_routing_entry (&e, msg);return kvm_add_routing_entry (&e);Signature · 1 line
1.7.1
Re: [PATCH 5/7] qemu-kvm: Move gsi bits from kvm_msix_vector_add to kvm_msi_add_message
Quoted 10 lines · Michael S. Tsirkin
Quoted 6 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Testing support and allocating a GSI for an MSI message is required both for MSI and MSI-X. At this chance, drop the aging version warning.
Signed-off-by Jan Kiszka <[email protected]>No objection, but I do note that this means running on an old kernel will lead to a silent failure.
Shouldn't be silent: The caller of msi_vector_use should check and process the error (virtio should even forward it to guest IIUC).
Quoted 2 lines
stderr output is not in fact much better: I think we should check the capability in
msix_init. Care coding this up?
I think the motivation to check on vector activation is that devices and guests without a need for MSI should not cause a failure if MSI is unsupported by KVM.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [PATCH 5/7] qemu-kvm: Move gsi bits from kvm_msix_vector_add to kvm_msi_add_message
Quoted 7 lines · Jan Kiszka
Quoted 6 lines · Michael S. Tsirkin
Quoted 5 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Testing support and allocating a GSI for an MSI message is required both for MSI and MSI-X. At this chance, drop the aging version warning.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Jan Kiszka <[email protected]>No objection, but I do note that this means running on an old kernel will lead to a silent failure.
Shouldn't be silent: The caller of
msi_vector_useshould check and process the error (virtio should even forward it to guest IIUC).
It does, but the msi/msix spec does not allow this so not all guest OSes can use this extended reporting.
Quoted 8 lines
Quoted 2 lines
stderr output is not in fact much better: I think we should check the capability in
msix_init. Care coding this up?I think the motivation to check on vector activation is that devices and guests without a need for MSI should not cause a failure if MSI is unsupported by KVM.
Jan
IMO it's better not to report MSI capability in this setup.
Quoted 3 lines
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
PATCH 6/7 qemu-kvm: Move entry comparison into kvm_msi_update_message
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
Checking the the update chances the message content is a common task for both MSI types.
--- hw/msix.c | 26 +++++++++++++------------- qemu-kvm.c | 14 +++++++++++++- 2 files changed, 26 insertions(+), 14 deletions(-)
hw/msix.c +13 -13
@@ -76,8 +76,10 @@ static void kvm_msix_message_from_vector(PCIDevice *dev, unsigned vector,static void kvm_msix_update (PCIDevice *dev,int vector,int was_masked,int is_masked) { -KVMMsiMessage e = {}, *entry; +KVMMsiMessage new_entry, *entry;int mask_cleared = was_masked && !is_masked; +int r; +/* It is only legal to change an entry when it is masked. Therefore, it is * enough to update the routing in kernel when mask is being cleared. */ if (!mask_cleared) {
@@ -86,19 +88,17 @@ static void kvm_msix_update(PCIDevice *dev, int vector,if (!dev->msix_entry_used[vector]) {return ; } - entry = dev->msix_irq_entries + vector; - e.gsi = entry->gsi; -kvm_msix_message_from_vector (dev, vector, &e); -if (memcmp (entry, &e,sizeof e) !=0 ) { -int r; - r =kvm_msi_message_update (entry, &e); -if (r) { -fprintf (stderr,"%s: kvm_update_msix failed: %s\n" , __func__, -strerror (-r)); -exit (1 ); - } - *entry = e; + entry = dev->msix_irq_entries + vector; +kvm_msix_message_from_vector (dev, vector, &new_entry); + r =kvm_msi_message_update (entry, &new_entry); +if (r <0 ) { +fprintf (stderr,"%s: kvm_update_msix failed: %s\n" , __func__, +strerror (-r)); +exit (1 ); + } +if (r >0 ) { + *entry = new_entry; r =kvm_commit_irq_routes ();if (r) { fprintf(stderr, "%s: kvm_commit_irq_routes failed: %s\n", __func__,
qemu-kvm.c +13 -1
@@ -1011,10 +1011,22 @@ int kvm_msi_message_del(KVMMsiMessage *msg)int kvm_msi_message_update (KVMMsiMessage *old,KVMMsiMessage *new) {struct kvm_irq_routing_entry e1, e2; +int ret; + + new->gsi = old->gsi; +if (memcmp (old, new,sizeof (KVMMsiMessage)) ==0 ) { +return 0 ; + }kvm_msi_routing_entry (&e1, old);kvm_msi_routing_entry (&e2, new); -return kvm_update_routing_entry (&e1, &e2); + + ret =kvm_update_routing_entry (&e1, &e2); +if (ret <0 ) { +return ret; + } + +return 1 ; }
Signature · 1 line
1.7.1
PATCH 7/7 qemu-kvm: Add in-kernel irqchip support for MSI
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
This adds the required bits to map classic MSI vectors on KVM IRQ routing entries and deliver them via the irqchip if enabled.
--- hw/msi.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/pci.h | 4 ++ 2 files changed, 115 insertions(+), 0 deletions(-)
hw/msi.c +111 -0
@@ -20,6 +20,7 @@#include "msi.h" #include "range.h" +#include "kvm.h" /* Eventually those constants should go to Linux pci_regs.h */ #define PCI_MSI_PENDING_32 0x10
@@ -109,6 +110,92 @@ bool msi_enabled(const PCIDevice *dev) PCI_MSI_FLAGS_ENABLE); } +static void kvm_msi_message_from_vector (PCIDevice *dev,unsigned vector, +KVMMsiMessage *kmm) +{ +uint16_t flags =pci_get_word (dev->config +msi_flags_off (dev)); +bool msi64bit = flags &PCI_MSI_FLAGS_64BIT ; +unsigned int nr_vectors =msi_nr_vectors (flags); + + kmm->addr_lo =pci_get_long (dev->config +msi_address_lo_off (dev)); +if (msi64bit) { + kmm->addr_hi =pci_get_long (dev->config +msi_address_hi_off (dev)); + }else { + kmm->addr_hi =0 ; + } + + kmm->data =pci_get_word (dev->config +msi_data_off (dev, msi64bit)); +if (nr_vectors >1 ) { + kmm->data &= ~(nr_vectors -1 ); + kmm->data |= vector; + } +} + +static void kvm_msi_update (PCIDevice *dev) +{ +uint16_t flags =pci_get_word (dev->config +msi_flags_off (dev)); +unsigned int nr_vectors =msi_nr_vectors (flags); +KVMMsiMessage new_entry, *entry; +bool changed = false; +unsigned int vector; +int r; + +for (vector =0 ; vector <32 ; vector++) { + entry = dev->msi_irq_entries + vector; + +if (vector >= nr_vectors) { +if (vector < dev->msi_entries_nr) { +kvm_msi_message_del (entry); + changed = true; + } + }else if (vector >= dev->msi_entries_nr) { +kvm_msi_message_from_vector (dev, vector, entry); + r =kvm_msi_message_add (entry); +if (r) { +fprintf (stderr,"%s: kvm_msi_add failed: %s\n" , __func__, +strerror (-r)); +exit (1 ); + } + changed = true; + }else { +kvm_msi_message_from_vector (dev, vector, &new_entry); + r =kvm_msi_message_update (entry, &new_entry); +if (r <0 ) { +fprintf (stderr,"%s: kvm_update_msi failed: %s\n" , + __func__,strerror (-r)); +exit (1 ); + } +if (r >0 ) { + *entry = new_entry; + changed = true; + } + } + } + dev->msi_entries_nr = nr_vectors; +if (changed) { + r =kvm_commit_irq_routes (); +if (r) { +fprintf (stderr,"%s: kvm_commit_irq_routes failed: %s\n" , __func__, +strerror (-r)); +exit (1 ); + } + } +} + +/* KVM specific MSI helpers */ +static void kvm_msi_free (PCIDevice *dev) +{ +unsigned int vector; + +for (vector =0 ; vector < dev->msi_entries_nr; ++vector) { +kvm_msi_message_del (&dev->msi_irq_entries[vector]); + } +if (dev->msi_entries_nr >0 ) { +kvm_commit_irq_routes (); + } + dev->msi_entries_nr =0 ; +} +int msi_init (struct PCIDevice *dev,uint8_t offset,unsigned int nr_vectors,bool msi64bit,bool msi_per_vector_mask) {
@@ -159,6 +246,12 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,pci_set_long (dev->wmask +msi_mask_off (dev, msi64bit),0xffffffff >> (PCI_MSI_VECTORS_MAX - nr_vectors)); } + +if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { + dev->msi_irq_entries =qemu_malloc (nr_vectors * +sizeof (*dev->msix_irq_entries)); + } +return config_offset; }
@@ -166,6 +259,11 @@ void msi_uninit(struct PCIDevice *dev) {uint16_t flags =pci_get_word (dev->config +msi_flags_off (dev));uint8_t cap_size =msi_cap_sizeof (flags); + +if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { +kvm_msi_free (dev); +qemu_free (dev->msi_irq_entries); + }pci_del_capability (dev,PCI_CAP_ID_MSIX , cap_size);MSI_DEV_PRINTF (dev,"uninit\n" ); }
@@ -175,6 +273,10 @@ void msi_reset(PCIDevice *dev)uint16_t flags;bool msi64bit; +if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { +kvm_msi_free (dev); + } + flags =pci_get_word (dev->config +msi_flags_off (dev)); flags &= ~(PCI_MSI_FLAGS_QSIZE |PCI_MSI_FLAGS_ENABLE ); msi64bit = flags &PCI_MSI_FLAGS_64BIT ;
@@ -224,6 +326,11 @@ void msi_notify(PCIDevice *dev, unsigned int vector)return ; } +if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { +kvm_set_irq (dev->msi_irq_entries[vector].gsi,1 ,NULL ); +return ; + } +if (msi64bit) { address =pci_get_quad (dev->config +msi_address_lo_off (dev)); }else {
@@ -312,6 +419,10 @@ void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len)pci_set_word (dev->config +msi_flags_off (dev), flags); } +if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { +kvm_msi_update (dev); + } +if (!msi_per_vector_mask) {/* if per vector masking isn't supported, there is no pending interrupt. */
hw/pci.h +4 -0
@@ -195,6 +195,10 @@ struct PCIDevice {ram_addr_t rom_offset;uint32_t rom_bar; +/* MSI entries */ +int msi_entries_nr; +struct KVMMsiMessage *msi_irq_entries; +/* How much space does an MSIX table need. */ /* The spec requires giving the table structure * a 4K aligned region all by itself. Align it to
Signature · 1 line
1.7.1
Re: [PATCH 7/7] qemu-kvm: Add in-kernel irqchip support for MSI
Quoted 36 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This adds the required bits to map classic MSI vectors on KVM IRQ routing entries and deliver them via the irqchip if enabled.
Signed-off-by Jan Kiszka <[email protected]>--- hw/msi.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/pci.h | 4 ++ 2 files changed, 115 insertions(+), 0 deletions(-)hw/msi.c +10
-0@@ -20,6 +20,7 @@#include "msi.h" #include "range.h" +#include "kvm.h" /* Eventually those constants should go to Linux pci_regs.h */ #define PCI_MSI_PENDING_32 0x10@@ -109,6 +110,92 @@ bool msi_enabled(const PCIDevice *dev) PCI_MSI_FLAGS_ENABLE); } +static void kvm_msi_message_from_vector (PCIDevice *dev,unsigned vector, +KVMMsiMessage *kmm) +{ +uint16_t flags =pci_get_word (dev->config +msi_flags_off (dev)); +bool msi64bit = flags &PCI_MSI_FLAGS_64BIT ; +unsigned int nr_vectors =msi_nr_vectors (flags); + + kmm->addr_lo =pci_get_long (dev->config +msi_address_lo_off (dev)); +if (msi64bit) {
hw/msi.c bool msi_enabled(const PCIDevice *dev) · line 131
Quoted 10 lines
hw/msi.c +10
-0+ kmm->addr_hi =pci_get_long (dev->config +msi_address_hi_off (dev)); + }else { + kmm->addr_hi =0 ; + } + + kmm->data =pci_get_word (dev->config +msi_data_off (dev, msi64bit)); +if (nr_vectors >1 ) { + kmm->data &= ~(nr_vectors -1 ); + kmm->data |= vector; + }
msi_notify already has similar logic.
Let's add msi_get_vector_address and msi_get_vector_data
hw/msi.c bool msi_enabled(const PCIDevice *dev) · line 143
Quoted 12 lines
hw/msi.c +12
-0+} + +static void kvm_msi_update (PCIDevice *dev) +{ +uint16_t flags =pci_get_word (dev->config +msi_flags_off (dev)); +unsigned int nr_vectors =msi_nr_vectors (flags); +KVMMsiMessage new_entry, *entry; +bool changed = false; +unsigned int vector; +int r; + +for (vector =0 ; vector <32 ; vector++) {
I'd say up to log_max_vecs: the array is allocated with nr_vectors.
Quoted 107 lines
hw/msi.c +75
-0+ entry = dev->msi_irq_entries + vector; + +if (vector >= nr_vectors) { +if (vector < dev->msi_entries_nr) { +kvm_msi_message_del (entry); + changed = true; + } + }else if (vector >= dev->msi_entries_nr) { +kvm_msi_message_from_vector (dev, vector, entry); + r =kvm_msi_message_add (entry); +if (r) { +fprintf (stderr,"%s: kvm_msi_add failed: %s\n" , __func__, +strerror (-r)); +exit (1 ); + } + changed = true; + }else { +kvm_msi_message_from_vector (dev, vector, &new_entry); + r =kvm_msi_message_update (entry, &new_entry); +if (r <0 ) { +fprintf (stderr,"%s: kvm_update_msi failed: %s\n" , + __func__,strerror (-r)); +exit (1 ); + } +if (r >0 ) { + *entry = new_entry; + changed = true; + } + } + } + dev->msi_entries_nr = nr_vectors; +if (changed) { + r =kvm_commit_irq_routes (); +if (r) { +fprintf (stderr,"%s: kvm_commit_irq_routes failed: %s\n" , __func__, +strerror (-r)); +exit (1 ); + } + } +} + +/* KVM specific MSI helpers */ +static void kvm_msi_free (PCIDevice *dev) +{ +unsigned int vector; + +for (vector =0 ; vector < dev->msi_entries_nr; ++vector) { +kvm_msi_message_del (&dev->msi_irq_entries[vector]); + } +if (dev->msi_entries_nr >0 ) { +kvm_commit_irq_routes (); + } + dev->msi_entries_nr =0 ; +} +int msi_init (struct PCIDevice *dev,uint8_t offset,unsigned int nr_vectors,bool msi64bit,bool msi_per_vector_mask) {@@ -159,6 +246,12 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,pci_set_long (dev->wmask +msi_mask_off (dev, msi64bit),0xffffffff >> (PCI_MSI_VECTORS_MAX - nr_vectors)); } + +if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { + dev->msi_irq_entries =qemu_malloc (nr_vectors * +sizeof (*dev->msix_irq_entries)); + } +return config_offset; }@@ -166,6 +259,11 @@ void msi_uninit(struct PCIDevice *dev) {uint16_t flags =pci_get_word (dev->config +msi_flags_off (dev));uint8_t cap_size =msi_cap_sizeof (flags); + +if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { +kvm_msi_free (dev); +qemu_free (dev->msi_irq_entries); + }pci_del_capability (dev,PCI_CAP_ID_MSIX , cap_size);MSI_DEV_PRINTF (dev,"uninit\n" ); }@@ -175,6 +273,10 @@ void msi_reset(PCIDevice *dev)uint16_t flags;bool msi64bit; +if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { +kvm_msi_free (dev); + } + flags =pci_get_word (dev->config +msi_flags_off (dev)); flags &= ~(PCI_MSI_FLAGS_QSIZE |PCI_MSI_FLAGS_ENABLE ); msi64bit = flags &PCI_MSI_FLAGS_64BIT ;@@ -224,6 +326,11 @@ void msi_notify(PCIDevice *dev, unsigned int vector)return ; } +if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { +kvm_set_irq (dev->msi_irq_entries[vector].gsi,1 ,NULL ); +return ; + } +if (msi64bit) { address =pci_get_quad (dev->config +msi_address_lo_off (dev)); }else {
hw/msi.c void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len) · line 425
Quoted 7 lines
hw/msi.c +4
-0@@ -312,6 +419,10 @@ void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len)pci_set_word (dev->config +msi_flags_off (dev), flags); } +if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { +kvm_msi_update (dev); + } +
Does this ignore per vector masking then?
Quoted 20 lines
hw/msi.c +0
-0if (!msi_per_vector_mask) {/* if per vector masking isn't supported, there is no pending interrupt. */ hw/pci.h +4
-0@@ -195,6 +195,10 @@ struct PCIDevice {ram_addr_t rom_offset;uint32_t rom_bar; +/* MSI entries */ +int msi_entries_nr; +struct KVMMsiMessage *msi_irq_entries; +/* How much space does an MSIX table need. */ /* The spec requires giving the table structure * a 4K aligned region all by itself. Align it to Signature · 1 line
1.7.1
Re: [PATCH 7/7] qemu-kvm: Add in-kernel irqchip support for MSI
Quoted 42 lines · Michael S. Tsirkin
Quoted 41 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This adds the required bits to map classic MSI vectors on KVM IRQ routing entries and deliver them via the irqchip if enabled.
Signed-off-by Jan Kiszka <[email protected]>--- hw/msi.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/pci.h | 4 ++ 2 files changed, 115 insertions(+), 0 deletions(-)hw/msi.c +15
-0@@ -20,6 +20,7 @@#include "msi.h" #include "range.h" +#include "kvm.h" /* Eventually those constants should go to Linux pci_regs.h */ #define PCI_MSI_PENDING_32 0x10@@ -109,6 +110,92 @@ bool msi_enabled(const PCIDevice *dev) PCI_MSI_FLAGS_ENABLE); } +static void kvm_msi_message_from_vector (PCIDevice *dev,unsigned vector, +KVMMsiMessage *kmm) +{ +uint16_t flags =pci_get_word (dev->config +msi_flags_off (dev)); +bool msi64bit = flags &PCI_MSI_FLAGS_64BIT ; +unsigned int nr_vectors =msi_nr_vectors (flags); + + kmm->addr_lo =pci_get_long (dev->config +msi_address_lo_off (dev)); +if (msi64bit) { + kmm->addr_hi =pci_get_long (dev->config +msi_address_hi_off (dev)); + }else { + kmm->addr_hi =0 ; + } +
Quoted 9 lines
hw/msi.c bool msi_enabled(const PCIDevice *dev) · line 131
Quoted 5 lines
hw/msi.c +5
-0+ kmm->data =pci_get_word (dev->config +msi_data_off (dev, msi64bit)); +if (nr_vectors >1 ) { + kmm->data &= ~(nr_vectors -1 ); + kmm->data |= vector; + }
msi_notifyalready has similar logic.
Let's addmsi_get_vector_addressandmsi_get_vector_data
Ok.
Quoted 16 lines
hw/msi.c bool msi_enabled(const PCIDevice *dev) · line 143
Quoted 13 lines
hw/msi.c +12
-0+} + +static void kvm_msi_update (PCIDevice *dev) +{ +uint16_t flags =pci_get_word (dev->config +msi_flags_off (dev)); +unsigned int nr_vectors =msi_nr_vectors (flags); +KVMMsiMessage new_entry, *entry; +bool changed = false; +unsigned int vector; +int r; + +for (vector =0 ; vector <32 ; vector++) {I'd say up to
log_max_vecs: the array is allocated withnr_vectors.
Right.
Quoted 111 lines
Quoted 111 lines
hw/msi.c +75
-0+ entry = dev->msi_irq_entries + vector; + +if (vector >= nr_vectors) { +if (vector < dev->msi_entries_nr) { +kvm_msi_message_del (entry); + changed = true; + } + }else if (vector >= dev->msi_entries_nr) { +kvm_msi_message_from_vector (dev, vector, entry); + r =kvm_msi_message_add (entry); +if (r) { +fprintf (stderr,"%s: kvm_msi_add failed: %s\n" , __func__, +strerror (-r)); +exit (1 ); + } + changed = true; + }else { +kvm_msi_message_from_vector (dev, vector, &new_entry); + r =kvm_msi_message_update (entry, &new_entry); +if (r <0 ) { +fprintf (stderr,"%s: kvm_update_msi failed: %s\n" , + __func__,strerror (-r)); +exit (1 ); + } +if (r >0 ) { + *entry = new_entry; + changed = true; + } + } + } + dev->msi_entries_nr = nr_vectors; +if (changed) { + r =kvm_commit_irq_routes (); +if (r) { +fprintf (stderr,"%s: kvm_commit_irq_routes failed: %s\n" , __func__, +strerror (-r)); +exit (1 ); + } + } +} + +/* KVM specific MSI helpers */ +static void kvm_msi_free (PCIDevice *dev) +{ +unsigned int vector; + +for (vector =0 ; vector < dev->msi_entries_nr; ++vector) { +kvm_msi_message_del (&dev->msi_irq_entries[vector]); + } +if (dev->msi_entries_nr >0 ) { +kvm_commit_irq_routes (); + } + dev->msi_entries_nr =0 ; +} +int msi_init (struct PCIDevice *dev,uint8_t offset,unsigned int nr_vectors,bool msi64bit,bool msi_per_vector_mask) {@@ -159,6 +246,12 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,pci_set_long (dev->wmask +msi_mask_off (dev, msi64bit),0xffffffff >> (PCI_MSI_VECTORS_MAX - nr_vectors)); } + +if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { + dev->msi_irq_entries =qemu_malloc (nr_vectors * +sizeof (*dev->msix_irq_entries)); + } +return config_offset; }@@ -166,6 +259,11 @@ void msi_uninit(struct PCIDevice *dev) {uint16_t flags =pci_get_word (dev->config +msi_flags_off (dev));uint8_t cap_size =msi_cap_sizeof (flags); + +if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { +kvm_msi_free (dev); +qemu_free (dev->msi_irq_entries); + }pci_del_capability (dev,PCI_CAP_ID_MSIX , cap_size);MSI_DEV_PRINTF (dev,"uninit\n" ); }@@ -175,6 +273,10 @@ void msi_reset(PCIDevice *dev)uint16_t flags;bool msi64bit; +if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { +kvm_msi_free (dev); + } + flags =pci_get_word (dev->config +msi_flags_off (dev)); flags &= ~(PCI_MSI_FLAGS_QSIZE |PCI_MSI_FLAGS_ENABLE ); msi64bit = flags &PCI_MSI_FLAGS_64BIT ;@@ -224,6 +326,11 @@ void msi_notify(PCIDevice *dev, unsigned int vector)return ; } +if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { +kvm_set_irq (dev->msi_irq_entries[vector].gsi,1 ,NULL ); +return ; + } +if (msi64bit) { address =pci_get_quad (dev->config +msi_address_lo_off (dev)); }else {@@ -312,6 +419,10 @@ void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len)pci_set_word (dev->config +msi_flags_off (dev), flags); }
Quoted 6 lines
hw/msi.c void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len) · line 425
Quoted 4 lines
hw/msi.c +4
-0+if (kvm_enabled () &&kvm_irqchip_in_kernel ()) { +kvm_msi_update (dev); + } +Does this ignore per vector masking then?
The hook has to be placed here independently.
But, yes, I do not consider to optional mask in kvm_msi_update. Not sure if it would buy us anything, but it could still be added on top later on.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux