allow drivers to fix kexec bus mastering hole

allow drivers to fix kexec bus mastering hole

[Dave Airlie]

kexec relies on disabling bus mastering on PCI devices to block wayward DMAs
left set by the previous kernel. However the drm midlayer was enabling bus
mastering for all PCI drivers before calling into them. This meant no matter
what they did, there was always going to be a small race window. The
first patch pushes the enable into the drivers, and the second radeon patch
moves bus mastering enable to later in the driver load procedure for the kms
driver. These two along with Jerome's sanity patch should close the race completely.

please check the first one to make sure I didn't miss any drivers.

Dave.

[PATCH 1/2] drm: move pci bus master enable into driver.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

The current enabling of bus mastering in the drm midlayer allows a large
race condition under kexec. When a kexec'ed kernel re-enables bus mastering
for the GPU, previously setup dma blocks may cause writes to random pieces
of memory. On radeon the writeback mechanism can cause these sorts of issues.

This patch doesn't fix the problem, but it moves the bus master enable under
the individual drivers control so they can move enabling it until later in
their load cycle and close the race.

Fix for radeon kms driver will be in a follow-up patch.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_pci.c               |    2 --
 drivers/gpu/drm/gma500/psb_drv.c        |    2 ++
 drivers/gpu/drm/i810/i810_dma.c         |    2 ++
 drivers/gpu/drm/i915/i915_dma.c         |    2 ++
 drivers/gpu/drm/mga/mga_dma.c           |    2 ++
 drivers/gpu/drm/nouveau/nouveau_state.c |    2 ++
 drivers/gpu/drm/r128/r128_drv.c         |    1 +
 drivers/gpu/drm/radeon/radeon_cp.c      |    2 ++
 drivers/gpu/drm/radeon/radeon_kms.c     |    2 ++
 drivers/gpu/drm/sis/sis_drv.c           |    2 ++
 drivers/gpu/drm/via/via_map.c           |    2 ++
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c     |    2 ++
 12 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index d4d10b7..13f3d93 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -324,8 +324,6 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 	if (ret)
 		goto err_g1;
 
-	pci_set_master(pdev);
-
 	dev->pdev = pdev;
 	dev->dev = &pdev->dev;
 
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index add3156..ff30b71 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -286,6 +286,8 @@ static int psb_driver_load(struct drm_device *dev, unsigned long chipset)
 	dev_priv->dev = dev;
 	dev->dev_private = (void *) dev_priv;
 
+	pci_set_master(dev->pdev);
+
 	if (!IS_PSB(dev)) {
 		if (pci_enable_msi(dev->pdev))
 			dev_warn(dev->dev, "Enabling MSI failed!\n");
diff --git a/drivers/gpu/drm/i810/i810_dma.c b/drivers/gpu/drm/i810/i810_dma.c
index 8f371e8..56989a3 100644
--- a/drivers/gpu/drm/i810/i810_dma.c
+++ b/drivers/gpu/drm/i810/i810_dma.c
@@ -1210,6 +1210,8 @@ int i810_driver_load(struct drm_device *dev, unsigned long flags)
 	dev->types[8] = _DRM_STAT_SECONDARY;
 	dev->types[9] = _DRM_STAT_DMA;
 
+	pci_set_master(dev->pdev);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a9533c5..da07a8c 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1917,6 +1917,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
 		goto free_priv;
 	}
 
+	pci_set_master(dev->pdev);
+
 	/* overlay on gen2 is broken and can't address above 1G */
 	if (IS_GEN2(dev))
 		dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(30));
diff --git a/drivers/gpu/drm/mga/mga_dma.c b/drivers/gpu/drm/mga/mga_dma.c
index 5ccb65de..507aa3d 100644
--- a/drivers/gpu/drm/mga/mga_dma.c
+++ b/drivers/gpu/drm/mga/mga_dma.c
@@ -403,6 +403,8 @@ int mga_driver_load(struct drm_device *dev, unsigned long flags)
 	dev_priv->usec_timeout = MGA_DEFAULT_USEC_TIMEOUT;
 	dev_priv->chipset = flags;
 
+	pci_set_master(dev->pdev);
+
 	dev_priv->mmio_base = pci_resource_start(dev->pdev, 1);
 	dev_priv->mmio_size = pci_resource_len(dev->pdev, 1);
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index 82478e0..37f7bba 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -1000,6 +1000,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	dev->dev_private = dev_priv;
 	dev_priv->dev = dev;
 
+	pci_set_master(dev->pdev);
+
 	dev_priv->flags = flags & NOUVEAU_FLAGS;
 
 	NV_DEBUG(dev, "vendor: 0x%X device: 0x%X class: 0x%X\n",
diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c
index 6a5f439..88718fa 100644
--- a/drivers/gpu/drm/r128/r128_drv.c
+++ b/drivers/gpu/drm/r128/r128_drv.c
@@ -85,6 +85,7 @@ static struct drm_driver driver = {
 
 int r128_driver_load(struct drm_device *dev, unsigned long flags)
 {
+	pci_set_master(dev->pdev);
 	return drm_vblank_init(dev, 1);
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_cp.c b/drivers/gpu/drm/radeon/radeon_cp.c
index 72ae826..0ebb7d4 100644
--- a/drivers/gpu/drm/radeon/radeon_cp.c
+++ b/drivers/gpu/drm/radeon/radeon_cp.c
@@ -2115,6 +2115,8 @@ int radeon_driver_load(struct drm_device *dev, unsigned long flags)
 		break;
 	}
 
+	pci_set_master(dev->pdev);
+
 	if (drm_pci_device_is_agp(dev))
 		dev_priv->flags |= RADEON_IS_AGP;
 	else if (pci_is_pcie(dev->pdev))
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index be2c122..601b6bd 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -57,6 +57,8 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
 	}
 	dev->dev_private = (void *)rdev;
 
+	pci_set_master(dev->pdev);
+
 	/* update BUS flag */
 	if (drm_pci_device_is_agp(dev)) {
 		flags |= RADEON_IS_AGP;
diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c
index bda96a8..c483363 100644
--- a/drivers/gpu/drm/sis/sis_drv.c
+++ b/drivers/gpu/drm/sis/sis_drv.c
@@ -42,6 +42,8 @@ static int sis_driver_load(struct drm_device *dev, unsigned long chipset)
 	drm_sis_private_t *dev_priv;
 	int ret;
 
+	pci_set_master(dev->pdev);
+
 	dev_priv = kzalloc(sizeof(drm_sis_private_t), GFP_KERNEL);
 	if (dev_priv == NULL)
 		return -ENOMEM;
diff --git a/drivers/gpu/drm/via/via_map.c b/drivers/gpu/drm/via/via_map.c
index 6cca9a7..1d88595 100644
--- a/drivers/gpu/drm/via/via_map.c
+++ b/drivers/gpu/drm/via/via_map.c
@@ -104,6 +104,8 @@ int via_driver_load(struct drm_device *dev, unsigned long chipset)
 
 	dev_priv->chipset = chipset;
 
+	pci_set_master(dev->pdev);
+
 	ret = drm_sman_init(&dev_priv->sman, 2, 12, 8);
 	if (ret) {
 		kfree(dev_priv);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index f390f5f..9364b7b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -400,6 +400,8 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
 	}
 	memset(dev_priv, 0, sizeof(*dev_priv));
 
+	pci_set_master(dev->pdev);
+
 	dev_priv->dev = dev;
 	dev_priv->vmw_chipset = chipset;
 	dev_priv->last_read_seqno = (uint32_t) -100;
-- 
1.7.7.3

[PATCH 2/2] drm/radeon/kms: move pci bus master enable further into driver.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

This doesn't completely close the kexec hole for all drivers, but it fixes
it for some cases, by enabling PCI bus mastering later.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/radeon/evergreen.c  |    2 ++
 drivers/gpu/drm/radeon/ni.c         |    2 ++
 drivers/gpu/drm/radeon/r100.c       |   10 ++++++++++
 drivers/gpu/drm/radeon/r300.c       |    5 +----
 drivers/gpu/drm/radeon/r420.c       |    4 ++--
 drivers/gpu/drm/radeon/r600.c       |    2 ++
 drivers/gpu/drm/radeon/radeon_kms.c |    2 --
 drivers/gpu/drm/radeon/rv770.c      |    2 ++
 8 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index 5d51035..bb01c04 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -3001,6 +3001,8 @@ static int evergreen_startup(struct radeon_device *rdev)
 	/* enable pcie gen2 link */
 	evergreen_pcie_gen2_enable(rdev);
 
+	pci_set_master(rdev->pdev);
+
 	if (ASIC_IS_DCE5(rdev)) {
 		if (!rdev->me_fw || !rdev->pfp_fw || !rdev->rlc_fw || !rdev->mc_fw) {
 			r = ni_init_microcode(rdev);
diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index 1d1047a..60b4b9b 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -1343,6 +1343,8 @@ static int cayman_startup(struct radeon_device *rdev)
 	/* enable pcie gen2 link */
 	evergreen_pcie_gen2_enable(rdev);
 
+	pci_set_master(rdev->pdev);
+
 	if (!rdev->me_fw || !rdev->pfp_fw || !rdev->rlc_fw || !rdev->mc_fw) {
 		r = ni_init_microcode(rdev);
 		if (r) {
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index dcc5b3c..567924c 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -596,6 +596,16 @@ int r100_pci_gart_init(struct radeon_device *rdev)
 void r100_enable_bm(struct radeon_device *rdev)
 {
 	uint32_t tmp;
+
+	pci_set_master(rdev->pdev);
+
+	if (rdev->family == CHIP_R300 ||
+	    rdev->family == CHIP_R350 ||
+	    rdev->family == CHIP_RV350 ||
+	    rdev->family == CHIP_R423 ||
+	    rdev->family == CHIP_RV410)
+		return;
+
 	/* Enable bus mastering */
 	tmp = RREG32(RADEON_BUS_CNTL) & ~RADEON_BUS_MASTER_DIS;
 	WREG32(RADEON_BUS_CNTL, tmp);
diff --git a/drivers/gpu/drm/radeon/r300.c b/drivers/gpu/drm/radeon/r300.c
index 400b26d..b1c171f 100644
--- a/drivers/gpu/drm/radeon/r300.c
+++ b/drivers/gpu/drm/radeon/r300.c
@@ -1372,10 +1372,7 @@ static int r300_startup(struct radeon_device *rdev)
 			return r;
 	}
 
-	if (rdev->family == CHIP_R300 ||
-	    rdev->family == CHIP_R350 ||
-	    rdev->family == CHIP_RV350)
-		r100_enable_bm(rdev);
+	r100_enable_bm(rdev);
 
 	if (rdev->flags & RADEON_IS_PCI) {
 		r = r100_pci_gart_enable(rdev);
diff --git a/drivers/gpu/drm/radeon/r420.c b/drivers/gpu/drm/radeon/r420.c
index 417fab8..b9123d6 100644
--- a/drivers/gpu/drm/radeon/r420.c
+++ b/drivers/gpu/drm/radeon/r420.c
@@ -400,8 +400,8 @@ int r420_init(struct radeon_device *rdev)
 	if (r) {
 		return r;
 	}
-	if (rdev->family == CHIP_R420)
-		r100_enable_bm(rdev);
+
+	r100_enable_bm(rdev);
 
 	if (rdev->flags & RADEON_IS_PCIE) {
 		r = rv370_pcie_gart_init(rdev);
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 936d107..36c9b86 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2468,6 +2468,8 @@ int r600_startup(struct radeon_device *rdev)
 	/* enable pcie gen2 link */
 	r600_pcie_gen2_enable(rdev);
 
+	pci_set_master(rdev->pdev);
+
 	if (!rdev->me_fw || !rdev->pfp_fw || !rdev->rlc_fw) {
 		r = r600_init_microcode(rdev);
 		if (r) {
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 601b6bd..be2c122 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -57,8 +57,6 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
 	}
 	dev->dev_private = (void *)rdev;
 
-	pci_set_master(dev->pdev);
-
 	/* update BUS flag */
 	if (drm_pci_device_is_agp(dev)) {
 		flags |= RADEON_IS_AGP;
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
index e4e2132..ac1b451 100644
--- a/drivers/gpu/drm/radeon/rv770.c
+++ b/drivers/gpu/drm/radeon/rv770.c
@@ -1043,6 +1043,8 @@ static int rv770_startup(struct radeon_device *rdev)
 	/* enable pcie gen2 link */
 	rv770_pcie_gen2_enable(rdev);
 
+	pci_set_master(rdev->pdev);
+
 	if (!rdev->me_fw || !rdev->pfp_fw || !rdev->rlc_fw) {
 		r = r600_init_microcode(rdev);
 		if (r) {
-- 
1.7.7.3

Re: [PATCH 2/2] drm/radeon/kms: move pci bus master enable further into driver.

[Alex Deucher]

On Mon, Dec 19, 2011 at 9:16 AM, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> This doesn't completely close the kexec hole for all drivers, but it fixes
> it for some cases, by enabling PCI bus mastering later.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/radeon/evergreen.c  |    2 ++
>  drivers/gpu/drm/radeon/ni.c         |    2 ++
>  drivers/gpu/drm/radeon/r100.c       |   10 ++++++++++
>  drivers/gpu/drm/radeon/r300.c       |    5 +----
>  drivers/gpu/drm/radeon/r420.c       |    4 ++--
>  drivers/gpu/drm/radeon/r600.c       |    2 ++
>  drivers/gpu/drm/radeon/radeon_kms.c |    2 --
>  drivers/gpu/drm/radeon/rv770.c      |    2 ++
>  8 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
> index 5d51035..bb01c04 100644
> --- a/drivers/gpu/drm/radeon/evergreen.c
> +++ b/drivers/gpu/drm/radeon/evergreen.c
> @@ -3001,6 +3001,8 @@ static int evergreen_startup(struct radeon_device *rdev)
>        /* enable pcie gen2 link */
>        evergreen_pcie_gen2_enable(rdev);
>
> +       pci_set_master(rdev->pdev);
> +
>        if (ASIC_IS_DCE5(rdev)) {
>                if (!rdev->me_fw || !rdev->pfp_fw || !rdev->rlc_fw || !rdev->mc_fw) {
>                        r = ni_init_microcode(rdev);
> diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
> index 1d1047a..60b4b9b 100644
> --- a/drivers/gpu/drm/radeon/ni.c
> +++ b/drivers/gpu/drm/radeon/ni.c
> @@ -1343,6 +1343,8 @@ static int cayman_startup(struct radeon_device *rdev)
>        /* enable pcie gen2 link */
>        evergreen_pcie_gen2_enable(rdev);
>
> +       pci_set_master(rdev->pdev);
> +
>        if (!rdev->me_fw || !rdev->pfp_fw || !rdev->rlc_fw || !rdev->mc_fw) {
>                r = ni_init_microcode(rdev);
>                if (r) {
> diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
> index dcc5b3c..567924c 100644
> --- a/drivers/gpu/drm/radeon/r100.c
> +++ b/drivers/gpu/drm/radeon/r100.c
> @@ -596,6 +596,16 @@ int r100_pci_gart_init(struct radeon_device *rdev)
>  void r100_enable_bm(struct radeon_device *rdev)
>  {
>        uint32_t tmp;
> +
> +       pci_set_master(rdev->pdev);
> +
> +       if (rdev->family == CHIP_R300 ||
> +           rdev->family == CHIP_R350 ||
> +           rdev->family == CHIP_RV350 ||
> +           rdev->family == CHIP_R423 ||
> +           rdev->family == CHIP_RV410)
> +               return;
> +

The logic is reversed for some of these.
RADEON_BUS_CNTL.RADEON_BUS_MASTER_DIS only exists on AGP and RS4xx
chips.  The logic should be:

/* bail early on PCIE asics */
if (rdev->family == CHIP_RV380 ||
    rdev->family == CHIP_R423 ||
    rdev->family == CHIP_RV410 ||
    rdev->family >= CHIP_RS600)
        return;

>        /* Enable bus mastering */
>        tmp = RREG32(RADEON_BUS_CNTL) & ~RADEON_BUS_MASTER_DIS;
>        WREG32(RADEON_BUS_CNTL, tmp);
> diff --git a/drivers/gpu/drm/radeon/r300.c b/drivers/gpu/drm/radeon/r300.c
> index 400b26d..b1c171f 100644
> --- a/drivers/gpu/drm/radeon/r300.c
> +++ b/drivers/gpu/drm/radeon/r300.c
> @@ -1372,10 +1372,7 @@ static int r300_startup(struct radeon_device *rdev)
>                        return r;
>        }
>
> -       if (rdev->family == CHIP_R300 ||
> -           rdev->family == CHIP_R350 ||
> -           rdev->family == CHIP_RV350)
> -               r100_enable_bm(rdev);
> +       r100_enable_bm(rdev);
>
>        if (rdev->flags & RADEON_IS_PCI) {
>                r = r100_pci_gart_enable(rdev);
> diff --git a/drivers/gpu/drm/radeon/r420.c b/drivers/gpu/drm/radeon/r420.c
> index 417fab8..b9123d6 100644
> --- a/drivers/gpu/drm/radeon/r420.c
> +++ b/drivers/gpu/drm/radeon/r420.c
> @@ -400,8 +400,8 @@ int r420_init(struct radeon_device *rdev)
>        if (r) {
>                return r;
>        }
> -       if (rdev->family == CHIP_R420)
> -               r100_enable_bm(rdev);
> +
> +       r100_enable_bm(rdev);
>
>        if (rdev->flags & RADEON_IS_PCIE) {
>                r = rv370_pcie_gart_init(rdev);
> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
> index 936d107..36c9b86 100644
> --- a/drivers/gpu/drm/radeon/r600.c
> +++ b/drivers/gpu/drm/radeon/r600.c
> @@ -2468,6 +2468,8 @@ int r600_startup(struct radeon_device *rdev)
>        /* enable pcie gen2 link */
>        r600_pcie_gen2_enable(rdev);
>
> +       pci_set_master(rdev->pdev);
> +
>        if (!rdev->me_fw || !rdev->pfp_fw || !rdev->rlc_fw) {
>                r = r600_init_microcode(rdev);
>                if (r) {
> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
> index 601b6bd..be2c122 100644
> --- a/drivers/gpu/drm/radeon/radeon_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> @@ -57,8 +57,6 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
>        }
>        dev->dev_private = (void *)rdev;
>
> -       pci_set_master(dev->pdev);
> -
>        /* update BUS flag */
>        if (drm_pci_device_is_agp(dev)) {
>                flags |= RADEON_IS_AGP;
> diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
> index e4e2132..ac1b451 100644
> --- a/drivers/gpu/drm/radeon/rv770.c
> +++ b/drivers/gpu/drm/radeon/rv770.c
> @@ -1043,6 +1043,8 @@ static int rv770_startup(struct radeon_device *rdev)
>        /* enable pcie gen2 link */
>        rv770_pcie_gen2_enable(rdev);
>
> +       pci_set_master(rdev->pdev);
> +
>        if (!rdev->me_fw || !rdev->pfp_fw || !rdev->rlc_fw) {
>                r = r600_init_microcode(rdev);
>                if (r) {
> --
> 1.7.7.3
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel