On Tue, May 3, 2022 at 1:33 PM Marc Zyngier <maz@xxxxxxxxxx> wrote: > > On Tue, 03 May 2022 19:49:13 +0100, > Raghavendra Rao Ananta <rananta@xxxxxxxxxx> wrote: > > > > Hi Marc, > > > > On Tue, May 3, 2022 at 10:24 AM Marc Zyngier <maz@xxxxxxxxxx> wrote: > > > > > > On Tue, 03 May 2022 00:38:44 +0100, > > > Raghavendra Rao Ananta <rananta@xxxxxxxxxx> wrote: > > > > > > > > Hello, > > > > > > > > Continuing the discussion from [1], the series tries to add support > > > > for the userspace to elect the hypercall services that it wishes > > > > to expose to the guest, rather than the guest discovering them > > > > unconditionally. The idea employed by the series was taken from > > > > [1] as suggested by Marc Z. > > > > > > As it took some time to get there, and that there was still a bunch of > > > things to address, I've taken the liberty to apply my own fixes to the > > > series. > > > > > > Please have a look at [1], and let me know if you're OK with the > > > result. If you are, I'll merge the series for 5.19. > > > > > > Thanks, > > > > > > M. > > > > > Thank you for speeding up the process; appreciate it. However, the > > series's selftest patches have a dependency on Oliver's > > PSCI_SYSTEM_SUSPEND's selftest patches [1][2]. Can we pull them in > > too? > > Urgh... I guess this is the time to set some ground rules: > > - Please don't introduce dependencies between series, that's > unmanageable. I really need to see each series independently, and if > there is a merge conflict, that's my job to fix (and I don't really > mind). > > - If there is a dependency between series, please post a version of > the required patches as a prefix to your series, assuming this > prefix is itself standalone. If it isn't, then something really is > wrong, and the series should be resplit. > > - You also should be basing your series on an *official* tag from > Linus' tree (preferably -rc1, -rc2 or -rc3), and not something > random like any odd commit from the KVM tree (I had conflicts while > applying this on -rc3, probably due to the non-advertised dependency > on Oliver's series). > Thanks for picking the dependency patches. I'll keep these mind the next time I push changes. > > > > aarch64/hypercalls.c: In function ‘guest_test_hvc’: > > aarch64/hypercalls.c:95:30: error: storage size of ‘res’ isn’t known > > 95 | struct arm_smccc_res res; > > | ^~~ > > aarch64/hypercalls.c:103:17: warning: implicit declaration of function > > ‘smccc_hvc’ [-Wimplicit-function-declaration] > > 103 | smccc_hvc(hc_info->func_id, hc_info->arg1, 0, > > 0, 0, 0, 0, 0, &res); > > | ^~~~~~~~~ > > > > I've picked the two patches, which means they will most likely appear > twice in the history. In the future, please reach out so that we can > organise this better. > > > Also, just a couple of readability nits in the fixed version: > > > > 1. Patch-2/9, hypercall.c:kvm_hvc_call_default_allowed(), in the > > 'default' case, do you think we should probably add a small comment > > that mentions we are checking for func_id in the PSCI range? > > Dumped a one-liner there. > > > 2. Patch-2/9, arm_hypercall.h, clear all the macros in this patch > > itself instead of doing it in increments (unless there's some reason > > that I'm missing)? > > Ah, rebasing leftovers, now gone. > > I've pushed an updated branch again, please have a look. > Thanks for addressing these. The series looks good now. Regards, Raghavendra > M. > > -- > Without deviation from the norm, progress is not possible.
