diff mbox series

[v5,08/30] KVM: arm64: make kvm_at() take an OP_AT_*

Message ID 20240822151113.1479789-9-joey.gouly@arm.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series Permission Overlay Extension | expand

Commit Message

Joey Gouly Aug. 22, 2024, 3:10 p.m. UTC
To allow using newer instructions that current assemblers don't know about,
replace the `at` instruction with the underlying SYS instruction.

Signed-off-by: Joey Gouly <joey.gouly@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Reviewed-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/include/asm/kvm_asm.h       | 3 ++-
 arch/arm64/kvm/hyp/include/hyp/fault.h | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

Comments

Will Deacon Aug. 23, 2024, 1:48 p.m. UTC | #1
On Thu, Aug 22, 2024 at 04:10:51PM +0100, Joey Gouly wrote:
> To allow using newer instructions that current assemblers don't know about,
> replace the `at` instruction with the underlying SYS instruction.
> 
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Reviewed-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/include/asm/kvm_asm.h       | 3 ++-
>  arch/arm64/kvm/hyp/include/hyp/fault.h | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)

Marc -- what would you like to do with this patch? I think the POE series
is really close now, so ideally I'd queue the lot on a branch in arm64
and you could pull the first ~10 patches into kvmarm if you need 'em.

Would what work for you, or did you have something else in mind (since
this one is also included in your series adding nv support for AT).

Cheers,

Will
Marc Zyngier Aug. 23, 2024, 2:24 p.m. UTC | #2
On Fri, 23 Aug 2024 14:48:11 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Thu, Aug 22, 2024 at 04:10:51PM +0100, Joey Gouly wrote:
> > To allow using newer instructions that current assemblers don't know about,
> > replace the `at` instruction with the underlying SYS instruction.
> > 
> > Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Oliver Upton <oliver.upton@linux.dev>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Reviewed-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/include/asm/kvm_asm.h       | 3 ++-
> >  arch/arm64/kvm/hyp/include/hyp/fault.h | 2 +-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> Marc -- what would you like to do with this patch? I think the POE series
> is really close now, so ideally I'd queue the lot on a branch in arm64
> and you could pull the first ~10 patches into kvmarm if you need 'em.
>
> Would what work for you, or did you have something else in mind (since
> this one is also included in your series adding nv support for AT).

Yup, that works for me. I can take that prefix as the base for the AT
series and drop my copy of this patch.

Thanks,

	M.
Marc Zyngier Aug. 30, 2024, 8:01 a.m. UTC | #3
Hi Will,

On Fri, 23 Aug 2024 14:48:11 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Thu, Aug 22, 2024 at 04:10:51PM +0100, Joey Gouly wrote:
> > To allow using newer instructions that current assemblers don't know about,
> > replace the `at` instruction with the underlying SYS instruction.
> > 
> > Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Oliver Upton <oliver.upton@linux.dev>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Reviewed-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/include/asm/kvm_asm.h       | 3 ++-
> >  arch/arm64/kvm/hyp/include/hyp/fault.h | 2 +-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> Marc -- what would you like to do with this patch? I think the POE series
> is really close now, so ideally I'd queue the lot on a branch in arm64
> and you could pull the first ~10 patches into kvmarm if you need 'em.
> 
> Would what work for you, or did you have something else in mind (since
> this one is also included in your series adding nv support for AT).

Is there any progress on this front? I am quite eager to queue the AT
series, but the dependency on this patch is preventing me to do so.

I can see there are outstanding questions on the POE series, so I was
wondering if we should consider reversing the dependency: I can create
a stable branch with this single patch, which you can pull as a prefix
of the POE series.

Please let me know what you prefer.

Thanks,

	M.
Will Deacon Aug. 30, 2024, 9:05 a.m. UTC | #4
Hey Marc,

On Fri, Aug 30, 2024 at 09:01:18AM +0100, Marc Zyngier wrote:
> On Fri, 23 Aug 2024 14:48:11 +0100,
> Will Deacon <will@kernel.org> wrote:
> > 
> > On Thu, Aug 22, 2024 at 04:10:51PM +0100, Joey Gouly wrote:
> > > To allow using newer instructions that current assemblers don't know about,
> > > replace the `at` instruction with the underlying SYS instruction.
> > > 
> > > Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> > > Cc: Marc Zyngier <maz@kernel.org>
> > > Cc: Oliver Upton <oliver.upton@linux.dev>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Will Deacon <will@kernel.org>
> > > Reviewed-by: Marc Zyngier <maz@kernel.org>
> > > ---
> > >  arch/arm64/include/asm/kvm_asm.h       | 3 ++-
> > >  arch/arm64/kvm/hyp/include/hyp/fault.h | 2 +-
> > >  2 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > Marc -- what would you like to do with this patch? I think the POE series
> > is really close now, so ideally I'd queue the lot on a branch in arm64
> > and you could pull the first ~10 patches into kvmarm if you need 'em.
> > 
> > Would what work for you, or did you have something else in mind (since
> > this one is also included in your series adding nv support for AT).
> 
> Is there any progress on this front? I am quite eager to queue the AT
> series, but the dependency on this patch is preventing me to do so.
> 
> I can see there are outstanding questions on the POE series, so I was
> wondering if we should consider reversing the dependency: I can create
> a stable branch with this single patch, which you can pull as a prefix
> of the POE series.

That sounds like a good idea. The uaccess discussion seems to have
stalled and I don't really want to merge the series without concluding
that.

So please go ahead with this single patch and I'll pull it in if things
start moving again.

Will
Will Deacon Aug. 30, 2024, 9:25 a.m. UTC | #5
On Thu, Aug 22, 2024 at 04:10:51PM +0100, Joey Gouly wrote:
> To allow using newer instructions that current assemblers don't know about,
> replace the `at` instruction with the underlying SYS instruction.
> 
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Oliver Upton <oliver.upton@linux.dev>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Reviewed-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/include/asm/kvm_asm.h       | 3 ++-
>  arch/arm64/kvm/hyp/include/hyp/fault.h | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)

Acked-by: Will Deacon <will@kernel.org>

> diff --git arch/arm64/include/asm/kvm_asm.h arch/arm64/include/asm/kvm_asm.h
> index 2181a11b9d92..38d7bfa3966a 100644
> --- arch/arm64/include/asm/kvm_asm.h
> +++ arch/arm64/include/asm/kvm_asm.h

FWIW (mainly for Marc): you seem to be missing the 'a/' and 'b/'
prefixes here, so my git would't accept the change when I tried to
apply locally for testing.

Will
Marc Zyngier Aug. 30, 2024, 11:23 a.m. UTC | #6
On Fri, 30 Aug 2024 10:25:27 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Thu, Aug 22, 2024 at 04:10:51PM +0100, Joey Gouly wrote:
> > To allow using newer instructions that current assemblers don't know about,
> > replace the `at` instruction with the underlying SYS instruction.
> > 
> > Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Oliver Upton <oliver.upton@linux.dev>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Reviewed-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/include/asm/kvm_asm.h       | 3 ++-
> >  arch/arm64/kvm/hyp/include/hyp/fault.h | 2 +-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> Acked-by: Will Deacon <will@kernel.org>
> 
> > diff --git arch/arm64/include/asm/kvm_asm.h arch/arm64/include/asm/kvm_asm.h
> > index 2181a11b9d92..38d7bfa3966a 100644
> > --- arch/arm64/include/asm/kvm_asm.h
> > +++ arch/arm64/include/asm/kvm_asm.h
> 
> FWIW (mainly for Marc): you seem to be missing the 'a/' and 'b/'
> prefixes here, so my git would't accept the change when I tried to
> apply locally for testing.

Seems like a spurious '--no-prefix' was added at patch formatting
time, That clashes with git-apply's default '-p1', which strips the
first component of the path.

There's probably a way to pass '-p0' to 'git am', but I don't feel
like trawling the git documentation by such a temperature...

	M.
Joey Gouly Aug. 30, 2024, 11:35 a.m. UTC | #7
On Fri, Aug 30, 2024 at 12:23:33PM +0100, Marc Zyngier wrote:
> On Fri, 30 Aug 2024 10:25:27 +0100,
> Will Deacon <will@kernel.org> wrote:
> > 
> > On Thu, Aug 22, 2024 at 04:10:51PM +0100, Joey Gouly wrote:
> > > To allow using newer instructions that current assemblers don't know about,
> > > replace the `at` instruction with the underlying SYS instruction.
> > > 
> > > Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> > > Cc: Marc Zyngier <maz@kernel.org>
> > > Cc: Oliver Upton <oliver.upton@linux.dev>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Will Deacon <will@kernel.org>
> > > Reviewed-by: Marc Zyngier <maz@kernel.org>
> > > ---
> > >  arch/arm64/include/asm/kvm_asm.h       | 3 ++-
> > >  arch/arm64/kvm/hyp/include/hyp/fault.h | 2 +-
> > >  2 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > Acked-by: Will Deacon <will@kernel.org>
> > 
> > > diff --git arch/arm64/include/asm/kvm_asm.h arch/arm64/include/asm/kvm_asm.h
> > > index 2181a11b9d92..38d7bfa3966a 100644
> > > --- arch/arm64/include/asm/kvm_asm.h
> > > +++ arch/arm64/include/asm/kvm_asm.h
> > 
> > FWIW (mainly for Marc): you seem to be missing the 'a/' and 'b/'
> > prefixes here, so my git would't accept the change when I tried to
> > apply locally for testing.
> 
> Seems like a spurious '--no-prefix' was added at patch formatting
> time, That clashes with git-apply's default '-p1', which strips the
> first component of the path.

I had --no-prefix in my .git/config for diffs, but I didn't realise that also
applied to git format-patch, sorry for that. I have removed it now.

If you want me to resend v5, or something else, let me know.

> 
> There's probably a way to pass '-p0' to 'git am', but I don't feel
> like trawling the git documentation by such a temperature...
> 
> 	M.

related to uaccess: Catalin is away, sure when he's back, so I'm hoping we can
resolve that when he's around.

Thanks,
Joey
Marc Zyngier Aug. 30, 2024, 11:58 a.m. UTC | #8
On Fri, 30 Aug 2024 10:05:22 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> Hey Marc,
> 
> On Fri, Aug 30, 2024 at 09:01:18AM +0100, Marc Zyngier wrote:
> > On Fri, 23 Aug 2024 14:48:11 +0100,
> > Will Deacon <will@kernel.org> wrote:
> > > 
> > > On Thu, Aug 22, 2024 at 04:10:51PM +0100, Joey Gouly wrote:
> > > > To allow using newer instructions that current assemblers don't know about,
> > > > replace the `at` instruction with the underlying SYS instruction.
> > > > 
> > > > Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> > > > Cc: Marc Zyngier <maz@kernel.org>
> > > > Cc: Oliver Upton <oliver.upton@linux.dev>
> > > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > > Cc: Will Deacon <will@kernel.org>
> > > > Reviewed-by: Marc Zyngier <maz@kernel.org>
> > > > ---
> > > >  arch/arm64/include/asm/kvm_asm.h       | 3 ++-
> > > >  arch/arm64/kvm/hyp/include/hyp/fault.h | 2 +-
> > > >  2 files changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > Marc -- what would you like to do with this patch? I think the POE series
> > > is really close now, so ideally I'd queue the lot on a branch in arm64
> > > and you could pull the first ~10 patches into kvmarm if you need 'em.
> > > 
> > > Would what work for you, or did you have something else in mind (since
> > > this one is also included in your series adding nv support for AT).
> > 
> > Is there any progress on this front? I am quite eager to queue the AT
> > series, but the dependency on this patch is preventing me to do so.
> > 
> > I can see there are outstanding questions on the POE series, so I was
> > wondering if we should consider reversing the dependency: I can create
> > a stable branch with this single patch, which you can pull as a prefix
> > of the POE series.
> 
> That sounds like a good idea. The uaccess discussion seems to have
> stalled and I don't really want to merge the series without concluding
> that.
> 
> So please go ahead with this single patch and I'll pull it in if things
> start moving again.

Now pushed to [1].

Thanks,

	M.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git/log/?h=arm64-shared-6.12
diff mbox series

Patch

diff --git arch/arm64/include/asm/kvm_asm.h arch/arm64/include/asm/kvm_asm.h
index 2181a11b9d92..38d7bfa3966a 100644
--- arch/arm64/include/asm/kvm_asm.h
+++ arch/arm64/include/asm/kvm_asm.h
@@ -9,6 +9,7 @@ 
 
 #include <asm/hyp_image.h>
 #include <asm/insn.h>
+#include <asm/sysreg.h>
 #include <asm/virt.h>
 
 #define ARM_EXIT_WITH_SERROR_BIT  31
@@ -259,7 +260,7 @@  extern u64 __kvm_get_mdcr_el2(void);
 	asm volatile(							\
 	"	mrs	%1, spsr_el2\n"					\
 	"	mrs	%2, elr_el2\n"					\
-	"1:	at	"at_op", %3\n"					\
+	"1:	" __msr_s(at_op, "%3") "\n"				\
 	"	isb\n"							\
 	"	b	9f\n"						\
 	"2:	msr	spsr_el2, %1\n"					\
diff --git arch/arm64/kvm/hyp/include/hyp/fault.h arch/arm64/kvm/hyp/include/hyp/fault.h
index 9e13c1bc2ad5..487c06099d6f 100644
--- arch/arm64/kvm/hyp/include/hyp/fault.h
+++ arch/arm64/kvm/hyp/include/hyp/fault.h
@@ -27,7 +27,7 @@  static inline bool __translate_far_to_hpfar(u64 far, u64 *hpfar)
 	 * saved the guest context yet, and we may return early...
 	 */
 	par = read_sysreg_par();
-	if (!__kvm_at("s1e1r", far))
+	if (!__kvm_at(OP_AT_S1E1R, far))
 		tmp = read_sysreg_par();
 	else
 		tmp = SYS_PAR_EL1_F; /* back to the guest */