Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Mon, 24 Feb 2020 23:47:42 +0000
From:   Marc Zyngier <maz@kernel.org>
To:     Zenghui Yu <yuzenghui@huawei.com>
Cc:     kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org, wanghaibin.wang@huawei.com,
        Yanlei Jia <jiayanlei@huawei.com>
Subject: Re: [PATCH] irqchip/gic-v3-its: Clear Valid before writing any bits
 else in VPENDBASER
In-Reply-To: <20200224025029.92-1-yuzenghui@huawei.com>
References: <20200224025029.92-1-yuzenghui@huawei.com>
Message-ID: <bb7cdb29eda9cf160bcf85a58a9fc63d@kernel.org>
User-Agent: Roundcube Webmail/1.3.10
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Zenghui,

On 2020-02-24 02:50, Zenghui Yu wrote:
> The Valid bit must be cleared before changing anything else when 
> writing
> GICR_VPENDBASER to avoid the UNPREDICTABLE behavior. This is exactly 
> what
> we've done on 32bit arm, but not on arm64.

I'm not quite sure how you decide that Valid must be cleared before 
changing
anything else. The reason why we do it on 32bit is that we cannot update
the full 64bit register at once, so we start by clearing Valid so that
we can update the rest. arm64 doesn't require that.

For the rest of discussion, let's ignore GICv4.1 32bit support (I'm
pretty sure nobody cares about that).

> This works fine on GICv4 where we only clear Valid for a vPE 
> deschedule.
> With the introduction of GICv4.1, we might also need to talk something 
> else
> (e.g., PendingLast, Doorbell) to the redistributor when clearing the 
> Valid.
> Let's port the 32bit gicr_write_vpendbaser() to arm64 so that hardware 
> can
> do the right thing after descheduling the vPE.

The spec says that:

"For a write that writes GICR_VPENDBASER.Valid from 1 to 0, if
GICR_VPENDBASER.PendingLast is written as 1 then 
GICR_VPENDBASER.PendingLast
takes an UNKNOWN value and GICR_VPENDBASER.Doorbell is treated as being 
0."

and

"When GICR_VPENDBASER.Valid is written from 1 to 0, if there are 
outstanding
enabled pending interrupts GICR_VPENDBASER.Doorbell is treated as 0."

which indicate that PendingLast/Doorbell have to be written at the same 
time
as we clear Valid. Can you point me to the bit of the v4.1 spec that 
makes
this "clear Valid before doing anything else" requirement explicit?

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...