linux-kernel - Re: [RFC] KVM: arm/arm64: Introduce KVM_DEV_ARM_ITS_CTRL

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMJs5B8PFCggN1-K-JZy_hOSQQeNYNPNaj_99YKG0y4hqC1CdA@mail.gmail.com>
Date:   Fri, 15 Sep 2017 10:56:06 -0700
From:   Christoffer Dall <cdall@...aro.org>
To:     Auger Eric <eric.auger@...hat.com>
Cc:     Marc Zyngier <marc.zyngier@....com>, eric.auger.pro@...il.com,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        Peter Maydell <peter.maydell@...aro.org>,
        Andre Przywara <andre.przywara@....com>,
        Haibin Wang <wanghaibin.wang@...wei.com>, wu.wubin@...wei.com
Subject: Re: [RFC] KVM: arm/arm64: Introduce KVM_DEV_ARM_ITS_CTRL_RESET

On Fri, Sep 15, 2017 at 5:26 AM, Auger Eric <eric.auger@...hat.com> wrote:
> Hi,
>
> On 14/09/2017 19:06, Marc Zyngier wrote:
>> On Thu, Sep 14 2017 at 10:57:28 am BST, Eric Auger <eric.auger@...hat.com> wrote:
>>> At the moment, the in-kernel emulated ITS is not properly reset.
>>> On guest restart/reset some registers keep their old values and
>>> internal structures like device, ITE, collection lists are not emptied.
>>>
>>> This may lead to various bugs. Among them, we can have incorrect state
>>> backup or failure when saving the ITS state at early guest boot stage.
>>>
>>> This patch introduces a new attribute, KVM_DEV_ARM_ITS_CTRL_RESET in
>>> the KVM_DEV_ARM_VGIC_GRP_CTRL group.
>>>
>>> Upon this action, we can invalidate the various memory structures
>>> pointed by GITS_BASERn and GITS_CBASER, free the ITS internal caches
>>> and reset the relevant registers.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@...hat.com>
>>>
>>> ---
>>>
>>> An alternative would consist in having the userspace writing
>>> individual registers with default values: GITS_BASERn, GITS_CBASER
>>> and GITS_CTLR. On kernel side we would reset related lists when
>>> detecting the valid bit is set to false.
>>
>> I'm not sure this is necessarily a "either/or" situation. It looks to me
>> that we're not completely doing the right thing when writing to the
>> GITS_BASER registers, and that writing a new value (with the valid bit
>> set or not) should have an action of some sort on the fate of the
>> existing mappings.
>
> I agree. I think whenever the GITS_BASERn or GITS_CBASER validity bit is
> reset, we should empty the internal lists and assure the code does not
> attempt to read the data structures in caches/RAM anymore.
>

I don't think that is likely to match the behavior suggested in the
GIC/ITS spec.  I doubt that hardware implementations will support
software changing the BASERs without turning off the GIC, and
therefore I don't think we'll see drivers doing this any time soon,
and I don't think we need to support that.

What I do think we should support is the ITS power management sequence
pointed out in Section 6.6 in the spec.  But I don't think this is
urgent, as we don't seem to have any guests that power down and power
up the ITS yet.


Thanks,
-Christoffer