Hi Cornelia and Ganapatrao,

On 9/17/24 11:13, Cornelia Huck wrote:
On Thu, Sep 12 2024, Ganapatrao Kulkarni <gankulka...@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 <coh...@redhat.com>
Signed-off-by: Ganapatrao Kulkarni <gankulka...@os.amperecomputing.com>
---
Changes since V1:
        Added code to enable MTE before reading register
id_aa64pfr1 (unmasked MTE bits).

This patch is boot tested on ARM64 with KVM and on X86 with TCG for mte=on
and default case(i.e, no mte).

  hw/arm/virt.c        | 72 ++++++++++++++++++++++++++------------------
  target/arm/cpu.c     |  7 +++--
  target/arm/cpu.h     |  2 ++
  target/arm/kvm.c     | 59 ++++++++++++++++++++++++++++++++++++
  target/arm/kvm_arm.h | 19 ++++++++++++
  5 files changed, 126 insertions(+), 33 deletions(-)

(...)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 849e2e21b3..29865609fb 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
@@ -119,6 +120,21 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t 
*cpus_to_try,
      if (vmfd < 0) {
          goto err;
      }
+
+    /*
+     * MTE bits of register id_aa64pfr1 are masked if MTE is
+     * not enabled and required to enable before VCPU
+     * is created. Hence enable MTE(if supported) before VCPU
+     * is created to read unmasked MTE bits.
+     */
Maybe

"KVM will mask the MTE bits in id_aa64pfr1 unless the VMM has enabled
the MTE KVM capability, so do it here for probing."

?

KVM_CAP_ARM_MTE must be set before creating the VCPUs, as

stated in the KVM API docs, so enabling this cap. here for later

probing the MTE bits correctly feels more like a consequence. So

I prefer the the original comment, which mentions that

requirement. But I think something shorter like the following

would work too:

"MTE capability must be enabled by the VMM before creating

any VCPUs. The MTE bits of the ID_AA64PFR1 register are masked

if MTE is not enabled, allowing them to be probed correctly."


Cheers,

Gustavo


+    if (kvm_arm_mte_supported()) {
+        KVMState kvm_state;
+
+        kvm_state.fd = kvmfd;
+        kvm_state.vmfd = vmfd;
+        kvm_vm_enable_cap(&kvm_state, KVM_CAP_ARM_MTE, 0);
+    }
+
      cpufd = ioctl(vmfd, KVM_CREATE_VCPU, 0);
      if (cpufd < 0) {
          goto err;

Reply via email to