[Xen-devel] Re: [GIT PULL] devel/pat + devel/kms.fixes-0.5

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Fri, Aug 06, 2010 at 09:03:01AM -0700, Jeremy Fitzhardinge wrote:
>  On 08/06/2010 07:06 AM, Konrad Rzeszutek Wilk wrote:
> >Hey Jeremy,
> >
> >Please pull from devel/pat (based off your xen/dom0/core tree) which
> >has one patch:
> >
> >Konrad Rzeszutek Wilk (1):
> >       xen/pat: make pte_flags(x) a pvops function.
> >
> >which is neccessary for the drivers/gpu/drm/radeon driver to work
> >properly with AGP based cards (which look to be the only ones that
> >try to set WC on pages).
> 
> Hm.  I introduced pte_flags() so that code which wants to get the
> flags but not the pfn can do so without needing to make a pvops
> call, which significantly reduces the number of pvops calls in the
> mm code.
> 
> However, the original rationale for making it non-pvops - nobody
> fiddles with pte flags - is no longer true with the PAT translation.
> And it is pretty ugly that pte_val and pte_flags will return
> different flags for a pte.  But I'm still leery about imposing the
> cost of making pte_flags a pvop.
> 
> Why is it needed?  Could the AGP/Radeon code use pte_val to get the
> flags values it wants?

So it isn't really the radeon code that fiddles with this. It is the 
CPA code.

The first fix I came with was this:

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index dd38bfb..42fc7fb 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -23,7 +23,7 @@
 #include <asm/pgalloc.h>
 #include <asm/proto.h>
 #include <asm/pat.h>
-
+#include <xen/xen.h>
 /*
  * The current flushing context - we pass it instead of 5 arguments:
  */
@@ -616,6 +616,10 @@ repeat:
                pgprot_t new_prot = pte_pgprot(old_pte);
                unsigned long pfn = pte_pfn(old_pte);
 
+               if (xen_pv_domain() && (pgprot_val(new_prot) & (_PAGE_PAT | 
_PAGE_PCD | _PAGE_PWT)) == _PAGE_PAT) {
+                       pgprot_val(new_prot) = (pgprot_val(new_prot) & 
~_PAGE_PAT) | _PAGE_PWT;
+               }
+
                pgprot_val(new_prot) &= ~pgprot_val(cpa->mask_clr);
                pgprot_val(new_prot) |= pgprot_val(cpa->mask_set);
 
(just typed it up, the machine with the patch is offline). But it is kind of 
ugly.

The other option is to remove the WARN_ON in the arch/x86xen/mmu.c for
xen_make_pte and also copy some of the logic from xen_pte_val to remove
the offending flag. 


>  
> >Also please pull from devel/kms.fixes-05 (based off your xen/dom0/agp)
> >which has the following patches:
> >
> >Daniel De Graaf (1):
> >       fb: propagate VM_IO to VMA.
> >
> >Konrad Rzeszutek Wilk (10):
> >       ttm: When TTM_PAGE_FLAG_DMA32 allocate pages under 4GB under Xen.
> >       ttm: Set VM_IO only on pages with TTM_MEMTYPE_FLAG_NEEDS_IOREMAP set.
> >       agp: Use Xen back-door to get bus address for legacy code.
> >       agp: Program the GART with the real physical address under Xen.
> >       agp: If _GFP_DMA_32 flag is set enforce pages to be under 4GB under 
> > Xen.
> >       agp-backend: Use Xen back-door to get bus address for scratch page.
> >       intel-agp: Warn when !USE_PCI_DMA_API and inserting bogus bus 
> > addresses.
> >       intel-agp: Use Xen back-door to get page's physical address for 
> > i8[1,3]0
> >       amd64-agp:Use Xen back-door to get page's physical address for AMD64 
> > chipsets.
> >       ttm: Change VMA flags if they != to the TTM flags.
> >
> >Some of them are marked as not for upstream consumption, while some of
> >the other are OK (and Daniel's is upstream). Those patches from both
> >branches make it possible to have Xorg working with the following
> >graphics cards on the PVOPS kernel:
> 
> I assume this depends on the pte_flags change?

It only appeared on the Radeon R100 (7000) which is an ancient AGP video card.
(and also on the Radeon 7200)

And the problem you encounter if you don't have the fix is a stream of
WARN in the kernel, nothing else. So you can pull in this branch without
affecting negativly folks and I can work on a providing a better
solution for the pte_flags(x) issue next week.


> >RIVA TNT2 Pro
> >GeForce 1 256
> >GeForce 4 Ti 4200
> >GeForce 6150
> >GeForce 6200
> >GeForce 7300
> >GeForce 8600 GT
> >Radeon R100 QD (7200)
> >Radeon RV100QY (7000)
> >Radeon HD 3200
> >Radeon HD 3450
> >Radeon RV710 [Radeon HD 4350]
> >Radeon ES1000
> >ICH5 82865G
> >ICH7 82G33/G31G
> >ICH8 82Q963/Q965
> >Matrox G450
> >XGI Z7/Z9 (XG20 core)
> >
> >Testing was carried out using Fedora Core 13, Fedora Core 11, and Ubuntu
> >Lucid 10.04 with the PVOPS kernel.
> >
> >The details are located at http://wiki.xensource.com/xenwiki/XenPVOPSDRM
> >For the NVidia cards I backported the 2.6.34 nouvoua driver in 2.6.32
> >and used that - it is pretty stable and even the experimental Gallium
> >drivers (3D) work well.
> 
>     J

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel