From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:57606) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sf2RO-0001nP-Uq for qemu-devel@nongnu.org; Thu, 14 Jun 2012 01:17:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Sf2RM-0002PW-WE for qemu-devel@nongnu.org; Thu, 14 Jun 2012 01:17:54 -0400 Received: from mail-yw0-f45.google.com ([209.85.213.45]:47574) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sf2RM-0002PN-Ql for qemu-devel@nongnu.org; Thu, 14 Jun 2012 01:17:52 -0400 Received: by yhoo21 with SMTP id o21so1296643yho.4 for ; Wed, 13 Jun 2012 22:17:51 -0700 (PDT) Message-ID: <4FD973F7.7080207@ozlabs.ru> Date: Thu, 14 Jun 2012 15:17:43 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <4FD968BB.2000505@ozlabs.ru> <4FD9693E.2090508@ozlabs.ru> <1339649800.24818.3.camel@ul30vt> In-Reply-To: <1339649800.24818.3.camel@ul30vt> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/3] msi/msix: added functions to API to set up message address and data List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: "qemu-devel@nongnu.org" , kvm-ppc@vger.kernel.org On 14/06/12 14:56, Alex Williamson wrote: > On Thu, 2012-06-14 at 14:31 +1000, Alexey Kardashevskiy wrote: >> 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 >> --- >> hw/msi.c | 14 ++++++++++++++ >> hw/msi.h | 1 + >> hw/msix.c | 10 ++++++++++ >> hw/msix.h | 3 +++ >> 4 files changed, 28 insertions(+), 0 deletions(-) >> >> diff --git a/hw/msi.c b/hw/msi.c >> index 5d6ceb6..124878a 100644 >> --- a/hw/msi.c >> +++ b/hw/msi.c >> @@ -358,3 +358,17 @@ unsigned int msi_nr_vectors_allocated(const PCIDevice *dev) >> uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev)); >> return msi_nr_vectors(flags); >> } >> + >> +void msi_set_address_data(PCIDevice *dev, uint64_t address, uint16_t data) >> +{ >> + 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), address); >> + } else { >> + pci_set_long(dev->config + msi_address_lo_off(dev), address); >> + } >> + pci_set_word(dev->config + msi_data_off(dev, msi64bit), data); >> +} > > Why not make it msi_set_message() and pass MSIMessage? I'd be great if > you tossed in a msi_get_message() as well, I think we need it to be able > to do a kvm_irqchip_add_msi_route() with MSI. Thanks, I am missing the point. What is that MSIMessage? It is just an address and data, making a struct from this is a bit too much :) I am totally unfamiliar with kvm_irqchip_add_msi_route to see the bigger picture, sorry. > Alex > >> + >> diff --git a/hw/msi.h b/hw/msi.h >> index 3040bb0..0acf434 100644 >> --- a/hw/msi.h >> +++ b/hw/msi.h >> @@ -34,6 +34,7 @@ void msi_reset(PCIDevice *dev); >> void msi_notify(PCIDevice *dev, unsigned int vector); >> void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len); >> unsigned int msi_nr_vectors_allocated(const PCIDevice *dev); >> +void msi_set_address_data(PCIDevice *dev, uint64_t address, uint16_t data); >> >> static inline bool msi_present(const PCIDevice *dev) >> { >> diff --git a/hw/msix.c b/hw/msix.c >> index 3835eaa..c57c299 100644 >> --- a/hw/msix.c >> +++ b/hw/msix.c >> @@ -414,3 +414,13 @@ void msix_unuse_all_vectors(PCIDevice *dev) >> return; >> msix_free_irq_entries(dev); >> } >> + >> +void msix_set_address_data(PCIDevice *dev, int vector, >> + uint64_t address, uint32_t data) >> +{ >> + uint8_t *table_entry = dev->msix_table_page + vector * PCI_MSIX_ENTRY_SIZE; >> + pci_set_quad(table_entry + PCI_MSIX_ENTRY_LOWER_ADDR, address); >> + pci_set_long(table_entry + PCI_MSIX_ENTRY_DATA, data); >> + table_entry[PCI_MSIX_ENTRY_VECTOR_CTRL] &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT; >> +} >> + >> diff --git a/hw/msix.h b/hw/msix.h >> index 5aba22b..e6bb696 100644 >> --- a/hw/msix.h >> +++ b/hw/msix.h >> @@ -29,4 +29,7 @@ void msix_notify(PCIDevice *dev, unsigned vector); >> >> void msix_reset(PCIDevice *dev); >> >> +void msix_set_address_data(PCIDevice *dev, int vector, >> + uint64_t address, uint32_t data); >> + >> #endif > > > -- Alexey From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexey Kardashevskiy Date: Thu, 14 Jun 2012 05:17:43 +0000 Subject: Re: [Qemu-devel] [PATCH 1/3] msi/msix: added functions to API to set up message address and data Message-Id: <4FD973F7.7080207@ozlabs.ru> List-Id: References: <4FD968BB.2000505@ozlabs.ru> <4FD9693E.2090508@ozlabs.ru> <1339649800.24818.3.camel@ul30vt> In-Reply-To: <1339649800.24818.3.camel@ul30vt> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Alex Williamson Cc: "qemu-devel@nongnu.org" , kvm-ppc@vger.kernel.org On 14/06/12 14:56, Alex Williamson wrote: > On Thu, 2012-06-14 at 14:31 +1000, Alexey Kardashevskiy wrote: >> 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 >> --- >> hw/msi.c | 14 ++++++++++++++ >> hw/msi.h | 1 + >> hw/msix.c | 10 ++++++++++ >> hw/msix.h | 3 +++ >> 4 files changed, 28 insertions(+), 0 deletions(-) >> >> diff --git a/hw/msi.c b/hw/msi.c >> index 5d6ceb6..124878a 100644 >> --- a/hw/msi.c >> +++ b/hw/msi.c >> @@ -358,3 +358,17 @@ unsigned int msi_nr_vectors_allocated(const PCIDevice *dev) >> uint16_t flags = pci_get_word(dev->config + msi_flags_off(dev)); >> return msi_nr_vectors(flags); >> } >> + >> +void msi_set_address_data(PCIDevice *dev, uint64_t address, uint16_t data) >> +{ >> + 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), address); >> + } else { >> + pci_set_long(dev->config + msi_address_lo_off(dev), address); >> + } >> + pci_set_word(dev->config + msi_data_off(dev, msi64bit), data); >> +} > > Why not make it msi_set_message() and pass MSIMessage? I'd be great if > you tossed in a msi_get_message() as well, I think we need it to be able > to do a kvm_irqchip_add_msi_route() with MSI. Thanks, I am missing the point. What is that MSIMessage? It is just an address and data, making a struct from this is a bit too much :) I am totally unfamiliar with kvm_irqchip_add_msi_route to see the bigger picture, sorry. > Alex > >> + >> diff --git a/hw/msi.h b/hw/msi.h >> index 3040bb0..0acf434 100644 >> --- a/hw/msi.h >> +++ b/hw/msi.h >> @@ -34,6 +34,7 @@ void msi_reset(PCIDevice *dev); >> void msi_notify(PCIDevice *dev, unsigned int vector); >> void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len); >> unsigned int msi_nr_vectors_allocated(const PCIDevice *dev); >> +void msi_set_address_data(PCIDevice *dev, uint64_t address, uint16_t data); >> >> static inline bool msi_present(const PCIDevice *dev) >> { >> diff --git a/hw/msix.c b/hw/msix.c >> index 3835eaa..c57c299 100644 >> --- a/hw/msix.c >> +++ b/hw/msix.c >> @@ -414,3 +414,13 @@ void msix_unuse_all_vectors(PCIDevice *dev) >> return; >> msix_free_irq_entries(dev); >> } >> + >> +void msix_set_address_data(PCIDevice *dev, int vector, >> + uint64_t address, uint32_t data) >> +{ >> + uint8_t *table_entry = dev->msix_table_page + vector * PCI_MSIX_ENTRY_SIZE; >> + pci_set_quad(table_entry + PCI_MSIX_ENTRY_LOWER_ADDR, address); >> + pci_set_long(table_entry + PCI_MSIX_ENTRY_DATA, data); >> + table_entry[PCI_MSIX_ENTRY_VECTOR_CTRL] &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT; >> +} >> + >> diff --git a/hw/msix.h b/hw/msix.h >> index 5aba22b..e6bb696 100644 >> --- a/hw/msix.h >> +++ b/hw/msix.h >> @@ -29,4 +29,7 @@ void msix_notify(PCIDevice *dev, unsigned vector); >> >> void msix_reset(PCIDevice *dev); >> >> +void msix_set_address_data(PCIDevice *dev, int vector, >> + uint64_t address, uint32_t data); >> + >> #endif > > > -- Alexey