All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Tobias Powalowski <t.powa@gmx.de>, Takashi Iwai <tiwai@suse.de>,
	Christoph Hellwig <hch@infradead.org>,
	Zachary Amsden <zach@vmware.com>
Subject: [PATCH] x86/paravirt: revert exports to restore old behaviour
Date: Tue, 27 Nov 2007 14:57:30 -0800	[thread overview]
Message-ID: <474CA0DA.3010005@goop.org> (raw)

Subdividing the paravirt_ops structure caused a regression in certain
non-GPL modules which try to use mmu_ops and cpu_ops.  This restores
the old behaviour, and makes it consistent with the
non-CONFIG_PARAVIRT case.

Tobias Powalowski <t.powa@gmx.de> reports:
> commit to .24 tree:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=93b1eab3d29e7ea32ee583de3362da84db06ded8
>
> introduces:
> +EXPORT_SYMBOL_GPL(pv_mmu_ops);
> +EXPORT_SYMBOL_GPL(pv_cpu_ops);
>
> pv_cpu_ops is for nvidia
> pv_mmu_ops' is for amd(ati)
>
> which will break 32bit systems with paravirt enabled and trying to compile
> the binary graphic drivers from amd(ati) and nvidia.

Takashi Iwai <tiwai@suse.de> adds:
> I took at this problem (as I have an nvidia card on one of my
> workstations), and found out that the following suffer from
> EXPORT_SYMBOL_GPL changes:
> 
> * local_disable_irq(), local_irq_save*(), etc.
> * MSR-related macros like rdmsr(), wrmsr(), read_cr0(), etc.
>   wbinvd(), too.
> * pmd_val(), pgd_val(), etc are all involved with pv_mm_ops.
>   pmd_large() and pmd_bad() is also indirectly involved.
>   __flush_tlb() and friends suffer, too.

Christoph Hellwig objects to this patch on the grounds that modules
shouldn't be using these operations anyway.  I don't think this is a
particularly good reason to reject the patch, for several reasons:

1. These operations are still available to modules when not using
   CONFIG_PARAVIRT, since they are implicitly exported as inline
   functions via the kernel headers.  Exporting the same functionality as
   GPL-only symbols just adds a gratuitious difference between
   CONFIG_PARAVIRT and non-CONFIG_PARAVIRT configurations.  If we really
   think these operations are not for module use (or non-GPL module use),
   then we should solve the problem in a general way.

2. It's a regression from previous kernels, which would work these
   modules even with CONFIG_PARAVIRT enabled.

3. The operations in question seem pretty reasonable for modules to
   use.  The control registers/MSRs can be accessed directly anyway, so there's
   no benefit in preventing modules from using standard interfaces.  And it seems
   reasonable to allow a graphics driver to create its own mappings if it wants.

Therefore, I think this patch should go in for 2.6.24.  If people
really think that these operations should not be available to modules,
then we can address that separately.

Signed-off-by: Jeremy Fitzhardinge <Jeremy.Fitzhardinge@citrix.com>
Cc: Tobias Powalowski <t.powa@gmx.de>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: Zachary Amsden <zach@vmware.com>

---
 arch/x86/kernel/paravirt_32.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff -r 0328c71b880c arch/x86/kernel/paravirt_32.c
--- a/arch/x86/kernel/paravirt_32.c	Tue Nov 27 11:21:09 2007 -0800
+++ b/arch/x86/kernel/paravirt_32.c	Tue Nov 27 11:21:16 2007 -0800
@@ -465,8 +465,8 @@ struct pv_mmu_ops pv_mmu_ops = {
 };
 
 EXPORT_SYMBOL_GPL(pv_time_ops);
-EXPORT_SYMBOL_GPL(pv_cpu_ops);
-EXPORT_SYMBOL_GPL(pv_mmu_ops);
+EXPORT_SYMBOL    (pv_cpu_ops);
+EXPORT_SYMBOL    (pv_mmu_ops);
 EXPORT_SYMBOL_GPL(pv_apic_ops);
 EXPORT_SYMBOL_GPL(pv_info);
 EXPORT_SYMBOL    (pv_irq_ops);


             reply	other threads:[~2007-11-27 22:57 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-11-27 22:57 Jeremy Fitzhardinge [this message]
2007-11-28 20:40 ` [PATCH] x86/paravirt: revert exports to restore old behaviour Adrian Bunk
2007-11-28 21:15   ` Jeremy Fitzhardinge
2007-11-28 22:39     ` Adrian Bunk
2007-11-28 23:57       ` Jeremy Fitzhardinge
2007-11-29 22:06         ` Adrian Bunk
  -- strict thread matches above, loose matches on Subject: below --
2007-11-13 10:39 REGRESSION: 2.6.24 breaks nvidia and amd/ati binary drivers, by exporting paravirt symbols as GPL Tobias Powalowski
2007-11-13 20:21 ` [PATCH] x86/paravirt: revert exports to restore old behaviour Jeremy Fitzhardinge
2007-11-13 22:22   ` Christoph Hellwig
2007-11-14  0:51     ` Zachary Amsden
2007-11-19 17:05       ` Takashi Iwai
2007-11-20  1:14         ` Jeremy Fitzhardinge
2007-11-20  6:25           ` Takashi Iwai
2007-11-14  1:22     ` Jeremy Fitzhardinge

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=474CA0DA.3010005@goop.org \
    --to=jeremy@goop.org \
    --cc=akpm@linux-foundation.org \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=t.powa@gmx.de \
    --cc=tiwai@suse.de \
    --cc=zach@vmware.com \
    /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.