On Wed, 27 Aug 2014 18:17:12 +0200
Greg Kurz <gk...@linux.vnet.ibm.com> wrote:
> On sPAPR, virtio devices are connected to the PCI bus and use MSI-X.
> Commit cc943c36faa192cd4b32af8fe5edb31894017d35 has modified MSI-X
> so that writes are made using the bus master address space.
...and follow the IOMMU path.
> Unfortunately, this address space does not have a MSI window: the
s/this/the IOMMU address space/
Sorry :)
--
Greg
> notification is silently dropped in unassigned_mem_write instead
> of reaching the guest... The most visible effect is that all
> virtio devices are non-fonctionnal on sPAPR since then. :(
>
> This patch does the following:
> 1) map the MSI window into the IOMMU address space for each PHB
> - since each PHB instantiates its own IOMMU address space, we
> can safely map the window at a fixed address (SPAPR_PCI_MSI_WINDOW)
> - no real need to keep the MSI window setup in a separate function,
> the spapr_pci_msi_init() code moves to spapr_phb_realize().
>
> 2) kill the global MSI window as it is not needed in the end
>
> Signed-off-by: Greg Kurz <gk...@linux.vnet.ibm.com>
> ---
> hw/ppc/spapr.c | 1 -
> hw/ppc/spapr_pci.c | 53
> +++++++++++++++++++------------------------
> include/hw/pci-host/spapr.h | 2 +-
> include/hw/ppc/spapr.h | 2 --
> 4 files changed, 25 insertions(+), 33 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d01978f..4196a70 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1377,7 +1377,6 @@ static void ppc_spapr_init(MachineState *machine)
> spapr_create_nvram(spapr);
>
> /* Set up PCI */
> - spapr_pci_msi_init(spapr, SPAPR_PCI_MSI_WINDOW);
> spapr_pci_rtas_init();
>
> phb = spapr_create_phb(spapr, 0);
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 9ed39a9..dadba5f 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -341,7 +341,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu,
> sPAPREnvironment *spapr,
> }
>
> /* Setup MSI/MSIX vectors in the device (via cfgspace or MSIX BAR) */
> - spapr_msi_setmsg(pdev, spapr->msi_win_addr, ret_intr_type ==
> RTAS_TYPE_MSIX,
> + spapr_msi_setmsg(pdev, SPAPR_PCI_MSI_WINDOW, ret_intr_type ==
> RTAS_TYPE_MSIX,
> irq, req_num);
>
> /* Add MSI device to cache */
> @@ -465,34 +465,6 @@ static const MemoryRegionOps spapr_msi_ops = {
> .endianness = DEVICE_LITTLE_ENDIAN
> };
>
> -void spapr_pci_msi_init(sPAPREnvironment *spapr, hwaddr addr)
> -{
> - uint64_t window_size = 4096;
> -
> - /*
> - * As MSI/MSIX interrupts trigger by writing at MSI/MSIX vectors,
> - * we need to allocate some memory to catch those writes coming
> - * from msi_notify()/msix_notify().
> - * As MSIMessage:addr is going to be the same and MSIMessage:data
> - * is going to be a VIRQ number, 4 bytes of the MSI MR will only
> - * be used.
> - *
> - * For KVM we want to ensure that this memory is a full page so that
> - * our memory slot is of page size granularity.
> - */
> -#ifdef CONFIG_KVM
> - if (kvm_enabled()) {
> - window_size = getpagesize();
> - }
> -#endif
> -
> - spapr->msi_win_addr = addr;
> - memory_region_init_io(&spapr->msiwindow, NULL, &spapr_msi_ops, spapr,
> - "msi", window_size);
> - memory_region_add_subregion(get_system_memory(), spapr->msi_win_addr,
> - &spapr->msiwindow);
> -}
> -
> /*
> * PHB PCI device
> */
> @@ -512,6 +484,7 @@ static void spapr_phb_realize(DeviceState *dev, Error
> **errp)
> char *namebuf;
> int i;
> PCIBus *bus;
> + uint64_t msi_window_size = 4096;
>
> if (sphb->index != -1) {
> hwaddr windows_base;
> @@ -604,6 +577,28 @@ static void spapr_phb_realize(DeviceState *dev, Error
> **errp)
> address_space_init(&sphb->iommu_as, &sphb->iommu_root,
> sphb->dtbusname);
>
> + /*
> + * As MSI/MSIX interrupts trigger by writing at MSI/MSIX vectors,
> + * we need to allocate some memory to catch those writes coming
> + * from msi_notify()/msix_notify().
> + * As MSIMessage:addr is going to be the same and MSIMessage:data
> + * is going to be a VIRQ number, 4 bytes of the MSI MR will only
> + * be used.
> + *
> + * For KVM we want to ensure that this memory is a full page so that
> + * our memory slot is of page size granularity.
> + */
> +#ifdef CONFIG_KVM
> + if (kvm_enabled()) {
> + msi_window_size = getpagesize();
> + }
> +#endif
> +
> + memory_region_init_io(&sphb->msiwindow, NULL, &spapr_msi_ops, spapr,
> + "msi", msi_window_size);
> + memory_region_add_subregion(&sphb->iommu_root, SPAPR_PCI_MSI_WINDOW,
> + &sphb->msiwindow);
> +
> pci_setup_iommu(bus, spapr_pci_dma_iommu, sphb);
>
> pci_bus_set_route_irq_fn(bus, spapr_route_intx_pin_to_irq);
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 32f0aa7..4ea2a0d 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -70,7 +70,7 @@ struct sPAPRPHBState {
>
> MemoryRegion memspace, iospace;
> hwaddr mem_win_addr, mem_win_size, io_win_addr, io_win_size;
> - MemoryRegion memwindow, iowindow;
> + MemoryRegion memwindow, iowindow, msiwindow;
>
> uint32_t dma_liobn;
> AddressSpace iommu_as;
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index bbba51a..832ad6b 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -13,8 +13,6 @@ struct sPAPRNVRAM;
> typedef struct sPAPREnvironment {
> struct VIOsPAPRBus *vio_bus;
> QLIST_HEAD(, sPAPRPHBState) phbs;
> - hwaddr msi_win_addr;
> - MemoryRegion msiwindow;
> struct sPAPRNVRAM *nvram;
> XICSState *icp;
>
>
>
--
Gregory Kurz kurzg...@fr.ibm.com
gk...@linux.vnet.ibm.com
Software Engineer @ IBM/Meiosys http://www.ibm.com
Tel +33 (0)562 165 496
"Anarchy is about taking complete responsibility for yourself."
Alan Moore.
