Messages in this thread | | | From | Vitaly Kuznetsov <> | Subject | Re: [PATCH char-misc 1/1] Drivers: hv: vmbus: Make synic_initialized flag per-cpu | Date | Wed, 01 Aug 2018 11:26:13 +0200 |
| |
"Michael Kelley (EOSG)" <Michael.H.Kelley@microsoft.com> writes:
> From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Tuesday, July 31, 2018 4:20 AM >> >> Alternatively, we can get rid of synic_initialized flag altogether: >> hv_synic_init() never fails in the first place but we can always >> implement something like: >> >> int hv_synic_is_initialized(void) { >> union hv_synic_scontrol sctrl; >> >> hv_get_synic_state(sctrl.as_uint64); >> >> return sctrl.enable; >> } >> >> as it doesn't seem that we need to check synic state on _other_ CPUs. >> > > I was trying to decide if there are any arguments in favor of one > approach vs. the other: a per-cpu flag in memory or checking > the synic_control "enable" bit. Seems like a wash to me, in which > case I have a slight preference for the per-cpu flag in memory vs. > creating another function to return sctrl.enable. But I'm completely > open to reasons why checking sctrl.enable is better.
Just a few thoughts: reading MSR is definitely slower but we avoid 'shadowing' the state, the reading is always correct. In case there's a chance the SynIC will get disabled from host side we can only find this out by doing MSR read. This is a purely theoretical possibility, I believe, we can go ahead with this patch.
-- Vitaly
| |