On Fri, Oct 06, 2023 at 12:19:29PM +0100, Andrew Cooper wrote:
> On 06/10/2023 12:02 pm, Roger Pau Monné wrote:
> > On Fri, Oct 06, 2023 at 11:47:48AM +0100, Andrew Cooper wrote:
> >> diff --git a/xen/common/domain.c b/xen/common/domain.c
> >> index b8281d7cff9d..df994bd30fd2 100644
> >> --- a/xen/common/domain.c
> >> +++ b/xen/common/domain.c
> >> @@ -1998,6 +1998,10 @@ long common_vcpu_op(int cmd, struct vcpu *v
> >> {
> >> 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;
> >>
> >> and a matching one for XENFEAT_vcpu_time_phys_area because I'm even more
> >> serious about this becoming a domain controllable setting following what
> >> OSSTest had to say overnight.
> > While this is all fine, please note that the newly added code
> > {,un}map_guest_area() is also used by the existing
> > VCPUOP_register_vcpu_info hypercall, and that one can't be disabled.
>
> Yeah, I'm aware we're stuck there, but a crap interface from the past is
> not an excuse not to do new interfaces properly.
Right, want I intended to say is that if we are worried the new
{,un}map_guest_area() might be buggy, and would like to have a way to
disable it, just preventing usage of
VCPUOP_register_{runstate,vcpu_info}_phys_area won't be enough as the
newly introduced function is also used by the existing
VCPUOP_register_vcpu_info.
Not that we shouldn't add the checks, just that those won't cover all
usages of {,un}map_guest_area().
Thanks, Roger.
