2018-07-02 10:01:34

by Jürgen Groß

[permalink] [raw]
Subject: [PATCH] xen: setup pv irq ops vector earlier

Setting pv_irq_ops for Xen PV domains should be done as early as
possible in order to support e.g. very early printk() usage.

Remove the no longer necessary conditional in xen_init_irq_ops()
from PVH V1 times to make clear this is a PV only function.

Cc: <[email protected]> # 4.14
Signed-off-by: Juergen Gross <[email protected]>
---
arch/x86/xen/enlighten_pv.c | 3 +--
arch/x86/xen/irq.c | 4 +---
2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 8d4e2e1ae60b..0f4cd9e5bed4 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -1213,6 +1213,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
pv_info = xen_info;
pv_init_ops.patch = paravirt_patch_default;
pv_cpu_ops = xen_cpu_ops;
+ xen_init_irq_ops();

x86_platform.get_nmi_reason = xen_get_nmi_reason;

@@ -1249,8 +1250,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
get_cpu_cap(&boot_cpu_data);
x86_configure_nx();

- xen_init_irq_ops();
-
/* Let's presume PV guests always boot on vCPU with id 0. */
per_cpu(xen_vcpu_id, 0) = 0;

diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
index 74179852e46c..7515a19fd324 100644
--- a/arch/x86/xen/irq.c
+++ b/arch/x86/xen/irq.c
@@ -128,8 +128,6 @@ static const struct pv_irq_ops xen_irq_ops __initconst = {

void __init xen_init_irq_ops(void)
{
- /* For PVH we use default pv_irq_ops settings. */
- if (!xen_feature(XENFEAT_hvm_callback_vector))
- pv_irq_ops = xen_irq_ops;
+ pv_irq_ops = xen_irq_ops;
x86_init.irqs.intr_init = xen_init_IRQ;
}
--
2.13.7



2018-07-02 11:48:18

by Jan Beulich

[permalink] [raw]
Subject: Re: [Xen-devel] [PATCH] xen: setup pv irq ops vector earlier

>>> On 02.07.18 at 12:00, <[email protected]> wrote:
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -1213,6 +1213,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
> pv_info = xen_info;
> pv_init_ops.patch = paravirt_patch_default;
> pv_cpu_ops = xen_cpu_ops;
> + xen_init_irq_ops();

Isn't this still too late? xen_setup_machphys_mapping(), for example,
has a WARN_ON(), which implies multiple printk()s.

Jan



2018-07-10 22:27:28

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH] xen: setup pv irq ops vector earlier

On 07/02/2018 06:00 AM, Juergen Gross wrote:
> Setting pv_irq_ops for Xen PV domains should be done as early as
> possible in order to support e.g. very early printk() usage.

Will printk() work as result of this move? We still, for example,
haven't set up console.

This will (probably) allow us not to crash (due to STI and such) but I
am not sure "support" is the right term here.

-boris


>
> Remove the no longer necessary conditional in xen_init_irq_ops()
> from PVH V1 times to make clear this is a PV only function.
>
> Cc: <[email protected]> # 4.14
> Signed-off-by: Juergen Gross <[email protected]>
> ---
> arch/x86/xen/enlighten_pv.c | 3 +--
> arch/x86/xen/irq.c | 4 +---
> 2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index 8d4e2e1ae60b..0f4cd9e5bed4 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -1213,6 +1213,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
> pv_info = xen_info;
> pv_init_ops.patch = paravirt_patch_default;
> pv_cpu_ops = xen_cpu_ops;
> + xen_init_irq_ops();
>
> x86_platform.get_nmi_reason = xen_get_nmi_reason;
>
> @@ -1249,8 +1250,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
> get_cpu_cap(&boot_cpu_data);
> x86_configure_nx();
>
> - xen_init_irq_ops();
> -
> /* Let's presume PV guests always boot on vCPU with id 0. */
> per_cpu(xen_vcpu_id, 0) = 0;
>
> diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
> index 74179852e46c..7515a19fd324 100644
> --- a/arch/x86/xen/irq.c
> +++ b/arch/x86/xen/irq.c
> @@ -128,8 +128,6 @@ static const struct pv_irq_ops xen_irq_ops __initconst = {
>
> void __init xen_init_irq_ops(void)
> {
> - /* For PVH we use default pv_irq_ops settings. */
> - if (!xen_feature(XENFEAT_hvm_callback_vector))
> - pv_irq_ops = xen_irq_ops;
> + pv_irq_ops = xen_irq_ops;
> x86_init.irqs.intr_init = xen_init_IRQ;
> }


2018-07-11 05:10:55

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH] xen: setup pv irq ops vector earlier

On 11/07/18 00:26, Boris Ostrovsky wrote:
> On 07/02/2018 06:00 AM, Juergen Gross wrote:
>> Setting pv_irq_ops for Xen PV domains should be done as early as
>> possible in order to support e.g. very early printk() usage.
>
> Will printk() work as result of this move? We still, for example,
> haven't set up console.

It will print to the kernel print buffer, so the output will be
available later.

> This will (probably) allow us not to crash (due to STI and such) but I
> am not sure "support" is the right term here.

Not crashing is big plus IMO. :-)


Juergen

2018-07-11 17:52:14

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH] xen: setup pv irq ops vector earlier

On 07/11/2018 01:08 AM, Juergen Gross wrote:
> On 11/07/18 00:26, Boris Ostrovsky wrote:
>> On 07/02/2018 06:00 AM, Juergen Gross wrote:
>>> Setting pv_irq_ops for Xen PV domains should be done as early as
>>> possible in order to support e.g. very early printk() usage.
>> Will printk() work as result of this move? We still, for example,
>> haven't set up console.
> It will print to the kernel print buffer, so the output will be
> available later.

Right, haven't thought about it.


>
>> This will (probably) allow us not to crash (due to STI and such) but I
>> am not sure "support" is the right term here.
> Not crashing is big plus IMO. :-)

Actually, no, this is not sufficient.

You need to move xen_vcpu_info_reset(0) up as well. Otherwise you will
not be able to dereference the percpu xen_vcpu in xen_save_fl().


-boris