Re: [RFC PATCH 06/17] KVM: arm64: Implement break-before-make sequence for parallel walks

From: Quentin Perret <qperret@google.com>
To: Oliver Upton <oupton@google.com>
Cc: kvm@vger.kernel.org, Marc Zyngier <maz@kernel.org>,
	Peter Shier <pshier@google.com>, Ben Gardon <bgardon@google.com>,
	David Matlack <dmatlack@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvmarm@lists.cs.columbia.edu,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH 06/17] KVM: arm64: Implement break-before-make sequence for parallel walks
Date: Wed, 20 Apr 2022 16:55:43 +0000	[thread overview]
Message-ID: <YmA7D7DyY7MDfli4@google.com> (raw)
In-Reply-To: <20220415215901.1737897-7-oupton@google.com>

On Friday 15 Apr 2022 at 21:58:50 (+0000), Oliver Upton wrote:
> +/*
> + * Used to indicate a pte for which a 'make-before-break' sequence is in

'break-before-make' presumably :-) ?

<snip>
> +static void stage2_make_pte(kvm_pte_t *ptep, kvm_pte_t new, struct kvm_pgtable_mm_ops *mm_ops)
> +{
> +	/* Yikes! We really shouldn't install to an entry we don't own. */
> +	WARN_ON(!stage2_pte_is_locked(*ptep));
> +
> +	if (stage2_pte_is_counted(new))
> +		mm_ops->get_page(ptep);
> +
> +	if (kvm_pte_valid(new)) {
> +		WRITE_ONCE(*ptep, new);
> +		dsb(ishst);
> +	} else {
> +		smp_store_release(ptep, new);
> +	}
> +}

I'm struggling a bit to understand this pattern. We currently use
smp_store_release() to install valid mappings, which this patch seems
to change. Is the behaviour change intentional? If so, could you please
share some details about the reasoning that applies here?

Thanks,
Quentin
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm