linux-kernel - RE: [PATCH 03/11] Drivers: hv: vmbus: Replace the per-CPU channel lists with a global array of channels

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 10 Apr 2020 17:29:43 +0000
From:   Michael Kelley <mikelley@...rosoft.com>
To:     "Andrea Parri (Microsoft)" <parri.andrea@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        Dexuan Cui <decui@...rosoft.com>,
        Boqun Feng <boqun.feng@...il.com>,
        vkuznets <vkuznets@...hat.com>
Subject: RE: [PATCH 03/11] Drivers: hv: vmbus: Replace the per-CPU channel
 lists with a global array of channels

From: Andrea Parri (Microsoft) <parri.andrea@...il.com> Sent: Sunday, April 5, 2020 5:15 PM
> 
> When Hyper-V sends an interrupt to the guest, the guest has to figure
> out which channel the interrupt is associated with.  Hyper-V sets a bit
> in a memory page that is shared with the guest, indicating a particular
> "relid" that the interrupt is associated with.  The current Linux code
> then uses a set of per-CPU linked lists to map a given "relid" to a
> pointer to a channel structure.
> 
> This design introduces a synchronization problem if the CPU that Hyper-V
> will interrupt for a certain channel is changed.  If the interrupt comes
> on the "old CPU" and the channel was already moved to the per-CPU list
> of the "new CPU", then the relid -> channel mapping will fail and the
> interrupt is dropped.  Similarly, if the interrupt comes on the new CPU
> but the channel was not moved to the per-CPU list of the new CPU, then
> the mapping will fail and the interrupt is dropped.
> 
> Relids are integers ranging from 0 to 2047.  The mapping from relids to
> channel structures can be done by setting up an array with 2048 entries,
> each entry being a pointer to a channel structure (hence total size ~16K
> bytes, which is not a problem).  The array is global, so there are no
> per-CPU linked lists to update.  The array can be searched and updated
> by loading from/storing to the array at the specified index.  With no
> per-CPU data structures, the above mentioned synchronization problem is
> avoided and the relid2channel() function gets simpler.
> 
> Suggested-by: Michael Kelley <mikelley@...rosoft.com>
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@...il.com>
> ---
> NOTE.  The inline comment in vmbus_channel_map_relid() follows the argument
> discussed in the RFC [1].  An attempt has been made to make this argument
> more 'explicit' (and, hopefully, more 'robust' against future changes) by
> adding a full memory barrier in vmbus_channel_map_relid(): the barrier takes
> the role of the 'implicit' full memory barriers from queue_work() and from
> check_ready_for_resume_event() referred to in the RFC.
> 
> [1] https://lore.kernel.org/linux-hyperv/20200403133826.GA25401@andrea/
> 
>  drivers/hv/channel_mgmt.c | 179 ++++++++++++++++++++++++--------------
>  drivers/hv/connection.c   |  38 +++-----
>  drivers/hv/hv.c           |   2 -
>  drivers/hv/hyperv_vmbus.h |  14 +--
>  drivers/hv/vmbus_drv.c    |  48 ++++++----
>  include/linux/hyperv.h    |   5 --
>  6 files changed, 160 insertions(+), 126 deletions(-)
> 

Reviewed-by: Michael Kelley <mikelley@...rosoft.com>