Re: [PATCH -next] irq-chip/gic-v3-its: Fixed an issue where the ITS executes the residual commands in the queue again when the ITS wakes up from sleep mode.

[xuqiang (M)]

The kernel sends three commands in the following sequence:

1.mapd(deviceA, ITT_addr1, valid:1)

2.mapti(deviceA):ITS write ITT_addr1 memory;

3.mapd(deviceA, ITT_addr1, valid:0) and kfree(ITT_addr1);

4.mapd(deviceA, ITT_addr2, valid:1);

5.mapti(deviceA):ITS write ITT_addr2 memory;

In this case, the processor enters the sleep mode. After the kernel 
performs the suspend operation, the firmware performs the store 
operation and saves GITS_CBASER and GITS_CWRITER registers.

Then, the processor is woken up, and the firmware restores GITS_CBASER 
and GITS_CWRITER registers. Because GITS_CWRITER register is not 0, ITS 
will read the above command sequence execution from the command queue, 
causing ITT_addr1 memory to be trampled.

Thanks,

                Xu


在 2020/11/4 2:19, Marc Zyngier 写道:
> On Tue, 03 Nov 2020 08:11:23 +0000,
> Xu Qiang <xuqiang36@xxxxxxxxxx> wrote:
> > During wakeup, the ATF restore interface restores the values of
> > the cbaser and cwriter registers. As a result, the ITS executes
> > the residual commands in the queue, which may cause memory corruption.
> >
> > To solve this problem, clear all data in the command queue
> > in the suspend interface of the ITS driver.
> >
> > Signed-off-by: Xu Qiang <xuqiang36@xxxxxxxxxx>
> > ---
> >   drivers/irqchip/irq-gic-v3-its.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> > index 0fec31931e11..b8487f78ac21 100644
> > --- a/drivers/irqchip/irq-gic-v3-its.c
> > +++ b/drivers/irqchip/irq-gic-v3-its.c
> > @@ -4741,6 +4741,14 @@ static int its_save_disable(void)
> >   	list_for_each_entry(its, &its_nodes, entry) {
> >   		void __iomem *base;
> >   
> > +		/*
> > +		 * Clear the command queue so that the ITS will not re-execute
> > +		 * the remaining commands in the command queue when
> > +		 * the cwriter and cbaser registers are restored
> > +		 * in the restore interface of the firmware.
> > +		 */
> > +		memset(its->cmd_base, 0, ITS_CMD_QUEUE_SZ);
> > +
> >   		if (!(its->flags & ITS_FLAGS_SAVE_SUSPEND_STATE))
> >   			continue;
> You are wiping the ITS queue before even stopping the ITS. How well is
> that going to work? What if there is something in flight?
>
> I don't understand what you are trying to do here, nor how ATF is
> involved. So please describe the whole sequence of events, and we'll
> decide whether that's something we need to fix.
>
> Thanks,
>
> 	M.