| Message ID | 4FAE07C8.7000502@ozlabs.ru |
|---|---|
| State | New |
| Headers | show |
Ping? On 12/05/12 16:48, Alexey Kardashevskiy wrote: > Added msi_set_address_data and msix_set_address_data in order to support > sPAPR-specific implementation of MSI/MSIX configuration mechanism. > POWER guest does not initialize these fields, it expects a supervisor to > get them initialized. > > Required for VFIO on POWER. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > 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); > +} > + > 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 > > >
On Wed, 2012-06-13 at 12:44 +1000, Alexey Kardashevskiy wrote: > Ping? FYI. The way it normally works on pseries is that RTAS, which is the part of the firmware that lives inside the partition alongside the OS, performs the accesses to configure the MSI/MSI-X. With qemu "pseries" machine we don't have such an RTAS currently, it's all implemented by qemu (or rather we have a tiny 5 instructions RTAS which does a hypercall to qemu). So we need to be able to do the configuration from within qemu. Alexey: Can this be used for supporting MSI/MSI-X with emulated devices and virtio as well ? Any chance you can do patches for that too ? Cheers, Ben. > > On 12/05/12 16:48, Alexey Kardashevskiy wrote: > > Added msi_set_address_data and msix_set_address_data in order to support > > sPAPR-specific implementation of MSI/MSIX configuration mechanism. > > POWER guest does not initialize these fields, it expects a supervisor to > > get them initialized. > > > > Required for VFIO on POWER. > > > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > --- > > 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); > > +} > > + > > 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 > > > > > > > >
On 13/06/12 13:22, Benjamin Herrenschmidt wrote: > On Wed, 2012-06-13 at 12:44 +1000, Alexey Kardashevskiy wrote: >> Ping? > > FYI. The way it normally works on pseries is that RTAS, which is the > part of the firmware that lives inside the partition alongside the OS, > performs the accesses to configure the MSI/MSI-X. > > With qemu "pseries" machine we don't have such an RTAS currently, it's > all implemented by qemu (or rather we have a tiny 5 instructions RTAS > which does a hypercall to qemu). > > So we need to be able to do the configuration from within qemu. > > Alexey: Can this be used for supporting MSI/MSI-X with emulated devices > and virtio as well ? This is exactly to support MSIX on virtio-pci devices as there were no other MSIX users on power :) VFIO came up later (and it does not require any change in spapr-msix code). > Any chance you can do patches for that too ? I am polishing descriptions now and post them a bit later. > Cheers, > Ben. > >> >> On 12/05/12 16:48, Alexey Kardashevskiy wrote: >>> Added msi_set_address_data and msix_set_address_data in order to support >>> sPAPR-specific implementation of MSI/MSIX configuration mechanism. >>> POWER guest does not initialize these fields, it expects a supervisor to >>> get them initialized. >>> >>> Required for VFIO on POWER. >>> >>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >>> --- >>> 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); >>> +} >>> + >>> 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
On Wed, 2012-06-13 at 13:39 +1000, Alexey Kardashevskiy wrote: > On 13/06/12 13:22, Benjamin Herrenschmidt wrote: > > On Wed, 2012-06-13 at 12:44 +1000, Alexey Kardashevskiy wrote: > >> Ping? > > > > FYI. The way it normally works on pseries is that RTAS, which is the > > part of the firmware that lives inside the partition alongside the OS, > > performs the accesses to configure the MSI/MSI-X. > > > > With qemu "pseries" machine we don't have such an RTAS currently, it's > > all implemented by qemu (or rather we have a tiny 5 instructions RTAS > > which does a hypercall to qemu). > > > > So we need to be able to do the configuration from within qemu. > > > > Alexey: Can this be used for supporting MSI/MSI-X with emulated devices > > and virtio as well ? > > This is exactly to support MSIX on virtio-pci devices as there were no other MSIX users on power :) > VFIO came up later (and it does not require any change in spapr-msix code). Then you should have mentioned that in the changeset comments :-) > > Any chance you can do patches for that too ? > > I am polishing descriptions now and post them a bit later. Ok, thanks. Cheers, Ben. > > > > Cheers, > > Ben. > > > >> > >> On 12/05/12 16:48, Alexey Kardashevskiy wrote: > >>> Added msi_set_address_data and msix_set_address_data in order to support > >>> sPAPR-specific implementation of MSI/MSIX configuration mechanism. > >>> POWER guest does not initialize these fields, it expects a supervisor to > >>> get them initialized. > >>> > >>> Required for VFIO on POWER. > >>> > >>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > >>> --- > >>> 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); > >>> +} > >>> + > >>> 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 > > > >
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); +} + 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
Added msi_set_address_data and msix_set_address_data in order to support sPAPR-specific implementation of MSI/MSIX configuration mechanism. POWER guest does not initialize these fields, it expects a supervisor to get them initialized. Required for VFIO on POWER. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- hw/msi.c | 14 ++++++++++++++ hw/msi.h | 1 + hw/msix.c | 10 ++++++++++ hw/msix.h | 3 +++ 4 files changed, 28 insertions(+), 0 deletions(-)