Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752634AbdGCG4S (ORCPT <rfc822;w@1wt.eu>);
        Mon, 3 Jul 2017 02:56:18 -0400
Received: from Galois.linutronix.de ([146.0.238.70]:41218 "EHLO
        Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752179AbdGCG4Q (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 3 Jul 2017 02:56:16 -0400
Date: Mon, 3 Jul 2017 08:56:13 +0200 (CEST)
From: Thomas Gleixner <tglx@linutronix.de>
To: Dou Liyang <douly.fnst@cn.fujitsu.com>
cc: x86@kernel.org, linux-kernel@vger.kernel.org,
        xen-devel@lists.xenproject.org, mingo@kernel.org, hpa@zytor.com,
        ebiederm@xmission.com, bhe@redhat.com, boris.ostrovsky@oracle.com,
        peterz@infradead.org, izumi.taku@jp.fujitsu.com
Subject: Re: [PATCH v5 10/12] x86/xen: Bypass intr mode setup in enlighten_pv
 system
In-Reply-To: <d3548720-714c-4199-b8d2-f6db29804ed9@cn.fujitsu.com>
Message-ID: <alpine.DEB.2.20.1707030842500.2188@nanos>
References: <cover.1498795030.git.douly.fnst@cn.fujitsu.com> <2545ef73fde4e3cf65080b056669dadd3578ff8a.1498795030.git.douly.fnst@cn.fujitsu.com> <alpine.DEB.2.20.1707022117180.2296@nanos> <d3548720-714c-4199-b8d2-f6db29804ed9@cn.fujitsu.com>
User-Agent: Alpine 2.20 (DEB 67 2015-01-07)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1769
Lines: 47

On Mon, 3 Jul 2017, Dou Liyang wrote:
> At 07/03/2017 03:18 AM, Thomas Gleixner wrote:
> > On Fri, 30 Jun 2017, Dou Liyang wrote:
> > 
> > > xen_smp_ops overwrites smp_prepare_cpus to xen_pv_smp_prepare_cpus
> > > which initializes interrupt itself.
> > > 
> > > Touching the intr_mode_init causes unexpected results on the system.
> > > 
> > > Bypass it in enlighten_pv system.
> > 
> > So that's the wrong patch order then. You broke XEN at some point with your
> > changes. You need to prevent that breakage upfront not after the fact.
> 
> Yes, I have considered to prevent that breakage in the patchset.
> 
> Actually, Until the 11th patch, we put the intr_mode_init ahead of
> time, which will break XEN.
> 
> Before the 11th patch, we just unify the code and do the preparation,
> Kernel will do the intr_mode_init like before, which will have no
> influence on XEN.
> 
> So we put the patch here before 11th patch.

Ok. That's good, but please explain it in the changelog. I had the
impression that this is fixing breakage you introduced earlier, but now
with your explanation it makes sense. So the patch order is correct.

Something like this wants to be in the changelog:

  XEN PV overrides smp_prepare_cpus(). xen_pv_smp_prepare_cpus()
  initializes interrupts in the XEN PV specific way and does not invoke
  native_smp_prepare_cpus(). As a consequence, x86_init.intr_mode_init() is
  not invoked either.

  The invocation of x86_init.intr_mode_init() will be moved from
  native_smp_prepare_cpus() in a follow up patch to solve <INSERT
  REASON/PROBLEM>.

  That move would cause the invocation of x86_init.intr_mode_init() for XEN
  PV platforms. To prevent that, override the default x86_init.intr_mode_init()
  callback with a noop().

Thanks,

	tglx