lkml.org 
[lkml]   [2017]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [RFC] KVM: arm/arm64: Introduce KVM_DEV_ARM_ITS_CTRL_RESET
    On Fri, Sep 15, 2017 at 5:26 AM, Auger Eric <eric.auger@redhat.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@redhat.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@redhat.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

    \
     
     \ /
      Last update: 2017-09-15 19:56    [W:5.305 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site