[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] x86/hvm/viridian: stop open coding updates to APIC registers




> -----Original Message-----
> From: Paul Durrant
> Sent: 10 December 2018 14:57
> To: 'Jan Beulich' <JBeulich@xxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Roger Pau Monne
> <roger.pau@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; xen-devel <xen-
> devel@xxxxxxxxxxxxxxxxxxxx>
> Subject: RE: [PATCH] x86/hvm/viridian: stop open coding updates to APIC
> registers
> 
> > -----Original Message-----
> > From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> > Sent: 10 December 2018 13:44
> > To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> > Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Roger Pau Monne
> > <roger.pau@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; xen-devel <xen-
> > devel@xxxxxxxxxxxxxxxxxxxx>
> > Subject: Re: [PATCH] x86/hvm/viridian: stop open coding updates to APIC
> > registers
> >
> > >>> On 07.12.18 at 18:50, <paul.durrant@xxxxxxxxxx> wrote:
> > > The code in viridian_synic_wrmsr() duplicates logic in
> > vlapic_reg_write()
> > > to update the ICR, ICR2 and TASKPRI registers. Instead of doing this,
> > > make vlapic_reg_write() non-static and call it.
> >
> > There's a side effect from this change, which I think should be called
> > out since perhaps it's actually a (minor) bug fix: vlapic_reg_write()
> > clears out vlapic->loaded, which the open coded Viridian code didn't
> > do.
> >
> 
> Yes, that was an anomaly and did look wrong.
> 
> > If it wasn't for this, I would have been uncertain whether exposing a
> > function that really was meant to be vlapic.c-internal is a good idea.
> >
> 
> Unfortunately the way that Microsoft specify the SynIC (as a "superset of
> the local APIC") means that the code does need to be closely tied. Do you
> want me to formulate a comment and submit a v2, or are you happy to add a
> comment on commit?

Oh, I see Andrew already committed it.

  Paul

> 
>   Paul

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.