| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6A3DF150A5B70D4F9B66A25E3F7C888D071C09E2@039-SN2MPN1-013.039d.mgd.msft.net>
Date: Wed, 16 Oct 2013 17:13:15 +0000
From: Bhushan Bharat-R65777 <R65777@...escale.com>
To: Sethi Varun-B16395 <B16395@...escale.com>,
"joro@...tes.org" <joro@...tes.org>,
"iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Yoder Stuart-B08248 <B08248@...escale.com>,
Wood Scott-B07421 <B07421@...escale.com>,
"alex.williamson@...hat.com" <alex.williamson@...hat.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@...tes.org; iommu@...ts.linux-foundation.org; linuxppc-
> > > dev@...ts.ozlabs.org; linux-kernel@...r.kernel.org; Yoder
> > > Stuart-B08248; Wood Scott-B07421; alex.williamson@...hat.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@...escale.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists