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=-10.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 71469C07E94 for ; Fri, 4 Jun 2021 13:10:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 53C0761242 for ; Fri, 4 Jun 2021 13:10:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230375AbhFDNLn (ORCPT ); Fri, 4 Jun 2021 09:11:43 -0400 Received: from foss.arm.com ([217.140.110.172]:38716 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230041AbhFDNLl (ORCPT ); Fri, 4 Jun 2021 09:11:41 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 72B462B; Fri, 4 Jun 2021 06:09:55 -0700 (PDT) Received: from [192.168.1.179] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id BA0CD3F774; Fri, 4 Jun 2021 06:09:52 -0700 (PDT) Subject: Re: [PATCH v13 7/8] KVM: arm64: ioctl to fetch/store tags in a guest To: Catalin Marinas Cc: Marc Zyngier , Will Deacon , James Morse , Julien Thierry , Suzuki K Poulose , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Dave Martin , Mark Rutland , Thomas Gleixner , qemu-devel@nongnu.org, Juan Quintela , "Dr. David Alan Gilbert" , Richard Henderson , Peter Maydell , Haibo Xu , Andrew Jones References: <20210524104513.13258-1-steven.price@arm.com> <20210524104513.13258-8-steven.price@arm.com> <20210603171336.GH20338@arm.com> <02c7682e-5fb6-29eb-9105-02e3521756a2@arm.com> <20210604114233.GE31173@arm.com> From: Steven Price Message-ID: Date: Fri, 4 Jun 2021 14:09:50 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210604114233.GE31173@arm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/06/2021 12:42, Catalin Marinas wrote: > On Fri, Jun 04, 2021 at 12:15:56PM +0100, Steven Price wrote: >> On 03/06/2021 18:13, Catalin Marinas wrote: >>> On Mon, May 24, 2021 at 11:45:12AM +0100, Steven Price wrote: >>>> 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 e89a5e275e25..baa33359e477 100644 >>>> --- a/arch/arm64/kvm/arm.c >>>> +++ b/arch/arm64/kvm/arm.c >>>> @@ -1345,6 +1345,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); >>>> + } >>> >>> I wonder whether we need an update of the user structure following a >>> fault, like how much was copied etc. In case of an error, some tags were >>> copied and the VMM may want to skip the page before continuing. But here >>> there's no such information provided. >>> >>> On the ptrace interface, we return 0 on the syscall if any bytes were >>> copied and update iov_len to such number. Maybe you want to still return >>> an error here but updating copy_tags.length would be nice (and, of >>> course, a copy_to_user() back). >> >> Good idea - as you suggest I'll make it update length with the number of >> bytes not processed. Although in general I think we're expecting the VMM >> to know where the memory is so this is more of a programming error - but >> could still be useful for debugging. > > Or update it to the number of bytes copied to be consistent with > ptrace()'s iov.len. On success, the structure is effectively left > unchanged. I was avoiding that because it confuses the error code when the initial copy_from_user() fails. In that case the structure is clearly unchanged, so you can only tell from a -EFAULT return that nothing happened. By returning the number of bytes left you can return an error code along with the information that the copy only half completed. It also seems cleaner to leave the structure unchanged if e.g. the flags or reserved fields are invalid rather than having to set length=0 to signal that nothing was done. Although I do feel like arguing whether to use a ptrace() interface or a copy_{to,from}_user() interface is somewhat ridiculous considering neither are exactly considered good. Rather than changing the structure we could return either an error code (if nothing was copied) or the number of bytes left. That way ioctl()==0 means complete success, >0 means partial success and <0 means complete failure and provides a detailed error code. The ioctl() can be repeated (with adjusted pointers) if it returns >0 and a detailed error is needed. Steve 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=-10.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 580F4C07E94 for ; Fri, 4 Jun 2021 13:12:01 +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 0C25E61242 for ; Fri, 4 Jun 2021 13:12:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0C25E61242 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:55074 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lp9cG-0005dY-6v for qemu-devel@archiver.kernel.org; Fri, 04 Jun 2021 09:12:00 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:58006) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lp9aO-00031T-Hv for qemu-devel@nongnu.org; Fri, 04 Jun 2021 09:10:04 -0400 Received: from foss.arm.com ([217.140.110.172]:59606) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lp9aH-0006ax-Ka for qemu-devel@nongnu.org; Fri, 04 Jun 2021 09:10:03 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 72B462B; Fri, 4 Jun 2021 06:09:55 -0700 (PDT) Received: from [192.168.1.179] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id BA0CD3F774; Fri, 4 Jun 2021 06:09:52 -0700 (PDT) Subject: Re: [PATCH v13 7/8] KVM: arm64: ioctl to fetch/store tags in a guest To: Catalin Marinas References: <20210524104513.13258-1-steven.price@arm.com> <20210524104513.13258-8-steven.price@arm.com> <20210603171336.GH20338@arm.com> <02c7682e-5fb6-29eb-9105-02e3521756a2@arm.com> <20210604114233.GE31173@arm.com> From: Steven Price Message-ID: Date: Fri, 4 Jun 2021 14:09:50 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210604114233.GE31173@arm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=217.140.110.172; envelope-from=steven.price@arm.com; helo=foss.arm.com X-Spam_score_int: -47 X-Spam_score: -4.8 X-Spam_bar: ---- X-Spam_report: (-4.8 / 5.0 requ) BAYES_00=-1.9, NICE_REPLY_A=-0.59, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham 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: , Cc: Mark Rutland , Peter Maydell , "Dr. David Alan Gilbert" , Andrew Jones , Haibo Xu , Suzuki K Poulose , qemu-devel@nongnu.org, Marc Zyngier , Juan Quintela , Richard Henderson , linux-kernel@vger.kernel.org, Dave Martin , James Morse , linux-arm-kernel@lists.infradead.org, Thomas Gleixner , Will Deacon , kvmarm@lists.cs.columbia.edu, Julien Thierry Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 04/06/2021 12:42, Catalin Marinas wrote: > On Fri, Jun 04, 2021 at 12:15:56PM +0100, Steven Price wrote: >> On 03/06/2021 18:13, Catalin Marinas wrote: >>> On Mon, May 24, 2021 at 11:45:12AM +0100, Steven Price wrote: >>>> 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 e89a5e275e25..baa33359e477 100644 >>>> --- a/arch/arm64/kvm/arm.c >>>> +++ b/arch/arm64/kvm/arm.c >>>> @@ -1345,6 +1345,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); >>>> + } >>> >>> I wonder whether we need an update of the user structure following a >>> fault, like how much was copied etc. In case of an error, some tags were >>> copied and the VMM may want to skip the page before continuing. But here >>> there's no such information provided. >>> >>> On the ptrace interface, we return 0 on the syscall if any bytes were >>> copied and update iov_len to such number. Maybe you want to still return >>> an error here but updating copy_tags.length would be nice (and, of >>> course, a copy_to_user() back). >> >> Good idea - as you suggest I'll make it update length with the number of >> bytes not processed. Although in general I think we're expecting the VMM >> to know where the memory is so this is more of a programming error - but >> could still be useful for debugging. > > Or update it to the number of bytes copied to be consistent with > ptrace()'s iov.len. On success, the structure is effectively left > unchanged. I was avoiding that because it confuses the error code when the initial copy_from_user() fails. In that case the structure is clearly unchanged, so you can only tell from a -EFAULT return that nothing happened. By returning the number of bytes left you can return an error code along with the information that the copy only half completed. It also seems cleaner to leave the structure unchanged if e.g. the flags or reserved fields are invalid rather than having to set length=0 to signal that nothing was done. Although I do feel like arguing whether to use a ptrace() interface or a copy_{to,from}_user() interface is somewhat ridiculous considering neither are exactly considered good. Rather than changing the structure we could return either an error code (if nothing was copied) or the number of bytes left. That way ioctl()==0 means complete success, >0 means partial success and <0 means complete failure and provides a detailed error code. The ioctl() can be repeated (with adjusted pointers) if it returns >0 and a detailed error is needed. Steve 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=-10.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 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 0F686C4708F for ; Fri, 4 Jun 2021 13:10:09 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 86EE361242 for ; Fri, 4 Jun 2021 13:10:08 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 86EE361242 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.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 E20AB4B0C2; Fri, 4 Jun 2021 09:10:07 -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 Se472b2stogU; Fri, 4 Jun 2021 09:10:03 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 3FE864B0DC; Fri, 4 Jun 2021 09:10:03 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 67C684B0B6 for ; Fri, 4 Jun 2021 09:10: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 r+3vaRSkb5T7 for ; Fri, 4 Jun 2021 09:09:56 -0400 (EDT) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 5DBD04081C for ; Fri, 4 Jun 2021 09:09:56 -0400 (EDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 72B462B; Fri, 4 Jun 2021 06:09:55 -0700 (PDT) Received: from [192.168.1.179] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id BA0CD3F774; Fri, 4 Jun 2021 06:09:52 -0700 (PDT) Subject: Re: [PATCH v13 7/8] KVM: arm64: ioctl to fetch/store tags in a guest To: Catalin Marinas References: <20210524104513.13258-1-steven.price@arm.com> <20210524104513.13258-8-steven.price@arm.com> <20210603171336.GH20338@arm.com> <02c7682e-5fb6-29eb-9105-02e3521756a2@arm.com> <20210604114233.GE31173@arm.com> From: Steven Price Message-ID: Date: Fri, 4 Jun 2021 14:09:50 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210604114233.GE31173@arm.com> Content-Language: en-GB Cc: "Dr. David Alan Gilbert" , qemu-devel@nongnu.org, Marc Zyngier , Juan Quintela , Richard Henderson , linux-kernel@vger.kernel.org, Dave Martin , linux-arm-kernel@lists.infradead.org, Thomas Gleixner , Will Deacon , kvmarm@lists.cs.columbia.edu 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 On 04/06/2021 12:42, Catalin Marinas wrote: > On Fri, Jun 04, 2021 at 12:15:56PM +0100, Steven Price wrote: >> On 03/06/2021 18:13, Catalin Marinas wrote: >>> On Mon, May 24, 2021 at 11:45:12AM +0100, Steven Price wrote: >>>> 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 e89a5e275e25..baa33359e477 100644 >>>> --- a/arch/arm64/kvm/arm.c >>>> +++ b/arch/arm64/kvm/arm.c >>>> @@ -1345,6 +1345,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); >>>> + } >>> >>> I wonder whether we need an update of the user structure following a >>> fault, like how much was copied etc. In case of an error, some tags were >>> copied and the VMM may want to skip the page before continuing. But here >>> there's no such information provided. >>> >>> On the ptrace interface, we return 0 on the syscall if any bytes were >>> copied and update iov_len to such number. Maybe you want to still return >>> an error here but updating copy_tags.length would be nice (and, of >>> course, a copy_to_user() back). >> >> Good idea - as you suggest I'll make it update length with the number of >> bytes not processed. Although in general I think we're expecting the VMM >> to know where the memory is so this is more of a programming error - but >> could still be useful for debugging. > > Or update it to the number of bytes copied to be consistent with > ptrace()'s iov.len. On success, the structure is effectively left > unchanged. I was avoiding that because it confuses the error code when the initial copy_from_user() fails. In that case the structure is clearly unchanged, so you can only tell from a -EFAULT return that nothing happened. By returning the number of bytes left you can return an error code along with the information that the copy only half completed. It also seems cleaner to leave the structure unchanged if e.g. the flags or reserved fields are invalid rather than having to set length=0 to signal that nothing was done. Although I do feel like arguing whether to use a ptrace() interface or a copy_{to,from}_user() interface is somewhat ridiculous considering neither are exactly considered good. Rather than changing the structure we could return either an error code (if nothing was copied) or the number of bytes left. That way ioctl()==0 means complete success, >0 means partial success and <0 means complete failure and provides a detailed error code. The ioctl() can be repeated (with adjusted pointers) if it returns >0 and a detailed error is needed. Steve _______________________________________________ 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=-11.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 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 68034C07E94 for ; Fri, 4 Jun 2021 13:16:50 +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 3C984613E7 for ; Fri, 4 Jun 2021 13:16:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3C984613E7 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.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:In-Reply-To:MIME-Version:Date: Message-ID:From:References:Cc:To:Subject:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=zwqupdPqdHuOnmzLoYpxfABgr8Oa71zsH3lZf3WBx+Q=; b=T3l1yueHtm9sBc4c4MH0pfxMQk 87SqxrtnZwoJE6JOrdzYX6Pw3oJqqhNPsh9F1Cmp9lAAmNP38SWh6WB+6JBRuh+uXes7PMIYXc5xF 3TTOCHbdljQp/NV1kfEip04wEC+anltR88iqhAsyVH6Wpt6yCqZNXzx2frNhQErMRAuuSNwtvN6T4 Ptba2ObknInlsjnfkDRXsfI/n2lasFdxL4jWGRPm/x24JAYlT1ZFTJH/IsYiT4AYMWeJixLdxT17n sqwau3Faf3B6rlG7DNcPoZpFFVl700UQ/w1g78qIW33rixSzDV5nFvMI1AHKsA+X9AQRvROAYPluL 1xnVlWBw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lp9f1-00DfiE-Cf; Fri, 04 Jun 2021 13:14:52 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lp9aK-00DdZx-SG for linux-arm-kernel@lists.infradead.org; Fri, 04 Jun 2021 13:10:07 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 72B462B; Fri, 4 Jun 2021 06:09:55 -0700 (PDT) Received: from [192.168.1.179] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id BA0CD3F774; Fri, 4 Jun 2021 06:09:52 -0700 (PDT) Subject: Re: [PATCH v13 7/8] KVM: arm64: ioctl to fetch/store tags in a guest To: Catalin Marinas Cc: Marc Zyngier , Will Deacon , James Morse , Julien Thierry , Suzuki K Poulose , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Dave Martin , Mark Rutland , Thomas Gleixner , qemu-devel@nongnu.org, Juan Quintela , "Dr. David Alan Gilbert" , Richard Henderson , Peter Maydell , Haibo Xu , Andrew Jones References: <20210524104513.13258-1-steven.price@arm.com> <20210524104513.13258-8-steven.price@arm.com> <20210603171336.GH20338@arm.com> <02c7682e-5fb6-29eb-9105-02e3521756a2@arm.com> <20210604114233.GE31173@arm.com> From: Steven Price Message-ID: Date: Fri, 4 Jun 2021 14:09:50 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210604114233.GE31173@arm.com> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210604_061001_071147_F9496686 X-CRM114-Status: GOOD ( 22.91 ) 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 On 04/06/2021 12:42, Catalin Marinas wrote: > On Fri, Jun 04, 2021 at 12:15:56PM +0100, Steven Price wrote: >> On 03/06/2021 18:13, Catalin Marinas wrote: >>> On Mon, May 24, 2021 at 11:45:12AM +0100, Steven Price wrote: >>>> 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 e89a5e275e25..baa33359e477 100644 >>>> --- a/arch/arm64/kvm/arm.c >>>> +++ b/arch/arm64/kvm/arm.c >>>> @@ -1345,6 +1345,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); >>>> + } >>> >>> I wonder whether we need an update of the user structure following a >>> fault, like how much was copied etc. In case of an error, some tags were >>> copied and the VMM may want to skip the page before continuing. But here >>> there's no such information provided. >>> >>> On the ptrace interface, we return 0 on the syscall if any bytes were >>> copied and update iov_len to such number. Maybe you want to still return >>> an error here but updating copy_tags.length would be nice (and, of >>> course, a copy_to_user() back). >> >> Good idea - as you suggest I'll make it update length with the number of >> bytes not processed. Although in general I think we're expecting the VMM >> to know where the memory is so this is more of a programming error - but >> could still be useful for debugging. > > Or update it to the number of bytes copied to be consistent with > ptrace()'s iov.len. On success, the structure is effectively left > unchanged. I was avoiding that because it confuses the error code when the initial copy_from_user() fails. In that case the structure is clearly unchanged, so you can only tell from a -EFAULT return that nothing happened. By returning the number of bytes left you can return an error code along with the information that the copy only half completed. It also seems cleaner to leave the structure unchanged if e.g. the flags or reserved fields are invalid rather than having to set length=0 to signal that nothing was done. Although I do feel like arguing whether to use a ptrace() interface or a copy_{to,from}_user() interface is somewhat ridiculous considering neither are exactly considered good. Rather than changing the structure we could return either an error code (if nothing was copied) or the number of bytes left. That way ioctl()==0 means complete success, >0 means partial success and <0 means complete failure and provides a detailed error code. The ioctl() can be repeated (with adjusted pointers) if it returns >0 and a detailed error is needed. Steve _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel