| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <86lfrszfe2.wl-maz@kernel.org>
Date: Wed, 04 Dec 2019 08:03:33 +0000
From: Marc Zyngier <maz@...nel.org>
To: Ivid Suvarna <ivid.suvarna@...il.com>
Cc: Yao HongBo <yaohongbo@...wei.com>,
"Guohanjun (Hanjun Guo)" <guohanjun@...wei.com>,
Yangyingliang <yangyingliang@...wei.com>,
Linuxarm <linuxarm@...wei.com>,
Kernel development list <linux-kernel@...r.kernel.org>,
james.morse@....com
Subject: Re: ITS restore/save state when HCC == 0
On Wed, 04 Dec 2019 06:13:23 +0000,
Ivid Suvarna <ivid.suvarna@...il.com> wrote:
>
> On Tue, Dec 3, 2019 at 9:46 PM Marc Zyngier <maz@...nel.org> wrote:
> >
> > + James, who wrote most (if not all) of the arm64 hibernate code
> >
> > On 2019-12-03 02:23, Yao HongBo wrote:
> > > On 12/2/2019 9:22 PM, Marc Zyngier wrote:
> > >> Hi Yaohongbo,
> > >>
> > >> In the future, please refrain from sending HTML emails, they
> > >> don't render very well and force me to reformat your email
> > >> by hand.
> > >
> > > Sorry. I'll pay attention to this next time.
> > >
> > >> On 2019-12-02 12:52, yaohongbo wrote:
> > >>> Hi, marc.
> > >>>
> > >>> I met a problem with GIC ITS when I try to power off gic logic in
> > >>> suspend.
> > >>>
> > >>> In hisilicon hip08, the value of GIC_TYPER.HCC is zero, so that
> > >>> ITS_FLAGS_SAVE_SUSPEND_STATE will have no chance to be set to 1.
> > >>
> > >> And that's a good thing. HCC indicates that you have collections
> > >> that
> > >> are backed by registers, and not memory. Which means that once the
> > >> GIC
> > >> is powered off, the state is lost.
> > >>
> > >>> It goes well for s4, when I simply remove the condition judgement
> > >>> in
> > >>> the code.
> > >>
> > >> What is "s4"? Doing so means you are reprogramming the ITS with
> > >> mappings
> > >> that already exist in the tables, and that is UNPRED territory.
> > >
> > > Sorry, I didn't describe it clearly.
> > > S4 means "suspend to disk".
> > > In s4, The its will reinit and malloc an new its address.
> >
> > Huh, hibernate... Yeah, this is not expected to work, I'm afraid.
> >
> > > My expectation is to reprogram the ITS with original mappings. If
> > > ITS_FLAGS_SAVE_SUSPEND_STATE
> > > is not set, i'll have no chance to use the original its table
> > > mappings.
> > >
> > > What should i do if i want to restore its state with hcc == 0?
> >
> > HCC is the least of the problems, and there are plenty more issues:
> >
> > - I'm not sure what guarantees that the tables are at the same
> > address in the booting kernel and the the resumed kernel.
> > That covers all the ITS tables and as well as the RDs'.
> >
> > - It could well be that restoring the ITS base addresses is enough
> > for everything to resume correctly, but this needs some serious
> > investigation. Worse case, we will need to replay the whole of
> > the ITS programming.
> >
> > - This is going to interact more or less badly with the normal suspend
> > to RAM code...
> >
> > - The ITS is only the tip of the iceberg. The whole of the SMMU setup
> > needs to be replayed, or devices won't resume correctly (I just tried
> > on a D05).
> >
> > Anyway, with the hack below, I've been able to get D05 to resume
> > up to the point where devices try to do DMA, and then it was dead.
> > There is definitely some work to be done there...
> >
> > M.
> >
> > diff --git a/drivers/irqchip/irq-gic-v3-its.c
> > b/drivers/irqchip/irq-gic-v3-its.c
> > index 4ba31de4a875..a05fc6bac203 100644
> > --- a/drivers/irqchip/irq-gic-v3-its.c
> > +++ b/drivers/irqchip/irq-gic-v3-its.c
> > @@ -27,6 +27,7 @@
> > #include <linux/of_platform.h>
> > #include <linux/percpu.h>
> > #include <linux/slab.h>
> > +#include <linux/suspend.h>
> > #include <linux/syscore_ops.h>
> >
> > #include <linux/irqchip.h>
> > @@ -42,6 +43,7 @@
> > #define ITS_FLAGS_WORKAROUND_CAVIUM_22375 (1ULL << 1)
> > #define ITS_FLAGS_WORKAROUND_CAVIUM_23144 (1ULL << 2)
> > #define ITS_FLAGS_SAVE_SUSPEND_STATE (1ULL << 3)
> > +#define ITS_FLAGS_SAVE_HIBERNATE_STATE (1ULL << 4)
> >
> > #define RDIST_FLAGS_PROPBASE_NEEDS_FLUSHING (1 << 0)
> > #define RDIST_FLAGS_RD_TABLES_PREALLOCATED (1 << 1)
> > @@ -3517,8 +3519,16 @@ static int its_save_disable(void)
> > raw_spin_lock(&its_lock);
> > list_for_each_entry(its, &its_nodes, entry) {
> > void __iomem *base;
> > + u64 flags;
> >
> > - if (!(its->flags & ITS_FLAGS_SAVE_SUSPEND_STATE))
> > + if (system_entering_hibernation())
> > + its->flags |= ITS_FLAGS_SAVE_HIBERNATE_STATE;
> > +
> > + flags = its->flags;
> > + flags &= (ITS_FLAGS_SAVE_SUSPEND_STATE |
> > + ITS_FLAGS_SAVE_HIBERNATE_STATE);
> > +
> > + if (!flags)
> > continue;
> >
> > base = its->base;
> > @@ -3559,11 +3569,17 @@ static void its_restore_enable(void)
> > raw_spin_lock(&its_lock);
> > list_for_each_entry(its, &its_nodes, entry) {
> > void __iomem *base;
> > + u64 flags;
> > int i;
> >
> > - if (!(its->flags & ITS_FLAGS_SAVE_SUSPEND_STATE))
> > + flags = its->flags;
> > + flags &= (ITS_FLAGS_SAVE_SUSPEND_STATE |
> > + ITS_FLAGS_SAVE_HIBERNATE_STATE);
> > + if (!flags)
> > continue;
> >
> > + its->flags &= ~ITS_FLAGS_SAVE_HIBERNATE_STATE;
> > +
> > base = its->base;
> >
> > /*
>
> How about this one to reinit GIC for restore:
> - https://source.codeaurora.org/quic/la/kernel/msm-4.14/commit/drivers/irqchip/irq-gic-v3.c?h=msm-4.14&id=b0079fb73c08e195498ba2b2ea9623b0cc0f5fed
That's not what we're concerned with at the moment, as there is much
more state that is currently missing. Save/restoring registers is the
easy part. What needs to be fixed is the way RD memory tables
potentially get moved around (and how they can then survive a kexec).
Once we've solved these issues, we'll look at the register state which
is likely to already be correct anyway.
M.
--
Jazz is not dead, it just smells funny.
Powered by blists - more mailing lists