Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1578909 > unrolled thread

[PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as __ro_after_init

Started byJess Frazelle <me@jessfraz.com>
First post2017-02-11 02:40 +0100
Last post2017-02-11 13:20 +0100
Articles 10 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as __ro_after_init Jess Frazelle <me@jessfraz.com> - 2017-02-11 02:40 +0100
    [PATCH v2 5/5] x86: set msi_domain_ops as __ro_after_init Jess Frazelle <me@jessfraz.com> - 2017-02-11 02:40 +0100
    [PATCH v2 3/5] pci: set msi_domain_ops as __ro_after_init Jess Frazelle <me@jessfraz.com> - 2017-02-11 02:40 +0100
      RE: [PATCH v2 3/5] pci: set msi_domain_ops as __ro_after_init KY Srinivasan <kys@microsoft.com> - 2017-02-12 05:10 +0100
      Re: [PATCH v2 3/5] pci: set msi_domain_ops as __ro_after_init Keith Busch <keith.busch@intel.com> - 2017-02-13 19:10 +0100
    Re: [PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as  __ro_after_init Thomas Gleixner <tglx@linutronix.de> - 2017-02-11 10:20 +0100
      Re: [PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as  __ro_after_init Thomas Gleixner <tglx@linutronix.de> - 2017-02-11 10:30 +0100
      Re: [PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as __ro_after_init Jess Frazelle <me@jessfraz.com> - 2017-02-11 11:50 +0100
        Re: [PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as  __ro_after_init Thomas Gleixner <tglx@linutronix.de> - 2017-02-11 13:10 +0100
          Re: [PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as __ro_after_init Jessica Frazelle <me@jessfraz.com> - 2017-02-11 13:20 +0100

#1578909 — [PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as __ro_after_init

FromJess Frazelle <me@jessfraz.com>
Date2017-02-11 02:40 +0100
Subject[PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as __ro_after_init
Message-ID<t9uPn-2IM-5@gated-at.bofh.it>
Marked msi_domain_ops structs as __ro_after_init when called only during init.
Marked syscore_ops structs as __ro_after_init when register_syscore_ops was
called only during init. Most of the caller functions were already annotated as
__init.
unregister_syscore_ops() was never called on these syscore_ops.
This protects the data structure from accidental corruption.

Suggested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Jess Frazelle <me@jessfraz.com>
---
 kernel/irq/generic-chip.c | 2 +-
 kernel/irq/msi.c          | 2 +-
 kernel/irq/pm.c           | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index ee32870079c9..cca63dbaabea 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -623,7 +623,7 @@ static void irq_gc_shutdown(void)
 	}
 }

-static struct syscore_ops irq_gc_syscore_ops = {
+static struct syscore_ops irq_gc_syscore_ops __ro_after_init = {
 	.suspend = irq_gc_suspend,
 	.resume = irq_gc_resume,
 	.shutdown = irq_gc_shutdown,
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index ee230063f033..0e5b723f710f 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -217,7 +217,7 @@ static int msi_domain_ops_check(struct irq_domain *domain,
 	return 0;
 }

-static struct msi_domain_ops msi_domain_ops_default = {
+static struct msi_domain_ops msi_domain_ops_default __ro_after_init = {
 	.get_hwirq	= msi_domain_ops_get_hwirq,
 	.msi_init	= msi_domain_ops_init,
 	.msi_check	= msi_domain_ops_check,
diff --git a/kernel/irq/pm.c b/kernel/irq/pm.c
index cea1de0161f1..d6b889bed323 100644
--- a/kernel/irq/pm.c
+++ b/kernel/irq/pm.c
@@ -185,7 +185,7 @@ static void irq_pm_syscore_resume(void)
 	resume_irqs(true);
 }

-static struct syscore_ops irq_pm_syscore_ops = {
+static struct syscore_ops irq_pm_syscore_ops __ro_after_init = {
 	.resume		= irq_pm_syscore_resume,
 };

--
2.11.0

[toc] | [next] | [standalone]


#1578910 — [PATCH v2 5/5] x86: set msi_domain_ops as __ro_after_init

FromJess Frazelle <me@jessfraz.com>
Date2017-02-11 02:40 +0100
Subject[PATCH v2 5/5] x86: set msi_domain_ops as __ro_after_init
Message-ID<t9uPo-2IM-13@gated-at.bofh.it>
In reply to#1578909
Marked msi_domain_ops structs as __ro_after_init when called only during init.
This protects the data structure from accidental corruption.

Suggested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Jess Frazelle <me@jessfraz.com>
---
 arch/x86/kernel/apic/msi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
index 015bbf30e3e3..27783a1e7166 100644
--- a/arch/x86/kernel/apic/msi.c
+++ b/arch/x86/kernel/apic/msi.c
@@ -121,7 +121,7 @@ void pci_msi_set_desc(msi_alloc_info_t *arg, struct msi_desc *desc)
 }
 EXPORT_SYMBOL_GPL(pci_msi_set_desc);

-static struct msi_domain_ops pci_msi_domain_ops = {
+static struct msi_domain_ops pci_msi_domain_ops __ro_after_init = {
 	.get_hwirq	= pci_msi_get_hwirq,
 	.msi_prepare	= pci_msi_prepare,
 	.set_desc	= pci_msi_set_desc,
@@ -207,7 +207,7 @@ static int dmar_msi_init(struct irq_domain *domain,
 	return 0;
 }

-static struct msi_domain_ops dmar_msi_domain_ops = {
+static struct msi_domain_ops dmar_msi_domain_ops __ro_after_init = {
 	.get_hwirq	= dmar_msi_get_hwirq,
 	.msi_init	= dmar_msi_init,
 };
@@ -304,7 +304,7 @@ static void hpet_msi_free(struct irq_domain *domain,
 	irq_clear_status_flags(virq, IRQ_MOVE_PCNTXT);
 }

-static struct msi_domain_ops hpet_msi_domain_ops = {
+static struct msi_domain_ops hpet_msi_domain_ops __ro_after_init = {
 	.get_hwirq	= hpet_msi_get_hwirq,
 	.msi_init	= hpet_msi_init,
 	.msi_free	= hpet_msi_free,
--
2.11.0

[toc] | [prev] | [next] | [standalone]


#1578913 — [PATCH v2 3/5] pci: set msi_domain_ops as __ro_after_init

FromJess Frazelle <me@jessfraz.com>
Date2017-02-11 02:40 +0100
Subject[PATCH v2 3/5] pci: set msi_domain_ops as __ro_after_init
Message-ID<t9uPo-2IM-19@gated-at.bofh.it>
In reply to#1578909
Marked msi_domain_ops structs as __ro_after_init when called only during init.
This protects the data structure from accidental corruption.

Suggested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Jess Frazelle <me@jessfraz.com>
---
 drivers/pci/host/pci-hyperv.c | 2 +-
 drivers/pci/host/vmd.c        | 2 +-
 drivers/pci/msi.c             | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 3efcc7bdc5fb..f05b93689d8f 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -958,7 +958,7 @@ static irq_hw_number_t hv_msi_domain_ops_get_hwirq(struct msi_domain_info *info,
 	return arg->msi_hwirq;
 }

-static struct msi_domain_ops hv_msi_ops = {
+static struct msi_domain_ops hv_msi_ops __ro_after_init = {
 	.get_hwirq	= hv_msi_domain_ops_get_hwirq,
 	.msi_prepare	= pci_msi_prepare,
 	.set_desc	= pci_msi_set_desc,
diff --git a/drivers/pci/host/vmd.c b/drivers/pci/host/vmd.c
index 18ef1a93c10a..152c461538e4 100644
--- a/drivers/pci/host/vmd.c
+++ b/drivers/pci/host/vmd.c
@@ -253,7 +253,7 @@ static void vmd_set_desc(msi_alloc_info_t *arg, struct msi_desc *desc)
 	arg->desc = desc;
 }

-static struct msi_domain_ops vmd_msi_domain_ops = {
+static struct msi_domain_ops vmd_msi_domain_ops __ro_after_init = {
 	.get_hwirq	= vmd_get_hwirq,
 	.msi_init	= vmd_msi_init,
 	.msi_free	= vmd_msi_free,
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 50c5003295ca..93141d5e2d1c 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -1413,7 +1413,7 @@ static void pci_msi_domain_set_desc(msi_alloc_info_t *arg,
 #define pci_msi_domain_set_desc		NULL
 #endif

-static struct msi_domain_ops pci_msi_domain_ops_default = {
+static struct msi_domain_ops pci_msi_domain_ops_default __ro_after_init = {
 	.set_desc	= pci_msi_domain_set_desc,
 	.msi_check	= pci_msi_domain_check_cap,
 	.handle_error	= pci_msi_domain_handle_error,
--
2.11.0

[toc] | [prev] | [next] | [standalone]


#1579159 — RE: [PATCH v2 3/5] pci: set msi_domain_ops as __ro_after_init

FromKY Srinivasan <kys@microsoft.com>
Date2017-02-12 05:10 +0100
SubjectRE: [PATCH v2 3/5] pci: set msi_domain_ops as __ro_after_init
Message-ID<t9TE5-1pR-1@gated-at.bofh.it>
In reply to#1578913

> -----Original Message-----
> From: Jess Frazelle [mailto:me@jessfraz.com]
> Sent: Friday, February 10, 2017 5:38 PM
> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; Stephen Hemminger
> <sthemmin@microsoft.com>; Bjorn Helgaas <bhelgaas@google.com>; Keith
> Busch <keith.busch@intel.com>; open list:Hyper-V CORE AND DRIVERS
> <devel@linuxdriverproject.org>; open list:PCI SUBSYSTEM <linux-
> pci@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>
> Cc: kernel-hardening@lists.openwall.com; Jess Frazelle <me@jessfraz.com>
> Subject: [PATCH v2 3/5] pci: set msi_domain_ops as __ro_after_init
> 
> Marked msi_domain_ops structs as __ro_after_init when called only during
> init.
> This protects the data structure from accidental corruption.
> 
> Suggested-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Jess Frazelle <me@jessfraz.com>

Acked-by: K. Y. Srinivasan <kys@microsoft.com>

K. Y

> ---
>  drivers/pci/host/pci-hyperv.c | 2 +-
>  drivers/pci/host/vmd.c        | 2 +-
>  drivers/pci/msi.c             | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 3efcc7bdc5fb..f05b93689d8f 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -958,7 +958,7 @@ static irq_hw_number_t
> hv_msi_domain_ops_get_hwirq(struct msi_domain_info *info,
>  	return arg->msi_hwirq;
>  }
> 
> -static struct msi_domain_ops hv_msi_ops = {
> +static struct msi_domain_ops hv_msi_ops __ro_after_init = {
>  	.get_hwirq	= hv_msi_domain_ops_get_hwirq,
>  	.msi_prepare	= pci_msi_prepare,
>  	.set_desc	= pci_msi_set_desc,
> diff --git a/drivers/pci/host/vmd.c b/drivers/pci/host/vmd.c
> index 18ef1a93c10a..152c461538e4 100644
> --- a/drivers/pci/host/vmd.c
> +++ b/drivers/pci/host/vmd.c
> @@ -253,7 +253,7 @@ static void vmd_set_desc(msi_alloc_info_t *arg,
> struct msi_desc *desc)
>  	arg->desc = desc;
>  }
> 
> -static struct msi_domain_ops vmd_msi_domain_ops = {
> +static struct msi_domain_ops vmd_msi_domain_ops __ro_after_init = {
>  	.get_hwirq	= vmd_get_hwirq,
>  	.msi_init	= vmd_msi_init,
>  	.msi_free	= vmd_msi_free,
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 50c5003295ca..93141d5e2d1c 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -1413,7 +1413,7 @@ static void
> pci_msi_domain_set_desc(msi_alloc_info_t *arg,
>  #define pci_msi_domain_set_desc		NULL
>  #endif
> 
> -static struct msi_domain_ops pci_msi_domain_ops_default = {
> +static struct msi_domain_ops pci_msi_domain_ops_default __ro_after_init
> = {
>  	.set_desc	= pci_msi_domain_set_desc,
>  	.msi_check	= pci_msi_domain_check_cap,
>  	.handle_error	= pci_msi_domain_handle_error,
> --
> 2.11.0

[toc] | [prev] | [next] | [standalone]


#1579976 — Re: [PATCH v2 3/5] pci: set msi_domain_ops as __ro_after_init

FromKeith Busch <keith.busch@intel.com>
Date2017-02-13 19:10 +0100
SubjectRe: [PATCH v2 3/5] pci: set msi_domain_ops as __ro_after_init
Message-ID<tatey-7cr-19@gated-at.bofh.it>
In reply to#1578913
On Fri, Feb 10, 2017 at 05:37:56PM -0800, Jess Frazelle wrote:
> Marked msi_domain_ops structs as __ro_after_init when called only during init.
> This protects the data structure from accidental corruption.
> 
> Suggested-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Jess Frazelle <me@jessfraz.com>
>
>  drivers/pci/host/pci-hyperv.c | 2 +-
>  drivers/pci/host/vmd.c        | 2 +-

Ok for vmd driver.

Acked-by: Keith Busch <keith.busch@intel.com>

>  drivers/pci/msi.c             | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

[toc] | [prev] | [next] | [standalone]


#1578962 — Re: [PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as __ro_after_init

FromThomas Gleixner <tglx@linutronix.de>
Date2017-02-11 10:20 +0100
SubjectRe: [PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as __ro_after_init
Message-ID<t9C0x-7jp-5@gated-at.bofh.it>
In reply to#1578909
On Fri, 10 Feb 2017, Jess Frazelle wrote:

> Marked msi_domain_ops structs as __ro_after_init when called only during init.
> Marked syscore_ops structs as __ro_after_init when register_syscore_ops was
> called only during init. Most of the caller functions were already annotated as
> __init.
> unregister_syscore_ops() was never called on these syscore_ops.
> This protects the data structure from accidental corruption.

Please be more careful with your changelogs. They should not start with
telling WHAT you have done. The WHAT we can see from the patch.

The interesting information which belongs into the changelog is: WHY and
which problem does it solve or which enhancement this is. Let me give you
an example:

  Function pointers are a target for attacks especially when they are
  located in statically allocated data structures. Some of these data
  structures are only modified during init and therefor can be made read
  only after init.

  struct msi_domain_ops can be made read only after init because they are
  only updated in the registration case.

  struct syscore_ops can be made read only after init when they are only
  registered, but never unregistered.

So this would be a proper change log explaning the patch.

Emphasis on WOULD, See below.

> -static struct syscore_ops irq_gc_syscore_ops = {
> +static struct syscore_ops irq_gc_syscore_ops __ro_after_init = {
>  	.suspend = irq_gc_suspend,
>  	.resume = irq_gc_resume,
>  	.shutdown = irq_gc_shutdown,

I seriously doubt that syscore_ops can be made __ro_after_init at all.

Assume the following:

last_init_function()
  register_syscore_ops(&a_ops)
    list_add(&a_ops->node, list);

apply_ro_after_init()
  // a_ops are now read only

cpuhotplug happens
  register_syscore_ops(&b_ops)
    list_add(&b_ops->node, list);

    ===> Kernel crashes with a write access on RO memory because it tries
    	 to link b_ops to a_ops.

The same is true for cpuhotunplug operations.

> -static struct msi_domain_ops msi_domain_ops_default = {
> +static struct msi_domain_ops msi_domain_ops_default __ro_after_init = {

This is pointless and just tells me that you did a mechanical search for
these ops and then blindly added __ro_after_init instead of analysing how
msi_domain_ops_default is used.

msi_domain_ops_default are never ever modified, so they should be made
'const' and not __ro_after_init. It's not that hard to figure that out from
the code.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]


#1578967 — Re: [PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as __ro_after_init

FromThomas Gleixner <tglx@linutronix.de>
Date2017-02-11 10:30 +0100
SubjectRe: [PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as __ro_after_init
Message-ID<t9Cad-7mN-15@gated-at.bofh.it>
In reply to#1578962

On Sat, 11 Feb 2017, Thomas Gleixner wrote:

> On Fri, 10 Feb 2017, Jess Frazelle wrote:
> 
> > Marked msi_domain_ops structs as __ro_after_init when called only during init.
> > Marked syscore_ops structs as __ro_after_init when register_syscore_ops was
> > called only during init. Most of the caller functions were already annotated as
> > __init.
> > unregister_syscore_ops() was never called on these syscore_ops.
> > This protects the data structure from accidental corruption.
> 
> Please be more careful with your changelogs. They should not start with
> telling WHAT you have done. The WHAT we can see from the patch.
> 
> The interesting information which belongs into the changelog is: WHY and
> which problem does it solve or which enhancement this is. Let me give you
> an example:
> 
>   Function pointers are a target for attacks especially when they are
>   located in statically allocated data structures. Some of these data
>   structures are only modified during init and therefor can be made read
>   only after init.
> 
>   struct msi_domain_ops can be made read only after init because they are
>   only updated in the registration case.
> 
>   struct syscore_ops can be made read only after init when they are only
>   registered, but never unregistered.
> 
> So this would be a proper change log explaning the patch.
> 
> Emphasis on WOULD, See below.
> 
> > -static struct syscore_ops irq_gc_syscore_ops = {
> > +static struct syscore_ops irq_gc_syscore_ops __ro_after_init = {
> >  	.suspend = irq_gc_suspend,
> >  	.resume = irq_gc_resume,
> >  	.shutdown = irq_gc_shutdown,
> 
> I seriously doubt that syscore_ops can be made __ro_after_init at all.
> 
> Assume the following:
> 
> last_init_function()
>   register_syscore_ops(&a_ops)
>     list_add(&a_ops->node, list);
> 
> apply_ro_after_init()
>   // a_ops are now read only
> 
> cpuhotplug happens
>   register_syscore_ops(&b_ops)
>     list_add(&b_ops->node, list);
> 
>     ===> Kernel crashes with a write access on RO memory because it tries
>     	 to link b_ops to a_ops.
> 
> The same is true for cpuhotunplug operations.

Sorry, cpuhotplug was the wrong example, but loading or unloading the KVM
modules will have that effect.

See vmx_init()/exit() and kvm_init()/exit() for reference.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]


#1579016

FromJess Frazelle <me@jessfraz.com>
Date2017-02-11 11:50 +0100
Message-ID<t9DpD-83k-9@gated-at.bofh.it>
In reply to#1578962

On February 11, 2017 1:14:52 AM PST, Thomas Gleixner <tglx@linutronix.de> wrote:
>On Fri, 10 Feb 2017, Jess Frazelle wrote:
>
>> Marked msi_domain_ops structs as __ro_after_init when called only
>during init.
>> Marked syscore_ops structs as __ro_after_init when
>register_syscore_ops was
>> called only during init. Most of the caller functions were already
>annotated as
>> __init.
>> unregister_syscore_ops() was never called on these syscore_ops.
>> This protects the data structure from accidental corruption.
>
>Please be more careful with your changelogs. They should not start with
>telling WHAT you have done. The WHAT we can see from the patch.
>
>The interesting information which belongs into the changelog is: WHY
>and
>which problem does it solve or which enhancement this is. Let me give
>you
>an example:
>
>  Function pointers are a target for attacks especially when they are
>  located in statically allocated data structures. Some of these data
> structures are only modified during init and therefor can be made read
>  only after init.
>
>struct msi_domain_ops can be made read only after init because they are
>  only updated in the registration case.
>
> struct syscore_ops can be made read only after init when they are only
>  registered, but never unregistered.
>
>So this would be a proper change log explaning the patch.

Thanks for the clarification.

>
>Emphasis on WOULD, See below.
>
>> -static struct syscore_ops irq_gc_syscore_ops = {
>> +static struct syscore_ops irq_gc_syscore_ops __ro_after_init = {
>>  	.suspend = irq_gc_suspend,
>>  	.resume = irq_gc_resume,
>>  	.shutdown = irq_gc_shutdown,
>
>I seriously doubt that syscore_ops can be made __ro_after_init at all.
>
>Assume the following:
>
>last_init_function()
>  register_syscore_ops(&a_ops)
>    list_add(&a_ops->node, list);
>
>apply_ro_after_init()
>  // a_ops are now read only
>
>cpuhotplug happens
>  register_syscore_ops(&b_ops)
>    list_add(&b_ops->node, list);
>
>  ===> Kernel crashes with a write access on RO memory because it tries
>    	 to link b_ops to a_ops.
>
>The same is true for cpuhotunplug operations.

This makes sense. Will remove.

>
>> -static struct msi_domain_ops msi_domain_ops_default = {
>> +static struct msi_domain_ops msi_domain_ops_default __ro_after_init
>= {
>
>This is pointless and just tells me that you did a mechanical search
>for
>these ops and then blindly added __ro_after_init instead of analysing
>how
>msi_domain_ops_default is used.
>
>msi_domain_ops_default are never ever modified, so they should be made
>'const' and not __ro_after_init. It's not that hard to figure that out
>from
>the code.

Will change to a const.

>
>Thanks,
>
>	tglx

[toc] | [prev] | [next] | [standalone]


#1579022 — Re: [PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as __ro_after_init

FromThomas Gleixner <tglx@linutronix.de>
Date2017-02-11 13:10 +0100
SubjectRe: [PATCH v2 1/5] irq: set {msi_domain,syscore}_ops as __ro_after_init
Message-ID<t9EF3-wX-9@gated-at.bofh.it>
In reply to#1579016
On Sat, 11 Feb 2017, Jess Frazelle wrote:
> On February 11, 2017 1:14:52 AM PST, Thomas Gleixner <tglx@linutronix.de> wrote:
> >The same is true for cpuhotunplug operations.
> 
> This makes sense. Will remove.

That's true for all other patches touching sysops as well. But instead of
giving up I'd recommend to look into the following:

Go through all callsites which use un/register_syscore_ops() and figure out
how many of them are possibly called post init. From a quick grep I can
only find the KVM module, but there might be more.

Lets assume it's KVM only. So you could do the following:

Put something like this into virt/kvm/kvm_main.c, which is a builtin file

static struct syscore_ops ops __ro_after_init = {
       ....
};

int __init foo()
{
    register_ops(&ops);
}

and because we know that kvm is single instance you can just have:

static struct syscore_ops *kvm_ops;

void kvm_set_sysop(*vmx_ops)
{
	kvm_ops = ops;
}

and then have the kvm_syscore callbacks:

static callback()
{
	if (kvm_ops)
	   kvm_ops->callback()
}

Sanity checks and serialization omitted. Then switch kvm_exit/init over to
it.

After that you can make all syscore_ops __ro_after_init, remove the export
from (un)register_syscore_ops() and make that __init.

Not much of an effort and probably worth the trouble.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]


#1579023

FromJessica Frazelle <me@jessfraz.com>
Date2017-02-11 13:20 +0100
Message-ID<t9EOJ-A6-1@gated-at.bofh.it>
In reply to#1579022
Thanks, I'll check them out.

On Sat, Feb 11, 2017 at 4:00 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Sat, 11 Feb 2017, Jess Frazelle wrote:
> > On February 11, 2017 1:14:52 AM PST, Thomas Gleixner <tglx@linutronix.de> wrote:
> > >The same is true for cpuhotunplug operations.
> >
> > This makes sense. Will remove.
>
> That's true for all other patches touching sysops as well. But instead of
> giving up I'd recommend to look into the following:
>
> Go through all callsites which use un/register_syscore_ops() and figure out
> how many of them are possibly called post init. From a quick grep I can
> only find the KVM module, but there might be more.
>
> Lets assume it's KVM only. So you could do the following:
>
> Put something like this into virt/kvm/kvm_main.c, which is a builtin file
>
> static struct syscore_ops ops __ro_after_init = {
>        ....
> };
>
> int __init foo()
> {
>     register_ops(&ops);
> }
>
> and because we know that kvm is single instance you can just have:
>
> static struct syscore_ops *kvm_ops;
>
> void kvm_set_sysop(*vmx_ops)
> {
>         kvm_ops = ops;
> }
>
> and then have the kvm_syscore callbacks:
>
> static callback()
> {
>         if (kvm_ops)
>            kvm_ops->callback()
> }
>
> Sanity checks and serialization omitted. Then switch kvm_exit/init over to
> it.
>
> After that you can make all syscore_ops __ro_after_init, remove the export
> from (un)register_syscore_ops() and make that __init.
>
> Not much of an effort and probably worth the trouble.
>
> Thanks,
>
>         tglx
>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web