On Mon, Oct 16, 2023 at 04:04:34PM +0200, Jan Beulich wrote:
> On 16.10.2023 16:01, Roger Pau Monné wrote:
> > 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.
>
> I don't follow this kind of reasoning. Why would a guest, whose code to
> use the new sub-functions has to be newly written, key its decision to
> the specific error code it gets, when at the same time you expose
> feature bits that it can use to decide whether to invoke the hypercall
> in the first place.
Because we don't control all possible guest implementations out there.
AIUI the point of introducing a way to disable the newly added
hypercalls is to make the interface between a Xen previous to the
introduction of the hypercalls vs a Xen that has the hypercalls
disabled equal, and that requires returning the same error code IMO,
or else interfaces are not equal.
Thanks, Roger.
