Thread (42 messages) 42 messages, 3 authors, 2021-07-21

Re: [PATCH 06/14] KVM: arm64: Tolerate re-creating hyp mappings to set ignored bits

From: Fuad Tabba <hidden>
Date: 2021-07-20 11:05:50
Also in: kvmarm, lkml

Hi Quentin,

On Tue, Jul 20, 2021 at 11:30 AM 'Quentin Perret' via kernel-team
[off-list ref] wrote:
Hi Fuad,

On Tuesday 20 Jul 2021 at 11:17:03 (+0100), Fuad Tabba wrote:
quoted
Hi Quentin,


On Mon, Jul 19, 2021 at 11:47 AM Quentin Perret [off-list ref] wrote:
quoted
The current hypervisor stage-1 mapping code doesn't allow changing an
existing valid mapping. Relax this condition by allowing changes that
only target ignored bits, as that will soon be needed to annotate shared
pages.

Signed-off-by: Quentin Perret <redacted>
---
 arch/arm64/kvm/hyp/pgtable.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index a0ac8c2bc174..34cf67997a82 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -362,6 +362,17 @@ static int hyp_set_prot_attr(enum kvm_pgtable_prot prot, kvm_pte_t *ptep)
        return 0;
 }

+static bool hyp_pte_needs_update(kvm_pte_t old, kvm_pte_t new)
+{
+       if (old == new)
+               return false;
+
+       if (!kvm_pte_valid(old))
+               return true;
+
+       return !WARN_ON((old ^ new) & ~KVM_PTE_LEAF_ATTR_IGNORED);
Wouldn't this return false if both ignored and non-ignored bits were
different, or is that not possible (judging by the WARN_ON)?
Correct, but that is intentional, see below ;)
quoted
If it is, then it would need an update, wouldn't it?
Maybe, but if you look at what the existing code does, we do skip the
update if the old mapping is valid and not equal to new. So I kept the
behaviour as close as possible to this -- if you change any bits outside
of SW bits you get a WARN and we skip the update, as we already do
today. But if you touch only SW bits and nothing else, then I let the
update go through.

That said, I don't think warning and then proceeding to update would be
terribly wrong, it's just that a change of behaviour felt a bit
unnecessary for this particular patch.
Thanks for the clarification. It makes sense to preserve the existing
behavior, but I was wondering if a comment would be good, describing
what merits a "needs update"?

Cheers,
/fuad
Thanks,
Quentin

--
To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help