Re: [PULL 04/29] arm/kvm: add support for MTE

["Alex Bennée" <alex.bennee@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 3284 bytes --]


Cornelia Huck <cohuck@redhat.com> writes:

> On Fri, May 19 2023, Peter Maydell <peter.maydell@linaro.org> wrote:
>
>> On Fri, 19 May 2023 at 14:31, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>
>>> On Fri, 19 May 2023 at 13:55, Alex Bennée <alex.bennee@linaro.org> wrote:
>>> >
>>> >
>>> > Peter Maydell <peter.maydell@linaro.org> writes:
>>> >
>>> > > From: Cornelia Huck <cohuck@redhat.com>
>>> > >
>>> > > Extend the 'mte' property for the virt machine to cover KVM as
>>> > > well. For KVM, we don't allocate tag memory, but instead enable the
>>> > > capability.
>>> > >
>>> > > If MTE has been enabled, we need to disable migration, as we do not
>>> > > yet have a way to migrate the tags as well. Therefore, MTE will stay
>>> > > off with KVM unless requested explicitly.
>>> > >
>>> > > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>>> > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>>> > > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> > > Message-id: 20230428095533.21747-2-cohuck@redhat.com
>>> > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> >
>>> > I bisected to this commit which causes a segfault on one of my test
>>> > kernels (6.3.2 arm64):
>>> >
>>> >   ➜  ag MTE .config
>>> >   486:CONFIG_ARM64_AS_HAS_MTE=y
>>> >   487:CONFIG_ARM64_MTE=y
>>> >   2949:CONFIG_WLAN_VENDOR_ADMTEK=y
>>> >   3573:# CONFIG_I2C_SIMTEC is not set
>>> >   5278:# CONFIG_DRM_PANEL_TPO_TD043MTEA1 is not set
>>> >   9749:CONFIG_ARCH_USE_MEMTEST=y
>>> >   9750:CONFIG_MEMTEST=y
>
> Sigh, this patch seems to be cursed :( Apologies for the fallout.
>
> (I'm wondering what makes this pop up, maybe the CONFIG_MEMTEST?)
>
>>>
>>> Try this entirely untested patch?
>>>
>>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>>> index f6a88e52ac2..f350661a928 100644
>>> --- a/target/arm/cpu.c
>>> +++ b/target/arm/cpu.c
>>> @@ -1593,6 +1593,15 @@ static void arm_cpu_realizefn(DeviceState *dev,
>>> Error **errp)
>>>          }
>>>      }
>>>
>>> +    /*
>>> +     * For TCG, we can only present MTE to the guest if the board gave us
>>> +     * tag RAM. Set has_mte appropriately so code below doesn't need to
>>> +     * care whether we're TCG or KVM when deciding if MTE is present.
>>> +     */
>>> +    if (tcg_enabled() || qtest_enabled()) {
>>> +        cpu->has_mte = cpu->tag_memory != NULL;
>>> +    }
>>> +
>>>      if (!tcg_enabled() && !qtest_enabled()) {
>>>          /*
>>>           * We assume that no accelerator except TCG (and the "not really an
>>
>> Hmm, actually I don't think that's the only fix needed. It's OK for
>> TCG, but for KVM I can't see anywhere in the code that ever sets
>> has_mte to false. We default it to on in the cpu.c code, but
>> then the board code only sets it to true if MTE is enabled
>> (via kvm_arm_enable_mte()).
>
> Hrm, do we need explicit init of this field? Probably needless to say
> that I didn't hit this problem in any of my tests... I suspect that only
> specific kernels hit this?

Yes - my local test kernel usually has a rotating case of all the bells
and whistles enabled so catches things like this. I didn't attempt to
bisect the config but here is the current state if its useful:


[-- Attachment #2: config for my failing kernel setup --]
[-- Type: application/gzip, Size: 58034 bytes --]

[-- Attachment #3: Type: text/plain, Size: 214 bytes --]


I think it would be worthwhile adding a specific MTE enabled kernel to
the avocado tests if none of the existing ones can be booted up with MTE
enabled.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro