Re: [PATCH] arm/kvm: add support for MTE

[Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com>]

On 29-07-2024 03:44 pm, Alex Bennée wrote:
> Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> writes:
> 
>> Hi Peter,
>>
>>
>> [Apologies for the delayed response]
>>
>> On 16-07-2024 09:15 pm, Peter Maydell wrote:
>>> On Tue, 9 Jul 2024 at 07:05, Ganapatrao Kulkarni
>>> <gankulkarni@os.amperecomputing.com> wrote:
>>>>
>>>> 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.
>>>>
>>>> This patch is rework of commit b320e21c48ce64853904bea6631c0158cc2ef227
>>>> which broke TCG since it made the TCG -cpu max
>>>> report the presence of MTE to the guest even if the board hadn't
>>>> enabled MTE by wiring up the tag RAM. This meant that if the guest
>>>> then tried to use MTE QEMU would segfault accessing the
>>>> non-existent tag RAM.
>>>>
>>>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>>>> Signed-off-by: Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com>
>>>> ---
>>> In target/arm/cpu.c:arm_cpu_realizefn() there is this code:
>>>       if (cpu_isar_feature(aa64_mte, cpu)) {
>>>           /*
>>>            * The architectural range of GM blocksize is 2-6, however qemu
>>>            * doesn't support blocksize of 2 (see HELPER(ldgm)).
>>>            */
>>>           if (tcg_enabled()) {
>>>               assert(cpu->gm_blocksize >= 3 && cpu->gm_blocksize <= 6);
>>>           }
>>> #ifndef CONFIG_USER_ONLY
>>>           /*
>>>            * If we do not have tag-memory provided by the machine,
>>>            * reduce MTE support to instructions enabled at EL0.
>>>            * This matches Cortex-A710 BROADCASTMTE input being LOW.
>>>            */
>>>           if (cpu->tag_memory == NULL) {
>>>               cpu->isar.id_aa64pfr1 =
>>>                   FIELD_DP64(cpu->isar.id_aa64pfr1, ID_AA64PFR1, MTE, 1);
>>>           }
>>> #endif
>>>       }
>>> With this patch, for KVM we will end up going through the
>>> "squash ID_AA64PFR1_EL1.MTE to 1" codepath, because KVM doesn't
>>> set cpu->tag_memory and this is still using that as its check.
>>>
>>
>> I looked at this function and it seems we are not entering this
>> function for KVM boot. I do see -DCONFIG_USER_ONLY added to make
>> files.
>>
>> Also Linux kernel wont detect/enable MTE until unless the
>> ID_AA64PFR1_EL1.MTE value is 2(b0010) and above.
>>
>>> More generally, how does the enabling of the MTE KVM cap
>>> interact with the ID_AA64PFR1_EL1 value that we read from
>>> the host in kvm_arm_get_host_cpu_features() ? We care that we
>>> have the right ID register values because we use ID field
>>> checks to determine whether the vcpu has a feature or not,
>>> even in the KVM case.
>>> Since Cornelia first wrote the patch this is based on, we've
>>> landed gdbstub support for MTE (so gdb can find out which
>>> addresses in the memory map have tags and read and write
>>> those tags). So I think the KVM MTE support now also needs to
>>> handle that. (See aarch64_cpu_register_gdb_commands() in
>>> target/arm/gdbstub64.c.)
>>
>> Ok sure, I will go through this file to add/update MTE part
> 
> So to be clear the current MTE gdbstub support is linux-user only.
> Gustavo has a series on the list that adds the system emulation part:
> 
>    Message-Id: <20240722160709.1677430-1-gustavo.romero@linaro.org>
>    Date: Mon, 22 Jul 2024 16:07:05 +0000
>    Subject: [PATCH 0/4] gdbstub: Add support for MTE in system mode
>    From: Gustavo Romero <gustavo.romero@linaro.org>
> 
> which of course is focused on TCG. But if the KVM guests sync to the same
> registers to cpregs I think most stuff should just work. However the
> current code uses the TCG only:
> 
>    allocation_tag_mem_probe
> 
> which I guess needs a KVM equivalent to query the tag memory?

Ok, thanks for the heads-up!.

Thanks,
Ganapat