From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54775) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ShsHV-0003N1-4a for qemu-devel@nongnu.org; Thu, 21 Jun 2012 21:03:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ShsHQ-0003pP-N5 for qemu-devel@nongnu.org; Thu, 21 Jun 2012 21:03:24 -0400 Received: from mail-pz0-f45.google.com ([209.85.210.45]:47415) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ShsHQ-0003on-EE for qemu-devel@nongnu.org; Thu, 21 Jun 2012 21:03:20 -0400 Received: by dadn2 with SMTP id n2so1659167dad.4 for ; Thu, 21 Jun 2012 18:03:18 -0700 (PDT) Message-ID: <4FE3C44D.9050207@ozlabs.ru> Date: Fri, 22 Jun 2012 11:03:09 +1000 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <4FD968BB.2000505@ozlabs.ru> <4FD9693E.2090508@ozlabs.ru> <1339649800.24818.3.camel@ul30vt> <4FD973F7.7080207@ozlabs.ru> <4FD97A94.2080709@siemens.com> <4FE2C33E.1080808@ozlabs.ru> <4FE2C4DA.40403@siemens.com> <4FE2CABB.4070203@ozlabs.ru> <4FE2CFC8.509@siemens.com> <4FE2F756.8020509@ozlabs.ru> <4FE2F9AF.2050006@siemens.com> <4FE2FC5D.8040503@ozlabs.ru> <4FE2FDE1.1090401@siemens.com> <4FE307DE.5070002@ozlabs.ru> <4FE30A2F.4030803@siemens.com> In-Reply-To: <4FE30A2F.4030803@siemens.com> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] msi/msix: added API to set MSI message address and data List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Alex Williamson , "qemu-devel@nongnu.org" , "kvm-ppc@vger.kernel.org" On 21/06/12 21:49, Jan Kiszka wrote: > On 2012-06-21 13:39, Alexey Kardashevskiy wrote: >> Added (msi|msix)_set_message() functions. >> >> 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. >> >> Signed-off-by: Alexey Kardashevskiy >> --- >> hw/msi.c | 13 +++++++++++++ >> hw/msi.h | 1 + >> hw/msix.c | 9 +++++++++ >> hw/msix.h | 2 ++ >> 4 files changed, 25 insertions(+) >> >> diff --git a/hw/msi.c b/hw/msi.c >> index 5233204..cc6102f 100644 >> --- a/hw/msi.c >> +++ b/hw/msi.c >> @@ -105,6 +105,19 @@ 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); >> } >> >> +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..6ec1f99 100644 >> --- a/hw/msi.h >> +++ b/hw/msi.h >> @@ -31,6 +31,7 @@ struct MSIMessage { >> >> extern bool msi_supported; >> >> +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..5f7d6d3 100644 >> --- a/hw/msix.c >> +++ b/hw/msix.c >> @@ -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..26a437e 100644 >> --- a/hw/msix.h >> +++ b/hw/msix.h >> @@ -4,6 +4,8 @@ >> #include "qemu-common.h" >> #include "pci.h" >> >> +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); >> > > Interface looks good as fas as I can tell (can't asses the POWER need > for clearing the mask bit on msix_set_message). I do not know exactly how x86 works (who/how allocates addresses for MSI/MSIX). On POWER at the moment I did the following thing in QEMU: - registered memory_region_init_io at some big address which the guest won't use, it is just for QEMU - put address from the previous step to the MSIX BAR via msix_set_message() when msi is being configured - then the sequence looks like: - vfio_msi_interrupt() calls msix_notify() - msix_notify() checks if it is masked via msix_is_masked() - and here PCI_MSIX_ENTRY_CTRL_MASKBIT must be unset - stl_le_phys() - here I get a notification in my MemoryRegionOps::write() and do qemu_irq_pulse() 2 reasons to do that: 1) I did not have to change either msix or vfio - cool for submitting patches; 2) neither POWER guest or qemu changes the msi or msix PCI config (it is done by different mechanism called RTAS), so I have to do this myself to support 1) and I do not have to care about someone breaking my settings >> -- >> 1.7.10 >> >> ps. double '-' and git version is an end-of-patch scissor as I read somewhere, cannot recall where exactly > > Check man git-am. Ahhh. Confused end-of-message with end-of-patch. I'll repost it. -- Alexey From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexey Kardashevskiy Date: Fri, 22 Jun 2012 01:03:09 +0000 Subject: Re: [PATCH] msi/msix: added API to set MSI message address and data Message-Id: <4FE3C44D.9050207@ozlabs.ru> List-Id: References: <4FD968BB.2000505@ozlabs.ru> <4FD9693E.2090508@ozlabs.ru> <1339649800.24818.3.camel@ul30vt> <4FD973F7.7080207@ozlabs.ru> <4FD97A94.2080709@siemens.com> <4FE2C33E.1080808@ozlabs.ru> <4FE2C4DA.40403@siemens.com> <4FE2CABB.4070203@ozlabs.ru> <4FE2CFC8.509@siemens.com> <4FE2F756.8020509@ozlabs.ru> <4FE2F9AF.2050006@siemens.com> <4FE2FC5D.8040503@ozlabs.ru> <4FE2FDE1.1090401@siemens.com> <4FE307DE.5070002@ozlabs.ru> <4FE30A2F.4030803@siemens.com> In-Reply-To: <4FE30A2F.4030803@siemens.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Jan Kiszka Cc: Alex Williamson , "qemu-devel@nongnu.org" , "kvm-ppc@vger.kernel.org" On 21/06/12 21:49, Jan Kiszka wrote: > On 2012-06-21 13:39, Alexey Kardashevskiy wrote: >> Added (msi|msix)_set_message() functions. >> >> 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. >> >> Signed-off-by: Alexey Kardashevskiy >> --- >> hw/msi.c | 13 +++++++++++++ >> hw/msi.h | 1 + >> hw/msix.c | 9 +++++++++ >> hw/msix.h | 2 ++ >> 4 files changed, 25 insertions(+) >> >> diff --git a/hw/msi.c b/hw/msi.c >> index 5233204..cc6102f 100644 >> --- a/hw/msi.c >> +++ b/hw/msi.c >> @@ -105,6 +105,19 @@ 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); >> } >> >> +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..6ec1f99 100644 >> --- a/hw/msi.h >> +++ b/hw/msi.h >> @@ -31,6 +31,7 @@ struct MSIMessage { >> >> extern bool msi_supported; >> >> +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..5f7d6d3 100644 >> --- a/hw/msix.c >> +++ b/hw/msix.c >> @@ -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..26a437e 100644 >> --- a/hw/msix.h >> +++ b/hw/msix.h >> @@ -4,6 +4,8 @@ >> #include "qemu-common.h" >> #include "pci.h" >> >> +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); >> > > Interface looks good as fas as I can tell (can't asses the POWER need > for clearing the mask bit on msix_set_message). I do not know exactly how x86 works (who/how allocates addresses for MSI/MSIX). On POWER at the moment I did the following thing in QEMU: - registered memory_region_init_io at some big address which the guest won't use, it is just for QEMU - put address from the previous step to the MSIX BAR via msix_set_message() when msi is being configured - then the sequence looks like: - vfio_msi_interrupt() calls msix_notify() - msix_notify() checks if it is masked via msix_is_masked() - and here PCI_MSIX_ENTRY_CTRL_MASKBIT must be unset - stl_le_phys() - here I get a notification in my MemoryRegionOps::write() and do qemu_irq_pulse() 2 reasons to do that: 1) I did not have to change either msix or vfio - cool for submitting patches; 2) neither POWER guest or qemu changes the msi or msix PCI config (it is done by different mechanism called RTAS), so I have to do this myself to support 1) and I do not have to care about someone breaking my settings >> -- >> 1.7.10 >> >> ps. double '-' and git version is an end-of-patch scissor as I read somewhere, cannot recall where exactly > > Check man git-am. Ahhh. Confused end-of-message with end-of-patch. I'll repost it. -- Alexey