From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48892) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sp1YH-00075K-Tc for qemu-devel@nongnu.org; Wed, 11 Jul 2012 14:22:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Sp1YD-0003bH-M2 for qemu-devel@nongnu.org; Wed, 11 Jul 2012 14:22:17 -0400 Received: from cantor2.suse.de ([195.135.220.15]:40639 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sp1YD-0003b7-CD for qemu-devel@nongnu.org; Wed, 11 Jul 2012 14:22:13 -0400 Mime-Version: 1.0 (Apple Message framework v1278) Content-Type: text/plain; charset=koi8-r From: Alexander Graf In-Reply-To: <4FF70B41.1050508@siemens.com> Date: Wed, 11 Jul 2012 20:22:05 +0200 Content-Transfer-Encoding: quoted-printable Message-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> <4FE3C44D.9050207@ozlabs.ru> <4FE3C72B.9000106@ozlabs.ru> <4FF12373.4010805@ozlabs.ru> <4FF14C9E.9030108@siemens.com> <4FF70B41.1050508@siemens.com> 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: Alexey Kardashevskiy , Alex Williamson , qemu-devel qemu-devel , "kvm-ppc@vger.kernel.org" , "Michael S. Tsirkin" On 06.07.2012, at 17:58, Jan Kiszka wrote: > On 2012-07-06 17:36, Alexander Graf wrote: >>=20 >> On 02.07.2012, at 09:24, Jan Kiszka wrote: >>=20 >>> On 2012-07-02 06:28, Alexey Kardashevskiy wrote: >>>> Ping? >>>>=20 >>>>=20 >>>> On 22/06/12 11:15, Alexey Kardashevskiy wrote: >>>>> Added (msi|msix)_set_message() function for whoever might >>>>> want to use them. >>>>>=20 >>>>> 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. >>>>>=20 >>>>> 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. >>>>>=20 >>>>> 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(+) >>>>>=20 >>>>> 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); >>>>> } >>>>>=20 >>>>> +void msi_set_message(PCIDevice *dev, MSIMessage msg) >>>>> +{ >>>>> + uint16_t flags =3D pci_get_word(dev->config + = msi_flags_off(dev)); >>>>> + bool msi64bit =3D 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 { >>>>>=20 >>>>> extern bool msi_supported; >>>>>=20 >>>>> +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; >>>>> } >>>>>=20 >>>>> +void msix_set_message(PCIDevice *dev, int vector, struct = MSIMessage msg) >>>>> +{ >>>>> + uint8_t *table_entry =3D 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] &=3D = ~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" >>>>>=20 >>>>> +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); >>>>=20 >>>>=20 >>>=20 >>> Acked-by: Jan Kiszka >>=20 >> So I take it that you expect this patch to go through my tree, not = yours? >=20 > I suppose you wanted to address Michael, right? He has to decide. Michael? Alex From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Date: Wed, 11 Jul 2012 18:22:05 +0000 Subject: Re: [PATCH] msi/msix: added API to set MSI message address and data Message-Id: 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> <4FE3C44D.9050207@ozlabs.ru> <4FE3C72B.9000106@ozlabs.ru> <4FF12373.4010805@ozlabs.ru> <4FF14C9E.9030108@siemens.com> <4FF70B41.1050508@siemens.com> In-Reply-To: <4FF70B41.1050508@siemens.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Jan Kiszka Cc: Alexey Kardashevskiy , Alex Williamson , qemu-devel qemu-devel , "kvm-ppc@vger.kernel.org" , "Michael S. Tsirkin" On 06.07.2012, at 17:58, Jan Kiszka wrote: > On 2012-07-06 17:36, Alexander Graf wrote: >> >> On 02.07.2012, at 09:24, Jan Kiszka wrote: >> >>> On 2012-07-02 06:28, Alexey Kardashevskiy wrote: >>>> Ping? >>>> >>>> >>>> On 22/06/12 11:15, Alexey Kardashevskiy wrote: >>>>> Added (msi|msix)_set_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. >>>>> >>>>> 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); >>>> >>>> >>> >>> Acked-by: Jan Kiszka >> >> So I take it that you expect this patch to go through my tree, not yours? > > I suppose you wanted to address Michael, right? He has to decide. Michael? Alex