Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 9 Mar 2022 16:50:45 +0000
From:   Quentin Perret <qperret@google.com>
To:     Kalesh Singh <kaleshsingh@google.com>
Cc:     Stephen Boyd <swboyd@chromium.org>, Will Deacon <will@kernel.org>,
        Marc Zyngier <maz@kernel.org>, Fuad Tabba <tabba@google.com>,
        Suren Baghdasaryan <surenb@google.com>,
        "Cc: Android Kernel" <kernel-team@android.com>,
        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>,
        Mark Rutland <mark.rutland@arm.com>,
        Mark Brown <broonie@kernel.org>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Peter Collingbourne <pcc@google.com>,
        "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>,
        Andrew Scull <ascull@google.com>,
        "moderated list:ARM64 PORT (AARCH64 ARCHITECTURE)" 
        <linux-arm-kernel@lists.infradead.org>,
        kvmarm <kvmarm@lists.cs.columbia.edu>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5 1/8] KVM: arm64: Introduce hyp_alloc_private_va_range()
Message-ID: <Yija5cY6j/B25Psw@google.com>
References: <20220307184935.1704614-1-kaleshsingh@google.com>
 <20220307184935.1704614-2-kaleshsingh@google.com>
 <CAE-0n52LmVRkrSNN=eJf+TYYnmesVjFv99nnetYvRWshm82rOg@mail.gmail.com>
 <CAC_TJvc6LYp95BXQc0DSBBBAZpYpixa+NyHKMLFWsBADD5Ubhg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAC_TJvc6LYp95BXQc0DSBBBAZpYpixa+NyHKMLFWsBADD5Ubhg@mail.gmail.com>
Precedence: bulk

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