Bug#503821: Purpose of features/all/xen/workaround-pte-file.patch?
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.
Reply to: