* Re: [RFC PATCH 01/13] vsock/vmci: remove unused VSOCK_DEFAULT_CONNECT_TIMEOUT
From: Stefan Hajnoczi @ 2019-10-09 11:41 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20190927112703.17745-2-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 549 bytes --]
On Fri, Sep 27, 2019 at 01:26:51PM +0200, Stefano Garzarella wrote:
> The VSOCK_DEFAULT_CONNECT_TIMEOUT definition was introduced with
> commit d021c344051af ("VSOCK: Introduce VM Sockets"), but it is
> never used in the net/vmw_vsock/vmci_transport.c.
>
> VSOCK_DEFAULT_CONNECT_TIMEOUT is used and defined in
> net/vmw_vsock/af_vsock.c
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> net/vmw_vsock/vmci_transport.c | 5 -----
> 1 file changed, 5 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 02/13] vsock: remove vm_sockets_get_local_cid()
From: Stefan Hajnoczi @ 2019-10-09 11:42 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20190927112703.17745-3-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 614 bytes --]
On Fri, Sep 27, 2019 at 01:26:52PM +0200, Stefano Garzarella wrote:
> vm_sockets_get_local_cid() is only used in virtio_transport_common.c.
> We can replace it calling the virtio_transport_get_ops() and
> using the get_local_cid() callback registered by the transport.
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> include/linux/vm_sockets.h | 2 --
> net/vmw_vsock/af_vsock.c | 10 ----------
> net/vmw_vsock/virtio_transport_common.c | 2 +-
> 3 files changed, 1 insertion(+), 13 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 03/13] vsock: remove include/linux/vm_sockets.h file
From: Stefan Hajnoczi @ 2019-10-09 11:43 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20190927112703.17745-4-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 619 bytes --]
On Fri, Sep 27, 2019 at 01:26:53PM +0200, Stefano Garzarella wrote:
> This header file now only includes the "uapi/linux/vm_sockets.h".
> We can include directly it when needed.
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> include/linux/vm_sockets.h | 13 -------------
> include/net/af_vsock.h | 2 +-
> include/net/vsock_addr.h | 2 +-
> net/vmw_vsock/vmci_transport_notify.h | 1 -
> 4 files changed, 2 insertions(+), 16 deletions(-)
> delete mode 100644 include/linux/vm_sockets.h
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 04/13] vsock: add 'transport' member in the struct vsock_sock
From: Stefan Hajnoczi @ 2019-10-09 11:46 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20190927112703.17745-5-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 741 bytes --]
On Fri, Sep 27, 2019 at 01:26:54PM +0200, Stefano Garzarella wrote:
> As a preparation to support multiple transports, this patch adds
> the 'transport' member at the 'struct vsock_sock'.
> This new field is initialized during the creation in the
> __vsock_create() function.
>
> This patch also renames the global 'transport' pointer to
> 'transport_single', since for now we're only supporting a single
> transport registered at run-time.
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> include/net/af_vsock.h | 1 +
> net/vmw_vsock/af_vsock.c | 56 +++++++++++++++++++++++++++-------------
> 2 files changed, 39 insertions(+), 18 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 05/13] vsock/virtio: add transport parameter to the virtio_transport_reset_no_sock()
From: Stefan Hajnoczi @ 2019-10-09 11:52 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20190927112703.17745-6-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]
On Fri, Sep 27, 2019 at 01:26:55PM +0200, Stefano Garzarella wrote:
> We are going to add 'struct vsock_sock *' parameter to
> virtio_transport_get_ops().
>
> In some cases, like in the virtio_transport_reset_no_sock(),
> we don't have any socket assigned to the packet received,
> so we can't use the virtio_transport_get_ops().
>
> In order to allow virtio_transport_reset_no_sock() to use the
> '.send_pkt' callback from the 'vhost_transport' or 'virtio_transport',
> we add the 'struct virtio_transport *' to it and to its caller:
> virtio_transport_recv_pkt().
>
> We moved the 'vhost_transport' and 'virtio_transport' definition,
> to pass their address to the virtio_transport_recv_pkt().
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> drivers/vhost/vsock.c | 94 +++++++-------
> include/linux/virtio_vsock.h | 3 +-
> net/vmw_vsock/virtio_transport.c | 160 ++++++++++++------------
> net/vmw_vsock/virtio_transport_common.c | 12 +-
> 4 files changed, 135 insertions(+), 134 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 06/13] vsock: add 'struct vsock_sock *' param to vsock_core_get_transport()
From: Stefan Hajnoczi @ 2019-10-09 11:54 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20190927112703.17745-7-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 554 bytes --]
On Fri, Sep 27, 2019 at 01:26:56PM +0200, Stefano Garzarella wrote:
> -const struct vsock_transport *vsock_core_get_transport(void)
> +const struct vsock_transport *vsock_core_get_transport(struct vsock_sock *vsk)
> {
> /* vsock_register_mutex not taken since only the transport uses this
> * function and only while registered.
> */
> - return transport_single;
This comment is about protecting transport_single. It no longer applies
when using vsk->transport. Please drop it.
Otherwise:
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 07/13] vsock: handle buffer_size sockopts in the core
From: Stefan Hajnoczi @ 2019-10-09 12:30 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20190927112703.17745-8-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2332 bytes --]
On Fri, Sep 27, 2019 at 01:26:57PM +0200, Stefano Garzarella wrote:
> @@ -140,18 +145,11 @@ struct vsock_transport {
> struct vsock_transport_send_notify_data *);
> int (*notify_send_post_enqueue)(struct vsock_sock *, ssize_t,
> struct vsock_transport_send_notify_data *);
> + int (*notify_buffer_size)(struct vsock_sock *, u64 *);
Is ->notify_buffer_size() called under lock_sock(sk)? If yes, please
document it.
> +static void vsock_update_buffer_size(struct vsock_sock *vsk,
> + const struct vsock_transport *transport,
> + u64 val)
> +{
> + if (val > vsk->buffer_max_size)
> + val = vsk->buffer_max_size;
> +
> + if (val < vsk->buffer_min_size)
> + val = vsk->buffer_min_size;
> +
> + if (val != vsk->buffer_size &&
> + transport && transport->notify_buffer_size)
> + transport->notify_buffer_size(vsk, &val);
Why does this function return an int if we don't check the return value?
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index fc046c071178..bac9e7430a2e 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -403,17 +403,13 @@ int virtio_transport_do_socket_init(struct vsock_sock *vsk,
> if (psk) {
> struct virtio_vsock_sock *ptrans = psk->trans;
>
> - vvs->buf_size = ptrans->buf_size;
> - vvs->buf_size_min = ptrans->buf_size_min;
> - vvs->buf_size_max = ptrans->buf_size_max;
> vvs->peer_buf_alloc = ptrans->peer_buf_alloc;
> - } else {
> - vvs->buf_size = VIRTIO_VSOCK_DEFAULT_BUF_SIZE;
> - vvs->buf_size_min = VIRTIO_VSOCK_DEFAULT_MIN_BUF_SIZE;
> - vvs->buf_size_max = VIRTIO_VSOCK_DEFAULT_MAX_BUF_SIZE;
> }
>
> - vvs->buf_alloc = vvs->buf_size;
> + if (vsk->buffer_size > VIRTIO_VSOCK_MAX_BUF_SIZE)
> + vsk->buffer_size = VIRTIO_VSOCK_MAX_BUF_SIZE;
Hmm...this could be outside the [min, max] range. I'm not sure how much
it matters.
Another issue is that this patch drops the VIRTIO_VSOCK_MAX_BUF_SIZE
limit that used to be enforced by virtio_transport_set_buffer_size().
Now the limit is only applied at socket init time. If the buffer size
is changed later then VIRTIO_VSOCK_MAX_BUF_SIZE can be exceeded. If
that doesn't matter, why even bother with VIRTIO_VSOCK_MAX_BUF_SIZE
here?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 08/13] vsock: move vsock_insert_unbound() in the vsock_create()
From: Stefan Hajnoczi @ 2019-10-09 12:34 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20190927112703.17745-9-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 960 bytes --]
On Fri, Sep 27, 2019 at 01:26:58PM +0200, Stefano Garzarella wrote:
> vsock_insert_unbound() was called only when 'sock' parameter of
> __vsock_create() was not null. This only happened when
> __vsock_create() was called by vsock_create().
>
> In order to simplify the multi-transports support, this patch
> moves vsock_insert_unbound() at the end of vsock_create().
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> net/vmw_vsock/af_vsock.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
Maybe transports shouldn't call __vsock_create() directly. They always
pass NULL as the parent socket, so we could have a more specific
function that transports call without a parent sock argument. This
would eliminate any concern over moving vsock_insert_unbound() out of
this function. In any case, I've checked the code and this patch is
correct.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 09/13] hv_sock: set VMADDR_CID_HOST in the hvs_remote_addr_init()
From: Stefan Hajnoczi @ 2019-10-09 12:35 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20190927112703.17745-10-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 394 bytes --]
On Fri, Sep 27, 2019 at 01:26:59PM +0200, Stefano Garzarella wrote:
> Remote peer is always the host, so we set VMADDR_CID_HOST as
> remote CID instead of VMADDR_CID_ANY.
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> net/vmw_vsock/hyperv_transport.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 10/13] vsock: add multi-transports support
From: Stefan Hajnoczi @ 2019-10-09 13:11 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20190927112703.17745-11-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2463 bytes --]
On Fri, Sep 27, 2019 at 01:27:00PM +0200, Stefano Garzarella wrote:
> RFC:
> - I'd like to move MODULE_ALIAS_NETPROTO(PF_VSOCK) to af_vsock.c.
> @Jorgen could this break the VMware products?
What will cause the vmw_vsock_vmci_transport.ko module to be loaded
after you remove MODULE_ALIAS_NETPROTO(PF_VSOCK)? Perhaps
drivers/misc/vmw_vmci/vmci_guest.c:vmci_guest_probe_device() could do
something when the guest driver loads. There would need to be something
equivalent for the host side too.
This will solve another issue too. Today the VMCI transport can be
loaded if an application creates an AF_VSOCK socket during early boot
before the virtio transport has been probed. This happens because the
VMCI transport uses MODULE_ALIAS_NETPROTO(PF_VSOCK) *and* it does not
probe whether this system is actually a VMware guest.
If we instead load the core af_vsock.ko module and transports are only
loaded based on hardware feature probing (e.g. the presence of VMware
guest mode, a virtio PCI adapter, etc) then transports will be
well-behaved.
> - DGRAM sockets are handled as before, I don't know if make sense work
> on it now, or when another transport will support DGRAM. The big
> issues here is that we cannot link 1-1 a socket to transport as
> for stream sockets since DGRAM is not connection-oriented.
Let's ignore DGRAM for now since only VMCI supports it and we therefore
do not require multi-transport support.
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> index 86f8f463e01a..2a081d19e20d 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -94,7 +94,13 @@ struct vsock_transport_send_notify_data {
> u64 data2; /* Transport-defined. */
> };
>
> +#define VSOCK_TRANSPORT_F_H2G 0x00000001
> +#define VSOCK_TRANSPORT_F_G2H 0x00000002
> +#define VSOCK_TRANSPORT_F_DGRAM 0x00000004
Documentation comments, please.
> +void vsock_core_unregister(const struct vsock_transport *t)
> +{
> + mutex_lock(&vsock_register_mutex);
> +
> + /* RFC-TODO: maybe we should check if there are open sockets
> + * assigned to that transport and avoid the unregistration
> + */
If unregister() is only called from module_exit() functions then holding
a reference to the transport module would be enough to prevent this
case. The transport could only be removed once all sockets have been
destroyed (and dropped their transport module reference).
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 11/13] vsock: add 'transport_hg' to handle g2h\h2g transports
From: Stefan Hajnoczi @ 2019-10-09 13:16 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20190927112703.17745-12-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 648 bytes --]
On Fri, Sep 27, 2019 at 01:27:01PM +0200, Stefano Garzarella wrote:
> VMCI transport provides both g2h and h2g behaviors in a single
> transport.
> We are able to set (or not) the g2h behavior, detecting if we
> are in a VMware guest (or not), but the h2g feature is always set.
> This prevents to load other h2g transports while we are in a
> VMware guest.
In the vhost_vsock.ko case we only register the h2g transport when
userspace has loaded the module (by opening /dev/vhost-vsock).
VMCI has something kind of similar: /dev/vmci and the
vmci_host_active_users counter. Maybe we can use this instead of
introducing the transport_hg concept?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 12/13] vsock: prevent transport modules unloading
From: Stefan Hajnoczi @ 2019-10-09 13:28 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20190927112703.17745-13-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 582 bytes --]
On Fri, Sep 27, 2019 at 01:27:02PM +0200, Stefano Garzarella wrote:
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index c5f46b8242ce..750b62711b01 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -416,13 +416,28 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
> return -ESOCKTNOSUPPORT;
> }
>
> - if (!vsk->transport)
> + /* We increase the module refcnt to prevent the tranport unloading
s/tranport/transport/
Otherwise:
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 13/13] vsock: fix bind() behaviour taking care of CID
From: Stefan Hajnoczi @ 2019-10-09 13:29 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20190927112703.17745-14-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 630 bytes --]
On Fri, Sep 27, 2019 at 01:27:03PM +0200, Stefano Garzarella wrote:
> When we are looking for a socket bound to a specific address,
> we also have to take into account the CID.
>
> This patch is useful with multi-transports support because it
> allows the binding of the same port with different CID, and
> it prevents a connection to a wrong socket bound to the same
> port, but with different CID.
>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> net/vmw_vsock/af_vsock.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 00/13] vsock: add multi-transports support
From: Stefan Hajnoczi @ 2019-10-09 13:29 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20190927112703.17745-1-sgarzare@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 299 bytes --]
On Fri, Sep 27, 2019 at 01:26:50PM +0200, Stefano Garzarella wrote:
> Hi all,
> this series adds the multi-transports support to vsock, following
> this proposal:
> https://www.spinics.net/lists/netdev/msg575792.html
Nice series! I have left a few comments but overall it looks promising.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH v3] x86/hyperv: make vapic support x2apic mode
From: Roman Kagan @ 2019-10-09 14:50 UTC (permalink / raw)
To: Michael Kelley, Lan Tianyu, Joerg Roedel, K. Y. Srinivasan,
Haiyang Zhang, Stephen Hemminger, Sasha Levin, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86@kernel.org,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: kvm@vger.kernel.org, Vitaly Kuznetsov
Now that there's Hyper-V IOMMU driver, Linux can switch to x2apic mode
when supported by the vcpus.
However, the apic access functions for Hyper-V enlightened apic assume
xapic mode only.
As a result, Linux fails to bring up secondary cpus when run as a guest
in QEMU/KVM with both hv_apic and x2apic enabled.
According to Michael Kelley, when in x2apic mode, the Hyper-V synthetic
apic MSRs behave exactly the same as the corresponding architectural
x2apic MSRs, so there's no need to override the apic accessors. The
only exception is hv_apic_eoi_write, which benefits from lazy EOI when
available; however, its implementation works for both xapic and x2apic
modes.
Fixes: 29217a474683 ("iommu/hyper-v: Add Hyper-V stub IOMMU driver")
Fixes: 6b48cb5f8347 ("X86/Hyper-V: Enlighten APIC access")
Cc: stable@vger.kernel.org
Suggested-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
v2 -> v3:
- do not introduce x2apic-capable hv_apic accessors; leave original
x2apic accessors instead
v1 -> v2:
- add ifdefs to handle !CONFIG_X86_X2APIC
arch/x86/hyperv/hv_apic.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index 5c056b8aebef..26eeff5bd535 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -261,10 +261,19 @@ void __init hv_apic_init(void)
if (ms_hyperv.hints & HV_X64_APIC_ACCESS_RECOMMENDED) {
pr_info("Hyper-V: Using MSR based APIC access\n");
+ /*
+ * With x2apic, architectural x2apic MSRs are equivalent to the
+ * respective synthetic MSRs, so there's no need to override
+ * the apic accessors. The only exception is
+ * hv_apic_eoi_write, because it benefits from lazy EOI when
+ * available, but it works for both xapic and x2apic modes.
+ */
apic_set_eoi_write(hv_apic_eoi_write);
- apic->read = hv_apic_read;
- apic->write = hv_apic_write;
- apic->icr_write = hv_apic_icr_write;
- apic->icr_read = hv_apic_icr_read;
+ if (!x2apic_enabled()) {
+ apic->read = hv_apic_read;
+ apic->write = hv_apic_write;
+ apic->icr_write = hv_apic_icr_write;
+ apic->icr_read = hv_apic_icr_read;
+ }
}
}
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v3] x86/hyperv: make vapic support x2apic mode
From: Vitaly Kuznetsov @ 2019-10-09 15:27 UTC (permalink / raw)
To: Roman Kagan, Michael Kelley
Cc: kvm@vger.kernel.org, Lan Tianyu, Joerg Roedel, K. Y. Srinivasan,
Haiyang Zhang, Stephen Hemminger, Sasha Levin, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86@kernel.org,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20191009145022.28442-1-rkagan@virtuozzo.com>
Roman Kagan <rkagan@virtuozzo.com> writes:
> Now that there's Hyper-V IOMMU driver, Linux can switch to x2apic mode
> when supported by the vcpus.
>
> However, the apic access functions for Hyper-V enlightened apic assume
> xapic mode only.
>
> As a result, Linux fails to bring up secondary cpus when run as a guest
> in QEMU/KVM with both hv_apic and x2apic enabled.
>
> According to Michael Kelley, when in x2apic mode, the Hyper-V synthetic
> apic MSRs behave exactly the same as the corresponding architectural
> x2apic MSRs, so there's no need to override the apic accessors. The
> only exception is hv_apic_eoi_write, which benefits from lazy EOI when
> available; however, its implementation works for both xapic and x2apic
> modes.
>
> Fixes: 29217a474683 ("iommu/hyper-v: Add Hyper-V stub IOMMU driver")
> Fixes: 6b48cb5f8347 ("X86/Hyper-V: Enlighten APIC access")
> Cc: stable@vger.kernel.org
> Suggested-by: Michael Kelley <mikelley@microsoft.com>
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> ---
> v2 -> v3:
> - do not introduce x2apic-capable hv_apic accessors; leave original
> x2apic accessors instead
>
> v1 -> v2:
> - add ifdefs to handle !CONFIG_X86_X2APIC
>
> arch/x86/hyperv/hv_apic.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> index 5c056b8aebef..26eeff5bd535 100644
> --- a/arch/x86/hyperv/hv_apic.c
> +++ b/arch/x86/hyperv/hv_apic.c
> @@ -261,10 +261,19 @@ void __init hv_apic_init(void)
>
> if (ms_hyperv.hints & HV_X64_APIC_ACCESS_RECOMMENDED) {
> pr_info("Hyper-V: Using MSR based APIC access\n");
This pr_info() becomes a bit misleading in x2apic mode, maybe do
something like
pr_info("Hyper-V: using Enlightened APIC (%s mode)",
x2apic_enabled() ? "x2apic" : "xapic");
> + /*
> + * With x2apic, architectural x2apic MSRs are equivalent to the
> + * respective synthetic MSRs, so there's no need to override
> + * the apic accessors. The only exception is
> + * hv_apic_eoi_write, because it benefits from lazy EOI when
> + * available, but it works for both xapic and x2apic modes.
> + */
> apic_set_eoi_write(hv_apic_eoi_write);
> - apic->read = hv_apic_read;
> - apic->write = hv_apic_write;
> - apic->icr_write = hv_apic_icr_write;
> - apic->icr_read = hv_apic_icr_read;
> + if (!x2apic_enabled()) {
> + apic->read = hv_apic_read;
> + apic->write = hv_apic_write;
> + apic->icr_write = hv_apic_icr_write;
> + apic->icr_read = hv_apic_icr_read;
> + }
> }
> }
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply
* RE: [PATCH v3] x86/hyperv: make vapic support x2apic mode
From: Michael Kelley @ 2019-10-09 15:41 UTC (permalink / raw)
To: vkuznets, Roman Kagan
Cc: kvm@vger.kernel.org, Tianyu Lan, Joerg Roedel, KY Srinivasan,
Haiyang Zhang, Stephen Hemminger, Sasha Levin, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86@kernel.org,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <87r23mx7lh.fsf@vitty.brq.redhat.com>
From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Wednesday, October 9, 2019 8:27 AM
>
> Roman Kagan <rkagan@virtuozzo.com> writes:
>
> > Now that there's Hyper-V IOMMU driver, Linux can switch to x2apic mode
> > when supported by the vcpus.
> >
> > However, the apic access functions for Hyper-V enlightened apic assume
> > xapic mode only.
> >
> > As a result, Linux fails to bring up secondary cpus when run as a guest
> > in QEMU/KVM with both hv_apic and x2apic enabled.
> >
> > According to Michael Kelley, when in x2apic mode, the Hyper-V synthetic
> > apic MSRs behave exactly the same as the corresponding architectural
> > x2apic MSRs, so there's no need to override the apic accessors. The
> > only exception is hv_apic_eoi_write, which benefits from lazy EOI when
> > available; however, its implementation works for both xapic and x2apic
> > modes.
> >
> > Fixes: 29217a474683 ("iommu/hyper-v: Add Hyper-V stub IOMMU driver")
> > Fixes: 6b48cb5f8347 ("X86/Hyper-V: Enlighten APIC access")
> > Cc: stable@vger.kernel.org
> > Suggested-by: Michael Kelley <mikelley@microsoft.com>
> > Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> > ---
> > v2 -> v3:
> > - do not introduce x2apic-capable hv_apic accessors; leave original
> > x2apic accessors instead
> >
> > v1 -> v2:
> > - add ifdefs to handle !CONFIG_X86_X2APIC
> >
> > arch/x86/hyperv/hv_apic.c | 17 +++++++++++++----
> > 1 file changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> > index 5c056b8aebef..26eeff5bd535 100644
> > --- a/arch/x86/hyperv/hv_apic.c
> > +++ b/arch/x86/hyperv/hv_apic.c
> > @@ -261,10 +261,19 @@ void __init hv_apic_init(void)
> >
> > if (ms_hyperv.hints & HV_X64_APIC_ACCESS_RECOMMENDED) {
> > pr_info("Hyper-V: Using MSR based APIC access\n");
>
> This pr_info() becomes a bit misleading in x2apic mode, maybe do
> something like
>
> pr_info("Hyper-V: using Enlightened APIC (%s mode)",
> x2apic_enabled() ? "x2apic" : "xapic");
Yes, I like this. But tweak the capitalization of the message:
pr_info("Hyper-V: Using enlightened APIC (%s mode)",
>
> > + /*
> > + * With x2apic, architectural x2apic MSRs are equivalent to the
> > + * respective synthetic MSRs, so there's no need to override
> > + * the apic accessors. The only exception is
> > + * hv_apic_eoi_write, because it benefits from lazy EOI when
> > + * available, but it works for both xapic and x2apic modes.
> > + */
> > apic_set_eoi_write(hv_apic_eoi_write);
> > - apic->read = hv_apic_read;
> > - apic->write = hv_apic_write;
> > - apic->icr_write = hv_apic_icr_write;
> > - apic->icr_read = hv_apic_icr_read;
> > + if (!x2apic_enabled()) {
> > + apic->read = hv_apic_read;
> > + apic->write = hv_apic_write;
> > + apic->icr_write = hv_apic_icr_write;
> > + apic->icr_read = hv_apic_icr_read;
> > + }
> > }
> > }
>
> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* [PATCH] mm/resource: Move child to new resource when release mem region.
From: lantianyu1986 @ 2019-10-10 7:28 UTC (permalink / raw)
To: dan.j.williams, dave.hansen, mingo, mpe, pasha.tatashin,
osalvador, richardw.yang, Tianyu.Lan, christophe.leroy, bp,
rdunlap, michael.h.kelley, kys, sashal
Cc: linux-kernel, vkuznets, linux-hyperv
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
When release mem region, old mem region may be splited to
two regions. Current allocate new struct resource for high
end mem region but not move child resources whose ranges are
in the high end range to new resource. When adjust old mem
region's range, adjust_resource() detects child region's range
is out of new range and return error. Move child resources to
high end resource before adjusting old mem range.
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
This patch is to prepare for memory hot-remove function
in Hyper-V balloon driver.
---
kernel/resource.c | 38 ++++++++++++++++++++++++++++++++++----
1 file changed, 34 insertions(+), 4 deletions(-)
diff --git a/kernel/resource.c b/kernel/resource.c
index 158f04ec1d4f..7856347adfd2 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -181,6 +181,38 @@ static struct resource *alloc_resource(gfp_t flags)
return res;
}
+static void move_child_to_newresource(struct resource *old,
+ struct resource *new)
+{
+ struct resource *tmp, **p, **np;
+
+ if (!old->child)
+ return;
+
+ p = &old->child;
+ np = &new->child;
+
+ for (;;) {
+ tmp = *p;
+ if (!tmp)
+ break;
+
+ if (tmp->start >= new->start && tmp->end <= new->end) {
+ tmp->parent = new;
+ *np = tmp;
+ np = &tmp->sibling;
+ *p = tmp->sibling;
+
+ if (!tmp->sibling)
+ *np = NULL;
+ continue;
+ }
+
+ p = &tmp->sibling;
+ }
+}
+
/* Return the conflict entry if you can't request it */
static struct resource * __request_resource(struct resource *root, struct resource *new)
{
@@ -1231,9 +1263,6 @@ EXPORT_SYMBOL(__release_region);
* Note:
* - Additional release conditions, such as overlapping region, can be
* supported after they are confirmed as valid cases.
- * - When a busy memory resource gets split into two entries, the code
- * assumes that all children remain in the lower address entry for
- * simplicity. Enhance this logic when necessary.
*/
int release_mem_region_adjustable(struct resource *parent,
resource_size_t start, resource_size_t size)
@@ -1316,11 +1345,12 @@ int release_mem_region_adjustable(struct resource *parent,
new_res->sibling = res->sibling;
new_res->child = NULL;
+ move_child_to_newresource(res, new_res);
+ res->sibling = new_res;
ret = __adjust_resource(res, res->start,
start - res->start);
if (ret)
break;
- res->sibling = new_res;
new_res = NULL;
}
--
2.14.5
^ permalink raw reply related
* Re: [RFC PATCH 06/13] vsock: add 'struct vsock_sock *' param to vsock_core_get_transport()
From: Stefano Garzarella @ 2019-10-10 8:50 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20191009115433.GG5747@stefanha-x1.localdomain>
On Wed, Oct 09, 2019 at 12:54:33PM +0100, Stefan Hajnoczi wrote:
> On Fri, Sep 27, 2019 at 01:26:56PM +0200, Stefano Garzarella wrote:
> > -const struct vsock_transport *vsock_core_get_transport(void)
> > +const struct vsock_transport *vsock_core_get_transport(struct vsock_sock *vsk)
> > {
> > /* vsock_register_mutex not taken since only the transport uses this
> > * function and only while registered.
> > */
> > - return transport_single;
>
> This comment is about protecting transport_single. It no longer applies
> when using vsk->transport. Please drop it.
Right, dropped.
>
> Otherwise:
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Thanks,
Stefano
^ permalink raw reply
* Re: [RFC PATCH 00/13] vsock: add multi-transports support
From: Stefano Garzarella @ 2019-10-10 8:45 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20191009132952.GO5747@stefanha-x1.localdomain>
On Wed, Oct 09, 2019 at 02:29:52PM +0100, Stefan Hajnoczi wrote:
> On Fri, Sep 27, 2019 at 01:26:50PM +0200, Stefano Garzarella wrote:
> > Hi all,
> > this series adds the multi-transports support to vsock, following
> > this proposal:
> > https://www.spinics.net/lists/netdev/msg575792.html
>
> Nice series! I have left a few comments but overall it looks promising.
Thank you very much for the comments!
I'll follow them and respin.
Cheers,
Stefano
^ permalink raw reply
* Re: [RFC PATCH 07/13] vsock: handle buffer_size sockopts in the core
From: Stefano Garzarella @ 2019-10-10 9:32 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20191009123026.GH5747@stefanha-x1.localdomain>
On Wed, Oct 09, 2019 at 01:30:26PM +0100, Stefan Hajnoczi wrote:
> On Fri, Sep 27, 2019 at 01:26:57PM +0200, Stefano Garzarella wrote:
> > @@ -140,18 +145,11 @@ struct vsock_transport {
> > struct vsock_transport_send_notify_data *);
> > int (*notify_send_post_enqueue)(struct vsock_sock *, ssize_t,
> > struct vsock_transport_send_notify_data *);
> > + int (*notify_buffer_size)(struct vsock_sock *, u64 *);
>
> Is ->notify_buffer_size() called under lock_sock(sk)? If yes, please
> document it.
Yes, it is. I'll document it!
>
> > +static void vsock_update_buffer_size(struct vsock_sock *vsk,
> > + const struct vsock_transport *transport,
> > + u64 val)
> > +{
> > + if (val > vsk->buffer_max_size)
> > + val = vsk->buffer_max_size;
> > +
> > + if (val < vsk->buffer_min_size)
> > + val = vsk->buffer_min_size;
> > +
> > + if (val != vsk->buffer_size &&
> > + transport && transport->notify_buffer_size)
> > + transport->notify_buffer_size(vsk, &val);
>
> Why does this function return an int if we don't check the return value?
>
Copy and past :-(
I'll fix it returning void since I don't think it can fail.
> > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > index fc046c071178..bac9e7430a2e 100644
> > --- a/net/vmw_vsock/virtio_transport_common.c
> > +++ b/net/vmw_vsock/virtio_transport_common.c
> > @@ -403,17 +403,13 @@ int virtio_transport_do_socket_init(struct vsock_sock *vsk,
> > if (psk) {
> > struct virtio_vsock_sock *ptrans = psk->trans;
> >
> > - vvs->buf_size = ptrans->buf_size;
> > - vvs->buf_size_min = ptrans->buf_size_min;
> > - vvs->buf_size_max = ptrans->buf_size_max;
> > vvs->peer_buf_alloc = ptrans->peer_buf_alloc;
> > - } else {
> > - vvs->buf_size = VIRTIO_VSOCK_DEFAULT_BUF_SIZE;
> > - vvs->buf_size_min = VIRTIO_VSOCK_DEFAULT_MIN_BUF_SIZE;
> > - vvs->buf_size_max = VIRTIO_VSOCK_DEFAULT_MAX_BUF_SIZE;
> > }
> >
> > - vvs->buf_alloc = vvs->buf_size;
> > + if (vsk->buffer_size > VIRTIO_VSOCK_MAX_BUF_SIZE)
> > + vsk->buffer_size = VIRTIO_VSOCK_MAX_BUF_SIZE;
>
> Hmm...this could be outside the [min, max] range. I'm not sure how much
> it matters.
The core guarantees that vsk->buffer_size is <= of the max, so since we are
lowering it, the max should be respected. For the min you are right,
but I think this limit is stricter than the min set by the user.
>
> Another issue is that this patch drops the VIRTIO_VSOCK_MAX_BUF_SIZE
> limit that used to be enforced by virtio_transport_set_buffer_size().
> Now the limit is only applied at socket init time. If the buffer size
> is changed later then VIRTIO_VSOCK_MAX_BUF_SIZE can be exceeded. If
> that doesn't matter, why even bother with VIRTIO_VSOCK_MAX_BUF_SIZE
> here?
>
The .notify_buffer_size() should avoid this issue, since it allows the
transport to limit the buffer size requested after the initialization.
But again the min set by the user can not be respected and in the
previous implementation we forced it to VIRTIO_VSOCK_MAX_BUF_SIZE.
Now we don't limit the min, but we guarantee only that vsk->buffer_size
is lower than VIRTIO_VSOCK_MAX_BUF_SIZE.
Can that be an acceptable compromise?
Thanks,
Stefano
^ permalink raw reply
* Re: [RFC PATCH 08/13] vsock: move vsock_insert_unbound() in the vsock_create()
From: Stefano Garzarella @ 2019-10-10 9:52 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20191009123423.GI5747@stefanha-x1.localdomain>
On Wed, Oct 09, 2019 at 01:34:23PM +0100, Stefan Hajnoczi wrote:
> On Fri, Sep 27, 2019 at 01:26:58PM +0200, Stefano Garzarella wrote:
> > vsock_insert_unbound() was called only when 'sock' parameter of
> > __vsock_create() was not null. This only happened when
> > __vsock_create() was called by vsock_create().
> >
> > In order to simplify the multi-transports support, this patch
> > moves vsock_insert_unbound() at the end of vsock_create().
> >
> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> > net/vmw_vsock/af_vsock.c | 13 +++++++++----
> > 1 file changed, 9 insertions(+), 4 deletions(-)
>
> Maybe transports shouldn't call __vsock_create() directly. They always
> pass NULL as the parent socket, so we could have a more specific
> function that transports call without a parent sock argument. This
> would eliminate any concern over moving vsock_insert_unbound() out of
> this function. In any case, I've checked the code and this patch is
> correct.
Yes, I agree with you, I can add a new patch to do this cleaning.
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Thanks,
Stefano
^ permalink raw reply
* [PATCH v4] x86/hyperv: make vapic support x2apic mode
From: Roman Kagan @ 2019-10-10 12:33 UTC (permalink / raw)
To: Michael Kelley, Lan Tianyu, Joerg Roedel, K. Y. Srinivasan,
Haiyang Zhang, Stephen Hemminger, Sasha Levin, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86@kernel.org,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
Vitaly Kuznetsov
Cc: kvm@vger.kernel.org
Now that there's Hyper-V IOMMU driver, Linux can switch to x2apic mode
when supported by the vcpus.
However, the apic access functions for Hyper-V enlightened apic assume
xapic mode only.
As a result, Linux fails to bring up secondary cpus when run as a guest
in QEMU/KVM with both hv_apic and x2apic enabled.
According to Michael Kelley, when in x2apic mode, the Hyper-V synthetic
apic MSRs behave exactly the same as the corresponding architectural
x2apic MSRs, so there's no need to override the apic accessors. The
only exception is hv_apic_eoi_write, which benefits from lazy EOI when
available; however, its implementation works for both xapic and x2apic
modes.
Fixes: 29217a474683 ("iommu/hyper-v: Add Hyper-V stub IOMMU driver")
Fixes: 6b48cb5f8347 ("X86/Hyper-V: Enlighten APIC access")
Cc: stable@vger.kernel.org
Suggested-by: Michael Kelley <mikelley@microsoft.com>
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
v3 -> v4:
- adjust the log message [Vitaly, Michael]
v2 -> v3:
- do not introduce x2apic-capable hv_apic accessors; leave original
x2apic accessors instead
v1 -> v2:
- add ifdefs to handle !CONFIG_X86_X2APIC
arch/x86/hyperv/hv_apic.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index 5c056b8aebef..e01078e93dd3 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -260,11 +260,21 @@ void __init hv_apic_init(void)
}
if (ms_hyperv.hints & HV_X64_APIC_ACCESS_RECOMMENDED) {
- pr_info("Hyper-V: Using MSR based APIC access\n");
+ pr_info("Hyper-V: Using enlightened APIC (%s mode)",
+ x2apic_enabled() ? "x2apic" : "xapic");
+ /*
+ * With x2apic, architectural x2apic MSRs are equivalent to the
+ * respective synthetic MSRs, so there's no need to override
+ * the apic accessors. The only exception is
+ * hv_apic_eoi_write, because it benefits from lazy EOI when
+ * available, but it works for both xapic and x2apic modes.
+ */
apic_set_eoi_write(hv_apic_eoi_write);
- apic->read = hv_apic_read;
- apic->write = hv_apic_write;
- apic->icr_write = hv_apic_icr_write;
- apic->icr_read = hv_apic_icr_read;
+ if (!x2apic_enabled()) {
+ apic->read = hv_apic_read;
+ apic->write = hv_apic_write;
+ apic->icr_write = hv_apic_icr_write;
+ apic->icr_read = hv_apic_icr_read;
+ }
}
}
--
2.21.0
^ permalink raw reply related
* Re: [RFC PATCH 10/13] vsock: add multi-transports support
From: Stefano Garzarella @ 2019-10-10 12:55 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20191009131123.GK5747@stefanha-x1.localdomain>
On Wed, Oct 09, 2019 at 02:11:23PM +0100, Stefan Hajnoczi wrote:
> On Fri, Sep 27, 2019 at 01:27:00PM +0200, Stefano Garzarella wrote:
> > RFC:
> > - I'd like to move MODULE_ALIAS_NETPROTO(PF_VSOCK) to af_vsock.c.
> > @Jorgen could this break the VMware products?
>
> What will cause the vmw_vsock_vmci_transport.ko module to be loaded
> after you remove MODULE_ALIAS_NETPROTO(PF_VSOCK)? Perhaps
> drivers/misc/vmw_vmci/vmci_guest.c:vmci_guest_probe_device() could do
> something when the guest driver loads.
Good idea, maybe we can call some function provided by vmci_transport
to register it as a guest (I'll remove the type from the transport
and I add it as a parameter of vsock_core_register())
> There would need to be something
> equivalent for the host side too.
Maybe in the vmci_host_do_init_context().
>
> This will solve another issue too. Today the VMCI transport can be
> loaded if an application creates an AF_VSOCK socket during early boot
> before the virtio transport has been probed. This happens because the
> VMCI transport uses MODULE_ALIAS_NETPROTO(PF_VSOCK) *and* it does not
> probe whether this system is actually a VMware guest.
>
> If we instead load the core af_vsock.ko module and transports are only
> loaded based on hardware feature probing (e.g. the presence of VMware
> guest mode, a virtio PCI adapter, etc) then transports will be
> well-behaved.
Yes, I completely agree with you. I'll try to follow your suggestion,
>
> > - DGRAM sockets are handled as before, I don't know if make sense work
> > on it now, or when another transport will support DGRAM. The big
> > issues here is that we cannot link 1-1 a socket to transport as
> > for stream sockets since DGRAM is not connection-oriented.
>
> Let's ignore DGRAM for now since only VMCI supports it and we therefore
> do not require multi-transpor) support.
Okay :)
>
> > diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> > index 86f8f463e01a..2a081d19e20d 100644
> > --- a/include/net/af_vsock.h
> > +++ b/include/net/af_vsock.h
> > @@ -94,7 +94,13 @@ struct vsock_transport_send_notify_data {
> > u64 data2; /* Transport-defined. */
> > };
> >
> > +#define VSOCK_TRANSPORT_F_H2G 0x00000001
> > +#define VSOCK_TRANSPORT_F_G2H 0x00000002
> > +#define VSOCK_TRANSPORT_F_DGRAM 0x00000004
>
> Documentation comments, please.
I'll fix!
>
> > +void vsock_core_unregister(const struct vsock_transport *t)
> > +{
> > + mutex_lock(&vsock_register_mutex);
> > +
> > + /* RFC-TODO: maybe we should check if there are open sockets
> > + * assigned to that transport and avoid the unregistration
> > + */
>
> If unregister() is only called from module_exit() functions then holding
> a reference to the transport module would be enough to prevent this
> case. The transport could only be removed once all sockets have been
> destroyed (and dropped their transport module reference).
Yes. I did this in
"[RFC PATCH 12/13] vsock: prevent transport modules unloading".
Maybe I can merge it in this patch...
Thanks,
Stefano
^ permalink raw reply
* Re: [RFC PATCH 11/13] vsock: add 'transport_hg' to handle g2h\h2g transports
From: Stefano Garzarella @ 2019-10-10 13:04 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, Sasha Levin, linux-hyperv, Stephen Hemminger, kvm,
Michael S. Tsirkin, Dexuan Cui, Haiyang Zhang, linux-kernel,
virtualization, Stefan Hajnoczi, David S. Miller, Jorgen Hansen
In-Reply-To: <20191009131643.GL5747@stefanha-x1.localdomain>
On Wed, Oct 09, 2019 at 02:16:43PM +0100, Stefan Hajnoczi wrote:
> On Fri, Sep 27, 2019 at 01:27:01PM +0200, Stefano Garzarella wrote:
> > VMCI transport provides both g2h and h2g behaviors in a single
> > transport.
> > We are able to set (or not) the g2h behavior, detecting if we
> > are in a VMware guest (or not), but the h2g feature is always set.
> > This prevents to load other h2g transports while we are in a
> > VMware guest.
>
> In the vhost_vsock.ko case we only register the h2g transport when
> userspace has loaded the module (by opening /dev/vhost-vsock).
>
> VMCI has something kind of similar: /dev/vmci and the
> vmci_host_active_users counter. Maybe we can use this instead of
> introducing the transport_hg concept?
Yes, maybe we can register the host in the vmci_host_do_init_context().
I also don't like a lot the transport_hg concept, so I'll try to found
an alternative.
Thanks,
Stefano
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox