From: Cornelia Huck <cohuck@redhat.com> To: Gavin Shan <gshan@redhat.com>, Thomas Huth <thuth@redhat.com>, kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>, Sean Christopherson <seanjc@google.com>, Steven Price <steven.price@arm.com> Cc: kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, kvm-riscv@lists.infradead.org, Marc Zyngier <maz@kernel.org>, James Morse <james.morse@arm.com>, Suzuki K Poulose <suzuki.poulose@arm.com>, Oliver Upton <oliver.upton@linux.dev>, Zenghui Yu <yuzenghui@huawei.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Janosch Frank <frankja@linux.ibm.com>, Claudio Imbrenda <imbrenda@linux.ibm.com>, David Hildenbrand <david@redhat.com>, linuxppc-dev@lists.ozlabs.org, Eric Auger <eric.auger@redhat.com> Subject: Re: [PATCH 6/7] KVM: arm64: Change return type of kvm_vm_ioctl_mte_copy_tags() to "int" Date: Wed, 08 Feb 2023 09:49:16 +0100 [thread overview] Message-ID: <87zg9oleyb.fsf@redhat.com> (raw) In-Reply-To: <d0b3a1e7-0864-f169-cdea-60ad95951b3f@redhat.com> On Wed, Feb 08 2023, Gavin Shan <gshan@redhat.com> wrote: > On 2/7/23 9:09 PM, Thomas Huth wrote: >> Oh, drat, I thought I had checked all return statements ... this must have fallen through the cracks, sorry! >> >> Anyway, this is already a problem now: The function is called from kvm_arch_vm_ioctl() (which still returns a long), which in turn is called from kvm_vm_ioctl() in virt/kvm/kvm_main.c. And that functions stores the return value in an "int r" variable. So the upper bits are already lost there. >> >> Also, how is this supposed to work from user space? The normal "ioctl()" libc function just returns an "int" ? Is this ioctl already used in a userspace application somewhere? ... at least in QEMU, I didn't spot it yet... >> We will need it in QEMU to implement migration with MTE (the current proposal simply adds a migration blocker when MTE is enabled, as there are various other things that need to be figured out for this to work.) But maybe other VMMs already use it (and have been lucky because they always dealt with shorter lengths?) > > The ioctl command KVM_ARM_MTE_COPY_TAGS was merged recently and not used > by QEMU yet. I think struct kvm_arm_copy_mte_tags::length needs to be > '__u32' instead of '__u64' in order to standardize the return value. > Something like below. Documentation/virt/kvm/api.rst::section-4.130 > needs update accordingly. > > struct kvm_arm_copy_mte_tags { > __u64 guest_ipa; > __u32 pad; > __u32 length; > void __user *addr; > __u64 flags; > __u64 reserved[2]; > }; Can we do this in a more compatible way, as we are dealing with an API? Like returning -EINVAL if length is too big?
WARNING: multiple messages have this Message-ID (diff)
From: Cornelia Huck <cohuck@redhat.com> To: Gavin Shan <gshan@redhat.com>, Thomas Huth <thuth@redhat.com>, kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>, Sean Christopherson <seanjc@google.com>, Steven Price <steven.price@arm.com> Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>, Janosch Frank <frankja@linux.ibm.com>, Suzuki K Poulose <suzuki.poulose@arm.com>, Marc Zyngier <maz@kernel.org>, David Hildenbrand <david@redhat.com>, linux-kernel@vger.kernel.org, Oliver Upton <oliver.upton@linux.dev>, Zenghui Yu <yuzenghui@huawei.com>, James Morse <james.morse@arm.com>, kvm-riscv@lists.infradead.org, kvmarm@lists.linux.dev, Christian Borntraeger <borntraeger@linux.ibm.com>, linuxppc-dev@lists.ozlabs.org, Eric Auger <eric.auger@redhat.com> Subject: Re: [PATCH 6/7] KVM: arm64: Change return type of kvm_vm_ioctl_mte_copy_tags() to "int" Date: Wed, 08 Feb 2023 09:49:16 +0100 [thread overview] Message-ID: <87zg9oleyb.fsf@redhat.com> (raw) In-Reply-To: <d0b3a1e7-0864-f169-cdea-60ad95951b3f@redhat.com> On Wed, Feb 08 2023, Gavin Shan <gshan@redhat.com> wrote: > On 2/7/23 9:09 PM, Thomas Huth wrote: >> Oh, drat, I thought I had checked all return statements ... this must have fallen through the cracks, sorry! >> >> Anyway, this is already a problem now: The function is called from kvm_arch_vm_ioctl() (which still returns a long), which in turn is called from kvm_vm_ioctl() in virt/kvm/kvm_main.c. And that functions stores the return value in an "int r" variable. So the upper bits are already lost there. >> >> Also, how is this supposed to work from user space? The normal "ioctl()" libc function just returns an "int" ? Is this ioctl already used in a userspace application somewhere? ... at least in QEMU, I didn't spot it yet... >> We will need it in QEMU to implement migration with MTE (the current proposal simply adds a migration blocker when MTE is enabled, as there are various other things that need to be figured out for this to work.) But maybe other VMMs already use it (and have been lucky because they always dealt with shorter lengths?) > > The ioctl command KVM_ARM_MTE_COPY_TAGS was merged recently and not used > by QEMU yet. I think struct kvm_arm_copy_mte_tags::length needs to be > '__u32' instead of '__u64' in order to standardize the return value. > Something like below. Documentation/virt/kvm/api.rst::section-4.130 > needs update accordingly. > > struct kvm_arm_copy_mte_tags { > __u64 guest_ipa; > __u32 pad; > __u32 length; > void __user *addr; > __u64 flags; > __u64 reserved[2]; > }; Can we do this in a more compatible way, as we are dealing with an API? Like returning -EINVAL if length is too big?
next prev parent reply other threads:[~2023-02-08 8:50 UTC|newest] Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-02-03 9:42 [PATCH 0/7] KVM: Standardize on "int" return types instead of "long" Thomas Huth 2023-02-03 9:42 ` Thomas Huth 2023-02-03 9:42 ` [PATCH 1/7] KVM: Standardize on "int" return types instead of "long" in kvm_main.c Thomas Huth 2023-02-03 9:42 ` Thomas Huth 2023-02-03 9:42 ` [PATCH 2/7] KVM: x86: Improve return type handling in kvm_vm_ioctl_get_nr_mmu_pages() Thomas Huth 2023-02-03 9:42 ` Thomas Huth 2023-02-03 17:48 ` Sean Christopherson 2023-02-03 17:48 ` Sean Christopherson 2023-02-07 9:26 ` Thomas Huth 2023-02-07 9:26 ` Thomas Huth 2023-02-07 16:25 ` Sean Christopherson 2023-02-07 16:25 ` Sean Christopherson 2023-02-03 9:42 ` [PATCH 3/7] KVM: Move KVM_GET_NR_MMU_PAGES into the deprecation section Thomas Huth 2023-02-03 9:42 ` Thomas Huth 2023-02-03 10:16 ` Nicholas Piggin 2023-02-03 10:16 ` Nicholas Piggin 2023-02-03 10:54 ` Thomas Huth 2023-02-03 10:54 ` Thomas Huth 2023-02-03 9:42 ` [PATCH 4/7] KVM: PPC: Standardize on "int" return types in the powerpc KVM code Thomas Huth 2023-02-03 9:42 ` Thomas Huth 2023-02-03 10:21 ` Nicholas Piggin 2023-02-03 10:21 ` Nicholas Piggin 2023-02-03 9:42 ` [PATCH 5/7] KVM: s390: Use "int" as return type for kvm_s390_get/set_skeys() Thomas Huth 2023-02-03 9:42 ` Thomas Huth 2023-02-07 15:36 ` Claudio Imbrenda 2023-02-07 15:36 ` Claudio Imbrenda 2023-02-03 9:42 ` [PATCH 6/7] KVM: arm64: Change return type of kvm_vm_ioctl_mte_copy_tags() to "int" Thomas Huth 2023-02-03 9:42 ` Thomas Huth 2023-02-07 0:09 ` Gavin Shan 2023-02-07 0:09 ` Gavin Shan 2023-02-07 10:09 ` Thomas Huth 2023-02-07 10:09 ` Thomas Huth 2023-02-07 22:16 ` Gavin Shan 2023-02-07 22:16 ` Gavin Shan 2023-02-08 8:49 ` Cornelia Huck [this message] 2023-02-08 8:49 ` Cornelia Huck 2023-02-08 11:51 ` Steven Price 2023-02-08 11:51 ` Steven Price 2023-02-08 12:16 ` Thomas Huth 2023-02-08 12:16 ` Thomas Huth 2023-02-03 9:42 ` [PATCH 7/7] KVM: Change return type of kvm_arch_vm_ioctl() " Thomas Huth 2023-02-03 9:42 ` Thomas Huth 2023-02-08 17:35 ` Claudio Imbrenda 2023-02-08 17:35 ` Claudio Imbrenda
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=87zg9oleyb.fsf@redhat.com \ --to=cohuck@redhat.com \ --cc=borntraeger@linux.ibm.com \ --cc=david@redhat.com \ --cc=eric.auger@redhat.com \ --cc=frankja@linux.ibm.com \ --cc=gshan@redhat.com \ --cc=imbrenda@linux.ibm.com \ --cc=james.morse@arm.com \ --cc=kvm-riscv@lists.infradead.org \ --cc=kvm@vger.kernel.org \ --cc=kvmarm@lists.linux.dev \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=maz@kernel.org \ --cc=oliver.upton@linux.dev \ --cc=pbonzini@redhat.com \ --cc=seanjc@google.com \ --cc=steven.price@arm.com \ --cc=suzuki.poulose@arm.com \ --cc=thuth@redhat.com \ --cc=yuzenghui@huawei.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.