Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 6 Aug 2014 20:38:33 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Roger Pau Monn?? <royger@FreeBSD.org>
Cc:        freebsd-current@freebsd.org, Nathan Whitehorn <nwhitehorn@freebsd.org>
Subject:   Re: r259580 breaks radeonkms
Message-ID:  <20140806173833.GJ93733@kib.kiev.ua>
In-Reply-To: <53E25D3E.7000200@FreeBSD.org>
References:  <53E178BC.4040201@freebsd.org> <53E1F6E3.1040304@FreeBSD.org> <5850878054da9ac1898035b6c5d010e5@sonic.net> <53E25D3E.7000200@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--w3DoQEssjZnO/AC+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Aug 06, 2014 at 06:52:14PM +0200, Roger Pau Monn?? wrote:
> On 06/08/14 16:35, Nathan Whitehorn wrote:
> >=20
> >=20
> > On 2014-08-06 02:35, Roger Pau Monn?? wrote:
> >> On 06/08/14 02:37, Nathan Whitehorn wrote:
> >>> Kernels with r269580 will panic when loading the radeonkms driver in
> >>> pmap_page_set_memattr(). This probably indicates a bug in radeonkms, =
but
> >>> the system is unusable in the meantime.
> >>> -Nathan
> >>
> >> I seem to be able to load radeonkms just fine after r269580:
> >=20
> > It's possible that it is related to actually using it, rather than
> > loading the module. I was only testing them together. I'm using vt and
> > the panic (page fault in kernel mode) occurs when TTM tries to set
> > memory attributes on some page while starting X. Before the panic, I see
> > all the normal Radeon module messages as you do, so the module seems to
> > have actually loaded correctly.  The KMS console also seems to be
> > functional enough to display the panic message, so I suspect it's X that
> > triggers it.
> > -Nathan
>=20
> Please try the attached patch, it seems to solve the panic for me. It
> also includes a fix for Intel i915 gem, which I'm not able to test
> because I don't have the hardware.
>=20
> Roger.
>=20

> From 9dd3a21d99ff2fc7bf3299359751d2399eee912a Mon Sep 17 00:00:00 2001
> From: Roger Pau Monne <roger.pau@citrix.com>
> Date: Wed, 6 Aug 2014 18:16:53 +0200
> Subject: [PATCH] drm: fix usage of vm_phys_fictitious_to_vm_page
>=20
> vm_phys_fictitious_to_vm_page should not be called directly, even when
> operating on a range that has been registered using
> vm_phys_fictitious_reg_range. PHYS_TO_VM_PAGE should be used instead
> because on arches that use VM_PHYSSEG_DENSE the page might come
> directly from vm_page_array.
>=20
> Reported by: nwhitehorn
> Sponsored by: Citrix Systems R&D
> ---
>  sys/dev/drm2/i915/i915_gem.c |    6 ++++--
>  sys/dev/drm2/ttm/ttm_bo_vm.c |    8 ++++++--
>  2 files changed, 10 insertions(+), 4 deletions(-)
>=20
> diff --git a/sys/dev/drm2/i915/i915_gem.c b/sys/dev/drm2/i915/i915_gem.c
> index a3acb60..6d46207 100644
> --- a/sys/dev/drm2/i915/i915_gem.c
> +++ b/sys/dev/drm2/i915/i915_gem.c
> @@ -1428,8 +1428,10 @@ retry:
> =20
>  	obj->fault_mappable =3D true;
>  	VM_OBJECT_WLOCK(vm_obj);
> -	m =3D vm_phys_fictitious_to_vm_page(dev->agp->base + obj->gtt_offset +
> -	    offset);
> +	m =3D PHYS_TO_VM_PAGE(dev->agp->base + obj->gtt_offset + offset);
> +	KASSERT((m->flags & PG_FICTITIOUS) !=3D 0,
> +	    ("physical address %#jx not fictitious",
> +	    (uintmax_t)(dev->agp->base + obj->gtt_offset + offset)));
>  	if (m =3D=3D NULL) {
>  		VM_OBJECT_WUNLOCK(vm_obj);
>  		cause =3D 60;
> diff --git a/sys/dev/drm2/ttm/ttm_bo_vm.c b/sys/dev/drm2/ttm/ttm_bo_vm.c
> index 83ec76c..7aa1ac0 100644
> --- a/sys/dev/drm2/ttm/ttm_bo_vm.c
> +++ b/sys/dev/drm2/ttm/ttm_bo_vm.c
> @@ -216,8 +216,12 @@ reserve:
>  	}
> =20
>  	if (bo->mem.bus.is_iomem) {
> -		m =3D vm_phys_fictitious_to_vm_page(bo->mem.bus.base +
> -		    bo->mem.bus.offset + offset);
> +		m =3D PHYS_TO_VM_PAGE(bo->mem.bus.base + bo->mem.bus.offset +
> +		    offset);
> +		KASSERT((m->flags & PG_FICTITIOUS) !=3D 0,
> +		    ("physical address %#jx not fictitious",
> +		    (uintmax_t)(bo->mem.bus.base + bo->mem.bus.offset
> +		    + offset)));
>  		pmap_page_set_memattr(m, ttm_io_prot(bo->mem.placement));
>  	} else {
>  		ttm =3D bo->ttm;

This should be fine.  In fact I considered doing similar change some
time ago, but for some reasons decided not to.  Still, it is not clear
why does it break with your changes, in particular, the PCI hole
where the aperture is mapped should be covered by vm_page_array.

--w3DoQEssjZnO/AC+
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJT4mgYAAoJEJDCuSvBvK1BmWUP/jMkMT8iHQ63WrK3aYiRpN7w
jjmTreBmUwlUJSxJJ9cFjwBqC/OXjLbmgoKtZx2Ru3gvUZ1Wm5x2OLAwU3Dhx7A1
aftGlUsQoStRR2FUOsUQPWTRCwp9mMCzTgieuQwX6P7j8JbJbZouAaqzKVTCTbXk
zQ+zStJ0F6dA++EOhCjD+GLNlLClJP+DCYHcvm1CKpDpiXsGI67jgGi5sE/4jsmv
eOMRkRlOVopQGFq9d9z7RMmCjWzIxsvrYI/bm0QgPGyVrFzyZoIeOsO89S4FbKd+
3x3aw49pOJcrML8Y48fxirXg0e0SUFhbifLb8H+j/QA5C9xrGKXI23vGc1Nz+LhN
buocbDy5HPxyyTGLEce3NljupdXRcxN2zIpB7UoWjb7J5BKSYZuIhzER5SCTMKoO
ShQ4iwBCDM0YVIPLgvvcYlcy7r+3mu5gypwHu74mD5Ow3GeSuxZIDD1FGtz78gol
xHgYZS88HsCsbRZt4VEbYFIA1V75ahot51TVO1HmfqmAkjR1xoPoKwXXe+fPAokz
NOGJ4d3qdWu/SAG/FMks+Aav9qozo8QIIpg3yYVYKIW6ovqfkNdwpBHqGU46iFDL
/DJ/qlF1do0MwYAo8hjMOH19fGfyZrQ9+3llbhXS3Pydews06H9XXOW7RvBcbJSl
CD60H7wxCvijGPj51cTC
=RXaN
-----END PGP SIGNATURE-----

--w3DoQEssjZnO/AC+--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140806173833.GJ93733>