Re: [Xen-devel] [PATCH v7 09/10] microcode: remove microcode_update_lock

[Chao Gao <chao.gao@xxxxxxxxx>]

On Wed, Jun 12, 2019 at 01:38:31AM -0600, Jan Beulich wrote:
>>>> On 11.06.19 at 18:04, <ashok.raj@xxxxxxxxx> wrote:
>> On Tue, Jun 11, 2019 at 08:46:04PM +0800, Chao Gao wrote:
>>> On Wed, Jun 05, 2019 at 08:53:46AM -0600, Jan Beulich wrote:
>>> >
>>> >> @@ -307,8 +303,7 @@ static int apply_microcode(const struct 
>>> >> microcode_patch 
>> *patch)
>>> >>  
>>> >>      mc_intel = patch->mc_intel;
>>> >>  
>>> >> -    /* serialize access to the physical write to MSR 0x79 */
>>> >> -    spin_lock_irqsave(&microcode_update_lock, flags);
>>> >> +    BUG_ON(local_irq_is_enabled());
>>> >>  
>>> >>      /*
>>> >>       * Writeback and invalidate caches before updating microcode to 
>>> >> avoid
>>> >
>>> >Thinking about it - what happens if we hit an NMI or #MC here?
>>> >watchdog_disable(), a call to which you add in an earlier patch,
>>> >doesn't really suppress the generation of NMIs, it only tells the
>>> >handler not to look at the accumulated statistics.
>>> 
>>> I think they should be suppressed. Ashok, could you confirm it?
>> 
>> I think the only sources would be the watchdog as you pointed out
>> which we already touch to keep it from expiring. The perf counters
>> i'm not an expert in, but i'll check. When we are in stop_machine() type
>> flow, its not clear if any of those would fire. (I might be wrong, but let
>> me check).
>
>Well, without disarming the watchdog NMI at the LAPIC / IO-APIC,
>how would it _not_ potentially fire?

We plan not to prevent NMI being fired. Instead, if one thread of a core
is updating microcode, other threads of this core would stop in the
handler of NMI until the update completion. Is this approach acceptable?

Thanks
Chao

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