[PATCH] msi/msix: added public API to set/get MSI message address, and data
agrhhh. sha1 of the patch changed after rebasing :)
Added (msi|msix)_(set|get)_message() function for whoever might
want to use them.
Currently msi_notify()/msix_notify() write to these vectors to
signal the guest about an interrupt so the correct values have to
written there by the guest or QEMU.
For example, POWER guest never initializes MSI/MSIX vectors, instead
it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
POWER we have to initialize MSI/MSIX message from QEMU.
As only set* function are required by now, the get functions were added
or made public for a symmetry.
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
hw/msi.c | 29 +
hw/msi.h |2 ++
hw/msix.c | 11 ++-
hw/msix.h |3 +++
4 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/hw/msi.c b/hw/msi.c
index 5233204..9ad84a4 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -105,6 +105,35 @@ static inline uint8_t msi_pending_off(const PCIDevice*
dev, bool msi64bit)
return dev-msi_cap + (msi64bit ? PCI_MSI_PENDING_64 : PCI_MSI_PENDING_32);
}
+MSIMessage msi_get_message(PCIDevice *dev)
+{
+uint16_t flags = pci_get_word(dev-config + msi_flags_off(dev));
+bool msi64bit = flags PCI_MSI_FLAGS_64BIT;
+MSIMessage msg;
+
+if (msi64bit) {
+msg.address = pci_get_quad(dev-config + msi_address_lo_off(dev));
+} else {
+msg.address = pci_get_long(dev-config + msi_address_lo_off(dev));
+}
+msg.data = pci_get_word(dev-config + msi_data_off(dev, msi64bit));
+
+return msg;
+}
+
+void msi_set_message(PCIDevice *dev, MSIMessage msg)
+{
+uint16_t flags = pci_get_word(dev-config + msi_flags_off(dev));
+bool msi64bit = flags PCI_MSI_FLAGS_64BIT;
+
+if (msi64bit) {
+pci_set_quad(dev-config + msi_address_lo_off(dev), msg.address);
+} else {
+pci_set_long(dev-config + msi_address_lo_off(dev), msg.address);
+}
+pci_set_word(dev-config + msi_data_off(dev, msi64bit), msg.data);
+}
+
bool msi_enabled(const PCIDevice *dev)
{
return msi_present(dev)
diff --git a/hw/msi.h b/hw/msi.h
index 75747ab..4b0f4f8 100644
--- a/hw/msi.h
+++ b/hw/msi.h
@@ -31,6 +31,8 @@ struct MSIMessage {
extern bool msi_supported;
+MSIMessage msi_get_message(PCIDevice *dev);
+void msi_set_message(PCIDevice *dev, MSIMessage msg);
bool msi_enabled(const PCIDevice *dev);
int msi_init(struct PCIDevice *dev, uint8_t offset,
unsigned int nr_vectors, bool msi64bit, bool msi_per_vector_mask);
diff --git a/hw/msix.c b/hw/msix.c
index ded3c55..9e8d8bb 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -35,7 +35,7 @@
#define MSIX_PAGE_PENDING (MSIX_PAGE_SIZE / 2)
#define MSIX_MAX_ENTRIES 32
-static MSIMessage msix_get_message(PCIDevice *dev, unsigned vector)
+MSIMessage msix_get_message(PCIDevice *dev, unsigned vector)
{
uint8_t *table_entry = dev-msix_table_page + vector * PCI_MSIX_ENTRY_SIZE;
MSIMessage msg;
@@ -45,6 +45,15 @@ static MSIMessage msix_get_message(PCIDevice *dev, unsigned
vector)
return msg;
}
+void msix_set_message(PCIDevice *dev, int vector, struct MSIMessage msg)
+{
+uint8_t *table_entry = dev-msix_table_page + vector * PCI_MSIX_ENTRY_SIZE;
+
+pci_set_quad(table_entry + PCI_MSIX_ENTRY_LOWER_ADDR, msg.address);
+pci_set_long(table_entry + PCI_MSIX_ENTRY_DATA, msg.data);
+table_entry[PCI_MSIX_ENTRY_VECTOR_CTRL] = ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
+}
+
/* Add MSI-X capability to the config space for the device. */
/* Given a bar and its size, add MSI-X table on top of it
* and fill MSI-X capability in the config space.
diff --git a/hw/msix.h b/hw/msix.h
index 50aee82..3374cf8 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -4,6 +4,9 @@
#include qemu-common.h
#include pci.h
+MSIMessage msix_get_message(PCIDevice *dev, unsigned vector);
+void msix_set_message(PCIDevice *dev, int vector, MSIMessage msg);
+
int msix_init(PCIDevice *pdev, unsigned short nentries,
MemoryRegion *bar,
unsigned bar_nr, unsigned bar_size);
--
1.7.10
On 21/06/12 16:53, Jan Kiszka wrote:
On 2012-06-21 08:46, Alexey Kardashevskiy wrote:
Ok, another try. Is it any better now? :)
No - posted the old version accidentally?
Jan
Normally QEMU expects the guest to initialize MSI/MSIX vectors.
However on POWER the guest uses RTAS subsystem to configure MSI/MSIX and
does not write these vectors to device's config space or MSIX BAR.
On the other hand, msi_notify()/msix_notify() write to these vectors to
signal the guest about an interrupt so we have to write correct vectors
to the devices in order not to change every user of MSI/MSIX.
The first aim is to support MSIX for virtio-pci on POWER. There is
another patch for POWER coming which introduces a special memory region
where MSI/MSIX vectors point to.
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
hw/msi.c | 14 ++
hw/msi.h |1 +
hw/msix.c |8
Re: [PATCH] msi/msix: added public API to set/get MSI message address, and data
On 2012-06-21 09:18, Alexey Kardashevskiy wrote:
agrhhh. sha1 of the patch changed after rebasing :)
Added (msi|msix)_(set|get)_message() function for whoever might
want to use them.
Currently msi_notify()/msix_notify() write to these vectors to
signal the guest about an interrupt so the correct values have to
written there by the guest or QEMU.
For example, POWER guest never initializes MSI/MSIX vectors, instead
it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
POWER we have to initialize MSI/MSIX message from QEMU.
As only set* function are required by now, the get functions were added
or made public for a symmetry.
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
hw/msi.c | 29 +
hw/msi.h |2 ++
hw/msix.c | 11 ++-
hw/msix.h |3 +++
4 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/hw/msi.c b/hw/msi.c
index 5233204..9ad84a4 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -105,6 +105,35 @@ static inline uint8_t msi_pending_off(const PCIDevice*
dev, bool msi64bit)
return dev-msi_cap + (msi64bit ? PCI_MSI_PENDING_64 :
PCI_MSI_PENDING_32);
}
+MSIMessage msi_get_message(PCIDevice *dev)
MSIMessage msi_get_message(PCIDevice *dev, unsigned vector)
+{
+uint16_t flags = pci_get_word(dev-config + msi_flags_off(dev));
+bool msi64bit = flags PCI_MSI_FLAGS_64BIT;
+MSIMessage msg;
+
+if (msi64bit) {
+msg.address = pci_get_quad(dev-config + msi_address_lo_off(dev));
+} else {
+msg.address = pci_get_long(dev-config + msi_address_lo_off(dev));
+}
+msg.data = pci_get_word(dev-config + msi_data_off(dev, msi64bit));
And I have this here in addition:
unsigned int nr_vectors = msi_nr_vectors(flags);
...
if (nr_vectors 1) {
msg.data = ~(nr_vectors - 1);
msg.data |= vector;
}
See PCI spec and existing code.
+
+return msg;
+}
+
+void msi_set_message(PCIDevice *dev, MSIMessage msg)
+{
+uint16_t flags = pci_get_word(dev-config + msi_flags_off(dev));
+bool msi64bit = flags PCI_MSI_FLAGS_64BIT;
+
+if (msi64bit) {
+pci_set_quad(dev-config + msi_address_lo_off(dev), msg.address);
+} else {
+pci_set_long(dev-config + msi_address_lo_off(dev), msg.address);
+}
+pci_set_word(dev-config + msi_data_off(dev, msi64bit), msg.data);
+}
+
bool msi_enabled(const PCIDevice *dev)
{
return msi_present(dev)
diff --git a/hw/msi.h b/hw/msi.h
index 75747ab..4b0f4f8 100644
--- a/hw/msi.h
+++ b/hw/msi.h
@@ -31,6 +31,8 @@ struct MSIMessage {
extern bool msi_supported;
+MSIMessage msi_get_message(PCIDevice *dev);
+void msi_set_message(PCIDevice *dev, MSIMessage msg);
bool msi_enabled(const PCIDevice *dev);
int msi_init(struct PCIDevice *dev, uint8_t offset,
unsigned int nr_vectors, bool msi64bit, bool
msi_per_vector_mask);
diff --git a/hw/msix.c b/hw/msix.c
index ded3c55..9e8d8bb 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -35,7 +35,7 @@
#define MSIX_PAGE_PENDING (MSIX_PAGE_SIZE / 2)
#define MSIX_MAX_ENTRIES 32
-static MSIMessage msix_get_message(PCIDevice *dev, unsigned vector)
+MSIMessage msix_get_message(PCIDevice *dev, unsigned vector)
{
uint8_t *table_entry = dev-msix_table_page + vector *
PCI_MSIX_ENTRY_SIZE;
MSIMessage msg;
@@ -45,6 +45,15 @@ static MSIMessage msix_get_message(PCIDevice *dev,
unsigned vector)
return msg;
}
+void msix_set_message(PCIDevice *dev, int vector, struct MSIMessage msg)
+{
+uint8_t *table_entry = dev-msix_table_page + vector *
PCI_MSIX_ENTRY_SIZE;
+
+pci_set_quad(table_entry + PCI_MSIX_ENTRY_LOWER_ADDR, msg.address);
+pci_set_long(table_entry + PCI_MSIX_ENTRY_DATA, msg.data);
+table_entry[PCI_MSIX_ENTRY_VECTOR_CTRL] = ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
+}
+
/* Add MSI-X capability to the config space for the device. */
/* Given a bar and its size, add MSI-X table on top of it
* and fill MSI-X capability in the config space.
diff --git a/hw/msix.h b/hw/msix.h
index 50aee82..3374cf8 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -4,6 +4,9 @@
#include qemu-common.h
#include pci.h
+MSIMessage msix_get_message(PCIDevice *dev, unsigned vector);
+void msix_set_message(PCIDevice *dev, int vector, MSIMessage msg);
+
int msix_init(PCIDevice *pdev, unsigned short nentries,
MemoryRegion *bar,
unsigned bar_nr, unsigned bar_size);
General remark: You will make the life of the maintainers easier by
formatting your patch in a way that a clean merge via git works without
hand-editing. E.g. this patch was no scissor line (---8--- etc.)
between introductory text and the patch description. And the subject is
not [PATCH]
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line unsubscribe
Re: [PATCH] msi/msix: added public API to set/get MSI message address, and data
On 21/06/12 20:38, Jan Kiszka wrote:
On 2012-06-21 12:28, Alexey Kardashevskiy wrote:
On 21/06/12 17:39, Jan Kiszka wrote:
On 2012-06-21 09:18, Alexey Kardashevskiy wrote:
agrhhh. sha1 of the patch changed after rebasing :)
Added (msi|msix)_(set|get)_message() function for whoever might
want to use them.
Currently msi_notify()/msix_notify() write to these vectors to
signal the guest about an interrupt so the correct values have to
written there by the guest or QEMU.
For example, POWER guest never initializes MSI/MSIX vectors, instead
it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
POWER we have to initialize MSI/MSIX message from QEMU.
As only set* function are required by now, the get functions were added
or made public for a symmetry.
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
hw/msi.c | 29 +
hw/msi.h |2 ++
hw/msix.c | 11 ++-
hw/msix.h |3 +++
4 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/hw/msi.c b/hw/msi.c
index 5233204..9ad84a4 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -105,6 +105,35 @@ static inline uint8_t msi_pending_off(const
PCIDevice* dev, bool msi64bit)
return dev-msi_cap + (msi64bit ? PCI_MSI_PENDING_64 :
PCI_MSI_PENDING_32);
}
+MSIMessage msi_get_message(PCIDevice *dev)
MSIMessage msi_get_message(PCIDevice *dev, unsigned vector)
Who/how/why is going to calculate the vector here?
+{
+uint16_t flags = pci_get_word(dev-config + msi_flags_off(dev));
+bool msi64bit = flags PCI_MSI_FLAGS_64BIT;
+MSIMessage msg;
+
+if (msi64bit) {
+msg.address = pci_get_quad(dev-config + msi_address_lo_off(dev));
+} else {
+msg.address = pci_get_long(dev-config + msi_address_lo_off(dev));
+}
+msg.data = pci_get_word(dev-config + msi_data_off(dev, msi64bit));
And I have this here in addition:
unsigned int nr_vectors = msi_nr_vectors(flags);
...
if (nr_vectors 1) {
msg.data = ~(nr_vectors - 1);
msg.data |= vector;
}
See PCI spec and existing code.
What for? I really do not get it why someone might want to read something
but not real value.
What PCI code should I look?
I'm not sure what your use case for reading the message is. For KVM
device assignment it is preparing an alternative message delivery path
for MSI vectors. And for this we will need vector notifier support for
MSI as well. You can check the MSI-X code for corresponding use cases of
msix_get_message.
And when we already have msi_get_message, another logical use case is
msi_notify. See msix.c again.
Aaaa.
I have no case for reading the message. All I need is writing. And I want it
public as I want to use
it from hw/spapr_pci.c. You suggested to add reading, I added get to be
_symmetric_ to set
(get returns what set wrote). You want a different thing which I can do but
it is not
msi_get_message(), it is something like msi_prepare_message(MSImessage msg) or
msi_set_vector(uint16_t data) or simply internal kitchen of msi_notify().
Still can do what you suggested, it just does not seem right.
--
Alexey
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] msi/msix: added public API to set/get MSI message address, and data
On 2012-06-21 12:50, Alexey Kardashevskiy wrote:
On 21/06/12 20:38, Jan Kiszka wrote:
On 2012-06-21 12:28, Alexey Kardashevskiy wrote:
On 21/06/12 17:39, Jan Kiszka wrote:
On 2012-06-21 09:18, Alexey Kardashevskiy wrote:
agrhhh. sha1 of the patch changed after rebasing :)
Added (msi|msix)_(set|get)_message() function for whoever might
want to use them.
Currently msi_notify()/msix_notify() write to these vectors to
signal the guest about an interrupt so the correct values have to
written there by the guest or QEMU.
For example, POWER guest never initializes MSI/MSIX vectors, instead
it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
POWER we have to initialize MSI/MSIX message from QEMU.
As only set* function are required by now, the get functions were added
or made public for a symmetry.
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
hw/msi.c | 29 +
hw/msi.h |2 ++
hw/msix.c | 11 ++-
hw/msix.h |3 +++
4 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/hw/msi.c b/hw/msi.c
index 5233204..9ad84a4 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -105,6 +105,35 @@ static inline uint8_t msi_pending_off(const
PCIDevice* dev, bool msi64bit)
return dev-msi_cap + (msi64bit ? PCI_MSI_PENDING_64 :
PCI_MSI_PENDING_32);
}
+MSIMessage msi_get_message(PCIDevice *dev)
MSIMessage msi_get_message(PCIDevice *dev, unsigned vector)
Who/how/why is going to calculate the vector here?
+{
+uint16_t flags = pci_get_word(dev-config + msi_flags_off(dev));
+bool msi64bit = flags PCI_MSI_FLAGS_64BIT;
+MSIMessage msg;
+
+if (msi64bit) {
+msg.address = pci_get_quad(dev-config +
msi_address_lo_off(dev));
+} else {
+msg.address = pci_get_long(dev-config +
msi_address_lo_off(dev));
+}
+msg.data = pci_get_word(dev-config + msi_data_off(dev, msi64bit));
And I have this here in addition:
unsigned int nr_vectors = msi_nr_vectors(flags);
...
if (nr_vectors 1) {
msg.data = ~(nr_vectors - 1);
msg.data |= vector;
}
See PCI spec and existing code.
What for? I really do not get it why someone might want to read something
but not real value.
What PCI code should I look?
I'm not sure what your use case for reading the message is. For KVM
device assignment it is preparing an alternative message delivery path
for MSI vectors. And for this we will need vector notifier support for
MSI as well. You can check the MSI-X code for corresponding use cases of
msix_get_message.
And when we already have msi_get_message, another logical use case is
msi_notify. See msix.c again.
Aaaa.
I have no case for reading the message. All I need is writing. And I want it
public as I want to use
it from hw/spapr_pci.c. You suggested to add reading, I added get to be
_symmetric_ to set
(get returns what set wrote). You want a different thing which I can do
but it is not
msi_get_message(), it is something like msi_prepare_message(MSImessage msg) or
msi_set_vector(uint16_t data) or simply internal kitchen of msi_notify().
Still can do what you suggested, it just does not seem right.
It is right - when looking at it from a different angle. ;)
I don't mind if you add msi_get_message now or leave this to me. Likely
the latter is better as you have no use case for msi_get_message (and
also msix_get_message!) outside of their modules, thus we should not
export those functions anyway.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] msi/msix: added public API to set/get MSI message address, and data
On Thu, 2012-06-21 at 12:56 +0200, Jan Kiszka wrote:
On 2012-06-21 12:50, Alexey Kardashevskiy wrote:
On 21/06/12 20:38, Jan Kiszka wrote:
On 2012-06-21 12:28, Alexey Kardashevskiy wrote:
On 21/06/12 17:39, Jan Kiszka wrote:
On 2012-06-21 09:18, Alexey Kardashevskiy wrote:
agrhhh. sha1 of the patch changed after rebasing :)
Added (msi|msix)_(set|get)_message() function for whoever might
want to use them.
Currently msi_notify()/msix_notify() write to these vectors to
signal the guest about an interrupt so the correct values have to
written there by the guest or QEMU.
For example, POWER guest never initializes MSI/MSIX vectors, instead
it uses RTAS hypercalls. So in order to support MSIX for virtio-pci on
POWER we have to initialize MSI/MSIX message from QEMU.
As only set* function are required by now, the get functions were
added
or made public for a symmetry.
Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
hw/msi.c | 29 +
hw/msi.h |2 ++
hw/msix.c | 11 ++-
hw/msix.h |3 +++
4 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/hw/msi.c b/hw/msi.c
index 5233204..9ad84a4 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -105,6 +105,35 @@ static inline uint8_t msi_pending_off(const
PCIDevice* dev, bool msi64bit)
return dev-msi_cap + (msi64bit ? PCI_MSI_PENDING_64 :
PCI_MSI_PENDING_32);
}
+MSIMessage msi_get_message(PCIDevice *dev)
MSIMessage msi_get_message(PCIDevice *dev, unsigned vector)
Who/how/why is going to calculate the vector here?
+{
+uint16_t flags = pci_get_word(dev-config + msi_flags_off(dev));
+bool msi64bit = flags PCI_MSI_FLAGS_64BIT;
+MSIMessage msg;
+
+if (msi64bit) {
+msg.address = pci_get_quad(dev-config +
msi_address_lo_off(dev));
+} else {
+msg.address = pci_get_long(dev-config +
msi_address_lo_off(dev));
+}
+msg.data = pci_get_word(dev-config + msi_data_off(dev, msi64bit));
And I have this here in addition:
unsigned int nr_vectors = msi_nr_vectors(flags);
...
if (nr_vectors 1) {
msg.data = ~(nr_vectors - 1);
msg.data |= vector;
}
See PCI spec and existing code.
What for? I really do not get it why someone might want to read something
but not real value.
What PCI code should I look?
I'm not sure what your use case for reading the message is. For KVM
device assignment it is preparing an alternative message delivery path
for MSI vectors. And for this we will need vector notifier support for
MSI as well. You can check the MSI-X code for corresponding use cases of
msix_get_message.
And when we already have msi_get_message, another logical use case is
msi_notify. See msix.c again.
Aaaa.
I have no case for reading the message. All I need is writing. And I want
it public as I want to use
it from hw/spapr_pci.c. You suggested to add reading, I added get to be
_symmetric_ to set
(get returns what set wrote). You want a different thing which I can do
but it is not
msi_get_message(), it is something like msi_prepare_message(MSImessage msg)
or
msi_set_vector(uint16_t data) or simply internal kitchen of msi_notify().
Still can do what you suggested, it just does not seem right.
It is right - when looking at it from a different angle. ;)
I don't mind if you add msi_get_message now or leave this to me. Likely
the latter is better as you have no use case for msi_get_message (and
also msix_get_message!) outside of their modules, thus we should not
export those functions anyway.
Curse my timezone for not getting to respond before Alexey dropped the
'get' function. msi_get_message(PCIDevice *dev, int vector) is
necessary for vfio so that we can make use of irqfds in KVM. I've
actually got this function coded into vfio for the moment because it's
so much fun to get pci changes into qemu. When MSI gets enabled, we
setup an eventfd in vfio for each vector (linux only supports a single
MSI vector, but we pretend we can support the fully specification). If
we can enabled irqfd, we walk through each vector, getting an MSIMessage
to program into KVM, which sets the MSI to be injected when the eventfd
fires. MSI is non-symmetric because the vectors have to be consecutive.
So it doesn't make sense to have a 'set' with a vector number, but a
'get' with the vector number is useful. Perhaps it should be
msi_set_message(PCIDevice *dev, MSIMessage *msg, int nr_vectors), but I
don't know how POWER is using it either. Thanks,
Alex
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
