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 > <paulmck@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx [mailto:linux-acpi-owner@xxxxxxxxxxxxxxx] 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@xxxxxxxxxxxxxxx [mailto:linux-acpi-owner@xxxxxxxxxxxxxxx] 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 <paulmck@xxxxxxxxxxxxxxxxxx> > > 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 <lv.zheng@xxxxxxxxx> > > Reported-by: Borislav Petkov <bp@xxxxxxxxx> > > Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx> > > 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@xxxxxxxxx Like this? Thanx, Paul ------------------------------------------------------------------------ commit 9ae87e58f7c40b39ff80737e3375672f16316d23 Author: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx> 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 <lv.zheng@xxxxxxxxx> Reported-by: Borislav Petkov <bp@xxxxxxxxx> 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@xxxxxxxxx Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx> 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); -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
