On Tue, Apr 26, 2022 at 02:53:54PM -0400, Tamas K Lengyel wrote: > On Tue, Apr 26, 2022 at 4:50 AM Roger Pau Monné <[email protected]> wrote: > > > > On Mon, Apr 25, 2022 at 11:40:11AM -0400, Tamas K Lengyel wrote: > > > On Mon, Apr 25, 2022 at 10:41 AM Roger Pau Monné <[email protected]> > > > wrote: > > > > > > > > On Wed, Apr 13, 2022 at 09:41:52AM -0400, Tamas K Lengyel wrote: > > > > > diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c > > > > > index 3079726a8b..30ca71432c 100644 > > > > > --- a/xen/arch/x86/monitor.c > > > > > +++ b/xen/arch/x86/monitor.c > > > > > @@ -332,6 +332,20 @@ int arch_monitor_domctl_event(struct domain *d, > > > > > break; > > > > > } > > > > > > > > > > + case XEN_DOMCTL_MONITOR_EVENT_VMEXIT: > > > > > + { > > > > > + bool old_status = ad->monitor.vmexit_enabled; > > > > > + > > > > > + if ( unlikely(old_status == requested_status) ) > > > > > + return -EEXIST; > > > > > > > > What about if the requested status is the same as the current one, but > > > > vmexit sync is not? > > > > > > You need to clear the currently registered event first, then register > > > the new one. > > > > > > > IOW, I'm not sure this check is helpful, and you could likely avoid > > > > the old_status local variable. > > > > > > It is helpful on the callee side. Usually the callee needs to keep > > > track of the state of what events are enabled so that it can clean up > > > after itself and if it ever runs into trying to set the event to > > > something it's already set to then that indicates its state being > > > out-of-sync. > > > > Hm, right. I wonder if you should also check that the ring is empty > > before changing the status? So that the callee doesn't change the > > status while requests are still pending on the ring from the previous > > type? > > No, that becomes tricky because really the only way to ensure the ring > remains empty from the userspace is to pause the domain, which is very > heavy handed. There is nothing wrong with asking Xen not to produce > more of a certain type of request while still being able to handle the > ones that are already on the ring. For setups where the two should > happen at the same time is where the toolstack first pauses the > domain, clears the ring, then disables the event. Both are valid > approaches.
OK, so we rely on the callee to drain the ring properly when wanting to change VMEXIT events. Thanks, Roger.
