| Message ID | 1381922582-28724-3-git-send-email-Varun.Sethi@freescale.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
> -----Original Message----- > From: Sethi Varun-B16395 > Sent: Wednesday, October 16, 2013 4:53 PM > To: joro@8bytes.org; iommu@lists.linux-foundation.org; linuxppc- > dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Yoder Stuart-B08248; Wood > Scott-B07421; alex.williamson@redhat.com; Bhushan Bharat-R65777 > Cc: Sethi Varun-B16395 > Subject: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe devices > > Once the PCIe device assigned to a guest VM (via VFIO) gets detached from the > iommu domain (when guest terminates), its PAMU table entry is disabled. So, this > would prevent the device from being used once it's assigned back to the host. > > This patch allows for creation of a default DMA window corresponding to the > device and subsequently enabling the PAMU table entry. Before we enable the > entry, we ensure that the device's bus master capability is disabled (device > quiesced). > > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com> > --- > drivers/iommu/fsl_pamu.c | 43 ++++++++++++++++++++++++++++-------- > drivers/iommu/fsl_pamu.h | 1 + > drivers/iommu/fsl_pamu_domain.c | 46 ++++++++++++++++++++++++++++++++++++--- > 3 files changed, 78 insertions(+), 12 deletions(-) > > diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index > cba0498..fb4a031 100644 > --- a/drivers/iommu/fsl_pamu.c > +++ b/drivers/iommu/fsl_pamu.c > @@ -225,6 +225,21 @@ static struct paace *pamu_get_spaace(struct paace *paace, > u32 wnum) > return spaace; > } > > +/* > + * Defaul PPAACE settings for an LIODN. > + */ > +static void setup_default_ppaace(struct paace *ppaace) { > + pamu_init_ppaace(ppaace); > + /* window size is 2^(WSE+1) bytes */ > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > + ppaace->wbah = 0; > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > + set_bf(ppaace->impl_attr, PAACE_IA_ATM, > + PAACE_ATM_NO_XLATE); > + set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > + PAACE_AP_PERMS_ALL); > +} > /** > * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves subwindows > * required for primary PAACE in the secondary > @@ -253,6 +268,24 @@ static unsigned long pamu_get_fspi_and_allocate(u32 > subwin_cnt) > return (spaace_addr - (unsigned long)spaact) / (sizeof(struct paace)); } > > +/* Reset the PAACE entry to the default state */ void > +enable_default_dma_window(int liodn) { > + struct paace *ppaace; > + > + ppaace = pamu_get_ppaace(liodn); > + if (!ppaace) { > + pr_debug("Invalid liodn entry\n"); > + return; > + } > + > + memset(ppaace, 0, sizeof(struct paace)); > + > + setup_default_ppaace(ppaace); > + mb(); > + pamu_enable_liodn(liodn); > +} > + > /* Release the subwindows reserved for a particular LIODN */ void > pamu_free_subwins(int liodn) { @@ -752,15 +785,7 @@ static void __init > setup_liodns(void) > continue; > } > ppaace = pamu_get_ppaace(liodn); > - pamu_init_ppaace(ppaace); > - /* window size is 2^(WSE+1) bytes */ > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > - ppaace->wbah = 0; > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > - set_bf(ppaace->impl_attr, PAACE_IA_ATM, > - PAACE_ATM_NO_XLATE); > - set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > - PAACE_AP_PERMS_ALL); > + setup_default_ppaace(ppaace); > if (of_device_is_compatible(node, "fsl,qman-portal")) > setup_qbman_paace(ppaace, QMAN_PORTAL_PAACE); > if (of_device_is_compatible(node, "fsl,qman")) diff --git > a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index 8fc1a12..0edcbbbb > 100644 > --- a/drivers/iommu/fsl_pamu.h > +++ b/drivers/iommu/fsl_pamu.h > @@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct device *dev); int > pamu_update_paace_stash(int liodn, u32 subwin, u32 value); int > pamu_disable_spaace(int liodn, u32 subwin); > u32 pamu_get_max_subwin_cnt(void); > +void enable_default_dma_window(int liodn); > > #endif /* __FSL_PAMU_H */ > diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c > index 966ae70..dd6cafc 100644 > --- a/drivers/iommu/fsl_pamu_domain.c > +++ b/drivers/iommu/fsl_pamu_domain.c > @@ -340,17 +340,57 @@ static inline struct device_domain_info > *find_domain(struct device *dev) > return dev->archdata.iommu_domain; > } > > +/* Disable device DMA capability and enable default DMA window */ > +static void disable_device_dma(struct device_domain_info *info, > + int enable_dma_window) > +{ > +#ifdef CONFIG_PCI > + if (info->dev->bus == &pci_bus_type) { > + struct pci_dev *pdev = NULL; > + pdev = to_pci_dev(info->dev); > + if (pci_is_enabled(pdev)) > + pci_disable_device(pdev); > + } > +#endif > + > + if (enable_dma_window) > + enable_default_dma_window(info->liodn); > +} > + > +static int check_for_shared_liodn(struct device_domain_info *info) { > + struct device_domain_info *tmp; > + > + /* > + * Sanity check, to ensure that this is not a > + * shared LIODN. In case of a PCIe controller > + * it's possible that all PCIe devices share > + * the same LIODN. > + */ > + list_for_each_entry(tmp, &info->domain->devices, link) { > + if (info->liodn == tmp->liodn) > + return 1; > + } > + > + return 0; > +} > + > static void remove_device_ref(struct device_domain_info *info, u32 win_cnt) { > unsigned long flags; > + int enable_dma_window = 0; > > list_del(&info->link); > spin_lock_irqsave(&iommu_lock, flags); > - if (win_cnt > 1) > - pamu_free_subwins(info->liodn); > - pamu_disable_liodn(info->liodn); > + if (!check_for_shared_liodn(info)) { One query; Do we really need to check for this? Otherwise this patch series looks good to me. Thanks -Bharat > + if (win_cnt > 1) > + pamu_free_subwins(info->liodn); > + pamu_disable_liodn(info->liodn); > + enable_dma_window = 1; > + } > spin_unlock_irqrestore(&iommu_lock, flags); > spin_lock_irqsave(&device_domain_lock, flags); > + disable_device_dma(info, enable_dma_window); > info->dev->archdata.iommu_domain = NULL; > kmem_cache_free(iommu_devinfo_cache, info); > spin_unlock_irqrestore(&device_domain_lock, flags); > -- > 1.7.9.5
> -----Original Message----- > From: Bhushan Bharat-R65777 > Sent: Wednesday, October 16, 2013 10:20 PM > To: Sethi Varun-B16395; joro@8bytes.org; iommu@lists.linux- > foundation.org; linuxppc-dev@lists.ozlabs.org; linux- > kernel@vger.kernel.org; Yoder Stuart-B08248; Wood Scott-B07421; > alex.williamson@redhat.com > Subject: RE: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe > devices > > > > > -----Original Message----- > > From: Sethi Varun-B16395 > > Sent: Wednesday, October 16, 2013 4:53 PM > > To: joro@8bytes.org; iommu@lists.linux-foundation.org; linuxppc- > > dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Yoder > > Stuart-B08248; Wood Scott-B07421; alex.williamson@redhat.com; Bhushan > > Bharat-R65777 > > Cc: Sethi Varun-B16395 > > Subject: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe > > devices > > > > Once the PCIe device assigned to a guest VM (via VFIO) gets detached > > from the iommu domain (when guest terminates), its PAMU table entry is > > disabled. So, this would prevent the device from being used once it's > assigned back to the host. > > > > This patch allows for creation of a default DMA window corresponding > > to the device and subsequently enabling the PAMU table entry. Before > > we enable the entry, we ensure that the device's bus master capability > > is disabled (device quiesced). > > > > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com> > > --- > > drivers/iommu/fsl_pamu.c | 43 ++++++++++++++++++++++++++++--- > ----- > > drivers/iommu/fsl_pamu.h | 1 + > > drivers/iommu/fsl_pamu_domain.c | 46 > ++++++++++++++++++++++++++++++++++++--- > > 3 files changed, 78 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index > > cba0498..fb4a031 100644 > > --- a/drivers/iommu/fsl_pamu.c > > +++ b/drivers/iommu/fsl_pamu.c > > @@ -225,6 +225,21 @@ static struct paace *pamu_get_spaace(struct paace > > *paace, > > u32 wnum) > > return spaace; > > } > > > > +/* > > + * Defaul PPAACE settings for an LIODN. > > + */ > > +static void setup_default_ppaace(struct paace *ppaace) { > > + pamu_init_ppaace(ppaace); > > + /* window size is 2^(WSE+1) bytes */ > > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > > + ppaace->wbah = 0; > > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > > + set_bf(ppaace->impl_attr, PAACE_IA_ATM, > > + PAACE_ATM_NO_XLATE); > > + set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > > + PAACE_AP_PERMS_ALL); > > +} > > /** > > * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves > subwindows > > * required for primary PAACE in the > secondary > > @@ -253,6 +268,24 @@ static unsigned long > > pamu_get_fspi_and_allocate(u32 > > subwin_cnt) > > return (spaace_addr - (unsigned long)spaact) / (sizeof(struct > > paace)); } > > > > +/* Reset the PAACE entry to the default state */ void > > +enable_default_dma_window(int liodn) { > > + struct paace *ppaace; > > + > > + ppaace = pamu_get_ppaace(liodn); > > + if (!ppaace) { > > + pr_debug("Invalid liodn entry\n"); > > + return; > > + } > > + > > + memset(ppaace, 0, sizeof(struct paace)); > > + > > + setup_default_ppaace(ppaace); > > + mb(); > > + pamu_enable_liodn(liodn); > > +} > > + > > /* Release the subwindows reserved for a particular LIODN */ void > > pamu_free_subwins(int liodn) { @@ -752,15 +785,7 @@ static void > > __init > > setup_liodns(void) > > continue; > > } > > ppaace = pamu_get_ppaace(liodn); > > - pamu_init_ppaace(ppaace); > > - /* window size is 2^(WSE+1) bytes */ > > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > > - ppaace->wbah = 0; > > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > > - set_bf(ppaace->impl_attr, PAACE_IA_ATM, > > - PAACE_ATM_NO_XLATE); > > - set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > > - PAACE_AP_PERMS_ALL); > > + setup_default_ppaace(ppaace); > > if (of_device_is_compatible(node, "fsl,qman-portal")) > > setup_qbman_paace(ppaace, QMAN_PORTAL_PAACE); > > if (of_device_is_compatible(node, "fsl,qman")) diff -- > git > > a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index > > 8fc1a12..0edcbbbb > > 100644 > > --- a/drivers/iommu/fsl_pamu.h > > +++ b/drivers/iommu/fsl_pamu.h > > @@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct device > > *dev); int pamu_update_paace_stash(int liodn, u32 subwin, u32 value); > > int pamu_disable_spaace(int liodn, u32 subwin); > > u32 pamu_get_max_subwin_cnt(void); > > +void enable_default_dma_window(int liodn); > > > > #endif /* __FSL_PAMU_H */ > > diff --git a/drivers/iommu/fsl_pamu_domain.c > > b/drivers/iommu/fsl_pamu_domain.c index 966ae70..dd6cafc 100644 > > --- a/drivers/iommu/fsl_pamu_domain.c > > +++ b/drivers/iommu/fsl_pamu_domain.c > > @@ -340,17 +340,57 @@ static inline struct device_domain_info > > *find_domain(struct device *dev) > > return dev->archdata.iommu_domain; > > } > > > > +/* Disable device DMA capability and enable default DMA window */ > > +static void disable_device_dma(struct device_domain_info *info, > > + int enable_dma_window) > > +{ > > +#ifdef CONFIG_PCI > > + if (info->dev->bus == &pci_bus_type) { > > + struct pci_dev *pdev = NULL; > > + pdev = to_pci_dev(info->dev); > > + if (pci_is_enabled(pdev)) > > + pci_disable_device(pdev); > > + } > > +#endif > > + > > + if (enable_dma_window) > > + enable_default_dma_window(info->liodn); > > +} > > + > > +static int check_for_shared_liodn(struct device_domain_info *info) { > > + struct device_domain_info *tmp; > > + > > + /* > > + * Sanity check, to ensure that this is not a > > + * shared LIODN. In case of a PCIe controller > > + * it's possible that all PCIe devices share > > + * the same LIODN. > > + */ > > + list_for_each_entry(tmp, &info->domain->devices, link) { > > + if (info->liodn == tmp->liodn) > > + return 1; > > + } > > + > > + return 0; > > +} > > + > > static void remove_device_ref(struct device_domain_info *info, u32 > win_cnt) { > > unsigned long flags; > > + int enable_dma_window = 0; > > > > list_del(&info->link); > > spin_lock_irqsave(&iommu_lock, flags); > > - if (win_cnt > 1) > > - pamu_free_subwins(info->liodn); > > - pamu_disable_liodn(info->liodn); > > + if (!check_for_shared_liodn(info)) { > > One query; Do we really need to check for this? > [Sethi Varun-B16395] Yes, just a sanity check to ensure that there are no more devices linked to this LIODN and we can disable it. -Varun > Otherwise this patch series looks good to me. > > Thanks > -Bharat > > > + if (win_cnt > 1) > > + pamu_free_subwins(info->liodn); > > + pamu_disable_liodn(info->liodn); > > + enable_dma_window = 1; > > + } > > spin_unlock_irqrestore(&iommu_lock, flags); > > spin_lock_irqsave(&device_domain_lock, flags); > > + disable_device_dma(info, enable_dma_window); > > info->dev->archdata.iommu_domain = NULL; > > kmem_cache_free(iommu_devinfo_cache, info); > > spin_unlock_irqrestore(&device_domain_lock, flags); > > -- > > 1.7.9.5
> > > > > > > -----Original Message----- > > > From: Sethi Varun-B16395 > > > Sent: Wednesday, October 16, 2013 4:53 PM > > > To: joro@8bytes.org; iommu@lists.linux-foundation.org; linuxppc- > > > dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Yoder > > > Stuart-B08248; Wood Scott-B07421; alex.williamson@redhat.com; > > > Bhushan > > > Bharat-R65777 > > > Cc: Sethi Varun-B16395 > > > Subject: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for > > > PCIe devices > > > > > > Once the PCIe device assigned to a guest VM (via VFIO) gets detached > > > from the iommu domain (when guest terminates), its PAMU table entry > > > is disabled. So, this would prevent the device from being used once > > > it's > > assigned back to the host. > > > > > > This patch allows for creation of a default DMA window corresponding > > > to the device and subsequently enabling the PAMU table entry. Before > > > we enable the entry, we ensure that the device's bus master > > > capability is disabled (device quiesced). > > > > > > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com> > > > --- > > > drivers/iommu/fsl_pamu.c | 43 ++++++++++++++++++++++++++++--- > > ----- > > > drivers/iommu/fsl_pamu.h | 1 + > > > drivers/iommu/fsl_pamu_domain.c | 46 > > ++++++++++++++++++++++++++++++++++++--- > > > 3 files changed, 78 insertions(+), 12 deletions(-) > > > > > > diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c > > > index > > > cba0498..fb4a031 100644 > > > --- a/drivers/iommu/fsl_pamu.c > > > +++ b/drivers/iommu/fsl_pamu.c > > > @@ -225,6 +225,21 @@ static struct paace *pamu_get_spaace(struct > > > paace *paace, > > > u32 wnum) > > > return spaace; > > > } > > > > > > +/* > > > + * Defaul PPAACE settings for an LIODN. > > > + */ > > > +static void setup_default_ppaace(struct paace *ppaace) { > > > + pamu_init_ppaace(ppaace); > > > + /* window size is 2^(WSE+1) bytes */ > > > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > > > + ppaace->wbah = 0; > > > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > > > + set_bf(ppaace->impl_attr, PAACE_IA_ATM, > > > + PAACE_ATM_NO_XLATE); > > > + set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > > > + PAACE_AP_PERMS_ALL); > > > +} > > > /** > > > * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves > > subwindows > > > * required for primary PAACE in the > > secondary > > > @@ -253,6 +268,24 @@ static unsigned long > > > pamu_get_fspi_and_allocate(u32 > > > subwin_cnt) > > > return (spaace_addr - (unsigned long)spaact) / (sizeof(struct > > > paace)); } > > > > > > +/* Reset the PAACE entry to the default state */ void > > > +enable_default_dma_window(int liodn) { > > > + struct paace *ppaace; > > > + > > > + ppaace = pamu_get_ppaace(liodn); > > > + if (!ppaace) { > > > + pr_debug("Invalid liodn entry\n"); > > > + return; > > > + } > > > + > > > + memset(ppaace, 0, sizeof(struct paace)); > > > + > > > + setup_default_ppaace(ppaace); > > > + mb(); > > > + pamu_enable_liodn(liodn); > > > +} > > > + > > > /* Release the subwindows reserved for a particular LIODN */ void > > > pamu_free_subwins(int liodn) { @@ -752,15 +785,7 @@ static void > > > __init > > > setup_liodns(void) > > > continue; > > > } > > > ppaace = pamu_get_ppaace(liodn); > > > - pamu_init_ppaace(ppaace); > > > - /* window size is 2^(WSE+1) bytes */ > > > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > > > - ppaace->wbah = 0; > > > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > > > - set_bf(ppaace->impl_attr, PAACE_IA_ATM, > > > - PAACE_ATM_NO_XLATE); > > > - set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > > > - PAACE_AP_PERMS_ALL); > > > + setup_default_ppaace(ppaace); > > > if (of_device_is_compatible(node, "fsl,qman-portal")) > > > setup_qbman_paace(ppaace, QMAN_PORTAL_PAACE); > > > if (of_device_is_compatible(node, "fsl,qman")) diff -- > > git > > > a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index > > > 8fc1a12..0edcbbbb > > > 100644 > > > --- a/drivers/iommu/fsl_pamu.h > > > +++ b/drivers/iommu/fsl_pamu.h > > > @@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct device > > > *dev); int pamu_update_paace_stash(int liodn, u32 subwin, u32 > > > value); int pamu_disable_spaace(int liodn, u32 subwin); > > > u32 pamu_get_max_subwin_cnt(void); > > > +void enable_default_dma_window(int liodn); > > > > > > #endif /* __FSL_PAMU_H */ > > > diff --git a/drivers/iommu/fsl_pamu_domain.c > > > b/drivers/iommu/fsl_pamu_domain.c index 966ae70..dd6cafc 100644 > > > --- a/drivers/iommu/fsl_pamu_domain.c > > > +++ b/drivers/iommu/fsl_pamu_domain.c > > > @@ -340,17 +340,57 @@ static inline struct device_domain_info > > > *find_domain(struct device *dev) > > > return dev->archdata.iommu_domain; } > > > > > > +/* Disable device DMA capability and enable default DMA window */ > > > +static void disable_device_dma(struct device_domain_info *info, > > > + int enable_dma_window) > > > +{ > > > +#ifdef CONFIG_PCI > > > + if (info->dev->bus == &pci_bus_type) { > > > + struct pci_dev *pdev = NULL; > > > + pdev = to_pci_dev(info->dev); > > > + if (pci_is_enabled(pdev)) > > > + pci_disable_device(pdev); > > > + } > > > +#endif > > > + > > > + if (enable_dma_window) > > > + enable_default_dma_window(info->liodn); > > > +} > > > + > > > +static int check_for_shared_liodn(struct device_domain_info *info) { > > > + struct device_domain_info *tmp; > > > + > > > + /* > > > + * Sanity check, to ensure that this is not a > > > + * shared LIODN. In case of a PCIe controller > > > + * it's possible that all PCIe devices share > > > + * the same LIODN. > > > + */ > > > + list_for_each_entry(tmp, &info->domain->devices, link) { > > > + if (info->liodn == tmp->liodn) > > > + return 1; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > static void remove_device_ref(struct device_domain_info *info, u32 > > win_cnt) { > > > unsigned long flags; > > > + int enable_dma_window = 0; > > > > > > list_del(&info->link); > > > spin_lock_irqsave(&iommu_lock, flags); > > > - if (win_cnt > 1) > > > - pamu_free_subwins(info->liodn); > > > - pamu_disable_liodn(info->liodn); > > > + if (!check_for_shared_liodn(info)) { > > > > One query; Do we really need to check for this? > > > [Sethi Varun-B16395] Yes, just a sanity check to ensure that there are no more > devices linked to this LIODN and we can disable it. Varun, trying to understand this; say there are two device under a PCI controller which share the LIODN of PCI controller, So both of the device must be unbound from kernel driver and then bind both to VFIO. Now when guest terminated then remove_device_ref() will be called for both of device but the sanity check will pass for the one which will be called later, is this right? Thanks -Bharat > > -Varun > > > Otherwise this patch series looks good to me. > > > > Thanks > > -Bharat > > > > > + if (win_cnt > 1) > > > + pamu_free_subwins(info->liodn); > > > + pamu_disable_liodn(info->liodn); > > > + enable_dma_window = 1; > > > + } > > > spin_unlock_irqrestore(&iommu_lock, flags); > > > spin_lock_irqsave(&device_domain_lock, flags); > > > + disable_device_dma(info, enable_dma_window); > > > info->dev->archdata.iommu_domain = NULL; > > > kmem_cache_free(iommu_devinfo_cache, info); > > > spin_unlock_irqrestore(&device_domain_lock, flags); > > > -- > > > 1.7.9.5
> -----Original Message----- > From: Bhushan Bharat-R65777 > Sent: Wednesday, October 16, 2013 10:43 PM > To: Sethi Varun-B16395; joro@8bytes.org; iommu@lists.linux- > foundation.org; linuxppc-dev@lists.ozlabs.org; linux- > kernel@vger.kernel.org; Yoder Stuart-B08248; Wood Scott-B07421; > alex.williamson@redhat.com > Subject: RE: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe > devices > > > > > > > > > > > > > -----Original Message----- > > > > From: Sethi Varun-B16395 > > > > Sent: Wednesday, October 16, 2013 4:53 PM > > > > To: joro@8bytes.org; iommu@lists.linux-foundation.org; linuxppc- > > > > dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Yoder > > > > Stuart-B08248; Wood Scott-B07421; alex.williamson@redhat.com; > > > > Bhushan > > > > Bharat-R65777 > > > > Cc: Sethi Varun-B16395 > > > > Subject: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for > > > > PCIe devices > > > > > > > > Once the PCIe device assigned to a guest VM (via VFIO) gets > > > > detached from the iommu domain (when guest terminates), its PAMU > > > > table entry is disabled. So, this would prevent the device from > > > > being used once it's > > > assigned back to the host. > > > > > > > > This patch allows for creation of a default DMA window > > > > corresponding to the device and subsequently enabling the PAMU > > > > table entry. Before we enable the entry, we ensure that the > > > > device's bus master capability is disabled (device quiesced). > > > > > > > > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com> > > > > --- > > > > drivers/iommu/fsl_pamu.c | 43 > ++++++++++++++++++++++++++++--- > > > ----- > > > > drivers/iommu/fsl_pamu.h | 1 + > > > > drivers/iommu/fsl_pamu_domain.c | 46 > > > ++++++++++++++++++++++++++++++++++++--- > > > > 3 files changed, 78 insertions(+), 12 deletions(-) > > > > > > > > diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c > > > > index > > > > cba0498..fb4a031 100644 > > > > --- a/drivers/iommu/fsl_pamu.c > > > > +++ b/drivers/iommu/fsl_pamu.c > > > > @@ -225,6 +225,21 @@ static struct paace *pamu_get_spaace(struct > > > > paace *paace, > > > > u32 wnum) > > > > return spaace; > > > > } > > > > > > > > +/* > > > > + * Defaul PPAACE settings for an LIODN. > > > > + */ > > > > +static void setup_default_ppaace(struct paace *ppaace) { > > > > + pamu_init_ppaace(ppaace); > > > > + /* window size is 2^(WSE+1) bytes */ > > > > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > > > > + ppaace->wbah = 0; > > > > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > > > > + set_bf(ppaace->impl_attr, PAACE_IA_ATM, > > > > + PAACE_ATM_NO_XLATE); > > > > + set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > > > > + PAACE_AP_PERMS_ALL); > > > > +} > > > > /** > > > > * pamu_get_fspi_and_allocate() - Allocates fspi index and > > > > reserves > > > subwindows > > > > * required for primary PAACE in > the > > > secondary > > > > @@ -253,6 +268,24 @@ static unsigned long > > > > pamu_get_fspi_and_allocate(u32 > > > > subwin_cnt) > > > > return (spaace_addr - (unsigned long)spaact) / (sizeof(struct > > > > paace)); } > > > > > > > > +/* Reset the PAACE entry to the default state */ void > > > > +enable_default_dma_window(int liodn) { > > > > + struct paace *ppaace; > > > > + > > > > + ppaace = pamu_get_ppaace(liodn); > > > > + if (!ppaace) { > > > > + pr_debug("Invalid liodn entry\n"); > > > > + return; > > > > + } > > > > + > > > > + memset(ppaace, 0, sizeof(struct paace)); > > > > + > > > > + setup_default_ppaace(ppaace); > > > > + mb(); > > > > + pamu_enable_liodn(liodn); > > > > +} > > > > + > > > > /* Release the subwindows reserved for a particular LIODN */ > > > > void pamu_free_subwins(int liodn) { @@ -752,15 +785,7 @@ static > > > > void __init > > > > setup_liodns(void) > > > > continue; > > > > } > > > > ppaace = pamu_get_ppaace(liodn); > > > > - pamu_init_ppaace(ppaace); > > > > - /* window size is 2^(WSE+1) bytes */ > > > > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, > 35); > > > > - ppaace->wbah = 0; > > > > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, > 0); > > > > - set_bf(ppaace->impl_attr, PAACE_IA_ATM, > > > > - PAACE_ATM_NO_XLATE); > > > > - set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > > > > - PAACE_AP_PERMS_ALL); > > > > + setup_default_ppaace(ppaace); > > > > if (of_device_is_compatible(node, "fsl,qman- > portal")) > > > > setup_qbman_paace(ppaace, > QMAN_PORTAL_PAACE); > > > > if (of_device_is_compatible(node, "fsl,qman")) > diff -- > > > git > > > > a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index > > > > 8fc1a12..0edcbbbb > > > > 100644 > > > > --- a/drivers/iommu/fsl_pamu.h > > > > +++ b/drivers/iommu/fsl_pamu.h > > > > @@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct > > > > device *dev); int pamu_update_paace_stash(int liodn, u32 subwin, > > > > u32 value); int pamu_disable_spaace(int liodn, u32 subwin); > > > > u32 pamu_get_max_subwin_cnt(void); > > > > +void enable_default_dma_window(int liodn); > > > > > > > > #endif /* __FSL_PAMU_H */ > > > > diff --git a/drivers/iommu/fsl_pamu_domain.c > > > > b/drivers/iommu/fsl_pamu_domain.c index 966ae70..dd6cafc 100644 > > > > --- a/drivers/iommu/fsl_pamu_domain.c > > > > +++ b/drivers/iommu/fsl_pamu_domain.c > > > > @@ -340,17 +340,57 @@ static inline struct device_domain_info > > > > *find_domain(struct device *dev) > > > > return dev->archdata.iommu_domain; } > > > > > > > > +/* Disable device DMA capability and enable default DMA window */ > > > > +static void disable_device_dma(struct device_domain_info *info, > > > > + int enable_dma_window) > > > > +{ > > > > +#ifdef CONFIG_PCI > > > > + if (info->dev->bus == &pci_bus_type) { > > > > + struct pci_dev *pdev = NULL; > > > > + pdev = to_pci_dev(info->dev); > > > > + if (pci_is_enabled(pdev)) > > > > + pci_disable_device(pdev); > > > > + } > > > > +#endif > > > > + > > > > + if (enable_dma_window) > > > > + enable_default_dma_window(info->liodn); > > > > +} > > > > + > > > > +static int check_for_shared_liodn(struct device_domain_info *info) > { > > > > + struct device_domain_info *tmp; > > > > + > > > > + /* > > > > + * Sanity check, to ensure that this is not a > > > > + * shared LIODN. In case of a PCIe controller > > > > + * it's possible that all PCIe devices share > > > > + * the same LIODN. > > > > + */ > > > > + list_for_each_entry(tmp, &info->domain->devices, link) { > > > > + if (info->liodn == tmp->liodn) > > > > + return 1; > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > static void remove_device_ref(struct device_domain_info *info, > > > > u32 > > > win_cnt) { > > > > unsigned long flags; > > > > + int enable_dma_window = 0; > > > > > > > > list_del(&info->link); > > > > spin_lock_irqsave(&iommu_lock, flags); > > > > - if (win_cnt > 1) > > > > - pamu_free_subwins(info->liodn); > > > > - pamu_disable_liodn(info->liodn); > > > > + if (!check_for_shared_liodn(info)) { > > > > > > One query; Do we really need to check for this? > > > > > [Sethi Varun-B16395] Yes, just a sanity check to ensure that there are > > no more devices linked to this LIODN and we can disable it. > > Varun, trying to understand this; say there are two device under a PCI > controller which share the LIODN of PCI controller, So both of the device > must be unbound from kernel driver and then bind both to VFIO. > > Now when guest terminated then remove_device_ref() will be called for > both of device but the sanity check will pass for the one which will be > called later, is this right? > Yes, when the first device is detached PAMU LIODN table entry is not disabled. The LIODN would only be disabled once all devices are detached. -Varun
> > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Sethi Varun-B16395 > > > > > Sent: Wednesday, October 16, 2013 4:53 PM > > > > > To: joro@8bytes.org; iommu@lists.linux-foundation.org; linuxppc- > > > > > dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Yoder > > > > > Stuart-B08248; Wood Scott-B07421; alex.williamson@redhat.com; > > > > > Bhushan > > > > > Bharat-R65777 > > > > > Cc: Sethi Varun-B16395 > > > > > Subject: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for > > > > > PCIe devices > > > > > > > > > > Once the PCIe device assigned to a guest VM (via VFIO) gets > > > > > detached from the iommu domain (when guest terminates), its PAMU > > > > > table entry is disabled. So, this would prevent the device from > > > > > being used once it's > > > > assigned back to the host. > > > > > > > > > > This patch allows for creation of a default DMA window > > > > > corresponding to the device and subsequently enabling the PAMU > > > > > table entry. Before we enable the entry, we ensure that the > > > > > device's bus master capability is disabled (device quiesced). > > > > > > > > > > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com> > > > > > --- > > > > > drivers/iommu/fsl_pamu.c | 43 > > ++++++++++++++++++++++++++++--- > > > > ----- > > > > > drivers/iommu/fsl_pamu.h | 1 + > > > > > drivers/iommu/fsl_pamu_domain.c | 46 > > > > ++++++++++++++++++++++++++++++++++++--- > > > > > 3 files changed, 78 insertions(+), 12 deletions(-) > > > > > > > > > > diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c > > > > > index > > > > > cba0498..fb4a031 100644 > > > > > --- a/drivers/iommu/fsl_pamu.c > > > > > +++ b/drivers/iommu/fsl_pamu.c > > > > > @@ -225,6 +225,21 @@ static struct paace *pamu_get_spaace(struct > > > > > paace *paace, > > > > > u32 wnum) > > > > > return spaace; > > > > > } > > > > > > > > > > +/* > > > > > + * Defaul PPAACE settings for an LIODN. > > > > > + */ > > > > > +static void setup_default_ppaace(struct paace *ppaace) { > > > > > + pamu_init_ppaace(ppaace); > > > > > + /* window size is 2^(WSE+1) bytes */ > > > > > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > > > > > + ppaace->wbah = 0; > > > > > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > > > > > + set_bf(ppaace->impl_attr, PAACE_IA_ATM, > > > > > + PAACE_ATM_NO_XLATE); > > > > > + set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > > > > > + PAACE_AP_PERMS_ALL); > > > > > +} > > > > > /** > > > > > * pamu_get_fspi_and_allocate() - Allocates fspi index and > > > > > reserves > > > > subwindows > > > > > * required for primary PAACE in > > the > > > > secondary > > > > > @@ -253,6 +268,24 @@ static unsigned long > > > > > pamu_get_fspi_and_allocate(u32 > > > > > subwin_cnt) > > > > > return (spaace_addr - (unsigned long)spaact) / (sizeof(struct > > > > > paace)); } > > > > > > > > > > +/* Reset the PAACE entry to the default state */ void > > > > > +enable_default_dma_window(int liodn) { > > > > > + struct paace *ppaace; > > > > > + > > > > > + ppaace = pamu_get_ppaace(liodn); > > > > > + if (!ppaace) { > > > > > + pr_debug("Invalid liodn entry\n"); > > > > > + return; > > > > > + } > > > > > + > > > > > + memset(ppaace, 0, sizeof(struct paace)); > > > > > + > > > > > + setup_default_ppaace(ppaace); > > > > > + mb(); > > > > > + pamu_enable_liodn(liodn); > > > > > +} > > > > > + > > > > > /* Release the subwindows reserved for a particular LIODN */ > > > > > void pamu_free_subwins(int liodn) { @@ -752,15 +785,7 @@ static > > > > > void __init > > > > > setup_liodns(void) > > > > > continue; > > > > > } > > > > > ppaace = pamu_get_ppaace(liodn); > > > > > - pamu_init_ppaace(ppaace); > > > > > - /* window size is 2^(WSE+1) bytes */ > > > > > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, > > 35); > > > > > - ppaace->wbah = 0; > > > > > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, > > 0); > > > > > - set_bf(ppaace->impl_attr, PAACE_IA_ATM, > > > > > - PAACE_ATM_NO_XLATE); > > > > > - set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > > > > > - PAACE_AP_PERMS_ALL); > > > > > + setup_default_ppaace(ppaace); > > > > > if (of_device_is_compatible(node, "fsl,qman- > > portal")) > > > > > setup_qbman_paace(ppaace, > > QMAN_PORTAL_PAACE); > > > > > if (of_device_is_compatible(node, "fsl,qman")) > > diff -- > > > > git > > > > > a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index > > > > > 8fc1a12..0edcbbbb > > > > > 100644 > > > > > --- a/drivers/iommu/fsl_pamu.h > > > > > +++ b/drivers/iommu/fsl_pamu.h > > > > > @@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct > > > > > device *dev); int pamu_update_paace_stash(int liodn, u32 > > > > > subwin, > > > > > u32 value); int pamu_disable_spaace(int liodn, u32 subwin); > > > > > u32 pamu_get_max_subwin_cnt(void); > > > > > +void enable_default_dma_window(int liodn); > > > > > > > > > > #endif /* __FSL_PAMU_H */ > > > > > diff --git a/drivers/iommu/fsl_pamu_domain.c > > > > > b/drivers/iommu/fsl_pamu_domain.c index 966ae70..dd6cafc 100644 > > > > > --- a/drivers/iommu/fsl_pamu_domain.c > > > > > +++ b/drivers/iommu/fsl_pamu_domain.c > > > > > @@ -340,17 +340,57 @@ static inline struct device_domain_info > > > > > *find_domain(struct device *dev) > > > > > return dev->archdata.iommu_domain; } > > > > > > > > > > +/* Disable device DMA capability and enable default DMA window > > > > > +*/ static void disable_device_dma(struct device_domain_info *info, > > > > > + int enable_dma_window) > > > > > +{ > > > > > +#ifdef CONFIG_PCI > > > > > + if (info->dev->bus == &pci_bus_type) { > > > > > + struct pci_dev *pdev = NULL; > > > > > + pdev = to_pci_dev(info->dev); > > > > > + if (pci_is_enabled(pdev)) > > > > > + pci_disable_device(pdev); > > > > > + } > > > > > +#endif > > > > > + > > > > > + if (enable_dma_window) > > > > > + enable_default_dma_window(info->liodn); > > > > > +} > > > > > + > > > > > +static int check_for_shared_liodn(struct device_domain_info > > > > > +*info) > > { > > > > > + struct device_domain_info *tmp; > > > > > + > > > > > + /* > > > > > + * Sanity check, to ensure that this is not a > > > > > + * shared LIODN. In case of a PCIe controller > > > > > + * it's possible that all PCIe devices share > > > > > + * the same LIODN. > > > > > + */ > > > > > + list_for_each_entry(tmp, &info->domain->devices, link) { > > > > > + if (info->liodn == tmp->liodn) > > > > > + return 1; > > > > > + } > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > static void remove_device_ref(struct device_domain_info *info, > > > > > u32 > > > > win_cnt) { > > > > > unsigned long flags; > > > > > + int enable_dma_window = 0; > > > > > > > > > > list_del(&info->link); > > > > > spin_lock_irqsave(&iommu_lock, flags); > > > > > - if (win_cnt > 1) > > > > > - pamu_free_subwins(info->liodn); > > > > > - pamu_disable_liodn(info->liodn); > > > > > + if (!check_for_shared_liodn(info)) { > > > > > > > > One query; Do we really need to check for this? > > > > > > > [Sethi Varun-B16395] Yes, just a sanity check to ensure that there > > > are no more devices linked to this LIODN and we can disable it. > > > > Varun, trying to understand this; say there are two device under a PCI > > controller which share the LIODN of PCI controller, So both of the > > device must be unbound from kernel driver and then bind both to VFIO. > > > > Now when guest terminated then remove_device_ref() will be called for > > both of device but the sanity check will pass for the one which will > > be called later, is this right? > > > Yes, when the first device is detached PAMU LIODN table entry is not disabled. > The LIODN would only be disabled once all devices are detached. Ok, thanks for clarification Patch series looks good to me. > > -Varun
> -----Original Message----- > From: Bhushan Bharat-R65777 > Sent: Wednesday, October 16, 2013 10:52 PM > To: Sethi Varun-B16395; joro@8bytes.org; iommu@lists.linux- > foundation.org; linuxppc-dev@lists.ozlabs.org; linux- > kernel@vger.kernel.org; Yoder Stuart-B08248; Wood Scott-B07421; > alex.williamson@redhat.com > Subject: RE: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe > devices > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Sethi Varun-B16395 > > > > > > Sent: Wednesday, October 16, 2013 4:53 PM > > > > > > To: joro@8bytes.org; iommu@lists.linux-foundation.org; > > > > > > linuxppc- dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; > > > > > > Yoder Stuart-B08248; Wood Scott-B07421; > > > > > > alex.williamson@redhat.com; Bhushan > > > > > > Bharat-R65777 > > > > > > Cc: Sethi Varun-B16395 > > > > > > Subject: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window > > > > > > for PCIe devices > > > > > > > > > > > > Once the PCIe device assigned to a guest VM (via VFIO) gets > > > > > > detached from the iommu domain (when guest terminates), its > > > > > > PAMU table entry is disabled. So, this would prevent the > > > > > > device from being used once it's > > > > > assigned back to the host. > > > > > > > > > > > > This patch allows for creation of a default DMA window > > > > > > corresponding to the device and subsequently enabling the PAMU > > > > > > table entry. Before we enable the entry, we ensure that the > > > > > > device's bus master capability is disabled (device quiesced). > > > > > > > > > > > > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com> > > > > > > --- > > > > > > drivers/iommu/fsl_pamu.c | 43 > > > ++++++++++++++++++++++++++++--- > > > > > ----- > > > > > > drivers/iommu/fsl_pamu.h | 1 + > > > > > > drivers/iommu/fsl_pamu_domain.c | 46 > > > > > ++++++++++++++++++++++++++++++++++++--- > > > > > > 3 files changed, 78 insertions(+), 12 deletions(-) > > > > > > > > > > > > diff --git a/drivers/iommu/fsl_pamu.c > > > > > > b/drivers/iommu/fsl_pamu.c index > > > > > > cba0498..fb4a031 100644 > > > > > > --- a/drivers/iommu/fsl_pamu.c > > > > > > +++ b/drivers/iommu/fsl_pamu.c > > > > > > @@ -225,6 +225,21 @@ static struct paace > > > > > > *pamu_get_spaace(struct paace *paace, > > > > > > u32 wnum) > > > > > > return spaace; > > > > > > } > > > > > > > > > > > > +/* > > > > > > + * Defaul PPAACE settings for an LIODN. > > > > > > + */ > > > > > > +static void setup_default_ppaace(struct paace *ppaace) { > > > > > > + pamu_init_ppaace(ppaace); > > > > > > + /* window size is 2^(WSE+1) bytes */ > > > > > > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > > > > > > + ppaace->wbah = 0; > > > > > > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > > > > > > + set_bf(ppaace->impl_attr, PAACE_IA_ATM, > > > > > > + PAACE_ATM_NO_XLATE); > > > > > > + set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > > > > > > + PAACE_AP_PERMS_ALL); > > > > > > +} > > > > > > /** > > > > > > * pamu_get_fspi_and_allocate() - Allocates fspi index and > > > > > > reserves > > > > > subwindows > > > > > > * required for primary PAACE > in > > > the > > > > > secondary > > > > > > @@ -253,6 +268,24 @@ static unsigned long > > > > > > pamu_get_fspi_and_allocate(u32 > > > > > > subwin_cnt) > > > > > > return (spaace_addr - (unsigned long)spaact) / > > > > > > (sizeof(struct paace)); } > > > > > > > > > > > > +/* Reset the PAACE entry to the default state */ void > > > > > > +enable_default_dma_window(int liodn) { > > > > > > + struct paace *ppaace; > > > > > > + > > > > > > + ppaace = pamu_get_ppaace(liodn); > > > > > > + if (!ppaace) { > > > > > > + pr_debug("Invalid liodn entry\n"); > > > > > > + return; > > > > > > + } > > > > > > + > > > > > > + memset(ppaace, 0, sizeof(struct paace)); > > > > > > + > > > > > > + setup_default_ppaace(ppaace); > > > > > > + mb(); > > > > > > + pamu_enable_liodn(liodn); > > > > > > +} > > > > > > + > > > > > > /* Release the subwindows reserved for a particular LIODN */ > > > > > > void pamu_free_subwins(int liodn) { @@ -752,15 +785,7 @@ > > > > > > static void __init > > > > > > setup_liodns(void) > > > > > > continue; > > > > > > } > > > > > > ppaace = pamu_get_ppaace(liodn); > > > > > > - pamu_init_ppaace(ppaace); > > > > > > - /* window size is 2^(WSE+1) bytes */ > > > > > > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, > > > 35); > > > > > > - ppaace->wbah = 0; > > > > > > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, > > > 0); > > > > > > - set_bf(ppaace->impl_attr, PAACE_IA_ATM, > > > > > > - PAACE_ATM_NO_XLATE); > > > > > > - set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > > > > > > - PAACE_AP_PERMS_ALL); > > > > > > + setup_default_ppaace(ppaace); > > > > > > if (of_device_is_compatible(node, "fsl,qman- > > > portal")) > > > > > > setup_qbman_paace(ppaace, > > > QMAN_PORTAL_PAACE); > > > > > > if (of_device_is_compatible(node, "fsl,qman")) > > > diff -- > > > > > git > > > > > > a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index > > > > > > 8fc1a12..0edcbbbb > > > > > > 100644 > > > > > > --- a/drivers/iommu/fsl_pamu.h > > > > > > +++ b/drivers/iommu/fsl_pamu.h > > > > > > @@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct > > > > > > device *dev); int pamu_update_paace_stash(int liodn, u32 > > > > > > subwin, > > > > > > u32 value); int pamu_disable_spaace(int liodn, u32 subwin); > > > > > > u32 pamu_get_max_subwin_cnt(void); > > > > > > +void enable_default_dma_window(int liodn); > > > > > > > > > > > > #endif /* __FSL_PAMU_H */ > > > > > > diff --git a/drivers/iommu/fsl_pamu_domain.c > > > > > > b/drivers/iommu/fsl_pamu_domain.c index 966ae70..dd6cafc > > > > > > 100644 > > > > > > --- a/drivers/iommu/fsl_pamu_domain.c > > > > > > +++ b/drivers/iommu/fsl_pamu_domain.c > > > > > > @@ -340,17 +340,57 @@ static inline struct device_domain_info > > > > > > *find_domain(struct device *dev) > > > > > > return dev->archdata.iommu_domain; } > > > > > > > > > > > > +/* Disable device DMA capability and enable default DMA > > > > > > +window */ static void disable_device_dma(struct > device_domain_info *info, > > > > > > + int enable_dma_window) > > > > > > +{ > > > > > > +#ifdef CONFIG_PCI > > > > > > + if (info->dev->bus == &pci_bus_type) { > > > > > > + struct pci_dev *pdev = NULL; > > > > > > + pdev = to_pci_dev(info->dev); > > > > > > + if (pci_is_enabled(pdev)) > > > > > > + pci_disable_device(pdev); > > > > > > + } > > > > > > +#endif > > > > > > + > > > > > > + if (enable_dma_window) > > > > > > + enable_default_dma_window(info->liodn); > > > > > > +} > > > > > > + > > > > > > +static int check_for_shared_liodn(struct device_domain_info > > > > > > +*info) > > > { > > > > > > + struct device_domain_info *tmp; > > > > > > + > > > > > > + /* > > > > > > + * Sanity check, to ensure that this is not a > > > > > > + * shared LIODN. In case of a PCIe controller > > > > > > + * it's possible that all PCIe devices share > > > > > > + * the same LIODN. > > > > > > + */ > > > > > > + list_for_each_entry(tmp, &info->domain->devices, link) { > > > > > > + if (info->liodn == tmp->liodn) > > > > > > + return 1; > > > > > > + } > > > > > > + > > > > > > + return 0; > > > > > > +} > > > > > > + > > > > > > static void remove_device_ref(struct device_domain_info > > > > > > *info, > > > > > > u32 > > > > > win_cnt) { > > > > > > unsigned long flags; > > > > > > + int enable_dma_window = 0; > > > > > > > > > > > > list_del(&info->link); > > > > > > spin_lock_irqsave(&iommu_lock, flags); > > > > > > - if (win_cnt > 1) > > > > > > - pamu_free_subwins(info->liodn); > > > > > > - pamu_disable_liodn(info->liodn); > > > > > > + if (!check_for_shared_liodn(info)) { > > > > > > > > > > One query; Do we really need to check for this? > > > > > > > > > [Sethi Varun-B16395] Yes, just a sanity check to ensure that there > > > > are no more devices linked to this LIODN and we can disable it. > > > > > > Varun, trying to understand this; say there are two device under a > > > PCI controller which share the LIODN of PCI controller, So both of > > > the device must be unbound from kernel driver and then bind both to > VFIO. > > > > > > Now when guest terminated then remove_device_ref() will be called > > > for both of device but the sanity check will pass for the one which > > > will be called later, is this right? > > > > > Yes, when the first device is detached PAMU LIODN table entry is not > disabled. > > The LIODN would only be disabled once all devices are detached. > > Ok, thanks for clarification > > Patch series looks good to me. Thanks. -Varun
On Wed, 2013-10-16 at 16:53 +0530, Varun Sethi wrote: > Once the PCIe device assigned to a guest VM (via VFIO) gets detached from the iommu domain > (when guest terminates), its PAMU table entry is disabled. So, this would prevent the device > from being used once it's assigned back to the host. > > This patch allows for creation of a default DMA window corresponding to the device > and subsequently enabling the PAMU table entry. Before we enable the entry, we ensure that > the device's bus master capability is disabled (device quiesced). > > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com> > --- > drivers/iommu/fsl_pamu.c | 43 ++++++++++++++++++++++++++++-------- > drivers/iommu/fsl_pamu.h | 1 + > drivers/iommu/fsl_pamu_domain.c | 46 ++++++++++++++++++++++++++++++++++++--- > 3 files changed, 78 insertions(+), 12 deletions(-) > > diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c > index cba0498..fb4a031 100644 > --- a/drivers/iommu/fsl_pamu.c > +++ b/drivers/iommu/fsl_pamu.c > @@ -225,6 +225,21 @@ static struct paace *pamu_get_spaace(struct paace *paace, u32 wnum) > return spaace; > } > > +/* > + * Defaul PPAACE settings for an LIODN. > + */ > +static void setup_default_ppaace(struct paace *ppaace) > +{ > + pamu_init_ppaace(ppaace); > + /* window size is 2^(WSE+1) bytes */ > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > + ppaace->wbah = 0; > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > + set_bf(ppaace->impl_attr, PAACE_IA_ATM, > + PAACE_ATM_NO_XLATE); > + set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > + PAACE_AP_PERMS_ALL); > +} > /** > * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves subwindows > * required for primary PAACE in the secondary > @@ -253,6 +268,24 @@ static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt) > return (spaace_addr - (unsigned long)spaact) / (sizeof(struct paace)); > } > > +/* Reset the PAACE entry to the default state */ > +void enable_default_dma_window(int liodn) > +{ > + struct paace *ppaace; > + > + ppaace = pamu_get_ppaace(liodn); > + if (!ppaace) { > + pr_debug("Invalid liodn entry\n"); > + return; > + } > + > + memset(ppaace, 0, sizeof(struct paace)); > + > + setup_default_ppaace(ppaace); > + mb(); > + pamu_enable_liodn(liodn); > +} > + > /* Release the subwindows reserved for a particular LIODN */ > void pamu_free_subwins(int liodn) > { > @@ -752,15 +785,7 @@ static void __init setup_liodns(void) > continue; > } > ppaace = pamu_get_ppaace(liodn); > - pamu_init_ppaace(ppaace); > - /* window size is 2^(WSE+1) bytes */ > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > - ppaace->wbah = 0; > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > - set_bf(ppaace->impl_attr, PAACE_IA_ATM, > - PAACE_ATM_NO_XLATE); > - set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > - PAACE_AP_PERMS_ALL); > + setup_default_ppaace(ppaace); > if (of_device_is_compatible(node, "fsl,qman-portal")) > setup_qbman_paace(ppaace, QMAN_PORTAL_PAACE); > if (of_device_is_compatible(node, "fsl,qman")) > diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h > index 8fc1a12..0edcbbbb 100644 > --- a/drivers/iommu/fsl_pamu.h > +++ b/drivers/iommu/fsl_pamu.h > @@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct device *dev); > int pamu_update_paace_stash(int liodn, u32 subwin, u32 value); > int pamu_disable_spaace(int liodn, u32 subwin); > u32 pamu_get_max_subwin_cnt(void); > +void enable_default_dma_window(int liodn); > > #endif /* __FSL_PAMU_H */ > diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c > index 966ae70..dd6cafc 100644 > --- a/drivers/iommu/fsl_pamu_domain.c > +++ b/drivers/iommu/fsl_pamu_domain.c > @@ -340,17 +340,57 @@ static inline struct device_domain_info *find_domain(struct device *dev) > return dev->archdata.iommu_domain; > } > > +/* Disable device DMA capability and enable default DMA window */ > +static void disable_device_dma(struct device_domain_info *info, > + int enable_dma_window) nit, bool? > +{ > +#ifdef CONFIG_PCI > + if (info->dev->bus == &pci_bus_type) { > + struct pci_dev *pdev = NULL; nit, unnecessary initialization > + pdev = to_pci_dev(info->dev); > + if (pci_is_enabled(pdev)) > + pci_disable_device(pdev); This looks suspicious. What's the case where you're finding devices where this is needed? Logically the driver that called pci_enable_device() should also call pci_disabled_device() when it's done. Maybe there's a case to be made that we can't rely on the driver, but then maybe there should be a pr_debug/info here to notify that somebody left the device running. There might also be the question of whether you should test the busmaster bit in the command register rather than trusting these indirect paths if it's really for cleanup. Thanks, Alex > + } > +#endif > + > + if (enable_dma_window) > + enable_default_dma_window(info->liodn); > +} > + > +static int check_for_shared_liodn(struct device_domain_info *info) > +{ > + struct device_domain_info *tmp; > + > + /* > + * Sanity check, to ensure that this is not a > + * shared LIODN. In case of a PCIe controller > + * it's possible that all PCIe devices share > + * the same LIODN. > + */ > + list_for_each_entry(tmp, &info->domain->devices, link) { > + if (info->liodn == tmp->liodn) > + return 1; > + } > + > + return 0; > +} > + > static void remove_device_ref(struct device_domain_info *info, u32 win_cnt) > { > unsigned long flags; > + int enable_dma_window = 0; > > list_del(&info->link); > spin_lock_irqsave(&iommu_lock, flags); > - if (win_cnt > 1) > - pamu_free_subwins(info->liodn); > - pamu_disable_liodn(info->liodn); > + if (!check_for_shared_liodn(info)) { > + if (win_cnt > 1) > + pamu_free_subwins(info->liodn); > + pamu_disable_liodn(info->liodn); > + enable_dma_window = 1; > + } > spin_unlock_irqrestore(&iommu_lock, flags); > spin_lock_irqsave(&device_domain_lock, flags); > + disable_device_dma(info, enable_dma_window); > info->dev->archdata.iommu_domain = NULL; > kmem_cache_free(iommu_devinfo_cache, info); > spin_unlock_irqrestore(&device_domain_lock, flags);
> -----Original Message----- > From: Alex Williamson [mailto:alex.williamson@redhat.com] > Sent: Tuesday, December 03, 2013 3:04 AM > To: Sethi Varun-B16395 > Cc: joro@8bytes.org; iommu@lists.linux-foundation.org; linuxppc- > dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Yoder Stuart-B08248; > Wood Scott-B07421; Bhushan Bharat-R65777 > Subject: Re: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe > devices > > On Wed, 2013-10-16 at 16:53 +0530, Varun Sethi wrote: > > Once the PCIe device assigned to a guest VM (via VFIO) gets detached > > from the iommu domain (when guest terminates), its PAMU table entry is > > disabled. So, this would prevent the device from being used once it's > assigned back to the host. > > > > This patch allows for creation of a default DMA window corresponding > > to the device and subsequently enabling the PAMU table entry. Before > > we enable the entry, we ensure that the device's bus master capability > is disabled (device quiesced). > > > > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com> > > --- > > drivers/iommu/fsl_pamu.c | 43 ++++++++++++++++++++++++++++--- > ----- > > drivers/iommu/fsl_pamu.h | 1 + > > drivers/iommu/fsl_pamu_domain.c | 46 > ++++++++++++++++++++++++++++++++++++--- > > 3 files changed, 78 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index > > cba0498..fb4a031 100644 > > --- a/drivers/iommu/fsl_pamu.c > > +++ b/drivers/iommu/fsl_pamu.c > > @@ -225,6 +225,21 @@ static struct paace *pamu_get_spaace(struct paace > *paace, u32 wnum) > > return spaace; > > } > > > > +/* > > + * Defaul PPAACE settings for an LIODN. > > + */ > > +static void setup_default_ppaace(struct paace *ppaace) { > > + pamu_init_ppaace(ppaace); > > + /* window size is 2^(WSE+1) bytes */ > > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > > + ppaace->wbah = 0; > > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > > + set_bf(ppaace->impl_attr, PAACE_IA_ATM, > > + PAACE_ATM_NO_XLATE); > > + set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > > + PAACE_AP_PERMS_ALL); > > +} > > /** > > * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves > subwindows > > * required for primary PAACE in the > secondary > > @@ -253,6 +268,24 @@ static unsigned long > pamu_get_fspi_and_allocate(u32 subwin_cnt) > > return (spaace_addr - (unsigned long)spaact) / (sizeof(struct > > paace)); } > > > > +/* Reset the PAACE entry to the default state */ void > > +enable_default_dma_window(int liodn) { > > + struct paace *ppaace; > > + > > + ppaace = pamu_get_ppaace(liodn); > > + if (!ppaace) { > > + pr_debug("Invalid liodn entry\n"); > > + return; > > + } > > + > > + memset(ppaace, 0, sizeof(struct paace)); > > + > > + setup_default_ppaace(ppaace); > > + mb(); > > + pamu_enable_liodn(liodn); > > +} > > + > > /* Release the subwindows reserved for a particular LIODN */ void > > pamu_free_subwins(int liodn) { @@ -752,15 +785,7 @@ static void > > __init setup_liodns(void) > > continue; > > } > > ppaace = pamu_get_ppaace(liodn); > > - pamu_init_ppaace(ppaace); > > - /* window size is 2^(WSE+1) bytes */ > > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > > - ppaace->wbah = 0; > > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > > - set_bf(ppaace->impl_attr, PAACE_IA_ATM, > > - PAACE_ATM_NO_XLATE); > > - set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > > - PAACE_AP_PERMS_ALL); > > + setup_default_ppaace(ppaace); > > if (of_device_is_compatible(node, "fsl,qman-portal")) > > setup_qbman_paace(ppaace, QMAN_PORTAL_PAACE); > > if (of_device_is_compatible(node, "fsl,qman")) diff -- > git > > a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index > > 8fc1a12..0edcbbbb 100644 > > --- a/drivers/iommu/fsl_pamu.h > > +++ b/drivers/iommu/fsl_pamu.h > > @@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct device > > *dev); int pamu_update_paace_stash(int liodn, u32 subwin, u32 > > value); int pamu_disable_spaace(int liodn, u32 subwin); > > u32 pamu_get_max_subwin_cnt(void); > > +void enable_default_dma_window(int liodn); > > > > #endif /* __FSL_PAMU_H */ > > diff --git a/drivers/iommu/fsl_pamu_domain.c > > b/drivers/iommu/fsl_pamu_domain.c index 966ae70..dd6cafc 100644 > > --- a/drivers/iommu/fsl_pamu_domain.c > > +++ b/drivers/iommu/fsl_pamu_domain.c > > @@ -340,17 +340,57 @@ static inline struct device_domain_info > *find_domain(struct device *dev) > > return dev->archdata.iommu_domain; > > } > > > > +/* Disable device DMA capability and enable default DMA window */ > > +static void disable_device_dma(struct device_domain_info *info, > > + int enable_dma_window) > > nit, bool? > > > +{ > > +#ifdef CONFIG_PCI > > + if (info->dev->bus == &pci_bus_type) { > > + struct pci_dev *pdev = NULL; > > nit, unnecessary initialization > > > + pdev = to_pci_dev(info->dev); > > + if (pci_is_enabled(pdev)) > > + pci_disable_device(pdev); > > This looks suspicious. What's the case where you're finding devices > where this is needed? Logically the driver that called > pci_enable_device() should also call pci_disabled_device() when it's > done. Maybe there's a case to be made that we can't rely on the driver, > but then maybe there should be a pr_debug/info here to notify that > somebody left the device running. There might also be the question of > whether you should test the busmaster bit in the command register rather > than trusting these indirect paths if it's really for cleanup. Thanks, > Once the device is detached from the domain, we enable a default DMA window for the device. Before enabling the default window, I want to ensure that device DMA is disabled. VFIO subsystem handles this for vfio-pci. But, can we assume that this would always be the case? -Varun
On Tue, 2013-12-03 at 18:15 +0000, Varun Sethi wrote: > > > -----Original Message----- > > From: Alex Williamson [mailto:alex.williamson@redhat.com] > > Sent: Tuesday, December 03, 2013 3:04 AM > > To: Sethi Varun-B16395 > > Cc: joro@8bytes.org; iommu@lists.linux-foundation.org; linuxppc- > > dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Yoder Stuart-B08248; > > Wood Scott-B07421; Bhushan Bharat-R65777 > > Subject: Re: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe > > devices > > > > On Wed, 2013-10-16 at 16:53 +0530, Varun Sethi wrote: > > > Once the PCIe device assigned to a guest VM (via VFIO) gets detached > > > from the iommu domain (when guest terminates), its PAMU table entry is > > > disabled. So, this would prevent the device from being used once it's > > assigned back to the host. > > > > > > This patch allows for creation of a default DMA window corresponding > > > to the device and subsequently enabling the PAMU table entry. Before > > > we enable the entry, we ensure that the device's bus master capability > > is disabled (device quiesced). > > > > > > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com> > > > --- > > > drivers/iommu/fsl_pamu.c | 43 ++++++++++++++++++++++++++++--- > > ----- > > > drivers/iommu/fsl_pamu.h | 1 + > > > drivers/iommu/fsl_pamu_domain.c | 46 > > ++++++++++++++++++++++++++++++++++++--- > > > 3 files changed, 78 insertions(+), 12 deletions(-) > > > > > > diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index > > > cba0498..fb4a031 100644 > > > --- a/drivers/iommu/fsl_pamu.c > > > +++ b/drivers/iommu/fsl_pamu.c > > > @@ -225,6 +225,21 @@ static struct paace *pamu_get_spaace(struct paace > > *paace, u32 wnum) > > > return spaace; > > > } > > > > > > +/* > > > + * Defaul PPAACE settings for an LIODN. > > > + */ > > > +static void setup_default_ppaace(struct paace *ppaace) { > > > + pamu_init_ppaace(ppaace); > > > + /* window size is 2^(WSE+1) bytes */ > > > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > > > + ppaace->wbah = 0; > > > + set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > > > + set_bf(ppaace->impl_attr, PAACE_IA_ATM, > > > + PAACE_ATM_NO_XLATE); > > > + set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > > > + PAACE_AP_PERMS_ALL); > > > +} > > > /** > > > * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves > > subwindows > > > * required for primary PAACE in the > > secondary > > > @@ -253,6 +268,24 @@ static unsigned long > > pamu_get_fspi_and_allocate(u32 subwin_cnt) > > > return (spaace_addr - (unsigned long)spaact) / (sizeof(struct > > > paace)); } > > > > > > +/* Reset the PAACE entry to the default state */ void > > > +enable_default_dma_window(int liodn) { > > > + struct paace *ppaace; > > > + > > > + ppaace = pamu_get_ppaace(liodn); > > > + if (!ppaace) { > > > + pr_debug("Invalid liodn entry\n"); > > > + return; > > > + } > > > + > > > + memset(ppaace, 0, sizeof(struct paace)); > > > + > > > + setup_default_ppaace(ppaace); > > > + mb(); > > > + pamu_enable_liodn(liodn); > > > +} > > > + > > > /* Release the subwindows reserved for a particular LIODN */ void > > > pamu_free_subwins(int liodn) { @@ -752,15 +785,7 @@ static void > > > __init setup_liodns(void) > > > continue; > > > } > > > ppaace = pamu_get_ppaace(liodn); > > > - pamu_init_ppaace(ppaace); > > > - /* window size is 2^(WSE+1) bytes */ > > > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); > > > - ppaace->wbah = 0; > > > - set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); > > > - set_bf(ppaace->impl_attr, PAACE_IA_ATM, > > > - PAACE_ATM_NO_XLATE); > > > - set_bf(ppaace->addr_bitfields, PAACE_AF_AP, > > > - PAACE_AP_PERMS_ALL); > > > + setup_default_ppaace(ppaace); > > > if (of_device_is_compatible(node, "fsl,qman-portal")) > > > setup_qbman_paace(ppaace, QMAN_PORTAL_PAACE); > > > if (of_device_is_compatible(node, "fsl,qman")) diff -- > > git > > > a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index > > > 8fc1a12..0edcbbbb 100644 > > > --- a/drivers/iommu/fsl_pamu.h > > > +++ b/drivers/iommu/fsl_pamu.h > > > @@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct device > > > *dev); int pamu_update_paace_stash(int liodn, u32 subwin, u32 > > > value); int pamu_disable_spaace(int liodn, u32 subwin); > > > u32 pamu_get_max_subwin_cnt(void); > > > +void enable_default_dma_window(int liodn); > > > > > > #endif /* __FSL_PAMU_H */ > > > diff --git a/drivers/iommu/fsl_pamu_domain.c > > > b/drivers/iommu/fsl_pamu_domain.c index 966ae70..dd6cafc 100644 > > > --- a/drivers/iommu/fsl_pamu_domain.c > > > +++ b/drivers/iommu/fsl_pamu_domain.c > > > @@ -340,17 +340,57 @@ static inline struct device_domain_info > > *find_domain(struct device *dev) > > > return dev->archdata.iommu_domain; > > > } > > > > > > +/* Disable device DMA capability and enable default DMA window */ > > > +static void disable_device_dma(struct device_domain_info *info, > > > + int enable_dma_window) > > > > nit, bool? > > > > > +{ > > > +#ifdef CONFIG_PCI > > > + if (info->dev->bus == &pci_bus_type) { > > > + struct pci_dev *pdev = NULL; > > > > nit, unnecessary initialization > > > > > + pdev = to_pci_dev(info->dev); > > > + if (pci_is_enabled(pdev)) > > > + pci_disable_device(pdev); > > > > This looks suspicious. What's the case where you're finding devices > > where this is needed? Logically the driver that called > > pci_enable_device() should also call pci_disabled_device() when it's > > done. Maybe there's a case to be made that we can't rely on the driver, > > but then maybe there should be a pr_debug/info here to notify that > > somebody left the device running. There might also be the question of > > whether you should test the busmaster bit in the command register rather > > than trusting these indirect paths if it's really for cleanup. Thanks, > > > Once the device is detached from the domain, we enable a default DMA > window for the device. Before enabling the default window, I want to > ensure that device DMA is disabled. VFIO subsystem handles this for > vfio-pci. But, can we assume that this would always be the case? That's what I'm asking. It seems like pci_enable/disable_device is part of the driver API and every enable should have a matching disable. If this is to catch cases where the disable never happened, then there should be some indication that the previous driver did something wrong and we should have some sort of printed warning. Then there's also the question of whether this fully does what you want it to. This is an indirect way of checking the busmaster bit in the command register. If you really want to make sure it's disabled, is it sufficient or should we be reading the actual command register? Thanks, Alex
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index cba0498..fb4a031 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -225,6 +225,21 @@ static struct paace *pamu_get_spaace(struct paace *paace, u32 wnum) return spaace; } +/* + * Defaul PPAACE settings for an LIODN. + */ +static void setup_default_ppaace(struct paace *ppaace) +{ + pamu_init_ppaace(ppaace); + /* window size is 2^(WSE+1) bytes */ + set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); + ppaace->wbah = 0; + set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); + set_bf(ppaace->impl_attr, PAACE_IA_ATM, + PAACE_ATM_NO_XLATE); + set_bf(ppaace->addr_bitfields, PAACE_AF_AP, + PAACE_AP_PERMS_ALL); +} /** * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves subwindows * required for primary PAACE in the secondary @@ -253,6 +268,24 @@ static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt) return (spaace_addr - (unsigned long)spaact) / (sizeof(struct paace)); } +/* Reset the PAACE entry to the default state */ +void enable_default_dma_window(int liodn) +{ + struct paace *ppaace; + + ppaace = pamu_get_ppaace(liodn); + if (!ppaace) { + pr_debug("Invalid liodn entry\n"); + return; + } + + memset(ppaace, 0, sizeof(struct paace)); + + setup_default_ppaace(ppaace); + mb(); + pamu_enable_liodn(liodn); +} + /* Release the subwindows reserved for a particular LIODN */ void pamu_free_subwins(int liodn) { @@ -752,15 +785,7 @@ static void __init setup_liodns(void) continue; } ppaace = pamu_get_ppaace(liodn); - pamu_init_ppaace(ppaace); - /* window size is 2^(WSE+1) bytes */ - set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35); - ppaace->wbah = 0; - set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0); - set_bf(ppaace->impl_attr, PAACE_IA_ATM, - PAACE_ATM_NO_XLATE); - set_bf(ppaace->addr_bitfields, PAACE_AF_AP, - PAACE_AP_PERMS_ALL); + setup_default_ppaace(ppaace); if (of_device_is_compatible(node, "fsl,qman-portal")) setup_qbman_paace(ppaace, QMAN_PORTAL_PAACE); if (of_device_is_compatible(node, "fsl,qman")) diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h index 8fc1a12..0edcbbbb 100644 --- a/drivers/iommu/fsl_pamu.h +++ b/drivers/iommu/fsl_pamu.h @@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct device *dev); int pamu_update_paace_stash(int liodn, u32 subwin, u32 value); int pamu_disable_spaace(int liodn, u32 subwin); u32 pamu_get_max_subwin_cnt(void); +void enable_default_dma_window(int liodn); #endif /* __FSL_PAMU_H */ diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c index 966ae70..dd6cafc 100644 --- a/drivers/iommu/fsl_pamu_domain.c +++ b/drivers/iommu/fsl_pamu_domain.c @@ -340,17 +340,57 @@ static inline struct device_domain_info *find_domain(struct device *dev) return dev->archdata.iommu_domain; } +/* Disable device DMA capability and enable default DMA window */ +static void disable_device_dma(struct device_domain_info *info, + int enable_dma_window) +{ +#ifdef CONFIG_PCI + if (info->dev->bus == &pci_bus_type) { + struct pci_dev *pdev = NULL; + pdev = to_pci_dev(info->dev); + if (pci_is_enabled(pdev)) + pci_disable_device(pdev); + } +#endif + + if (enable_dma_window) + enable_default_dma_window(info->liodn); +} + +static int check_for_shared_liodn(struct device_domain_info *info) +{ + struct device_domain_info *tmp; + + /* + * Sanity check, to ensure that this is not a + * shared LIODN. In case of a PCIe controller + * it's possible that all PCIe devices share + * the same LIODN. + */ + list_for_each_entry(tmp, &info->domain->devices, link) { + if (info->liodn == tmp->liodn) + return 1; + } + + return 0; +} + static void remove_device_ref(struct device_domain_info *info, u32 win_cnt) { unsigned long flags; + int enable_dma_window = 0; list_del(&info->link); spin_lock_irqsave(&iommu_lock, flags); - if (win_cnt > 1) - pamu_free_subwins(info->liodn); - pamu_disable_liodn(info->liodn); + if (!check_for_shared_liodn(info)) { + if (win_cnt > 1) + pamu_free_subwins(info->liodn); + pamu_disable_liodn(info->liodn); + enable_dma_window = 1; + } spin_unlock_irqrestore(&iommu_lock, flags); spin_lock_irqsave(&device_domain_lock, flags); + disable_device_dma(info, enable_dma_window); info->dev->archdata.iommu_domain = NULL; kmem_cache_free(iommu_devinfo_cache, info); spin_unlock_irqrestore(&device_domain_lock, flags);
Once the PCIe device assigned to a guest VM (via VFIO) gets detached from the iommu domain (when guest terminates), its PAMU table entry is disabled. So, this would prevent the device from being used once it's assigned back to the host. This patch allows for creation of a default DMA window corresponding to the device and subsequently enabling the PAMU table entry. Before we enable the entry, we ensure that the device's bus master capability is disabled (device quiesced). Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com> --- drivers/iommu/fsl_pamu.c | 43 ++++++++++++++++++++++++++++-------- drivers/iommu/fsl_pamu.h | 1 + drivers/iommu/fsl_pamu_domain.c | 46 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 78 insertions(+), 12 deletions(-)