* Re: [PATCH] x86/mm/pat: Fix BUG_ON in mmap_mem on QEMU/i386
[not found] ` <20160405110947.GB10109@pd.tnic>
@ 2016-04-05 15:24 ` Toshi Kani
2016-04-08 16:34 ` Toshi Kani
[not found] ` <1460133294.20338.82.camel@hpe.com>
0 siblings, 2 replies; 4+ messages in thread
From: Toshi Kani @ 2016-04-05 15:24 UTC (permalink / raw)
To: Borislav Petkov
Cc: ying.huang, x86, linux-kernel, hpa, xen-devel, tglx, mingo
+xen-devl
On Tue, 2016-04-05 at 13:09 +0200, Borislav Petkov wrote:
> On Fri, Apr 01, 2016 at 04:19:45PM -0600, Toshi Kani wrote:
> >
> > The following BUG_ON error was reported on QEMU/i386:
> >
> > kernel BUG at arch/x86/mm/physaddr.c:79!
> > Call Trace:
> > phys_mem_access_prot_allowed
> > mmap_mem
> > ? mmap_region
> > mmap_region
> > do_mmap
> > vm_mmap_pgoff
> > SyS_mmap_pgoff
> > do_int80_syscall_32
> > entry_INT80_32
> >
> > after commit edfe63ec97ed ("x86/mtrr: Fix Xorg crashes in Qemu
> > sessions").
> >
> > PAT is now set to disabled state when MTRRs are disabled...
> "... thus reactivating the __pa(high_memory) check in
> phys_mem_access_prot_allowed()."
Will do.
> >
> > When the system does not have much memory, 'high_memory' points to
> What does "much memory" mean, exactly?
I meant to say when a 32-bit system does not have ZONE_HIGHMEM,
__pa(high_memory) points to the maximum memory address + 1.
I will remove this sentence since it is irrelevant to this BUG_ON. Even if
a 32-bit system does have ZONE_HIGHMEM, slow_virt_to_phys() still returns 0
for high_memory because it is set to the maximum direct mapped address + 1
in this case. This address is not covered by page table, either.
But this made me realized that this high_memory check can be harmful in
such case, ie. __pa(high_memory) is not the maximum memory address when
ZONE_HIGHMEM is present.
I assume when this code block was originally added, legacy systems without
MTRRs did not have ZONE_HIGHMEM. However, MTRRs are also disabled on Xen.
Reactivating this code may cause an issue on Xen 32-bit guests with
ZONE_HIGHMEM.
Question to Xen folks: Does Xen support 32-bit guests with ZONE_HIGHMEM?
If yes, a safer fix may be to remove this code block since it was deadcode
anyway...
> > the maximum memory address + 1, which is empty. When
> > CONFIG_DEBUG_VIRTUAL is also set, __pa() calls __phys_addr(), which
> > in turn calls slow_virt_to_phys() for high_memory. Because
> > high_memory does not point to a valid memory address, this address
> > is not mapped...
> "... and slow_virt_to_phys() returns 0."
Will do.
> > Hence, BUG_ON.
> >
> > Use __pa_nodebug() as the code does not expect a valid virtual
> > mapping for high_memory.
> >
> > Reported-by: kernel test robot <ying.huang@linux.intel.com>
> > Link: https://lkml.org/lkml/2016/4/1/608
> > Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
> > Thomas Gleixner <tglx@linutronix.de>
> > Ingo Molnar <mingo@kernel.org>
> > H. Peter Anvin <hpa@zytor.com>
> > Borislav Petkov <bp@suse.de>
> > ---
> > This patch is based on -tip.
> > ---
> > arch/x86/mm/pat.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/mm/pat.c b/arch/x86/mm/pat.c
> > index c4c3ddc..26b7202 100644
> > --- a/arch/x86/mm/pat.c
> > +++ b/arch/x86/mm/pat.c
> > @@ -792,7 +792,7 @@ int phys_mem_access_prot_allowed(struct file *file,
> > unsigned long pfn,
> > boot_cpu_has(X86_FEATURE_K6_MTRR) ||
> > boot_cpu_has(X86_FEATURE_CYRIX_ARR) ||
> > boot_cpu_has(X86_FEATURE_CENTAUR_MCR)) &&
> > - (pfn << PAGE_SHIFT) >= __pa(high_memory)) {
> > + (pfn << PAGE_SHIFT) >= __pa_nodebug(high_memory)) {
> > pcm = _PAGE_CACHE_MODE_UC;
> > }
> > #endif
> Modulo the minor formulations issues above,
>
> Reviewed-by: Borislav Petkov <bp@suse.de>
>
> AFAIU, it makes sense to do the "nodebug" check here anyway - we
> basically only want to *check* the address and if outside of available
> memory, map UC. We shouldn't be exploding just because we're checking.
>
> But this is just me, someone should doublecheck this train of thought
> for sanity.
Yes, let's check with Xen on this.
Thanks,
-Toshi
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/mm/pat: Fix BUG_ON in mmap_mem on QEMU/i386
2016-04-05 15:24 ` [PATCH] x86/mm/pat: Fix BUG_ON in mmap_mem on QEMU/i386 Toshi Kani
@ 2016-04-08 16:34 ` Toshi Kani
[not found] ` <1460133294.20338.82.camel@hpe.com>
1 sibling, 0 replies; 4+ messages in thread
From: Toshi Kani @ 2016-04-08 16:34 UTC (permalink / raw)
To: Borislav Petkov
Cc: ying.huang, x86, linux-kernel, hpa, xen-devel, tglx, mingo
On Tue, 2016-04-05 at 09:24 -0600, Toshi Kani wrote:
> +xen-devl
>
> On Tue, 2016-04-05 at 13:09 +0200, Borislav Petkov wrote:
> > On Fri, Apr 01, 2016 at 04:19:45PM -0600, Toshi Kani wrote:
> > >
:
> > >
> > > When the system does not have much memory, 'high_memory' points to
> >
> > What does "much memory" mean, exactly?
>
> I meant to say when a 32-bit system does not have ZONE_HIGHMEM,
> __pa(high_memory) points to the maximum memory address + 1.
>
> I will remove this sentence since it is irrelevant to this BUG_ON. Even
> if a 32-bit system does have ZONE_HIGHMEM, slow_virt_to_phys() still
> returns 0 for high_memory because it is set to the maximum direct mapped
> address + 1 in this case. This address is not covered by page table,
> either.
>
> But this made me realized that this high_memory check can be harmful in
> such case, ie. __pa(high_memory) is not the maximum memory address when
> ZONE_HIGHMEM is present.
>
> I assume when this code block was originally added, legacy systems
> without MTRRs did not have ZONE_HIGHMEM. However, MTRRs are also
> disabled on Xen. Reactivating this code may cause an issue on Xen 32-bit
> guests with ZONE_HIGHMEM.
>
> Question to Xen folks: Does Xen support 32-bit guests with ZONE_HIGHMEM?
>
> If yes, a safer fix may be to remove this code block since it was
> deadcode anyway...
I have not heard a confirmation from Xen folks, but I believe ZONE_HIGHMEM
is supported on 32-bit Xen guests. So, unless someone has an objection, I
am going to remove this code block in the next version of this patch.
Thanks,
-Toshi
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/mm/pat: Fix BUG_ON in mmap_mem on QEMU/i386
[not found] ` <5707E392.2090106@citrix.com>
@ 2016-04-08 16:56 ` Toshi Kani
0 siblings, 0 replies; 4+ messages in thread
From: Toshi Kani @ 2016-04-08 16:56 UTC (permalink / raw)
To: David Vrabel, Borislav Petkov
Cc: ying.huang, x86, linux-kernel, hpa, xen-devel, tglx, mingo
On Fri, 2016-04-08 at 18:00 +0100, David Vrabel wrote:
> On 08/04/16 17:34, Toshi Kani wrote:
> >
> > On Tue, 2016-04-05 at 09:24 -0600, Toshi Kani wrote:
> > >
> > > +xen-devl
> > >
> > > On Tue, 2016-04-05 at 13:09 +0200, Borislav Petkov wrote:
> > > >
> > > > On Fri, Apr 01, 2016 at 04:19:45PM -0600, Toshi Kani wrote:
> > > > >
> > :
> > > > >
> > > > > When the system does not have much memory, 'high_memory' points
> > > > > to What does "much memory" mean, exactly?
> > >
> > > I meant to say when a 32-bit system does not have ZONE_HIGHMEM,
> > > __pa(high_memory) points to the maximum memory address + 1.
> > >
> > > I will remove this sentence since it is irrelevant to this
> > > BUG_ON. Even if a 32-bit system does have ZONE_HIGHMEM,
> > > slow_virt_to_phys() still returns 0 for high_memory because it is set
> > > to the maximum direct mapped address + 1 in this case. This address
> > > is not covered by page table, either.
> > >
> > > But this made me realized that this high_memory check can be harmful
> > > in such case, ie. __pa(high_memory) is not the maximum memory address
> > > when ZONE_HIGHMEM is present.
> > >
> > > I assume when this code block was originally added, legacy systems
> > > without MTRRs did not have ZONE_HIGHMEM. However, MTRRs are also
> > > disabled on Xen. Reactivating this code may cause an issue on Xen 32-
> > > bit guests with ZONE_HIGHMEM.
> > >
> > > Question to Xen folks: Does Xen support 32-bit guests with
> > > ZONE_HIGHMEM?
> > >
> > > If yes, a safer fix may be to remove this code block since it was
> > > deadcode anyway...
> >
> > I have not heard a confirmation from Xen folks, but I believe
> > ZONE_HIGHMEM is supported on 32-bit Xen guests. So, unless someone has
> > an objection, I am going to remove this code block in the next version
> > of this patch.
>
> 32-bit Xen guests have highmem, yes.
Thanks David!
-Toshi
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/mm/pat: Fix BUG_ON in mmap_mem on QEMU/i386
[not found] ` <1460133294.20338.82.camel@hpe.com>
[not found] ` <5707E392.2090106@citrix.com>
@ 2016-04-08 17:00 ` David Vrabel
1 sibling, 0 replies; 4+ messages in thread
From: David Vrabel @ 2016-04-08 17:00 UTC (permalink / raw)
To: Toshi Kani, Borislav Petkov
Cc: ying.huang, x86, linux-kernel, hpa, xen-devel, tglx, mingo
On 08/04/16 17:34, Toshi Kani wrote:
> On Tue, 2016-04-05 at 09:24 -0600, Toshi Kani wrote:
>> +xen-devl
>>
>> On Tue, 2016-04-05 at 13:09 +0200, Borislav Petkov wrote:
>>> On Fri, Apr 01, 2016 at 04:19:45PM -0600, Toshi Kani wrote:
>>>>
> :
>>>>
>>>> When the system does not have much memory, 'high_memory' points to
>>>
>>> What does "much memory" mean, exactly?
>>
>> I meant to say when a 32-bit system does not have ZONE_HIGHMEM,
>> __pa(high_memory) points to the maximum memory address + 1.
>>
>> I will remove this sentence since it is irrelevant to this BUG_ON. Even
>> if a 32-bit system does have ZONE_HIGHMEM, slow_virt_to_phys() still
>> returns 0 for high_memory because it is set to the maximum direct mapped
>> address + 1 in this case. This address is not covered by page table,
>> either.
>>
>> But this made me realized that this high_memory check can be harmful in
>> such case, ie. __pa(high_memory) is not the maximum memory address when
>> ZONE_HIGHMEM is present.
>>
>> I assume when this code block was originally added, legacy systems
>> without MTRRs did not have ZONE_HIGHMEM. However, MTRRs are also
>> disabled on Xen. Reactivating this code may cause an issue on Xen 32-bit
>> guests with ZONE_HIGHMEM.
>>
>> Question to Xen folks: Does Xen support 32-bit guests with ZONE_HIGHMEM?
>>
>> If yes, a safer fix may be to remove this code block since it was
>> deadcode anyway...
>
> I have not heard a confirmation from Xen folks, but I believe ZONE_HIGHMEM
> is supported on 32-bit Xen guests. So, unless someone has an objection, I
> am going to remove this code block in the next version of this patch.
32-bit Xen guests have highmem, yes.
David
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-04-08 17:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1459549185-14911-1-git-send-email-toshi.kani@hpe.com>
[not found] ` <20160405110947.GB10109@pd.tnic>
2016-04-05 15:24 ` [PATCH] x86/mm/pat: Fix BUG_ON in mmap_mem on QEMU/i386 Toshi Kani
2016-04-08 16:34 ` Toshi Kani
[not found] ` <1460133294.20338.82.camel@hpe.com>
[not found] ` <5707E392.2090106@citrix.com>
2016-04-08 16:56 ` Toshi Kani
2016-04-08 17:00 ` David Vrabel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).