| Message ID | 20240709060448.251881-1-gankulkarni@os.amperecomputing.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm/kvm: add support for MTE | expand |
On Mon, Jul 08 2024, 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. So, the main difference to my original patch is that we don't end up with MTE in the max model if we didn't configure tag memory, but the rest of the behaviour stays the same? > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > Signed-off-by: Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> (...) > +void kvm_arm_enable_mte(Object *cpuobj, Error **errp) > +{ > + static bool tried_to_enable; > + static bool succeeded_to_enable; > + Error *mte_migration_blocker = NULL; > + int ret; > + > + if (!tried_to_enable) { > + /* > + * MTE on KVM is enabled on a per-VM basis (and retrying doesn't make > + * sense), and we only want a single migration blocker as well. > + */ > + tried_to_enable = true; > + > + ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_MTE, 0); > + if (ret) { > + error_setg_errno(errp, -ret, "Failed to enable KVM_CAP_ARM_MTE"); > + return; > + } > + > + /* TODO: Migration is not supported with MTE enabled */ Do you have a plan for enabling migration in the future? From what I remember, pre-copy support should be doable within QEMU (with a similar approach to e.g. s390 skey), but post-copy would need a kernel API extension to support getting additional data while faulting in a page. > + error_setg(&mte_migration_blocker, > + "Live migration disabled due to MTE enabled"); > + if (migrate_add_blocker(&mte_migration_blocker, errp)) { > + error_free(mte_migration_blocker); > + return; > + } > + succeeded_to_enable = true; > + } > + if (succeeded_to_enable) { > + object_property_set_bool(cpuobj, "has_mte", true, NULL); > + } > +}
On 10-07-2024 09:12 pm, Cornelia Huck wrote: > On Mon, Jul 08 2024, 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. > > So, the main difference to my original patch is that we don't end up > with MTE in the max model if we didn't configure tag memory, but the > rest of the behaviour stays the same? Yes most of the patch is same. What I changed is to fix the code which was advertising MTE feature through PFR1 register for TCG based boot, irrespective of the tagged RAM allocated. > >> >> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >> Signed-off-by: Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> > > (...) > >> +void kvm_arm_enable_mte(Object *cpuobj, Error **errp) >> +{ >> + static bool tried_to_enable; >> + static bool succeeded_to_enable; >> + Error *mte_migration_blocker = NULL; >> + int ret; >> + >> + if (!tried_to_enable) { >> + /* >> + * MTE on KVM is enabled on a per-VM basis (and retrying doesn't make >> + * sense), and we only want a single migration blocker as well. >> + */ >> + tried_to_enable = true; >> + >> + ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_MTE, 0); >> + if (ret) { >> + error_setg_errno(errp, -ret, "Failed to enable KVM_CAP_ARM_MTE"); >> + return; >> + } >> + >> + /* TODO: Migration is not supported with MTE enabled */ > > Do you have a plan for enabling migration in the future? From what I > remember, pre-copy support should be doable within QEMU (with a similar > approach to e.g. s390 skey), but post-copy would need a kernel API > extension to support getting additional data while faulting in a page. > No plan at the moment. >> + error_setg(&mte_migration_blocker, >> + "Live migration disabled due to MTE enabled"); >> + if (migrate_add_blocker(&mte_migration_blocker, errp)) { >> + error_free(mte_migration_blocker); >> + return; >> + } >> + succeeded_to_enable = true; >> + } >> + if (succeeded_to_enable) { >> + object_property_set_bool(cpuobj, "has_mte", true, NULL); >> + } >> +} > Thanks, Ganapat
Hi Peter/Richard, On 09-07-2024 11:34 am, Ganapatrao Kulkarni 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> > --- > > This patch is boot tested on ARM64 with KVM and on X86 with TCG for mte=on > and default case(no mte). Any comments on this patch? Can this patch be merged in this merge cycle/window? > > hw/arm/virt.c | 72 ++++++++++++++++++++++++++------------------ > target/arm/cpu.h | 1 + > target/arm/kvm.c | 40 ++++++++++++++++++++++++ > target/arm/kvm_arm.h | 19 ++++++++++++ > 4 files changed, 102 insertions(+), 30 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index b0c68d66a3..cc9db79ec3 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -2206,7 +2206,7 @@ static void machvirt_init(MachineState *machine) > exit(1); > } > > - if (vms->mte && (kvm_enabled() || hvf_enabled())) { > + if (vms->mte && hvf_enabled()) { > error_report("mach-virt: %s does not support providing " > "MTE to the guest CPU", > current_accel_name()); > @@ -2276,39 +2276,51 @@ static void machvirt_init(MachineState *machine) > } > > if (vms->mte) { > - /* Create the memory region only once, but link to all cpus. */ > - if (!tag_sysmem) { > - /* > - * The property exists only if MemTag is supported. > - * If it is, we must allocate the ram to back that up. > - */ > - if (!object_property_find(cpuobj, "tag-memory")) { > - error_report("MTE requested, but not supported " > - "by the guest CPU"); > - exit(1); > + if (tcg_enabled()) { > + /* Create the memory region only once, but link to all cpus. */ > + if (!tag_sysmem) { > + /* > + * The property exists only if MemTag is supported. > + * If it is, we must allocate the ram to back that up. > + */ > + if (!object_property_find(cpuobj, "tag-memory")) { > + error_report("MTE requested, but not supported " > + "by the guest CPU"); > + exit(1); > + } > + > + tag_sysmem = g_new(MemoryRegion, 1); > + memory_region_init(tag_sysmem, OBJECT(machine), > + "tag-memory", UINT64_MAX / 32); > + > + if (vms->secure) { > + secure_tag_sysmem = g_new(MemoryRegion, 1); > + memory_region_init(secure_tag_sysmem, OBJECT(machine), > + "secure-tag-memory", > + UINT64_MAX / 32); > + > + /* As with ram, secure-tag takes precedence over tag. */ > + memory_region_add_subregion_overlap(secure_tag_sysmem, > + 0, tag_sysmem, -1); > + } > } > > - tag_sysmem = g_new(MemoryRegion, 1); > - memory_region_init(tag_sysmem, OBJECT(machine), > - "tag-memory", UINT64_MAX / 32); > - > + object_property_set_link(cpuobj, "tag-memory", > + OBJECT(tag_sysmem), &error_abort); > if (vms->secure) { > - secure_tag_sysmem = g_new(MemoryRegion, 1); > - memory_region_init(secure_tag_sysmem, OBJECT(machine), > - "secure-tag-memory", UINT64_MAX / 32); > - > - /* As with ram, secure-tag takes precedence over tag. */ > - memory_region_add_subregion_overlap(secure_tag_sysmem, 0, > - tag_sysmem, -1); > + object_property_set_link(cpuobj, "secure-tag-memory", > + OBJECT(secure_tag_sysmem), > + &error_abort); > } > - } > - > - object_property_set_link(cpuobj, "tag-memory", OBJECT(tag_sysmem), > - &error_abort); > - if (vms->secure) { > - object_property_set_link(cpuobj, "secure-tag-memory", > - OBJECT(secure_tag_sysmem), > - &error_abort); > + } else if (kvm_enabled()) { > + if (!kvm_arm_mte_supported()) { > + error_report("MTE requested, but not supported by KVM"); > + exit(1); > + } > + kvm_arm_enable_mte(cpuobj, &error_abort); > + } else { > + error_report("MTE requested, but not supported "); > + exit(1); > } > } > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index d8eb986a04..661d35d8d8 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -1054,6 +1054,7 @@ struct ArchCPU { > bool prop_pauth_impdef; > bool prop_pauth_qarma3; > bool prop_lpa2; > + OnOffAuto prop_mte; > > /* DCZ blocksize, in log_2(words), ie low 4 bits of DCZID_EL0 */ > uint8_t dcz_blocksize; > diff --git a/target/arm/kvm.c b/target/arm/kvm.c > index 70f79eda33..0267a013e4 100644 > --- a/target/arm/kvm.c > +++ b/target/arm/kvm.c > @@ -39,6 +39,7 @@ > #include "hw/acpi/acpi.h" > #include "hw/acpi/ghes.h" > #include "target/arm/gtimer.h" > +#include "migration/blocker.h" > > const KVMCapabilityInfo kvm_arch_required_capabilities[] = { > KVM_CAP_LAST_INFO > @@ -1793,6 +1794,11 @@ bool kvm_arm_sve_supported(void) > return kvm_check_extension(kvm_state, KVM_CAP_ARM_SVE); > } > > +bool kvm_arm_mte_supported(void) > +{ > + return kvm_check_extension(kvm_state, KVM_CAP_ARM_MTE); > +} > + > QEMU_BUILD_BUG_ON(KVM_ARM64_SVE_VQ_MIN != 1); > > uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu) > @@ -2422,3 +2428,37 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) > } > return 0; > } > + > +void kvm_arm_enable_mte(Object *cpuobj, Error **errp) > +{ > + static bool tried_to_enable; > + static bool succeeded_to_enable; > + Error *mte_migration_blocker = NULL; > + int ret; > + > + if (!tried_to_enable) { > + /* > + * MTE on KVM is enabled on a per-VM basis (and retrying doesn't make > + * sense), and we only want a single migration blocker as well. > + */ > + tried_to_enable = true; > + > + ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_MTE, 0); > + if (ret) { > + error_setg_errno(errp, -ret, "Failed to enable KVM_CAP_ARM_MTE"); > + return; > + } > + > + /* TODO: Migration is not supported with MTE enabled */ > + error_setg(&mte_migration_blocker, > + "Live migration disabled due to MTE enabled"); > + if (migrate_add_blocker(&mte_migration_blocker, errp)) { > + error_free(mte_migration_blocker); > + return; > + } > + succeeded_to_enable = true; > + } > + if (succeeded_to_enable) { > + object_property_set_bool(cpuobj, "has_mte", true, NULL); > + } > +} > diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h > index cfaa0d9bc7..4d293618a7 100644 > --- a/target/arm/kvm_arm.h > +++ b/target/arm/kvm_arm.h > @@ -188,6 +188,13 @@ bool kvm_arm_pmu_supported(void); > */ > bool kvm_arm_sve_supported(void); > > +/** > + * kvm_arm_mte_supported: > + * > + * Returns: true if KVM can enable MTE, and false otherwise. > + */ > +bool kvm_arm_mte_supported(void); > + > /** > * kvm_arm_get_max_vm_ipa_size: > * @ms: Machine state handle > @@ -214,6 +221,8 @@ void kvm_arm_pvtime_init(ARMCPU *cpu, uint64_t ipa); > > int kvm_arm_set_irq(int cpu, int irqtype, int irq, int level); > > +void kvm_arm_enable_mte(Object *cpuobj, Error **errp); > + > #else > > /* > @@ -235,6 +244,11 @@ static inline bool kvm_arm_sve_supported(void) > return false; > } > > +static inline bool kvm_arm_mte_supported(void) > +{ > + return false; > +} > + > /* > * These functions should never actually be called without KVM support. > */ > @@ -283,6 +297,11 @@ static inline uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu) > g_assert_not_reached(); > } > > +static inline void kvm_arm_enable_mte(Object *cpuobj, Error **errp) > +{ > + g_assert_not_reached(); > +} > + > #endif > > #endif Thanks, Ganapat
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. 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.) thanks -- PMM
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 Thanks, Ganapat
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? > > > Thanks, > Ganapat
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
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
Hi Peter, 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. > > 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. Using ID_AA64PFR1_EL1.MTE bits seems to have issues with the Linux kernel implementation. This register is sensitized to user space and value read differs time to time. ID_AA64PFR1_EL1 register read at the beginning of qemu code will have the MTE bits cleared/masked. However ID_AA64PFR1_EL1.MTE bits are unmasked and shows the real value of MTE after ioctl KVM_CAP_ARM_MTE is executed to enable MTE. In QEMU,. isar.id_aa64pfr1 is read at the very beginning and cached, by creating dummy(kvm_arm_create_scratch_host_vcpu) interfaces(fds) for kvm, vm and vcpu. At later stages use of isar.id_aa64pfr1.mte at function like arm_cpu_realizefn does not show the right value and code never enters the "if (cpu_isar_feature(aa64_mte, cpu)" loop. Not sure about other feature bits, but for MTE, using isar.id_aa64pfr1 may not be appropriate but I do see it is getting used already many places of the code. I am not sure how it is behaving on emulator/TCG mode? Having said that, I have tried to remove the sensitization of ID_AA64PFR1_EL1 for MTE bits in the kernel, but that will have bigger problem like the VM boot crashes with the default compiled qemu binary with normal VM boot itself, since VM detects as MTE present and starts init of it. > > 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.) > > thanks > -- PMM Thanks, Ganapat
On Fri, Aug 02 2024, Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> wrote: > Hi Peter, > > 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. >> >> 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. > > Using ID_AA64PFR1_EL1.MTE bits seems to have issues with the Linux > kernel implementation. > This register is sensitized to user space and value read differs time to > time. ID_AA64PFR1_EL1 register read at the beginning of qemu code will > have the MTE bits cleared/masked. However ID_AA64PFR1_EL1.MTE bits are > unmasked and shows the real value of MTE after ioctl KVM_CAP_ARM_MTE is > executed to enable MTE. > In QEMU,. isar.id_aa64pfr1 is read at the very beginning and cached, by > creating dummy(kvm_arm_create_scratch_host_vcpu) interfaces(fds) for > kvm, vm and vcpu. At later stages use of isar.id_aa64pfr1.mte at > function like arm_cpu_realizefn does not show the right value and code > never enters the "if (cpu_isar_feature(aa64_mte, cpu)" loop. > > Not sure about other feature bits, but for MTE, using isar.id_aa64pfr1 > may not be appropriate but I do see it is getting used already many > places of the code. I am not sure how it is behaving on emulator/TCG mode? > > Having said that, I have tried to remove the sensitization of > ID_AA64PFR1_EL1 for MTE bits in the kernel, but that will have bigger > problem like the VM boot crashes with the default compiled qemu binary > with normal VM boot itself, since VM detects as MTE present and starts > init of it. Not sure if you've seen https://lore.kernel.org/all/20240723072004.1470688-1-shahuang@redhat.com/, which aims to make aa64pfr1 writable by userspace -- but still needs special handling for MTE. (That series also masks out MTEX and MTE_frac because the Linux kernel does not yet handle them; might become relevant if we want to support it from QEMU at some time in the future, but then the command line would also need some thought.) I fear that we'll be stuck with special handling for the MTE bits in aa64pfr1 because the kernel will not want to break its userspace API.
Hi Peter, On 16-07-2024 09:15 pm, Peter Maydell wrote: > > 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. > > 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 Linux kernel masks the MTE bits of register id_aa64pfr1 until unless the MTE is enabled for that VM. I have modified to enable MTE(KVM_CAP_ARM_MTE) before we read the register id_aa64pfr1 in kvm_arm_get_host_cpu_features to make sure we get the unmasked/actual MTE bits. I will post this change in the V2 patchset. > 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.) I looked at this code and it looks like, complete code is under ifdef CONFIG_USER_ONLY and for kvm(target aarch64-softmmu) this is not getting enabled. Are you asking to remove these ifdef and make mte-gdbstub commands available for the KVM mode as well?
On Tue, 10 Sept 2024 at 12:57, Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> wrote: > On 16-07-2024 09:15 pm, Peter Maydell wrote: > > 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.) > > I looked at this code and it looks like, complete code is under > ifdef CONFIG_USER_ONLY and for kvm(target aarch64-softmmu) this is not > getting enabled. Are you asking to remove these ifdef and make > mte-gdbstub commands available for the KVM mode as well? The system mode support for mte gdbstub is just about to land. The current patchset is this one: https://patchew.org/QEMU/20240906143316.657436-1-gustavo.romero@linaro.org/ thanks -- PMM
On 10-09-2024 05:53 pm, Peter Maydell wrote: > On Tue, 10 Sept 2024 at 12:57, Ganapatrao Kulkarni > <gankulkarni@os.amperecomputing.com> wrote: >> On 16-07-2024 09:15 pm, Peter Maydell wrote: >>> 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.) >> >> I looked at this code and it looks like, complete code is under >> ifdef CONFIG_USER_ONLY and for kvm(target aarch64-softmmu) this is not >> getting enabled. Are you asking to remove these ifdef and make >> mte-gdbstub commands available for the KVM mode as well? > > The system mode support for mte gdbstub is just about > to land. The current patchset is this one: > https://patchew.org/QEMU/20240906143316.657436-1-gustavo.romero@linaro.org/ > Thanks. I applied these patches to qemu and compiled gdb as said in the cover-letter. Below is the log of the run, help me to interpret the logs. [root@sut01sys-r214 mte-qemu]# make -C build -j 32 run-tcg-tests-aarch64-softmmu make: Entering directory '/home/ganapat/upstream/mte-qemu/build' BUILD aarch64-softmmu guest-tests RUN aarch64-softmmu guest-tests TEST mte on aarch64 TEST hello on aarch64 TEST interrupt on aarch64 TEST memory on aarch64 TEST memory-sve on aarch64 TEST hello-with-libbb.so on aarch64 TEST interrupt-with-libbb.so on aarch64 TEST memory-with-libbb.so on aarch64 TEST hello-with-libempty.so on aarch64 TEST interrupt-with-libempty.so on aarch64 TEST memory-with-libempty.so on aarch64 TEST interrupt-with-libinline.so on aarch64 TEST hello-with-libinline.so on aarch64 TEST memory-with-libinline.so on aarch64 TEST hello-with-libinsn.so on aarch64 TEST memory-with-libinsn.so on aarch64 TEST interrupt-with-libinsn.so on aarch64 TEST hello-with-libmem.so on aarch64 TEST interrupt-with-libmem.so on aarch64 TEST hello-with-libsyscall.so on aarch64 TEST memory-with-libmem.so on aarch64 TEST interrupt-with-libsyscall.so on aarch64 TEST softmmu gdbstub support on aarch64 TEST memory-with-libsyscall.so on aarch64 TEST softmmu gdbstub support on aarch64 TEST softmmu gdbstub untimely packets on aarch64 TEST softmmu gdbstub support on aarch64 TEST gdbstub MTE support on aarch64 TEST memory-record on aarch64 qemu-system-aarch64: -gdb unix:path=/tmp/tmp80lbp057qemu-gdbstub/gdbstub.socket,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/tmp80lbp057qemu-gdbstub/gdbstub.socket,server=on qemu-system-aarch64: -gdb unix:path=/tmp/tmp8h7kx8kyqemu-gdbstub/gdbstub.socket,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/tmp8h7kx8kyqemu-gdbstub/gdbstub.socket,server=on qemu-system-aarch64: -gdb unix:path=/tmp/tmpo0448vk6qemu-gdbstub/gdbstub.socket,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/tmpo0448vk6qemu-gdbstub/gdbstub.socket,server=on qemu-system-aarch64: -gdb unix:path=/tmp/tmplvwd79e3qemu-gdbstub/gdbstub.socket,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/tmplvwd79e3qemu-gdbstub/gdbstub.socket,server=on qemu-system-aarch64: -gdb unix:path=/tmp/tmpv1t2ffjfqemu-gdbstub/gdbstub.socket,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/tmpv1t2ffjfqemu-gdbstub/gdbstub.socket,server=on TEST memory-replay on aarch64 qemu-system-aarch64: QEMU: Terminated via GDBstub qemu-system-aarch64: QEMU: Terminated via GDBstub GREP file untimely-packet.gdb.err qemu-system-aarch64: QEMU: Terminated via GDBstub qemu-system-aarch64: QEMU: Terminated via GDBstub make: Leaving directory '/home/ganapat/upstream/mte-qemu/build' [root@sut01sys-r214 mte-qemu]# Last few lines of untimely-packet.gdb.err below: [remote] Packet received: 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 [24 bytes omitted] [remote] Sending packet: $qfThreadInfo#bb [remote] Received Ack [remote] Packet received: mp01.01 [remote] Sending packet: $qsThreadInfo#c8 [remote] Received Ack [remote] Packet received: l [remote] Sending packet: $p56#db [remote] Received Ack [remote] Packet received: 000000000000ffff [remote] packet_ok: Packet p (fetch-register) is supported [remote] Sending packet: $p57#dc [remote] Received Ack [remote] Packet received: 000000000000ffff [remote] Sending packet: $m400027b0,4#8c [remote] Received Ack [remote] Packet received: 80c2ff10 [remote] Sending packet: $m400027ac,4#be [remote] Received Ack [remote] Packet received: 1f2003d5 [remote] Sending packet: $m400027b0,4#8c [remote] Received Ack [remote] Packet received: 80c2ff10 [remote] Sending packet: $qSymbol::#5b [remote] Received Ack [remote] Packet received: [remote] packet_ok: Packet qSymbol (symbol-lookup) is NOT supported [remote] start_remote_1: exit [remote] Sending packet: $D;1#b0 [remote] Received Ack [remote] Packet received: OK
On Wed, 11 Sept 2024 at 07:50, Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> wrote: > > > > On 10-09-2024 05:53 pm, Peter Maydell wrote: > > On Tue, 10 Sept 2024 at 12:57, Ganapatrao Kulkarni > > <gankulkarni@os.amperecomputing.com> wrote: > >> On 16-07-2024 09:15 pm, Peter Maydell wrote: > >>> 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.) > >> > >> I looked at this code and it looks like, complete code is under > >> ifdef CONFIG_USER_ONLY and for kvm(target aarch64-softmmu) this is not > >> getting enabled. Are you asking to remove these ifdef and make > >> mte-gdbstub commands available for the KVM mode as well? > > > > The system mode support for mte gdbstub is just about > > to land. The current patchset is this one: > > https://patchew.org/QEMU/20240906143316.657436-1-gustavo.romero@linaro.org/ > > > > Thanks. > I applied these patches to qemu and compiled gdb as said in the > cover-letter. Below is the log of the run, help me to interpret the logs. I've cc'd Gustavo since he's the author of the patches. > [root@sut01sys-r214 mte-qemu]# make -C build -j 32 > run-tcg-tests-aarch64-softmmu > make: Entering directory '/home/ganapat/upstream/mte-qemu/build' > BUILD aarch64-softmmu guest-tests > RUN aarch64-softmmu guest-tests > TEST mte on aarch64 > TEST hello on aarch64 > TEST interrupt on aarch64 > TEST memory on aarch64 > TEST memory-sve on aarch64 > TEST hello-with-libbb.so on aarch64 > TEST interrupt-with-libbb.so on aarch64 > TEST memory-with-libbb.so on aarch64 > TEST hello-with-libempty.so on aarch64 > TEST interrupt-with-libempty.so on aarch64 > TEST memory-with-libempty.so on aarch64 > TEST interrupt-with-libinline.so on aarch64 > TEST hello-with-libinline.so on aarch64 > TEST memory-with-libinline.so on aarch64 > TEST hello-with-libinsn.so on aarch64 > TEST memory-with-libinsn.so on aarch64 > TEST interrupt-with-libinsn.so on aarch64 > TEST hello-with-libmem.so on aarch64 > TEST interrupt-with-libmem.so on aarch64 > TEST hello-with-libsyscall.so on aarch64 > TEST memory-with-libmem.so on aarch64 > TEST interrupt-with-libsyscall.so on aarch64 > TEST softmmu gdbstub support on aarch64 > TEST memory-with-libsyscall.so on aarch64 > TEST softmmu gdbstub support on aarch64 > TEST softmmu gdbstub untimely packets on aarch64 > TEST softmmu gdbstub support on aarch64 > TEST gdbstub MTE support on aarch64 > TEST memory-record on aarch64 > qemu-system-aarch64: -gdb > unix:path=/tmp/tmp80lbp057qemu-gdbstub/gdbstub.socket,server=on: info: > QEMU waiting for connection on: > disconnected:unix:/tmp/tmp80lbp057qemu-gdbstub/gdbstub.socket,server=on > qemu-system-aarch64: -gdb > unix:path=/tmp/tmp8h7kx8kyqemu-gdbstub/gdbstub.socket,server=on: info: > QEMU waiting for connection on: > disconnected:unix:/tmp/tmp8h7kx8kyqemu-gdbstub/gdbstub.socket,server=on > qemu-system-aarch64: -gdb > unix:path=/tmp/tmpo0448vk6qemu-gdbstub/gdbstub.socket,server=on: info: > QEMU waiting for connection on: > disconnected:unix:/tmp/tmpo0448vk6qemu-gdbstub/gdbstub.socket,server=on > qemu-system-aarch64: -gdb > unix:path=/tmp/tmplvwd79e3qemu-gdbstub/gdbstub.socket,server=on: info: > QEMU waiting for connection on: > disconnected:unix:/tmp/tmplvwd79e3qemu-gdbstub/gdbstub.socket,server=on > qemu-system-aarch64: -gdb > unix:path=/tmp/tmpv1t2ffjfqemu-gdbstub/gdbstub.socket,server=on: info: > QEMU waiting for connection on: > disconnected:unix:/tmp/tmpv1t2ffjfqemu-gdbstub/gdbstub.socket,server=on > TEST memory-replay on aarch64 > qemu-system-aarch64: QEMU: Terminated via GDBstub > qemu-system-aarch64: QEMU: Terminated via GDBstub > GREP file untimely-packet.gdb.err > qemu-system-aarch64: QEMU: Terminated via GDBstub > qemu-system-aarch64: QEMU: Terminated via GDBstub > make: Leaving directory '/home/ganapat/upstream/mte-qemu/build' > [root@sut01sys-r214 mte-qemu]# > > Last few lines of untimely-packet.gdb.err below: > > [remote] Packet received: > 00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 > [24 bytes omitted] > [remote] Sending packet: $qfThreadInfo#bb > [remote] Received Ack > [remote] Packet received: mp01.01 > [remote] Sending packet: $qsThreadInfo#c8 > [remote] Received Ack > [remote] Packet received: l > [remote] Sending packet: $p56#db > [remote] Received Ack > [remote] Packet received: 000000000000ffff > [remote] packet_ok: Packet p (fetch-register) is supported > [remote] Sending packet: $p57#dc > [remote] Received Ack > [remote] Packet received: 000000000000ffff > [remote] Sending packet: $m400027b0,4#8c > [remote] Received Ack > [remote] Packet received: 80c2ff10 > [remote] Sending packet: $m400027ac,4#be > [remote] Received Ack > [remote] Packet received: 1f2003d5 > [remote] Sending packet: $m400027b0,4#8c > [remote] Received Ack > [remote] Packet received: 80c2ff10 > [remote] Sending packet: $qSymbol::#5b > [remote] Received Ack > [remote] Packet received: > [remote] packet_ok: Packet qSymbol (symbol-lookup) is NOT supported > [remote] start_remote_1: exit > [remote] Sending packet: $D;1#b0 > [remote] Received Ack > [remote] Packet received: OK > > > -- > Thanks, > Ganapat/GK -- PMM
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index b0c68d66a3..cc9db79ec3 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -2206,7 +2206,7 @@ static void machvirt_init(MachineState *machine) exit(1); } - if (vms->mte && (kvm_enabled() || hvf_enabled())) { + if (vms->mte && hvf_enabled()) { error_report("mach-virt: %s does not support providing " "MTE to the guest CPU", current_accel_name()); @@ -2276,39 +2276,51 @@ static void machvirt_init(MachineState *machine) } if (vms->mte) { - /* Create the memory region only once, but link to all cpus. */ - if (!tag_sysmem) { - /* - * The property exists only if MemTag is supported. - * If it is, we must allocate the ram to back that up. - */ - if (!object_property_find(cpuobj, "tag-memory")) { - error_report("MTE requested, but not supported " - "by the guest CPU"); - exit(1); + if (tcg_enabled()) { + /* Create the memory region only once, but link to all cpus. */ + if (!tag_sysmem) { + /* + * The property exists only if MemTag is supported. + * If it is, we must allocate the ram to back that up. + */ + if (!object_property_find(cpuobj, "tag-memory")) { + error_report("MTE requested, but not supported " + "by the guest CPU"); + exit(1); + } + + tag_sysmem = g_new(MemoryRegion, 1); + memory_region_init(tag_sysmem, OBJECT(machine), + "tag-memory", UINT64_MAX / 32); + + if (vms->secure) { + secure_tag_sysmem = g_new(MemoryRegion, 1); + memory_region_init(secure_tag_sysmem, OBJECT(machine), + "secure-tag-memory", + UINT64_MAX / 32); + + /* As with ram, secure-tag takes precedence over tag. */ + memory_region_add_subregion_overlap(secure_tag_sysmem, + 0, tag_sysmem, -1); + } } - tag_sysmem = g_new(MemoryRegion, 1); - memory_region_init(tag_sysmem, OBJECT(machine), - "tag-memory", UINT64_MAX / 32); - + object_property_set_link(cpuobj, "tag-memory", + OBJECT(tag_sysmem), &error_abort); if (vms->secure) { - secure_tag_sysmem = g_new(MemoryRegion, 1); - memory_region_init(secure_tag_sysmem, OBJECT(machine), - "secure-tag-memory", UINT64_MAX / 32); - - /* As with ram, secure-tag takes precedence over tag. */ - memory_region_add_subregion_overlap(secure_tag_sysmem, 0, - tag_sysmem, -1); + object_property_set_link(cpuobj, "secure-tag-memory", + OBJECT(secure_tag_sysmem), + &error_abort); } - } - - object_property_set_link(cpuobj, "tag-memory", OBJECT(tag_sysmem), - &error_abort); - if (vms->secure) { - object_property_set_link(cpuobj, "secure-tag-memory", - OBJECT(secure_tag_sysmem), - &error_abort); + } else if (kvm_enabled()) { + if (!kvm_arm_mte_supported()) { + error_report("MTE requested, but not supported by KVM"); + exit(1); + } + kvm_arm_enable_mte(cpuobj, &error_abort); + } else { + error_report("MTE requested, but not supported "); + exit(1); } } diff --git a/target/arm/cpu.h b/target/arm/cpu.h index d8eb986a04..661d35d8d8 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -1054,6 +1054,7 @@ struct ArchCPU { bool prop_pauth_impdef; bool prop_pauth_qarma3; bool prop_lpa2; + OnOffAuto prop_mte; /* DCZ blocksize, in log_2(words), ie low 4 bits of DCZID_EL0 */ uint8_t dcz_blocksize; diff --git a/target/arm/kvm.c b/target/arm/kvm.c index 70f79eda33..0267a013e4 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -39,6 +39,7 @@ #include "hw/acpi/acpi.h" #include "hw/acpi/ghes.h" #include "target/arm/gtimer.h" +#include "migration/blocker.h" const KVMCapabilityInfo kvm_arch_required_capabilities[] = { KVM_CAP_LAST_INFO @@ -1793,6 +1794,11 @@ bool kvm_arm_sve_supported(void) return kvm_check_extension(kvm_state, KVM_CAP_ARM_SVE); } +bool kvm_arm_mte_supported(void) +{ + return kvm_check_extension(kvm_state, KVM_CAP_ARM_MTE); +} + QEMU_BUILD_BUG_ON(KVM_ARM64_SVE_VQ_MIN != 1); uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu) @@ -2422,3 +2428,37 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) } return 0; } + +void kvm_arm_enable_mte(Object *cpuobj, Error **errp) +{ + static bool tried_to_enable; + static bool succeeded_to_enable; + Error *mte_migration_blocker = NULL; + int ret; + + if (!tried_to_enable) { + /* + * MTE on KVM is enabled on a per-VM basis (and retrying doesn't make + * sense), and we only want a single migration blocker as well. + */ + tried_to_enable = true; + + ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_MTE, 0); + if (ret) { + error_setg_errno(errp, -ret, "Failed to enable KVM_CAP_ARM_MTE"); + return; + } + + /* TODO: Migration is not supported with MTE enabled */ + error_setg(&mte_migration_blocker, + "Live migration disabled due to MTE enabled"); + if (migrate_add_blocker(&mte_migration_blocker, errp)) { + error_free(mte_migration_blocker); + return; + } + succeeded_to_enable = true; + } + if (succeeded_to_enable) { + object_property_set_bool(cpuobj, "has_mte", true, NULL); + } +} diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h index cfaa0d9bc7..4d293618a7 100644 --- a/target/arm/kvm_arm.h +++ b/target/arm/kvm_arm.h @@ -188,6 +188,13 @@ bool kvm_arm_pmu_supported(void); */ bool kvm_arm_sve_supported(void); +/** + * kvm_arm_mte_supported: + * + * Returns: true if KVM can enable MTE, and false otherwise. + */ +bool kvm_arm_mte_supported(void); + /** * kvm_arm_get_max_vm_ipa_size: * @ms: Machine state handle @@ -214,6 +221,8 @@ void kvm_arm_pvtime_init(ARMCPU *cpu, uint64_t ipa); int kvm_arm_set_irq(int cpu, int irqtype, int irq, int level); +void kvm_arm_enable_mte(Object *cpuobj, Error **errp); + #else /* @@ -235,6 +244,11 @@ static inline bool kvm_arm_sve_supported(void) return false; } +static inline bool kvm_arm_mte_supported(void) +{ + return false; +} + /* * These functions should never actually be called without KVM support. */ @@ -283,6 +297,11 @@ static inline uint32_t kvm_arm_sve_get_vls(ARMCPU *cpu) g_assert_not_reached(); } +static inline void kvm_arm_enable_mte(Object *cpuobj, Error **errp) +{ + g_assert_not_reached(); +} + #endif #endif