On Thu, Apr 08, 2021 at 08:20:15AM +0200, Jan Beulich wrote:
> On 07.04.2021 18:27, Roger Pau Monné wrote:
> > On Wed, Apr 07, 2021 at 04:55:43PM +0200, Jan Beulich wrote:
> >> On 31.03.2021 12:32, Roger Pau Monne wrote:
> >>> Add a new vlapic_set_irq_callback helper in order to inject a vector
> >>> and set a callback to be executed when the guest performs the end of
> >>> interrupt acknowledgment.
> >>>
> >>> Such functionality will be used to migrate the current ad hoc handling
> >>> done in vlapic_handle_EOI for the vectors that require some logic to
> >>> be executed when the end of interrupt is performed.
> >>>
> >>> The setter of the callback will be in charge for setting the callback
> >>> again on resume. That is the reason why vlapic_set_callback is not a
> >>> static function.
> >>
> >> I'm struggling with your use of "resume" here: Resuming from S3
> >> doesn't require re-doing anything that's kept in memory, does it?
> >> So what meaning does the word have here?
> > 
> > Right, I can see the confusion. Resume here means a guest being
> > migrated or restored, not Xen itself being resumed. Callbacks are not
> > part of the exported guest state, and hence any emulated device that
> > requires a callback will have to register it as part of loading the
> > saved state.
> > 
> >> Apart from this, and with the xzalloc_array() change requested
> >> by Andrew, this looks good to me.
> 
> In which case with this change and "resume" replaced suitably in the
> description

I've worded it as:

"The setter of the callback will be in charge for setting the callback
again on guest restore or resume, as callbacks are not saved as part
of the vlapic state. That is the reason why vlapic_set_callback is not
a static function."

> Reviewed-by: Jan Beulich <jbeul...@suse.com>

Thanks. I also want to make the warning message in vlapic_set_callback
conditional to the vector being pending in ISR or IRR, so I will not
add your RB this time if that's fine.

Roger.

Reply via email to