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:0.068 / U:2.948 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site