diff mbox series

[2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

Message ID 20220615193116.806312-3-coltonlewis@google.com (mailing list archive)
State New, archived
Headers show
Series Fix filename reporting in guest asserts | expand

Commit Message

Colton Lewis June 15, 2022, 7:31 p.m. UTC
Increase UCALL_MAX_ARGS to 7 to allow GUEST_ASSERT_4 to pass 3 builtin
ucall arguments specified in guest_assert_builtin_args plus 4
user-specified arguments.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 tools/testing/selftests/kvm/include/ucall_common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andrew Jones June 16, 2022, 12:10 p.m. UTC | #1
On Wed, Jun 15, 2022 at 07:31:14PM +0000, Colton Lewis wrote:
> Increase UCALL_MAX_ARGS to 7 to allow GUEST_ASSERT_4 to pass 3 builtin
> ucall arguments specified in guest_assert_builtin_args plus 4
> user-specified arguments.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/include/ucall_common.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
> index dbe872870b83..568c562f14cd 100644
> --- a/tools/testing/selftests/kvm/include/ucall_common.h
> +++ b/tools/testing/selftests/kvm/include/ucall_common.h
> @@ -16,7 +16,7 @@ enum {
>  	UCALL_UNHANDLED,
>  };
>  
> -#define UCALL_MAX_ARGS 6
> +#define UCALL_MAX_ARGS 7
>  
>  struct ucall {
>  	uint64_t cmd;
> -- 
> 2.36.1.476.g0c4daa206d-goog
>

We probably want to ensure all architectures are good with this. afaict,
riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
for example.

Thanks,
drew
Colton Lewis June 17, 2022, 5:05 p.m. UTC | #2
On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> We probably want to ensure all architectures are good with this. afaict,
> riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> for example.

All architectures use UCALL_MAX_ARGS for that. Are you saying there
might be limitations beyond the value of the macro? If so, who should
verify whether this is ok?
Sean Christopherson June 18, 2022, 12:09 a.m. UTC | #3
On Fri, Jun 17, 2022, Colton Lewis wrote:
> On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> > We probably want to ensure all architectures are good with this. afaict,
> > riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> > for example.
> 
> All architectures use UCALL_MAX_ARGS for that. Are you saying there
> might be limitations beyond the value of the macro? If so, who should
> verify whether this is ok?

I thought there were architectural limitations too, but I believe I was thinking
of vcpu_args_set(), where the number of params is limited by the function call
ABI, e.g. the number of registers.

Unless there's something really, really subtle going on, all architectures pass
the actual ucall struct purely through memory.  Actually, that code is ripe for
deduplication, and amazingly it doesn't conflict with Colton's series.  Patches
incoming...
Andrew Jones June 20, 2022, 7:21 a.m. UTC | #4
On Sat, Jun 18, 2022 at 12:09:11AM +0000, Sean Christopherson wrote:
> On Fri, Jun 17, 2022, Colton Lewis wrote:
> > On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> > > We probably want to ensure all architectures are good with this. afaict,
> > > riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> > > for example.
> > 
> > All architectures use UCALL_MAX_ARGS for that. Are you saying there
> > might be limitations beyond the value of the macro? If so, who should
> > verify whether this is ok?
> 
> I thought there were architectural limitations too, but I believe I was thinking
> of vcpu_args_set(), where the number of params is limited by the function call
> ABI, e.g. the number of registers.
> 
> Unless there's something really, really subtle going on, all architectures pass
> the actual ucall struct purely through memory.  Actually, that code is ripe for
> deduplication, and amazingly it doesn't conflict with Colton's series.  Patches
> incoming...
>

RISC-V uses sbi_ecall() for their implementation of ucall(). CC'ing Anup
for confirmation, but if I understand the SBI spec correctly, then inputs
are limited to registers a0-a5.

Thanks,
drew
Anup Patel June 20, 2022, 8:59 a.m. UTC | #5
On Mon, Jun 20, 2022 at 12:51 PM Andrew Jones <drjones@redhat.com> wrote:
>
> On Sat, Jun 18, 2022 at 12:09:11AM +0000, Sean Christopherson wrote:
> > On Fri, Jun 17, 2022, Colton Lewis wrote:
> > > On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> > > > We probably want to ensure all architectures are good with this. afaict,
> > > > riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> > > > for example.
> > >
> > > All architectures use UCALL_MAX_ARGS for that. Are you saying there
> > > might be limitations beyond the value of the macro? If so, who should
> > > verify whether this is ok?
> >
> > I thought there were architectural limitations too, but I believe I was thinking
> > of vcpu_args_set(), where the number of params is limited by the function call
> > ABI, e.g. the number of registers.
> >
> > Unless there's something really, really subtle going on, all architectures pass
> > the actual ucall struct purely through memory.  Actually, that code is ripe for
> > deduplication, and amazingly it doesn't conflict with Colton's series.  Patches
> > incoming...
> >
>
> RISC-V uses sbi_ecall() for their implementation of ucall(). CC'ing Anup
> for confirmation, but if I understand the SBI spec correctly, then inputs
> are limited to registers a0-a5.

Yes, we only have 6 parameters in ucall() since it is based on SBI spec.

Actually, a6 and a7 are used to identify the type of SBI call (i.e. extension
and function number) whereas a0-a5 are function parameters.

Regards,
Anup

>
> Thanks,
> drew
>
Andrew Jones June 20, 2022, 1:20 p.m. UTC | #6
On Mon, Jun 20, 2022 at 09:21:11AM +0200, Andrew Jones wrote:
> On Sat, Jun 18, 2022 at 12:09:11AM +0000, Sean Christopherson wrote:
> > On Fri, Jun 17, 2022, Colton Lewis wrote:
> > > On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> > > > We probably want to ensure all architectures are good with this. afaict,
> > > > riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> > > > for example.
> > > 
> > > All architectures use UCALL_MAX_ARGS for that. Are you saying there
> > > might be limitations beyond the value of the macro? If so, who should
> > > verify whether this is ok?
> > 
> > I thought there were architectural limitations too, but I believe I was thinking
> > of vcpu_args_set(), where the number of params is limited by the function call
> > ABI, e.g. the number of registers.
> > 
> > Unless there's something really, really subtle going on, all architectures pass
> > the actual ucall struct purely through memory.  Actually, that code is ripe for
> > deduplication, and amazingly it doesn't conflict with Colton's series.  Patches
> > incoming...
> >
> 
> RISC-V uses sbi_ecall() for their implementation of ucall(). CC'ing Anup
> for confirmation, but if I understand the SBI spec correctly, then inputs
> are limited to registers a0-a5.

Ah, never mind... I see SBI is limited to 6 registers, but of course it
only needs one register to pass the uc address... We can make
UCALL_MAX_ARGS whatever we want.

Thanks,
drew
Andrew Jones June 20, 2022, 1:21 p.m. UTC | #7
On Wed, Jun 15, 2022 at 07:31:14PM +0000, Colton Lewis wrote:
> Increase UCALL_MAX_ARGS to 7 to allow GUEST_ASSERT_4 to pass 3 builtin
> ucall arguments specified in guest_assert_builtin_args plus 4
> user-specified arguments.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/include/ucall_common.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
> index dbe872870b83..568c562f14cd 100644
> --- a/tools/testing/selftests/kvm/include/ucall_common.h
> +++ b/tools/testing/selftests/kvm/include/ucall_common.h
> @@ -16,7 +16,7 @@ enum {
>  	UCALL_UNHANDLED,
>  };
>  
> -#define UCALL_MAX_ARGS 6
> +#define UCALL_MAX_ARGS 7
>  
>  struct ucall {
>  	uint64_t cmd;
> -- 
> 2.36.1.476.g0c4daa206d-goog
>

Reviewed-by: Andrew Jones <drjones@redhat.com>
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
index dbe872870b83..568c562f14cd 100644
--- a/tools/testing/selftests/kvm/include/ucall_common.h
+++ b/tools/testing/selftests/kvm/include/ucall_common.h
@@ -16,7 +16,7 @@  enum {
 	UCALL_UNHANDLED,
 };
 
-#define UCALL_MAX_ARGS 6
+#define UCALL_MAX_ARGS 7
 
 struct ucall {
 	uint64_t cmd;