| Message ID | 20200925093607.3051-1-steven.price@arm.com |
|---|---|
| Headers | show |
| Series | MTE support for KVM guest | expand |
On Fri, Sep 25, 2020 at 10:36:05AM +0100, Steven Price wrote: > Version 3 of adding MTE support for KVM guests. See the previous (v2) > posting for background: > > https://lore.kernel.org/r/20200904160018.29481-1-steven.price%40arm.com > > These patches add support to KVM to enable MTE within a guest. They are > based on Catalin's v9 MTE user-space support series[1] (currently in > next). > > Changes since v2: > > * MTE is no longer a VCPU feature, instead it is a VM cap. > > * Being a VM cap means easier probing (check for KVM_CAP_ARM_MTE). > > * The cap must be set before any VCPUs are created, preventing any > shenanigans where MTE is enabled for the guest after memory accesses > have been performed. > > [1] https://lore.kernel.org/r/20200904103029.32083-1-catalin.marinas@arm.com > > Steven Price (2): > arm64: kvm: Save/restore MTE registers > arm64: kvm: Introduce MTE VCPU feature > > arch/arm64/include/asm/kvm_emulate.h | 3 +++ > arch/arm64/include/asm/kvm_host.h | 7 +++++++ > arch/arm64/include/asm/sysreg.h | 3 ++- > arch/arm64/kvm/arm.c | 9 +++++++++ > arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 14 ++++++++++++++ > arch/arm64/kvm/mmu.c | 15 +++++++++++++++ > arch/arm64/kvm/sys_regs.c | 20 +++++++++++++++----- > include/uapi/linux/kvm.h | 1 + > 8 files changed, 66 insertions(+), 6 deletions(-) > > -- > 2.20.1 > > Hi Steven, These patches look fine to me, but I'd prefer we have a working implementation in QEMU before we get too excited about the KVM bits. kvmtool isn't sufficient since it doesn't support migration (at least afaik). In the past we've implemented features in KVM that look fine, but then issues have been discovered when trying to enable them from QEMU, where we also support migration. This feature looks like there's risk of issues with the userspace side. Although these two patches would probably stay the same, even if userspace requires more support. Thanks, drew
On 02/10/2020 15:36, Andrew Jones wrote: > On Fri, Sep 25, 2020 at 10:36:05AM +0100, Steven Price wrote: >> Version 3 of adding MTE support for KVM guests. See the previous (v2) >> posting for background: >> >> https://lore.kernel.org/r/20200904160018.29481-1-steven.price%40arm.com >> >> These patches add support to KVM to enable MTE within a guest. They are >> based on Catalin's v9 MTE user-space support series[1] (currently in >> next). >> >> Changes since v2: >> >> * MTE is no longer a VCPU feature, instead it is a VM cap. >> >> * Being a VM cap means easier probing (check for KVM_CAP_ARM_MTE). >> >> * The cap must be set before any VCPUs are created, preventing any >> shenanigans where MTE is enabled for the guest after memory accesses >> have been performed. >> >> [1] https://lore.kernel.org/r/20200904103029.32083-1-catalin.marinas@arm.com >> >> Steven Price (2): >> arm64: kvm: Save/restore MTE registers >> arm64: kvm: Introduce MTE VCPU feature >> >> arch/arm64/include/asm/kvm_emulate.h | 3 +++ >> arch/arm64/include/asm/kvm_host.h | 7 +++++++ >> arch/arm64/include/asm/sysreg.h | 3 ++- >> arch/arm64/kvm/arm.c | 9 +++++++++ >> arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 14 ++++++++++++++ >> arch/arm64/kvm/mmu.c | 15 +++++++++++++++ >> arch/arm64/kvm/sys_regs.c | 20 +++++++++++++++----- >> include/uapi/linux/kvm.h | 1 + >> 8 files changed, 66 insertions(+), 6 deletions(-) >> >> -- >> 2.20.1 >> >> > > Hi Steven, > > These patches look fine to me, but I'd prefer we have a working > implementation in QEMU before we get too excited about the KVM > bits. kvmtool isn't sufficient since it doesn't support migration > (at least afaik). In the past we've implemented features in KVM > that look fine, but then issues have been discovered when trying > to enable them from QEMU, where we also support migration. This > feature looks like there's risk of issues with the userspace side. > Although these two patches would probably stay the same, even if > userspace requires more support. I agree kvmtool isn't a great test because it doesn't support migration. The support in this series is just the basic support for MTE in a guest and we'd need to wait for the QEMU implementation before deciding whether we need any extra support (e.g. kernel interfaces for reading/writing tags as discussed before). However, I don't think there's much danger of the support in this series changing - so extra support can be added when/if it's needed, but I don't think we need to block these series on that - QEMU can just probe for whatever additional support it needs before enabling MTE in a guest. I plan to rebase/repost after -rc1 when the user space support has been merged. Steve
On Fri, Oct 02, 2020 at 04:38:11PM +0100, Steven Price wrote: > On 02/10/2020 15:36, Andrew Jones wrote: > > On Fri, Sep 25, 2020 at 10:36:05AM +0100, Steven Price wrote: > > > Version 3 of adding MTE support for KVM guests. See the previous (v2) > > > posting for background: > > > > > > https://lore.kernel.org/r/20200904160018.29481-1-steven.price%40arm.com > > > > > > These patches add support to KVM to enable MTE within a guest. They are > > > based on Catalin's v9 MTE user-space support series[1] (currently in > > > next). > > > > > > Changes since v2: > > > > > > * MTE is no longer a VCPU feature, instead it is a VM cap. > > > > > > * Being a VM cap means easier probing (check for KVM_CAP_ARM_MTE). > > > > > > * The cap must be set before any VCPUs are created, preventing any > > > shenanigans where MTE is enabled for the guest after memory accesses > > > have been performed. > > > > > > [1] https://lore.kernel.org/r/20200904103029.32083-1-catalin.marinas@arm.com > > > > > > Steven Price (2): > > > arm64: kvm: Save/restore MTE registers > > > arm64: kvm: Introduce MTE VCPU feature > > > > > > arch/arm64/include/asm/kvm_emulate.h | 3 +++ > > > arch/arm64/include/asm/kvm_host.h | 7 +++++++ > > > arch/arm64/include/asm/sysreg.h | 3 ++- > > > arch/arm64/kvm/arm.c | 9 +++++++++ > > > arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 14 ++++++++++++++ > > > arch/arm64/kvm/mmu.c | 15 +++++++++++++++ > > > arch/arm64/kvm/sys_regs.c | 20 +++++++++++++++----- > > > include/uapi/linux/kvm.h | 1 + > > > 8 files changed, 66 insertions(+), 6 deletions(-) > > > > > > -- > > > 2.20.1 > > > > > > > > > > Hi Steven, > > > > These patches look fine to me, but I'd prefer we have a working > > implementation in QEMU before we get too excited about the KVM > > bits. kvmtool isn't sufficient since it doesn't support migration > > (at least afaik). In the past we've implemented features in KVM > > that look fine, but then issues have been discovered when trying > > to enable them from QEMU, where we also support migration. This > > feature looks like there's risk of issues with the userspace side. > > Although these two patches would probably stay the same, even if > > userspace requires more support. > > I agree kvmtool isn't a great test because it doesn't support migration. The > support in this series is just the basic support for MTE in a guest and we'd > need to wait for the QEMU implementation before deciding whether we need any > extra support (e.g. kernel interfaces for reading/writing tags as discussed > before). > > However, I don't think there's much danger of the support in this series > changing - so extra support can be added when/if it's needed, but I don't > think we need to block these series on that - QEMU can just probe for > whatever additional support it needs before enabling MTE in a guest. I plan > to rebase/repost after -rc1 when the user space support has been merged. > Fair enough, but it feels like we'll be merging half a feature, leaving the other half for somebody else to pick up later. Thanks, drew