> -----Original Message----- > From: Roger Pau Monné <roger....@citrix.com> > Sent: 13 August 2020 09:58 > To: p...@xen.org > Cc: xen-devel@lists.xenproject.org; 'Wei Liu' <w...@xen.org>; 'Jan Beulich' > <jbeul...@suse.com>; 'Andrew > Cooper' <andrew.coop...@citrix.com> > Subject: Re: [PATCH 4/5] x86/viridian: switch synic to use the new EOI > callback > > On Thu, Aug 13, 2020 at 09:33:43AM +0100, Paul Durrant wrote: > > > -----Original Message----- > > > From: Roger Pau Monne <roger....@citrix.com> > > > Sent: 12 August 2020 13:47 > > > To: xen-devel@lists.xenproject.org > > > Cc: Roger Pau Monne <roger....@citrix.com>; Paul Durrant <p...@xen.org>; > > > Wei Liu <w...@xen.org>; Jan > > > Beulich <jbeul...@suse.com>; Andrew Cooper <andrew.coop...@citrix.com> > > > Subject: [PATCH 4/5] x86/viridian: switch synic to use the new EOI > > > callback > > > > > > Switch synic interrupts to use an EOI callback in order to execute the > > > logic tied to the end of interrupt. This allows to remove the synic > > > call in vlapic_handle_EOI. > > > > > > Move and rename viridian_synic_ack_sint now that it can be made > > > static. > > > > > > Signed-off-by: Roger Pau Monné <roger....@citrix.com> > > > --- > > > I'm unsure about the logic in viridian_synic_deliver_timer_msg, as it > > > seems to only set the vector in msg_pending when the message is > > > already pending? > > > > See section 11.10.3 of the TLFS (SynIC Message Flags)... > > > > "The MessagePending flag indicates whether or not there are any > > messages pending in the message queue of the synthetic interrupt > > source. If there are, then an “end of message” must be performed by > > the guest after emptying the message slot. This allows for > > opportunistic writes to the EOM MSR (only when required). Note that > > this flag may be set by the hypervisor upon message delivery or at > > any time afterwards. The flag should be tested after the message > > slot has been emptied and if set, then there are one or more pending > > messages and the “end of message” should be performed." > > > > IOW it's a bit like APIC assist in that it tries to avoid a VMEXIT > > (in this case an access to the EOM MSR) unless it is necessary. > > > > Reading the code again I think it may well be possible to get rid of > > the 'msg_pending' flag since it only appears to be an optimization > > to avoid testing 'message_type'. I'll try dropping it and see what > > breaks. >
Well nothing apparently broke. The EOM handler basically becomes a no-op too, but I think this is fine because we only use the synic for delivering timer messages at the moment. Paul
