On Mon, Oct 16, 2023 at 03:58:22PM +0200, Jan Beulich wrote:
> On 16.10.2023 15:00, Roger Pau Monné wrote:
> > On Mon, Oct 16, 2023 at 02:35:44PM +0200, Jan Beulich wrote:
> >> On 06.10.2023 15:00, Roger Pau Monne wrote:
> >>> --- a/xen/common/domain.c
> >>> +++ b/xen/common/domain.c
> >>> @@ -1998,6 +1998,10 @@ long common_vcpu_op(int cmd, struct vcpu *v,
> >>> XEN_GUEST_HANDLE_PARAM(void) arg)
> >>> {
> >>> struct vcpu_register_runstate_memory_area area;
> >>>
> >>> + rc = -ENOSYS;
> >>> + if ( 0 /* TODO: Dom's XENFEAT_runstate_phys_area setting */ )
> >>> + break;
> >>> +
> >>> rc = -EFAULT;
> >>> if ( copy_from_guest(&area.addr.p, arg, 1) )
> >>> break;
> >>
> >> ENOSYS is not correct here. EPERM, EACCES, or EOPNOTSUPP would all be more
> >> correct.
> >
> > I don't think so, common_vcpu_op() default case does return -ENOSYS,
> > and the point of this path is to mimic the behavior of an hypervisor
> > that doesn't have the hypercalls implemented, hence -ENOSYS is the
> > correct behavior.
>
> Besides that other ENOSYS being wrong too, I question such "mimic-ing".
> Imo error codes should be the best representation of the real reason,
> not be arbitrarily "made up".
The point is for the guest to not take any action that it won't take
on an hypervisor that doesn't have the hypercalls implemented, and the
only way to be sure about that is to return the same exact error code.
Thanks, Roger.
