Re: [PATCH v5 1/8] KVM: arm64: Introduce hyp_alloc_private_va_range()

From: Quentin Perret <qperret@google.com>
To: Kalesh Singh <kaleshsingh@google.com>
Cc: "Cc: Android Kernel" <kernel-team@android.com>,
	"moderated list:ARM64 PORT \(AARCH64 ARCHITECTURE\)"
	<linux-arm-kernel@lists.infradead.org>,
	Will Deacon <will@kernel.org>,
	Peter Collingbourne <pcc@google.com>,
	Marc Zyngier <maz@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Stephen Boyd <swboyd@chromium.org>,
	"Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>,
	Mark Brown <broonie@kernel.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Suren Baghdasaryan <surenb@google.com>,
	kvmarm <kvmarm@lists.cs.columbia.edu>
Subject: Re: [PATCH v5 1/8] KVM: arm64: Introduce hyp_alloc_private_va_range()
Date: Wed, 9 Mar 2022 16:50:45 +0000	[thread overview]
Message-ID: <Yija5cY6j/B25Psw@google.com> (raw)
In-Reply-To: <CAC_TJvc6LYp95BXQc0DSBBBAZpYpixa+NyHKMLFWsBADD5Ubhg@mail.gmail.com>

On Tuesday 08 Mar 2022 at 15:09:18 (-0800), Kalesh Singh wrote:
> On Tue, Mar 8, 2022 at 12:21 PM Stephen Boyd <swboyd@chromium.org> wrote:
> > It looks odd to use an error pointer casted to unsigned long to return
> > from an address allocation function. Why not pass a pointer for base
> > like the function was written before and return an int from this
> > function with 0 for success and negative error value?Otherwise some
> > sort of define should made like DMA_MAPPING_ERROR and that can be used
> > to indicate to the caller that the allocation failed, or a simple zero
> > may work?
> 
> I wanted to keep consistent between the pkvm and traditional nvhe
> code. I will refactor both *alloc_private_va_range() functions to take
> a pointer and return an int error if that's preferred. There would
> still be a case of this kind of cast in
> __pkvm_create_private_mapping() which does return an unsigned long
> address or ERR_PTR(...). It looks like it was made to return the
> address to facilitate use as a hypercall (@Quentin CMIW).

Yep, passing everything by value was much easier to cross the EL1/EL2
boundary as that avoids having the hypervisor map kernel memory and all
that fun. But Stephen's point is fair, so no objection from to keep this
little dance confined to the hypercall wrapper and make the function
signature nicer and easier to use for the rest of the code.

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