From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S942104AbdAIXO1 (ORCPT ); Mon, 9 Jan 2017 18:14:27 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:60423 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S938885AbdAIXOX (ORCPT ); Mon, 9 Jan 2017 18:14:23 -0500 Date: Mon, 9 Jan 2017 15:14:16 -0800 From: "Paul E. McKenney" To: "Rafael J. Wysocki" Cc: Borislav Petkov , "Zheng, Lv" , "Wysocki, Rafael J" , "Moore, Robert" , J?rg R?del , lkml , Linux ACPI Subject: Re: 174cc7187e6f ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel Reply-To: paulmck@linux.vnet.ibm.com References: <20170108010158.b62eovaxsbmhfnkb@pd.tnic> <20170108130355.vxhjmj6dlkqw6hyq@pd.tnic> <1AE640813FDE7649BE1B193DEA596E886CE27B7E@SHSMSX101.ccr.corp.intel.com> <1AE640813FDE7649BE1B193DEA596E886CE27BEE@SHSMSX101.ccr.corp.intel.com> <20170109093329.jd7uwlcpci4icpd3@pd.tnic> <20170109221831.GC3800@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17010923-0028-0000-0000-000006699022 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00006404; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000199; SDB=6.00805275; UDB=6.00391913; IPR=6.00582896; BA=6.00005038; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00013871; XFM=3.00000011; UTC=2017-01-09 23:14:20 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17010923-0029-0000-0000-0000323F193A Message-Id: <20170109231416.GF3800@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-01-09_16:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1701090311 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jan 09, 2017 at 11:25:39PM +0100, Rafael J. Wysocki wrote: > Hi Paul, > > On Mon, Jan 9, 2017 at 11:18 PM, Paul E. McKenney > wrote: > > On Mon, Jan 09, 2017 at 10:33:29AM +0100, Borislav Petkov wrote: > >> + Paul for comment. > >> > >> Leaving in the rest for him. > >> > >> On Mon, Jan 09, 2017 at 02:36:33AM +0000, Zheng, Lv wrote: > >> > Hi, > >> > > >> > > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of Zheng, > >> > > Lv > >> > > Subject: RE: 174cc7187e6f ACPICA: Tables: Back port acpi_get_table_with_size() and > >> > > early_acpi_os_unmap_memory() from Linux kernel > >> > > > >> > > Hi, > >> > > > >> > > > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-owner@vger.kernel.org] On Behalf Of > >> > > Borislav > >> > > > Petkov > >> > > > Subject: Re: 174cc7187e6f ACPICA: Tables: Back port acpi_get_table_with_size() and > >> > > > early_acpi_os_unmap_memory() from Linux kernel > >> > > > > >> > > > On Sun, Jan 08, 2017 at 03:20:20AM +0100, Rafael J. Wysocki wrote: > >> > > > > drivers/iommu/amd_iommu_init.c | 2 +- > >> > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > > > > >> > > > > Index: linux-pm/drivers/iommu/amd_iommu_init.c > >> > > > > =================================================================== > >> > > > > --- linux-pm.orig/drivers/iommu/amd_iommu_init.c > >> > > > > +++ linux-pm/drivers/iommu/amd_iommu_init.c > >> > > > > @@ -2230,7 +2230,7 @@ static int __init early_amd_iommu_init(v > >> > > > > */ > >> > > > > ret = check_ivrs_checksum(ivrs_base); > >> > > > > if (ret) > >> > > > > - return ret; > >> > > > > + goto out; > >> > > > > > >> > > > > amd_iommu_target_ivhd_type = get_highest_supported_ivhd_type(ivrs_base); > >> > > > > DUMP_printk("Using IVHD type %#x\n", amd_iommu_target_ivhd_type); > >> > > > > >> > > > Good catch, this one needs to be applied regardless. > >> > > > > >> > > > However, it doesn't fix my issue though. > >> > > > > >> > > > But I think I have it - I went and applied the well-proven debugging > >> > > > technique of sprinkling printks around. Here's what I'm seeing: > >> > > > > >> > > > early_amd_iommu_init() > >> > > > |-> acpi_put_table(ivrs_base); > >> > > > |-> acpi_tb_put_table(table_desc); > >> > > > |-> acpi_tb_invalidate_table(table_desc); > >> > > > |-> acpi_tb_release_table(...) > >> > > > |-> acpi_os_unmap_memory > >> > > > |-> acpi_os_unmap_iomem > >> > > > |-> acpi_os_map_cleanup > >> > > > |-> synchronize_rcu_expedited <-- the kernel/rcu/tree_exp.h version with CONFIG_PREEMPT_RCU=y > >> > > > > >> > > > Now that function goes and sends IPIs, i.e., schedule_work() > >> > > > but this is too early - we haven't even done workqueue_init(). > >> > > > Actually, from looking at the callstack, we do > >> > > > kernel_init_freeable->native_smp_prepare_cpus() and workqueue_init() > >> > > > comes next. > >> > > > > >> > > > And this makes sense because the splat rIP points to __queue_work() but > >> > > > we haven't done that yet. > >> > > > > >> > > > So that acpi_put_table() is happening too early. Looks like AMD IOMMU > >> > > > should not put the table but WTH do I know?! > >> > > > > >> > > > In any case, commenting out: > >> > > > > >> > > > acpi_put_table(ivrs_base); > >> > > > ivrs_base = NULL; > >> > > > > >> > > > and the end of early_amd_iommu_init() makes the box boot again. > >> > > > >> > > So please help to comment out these 2 lines (with descriptions and do not delete them). > >> > > Until acpi_os_unmap_memory() is able to handle such an early case. > >> > > >> > IMO, synchronize_rcu_expedited() should be improved: > >> > If rcu_init() isn't called or there is nothing to synchronize, schedule_work() shouldn't be invoked. > > > > Indeed it should! > > > > Does the (untested) patch below fix things for you? > > > > If so, does this need to go into 4.10? (My default workflow would get > > it into 4.11 or 4.12, so please speak up if you need it.) > > Yes it should go into 4.10 (if it fixes the problem) as the reported > regression was introduced in 4.10-rc1. OK, I will test with rcutorture, but I also need a Tested-by for the specific problem at hand. > > ------------------------------------------------------------------------ > > > > commit 1b7feb708241f1662cfd529118468c9f9c0b1449 > > Author: Paul E. McKenney > > Date: Mon Jan 9 14:10:50 2017 -0800 > > > > rcu: Make synchronize_rcu_expedited() safe for early boot > > > > The synchronize_rcu_expedited() function does not check for early-boot > > use, which can result in failures if it is invoked before the scheduler > > has started. Given that the rcupdate.rcu_expedited kernel parameter > > causes all calls to synchronize_rcu() to be directed instead to > > synchronize_rcu_expedited(), a usage restriction does not make sense. > > > > This commit therefore adds a rcu_scheduler_active check to > > synchronize_rcu_expedited(), so that it is a no-op before the scheduler > > starts. This behavior is correct because there is only a single CPU > > running during that time. > > > > Reported-by: Lv Zheng > > Reported-by: Borislav Petkov > > Signed-off-by: Paul E. McKenney > > When applying this, please also add: > > Fixes: 174cc7187e6f ("ACPICA: Tables: Back port > acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux > kernel") > Link: https://lkml.kernel.org/r/4034dde8-ffc1-18e2-f40c-00cf37471793@intel.com Like this? Thanx, Paul ------------------------------------------------------------------------ commit 9ae87e58f7c40b39ff80737e3375672f16316d23 Author: Paul E. McKenney Date: Mon Jan 9 14:10:50 2017 -0800 rcu: Make synchronize_rcu_expedited() safe for early boot The synchronize_rcu_expedited() function does not check for early-boot use, which can result in failures if it is invoked before the scheduler has started. Given that the rcupdate.rcu_expedited kernel parameter causes all calls to synchronize_rcu() to be directed instead to synchronize_rcu_expedited(), a usage restriction does not make sense. This commit therefore adds a rcu_scheduler_active check to synchronize_rcu_expedited(), so that it is a no-op before the scheduler starts. This behavior is correct because there is only a single CPU running during that time. Reported-by: Lv Zheng Reported-by: Borislav Petkov Fixes: 174cc7187e6f ("ACPICA: Tables: Back port acpi_get_table_with_size() and early_acpi_os_unmap_memory() from Linux kernel") Link: Link: https://lkml.kernel.org/r/4034dde8-ffc1-18e2-f40c-00cf37471793@intel.com Signed-off-by: Paul E. McKenney diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h index dfc3ba5a429e..a6c3d86480de 100644 --- a/kernel/rcu/tree_exp.h +++ b/kernel/rcu/tree_exp.h @@ -690,6 +690,8 @@ void synchronize_rcu_expedited(void) { struct rcu_state *rsp = rcu_state_p; + if (!rcu_scheduler_active) + return; _synchronize_rcu_expedited(rsp, sync_rcu_exp_handler); } EXPORT_SYMBOL_GPL(synchronize_rcu_expedited);