* Re: [PATCH net-next v11 08/13] selftests/vsock: add vm_dmesg_{warn,oops}_count() helpers
From: Stefano Garzarella @ 2025-11-21 14:42 UTC (permalink / raw)
To: Bobby Eshleman
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan, linux-kernel,
virtualization, netdev, kvm, linux-hyperv, linux-kselftest,
berrange, Sargun Dhillon, Bobby Eshleman
In-Reply-To: <20251120-vsock-vmtest-v11-8-55cbc80249a7@meta.com>
On Thu, Nov 20, 2025 at 09:44:40PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>These functions are reused by the VM tests to collect and compare dmesg
>warnings and oops counts. The future VM-specific tests use them heavily.
>This patches relies on vm_ssh() already supporting namespaces.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v11:
>- break these out into an earlier patch so that they can be used
> directly in new patches (instead of causing churn by adding this
> later)
>---
> tools/testing/selftests/vsock/vmtest.sh | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
>diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
>index 4da91828a6a0..1623e4da15e2 100755
>--- a/tools/testing/selftests/vsock/vmtest.sh
>+++ b/tools/testing/selftests/vsock/vmtest.sh
>@@ -389,6 +389,17 @@ host_wait_for_listener() {
> fi
> }
>
>+vm_dmesg_oops_count() {
>+ local ns=$1
>+
>+ vm_ssh "${ns}" -- dmesg 2>/dev/null | grep -c -i 'Oops'
>+}
>+
>+vm_dmesg_warn_count() {
>+ local ns=$1
>+
>+ vm_ssh "${ns}" -- dmesg --level=warn 2>/dev/null | grep -c -i 'vsock'
>+}
>
> vm_vsock_test() {
> local ns=$1
>@@ -596,8 +607,8 @@ run_shared_vm_test() {
>
> host_oops_cnt_before=$(dmesg | grep -c -i 'Oops')
> host_warn_cnt_before=$(dmesg --level=warn | grep -c -i 'vsock')
>- vm_oops_cnt_before=$(vm_ssh -- dmesg | grep -c -i 'Oops')
>- vm_warn_cnt_before=$(vm_ssh -- dmesg --level=warn | grep -c -i 'vsock')
>+ vm_oops_cnt_before=$(vm_dmesg_oops_count "init_ns")
>+ vm_warn_cnt_before=$(vm_dmesg_warn_count "init_ns")
>
> name=$(echo "${1}" | awk '{ print $1 }')
> eval test_"${name}"
>@@ -615,13 +626,13 @@ run_shared_vm_test() {
> rc=$KSFT_FAIL
> fi
>
>- vm_oops_cnt_after=$(vm_ssh -- dmesg | grep -i 'Oops' | wc -l)
>+ vm_oops_cnt_after=$(vm_dmesg_oops_count "init_ns")
> if [[ ${vm_oops_cnt_after} -gt ${vm_oops_cnt_before} ]]; then
> echo "FAIL: kernel oops detected on vm" | log_host
> rc=$KSFT_FAIL
> fi
>
>- vm_warn_cnt_after=$(vm_ssh -- dmesg --level=warn | grep -c -i 'vsock')
>+ vm_warn_cnt_after=$(vm_dmesg_warn_count "init_ns")
> if [[ ${vm_warn_cnt_after} -gt ${vm_warn_cnt_before} ]]; then
> echo "FAIL: kernel warning detected on vm" | log_host
> rc=$KSFT_FAIL
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v11 05/13] vsock: add netns support to virtio transports
From: Stefano Garzarella @ 2025-11-21 14:39 UTC (permalink / raw)
To: Bobby Eshleman
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan, linux-kernel,
virtualization, netdev, kvm, linux-hyperv, linux-kselftest,
berrange, Sargun Dhillon, Bobby Eshleman
In-Reply-To: <20251120-vsock-vmtest-v11-5-55cbc80249a7@meta.com>
On Thu, Nov 20, 2025 at 09:44:37PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Add netns support to loopback and vhost. Keep netns disabled for
>virtio-vsock, but add necessary changes to comply with common API
>updates.
>
>This is the patch in the series when vhost-vsock namespaces actually
>come online. Hence, vhost_transport_supports_local_mode() is switched
>to return true.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v11:
>- reorder with the skb ownership patch for loopback (Stefano)
>- toggle vhost_transport_supports_local_mode() to true
>
>Changes in v10:
>- Splitting patches complicates the series with meaningless placeholder
> values that eventually get replaced anyway, so to avoid that this
> patch combines into one. Links to previous patches here:
> - Link: https://lore.kernel.org/all/20251111-vsock-vmtest-v9-3-852787a37bed@meta.com/
> - Link: https://lore.kernel.org/all/20251111-vsock-vmtest-v9-6-852787a37bed@meta.com/
> - Link: https://lore.kernel.org/all/20251111-vsock-vmtest-v9-7-852787a37bed@meta.com/
>- remove placeholder values (Stefano)
>- update comment describe net/net_mode for
> virtio_transport_reset_no_sock()
>---
> drivers/vhost/vsock.c | 47 ++++++++++++++++++------
> include/linux/virtio_vsock.h | 8 +++--
> net/vmw_vsock/virtio_transport.c | 10 ++++--
> net/vmw_vsock/virtio_transport_common.c | 63 ++++++++++++++++++++++++---------
> net/vmw_vsock/vsock_loopback.c | 8 +++--
> 5 files changed, 103 insertions(+), 33 deletions(-)
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index 4e3856aa2479..e73a6499b9fe 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -46,6 +46,11 @@ static DEFINE_READ_MOSTLY_HASHTABLE(vhost_vsock_hash, 8);
> struct vhost_vsock {
> struct vhost_dev dev;
> struct vhost_virtqueue vqs[2];
>+ struct net *net;
>+ netns_tracker ns_tracker;
>+
>+ /* The ns mode at the time vhost_vsock was created */
>+ enum vsock_net_mode net_mode;
>
> /* Link to global vhost_vsock_hash, writes use vhost_vsock_mutex */
> struct hlist_node hash;
>@@ -66,13 +71,14 @@ static u32 vhost_transport_get_local_cid(void)
>
> static bool vhost_transport_supports_local_mode(void)
> {
>- return false;
>+ return true;
> }
>
> /* Callers that dereference the return value must hold vhost_vsock_mutex or the
> * RCU read lock.
> */
>-static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
>+static struct vhost_vsock *vhost_vsock_get(u32 guest_cid, struct net *net,
>+ enum vsock_net_mode mode)
> {
> struct vhost_vsock *vsock;
>
>@@ -83,9 +89,10 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
> if (other_cid == 0)
> continue;
>
>- if (other_cid == guest_cid)
>+ if (other_cid == guest_cid &&
>+ vsock_net_check_mode(net, mode, vsock->net,
>+ vsock->net_mode))
> return vsock;
>-
> }
>
> return NULL;
>@@ -274,7 +281,8 @@ static void vhost_transport_send_pkt_work(struct vhost_work *work)
> }
>
> static int
>-vhost_transport_send_pkt(struct sk_buff *skb)
>+vhost_transport_send_pkt(struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct vhost_vsock *vsock;
>@@ -283,7 +291,7 @@ vhost_transport_send_pkt(struct sk_buff *skb)
> rcu_read_lock();
>
> /* Find the vhost_vsock according to guest context id */
>- vsock = vhost_vsock_get(le64_to_cpu(hdr->dst_cid));
>+ vsock = vhost_vsock_get(le64_to_cpu(hdr->dst_cid), net, net_mode);
> if (!vsock) {
> rcu_read_unlock();
> kfree_skb(skb);
>@@ -310,7 +318,8 @@ vhost_transport_cancel_pkt(struct vsock_sock *vsk)
> rcu_read_lock();
>
> /* Find the vhost_vsock according to guest context id */
>- vsock = vhost_vsock_get(vsk->remote_addr.svm_cid);
>+ vsock = vhost_vsock_get(vsk->remote_addr.svm_cid,
>+ sock_net(sk_vsock(vsk)), vsk->net_mode);
> if (!vsock)
> goto out;
>
>@@ -470,11 +479,12 @@ static struct virtio_transport vhost_transport = {
> static bool vhost_transport_seqpacket_allow(struct vsock_sock *vsk,
> u32 remote_cid)
> {
>+ struct net *net = sock_net(sk_vsock(vsk));
> struct vhost_vsock *vsock;
> bool seqpacket_allow = false;
>
> rcu_read_lock();
>- vsock = vhost_vsock_get(remote_cid);
>+ vsock = vhost_vsock_get(remote_cid, net, vsk->net_mode);
>
> if (vsock)
> seqpacket_allow = vsock->seqpacket_allow;
>@@ -545,7 +555,8 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
> if (le64_to_cpu(hdr->src_cid) == vsock->guest_cid &&
> le64_to_cpu(hdr->dst_cid) ==
> vhost_transport_get_local_cid())
>- virtio_transport_recv_pkt(&vhost_transport, skb);
>+ virtio_transport_recv_pkt(&vhost_transport, skb,
>+ vsock->net, vsock->net_mode);
> else
> kfree_skb(skb);
>
>@@ -662,6 +673,7 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
> {
> struct vhost_virtqueue **vqs;
> struct vhost_vsock *vsock;
>+ struct net *net;
> int ret;
>
> /* This struct is large and allocation could fail, fall back to vmalloc
>@@ -677,6 +689,17 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
> goto out;
> }
>
>+ net = current->nsproxy->net_ns;
>+ vsock->net = get_net_track(net, &vsock->ns_tracker, GFP_KERNEL);
>+
>+ /* Store the mode of the namespace at the time of creation. If this
>+ * namespace later changes from "global" to "local", we want this vsock
>+ * to continue operating normally and not suddenly break. For that
>+ * reason, we save the mode here and later use it when performing
>+ * socket lookups with vsock_net_check_mode() (see vhost_vsock_get()).
>+ */
>+ vsock->net_mode = vsock_net_mode(net);
>+
> vsock->guest_cid = 0; /* no CID assigned yet */
> vsock->seqpacket_allow = false;
>
>@@ -716,7 +739,8 @@ static void vhost_vsock_reset_orphans(struct sock *sk)
> */
>
> /* If the peer is still valid, no need to reset connection */
>- if (vhost_vsock_get(vsk->remote_addr.svm_cid))
>+ if (vhost_vsock_get(vsk->remote_addr.svm_cid, sock_net(sk),
>+ vsk->net_mode))
> return;
>
> /* If the close timeout is pending, let it expire. This avoids races
>@@ -761,6 +785,7 @@ static int vhost_vsock_dev_release(struct inode *inode, struct file *file)
> virtio_vsock_skb_queue_purge(&vsock->send_pkt_queue);
>
> vhost_dev_cleanup(&vsock->dev);
>+ put_net_track(vsock->net, &vsock->ns_tracker);
> kfree(vsock->dev.vqs);
> vhost_vsock_free(vsock);
> return 0;
>@@ -787,7 +812,7 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 guest_cid)
>
> /* Refuse if CID is already in use */
> mutex_lock(&vhost_vsock_mutex);
>- other = vhost_vsock_get(guest_cid);
>+ other = vhost_vsock_get(guest_cid, vsock->net, vsock->net_mode);
> if (other && other != vsock) {
> mutex_unlock(&vhost_vsock_mutex);
> return -EADDRINUSE;
>diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>index 0c67543a45c8..5ed6136a4ed4 100644
>--- a/include/linux/virtio_vsock.h
>+++ b/include/linux/virtio_vsock.h
>@@ -173,6 +173,8 @@ struct virtio_vsock_pkt_info {
> u32 remote_cid, remote_port;
> struct vsock_sock *vsk;
> struct msghdr *msg;
>+ struct net *net;
>+ enum vsock_net_mode net_mode;
> u32 pkt_len;
> u16 type;
> u16 op;
>@@ -185,7 +187,8 @@ struct virtio_transport {
> struct vsock_transport transport;
>
> /* Takes ownership of the packet */
>- int (*send_pkt)(struct sk_buff *skb);
>+ int (*send_pkt)(struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode);
>
> /* Used in MSG_ZEROCOPY mode. Checks, that provided data
> * (number of buffers) could be transmitted with zerocopy
>@@ -280,7 +283,8 @@ virtio_transport_dgram_enqueue(struct vsock_sock *vsk,
> void virtio_transport_destruct(struct vsock_sock *vsk);
>
> void virtio_transport_recv_pkt(struct virtio_transport *t,
>- struct sk_buff *skb);
>+ struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode);
> void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct sk_buff *skb);
> u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
> void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
>diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>index af4fbce0baab..106d3f25a5cb 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -243,7 +243,8 @@ static int virtio_transport_send_skb_fast_path(struct virtio_vsock *vsock, struc
> }
>
> static int
>-virtio_transport_send_pkt(struct sk_buff *skb)
>+virtio_transport_send_pkt(struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct virtio_vsock_hdr *hdr;
> struct virtio_vsock *vsock;
>@@ -675,7 +676,12 @@ static void virtio_transport_rx_work(struct work_struct *work)
> virtio_vsock_skb_put(skb, payload_len);
>
> virtio_transport_deliver_tap_pkt(skb);
>- virtio_transport_recv_pkt(&virtio_transport, skb);
>+
>+ /* Force virtio-transport into global mode since it
>+ * does not yet support local-mode namespacing.
>+ */
>+ virtio_transport_recv_pkt(&virtio_transport, skb,
>+ NULL, VSOCK_NET_MODE_GLOBAL);
> }
> } while (!virtqueue_enable_cb(vq));
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 675eb9d83549..5bb498caa19e 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -413,7 +413,7 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>
> virtio_transport_inc_tx_pkt(vvs, skb);
>
>- ret = t_ops->send_pkt(skb);
>+ ret = t_ops->send_pkt(skb, info->net, info->net_mode);
> if (ret < 0)
> break;
>
>@@ -527,6 +527,8 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk)
> struct virtio_vsock_pkt_info info = {
> .op = VIRTIO_VSOCK_OP_CREDIT_UPDATE,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1067,6 +1069,8 @@ int virtio_transport_connect(struct vsock_sock *vsk)
> struct virtio_vsock_pkt_info info = {
> .op = VIRTIO_VSOCK_OP_REQUEST,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1082,6 +1086,8 @@ int virtio_transport_shutdown(struct vsock_sock *vsk, int mode)
> (mode & SEND_SHUTDOWN ?
> VIRTIO_VSOCK_SHUTDOWN_SEND : 0),
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1108,6 +1114,8 @@ virtio_transport_stream_enqueue(struct vsock_sock *vsk,
> .msg = msg,
> .pkt_len = len,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1145,6 +1153,8 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
> .op = VIRTIO_VSOCK_OP_RST,
> .reply = !!skb,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> /* Send RST only if the original pkt is not a RST pkt */
>@@ -1156,9 +1166,14 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
>
> /* Normally packets are associated with a socket. There may be no socket if an
> * attempt was made to connect to a socket that does not exist.
>+ *
>+ * net and net_mode refer to the namespace of whoever sent the invalid message.
>+ * For loopback, this is the namespace of the socket. For vhost, this is the
>+ * namespace of the VM (i.e., vhost_vsock).
> */
> static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
>- struct sk_buff *skb)
>+ struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct virtio_vsock_pkt_info info = {
>@@ -1171,6 +1186,13 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> * sock_net(sk) until the reply skb is freed.
> */
> .vsk = vsock_sk(skb->sk),
>+
>+ /* net or net_mode are not defined here because we pass
>+ * net and net_mode directly to t->send_pkt(), instead of
>+ * relying on virtio_transport_send_pkt_info() to pass them to
>+ * t->send_pkt(). They are not needed by
>+ * virtio_transport_alloc_skb().
>+ */
> };
> struct sk_buff *reply;
>
>@@ -1189,7 +1211,7 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> if (!reply)
> return -ENOMEM;
>
>- return t->send_pkt(reply);
>+ return t->send_pkt(reply, net, net_mode);
> }
>
> /* This function should be called with sk_lock held and SOCK_DONE set */
>@@ -1471,6 +1493,8 @@ virtio_transport_send_response(struct vsock_sock *vsk,
> .remote_port = le32_to_cpu(hdr->src_port),
> .reply = true,
> .vsk = vsk,
>+ .net = sock_net(sk_vsock(vsk)),
>+ .net_mode = vsk->net_mode,
> };
>
> return virtio_transport_send_pkt_info(vsk, &info);
>@@ -1513,12 +1537,14 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> int ret;
>
> if (le16_to_cpu(hdr->op) != VIRTIO_VSOCK_OP_REQUEST) {
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk),
>+ vsk->net_mode);
> return -EINVAL;
> }
>
> if (sk_acceptq_is_full(sk)) {
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk),
>+ vsk->net_mode);
> return -ENOMEM;
> }
>
>@@ -1526,13 +1552,15 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> * Subsequent enqueues would lead to a memory leak.
> */
> if (sk->sk_shutdown == SHUTDOWN_MASK) {
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk),
>+ vsk->net_mode);
> return -ESHUTDOWN;
> }
>
> child = vsock_create_connected(sk);
> if (!child) {
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk),
>+ vsk->net_mode);
> return -ENOMEM;
> }
>
>@@ -1554,7 +1582,8 @@ virtio_transport_recv_listen(struct sock *sk, struct sk_buff *skb,
> */
> if (ret || vchild->transport != &t->transport) {
> release_sock(child);
>- virtio_transport_reset_no_sock(t, skb);
>+ virtio_transport_reset_no_sock(t, skb, sock_net(sk),
>+ vsk->net_mode);
> sock_put(child);
> return ret;
> }
>@@ -1582,7 +1611,8 @@ static bool virtio_transport_valid_type(u16 type)
> * lock.
> */
> void virtio_transport_recv_pkt(struct virtio_transport *t,
>- struct sk_buff *skb)
>+ struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct virtio_vsock_hdr *hdr = virtio_vsock_hdr(skb);
> struct sockaddr_vm src, dst;
>@@ -1605,24 +1635,25 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> le32_to_cpu(hdr->fwd_cnt));
>
> if (!virtio_transport_valid_type(le16_to_cpu(hdr->type))) {
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
> goto free_pkt;
> }
>
> /* The socket must be in connected or bound table
> * otherwise send reset back
> */
>- sk = vsock_find_connected_socket(&src, &dst);
>+ sk = vsock_find_connected_socket_net(&src, &dst, net, net_mode);
> if (!sk) {
>- sk = vsock_find_bound_socket(&dst);
>+ sk = vsock_find_bound_socket_net(&dst, net, net_mode);
> if (!sk) {
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net,
>+ net_mode);
> goto free_pkt;
> }
> }
>
> if (virtio_transport_get_type(sk) != le16_to_cpu(hdr->type)) {
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
> sock_put(sk);
> goto free_pkt;
> }
>@@ -1641,7 +1672,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> */
> if (sock_flag(sk, SOCK_DONE) ||
> (sk->sk_state != TCP_LISTEN && vsk->transport != &t->transport)) {
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
> release_sock(sk);
> sock_put(sk);
> goto free_pkt;
>@@ -1673,7 +1704,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
> kfree_skb(skb);
> break;
> default:
>- (void)virtio_transport_reset_no_sock(t, skb);
>+ (void)virtio_transport_reset_no_sock(t, skb, net, net_mode);
> kfree_skb(skb);
> break;
> }
>diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
>index 1e25c1a6b43f..a730fa74d2d9 100644
>--- a/net/vmw_vsock/vsock_loopback.c
>+++ b/net/vmw_vsock/vsock_loopback.c
>@@ -31,7 +31,8 @@ static bool vsock_loopback_supports_local_mode(void)
> return true;
> }
>
>-static int vsock_loopback_send_pkt(struct sk_buff *skb)
>+static int vsock_loopback_send_pkt(struct sk_buff *skb, struct net *net,
>+ enum vsock_net_mode net_mode)
> {
> struct vsock_loopback *vsock = &the_vsock_loopback;
> int len = skb->len;
>@@ -138,7 +139,10 @@ static void vsock_loopback_work(struct work_struct *work)
> */
> virtio_transport_consume_skb_sent(skb, false);
> virtio_transport_deliver_tap_pkt(skb);
>- virtio_transport_recv_pkt(&loopback_transport, skb);
>+
>+ virtio_transport_recv_pkt(&loopback_transport, skb,
>+ sock_net(skb->sk),
>+ vsock_sk(skb->sk)->net_mode);
> }
> }
>
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v11 04/13] virtio: set skb owner of virtio_transport_reset_no_sock() reply
From: Stefano Garzarella @ 2025-11-21 14:24 UTC (permalink / raw)
To: Bobby Eshleman
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan, linux-kernel,
virtualization, netdev, kvm, linux-hyperv, linux-kselftest,
berrange, Sargun Dhillon, Bobby Eshleman
In-Reply-To: <20251120-vsock-vmtest-v11-4-55cbc80249a7@meta.com>
On Thu, Nov 20, 2025 at 09:44:36PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Associate reply packets with the sending socket. When vsock must reply
>with an RST packet and there exists a sending socket (e.g., for
>loopback), setting the skb owner to the socket correctly handles
>reference counting between the skb and sk (i.e., the sk stays alive
>until the skb is freed).
>
>This allows the net namespace to be used for socket lookups for the
>duration of the reply skb's lifetime, preventing race conditions between
>the namespace lifecycle and vsock socket search using the namespace
>pointer.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v11:
>- move before adding to netns support (Stefano)
>
>Changes in v10:
>- break this out into its own patch for easy revert (Stefano)
>---
> net/vmw_vsock/virtio_transport_common.c | 6 ++++++
> 1 file changed, 6 insertions(+)
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index dcc8a1d5851e..675eb9d83549 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -1165,6 +1165,12 @@ static int virtio_transport_reset_no_sock(const struct virtio_transport *t,
> .op = VIRTIO_VSOCK_OP_RST,
> .type = le16_to_cpu(hdr->type),
> .reply = true,
>+
>+ /* Set sk owner to socket we are replying to (may be NULL for
>+ * non-loopback). This keeps a reference to the sock and
>+ * sock_net(sk) until the reply skb is freed.
>+ */
>+ .vsk = vsock_sk(skb->sk),
> };
> struct sk_buff *reply;
>
>
>--
>2.47.3
>
^ permalink raw reply
* Re: [PATCH net-next v11 03/13] vsock: reject bad VSOCK_NET_MODE_LOCAL configuration for G2H
From: Stefano Garzarella @ 2025-11-21 14:24 UTC (permalink / raw)
To: Bobby Eshleman
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang,
Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan, linux-kernel,
virtualization, netdev, kvm, linux-hyperv, linux-kselftest,
berrange, Sargun Dhillon, Bobby Eshleman
In-Reply-To: <20251120-vsock-vmtest-v11-3-55cbc80249a7@meta.com>
On Thu, Nov 20, 2025 at 09:44:35PM -0800, Bobby Eshleman wrote:
>From: Bobby Eshleman <bobbyeshleman@meta.com>
>
>Reject setting VSOCK_NET_MODE_LOCAL with -EOPNOTSUPP if a G2H transport
>is operational. Additionally, reject G2H transport registration if there
>already exists a namespace in local mode.
>
>G2H sockets break in local mode because the G2H transports don't support
>namespacing yet. The current approach is to coerce packets coming out of
>G2H transports into VSOCK_NET_MODE_GLOBAL mode, but it is not possible
>to coerce sockets in the same way because it cannot be deduced which
>transport will be used by the socket. Specifically, when bound to
>VMADDR_CID_ANY in a nested VM (both G2H and H2G available), it is not
>until a packet is received and matched to the bound socket that we
>assign the transport. This presents a chicken-and-egg problem, because
>we need the namespace to lookup the socket and resolve the transport,
>but we need the transport to know how to use the namespace during
>lookup.
>
>For that reason, this patch prevents VSOCK_NET_MODE_LOCAL from being
>used on systems that support G2H, even nested systems that also have H2G
>transports.
>
>Local mode is blocked based on detecting the presence of G2H devices
>(when possible, as hyperv is special). This means that a host kernel
>with G2H support compiled in (or has the module loaded), will still
>support local mode if there is no G2H (e.g., virtio-vsock) device
>detected. This enables using the same kernel in the host and in the
>guest, as we do in kselftest.
>
>Systems with only namespace-aware transports (vhost-vsock, loopback) can
>still use both VSOCK_NET_MODE_GLOBAL and VSOCK_NET_MODE_LOCAL modes as
>intended.
>
>Add supports_local_mode() transport callback to indicate
>transport-specific local mode support.
>
>These restrictions can be lifted in a future patch series when G2H
>transports gain namespace support.
>
>Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
>---
>Changes in v11:
>- vhost_transport_supports_local_mode() returns false to keep things
> disabled until support comes online (Stefano)
>- add comment above supports_local_mode() cb to clarify (Stefano)
>- Remove redundant `ret = 0` initialization in vsock_net_mode_string()
> (Stefano)
>- Refactor vsock_net_mode_string() to separate parsing from validation
> (Stefano)
>- vmci returns false for supports_local_mode(), with comment
>
>Changes in v10:
>- move this patch before any transports bring online namespacing (Stefano)
>- move vsock_net_mode_string into critical section (Stefano)
>- add ->supports_local_mode() callback to transports (Stefano)
>---
> drivers/vhost/vsock.c | 6 ++++++
> include/net/af_vsock.h | 11 +++++++++++
> net/vmw_vsock/af_vsock.c | 32 ++++++++++++++++++++++++++++++++
> net/vmw_vsock/hyperv_transport.c | 6 ++++++
> net/vmw_vsock/virtio_transport.c | 13 +++++++++++++
> net/vmw_vsock/vmci_transport.c | 12 ++++++++++++
> net/vmw_vsock/vsock_loopback.c | 6 ++++++
> 7 files changed, 86 insertions(+)
>
>diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>index 69074656263d..4e3856aa2479 100644
>--- a/drivers/vhost/vsock.c
>+++ b/drivers/vhost/vsock.c
>@@ -64,6 +64,11 @@ static u32 vhost_transport_get_local_cid(void)
> return VHOST_VSOCK_DEFAULT_HOST_CID;
> }
>
>+static bool vhost_transport_supports_local_mode(void)
>+{
>+ return false;
>+}
>+
> /* Callers that dereference the return value must hold vhost_vsock_mutex or the
> * RCU read lock.
> */
>@@ -412,6 +417,7 @@ static struct virtio_transport vhost_transport = {
> .module = THIS_MODULE,
>
> .get_local_cid = vhost_transport_get_local_cid,
>+ .supports_local_mode = vhost_transport_supports_local_mode,
>
> .init = virtio_transport_do_socket_init,
> .destruct = virtio_transport_destruct,
>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>index 59d97a143204..e24ef1d9fe02 100644
>--- a/include/net/af_vsock.h
>+++ b/include/net/af_vsock.h
>@@ -180,6 +180,17 @@ struct vsock_transport {
> /* Addressing. */
> u32 (*get_local_cid)(void);
>
>+ /* Return true if the transport is compatible with
>+ * VSOCK_NET_MODE_LOCAL. Otherwise, return false.
>+ *
>+ * Transports should return false if they lack local-mode namespace
>+ * support (e.g., G2H transports like hyperv-vsock and vmci-vsock).
>+ * virtio-vsock returns true only if no device is present in order to
>+ * enable local mode in nested scenarios in which virtio-vsock is
>+ * loaded or built-in, but nonetheless unusable by sockets.
>+ */
>+ bool (*supports_local_mode)(void);
>+
> /* Read a single skb */
> int (*read_skb)(struct vsock_sock *, skb_read_actor_t);
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 243c0d588682..120adb9dad9f 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -91,6 +91,12 @@
> * and locked down by a namespace manager. The default is "global". The mode
> * is set per-namespace.
> *
>+ * Note: LOCAL mode is only supported when using namespace-aware transports
>+ * (vhost-vsock, loopback). If a guest-to-host transport (virtio-vsock,
>+ * hyperv-vsock, vmci-vsock) is operational, attempts to set LOCAL mode will
>+ * fail with EOPNOTSUPP, as these transports do not support per-namespace
>+ * isolation.
>+ *
> * The modes affect the allocation and accessibility of CIDs as follows:
> *
> * - global - access and allocation are all system-wide
>@@ -2794,6 +2800,15 @@ static int vsock_net_mode_string(const struct ctl_table *table, int write,
> else
> return -EINVAL;
>
>+ mutex_lock(&vsock_register_mutex);
>+ if (mode == VSOCK_NET_MODE_LOCAL &&
>+ transport_g2h && transport_g2h->supports_local_mode &&
>+ !transport_g2h->supports_local_mode()) {
>+ mutex_unlock(&vsock_register_mutex);
>+ return -EOPNOTSUPP;
>+ }
>+ mutex_unlock(&vsock_register_mutex);
Wait, I think we already discussed about this, vsock_net_write_mode()
must be called with the lock held.
See
https://lore.kernel.org/netdev/aRTTwuuXSz5CvNjt@devvm11784.nha0.facebook.com/
Since I guess we need another version of this patch, can you check the
commit description to see if it reflects what we are doing now
(e.g vhost is not enabled)?
Also I don't understand why for vhost we will enable it later, but for
virtio_transport and vsock_loopback we are enabling it now, also if this
patch is before the support on that transports. I'm a bit confused.
If something is unclear, let's discuss it before sending a new version.
What I had in mind was, add this patch and explain why we need this new
callback (like you did), but enable the support in the patches that
really enable it for any transport. But maybe what is not clear to me is
that we need this only for G2H. But now I'm confused about the
discussion around vmci H2G. We decided to discard also that one, but
here we are not checking that?
I mean here we are calling supports_local_mode() only on G2H IIUC.
>+
> if (!vsock_net_write_mode(net, mode))
> return -EPERM;
>
>@@ -2938,6 +2953,7 @@ int vsock_core_register(const struct vsock_transport *t, int features)
> {
> const struct vsock_transport *t_h2g, *t_g2h, *t_dgram, *t_local;
> int err = mutex_lock_interruptible(&vsock_register_mutex);
>+ struct net *net;
>
> if (err)
> return err;
>@@ -2960,6 +2976,22 @@ int vsock_core_register(const struct vsock_transport *t, int features)
> err = -EBUSY;
> goto err_busy;
> }
>+
>+ /* G2H sockets break in LOCAL mode namespaces because G2H
And also here we are talking about only of G2H, so what happen if vmci
is loaded as H2G?
IMO we should discuss this a bit more and make it a bit more generic,
like check all the transports.
Thanks,
Stefano
>+ * transports don't support them yet. Block registering new G2H
>+ * transports if we already have local mode namespaces on the
>+ * system.
>+ */
>+ rcu_read_lock();
>+ for_each_net_rcu(net) {
>+ if (vsock_net_mode(net) == VSOCK_NET_MODE_LOCAL) {
>+ rcu_read_unlock();
>+ err = -EOPNOTSUPP;
>+ goto err_busy;
>+ }
>+ }
>+ rcu_read_unlock();
>+
> t_g2h = t;
> }
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index 432fcbbd14d4..279f04fcd81a 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -833,10 +833,16 @@ int hvs_notify_set_rcvlowat(struct vsock_sock *vsk, int val)
> return -EOPNOTSUPP;
> }
>
>+static bool hvs_supports_local_mode(void)
>+{
>+ return false;
>+}
>+
> static struct vsock_transport hvs_transport = {
> .module = THIS_MODULE,
>
> .get_local_cid = hvs_get_local_cid,
>+ .supports_local_mode = hvs_supports_local_mode,
>
> .init = hvs_sock_init,
> .destruct = hvs_destruct,
>diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>index d365a4b371d0..af4fbce0baab 100644
>--- a/net/vmw_vsock/virtio_transport.c
>+++ b/net/vmw_vsock/virtio_transport.c
>@@ -94,6 +94,18 @@ static u32 virtio_transport_get_local_cid(void)
> return ret;
> }
>
>+static bool virtio_transport_supports_local_mode(void)
>+{
>+ struct virtio_vsock *vsock;
>+
>+ rcu_read_lock();
>+ vsock = rcu_dereference(the_virtio_vsock);
>+ rcu_read_unlock();
>+
>+ /* Local mode is supported only when no G2H device is present. */
>+ return vsock ? false : true;
>+}
>+
> /* Caller need to hold vsock->tx_lock on vq */
> static int virtio_transport_send_skb(struct sk_buff *skb, struct virtqueue *vq,
> struct virtio_vsock *vsock, gfp_t gfp)
>@@ -544,6 +556,7 @@ static struct virtio_transport virtio_transport = {
> .module = THIS_MODULE,
>
> .get_local_cid = virtio_transport_get_local_cid,
>+ .supports_local_mode = virtio_transport_supports_local_mode,
>
> .init = virtio_transport_do_socket_init,
> .destruct = virtio_transport_destruct,
>diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>index 7eccd6708d66..e392d3d1fd90 100644
>--- a/net/vmw_vsock/vmci_transport.c
>+++ b/net/vmw_vsock/vmci_transport.c
>@@ -2033,6 +2033,17 @@ static u32 vmci_transport_get_local_cid(void)
> return vmci_get_context_id();
> }
>
>+static bool vmci_transport_supports_local_mode(void)
>+{
>+ /* Local mode is not yet compatible with vmci because there is no clear
>+ * mechanism yet for attaching a namespace to a VM, or for handling the
>+ * namespacing for when neither H2G or G2H is registered (as is the
>+ * case for MODULE_ALIAS_NETPROTO(PF_VSOCK) loading. To simplify, we
>+ * keep local mode off for now.
>+ */
>+ return false;
>+}
>+
> static struct vsock_transport vmci_transport = {
> .module = THIS_MODULE,
> .init = vmci_transport_socket_init,
>@@ -2062,6 +2073,7 @@ static struct vsock_transport vmci_transport = {
> .notify_send_post_enqueue = vmci_transport_notify_send_post_enqueue,
> .shutdown = vmci_transport_shutdown,
> .get_local_cid = vmci_transport_get_local_cid,
>+ .supports_local_mode = vmci_transport_supports_local_mode,
> };
>
> static bool vmci_check_transport(struct vsock_sock *vsk)
>diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
>index 8722337a4f80..1e25c1a6b43f 100644
>--- a/net/vmw_vsock/vsock_loopback.c
>+++ b/net/vmw_vsock/vsock_loopback.c
>@@ -26,6 +26,11 @@ static u32 vsock_loopback_get_local_cid(void)
> return VMADDR_CID_LOCAL;
> }
>
>+static bool vsock_loopback_supports_local_mode(void)
>+{
>+ return true;
>+}
>+
> static int vsock_loopback_send_pkt(struct sk_buff *skb)
> {
> struct vsock_loopback *vsock = &the_vsock_loopback;
>@@ -58,6 +63,7 @@ static struct virtio_transport loopback_transport = {
> .module = THIS_MODULE,
>
> .get_local_cid = vsock_loopback_get_local_cid,
>+ .supports_local_mode = vsock_loopback_supports_local_mode,
>
> .init = virtio_transport_do_socket_init,
> .destruct = virtio_transport_destruct,
>
>--
>2.47.3
>
^ permalink raw reply
* [PATCH v2 3/3] x86/hyperv: Remove ASM_CALL_CONSTRAINT with VMMCALL insn
From: Uros Bizjak @ 2025-11-21 14:14 UTC (permalink / raw)
To: linux-hyperv, x86, linux-kernel
Cc: Uros Bizjak, Michael Kelley, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin
In-Reply-To: <20251121141437.205481-1-ubizjak@gmail.com>
Unlike CALL instruction, VMMCALL does not push to the stack, so it's
OK to allow the compiler to insert it before the frame pointer gets
set up by the containing function. ASM_CALL_CONSTRAINT is for CALLs
that must be inserted after the frame pointer is set up, so it is
over-constraining here and can be removed.
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Tested-by: Michael Kelley <mhklinux@outlook.com>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Wei Liu <wei.liu@kernel.org>
Cc: Dexuan Cui <decui@microsoft.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
v2: Expand commit message and include ASM_CALL_CONSTRAINT explanation
---
arch/x86/hyperv/ivm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
index 7365d8f43181..be7fad43a88d 100644
--- a/arch/x86/hyperv/ivm.c
+++ b/arch/x86/hyperv/ivm.c
@@ -392,7 +392,7 @@ u64 hv_snp_hypercall(u64 control, u64 param1, u64 param2)
register u64 __r8 asm("r8") = param2;
asm volatile("vmmcall"
- : "=a" (hv_status), ASM_CALL_CONSTRAINT,
+ : "=a" (hv_status),
"+c" (control), "+d" (param1), "+r" (__r8)
: : "cc", "memory", "r9", "r10", "r11");
--
2.51.1
^ permalink raw reply related
* [PATCH v2 2/3] x86/hyperv: Use savesegment() instead of inline asm() to save segment registers
From: Uros Bizjak @ 2025-11-21 14:14 UTC (permalink / raw)
To: linux-hyperv, x86, linux-kernel
Cc: Uros Bizjak, Wei Liu, Michael Kelley, K. Y. Srinivasan,
Haiyang Zhang, Dexuan Cui, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin
In-Reply-To: <20251121141437.205481-1-ubizjak@gmail.com>
Use standard savesegment() utility macro to save segment registers.
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Acked-by: Wei Liu <wei.liu@kernel.org>
Tested-by: Michael Kelley <mhklinux@outlook.com>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Dexuan Cui <decui@microsoft.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
arch/x86/hyperv/ivm.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
index 651771534cae..7365d8f43181 100644
--- a/arch/x86/hyperv/ivm.c
+++ b/arch/x86/hyperv/ivm.c
@@ -25,6 +25,7 @@
#include <asm/e820/api.h>
#include <asm/desc.h>
#include <asm/msr.h>
+#include <asm/segment.h>
#include <uapi/asm/vmx.h>
#ifdef CONFIG_AMD_MEM_ENCRYPT
@@ -315,16 +316,16 @@ int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip, unsigned int cpu)
vmsa->gdtr.base = gdtr.address;
vmsa->gdtr.limit = gdtr.size;
- asm volatile("movl %%es, %%eax;" : "=a" (vmsa->es.selector));
+ savesegment(es, vmsa->es.selector);
hv_populate_vmcb_seg(vmsa->es, vmsa->gdtr.base);
- asm volatile("movl %%cs, %%eax;" : "=a" (vmsa->cs.selector));
+ savesegment(cs, vmsa->cs.selector);
hv_populate_vmcb_seg(vmsa->cs, vmsa->gdtr.base);
- asm volatile("movl %%ss, %%eax;" : "=a" (vmsa->ss.selector));
+ savesegment(ss, vmsa->ss.selector);
hv_populate_vmcb_seg(vmsa->ss, vmsa->gdtr.base);
- asm volatile("movl %%ds, %%eax;" : "=a" (vmsa->ds.selector));
+ savesegment(ds, vmsa->ds.selector);
hv_populate_vmcb_seg(vmsa->ds, vmsa->gdtr.base);
vmsa->efer = native_read_msr(MSR_EFER);
--
2.51.1
^ permalink raw reply related
* [PATCH v2 1/3] x86: Use MOVL when reading segment registers
From: Uros Bizjak @ 2025-11-21 14:14 UTC (permalink / raw)
To: linux-hyperv, x86, linux-kernel
Cc: Uros Bizjak, Michael Kelley, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin
Use MOVL when reading segment registers to avoid 0x66 operand-size
override insn prefix. The segment value is always 16-bit and gets
zero-extended to the full 32-bit size.
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Tested-by: Michael Kelley <mhklinux@outlook.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
---
arch/x86/include/asm/segment.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
index f59ae7186940..9f5be2bbd291 100644
--- a/arch/x86/include/asm/segment.h
+++ b/arch/x86/include/asm/segment.h
@@ -348,7 +348,7 @@ static inline void __loadsegment_fs(unsigned short value)
* Save a segment register away:
*/
#define savesegment(seg, value) \
- asm("mov %%" #seg ",%0":"=r" (value) : : "memory")
+ asm("movl %%" #seg ",%k0" : "=r" (value) : : "memory")
#endif /* !__ASSEMBLER__ */
#endif /* __KERNEL__ */
--
2.51.1
^ permalink raw reply related
* [PATCH 6/6] sysfb: Move edid_info into sysfb_primary_display
From: Thomas Zimmermann @ 2025-11-21 13:36 UTC (permalink / raw)
To: ardb, javierm, arnd
Cc: x86, linux-arm-kernel, linux-kernel, linux-efi, loongarch,
linux-riscv, dri-devel, linux-hyperv, linux-pci, linux-fbdev,
Thomas Zimmermann
In-Reply-To: <20251121135624.494768-1-tzimmermann@suse.de>
Move x86's edid_info into sysfb_primary_display as a new field named
edid. Adapt all users.
An instance of edid_info has only been defined on x86. With the move
into sysfb_primary_display, it becomes available on all architectures.
Therefore remove this contraint from CONFIG_FIRMWARE_EDID.
x86 fills the EDID data from boot_params.edid_info. DRM drivers pick
up the raw data and make it available to DRM clients. Replace the
drivers' references to edid_info and instead use the sysfb_display_info
as passed from sysfb.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
arch/x86/kernel/setup.c | 6 +-----
drivers/gpu/drm/sysfb/efidrm.c | 5 ++---
drivers/gpu/drm/sysfb/vesadrm.c | 5 ++---
drivers/video/Kconfig | 1 -
drivers/video/fbdev/core/fbmon.c | 8 +++++---
include/linux/sysfb.h | 6 ++++++
include/video/edid.h | 4 ----
7 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 675e4b9deb1f..d9bfe2032cd9 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -215,10 +215,6 @@ arch_initcall(init_x86_sysctl);
struct sysfb_display_info sysfb_primary_display;
EXPORT_SYMBOL(sysfb_primary_display);
-#if defined(CONFIG_FIRMWARE_EDID)
-struct edid_info edid_info;
-EXPORT_SYMBOL_GPL(edid_info);
-#endif
extern int root_mountflags;
@@ -530,7 +526,7 @@ static void __init parse_boot_params(void)
ROOT_DEV = old_decode_dev(boot_params.hdr.root_dev);
sysfb_primary_display.screen = boot_params.screen_info;
#if defined(CONFIG_FIRMWARE_EDID)
- edid_info = boot_params.edid_info;
+ sysfb_primary_display.edid = boot_params.edid_info;
#endif
#ifdef CONFIG_X86_32
apm_info.bios = boot_params.apm_bios_info;
diff --git a/drivers/gpu/drm/sysfb/efidrm.c b/drivers/gpu/drm/sysfb/efidrm.c
index 29533ae8fbbf..50e0aeef709c 100644
--- a/drivers/gpu/drm/sysfb/efidrm.c
+++ b/drivers/gpu/drm/sysfb/efidrm.c
@@ -24,7 +24,6 @@
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
-#include <video/edid.h>
#include <video/pixel_format.h>
#include "drm_sysfb_helper.h"
@@ -207,8 +206,8 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
&format->format, width, height, stride);
#if defined(CONFIG_FIRMWARE_EDID)
- if (drm_edid_header_is_valid(edid_info.dummy) == 8)
- sysfb->edid = edid_info.dummy;
+ if (drm_edid_header_is_valid(dpy->edid.dummy) == 8)
+ sysfb->edid = dpy->edid.dummy;
#endif
sysfb->fb_mode = drm_sysfb_mode(width, height, 0, 0);
sysfb->fb_format = format;
diff --git a/drivers/gpu/drm/sysfb/vesadrm.c b/drivers/gpu/drm/sysfb/vesadrm.c
index 16fc223f8c5b..0680638b8131 100644
--- a/drivers/gpu/drm/sysfb/vesadrm.c
+++ b/drivers/gpu/drm/sysfb/vesadrm.c
@@ -25,7 +25,6 @@
#include <drm/drm_print.h>
#include <drm/drm_probe_helper.h>
-#include <video/edid.h>
#include <video/pixel_format.h>
#include <video/vga.h>
@@ -474,8 +473,8 @@ static struct vesadrm_device *vesadrm_device_create(struct drm_driver *drv,
}
#if defined(CONFIG_FIRMWARE_EDID)
- if (drm_edid_header_is_valid(edid_info.dummy) == 8)
- sysfb->edid = edid_info.dummy;
+ if (drm_edid_header_is_valid(dpy->edid.dummy) == 8)
+ sysfb->edid = dpy->edid.dummy;
#endif
sysfb->fb_mode = drm_sysfb_mode(width, height, 0, 0);
sysfb->fb_format = format;
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index d51777df12d1..ad55e7d62159 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -63,7 +63,6 @@ endif # HAS_IOMEM
config FIRMWARE_EDID
bool "Enable firmware EDID"
- depends on X86
help
This enables access to the EDID transferred from the firmware.
On x86, this is from the VESA BIOS. DRM display drivers will
diff --git a/drivers/video/fbdev/core/fbmon.c b/drivers/video/fbdev/core/fbmon.c
index 0a65bef01e3c..07df7e98f8a3 100644
--- a/drivers/video/fbdev/core/fbmon.c
+++ b/drivers/video/fbdev/core/fbmon.c
@@ -32,11 +32,13 @@
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/slab.h>
-#include <video/edid.h>
+#include <linux/string_choices.h>
+#include <linux/sysfb.h>
+
#include <video/of_videomode.h>
#include <video/videomode.h>
+
#include "../edid.h"
-#include <linux/string_choices.h>
/*
* EDID parser
@@ -1504,7 +1506,7 @@ const unsigned char *fb_firmware_edid(struct device *device)
res = &dev->resource[PCI_ROM_RESOURCE];
if (res && res->flags & IORESOURCE_ROM_SHADOW)
- edid = edid_info.dummy;
+ edid = sysfb_primary_display.edid.dummy;
return edid;
}
diff --git a/include/linux/sysfb.h b/include/linux/sysfb.h
index e8bde392c690..5226efde9ad4 100644
--- a/include/linux/sysfb.h
+++ b/include/linux/sysfb.h
@@ -12,6 +12,8 @@
#include <linux/screen_info.h>
#include <linux/types.h>
+#include <video/edid.h>
+
struct device;
struct platform_device;
struct screen_info;
@@ -62,6 +64,10 @@ struct efifb_dmi_info {
struct sysfb_display_info {
struct screen_info screen;
+
+#if defined(CONFIG_FIRMWARE_EDID)
+ struct edid_info edid;
+#endif
};
extern struct sysfb_display_info sysfb_primary_display;
diff --git a/include/video/edid.h b/include/video/edid.h
index c2b186b1933a..52aabb706032 100644
--- a/include/video/edid.h
+++ b/include/video/edid.h
@@ -4,8 +4,4 @@
#include <uapi/video/edid.h>
-#if defined(CONFIG_FIRMWARE_EDID)
-extern struct edid_info edid_info;
-#endif
-
#endif /* __linux_video_edid_h__ */
--
2.51.1
^ permalink raw reply related
* [PATCH 3/6] sysfb: Add struct sysfb_display_info
From: Thomas Zimmermann @ 2025-11-21 13:36 UTC (permalink / raw)
To: ardb, javierm, arnd
Cc: x86, linux-arm-kernel, linux-kernel, linux-efi, loongarch,
linux-riscv, dri-devel, linux-hyperv, linux-pci, linux-fbdev,
Thomas Zimmermann
In-Reply-To: <20251121135624.494768-1-tzimmermann@suse.de>
Add struct sysfb_display_info to wrap display-related state. For now
it contains only the screen's video mode. Later EDID will be added as
well.
This struct will be helpful for passing display state to sysfb drivers
or from the EFI stub library.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
include/linux/sysfb.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/sysfb.h b/include/linux/sysfb.h
index 8527a50a5290..8b37247528bf 100644
--- a/include/linux/sysfb.h
+++ b/include/linux/sysfb.h
@@ -8,6 +8,7 @@
*/
#include <linux/err.h>
+#include <linux/screen_info.h>
#include <linux/types.h>
#include <linux/platform_data/simplefb.h>
@@ -60,6 +61,10 @@ struct efifb_dmi_info {
int flags;
};
+struct sysfb_display_info {
+ struct screen_info screen;
+};
+
#ifdef CONFIG_SYSFB
void sysfb_disable(struct device *dev);
--
2.51.1
^ permalink raw reply related
* [PATCH 5/6] sysfb: Pass sysfb_primary_display to devices
From: Thomas Zimmermann @ 2025-11-21 13:36 UTC (permalink / raw)
To: ardb, javierm, arnd
Cc: x86, linux-arm-kernel, linux-kernel, linux-efi, loongarch,
linux-riscv, dri-devel, linux-hyperv, linux-pci, linux-fbdev,
Thomas Zimmermann
In-Reply-To: <20251121135624.494768-1-tzimmermann@suse.de>
Instead of screen_info, store a copy of sysfb_primary_display as
device data. Pick it up in drivers. Later changes will add additional
data to the display info, such as EDID information.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/firmware/sysfb.c | 5 +++--
drivers/gpu/drm/sysfb/efidrm.c | 9 ++++++---
drivers/gpu/drm/sysfb/vesadrm.c | 9 ++++++---
drivers/video/fbdev/efifb.c | 10 ++++++----
drivers/video/fbdev/vesafb.c | 10 ++++++----
drivers/video/fbdev/vga16fb.c | 8 +++++---
6 files changed, 32 insertions(+), 19 deletions(-)
diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c
index 1f671f9219b0..8833582c1883 100644
--- a/drivers/firmware/sysfb.c
+++ b/drivers/firmware/sysfb.c
@@ -141,7 +141,8 @@ static struct device *sysfb_parent_dev(const struct screen_info *si)
static __init int sysfb_init(void)
{
- struct screen_info *si = &sysfb_primary_display.screen;
+ struct sysfb_display_info *dpy = &sysfb_primary_display;
+ struct screen_info *si = &dpy->screen;
struct device *parent;
unsigned int type;
struct simplefb_platform_data mode;
@@ -202,7 +203,7 @@ static __init int sysfb_init(void)
sysfb_set_efifb_fwnode(si, pd);
- ret = platform_device_add_data(pd, si, sizeof(*si));
+ ret = platform_device_add_data(pd, dpy, sizeof(*dpy));
if (ret)
goto err;
diff --git a/drivers/gpu/drm/sysfb/efidrm.c b/drivers/gpu/drm/sysfb/efidrm.c
index 1b683d55d6ea..29533ae8fbbf 100644
--- a/drivers/gpu/drm/sysfb/efidrm.c
+++ b/drivers/gpu/drm/sysfb/efidrm.c
@@ -4,7 +4,7 @@
#include <linux/efi.h>
#include <linux/limits.h>
#include <linux/platform_device.h>
-#include <linux/screen_info.h>
+#include <linux/sysfb.h>
#include <drm/clients/drm_client_setup.h>
#include <drm/drm_atomic.h>
@@ -141,6 +141,7 @@ static const struct drm_mode_config_funcs efidrm_mode_config_funcs = {
static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
struct platform_device *pdev)
{
+ const struct sysfb_display_info *dpy;
const struct screen_info *si;
const struct drm_format_info *format;
int width, height, stride;
@@ -160,9 +161,11 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
size_t nformats;
int ret;
- si = dev_get_platdata(&pdev->dev);
- if (!si)
+ dpy = dev_get_platdata(&pdev->dev);
+ if (!dpy)
return ERR_PTR(-ENODEV);
+ si = &dpy->screen;
+
if (screen_info_video_type(si) != VIDEO_TYPE_EFI)
return ERR_PTR(-ENODEV);
diff --git a/drivers/gpu/drm/sysfb/vesadrm.c b/drivers/gpu/drm/sysfb/vesadrm.c
index 7b7b5ba26317..16fc223f8c5b 100644
--- a/drivers/gpu/drm/sysfb/vesadrm.c
+++ b/drivers/gpu/drm/sysfb/vesadrm.c
@@ -4,7 +4,7 @@
#include <linux/ioport.h>
#include <linux/limits.h>
#include <linux/platform_device.h>
-#include <linux/screen_info.h>
+#include <linux/sysfb.h>
#include <drm/clients/drm_client_setup.h>
#include <drm/drm_atomic.h>
@@ -391,6 +391,7 @@ static const struct drm_mode_config_funcs vesadrm_mode_config_funcs = {
static struct vesadrm_device *vesadrm_device_create(struct drm_driver *drv,
struct platform_device *pdev)
{
+ const struct sysfb_display_info *dpy;
const struct screen_info *si;
const struct drm_format_info *format;
int width, height, stride;
@@ -410,9 +411,11 @@ static struct vesadrm_device *vesadrm_device_create(struct drm_driver *drv,
size_t nformats;
int ret;
- si = dev_get_platdata(&pdev->dev);
- if (!si)
+ dpy = dev_get_platdata(&pdev->dev);
+ if (!dpy)
return ERR_PTR(-ENODEV);
+ si = &dpy->screen;
+
if (screen_info_video_type(si) != VIDEO_TYPE_VLFB)
return ERR_PTR(-ENODEV);
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 0e1bd3dba255..47ebc0107209 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -15,7 +15,7 @@
#include <linux/fb.h>
#include <linux/platform_device.h>
#include <linux/printk.h>
-#include <linux/screen_info.h>
+#include <linux/sysfb.h>
#include <video/vga.h>
#include <asm/efi.h>
#include <drm/drm_utils.h> /* For drm_get_panel_orientation_quirk */
@@ -345,6 +345,7 @@ ATTRIBUTE_GROUPS(efifb);
static int efifb_probe(struct platform_device *dev)
{
+ struct sysfb_display_info *dpy;
struct screen_info *si;
struct fb_info *info;
struct efifb_par *par;
@@ -360,10 +361,11 @@ static int efifb_probe(struct platform_device *dev)
* driver. We get a copy of the attached screen_info, so that we can
* modify its values without affecting later drivers.
*/
- si = dev_get_platdata(&dev->dev);
- if (!si)
+ dpy = dev_get_platdata(&dev->dev);
+ if (!dpy)
return -ENODEV;
- si = devm_kmemdup(&dev->dev, si, sizeof(*si), GFP_KERNEL);
+
+ si = devm_kmemdup(&dev->dev, &dpy->screen, sizeof(*si), GFP_KERNEL);
if (!si)
return -ENOMEM;
diff --git a/drivers/video/fbdev/vesafb.c b/drivers/video/fbdev/vesafb.c
index a81df8865143..10d3d9ae73aa 100644
--- a/drivers/video/fbdev/vesafb.c
+++ b/drivers/video/fbdev/vesafb.c
@@ -20,7 +20,7 @@
#include <linux/ioport.h>
#include <linux/init.h>
#include <linux/platform_device.h>
-#include <linux/screen_info.h>
+#include <linux/sysfb.h>
#include <linux/io.h>
#include <video/vga.h>
@@ -243,6 +243,7 @@ static int vesafb_setup(char *options)
static int vesafb_probe(struct platform_device *dev)
{
+ struct sysfb_display_info *dpy;
struct screen_info *si;
struct fb_info *info;
struct vesafb_par *par;
@@ -257,10 +258,11 @@ static int vesafb_probe(struct platform_device *dev)
* driver. We get a copy of the attached screen_info, so that we can
* modify its values without affecting later drivers.
*/
- si = dev_get_platdata(&dev->dev);
- if (!si)
+ dpy = dev_get_platdata(&dev->dev);
+ if (!dpy)
return -ENODEV;
- si = devm_kmemdup(&dev->dev, si, sizeof(*si), GFP_KERNEL);
+
+ si = devm_kmemdup(&dev->dev, &dpy->screen, sizeof(*si), GFP_KERNEL);
if (!si)
return -ENOMEM;
diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
index eedab14c7d51..7853d84f10df 100644
--- a/drivers/video/fbdev/vga16fb.c
+++ b/drivers/video/fbdev/vga16fb.c
@@ -21,7 +21,7 @@
#include <linux/ioport.h>
#include <linux/init.h>
#include <linux/platform_device.h>
-#include <linux/screen_info.h>
+#include <linux/sysfb.h>
#include <asm/io.h>
#include <video/vga.h>
@@ -1305,15 +1305,17 @@ static const struct fb_ops vga16fb_ops = {
static int vga16fb_probe(struct platform_device *dev)
{
+ struct sysfb_display_info *dpy;
struct screen_info *si;
struct fb_info *info;
struct vga16fb_par *par;
int i;
int ret = 0;
- si = dev_get_platdata(&dev->dev);
- if (!si)
+ dpy = dev_get_platdata(&dev->dev);
+ if (!dpy)
return -ENODEV;
+ si = &dpy->screen;
ret = check_mode_supported(si);
if (ret)
--
2.51.1
^ permalink raw reply related
* [PATCH 2/6] efi: sysfb_efi: Reduce number of references to global screen_info
From: Thomas Zimmermann @ 2025-11-21 13:36 UTC (permalink / raw)
To: ardb, javierm, arnd
Cc: x86, linux-arm-kernel, linux-kernel, linux-efi, loongarch,
linux-riscv, dri-devel, linux-hyperv, linux-pci, linux-fbdev,
Thomas Zimmermann
In-Reply-To: <20251121135624.494768-1-tzimmermann@suse.de>
Replace usage of global screen_info with local pointers. This will
later reduce churn when screen_info is being moved.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/firmware/efi/sysfb_efi.c | 81 ++++++++++++++++---------------
drivers/firmware/sysfb.c | 4 +-
drivers/firmware/sysfb_simplefb.c | 2 +-
include/linux/sysfb.h | 9 ++--
4 files changed, 51 insertions(+), 45 deletions(-)
diff --git a/drivers/firmware/efi/sysfb_efi.c b/drivers/firmware/efi/sysfb_efi.c
index 1e509595ac03..8e0f9d08397f 100644
--- a/drivers/firmware/efi/sysfb_efi.c
+++ b/drivers/firmware/efi/sysfb_efi.c
@@ -92,7 +92,7 @@ void efifb_setup_from_dmi(struct screen_info *si, const char *opt)
})
#ifdef CONFIG_EFI
-static int __init efifb_set_system(const struct dmi_system_id *id)
+static int __init efifb_set_system(struct screen_info *si, const struct dmi_system_id *id)
{
struct efifb_dmi_info *info = id->driver_data;
@@ -101,14 +101,14 @@ static int __init efifb_set_system(const struct dmi_system_id *id)
return 0;
/* Trust the bootloader over the DMI tables */
- if (screen_info.lfb_base == 0) {
+ if (si->lfb_base == 0) {
#if defined(CONFIG_PCI)
struct pci_dev *dev = NULL;
int found_bar = 0;
#endif
if (info->base) {
- screen_info.lfb_base = choose_value(info->base,
- screen_info.lfb_base, OVERRIDE_BASE,
+ si->lfb_base = choose_value(info->base,
+ si->lfb_base, OVERRIDE_BASE,
info->flags);
#if defined(CONFIG_PCI)
@@ -135,49 +135,53 @@ static int __init efifb_set_system(const struct dmi_system_id *id)
start = pci_resource_start(dev, i);
end = pci_resource_end(dev, i);
- if (screen_info.lfb_base >= start &&
- screen_info.lfb_base < end) {
+ if (si->lfb_base >= start && si->lfb_base < end) {
found_bar = 1;
break;
}
}
}
if (!found_bar)
- screen_info.lfb_base = 0;
+ si->lfb_base = 0;
#endif
}
}
- if (screen_info.lfb_base) {
- screen_info.lfb_linelength = choose_value(info->stride,
- screen_info.lfb_linelength, OVERRIDE_STRIDE,
+ if (si->lfb_base) {
+ si->lfb_linelength = choose_value(info->stride,
+ si->lfb_linelength, OVERRIDE_STRIDE,
info->flags);
- screen_info.lfb_width = choose_value(info->width,
- screen_info.lfb_width, OVERRIDE_WIDTH,
+ si->lfb_width = choose_value(info->width,
+ si->lfb_width, OVERRIDE_WIDTH,
info->flags);
- screen_info.lfb_height = choose_value(info->height,
- screen_info.lfb_height, OVERRIDE_HEIGHT,
+ si->lfb_height = choose_value(info->height,
+ si->lfb_height, OVERRIDE_HEIGHT,
info->flags);
- if (screen_info.orig_video_isVGA == 0)
- screen_info.orig_video_isVGA = VIDEO_TYPE_EFI;
+ if (si->orig_video_isVGA == 0)
+ si->orig_video_isVGA = VIDEO_TYPE_EFI;
} else {
- screen_info.lfb_linelength = 0;
- screen_info.lfb_width = 0;
- screen_info.lfb_height = 0;
- screen_info.orig_video_isVGA = 0;
+ si->lfb_linelength = 0;
+ si->lfb_width = 0;
+ si->lfb_height = 0;
+ si->orig_video_isVGA = 0;
return 0;
}
printk(KERN_INFO "efifb: dmi detected %s - framebuffer at 0x%08x "
"(%dx%d, stride %d)\n", id->ident,
- screen_info.lfb_base, screen_info.lfb_width,
- screen_info.lfb_height, screen_info.lfb_linelength);
+ si->lfb_base, si->lfb_width,
+ si->lfb_height, si->lfb_linelength);
return 1;
}
+static int __init efifb_set_system_callback(const struct dmi_system_id *id)
+{
+ return efifb_set_system(&screen_info, id);
+}
+
#define EFIFB_DMI_SYSTEM_ID(vendor, name, enumid) \
{ \
- efifb_set_system, \
+ efifb_set_system_callback, \
name, \
{ \
DMI_MATCH(DMI_BIOS_VENDOR, vendor), \
@@ -284,12 +288,13 @@ static const struct dmi_system_id efifb_dmi_swap_width_height[] __initconst = {
{},
};
-static bool efifb_overlaps_pci_range(const struct of_pci_range *range)
+static bool efifb_overlaps_pci_range(const struct screen_info *si,
+ const struct of_pci_range *range)
{
- u64 fb_base = screen_info.lfb_base;
+ u64 fb_base = si->lfb_base;
- if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
- fb_base |= (u64)(unsigned long)screen_info.ext_lfb_base << 32;
+ if (si->capabilities & VIDEO_CAPABILITY_64BIT_BASE)
+ fb_base |= (u64)(unsigned long)si->ext_lfb_base << 32;
return fb_base >= range->cpu_addr &&
fb_base < (range->cpu_addr + range->size);
@@ -311,7 +316,7 @@ static struct device_node *find_pci_overlap_node(void)
}
for_each_of_pci_range(&parser, &range)
- if (efifb_overlaps_pci_range(&range))
+ if (efifb_overlaps_pci_range(&screen_info, &range))
return np;
}
return NULL;
@@ -349,25 +354,25 @@ static const struct fwnode_operations efifb_fwnode_ops = {
static struct fwnode_handle efifb_fwnode;
-__init void sysfb_apply_efi_quirks(void)
+__init void sysfb_apply_efi_quirks(struct screen_info *si)
{
- if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI ||
- !(screen_info.capabilities & VIDEO_CAPABILITY_SKIP_QUIRKS))
+ if (si->orig_video_isVGA != VIDEO_TYPE_EFI ||
+ !(si->capabilities & VIDEO_CAPABILITY_SKIP_QUIRKS))
dmi_check_system(efifb_dmi_system_table);
- if (screen_info.orig_video_isVGA == VIDEO_TYPE_EFI &&
+ if (si->orig_video_isVGA == VIDEO_TYPE_EFI &&
dmi_check_system(efifb_dmi_swap_width_height)) {
- u16 temp = screen_info.lfb_width;
+ u16 temp = si->lfb_width;
- screen_info.lfb_width = screen_info.lfb_height;
- screen_info.lfb_height = temp;
- screen_info.lfb_linelength = 4 * screen_info.lfb_width;
+ si->lfb_width = si->lfb_height;
+ si->lfb_height = temp;
+ si->lfb_linelength = 4 * si->lfb_width;
}
}
-__init void sysfb_set_efifb_fwnode(struct platform_device *pd)
+__init void sysfb_set_efifb_fwnode(const struct screen_info *si, struct platform_device *pd)
{
- if (screen_info.orig_video_isVGA == VIDEO_TYPE_EFI && IS_ENABLED(CONFIG_PCI)) {
+ if (si->orig_video_isVGA == VIDEO_TYPE_EFI && IS_ENABLED(CONFIG_PCI)) {
fwnode_init(&efifb_fwnode, &efifb_fwnode_ops);
pd->dev.fwnode = &efifb_fwnode;
}
diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c
index 889e5b05c739..916b28538a29 100644
--- a/drivers/firmware/sysfb.c
+++ b/drivers/firmware/sysfb.c
@@ -155,7 +155,7 @@ static __init int sysfb_init(void)
if (disabled)
goto unlock_mutex;
- sysfb_apply_efi_quirks();
+ sysfb_apply_efi_quirks(si);
parent = sysfb_parent_dev(si);
if (IS_ERR(parent)) {
@@ -200,7 +200,7 @@ static __init int sysfb_init(void)
pd->dev.parent = parent;
- sysfb_set_efifb_fwnode(pd);
+ sysfb_set_efifb_fwnode(si, pd);
ret = platform_device_add_data(pd, si, sizeof(*si));
if (ret)
diff --git a/drivers/firmware/sysfb_simplefb.c b/drivers/firmware/sysfb_simplefb.c
index 592d8a644619..71f542e37732 100644
--- a/drivers/firmware/sysfb_simplefb.c
+++ b/drivers/firmware/sysfb_simplefb.c
@@ -117,7 +117,7 @@ __init struct platform_device *sysfb_create_simplefb(const struct screen_info *s
pd->dev.parent = parent;
- sysfb_set_efifb_fwnode(pd);
+ sysfb_set_efifb_fwnode(si, pd);
ret = platform_device_add_resources(pd, &res, 1);
if (ret)
diff --git a/include/linux/sysfb.h b/include/linux/sysfb.h
index b449665c686a..8527a50a5290 100644
--- a/include/linux/sysfb.h
+++ b/include/linux/sysfb.h
@@ -82,16 +82,17 @@ static inline bool sysfb_handles_screen_info(void)
#ifdef CONFIG_EFI
extern struct efifb_dmi_info efifb_dmi_list[];
-void sysfb_apply_efi_quirks(void);
-void sysfb_set_efifb_fwnode(struct platform_device *pd);
+void sysfb_apply_efi_quirks(struct screen_info *si);
+void sysfb_set_efifb_fwnode(const struct screen_info *si, struct platform_device *pd);
#else /* CONFIG_EFI */
-static inline void sysfb_apply_efi_quirks(void)
+static inline void sysfb_apply_efi_quirks(struct screen_info *si)
{
}
-static inline void sysfb_set_efifb_fwnode(struct platform_device *pd)
+static inline void sysfb_set_efifb_fwnode(const struct screen_info *si,
+ struct platform_device *pd)
{
}
--
2.51.1
^ permalink raw reply related
* [PATCH 4/6] sysfb: Replace screen_info with sysfb_primary_display
From: Thomas Zimmermann @ 2025-11-21 13:36 UTC (permalink / raw)
To: ardb, javierm, arnd
Cc: x86, linux-arm-kernel, linux-kernel, linux-efi, loongarch,
linux-riscv, dri-devel, linux-hyperv, linux-pci, linux-fbdev,
Thomas Zimmermann
In-Reply-To: <20251121135624.494768-1-tzimmermann@suse.de>
Replace the global screen_info with sysfb_primary_display of type
struct sysfb_display_info. Adapt all users of screen_info.
Instances of screen_info are defined for x86, loongarch and EFI,
with only one instance compiled into a specific build. Replace all
of them with sysfb_primary_display.
All existing users of screen_info are updated by pointing them to
sysfb_primary_display.screen instead. This introduces some churn to
the code, but has no impact on functionality.
Boot parameters and EFI config tables are unchanged. They transfer
screen_info as before. The logic in EFI's alloc_screen_info() changes
slightly, as it now returns the screen field of sysfb_primary_display.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
arch/arm64/kernel/image-vars.h | 2 +-
arch/loongarch/kernel/efi.c | 11 ++++++-----
arch/loongarch/kernel/image-vars.h | 2 +-
arch/riscv/kernel/image-vars.h | 2 +-
arch/x86/kernel/kexec-bzimage64.c | 4 +++-
arch/x86/kernel/setup.c | 10 ++++++----
arch/x86/video/video-common.c | 4 ++--
drivers/firmware/efi/earlycon.c | 8 ++++----
drivers/firmware/efi/efi-init.c | 14 +++++++-------
drivers/firmware/efi/libstub/efi-stub-entry.c | 18 +++++++++++++-----
drivers/firmware/efi/sysfb_efi.c | 4 ++--
drivers/firmware/sysfb.c | 6 +++---
drivers/hv/vmbus_drv.c | 6 +++---
drivers/pci/vgaarb.c | 4 ++--
drivers/video/screen_info_pci.c | 5 +++--
include/linux/screen_info.h | 2 --
include/linux/sysfb.h | 5 +++--
17 files changed, 60 insertions(+), 47 deletions(-)
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 5369763606e7..a7867bd8cfb6 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -38,7 +38,7 @@ PROVIDE(__efistub__end = _end);
PROVIDE(__efistub___inittext_end = __inittext_end);
PROVIDE(__efistub__edata = _edata);
#if defined(CONFIG_EFI_EARLYCON) || defined(CONFIG_SYSFB)
-PROVIDE(__efistub_screen_info = screen_info);
+PROVIDE(__efistub_sysfb_primary_display = sysfb_primary_display);
#endif
PROVIDE(__efistub__ctype = _ctype);
diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
index 860a3bc030e0..b11188413f4d 100644
--- a/arch/loongarch/kernel/efi.c
+++ b/arch/loongarch/kernel/efi.c
@@ -18,7 +18,7 @@
#include <linux/kobject.h>
#include <linux/memblock.h>
#include <linux/reboot.h>
-#include <linux/screen_info.h>
+#include <linux/sysfb.h>
#include <linux/uaccess.h>
#include <asm/early_ioremap.h>
@@ -75,8 +75,8 @@ bool efi_poweroff_required(void)
unsigned long __initdata screen_info_table = EFI_INVALID_TABLE_ADDR;
#if defined(CONFIG_SYSFB) || defined(CONFIG_EFI_EARLYCON)
-struct screen_info screen_info __section(".data");
-EXPORT_SYMBOL_GPL(screen_info);
+struct sysfb_display_info sysfb_primary_display __section(".data");
+EXPORT_SYMBOL_GPL(sysfb_primary_display);
#endif
static void __init init_screen_info(void)
@@ -91,11 +91,12 @@ static void __init init_screen_info(void)
pr_err("Could not map screen_info config table\n");
return;
}
- screen_info = *si;
+ sysfb_primary_display.screen = *si;
memset(si, 0, sizeof(*si));
early_memunmap(si, sizeof(*si));
- memblock_reserve(__screen_info_lfb_base(&screen_info), screen_info.lfb_size);
+ memblock_reserve(__screen_info_lfb_base(&sysfb_primary_display.screen),
+ sysfb_primary_display.screen.lfb_size);
}
void __init efi_init(void)
diff --git a/arch/loongarch/kernel/image-vars.h b/arch/loongarch/kernel/image-vars.h
index 41ddcf56d21c..e557ebd46c2b 100644
--- a/arch/loongarch/kernel/image-vars.h
+++ b/arch/loongarch/kernel/image-vars.h
@@ -12,7 +12,7 @@ __efistub_kernel_entry = kernel_entry;
__efistub_kernel_asize = kernel_asize;
__efistub_kernel_fsize = kernel_fsize;
#if defined(CONFIG_EFI_EARLYCON) || defined(CONFIG_SYSFB)
-__efistub_screen_info = screen_info;
+__efistub_sysfb_primary_display = sysfb_primary_display;
#endif
#endif
diff --git a/arch/riscv/kernel/image-vars.h b/arch/riscv/kernel/image-vars.h
index 3df30dd1c458..3bd9d06a8b8f 100644
--- a/arch/riscv/kernel/image-vars.h
+++ b/arch/riscv/kernel/image-vars.h
@@ -29,7 +29,7 @@ __efistub__end = _end;
__efistub__edata = _edata;
__efistub___init_text_end = __init_text_end;
#if defined(CONFIG_EFI_EARLYCON) || defined(CONFIG_SYSFB)
-__efistub_screen_info = screen_info;
+__efistub_sysfb_primary_display = sysfb_primary_display;
#endif
#endif
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index c3244ac680d1..7508d0ccc740 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -20,6 +20,7 @@
#include <linux/of_fdt.h>
#include <linux/efi.h>
#include <linux/random.h>
+#include <linux/sysfb.h>
#include <asm/bootparam.h>
#include <asm/setup.h>
@@ -303,7 +304,8 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params,
params->hdr.hardware_subarch = boot_params.hdr.hardware_subarch;
/* Copying screen_info will do? */
- memcpy(¶ms->screen_info, &screen_info, sizeof(struct screen_info));
+ memcpy(¶ms->screen_info, &sysfb_primary_display.screen,
+ sizeof(sysfb_primary_display.screen));
/* Fill in memsize later */
params->screen_info.ext_mem_k = 0;
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 1b2edd07a3e1..675e4b9deb1f 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -22,6 +22,7 @@
#include <linux/random.h>
#include <linux/root_dev.h>
#include <linux/static_call.h>
+#include <linux/sysfb.h>
#include <linux/swiotlb.h>
#include <linux/tboot.h>
#include <linux/usb/xhci-dbgp.h>
@@ -211,8 +212,9 @@ arch_initcall(init_x86_sysctl);
/*
* Setup options
*/
-struct screen_info screen_info;
-EXPORT_SYMBOL(screen_info);
+
+struct sysfb_display_info sysfb_primary_display;
+EXPORT_SYMBOL(sysfb_primary_display);
#if defined(CONFIG_FIRMWARE_EDID)
struct edid_info edid_info;
EXPORT_SYMBOL_GPL(edid_info);
@@ -526,7 +528,7 @@ static void __init parse_setup_data(void)
static void __init parse_boot_params(void)
{
ROOT_DEV = old_decode_dev(boot_params.hdr.root_dev);
- screen_info = boot_params.screen_info;
+ sysfb_primary_display.screen = boot_params.screen_info;
#if defined(CONFIG_FIRMWARE_EDID)
edid_info = boot_params.edid_info;
#endif
@@ -1254,7 +1256,7 @@ void __init setup_arch(char **cmdline_p)
#ifdef CONFIG_VT
#if defined(CONFIG_VGA_CONSOLE)
if (!efi_enabled(EFI_BOOT) || (efi_mem_type(0xa0000) != EFI_CONVENTIONAL_MEMORY))
- vgacon_register_screen(&screen_info);
+ vgacon_register_screen(&sysfb_primary_display.screen);
#endif
#endif
x86_init.oem.banner();
diff --git a/arch/x86/video/video-common.c b/arch/x86/video/video-common.c
index e0aeee99bc99..152789f00fcd 100644
--- a/arch/x86/video/video-common.c
+++ b/arch/x86/video/video-common.c
@@ -9,7 +9,7 @@
#include <linux/module.h>
#include <linux/pci.h>
-#include <linux/screen_info.h>
+#include <linux/sysfb.h>
#include <linux/vgaarb.h>
#include <asm/video.h>
@@ -29,7 +29,7 @@ EXPORT_SYMBOL(pgprot_framebuffer);
bool video_is_primary_device(struct device *dev)
{
#ifdef CONFIG_SCREEN_INFO
- struct screen_info *si = &screen_info;
+ struct screen_info *si = &sysfb_primary_display.screen;
struct resource res[SCREEN_INFO_MAX_RESOURCES];
ssize_t i, numres;
#endif
diff --git a/drivers/firmware/efi/earlycon.c b/drivers/firmware/efi/earlycon.c
index fac3a295c57f..d63849457164 100644
--- a/drivers/firmware/efi/earlycon.c
+++ b/drivers/firmware/efi/earlycon.c
@@ -9,7 +9,7 @@
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/serial_core.h>
-#include <linux/screen_info.h>
+#include <linux/sysfb.h>
#include <linux/string.h>
#include <asm/early_ioremap.h>
@@ -32,7 +32,7 @@ static void *efi_fb;
*/
static int __init efi_earlycon_remap_fb(void)
{
- const struct screen_info *si = &screen_info;
+ const struct screen_info *si = &sysfb_primary_display.screen;
/* bail if there is no bootconsole or it was unregistered already */
if (!earlycon_console || !console_is_registered(earlycon_console))
@@ -147,7 +147,7 @@ static void efi_earlycon_write_char(u32 *dst, unsigned char c, unsigned int h,
static void
efi_earlycon_write(struct console *con, const char *str, unsigned int num)
{
- const struct screen_info *si = &screen_info;
+ const struct screen_info *si = &sysfb_primary_display.screen;
u32 cur_efi_x = efi_x;
unsigned int len;
const char *s;
@@ -227,7 +227,7 @@ void __init efi_earlycon_reprobe(void)
static int __init efi_earlycon_setup(struct earlycon_device *device,
const char *opt)
{
- const struct screen_info *si = &screen_info;
+ const struct screen_info *si = &sysfb_primary_display.screen;
u16 xres, yres;
u32 i;
diff --git a/drivers/firmware/efi/efi-init.c b/drivers/firmware/efi/efi-init.c
index a65c2d5b9e7b..ced12c82d2a9 100644
--- a/drivers/firmware/efi/efi-init.c
+++ b/drivers/firmware/efi/efi-init.c
@@ -19,7 +19,7 @@
#include <linux/of_address.h>
#include <linux/of_fdt.h>
#include <linux/platform_device.h>
-#include <linux/screen_info.h>
+#include <linux/sysfb.h>
#include <asm/efi.h>
@@ -61,8 +61,8 @@ extern __weak const efi_config_table_type_t efi_arch_tables[];
* everything else can get it from here.
*/
#if !defined(CONFIG_X86) && (defined(CONFIG_SYSFB) || defined(CONFIG_EFI_EARLYCON))
-struct screen_info screen_info __section(".data");
-EXPORT_SYMBOL_GPL(screen_info);
+struct sysfb_display_info sysfb_primary_display __section(".data");
+EXPORT_SYMBOL_GPL(sysfb_primary_display);
#endif
static void __init init_screen_info(void)
@@ -75,13 +75,13 @@ static void __init init_screen_info(void)
pr_err("Could not map screen_info config table\n");
return;
}
- screen_info = *si;
+ sysfb_primary_display.screen = *si;
memset(si, 0, sizeof(*si));
early_memunmap(si, sizeof(*si));
- if (memblock_is_map_memory(screen_info.lfb_base))
- memblock_mark_nomap(screen_info.lfb_base,
- screen_info.lfb_size);
+ if (memblock_is_map_memory(sysfb_primary_display.screen.lfb_base))
+ memblock_mark_nomap(sysfb_primary_display.screen.lfb_base,
+ sysfb_primary_display.screen.lfb_size);
if (IS_ENABLED(CONFIG_EFI_EARLYCON))
efi_earlycon_reprobe();
diff --git a/drivers/firmware/efi/libstub/efi-stub-entry.c b/drivers/firmware/efi/libstub/efi-stub-entry.c
index a6c049835190..401ecbbdf331 100644
--- a/drivers/firmware/efi/libstub/efi-stub-entry.c
+++ b/drivers/firmware/efi/libstub/efi-stub-entry.c
@@ -1,13 +1,18 @@
// SPDX-License-Identifier: GPL-2.0-only
#include <linux/efi.h>
-#include <linux/screen_info.h>
+#include <linux/sysfb.h>
#include <asm/efi.h>
#include "efistub.h"
-static unsigned long screen_info_offset;
+static unsigned long kernel_image_offset;
+
+static void *kernel_image_addr(void *addr)
+{
+ return addr + kernel_image_offset;
+}
struct screen_info *alloc_screen_info(void)
{
@@ -16,8 +21,11 @@ struct screen_info *alloc_screen_info(void)
if (IS_ENABLED(CONFIG_X86) ||
IS_ENABLED(CONFIG_EFI_EARLYCON) ||
- IS_ENABLED(CONFIG_SYSFB))
- return (void *)&screen_info + screen_info_offset;
+ IS_ENABLED(CONFIG_SYSFB)) {
+ struct sysfb_display_info *dpy = kernel_image_addr(&sysfb_primary_display);
+
+ return &dpy->screen;
+ }
return NULL;
}
@@ -73,7 +81,7 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
return status;
}
- screen_info_offset = image_addr - (unsigned long)image->image_base;
+ kernel_image_offset = image_addr - (unsigned long)image->image_base;
status = efi_stub_common(handle, image, image_addr, cmdline_ptr);
diff --git a/drivers/firmware/efi/sysfb_efi.c b/drivers/firmware/efi/sysfb_efi.c
index 8e0f9d08397f..46ad95084b50 100644
--- a/drivers/firmware/efi/sysfb_efi.c
+++ b/drivers/firmware/efi/sysfb_efi.c
@@ -176,7 +176,7 @@ static int __init efifb_set_system(struct screen_info *si, const struct dmi_syst
static int __init efifb_set_system_callback(const struct dmi_system_id *id)
{
- return efifb_set_system(&screen_info, id);
+ return efifb_set_system(&sysfb_primary_display.screen, id);
}
#define EFIFB_DMI_SYSTEM_ID(vendor, name, enumid) \
@@ -316,7 +316,7 @@ static struct device_node *find_pci_overlap_node(void)
}
for_each_of_pci_range(&parser, &range)
- if (efifb_overlaps_pci_range(&screen_info, &range))
+ if (efifb_overlaps_pci_range(&sysfb_primary_display.screen, &range))
return np;
}
return NULL;
diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c
index 916b28538a29..1f671f9219b0 100644
--- a/drivers/firmware/sysfb.c
+++ b/drivers/firmware/sysfb.c
@@ -66,7 +66,7 @@ static bool sysfb_unregister(void)
*/
void sysfb_disable(struct device *dev)
{
- struct screen_info *si = &screen_info;
+ struct screen_info *si = &sysfb_primary_display.screen;
struct device *parent;
mutex_lock(&disable_lock);
@@ -92,7 +92,7 @@ EXPORT_SYMBOL_GPL(sysfb_disable);
*/
bool sysfb_handles_screen_info(void)
{
- const struct screen_info *si = &screen_info;
+ const struct screen_info *si = &sysfb_primary_display.screen;
return !!screen_info_video_type(si);
}
@@ -141,7 +141,7 @@ static struct device *sysfb_parent_dev(const struct screen_info *si)
static __init int sysfb_init(void)
{
- struct screen_info *si = &screen_info;
+ struct screen_info *si = &sysfb_primary_display.screen;
struct device *parent;
unsigned int type;
struct simplefb_platform_data mode;
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 69591dc7bad2..e7ebfa4cee45 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -29,7 +29,7 @@
#include <linux/delay.h>
#include <linux/panic_notifier.h>
#include <linux/ptrace.h>
-#include <linux/screen_info.h>
+#include <linux/sysfb.h>
#include <linux/efi.h>
#include <linux/random.h>
#include <linux/kernel.h>
@@ -2282,8 +2282,8 @@ static void __maybe_unused vmbus_reserve_fb(void)
if (efi_enabled(EFI_BOOT)) {
/* Gen2 VM: get FB base from EFI framebuffer */
if (IS_ENABLED(CONFIG_SYSFB)) {
- start = screen_info.lfb_base;
- size = max_t(__u32, screen_info.lfb_size, 0x800000);
+ start = sysfb_primary_display.screen.lfb_base;
+ size = max_t(__u32, sysfb_primary_display.screen.lfb_size, 0x800000);
}
} else {
/* Gen1 VM: get FB base from PCI */
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index 436fa7f4c387..805be9ea4a34 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -26,7 +26,7 @@
#include <linux/poll.h>
#include <linux/miscdevice.h>
#include <linux/slab.h>
-#include <linux/screen_info.h>
+#include <linux/sysfb.h>
#include <linux/vt.h>
#include <linux/console.h>
#include <linux/acpi.h>
@@ -557,7 +557,7 @@ EXPORT_SYMBOL(vga_put);
static bool vga_is_firmware_default(struct pci_dev *pdev)
{
#if defined CONFIG_X86
- return pdev == screen_info_pci_dev(&screen_info);
+ return pdev == screen_info_pci_dev(&sysfb_primary_display.screen);
#else
return false;
#endif
diff --git a/drivers/video/screen_info_pci.c b/drivers/video/screen_info_pci.c
index 66bfc1d0a6dc..8f34d8a74f09 100644
--- a/drivers/video/screen_info_pci.c
+++ b/drivers/video/screen_info_pci.c
@@ -4,6 +4,7 @@
#include <linux/printk.h>
#include <linux/screen_info.h>
#include <linux/string.h>
+#include <linux/sysfb.h>
static struct pci_dev *screen_info_lfb_pdev;
static size_t screen_info_lfb_bar;
@@ -26,7 +27,7 @@ static bool __screen_info_relocation_is_valid(const struct screen_info *si, stru
void screen_info_apply_fixups(void)
{
- struct screen_info *si = &screen_info;
+ struct screen_info *si = &sysfb_primary_display.screen;
if (screen_info_lfb_pdev) {
struct resource *pr = &screen_info_lfb_pdev->resource[screen_info_lfb_bar];
@@ -75,7 +76,7 @@ static void screen_info_fixup_lfb(struct pci_dev *pdev)
.flags = IORESOURCE_MEM,
};
const struct resource *pr;
- const struct screen_info *si = &screen_info;
+ const struct screen_info *si = &sysfb_primary_display.screen;
if (screen_info_lfb_pdev)
return; // already found
diff --git a/include/linux/screen_info.h b/include/linux/screen_info.h
index 1690706206e8..c022403c599a 100644
--- a/include/linux/screen_info.h
+++ b/include/linux/screen_info.h
@@ -151,6 +151,4 @@ static inline struct pci_dev *screen_info_pci_dev(const struct screen_info *si)
}
#endif
-extern struct screen_info screen_info;
-
#endif /* _SCREEN_INFO_H */
diff --git a/include/linux/sysfb.h b/include/linux/sysfb.h
index 8b37247528bf..e8bde392c690 100644
--- a/include/linux/sysfb.h
+++ b/include/linux/sysfb.h
@@ -8,11 +8,10 @@
*/
#include <linux/err.h>
+#include <linux/platform_data/simplefb.h>
#include <linux/screen_info.h>
#include <linux/types.h>
-#include <linux/platform_data/simplefb.h>
-
struct device;
struct platform_device;
struct screen_info;
@@ -65,6 +64,8 @@ struct sysfb_display_info {
struct screen_info screen;
};
+extern struct sysfb_display_info sysfb_primary_display;
+
#ifdef CONFIG_SYSFB
void sysfb_disable(struct device *dev);
--
2.51.1
^ permalink raw reply related
* [PATCH 1/6] efi: earlycon: Reduce number of references to global screen_info
From: Thomas Zimmermann @ 2025-11-21 13:36 UTC (permalink / raw)
To: ardb, javierm, arnd
Cc: x86, linux-arm-kernel, linux-kernel, linux-efi, loongarch,
linux-riscv, dri-devel, linux-hyperv, linux-pci, linux-fbdev,
Thomas Zimmermann
In-Reply-To: <20251121135624.494768-1-tzimmermann@suse.de>
Replace usage of global screen_info with local pointers. This will
later reduce churn when screen_info is being moved.
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
drivers/firmware/efi/earlycon.c | 40 ++++++++++++++++-----------------
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/drivers/firmware/efi/earlycon.c b/drivers/firmware/efi/earlycon.c
index d18a1a5de144..fac3a295c57f 100644
--- a/drivers/firmware/efi/earlycon.c
+++ b/drivers/firmware/efi/earlycon.c
@@ -32,12 +32,13 @@ static void *efi_fb;
*/
static int __init efi_earlycon_remap_fb(void)
{
+ const struct screen_info *si = &screen_info;
+
/* bail if there is no bootconsole or it was unregistered already */
if (!earlycon_console || !console_is_registered(earlycon_console))
return 0;
- efi_fb = memremap(fb_base, screen_info.lfb_size,
- fb_wb ? MEMREMAP_WB : MEMREMAP_WC);
+ efi_fb = memremap(fb_base, si->lfb_size, fb_wb ? MEMREMAP_WB : MEMREMAP_WC);
return efi_fb ? 0 : -ENOMEM;
}
@@ -71,12 +72,12 @@ static __ref void efi_earlycon_unmap(void *addr, unsigned long len)
early_memunmap(addr, len);
}
-static void efi_earlycon_clear_scanline(unsigned int y)
+static void efi_earlycon_clear_scanline(unsigned int y, const struct screen_info *si)
{
unsigned long *dst;
u16 len;
- len = screen_info.lfb_linelength;
+ len = si->lfb_linelength;
dst = efi_earlycon_map(y*len, len);
if (!dst)
return;
@@ -85,7 +86,7 @@ static void efi_earlycon_clear_scanline(unsigned int y)
efi_earlycon_unmap(dst, len);
}
-static void efi_earlycon_scroll_up(void)
+static void efi_earlycon_scroll_up(const struct screen_info *si)
{
unsigned long *dst, *src;
u16 maxlen = 0;
@@ -99,8 +100,8 @@ static void efi_earlycon_scroll_up(void)
}
maxlen *= 4;
- len = screen_info.lfb_linelength;
- height = screen_info.lfb_height;
+ len = si->lfb_linelength;
+ height = si->lfb_height;
for (i = 0; i < height - font->height; i++) {
dst = efi_earlycon_map(i*len, len);
@@ -120,7 +121,8 @@ static void efi_earlycon_scroll_up(void)
}
}
-static void efi_earlycon_write_char(u32 *dst, unsigned char c, unsigned int h)
+static void efi_earlycon_write_char(u32 *dst, unsigned char c, unsigned int h,
+ const struct screen_info *si)
{
const u32 color_black = 0x00000000;
const u32 color_white = 0x00ffffff;
@@ -145,13 +147,12 @@ static void efi_earlycon_write_char(u32 *dst, unsigned char c, unsigned int h)
static void
efi_earlycon_write(struct console *con, const char *str, unsigned int num)
{
- struct screen_info *si;
+ const struct screen_info *si = &screen_info;
u32 cur_efi_x = efi_x;
unsigned int len;
const char *s;
void *dst;
- si = &screen_info;
len = si->lfb_linelength;
while (num) {
@@ -174,7 +175,7 @@ efi_earlycon_write(struct console *con, const char *str, unsigned int num)
x = efi_x;
while (n-- > 0) {
- efi_earlycon_write_char(dst + x*4, *s, h);
+ efi_earlycon_write_char(dst + x*4, *s, h, si);
x += font->width;
s++;
}
@@ -207,10 +208,10 @@ efi_earlycon_write(struct console *con, const char *str, unsigned int num)
cur_line_y = (cur_line_y + 1) % max_line_y;
efi_y -= font->height;
- efi_earlycon_scroll_up();
+ efi_earlycon_scroll_up(si);
for (i = 0; i < font->height; i++)
- efi_earlycon_clear_scanline(efi_y + i);
+ efi_earlycon_clear_scanline(efi_y + i, si);
}
}
}
@@ -226,22 +227,21 @@ void __init efi_earlycon_reprobe(void)
static int __init efi_earlycon_setup(struct earlycon_device *device,
const char *opt)
{
- struct screen_info *si;
+ const struct screen_info *si = &screen_info;
u16 xres, yres;
u32 i;
fb_wb = opt && !strcmp(opt, "ram");
- if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI) {
+ if (si->orig_video_isVGA != VIDEO_TYPE_EFI) {
fb_probed = true;
return -ENODEV;
}
- fb_base = screen_info.lfb_base;
- if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
- fb_base |= (u64)screen_info.ext_lfb_base << 32;
+ fb_base = si->lfb_base;
+ if (si->capabilities & VIDEO_CAPABILITY_64BIT_BASE)
+ fb_base |= (u64)si->ext_lfb_base << 32;
- si = &screen_info;
xres = si->lfb_width;
yres = si->lfb_height;
@@ -266,7 +266,7 @@ static int __init efi_earlycon_setup(struct earlycon_device *device,
efi_y -= font->height;
for (i = 0; i < (yres - efi_y) / font->height; i++)
- efi_earlycon_scroll_up();
+ efi_earlycon_scroll_up(si);
device->con->write = efi_earlycon_write;
earlycon_console = device->con;
--
2.51.1
^ permalink raw reply related
* [PATCH 0/6] arch,sysfb: Move screen and edid info into single place
From: Thomas Zimmermann @ 2025-11-21 13:36 UTC (permalink / raw)
To: ardb, javierm, arnd
Cc: x86, linux-arm-kernel, linux-kernel, linux-efi, loongarch,
linux-riscv, dri-devel, linux-hyperv, linux-pci, linux-fbdev,
Thomas Zimmermann
Replace screen_info and edid_info with sysfb_primary_device of type
struct sysfb_display_info. Update all users.
Sysfb DRM drivers currently fetch the global edid_info directly, when
they should get that information together with the screen_info from their
device. Wrapping screen_info and edid_info in sysfb_primary_display and
passing this to drivers enables this.
Replacing both with sysfb_primary_display has been motivate by the EFI
stub. EFI wants to transfer EDID via config table in a single entry.
Using struct sysfb_display_info this will become easily possible. Hence
accept some churn in architecture code for the long-term improvements.
Patches 1 and 2 reduce the exposure of screen_info in EFI-related code.
Patch 3 adds struct sysfb_display_info.
Patch 4 replaces scren_info with sysfb_primary_display. This results in
several changes throught the kernel, but is really just a refactoring.
Patch 5 updates sysfb to transfer sysfb_primary_display to the related
drivers.
Patch 6 moves edid_info into sysfb_primary_display. This resolves some
drivers' reference to the global edid_info, but also makes the EDID data
available on non-x86 architectures.
The short-term benefit of this series is in patches 5 and 6. With
sysfb_primary_display in place a follow-up series will improve EFI support
for EDID as outlined in the series at [1] and [2].
[1] https://lore.kernel.org/dri-devel/20251015160816.525825-1-tzimmermann@suse.de/
[2] https://lore.kernel.org/linux-efi/20251119123011.1187249-5-ardb+git@google.com/
Thomas Zimmermann (6):
efi: earlycon: Reduce number of references to global screen_info
efi: sysfb_efi: Reduce number of references to global screen_info
sysfb: Add struct sysfb_display_info
sysfb: Replace screen_info with sysfb_primary_display
sysfb: Pass sysfb_primary_display to devices
sysfb: Move edid_info into sysfb_primary_display
arch/arm64/kernel/image-vars.h | 2 +-
arch/loongarch/kernel/efi.c | 11 +--
arch/loongarch/kernel/image-vars.h | 2 +-
arch/riscv/kernel/image-vars.h | 2 +-
arch/x86/kernel/kexec-bzimage64.c | 4 +-
arch/x86/kernel/setup.c | 16 ++--
arch/x86/video/video-common.c | 4 +-
drivers/firmware/efi/earlycon.c | 42 +++++-----
drivers/firmware/efi/efi-init.c | 14 ++--
drivers/firmware/efi/libstub/efi-stub-entry.c | 18 +++--
drivers/firmware/efi/sysfb_efi.c | 81 ++++++++++---------
drivers/firmware/sysfb.c | 13 +--
drivers/firmware/sysfb_simplefb.c | 2 +-
drivers/gpu/drm/sysfb/efidrm.c | 14 ++--
drivers/gpu/drm/sysfb/vesadrm.c | 14 ++--
drivers/hv/vmbus_drv.c | 6 +-
drivers/pci/vgaarb.c | 4 +-
drivers/video/Kconfig | 1 -
drivers/video/fbdev/core/fbmon.c | 8 +-
drivers/video/fbdev/efifb.c | 10 ++-
drivers/video/fbdev/vesafb.c | 10 ++-
drivers/video/fbdev/vga16fb.c | 8 +-
drivers/video/screen_info_pci.c | 5 +-
include/linux/screen_info.h | 2 -
include/linux/sysfb.h | 23 ++++--
include/video/edid.h | 4 -
26 files changed, 177 insertions(+), 143 deletions(-)
base-commit: 57e807d4454add8b60e8807ad1cf812141f34cdb
--
2.51.1
^ permalink raw reply
* Re: [PATCH 6.6 and older] uio_hv_generic: Enable user space to manage interrupt_mask for subchannels
From: Salvatore Bonaccorso @ 2025-11-21 12:43 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Naman Jain, Wei Liu, Dexuan Cui, Michael Kelley, Long Li,
Saurabh Sengar, K . Y . Srinivasan, Haiyang Zhang, Tianyu Lan,
linux-hyperv, linux-kernel, stable, Peter Morrow
In-Reply-To: <2025112109-legroom-resend-643f@gregkh>
Hi,
On Fri, Nov 21, 2025 at 11:10:43AM +0100, Greg Kroah-Hartman wrote:
> On Sat, Nov 15, 2025 at 02:29:37PM +0530, Naman Jain wrote:
> > From: Long Li <longli@microsoft.com>
> >
> > Enable the user space to manage interrupt_mask for subchannels through
> > irqcontrol interface for uio device. Also remove the memory barrier
> > when monitor bit is enabled as it is not necessary.
> >
> > This is a backport of the upstream commit
> > d062463edf17 ("uio_hv_generic: Set event for all channels on the device")
> > with some modifications to resolve merge conflicts and take care of
> > missing support for slow devices on older kernels.
> > Original change was not a fix, but it needs to be backported to fix a
> > NULL pointer crash resulting from missing interrupt mask setting.
> >
> > Commit 37bd91f22794 ("uio_hv_generic: Let userspace take care of interrupt mask")
> > removed the default setting of interrupt_mask for channels (including
> > subchannels) in the uio_hv_generic driver, as it relies on the user space
> > to take care of managing it. This approach works fine when user space
> > can control this setting using the irqcontrol interface provided for uio
> > devices. Support for setting the interrupt mask through this interface for
> > subchannels came only after commit d062463edf17 ("uio_hv_generic: Set event
> > for all channels on the device"). On older kernels, this change is not
> > present. With uio_hv_generic no longer setting the interrupt_mask, and
> > userspace not having the capability to set it, it remains unset,
> > and interrupts can come for the subchannels, which can result in a crash
> > in hv_uio_channel_cb. Backport the change to older kernels, where this
> > change was not present, to allow userspace to set the interrupt mask
> > properly for subchannels. Additionally, this patch also adds certain
> > checks for primary vs subchannels in the hv_uio_channel_cb, which can
> > gracefully handle these two cases and prevent the NULL pointer crashes.
> >
> > Signed-off-by: Long Li <longli@microsoft.com>
> > Fixes: 37bd91f22794 ("uio_hv_generic: Let userspace take care of interrupt mask")
>
> This is a 6.12.y commit id, so a fix for 6.6.y does not make sense :(
Should maybe be updated to reflect the original upstream commit. In
fact b15b7d2a1b09 ("uio_hv_generic: Let userspace take care of
interrupt mask") was backported to various stable series:
v5.4.301: 540aac117eaea5723cef5e4cbf3035c4ac654d92 uio_hv_generic: Let userspace take care of interrupt mask
v5.10.246: 65d40acd911c7011745cbbd2aaac34eb5266d11e uio_hv_generic: Let userspace take care of interrupt mask
v5.15.195: a44f61f878f32071d6378e8dd7c2d47f9490c8f7 uio_hv_generic: Let userspace take care of interrupt mask
v6.1.156: 01ce972e6f9974a7c76943bcb7e93746917db83a uio_hv_generic: Let userspace take care of interrupt mask
v6.6.112: 2af39ab5e6dc46b835a52e80a22d0cad430985e3 uio_hv_generic: Let userspace take care of interrupt mask
v6.12.53: 37bd91f22794dc05436130d6983302cb90ecfe7e uio_hv_generic: Let userspace take care of interrupt mask
v6.17.3: e29587c07537929684faa365027f4b0d87521e1b uio_hv_generic: Let userspace take care of interrupt mask
And Peter just confirmed in
https://lore.kernel.org/stable/CAFcZKTyOcDqDJRB4sgN7Q-dabBU0eg7KKs=yBJhB=CNDyy7scQ@mail.gmail.com/
that he is seeing the problem now as well after updating from
6.1.153-1 to 6.1.158-1 in Debian.
> > Closes: https://bugs.debian.org/1120602
> > Cc: <stable@vger.kernel.org> # 6.6.x and older
>
> How "old" do you want this? Can you fix the Fixes: line up and resend
> with this info?
It is at least relevant for back in 6.1.y now, but I'm not sure about
the older series. I will let Naman speak up.
I guess the proper fixes tracking is a bit "tricky" because it only
affected some of the stable series, namely those which had a backport
of b15b7d2a1b09 ("uio_hv_generic: Let userspace take care of interrupt
mask") done before the including a backport of d062463edf17
("uio_hv_generic: Set event for all channels on the device"). So this
is the reason why we seeing it first in 6.12.y stable series (but now
as well on olders), but not a problem on 6.17.y.
Hope this explanation helps, please keep in mind that I'm no expert
here by no means, just helping to report it from downstream Debian up
here.
Regards,
Salvatore
^ permalink raw reply
* Re: [PATCH 6.6 and older] uio_hv_generic: Enable user space to manage interrupt_mask for subchannels
From: Greg Kroah-Hartman @ 2025-11-21 10:10 UTC (permalink / raw)
To: Naman Jain
Cc: Wei Liu, Dexuan Cui, Michael Kelley, Long Li, Saurabh Sengar,
K . Y . Srinivasan, Haiyang Zhang, Tianyu Lan, linux-hyperv,
linux-kernel, stable, Salvatore Bonaccorso, Peter Morrow
In-Reply-To: <20251115085937.2237-1-namjain@linux.microsoft.com>
On Sat, Nov 15, 2025 at 02:29:37PM +0530, Naman Jain wrote:
> From: Long Li <longli@microsoft.com>
>
> Enable the user space to manage interrupt_mask for subchannels through
> irqcontrol interface for uio device. Also remove the memory barrier
> when monitor bit is enabled as it is not necessary.
>
> This is a backport of the upstream commit
> d062463edf17 ("uio_hv_generic: Set event for all channels on the device")
> with some modifications to resolve merge conflicts and take care of
> missing support for slow devices on older kernels.
> Original change was not a fix, but it needs to be backported to fix a
> NULL pointer crash resulting from missing interrupt mask setting.
>
> Commit 37bd91f22794 ("uio_hv_generic: Let userspace take care of interrupt mask")
> removed the default setting of interrupt_mask for channels (including
> subchannels) in the uio_hv_generic driver, as it relies on the user space
> to take care of managing it. This approach works fine when user space
> can control this setting using the irqcontrol interface provided for uio
> devices. Support for setting the interrupt mask through this interface for
> subchannels came only after commit d062463edf17 ("uio_hv_generic: Set event
> for all channels on the device"). On older kernels, this change is not
> present. With uio_hv_generic no longer setting the interrupt_mask, and
> userspace not having the capability to set it, it remains unset,
> and interrupts can come for the subchannels, which can result in a crash
> in hv_uio_channel_cb. Backport the change to older kernels, where this
> change was not present, to allow userspace to set the interrupt mask
> properly for subchannels. Additionally, this patch also adds certain
> checks for primary vs subchannels in the hv_uio_channel_cb, which can
> gracefully handle these two cases and prevent the NULL pointer crashes.
>
> Signed-off-by: Long Li <longli@microsoft.com>
> Fixes: 37bd91f22794 ("uio_hv_generic: Let userspace take care of interrupt mask")
This is a 6.12.y commit id, so a fix for 6.6.y does not make sense :(
> Closes: https://bugs.debian.org/1120602
> Cc: <stable@vger.kernel.org> # 6.6.x and older
How "old" do you want this? Can you fix the Fixes: line up and resend
with this info?
thanks,
greg k-h
^ permalink raw reply
* Re: Bug#1120602: [REGRESSION 6.12.y] hyper-v: BUG: kernel NULL pointer dereference, address: 00000000000000a0: RIP: 0010:hv_uio_channel_cb+0xd/0x20 [uio_hv_generic]
From: Peter Morrow @ 2025-11-21 10:04 UTC (permalink / raw)
To: Naman Jain
Cc: Salvatore Bonaccorso, 1120602, Long Li, linux-hyperv,
linux-kernel, regressions, stable, John Starks, Michael Kelley,
Tianyu Lan, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Greg Kroah-Hartman
In-Reply-To: <25aff5ca-b5e1-4907-bd12-6571f8454146@linux.microsoft.com>
Hi Naman/Salvatore,
Is it possible to get this fixed in the 6.1 LTS series too? I just ran
into this crash when moving from bookworm based Debian kernel
6.1.153-1 to 6.1.158-1. I saw that "uio_hv_generic: Let userspace take
care of interrupt mask" appeared in 6.1.156.
Thanks,
Peter.
On Sat, 15 Nov 2025 at 09:04, Naman Jain <namjain@linux.microsoft.com> wrote:
>
>
>
> On 11/15/2025 3:14 AM, Salvatore Bonaccorso wrote:
> > Hi,
> >
> > On Fri, Nov 14, 2025 at 08:05:55PM +0530, Naman Jain wrote:
> >>
> >>
> >> On 11/14/2025 5:19 PM, Peter Morrow wrote:
> >>> Hi Naman,
> >>>
> >>> On Fri, 14 Nov 2025 at 06:03, Naman Jain <namjain@linux.microsoft.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 11/13/2025 11:59 PM, Salvatore Bonaccorso wrote:
> >>>>> Peter Morrow reported in Debian a regression, reported in
> >>>>> https://bugs.debian.org/1120602 . The regression was seen after
> >>>>> updating, to 6.12.57-1 in Debian, but details on the offending commit
> >>>>> follows.
> >>>>>
> >>>>> His report was as follows:
> >>>>>
> >>>>>> Dear Maintainer,
> >>>>>>
> >>>>>> I'm seeing a kernel crash quite soon after boot on a debian trixie based
> >>>>>> system running 6.12.57+deb13-amd64, unfortunately the kernel panics before
> >>>>>> I can access the system to gather more information. Thus I'll provide details
> >>>>>> of the system using a previously known good version. The panic is happening
> >>>>>> 100% of the time unfortunately. I have access to the serial console however
> >>>>>> so can enable any required verbose logging during boot if necessary.
> >>>>>>
> >>>>>> Crucially the crash is not seen with kernel version 6.12.41+deb13-amd64 with the
> >>>>>> same userspace. We had pinned to that version until very recently to in order
> >>>>>> to work around https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1109676
> >>>>>>
> >>>>>> I'm running a dpdk application here (VPP) on Azure, VM form factor is a
> >>>>>> "Standard DS3 v2 (4 vcpus, 14 GiB memory)".
> >>>>>>
> >>>>>> The only relevant upstream commit in this area (as far as I can see) is:
> >>>>>>
> >>>>>> https://lore.kernel.org/linux-hyperv/1bb599ee-fe28-409d-b430-2fc086268936@linux.microsoft.com/
> >>>>>>
> >>>>>> The comment regarding avoiding races at start adds a bit more weight behind this
> >>>>>> hunch, though it's only a hunch as I am most definitely nowhere near an expert
> >>>>>> in this area.
> >>>>>>
> >>>>>> -- Package-specific info:
> >>>>>>
> >>>>>> [ 19.625535] BUG: kernel NULL pointer dereference, address: 00000000000000a0
> >>>>>> [ 19.628874] #PF: supervisor read access in kernel mode
> >>>>>> [ 19.630841] #PF: error_code(0x0000) - not-present page
> >>>>>> [ 19.632788] PGD 0 P4D 0
> >>>>>> [ 19.633905] Oops: Oops: 0000 [#1] PREEMPT SMP PTI
> >>>>>> [ 19.635586] CPU: 3 UID: 0 PID: 0 Comm: swapper/3 Not tainted 6.12.57+deb13-amd64 #1 Debian 6.12.57-1
> >>>>>> [ 19.640216] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v4.1 09/28/2024
> >>>>>> [ 19.644514] RIP: 0010:hv_uio_channel_cb+0xd/0x20 [uio_hv_generic]
> >>>>>> [ 19.646994] Code: 02 00 00 5b 5d e9 53 98 69 e9 0f 1f 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 48 8b 47 10 <48> 8b b8 a0 00 00 00 f0 83 44 24 fc 00 e9 51 6f fa ff 90 90 90 90
> >>>>>> [ 19.654377] RSP: 0018:ffffb15ac01a4fa8 EFLAGS: 00010046
> >>>>>> [ 19.656385] RAX: 0000000000000000 RBX: 0000000000000015 RCX: 0000000000000015
> >>>>>> [ 19.659240] RDX: 0000000000000001 RSI: ffffffffffffffff RDI: ffff8ff69c759400
> >>>>>> [ 19.662168] RBP: ffff8ff548790200 R08: ffff8ff548790200 R09: 00fca75150b080e9
> >>>>>> [ 19.665239] R10: 0000000000000000 R11: ffffb15ac01a4ff8 R12: ffff8ff871dc1480
> >>>>>> [ 19.668193] R13: ffff8ff69c759400 R14: ffff8ff69c7596a0 R15: ffffffffc106e160
> >>>>>> [ 19.671106] FS: 0000000000000000(0000) GS:ffff8ff871d80000(0000) knlGS:0000000000000000
> >>>>>> [ 19.674281] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >>>>>> [ 19.676533] CR2: 00000000000000a0 CR3: 0000000100ba6003 CR4: 00000000003706f0
> >>>>>> [ 19.679385] Call Trace:
> >>>>>> [ 19.680361] <IRQ>
> >>>>>> [ 19.681181] vmbus_isr+0x1a5/0x210 [hv_vmbus]
> >>>>>> [ 19.682916] __sysvec_hyperv_callback+0x32/0x60
> >>>>>> [ 19.684991] sysvec_hyperv_callback+0x6c/0x90
> >>>>>> [ 19.686665] </IRQ>
> >>>>>> [ 19.687509] <TASK>
> >>>>>> [ 19.688366] asm_sysvec_hyperv_callback+0x1a/0x20
> >>>>>> [ 19.690262] RIP: 0010:pv_native_safe_halt+0xf/0x20
> >>>>>> [ 19.692067] Code: 09 e9 c5 08 01 00 0f 1f 44 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 66 90 0f 00 2d e5 3b 31 00 fb f4 <c3> cc cc cc cc 66 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90
> >>>>>> [ 19.699119] RSP: 0018:ffffb15ac0103ed8 EFLAGS: 00000246
> >>>>>> [ 19.701412] RAX: 0000000000000003 RBX: ffff8ff5403b1fc0 RCX: ffff8ff54c64ce30
> >>>>>> [ 19.704328] RDX: 0000000000000000 RSI: 0000000000000003 RDI: 000000000001f894
> >>>>>> [ 19.706910] RBP: 0000000000000003 R08: 000000000bb760d9 R09: 00fca75150b080e9
> >>>>>> [ 19.709762] R10: 0000000000000003 R11: 0000000000000001 R12: 0000000000000000
> >>>>>> [ 19.712510] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> >>>>>> [ 19.715173] default_idle+0x9/0x20
> >>>>>> [ 19.716846] default_idle_call+0x29/0x100
> >>>>>> [ 19.718623] do_idle+0x1fe/0x240
> >>>>>> [ 19.720045] cpu_startup_entry+0x29/0x30
> >>>>>> [ 19.721595] start_secondary+0x11e/0x140
> >>>>>> [ 19.723080] common_startup_64+0x13e/0x141
> >>>>>> [ 19.725222] </TASK>
> >>>>>> [ 19.726387] Modules linked in: isofs cdrom uio_hv_generic uio binfmt_misc intel_rapl_msr intel_rapl_common intel_uncore_frequency_common isst_if_mbox_msr isst_if_common rpcrdma skx_edac_common nfit sunrpc libnvdimm crct10dif_pclmul ghash_clmulni_intel sha512_ssse3 sha256_ssse3 rdma_ucm ib_iser sha1_ssse3 rdma_cm aesni_intel iw_cm gf128mul crypto_simd libiscsi cryptd ib_umad ib_ipoib scsi_transport_iscsi ib_cm rapl sg hv_utils hv_balloon evdev pcspkr joydev mpls_router ip_tunnel ramoops configfs pstore_blk efi_pstore pstore_zone nfnetlink vsock_loopback vmw_vsock_virtio_transport_common hv_sock vmw_vsock_vmci_transport vsock vmw_vmci efivarfs ip_tables x_tables autofs4 overlay squashfs dm_verity dm_bufio reed_solomon dm_mod loop ext4 crc16 mbcache jbd2 crc32c_generic mlx5_ib ib_uverbs ib_core mlx5_core mlxfw pci_hyperv pci_hyperv_intf hyperv_drm drm_shmem_helper sd_mod drm_kms_helper hv_storvsc scsi_transport_fc drm scsi_mod hid_generic hid_hyperv hid serio_raw hv_netvsc hyperv_keyboard scsi_common hv_vmbus
> >>>>>> [ 19.726466] crc32_pclmul crc32c_intel
> >>>>>> [ 19.765771] CR2: 00000000000000a0
> >>>>>> [ 19.767524] ---[ end trace 0000000000000000 ]---
> >>>>>> [ 19.800433] RIP: 0010:hv_uio_channel_cb+0xd/0x20 [uio_hv_generic]
> >>>>>> [ 19.803170] Code: 02 00 00 5b 5d e9 53 98 69 e9 0f 1f 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 48 8b 47 10 <48> 8b b8 a0 00 00 00 f0 83 44 24 fc 00 e9 51 6f fa ff 90 90 90 90
> >>>>>> [ 19.811041] RSP: 0018:ffffb15ac01a4fa8 EFLAGS: 00010046
> >>>>>> [ 19.813466] RAX: 0000000000000000 RBX: 0000000000000015 RCX: 0000000000000015
> >>>>>> [ 19.816504] RDX: 0000000000000001 RSI: ffffffffffffffff RDI: ffff8ff69c759400
> >>>>>> [ 19.819484] RBP: ffff8ff548790200 R08: ffff8ff548790200 R09: 00fca75150b080e9
> >>>>>> [ 19.822625] R10: 0000000000000000 R11: ffffb15ac01a4ff8 R12: ffff8ff871dc1480
> >>>>>> [ 19.825569] R13: ffff8ff69c759400 R14: ffff8ff69c7596a0 R15: ffffffffc106e160
> >>>>>> [ 19.828804] FS: 0000000000000000(0000) GS:ffff8ff871d80000(0000) knlGS:0000000000000000
> >>>>>> [ 19.832214] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >>>>>> [ 19.834709] CR2: 00000000000000a0 CR3: 0000000100ba6003 CR4: 00000000003706f0
> >>>>>> [ 19.837976] Kernel panic - not syncing: Fatal exception in interrupt
> >>>>>> [ 19.841825] Kernel Offset: 0x28a00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
> >>>>>> [ 19.896620] ---[ end Kernel panic - not syncing: Fatal exception in interrupt ]---
> >>>>>>
> >>>>
> >>>> <snip>
> >>>>
> >>>>> The offending commit appers to be the backport of b15b7d2a1b09
> >>>>> ("uio_hv_generic: Let userspace take care of interrupt mask") for
> >>>>> 6.12.y.
> >>>>>
> >>>>> Peter confirmed that reverting this commit on top of 6.12.57-1 as
> >>>>> packaged in Debian resolves indeed the issue. Interestingly the issue
> >>>>> is *not* seen with 6.17.7 based kernel in Debian.
> >>>>>
> >>>>> #regzbot introduced: 37bd91f22794dc05436130d6983302cb90ecfe7e
> >>>>> #regzbot monitor: https://bugs.debian.org/1120602
> >>>>>
> >>>>> Thank you already!
> >>>>>
> >>>>> Regards,
> >>>>> Salvatore
> >>>>
> >>>> Hi Peter, Salvatore,
> >>>> Thanks for reporting this crash, and sorry for the trouble. Here is my
> >>>> analysis.
> >>>>
> >>>> On 6.17.7, where commit d062463edf17 ("uio_hv_generic: Set event for all
> >>>> channels on the device") is present, hv_uio_irqcontrol() supports
> >>>> setting of interrupt mask from userspace for sub-channels as well.
> >>>>
> >>>> This aligns with commit e29587c07537 ("uio_hv_generic: Let userspace
> >>>> take care of interrupt mask") which relies on userspace to manage
> >>>> interrupt mask, so it safely removes the interrupt mask management logic
> >>>> in the driver.
> >>>>
> >>>> However, in 6.12.57, the first commit is not present, but the second one
> >>>> is, so there is no way to disable interrupt mask for sub-channels and
> >>>> interrupt_mask stays 0, which means interrupts are not masked. So we may
> >>>> be having an interrupt callback being handled for a sub-channel, where
> >>>> we do not expect it to come. This may be causing this issue.
> >>>>
> >>>> This would have led to a crash in hv_uio_channel_cb() for sub-channels:
> >>>> struct hv_device *hv_dev = chan->device_obj;
> >>>>
> >>>>
> >>>> I have ported commit d062463edf17 ("uio_hv_generic: Set event for all
> >>>> channels on the device") on 6.12.57, and resolved some merge conflicts.
> >>>> Could you please help with testing this, if it works for you.
> >>>
> >>> Applying the patch against the debian 6.12.57 kernel worked, I am no
> >>> longer seeing that panic on boot:
> >>>
> >>> gnos@vEdge:~$ uname -a
> >>> Linux vEdge 6.12+unreleased-amd64 #1 SMP PREEMPT_DYNAMIC Debian
> >>> 6.12.57-1a~test (2025-11-14) x86_64 GNU/Linux
> >>> gnos@vEdge:~$ uptime
> >>> 11:46:33 up 4 min, 1 user, load average: 3.31, 2.07, 0.89
> >>> gnos@vEdge:~$ sudo dmidecode -t system
> >>> # dmidecode 3.6
> >>> Getting SMBIOS data from sysfs.
> >>> SMBIOS 3.1.0 present.
> >>>
> >>> Handle 0x0001, DMI type 1, 27 bytes
> >>> System Information
> >>> Manufacturer: Microsoft Corporation
> >>> Product Name: Virtual Machine
> >>> Version: Hyper-V UEFI Release v4.1
> >>> Serial Number: 0000-0002-8036-1108-7588-3134-50
> >>> UUID: 26e86d6e-140c-496a-862c-a3b3bbcd16ad
> >>> Wake-up Type: Power Switch
> >>> SKU Number: None
> >>> Family: Virtual Machine
> >>>
> >>> Handle 0x0010, DMI type 32, 11 bytes
> >>> System Boot Information
> >>> Status: No errors detected
> >>>
> >>> gnos@vEdge:~$
> >>>
> >>> Thanks a lot for the quick analysis!
> >>>
> >>> Peter.
> >>
> >> Hi Peter,
> >>
> >> Thanks for confirming. I am discussing this with Long Li, to hear his
> >> thoughts on this, and have kept the patch ready.
> >> Porting the same on 6.6 and older kernels would be a little different since
> >> we don't have commit 547fa4ffd799 ("uio_hv_generic: Enable interrupt for low
> >> speed VMBus devices") on these kernels and this would lead to merge
> >> conflicts, which needs to be handled separately.
> >>
> >> Meanwhile, if I should be including any tags in the fix patch for debian
> >> bug, please let me know.
> >
> > Thank you very much for the quick analysis and fix.
> >
> > If you can add a Closes: https://bugs.debian.org/1120602 that would
> > make our tracking for the fixes easier. But not sure if this is
> > allowed for proposing the backport for a stable series, as it did not
> > affect the upper releases.
> >
> > In any case your work is much appreciated!
> >
> > Regards,
> > Salvatore
>
> Hi,
> I have sent the patches now to the list. Please consider adding your
> tested-by if you find it alright.
>
> Thanks.
>
> Regards,
> Naman
^ permalink raw reply
* [PATCH net-next] net: hyperv: convert to use .get_rx_ring_count
From: Breno Leitao @ 2025-11-21 9:59 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: linux-hyperv, netdev, linux-kernel, kernel-team, Breno Leitao
Convert the hyperv netvsc driver to use the new .get_rx_ring_count
ethtool operation instead of implementing .get_rxnfc solely for handling
ETHTOOL_GRXRINGS command. This simplifies the code by replacing the
switch statement with a direct return of the queue count.
The new callback provides the same functionality in a more direct way,
following the ongoing ethtool API modernization.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
Note: This was compile-tested only.
---
drivers/net/hyperv/netvsc_drv.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 39c892e46cb0..3d47d749ef9f 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -1624,22 +1624,15 @@ netvsc_get_rxfh_fields(struct net_device *ndev,
return 0;
}
-static int
-netvsc_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *info,
- u32 *rules)
+static u32 netvsc_get_rx_ring_count(struct net_device *dev)
{
struct net_device_context *ndc = netdev_priv(dev);
struct netvsc_device *nvdev = rtnl_dereference(ndc->nvdev);
if (!nvdev)
- return -ENODEV;
-
- switch (info->cmd) {
- case ETHTOOL_GRXRINGS:
- info->data = nvdev->num_chn;
return 0;
- }
- return -EOPNOTSUPP;
+
+ return nvdev->num_chn;
}
static int
@@ -1969,7 +1962,7 @@ static const struct ethtool_ops ethtool_ops = {
.get_channels = netvsc_get_channels,
.set_channels = netvsc_set_channels,
.get_ts_info = ethtool_op_get_ts_info,
- .get_rxnfc = netvsc_get_rxnfc,
+ .get_rx_ring_count = netvsc_get_rx_ring_count,
.get_rxfh_key_size = netvsc_get_rxfh_key_size,
.get_rxfh_indir_size = netvsc_rss_indir_size,
.get_rxfh = netvsc_get_rxfh,
---
base-commit: e2c20036a8879476c88002730d8a27f4e3c32d4b
change-id: 20251118-hyperv_gxrings-1285e8fa7407
Best regards,
--
Breno Leitao <leitao@debian.org>
^ permalink raw reply related
* Re: [PATCH 1/3] x86: Use MOVL when reading segment registers
From: Uros Bizjak @ 2025-11-21 8:09 UTC (permalink / raw)
To: Michael Kelley
Cc: linux-hyperv@vger.kernel.org, x86@kernel.org,
linux-kernel@vger.kernel.org, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin
In-Reply-To: <SN6PR02MB41571E23242892F03B528C56D4D5A@SN6PR02MB4157.namprd02.prod.outlook.com>
On Fri, Nov 21, 2025 at 2:03 AM Michael Kelley <mhklinux@outlook.com> wrote:
>
> From: Uros Bizjak <ubizjak@gmail.com>
> >
> > Use MOVL when reading segment registers to avoid 0x66 operand-size
> > override insn prefix. The segment value is always 16-bit and gets
> > zero-extended to the full 32-bit size.
> >
> > Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: Dave Hansen <dave.hansen@linux.intel.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > ---
> > arch/x86/include/asm/segment.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
> > index f59ae7186940..9f5be2bbd291 100644
> > --- a/arch/x86/include/asm/segment.h
> > +++ b/arch/x86/include/asm/segment.h
> > @@ -348,7 +348,7 @@ static inline void __loadsegment_fs(unsigned short value)
> > * Save a segment register away:
> > */
> > #define savesegment(seg, value) \
> > - asm("mov %%" #seg ",%0":"=r" (value) : : "memory")
> > + asm("movl %%" #seg ",%k0" : "=r" (value) : : "memory")
> >
> > #endif /* !__ASSEMBLER__ */
> > #endif /* __KERNEL__ */
> > --
> > 2.51.1
> >
>
> I've built a linux-next20251119 kernel plus the three patches in this
> series, and tested it in an SEV-SNP VM in the Azure public cloud. The VM
> is a Standard DC16ads v5 (16 vcpus, 64 GiB memory) running Ubuntu
> 20.04. It boots and does basic smoke tests with no issues. So, for all
> three patches,
>
> Tested-by: Michael Kelley <mhklinux@outlook.com>
>
> I do have a comment on the commit message for Patch 3 (I would have
> replied there, but for unknown reasons the third patch didn't show up
> in my LKML feed). The commit message says "VMMCALL ... may be inserted
> before the frame pointer". This text was somewhat ambiguous to me.
> I initially read it as "the compiler might insert VMCALL before the
> frame pointer is set up". But I think you meant "it's OK to allow the
> compiler to insert the VMMCALL before the frame point is set up".
> Maybe the intended meaning is obvious to experts in this area,
> but I'm new enough that it was confusing to me. ;-)
>
> To avoid any future confusion, I'd suggest this wording, which is explicit
> about why this patch is appropriate:
>
> Unlike the CALL instruction, VMMCALL does not push to the stack, so
> it's OK to allow the compiler to insert it before the frame pointer gets
> set up by the containing function. ASM_CALL_CONSTRAINT is for CALLs
> that must be after the frame pointer is set up, so it is over-constraining
> here and can be removed.
>
> FWIW, removing the ASM_CALL_CONSTRAINT does not change
> the generated code for hv_snp_hypercall().
Michael,
thanks for your testing and rewording suggestion! After reading it
again a couple of times, I agree that the original text is a bit too
terse, and adding your words indeed clear it up considerably! I have
changed the patch description as you proposed in v2.
BR,
Uros.
^ permalink raw reply
* RE: [PATCH v6 5/5] Drivers: hv: Add support for movable memory regions
From: Michael Kelley @ 2025-11-21 5:45 UTC (permalink / raw)
To: Stanislav Kinsburskii
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <aR5kjh-d6UAqy88t@skinsburskii.localdomain>
From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Wednesday, November 19, 2025 4:45 PM
>
> On Tue, Nov 18, 2025 at 04:29:56PM +0000, Michael Kelley wrote:
> > From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Monday, November 17, 2025 8:53 AM
[snip]
> > > diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> > > index 0b8c391a0342..5f1637cbb6e3 100644
> > > --- a/drivers/hv/Kconfig
> > > +++ b/drivers/hv/Kconfig
> > > @@ -75,6 +75,7 @@ config MSHV_ROOT
> > > depends on PAGE_SIZE_4KB
> > > select EVENTFD
> > > select VIRT_XFER_TO_GUEST_WORK
> > > + select HMM_MIRROR
> >
> > Couldn't you also do "select MMU_NOTIFIER" to avoid the #ifdef's
> > and stubs for when it isn't selected? There are other Linux kernel
> > drivers that select it. Or is the intent to allow building an image that
> > doesn't support unpinned memory, and the #ifdef's save space?
> >
>
> That's an interesting question. This driver can function without MMU notifiers
> by pinning all memory, which might be advantageous for certain real-time
> applications.
> However, since most other virtualization solutions use MMU_NOTIFIER, there
> doesn't appear to be a strong reason for this driver to deviate.
I'm not clear on your last sentence. Could you elaborate?
>
> > > default n
> > > help
> > > Select this option to enable support for booting and running as root
[snip]
> > > +
> > > + for (i = 0; i < page_count; i++)
> > > + region->pages[page_offset + i] = hmm_pfn_to_page(pfns[i]);
> >
> > The comment with hmm_pfn_to_page() says that the caller is assumed to
> > have tested the pfn for HMM_PFN_VALID, which I don't see done anywhere.
> > Is there a reason it's not necessary to test?
>
> The reason is the HMM_PFN_REQ_FAULT range flag, which requires all PFNs to be
> faulted and populated, or mshv_region_hmm_fault_and_lock will return -EFAULT.
> Additionally, note that mshv_region_hmm_fault_and_lock returns with
> region->mutex held upon success, ensuring that no page can be moved until the
> lock is released.
OK, that makes sense.
>
> > > +
> > > + if (PageHuge(region->pages[page_offset]))
> > > + region->flags.large_pages = true;
> >
> > See comment below in mshv_region_handle_gfn_fault().
> >
> > > +
> > > + ret = mshv_region_remap_pages(region, region->hv_map_flags,
> > > + page_offset, page_count);
> > > +
> > > + mutex_unlock(®ion->mutex);
> > > +out:
> > > + kfree(pfns);
> > > + return ret;
> > > +}
> > > +#else /* CONFIG_MMU_NOTIFIER */
> > > +static int mshv_region_range_fault(struct mshv_mem_region *region,
> > > + u64 page_offset, u64 page_count)
> > > +{
> > > + return -ENODEV;
> > > +}
> > > +#endif /* CONFIG_MMU_NOTIFIER */
> > > +
> > > +static bool mshv_region_handle_gfn_fault(struct mshv_mem_region *region, u64 gfn)
> > > +{
> > > + u64 page_offset, page_count;
> > > + int ret;
> > > +
> > > + if (WARN_ON_ONCE(region->flags.range_pinned))
> > > + return false;
> > > +
> > > + /* Align the page offset to the nearest MSHV_MAP_FAULT_IN_PAGES. */
> > > + page_offset = ALIGN_DOWN(gfn - region->start_gfn,
> > > + MSHV_MAP_FAULT_IN_PAGES);
> > > +
> > > + /* Map more pages than requested to reduce the number of faults. */
> > > + page_count = min(region->nr_pages - page_offset,
> > > + MSHV_MAP_FAULT_IN_PAGES);
> >
> > These computations make the range defined by page_offset and page_count
> > start on a 512 page boundary relative to start_gfn, and have a size that is a
> > multiple of 512 pages. But they don't ensure that the range aligns to a large page
> > boundary within gfn space since region->start_gfn may not be a multiple of
> > 512. Then mshv_region_range_fault() tests the first page of the range for
> > being a large page, and if so, sets region->large_pages. This doesn't make
> > sense to me if the range doesn't align to a large page boundary.
> >
> > Does this code need to make sure that the range is aligned to a large
> > page boundary in gfn space? Or am I misunderstanding what the
> > region->large_pages flag means? Given the fixes in this v6 of the
> > patch set, I was thinking that region->large_pages means that every
> > large page aligned area within the region is a large page. If region->
> > start_gfn and region->nr_pages aren't multiples of 512, then there
> > may be an initial range and a final range that aren't large pages,
> > but everything in between is. If that's not a correct understanding,
> > could you clarify the exact meaning of the region->large_pages
> > flag?
> >
>
> That's a good catch. Initially, the approach to memory deposit involved pinning
> and depositing all pages. The code assumes that if the first page in the region
> is huge, it is sufficient to use the "map huge page" flag in the hypercall.
Right. But even for pinned regions as coded today, is that assumption
correct? Due to memory being fragmented at the time of region creation,
it would be possible that some 2Meg ranges in a region are backed by a large
page, while other 2Meg ranges are not. In that case, a single per-region flag
isn't enough information. Or does the hypercall work OK if the "map huge
page" flag is passed when the range isn't a huge page? I'm not clear on what
the hypercall requires as input.
>
> With this series, the region is sparse by default, reducing the likelihood of
> huge pages in the region. As a result, using this flag seems neither correct
> nor reasonable.
Yep.
>
> Ideally, whether to use the flag should be determined during each guest memory
> map/unmap operation, rather than relying on the flag set during the initial
> region mapping.
>
> For now, I will remove the large_pages flag for movable regions in this
> series, as it is the least intrusive change. However, I plan to investigate
> this further and potentially replace the large_pages flag with a runtime
> check in the next series.
OK. So what is the impact? Losing the perf benefit of mapping guest
memory in the SLAT as a 2 Meg large page vs. a bunch of individual 4K
pages? Anything else?
>
> > > +
> > > + ret = mshv_region_range_fault(region, page_offset, page_count);
> > > +
> > > + WARN_ONCE(ret,
> > > + "p%llu: GPA intercept failed: region %#llx-%#llx, gfn %#llx, page_offset %llu, page_count %llu\n",
> > > + region->partition->pt_id, region->start_uaddr,
> > > + region->start_uaddr + (region->nr_pages << HV_HYP_PAGE_SHIFT),
> > > + gfn, page_offset, page_count);
> > > +
> > > + return !ret;
> > > +}
> > > +
> > > +/**
> > > + * mshv_handle_gpa_intercept - Handle GPA (Guest Physical Address) intercepts.
> > > + * @vp: Pointer to the virtual processor structure.
> > > + *
> > > + * This function processes GPA intercepts by identifying the memory region
> > > + * corresponding to the intercepted GPA, aligning the page offset, and
> > > + * mapping the required pages. It ensures that the region is valid and
> > > + * handles faults efficiently by mapping multiple pages at once.
> > > + *
> > > + * Return: true if the intercept was handled successfully, false otherwise.
> > > + */
> > > +static bool mshv_handle_gpa_intercept(struct mshv_vp *vp)
> > > +{
> > > + struct mshv_partition *p = vp->vp_partition;
> > > + struct mshv_mem_region *region;
> > > + struct hv_x64_memory_intercept_message *msg;
> > > + u64 gfn;
> > > +
> > > + msg = (struct hv_x64_memory_intercept_message *)
> > > + vp->vp_intercept_msg_page->u.payload;
> > > +
> > > + gfn = HVPFN_DOWN(msg->guest_physical_address);
> > > +
> > > + region = mshv_partition_region_by_gfn(p, gfn);
> > > + if (!region)
> > > + return false;
> >
> > Does it ever happen that the gfn is legitimately not found in any
> > region, perhaps due to a race? I think the vp_mutex is held here,
> > so maybe that protects the region layout for the VP and "not found"
> > should never occur. If so, should there be a WARN_ON here?
> >
> > If "gfn not found" can be legitimate, perhaps a comment to
> > explain the circumstances would be helpful.
> >
>
> This is possible, if hypervisor returns some invalid GFN.
> But there is also a possibility, that this code can race with region removal from a guest.
> I'll address it in the next revision.
In either of these cases, what happens next? The MSHV_RUN_VP ioctl
will return to user space with the unhandled HVMSG_GPA_INTERCEPT
message. Is there anything user space can do to enable the VP to make
progress past the fault? Or does user space just have to terminate the
guest VM?
>
> > > +
> > > + if (WARN_ON_ONCE(!region->flags.is_ram))
> > > + return false;
> > > +
> > > + if (WARN_ON_ONCE(region->flags.range_pinned))
> > > + return false;
> > > +
> > > + return mshv_region_handle_gfn_fault(region, gfn);
> > > +}
> > > +
> > > +#else /* CONFIG_X86_64 */
> > > +
> > > +static bool mshv_handle_gpa_intercept(struct mshv_vp *vp) { return false; }
> > > +
> > > +#endif /* CONFIG_X86_64 */
> > > +
> > > +static bool mshv_vp_handle_intercept(struct mshv_vp *vp)
> > > +{
> > > + switch (vp->vp_intercept_msg_page->header.message_type) {
> > > + case HVMSG_GPA_INTERCEPT:
> > > + return mshv_handle_gpa_intercept(vp);
> > > + }
> > > + return false;
> > > +}
> > > +
> > > static long mshv_vp_ioctl_run_vp(struct mshv_vp *vp, void __user *ret_msg)
> > > {
> > > long rc;
> > >
> > > - if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
> > > - rc = mshv_run_vp_with_root_scheduler(vp);
> > > - else
> > > - rc = mshv_run_vp_with_hyp_scheduler(vp);
> > > + do {
> > > + if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
> > > + rc = mshv_run_vp_with_root_scheduler(vp);
> > > + else
> > > + rc = mshv_run_vp_with_hyp_scheduler(vp);
> > > + } while (rc == 0 && mshv_vp_handle_intercept(vp));
> > >
> > > if (rc)
> > > return rc;
> > > @@ -1194,6 +1385,110 @@ mshv_partition_region_by_gfn(struct mshv_partition *partition, u64 gfn)
> > > return NULL;
> > > }
> > >
> > > +#if defined(CONFIG_MMU_NOTIFIER)
> > > +static void mshv_region_movable_fini(struct mshv_mem_region *region)
> > > +{
> > > + if (region->flags.range_pinned)
> > > + return;
> > > +
> > > + mmu_interval_notifier_remove(®ion->mni);
> > > +}
> > > +
> > > +/**
> > > + * mshv_region_interval_invalidate - Invalidate a range of memory region
> > > + * @mni: Pointer to the mmu_interval_notifier structure
> > > + * @range: Pointer to the mmu_notifier_range structure
> > > + * @cur_seq: Current sequence number for the interval notifier
> > > + *
> > > + * This function invalidates a memory region by remapping its pages with
> > > + * no access permissions. It locks the region's mutex to ensure thread safety
> > > + * and updates the sequence number for the interval notifier. If the range
> > > + * is blockable, it uses a blocking lock; otherwise, it attempts a non-blocking
> > > + * lock and returns false if unsuccessful.
> > > + *
> > > + * NOTE: Failure to invalidate a region is a serious error, as the pages will
> > > + * be considered freed while they are still mapped by the hypervisor.
> > > + * Any attempt to access such pages will likely crash the system.
> > > + *
> > > + * Return: true if the region was successfully invalidated, false otherwise.
> > > + */
> > > +static bool
> > > +mshv_region_interval_invalidate(struct mmu_interval_notifier *mni,
> > > + const struct mmu_notifier_range *range,
> > > + unsigned long cur_seq)
> > > +{
> > > + struct mshv_mem_region *region = container_of(mni,
> > > + struct mshv_mem_region,
> > > + mni);
> > > + u64 page_offset, page_count;
> > > + unsigned long mstart, mend;
> > > + int ret = -EPERM;
> > > +
> > > + if (mmu_notifier_range_blockable(range))
> > > + mutex_lock(®ion->mutex);
> > > + else if (!mutex_trylock(®ion->mutex))
> > > + goto out_fail;
> > > +
> > > + mmu_interval_set_seq(mni, cur_seq);
> > > +
> > > + mstart = max(range->start, region->start_uaddr);
> > > + mend = min(range->end, region->start_uaddr +
> > > + (region->nr_pages << HV_HYP_PAGE_SHIFT));
> >
> > I'm pretty sure region->start_uaddr is always page aligned. But what
> > about range->start and range->end? The code here and below assumes
> > they are page aligned. It also assumes that range->end is greater than
> > range->start so the computation of page_count doesn't wrap and so
> > page_count is >= 1. I don't know whether checks for these assumptions
> > are appropriate.
> >
>
> There is a check for memory region size to be non-zero and page aligned
> in mshv_partition_ioct_set_memory function, which is the only caller for
> memory region creation. And region start is defined in PFNs.
>
Right -- no disagreement that the region start and size are page aligned
and non-zero. But what about the range that is being invalidated?
(i.e., range->start and range->end) The values in that range are coming
from the mm subsystem, and aren't governed by how a region is created.
If that range is a subset of the MSHV region, then
mshv_region_internal_invalidate() will be operating on whatever subset
was provided in the 'range' argument.
mshv_region_interval_invalidate() is ultimately called from
mmu_notifier_invalidate_range_start(), which has about 30 different
callers in the kernel, mostly in the mm subsystem. It wasn't
clear to me what rules, if any, those 30 callers are following when they
set up the range to be invalidated.
Michael
^ permalink raw reply
* [PATCH net-next v11 13/13] selftests/vsock: add tests for namespace deletion and mode changes
From: Bobby Eshleman @ 2025-11-21 5:44 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan
Cc: linux-kernel, virtualization, netdev, kvm, linux-hyperv,
linux-kselftest, berrange, Sargun Dhillon, Bobby Eshleman,
Bobby Eshleman
In-Reply-To: <20251120-vsock-vmtest-v11-0-55cbc80249a7@meta.com>
From: Bobby Eshleman <bobbyeshleman@meta.com>
Add tests that validate vsock sockets are resilient to deleting
namespaces or changing namespace modes from global to local. The vsock
sockets should still function normally.
The function check_ns_changes_dont_break_connection() is added to re-use
the step-by-step logic of 1) setup connections, 2) do something that
would maybe break the connections, 3) check that the connections are
still ok.
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v11:
- remove pipefile (Stefano)
Changes in v9:
- more consistent shell style
- clarify -u usage comment for pipefile
---
tools/testing/selftests/vsock/vmtest.sh | 119 ++++++++++++++++++++++++++++++++
1 file changed, 119 insertions(+)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
index dfa895abfc7f..5f0b24845fad 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -69,6 +69,12 @@ readonly TEST_NAMES=(
ns_same_local_loopback_ok
ns_same_local_host_connect_to_local_vm_ok
ns_same_local_vm_connect_to_local_host_ok
+ ns_mode_change_connection_continue_vm_ok
+ ns_mode_change_connection_continue_host_ok
+ ns_mode_change_connection_continue_both_ok
+ ns_delete_vm_ok
+ ns_delete_host_ok
+ ns_delete_both_ok
)
readonly TEST_DESCS=(
# vm_server_host_client
@@ -139,6 +145,24 @@ readonly TEST_DESCS=(
# ns_same_local_vm_connect_to_local_host_ok
"Run vsock_test client in VM in a local ns with server in same ns."
+
+ # ns_mode_change_connection_continue_vm_ok
+ "Check that changing NS mode of VM namespace from global to local after a connection is established doesn't break the connection"
+
+ # ns_mode_change_connection_continue_host_ok
+ "Check that changing NS mode of host namespace from global to local after a connection is established doesn't break the connection"
+
+ # ns_mode_change_connection_continue_both_ok
+ "Check that changing NS mode of host and VM namespaces from global to local after a connection is established doesn't break the connection"
+
+ # ns_delete_vm_ok
+ "Check that deleting the VM's namespace does not break the socket connection"
+
+ # ns_delete_host_ok
+ "Check that deleting the host's namespace does not break the socket connection"
+
+ # ns_delete_both_ok
+ "Check that deleting the VM and host's namespaces does not break the socket connection"
)
readonly USE_SHARED_VM=(
@@ -1288,6 +1312,101 @@ test_ns_vm_local_mode_rejected() {
return "${KSFT_PASS}"
}
+check_ns_changes_dont_break_connection() {
+ local pipefile pidfile outfile
+ local ns0="global0"
+ local ns1="global1"
+ local port=12345
+ local pids=()
+ local rc=0
+
+ init_namespaces
+
+ pidfile="$(create_pidfile)"
+ if ! vm_start "${pidfile}" "${ns0}"; then
+ return "${KSFT_FAIL}"
+ fi
+ vm_wait_for_ssh "${ns0}"
+
+ outfile=$(mktemp)
+ vm_ssh "${ns0}" -- \
+ socat VSOCK-LISTEN:"${port}",fork STDOUT > "${outfile}" 2>/dev/null &
+ pids+=($!)
+ vm_wait_for_listener "${ns0}" "${port}" "vsock"
+
+ # We use a pipe here so that we can echo into the pipe instead of using
+ # socat and a unix socket file. We just need a name for the pipe (not a
+ # regular file) so use -u.
+ pipefile=$(mktemp -u /tmp/vmtest_pipe_XXXX)
+ ip netns exec "${ns1}" \
+ socat PIPE:"${pipefile}" VSOCK-CONNECT:"${VSOCK_CID}":"${port}" &
+ pids+=($!)
+
+ timeout "${WAIT_PERIOD}" \
+ bash -c 'while [[ ! -e '"${pipefile}"' ]]; do sleep 1; done; exit 0'
+
+ if [[ $2 == "delete" ]]; then
+ if [[ "$1" == "vm" ]]; then
+ ip netns del "${ns0}"
+ elif [[ "$1" == "host" ]]; then
+ ip netns del "${ns1}"
+ elif [[ "$1" == "both" ]]; then
+ ip netns del "${ns0}"
+ ip netns del "${ns1}"
+ fi
+ elif [[ $2 == "change_mode" ]]; then
+ if [[ "$1" == "vm" ]]; then
+ ns_set_mode "${ns0}" "local"
+ elif [[ "$1" == "host" ]]; then
+ ns_set_mode "${ns1}" "local"
+ elif [[ "$1" == "both" ]]; then
+ ns_set_mode "${ns0}" "local"
+ ns_set_mode "${ns1}" "local"
+ fi
+ fi
+
+ echo "TEST" > "${pipefile}"
+
+ timeout "${WAIT_PERIOD}" \
+ bash -c 'while [[ ! -s '"${outfile}"' ]]; do sleep 1; done; exit 0'
+
+ if grep -q "TEST" "${outfile}"; then
+ rc="${KSFT_PASS}"
+ else
+ rc="${KSFT_FAIL}"
+ fi
+
+ terminate_pidfiles "${pidfile}"
+ terminate_pids "${pids[@]}"
+ rm -f "${outfile}" "${pipefile}"
+
+ return "${rc}"
+}
+
+test_ns_mode_change_connection_continue_vm_ok() {
+ check_ns_changes_dont_break_connection "vm" "change_mode"
+}
+
+test_ns_mode_change_connection_continue_host_ok() {
+ check_ns_changes_dont_break_connection "host" "change_mode"
+}
+
+test_ns_mode_change_connection_continue_both_ok() {
+ check_ns_changes_dont_break_connection "both" "change_mode"
+}
+
+test_ns_delete_vm_ok() {
+ check_ns_changes_dont_break_connection "vm" "delete"
+}
+
+test_ns_delete_host_ok() {
+ check_ns_changes_dont_break_connection "host" "delete"
+}
+
+test_ns_delete_both_ok() {
+ check_ns_changes_dont_break_connection "both" "delete"
+}
+
shared_vm_test() {
local tname
--
2.47.3
^ permalink raw reply related
* [PATCH net-next v11 12/13] selftests/vsock: add tests for host <-> vm connectivity with namespaces
From: Bobby Eshleman @ 2025-11-21 5:44 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan
Cc: linux-kernel, virtualization, netdev, kvm, linux-hyperv,
linux-kselftest, berrange, Sargun Dhillon, Bobby Eshleman,
Bobby Eshleman
In-Reply-To: <20251120-vsock-vmtest-v11-0-55cbc80249a7@meta.com>
From: Bobby Eshleman <bobbyeshleman@meta.com>
Add tests to validate namespace correctness using vsock_test and socat.
The vsock_test tool is used to validate expected success tests, but
socat is used for expected failure tests. socat is used to ensure that
connections are rejected outright instead of failing due to some other
socket behavior (as tested in vsock_test). Additionally, socat is
already required for tunneling TCP traffic from vsock_test. Using only
one of the vsock_test tests like 'test_stream_client_close_client' would
have yielded a similar result, but doing so wouldn't remove the socat
dependency.
Additionally, check for the dependency socat. socat needs special
handling beyond just checking if it is on the path because it must be
compiled with support for both vsock and unix. The function
check_socat() checks that this support exists.
Add more padding to test name printf strings because the tests added in
this patch would otherwise overflow.
Add vm_dmesg_start() and vm_dmesg_check() to encapsulate checking dmesg
for oops and warnings.
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v11:
- add 'sleep "${WAIT_PERIOD}"' after any non-TCP socat LISTEN cmd
(Stefano)
- add host_wait_for_listener() after any socat TCP-LISTEN (Stefano)
- reuse vm_dmesg_{oops,warn}_count() inside vm_dmesg_check()
- fix copy-paste in test_ns_same_local_vm_connect_to_local_host_ok()
(Stefano)
Changes in v10:
- add vm_dmesg_start() and vm_dmesg_check()
Changes in v9:
- consistent variable quoting
---
tools/testing/selftests/vsock/vmtest.sh | 557 +++++++++++++++++++++++++++++++-
1 file changed, 555 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
index f84da1e8ad14..dfa895abfc7f 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -7,6 +7,7 @@
# * virtme-ng
# * busybox-static (used by virtme-ng)
# * qemu (used by virtme-ng)
+# * socat
#
# shellcheck disable=SC2317,SC2119
@@ -55,6 +56,19 @@ readonly TEST_NAMES=(
ns_local_same_cid_ok
ns_global_local_same_cid_ok
ns_local_global_same_cid_ok
+ ns_diff_global_host_connect_to_global_vm_ok
+ ns_diff_global_host_connect_to_local_vm_fails
+ ns_diff_global_vm_connect_to_global_host_ok
+ ns_diff_global_vm_connect_to_local_host_fails
+ ns_diff_local_host_connect_to_local_vm_fails
+ ns_diff_local_vm_connect_to_local_host_fails
+ ns_diff_global_to_local_loopback_local_fails
+ ns_diff_local_to_global_loopback_fails
+ ns_diff_local_to_local_loopback_fails
+ ns_diff_global_to_global_loopback_ok
+ ns_same_local_loopback_ok
+ ns_same_local_host_connect_to_local_vm_ok
+ ns_same_local_vm_connect_to_local_host_ok
)
readonly TEST_DESCS=(
# vm_server_host_client
@@ -86,6 +100,45 @@ readonly TEST_DESCS=(
# ns_local_global_same_cid_ok
"Check QEMU successfully starts one VM in a local ns and then another VM in a global ns with the same CID."
+
+ # ns_diff_global_host_connect_to_global_vm_ok
+ "Run vsock_test client in global ns with server in VM in another global ns."
+
+ # ns_diff_global_host_connect_to_local_vm_fails
+ "Run socat to test a process in a global ns fails to connect to a VM in a local ns."
+
+ # ns_diff_global_vm_connect_to_global_host_ok
+ "Run vsock_test client in VM in a global ns with server in another global ns."
+
+ # ns_diff_global_vm_connect_to_local_host_fails
+ "Run socat to test a VM in a global ns fails to connect to a host process in a local ns."
+
+ # ns_diff_local_host_connect_to_local_vm_fails
+ "Run socat to test a host process in a local ns fails to connect to a VM in another local ns."
+
+ # ns_diff_local_vm_connect_to_local_host_fails
+ "Run socat to test a VM in a local ns fails to connect to a host process in another local ns."
+
+ # ns_diff_global_to_local_loopback_local_fails
+ "Run socat to test a loopback vsock in a global ns fails to connect to a vsock in a local ns."
+
+ # ns_diff_local_to_global_loopback_fails
+ "Run socat to test a loopback vsock in a local ns fails to connect to a vsock in a global ns."
+
+ # ns_diff_local_to_local_loopback_fails
+ "Run socat to test a loopback vsock in a local ns fails to connect to a vsock in another local ns."
+
+ # ns_diff_global_to_global_loopback_ok
+ "Run socat to test a loopback vsock in a global ns successfully connects to a vsock in another global ns."
+
+ # ns_same_local_loopback_ok
+ "Run socat to test a loopback vsock in a local ns successfully connects to a vsock in the same ns."
+
+ # ns_same_local_host_connect_to_local_vm_ok
+ "Run vsock_test client in a local ns with server in VM in same ns."
+
+ # ns_same_local_vm_connect_to_local_host_ok
+ "Run vsock_test client in VM in a local ns with server in same ns."
)
readonly USE_SHARED_VM=(
@@ -117,7 +170,7 @@ usage() {
for ((i = 0; i < ${#TEST_NAMES[@]}; i++)); do
name=${TEST_NAMES[${i}]}
desc=${TEST_DESCS[${i}]}
- printf "\t%-35s%-35s\n" "${name}" "${desc}"
+ printf "\t%-55s%-35s\n" "${name}" "${desc}"
done
echo
@@ -236,7 +289,7 @@ check_args() {
}
check_deps() {
- for dep in vng ${QEMU} busybox pkill ssh ss; do
+ for dep in vng ${QEMU} busybox pkill ssh ss socat; do
if [[ ! -x $(command -v "${dep}") ]]; then
echo -e "skip: dependency ${dep} not found!\n"
exit "${KSFT_SKIP}"
@@ -287,6 +340,20 @@ check_vng() {
fi
}
+check_socat() {
+ local support_string
+
+ support_string="$(socat -V)"
+
+ if [[ "${support_string}" != *"WITH_VSOCK 1"* ]]; then
+ die "err: socat is missing vsock support"
+ fi
+
+ if [[ "${support_string}" != *"WITH_UNIX 1"* ]]; then
+ die "err: socat is missing unix support"
+ fi
+}
+
handle_build() {
if [[ ! "${BUILD}" -eq 1 ]]; then
return
@@ -335,6 +402,14 @@ terminate_pidfiles() {
done
}
+terminate_pids() {
+ local pid
+
+ for pid in "$@"; do
+ kill -SIGTERM "${pid}" &>/dev/null || :
+ done
+}
+
vm_start() {
local pidfile=$1
local ns=$2
@@ -473,6 +548,28 @@ vm_dmesg_warn_count() {
vm_ssh "${ns}" -- dmesg --level=warn 2>/dev/null | grep -c -i 'vsock'
}
+vm_dmesg_check() {
+ local pidfile=$1
+ local ns=$2
+ local oops_before=$3
+ local warn_before=$4
+ local oops_after warn_after
+
+ oops_after=$(vm_dmesg_oops_count "${ns}")
+ if [[ "${oops_after}" -gt "${oops_before}" ]]; then
+ echo "FAIL: kernel oops detected on vm in ns ${ns}" | log_host
+ return 1
+ fi
+
+ warn_after=$(vm_dmesg_warn_count "${ns}")
+ if [[ "${warn_after}" -gt "${warn_before}" ]]; then
+ echo "FAIL: kernel warning detected on vm in ns ${ns}" | log_host
+ return 1
+ fi
+
+ return 0
+}
+
vm_vsock_test() {
local ns=$1
local host=$2
@@ -597,6 +694,461 @@ test_ns_host_vsock_ns_mode_ok() {
return "${KSFT_PASS}"
}
+test_ns_diff_global_host_connect_to_global_vm_ok() {
+ local oops_before warn_before
+ local pids pid pidfile
+ local ns0 ns1 port
+ declare -a pids
+ local unixfile
+ ns0="global0"
+ ns1="global1"
+ port=1234
+ local rc
+
+ init_namespaces
+
+ pidfile="$(create_pidfile)"
+
+ if ! vm_start "${pidfile}" "${ns0}"; then
+ return "${KSFT_FAIL}"
+ fi
+
+ vm_wait_for_ssh "${ns0}"
+ oops_before=$(vm_dmesg_oops_count "${ns0}")
+ warn_before=$(vm_dmesg_warn_count "${ns0}")
+
+ unixfile=$(mktemp -u /tmp/XXXX.sock)
+ ip netns exec "${ns1}" \
+ socat TCP-LISTEN:"${TEST_HOST_PORT}",fork \
+ UNIX-CONNECT:"${unixfile}" &
+ pids+=($!)
+ host_wait_for_listener "${ns1}" "${TEST_HOST_PORT}" "tcp"
+
+ ip netns exec "${ns0}" socat UNIX-LISTEN:"${unixfile}",fork \
+ TCP-CONNECT:localhost:"${TEST_HOST_PORT}" &
+ pids+=($!)
+ host_wait_for_listener "${ns0}" "${unixfile}" "unix"
+
+ vm_vsock_test "${ns0}" "server" 2 "${TEST_GUEST_PORT}"
+ vm_wait_for_listener "${ns0}" "${TEST_GUEST_PORT}" "tcp"
+ host_vsock_test "${ns1}" "127.0.0.1" "${VSOCK_CID}" "${TEST_HOST_PORT}"
+ rc=$?
+
+ vm_dmesg_check "${pidfile}" "${ns0}" "${oops_before}" "${warn_before}"
+ dmesg_rc=$?
+
+ terminate_pids "${pids[@]}"
+ terminate_pidfiles "${pidfile}"
+
+ if [[ "${rc}" -ne 0 ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+}
+
+test_ns_diff_global_host_connect_to_local_vm_fails() {
+ local oops_before warn_before
+ local ns0="global0"
+ local ns1="local0"
+ local port=12345
+ local dmesg_rc
+ local pidfile
+ local result
+ local pid
+
+ init_namespaces
+
+ outfile=$(mktemp)
+
+ pidfile="$(create_pidfile)"
+ if ! vm_start "${pidfile}" "${ns1}"; then
+ log_host "failed to start vm (cid=${VSOCK_CID}, ns=${ns0})"
+ return "${KSFT_FAIL}"
+ fi
+
+ vm_wait_for_ssh "${ns1}"
+ oops_before=$(vm_dmesg_oops_count "${ns1}")
+ warn_before=$(vm_dmesg_warn_count "${ns1}")
+
+ vm_ssh "${ns1}" -- socat VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" &
+ vm_wait_for_listener "${ns1}" "${port}" "vsock"
+ echo TEST | ip netns exec "${ns0}" \
+ socat STDIN VSOCK-CONNECT:"${VSOCK_CID}":"${port}" 2>/dev/null
+
+ vm_dmesg_check "${pidfile}" "${ns1}" "${oops_before}" "${warn_before}"
+ dmesg_rc=$?
+
+ terminate_pidfiles "${pidfile}"
+ result=$(cat "${outfile}")
+ rm -f "${outfile}"
+
+ if [[ "${result}" == "TEST" ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+}
+
+test_ns_diff_global_vm_connect_to_global_host_ok() {
+ local oops_before warn_before
+ local ns0="global0"
+ local ns1="global1"
+ local port=12345
+ local unixfile
+ local dmesg_rc
+ local pidfile
+ local pids
+ local rc
+
+ init_namespaces
+
+ declare -a pids
+
+ log_host "Setup socat bridge from ns ${ns0} to ns ${ns1} over port ${port}"
+
+ unixfile=$(mktemp -u /tmp/XXXX.sock)
+
+ ip netns exec "${ns0}" \
+ socat TCP-LISTEN:"${port}" UNIX-CONNECT:"${unixfile}" &
+ pids+=($!)
+ host_wait_for_listener "${ns0}" "${port}" "tcp"
+
+ ip netns exec "${ns1}" \
+ socat UNIX-LISTEN:"${unixfile}" TCP-CONNECT:127.0.0.1:"${port}" &
+ pids+=($!)
+ host_wait_for_listener "${ns1}" "${unixfile}" "unix"
+
+ log_host "Launching ${VSOCK_TEST} in ns ${ns1}"
+ host_vsock_test "${ns1}" "server" "${VSOCK_CID}" "${port}"
+
+ pidfile="$(create_pidfile)"
+ if ! vm_start "${pidfile}" "${ns0}"; then
+ log_host "failed to start vm (cid=${cid}, ns=${ns0})"
+ terminate_pids "${pids[@]}"
+ rm -f "${unixfile}"
+ return "${KSFT_FAIL}"
+ fi
+
+ vm_wait_for_ssh "${ns0}"
+
+ oops_before=$(vm_dmesg_oops_count "${ns0}")
+ warn_before=$(vm_dmesg_warn_count "${ns0}")
+
+ vm_vsock_test "${ns0}" "10.0.2.2" 2 "${port}"
+ rc=$?
+
+ vm_dmesg_check "${pidfile}" "${ns0}" "${oops_before}" "${warn_before}"
+ dmesg_rc=$?
+
+ terminate_pidfiles "${pidfile}"
+ terminate_pids "${pids[@]}"
+ rm -f "${unixfile}"
+
+ if [[ "${rc}" -ne 0 ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+
+}
+
+test_ns_diff_global_vm_connect_to_local_host_fails() {
+ local ns0="global0"
+ local ns1="local0"
+ local port=12345
+ local oops_before warn_before
+ local dmesg_rc
+ local pidfile
+ local result
+ local pid
+
+ init_namespaces
+
+ log_host "Launching socat in ns ${ns1}"
+ outfile=$(mktemp)
+
+ ip netns exec "${ns1}" socat VSOCK-LISTEN:"${port}" STDOUT &> "${outfile}" &
+ pid=$!
+ host_wait_for_listener "${ns1}" "${port}" "vsock"
+
+ pidfile="$(create_pidfile)"
+ if ! vm_start "${pidfile}" "${ns0}"; then
+ log_host "failed to start vm (cid=${cid}, ns=${ns0})"
+ terminate_pids "${pid}"
+ rm -f "${outfile}"
+ return "${KSFT_FAIL}"
+ fi
+
+ vm_wait_for_ssh "${ns0}"
+
+ oops_before=$(vm_dmesg_oops_count "${ns0}")
+ warn_before=$(vm_dmesg_warn_count "${ns0}")
+
+ vm_ssh "${ns0}" -- \
+ bash -c "echo TEST | socat STDIN VSOCK-CONNECT:2:${port}" 2>&1 | log_guest
+
+ vm_dmesg_check "${pidfile}" "${ns0}" "${oops_before}" "${warn_before}"
+ dmesg_rc=$?
+
+ terminate_pidfiles "${pidfile}"
+ terminate_pids "${pid}"
+
+ result=$(cat "${outfile}")
+ rm -f "${outfile}"
+
+ if [[ "${result}" != TEST ]] && [[ "${dmesg_rc}" -eq 0 ]]; then
+ return "${KSFT_PASS}"
+ fi
+
+ return "${KSFT_FAIL}"
+}
+
+test_ns_diff_local_host_connect_to_local_vm_fails() {
+ local ns0="local0"
+ local ns1="local1"
+ local port=12345
+ local oops_before warn_before
+ local dmesg_rc
+ local pidfile
+ local result
+ local pid
+
+ init_namespaces
+
+ outfile=$(mktemp)
+
+ pidfile="$(create_pidfile)"
+ if ! vm_start "${pidfile}" "${ns1}"; then
+ log_host "failed to start vm (cid=${cid}, ns=${ns0})"
+ return "${KSFT_FAIL}"
+ fi
+
+ vm_wait_for_ssh "${ns1}"
+ oops_before=$(vm_dmesg_oops_count "${ns1}")
+ warn_before=$(vm_dmesg_warn_count "${ns1}")
+
+ vm_ssh "${ns1}" -- socat VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" &
+ vm_wait_for_listener "${ns1}" "${port}" "vsock"
+
+ echo TEST | ip netns exec "${ns0}" \
+ socat STDIN VSOCK-CONNECT:"${VSOCK_CID}":"${port}" 2>/dev/null
+
+ vm_dmesg_check "${pidfile}" "${ns1}" "${oops_before}" "${warn_before}"
+ dmesg_rc=$?
+
+ terminate_pidfiles "${pidfile}"
+
+ result=$(cat "${outfile}")
+ rm -f "${outfile}"
+
+ if [[ "${result}" != TEST ]] && [[ "${dmesg_rc}" -eq 0 ]]; then
+ return "${KSFT_PASS}"
+ fi
+
+ return "${KSFT_FAIL}"
+}
+
+test_ns_diff_local_vm_connect_to_local_host_fails() {
+ local oops_before warn_before
+ local ns0="local0"
+ local ns1="local1"
+ local port=12345
+ local dmesg_rc
+ local pidfile
+ local result
+ local pid
+
+ init_namespaces
+
+ log_host "Launching socat in ns ${ns1}"
+ outfile=$(mktemp)
+ ip netns exec "${ns1}" socat VSOCK-LISTEN:"${port}" STDOUT &> "${outfile}" &
+ pid=$!
+ host_wait_for_listener "${ns1}" "${port}" "vsock"
+
+ pidfile="$(create_pidfile)"
+ if ! vm_start "${pidfile}" "${ns0}"; then
+ log_host "failed to start vm (cid=${cid}, ns=${ns0})"
+ rm -f "${outfile}"
+ return "${KSFT_FAIL}"
+ fi
+
+ vm_wait_for_ssh "${ns0}"
+ oops_before=$(vm_dmesg_oops_count "${ns0}")
+ warn_before=$(vm_dmesg_warn_count "${ns0}")
+
+ vm_ssh "${ns0}" -- \
+ bash -c "echo TEST | socat STDIN VSOCK-CONNECT:2:${port}" 2>&1 | log_guest
+
+ vm_dmesg_check "${pidfile}" "${ns0}" "${oops_before}" "${warn_before}"
+ dmesg_rc=$?
+
+ terminate_pidfiles "${pidfile}"
+ terminate_pids "${pid}"
+
+ result=$(cat "${outfile}")
+ rm -f "${outfile}"
+
+ if [[ "${result}" != TEST ]] && [[ "${dmesg_rc}" -eq 0 ]]; then
+ return "${KSFT_PASS}"
+ fi
+
+ return "${KSFT_FAIL}"
+}
+
+__test_loopback_two_netns() {
+ local ns0=$1
+ local ns1=$2
+ local port=12345
+ local result
+ local pid
+
+ modprobe vsock_loopback &> /dev/null || :
+
+ log_host "Launching socat in ns ${ns1}"
+ outfile=$(mktemp)
+
+ ip netns exec "${ns1}" socat VSOCK-LISTEN:"${port}" STDOUT > "${outfile}" 2>/dev/null &
+ pid=$!
+ host_wait_for_listener "${ns1}" "${port}" "vsock"
+
+ log_host "Launching socat in ns ${ns0}"
+ echo TEST | ip netns exec "${ns0}" socat STDIN VSOCK-CONNECT:1:"${port}" 2>/dev/null
+ terminate_pids "${pid}"
+
+ result=$(cat "${outfile}")
+ rm -f "${outfile}"
+
+ if [[ "${result}" == TEST ]]; then
+ return 0
+ fi
+
+ return 1
+}
+
+test_ns_diff_global_to_local_loopback_local_fails() {
+ init_namespaces
+
+ if ! __test_loopback_two_netns "global0" "local0"; then
+ return "${KSFT_PASS}"
+ fi
+
+ return "${KSFT_FAIL}"
+}
+
+test_ns_diff_local_to_global_loopback_fails() {
+ init_namespaces
+
+ if ! __test_loopback_two_netns "local0" "global0"; then
+ return "${KSFT_PASS}"
+ fi
+
+ return "${KSFT_FAIL}"
+}
+
+test_ns_diff_local_to_local_loopback_fails() {
+ init_namespaces
+
+ if ! __test_loopback_two_netns "local0" "local1"; then
+ return "${KSFT_PASS}"
+ fi
+
+ return "${KSFT_FAIL}"
+}
+
+test_ns_diff_global_to_global_loopback_ok() {
+ init_namespaces
+
+ if __test_loopback_two_netns "global0" "global1"; then
+ return "${KSFT_PASS}"
+ fi
+
+ return "${KSFT_FAIL}"
+}
+
+test_ns_same_local_loopback_ok() {
+ init_namespaces
+
+ if __test_loopback_two_netns "local0" "local0"; then
+ return "${KSFT_PASS}"
+ fi
+
+ return "${KSFT_FAIL}"
+}
+
+test_ns_same_local_host_connect_to_local_vm_ok() {
+ local oops_before warn_before
+ local ns="local0"
+ local port=1234
+ local dmesg_rc
+ local pidfile
+ local rc
+
+ init_namespaces
+
+ pidfile="$(create_pidfile)"
+
+ if ! vm_start "${pidfile}" "${ns}"; then
+ return "${KSFT_FAIL}"
+ fi
+
+ vm_wait_for_ssh "${ns}"
+ oops_before=$(vm_dmesg_oops_count "${ns}")
+ warn_before=$(vm_dmesg_warn_count "${ns}")
+
+ vm_vsock_test "${ns}" "server" 2 "${TEST_GUEST_PORT}"
+ host_vsock_test "${ns}" "127.0.0.1" "${VSOCK_CID}" "${TEST_HOST_PORT}"
+ rc=$?
+
+ vm_dmesg_check "${pidfile}" "${ns}" "${oops_before}" "${warn_before}"
+ dmesg_rc=$?
+
+ terminate_pidfiles "${pidfile}"
+
+ if [[ "${rc}" -ne 0 ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+}
+
+test_ns_same_local_vm_connect_to_local_host_ok() {
+ local oops_before warn_before
+ local ns="local0"
+ local port=1234
+ local dmesg_rc
+ local pidfile
+ local rc
+
+ init_namespaces
+
+ pidfile="$(create_pidfile)"
+
+ if ! vm_start "${pidfile}" "${ns}"; then
+ return "${KSFT_FAIL}"
+ fi
+
+ vm_wait_for_ssh "${ns}"
+ oops_before=$(vm_dmesg_oops_count "${ns}")
+ warn_before=$(vm_dmesg_warn_count "${ns}")
+
+ host_vsock_test "${ns}" "server" "${VSOCK_CID}" "${port}"
+ vm_vsock_test "${ns}" "10.0.2.2" 2 "${port}"
+ rc=$?
+
+ vm_dmesg_check "${pidfile}" "${ns}" "${oops_before}" "${warn_before}"
+ dmesg_rc=$?
+
+ terminate_pidfiles "${pidfile}"
+
+ if [[ "${rc}" -ne 0 ]] || [[ "${dmesg_rc}" -ne 0 ]]; then
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+}
+
namespaces_can_boot_same_cid() {
local ns0=$1
local ns1=$2
@@ -894,6 +1446,7 @@ fi
check_args "${ARGS[@]}"
check_deps
check_vng
+check_socat
handle_build
echo "1..${#ARGS[@]}"
--
2.47.3
^ permalink raw reply related
* [PATCH net-next v11 11/13] selftests/vsock: add namespace tests for CID collisions
From: Bobby Eshleman @ 2025-11-21 5:44 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan
Cc: linux-kernel, virtualization, netdev, kvm, linux-hyperv,
linux-kselftest, berrange, Sargun Dhillon, Bobby Eshleman,
Bobby Eshleman
In-Reply-To: <20251120-vsock-vmtest-v11-0-55cbc80249a7@meta.com>
From: Bobby Eshleman <bobbyeshleman@meta.com>
Add tests to verify CID collision rules across different vsock namespace
modes.
1. Two VMs with the same CID cannot start in different global namespaces
(ns_global_same_cid_fails)
2. Two VMs with the same CID can start in different local namespaces
(ns_local_same_cid_ok)
3. VMs with the same CID can coexist when one is in a global namespace
and another is in a local namespace (ns_global_local_same_cid_ok and
ns_local_global_same_cid_ok)
The tests ns_global_local_same_cid_ok and ns_local_global_same_cid_ok
make sure that ordering does not matter.
The tests use a shared helper function namespaces_can_boot_same_cid()
that attempts to start two VMs with identical CIDs in the specified
namespaces and verifies whether VM initialization failed or succeeded.
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v11:
- check vm_start() rc in namespaces_can_boot_same_cid() (Stefano)
- fix ns_local_same_cid_ok() to use local0 and local1 instead of reusing
local0 twice. This check should pass, ensuring local namespaces do not
collide (Stefano)
---
tools/testing/selftests/vsock/vmtest.sh | 78 +++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
index 2e077e8a1777..f84da1e8ad14 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -51,6 +51,10 @@ readonly TEST_NAMES=(
ns_host_vsock_ns_mode_ok
ns_host_vsock_ns_mode_write_once_ok
ns_vm_local_mode_rejected
+ ns_global_same_cid_fails
+ ns_local_same_cid_ok
+ ns_global_local_same_cid_ok
+ ns_local_global_same_cid_ok
)
readonly TEST_DESCS=(
# vm_server_host_client
@@ -70,6 +74,18 @@ readonly TEST_DESCS=(
# ns_vm_local_mode_rejected
"Test that guest VM with G2H transport cannot set namespace mode to 'local'"
+
+ # ns_global_same_cid_fails
+ "Check QEMU fails to start two VMs with same CID in two different global namespaces."
+
+ # ns_local_same_cid_ok
+ "Check QEMU successfully starts two VMs with same CID in two different local namespaces."
+
+ # ns_global_local_same_cid_ok
+ "Check QEMU successfully starts one VM in a global ns and then another VM in a local ns with the same CID."
+
+ # ns_local_global_same_cid_ok
+ "Check QEMU successfully starts one VM in a local ns and then another VM in a global ns with the same CID."
)
readonly USE_SHARED_VM=(
@@ -581,6 +597,68 @@ test_ns_host_vsock_ns_mode_ok() {
return "${KSFT_PASS}"
}
+namespaces_can_boot_same_cid() {
+ local ns0=$1
+ local ns1=$2
+ local pidfile1 pidfile2
+ local rc
+
+ pidfile1="$(create_pidfile)"
+
+ # The first VM should be able to start. If it can't then we have
+ # problems and need to return non-zero.
+ if ! vm_start "${pidfile1}" "${ns0}"; then
+ return 1
+ fi
+
+ pidfile2="$(create_pidfile)"
+ vm_start "${pidfile2}" "${ns1}"
+ rc=$?
+ terminate_pidfiles "${pidfile1}" "${pidfile2}"
+
+ return "${rc}"
+}
+
+test_ns_global_same_cid_fails() {
+ init_namespaces
+
+ if namespaces_can_boot_same_cid "global0" "global1"; then
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+}
+
+test_ns_local_global_same_cid_ok() {
+ init_namespaces
+
+ if namespaces_can_boot_same_cid "local0" "global0"; then
+ return "${KSFT_PASS}"
+ fi
+
+ return "${KSFT_FAIL}"
+}
+
+test_ns_global_local_same_cid_ok() {
+ init_namespaces
+
+ if namespaces_can_boot_same_cid "global0" "local0"; then
+ return "${KSFT_PASS}"
+ fi
+
+ return "${KSFT_FAIL}"
+}
+
+test_ns_local_same_cid_ok() {
+ init_namespaces
+
+ if namespaces_can_boot_same_cid "local0" "local1"; then
+ return "${KSFT_PASS}"
+ fi
+
+ return "${KSFT_FAIL}"
+}
+
test_ns_host_vsock_ns_mode_write_once_ok() {
for mode in "${NS_MODES[@]}"; do
local ns="${mode}0"
--
2.47.3
^ permalink raw reply related
* [PATCH net-next v11 10/13] selftests/vsock: add tests for proc sys vsock ns_mode
From: Bobby Eshleman @ 2025-11-21 5:44 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan
Cc: linux-kernel, virtualization, netdev, kvm, linux-hyperv,
linux-kselftest, berrange, Sargun Dhillon, Bobby Eshleman,
Bobby Eshleman
In-Reply-To: <20251120-vsock-vmtest-v11-0-55cbc80249a7@meta.com>
From: Bobby Eshleman <bobbyeshleman@meta.com>
Add tests for the /proc/sys/net/vsock/ns_mode interface. Namely,
that it accepts "global" and "local" strings and enforces a write-once
policy.
Start a convention of commenting the test name over the test
description. Add test name comments over test descriptions that existed
before this convention.
Add a check_netns() function that checks if the test requires namespaces
and if the current kernel supports namespaces. Skip tests that require
namespaces if the system does not have namespace support.
Add a test to verify that guest VMs with an active G2H transport
(virtio-vsock) cannot set namespace mode to 'local'. This validates
the mutual exclusion between G2H transports and LOCAL mode.
This patch is the first to add tests that do *not* re-use the same
shared VM. For that reason, it adds a run_tests() function to run these
tests and filter out the shared VM tests.
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
Changes in v11:
- Document ns_ prefix above TEST_NAMES (Stefano)
Changes in v10:
- Remove extraneous add_namespaces/del_namespaces calls.
- Rename run_tests() to run_ns_tests() since it is designed to only
run ns tests.
Changes in v9:
- add test ns_vm_local_mode_rejected to check that guests cannot use
local mode
---
tools/testing/selftests/vsock/vmtest.sh | 143 +++++++++++++++++++++++++++++++-
1 file changed, 141 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
index e32997db322d..2e077e8a1777 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -41,14 +41,43 @@ readonly KERNEL_CMDLINE="\
virtme.ssh virtme_ssh_channel=tcp virtme_ssh_user=$USER \
"
readonly LOG=$(mktemp /tmp/vsock_vmtest_XXXX.log)
-readonly TEST_NAMES=(vm_server_host_client vm_client_host_server vm_loopback)
+
+# Namespace tests must use the ns_ prefix. This is checked in check_netns() and
+# is used to determine if a test needs namespace setup before test execution.
+readonly TEST_NAMES=(
+ vm_server_host_client
+ vm_client_host_server
+ vm_loopback
+ ns_host_vsock_ns_mode_ok
+ ns_host_vsock_ns_mode_write_once_ok
+ ns_vm_local_mode_rejected
+)
readonly TEST_DESCS=(
+ # vm_server_host_client
"Run vsock_test in server mode on the VM and in client mode on the host."
+
+ # vm_client_host_server
"Run vsock_test in client mode on the VM and in server mode on the host."
+
+ # vm_loopback
"Run vsock_test using the loopback transport in the VM."
+
+ # ns_host_vsock_ns_mode_ok
+ "Check /proc/sys/net/vsock/ns_mode strings on the host."
+
+ # ns_host_vsock_ns_mode_write_once_ok
+ "Check /proc/sys/net/vsock/ns_mode is write-once on the host."
+
+ # ns_vm_local_mode_rejected
+ "Test that guest VM with G2H transport cannot set namespace mode to 'local'"
)
-readonly USE_SHARED_VM=(vm_server_host_client vm_client_host_server vm_loopback)
+readonly USE_SHARED_VM=(
+ vm_server_host_client
+ vm_client_host_server
+ vm_loopback
+ ns_vm_local_mode_rejected
+)
readonly NS_MODES=("local" "global")
VERBOSE=0
@@ -205,6 +234,20 @@ check_deps() {
fi
}
+check_netns() {
+ local tname=$1
+
+ # If the test requires NS support, check if NS support exists
+ # using /proc/self/ns
+ if [[ "${tname}" =~ ^ns_ ]] &&
+ [[ ! -e /proc/self/ns ]]; then
+ log_host "No NS support detected for test ${tname}"
+ return 1
+ fi
+
+ return 0
+}
+
check_vng() {
local tested_versions
local version
@@ -528,6 +571,32 @@ log_guest() {
LOG_PREFIX=guest log "$@"
}
+test_ns_host_vsock_ns_mode_ok() {
+ for mode in "${NS_MODES[@]}"; do
+ if ! ns_set_mode "${mode}0" "${mode}"; then
+ return "${KSFT_FAIL}"
+ fi
+ done
+
+ return "${KSFT_PASS}"
+}
+
+test_ns_host_vsock_ns_mode_write_once_ok() {
+ for mode in "${NS_MODES[@]}"; do
+ local ns="${mode}0"
+ if ! ns_set_mode "${ns}" "${mode}"; then
+ return "${KSFT_FAIL}"
+ fi
+
+ # try writing again and expect failure
+ if ns_set_mode "${ns}" "${mode}"; then
+ return "${KSFT_FAIL}"
+ fi
+ done
+
+ return "${KSFT_PASS}"
+}
+
test_vm_server_host_client() {
if ! vm_vsock_test "init_ns" "server" 2 "${TEST_GUEST_PORT}"; then
return "${KSFT_FAIL}"
@@ -569,6 +638,26 @@ test_vm_loopback() {
return "${KSFT_PASS}"
}
+test_ns_vm_local_mode_rejected() {
+ # Guest VMs have a G2H transport (virtio-vsock) active, so they
+ # should not be able to set namespace mode to 'local'.
+ # This test verifies that the sysctl write fails as expected.
+
+ # Try to set local mode in the guest's init_ns
+ if vm_ssh init_ns "echo local | tee /proc/sys/net/vsock/ns_mode &>/dev/null"; then
+ return "${KSFT_FAIL}"
+ fi
+
+ # Verify mode is still 'global'
+ local mode
+ mode=$(vm_ssh init_ns "cat /proc/sys/net/vsock/ns_mode")
+ if [[ "${mode}" != "global" ]]; then
+ return "${KSFT_FAIL}"
+ fi
+
+ return "${KSFT_PASS}"
+}
+
shared_vm_test() {
local tname
@@ -601,6 +690,11 @@ run_shared_vm_tests() {
continue
fi
+ if ! check_netns "${arg}"; then
+ check_result "${KSFT_SKIP}" "${arg}"
+ continue
+ fi
+
run_shared_vm_test "${arg}"
check_result "$?" "${arg}"
done
@@ -654,6 +748,49 @@ run_shared_vm_test() {
return "${rc}"
}
+run_ns_tests() {
+ for arg in "${ARGS[@]}"; do
+ if shared_vm_test "${arg}"; then
+ continue
+ fi
+
+ if ! check_netns "${arg}"; then
+ check_result "${KSFT_SKIP}" "${arg}"
+ continue
+ fi
+
+ add_namespaces
+
+ name=$(echo "${arg}" | awk '{ print $1 }')
+ log_host "Executing test_${name}"
+
+ host_oops_before=$(dmesg 2>/dev/null | grep -c -i 'Oops')
+ host_warn_before=$(dmesg --level=warn 2>/dev/null | grep -c -i 'vsock')
+ eval test_"${name}"
+ rc=$?
+
+ host_oops_after=$(dmesg 2>/dev/null | grep -c -i 'Oops')
+ if [[ "${host_oops_after}" -gt "${host_oops_before}" ]]; then
+ echo "FAIL: kernel oops detected on host" | log_host
+ check_result "${KSFT_FAIL}" "${name}"
+ del_namespaces
+ continue
+ fi
+
+ host_warn_after=$(dmesg --level=warn 2>/dev/null | grep -c -i 'vsock')
+ if [[ "${host_warn_after}" -gt "${host_warn_before}" ]]; then
+ echo "FAIL: kernel warning detected on host" | log_host
+ check_result "${KSFT_FAIL}" "${name}"
+ del_namespaces
+ continue
+ fi
+
+ check_result "${rc}" "${name}"
+
+ del_namespaces
+ done
+}
+
BUILD=0
QEMU="qemu-system-$(uname -m)"
@@ -699,6 +836,8 @@ if shared_vm_tests_requested "${ARGS[@]}"; then
terminate_pidfiles "${pidfile}"
fi
+run_ns_tests "${ARGS[@]}"
+
echo "SUMMARY: PASS=${cnt_pass} SKIP=${cnt_skip} FAIL=${cnt_fail}"
echo "Log: ${LOG}"
--
2.47.3
^ permalink raw reply related
* [PATCH net-next v11 09/13] selftests/vsock: use ss to wait for listeners instead of /proc/net
From: Bobby Eshleman @ 2025-11-21 5:44 UTC (permalink / raw)
To: Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Stefan Hajnoczi, Michael S. Tsirkin,
Jason Wang, Eugenio Pérez, Xuan Zhuo, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Bryan Tan, Vishnu Dasa,
Broadcom internal kernel review list, Shuah Khan
Cc: linux-kernel, virtualization, netdev, kvm, linux-hyperv,
linux-kselftest, berrange, Sargun Dhillon, Bobby Eshleman,
Bobby Eshleman
In-Reply-To: <20251120-vsock-vmtest-v11-0-55cbc80249a7@meta.com>
From: Bobby Eshleman <bobbyeshleman@meta.com>
Replace /proc/net parsing with ss(8) for detecting listening sockets in
wait_for_listener() functions and add support for TCP, VSOCK, and Unix
socket protocols.
The previous implementation parsed /proc/net/tcp using awk to detect
listening sockets, but this approach could not support vsock because
vsock does not export socket information to /proc/net/.
Instead, use ss so that we can detect listeners on tcp, vsock, and unix.
The protocol parameter is now required for all wait_for_listener family
functions (wait_for_listener, vm_wait_for_listener,
host_wait_for_listener) to explicitly specify which socket type to wait
for.
ss is added to the dependency check in check_deps().
Signed-off-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
tools/testing/selftests/vsock/vmtest.sh | 47 +++++++++++++++++++++------------
1 file changed, 30 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
index 1623e4da15e2..e32997db322d 100755
--- a/tools/testing/selftests/vsock/vmtest.sh
+++ b/tools/testing/selftests/vsock/vmtest.sh
@@ -191,7 +191,7 @@ check_args() {
}
check_deps() {
- for dep in vng ${QEMU} busybox pkill ssh; do
+ for dep in vng ${QEMU} busybox pkill ssh ss; do
if [[ ! -x $(command -v "${dep}") ]]; then
echo -e "skip: dependency ${dep} not found!\n"
exit "${KSFT_SKIP}"
@@ -346,21 +346,32 @@ wait_for_listener()
local port=$1
local interval=$2
local max_intervals=$3
- local protocol=tcp
- local pattern
+ local protocol=$4
local i
- pattern=":$(printf "%04X" "${port}") "
-
- # for tcp protocol additionally check the socket state
- [ "${protocol}" = "tcp" ] && pattern="${pattern}0A"
-
for i in $(seq "${max_intervals}"); do
- if awk -v pattern="${pattern}" \
- 'BEGIN {rc=1} $2" "$4 ~ pattern {rc=0} END {exit rc}' \
- /proc/net/"${protocol}"*; then
+ case "${protocol}" in
+ tcp)
+ if ss --listening --tcp --numeric | grep -q ":${port} "; then
+ break
+ fi
+ ;;
+ vsock)
+ if ss --listening --vsock --numeric | grep -q ":${port} "; then
+ break
+ fi
+ ;;
+ unix)
+ # For unix sockets, port is actually the socket path
+ if ss --listening --unix | grep -q "${port}"; then
+ break
+ fi
+ ;;
+ *)
+ echo "Unknown protocol: ${protocol}" >&2
break
- fi
+ ;;
+ esac
sleep "${interval}"
done
}
@@ -368,23 +379,25 @@ wait_for_listener()
vm_wait_for_listener() {
local ns=$1
local port=$2
+ local protocol=$3
vm_ssh "${ns}" <<EOF
$(declare -f wait_for_listener)
-wait_for_listener ${port} ${WAIT_PERIOD} ${WAIT_PERIOD_MAX}
+wait_for_listener ${port} ${WAIT_PERIOD} ${WAIT_PERIOD_MAX} ${protocol}
EOF
}
host_wait_for_listener() {
local ns=$1
local port=$2
+ local protocol=$3
if [[ "${ns}" == "init_ns" ]]; then
- wait_for_listener "${port}" "${WAIT_PERIOD}" "${WAIT_PERIOD_MAX}"
+ wait_for_listener "${port}" "${WAIT_PERIOD}" "${WAIT_PERIOD_MAX}" "${protocol}"
else
ip netns exec "${ns}" bash <<-EOF
$(declare -f wait_for_listener)
- wait_for_listener ${port} ${WAIT_PERIOD} ${WAIT_PERIOD_MAX}
+ wait_for_listener ${port} ${WAIT_PERIOD} ${WAIT_PERIOD_MAX} ${protocol}
EOF
fi
}
@@ -431,7 +444,7 @@ vm_vsock_test() {
return $rc
fi
- vm_wait_for_listener "${ns}" "${port}"
+ vm_wait_for_listener "${ns}" "${port}" "tcp"
rc=$?
fi
set +o pipefail
@@ -472,7 +485,7 @@ host_vsock_test() {
return $rc
fi
- host_wait_for_listener "${ns}" "${port}"
+ host_wait_for_listener "${ns}" "${port}" "tcp"
rc=$?
fi
set +o pipefail
--
2.47.3
^ permalink raw reply related
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