Re: [Xen-devel] [PATCH v2 2/2] x86/apic: simplify disconnect_bsp_APIC setup of LVT{0/1}

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

On Mon, Jan 27, 2020 at 04:47:48PM +0000, Andrew Cooper wrote:
> On 27/01/2020 16:43, Jan Beulich wrote:
> > On 23.01.2020 19:06, Roger Pau Monne wrote:
> >> There's no need to read the current values of LVT{0/1} for the
> >> purposes of the function, which seem to be to save the currently
> >> selected vector: in the destination modes used (ExtINT and NMI) the
> >> vector field is ignored and hence can be set to 0.
> > The question is - is there firmware putting data in these fields
> > that it expects to survive unmodified? Such behavior would be
> > highly suspect (to me at least), but you never know. There ought
> > to be a reason it's been done this way not just in Xen, but also
> > in Linux. IOW may I ask that you at least make an attempt to
> > submit the same change for Linux, to see what the feedback is?
> 
> I can tell you what the Linux feedback will be.
> 
> This is not a fastpath.  Always do RMW, because life is too short to
> keep on unbreaking hardware which makes such assumptions.

We already do such kinds of direct writes to LVT{0/1}, see
clear_local_APIC where Xen clears the registers by directly writing
APIC_LVT_MASKED to them (no RMW). As the modified code follows a call
to clear_local_APIC there's nothing to preserve at this point.

Note that init_bsp_APIC and smp_callin also call clear_local_APIC, so
there's no data in those registers that could survive (regardless of
my added call to clear_local_APIC in the previous patch).

I'm not arguing this is correct (not sure it's incorrect either), but
given how things are handled ATM it seems quite dumb to do a RMW in
disconnect_bsp_APIC: it gives the false impression Xen is preserving
something, when the register has already been wiped at startup.

Thanks, Roger.

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