Re: [PATCH v2 01/11] x86/hvm: drop vcpu parameter from vlapic EOI callbacks

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Oct 02, 2020 at 10:48:07AM +0200, Jan Beulich wrote:
> On 30.09.2020 12:40, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/hvm/vlapic.c
> > +++ b/xen/arch/x86/hvm/vlapic.c
> > @@ -459,13 +459,10 @@ void vlapic_EOI_set(struct vlapic *vlapic)
> >  
> >  void vlapic_handle_EOI(struct vlapic *vlapic, u8 vector)
> >  {
> > -    struct vcpu *v = vlapic_vcpu(vlapic);
> > -    struct domain *d = v->domain;
> > -
> >      if ( vlapic_test_vector(vector, &vlapic->regs->data[APIC_TMR]) )
> > -        vioapic_update_EOI(d, vector);
> > +        vioapic_update_EOI(vector);
> >  
> > -    hvm_dpci_msi_eoi(d, vector);
> > +    hvm_dpci_msi_eoi(vector);
> >  }
> 
> What about viridian_synic_wrmsr() -> vlapic_EOI_set() ->
> vlapic_handle_EOI()? You'd probably have noticed this if you
> had tried to (consistently) drop the respective parameters from
> the intermediate functions as well.
> 
> Question of course is in how far viridian_synic_wrmsr() for
> HV_X64_MSR_EOI makes much sense when v != current. Paul, Wei?

There's already an assert at the top of viridian_synic_wrmsr of v ==
current, which I assume is why I thought this change was fine. I can
purge the passing of v (current) further, but it wasn't really needed
for the rest of the series.

> A secondary question of course is whether passing around the
> pointers isn't really cheaper than the obtaining of 'current'.

Well, while there's indeed a performance aspect here, I think
using current is much clearer than passing a vcpu around. I could
rename the parameter to curr or some such, but I think using
current and not passing a vcpu parameter is clearer.

Thanks, Roger.