All of lore.kernel.org
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: "H. Peter Anvin" <hpa@linux.intel.com>
Cc: Robin Holt <holt@sgi.com>, Matthew Garrett <mjg59@srcf.ucam.org>,
	linux-kernel@vger.kernel.org, "Sakkinen,
	Jarkko" <jarkko.sakkinen@intel.com>
Subject: Re: [PATCH] phys_efi_set_virtual_address_map needs va, no pa.
Date: Thu, 21 Jun 2012 15:16:06 -0400	[thread overview]
Message-ID: <20120621191606.GA6885@phenom.dumpdata.com> (raw)
In-Reply-To: <4FE26EF9.5060004@linux.intel.com>

On Wed, Jun 20, 2012 at 05:46:49PM -0700, H. Peter Anvin wrote:
> On 06/20/2012 05:27 PM, Robin Holt wrote:
> > 
> > What do you need from me?  If you want me to help with this, I have a
> > _WHOLE_ lot of learning to do.  Can you give me any pointers?
> > 
> > We are trying to get this finally fixed.  We have had work-around code
> > in SLES11 SP1, SLES11 SP2, and RHEL 6.x.  I would love to get this fixed
> > for future distro snaps.
> > 
> 
> If you want to tackle it, the task is basically that when we modify the
> pgds in 32-bit legacy (non-PAE) mode, we should make the corresponding
> modifications to initial_page_table, and in 64-bit mode to
> real_mode_header->trampoline_pgd.  It might be worthwhile to introduce a
> common pointer for both, obviously.
> 
> This is currently handled via something called the pgd_list (when we
> update the top level kernel address space we walk pgd_list and update
> them all), but there are two issues:
> 
> 1. Obviously, in the case of the 1:1 map, we don't just need to maintain
> the kernel area, but the "user space" part of the address space should
> contain a copy, as well.
> 
> 2. To complicate things, there is code in there to grab an mm lock for
> the benefit of Xen.  The 1:1 map doesn't have an mm associated with it,
> so I'm not quite sure how that is to be handled.  Perhaps Xen just plain
> won't need it and we can just bypass it, but I have no bloody idea.

You mean this?

79e53d8        (Andrea Arcangeli       2011-02-16 15:45:22 -0800       127)    spin_lock(&pgd_lock);
4f76cd38        (Jeremy Fitzhardinge    2008-03-17 16:36:55 -0700       128)    pgd_list_del(pgd);
a79e53d8        (Andrea Arcangeli       2011-02-16 15:45:22 -0800       129)    spin_unlock(&pgd_lock);

which says:
    x86/mm: Fix pgd_lock deadlock
    
    It's forbidden to take the page_table_lock with the irq disabled
    or if there's contention the IPIs (for tlb flushes) sent with
    the page_table_lock held will never run leading to a deadlock.
    
    Nobody takes the pgd_lock from irq context so the _irqsave can be
    removed.
    
Looking before that git commit I see Jeremy's 4f76cd38 unification of
the 32-bit and 64-bit pgtable, and before that:

1da177e4c3f41524e886b7f1b8a0c1fc7321cac2
Linux-2.6.12-rc2

I am not really convienced that lock was put there for Xen as the
git history seems to point to well, way ancient stuff.

Or are you referring to something else?

> 
> It is also a bit "cute" how we seem to make a function call to indirect
> through a pointer (why on Earth is pgd_page_get_mm() not an inline?!),
> and then grab a lock unconditionally, regardless of if we are affected
> by Xen or not.
> 
> 	-hpa
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

      parent reply	other threads:[~2012-06-21 19:23 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-20  8:24 [PATCH] phys_efi_set_virtual_address_map needs va, no pa Robin Holt
2012-06-20 12:07 ` Matthew Garrett
2012-06-20 20:41   ` H. Peter Anvin
2012-06-21  0:27     ` Robin Holt
2012-06-21  0:46       ` H. Peter Anvin
2012-06-21 16:52         ` Robin Holt
2012-06-22  0:35           ` H. Peter Anvin
2012-06-21 19:16         ` Konrad Rzeszutek Wilk [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120621191606.GA6885@phenom.dumpdata.com \
    --to=konrad.wilk@oracle.com \
    --cc=holt@sgi.com \
    --cc=hpa@linux.intel.com \
    --cc=jarkko.sakkinen@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjg59@srcf.ucam.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.