From: David Brazdil <dbrazdil@google.com>
To: Quentin Perret <qperret@google.com>
Cc: Marc Zyngier <maz@kernel.org>, James Morse <james.morse@arm.com>,
Alexandru Elisei <alexandru.elisei@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>, Fuad Tabba <tabba@google.com>,
Andrew Walbran <qwandor@google.com>,
linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org,
kernel-team@android.com
Subject: Re: [PATCH v2 07/15] KVM: arm64: Introduce kvm_share_hyp()
Date: Thu, 21 Oct 2021 10:07:47 +0000 [thread overview]
Message-ID: <YXE7842op3n8+JXv@google.com> (raw)
In-Reply-To: <20211019121304.2732332-8-qperret@google.com>
Hi Quentin,
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 0019b2309f70..0cc4b295e525 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -299,6 +299,17 @@ static int pkvm_share_hyp(phys_addr_t start, phys_addr_t end)
> return 0;
> }
>
> +int kvm_share_hyp(void *from, void *to)
> +{
> + if (is_kernel_in_hyp_mode())
> + return 0;
> +
> + if (kvm_host_owns_hyp_mappings())
> + return create_hyp_mappings(from, to, PAGE_HYP);
> +
> + return pkvm_share_hyp(kvm_kaddr_to_phys(from), kvm_kaddr_to_phys(to));
We should be careful about vmalloc memory here. kvm_kaddr_to_phys will
happily return the physical address but the range is not guaranteed to
be physically contiguous. It doesn't look like this series ever shares
vmalloc memory, but we should make it harder for users of this function
to shoot themselves in the foot.
One option would be to turn this into a loop and call pkvm_share_hyp on
each physical page. But since the hypervisor has no means of making
those pages virtually contigous anyway, probably not the right approach.
We could make it possible to share vmalloc buffer that fit into a page
and reject others. There we still need to be careful about the upper
bound because the way it's written now, 'to' can be treated as
exclusive and 'kvm_kaddr_to_phys(to)' would not always return the page
after 'kvm_kaddr_to_phys(from)'.
-David
WARNING: multiple messages have this Message-ID (diff)
From: David Brazdil <dbrazdil@google.com>
To: Quentin Perret <qperret@google.com>
Cc: kernel-team@android.com, Andrew Walbran <qwandor@google.com>,
Marc Zyngier <maz@kernel.org>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>,
kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH v2 07/15] KVM: arm64: Introduce kvm_share_hyp()
Date: Thu, 21 Oct 2021 10:07:47 +0000 [thread overview]
Message-ID: <YXE7842op3n8+JXv@google.com> (raw)
In-Reply-To: <20211019121304.2732332-8-qperret@google.com>
Hi Quentin,
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 0019b2309f70..0cc4b295e525 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -299,6 +299,17 @@ static int pkvm_share_hyp(phys_addr_t start, phys_addr_t end)
> return 0;
> }
>
> +int kvm_share_hyp(void *from, void *to)
> +{
> + if (is_kernel_in_hyp_mode())
> + return 0;
> +
> + if (kvm_host_owns_hyp_mappings())
> + return create_hyp_mappings(from, to, PAGE_HYP);
> +
> + return pkvm_share_hyp(kvm_kaddr_to_phys(from), kvm_kaddr_to_phys(to));
We should be careful about vmalloc memory here. kvm_kaddr_to_phys will
happily return the physical address but the range is not guaranteed to
be physically contiguous. It doesn't look like this series ever shares
vmalloc memory, but we should make it harder for users of this function
to shoot themselves in the foot.
One option would be to turn this into a loop and call pkvm_share_hyp on
each physical page. But since the hypervisor has no means of making
those pages virtually contigous anyway, probably not the right approach.
We could make it possible to share vmalloc buffer that fit into a page
and reject others. There we still need to be careful about the upper
bound because the way it's written now, 'to' can be treated as
exclusive and 'kvm_kaddr_to_phys(to)' would not always return the page
after 'kvm_kaddr_to_phys(from)'.
-David
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
WARNING: multiple messages have this Message-ID (diff)
From: David Brazdil <dbrazdil@google.com>
To: Quentin Perret <qperret@google.com>
Cc: Marc Zyngier <maz@kernel.org>, James Morse <james.morse@arm.com>,
Alexandru Elisei <alexandru.elisei@arm.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Will Deacon <will@kernel.org>, Fuad Tabba <tabba@google.com>,
Andrew Walbran <qwandor@google.com>,
linux-arm-kernel@lists.infradead.org,
kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org,
kernel-team@android.com
Subject: Re: [PATCH v2 07/15] KVM: arm64: Introduce kvm_share_hyp()
Date: Thu, 21 Oct 2021 10:07:47 +0000 [thread overview]
Message-ID: <YXE7842op3n8+JXv@google.com> (raw)
In-Reply-To: <20211019121304.2732332-8-qperret@google.com>
Hi Quentin,
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 0019b2309f70..0cc4b295e525 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -299,6 +299,17 @@ static int pkvm_share_hyp(phys_addr_t start, phys_addr_t end)
> return 0;
> }
>
> +int kvm_share_hyp(void *from, void *to)
> +{
> + if (is_kernel_in_hyp_mode())
> + return 0;
> +
> + if (kvm_host_owns_hyp_mappings())
> + return create_hyp_mappings(from, to, PAGE_HYP);
> +
> + return pkvm_share_hyp(kvm_kaddr_to_phys(from), kvm_kaddr_to_phys(to));
We should be careful about vmalloc memory here. kvm_kaddr_to_phys will
happily return the physical address but the range is not guaranteed to
be physically contiguous. It doesn't look like this series ever shares
vmalloc memory, but we should make it harder for users of this function
to shoot themselves in the foot.
One option would be to turn this into a loop and call pkvm_share_hyp on
each physical page. But since the hypervisor has no means of making
those pages virtually contigous anyway, probably not the right approach.
We could make it possible to share vmalloc buffer that fit into a page
and reject others. There we still need to be careful about the upper
bound because the way it's written now, 'to' can be treated as
exclusive and 'kvm_kaddr_to_phys(to)' would not always return the page
after 'kvm_kaddr_to_phys(from)'.
-David
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-10-21 10:07 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-19 12:12 [PATCH v2 00/15] KVM: arm64: pkvm: Implement unshare hypercall Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` [PATCH v2 01/15] KVM: arm64: Check if running in VHE from kvm_host_owns_hyp_mappings() Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` [PATCH v2 02/15] KVM: arm64: Provide {get,put}_page() stubs for early hyp allocator Quentin Perret
2021-10-19 12:12 ` [PATCH v2 02/15] KVM: arm64: Provide {get, put}_page() " Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` [PATCH v2 03/15] KVM: arm64: Refcount hyp stage-1 pgtable pages Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` [PATCH v2 04/15] KVM: arm64: Fixup hyp stage-1 refcount Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` [PATCH v2 05/15] KVM: arm64: Hook up ->page_count() for hypervisor stage-1 page-table Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` [PATCH v2 06/15] KVM: arm64: Implement kvm_pgtable_hyp_unmap() at EL2 Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` [PATCH v2 07/15] KVM: arm64: Introduce kvm_share_hyp() Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-21 10:07 ` David Brazdil [this message]
2021-10-21 10:07 ` David Brazdil
2021-10-21 10:07 ` David Brazdil
2021-10-19 12:12 ` [PATCH v2 08/15] KVM: arm64: pkvm: Refcount the pages shared with EL2 Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` [PATCH v2 09/15] KVM: arm64: Extend pkvm_page_state enumeration to handle absent pages Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` [PATCH v2 10/15] KVM: arm64: Introduce wrappers for host and hyp spin lock accessors Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:12 ` Quentin Perret
2021-10-19 12:13 ` [PATCH v2 11/15] KVM: arm64: Implement do_share() helper for sharing memory Quentin Perret
2021-10-19 12:13 ` Quentin Perret
2021-10-19 12:13 ` Quentin Perret
2021-10-19 12:13 ` [PATCH v2 12/15] KVM: arm64: Implement __pkvm_host_share_hyp() using do_share() Quentin Perret
2021-10-19 12:13 ` Quentin Perret
2021-10-19 12:13 ` Quentin Perret
2021-10-19 12:13 ` [PATCH v2 13/15] KVM: arm64: Implement do_unshare() helper for unsharing memory Quentin Perret
2021-10-19 12:13 ` Quentin Perret
2021-10-19 12:13 ` Quentin Perret
2021-10-19 12:13 ` [PATCH v2 14/15] KVM: arm64: Expose unshare hypercall to the host Quentin Perret
2021-10-19 12:13 ` Quentin Perret
2021-10-19 12:13 ` Quentin Perret
2021-10-19 12:13 ` [PATCH v2 15/15] KVM: arm64: pkvm: Unshare guest structs during teardown Quentin Perret
2021-10-19 12:13 ` Quentin Perret
2021-10-19 12:13 ` Quentin Perret
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=YXE7842op3n8+JXv@google.com \
--to=dbrazdil@google.com \
--cc=alexandru.elisei@arm.com \
--cc=catalin.marinas@arm.com \
--cc=james.morse@arm.com \
--cc=kernel-team@android.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maz@kernel.org \
--cc=qperret@google.com \
--cc=qwandor@google.com \
--cc=suzuki.poulose@arm.com \
--cc=tabba@google.com \
--cc=will@kernel.org \
/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: link
Be 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.