Skip to contentSkip to conversation

qemu: msi irq allocation api

PATCH In review Michael S. Tsirkin 4 participants 96 messages May 20, 2009 – May 24, 2009

Subsystems (2)
lore
Share & tools
lore

Michael S. Tsirkin

author maintainer
#0 lore raw

PATCH 1/1 qemu: msi irq allocation api

define api for allocating/setting up msi-x irqs, and for updating them with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

---
 hw/apic.c     |    1 -
 hw/ioapic.c   |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/irq.c      |   10 ++++++++
 hw/irq.h      |    5 ++++
 hw/pc.c       |    1 +
 hw/pc.h       |    2 +
 hw/pci.c      |    2 +
 hw/pci.h      |   10 ++++++++
 qemu-common.h |    1 +
 9 files changed, 96 insertions(+), 1 deletions(-)
hw/apic.c +0 -1
index d63d74b..2d2de69 100644
@@ -929,4 +929,3 @@ int apic_init(CPUState *env)
     local_apics[s->id] = s;
     return 0;
 }
-
hw/ioapic.c +65 -0

86 lines not on this page · raw message

hw/irq.c +10 -0
index 7703f62..9180381 100644
@@ -75,3 +75,13 @@ qemu_irq qemu_irq_invert(qemu_irq irq)
     qemu_irq_raise(irq);
     return qemu_allocate_irqs(qemu_notirq, irq, 1)[0];
 }
+
+void *qemu_irq_get_opaque(qemu_irq irq)
+{
+    return irq->opaque;
+}
+
+int qemu_irq_get_vector(qemu_irq irq)
+{
+    return irq->n;
+}
hw/irq.h +5 -0
index 5daae44..0e3144d 100644
@@ -32,4 +32,9 @@ void qemu_free_irqs(qemu_irq *s);
 /* Returns a new IRQ with opposite polarity.  */
 qemu_irq qemu_irq_invert(qemu_irq irq);
 
+/* Get the pointer stored in the irq. */
+void *qemu_irq_get_opaque(qemu_irq irq);
+
+/* Get vector stored in the irq */
+int qemu_irq_get_vector(qemu_irq irq);
 #endif
hw/pc.c +1 -0
index 61f6e7b..1b287c3 100644
@@ -962,6 +962,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
     if (pci_enabled) {
         pci_bus = i440fx_init(&i440fx_state, i8259);
         piix3_devfn = piix3_init(pci_bus, -1);
+        pci_msi_ops = &ioapic_msi_ops;
     } else {
         pci_bus = NULL;
     }
hw/pc.h +2 -0
index 50e6c39..2013aa9 100644
@@ -55,6 +55,8 @@ void ioapic_set_irq(void *opaque, int vector, int level);
 void apic_reset_irq_delivered(void);
 int apic_get_irq_delivered(void);
 
+extern struct pci_msi_ops ioapic_msi_ops;
+
 /* i8254.c */
 
 #define PIT_FREQ 1193182
hw/pci.c +2 -0
index b8186f6..cd453c9 100644
@@ -57,6 +57,8 @@ static uint16_t pci_default_sub_device_id = PCI_SUBDEVICE_ID_QEMU;
 static int pci_irq_index;
 static PCIBus *first_bus;
 
+struct pci_msi_ops *pci_msi_ops;
+
 static void pcibus_save(QEMUFile *f, void *opaque)
 {
     PCIBus *bus = (PCIBus *)opaque;
hw/pci.h +10 -0
index a629e60..8883f08 100644
@@ -280,4 +280,14 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
 PCIBus *sh_pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
                             qemu_irq *pic, int devfn_min, int nirq);
 
+/* MSI/MSI-X */
+
+struct pci_msi_ops {
+    qemu_irq *(*allocate)(int nentries);
+    int (*update)(qemu_irq, uint64_t addr, uint32_t data, int masked);
+    void (*free)(qemu_irq *);
+};
+
+extern struct pci_msi_ops *pci_msi_ops;
+
 #endif
qemu-common.h +1 -0
index c90c3e3..d5a1112 100644
@@ -178,6 +178,7 @@ typedef struct PCIDevice PCIDevice;
 typedef struct SerialState SerialState;
 typedef struct IRQState *qemu_irq;
 struct pcmcia_card_s;
+struct pci_msi_ops;
 
 /* CPU save/load.  */
 void cpu_save(QEMUFile *f, void *opaque);
Signature · 1 line
1.6.3.1.56.g79e1.dirty
#1 lore raw
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

For example, irq.c could support opaque packet payload of unspecified/predefined size. MSI packet structure should be defined in ioapic.c.

The pci_msi_ops structure could be 'const', or do you expect it to change during execution?

#2 lore raw
In reply to Michael S. Tsirkin
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

For example, irq.c could support opaque packet payload of unspecified/predefined size. MSI packet structure should be defined in ioapic.c.

The pci_msi_ops structure could be 'const', or do you expect it to change during execution?

#3 lore raw
Quoted 5 lines · Blue Swirl

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Is the packet structure visible to software?

Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
#4 lore raw
In reply to Blue Swirl
Quoted 5 lines · Blue Swirl

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Is the packet structure visible to software?

Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
#5 lore raw
Quoted 10 lines · Avi Kivity
Quoted 6 lines · Blue Swirl

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Is the packet structure visible to software?

Yes, and the content of the packets is defined by software. If there is framing outside the user data (I don't know), this is not visible.

#6 lore raw
In reply to Avi Kivity
Quoted 10 lines · Avi Kivity
Quoted 6 lines · Blue Swirl

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Is the packet structure visible to software?

Yes, and the content of the packets is defined by software. If there is framing outside the user data (I don't know), this is not visible.

author maintainer
#7 lore raw
In reply to Blue Swirl
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has MSI, not "mondos". What code would benefit from this abstraction?

Quoted 3 lines

For example, irq.c could support opaque packet payload of unspecified/predefined size. MSI packet structure should be defined in ioapic.c.

Note that MSI does not have packets and MSI interrupts do not pass any payload.

Quoted 2 lines

The pci_msi_ops structure could be 'const', or do you expect it to change during execution?

Right. I'll fix that.

Signature · 1 line
MST
author maintainer
#8 lore raw
In reply to Blue Swirl
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has MSI, not "mondos". What code would benefit from this abstraction?

Quoted 3 lines

For example, irq.c could support opaque packet payload of unspecified/predefined size. MSI packet structure should be defined in ioapic.c.

Note that MSI does not have packets and MSI interrupts do not pass any payload.

Quoted 2 lines

The pci_msi_ops structure could be 'const', or do you expect it to change during execution?

Right. I'll fix that.

Signature · 1 line
MST
#9 lore raw
Quoted 9 lines · Michael S. Tsirkin
Quoted 8 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Quoted 7 lines
Quoted 3 lines

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Sparc64 emulation, of course. I think also the API would be neater.

Quoted 6 lines
Quoted 3 lines

For example, irq.c could support opaque packet payload of unspecified/predefined size. MSI packet structure should be defined in ioapic.c.

Note that MSI does not have packets and MSI interrupts do not pass any payload.

I don't know too much about MSI, what's the 'data' field in msi_state then?

author maintainer
#10 lore raw
Quoted 20 lines · Blue Swirl
Quoted 9 lines · Michael S. Tsirkin
Quoted 8 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Quoted 7 lines
Quoted 3 lines

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Sparc64 emulation, of course. I think also the API would be neater.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

For example, irq.c could support opaque packet payload of unspecified/predefined size. MSI packet structure should be defined in ioapic.c.

Note that MSI does not have packets and MSI interrupts do not pass any payload.

I don't know too much about MSI, what's the 'data' field in msi_state then?

opaque stuff that apic uses to select irq and cpu.

Signature · 1 line
MST
author maintainer
#11 lore raw
In reply to Blue Swirl
Quoted 20 lines · Blue Swirl
Quoted 9 lines · Michael S. Tsirkin
Quoted 8 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Quoted 7 lines
Quoted 3 lines

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Sparc64 emulation, of course. I think also the API would be neater.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

For example, irq.c could support opaque packet payload of unspecified/predefined size. MSI packet structure should be defined in ioapic.c.

Note that MSI does not have packets and MSI interrupts do not pass any payload.

I don't know too much about MSI, what's the 'data' field in msi_state then?

opaque stuff that apic uses to select irq and cpu.

Signature · 1 line
MST
author maintainer
#12 lore raw
In reply to Blue Swirl
Quoted 15 lines · Blue Swirl
Quoted 14 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Quoted 4 lines
Quoted 2 lines

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

Signature · 1 line
MST
#13 lore raw
Quoted 16 lines · Michael S. Tsirkin
Quoted 15 lines · Blue Swirl
Quoted 14 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

#14 lore raw
In reply to Michael S. Tsirkin
Quoted 16 lines · Michael S. Tsirkin
Quoted 15 lines · Blue Swirl
Quoted 14 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

author maintainer
#15 lore raw
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

I'm sorry, I don't really know anything about sparc. All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

Signature · 1 line
MST
author maintainer
#16 lore raw
In reply to Blue Swirl
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

I'm sorry, I don't really know anything about sparc. All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

Signature · 1 line
MST
author maintainer
#17 lore raw
Quoted 29 lines · Michael S. Tsirkin
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

Quoted 7 lines

I'm sorry, I don't really know anything about sparc. All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

Just to clarify, the main difference is that with MSI/MSI-X both data and address fields are mostly static, modifying them involves ioapic and device updates which might be an expensive operation (e.g. with kvm, needs an extra system call).

So I don't think it makes sense to pass MSI-X data field with each call to qemu_set_irq.

Signature · 1 line
MST
#18 lore raw
In reply to Michael S. Tsirkin
Quoted 30 lines · Michael S. Tsirkin
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

Quoted 7 lines

I'm sorry, I don't really know anything about sparc.
All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

But maybe this API would be useful for both MSI and mondo packets:
/* Get vector data stored in the irq */
void *qemu_irq_get_data(qemu_irq irq);

/* Set vector data stored in the irq */
void qemu_irq_set_data(qemu_irq irq, void *data);

You'd use a struct msi_state as the opaque (not using the array indexed by irq number), I'd use some kind of
struct mondo {
uint64_t packet[8];
};

Instead of ioapic_update_msi(), you'd call qemu_irq_set_data().

author maintainer
#19 lore raw
Quoted 35 lines · Blue Swirl
Quoted 34 lines · Michael S. Tsirkin
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

I'm sorry, I don't really know anything about sparc.
All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

Quoted 7 lines
Quoted 3 lines

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

But maybe this API would be useful for both MSI and mondo packets:
/* Get vector data stored in the irq */
void *qemu_irq_get_data(qemu_irq irq);

Hmm. Yes, it would work for MSI.
However I am thinking ahead to kvm support, which needs to update the irqchip (with a system call) on each state update. So
- I really need a callback on data update
- data update might fail

Which if added to the generic API would make it too complex, IMO.

Quoted 9 lines

/* Set vector data stored in the irq */
void qemu_irq_set_data(qemu_irq irq, void *data); You'd use a struct msi_state as the opaque (not using the array indexed by irq number), I'd use some kind of
struct mondo {
uint64_t packet[8];
};

Instead of ioapic_update_msi(), you'd call qemu_irq_set_data().

I guess it's a matter of taste, personally I prefer type-safe functions to hiding everything in typeless void* buffers.

Signature · 1 line
MST
author maintainer
#20 lore raw
In reply to Blue Swirl
Quoted 35 lines · Blue Swirl
Quoted 34 lines · Michael S. Tsirkin
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

I'm sorry, I don't really know anything about sparc.
All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

Quoted 7 lines
Quoted 3 lines

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

But maybe this API would be useful for both MSI and mondo packets:
/* Get vector data stored in the irq */
void *qemu_irq_get_data(qemu_irq irq);

Hmm. Yes, it would work for MSI.
However I am thinking ahead to kvm support, which needs to update the irqchip (with a system call) on each state update. So
- I really need a callback on data update
- data update might fail

Which if added to the generic API would make it too complex, IMO.

Quoted 9 lines

/* Set vector data stored in the irq */
void qemu_irq_set_data(qemu_irq irq, void *data); You'd use a struct msi_state as the opaque (not using the array indexed by irq number), I'd use some kind of
struct mondo {
uint64_t packet[8];
};

Instead of ioapic_update_msi(), you'd call qemu_irq_set_data().

I guess it's a matter of taste, personally I prefer type-safe functions to hiding everything in typeless void* buffers.

Signature · 1 line
MST
#21 lore raw
In reply to Michael S. Tsirkin
Quoted 30 lines · Michael S. Tsirkin
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

Quoted 7 lines

I'm sorry, I don't really know anything about sparc.
All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

But maybe this API would be useful for both MSI and mondo packets:
/* Get vector data stored in the irq */
void *qemu_irq_get_data(qemu_irq irq);

/* Set vector data stored in the irq */
void qemu_irq_set_data(qemu_irq irq, void *data);

You'd use a struct msi_state as the opaque (not using the array indexed by irq number), I'd use some kind of
struct mondo {
uint64_t packet[8];
};

Instead of ioapic_update_msi(), you'd call qemu_irq_set_data().

author maintainer
#22 lore raw
In reply to Michael S. Tsirkin
Quoted 29 lines · Michael S. Tsirkin
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

Quoted 7 lines

I'm sorry, I don't really know anything about sparc. All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

Just to clarify, the main difference is that with MSI/MSI-X both data and address fields are mostly static, modifying them involves ioapic and device updates which might be an expensive operation (e.g. with kvm, needs an extra system call).

So I don't think it makes sense to pass MSI-X data field with each call to qemu_set_irq.

Signature · 1 line
MST
#23 lore raw
Quoted 39 lines · Michael S. Tsirkin
Quoted 29 lines · Michael S. Tsirkin
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

Quoted 7 lines

I'm sorry, I don't really know anything about sparc. All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

Quoted 7 lines

Just to clarify, the main difference is that with MSI/MSI-X
both data and address fields are mostly static, modifying them involves ioapic and device updates which might be an expensive operation (e.g. with kvm, needs an extra system call).

So I don't think it makes sense to pass MSI-X data field with each call to qemu_set_irq.

No, but I think the Sparc situation is the same, the packet data is static for the interrupt source in question.

author maintainer
#24 lore raw
Quoted 45 lines · Blue Swirl
Quoted 44 lines · Michael S. Tsirkin
Quoted 29 lines · Michael S. Tsirkin
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

Quoted 7 lines

I'm sorry, I don't really know anything about sparc. All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

Just to clarify, the main difference is that with MSI/MSI-X
both data and address fields are mostly static, modifying them involves ioapic and device updates which might be an expensive operation (e.g. with kvm, needs an extra system call).

Quoted 5 lines
Quoted 2 lines

So I don't think it makes sense to pass MSI-X data field with each call to qemu_set_irq.

No, but I think the Sparc situation is the same, the packet data is static for the interrupt source in question.

So, ok, we could add data update callback and then MSI and sparc would do their thing there. I'm not convinced I like all this play with untyped buffers, do you think it's helpful?

If yes, maybe I'll try to code it up and see how does it look.

Signature · 1 line
MST
author maintainer
#25 lore raw
In reply to Blue Swirl
Quoted 45 lines · Blue Swirl
Quoted 44 lines · Michael S. Tsirkin
Quoted 29 lines · Michael S. Tsirkin
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

Quoted 7 lines

I'm sorry, I don't really know anything about sparc. All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

Just to clarify, the main difference is that with MSI/MSI-X
both data and address fields are mostly static, modifying them involves ioapic and device updates which might be an expensive operation (e.g. with kvm, needs an extra system call).

Quoted 5 lines
Quoted 2 lines

So I don't think it makes sense to pass MSI-X data field with each call to qemu_set_irq.

No, but I think the Sparc situation is the same, the packet data is static for the interrupt source in question.

So, ok, we could add data update callback and then MSI and sparc would do their thing there. I'm not convinced I like all this play with untyped buffers, do you think it's helpful?

If yes, maybe I'll try to code it up and see how does it look.

Signature · 1 line
MST
#26 lore raw
Quoted 53 lines · Michael S. Tsirkin
Quoted 45 lines · Blue Swirl
Quoted 44 lines · Michael S. Tsirkin
Quoted 29 lines · Michael S. Tsirkin
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

Quoted 7 lines

I'm sorry, I don't really know anything about sparc. All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

Just to clarify, the main difference is that with MSI/MSI-X
both data and address fields are mostly static, modifying them involves ioapic and device updates which might be an expensive operation (e.g. with kvm, needs an extra system call).

Quoted 5 lines
Quoted 2 lines

So I don't think it makes sense to pass MSI-X data field with each call to qemu_set_irq.

No, but I think the Sparc situation is the same, the packet data is static for the interrupt source in question.

Quoted 5 lines

So, ok, we could add data update callback and then MSI and sparc
would do their thing there. I'm not convinced I like all this play with untyped buffers, do you think it's helpful?

If yes, maybe I'll try to code it up and see how does it look.

Well, get/set_data could be limited to irq.c, ioapic.c could export something like get/set_msi_data. MSI callers should only use get/set_msi_data. Ditto for get/set_mondo_data.

#27 lore raw
In reply to Michael S. Tsirkin
Quoted 53 lines · Michael S. Tsirkin
Quoted 45 lines · Blue Swirl
Quoted 44 lines · Michael S. Tsirkin
Quoted 29 lines · Michael S. Tsirkin
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

Quoted 7 lines

I'm sorry, I don't really know anything about sparc. All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

Just to clarify, the main difference is that with MSI/MSI-X
both data and address fields are mostly static, modifying them involves ioapic and device updates which might be an expensive operation (e.g. with kvm, needs an extra system call).

Quoted 5 lines
Quoted 2 lines

So I don't think it makes sense to pass MSI-X data field with each call to qemu_set_irq.

No, but I think the Sparc situation is the same, the packet data is static for the interrupt source in question.

Quoted 5 lines

So, ok, we could add data update callback and then MSI and sparc
would do their thing there. I'm not convinced I like all this play with untyped buffers, do you think it's helpful?

If yes, maybe I'll try to code it up and see how does it look.

Well, get/set_data could be limited to irq.c, ioapic.c could export something like get/set_msi_data. MSI callers should only use get/set_msi_data. Ditto for get/set_mondo_data.

author maintainer
#28 lore raw
Quoted 58 lines · Blue Swirl
Quoted 57 lines · Michael S. Tsirkin
Quoted 45 lines · Blue Swirl
Quoted 44 lines · Michael S. Tsirkin
Quoted 29 lines · Michael S. Tsirkin
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

Quoted 7 lines

I'm sorry, I don't really know anything about sparc. All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

Just to clarify, the main difference is that with MSI/MSI-X
both data and address fields are mostly static, modifying them involves ioapic and device updates which might be an expensive operation (e.g. with kvm, needs an extra system call).

Quoted 5 lines
Quoted 2 lines

So I don't think it makes sense to pass MSI-X data field with each call to qemu_set_irq.

No, but I think the Sparc situation is the same, the packet data is static for the interrupt source in question.

So, ok, we could add data update callback and then MSI and sparc
would do their thing there. I'm not convinced I like all this play with untyped buffers, do you think it's helpful?

Quoted 5 lines
Quoted 1 line

If yes, maybe I'll try to code it up and see how does it look.

Well, get/set_data could be limited to irq.c, ioapic.c could export something like get/set_msi_data. MSI callers should only use get/set_msi_data. Ditto for get/set_mondo_data.

I started coding it up and got lost in a maze of callbacks and void pointers :)

And I also remembered that besides data, there's a mask and pending bits in msi (it's ignored in the patch I sent, but I'm fixing that), so there needs to be an interface for this as well.

I am guessing that using qemu_irq_get_opaque and qemu_irq_get_vector that I added, you'll find it's easy to implement mondos in very few lines of code. We can always add another level of indirection later when sparc code surfaces, if we find it helps.

Signature · 1 line
MST
author maintainer
#29 lore raw
In reply to Blue Swirl
Quoted 58 lines · Blue Swirl
Quoted 57 lines · Michael S. Tsirkin
Quoted 45 lines · Blue Swirl
Quoted 44 lines · Michael S. Tsirkin
Quoted 29 lines · Michael S. Tsirkin
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

Quoted 7 lines

I'm sorry, I don't really know anything about sparc. All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

Just to clarify, the main difference is that with MSI/MSI-X
both data and address fields are mostly static, modifying them involves ioapic and device updates which might be an expensive operation (e.g. with kvm, needs an extra system call).

Quoted 5 lines
Quoted 2 lines

So I don't think it makes sense to pass MSI-X data field with each call to qemu_set_irq.

No, but I think the Sparc situation is the same, the packet data is static for the interrupt source in question.

So, ok, we could add data update callback and then MSI and sparc
would do their thing there. I'm not convinced I like all this play with untyped buffers, do you think it's helpful?

Quoted 5 lines
Quoted 1 line

If yes, maybe I'll try to code it up and see how does it look.

Well, get/set_data could be limited to irq.c, ioapic.c could export something like get/set_msi_data. MSI callers should only use get/set_msi_data. Ditto for get/set_mondo_data.

I started coding it up and got lost in a maze of callbacks and void pointers :)

And I also remembered that besides data, there's a mask and pending bits in msi (it's ignored in the patch I sent, but I'm fixing that), so there needs to be an interface for this as well.

I am guessing that using qemu_irq_get_opaque and qemu_irq_get_vector that I added, you'll find it's easy to implement mondos in very few lines of code. We can always add another level of indirection later when sparc code surfaces, if we find it helps.

Signature · 1 line
MST
#30 lore raw
In reply to Michael S. Tsirkin
Quoted 39 lines · Michael S. Tsirkin
Quoted 29 lines · Michael S. Tsirkin
Quoted 20 lines · Blue Swirl
Quoted 19 lines · Michael S. Tsirkin
Quoted 18 lines · Blue Swirl
Quoted 16 lines · Michael S. Tsirkin
Quoted 11 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

Sparc64 emulation, of course. I think also the API would be neater.

Since "mondos" are not interrupts, why use irqs for them?

I just said above that they are used for interrupt vector data. What makes you think they are not interrupts?

Quoted 7 lines

I'm sorry, I don't really know anything about sparc. All I am saying is that in PCI, interrupts never pass data, so qemu_set_irq as it is now, is a good API to send them.

For the sparc feature you describe, you probably want to add a message data parameter to qemu_set_irq, but it's not really useful for MSI.

Quoted 7 lines

Just to clarify, the main difference is that with MSI/MSI-X
both data and address fields are mostly static, modifying them involves ioapic and device updates which might be an expensive operation (e.g. with kvm, needs an extra system call).

So I don't think it makes sense to pass MSI-X data field with each call to qemu_set_irq.

No, but I think the Sparc situation is the same, the packet data is static for the interrupt source in question.

#31 lore raw
In reply to Michael S. Tsirkin
Quoted 9 lines · Michael S. Tsirkin
Quoted 8 lines · Blue Swirl
Quoted 6 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them
with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

Quoted 7 lines
Quoted 3 lines

Sparc64 also uses packets ("mondos", not implemented yet) for interrupt vector data, there the packet size is 8 * 64 bits. I think we should aim for a more generic API that covers this case also.

Are you sure this is a good idea? MSI is tied to PCI, and PCI only has
MSI, not "mondos". What code would benefit from this abstraction?

Sparc64 emulation, of course. I think also the API would be neater.

Quoted 6 lines
Quoted 3 lines

For example, irq.c could support opaque packet payload of unspecified/predefined size. MSI packet structure should be defined in ioapic.c.

Note that MSI does not have packets and MSI interrupts do not pass any payload.

I don't know too much about MSI, what's the 'data' field in msi_state then?

#32 lore raw
In reply to Michael S. Tsirkin
Quoted 4 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

I though the point of MSI is that they are just a regular memory writes, and don't require any special bus support.

Paul

#33 lore raw
In reply to Michael S. Tsirkin
Quoted 4 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

I though the point of MSI is that they are just a regular memory writes, and don't require any special bus support.

Paul

#34 lore raw
Quoted 11 lines · Paul Brook
Quoted 5 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

I though the point of MSI is that they are just a regular memory writes, and don't require any special bus support.

The PCI bus doesn't need any special support (I think) but something on the other end needs to interpret those writes.

In any case we need some internal API for this, and qemu_irq looks like a good choice.

Signature · 1 line
error compiling committee.c: too many arguments to function
#35 lore raw
In reply to Paul Brook
Quoted 10 lines · Paul Brook
Quoted 5 lines · Michael S. Tsirkin

define api for allocating/setting up msi-x irqs, and for updating them with msi-x vector information, supply implementation in ioapic. Please comment on this API: I intend to port my msi-x patch to work on top of it.

I though the point of MSI is that they are just a regular memory writes, and don't require any special bus support.

The PCI bus doesn't need any special support (I think) but something on the other end needs to interpret those writes.

In any case we need some internal API for this, and qemu_irq looks like a good choice.

Signature · 1 line
error compiling committee.c: too many arguments to function
#36 lore raw
Quoted 2 lines

The PCI bus doesn't need any special support (I think) but something on the other end needs to interpret those writes.

Sure. But there's definitely nothing PCI specific about it. I assumed this would all be contained within the APIC.

Quoted 2 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

Paul

#37 lore raw
In reply to Avi Kivity
Quoted 2 lines

The PCI bus doesn't need any special support (I think) but something on the other end needs to interpret those writes.

Sure. But there's definitely nothing PCI specific about it. I assumed this would all be contained within the APIC.

Quoted 2 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

Paul

author maintainer
#38 lore raw
Quoted 5 lines · Paul Brook
Quoted 2 lines

The PCI bus doesn't need any special support (I think) but something on the other end needs to interpret those writes.

Sure. But there's definitely nothing PCI specific about it. I assumed this would all be contained within the APIC.

Exactly. APIC supplies callbacks to set up/free/mask/unmask MSI interrupts. For kvm, we'll have another implementation that passes these requests on to kernel.

Quoted 6 lines
Quoted 2 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

Paul

emulated PCI devices such as virtio.
Hope to send a patch shortly.

Signature · 1 line
MST
author maintainer
#39 lore raw
In reply to Paul Brook
Quoted 5 lines · Paul Brook
Quoted 2 lines

The PCI bus doesn't need any special support (I think) but something on the other end needs to interpret those writes.

Sure. But there's definitely nothing PCI specific about it. I assumed this would all be contained within the APIC.

Exactly. APIC supplies callbacks to set up/free/mask/unmask MSI interrupts. For kvm, we'll have another implementation that passes these requests on to kernel.

Quoted 6 lines
Quoted 2 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

Paul

emulated PCI devices such as virtio.
Hope to send a patch shortly.

Signature · 1 line
MST
#40 lore raw
In reply to Paul Brook
Quoted 7 lines · Paul Brook
Quoted 3 lines

The PCI bus doesn't need any special support (I think) but something on the other end needs to interpret those writes.

Sure. But there's definitely nothing PCI specific about it. I assumed this would all be contained within the APIC.

MSIs are defined by PCI and their configuration is done using the PCI configuration space.

Quoted 6 lines
Quoted 3 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

virtio, emulated devices capable of supporting MSI (e1000?), device assignment (not yet in qemu.git).

Signature · 1 line
error compiling committee.c: too many arguments to function
#41 lore raw
In reply to Paul Brook
Quoted 6 lines · Paul Brook
Quoted 3 lines

The PCI bus doesn't need any special support (I think) but something on the other end needs to interpret those writes.

Sure. But there's definitely nothing PCI specific about it. I assumed this would all be contained within the APIC.

MSIs are defined by PCI and their configuration is done using the PCI configuration space.

Quoted 6 lines
Quoted 3 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

virtio, emulated devices capable of supporting MSI (e1000?), device assignment (not yet in qemu.git).

Signature · 1 line
error compiling committee.c: too many arguments to function
#42 lore raw
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

The PCI bus doesn't need any special support (I think) but something on the other end needs to interpret those writes.

Sure. But there's definitely nothing PCI specific about it. I assumed this would all be contained within the APIC.

MSIs are defined by PCI and their configuration is done using the PCI configuration space.

A MSI is just a regular memory write, and the PCI spec explicitly states that a target (e.g. the APIC) is unable to distinguish between a MSI and any other write. The PCI config bits just provide a way of telling the device where/what to write.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

virtio, emulated devices capable of supporting MSI (e1000?), device assignment (not yet in qemu.git).

It probably makes sense to have common infrastructure in pci.c to expose/implement device side MSI functionality. However I see no need for a direct API between the device and the APIC. We already have an API for memory accesses and MMIO regions. I'm pretty sure a system could implement MSI by pointing the device at system ram, and having the CPU periodically poll that.

Paul

#43 lore raw
In reply to Avi Kivity
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

The PCI bus doesn't need any special support (I think) but something on the other end needs to interpret those writes.

Sure. But there's definitely nothing PCI specific about it. I assumed this would all be contained within the APIC.

MSIs are defined by PCI and their configuration is done using the PCI configuration space.

A MSI is just a regular memory write, and the PCI spec explicitly states that a target (e.g. the APIC) is unable to distinguish between a MSI and any other write. The PCI config bits just provide a way of telling the device where/what to write.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

virtio, emulated devices capable of supporting MSI (e1000?), device assignment (not yet in qemu.git).

It probably makes sense to have common infrastructure in pci.c to expose/implement device side MSI functionality. However I see no need for a direct API between the device and the APIC. We already have an API for memory accesses and MMIO regions. I'm pretty sure a system could implement MSI by pointing the device at system ram, and having the CPU periodically poll that.

Paul

#44 lore raw
Quoted 9 lines · Paul Brook
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

virtio, emulated devices capable of supporting MSI (e1000?), device assignment (not yet in qemu.git).

Quoted 6 lines

It probably makes sense to have common infrastructure in pci.c to expose/implement device side MSI functionality. However I see no need for a direct API between the device and the APIC. We already have an API for memory accesses and MMIO regions. I'm pretty sure a system could implement MSI by pointing the device at system ram, and having the CPU periodically poll that.

Instead of writing directly, let's abstract it behind a qemu_set_irq(). This is easier for device authors. The default implementation of the irq callback could write to apic memory, while for kvm we can directly trigger the interrupt via the kvm APIs.

Signature · 1 line
error compiling committee.c: too many arguments to function
#45 lore raw
In reply to Paul Brook
Quoted 11 lines · Paul Brook
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

virtio, emulated devices capable of supporting MSI (e1000?), device assignment (not yet in qemu.git).

It probably makes sense to have common infrastructure in pci.c to expose/implement device side MSI functionality. However I see no need for a direct API between the device and the APIC. We already have an API for memory accesses and MMIO regions. I'm pretty sure a system could implement MSI by pointing the device at system ram, and having the CPU periodically poll that.

Instead of writing directly, let's abstract it behind a qemu_set_irq(). This is easier for device authors. The default implementation of the irq callback could write to apic memory, while for kvm we can directly trigger the interrupt via the kvm APIs.

Signature · 1 line
error compiling committee.c: too many arguments to function
author maintainer
#46 lore raw
Quoted 18 lines · Avi Kivity
Quoted 9 lines · Paul Brook
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

virtio, emulated devices capable of supporting MSI (e1000?), device assignment (not yet in qemu.git).

Quoted 7 lines

It probably makes sense to have common infrastructure in pci.c to expose/implement device side MSI functionality. However I see no need for a direct API between the device and the APIC. We already have an API for memory accesses and MMIO regions. I'm pretty sure a system could implement MSI by pointing the device at system ram, and having the CPU periodically poll that.

Quoted 4 lines

Instead of writing directly, let's abstract it behind a qemu_set_irq(). This is easier for device authors. The default implementation of the irq callback could write to apic memory, while for kvm we can directly trigger the interrupt via the kvm APIs.

Right.

Signature · 1 line
MST
author maintainer
#47 lore raw
In reply to Avi Kivity
Quoted 18 lines · Avi Kivity
Quoted 9 lines · Paul Brook
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

virtio, emulated devices capable of supporting MSI (e1000?), device assignment (not yet in qemu.git).

Quoted 7 lines

It probably makes sense to have common infrastructure in pci.c to expose/implement device side MSI functionality. However I see no need for a direct API between the device and the APIC. We already have an API for memory accesses and MMIO regions. I'm pretty sure a system could implement MSI by pointing the device at system ram, and having the CPU periodically poll that.

Quoted 4 lines

Instead of writing directly, let's abstract it behind a qemu_set_irq(). This is easier for device authors. The default implementation of the irq callback could write to apic memory, while for kvm we can directly trigger the interrupt via the kvm APIs.

Right.

Signature · 1 line
MST
#48 lore raw
In reply to Avi Kivity
Quoted 16 lines · Avi Kivity
Quoted 8 lines · Paul Brook
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

virtio, emulated devices capable of supporting MSI (e1000?), device assignment (not yet in qemu.git).

Quoted 6 lines

It probably makes sense to have common infrastructure in pci.c to expose/implement device side MSI functionality. However I see no need for a direct API between the device and the APIC. We already have an API for memory accesses and MMIO regions. I'm pretty sure a system could implement MSI by pointing the device at system ram, and having the CPU periodically poll that.

Quoted 4 lines

Instead of writing directly, let's abstract it behind a qemu_set_irq(). This is easier for device authors. The default implementation of the irq callback could write to apic memory, while for kvm we can directly trigger the interrupt via the kvm APIs.

I'm still not convinced.

A tight coupling between PCI devices and the APIC is just going to cause us problems later one. I'm going to come back to the fact that these are memory writes so once we get IOMMU support they will presumably be subject to remapping by that, just like any other memory access.

Even ignoring that, qemu_irq isn't really the right interface. A MSI is a one- off event, not a level state. OTOH stl_phys is exactly the right interface.

The KVM interface should be contained within the APIC implementation.

Paul

#49 lore raw
In reply to Avi Kivity
Quoted 16 lines · Avi Kivity
Quoted 8 lines · Paul Brook
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

virtio, emulated devices capable of supporting MSI (e1000?), device assignment (not yet in qemu.git).

Quoted 6 lines

It probably makes sense to have common infrastructure in pci.c to expose/implement device side MSI functionality. However I see no need for a direct API between the device and the APIC. We already have an API for memory accesses and MMIO regions. I'm pretty sure a system could implement MSI by pointing the device at system ram, and having the CPU periodically poll that.

Quoted 4 lines

Instead of writing directly, let's abstract it behind a qemu_set_irq(). This is easier for device authors. The default implementation of the irq callback could write to apic memory, while for kvm we can directly trigger the interrupt via the kvm APIs.

I'm still not convinced.

A tight coupling between PCI devices and the APIC is just going to cause us problems later one. I'm going to come back to the fact that these are memory writes so once we get IOMMU support they will presumably be subject to remapping by that, just like any other memory access.

Even ignoring that, qemu_irq isn't really the right interface. A MSI is a one- off event, not a level state. OTOH stl_phys is exactly the right interface.

The KVM interface should be contained within the APIC implementation.

Paul

#50 lore raw
Quoted 6 lines · Paul Brook
Quoted 5 lines

Instead of writing directly, let's abstract it behind a qemu_set_irq(). This is easier for device authors. The default implementation of the irq callback could write to apic memory, while for kvm we can directly trigger the interrupt via the kvm APIs.

Quoted 7 lines

I'm still not convinced.

A tight coupling between PCI devices and the APIC is just going to cause us problems later one. I'm going to come back to the fact that these are memory writes so once we get IOMMU support they will presumably be subject to remapping by that, just like any other memory access.

I'm not suggesting the qemu_irq will extend all the way to the apic. Think of it as connecting the device core with its interrupt unit.

Quoted 3 lines

Even ignoring that, qemu_irq isn't really the right interface. A MSI is a one- off event, not a level state. OTOH stl_phys is exactly the right interface.

The qemu_irq callback should do an stl_phys(). The device is happy since it's using the same API it uses for non-MSI. The APIC is happy since it isn't connected directly to the device. stl_phys() is happy since it sees more traffic and can serve more ads. kvm is happy since it can hijack the callback to throw the interrupt directly into the kernel.

Quoted 2 lines

The KVM interface should be contained within the APIC implementation.

Tricky, but doable.

Signature · 1 line
error compiling committee.c: too many arguments to function
#51 lore raw
In reply to Paul Brook
Quoted 10 lines · Paul Brook
Quoted 5 lines

Instead of writing directly, let's abstract it behind a qemu_set_irq(). This is easier for device authors. The default implementation of the irq callback could write to apic memory, while for kvm we can directly trigger the interrupt via the kvm APIs.

I'm still not convinced.

A tight coupling between PCI devices and the APIC is just going to cause us problems later one. I'm going to come back to the fact that these are memory writes so once we get IOMMU support they will presumably be subject to remapping by that, just like any other memory access.

I'm not suggesting the qemu_irq will extend all the way to the apic. Think of it as connecting the device core with its interrupt unit.

Quoted 3 lines

Even ignoring that, qemu_irq isn't really the right interface. A MSI is a one- off event, not a level state. OTOH stl_phys is exactly the right interface.

The qemu_irq callback should do an stl_phys(). The device is happy since it's using the same API it uses for non-MSI. The APIC is happy since it isn't connected directly to the device. stl_phys() is happy since it sees more traffic and can serve more ads. kvm is happy since it can hijack the callback to throw the interrupt directly into the kernel.

Quoted 2 lines

The KVM interface should be contained within the APIC implementation.

Tricky, but doable.

Signature · 1 line
error compiling committee.c: too many arguments to function
author maintainer
#52 lore raw
Quoted 23 lines · Avi Kivity
Quoted 6 lines · Paul Brook
Quoted 5 lines

Instead of writing directly, let's abstract it behind a qemu_set_irq(). This is easier for device authors. The default implementation of the irq callback could write to apic memory, while for kvm we can directly trigger the interrupt via the kvm APIs.

Quoted 8 lines

I'm still not convinced.

A tight coupling between PCI devices and the APIC is just going to cause us problems later one. I'm going to come back to the fact that these are memory writes so once we get IOMMU support they will presumably be subject to remapping by that, just like any other memory access.

I'm not suggesting the qemu_irq will extend all the way to the apic. Think of it as connecting the device core with its interrupt unit.

Quoted 3 lines

Even ignoring that, qemu_irq isn't really the right interface. A MSI is a one- off event, not a level state. OTOH stl_phys is exactly the right interface.

Quoted 6 lines

The qemu_irq callback should do an stl_phys(). The device is happy since it's using the same API it uses for non-MSI. The APIC is happy since it isn't connected directly to the device. stl_phys() is happy since it sees more traffic and can serve more ads. kvm is happy since it can hijack the callback to throw the interrupt directly into the kernel.

I like the monetization angle here.

Quoted 4 lines
Quoted 2 lines

The KVM interface should be contained within the APIC implementation.

Tricky, but doable.

I'd rather keep it simple.

Signature · 1 line
MST
#53 lore raw
In reply to Avi Kivity
Quoted 8 lines
Quoted 4 lines

A tight coupling between PCI devices and the APIC is just going to cause us problems later one. I'm going to come back to the fact that these are memory writes so once we get IOMMU support they will presumably be subject to remapping by that, just like any other memory access.

I'm not suggesting the qemu_irq will extend all the way to the apic. Think of it as connecting the device core with its interrupt unit.

Quoted 6 lines
Quoted 3 lines

Even ignoring that, qemu_irq isn't really the right interface. A MSI is a one- off event, not a level state. OTOH stl_phys is exactly the right interface.

The qemu_irq callback should do an stl_phys(). The device is happy since it's using the same API it uses for non-MSI.

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

Either way, this is an implementation detail between pci.c and individual devices. It has nothing to do with the APIC.

Paul

#54 lore raw
In reply to Avi Kivity
Quoted 8 lines
Quoted 4 lines

A tight coupling between PCI devices and the APIC is just going to cause us problems later one. I'm going to come back to the fact that these are memory writes so once we get IOMMU support they will presumably be subject to remapping by that, just like any other memory access.

I'm not suggesting the qemu_irq will extend all the way to the apic. Think of it as connecting the device core with its interrupt unit.

Quoted 6 lines
Quoted 3 lines

Even ignoring that, qemu_irq isn't really the right interface. A MSI is a one- off event, not a level state. OTOH stl_phys is exactly the right interface.

The qemu_irq callback should do an stl_phys(). The device is happy since it's using the same API it uses for non-MSI.

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

Either way, this is an implementation detail between pci.c and individual devices. It has nothing to do with the APIC.

Paul

author maintainer
#55 lore raw
Quoted 12 lines · Paul Brook
Quoted 12 lines
Quoted 4 lines

A tight coupling between PCI devices and the APIC is just going to cause us problems later one. I'm going to come back to the fact that these are memory writes so once we get IOMMU support they will presumably be subject to remapping by that, just like any other memory access.

I'm not suggesting the qemu_irq will extend all the way to the apic. Think of it as connecting the device core with its interrupt unit.

Quoted 3 lines

Even ignoring that, qemu_irq isn't really the right interface. A MSI is a one- off event, not a level state. OTOH stl_phys is exactly the right interface.

Quoted 6 lines
Quoted 2 lines

The qemu_irq callback should do an stl_phys(). The device is happy since it's using the same API it uses for non-MSI.

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

So, is qemu_send_msi better than qemu_set_irq.

Quoted 4 lines

Either way, this is an implementation detail between pci.c and individual devices. It has nothing to do with the APIC.

Paul

Signature · 1 line
MST
author maintainer
#56 lore raw
In reply to Paul Brook
Quoted 12 lines · Paul Brook
Quoted 12 lines
Quoted 4 lines

A tight coupling between PCI devices and the APIC is just going to cause us problems later one. I'm going to come back to the fact that these are memory writes so once we get IOMMU support they will presumably be subject to remapping by that, just like any other memory access.

I'm not suggesting the qemu_irq will extend all the way to the apic. Think of it as connecting the device core with its interrupt unit.

Quoted 3 lines

Even ignoring that, qemu_irq isn't really the right interface. A MSI is a one- off event, not a level state. OTOH stl_phys is exactly the right interface.

Quoted 6 lines
Quoted 2 lines

The qemu_irq callback should do an stl_phys(). The device is happy since it's using the same API it uses for non-MSI.

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

So, is qemu_send_msi better than qemu_set_irq.

Quoted 4 lines

Either way, this is an implementation detail between pci.c and individual devices. It has nothing to do with the APIC.

Paul

Signature · 1 line
MST
#57 lore raw
Quoted 5 lines
Quoted 3 lines

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

So, is qemu_send_msi better than qemu_set_irq.

Neither. pci_send_msi, which is a trivial wrapper around stl_phys.

Paul

#58 lore raw
In reply to Michael S. Tsirkin
Quoted 5 lines
Quoted 3 lines

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

So, is qemu_send_msi better than qemu_set_irq.

Neither. pci_send_msi, which is a trivial wrapper around stl_phys.

Paul

#59 lore raw
Quoted 7 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

So, is qemu_send_msi better than qemu_set_irq.

Neither. pci_send_msi, which is a trivial wrapper around stl_phys.

To clarify, you seem to be trying to fuse two largely separate features together.

MSI is a standard PCI device capability[1] that involves the device performing a 32-bit memory write when something interesting occurs. These writes may or may not be directed at a APIC.

The x86 APIC has a memory mapped interface that allows generation of CPU interrupts in response response to memory writes. These may or may not come from an MSI capable PCI device.

Paul

[1] Note a *device* capability, not a bus capability.

#60 lore raw
In reply to Paul Brook
Quoted 7 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

So, is qemu_send_msi better than qemu_set_irq.

Neither. pci_send_msi, which is a trivial wrapper around stl_phys.

To clarify, you seem to be trying to fuse two largely separate features together.

MSI is a standard PCI device capability[1] that involves the device performing a 32-bit memory write when something interesting occurs. These writes may or may not be directed at a APIC.

The x86 APIC has a memory mapped interface that allows generation of CPU interrupts in response response to memory writes. These may or may not come from an MSI capable PCI device.

Paul

[1] Note a *device* capability, not a bus capability.

author maintainer
#61 lore raw
Quoted 16 lines · Paul Brook
Quoted 7 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

So, is qemu_send_msi better than qemu_set_irq.

Neither. pci_send_msi, which is a trivial wrapper around stl_phys.

To clarify, you seem to be trying to fuse two largely separate features together.

MSI is a standard PCI device capability[1] that involves the device performing a 32-bit memory write when something interesting occurs. These writes may or may not be directed at a APIC.

Quoted 7 lines

The x86 APIC has a memory mapped interface that allows generation of CPU interrupts in response response to memory writes. These may or may not come from an MSI capable PCI device.

Paul

[1] Note a *device* capability, not a bus capability.

Paul, so I went over specs, and what you say about APIC here does not seem to be what Intel actually implemented. Specifically, Intel implemented *MSI support in APIC*. This lets PCI devices, but not the CPU, signal interrupts by memory writes.

For example, after reset, when CPU writes to address 0xfee00000 this is an access to a reserved register in APIC, but when PCI device does write to 0xfee00000, this triggers an interrupt to destination 0.

See section 9.12 in Intel® 64 and IA-32 Architectures Software Developer’s Manual Volume 3A: System Programming Guide, Part 1
http://www.intel.com/Assets/PDF/manual/253668.pdf

So it seems that what we need to do in pci is:

if (!msi_ops || msi_ops->send_msi(address, data))

	stl_phy(address, data);

where send_msi is wired to apic_send_msi and where apic_send_msi returns an error for an address outside of the MSI range 0xfee00000 - 0xfeefffff

Makes sense?

Signature · 5 lines
MST
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization
author maintainer
#62 lore raw
In reply to Paul Brook
Quoted 16 lines · Paul Brook
Quoted 7 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

So, is qemu_send_msi better than qemu_set_irq.

Neither. pci_send_msi, which is a trivial wrapper around stl_phys.

To clarify, you seem to be trying to fuse two largely separate features together.

MSI is a standard PCI device capability[1] that involves the device performing a 32-bit memory write when something interesting occurs. These writes may or may not be directed at a APIC.

Quoted 7 lines

The x86 APIC has a memory mapped interface that allows generation of CPU interrupts in response response to memory writes. These may or may not come from an MSI capable PCI device.

Paul

[1] Note a *device* capability, not a bus capability.

Paul, so I went over specs, and what you say about APIC here does not seem to be what Intel actually implemented. Specifically, Intel implemented *MSI support in APIC*. This lets PCI devices, but not the CPU, signal interrupts by memory writes.

For example, after reset, when CPU writes to address 0xfee00000 this is an access to a reserved register in APIC, but when PCI device does write to 0xfee00000, this triggers an interrupt to destination 0.

See section 9.12 in Intel® 64 and IA-32 Architectures Software Developer’s Manual Volume 3A: System Programming Guide, Part 1
http://www.intel.com/Assets/PDF/manual/253668.pdf

So it seems that what we need to do in pci is:

if (!msi_ops || msi_ops->send_msi(address, data))

	stl_phy(address, data);

where send_msi is wired to apic_send_msi and where apic_send_msi returns an error for an address outside of the MSI range 0xfee00000 - 0xfeefffff

Makes sense?

Signature · 1 line
MST
author maintainer
#63 lore raw
Quoted 40 lines · Michael S. Tsirkin
Quoted 16 lines · Paul Brook
Quoted 7 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

So, is qemu_send_msi better than qemu_set_irq.

Neither. pci_send_msi, which is a trivial wrapper around stl_phys.

To clarify, you seem to be trying to fuse two largely separate features together.

MSI is a standard PCI device capability[1] that involves the device performing a 32-bit memory write when something interesting occurs. These writes may or may not be directed at a APIC.

Quoted 7 lines

The x86 APIC has a memory mapped interface that allows generation of CPU interrupts in response response to memory writes. These may or may not come from an MSI capable PCI device.

Paul

[1] Note a *device* capability, not a bus capability.

Paul, so I went over specs, and what you say about APIC here does not seem to be what Intel actually implemented. Specifically, Intel implemented *MSI support in APIC*. This lets PCI devices, but not the CPU, signal interrupts by memory writes.

For example, after reset, when CPU writes to address 0xfee00000 this is an access to a reserved register in APIC, but when PCI device does write to 0xfee00000, this triggers an interrupt to destination 0.

See section 9.12 in Intel® 64 and IA-32 Architectures Software Developer’s Manual Volume 3A: System Programming Guide, Part 1
http://www.intel.com/Assets/PDF/manual/253668.pdf

So it seems that what we need to do in pci is:

Quoted 8 lines

if (!msi_ops || msi_ops->send_msi(address, data))

	stl_phy(address, data);

where send_msi is wired to apic_send_msi and where apic_send_msi returns an error for an address outside of the MSI range 0xfee00000 - 0xfeefffff

Makes sense?

So I ended up with these ops:
allocate
free
update
send
which APIC will define and MSI emulation will use. Here, send will return error for addresses outside 0xfeexxxxx range, and device will do a plain stl_phy.

Signature · 5 lines
MST
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization
author maintainer
#64 lore raw
In reply to Michael S. Tsirkin
Quoted 40 lines · Michael S. Tsirkin
Quoted 16 lines · Paul Brook
Quoted 7 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

So, is qemu_send_msi better than qemu_set_irq.

Neither. pci_send_msi, which is a trivial wrapper around stl_phys.

To clarify, you seem to be trying to fuse two largely separate features together.

MSI is a standard PCI device capability[1] that involves the device performing a 32-bit memory write when something interesting occurs. These writes may or may not be directed at a APIC.

Quoted 7 lines

The x86 APIC has a memory mapped interface that allows generation of CPU interrupts in response response to memory writes. These may or may not come from an MSI capable PCI device.

Paul

[1] Note a *device* capability, not a bus capability.

Paul, so I went over specs, and what you say about APIC here does not seem to be what Intel actually implemented. Specifically, Intel implemented *MSI support in APIC*. This lets PCI devices, but not the CPU, signal interrupts by memory writes.

For example, after reset, when CPU writes to address 0xfee00000 this is an access to a reserved register in APIC, but when PCI device does write to 0xfee00000, this triggers an interrupt to destination 0.

See section 9.12 in Intel® 64 and IA-32 Architectures Software Developer’s Manual Volume 3A: System Programming Guide, Part 1
http://www.intel.com/Assets/PDF/manual/253668.pdf

So it seems that what we need to do in pci is:

Quoted 8 lines

if (!msi_ops || msi_ops->send_msi(address, data))

	stl_phy(address, data);

where send_msi is wired to apic_send_msi and where apic_send_msi returns an error for an address outside of the MSI range 0xfee00000 - 0xfeefffff

Makes sense?

So I ended up with these ops:
allocate
free
update
send
which APIC will define and MSI emulation will use. Here, send will return error for addresses outside 0xfeexxxxx range, and device will do a plain stl_phy.

Signature · 1 line
MST
author maintainer
#65 lore raw
In reply to Paul Brook
Quoted 7 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

So, is qemu_send_msi better than qemu_set_irq.

Neither. pci_send_msi,

Works for me.

Quoted 1 line

which is a trivial wrapper around stl_phys.

OK, but I'm adding another level of indirection in the middle, to allow us to tie in a kvm backend.

Signature · 1 line
MST
author maintainer
#66 lore raw
In reply to Paul Brook
Quoted 7 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

So, is qemu_send_msi better than qemu_set_irq.

Neither. pci_send_msi,

Works for me.

Quoted 1 line

which is a trivial wrapper around stl_phys.

OK, but I'm adding another level of indirection in the middle, to allow us to tie in a kvm backend.

Signature · 1 line
MST
#67 lore raw
Quoted 4 lines
Quoted 1 line

which is a trivial wrapper around stl_phys.

OK, but I'm adding another level of indirection in the middle, to allow us to tie in a kvm backend.

kvm has no business messing with the PCI device code.

Paul

#68 lore raw
Quoted 8 lines · Paul Brook
Quoted 5 lines
Quoted 2 lines

which is a trivial wrapper around stl_phys.

OK, but I'm adding another level of indirection in the middle, to allow us to tie in a kvm backend.

kvm has no business messing with the PCI device code.

kvm has a fast path for irq injection. If qemu wants to support it we need some abstraction here.

Signature · 1 line
error compiling committee.c: too many arguments to function
#69 lore raw
Quoted 10 lines · Avi Kivity
Quoted 6 lines · Paul Brook
Quoted 4 lines
Quoted 1 line

which is a trivial wrapper around stl_phys.

OK, but I'm adding another level of indirection in the middle, to allow us to tie in a kvm backend.

kvm has no business messing with the PCI device code.

kvm has a fast path for irq injection. If qemu wants to support it we need some abstraction here.

Fast path from where to where? Having the PCI layer bypass/re-implement the APIC and inject the interrupt directly into the cpu core sounds a particularly bad idea.

Paul

#70 lore raw
Quoted 11 lines · Paul Brook
Quoted 9 lines · Avi Kivity
Quoted 7 lines · Paul Brook
Quoted 5 lines
Quoted 2 lines

which is a trivial wrapper around stl_phys.

OK, but I'm adding another level of indirection in the middle, to allow us to tie in a kvm backend.

kvm has no business messing with the PCI device code.

Quoted 8 lines
Quoted 3 lines

kvm has a fast path for irq injection. If qemu wants to support it we need some abstraction here.

Fast path from where to where? Having the PCI layer bypass/re-implement the APIC and inject the interrupt directly into the cpu core sounds a particularly bad idea.

kvm implements the APIC in the host kernel (qemu upstream doesn't support this yet). The fast path is wired to the in-kernel APIC, not the cpu core directly.

The idea is to wire it to UIO for device assignment, to a virtio-device implemented in the kernel, and to qemu.

Signature · 1 line
error compiling committee.c: too many arguments to function
#71 lore raw
In reply to Paul Brook
Quoted 9 lines · Paul Brook
Quoted 7 lines · Avi Kivity
Quoted 5 lines · Paul Brook
Quoted 5 lines
Quoted 2 lines

which is a trivial wrapper around stl_phys.

OK, but I'm adding another level of indirection in the middle, to allow us to tie in a kvm backend.

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

kvm has no business messing with the PCI device code.

kvm has a fast path for irq injection. If qemu wants to support it we need some abstraction here.

Fast path from where to where? Having the PCI layer bypass/re-implement the APIC and inject the interrupt directly into the cpu core sounds a particularly bad idea.

kvm implements the APIC in the host kernel (qemu upstream doesn't support this yet). The fast path is wired to the in-kernel APIC, not the cpu core directly.

The idea is to wire it to UIO for device assignment, to a virtio-device implemented in the kernel, and to qemu.

Signature · 1 line
error compiling committee.c: too many arguments to function
#72 lore raw
Quoted 9 lines
Quoted 8 lines
Quoted 4 lines
Quoted 1 line

kvm has no business messing with the PCI device code.

kvm has a fast path for irq injection. If qemu wants to support it we need some abstraction here.

Fast path from where to where? Having the PCI layer bypass/re-implement the APIC and inject the interrupt directly into the cpu core sounds a particularly bad idea.

Quoted 6 lines

kvm implements the APIC in the host kernel (qemu upstream doesn't support this yet). The fast path is wired to the in-kernel APIC, not the cpu core directly.

The idea is to wire it to UIO for device assignment, to a virtio-device implemented in the kernel, and to qemu.

I still don't see why you're trying to bypass straight from the pci layer to the apic. Why can't you just pass the apic MMIO writes to the kernel? You've presumably got to update the apic state anyway.

Paul

#73 lore raw
In reply to Avi Kivity
Quoted 9 lines
Quoted 8 lines
Quoted 4 lines
Quoted 1 line

kvm has no business messing with the PCI device code.

kvm has a fast path for irq injection. If qemu wants to support it we need some abstraction here.

Fast path from where to where? Having the PCI layer bypass/re-implement the APIC and inject the interrupt directly into the cpu core sounds a particularly bad idea.

Quoted 6 lines

kvm implements the APIC in the host kernel (qemu upstream doesn't support this yet). The fast path is wired to the in-kernel APIC, not the cpu core directly.

The idea is to wire it to UIO for device assignment, to a virtio-device implemented in the kernel, and to qemu.

I still don't see why you're trying to bypass straight from the pci layer to the apic. Why can't you just pass the apic MMIO writes to the kernel? You've presumably got to update the apic state anyway.

Paul

#74 lore raw
Quoted 4 lines · Paul Brook
Quoted 4 lines

kvm implements the APIC in the host kernel (qemu upstream doesn't support this yet). The fast path is wired to the in-kernel APIC, not the cpu core directly.

Quoted 8 lines
Quoted 3 lines

The idea is to wire it to UIO for device assignment, to a virtio-device implemented in the kernel, and to qemu.

I still don't see why you're trying to bypass straight from the pci layer to the apic. Why can't you just pass the apic MMIO writes to the kernel? You've presumably got to update the apic state anyway.

The fast path is an eventfd so that we don't have to teach all the clients about the details of MSI. Userspace programs the MSI details into kvm and hands the client an eventfd. All the client has to do is bang on the eventfd for the interrupt to be queued. The eventfd provides event coalescing and is equally useful from the kernel and userspace, and can be used with targets other than kvm.

Signature · 1 line
error compiling committee.c: too many arguments to function
#75 lore raw
In reply to Paul Brook
Quoted 10 lines · Paul Brook
Quoted 7 lines

kvm implements the APIC in the host kernel (qemu upstream doesn't support this yet). The fast path is wired to the in-kernel APIC, not the cpu core directly.

The idea is to wire it to UIO for device assignment, to a virtio-device implemented in the kernel, and to qemu.

I still don't see why you're trying to bypass straight from the pci layer to the apic. Why can't you just pass the apic MMIO writes to the kernel? You've presumably got to update the apic state anyway.

The fast path is an eventfd so that we don't have to teach all the clients about the details of MSI. Userspace programs the MSI details into kvm and hands the client an eventfd. All the client has to do is bang on the eventfd for the interrupt to be queued. The eventfd provides event coalescing and is equally useful from the kernel and userspace, and can be used with targets other than kvm.

Signature · 1 line
error compiling committee.c: too many arguments to function
#76 lore raw
Quoted 12 lines · Avi Kivity
Quoted 10 lines · Paul Brook
Quoted 6 lines

kvm implements the APIC in the host kernel (qemu upstream doesn't support this yet). The fast path is wired to the in-kernel APIC, not the cpu core directly.

The idea is to wire it to UIO for device assignment, to a virtio-device implemented in the kernel, and to qemu.

I still don't see why you're trying to bypass straight from the pci layer to the apic. Why can't you just pass the apic MMIO writes to the kernel? You've presumably got to update the apic state anyway.

Quoted 6 lines

The fast path is an eventfd so that we don't have to teach all the clients about the details of MSI. Userspace programs the MSI details into kvm and hands the client an eventfd. All the client has to do is bang on the eventfd for the interrupt to be queued. The eventfd provides event coalescing and is equally useful from the kernel and userspace, and can be used with targets other than kvm.

So presumably if a device triggers an APIC interrupt using a write that isn't one of the currently configured PCI devices, it all explodes horribly?

Paul

#77 lore raw
In reply to Avi Kivity
Quoted 12 lines · Avi Kivity
Quoted 10 lines · Paul Brook
Quoted 6 lines

kvm implements the APIC in the host kernel (qemu upstream doesn't support this yet). The fast path is wired to the in-kernel APIC, not the cpu core directly.

The idea is to wire it to UIO for device assignment, to a virtio-device implemented in the kernel, and to qemu.

I still don't see why you're trying to bypass straight from the pci layer to the apic. Why can't you just pass the apic MMIO writes to the kernel? You've presumably got to update the apic state anyway.

Quoted 6 lines

The fast path is an eventfd so that we don't have to teach all the clients about the details of MSI. Userspace programs the MSI details into kvm and hands the client an eventfd. All the client has to do is bang on the eventfd for the interrupt to be queued. The eventfd provides event coalescing and is equally useful from the kernel and userspace, and can be used with targets other than kvm.

So presumably if a device triggers an APIC interrupt using a write that isn't one of the currently configured PCI devices, it all explodes horribly?

Paul

#78 lore raw
Quoted 11 lines · Paul Brook
Quoted 7 lines

The fast path is an eventfd so that we don't have to teach all the clients about the details of MSI. Userspace programs the MSI details into kvm and hands the client an eventfd. All the client has to do is bang on the eventfd for the interrupt to be queued. The eventfd provides event coalescing and is equally useful from the kernel and userspace, and can be used with targets other than kvm.

So presumably if a device triggers an APIC interrupt using a write that isn't one of the currently configured PCI devices, it all explodes horribly?

I don't follow. Can you elaborate on your scenario?

Signature · 1 line
error compiling committee.c: too many arguments to function
#79 lore raw
In reply to Paul Brook
Quoted 10 lines · Paul Brook
Quoted 7 lines

The fast path is an eventfd so that we don't have to teach all the clients about the details of MSI. Userspace programs the MSI details into kvm and hands the client an eventfd. All the client has to do is bang on the eventfd for the interrupt to be queued. The eventfd provides event coalescing and is equally useful from the kernel and userspace, and can be used with targets other than kvm.

So presumably if a device triggers an APIC interrupt using a write that isn't one of the currently configured PCI devices, it all explodes horribly?

I don't follow. Can you elaborate on your scenario?

Signature · 1 line
error compiling committee.c: too many arguments to function
author maintainer
#80 lore raw
In reply to Paul Brook
Quoted 13 lines · Paul Brook
Quoted 13 lines
Quoted 8 lines
Quoted 4 lines
Quoted 1 line

kvm has no business messing with the PCI device code.

kvm has a fast path for irq injection. If qemu wants to support it we need some abstraction here.

Fast path from where to where? Having the PCI layer bypass/re-implement the APIC and inject the interrupt directly into the cpu core sounds a particularly bad idea.

kvm implements the APIC in the host kernel (qemu upstream doesn't support this yet). The fast path is wired to the in-kernel APIC, not the cpu core directly.

Quoted 8 lines
Quoted 2 lines

The idea is to wire it to UIO for device assignment, to a virtio-device implemented in the kernel, and to qemu.

I still don't see why you're trying to bypass straight from the pci layer to the apic. Why can't you just pass the apic MMIO writes to the kernel? You've presumably got to update the apic state anyway.

Paul

As far as I can tell, at least on Intel, MSI interrupts are not MMIO writes. They are PCI memory writes to a hard-coded address range that are passed to APIC. I don't think MMIO writes can triger MSI,
or at least this does not seem to be documented.

Signature · 1 line
MST
author maintainer
#81 lore raw
In reply to Paul Brook
Quoted 13 lines · Paul Brook
Quoted 13 lines
Quoted 8 lines
Quoted 4 lines
Quoted 1 line

kvm has no business messing with the PCI device code.

kvm has a fast path for irq injection. If qemu wants to support it we need some abstraction here.

Fast path from where to where? Having the PCI layer bypass/re-implement the APIC and inject the interrupt directly into the cpu core sounds a particularly bad idea.

kvm implements the APIC in the host kernel (qemu upstream doesn't support this yet). The fast path is wired to the in-kernel APIC, not the cpu core directly.

Quoted 8 lines
Quoted 2 lines

The idea is to wire it to UIO for device assignment, to a virtio-device implemented in the kernel, and to qemu.

I still don't see why you're trying to bypass straight from the pci layer to the apic. Why can't you just pass the apic MMIO writes to the kernel? You've presumably got to update the apic state anyway.

Paul

As far as I can tell, at least on Intel, MSI interrupts are not MMIO writes. They are PCI memory writes to a hard-coded address range that are passed to APIC. I don't think MMIO writes can triger MSI,
or at least this does not seem to be documented.

Signature · 1 line
MST
#82 lore raw
In reply to Avi Kivity
Quoted 10 lines · Avi Kivity
Quoted 6 lines · Paul Brook
Quoted 4 lines
Quoted 1 line

which is a trivial wrapper around stl_phys.

OK, but I'm adding another level of indirection in the middle, to allow us to tie in a kvm backend.

kvm has no business messing with the PCI device code.

kvm has a fast path for irq injection. If qemu wants to support it we need some abstraction here.

Fast path from where to where? Having the PCI layer bypass/re-implement the APIC and inject the interrupt directly into the cpu core sounds a particularly bad idea.

Paul

#83 lore raw
In reply to Paul Brook
Quoted 8 lines · Paul Brook
Quoted 5 lines
Quoted 2 lines

which is a trivial wrapper around stl_phys.

OK, but I'm adding another level of indirection in the middle, to allow us to tie in a kvm backend.

kvm has no business messing with the PCI device code.

kvm has a fast path for irq injection. If qemu wants to support it we need some abstraction here.

Signature · 1 line
error compiling committee.c: too many arguments to function
author maintainer
#84 lore raw
In reply to Paul Brook
Quoted 6 lines · Paul Brook
Quoted 4 lines
Quoted 1 line

which is a trivial wrapper around stl_phys.

OK, but I'm adding another level of indirection in the middle, to allow us to tie in a kvm backend.

kvm has no business messing with the PCI device code.

Yes it has :)

kvm needs data on MSI entries: that's the interface current kernel exposes for injecting these interrupts.

I think we also need to support in-kernel devices which would inject MSI interrupt directly from kernel. For these, kvm would need to know when mask bit changes and give us info on pending bit.

That's a fair amount of PCI specific code in kvm.

Signature · 1 line
MST
author maintainer
#85 lore raw
In reply to Paul Brook
Quoted 6 lines · Paul Brook
Quoted 4 lines
Quoted 1 line

which is a trivial wrapper around stl_phys.

OK, but I'm adding another level of indirection in the middle, to allow us to tie in a kvm backend.

kvm has no business messing with the PCI device code.

Yes it has :)

kvm needs data on MSI entries: that's the interface current kernel exposes for injecting these interrupts.

I think we also need to support in-kernel devices which would inject MSI interrupt directly from kernel. For these, kvm would need to know when mask bit changes and give us info on pending bit.

That's a fair amount of PCI specific code in kvm.

Signature · 1 line
MST
#86 lore raw
Quoted 9 lines · Michael S. Tsirkin

kvm needs data on MSI entries: that's the interface current kernel exposes for injecting these interrupts.

I think we also need to support in-kernel devices which would inject MSI interrupt directly from kernel. For these, kvm would need to know when mask bit changes and give us info on pending bit.

That's a fair amount of PCI specific code in kvm

My plan is to get rid of it. The mask stuff need not be pci specific.

Signature · 1 line
error compiling committee.c: too many arguments to function
#87 lore raw
In reply to Michael S. Tsirkin
Quoted 9 lines · Michael S. Tsirkin

kvm needs data on MSI entries: that's the interface current kernel exposes for injecting these interrupts.

I think we also need to support in-kernel devices which would inject MSI interrupt directly from kernel. For these, kvm would need to know when mask bit changes and give us info on pending bit.

That's a fair amount of PCI specific code in kvm

My plan is to get rid of it. The mask stuff need not be pci specific.

Signature · 1 line
error compiling committee.c: too many arguments to function
#88 lore raw
In reply to Michael S. Tsirkin
Quoted 4 lines
Quoted 1 line

which is a trivial wrapper around stl_phys.

OK, but I'm adding another level of indirection in the middle, to allow us to tie in a kvm backend.

kvm has no business messing with the PCI device code.

Paul

author maintainer
#89 lore raw
In reply to Paul Brook
Quoted 7 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

So, is qemu_send_msi better than qemu_set_irq.

Neither. pci_send_msi, which is a trivial wrapper around stl_phys.

I guess I'll start with that. This only works if target_phys_addr_t is a 64 bit field (because MSI addresses are typically outside the 32 bit memory space), I guess the simplest solution is to disable MSI if it's not so.

Signature · 1 line
MST
author maintainer
#90 lore raw
In reply to Paul Brook
Quoted 7 lines · Paul Brook
Quoted 5 lines
Quoted 3 lines

MSI provides multiple edge triggered interrupts, whereas traditional mode provides a single level triggered interrupt. My guess is most devices will want to treat these differently anyway.

So, is qemu_send_msi better than qemu_set_irq.

Neither. pci_send_msi, which is a trivial wrapper around stl_phys.

I guess I'll start with that. This only works if target_phys_addr_t is a 64 bit field (because MSI addresses are typically outside the 32 bit memory space), I guess the simplest solution is to disable MSI if it's not so.

Signature · 1 line
MST
author maintainer
#91 lore raw
In reply to Avi Kivity
Quoted 16 lines · Avi Kivity
Quoted 6 lines · Paul Brook
Quoted 5 lines

Instead of writing directly, let's abstract it behind a qemu_set_irq(). This is easier for device authors. The default implementation of the irq callback could write to apic memory, while for kvm we can directly trigger the interrupt via the kvm APIs.

Quoted 8 lines

I'm still not convinced.

A tight coupling between PCI devices and the APIC is just going to cause us problems later one. I'm going to come back to the fact that these are memory writes so once we get IOMMU support they will presumably be subject to remapping by that, just like any other memory access.

Quoted 8 lines

I'm not suggesting the qemu_irq will extend all the way to the apic. Think of it as connecting the device core with its interrupt unit.

Quoted 3 lines

Even ignoring that, qemu_irq isn't really the right interface. A MSI is a one- off event, not a level state. OTOH stl_phys is exactly the right interface.

The qemu_irq callback should do an stl_phys().

Actually, it seems we can't do it this way now as stl_phys only gets a 32 bit address. So I'll use apic_deliver for now, but yes, it will be easy to later rewrite MSI implementation this way if that limitatiuon is lifted.

Quoted 14 lines

The device is happy
since it's using the same API it uses for non-MSI. The APIC is happy since it isn't connected directly to the device. stl_phys() is happy since it sees more traffic and can serve more ads. kvm is happy since it can hijack the callback to throw the interrupt directly into the kernel.

Quoted 2 lines

The KVM interface should be contained within the APIC implementation.

Tricky, but doable.

Signature · 1 line
error compiling committee.c: too many arguments to function
Signature · 1 line
MST
author maintainer
#92 lore raw
In reply to Avi Kivity
Quoted 23 lines · Avi Kivity
Quoted 6 lines · Paul Brook
Quoted 5 lines

Instead of writing directly, let's abstract it behind a qemu_set_irq(). This is easier for device authors. The default implementation of the irq callback could write to apic memory, while for kvm we can directly trigger the interrupt via the kvm APIs.

Quoted 8 lines

I'm still not convinced.

A tight coupling between PCI devices and the APIC is just going to cause us problems later one. I'm going to come back to the fact that these are memory writes so once we get IOMMU support they will presumably be subject to remapping by that, just like any other memory access.

I'm not suggesting the qemu_irq will extend all the way to the apic. Think of it as connecting the device core with its interrupt unit.

Quoted 3 lines

Even ignoring that, qemu_irq isn't really the right interface. A MSI is a one- off event, not a level state. OTOH stl_phys is exactly the right interface.

Quoted 6 lines

The qemu_irq callback should do an stl_phys(). The device is happy since it's using the same API it uses for non-MSI. The APIC is happy since it isn't connected directly to the device. stl_phys() is happy since it sees more traffic and can serve more ads. kvm is happy since it can hijack the callback to throw the interrupt directly into the kernel.

I like the monetization angle here.

Quoted 4 lines
Quoted 2 lines

The KVM interface should be contained within the APIC implementation.

Tricky, but doable.

I'd rather keep it simple.

Signature · 1 line
MST
author maintainer
#93 lore raw
In reply to Avi Kivity
Quoted 16 lines · Avi Kivity
Quoted 6 lines · Paul Brook
Quoted 5 lines

Instead of writing directly, let's abstract it behind a qemu_set_irq(). This is easier for device authors. The default implementation of the irq callback could write to apic memory, while for kvm we can directly trigger the interrupt via the kvm APIs.

Quoted 8 lines

I'm still not convinced.

A tight coupling between PCI devices and the APIC is just going to cause us problems later one. I'm going to come back to the fact that these are memory writes so once we get IOMMU support they will presumably be subject to remapping by that, just like any other memory access.

Quoted 8 lines

I'm not suggesting the qemu_irq will extend all the way to the apic. Think of it as connecting the device core with its interrupt unit.

Quoted 3 lines

Even ignoring that, qemu_irq isn't really the right interface. A MSI is a one- off event, not a level state. OTOH stl_phys is exactly the right interface.

The qemu_irq callback should do an stl_phys().

Actually, it seems we can't do it this way now as stl_phys only gets a 32 bit address. So I'll use apic_deliver for now, but yes, it will be easy to later rewrite MSI implementation this way if that limitatiuon is lifted.

Quoted 14 lines

The device is happy
since it's using the same API it uses for non-MSI. The APIC is happy since it isn't connected directly to the device. stl_phys() is happy since it sees more traffic and can serve more ads. kvm is happy since it can hijack the callback to throw the interrupt directly into the kernel.

Quoted 2 lines

The KVM interface should be contained within the APIC implementation.

Tricky, but doable.

Signature · 1 line
error compiling committee.c: too many arguments to function
Signature · 1 line
MST
author maintainer
#94 lore raw
In reply to Paul Brook
Quoted 22 lines · Paul Brook
Quoted 16 lines · Avi Kivity
Quoted 8 lines · Paul Brook
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

virtio, emulated devices capable of supporting MSI (e1000?), device assignment (not yet in qemu.git).

Quoted 6 lines

It probably makes sense to have common infrastructure in pci.c to expose/implement device side MSI functionality. However I see no need for a direct API between the device and the APIC. We already have an API for memory accesses and MMIO regions. I'm pretty sure a system could implement MSI by pointing the device at system ram, and having the CPU periodically poll that.

Quoted 4 lines

Instead of writing directly, let's abstract it behind a qemu_set_irq(). This is easier for device authors. The default implementation of the irq callback could write to apic memory, while for kvm we can directly trigger the interrupt via the kvm APIs.

Quoted 6 lines

I'm still not convinced.

A tight coupling between PCI devices and the APIC is just going to cause us problems later one. I'm going to come back to the fact that these are memory writes so once we get IOMMU support they will presumably be subject to remapping by that, just like any other memory access.

Actually, MSI messages are handled by IOMMU as interrupts, not as regular memory accesses. iommu book has comments such as
• Interrupt addresses are never translated to memory addresses, but other special address ranges may be reclaimed to be backed with memory.

Quoted 2 lines

Even ignoring that, qemu_irq isn't really the right interface. A MSI is a one- off event, not a level state.

Yes, I just chose to ignore the level value. It does not look like such a big issue ... Do you advocate a new qemu_msi structure then?

Quoted 1 line

OTOH stl_phys is exactly the right interface.

Not really. These are writes but not into physical memory. For example, on intel 32 bit, stl_phys gets a 32 bit address, MSI writes encode the interrupt vector in high 32 bit bit of the address - way outside actual physical memory.

Quoted 3 lines

The KVM interface should be contained within the APIC implementation.

Paul

Unfortunately kvm capabilities that are present in current kernels do not map well to this interface. You need to perform expensive set up for each interrupt vector you are going to use, be it MSI or regular interrupt.

Signature · 5 lines
MST
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization
author maintainer
#95 lore raw
In reply to Paul Brook
Quoted 22 lines · Paul Brook
Quoted 16 lines · Avi Kivity
Quoted 8 lines · Paul Brook
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

In any case we need some internal API for this, and qemu_irq looks like a good choice.

What do you expect to be using this API?

virtio, emulated devices capable of supporting MSI (e1000?), device assignment (not yet in qemu.git).

Quoted 6 lines

It probably makes sense to have common infrastructure in pci.c to expose/implement device side MSI functionality. However I see no need for a direct API between the device and the APIC. We already have an API for memory accesses and MMIO regions. I'm pretty sure a system could implement MSI by pointing the device at system ram, and having the CPU periodically poll that.

Quoted 4 lines

Instead of writing directly, let's abstract it behind a qemu_set_irq(). This is easier for device authors. The default implementation of the irq callback could write to apic memory, while for kvm we can directly trigger the interrupt via the kvm APIs.

Quoted 6 lines

I'm still not convinced.

A tight coupling between PCI devices and the APIC is just going to cause us problems later one. I'm going to come back to the fact that these are memory writes so once we get IOMMU support they will presumably be subject to remapping by that, just like any other memory access.

Actually, MSI messages are handled by IOMMU as interrupts, not as regular memory accesses. iommu book has comments such as
• Interrupt addresses are never translated to memory addresses, but other special address ranges may be reclaimed to be backed with memory.

Quoted 2 lines

Even ignoring that, qemu_irq isn't really the right interface. A MSI is a one- off event, not a level state.

Yes, I just chose to ignore the level value. It does not look like such a big issue ... Do you advocate a new qemu_msi structure then?

Quoted 1 line

OTOH stl_phys is exactly the right interface.

Not really. These are writes but not into physical memory. For example, on intel 32 bit, stl_phys gets a 32 bit address, MSI writes encode the interrupt vector in high 32 bit bit of the address - way outside actual physical memory.

Quoted 3 lines

The KVM interface should be contained within the APIC implementation.

Paul

Unfortunately kvm capabilities that are present in current kernels do not map well to this interface. You need to perform expensive set up for each interrupt vector you are going to use, be it MSI or regular interrupt.

Signature · 1 line
MST