On Wed, 2008-11-05 at 14:50 +0100, Bastian Blank wrote:
> On Wed, Nov 05, 2008 at 01:10:30PM +0000, Ian Campbell wrote:
> > On Wed, 2008-11-05 at 10:05 +0000, Ian Campbell wrote:
> > > > What happens if you use "nopat" to disable the usage of PAT?
> > > CONFIG_X86_PAT is disabled anyway so it makes no difference.
> > Although it does turn out that PAT is implicated...
> 
> Ah.
> 
> >  /* Set of bits not changed in pte_modify */
> > -#define _PAGE_CHG_MASK     (PTE_MASK | _PAGE_CACHE_MASK | _PAGE_IO |       
> > \
> > +#define _PAGE_CHG_MASK     (PTE_MASK | _PAGE_PCD | _PAGE_PWT | _PAGE_IO |  
> > \
> >                      _PAGE_ACCESSED | _PAGE_DIRTY)
> 
> This is the direct expansion, why?

Just because the native version uses the expanded version and I was
playing with it earlier. I decided to keep it since the difference
seemed unnecessary.

> > +#else
> > +   /* This is identical to page table setting without PAT */
> > +   if (ret_type) {
> > +           if (req_type == -1) {
> > +                   *ret_type = _PAGE_CACHE_WB;
> > +           } else {
> > +                   *ret_type = req_type;
> > +           }
> > +   }
> > +   return 0;
> > +#endif
> 
> Code duplication. Better force pat_wc_enabled to 0 and let the compiler
> do this instead of forcing the knowledge into the preprocessor.

pat_wc_enable is already const int = 0, I just did it via the
preprocessor to prove that _PAGE_CACHE_WC was unused at build time,
about 90% of the patch is only because of that. I could drop everything
apart from the _PAGE_CACHE_MASK and the Kconfig change and the fix would
still be valid. I just felt that making the code completely unavailable
was safer in case something sneaks in since it will be a build failure.

In this case I could have simply ifdef'd the main function body, I've
made that change.

> I would also make several parts dependant on _PAGE_CACHE_WC instead of
> X86_PAT.

OK.

Fresh patch attached. I'll test the proper Debian config rather than my
cut down one now.

If I remove workaround-pte-file.patch is the correct way to do that to
remove the reference from debian/patches/series/7-extra or to add a "-"
type reference to 10-extra?

Ian.

Index: sid-xen/include/asm-x86/mach-xen/asm/pgtable.h
===================================================================
--- sid-xen.orig/include/asm-x86/mach-xen/asm/pgtable.h 2008-11-05 
13:07:33.000000000 +0000
+++ sid-xen/include/asm-x86/mach-xen/asm/pgtable.h      2008-11-05 
14:08:15.000000000 +0000
@@ -67,18 +67,20 @@
                         _PAGE_DIRTY | __kernel_page_user)
 
 /* Set of bits not changed in pte_modify */
-#define _PAGE_CHG_MASK (PTE_MASK | _PAGE_CACHE_MASK | _PAGE_IO |       \
+#define _PAGE_CHG_MASK (PTE_MASK | _PAGE_PCD | _PAGE_PWT | _PAGE_IO |  \
                         _PAGE_ACCESSED | _PAGE_DIRTY)
 
 /*
  * PAT settings are part of the hypervisor interface, which sets the
  * MSR to 0x050100070406 (i.e. WB, WT, UC-, UC, WC, WP [, UC, UC]).
  */
-#define _PAGE_CACHE_MASK       (_PAGE_PCD | _PAGE_PWT | _PAGE_PAT)
+#define _PAGE_CACHE_MASK       (_PAGE_PCD | _PAGE_PWT)
 #define _PAGE_CACHE_WB         (0)
+#ifdef CONFIG_X86_PAT
 #define _PAGE_CACHE_WT         (_PAGE_PWT)
 #define _PAGE_CACHE_WC         (_PAGE_PAT)
 #define _PAGE_CACHE_WP         (_PAGE_PAT | _PAGE_PWT)
+#endif
 #define _PAGE_CACHE_UC_MINUS   (_PAGE_PCD)
 #define _PAGE_CACHE_UC         (_PAGE_PCD | _PAGE_PWT)
 
Index: sid-xen/arch/x86/Kconfig
===================================================================
--- sid-xen.orig/arch/x86/Kconfig       2008-11-05 13:07:33.000000000 +0000
+++ sid-xen/arch/x86/Kconfig    2008-11-05 14:08:15.000000000 +0000
@@ -1141,6 +1141,7 @@
        bool
        prompt "x86 PAT support"
        depends on MTRR
+       depends on !XEN
        help
          Use PAT attributes to setup page level cache control.
 
Index: sid-xen/arch/x86/mm/ioremap-xen.c
===================================================================
--- sid-xen.orig/arch/x86/mm/ioremap-xen.c      2008-11-05 13:07:33.000000000 
+0000
+++ sid-xen/arch/x86/mm/ioremap-xen.c   2008-11-05 14:08:15.000000000 +0000
@@ -255,9 +255,11 @@
        default:
                err = _set_memory_uc(vaddr, nrpages);
                break;
+#ifdef _PAGE_CACHE_WC
        case _PAGE_CACHE_WC:
                err = _set_memory_wc(vaddr, nrpages);
                break;
+#endif
        case _PAGE_CACHE_WB:
                err = _set_memory_wb(vaddr, nrpages);
                break;
@@ -340,11 +342,17 @@
                 * - request is uc-, return cannot be write-combine
                 * - request is write-combine, return cannot be write-back
                 */
-               if ((prot_val == _PAGE_CACHE_UC_MINUS &&
+               if (
+#ifdef _PAGE_CACHE_WC
+                   (prot_val == _PAGE_CACHE_UC_MINUS &&
                     (new_prot_val == _PAGE_CACHE_WB ||
                      new_prot_val == _PAGE_CACHE_WC)) ||
                    (prot_val == _PAGE_CACHE_WC &&
-                    new_prot_val == _PAGE_CACHE_WB)) {
+                    new_prot_val == _PAGE_CACHE_WB)
+#else
+                   (prot_val == _PAGE_CACHE_UC_MINUS && new_prot_val == 
_PAGE_CACHE_WB)
+#endif
+                  ) {
                        pr_debug(
                "ioremap error for 0x%llx-0x%llx, requested 0x%lx, got 0x%lx\n",
                                (unsigned long long)phys_addr,
@@ -364,9 +372,11 @@
        case _PAGE_CACHE_UC_MINUS:
                prot = PAGE_KERNEL_UC_MINUS;
                break;
+#ifdef _PAGE_CACHE_WC
        case _PAGE_CACHE_WC:
                prot = PAGE_KERNEL_WC;
                break;
+#endif
        case _PAGE_CACHE_WB:
                prot = PAGE_KERNEL;
                break;
@@ -445,10 +455,12 @@
  */
 void __iomem *ioremap_wc(unsigned long phys_addr, unsigned long size)
 {
+#ifdef CONFIG_X86_PAT
        if (pat_wc_enabled)
                return __ioremap_caller(phys_addr, size, _PAGE_CACHE_WC,
                                        __builtin_return_address(0));
        else
+#endif
                return ioremap_nocache(phys_addr, size);
 }
 EXPORT_SYMBOL(ioremap_wc);
Index: sid-xen/arch/x86/mm/pageattr-xen.c
===================================================================
--- sid-xen.orig/arch/x86/mm/pageattr-xen.c     2008-11-05 13:07:33.000000000 
+0000
+++ sid-xen/arch/x86/mm/pageattr-xen.c  2008-11-05 14:08:15.000000000 +0000
@@ -817,14 +817,17 @@
 }
 EXPORT_SYMBOL(set_memory_uc);
 
+#ifdef CONFIG_X86_PAT
 int _set_memory_wc(unsigned long addr, int numpages)
 {
        return change_page_attr_set(addr, numpages,
                                    __pgprot(_PAGE_CACHE_WC));
 }
+#endif
 
 int set_memory_wc(unsigned long addr, int numpages)
 {
+#ifdef CONFIG_X86_PAT
        if (!pat_wc_enabled)
                return set_memory_uc(addr, numpages);
 
@@ -833,6 +836,9 @@
                return -EINVAL;
 
        return _set_memory_wc(addr, numpages);
+#else
+       return set_memory_uc(addr, numpages);
+#endif
 }
 EXPORT_SYMBOL(set_memory_wc);
 
Index: sid-xen/arch/x86/mm/pat-xen.c
===================================================================
--- sid-xen.orig/arch/x86/mm/pat-xen.c  2008-11-05 13:07:33.000000000 +0000
+++ sid-xen/arch/x86/mm/pat-xen.c       2008-11-05 14:09:30.000000000 +0000
@@ -116,9 +116,15 @@
                case _PAGE_CACHE_UC:            return "uncached";
                case _PAGE_CACHE_UC_MINUS:      return "uncached-minus";
                case _PAGE_CACHE_WB:            return "write-back";
+#ifdef _PAGE_CACHE_WC
                case _PAGE_CACHE_WC:            return "write-combining";
+#endif
+#ifdef _PAGE_CACHE_WP
                case _PAGE_CACHE_WP:            return "write-protected";
+#endif
+#ifdef _PAGE_CACHE_WT
                case _PAGE_CACHE_WT:            return "write-through";
+#endif
                default:                        return "broken";
        }
 }
@@ -157,6 +163,7 @@
  * The intersection is based on "Effective Memory Type" tables in IA-32
  * SDM vol 3a
  */
+#ifdef CONFIG_X86_PAT
 static int pat_x_mtrr_type(u64 start, u64 end, unsigned long prot,
                                unsigned long *ret_prot)
 {
@@ -199,6 +206,7 @@
 
        return 0;
 }
+#endif
 
 /*
  * req_type typically has one of the:
@@ -218,10 +226,12 @@
 int reserve_memtype(u64 start, u64 end, unsigned long req_type,
                        unsigned long *ret_type)
 {
+#ifdef CONFIG_X86_PAT
        struct memtype *new_entry = NULL;
        struct memtype *parse;
        unsigned long actual_type;
        int err = 0;
+#endif
 
        /* Only track when pat_wc_enabled */
        if (!pat_wc_enabled) {
@@ -236,6 +246,7 @@
                return 0;
        }
 
+#ifdef CONFIG_X86_PAT
        /* Low ISA region is always mapped WB in page table. No need to track */
        if (start >= ISA_START_ADDRESS && (end - 1) <= ISA_END_ADDRESS) {
                if (ret_type)
@@ -431,6 +442,7 @@
 
        spin_unlock(&memtype_lock);
        return err;
+#endif
 }
 
 int free_memtype(u64 start, u64 end)

-- 
Ian Campbell

Licensed and bonded.




-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]

Reply via email to