[0/2] RISC-V: KVM: Require alternatives

Message ID	20230322192858.1189272-1-ajones@ventanamicro.com (mailing list archive)
Headers	show Return-Path: <linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org> From: Andrew Jones <ajones@ventanamicro.com> To: linux-riscv@lists.infradead.org, kvm-riscv@lists.infradead.org Cc: 'Palmer Dabbelt ' <palmer@dabbelt.com>, 'Anup Patel ' <anup@brainfault.org>, 'Paul Walmsley ' <paul.walmsley@sifive.com>, 'Atish Patra ' <atishp@atishpatra.org>, 'Albert Ou ' <aou@eecs.berkeley.edu> Subject: [PATCH 0/2] RISC-V: KVM: Require alternatives Date: Wed, 22 Mar 2023 20:28:56 +0100 Message-Id: <20230322192858.1189272-1-ajones@ventanamicro.com> MIME-Version: 1.0 Precedence: list Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" <linux-riscv-bounces@lists.infradead.org> Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org
Series	RISC-V: KVM: Require alternatives \| expand [0/2] RISC-V: KVM: Require alternatives [1/2] RISC-V: KVM: Alphabetize selects [2/2] RISC-V: KVM: Require alternatives

Andrew Jones March 22, 2023, 7:28 p.m. UTC

KVM makes use of riscv_has_extension_unlikely() to check for the
svinval extension. riscv_has_extension_unlikely() is built on
alternatives, which means KVM should ensure alternatives support
is available.

The first patch takes the opportunity to cleanup KVM's select
list. The second patch selects RISCV_ALTERNATIVE.

Thanks,
drew

Andrew Jones (2):
  RISC-V: KVM: Alphabetize selects
  RISC-V: KVM: Require alternatives

 arch/riscv/kvm/Kconfig | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Conor Dooley March 22, 2023, 7:40 p.m. UTC | #1

On Wed, Mar 22, 2023 at 08:28:56PM +0100, Andrew Jones wrote:
> KVM makes use of riscv_has_extension_unlikely() to check for the
> svinval extension. riscv_has_extension_unlikely() is built on
> alternatives, which means KVM should ensure alternatives support
> is available.
> 
> The first patch takes the opportunity to cleanup KVM's select
> list. The second patch selects RISCV_ALTERNATIVE.

Reminds me, I need to re-submit my patch doing that for the top-level
RISC-V Kconfig...
For the pair:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

Conor Dooley March 23, 2023, 5:57 p.m. UTC | #2

On Wed, Mar 22, 2023 at 07:40:14PM +0000, Conor Dooley wrote:
> On Wed, Mar 22, 2023 at 08:28:56PM +0100, Andrew Jones wrote:
> > KVM makes use of riscv_has_extension_unlikely() to check for the
> > svinval extension. riscv_has_extension_unlikely() is built on
> > alternatives, which means KVM should ensure alternatives support
> > is available.
> > 
> > The first patch takes the opportunity to cleanup KVM's select
> > list. The second patch selects RISCV_ALTERNATIVE.
> 
> Reminds me, I need to re-submit my patch doing that for the top-level
> RISC-V Kconfig...
> For the pair:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Actually, I would like to take this back for patch 2.
Per the discussion on the other thread about XIP [1], I don't think
that KVM should be selecting alternatives like this.
Would you mind if I picked up these patches & submitted them as a v2,
alongside a patch trying to make sure that we do not clip the wings of
of XIP kernels by selecting RISCV_ALTERNATIVE?

Cheers,
Conor.

1 - https://lore.kernel.org/all/20230322120907.2968494-1-Jason@zx2c4.com/

Andrew Jones March 23, 2023, 7:05 p.m. UTC | #3

On Thu, Mar 23, 2023 at 05:57:14PM +0000, Conor Dooley wrote:
> On Wed, Mar 22, 2023 at 07:40:14PM +0000, Conor Dooley wrote:
> > On Wed, Mar 22, 2023 at 08:28:56PM +0100, Andrew Jones wrote:
> > > KVM makes use of riscv_has_extension_unlikely() to check for the
> > > svinval extension. riscv_has_extension_unlikely() is built on
> > > alternatives, which means KVM should ensure alternatives support
> > > is available.
> > > 
> > > The first patch takes the opportunity to cleanup KVM's select
> > > list. The second patch selects RISCV_ALTERNATIVE.
> > 
> > Reminds me, I need to re-submit my patch doing that for the top-level
> > RISC-V Kconfig...
> > For the pair:
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Actually, I would like to take this back for patch 2.
> Per the discussion on the other thread about XIP [1], I don't think
> that KVM should be selecting alternatives like this.
> Would you mind if I picked up these patches & submitted them as a v2,
> alongside a patch trying to make sure that we do not clip the wings of
> of XIP kernels by selecting RISCV_ALTERNATIVE?
> 

Sounds good to me. Thanks, Conor.

drew

Andrew Jones March 24, 2023, 11:32 a.m. UTC | #4

On Thu, Mar 23, 2023 at 05:57:14PM +0000, Conor Dooley wrote:
> On Wed, Mar 22, 2023 at 07:40:14PM +0000, Conor Dooley wrote:
> > On Wed, Mar 22, 2023 at 08:28:56PM +0100, Andrew Jones wrote:
> > > KVM makes use of riscv_has_extension_unlikely() to check for the
> > > svinval extension. riscv_has_extension_unlikely() is built on
> > > alternatives, which means KVM should ensure alternatives support
> > > is available.
> > > 
> > > The first patch takes the opportunity to cleanup KVM's select
> > > list. The second patch selects RISCV_ALTERNATIVE.
> > 
> > Reminds me, I need to re-submit my patch doing that for the top-level
> > RISC-V Kconfig...
> > For the pair:
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Actually, I would like to take this back for patch 2.
> Per the discussion on the other thread about XIP [1], I don't think
> that KVM should be selecting alternatives like this.
> Would you mind if I picked up these patches & submitted them as a v2,
> alongside a patch trying to make sure that we do not clip the wings of
> of XIP kernels by selecting RISCV_ALTERNATIVE?

Hi Conor,

I take it that resubmitting these patches is no longer part of the plan.
Should I rebase on "[PATCH v1 0/2] RISC-V: Fixes for
riscv_has_extension[un]likely()'s alternative dependency" and change the
select to a depends on?

Thanks,
drew

Conor Dooley March 24, 2023, 11:52 a.m. UTC | #5

On Fri, Mar 24, 2023 at 12:32:59PM +0100, Andrew Jones wrote:
> On Thu, Mar 23, 2023 at 05:57:14PM +0000, Conor Dooley wrote:
> > On Wed, Mar 22, 2023 at 07:40:14PM +0000, Conor Dooley wrote:
> > > On Wed, Mar 22, 2023 at 08:28:56PM +0100, Andrew Jones wrote:
> > > > KVM makes use of riscv_has_extension_unlikely() to check for the
> > > > svinval extension. riscv_has_extension_unlikely() is built on
> > > > alternatives, which means KVM should ensure alternatives support
> > > > is available.
> > > > 
> > > > The first patch takes the opportunity to cleanup KVM's select
> > > > list. The second patch selects RISCV_ALTERNATIVE.
> > > 
> > > Reminds me, I need to re-submit my patch doing that for the top-level
> > > RISC-V Kconfig...
> > > For the pair:
> > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > 
> > Actually, I would like to take this back for patch 2.
> > Per the discussion on the other thread about XIP [1], I don't think
> > that KVM should be selecting alternatives like this.
> > Would you mind if I picked up these patches & submitted them as a v2,
> > alongside a patch trying to make sure that we do not clip the wings of
> > of XIP kernels by selecting RISCV_ALTERNATIVE?
> 
> Hi Conor,
> 
> I take it that resubmitting these patches is no longer part of the plan.

Ah crap, sorry. I meant to reply here after submitting and forgot.

> Should I rebase on "[PATCH v1 0/2] RISC-V: Fixes for
> riscv_has_extension[un]likely()'s alternative dependency" and change the
> select to a depends on?

I don't think you need to. Does KVM actually make use of alternatives,
other than for riscv_has_extension_unlikely(), that are not gated by
extension/erratum specific config options?

With my patch 2/2, alternatives are always enabled for !XIP_KERNEL
builds, and will fall back to the "slow" path in
riscv_has_extension_unlikely() otherwise.
If KVM doesn't need alternatives for another reason, I don't think you
need to introduce a dependency on them and just inherit the decision
made by CONFIG_RISCV.

Andrew Jones March 24, 2023, 11:56 a.m. UTC | #6

On Fri, Mar 24, 2023 at 11:52:13AM +0000, Conor Dooley wrote:
> On Fri, Mar 24, 2023 at 12:32:59PM +0100, Andrew Jones wrote:
> > On Thu, Mar 23, 2023 at 05:57:14PM +0000, Conor Dooley wrote:
> > > On Wed, Mar 22, 2023 at 07:40:14PM +0000, Conor Dooley wrote:
> > > > On Wed, Mar 22, 2023 at 08:28:56PM +0100, Andrew Jones wrote:
> > > > > KVM makes use of riscv_has_extension_unlikely() to check for the
> > > > > svinval extension. riscv_has_extension_unlikely() is built on
> > > > > alternatives, which means KVM should ensure alternatives support
> > > > > is available.
> > > > > 
> > > > > The first patch takes the opportunity to cleanup KVM's select
> > > > > list. The second patch selects RISCV_ALTERNATIVE.
> > > > 
> > > > Reminds me, I need to re-submit my patch doing that for the top-level
> > > > RISC-V Kconfig...
> > > > For the pair:
> > > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > > 
> > > Actually, I would like to take this back for patch 2.
> > > Per the discussion on the other thread about XIP [1], I don't think
> > > that KVM should be selecting alternatives like this.
> > > Would you mind if I picked up these patches & submitted them as a v2,
> > > alongside a patch trying to make sure that we do not clip the wings of
> > > of XIP kernels by selecting RISCV_ALTERNATIVE?
> > 
> > Hi Conor,
> > 
> > I take it that resubmitting these patches is no longer part of the plan.
> 
> Ah crap, sorry. I meant to reply here after submitting and forgot.
> 
> > Should I rebase on "[PATCH v1 0/2] RISC-V: Fixes for
> > riscv_has_extension[un]likely()'s alternative dependency" and change the
> > select to a depends on?
> 
> I don't think you need to. Does KVM actually make use of alternatives,
> other than for riscv_has_extension_unlikely(), that are not gated by
> extension/erratum specific config options?
> 
> With my patch 2/2, alternatives are always enabled for !XIP_KERNEL
> builds, and will fall back to the "slow" path in
> riscv_has_extension_unlikely() otherwise.
> If KVM doesn't need alternatives for another reason, I don't think you
> need to introduce a dependency on them and just inherit the decision
> made by CONFIG_RISCV.

Ack, thanks, Conor.

Anup, we can either drop patch 2/2 or the whole series, as you like.

Thanks,
drew

Anup Patel March 24, 2023, 12:21 p.m. UTC | #7

On Fri, Mar 24, 2023 at 5:26 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Fri, Mar 24, 2023 at 11:52:13AM +0000, Conor Dooley wrote:
> > On Fri, Mar 24, 2023 at 12:32:59PM +0100, Andrew Jones wrote:
> > > On Thu, Mar 23, 2023 at 05:57:14PM +0000, Conor Dooley wrote:
> > > > On Wed, Mar 22, 2023 at 07:40:14PM +0000, Conor Dooley wrote:
> > > > > On Wed, Mar 22, 2023 at 08:28:56PM +0100, Andrew Jones wrote:
> > > > > > KVM makes use of riscv_has_extension_unlikely() to check for the
> > > > > > svinval extension. riscv_has_extension_unlikely() is built on
> > > > > > alternatives, which means KVM should ensure alternatives support
> > > > > > is available.
> > > > > >
> > > > > > The first patch takes the opportunity to cleanup KVM's select
> > > > > > list. The second patch selects RISCV_ALTERNATIVE.
> > > > >
> > > > > Reminds me, I need to re-submit my patch doing that for the top-level
> > > > > RISC-V Kconfig...
> > > > > For the pair:
> > > > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > > >
> > > > Actually, I would like to take this back for patch 2.
> > > > Per the discussion on the other thread about XIP [1], I don't think
> > > > that KVM should be selecting alternatives like this.
> > > > Would you mind if I picked up these patches & submitted them as a v2,
> > > > alongside a patch trying to make sure that we do not clip the wings of
> > > > of XIP kernels by selecting RISCV_ALTERNATIVE?
> > >
> > > Hi Conor,
> > >
> > > I take it that resubmitting these patches is no longer part of the plan.
> >
> > Ah crap, sorry. I meant to reply here after submitting and forgot.
> >
> > > Should I rebase on "[PATCH v1 0/2] RISC-V: Fixes for
> > > riscv_has_extension[un]likely()'s alternative dependency" and change the
> > > select to a depends on?
> >
> > I don't think you need to. Does KVM actually make use of alternatives,
> > other than for riscv_has_extension_unlikely(), that are not gated by
> > extension/erratum specific config options?
> >
> > With my patch 2/2, alternatives are always enabled for !XIP_KERNEL
> > builds, and will fall back to the "slow" path in
> > riscv_has_extension_unlikely() otherwise.
> > If KVM doesn't need alternatives for another reason, I don't think you
> > need to introduce a dependency on them and just inherit the decision
> > made by CONFIG_RISCV.
>
> Ack, thanks, Conor.
>
> Anup, we can either drop patch 2/2 or the whole series, as you like.

If alternatives are going to be always enabled for !XIP_KERNEL then
we can drop PATCH2. I am okay taking PATCH1.

Regards,
Anup

[0/2] RISC-V: KVM: Require alternatives

Message

Comments