From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40878) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Shexs-0003hA-6r for qemu-devel@nongnu.org; Thu, 21 Jun 2012 06:50:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Shexn-0002lw-CP for qemu-devel@nongnu.org; Thu, 21 Jun 2012 06:50:15 -0400 Received: from mail-pz0-f45.google.com ([209.85.210.45]:34159) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Shexn-0002iy-25 for qemu-devel@nongnu.org; Thu, 21 Jun 2012 06:50:11 -0400 Received: by dadn2 with SMTP id n2so728713dad.4 for ; Thu, 21 Jun 2012 03:50:08 -0700 (PDT) Message-ID: <4FE2FC5D.8040503@ozlabs.ru> Date: Thu, 21 Jun 2012 20:50:05 +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> In-Reply-To: <4FE2F9AF.2050006@siemens.com> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] msi/msix: added public API to set/get 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 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 >>>> --- >>>> 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexey Kardashevskiy Date: Thu, 21 Jun 2012 10:50:05 +0000 Subject: Re: [PATCH] msi/msix: added public API to set/get MSI message address, and data Message-Id: <4FE2FC5D.8040503@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> In-Reply-To: <4FE2F9AF.2050006@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 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 >>>> --- >>>> 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