From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 97E03C2B9F4 for ; Tue, 22 Jun 2021 08:57:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7A1DC6113D for ; Tue, 22 Jun 2021 08:57:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229769AbhFVI7R (ORCPT ); Tue, 22 Jun 2021 04:59:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53520 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229490AbhFVI7Q (ORCPT ); Tue, 22 Jun 2021 04:59:16 -0400 Received: from mail-oi1-x234.google.com (mail-oi1-x234.google.com [IPv6:2607:f8b0:4864:20::234]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C87C8C061574 for ; Tue, 22 Jun 2021 01:56:59 -0700 (PDT) Received: by mail-oi1-x234.google.com with SMTP id d19so22997256oic.7 for ; Tue, 22 Jun 2021 01:56:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=gsnx3WWXGHiAO3TPKJpzIec1T+lt/uyQth3YVlL9pnc=; b=rVxG5kaR90ysvW7okvOvabn5lo+ydrxWE4ngLY16gB2qCaPmwTFGDpC/HrF4kLM7Dh bmY6PONRopxgaQqJRL8C2B6E6T/6y3QFUi/LNCbq1ZzOp+diERhvt4+bpHMkBCnO0scx DXifc9h3lxHiRPbdXOMAzd4bH7KEK2MlM8v/Yd030HIeiNZLuWwbecnSMJiQky1G6Hxp sXckYlybo1FVWHHRUsfV4IkBLZh9reVDIGFOp4yR29oG8VOylYV7WmQmcitA1S+SIZE4 4lZGrcqBU1UQzS9KNoPlW9Z0A2EAej47g0WSs0eME0KQTZMH6T3hP9m4TU2Pmul6yHsG rUyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=gsnx3WWXGHiAO3TPKJpzIec1T+lt/uyQth3YVlL9pnc=; b=Q2G736zcz2ewo+zZIW9lTLtfd9ePYL03Z+ropXtb7y2+inSLvUQHqeDHnz1/iKJLgA AolBEbVC5AjXZ00oCgw8k/BHuozSKbxx7KqxQitoGB7UF3689GJ2FqdNuQdK8cv/oMz0 t6xRAbyULIJg/zG6h/mJ3X2G24K65x9+5kZcwQc7M0gPaL7W9pP9VukHgtQaC3YOAJOo zfm+ZhbAoz/y6m/DTfAl6gfB/b/AkRts0tC9SFnvo1SII8Qtx0H7qEIDfpK2D3dZK2M2 j0kvkA+Jtz7zIBE83cBWDte80YqC9H9inIh1FiJbI2F1SGsmH3KVVXdsJQ0IpZxrEBws RwJw== X-Gm-Message-State: AOAM532UGs80GmvotdSadM0ubHBzke2/V1w8oIYEMYgqlbOusGSguX37 lNLT4CYkgqncWNR+dW4hrluTKWCAhUbIqsgc5ZG0Jw== X-Google-Smtp-Source: ABdhPJzNy4tmmTU4hyu2JtcL2mUCBLsLb91CbtjqvgbJMOuxDhSiRtfNgTHpct2a5+XVH5QglaC1zgDWvalpN5+uc3c= X-Received: by 2002:aca:b38a:: with SMTP id c132mr2304058oif.90.1624352218925; Tue, 22 Jun 2021 01:56:58 -0700 (PDT) MIME-Version: 1.0 References: <20210621111716.37157-1-steven.price@arm.com> <20210621111716.37157-6-steven.price@arm.com> In-Reply-To: <20210621111716.37157-6-steven.price@arm.com> From: Fuad Tabba Date: Tue, 22 Jun 2021 09:56:22 +0100 Message-ID: Subject: Re: [PATCH v17 5/6] KVM: arm64: ioctl to fetch/store tags in a guest To: Steven Price Cc: Catalin Marinas , Marc Zyngier , Will Deacon , "Dr. David Alan Gilbert" , qemu-devel@nongnu.org, Dave Martin , Juan Quintela , Richard Henderson , linux-kernel@vger.kernel.org, Thomas Gleixner , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Mon, Jun 21, 2021 at 12:18 PM Steven Price wrote: > > The VMM may not wish to have it's own mapping of guest memory mapped > with PROT_MTE because this causes problems if the VMM has tag checking > enabled (the guest controls the tags in physical RAM and it's unlikely > the tags are correct for the VMM). > > Instead add a new ioctl which allows the VMM to easily read/write the > tags from guest memory, allowing the VMM's mapping to be non-PROT_MTE > while the VMM can still read/write the tags for the purpose of > migration. > > Reviewed-by: Catalin Marinas > Signed-off-by: Steven Price > --- > arch/arm64/include/asm/kvm_host.h | 3 ++ > arch/arm64/include/asm/mte-def.h | 1 + > arch/arm64/include/uapi/asm/kvm.h | 11 +++++ > arch/arm64/kvm/arm.c | 7 +++ > arch/arm64/kvm/guest.c | 82 +++++++++++++++++++++++++++++++ > include/uapi/linux/kvm.h | 1 + > 6 files changed, 105 insertions(+) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 309e36cc1b42..6a2ac4636d42 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -729,6 +729,9 @@ int kvm_arm_vcpu_arch_get_attr(struct kvm_vcpu *vcpu, > int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu, > struct kvm_device_attr *attr); > > +long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm, > + struct kvm_arm_copy_mte_tags *copy_tags); > + > /* Guest/host FPSIMD coordination helpers */ > int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu); > void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu); > diff --git a/arch/arm64/include/asm/mte-def.h b/arch/arm64/include/asm/mte-def.h > index cf241b0f0a42..626d359b396e 100644 > --- a/arch/arm64/include/asm/mte-def.h > +++ b/arch/arm64/include/asm/mte-def.h > @@ -7,6 +7,7 @@ > > #define MTE_GRANULE_SIZE UL(16) > #define MTE_GRANULE_MASK (~(MTE_GRANULE_SIZE - 1)) > +#define MTE_GRANULES_PER_PAGE (PAGE_SIZE / MTE_GRANULE_SIZE) > #define MTE_TAG_SHIFT 56 > #define MTE_TAG_SIZE 4 > #define MTE_TAG_MASK GENMASK((MTE_TAG_SHIFT + (MTE_TAG_SIZE - 1)), MTE_TAG_SHIFT) > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > index 24223adae150..b3edde68bc3e 100644 > --- a/arch/arm64/include/uapi/asm/kvm.h > +++ b/arch/arm64/include/uapi/asm/kvm.h > @@ -184,6 +184,17 @@ struct kvm_vcpu_events { > __u32 reserved[12]; > }; > > +struct kvm_arm_copy_mte_tags { > + __u64 guest_ipa; > + __u64 length; > + void __user *addr; > + __u64 flags; > + __u64 reserved[2]; > +}; > + > +#define KVM_ARM_TAGS_TO_GUEST 0 > +#define KVM_ARM_TAGS_FROM_GUEST 1 > + > /* If you need to interpret the index values, here is the key: */ > #define KVM_REG_ARM_COPROC_MASK 0x000000000FFF0000 > #define KVM_REG_ARM_COPROC_SHIFT 16 > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 28ce26a68f09..511f3716fe33 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -1359,6 +1359,13 @@ long kvm_arch_vm_ioctl(struct file *filp, > > return 0; > } > + case KVM_ARM_MTE_COPY_TAGS: { > + struct kvm_arm_copy_mte_tags copy_tags; > + > + if (copy_from_user(©_tags, argp, sizeof(copy_tags))) > + return -EFAULT; > + return kvm_vm_ioctl_mte_copy_tags(kvm, ©_tags); > + } > default: > return -EINVAL; > } > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c > index 5cb4a1cd5603..4ddb20017b2f 100644 > --- a/arch/arm64/kvm/guest.c > +++ b/arch/arm64/kvm/guest.c > @@ -995,3 +995,85 @@ int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu, > > return ret; > } > + > +long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm, > + struct kvm_arm_copy_mte_tags *copy_tags) > +{ > + gpa_t guest_ipa = copy_tags->guest_ipa; > + size_t length = copy_tags->length; > + void __user *tags = copy_tags->addr; > + gpa_t gfn; > + bool write = !(copy_tags->flags & KVM_ARM_TAGS_FROM_GUEST); > + int ret = 0; > + > + if (!kvm_has_mte(kvm)) > + return -EINVAL; > + > + if (copy_tags->reserved[0] || copy_tags->reserved[1]) > + return -EINVAL; > + > + if (copy_tags->flags & ~KVM_ARM_TAGS_FROM_GUEST) > + return -EINVAL; > + > + if (length & ~PAGE_MASK || guest_ipa & ~PAGE_MASK) > + return -EINVAL; > + > + gfn = gpa_to_gfn(guest_ipa); > + > + mutex_lock(&kvm->slots_lock); > + > + while (length > 0) { > + kvm_pfn_t pfn = gfn_to_pfn_prot(kvm, gfn, write, NULL); > + void *maddr; > + unsigned long num_tags; > + struct page *page; > + > + if (is_error_noslot_pfn(pfn)) { > + ret = -EFAULT; > + goto out; > + } > + > + page = pfn_to_online_page(pfn); > + if (!page) { > + /* Reject ZONE_DEVICE memory */ > + ret = -EFAULT; > + goto out; > + } > + maddr = page_address(page); > + > + if (!write) { > + if (test_bit(PG_mte_tagged, &page->flags)) > + num_tags = mte_copy_tags_to_user(tags, maddr, > + MTE_GRANULES_PER_PAGE); > + else > + /* No tags in memory, so write zeros */ > + num_tags = MTE_GRANULES_PER_PAGE - > + clear_user(tags, MTE_GRANULES_PER_PAGE); > + kvm_release_pfn_clean(pfn); > + } else { > + num_tags = mte_copy_tags_from_user(maddr, tags, > + MTE_GRANULES_PER_PAGE); > + kvm_release_pfn_dirty(pfn); > + } > + > + if (num_tags != MTE_GRANULES_PER_PAGE) { > + ret = -EFAULT; > + goto out; > + } > + > + /* Set the flag after checking the write completed fully */ > + if (write) > + set_bit(PG_mte_tagged, &page->flags); > + > + gfn++; > + tags += num_tags; > + length -= PAGE_SIZE; > + } > + > +out: > + mutex_unlock(&kvm->slots_lock); > + /* If some data has been copied report the number of bytes copied */ > + if (length != copy_tags->length) > + return copy_tags->length - length; I'm not sure if this is actually an issue, but a couple of comments on the return value if there is an error after a partial copy has been done. If mte_copy_tags_to_user or mte_copy_tags_from_user don't return MTE_GRANULES_PER_PAGE, then the check for num_tags would fail, but some of the tags would have been copied, which wouldn't be reflected in length. That said, on a write the tagged bit wouldn't be set, and on read then the return value would be conservative, but not incorrect. That said, even though it is described that way in the documentation (rather deep in the description though), it might be confusing to return a non-negative value on an error. The other kvm ioctl I could find that does something similar, KVM_S390_GET_IRQ_STATE, seems to always return a -ERROR on error, rather than the number of bytes copied. Cheers, /fuad > + return ret; > +} > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index d4da58ddcad7..da1edd2b4046 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -1429,6 +1429,7 @@ struct kvm_s390_ucas_mapping { > /* Available with KVM_CAP_PMU_EVENT_FILTER */ > #define KVM_SET_PMU_EVENT_FILTER _IOW(KVMIO, 0xb2, struct kvm_pmu_event_filter) > #define KVM_PPC_SVM_OFF _IO(KVMIO, 0xb3) > +#define KVM_ARM_MTE_COPY_TAGS _IOR(KVMIO, 0xb4, struct kvm_arm_copy_mte_tags) > > /* ioctl for vm fd */ > #define KVM_CREATE_DEVICE _IOWR(KVMIO, 0xe0, struct kvm_create_device) > -- > 2.20.1 > > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5BEC5C2B9F4 for ; Tue, 22 Jun 2021 08:59:51 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 021B46113D for ; Tue, 22 Jun 2021 08:59:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 021B46113D Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:39762 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lvcG6-0003fF-2h for qemu-devel@archiver.kernel.org; Tue, 22 Jun 2021 04:59:50 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:54636) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lvcDO-0007xl-Dl for qemu-devel@nongnu.org; Tue, 22 Jun 2021 04:57:02 -0400 Received: from mail-oi1-x22e.google.com ([2607:f8b0:4864:20::22e]:44983) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lvcDM-0003oi-Du for qemu-devel@nongnu.org; Tue, 22 Jun 2021 04:57:02 -0400 Received: by mail-oi1-x22e.google.com with SMTP id 14so354052oir.11 for ; Tue, 22 Jun 2021 01:56:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=gsnx3WWXGHiAO3TPKJpzIec1T+lt/uyQth3YVlL9pnc=; b=rVxG5kaR90ysvW7okvOvabn5lo+ydrxWE4ngLY16gB2qCaPmwTFGDpC/HrF4kLM7Dh bmY6PONRopxgaQqJRL8C2B6E6T/6y3QFUi/LNCbq1ZzOp+diERhvt4+bpHMkBCnO0scx DXifc9h3lxHiRPbdXOMAzd4bH7KEK2MlM8v/Yd030HIeiNZLuWwbecnSMJiQky1G6Hxp sXckYlybo1FVWHHRUsfV4IkBLZh9reVDIGFOp4yR29oG8VOylYV7WmQmcitA1S+SIZE4 4lZGrcqBU1UQzS9KNoPlW9Z0A2EAej47g0WSs0eME0KQTZMH6T3hP9m4TU2Pmul6yHsG rUyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=gsnx3WWXGHiAO3TPKJpzIec1T+lt/uyQth3YVlL9pnc=; b=Q4/jLBzqtbCVo+GoImB2YxjWIJqiVT2BzKjsjYnRXTdAWa+C+HX/87THGFB6iBYlHW WN9fCnscyv4PBKtdIaJ8Ps0HNMyc/RrSk36iZPHeLpVGCqaQfNkrjbfvspUjSSTYIuqw yrXNouleVqGw180UHSAzOLPmZO58lcsJq+1PSHo8BnsqqMhkD1VDQeZiDmZXmXoA4XHh Z0zes9giA6SQCjfDVQ9pZinBHar5Art0Y/QuqljaA7TDz/ohHwBvOGuw/VFLbHUUdslH PIDDLul2pUwjsBMFjkq/uI9oqUTmdjLjpD3EzhchtMxDCvWp+ZWrZXIWlVz4IvAvHHoP SB8g== X-Gm-Message-State: AOAM530KZxSaLQco93O0VR12i8S52qE+ue9IhJBNDBLPO7DPVfaCCKIA hcoz7QjhaSaVJt8D8JocE0wwh8OlrJ8CWnm3CRwxhA== X-Google-Smtp-Source: ABdhPJzNy4tmmTU4hyu2JtcL2mUCBLsLb91CbtjqvgbJMOuxDhSiRtfNgTHpct2a5+XVH5QglaC1zgDWvalpN5+uc3c= X-Received: by 2002:aca:b38a:: with SMTP id c132mr2304058oif.90.1624352218925; Tue, 22 Jun 2021 01:56:58 -0700 (PDT) MIME-Version: 1.0 References: <20210621111716.37157-1-steven.price@arm.com> <20210621111716.37157-6-steven.price@arm.com> In-Reply-To: <20210621111716.37157-6-steven.price@arm.com> From: Fuad Tabba Date: Tue, 22 Jun 2021 09:56:22 +0100 Message-ID: Subject: Re: [PATCH v17 5/6] KVM: arm64: ioctl to fetch/store tags in a guest To: Steven Price Cc: Catalin Marinas , Marc Zyngier , Will Deacon , "Dr. David Alan Gilbert" , qemu-devel@nongnu.org, Dave Martin , Juan Quintela , Richard Henderson , linux-kernel@vger.kernel.org, Thomas Gleixner , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="UTF-8" Received-SPF: pass client-ip=2607:f8b0:4864:20::22e; envelope-from=tabba@google.com; helo=mail-oi1-x22e.google.com X-Spam_score_int: -155 X-Spam_score: -15.6 X-Spam_bar: --------------- X-Spam_report: (-15.6 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_MED=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, ENV_AND_HDR_SPF_MATCH=-0.5, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URI_DOTEDU=1.999, USER_IN_DEF_DKIM_WL=-7.5, USER_IN_DEF_SPF_WL=-7.5 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" Hi, On Mon, Jun 21, 2021 at 12:18 PM Steven Price wrote: > > The VMM may not wish to have it's own mapping of guest memory mapped > with PROT_MTE because this causes problems if the VMM has tag checking > enabled (the guest controls the tags in physical RAM and it's unlikely > the tags are correct for the VMM). > > Instead add a new ioctl which allows the VMM to easily read/write the > tags from guest memory, allowing the VMM's mapping to be non-PROT_MTE > while the VMM can still read/write the tags for the purpose of > migration. > > Reviewed-by: Catalin Marinas > Signed-off-by: Steven Price > --- > arch/arm64/include/asm/kvm_host.h | 3 ++ > arch/arm64/include/asm/mte-def.h | 1 + > arch/arm64/include/uapi/asm/kvm.h | 11 +++++ > arch/arm64/kvm/arm.c | 7 +++ > arch/arm64/kvm/guest.c | 82 +++++++++++++++++++++++++++++++ > include/uapi/linux/kvm.h | 1 + > 6 files changed, 105 insertions(+) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 309e36cc1b42..6a2ac4636d42 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -729,6 +729,9 @@ int kvm_arm_vcpu_arch_get_attr(struct kvm_vcpu *vcpu, > int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu, > struct kvm_device_attr *attr); > > +long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm, > + struct kvm_arm_copy_mte_tags *copy_tags); > + > /* Guest/host FPSIMD coordination helpers */ > int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu); > void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu); > diff --git a/arch/arm64/include/asm/mte-def.h b/arch/arm64/include/asm/mte-def.h > index cf241b0f0a42..626d359b396e 100644 > --- a/arch/arm64/include/asm/mte-def.h > +++ b/arch/arm64/include/asm/mte-def.h > @@ -7,6 +7,7 @@ > > #define MTE_GRANULE_SIZE UL(16) > #define MTE_GRANULE_MASK (~(MTE_GRANULE_SIZE - 1)) > +#define MTE_GRANULES_PER_PAGE (PAGE_SIZE / MTE_GRANULE_SIZE) > #define MTE_TAG_SHIFT 56 > #define MTE_TAG_SIZE 4 > #define MTE_TAG_MASK GENMASK((MTE_TAG_SHIFT + (MTE_TAG_SIZE - 1)), MTE_TAG_SHIFT) > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > index 24223adae150..b3edde68bc3e 100644 > --- a/arch/arm64/include/uapi/asm/kvm.h > +++ b/arch/arm64/include/uapi/asm/kvm.h > @@ -184,6 +184,17 @@ struct kvm_vcpu_events { > __u32 reserved[12]; > }; > > +struct kvm_arm_copy_mte_tags { > + __u64 guest_ipa; > + __u64 length; > + void __user *addr; > + __u64 flags; > + __u64 reserved[2]; > +}; > + > +#define KVM_ARM_TAGS_TO_GUEST 0 > +#define KVM_ARM_TAGS_FROM_GUEST 1 > + > /* If you need to interpret the index values, here is the key: */ > #define KVM_REG_ARM_COPROC_MASK 0x000000000FFF0000 > #define KVM_REG_ARM_COPROC_SHIFT 16 > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 28ce26a68f09..511f3716fe33 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -1359,6 +1359,13 @@ long kvm_arch_vm_ioctl(struct file *filp, > > return 0; > } > + case KVM_ARM_MTE_COPY_TAGS: { > + struct kvm_arm_copy_mte_tags copy_tags; > + > + if (copy_from_user(©_tags, argp, sizeof(copy_tags))) > + return -EFAULT; > + return kvm_vm_ioctl_mte_copy_tags(kvm, ©_tags); > + } > default: > return -EINVAL; > } > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c > index 5cb4a1cd5603..4ddb20017b2f 100644 > --- a/arch/arm64/kvm/guest.c > +++ b/arch/arm64/kvm/guest.c > @@ -995,3 +995,85 @@ int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu, > > return ret; > } > + > +long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm, > + struct kvm_arm_copy_mte_tags *copy_tags) > +{ > + gpa_t guest_ipa = copy_tags->guest_ipa; > + size_t length = copy_tags->length; > + void __user *tags = copy_tags->addr; > + gpa_t gfn; > + bool write = !(copy_tags->flags & KVM_ARM_TAGS_FROM_GUEST); > + int ret = 0; > + > + if (!kvm_has_mte(kvm)) > + return -EINVAL; > + > + if (copy_tags->reserved[0] || copy_tags->reserved[1]) > + return -EINVAL; > + > + if (copy_tags->flags & ~KVM_ARM_TAGS_FROM_GUEST) > + return -EINVAL; > + > + if (length & ~PAGE_MASK || guest_ipa & ~PAGE_MASK) > + return -EINVAL; > + > + gfn = gpa_to_gfn(guest_ipa); > + > + mutex_lock(&kvm->slots_lock); > + > + while (length > 0) { > + kvm_pfn_t pfn = gfn_to_pfn_prot(kvm, gfn, write, NULL); > + void *maddr; > + unsigned long num_tags; > + struct page *page; > + > + if (is_error_noslot_pfn(pfn)) { > + ret = -EFAULT; > + goto out; > + } > + > + page = pfn_to_online_page(pfn); > + if (!page) { > + /* Reject ZONE_DEVICE memory */ > + ret = -EFAULT; > + goto out; > + } > + maddr = page_address(page); > + > + if (!write) { > + if (test_bit(PG_mte_tagged, &page->flags)) > + num_tags = mte_copy_tags_to_user(tags, maddr, > + MTE_GRANULES_PER_PAGE); > + else > + /* No tags in memory, so write zeros */ > + num_tags = MTE_GRANULES_PER_PAGE - > + clear_user(tags, MTE_GRANULES_PER_PAGE); > + kvm_release_pfn_clean(pfn); > + } else { > + num_tags = mte_copy_tags_from_user(maddr, tags, > + MTE_GRANULES_PER_PAGE); > + kvm_release_pfn_dirty(pfn); > + } > + > + if (num_tags != MTE_GRANULES_PER_PAGE) { > + ret = -EFAULT; > + goto out; > + } > + > + /* Set the flag after checking the write completed fully */ > + if (write) > + set_bit(PG_mte_tagged, &page->flags); > + > + gfn++; > + tags += num_tags; > + length -= PAGE_SIZE; > + } > + > +out: > + mutex_unlock(&kvm->slots_lock); > + /* If some data has been copied report the number of bytes copied */ > + if (length != copy_tags->length) > + return copy_tags->length - length; I'm not sure if this is actually an issue, but a couple of comments on the return value if there is an error after a partial copy has been done. If mte_copy_tags_to_user or mte_copy_tags_from_user don't return MTE_GRANULES_PER_PAGE, then the check for num_tags would fail, but some of the tags would have been copied, which wouldn't be reflected in length. That said, on a write the tagged bit wouldn't be set, and on read then the return value would be conservative, but not incorrect. That said, even though it is described that way in the documentation (rather deep in the description though), it might be confusing to return a non-negative value on an error. The other kvm ioctl I could find that does something similar, KVM_S390_GET_IRQ_STATE, seems to always return a -ERROR on error, rather than the number of bytes copied. Cheers, /fuad > + return ret; > +} > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index d4da58ddcad7..da1edd2b4046 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -1429,6 +1429,7 @@ struct kvm_s390_ucas_mapping { > /* Available with KVM_CAP_PMU_EVENT_FILTER */ > #define KVM_SET_PMU_EVENT_FILTER _IOW(KVMIO, 0xb2, struct kvm_pmu_event_filter) > #define KVM_PPC_SVM_OFF _IO(KVMIO, 0xb3) > +#define KVM_ARM_MTE_COPY_TAGS _IOR(KVMIO, 0xb4, struct kvm_arm_copy_mte_tags) > > /* ioctl for vm fd */ > #define KVM_CREATE_DEVICE _IOWR(KVMIO, 0xe0, struct kvm_create_device) > -- > 2.20.1 > > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.5 required=3.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED,DKIM_INVALID,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B1B6AC48BE5 for ; Tue, 22 Jun 2021 08:57:04 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 0D9B36137D for ; Tue, 22 Jun 2021 08:57:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0D9B36137D Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 7E8574066E; Tue, 22 Jun 2021 04:57:03 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@google.com Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tLdhT2PQyr11; Tue, 22 Jun 2021 04:57:02 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 2A87F406DD; Tue, 22 Jun 2021 04:57:02 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 4A5FB4064F for ; Tue, 22 Jun 2021 04:57:01 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id mp+WqJId-64x for ; Tue, 22 Jun 2021 04:56:59 -0400 (EDT) Received: from mail-oi1-f176.google.com (mail-oi1-f176.google.com [209.85.167.176]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id D995840642 for ; Tue, 22 Jun 2021 04:56:59 -0400 (EDT) Received: by mail-oi1-f176.google.com with SMTP id s17so174738oij.0 for ; Tue, 22 Jun 2021 01:56:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=gsnx3WWXGHiAO3TPKJpzIec1T+lt/uyQth3YVlL9pnc=; b=rVxG5kaR90ysvW7okvOvabn5lo+ydrxWE4ngLY16gB2qCaPmwTFGDpC/HrF4kLM7Dh bmY6PONRopxgaQqJRL8C2B6E6T/6y3QFUi/LNCbq1ZzOp+diERhvt4+bpHMkBCnO0scx DXifc9h3lxHiRPbdXOMAzd4bH7KEK2MlM8v/Yd030HIeiNZLuWwbecnSMJiQky1G6Hxp sXckYlybo1FVWHHRUsfV4IkBLZh9reVDIGFOp4yR29oG8VOylYV7WmQmcitA1S+SIZE4 4lZGrcqBU1UQzS9KNoPlW9Z0A2EAej47g0WSs0eME0KQTZMH6T3hP9m4TU2Pmul6yHsG rUyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=gsnx3WWXGHiAO3TPKJpzIec1T+lt/uyQth3YVlL9pnc=; b=poArwzEZqGXSxrm7zyNErptkTKkfQaR9S6RK8OoQiNsKjP5UmwnBmPKr5QNG48ozUe f7B+/31qKHOQ16rj/ZJcmYjFRbqnq1s4v47SJKjDZB9liZRtRYa6TGn+467QrYHKlJ1H ftT2hYVy6dIUmPIoulkWb/Qw+cPNJknLmAdWRgeUnBIqS7JHPYOOO+3S4x2FuktLu7L9 oEbrQrheTuB/tAUSGXuORD++kgEjLtLuSiBoe82Qtsw9JmDIF7L3ckadai2EVRat5Y2O A4S4YYHkwq/nhzpR2rwxZEKZtNuQ5EEuC0NiJmpw19WER7MMmQ0BItn2vnfFnsEw17ef eSBQ== X-Gm-Message-State: AOAM533FpXmJXPliLyooxCFVdUu9vXryTuFAb6KiWyCPs4zjSIofdgHG WmzrOFj2aH5dDUmrhdcsRf2rcfWB+65Gv2IBMRXIWA== X-Google-Smtp-Source: ABdhPJzNy4tmmTU4hyu2JtcL2mUCBLsLb91CbtjqvgbJMOuxDhSiRtfNgTHpct2a5+XVH5QglaC1zgDWvalpN5+uc3c= X-Received: by 2002:aca:b38a:: with SMTP id c132mr2304058oif.90.1624352218925; Tue, 22 Jun 2021 01:56:58 -0700 (PDT) MIME-Version: 1.0 References: <20210621111716.37157-1-steven.price@arm.com> <20210621111716.37157-6-steven.price@arm.com> In-Reply-To: <20210621111716.37157-6-steven.price@arm.com> From: Fuad Tabba Date: Tue, 22 Jun 2021 09:56:22 +0100 Message-ID: Subject: Re: [PATCH v17 5/6] KVM: arm64: ioctl to fetch/store tags in a guest To: Steven Price Cc: Juan Quintela , Catalin Marinas , Richard Henderson , qemu-devel@nongnu.org, "Dr. David Alan Gilbert" , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, Marc Zyngier , Thomas Gleixner , Will Deacon , Dave Martin , linux-kernel@vger.kernel.org X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu Hi, On Mon, Jun 21, 2021 at 12:18 PM Steven Price wrote: > > The VMM may not wish to have it's own mapping of guest memory mapped > with PROT_MTE because this causes problems if the VMM has tag checking > enabled (the guest controls the tags in physical RAM and it's unlikely > the tags are correct for the VMM). > > Instead add a new ioctl which allows the VMM to easily read/write the > tags from guest memory, allowing the VMM's mapping to be non-PROT_MTE > while the VMM can still read/write the tags for the purpose of > migration. > > Reviewed-by: Catalin Marinas > Signed-off-by: Steven Price > --- > arch/arm64/include/asm/kvm_host.h | 3 ++ > arch/arm64/include/asm/mte-def.h | 1 + > arch/arm64/include/uapi/asm/kvm.h | 11 +++++ > arch/arm64/kvm/arm.c | 7 +++ > arch/arm64/kvm/guest.c | 82 +++++++++++++++++++++++++++++++ > include/uapi/linux/kvm.h | 1 + > 6 files changed, 105 insertions(+) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 309e36cc1b42..6a2ac4636d42 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -729,6 +729,9 @@ int kvm_arm_vcpu_arch_get_attr(struct kvm_vcpu *vcpu, > int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu, > struct kvm_device_attr *attr); > > +long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm, > + struct kvm_arm_copy_mte_tags *copy_tags); > + > /* Guest/host FPSIMD coordination helpers */ > int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu); > void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu); > diff --git a/arch/arm64/include/asm/mte-def.h b/arch/arm64/include/asm/mte-def.h > index cf241b0f0a42..626d359b396e 100644 > --- a/arch/arm64/include/asm/mte-def.h > +++ b/arch/arm64/include/asm/mte-def.h > @@ -7,6 +7,7 @@ > > #define MTE_GRANULE_SIZE UL(16) > #define MTE_GRANULE_MASK (~(MTE_GRANULE_SIZE - 1)) > +#define MTE_GRANULES_PER_PAGE (PAGE_SIZE / MTE_GRANULE_SIZE) > #define MTE_TAG_SHIFT 56 > #define MTE_TAG_SIZE 4 > #define MTE_TAG_MASK GENMASK((MTE_TAG_SHIFT + (MTE_TAG_SIZE - 1)), MTE_TAG_SHIFT) > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > index 24223adae150..b3edde68bc3e 100644 > --- a/arch/arm64/include/uapi/asm/kvm.h > +++ b/arch/arm64/include/uapi/asm/kvm.h > @@ -184,6 +184,17 @@ struct kvm_vcpu_events { > __u32 reserved[12]; > }; > > +struct kvm_arm_copy_mte_tags { > + __u64 guest_ipa; > + __u64 length; > + void __user *addr; > + __u64 flags; > + __u64 reserved[2]; > +}; > + > +#define KVM_ARM_TAGS_TO_GUEST 0 > +#define KVM_ARM_TAGS_FROM_GUEST 1 > + > /* If you need to interpret the index values, here is the key: */ > #define KVM_REG_ARM_COPROC_MASK 0x000000000FFF0000 > #define KVM_REG_ARM_COPROC_SHIFT 16 > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 28ce26a68f09..511f3716fe33 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -1359,6 +1359,13 @@ long kvm_arch_vm_ioctl(struct file *filp, > > return 0; > } > + case KVM_ARM_MTE_COPY_TAGS: { > + struct kvm_arm_copy_mte_tags copy_tags; > + > + if (copy_from_user(©_tags, argp, sizeof(copy_tags))) > + return -EFAULT; > + return kvm_vm_ioctl_mte_copy_tags(kvm, ©_tags); > + } > default: > return -EINVAL; > } > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c > index 5cb4a1cd5603..4ddb20017b2f 100644 > --- a/arch/arm64/kvm/guest.c > +++ b/arch/arm64/kvm/guest.c > @@ -995,3 +995,85 @@ int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu, > > return ret; > } > + > +long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm, > + struct kvm_arm_copy_mte_tags *copy_tags) > +{ > + gpa_t guest_ipa = copy_tags->guest_ipa; > + size_t length = copy_tags->length; > + void __user *tags = copy_tags->addr; > + gpa_t gfn; > + bool write = !(copy_tags->flags & KVM_ARM_TAGS_FROM_GUEST); > + int ret = 0; > + > + if (!kvm_has_mte(kvm)) > + return -EINVAL; > + > + if (copy_tags->reserved[0] || copy_tags->reserved[1]) > + return -EINVAL; > + > + if (copy_tags->flags & ~KVM_ARM_TAGS_FROM_GUEST) > + return -EINVAL; > + > + if (length & ~PAGE_MASK || guest_ipa & ~PAGE_MASK) > + return -EINVAL; > + > + gfn = gpa_to_gfn(guest_ipa); > + > + mutex_lock(&kvm->slots_lock); > + > + while (length > 0) { > + kvm_pfn_t pfn = gfn_to_pfn_prot(kvm, gfn, write, NULL); > + void *maddr; > + unsigned long num_tags; > + struct page *page; > + > + if (is_error_noslot_pfn(pfn)) { > + ret = -EFAULT; > + goto out; > + } > + > + page = pfn_to_online_page(pfn); > + if (!page) { > + /* Reject ZONE_DEVICE memory */ > + ret = -EFAULT; > + goto out; > + } > + maddr = page_address(page); > + > + if (!write) { > + if (test_bit(PG_mte_tagged, &page->flags)) > + num_tags = mte_copy_tags_to_user(tags, maddr, > + MTE_GRANULES_PER_PAGE); > + else > + /* No tags in memory, so write zeros */ > + num_tags = MTE_GRANULES_PER_PAGE - > + clear_user(tags, MTE_GRANULES_PER_PAGE); > + kvm_release_pfn_clean(pfn); > + } else { > + num_tags = mte_copy_tags_from_user(maddr, tags, > + MTE_GRANULES_PER_PAGE); > + kvm_release_pfn_dirty(pfn); > + } > + > + if (num_tags != MTE_GRANULES_PER_PAGE) { > + ret = -EFAULT; > + goto out; > + } > + > + /* Set the flag after checking the write completed fully */ > + if (write) > + set_bit(PG_mte_tagged, &page->flags); > + > + gfn++; > + tags += num_tags; > + length -= PAGE_SIZE; > + } > + > +out: > + mutex_unlock(&kvm->slots_lock); > + /* If some data has been copied report the number of bytes copied */ > + if (length != copy_tags->length) > + return copy_tags->length - length; I'm not sure if this is actually an issue, but a couple of comments on the return value if there is an error after a partial copy has been done. If mte_copy_tags_to_user or mte_copy_tags_from_user don't return MTE_GRANULES_PER_PAGE, then the check for num_tags would fail, but some of the tags would have been copied, which wouldn't be reflected in length. That said, on a write the tagged bit wouldn't be set, and on read then the return value would be conservative, but not incorrect. That said, even though it is described that way in the documentation (rather deep in the description though), it might be confusing to return a non-negative value on an error. The other kvm ioctl I could find that does something similar, KVM_S390_GET_IRQ_STATE, seems to always return a -ERROR on error, rather than the number of bytes copied. Cheers, /fuad > + return ret; > +} > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index d4da58ddcad7..da1edd2b4046 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -1429,6 +1429,7 @@ struct kvm_s390_ucas_mapping { > /* Available with KVM_CAP_PMU_EVENT_FILTER */ > #define KVM_SET_PMU_EVENT_FILTER _IOW(KVMIO, 0xb2, struct kvm_pmu_event_filter) > #define KVM_PPC_SVM_OFF _IO(KVMIO, 0xb3) > +#define KVM_ARM_MTE_COPY_TAGS _IOR(KVMIO, 0xb4, struct kvm_arm_copy_mte_tags) > > /* ioctl for vm fd */ > #define KVM_CREATE_DEVICE _IOWR(KVMIO, 0xe0, struct kvm_create_device) > -- > 2.20.1 > > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 412E5C2B9F4 for ; Tue, 22 Jun 2021 08:59:09 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 01A746113D for ; Tue, 22 Jun 2021 08:59:08 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 01A746113D Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=aWrMP0Fool1Kfo4aAZa9T2geeArdP3O0OtPgbBwgAXA=; b=HTS8LR5eFxNu31 ryMjM5Y8iwevJH3RxuKmD3KbdRVuUPugJBQomTNaMbMFqBOjaKVjOG756ciTiX7D43NBO9mQNRLrx Gjg0qoPn+/rIjE5ZbgHO5+Y8Ark56HycdD1uJcOi4VkEmftgF7OX4r7WdO1Wl7cgfFbaq8typDdXN Ov8EWGvqeKH+vHfEoOKmQLxrXnHVlovMl8xQDmwnjclFOCuan6gRPHNR9cudnr/gQQg/OGTSxwpus ovFOvBBFOcxNRSgxMsQROopSP8j+NWT39t9qAeAv0MocInitGDxDmBBWf7tAgsSDrD4fONRxBeXM+ Oo8GtYqeYzvP4x0xbQww==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lvcDR-006JrO-4b; Tue, 22 Jun 2021 08:57:05 +0000 Received: from mail-oi1-x235.google.com ([2607:f8b0:4864:20::235]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lvcDM-006JqW-1d for linux-arm-kernel@lists.infradead.org; Tue, 22 Jun 2021 08:57:01 +0000 Received: by mail-oi1-x235.google.com with SMTP id x196so22981592oif.10 for ; Tue, 22 Jun 2021 01:56:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=gsnx3WWXGHiAO3TPKJpzIec1T+lt/uyQth3YVlL9pnc=; b=rVxG5kaR90ysvW7okvOvabn5lo+ydrxWE4ngLY16gB2qCaPmwTFGDpC/HrF4kLM7Dh bmY6PONRopxgaQqJRL8C2B6E6T/6y3QFUi/LNCbq1ZzOp+diERhvt4+bpHMkBCnO0scx DXifc9h3lxHiRPbdXOMAzd4bH7KEK2MlM8v/Yd030HIeiNZLuWwbecnSMJiQky1G6Hxp sXckYlybo1FVWHHRUsfV4IkBLZh9reVDIGFOp4yR29oG8VOylYV7WmQmcitA1S+SIZE4 4lZGrcqBU1UQzS9KNoPlW9Z0A2EAej47g0WSs0eME0KQTZMH6T3hP9m4TU2Pmul6yHsG rUyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=gsnx3WWXGHiAO3TPKJpzIec1T+lt/uyQth3YVlL9pnc=; b=LyJkAuusU0oe+8542o4Y+WRQ+KF0qdknUomRoIqNUAq6v9ATQ7BAjbAjS62yybK5bD 41osFuE56HojG3mFhHaY8iI3uohOfx4YVRZTGOC0VT77v1AacVJ+d7ZFQtjxdcG74waO JBPhPalxBu8R3RE/KZ/TAhuhcolYVQAnjC69vS2OhS6SsBQwCQ5DRkMVtjJGl4DNYA4U 6bg0tW2nN2jV/SDu4v+gkEqiwDmu9Jw3l4l4wzBd1cQYx0+AJRNc/mq97D/pCZUxJ67y Tl4+bJOUzPPDaG7bg7iuGvfsQDjGdVR2EEwoc5OzO77ZodOJuQE9mEyH1PeS1S4HDjAg vm8Q== X-Gm-Message-State: AOAM532GAbzfRZ4U7Cy/JEXuoGzdwacbwWXyarAqhVNI95yeX/FgSjKI 5bdnXRxmTzptoydJ+d5LIame8sP1qz5g/d0FciZqrg== X-Google-Smtp-Source: ABdhPJzNy4tmmTU4hyu2JtcL2mUCBLsLb91CbtjqvgbJMOuxDhSiRtfNgTHpct2a5+XVH5QglaC1zgDWvalpN5+uc3c= X-Received: by 2002:aca:b38a:: with SMTP id c132mr2304058oif.90.1624352218925; Tue, 22 Jun 2021 01:56:58 -0700 (PDT) MIME-Version: 1.0 References: <20210621111716.37157-1-steven.price@arm.com> <20210621111716.37157-6-steven.price@arm.com> In-Reply-To: <20210621111716.37157-6-steven.price@arm.com> From: Fuad Tabba Date: Tue, 22 Jun 2021 09:56:22 +0100 Message-ID: Subject: Re: [PATCH v17 5/6] KVM: arm64: ioctl to fetch/store tags in a guest To: Steven Price Cc: Catalin Marinas , Marc Zyngier , Will Deacon , "Dr. David Alan Gilbert" , qemu-devel@nongnu.org, Dave Martin , Juan Quintela , Richard Henderson , linux-kernel@vger.kernel.org, Thomas Gleixner , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210622_015700_179046_651FE383 X-CRM114-Status: GOOD ( 40.79 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi, On Mon, Jun 21, 2021 at 12:18 PM Steven Price wrote: > > The VMM may not wish to have it's own mapping of guest memory mapped > with PROT_MTE because this causes problems if the VMM has tag checking > enabled (the guest controls the tags in physical RAM and it's unlikely > the tags are correct for the VMM). > > Instead add a new ioctl which allows the VMM to easily read/write the > tags from guest memory, allowing the VMM's mapping to be non-PROT_MTE > while the VMM can still read/write the tags for the purpose of > migration. > > Reviewed-by: Catalin Marinas > Signed-off-by: Steven Price > --- > arch/arm64/include/asm/kvm_host.h | 3 ++ > arch/arm64/include/asm/mte-def.h | 1 + > arch/arm64/include/uapi/asm/kvm.h | 11 +++++ > arch/arm64/kvm/arm.c | 7 +++ > arch/arm64/kvm/guest.c | 82 +++++++++++++++++++++++++++++++ > include/uapi/linux/kvm.h | 1 + > 6 files changed, 105 insertions(+) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 309e36cc1b42..6a2ac4636d42 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -729,6 +729,9 @@ int kvm_arm_vcpu_arch_get_attr(struct kvm_vcpu *vcpu, > int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu, > struct kvm_device_attr *attr); > > +long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm, > + struct kvm_arm_copy_mte_tags *copy_tags); > + > /* Guest/host FPSIMD coordination helpers */ > int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu); > void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu); > diff --git a/arch/arm64/include/asm/mte-def.h b/arch/arm64/include/asm/mte-def.h > index cf241b0f0a42..626d359b396e 100644 > --- a/arch/arm64/include/asm/mte-def.h > +++ b/arch/arm64/include/asm/mte-def.h > @@ -7,6 +7,7 @@ > > #define MTE_GRANULE_SIZE UL(16) > #define MTE_GRANULE_MASK (~(MTE_GRANULE_SIZE - 1)) > +#define MTE_GRANULES_PER_PAGE (PAGE_SIZE / MTE_GRANULE_SIZE) > #define MTE_TAG_SHIFT 56 > #define MTE_TAG_SIZE 4 > #define MTE_TAG_MASK GENMASK((MTE_TAG_SHIFT + (MTE_TAG_SIZE - 1)), MTE_TAG_SHIFT) > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > index 24223adae150..b3edde68bc3e 100644 > --- a/arch/arm64/include/uapi/asm/kvm.h > +++ b/arch/arm64/include/uapi/asm/kvm.h > @@ -184,6 +184,17 @@ struct kvm_vcpu_events { > __u32 reserved[12]; > }; > > +struct kvm_arm_copy_mte_tags { > + __u64 guest_ipa; > + __u64 length; > + void __user *addr; > + __u64 flags; > + __u64 reserved[2]; > +}; > + > +#define KVM_ARM_TAGS_TO_GUEST 0 > +#define KVM_ARM_TAGS_FROM_GUEST 1 > + > /* If you need to interpret the index values, here is the key: */ > #define KVM_REG_ARM_COPROC_MASK 0x000000000FFF0000 > #define KVM_REG_ARM_COPROC_SHIFT 16 > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 28ce26a68f09..511f3716fe33 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -1359,6 +1359,13 @@ long kvm_arch_vm_ioctl(struct file *filp, > > return 0; > } > + case KVM_ARM_MTE_COPY_TAGS: { > + struct kvm_arm_copy_mte_tags copy_tags; > + > + if (copy_from_user(©_tags, argp, sizeof(copy_tags))) > + return -EFAULT; > + return kvm_vm_ioctl_mte_copy_tags(kvm, ©_tags); > + } > default: > return -EINVAL; > } > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c > index 5cb4a1cd5603..4ddb20017b2f 100644 > --- a/arch/arm64/kvm/guest.c > +++ b/arch/arm64/kvm/guest.c > @@ -995,3 +995,85 @@ int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu, > > return ret; > } > + > +long kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm, > + struct kvm_arm_copy_mte_tags *copy_tags) > +{ > + gpa_t guest_ipa = copy_tags->guest_ipa; > + size_t length = copy_tags->length; > + void __user *tags = copy_tags->addr; > + gpa_t gfn; > + bool write = !(copy_tags->flags & KVM_ARM_TAGS_FROM_GUEST); > + int ret = 0; > + > + if (!kvm_has_mte(kvm)) > + return -EINVAL; > + > + if (copy_tags->reserved[0] || copy_tags->reserved[1]) > + return -EINVAL; > + > + if (copy_tags->flags & ~KVM_ARM_TAGS_FROM_GUEST) > + return -EINVAL; > + > + if (length & ~PAGE_MASK || guest_ipa & ~PAGE_MASK) > + return -EINVAL; > + > + gfn = gpa_to_gfn(guest_ipa); > + > + mutex_lock(&kvm->slots_lock); > + > + while (length > 0) { > + kvm_pfn_t pfn = gfn_to_pfn_prot(kvm, gfn, write, NULL); > + void *maddr; > + unsigned long num_tags; > + struct page *page; > + > + if (is_error_noslot_pfn(pfn)) { > + ret = -EFAULT; > + goto out; > + } > + > + page = pfn_to_online_page(pfn); > + if (!page) { > + /* Reject ZONE_DEVICE memory */ > + ret = -EFAULT; > + goto out; > + } > + maddr = page_address(page); > + > + if (!write) { > + if (test_bit(PG_mte_tagged, &page->flags)) > + num_tags = mte_copy_tags_to_user(tags, maddr, > + MTE_GRANULES_PER_PAGE); > + else > + /* No tags in memory, so write zeros */ > + num_tags = MTE_GRANULES_PER_PAGE - > + clear_user(tags, MTE_GRANULES_PER_PAGE); > + kvm_release_pfn_clean(pfn); > + } else { > + num_tags = mte_copy_tags_from_user(maddr, tags, > + MTE_GRANULES_PER_PAGE); > + kvm_release_pfn_dirty(pfn); > + } > + > + if (num_tags != MTE_GRANULES_PER_PAGE) { > + ret = -EFAULT; > + goto out; > + } > + > + /* Set the flag after checking the write completed fully */ > + if (write) > + set_bit(PG_mte_tagged, &page->flags); > + > + gfn++; > + tags += num_tags; > + length -= PAGE_SIZE; > + } > + > +out: > + mutex_unlock(&kvm->slots_lock); > + /* If some data has been copied report the number of bytes copied */ > + if (length != copy_tags->length) > + return copy_tags->length - length; I'm not sure if this is actually an issue, but a couple of comments on the return value if there is an error after a partial copy has been done. If mte_copy_tags_to_user or mte_copy_tags_from_user don't return MTE_GRANULES_PER_PAGE, then the check for num_tags would fail, but some of the tags would have been copied, which wouldn't be reflected in length. That said, on a write the tagged bit wouldn't be set, and on read then the return value would be conservative, but not incorrect. That said, even though it is described that way in the documentation (rather deep in the description though), it might be confusing to return a non-negative value on an error. The other kvm ioctl I could find that does something similar, KVM_S390_GET_IRQ_STATE, seems to always return a -ERROR on error, rather than the number of bytes copied. Cheers, /fuad > + return ret; > +} > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index d4da58ddcad7..da1edd2b4046 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -1429,6 +1429,7 @@ struct kvm_s390_ucas_mapping { > /* Available with KVM_CAP_PMU_EVENT_FILTER */ > #define KVM_SET_PMU_EVENT_FILTER _IOW(KVMIO, 0xb2, struct kvm_pmu_event_filter) > #define KVM_PPC_SVM_OFF _IO(KVMIO, 0xb3) > +#define KVM_ARM_MTE_COPY_TAGS _IOR(KVMIO, 0xb4, struct kvm_arm_copy_mte_tags) > > /* ioctl for vm fd */ > #define KVM_CREATE_DEVICE _IOWR(KVMIO, 0xe0, struct kvm_create_device) > -- > 2.20.1 > > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel