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

[Ganapatrao Kulkarni]

On 29-07-2024 04:10 pm, Ganapatrao Kulkarni wrote:
> 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.

My bad, please ignore my previous/above comment.
I did not hit this issue since cpu_isar_feature(aa64_mte, cpu) is 
returning zero/false on my ARM64 platform. Then I dumped the register 
id_aa64pfr1 at QEMU(qemu-system-aarch64) as well in Linux(vanilla 6.10) 
kernel(for ioctl KVM_GET_ONE_REG) and to my surprise, in qemu the value 
is 0x21 however the value at kernel is 0x321(expected value).

Root-caused and it is due to, kernel is hiding[1] the MTE bits of 
ID_AA64PFR1_EL1 register from user/qemu. Need to send the kernel patch 
upstream to revert it, otherwise this check in qemu is dummy.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v6.11-rc1&id=2ac638fc5724f011f8ba1b425667c5592e1571ce

> > > 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