* Re: [PATCH net-next v9 4/4] tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present
From: Michael S. Tsirkin @ 2026-04-28 13:22 UTC (permalink / raw)
To: Simon Schippers
Cc: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, eperezma, leiyang, stephen, jon, tim.gebauer,
netdev, linux-kernel, kvm, virtualization
In-Reply-To: <cdc3cbc8-cf6a-437c-b34c-dd8ced65d7f8@tu-dortmund.de>
On Tue, Apr 28, 2026 at 03:10:44PM +0200, Simon Schippers wrote:
> On 4/28/26 14:50, Michael S. Tsirkin wrote:
> > On Tue, Apr 28, 2026 at 02:38:59PM +0200, Simon Schippers wrote:
> >> This commit prevents tail-drop when a qdisc is present and the ptr_ring
> >> becomes full. Once an entry is successfully produced and the ptr_ring
> >> reaches capacity, the netdev queue is stopped instead of dropping
> >> subsequent packets.
> >>
> >> If producing an entry fails anyways due to a race, tun_net_xmit returns
> >> NETDEV_TX_BUSY, again avoiding a drop. Such races are expected because
> >> LLTX is enabled and the transmit path operates without the usual locking.
> >>
> >> If no qdisc is present, the previous tail-drop behavior is preserved.
> >>
> >> The existing __tun_wake_queue() function of the consumer races with the
> >> producer for waking/stopping the netdev queue: the consumer may drain
> >> the ring just as the producer stops the queue, leading to a permanent
> >> stall. To avoid this, the producer re-checks the ring after stopping
> >> and wakes the queue itself if space was just made. An
> >> smp_mb__after_atomic() is required so the re-peek of the ring sees any
> >> drain that the consumer performed.
> >> smp_mb__after_atomic() pairs with the test_and_clear_bit() inside of
> >> netif_wake_subqueue():
> >>
> >> Consumer CPU Producer CPU
> >> ======================== =========================
> >> __ptr_ring_consume()
> >> netif_wake_subqueue() netif_tx_stop_queue()
> >> /\ smp_mb__after_atomic()
> >> || __ptr_ring_produce_peek()
> >> contains RMW operation
> >> test_and_clear_bit()
> >> /\
> >> ||
> >> "Fully ordered RMW:
> >> smp_mb() before + after"
> >> - atomic_t.txt
> >>
> >> Benchmarks:
> >> The benchmarks show a slight regression in raw transmission performance,
> >> though no packets are lost anymore.
> >
> > Could you include the packets received as well?
> > To demonstrate the gains/lack of loss.
> >
>
> Do you mean the number of packets received by the VM?
> They should just be the same as the number sent (shown below), right?
Minus the loss? Which this is about, right?
> I assume they would be visible as RX-DRP for TAP.
> For TAP + vhost-net I would have to rewrite the XDP drop
> program to count the number of dropped packets...
> And I would have to automate it...
>
> >>
> >> The previously introduced threshold to only wake after the queue stopped
> >> and half of the ring was consumed showed to be a descent choice:
> >> Waking the queue whenever a consume made space in the ring strongly
> >> degrades performance for tap, while waking only when the ring is empty
> >> is too late and also hurts throughput for tap & tap+vhost-net.
> >> Other ratios (3/4, 7/8) showed similar results (not shown here), so
> >> 1/2 was chosen for the sake of simplicity for both tun/tap and
> >> tun/tap+vhost-net.
> >>
> >> Test setup:
> >> AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU threads;
> >> Average over 50 runs @ 100,000,000 packets. SRSO and spectre v2
> >> mitigations disabled.
> >>
> >> Note for tap+vhost-net:
> >> XDP drop program active in VM -> ~2.5x faster, slower for tap due to
> >> more syscalls (high utilization of entry_SYSRETQ_unsafe_stack in perf)
> >>
> >> +--------------------------+--------------+----------------+----------+
> >> | 1 thread | Stock | Patched with | diff |
> >> | sending | | fq_codel qdisc | |
> >> +------------+-------------+--------------+----------------+----------+
> >> | TAP | Transmitted | 1.136 Mpps | 1.130 Mpps | -0.6% |
> >> | +-------------+--------------+----------------+----------+
> >> | | Lost/s | 3.758 Mpps | 0 pps | |
> >> +------------+-------------+--------------+----------------+----------+
> >> | TAP | Transmitted | 3.858 Mpps | 3.816 Mpps | -1.1% |
> >> | +-------------+--------------+----------------+----------+
> >> | +vhost-net | Lost/s | 789.8 Kpps | 0 pps | |
> >> +------------+-------------+--------------+----------------+----------+
> >>
> >> +--------------------------+--------------+----------------+----------+
> >> | 2 threads | Stock | Patched with | diff |
> >> | sending | | fq_codel qdisc | |
> >> +------------+-------------+--------------+----------------+----------+
> >> | TAP | Transmitted | 1.117 Mpps | 1.087 Mpps | -2.7% |
> >> | +-------------+--------------+----------------+----------+
> >> | | Lost/s | 8.476 Mpps | 0 pps | |
> >> +------------+-------------+--------------+----------------+----------+
> >> | TAP | Transmitted | 3.679 Mpps | 3.464 Mpps | -5.8% |
> >> | +-------------+--------------+----------------+----------+
> >> | +vhost-net | Lost/s | 5.306 Mpps | 0 pps | |
> >> +------------+-------------+--------------+----------------+----------+
> >>
> >> Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
> >> Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
> >> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
> >> ---
> >> drivers/net/tun.c | 30 ++++++++++++++++++++++++++++--
> >> 1 file changed, 28 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >> index efe809597622..c2a1618cc9db 100644
> >> --- a/drivers/net/tun.c
> >> +++ b/drivers/net/tun.c
> >> @@ -1011,6 +1011,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >> struct netdev_queue *queue;
> >> struct tun_file *tfile;
> >> int len = skb->len;
> >> + bool qdisc_present;
> >> + int ret;
> >>
> >> rcu_read_lock();
> >> tfile = rcu_dereference(tun->tfiles[txq]);
> >> @@ -1065,13 +1067,37 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >>
> >> nf_reset_ct(skb);
> >>
> >> - if (ptr_ring_produce(&tfile->tx_ring, skb)) {
> >> + queue = netdev_get_tx_queue(dev, txq);
> >> + qdisc_present = !qdisc_txq_has_no_queue(queue);
> >> +
> >> + spin_lock(&tfile->tx_ring.producer_lock);
> >> + ret = __ptr_ring_produce(&tfile->tx_ring, skb);
> >> + if (__ptr_ring_produce_peek(&tfile->tx_ring) && qdisc_present) {
> >> + netif_tx_stop_queue(queue);
> >> + /* Re-peek and wake if the consumer drained the ring
> >> + * concurrently in a race. smp_mb__after_atomic() pairs
> >> + * with the test_and_clear_bit() of netif_wake_subqueue()
> >> + * in __tun_wake_queue().
> >> + */
> >> + smp_mb__after_atomic();
> >> + if (!__ptr_ring_produce_peek(&tfile->tx_ring))
> >> + netif_tx_wake_queue(queue);
> >> + }
> >> + spin_unlock(&tfile->tx_ring.producer_lock);
> >> +
> >> + if (ret) {
> >> + /* If a qdisc is attached to our virtual device,
> >> + * returning NETDEV_TX_BUSY is allowed.
> >> + */
> >> + if (qdisc_present) {
> >> + rcu_read_unlock();
> >> + return NETDEV_TX_BUSY;
> >> + }
> >> drop_reason = SKB_DROP_REASON_FULL_RING;
> >> goto drop;
> >> }
> >>
> >> /* dev->lltx requires to do our own update of trans_start */
> >> - queue = netdev_get_tx_queue(dev, txq);
> >> txq_trans_cond_update(queue);
> >>
> >> /* Notify and wake up reader process */
> >> --
> >> 2.43.0
> >
^ permalink raw reply
* Re: [PATCH net-next v9 4/4] tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present
From: Simon Schippers @ 2026-04-28 13:41 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, eperezma, leiyang, stephen, jon, tim.gebauer,
netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20260428092150-mutt-send-email-mst@kernel.org>
On 4/28/26 15:22, Michael S. Tsirkin wrote:
> On Tue, Apr 28, 2026 at 03:10:44PM +0200, Simon Schippers wrote:
>> On 4/28/26 14:50, Michael S. Tsirkin wrote:
>>> On Tue, Apr 28, 2026 at 02:38:59PM +0200, Simon Schippers wrote:
>>>> This commit prevents tail-drop when a qdisc is present and the ptr_ring
>>>> becomes full. Once an entry is successfully produced and the ptr_ring
>>>> reaches capacity, the netdev queue is stopped instead of dropping
>>>> subsequent packets.
>>>>
>>>> If producing an entry fails anyways due to a race, tun_net_xmit returns
>>>> NETDEV_TX_BUSY, again avoiding a drop. Such races are expected because
>>>> LLTX is enabled and the transmit path operates without the usual locking.
>>>>
>>>> If no qdisc is present, the previous tail-drop behavior is preserved.
>>>>
>>>> The existing __tun_wake_queue() function of the consumer races with the
>>>> producer for waking/stopping the netdev queue: the consumer may drain
>>>> the ring just as the producer stops the queue, leading to a permanent
>>>> stall. To avoid this, the producer re-checks the ring after stopping
>>>> and wakes the queue itself if space was just made. An
>>>> smp_mb__after_atomic() is required so the re-peek of the ring sees any
>>>> drain that the consumer performed.
>>>> smp_mb__after_atomic() pairs with the test_and_clear_bit() inside of
>>>> netif_wake_subqueue():
>>>>
>>>> Consumer CPU Producer CPU
>>>> ======================== =========================
>>>> __ptr_ring_consume()
>>>> netif_wake_subqueue() netif_tx_stop_queue()
>>>> /\ smp_mb__after_atomic()
>>>> || __ptr_ring_produce_peek()
>>>> contains RMW operation
>>>> test_and_clear_bit()
>>>> /\
>>>> ||
>>>> "Fully ordered RMW:
>>>> smp_mb() before + after"
>>>> - atomic_t.txt
>>>>
>>>> Benchmarks:
>>>> The benchmarks show a slight regression in raw transmission performance,
>>>> though no packets are lost anymore.
>>>
>>> Could you include the packets received as well?
>>> To demonstrate the gains/lack of loss.
>>>
>>
>> Do you mean the number of packets received by the VM?
>> They should just be the same as the number sent (shown below), right?
>
> Minus the loss? Which this is about, right?
Yes. I simply calculated "Lost/s":
elapsed_time = 100e6 / sent_pps
Lost/s = total_errors / elapsed_time
To get back total_errors for example for TAP
1 thread sending:
elapsed_time = 100e6 / 1.136Mpps = 88s
3758 Mpps = total_errors / 88s
<=> total_errors = 331 million packets
So, out of 431 million packets sent, 100 million were successfully
delivered and 331 million were lost.
>
>> I assume they would be visible as RX-DRP for TAP.
>> For TAP + vhost-net I would have to rewrite the XDP drop
>> program to count the number of dropped packets...
>> And I would have to automate it...
>>
>>>>
>>>> The previously introduced threshold to only wake after the queue stopped
>>>> and half of the ring was consumed showed to be a descent choice:
>>>> Waking the queue whenever a consume made space in the ring strongly
>>>> degrades performance for tap, while waking only when the ring is empty
>>>> is too late and also hurts throughput for tap & tap+vhost-net.
>>>> Other ratios (3/4, 7/8) showed similar results (not shown here), so
>>>> 1/2 was chosen for the sake of simplicity for both tun/tap and
>>>> tun/tap+vhost-net.
>>>>
>>>> Test setup:
>>>> AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU threads;
>>>> Average over 50 runs @ 100,000,000 packets. SRSO and spectre v2
>>>> mitigations disabled.
>>>>
>>>> Note for tap+vhost-net:
>>>> XDP drop program active in VM -> ~2.5x faster, slower for tap due to
>>>> more syscalls (high utilization of entry_SYSRETQ_unsafe_stack in perf)
>>>>
>>>> +--------------------------+--------------+----------------+----------+
>>>> | 1 thread | Stock | Patched with | diff |
>>>> | sending | | fq_codel qdisc | |
>>>> +------------+-------------+--------------+----------------+----------+
>>>> | TAP | Transmitted | 1.136 Mpps | 1.130 Mpps | -0.6% |
>>>> | +-------------+--------------+----------------+----------+
>>>> | | Lost/s | 3.758 Mpps | 0 pps | |
>>>> +------------+-------------+--------------+----------------+----------+
>>>> | TAP | Transmitted | 3.858 Mpps | 3.816 Mpps | -1.1% |
>>>> | +-------------+--------------+----------------+----------+
>>>> | +vhost-net | Lost/s | 789.8 Kpps | 0 pps | |
>>>> +------------+-------------+--------------+----------------+----------+
>>>>
>>>> +--------------------------+--------------+----------------+----------+
>>>> | 2 threads | Stock | Patched with | diff |
>>>> | sending | | fq_codel qdisc | |
>>>> +------------+-------------+--------------+----------------+----------+
>>>> | TAP | Transmitted | 1.117 Mpps | 1.087 Mpps | -2.7% |
>>>> | +-------------+--------------+----------------+----------+
>>>> | | Lost/s | 8.476 Mpps | 0 pps | |
>>>> +------------+-------------+--------------+----------------+----------+
>>>> | TAP | Transmitted | 3.679 Mpps | 3.464 Mpps | -5.8% |
>>>> | +-------------+--------------+----------------+----------+
>>>> | +vhost-net | Lost/s | 5.306 Mpps | 0 pps | |
>>>> +------------+-------------+--------------+----------------+----------+
>>>>
>>>> Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
>>>> Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
>>>> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
>>>> ---
>>>> drivers/net/tun.c | 30 ++++++++++++++++++++++++++++--
>>>> 1 file changed, 28 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>>>> index efe809597622..c2a1618cc9db 100644
>>>> --- a/drivers/net/tun.c
>>>> +++ b/drivers/net/tun.c
>>>> @@ -1011,6 +1011,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>>>> struct netdev_queue *queue;
>>>> struct tun_file *tfile;
>>>> int len = skb->len;
>>>> + bool qdisc_present;
>>>> + int ret;
>>>>
>>>> rcu_read_lock();
>>>> tfile = rcu_dereference(tun->tfiles[txq]);
>>>> @@ -1065,13 +1067,37 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>>>>
>>>> nf_reset_ct(skb);
>>>>
>>>> - if (ptr_ring_produce(&tfile->tx_ring, skb)) {
>>>> + queue = netdev_get_tx_queue(dev, txq);
>>>> + qdisc_present = !qdisc_txq_has_no_queue(queue);
>>>> +
>>>> + spin_lock(&tfile->tx_ring.producer_lock);
>>>> + ret = __ptr_ring_produce(&tfile->tx_ring, skb);
>>>> + if (__ptr_ring_produce_peek(&tfile->tx_ring) && qdisc_present) {
>>>> + netif_tx_stop_queue(queue);
>>>> + /* Re-peek and wake if the consumer drained the ring
>>>> + * concurrently in a race. smp_mb__after_atomic() pairs
>>>> + * with the test_and_clear_bit() of netif_wake_subqueue()
>>>> + * in __tun_wake_queue().
>>>> + */
>>>> + smp_mb__after_atomic();
>>>> + if (!__ptr_ring_produce_peek(&tfile->tx_ring))
>>>> + netif_tx_wake_queue(queue);
>>>> + }
>>>> + spin_unlock(&tfile->tx_ring.producer_lock);
>>>> +
>>>> + if (ret) {
>>>> + /* If a qdisc is attached to our virtual device,
>>>> + * returning NETDEV_TX_BUSY is allowed.
>>>> + */
>>>> + if (qdisc_present) {
>>>> + rcu_read_unlock();
>>>> + return NETDEV_TX_BUSY;
>>>> + }
>>>> drop_reason = SKB_DROP_REASON_FULL_RING;
>>>> goto drop;
>>>> }
>>>>
>>>> /* dev->lltx requires to do our own update of trans_start */
>>>> - queue = netdev_get_tx_queue(dev, txq);
>>>> txq_trans_cond_update(queue);
>>>>
>>>> /* Notify and wake up reader process */
>>>> --
>>>> 2.43.0
>>>
>
^ permalink raw reply
* Re: [PATCH net-next v9 4/4] tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present
From: Michael S. Tsirkin @ 2026-04-28 14:10 UTC (permalink / raw)
To: Simon Schippers
Cc: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, eperezma, leiyang, stephen, jon, tim.gebauer,
netdev, linux-kernel, kvm, virtualization
In-Reply-To: <d00bea39-7748-46ad-9c31-27326041f03c@tu-dortmund.de>
On Tue, Apr 28, 2026 at 03:41:20PM +0200, Simon Schippers wrote:
> On 4/28/26 15:22, Michael S. Tsirkin wrote:
> > On Tue, Apr 28, 2026 at 03:10:44PM +0200, Simon Schippers wrote:
> >> On 4/28/26 14:50, Michael S. Tsirkin wrote:
> >>> On Tue, Apr 28, 2026 at 02:38:59PM +0200, Simon Schippers wrote:
> >>>> This commit prevents tail-drop when a qdisc is present and the ptr_ring
> >>>> becomes full. Once an entry is successfully produced and the ptr_ring
> >>>> reaches capacity, the netdev queue is stopped instead of dropping
> >>>> subsequent packets.
> >>>>
> >>>> If producing an entry fails anyways due to a race, tun_net_xmit returns
> >>>> NETDEV_TX_BUSY, again avoiding a drop. Such races are expected because
> >>>> LLTX is enabled and the transmit path operates without the usual locking.
> >>>>
> >>>> If no qdisc is present, the previous tail-drop behavior is preserved.
> >>>>
> >>>> The existing __tun_wake_queue() function of the consumer races with the
> >>>> producer for waking/stopping the netdev queue: the consumer may drain
> >>>> the ring just as the producer stops the queue, leading to a permanent
> >>>> stall. To avoid this, the producer re-checks the ring after stopping
> >>>> and wakes the queue itself if space was just made. An
> >>>> smp_mb__after_atomic() is required so the re-peek of the ring sees any
> >>>> drain that the consumer performed.
> >>>> smp_mb__after_atomic() pairs with the test_and_clear_bit() inside of
> >>>> netif_wake_subqueue():
> >>>>
> >>>> Consumer CPU Producer CPU
> >>>> ======================== =========================
> >>>> __ptr_ring_consume()
> >>>> netif_wake_subqueue() netif_tx_stop_queue()
> >>>> /\ smp_mb__after_atomic()
> >>>> || __ptr_ring_produce_peek()
> >>>> contains RMW operation
> >>>> test_and_clear_bit()
> >>>> /\
> >>>> ||
> >>>> "Fully ordered RMW:
> >>>> smp_mb() before + after"
> >>>> - atomic_t.txt
> >>>>
> >>>> Benchmarks:
> >>>> The benchmarks show a slight regression in raw transmission performance,
> >>>> though no packets are lost anymore.
> >>>
> >>> Could you include the packets received as well?
> >>> To demonstrate the gains/lack of loss.
> >>>
> >>
> >> Do you mean the number of packets received by the VM?
> >> They should just be the same as the number sent (shown below), right?
> >
> > Minus the loss? Which this is about, right?
>
> Yes. I simply calculated "Lost/s":
>
> elapsed_time = 100e6 / sent_pps
> Lost/s = total_errors / elapsed_time
>
>
> To get back total_errors for example for TAP
> 1 thread sending:
>
> elapsed_time = 100e6 / 1.136Mpps = 88s
>
> 3758 Mpps = total_errors / 88s
> <=> total_errors = 331 million packets
>
> So, out of 431 million packets sent, 100 million were successfully
> delivered and 331 million were lost.
That is my issue.
I kind of have trouble mapping that to the table below.
For example:
| TAP | Transmitted | 1.136 Mpps | 1.130 Mpps | -0.6% |
| +-------------+--------------+----------------+----------+
| | Lost/s | 3.758 Mpps | 0 pps | |
how can # of lost packets exceed the # of transmitted packets?
Thanks!
> >
> >> I assume they would be visible as RX-DRP for TAP.
> >> For TAP + vhost-net I would have to rewrite the XDP drop
> >> program to count the number of dropped packets...
> >> And I would have to automate it...
> >>
> >>>>
> >>>> The previously introduced threshold to only wake after the queue stopped
> >>>> and half of the ring was consumed showed to be a descent choice:
> >>>> Waking the queue whenever a consume made space in the ring strongly
> >>>> degrades performance for tap, while waking only when the ring is empty
> >>>> is too late and also hurts throughput for tap & tap+vhost-net.
> >>>> Other ratios (3/4, 7/8) showed similar results (not shown here), so
> >>>> 1/2 was chosen for the sake of simplicity for both tun/tap and
> >>>> tun/tap+vhost-net.
> >>>>
> >>>> Test setup:
> >>>> AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU threads;
> >>>> Average over 50 runs @ 100,000,000 packets. SRSO and spectre v2
> >>>> mitigations disabled.
> >>>>
> >>>> Note for tap+vhost-net:
> >>>> XDP drop program active in VM -> ~2.5x faster, slower for tap due to
> >>>> more syscalls (high utilization of entry_SYSRETQ_unsafe_stack in perf)
> >>>>
> >>>> +--------------------------+--------------+----------------+----------+
> >>>> | 1 thread | Stock | Patched with | diff |
> >>>> | sending | | fq_codel qdisc | |
> >>>> +------------+-------------+--------------+----------------+----------+
> >>>> | TAP | Transmitted | 1.136 Mpps | 1.130 Mpps | -0.6% |
> >>>> | +-------------+--------------+----------------+----------+
> >>>> | | Lost/s | 3.758 Mpps | 0 pps | |
> >>>> +------------+-------------+--------------+----------------+----------+
> >>>> | TAP | Transmitted | 3.858 Mpps | 3.816 Mpps | -1.1% |
> >>>> | +-------------+--------------+----------------+----------+
> >>>> | +vhost-net | Lost/s | 789.8 Kpps | 0 pps | |
> >>>> +------------+-------------+--------------+----------------+----------+
> >>>>
> >>>> +--------------------------+--------------+----------------+----------+
> >>>> | 2 threads | Stock | Patched with | diff |
> >>>> | sending | | fq_codel qdisc | |
> >>>> +------------+-------------+--------------+----------------+----------+
> >>>> | TAP | Transmitted | 1.117 Mpps | 1.087 Mpps | -2.7% |
> >>>> | +-------------+--------------+----------------+----------+
> >>>> | | Lost/s | 8.476 Mpps | 0 pps | |
> >>>> +------------+-------------+--------------+----------------+----------+
> >>>> | TAP | Transmitted | 3.679 Mpps | 3.464 Mpps | -5.8% |
> >>>> | +-------------+--------------+----------------+----------+
> >>>> | +vhost-net | Lost/s | 5.306 Mpps | 0 pps | |
> >>>> +------------+-------------+--------------+----------------+----------+
> >>>>
> >>>> Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
> >>>> Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
> >>>> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
> >>>> ---
> >>>> drivers/net/tun.c | 30 ++++++++++++++++++++++++++++--
> >>>> 1 file changed, 28 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >>>> index efe809597622..c2a1618cc9db 100644
> >>>> --- a/drivers/net/tun.c
> >>>> +++ b/drivers/net/tun.c
> >>>> @@ -1011,6 +1011,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >>>> struct netdev_queue *queue;
> >>>> struct tun_file *tfile;
> >>>> int len = skb->len;
> >>>> + bool qdisc_present;
> >>>> + int ret;
> >>>>
> >>>> rcu_read_lock();
> >>>> tfile = rcu_dereference(tun->tfiles[txq]);
> >>>> @@ -1065,13 +1067,37 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >>>>
> >>>> nf_reset_ct(skb);
> >>>>
> >>>> - if (ptr_ring_produce(&tfile->tx_ring, skb)) {
> >>>> + queue = netdev_get_tx_queue(dev, txq);
> >>>> + qdisc_present = !qdisc_txq_has_no_queue(queue);
> >>>> +
> >>>> + spin_lock(&tfile->tx_ring.producer_lock);
> >>>> + ret = __ptr_ring_produce(&tfile->tx_ring, skb);
> >>>> + if (__ptr_ring_produce_peek(&tfile->tx_ring) && qdisc_present) {
> >>>> + netif_tx_stop_queue(queue);
> >>>> + /* Re-peek and wake if the consumer drained the ring
> >>>> + * concurrently in a race. smp_mb__after_atomic() pairs
> >>>> + * with the test_and_clear_bit() of netif_wake_subqueue()
> >>>> + * in __tun_wake_queue().
> >>>> + */
> >>>> + smp_mb__after_atomic();
> >>>> + if (!__ptr_ring_produce_peek(&tfile->tx_ring))
> >>>> + netif_tx_wake_queue(queue);
> >>>> + }
> >>>> + spin_unlock(&tfile->tx_ring.producer_lock);
> >>>> +
> >>>> + if (ret) {
> >>>> + /* If a qdisc is attached to our virtual device,
> >>>> + * returning NETDEV_TX_BUSY is allowed.
> >>>> + */
> >>>> + if (qdisc_present) {
> >>>> + rcu_read_unlock();
> >>>> + return NETDEV_TX_BUSY;
> >>>> + }
> >>>> drop_reason = SKB_DROP_REASON_FULL_RING;
> >>>> goto drop;
> >>>> }
> >>>>
> >>>> /* dev->lltx requires to do our own update of trans_start */
> >>>> - queue = netdev_get_tx_queue(dev, txq);
> >>>> txq_trans_cond_update(queue);
> >>>>
> >>>> /* Notify and wake up reader process */
> >>>> --
> >>>> 2.43.0
> >>>
> >
^ permalink raw reply
* Re: [PATCH net-next v9 4/4] tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present
From: Simon Schippers @ 2026-04-28 14:18 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, eperezma, leiyang, stephen, jon, tim.gebauer,
netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20260428100731-mutt-send-email-mst@kernel.org>
On 4/28/26 16:10, Michael S. Tsirkin wrote:
> On Tue, Apr 28, 2026 at 03:41:20PM +0200, Simon Schippers wrote:
>> On 4/28/26 15:22, Michael S. Tsirkin wrote:
>>> On Tue, Apr 28, 2026 at 03:10:44PM +0200, Simon Schippers wrote:
>>>> On 4/28/26 14:50, Michael S. Tsirkin wrote:
>>>>> On Tue, Apr 28, 2026 at 02:38:59PM +0200, Simon Schippers wrote:
>>>>>> This commit prevents tail-drop when a qdisc is present and the ptr_ring
>>>>>> becomes full. Once an entry is successfully produced and the ptr_ring
>>>>>> reaches capacity, the netdev queue is stopped instead of dropping
>>>>>> subsequent packets.
>>>>>>
>>>>>> If producing an entry fails anyways due to a race, tun_net_xmit returns
>>>>>> NETDEV_TX_BUSY, again avoiding a drop. Such races are expected because
>>>>>> LLTX is enabled and the transmit path operates without the usual locking.
>>>>>>
>>>>>> If no qdisc is present, the previous tail-drop behavior is preserved.
>>>>>>
>>>>>> The existing __tun_wake_queue() function of the consumer races with the
>>>>>> producer for waking/stopping the netdev queue: the consumer may drain
>>>>>> the ring just as the producer stops the queue, leading to a permanent
>>>>>> stall. To avoid this, the producer re-checks the ring after stopping
>>>>>> and wakes the queue itself if space was just made. An
>>>>>> smp_mb__after_atomic() is required so the re-peek of the ring sees any
>>>>>> drain that the consumer performed.
>>>>>> smp_mb__after_atomic() pairs with the test_and_clear_bit() inside of
>>>>>> netif_wake_subqueue():
>>>>>>
>>>>>> Consumer CPU Producer CPU
>>>>>> ======================== =========================
>>>>>> __ptr_ring_consume()
>>>>>> netif_wake_subqueue() netif_tx_stop_queue()
>>>>>> /\ smp_mb__after_atomic()
>>>>>> || __ptr_ring_produce_peek()
>>>>>> contains RMW operation
>>>>>> test_and_clear_bit()
>>>>>> /\
>>>>>> ||
>>>>>> "Fully ordered RMW:
>>>>>> smp_mb() before + after"
>>>>>> - atomic_t.txt
>>>>>>
>>>>>> Benchmarks:
>>>>>> The benchmarks show a slight regression in raw transmission performance,
>>>>>> though no packets are lost anymore.
>>>>>
>>>>> Could you include the packets received as well?
>>>>> To demonstrate the gains/lack of loss.
>>>>>
>>>>
>>>> Do you mean the number of packets received by the VM?
>>>> They should just be the same as the number sent (shown below), right?
>>>
>>> Minus the loss? Which this is about, right?
>>
>> Yes. I simply calculated "Lost/s":
>>
>> elapsed_time = 100e6 / sent_pps
>> Lost/s = total_errors / elapsed_time
>>
>>
>> To get back total_errors for example for TAP
>> 1 thread sending:
>>
>> elapsed_time = 100e6 / 1.136Mpps = 88s
>>
>> 3758 Mpps = total_errors / 88s
>> <=> total_errors = 331 million packets
>>
>> So, out of 431 million packets sent, 100 million were successfully
>> delivered and 331 million were lost.
>
> That is my issue.
>
> I kind of have trouble mapping that to the table below.
> For example:
>
> | TAP | Transmitted | 1.136 Mpps | 1.130 Mpps | -0.6% |
> | +-------------+--------------+----------------+----------+
> | | Lost/s | 3.758 Mpps | 0 pps | |
>
> how can # of lost packets exceed the # of transmitted packets?
>
> Thanks!
I just do use the sample script [1]:
./pktgen_sample02_multiqueue.sh -n 100000000 ...
... and this runs until 100_000_000 packets were sucessfully
transmitted, independently of the lost packets/errors.
[1] Link: https://www.kernel.org/doc/html/latest/networking/pktgen.html#sample-scripts
>
>
>>>
>>>> I assume they would be visible as RX-DRP for TAP.
>>>> For TAP + vhost-net I would have to rewrite the XDP drop
>>>> program to count the number of dropped packets...
>>>> And I would have to automate it...
>>>>
>>>>>>
>>>>>> The previously introduced threshold to only wake after the queue stopped
>>>>>> and half of the ring was consumed showed to be a descent choice:
>>>>>> Waking the queue whenever a consume made space in the ring strongly
>>>>>> degrades performance for tap, while waking only when the ring is empty
>>>>>> is too late and also hurts throughput for tap & tap+vhost-net.
>>>>>> Other ratios (3/4, 7/8) showed similar results (not shown here), so
>>>>>> 1/2 was chosen for the sake of simplicity for both tun/tap and
>>>>>> tun/tap+vhost-net.
>>>>>>
>>>>>> Test setup:
>>>>>> AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU threads;
>>>>>> Average over 50 runs @ 100,000,000 packets. SRSO and spectre v2
>>>>>> mitigations disabled.
>>>>>>
>>>>>> Note for tap+vhost-net:
>>>>>> XDP drop program active in VM -> ~2.5x faster, slower for tap due to
>>>>>> more syscalls (high utilization of entry_SYSRETQ_unsafe_stack in perf)
>>>>>>
>>>>>> +--------------------------+--------------+----------------+----------+
>>>>>> | 1 thread | Stock | Patched with | diff |
>>>>>> | sending | | fq_codel qdisc | |
>>>>>> +------------+-------------+--------------+----------------+----------+
>>>>>> | TAP | Transmitted | 1.136 Mpps | 1.130 Mpps | -0.6% |
>>>>>> | +-------------+--------------+----------------+----------+
>>>>>> | | Lost/s | 3.758 Mpps | 0 pps | |
>>>>>> +------------+-------------+--------------+----------------+----------+
>>>>>> | TAP | Transmitted | 3.858 Mpps | 3.816 Mpps | -1.1% |
>>>>>> | +-------------+--------------+----------------+----------+
>>>>>> | +vhost-net | Lost/s | 789.8 Kpps | 0 pps | |
>>>>>> +------------+-------------+--------------+----------------+----------+
>>>>>>
>>>>>> +--------------------------+--------------+----------------+----------+
>>>>>> | 2 threads | Stock | Patched with | diff |
>>>>>> | sending | | fq_codel qdisc | |
>>>>>> +------------+-------------+--------------+----------------+----------+
>>>>>> | TAP | Transmitted | 1.117 Mpps | 1.087 Mpps | -2.7% |
>>>>>> | +-------------+--------------+----------------+----------+
>>>>>> | | Lost/s | 8.476 Mpps | 0 pps | |
>>>>>> +------------+-------------+--------------+----------------+----------+
>>>>>> | TAP | Transmitted | 3.679 Mpps | 3.464 Mpps | -5.8% |
>>>>>> | +-------------+--------------+----------------+----------+
>>>>>> | +vhost-net | Lost/s | 5.306 Mpps | 0 pps | |
>>>>>> +------------+-------------+--------------+----------------+----------+
>>>>>>
>>>>>> Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
>>>>>> Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
>>>>>> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
>>>>>> ---
>>>>>> drivers/net/tun.c | 30 ++++++++++++++++++++++++++++--
>>>>>> 1 file changed, 28 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>>>>>> index efe809597622..c2a1618cc9db 100644
>>>>>> --- a/drivers/net/tun.c
>>>>>> +++ b/drivers/net/tun.c
>>>>>> @@ -1011,6 +1011,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>>>>>> struct netdev_queue *queue;
>>>>>> struct tun_file *tfile;
>>>>>> int len = skb->len;
>>>>>> + bool qdisc_present;
>>>>>> + int ret;
>>>>>>
>>>>>> rcu_read_lock();
>>>>>> tfile = rcu_dereference(tun->tfiles[txq]);
>>>>>> @@ -1065,13 +1067,37 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>>>>>>
>>>>>> nf_reset_ct(skb);
>>>>>>
>>>>>> - if (ptr_ring_produce(&tfile->tx_ring, skb)) {
>>>>>> + queue = netdev_get_tx_queue(dev, txq);
>>>>>> + qdisc_present = !qdisc_txq_has_no_queue(queue);
>>>>>> +
>>>>>> + spin_lock(&tfile->tx_ring.producer_lock);
>>>>>> + ret = __ptr_ring_produce(&tfile->tx_ring, skb);
>>>>>> + if (__ptr_ring_produce_peek(&tfile->tx_ring) && qdisc_present) {
>>>>>> + netif_tx_stop_queue(queue);
>>>>>> + /* Re-peek and wake if the consumer drained the ring
>>>>>> + * concurrently in a race. smp_mb__after_atomic() pairs
>>>>>> + * with the test_and_clear_bit() of netif_wake_subqueue()
>>>>>> + * in __tun_wake_queue().
>>>>>> + */
>>>>>> + smp_mb__after_atomic();
>>>>>> + if (!__ptr_ring_produce_peek(&tfile->tx_ring))
>>>>>> + netif_tx_wake_queue(queue);
>>>>>> + }
>>>>>> + spin_unlock(&tfile->tx_ring.producer_lock);
>>>>>> +
>>>>>> + if (ret) {
>>>>>> + /* If a qdisc is attached to our virtual device,
>>>>>> + * returning NETDEV_TX_BUSY is allowed.
>>>>>> + */
>>>>>> + if (qdisc_present) {
>>>>>> + rcu_read_unlock();
>>>>>> + return NETDEV_TX_BUSY;
>>>>>> + }
>>>>>> drop_reason = SKB_DROP_REASON_FULL_RING;
>>>>>> goto drop;
>>>>>> }
>>>>>>
>>>>>> /* dev->lltx requires to do our own update of trans_start */
>>>>>> - queue = netdev_get_tx_queue(dev, txq);
>>>>>> txq_trans_cond_update(queue);
>>>>>>
>>>>>> /* Notify and wake up reader process */
>>>>>> --
>>>>>> 2.43.0
>>>>>
>>>
>
^ permalink raw reply
* Re: [PATCH net-next v9 4/4] tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present
From: Michael S. Tsirkin @ 2026-04-28 14:32 UTC (permalink / raw)
To: Simon Schippers
Cc: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, eperezma, leiyang, stephen, jon, tim.gebauer,
netdev, linux-kernel, kvm, virtualization
In-Reply-To: <f4274173-23ef-43c3-aab4-b64678b440ec@tu-dortmund.de>
On Tue, Apr 28, 2026 at 04:18:54PM +0200, Simon Schippers wrote:
> On 4/28/26 16:10, Michael S. Tsirkin wrote:
> > On Tue, Apr 28, 2026 at 03:41:20PM +0200, Simon Schippers wrote:
> >> On 4/28/26 15:22, Michael S. Tsirkin wrote:
> >>> On Tue, Apr 28, 2026 at 03:10:44PM +0200, Simon Schippers wrote:
> >>>> On 4/28/26 14:50, Michael S. Tsirkin wrote:
> >>>>> On Tue, Apr 28, 2026 at 02:38:59PM +0200, Simon Schippers wrote:
> >>>>>> This commit prevents tail-drop when a qdisc is present and the ptr_ring
> >>>>>> becomes full. Once an entry is successfully produced and the ptr_ring
> >>>>>> reaches capacity, the netdev queue is stopped instead of dropping
> >>>>>> subsequent packets.
> >>>>>>
> >>>>>> If producing an entry fails anyways due to a race, tun_net_xmit returns
> >>>>>> NETDEV_TX_BUSY, again avoiding a drop. Such races are expected because
> >>>>>> LLTX is enabled and the transmit path operates without the usual locking.
> >>>>>>
> >>>>>> If no qdisc is present, the previous tail-drop behavior is preserved.
> >>>>>>
> >>>>>> The existing __tun_wake_queue() function of the consumer races with the
> >>>>>> producer for waking/stopping the netdev queue: the consumer may drain
> >>>>>> the ring just as the producer stops the queue, leading to a permanent
> >>>>>> stall. To avoid this, the producer re-checks the ring after stopping
> >>>>>> and wakes the queue itself if space was just made. An
> >>>>>> smp_mb__after_atomic() is required so the re-peek of the ring sees any
> >>>>>> drain that the consumer performed.
> >>>>>> smp_mb__after_atomic() pairs with the test_and_clear_bit() inside of
> >>>>>> netif_wake_subqueue():
> >>>>>>
> >>>>>> Consumer CPU Producer CPU
> >>>>>> ======================== =========================
> >>>>>> __ptr_ring_consume()
> >>>>>> netif_wake_subqueue() netif_tx_stop_queue()
> >>>>>> /\ smp_mb__after_atomic()
> >>>>>> || __ptr_ring_produce_peek()
> >>>>>> contains RMW operation
> >>>>>> test_and_clear_bit()
> >>>>>> /\
> >>>>>> ||
> >>>>>> "Fully ordered RMW:
> >>>>>> smp_mb() before + after"
> >>>>>> - atomic_t.txt
> >>>>>>
> >>>>>> Benchmarks:
> >>>>>> The benchmarks show a slight regression in raw transmission performance,
> >>>>>> though no packets are lost anymore.
> >>>>>
> >>>>> Could you include the packets received as well?
> >>>>> To demonstrate the gains/lack of loss.
> >>>>>
> >>>>
> >>>> Do you mean the number of packets received by the VM?
> >>>> They should just be the same as the number sent (shown below), right?
> >>>
> >>> Minus the loss? Which this is about, right?
> >>
> >> Yes. I simply calculated "Lost/s":
> >>
> >> elapsed_time = 100e6 / sent_pps
> >> Lost/s = total_errors / elapsed_time
> >>
> >>
> >> To get back total_errors for example for TAP
> >> 1 thread sending:
> >>
> >> elapsed_time = 100e6 / 1.136Mpps = 88s
> >>
> >> 3758 Mpps = total_errors / 88s
> >> <=> total_errors = 331 million packets
> >>
> >> So, out of 431 million packets sent, 100 million were successfully
> >> delivered and 331 million were lost.
> >
> > That is my issue.
> >
> > I kind of have trouble mapping that to the table below.
> > For example:
> >
> > | TAP | Transmitted | 1.136 Mpps | 1.130 Mpps | -0.6% |
> > | +-------------+--------------+----------------+----------+
> > | | Lost/s | 3.758 Mpps | 0 pps | |
> >
> > how can # of lost packets exceed the # of transmitted packets?
> >
> > Thanks!
>
> I just do use the sample script [1]:
>
> ./pktgen_sample02_multiqueue.sh -n 100000000 ...
>
> ... and this runs until 100_000_000 packets were sucessfully
> transmitted, independently of the lost packets/errors.
>
> [1] Link: https://www.kernel.org/doc/html/latest/networking/pktgen.html#sample-scripts
Confused. Are you saying "transmitted" is actually "received"? And the #
of packets sent is Transmitted + Lost?
> >
> >
> >>>
> >>>> I assume they would be visible as RX-DRP for TAP.
> >>>> For TAP + vhost-net I would have to rewrite the XDP drop
> >>>> program to count the number of dropped packets...
> >>>> And I would have to automate it...
> >>>>
> >>>>>>
> >>>>>> The previously introduced threshold to only wake after the queue stopped
> >>>>>> and half of the ring was consumed showed to be a descent choice:
> >>>>>> Waking the queue whenever a consume made space in the ring strongly
> >>>>>> degrades performance for tap, while waking only when the ring is empty
> >>>>>> is too late and also hurts throughput for tap & tap+vhost-net.
> >>>>>> Other ratios (3/4, 7/8) showed similar results (not shown here), so
> >>>>>> 1/2 was chosen for the sake of simplicity for both tun/tap and
> >>>>>> tun/tap+vhost-net.
> >>>>>>
> >>>>>> Test setup:
> >>>>>> AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU threads;
> >>>>>> Average over 50 runs @ 100,000,000 packets. SRSO and spectre v2
> >>>>>> mitigations disabled.
> >>>>>>
> >>>>>> Note for tap+vhost-net:
> >>>>>> XDP drop program active in VM -> ~2.5x faster, slower for tap due to
> >>>>>> more syscalls (high utilization of entry_SYSRETQ_unsafe_stack in perf)
> >>>>>>
> >>>>>> +--------------------------+--------------+----------------+----------+
> >>>>>> | 1 thread | Stock | Patched with | diff |
> >>>>>> | sending | | fq_codel qdisc | |
> >>>>>> +------------+-------------+--------------+----------------+----------+
> >>>>>> | TAP | Transmitted | 1.136 Mpps | 1.130 Mpps | -0.6% |
> >>>>>> | +-------------+--------------+----------------+----------+
> >>>>>> | | Lost/s | 3.758 Mpps | 0 pps | |
> >>>>>> +------------+-------------+--------------+----------------+----------+
> >>>>>> | TAP | Transmitted | 3.858 Mpps | 3.816 Mpps | -1.1% |
> >>>>>> | +-------------+--------------+----------------+----------+
> >>>>>> | +vhost-net | Lost/s | 789.8 Kpps | 0 pps | |
> >>>>>> +------------+-------------+--------------+----------------+----------+
> >>>>>>
> >>>>>> +--------------------------+--------------+----------------+----------+
> >>>>>> | 2 threads | Stock | Patched with | diff |
> >>>>>> | sending | | fq_codel qdisc | |
> >>>>>> +------------+-------------+--------------+----------------+----------+
> >>>>>> | TAP | Transmitted | 1.117 Mpps | 1.087 Mpps | -2.7% |
> >>>>>> | +-------------+--------------+----------------+----------+
> >>>>>> | | Lost/s | 8.476 Mpps | 0 pps | |
> >>>>>> +------------+-------------+--------------+----------------+----------+
> >>>>>> | TAP | Transmitted | 3.679 Mpps | 3.464 Mpps | -5.8% |
> >>>>>> | +-------------+--------------+----------------+----------+
> >>>>>> | +vhost-net | Lost/s | 5.306 Mpps | 0 pps | |
> >>>>>> +------------+-------------+--------------+----------------+----------+
> >>>>>>
> >>>>>> Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
> >>>>>> Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
> >>>>>> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
> >>>>>> ---
> >>>>>> drivers/net/tun.c | 30 ++++++++++++++++++++++++++++--
> >>>>>> 1 file changed, 28 insertions(+), 2 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >>>>>> index efe809597622..c2a1618cc9db 100644
> >>>>>> --- a/drivers/net/tun.c
> >>>>>> +++ b/drivers/net/tun.c
> >>>>>> @@ -1011,6 +1011,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >>>>>> struct netdev_queue *queue;
> >>>>>> struct tun_file *tfile;
> >>>>>> int len = skb->len;
> >>>>>> + bool qdisc_present;
> >>>>>> + int ret;
> >>>>>>
> >>>>>> rcu_read_lock();
> >>>>>> tfile = rcu_dereference(tun->tfiles[txq]);
> >>>>>> @@ -1065,13 +1067,37 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >>>>>>
> >>>>>> nf_reset_ct(skb);
> >>>>>>
> >>>>>> - if (ptr_ring_produce(&tfile->tx_ring, skb)) {
> >>>>>> + queue = netdev_get_tx_queue(dev, txq);
> >>>>>> + qdisc_present = !qdisc_txq_has_no_queue(queue);
> >>>>>> +
> >>>>>> + spin_lock(&tfile->tx_ring.producer_lock);
> >>>>>> + ret = __ptr_ring_produce(&tfile->tx_ring, skb);
> >>>>>> + if (__ptr_ring_produce_peek(&tfile->tx_ring) && qdisc_present) {
> >>>>>> + netif_tx_stop_queue(queue);
> >>>>>> + /* Re-peek and wake if the consumer drained the ring
> >>>>>> + * concurrently in a race. smp_mb__after_atomic() pairs
> >>>>>> + * with the test_and_clear_bit() of netif_wake_subqueue()
> >>>>>> + * in __tun_wake_queue().
> >>>>>> + */
> >>>>>> + smp_mb__after_atomic();
> >>>>>> + if (!__ptr_ring_produce_peek(&tfile->tx_ring))
> >>>>>> + netif_tx_wake_queue(queue);
> >>>>>> + }
> >>>>>> + spin_unlock(&tfile->tx_ring.producer_lock);
> >>>>>> +
> >>>>>> + if (ret) {
> >>>>>> + /* If a qdisc is attached to our virtual device,
> >>>>>> + * returning NETDEV_TX_BUSY is allowed.
> >>>>>> + */
> >>>>>> + if (qdisc_present) {
> >>>>>> + rcu_read_unlock();
> >>>>>> + return NETDEV_TX_BUSY;
> >>>>>> + }
> >>>>>> drop_reason = SKB_DROP_REASON_FULL_RING;
> >>>>>> goto drop;
> >>>>>> }
> >>>>>>
> >>>>>> /* dev->lltx requires to do our own update of trans_start */
> >>>>>> - queue = netdev_get_tx_queue(dev, txq);
> >>>>>> txq_trans_cond_update(queue);
> >>>>>>
> >>>>>> /* Notify and wake up reader process */
> >>>>>> --
> >>>>>> 2.43.0
> >>>>>
> >>>
> >
^ permalink raw reply
* Re: [PATCH net-next v9 4/4] tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present
From: Simon Schippers @ 2026-04-28 14:55 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: willemdebruijn.kernel, jasowang, andrew+netdev, davem, edumazet,
kuba, pabeni, eperezma, leiyang, stephen, jon, tim.gebauer,
netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20260428103108-mutt-send-email-mst@kernel.org>
On 4/28/26 16:32, Michael S. Tsirkin wrote:
> On Tue, Apr 28, 2026 at 04:18:54PM +0200, Simon Schippers wrote:
>> On 4/28/26 16:10, Michael S. Tsirkin wrote:
>>> On Tue, Apr 28, 2026 at 03:41:20PM +0200, Simon Schippers wrote:
>>>> On 4/28/26 15:22, Michael S. Tsirkin wrote:
>>>>> On Tue, Apr 28, 2026 at 03:10:44PM +0200, Simon Schippers wrote:
>>>>>> On 4/28/26 14:50, Michael S. Tsirkin wrote:
>>>>>>> On Tue, Apr 28, 2026 at 02:38:59PM +0200, Simon Schippers wrote:
>>>>>>>> This commit prevents tail-drop when a qdisc is present and the ptr_ring
>>>>>>>> becomes full. Once an entry is successfully produced and the ptr_ring
>>>>>>>> reaches capacity, the netdev queue is stopped instead of dropping
>>>>>>>> subsequent packets.
>>>>>>>>
>>>>>>>> If producing an entry fails anyways due to a race, tun_net_xmit returns
>>>>>>>> NETDEV_TX_BUSY, again avoiding a drop. Such races are expected because
>>>>>>>> LLTX is enabled and the transmit path operates without the usual locking.
>>>>>>>>
>>>>>>>> If no qdisc is present, the previous tail-drop behavior is preserved.
>>>>>>>>
>>>>>>>> The existing __tun_wake_queue() function of the consumer races with the
>>>>>>>> producer for waking/stopping the netdev queue: the consumer may drain
>>>>>>>> the ring just as the producer stops the queue, leading to a permanent
>>>>>>>> stall. To avoid this, the producer re-checks the ring after stopping
>>>>>>>> and wakes the queue itself if space was just made. An
>>>>>>>> smp_mb__after_atomic() is required so the re-peek of the ring sees any
>>>>>>>> drain that the consumer performed.
>>>>>>>> smp_mb__after_atomic() pairs with the test_and_clear_bit() inside of
>>>>>>>> netif_wake_subqueue():
>>>>>>>>
>>>>>>>> Consumer CPU Producer CPU
>>>>>>>> ======================== =========================
>>>>>>>> __ptr_ring_consume()
>>>>>>>> netif_wake_subqueue() netif_tx_stop_queue()
>>>>>>>> /\ smp_mb__after_atomic()
>>>>>>>> || __ptr_ring_produce_peek()
>>>>>>>> contains RMW operation
>>>>>>>> test_and_clear_bit()
>>>>>>>> /\
>>>>>>>> ||
>>>>>>>> "Fully ordered RMW:
>>>>>>>> smp_mb() before + after"
>>>>>>>> - atomic_t.txt
>>>>>>>>
>>>>>>>> Benchmarks:
>>>>>>>> The benchmarks show a slight regression in raw transmission performance,
>>>>>>>> though no packets are lost anymore.
>>>>>>>
>>>>>>> Could you include the packets received as well?
>>>>>>> To demonstrate the gains/lack of loss.
>>>>>>>
>>>>>>
>>>>>> Do you mean the number of packets received by the VM?
>>>>>> They should just be the same as the number sent (shown below), right?
>>>>>
>>>>> Minus the loss? Which this is about, right?
>>>>
>>>> Yes. I simply calculated "Lost/s":
>>>>
>>>> elapsed_time = 100e6 / sent_pps
>>>> Lost/s = total_errors / elapsed_time
>>>>
>>>>
>>>> To get back total_errors for example for TAP
>>>> 1 thread sending:
>>>>
>>>> elapsed_time = 100e6 / 1.136Mpps = 88s
>>>>
>>>> 3758 Mpps = total_errors / 88s
>>>> <=> total_errors = 331 million packets
>>>>
>>>> So, out of 431 million packets sent, 100 million were successfully
>>>> delivered and 331 million were lost.
>>>
>>> That is my issue.
>>>
>>> I kind of have trouble mapping that to the table below.
>>> For example:
>>>
>>> | TAP | Transmitted | 1.136 Mpps | 1.130 Mpps | -0.6% |
>>> | +-------------+--------------+----------------+----------+
>>> | | Lost/s | 3.758 Mpps | 0 pps | |
>>>
>>> how can # of lost packets exceed the # of transmitted packets?
>>>
>>> Thanks!
>>
>> I just do use the sample script [1]:
>>
>> ./pktgen_sample02_multiqueue.sh -n 100000000 ...
>>
>> ... and this runs until 100_000_000 packets were sucessfully
>> transmitted, independently of the lost packets/errors.
>>
>> [1] Link: https://www.kernel.org/doc/html/latest/networking/pktgen.html#sample-scripts
>
> Confused. Are you saying "transmitted" is actually "received"? And the #
> of packets sent is Transmitted + Lost?
Sorry for my confusing answer.
Yes, "transmitted" in the table should be changed to "received".
And yes, as you said, the real # transmitted then is:
Received + Lost = 1.136 + 3.758 = 4.894 Mpps.
>
>>>
>>>
>>>>>
>>>>>> I assume they would be visible as RX-DRP for TAP.
>>>>>> For TAP + vhost-net I would have to rewrite the XDP drop
>>>>>> program to count the number of dropped packets...
>>>>>> And I would have to automate it...
>>>>>>
>>>>>>>>
>>>>>>>> The previously introduced threshold to only wake after the queue stopped
>>>>>>>> and half of the ring was consumed showed to be a descent choice:
>>>>>>>> Waking the queue whenever a consume made space in the ring strongly
>>>>>>>> degrades performance for tap, while waking only when the ring is empty
>>>>>>>> is too late and also hurts throughput for tap & tap+vhost-net.
>>>>>>>> Other ratios (3/4, 7/8) showed similar results (not shown here), so
>>>>>>>> 1/2 was chosen for the sake of simplicity for both tun/tap and
>>>>>>>> tun/tap+vhost-net.
>>>>>>>>
>>>>>>>> Test setup:
>>>>>>>> AMD Ryzen 5 5600X at 4.3 GHz, 3200 MHz RAM, isolated QEMU threads;
>>>>>>>> Average over 50 runs @ 100,000,000 packets. SRSO and spectre v2
>>>>>>>> mitigations disabled.
>>>>>>>>
>>>>>>>> Note for tap+vhost-net:
>>>>>>>> XDP drop program active in VM -> ~2.5x faster, slower for tap due to
>>>>>>>> more syscalls (high utilization of entry_SYSRETQ_unsafe_stack in perf)
>>>>>>>>
>>>>>>>> +--------------------------+--------------+----------------+----------+
>>>>>>>> | 1 thread | Stock | Patched with | diff |
>>>>>>>> | sending | | fq_codel qdisc | |
>>>>>>>> +------------+-------------+--------------+----------------+----------+
>>>>>>>> | TAP | Transmitted | 1.136 Mpps | 1.130 Mpps | -0.6% |
>>>>>>>> | +-------------+--------------+----------------+----------+
>>>>>>>> | | Lost/s | 3.758 Mpps | 0 pps | |
>>>>>>>> +------------+-------------+--------------+----------------+----------+
>>>>>>>> | TAP | Transmitted | 3.858 Mpps | 3.816 Mpps | -1.1% |
>>>>>>>> | +-------------+--------------+----------------+----------+
>>>>>>>> | +vhost-net | Lost/s | 789.8 Kpps | 0 pps | |
>>>>>>>> +------------+-------------+--------------+----------------+----------+
>>>>>>>>
>>>>>>>> +--------------------------+--------------+----------------+----------+
>>>>>>>> | 2 threads | Stock | Patched with | diff |
>>>>>>>> | sending | | fq_codel qdisc | |
>>>>>>>> +------------+-------------+--------------+----------------+----------+
>>>>>>>> | TAP | Transmitted | 1.117 Mpps | 1.087 Mpps | -2.7% |
>>>>>>>> | +-------------+--------------+----------------+----------+
>>>>>>>> | | Lost/s | 8.476 Mpps | 0 pps | |
>>>>>>>> +------------+-------------+--------------+----------------+----------+
>>>>>>>> | TAP | Transmitted | 3.679 Mpps | 3.464 Mpps | -5.8% |
>>>>>>>> | +-------------+--------------+----------------+----------+
>>>>>>>> | +vhost-net | Lost/s | 5.306 Mpps | 0 pps | |
>>>>>>>> +------------+-------------+--------------+----------------+----------+
>>>>>>>>
>>>>>>>> Co-developed-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
>>>>>>>> Signed-off-by: Tim Gebauer <tim.gebauer@tu-dortmund.de>
>>>>>>>> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>
>>>>>>>> ---
>>>>>>>> drivers/net/tun.c | 30 ++++++++++++++++++++++++++++--
>>>>>>>> 1 file changed, 28 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>>>>>>>> index efe809597622..c2a1618cc9db 100644
>>>>>>>> --- a/drivers/net/tun.c
>>>>>>>> +++ b/drivers/net/tun.c
>>>>>>>> @@ -1011,6 +1011,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>>>>>>>> struct netdev_queue *queue;
>>>>>>>> struct tun_file *tfile;
>>>>>>>> int len = skb->len;
>>>>>>>> + bool qdisc_present;
>>>>>>>> + int ret;
>>>>>>>>
>>>>>>>> rcu_read_lock();
>>>>>>>> tfile = rcu_dereference(tun->tfiles[txq]);
>>>>>>>> @@ -1065,13 +1067,37 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>>>>>>>>
>>>>>>>> nf_reset_ct(skb);
>>>>>>>>
>>>>>>>> - if (ptr_ring_produce(&tfile->tx_ring, skb)) {
>>>>>>>> + queue = netdev_get_tx_queue(dev, txq);
>>>>>>>> + qdisc_present = !qdisc_txq_has_no_queue(queue);
>>>>>>>> +
>>>>>>>> + spin_lock(&tfile->tx_ring.producer_lock);
>>>>>>>> + ret = __ptr_ring_produce(&tfile->tx_ring, skb);
>>>>>>>> + if (__ptr_ring_produce_peek(&tfile->tx_ring) && qdisc_present) {
>>>>>>>> + netif_tx_stop_queue(queue);
>>>>>>>> + /* Re-peek and wake if the consumer drained the ring
>>>>>>>> + * concurrently in a race. smp_mb__after_atomic() pairs
>>>>>>>> + * with the test_and_clear_bit() of netif_wake_subqueue()
>>>>>>>> + * in __tun_wake_queue().
>>>>>>>> + */
>>>>>>>> + smp_mb__after_atomic();
>>>>>>>> + if (!__ptr_ring_produce_peek(&tfile->tx_ring))
>>>>>>>> + netif_tx_wake_queue(queue);
>>>>>>>> + }
>>>>>>>> + spin_unlock(&tfile->tx_ring.producer_lock);
>>>>>>>> +
>>>>>>>> + if (ret) {
>>>>>>>> + /* If a qdisc is attached to our virtual device,
>>>>>>>> + * returning NETDEV_TX_BUSY is allowed.
>>>>>>>> + */
>>>>>>>> + if (qdisc_present) {
>>>>>>>> + rcu_read_unlock();
>>>>>>>> + return NETDEV_TX_BUSY;
>>>>>>>> + }
>>>>>>>> drop_reason = SKB_DROP_REASON_FULL_RING;
>>>>>>>> goto drop;
>>>>>>>> }
>>>>>>>>
>>>>>>>> /* dev->lltx requires to do our own update of trans_start */
>>>>>>>> - queue = netdev_get_tx_queue(dev, txq);
>>>>>>>> txq_trans_cond_update(queue);
>>>>>>>>
>>>>>>>> /* Notify and wake up reader process */
>>>>>>>> --
>>>>>>>> 2.43.0
>>>>>>>
>>>>>
>>>
>
^ permalink raw reply
* [RFC PATCH v7 0/4] virtio: add noirq system sleep PM callbacks for virtio-mmio
From: Sungho Bae @ 2026-04-28 15:07 UTC (permalink / raw)
To: mst, jasowang
Cc: xuanzhuo, eperezma, virtualization, linux-kernel, Sungho Bae
From: Sungho Bae <baver.bae@lge.com>
Hi all,
Some virtio-mmio based devices, such as virtio-clock or virtio-regulator,
must become operational before other devices have their regular PM restore
callbacks invoked, because those other devices depend on them.
Generally, PM framework provides the three phases (freeze, freeze_late,
freeze_noirq) for the system sleep sequence, and the corresponding resume
phases. But, virtio core only supports the normal freeze/restore phase,
so virtio drivers have no way to participate in the noirq phase, which runs
with IRQs disabled and is guaranteed to run before any normal-phase restore
callbacks.
This series adds the infrastructure and the virtio-mmio transport
wiring so that virtio drivers can implement freeze_noirq/restore_noirq
callbacks.
Design overview
===============
The noirq phase runs with device IRQ handlers disabled and must avoid
sleepable operations. The main constraints addressed are:
- might_sleep() in virtio_add_status() and virtio_features_ok().
- virtio_synchronize_cbs() in virtio_reset_device() (IRQs are already
quiesced).
- spin_lock_irq() in virtio_config_core_enable() (not safe to call
with interrupts already disabled).
- Memory allocation during vq setup (virtqueue_reinit_vring() reuses
existing buffers instead).
The series provides noirq-safe variants for each of these, plus a new
config_ops->reset_vqs() callback that lets the transport reprogram
queue registers without freeing/reallocating vring memory.
Not all transports can safely perform these operations in the noirq phase.
Transports like virtio-ccw issue channel commands and wait for a completion
interrupt, which will never arrive while device interrupts are masked at
the interrupt controller. A new boolean field config_ops->noirq_safe marks
transports that implement reset/status operations via simple MMIO
reads/writes and are therefore safe to use in noirq context. The noirq
helpers assert this flag at runtime, and virtio_device_freeze_noirq()
enforces it at freeze time, returning -EOPNOTSUPP early to prevent
a deadlock on resume.
When a driver implements restore_noirq, the device bring-up (reset ->
ACKNOWLEDGE -> DRIVER -> finalize_features -> FEATURES_OK) happens in
the noirq phase. The subsequent normal-phase virtio_device_restore()
detects this and skips the redundant re-initialization.
Patch breakdown
===============
Patch 1 is a preparatory refactoring with no functional change.
Patches 2-3 add the core infrastructure. Patch 4 wires it up for
virtio-mmio.
1. virtio: separate PM restore and reset_done paths
Splits virtio_device_restore_priv() into independent
virtio_device_restore() and virtio_device_reset_done() paths,
using a shared virtio_device_reinit() helper. This is a pure
refactoring to make the restore path independently extensible
without complicating the boolean dispatch.
2. virtio_ring: export virtqueue_reinit_vring() for noirq restore
Adds virtqueue_reinit_vring(), an exported wrapper that resets
vring indices and descriptor state in place without any memory
allocation, making it safe to call from noirq context. Also
resets IN_ORDER-specific state (free_head, batch_last.id) in
virtqueue_init() to keep the ring consistent after reinit, and
adds runtime WARN_ON checks for unexpected in-flight descriptor
state and split-ring index consistency.
3. virtio: add noirq system sleep PM infrastructure
Adds noirq-safe helpers (virtio_add_status_noirq,
virtio_features_ok_noirq, virtio_reset_device_noirq,
virtio_config_core_enable_noirq, virtio_device_ready_noirq) and
the freeze_noirq/restore_noirq driver callbacks plus the
config_ops->reset_vqs() transport hook. Introduces
config_ops->noirq_safe to mark transports whose reset/status
operations are safe in noirq context (e.g. simple MMIO), and
enforces early -EOPNOTSUPP in virtio_device_freeze_noirq() when
the transport does not meet noirq requirements. Modifies
virtio_device_restore() to skip bring-up when restore_noirq
already ran.
4. virtio-mmio: wire up noirq system sleep PM callbacks
Implements vm_reset_vqs() which iterates existing virtqueues,
reinitializes the vring state via virtqueue_reinit_vring(), and
reprograms the MMIO queue registers. Adds
virtio_mmio_freeze_noirq/virtio_mmio_restore_noirq and registers
them via SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(). Sets .noirq_safe = true
in virtio_mmio_config_ops to declare that MMIO-based status and
reset operations are safe during the noirq PM phase.
Testing
=======
Build-tested with arm64 cross-compilation.
(make ARCH=arm64 M=drivers/virtio)
Runtime-tested on an internal virtio-mmio platform with virtio-clock,
confirming that the clock device works well before other devices' normal
restore() callbacks run.
Changes
=======
v7:
virtio: add noirq system sleep PM infrastructure
- Configured virtio_noirq_state to have 3 states to differentiate
between restore_noirq failures and skipping the noirq phase.
- Re-verified the PM callback combinations.
virtio-mmio: wire up noirq system sleep PM callbacks
- Aligned both conditions for GUEST_PAGE_SIZE and virtio_device_reinit()
v6:
virtio_ring: export virtqueue_reinit_vring() for noirq restore
- Made virtqueue_reinit_vring() fail with -EBUSY on precondition
violations and propagate that error.
virtio: add noirq system sleep PM infrastructure
- Make noirq restore failure terminal for the same device:
.restore is not a same-device fallback for .restore_noirq failure.
- Decouple noirq failure from pre-freeze dev->failed snapshot.
- Add noirq_safe validation in virtio_device_freeze() to catch transport
mismatch early.
virtio-mmio: wire up noirq system sleep PM callbacks
- Make vm_reset_vqs() handle virtqueue_reinit_vring() failures
to prevent continuing noirq restore with a potentially corrupted
split free-list state.
v5:
virtio: add noirq system sleep PM infrastructure
- Preserve FAILED across restore_noirq() failure by recording the
failure in dev->failed before falling back to the normal restore path.
- Document the restore/restore_noirq fallback contract more clearly,
especially for drivers that preserve virtqueues across suspend.
v4:
virtio_ring: export virtqueue_reinit_vring() for noirq restore
- Reinit safety was tightened by resetting IN_ORDER-specific state.
- Added extra split-ring consistency WARN_ON checks.
- Clarified caller preconditions for noirq-safe vring reinit.
virtio: add noirq system sleep PM infrastructure
- Added config_ops->noirq_safe to explicitly mark transports that are
safe in noirq PM phase.
- Enforced early -EOPNOTSUPP checks in freeze_noirq for unsupported
transport combinations (noirq_safe/reset_vqs requirements).
- Added defensive runtime guards/warnings in noirq helper and restore
paths.
- Discussed the freeze-before-freeze_noirq abort scenario raised in
review and concluded that the fallback restore path is intentionally
handled by regular restore after core reinit/reset, so reset_vqs is
kept limited to the noirq restore flow.
virtio-mmio: wire up noirq system sleep PM callbacks
- Marked virtio-mmio transport as noirq-capable by setting .noirq_safe
in virtio_mmio_config_ops.
v3:
virtio: separate PM restore and reset_done paths
- Refined restore flow to explicitly handle the no-driver case after
reinit, improving clarity and avoiding unnecessary driver-path
assumptions.
virtio_ring: export virtqueue_reinit_vring() for noirq restore
- Hardened virtqueue_reinit_vring() with stronger safety notes and
a runtime WARN_ON check to catch reinit with unexpected free-list
state.
virtio: add noirq system sleep PM infrastructure
- Added explicit noirq restore completion tracking noirq_restore_done
and updated PM sequencing to use it, plus early freeze_noirq
validation for missing reset_vqs support.
virtio-mmio: wire up noirq system sleep PM callbacks
- Updated virtio-mmio restore path to skip legacy GUEST_PAGE_SIZE
rewrite when noirq restore already completed.
v2:
virtio-mmio: wire up noirq system sleep PM callbacks
- The code that was duplicated in vm_setup_vq() and vm_reset_vqs() has
been moved to vm_active_vq() function.
Sungho Bae (4):
virtio: separate PM restore and reset_done paths
virtio_ring: export virtqueue_reinit_vring() for noirq restore
virtio: add noirq system sleep PM infrastructure
virtio-mmio: wire up noirq system sleep PM callbacks
drivers/virtio/virtio.c | 325 +++++++++++++++++++++++++++++++---
drivers/virtio/virtio_mmio.c | 137 +++++++++-----
drivers/virtio/virtio_ring.c | 58 ++++++
include/linux/virtio.h | 42 +++++
include/linux/virtio_config.h | 39 ++++
include/linux/virtio_ring.h | 3 +
6 files changed, 538 insertions(+), 66 deletions(-)
--
2.43.0
^ permalink raw reply
* [RFC PATCH v7 1/4] virtio: separate PM restore and reset_done paths
From: Sungho Bae @ 2026-04-28 15:07 UTC (permalink / raw)
To: mst, jasowang
Cc: xuanzhuo, eperezma, virtualization, linux-kernel, Sungho Bae
In-Reply-To: <20260428150742.23999-1-baver.bae@gmail.com>
From: Sungho Bae <baver.bae@lge.com>
Refactor virtio_device_restore_priv() by extracting the common device
re-initialization sequence into virtio_device_reinit(). This helper
performs the full bring-up sequence: reset, status acknowledgment,
feature finalization, and feature negotiation.
virtio_device_restore() and virtio_device_reset_done() now each call
virtio_device_reinit() directly instead of going through a boolean-
dispatched wrapper. This makes each path independently readable and
extensible without further complicating the dispatch logic.
A follow-up series will add noirq PM callbacks that only affect the
restore path; having the two paths separated avoids adding more
conditionals to a shared function.
No functional change.
Signed-off-by: Sungho Bae <baver.bae@lge.com>
---
drivers/virtio/virtio.c | 81 +++++++++++++++++++++++++----------------
1 file changed, 50 insertions(+), 31 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 5bdc6b82b30b..98f1875f8df1 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -588,7 +588,7 @@ void unregister_virtio_device(struct virtio_device *dev)
}
EXPORT_SYMBOL_GPL(unregister_virtio_device);
-static int virtio_device_restore_priv(struct virtio_device *dev, bool restore)
+static int virtio_device_reinit(struct virtio_device *dev)
{
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
int ret;
@@ -613,35 +613,9 @@ static int virtio_device_restore_priv(struct virtio_device *dev, bool restore)
ret = dev->config->finalize_features(dev);
if (ret)
- goto err;
-
- ret = virtio_features_ok(dev);
- if (ret)
- goto err;
-
- if (restore) {
- if (drv->restore) {
- ret = drv->restore(dev);
- if (ret)
- goto err;
- }
- } else {
- ret = drv->reset_done(dev);
- if (ret)
- goto err;
- }
-
- /* If restore didn't do it, mark device DRIVER_OK ourselves. */
- if (!(dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK))
- virtio_device_ready(dev);
-
- virtio_config_core_enable(dev);
-
- return 0;
+ return ret;
-err:
- virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
- return ret;
+ return virtio_features_ok(dev);
}
#ifdef CONFIG_PM_SLEEP
@@ -668,7 +642,33 @@ EXPORT_SYMBOL_GPL(virtio_device_freeze);
int virtio_device_restore(struct virtio_device *dev)
{
- return virtio_device_restore_priv(dev, true);
+ struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+ int ret;
+
+ ret = virtio_device_reinit(dev);
+ if (ret)
+ goto err;
+
+ if (!drv)
+ return 0;
+
+ if (drv->restore) {
+ ret = drv->restore(dev);
+ if (ret)
+ goto err;
+ }
+
+ /* If restore didn't do it, mark device DRIVER_OK ourselves. */
+ if (!(dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK))
+ virtio_device_ready(dev);
+
+ virtio_config_core_enable(dev);
+
+ return 0;
+
+err:
+ virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
+ return ret;
}
EXPORT_SYMBOL_GPL(virtio_device_restore);
#endif
@@ -698,11 +698,30 @@ EXPORT_SYMBOL_GPL(virtio_device_reset_prepare);
int virtio_device_reset_done(struct virtio_device *dev)
{
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+ int ret;
if (!drv || !drv->reset_done)
return -EOPNOTSUPP;
- return virtio_device_restore_priv(dev, false);
+ ret = virtio_device_reinit(dev);
+ if (ret)
+ goto err;
+
+ ret = drv->reset_done(dev);
+ if (ret)
+ goto err;
+
+ /* If reset_done didn't do it, mark device DRIVER_OK ourselves. */
+ if (!(dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK))
+ virtio_device_ready(dev);
+
+ virtio_config_core_enable(dev);
+
+ return 0;
+
+err:
+ virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
+ return ret;
}
EXPORT_SYMBOL_GPL(virtio_device_reset_done);
--
2.43.0
^ permalink raw reply related
* [RFC PATCH v7 2/4] virtio_ring: export virtqueue_reinit_vring() for noirq restore
From: Sungho Bae @ 2026-04-28 15:07 UTC (permalink / raw)
To: mst, jasowang
Cc: xuanzhuo, eperezma, virtualization, linux-kernel, Sungho Bae
In-Reply-To: <20260428150742.23999-1-baver.bae@gmail.com>
From: Sungho Bae <baver.bae@lge.com>
After a device reset in noirq context the existing vrings must be
re-initialized without any memory allocation, because GFP_KERNEL is
not available.
The internal helpers virtqueue_reset_split() and
virtqueue_reset_packed() already reset vring indices and descriptor
state in place. Add a thin exported wrapper, virtqueue_reinit_vring(),
that dispatches to the appropriate helper based on the ring layout.
This will be used by a subsequent patch that adds noirq system-sleep
PM callbacks for virtio-mmio.
Signed-off-by: Sungho Bae <baver.bae@lge.com>
---
drivers/virtio/virtio_ring.c | 58 ++++++++++++++++++++++++++++++++++++
include/linux/virtio_ring.h | 3 ++
2 files changed, 61 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index fbca7ce1c6bf..d3339b820f6b 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -506,6 +506,15 @@ static void virtqueue_init(struct vring_virtqueue *vq, u32 num)
vq->event_triggered = false;
vq->num_added = 0;
+ /*
+ * Keep IN_ORDER state aligned with a freshly initialized/reset queue.
+ * For packed IN_ORDER, free_head is unused but harmlessly reset.
+ */
+ if (virtqueue_is_in_order(vq)) {
+ vq->free_head = 0;
+ vq->batch_last.id = UINT_MAX;
+ }
+
#ifdef DEBUG
vq->in_use = false;
vq->last_add_time_valid = false;
@@ -3936,5 +3945,54 @@ void virtqueue_map_sync_single_range_for_device(const struct virtqueue *_vq,
}
EXPORT_SYMBOL_GPL(virtqueue_map_sync_single_range_for_device);
+/**
+ * virtqueue_reinit_vring - reinitialize vring state without reallocation
+ * @_vq: the virtqueue
+ *
+ * Reset the avail/used indices and descriptor state of an existing
+ * virtqueue so it can be reused after a device reset. No memory is
+ * allocated or freed, making this safe for use in noirq context.
+ *
+ * Preconditions for callers:
+ * 1) The vq must be fully quiesced (no concurrent add/get/kick/IRQ callback).
+ * 2) Transport/device side must already have stopped/reset this queue.
+ * 3) All in-flight buffers must already be completed or detached.
+ *
+ * If called with outstanding descriptors, free-list state can be corrupted:
+ * num_free is restored to full capacity while desc_extra next-chain/free_head
+ * may still represent a partially consumed list.
+ *
+ * Return:
+ * 0 on success, or -EBUSY if preconditions are not met.
+ */
+int virtqueue_reinit_vring(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ unsigned int num = virtqueue_is_packed(vq) ?
+ vq->packed.vring.num : vq->split.vring.num;
+
+ /* All in-flight descriptors must be completed or detached */
+ if (WARN_ON(vq->vq.num_free != num))
+ return -EBUSY;
+
+ if (virtqueue_is_packed(vq)) {
+ virtqueue_reset_packed(vq);
+ } else {
+ /*
+ * Split queue shadow index should match the visible avail
+ * index when the queue is fully quiesced.
+ */
+ if (WARN_ON(vq->split.avail_idx_shadow !=
+ virtio16_to_cpu(vq->vq.vdev,
+ vq->split.vring.avail->idx)))
+ return -EBUSY;
+
+ virtqueue_reset_split(vq);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(virtqueue_reinit_vring);
+
MODULE_DESCRIPTION("Virtio ring implementation");
MODULE_LICENSE("GPL");
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index c97a12c1cda3..8b421fef4fef 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -118,6 +118,9 @@ void vring_del_virtqueue(struct virtqueue *vq);
/* Filter out transport-specific feature bits. */
void vring_transport_features(struct virtio_device *vdev);
+/* Reinitialize a virtqueue without reallocation (safe in noirq context) */
+int virtqueue_reinit_vring(struct virtqueue *_vq);
+
irqreturn_t vring_interrupt(int irq, void *_vq);
u32 vring_notification_data(struct virtqueue *_vq);
--
2.43.0
^ permalink raw reply related
* [RFC PATCH v7 3/4] virtio: add noirq system sleep PM infrastructure
From: Sungho Bae @ 2026-04-28 15:07 UTC (permalink / raw)
To: mst, jasowang
Cc: xuanzhuo, eperezma, virtualization, linux-kernel, Sungho Bae
In-Reply-To: <20260428150742.23999-1-baver.bae@gmail.com>
From: Sungho Bae <baver.bae@lge.com>
Some virtio-mmio devices, such as virtio-clock or virtio-regulator,
must become operational before the regular PM restore callback runs
because other devices may depend on them.
Add the core infrastructure needed to support noirq system-sleep PM
callbacks for virtio transports:
- virtio_add_status_noirq(): status helper without might_sleep().
- virtio_features_ok_noirq(): feature negotiation without might_sleep().
- virtio_reset_device_noirq(): device reset that skips
virtio_synchronize_cbs() (IRQ handlers are already quiesced in the
noirq phase).
- virtio_device_reinit_noirq(): full noirq bring-up sequence using the
above helpers.
- virtio_config_core_enable_noirq(): config enable with irqsave
locking.
- virtio_device_ready_noirq(): marks DRIVER_OK without
virtio_synchronize_cbs().
Not all transports can safely call reset, get_status, set_status, or
finalize_features during the noirq phase: transports like virtio-ccw
issue channel commands and wait for a completion interrupt, which will
never be delivered because device interrupts are masked at the interrupt
controller during noirq suspend/resume. To address this, introduce a
boolean field noirq_safe in struct virtio_config_ops. Transports that
implement the above operations via simple MMIO reads/writes (e.g.
virtio-mmio) set this flag; all others leave it at the default false.
The noirq helpers assert noirq_safe via WARN_ON at runtime.
virtio_device_freeze_noirq() enforces the contract at freeze time,
returning -EOPNOTSUPP early if the driver provides restore_noirq but
the transport does not meet the requirements, to prevent a deadlock on
resume. virtio_device_restore_noirq() performs a second check as a
safety net in case freeze_noirq was not called.
Add freeze_noirq/restore_noirq callbacks to struct virtio_driver and
provide matching helper wrappers in the virtio core:
- virtio_device_freeze_noirq(): validates noirq_safe and reset_vqs
requirements, then forwards to drv->freeze_noirq().
- virtio_device_restore_noirq(): guards against unsafe transports,
runs the noirq bring-up sequence, resets existing vrings via the
new config_ops->reset_vqs() hook, then calls drv->restore_noirq().
Modify virtio_device_restore() so that when a driver provides
restore_noirq, the normal-phase restore skips the re-initialization
that was already done in the noirq phase.
Signed-off-by: Sungho Bae <baver.bae@lge.com>
---
drivers/virtio/virtio.c | 262 +++++++++++++++++++++++++++++++++-
include/linux/virtio.h | 42 ++++++
include/linux/virtio_config.h | 39 +++++
3 files changed, 339 insertions(+), 4 deletions(-)
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 98f1875f8df1..50e23f10be40 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -193,6 +193,17 @@ static void virtio_config_core_enable(struct virtio_device *dev)
spin_unlock_irq(&dev->config_lock);
}
+static void virtio_config_core_enable_noirq(struct virtio_device *dev)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&dev->config_lock, flags);
+ dev->config_core_enabled = true;
+ if (dev->config_change_pending)
+ __virtio_config_changed(dev);
+ spin_unlock_irqrestore(&dev->config_lock, flags);
+}
+
void virtio_add_status(struct virtio_device *dev, unsigned int status)
{
might_sleep();
@@ -200,6 +211,21 @@ void virtio_add_status(struct virtio_device *dev, unsigned int status)
}
EXPORT_SYMBOL_GPL(virtio_add_status);
+/*
+ * Same as virtio_add_status() but without the might_sleep() assertion,
+ * so it is safe to call from noirq context.
+ *
+ * Requires the transport to have set config_ops->noirq_safe, which declares
+ * that reset, get_status, and set_status do not wait for a completion
+ * interrupt and are therefore safe during the noirq PM phase.
+ */
+void virtio_add_status_noirq(struct virtio_device *dev, unsigned int status)
+{
+ WARN_ON(!dev->config->noirq_safe);
+ dev->config->set_status(dev, dev->config->get_status(dev) | status);
+}
+EXPORT_SYMBOL_GPL(virtio_add_status_noirq);
+
/* Do some validation, then set FEATURES_OK */
static int virtio_features_ok(struct virtio_device *dev)
{
@@ -234,6 +260,38 @@ static int virtio_features_ok(struct virtio_device *dev)
return 0;
}
+/* noirq-safe variant: no might_sleep(), uses virtio_add_status_noirq() */
+static int virtio_features_ok_noirq(struct virtio_device *dev)
+{
+ unsigned int status;
+
+ if (virtio_check_mem_acc_cb(dev)) {
+ if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1)) {
+ dev_warn(&dev->dev,
+ "device must provide VIRTIO_F_VERSION_1\n");
+ return -ENODEV;
+ }
+
+ if (!virtio_has_feature(dev, VIRTIO_F_ACCESS_PLATFORM)) {
+ dev_warn(&dev->dev,
+ "device must provide VIRTIO_F_ACCESS_PLATFORM\n");
+ return -ENODEV;
+ }
+ }
+
+ if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1))
+ return 0;
+
+ virtio_add_status_noirq(dev, VIRTIO_CONFIG_S_FEATURES_OK);
+ status = dev->config->get_status(dev);
+ if (!(status & VIRTIO_CONFIG_S_FEATURES_OK)) {
+ dev_err(&dev->dev, "virtio: device refuses features: %x\n",
+ status);
+ return -ENODEV;
+ }
+ return 0;
+}
+
/**
* virtio_reset_device - quiesce device for removal
* @dev: the device to reset
@@ -267,6 +325,28 @@ void virtio_reset_device(struct virtio_device *dev)
}
EXPORT_SYMBOL_GPL(virtio_reset_device);
+/**
+ * virtio_reset_device_noirq - noirq-safe variant of virtio_reset_device()
+ * @dev: the device to reset
+ *
+ * Requires the transport to have set config_ops->noirq_safe.
+ */
+void virtio_reset_device_noirq(struct virtio_device *dev)
+{
+ WARN_ON(!dev->config->noirq_safe);
+
+#ifdef CONFIG_VIRTIO_HARDEN_NOTIFICATION
+ /*
+ * The noirq stage runs with device IRQ handlers disabled, so
+ * virtio_synchronize_cbs() must not be called here.
+ */
+ virtio_break_device(dev);
+#endif
+
+ dev->config->reset(dev);
+}
+EXPORT_SYMBOL_GPL(virtio_reset_device_noirq);
+
static int virtio_dev_probe(struct device *_d)
{
int err, i;
@@ -539,6 +619,7 @@ int register_virtio_device(struct virtio_device *dev)
dev->config_driver_disabled = false;
dev->config_core_enabled = false;
dev->config_change_pending = false;
+ dev->noirq_state = VIRTIO_NOIRQ_NONE;
INIT_LIST_HEAD(&dev->vqs);
spin_lock_init(&dev->vqs_list_lock);
@@ -618,6 +699,47 @@ static int virtio_device_reinit(struct virtio_device *dev)
return virtio_features_ok(dev);
}
+/*
+ * noirq-safe variant of virtio_device_reinit().
+ *
+ * Requires the transport to declare config_ops->noirq_safe, which means
+ * reset, get_status, set_status, and finalize_features are safe to call
+ * during the noirq PM phase.
+ */
+static int virtio_device_reinit_noirq(struct virtio_device *dev)
+{
+ struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+ int ret;
+
+ /*
+ * We always start by resetting the device, in case a previous
+ * driver messed it up.
+ */
+ virtio_reset_device_noirq(dev);
+
+ /* Acknowledge that we've seen the device. */
+ virtio_add_status_noirq(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
+
+ /*
+ * Maybe driver failed before freeze.
+ * Restore the failed status, for debugging.
+ */
+ if (dev->failed)
+ virtio_add_status_noirq(dev, VIRTIO_CONFIG_S_FAILED);
+
+ if (!drv)
+ return 0;
+
+ /* We have a driver! */
+ virtio_add_status_noirq(dev, VIRTIO_CONFIG_S_DRIVER);
+
+ ret = dev->config->finalize_features(dev);
+ if (ret)
+ return ret;
+
+ return virtio_features_ok_noirq(dev);
+}
+
#ifdef CONFIG_PM_SLEEP
int virtio_device_freeze(struct virtio_device *dev)
{
@@ -627,6 +749,20 @@ int virtio_device_freeze(struct virtio_device *dev)
virtio_config_core_disable(dev);
dev->failed = dev->config->get_status(dev) & VIRTIO_CONFIG_S_FAILED;
+ dev->noirq_state = VIRTIO_NOIRQ_NONE;
+
+ /*
+ * If the driver provides restore_noirq, verify that the transport
+ * supports noirq PM. It will fail early so the PM core can abort
+ * the transition gracefully, rather than silently skipping noirq
+ * restore and then failing in the normal restore path.
+ */
+ if (drv && drv->restore_noirq && !dev->config->noirq_safe) {
+ dev_warn(&dev->dev,
+ "transport does not support noirq PM\n");
+ virtio_config_core_enable(dev);
+ return -EOPNOTSUPP;
+ }
if (drv && drv->freeze) {
ret = drv->freeze(dev);
@@ -645,12 +781,35 @@ int virtio_device_restore(struct virtio_device *dev)
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
int ret;
- ret = virtio_device_reinit(dev);
- if (ret)
+ /*
+ * If the driver implements restore_noirq and the noirq phase was
+ * actually entered (freeze_noirq ran), but restore_noirq did not
+ * complete successfully, the noirq phase must have failed. PM core
+ * may continue later resume phases for global recovery, but virtio
+ * does not use the normal restore path as an implicit same-device
+ * fallback.
+ */
+ if (drv && drv->restore_noirq &&
+ dev->noirq_state == VIRTIO_NOIRQ_ENTERED) {
+ ret = -EIO;
goto err;
+ }
- if (!drv)
- return 0;
+ /*
+ * Re-initialization is needed only for drivers that do not
+ * implement restore_noirq. When restore_noirq exists, either:
+ * - NOIRQ_NONE: noirq phase was never entered, so no noirq-specific
+ * teardown occurred and the device is still live.
+ * - NOIRQ_RESTORED: noirq phase already performed reinit.
+ * (NOIRQ_ENTERED is caught above as -EIO.)
+ */
+ if (!drv || !drv->restore_noirq) {
+ ret = virtio_device_reinit(dev);
+ if (ret)
+ goto err;
+ if (!drv)
+ return 0;
+ }
if (drv->restore) {
ret = drv->restore(dev);
@@ -671,6 +830,101 @@ int virtio_device_restore(struct virtio_device *dev)
return ret;
}
EXPORT_SYMBOL_GPL(virtio_device_restore);
+
+int virtio_device_freeze_noirq(struct virtio_device *dev)
+{
+ struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+
+ if (!drv)
+ return 0;
+
+ /*
+ * restore_noirq requires that the transport's config ops
+ * (reset, get_status, set_status) are safe to call during the noirq
+ * PM phase. Catch the mismatch early at freeze time so the PM core
+ * can abort cleanly rather than deadlocking on resume.
+ */
+ if (drv->restore_noirq && !dev->config->noirq_safe) {
+ dev_warn(&dev->dev,
+ "transport does not support noirq PM\n");
+ return -EOPNOTSUPP;
+ }
+
+ /*
+ * If the driver provides restore_noirq and has active vqs,
+ * the transport must support reset_vqs to restore them.
+ * Fail here so the PM core can abort the transition gracefully,
+ * rather than hitting -EOPNOTSUPP on resume.
+ */
+ if (drv->restore_noirq && !list_empty(&dev->vqs) &&
+ !dev->config->reset_vqs) {
+ dev_warn(&dev->dev,
+ "transport does not support noirq PM restore with active vqs (missing reset_vqs)\n");
+ return -EOPNOTSUPP;
+ }
+
+ /* Mark that the noirq phase has been entered. */
+ dev->noirq_state = VIRTIO_NOIRQ_ENTERED;
+
+ if (drv->freeze_noirq)
+ return drv->freeze_noirq(dev);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(virtio_device_freeze_noirq);
+
+int virtio_device_restore_noirq(struct virtio_device *dev)
+{
+ struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+ int ret;
+
+ if (!drv || !drv->restore_noirq)
+ return 0;
+
+ /*
+ * All transport ops called below (reset, get_status, set_status) must
+ * be noirq-safe. Return early if not - this should normally have
+ * been caught at freeze_noirq time.
+ */
+ if (!dev->config->noirq_safe) {
+ dev_warn(&dev->dev,
+ "transport does not support noirq PM; skipping restore\n");
+ return -EOPNOTSUPP;
+ }
+
+ ret = virtio_device_reinit_noirq(dev);
+ if (ret)
+ goto err;
+
+ if (!list_empty(&dev->vqs)) {
+ if (!dev->config->reset_vqs) {
+ ret = -EOPNOTSUPP;
+ goto err;
+ }
+
+ ret = dev->config->reset_vqs(dev);
+ if (ret)
+ goto err;
+ }
+
+ ret = drv->restore_noirq(dev);
+ if (ret)
+ goto err;
+
+ /* Mark that noirq restore has completed successfully. */
+ dev->noirq_state = VIRTIO_NOIRQ_RESTORED;
+
+ /* If restore_noirq set DRIVER_OK, enable config now. */
+ if (dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK)
+ virtio_config_core_enable_noirq(dev);
+
+ return 0;
+
+err:
+ virtio_add_status_noirq(dev, VIRTIO_CONFIG_S_FAILED);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(virtio_device_restore_noirq);
#endif
int virtio_device_reset_prepare(struct virtio_device *dev)
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 3bbc4cb6a672..937bc3c56bb8 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -143,6 +143,18 @@ struct virtio_admin_cmd {
int ret;
};
+/**
+ * enum virtio_noirq_state - tracks noirq PM phase progress
+ * @VIRTIO_NOIRQ_NONE: noirq phase was not entered (only freeze ran)
+ * @VIRTIO_NOIRQ_ENTERED: freeze_noirq ran; restore_noirq is expected
+ * @VIRTIO_NOIRQ_RESTORED: restore_noirq completed successfully
+ */
+enum virtio_noirq_state {
+ VIRTIO_NOIRQ_NONE,
+ VIRTIO_NOIRQ_ENTERED,
+ VIRTIO_NOIRQ_RESTORED,
+};
+
/**
* struct virtio_device - representation of a device using virtio
* @index: unique position on the virtio bus
@@ -151,6 +163,7 @@ struct virtio_admin_cmd {
* @config_driver_disabled: configuration change reporting disabled by
* a driver
* @config_change_pending: configuration change reported while disabled
+ * @noirq_state: tracks noirq PM phase progress for restore coordination
* @config_lock: protects configuration change reporting
* @vqs_list_lock: protects @vqs.
* @dev: underlying device.
@@ -171,6 +184,7 @@ struct virtio_device {
bool config_core_enabled;
bool config_driver_disabled;
bool config_change_pending;
+ enum virtio_noirq_state noirq_state;
spinlock_t config_lock;
spinlock_t vqs_list_lock;
struct device dev;
@@ -209,8 +223,12 @@ void virtio_config_driver_enable(struct virtio_device *dev);
#ifdef CONFIG_PM_SLEEP
int virtio_device_freeze(struct virtio_device *dev);
int virtio_device_restore(struct virtio_device *dev);
+int virtio_device_freeze_noirq(struct virtio_device *dev);
+int virtio_device_restore_noirq(struct virtio_device *dev);
#endif
void virtio_reset_device(struct virtio_device *dev);
+void virtio_reset_device_noirq(struct virtio_device *dev);
+void virtio_add_status_noirq(struct virtio_device *dev, unsigned int status);
int virtio_device_reset_prepare(struct virtio_device *dev);
int virtio_device_reset_done(struct virtio_device *dev);
@@ -237,6 +255,28 @@ size_t virtio_max_dma_size(const struct virtio_device *vdev);
* changes; may be called in interrupt context.
* @freeze: optional function to call during suspend/hibernation.
* @restore: optional function to call on resume.
+ * When @restore_noirq is not implemented, core resets and reinitializes
+ * the device before calling this. When @restore_noirq succeeded, core
+ * skips reinitialization; drivers should avoid calling virtio_device_ready()
+ * if DRIVER_OK was already set in the noirq phase.
+ * When @restore_noirq failed, this callback is not invoked for same-device
+ * recovery; the saved noirq error is propagated instead.
+ * When the noirq phase was entirely skipped (e.g. suspend aborted before
+ * suspend_noirq), core skips reinitialization for drivers that implement
+ * @restore_noirq and calls @restore (if provided) to undo the freeze()
+ * quiesce. Drivers without @restore_noirq follow the normal reinit +
+ * restore path.
+ * @freeze_noirq: optional function to call during noirq suspend/hibernation.
+ * @restore_noirq: optional function to call on noirq resume.
+ * If this callback fails, PM core may still continue later resume phases
+ * for global system recovery. Virtio does not treat @restore as an
+ * implicit same-device fallback for @restore_noirq failure; drivers should
+ * only implement @restore_noirq when noirq resume is their required
+ * recovery point.
+ * A noirq restore failure is detected by the normal restore path
+ * (noirq_state == VIRTIO_NOIRQ_ENTERED, meaning freeze_noirq ran but
+ * restore_noirq did not complete) and returns -EIO instead of attempting
+ * same-device recovery.
* @reset_prepare: optional function to call when a transport specific reset
* occurs.
* @reset_done: optional function to call after transport specific reset
@@ -258,6 +298,8 @@ struct virtio_driver {
void (*config_changed)(struct virtio_device *dev);
int (*freeze)(struct virtio_device *dev);
int (*restore)(struct virtio_device *dev);
+ int (*freeze_noirq)(struct virtio_device *dev);
+ int (*restore_noirq)(struct virtio_device *dev);
int (*reset_prepare)(struct virtio_device *dev);
int (*reset_done)(struct virtio_device *dev);
void (*shutdown)(struct virtio_device *dev);
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 69f84ea85d71..0110b091f634 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -70,6 +70,9 @@ struct virtqueue_info {
* vqs_info: array of virtqueue info structures
* Returns 0 on success or error status
* @del_vqs: free virtqueues found by find_vqs().
+ * @reset_vqs: reinitialize existing virtqueues without allocating or
+ * freeing them (optional). Used during noirq restore.
+ * Returns 0 on success or error status.
* @synchronize_cbs: synchronize with the virtqueue callbacks (optional)
* The function guarantees that all memory operations on the
* queue before it are visible to the vring_interrupt() that is
@@ -108,6 +111,14 @@ struct virtqueue_info {
* Returns 0 on success or error status
* If disable_vq_and_reset is set, then enable_vq_after_reset must also be
* set.
+ * @noirq_safe: set to true if @reset, @get_status, @set_status, and
+ * @finalize_features are safe to call during the noirq phase of system
+ * suspend/resume. Transports that implement these operations via simple
+ * MMIO reads/writes (e.g. virtio-mmio) can set this flag. Transports
+ * that issue channel commands and wait for a completion interrupt (e.g.
+ * virtio-ccw) must NOT set it, because device interrupts are masked at
+ * the interrupt controller during the noirq phase, which would cause the
+ * wait to hang.
*/
struct virtio_config_ops {
void (*get)(struct virtio_device *vdev, unsigned offset,
@@ -123,6 +134,7 @@ struct virtio_config_ops {
struct virtqueue_info vqs_info[],
struct irq_affinity *desc);
void (*del_vqs)(struct virtio_device *);
+ int (*reset_vqs)(struct virtio_device *vdev);
void (*synchronize_cbs)(struct virtio_device *);
u64 (*get_features)(struct virtio_device *vdev);
void (*get_extended_features)(struct virtio_device *vdev,
@@ -137,6 +149,7 @@ struct virtio_config_ops {
struct virtio_shm_region *region, u8 id);
int (*disable_vq_and_reset)(struct virtqueue *vq);
int (*enable_vq_after_reset)(struct virtqueue *vq);
+ bool noirq_safe;
};
/**
@@ -371,6 +384,32 @@ void virtio_device_ready(struct virtio_device *dev)
dev->config->set_status(dev, status | VIRTIO_CONFIG_S_DRIVER_OK);
}
+/**
+ * virtio_device_ready_noirq - noirq-safe variant of virtio_device_ready()
+ * @dev: the virtio device
+ *
+ * Requires the transport to have set config_ops->noirq_safe, which declares
+ * that get_status and set_status do not wait for a completion interrupt.
+ */
+static inline
+void virtio_device_ready_noirq(struct virtio_device *dev)
+{
+ unsigned int status = dev->config->get_status(dev);
+
+ WARN_ON(!dev->config->noirq_safe);
+ WARN_ON(status & VIRTIO_CONFIG_S_DRIVER_OK);
+
+#ifdef CONFIG_VIRTIO_HARDEN_NOTIFICATION
+ /*
+ * The noirq stage runs with device IRQ handlers disabled, so
+ * virtio_synchronize_cbs() must not be called here.
+ */
+ __virtio_unbreak_device(dev);
+#endif
+
+ dev->config->set_status(dev, status | VIRTIO_CONFIG_S_DRIVER_OK);
+}
+
static inline
const char *virtio_bus_name(struct virtio_device *vdev)
{
--
2.43.0
^ permalink raw reply related
* [RFC PATCH v7 4/4] virtio-mmio: wire up noirq system sleep PM callbacks
From: Sungho Bae @ 2026-04-28 15:07 UTC (permalink / raw)
To: mst, jasowang
Cc: xuanzhuo, eperezma, virtualization, linux-kernel, Sungho Bae
In-Reply-To: <20260428150742.23999-1-baver.bae@gmail.com>
From: Sungho Bae <baver.bae@lge.com>
Add noirq system-sleep PM support to the virtio-mmio transport.
This change wires noirq freeze/restore callbacks into virtio-mmio and
hooks queue reset/reactivation into the transport config ops so virtqueues
can be reinitialized and reused across suspend/resume.
For legacy (v1) devices, keep GUEST_PAGE_SIZE programming aligned with the
noirq restore path while avoiding duplicate programming in normal restore.
This enables virtio-mmio based devices to participate safely in the noirq
PM phase, which is required for early-restore users.
Signed-off-by: Sungho Bae <baver.bae@lge.com>
---
drivers/virtio/virtio_mmio.c | 137 +++++++++++++++++++++++++----------
1 file changed, 97 insertions(+), 40 deletions(-)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 595c2274fbb5..4f4836103b88 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -336,6 +336,77 @@ static void vm_del_vqs(struct virtio_device *vdev)
free_irq(platform_get_irq(vm_dev->pdev, 0), vm_dev);
}
+static int vm_active_vq(struct virtio_device *vdev, struct virtqueue *vq)
+{
+ struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
+ int q_num = virtqueue_get_vring_size(vq);
+
+ writel(q_num, vm_dev->base + VIRTIO_MMIO_QUEUE_NUM);
+ if (vm_dev->version == 1) {
+ u64 q_pfn = virtqueue_get_desc_addr(vq) >> PAGE_SHIFT;
+
+ /*
+ * virtio-mmio v1 uses a 32bit QUEUE PFN. If we have something
+ * that doesn't fit in 32bit, fail the setup rather than
+ * pretending to be successful.
+ */
+ if (q_pfn >> 32) {
+ dev_err(&vdev->dev,
+ "platform bug: legacy virtio-mmio must not be used with RAM above 0x%llxGB\n",
+ 0x1ULL << (32 + PAGE_SHIFT - 30));
+ return -E2BIG;
+ }
+
+ writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_QUEUE_ALIGN);
+ writel(q_pfn, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN);
+ } else {
+ u64 addr;
+
+ addr = virtqueue_get_desc_addr(vq);
+ writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_LOW);
+ writel((u32)(addr >> 32),
+ vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_HIGH);
+
+ addr = virtqueue_get_avail_addr(vq);
+ writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_LOW);
+ writel((u32)(addr >> 32),
+ vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_HIGH);
+
+ addr = virtqueue_get_used_addr(vq);
+ writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_USED_LOW);
+ writel((u32)(addr >> 32),
+ vm_dev->base + VIRTIO_MMIO_QUEUE_USED_HIGH);
+
+ writel(1, vm_dev->base + VIRTIO_MMIO_QUEUE_READY);
+ }
+
+ return 0;
+}
+
+static int vm_reset_vqs(struct virtio_device *vdev)
+{
+ struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
+ struct virtqueue *vq;
+ int err;
+
+ virtio_device_for_each_vq(vdev, vq) {
+ /* Re-initialize vring state */
+ err = virtqueue_reinit_vring(vq);
+ if (err < 0)
+ return err;
+
+ /* Select the queue we're interested in */
+ writel(vq->index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL);
+
+ /* Activate the queue */
+ err = vm_active_vq(vdev, vq);
+ if (err < 0)
+ return err;
+ }
+
+ return 0;
+}
+
static void vm_synchronize_cbs(struct virtio_device *vdev)
{
struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
@@ -388,45 +459,9 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned int in
vq->num_max = num;
/* Activate the queue */
- writel(virtqueue_get_vring_size(vq), vm_dev->base + VIRTIO_MMIO_QUEUE_NUM);
- if (vm_dev->version == 1) {
- u64 q_pfn = virtqueue_get_desc_addr(vq) >> PAGE_SHIFT;
-
- /*
- * virtio-mmio v1 uses a 32bit QUEUE PFN. If we have something
- * that doesn't fit in 32bit, fail the setup rather than
- * pretending to be successful.
- */
- if (q_pfn >> 32) {
- dev_err(&vdev->dev,
- "platform bug: legacy virtio-mmio must not be used with RAM above 0x%llxGB\n",
- 0x1ULL << (32 + PAGE_SHIFT - 30));
- err = -E2BIG;
- goto error_bad_pfn;
- }
-
- writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_QUEUE_ALIGN);
- writel(q_pfn, vm_dev->base + VIRTIO_MMIO_QUEUE_PFN);
- } else {
- u64 addr;
-
- addr = virtqueue_get_desc_addr(vq);
- writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_LOW);
- writel((u32)(addr >> 32),
- vm_dev->base + VIRTIO_MMIO_QUEUE_DESC_HIGH);
-
- addr = virtqueue_get_avail_addr(vq);
- writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_LOW);
- writel((u32)(addr >> 32),
- vm_dev->base + VIRTIO_MMIO_QUEUE_AVAIL_HIGH);
-
- addr = virtqueue_get_used_addr(vq);
- writel((u32)addr, vm_dev->base + VIRTIO_MMIO_QUEUE_USED_LOW);
- writel((u32)(addr >> 32),
- vm_dev->base + VIRTIO_MMIO_QUEUE_USED_HIGH);
-
- writel(1, vm_dev->base + VIRTIO_MMIO_QUEUE_READY);
- }
+ err = vm_active_vq(vdev, vq);
+ if (err < 0)
+ goto error_bad_pfn;
return vq;
@@ -528,11 +563,13 @@ static const struct virtio_config_ops virtio_mmio_config_ops = {
.reset = vm_reset,
.find_vqs = vm_find_vqs,
.del_vqs = vm_del_vqs,
+ .reset_vqs = vm_reset_vqs,
.get_features = vm_get_features,
.finalize_features = vm_finalize_features,
.bus_name = vm_bus_name,
.get_shm_region = vm_get_shm_region,
.synchronize_cbs = vm_synchronize_cbs,
+ .noirq_safe = true,
};
#ifdef CONFIG_PM_SLEEP
@@ -546,15 +583,35 @@ static int virtio_mmio_freeze(struct device *dev)
static int virtio_mmio_restore(struct device *dev)
{
struct virtio_mmio_device *vm_dev = dev_get_drvdata(dev);
+ struct virtio_driver *drv = drv_to_virtio(vm_dev->vdev.dev.driver);
- if (vm_dev->version == 1)
+ if (vm_dev->version == 1 && (!drv || !drv->restore_noirq))
writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
return virtio_device_restore(&vm_dev->vdev);
}
+static int virtio_mmio_freeze_noirq(struct device *dev)
+{
+ struct virtio_mmio_device *vm_dev = dev_get_drvdata(dev);
+
+ return virtio_device_freeze_noirq(&vm_dev->vdev);
+}
+
+static int virtio_mmio_restore_noirq(struct device *dev)
+{
+ struct virtio_mmio_device *vm_dev = dev_get_drvdata(dev);
+
+ if (vm_dev->version == 1)
+ writel(PAGE_SIZE, vm_dev->base + VIRTIO_MMIO_GUEST_PAGE_SIZE);
+
+ return virtio_device_restore_noirq(&vm_dev->vdev);
+}
+
static const struct dev_pm_ops virtio_mmio_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(virtio_mmio_freeze, virtio_mmio_restore)
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(virtio_mmio_freeze_noirq,
+ virtio_mmio_restore_noirq)
};
#endif
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] hv_sock: fix ARM64 support
From: Hamza Mahfooz @ 2026-04-28 17:15 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Long Li, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Michael Kelley, Himadri Pandya,
linux-hyperv, virtualization, linux-kernel
In-Reply-To: <afCxfHKA7hJilGM3@sgarzare-redhat>
On Tue, Apr 28, 2026 at 03:12:40PM +0200, Stefano Garzarella wrote:
> No version number in the subject?
I generally on increment the version number if I make changes to the code
itself I did try to change the subject prefix to "PATCH RESEND" but it
appears something is off about my config that prevented from going
through.
>
> Please next time follow
> https://docs.kernel.org/process/submitting-patches.html#subject-line
>
> Common tags might include a version descriptor if the multiple versions
> of the patch have been sent out in response to comments (i.e., “v1, v2,
> v3”), or “RFC” to indicate a request for comments.
>
> On Tue, Apr 28, 2026 at 08:53:39AM -0400, Hamza Mahfooz wrote:
> > VMBUS ring buffers must be page aligned. Therefore, the current value of
> > 24K presents a challenge on ARM64 kernels (with 64K pages). So, use
> > VMBUS_RING_SIZE() to ensure they are always aligned and large enough to
> > hold all of the relevant data.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 77ffe33363c0 ("hv_sock: use HV_HYP_PAGE_SIZE for Hyper-V communication")
> > Tested-by: Dexuan Cui <decui@microsoft.com>
> > Reviewed-by: Dexuan Cui <decui@microsoft.com>
> > Signed-off-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
> > ---
> > net/vmw_vsock/hyperv_transport.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> Acked-by: Stefano Garzarella <sgarzare@redhat.com>
>
> >
> > diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> > index 069386a74557..40f09b23efa3 100644
> > --- a/net/vmw_vsock/hyperv_transport.c
> > +++ b/net/vmw_vsock/hyperv_transport.c
> > @@ -375,10 +375,10 @@ static void hvs_open_connection(struct vmbus_channel *chan)
> > } else {
> > sndbuf = max_t(int, sk->sk_sndbuf, RINGBUFFER_HVS_SND_SIZE);
> > sndbuf = min_t(int, sndbuf, RINGBUFFER_HVS_MAX_SIZE);
> > - sndbuf = ALIGN(sndbuf, HV_HYP_PAGE_SIZE);
> > + sndbuf = VMBUS_RING_SIZE(sndbuf);
> > rcvbuf = max_t(int, sk->sk_rcvbuf, RINGBUFFER_HVS_RCV_SIZE);
> > rcvbuf = min_t(int, rcvbuf, RINGBUFFER_HVS_MAX_SIZE);
> > - rcvbuf = ALIGN(rcvbuf, HV_HYP_PAGE_SIZE);
> > + rcvbuf = VMBUS_RING_SIZE(rcvbuf);
> > }
> >
> > chan->max_pkt_size = HVS_MAX_PKT_SIZE;
> > --
> > 2.54.0
> >
>
^ permalink raw reply
* RE: [PATCH 2/2] drm/hyperv: use VMBUS_RING_SIZE()
From: Dexuan Cui @ 2026-04-28 21:05 UTC (permalink / raw)
To: Michael Kelley, Hamza Mahfooz, Saurabh Singh Sengar
Cc: linux-kernel@vger.kernel.org, KY Srinivasan, Haiyang Zhang,
Wei Liu, Long Li, Stefano Garzarella, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Himadri Pandya, linux-hyperv@vger.kernel.org,
virtualization@lists.linux.dev, netdev@vger.kernel.org,
Saurabh Sengar, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Deepak Rawat,
dri-devel@lists.freedesktop.org, stable@kernel.vger.org
In-Reply-To: <SN6PR02MB41571A5B77A5FDDFE17AEF19D4362@SN6PR02MB4157.namprd02.prod.outlook.com>
> From: Michael Kelley <mhklinux@outlook.com>
> Sent: Monday, April 27, 2026 12:06 PM
> > IMO the Fixes tag is unnecessary because the existing
> > VMBUS_RING_BUFSIZE
> > is 256KB, which is already aligned to 4KB, 16KB and 64KB.
> >
> > VMBUS_RING_SIZE(256 * 1024) is still 256KB.
>
> Not always. If PAGE_SIZE is 64KiB, VMBUS_RING_SIZE(256 * 1024) is
> 320KiB. If PAGE_SIZE is 16KiB or 4KiB, then VMBUS_RING_SIZE(256 * 1024)
> is indeed 256 KiB. See the explanation in the comment for
> VMBUS_RING_SIZE.
>
> Michael
Thanks for correcting me!
I didn't realize that sizeof(struct hv_ring_buffer) is based on
PAGE_SIZE, not on HV_HYP_PAGE_SIZE.
However, it looks like the Fixes tag is still not needed:
without the patch, we always pass two arguments of 256KB to
vmbus_open().
with the patch, we still pass 256KB to vmbus_open() in the case of
PAGE_SIZE=4KB or 16KB, and we pass 320KB in the case of
PAGE_SIZE=64KB.
Both 320K and 256KB are multiples of PAGE_SIZE, so
vmbus_open() -> vmbus_alloc_ring() doesn't return -EINVAL.
In the case of PAGE_SIZE=64KB, it's OK to pass 256KB to vmbus_open()
here since the hyperv-drm driver doesn't really have to use a slightly
bigger VMBus ringbuffer size.
Thanks,
Dexuan
^ permalink raw reply
* RE: [PATCH 2/2] drm/hyperv: use VMBUS_RING_SIZE()
From: Dexuan Cui @ 2026-04-28 21:07 UTC (permalink / raw)
To: Hamza Mahfooz, linux-kernel@vger.kernel.org
Cc: KY Srinivasan, Haiyang Zhang, Wei Liu, Long Li,
Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Himadri Pandya, Michael Kelley,
linux-hyperv@vger.kernel.org, virtualization@lists.linux.dev,
netdev@vger.kernel.org, Saurabh Sengar, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Deepak Rawat, dri-devel@lists.freedesktop.org,
stable@kernel.vger.org
In-Reply-To: <20260425181719.1538483-2-hamzamahfooz@linux.microsoft.com>
> From: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
> Sent: Saturday, April 25, 2026 11:17 AM
>
> Cc: stable@kernel.vger.org
I think this should be
Cc: stable@vger.kernel.org
^ permalink raw reply
* RE: [PATCH] hv_sock: fix ARM64 support
From: Dexuan Cui @ 2026-04-28 21:24 UTC (permalink / raw)
To: Hamza Mahfooz, netdev@vger.kernel.org
Cc: KY Srinivasan, Haiyang Zhang, Wei Liu, Long Li,
Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Michael Kelley, Himadri Pandya,
linux-hyperv@vger.kernel.org, virtualization@lists.linux.dev,
linux-kernel@vger.kernel.org
In-Reply-To: <20260428125339.13963-1-hamzamahfooz@linux.microsoft.com>
> From: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
> Sent: Tuesday, April 28, 2026 5:54 AM
> Subject: [PATCH] hv_sock: fix ARM64 support
Typically, for a change to net/, you'd want to add a "net" or "net-next"
after the "PATCH", i.e.
[PATCH net]
or
[PATCH net v2]
See "Documentation/process/maintainer-netdev.rst"
^ permalink raw reply
* Re: [PATCH] hv_sock: fix ARM64 support
From: Jakub Kicinski @ 2026-04-28 23:48 UTC (permalink / raw)
To: Dexuan Cui
Cc: Hamza Mahfooz, netdev@vger.kernel.org, KY Srinivasan,
Haiyang Zhang, Wei Liu, Long Li, Stefano Garzarella,
David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
Michael Kelley, Himadri Pandya, linux-hyperv@vger.kernel.org,
virtualization@lists.linux.dev, linux-kernel@vger.kernel.org
In-Reply-To: <SA1PR21MB69211500C7F60FC29F1BAA79BF372@SA1PR21MB6921.namprd21.prod.outlook.com>
On Tue, 28 Apr 2026 21:24:59 +0000 Dexuan Cui wrote:
> > Sent: Tuesday, April 28, 2026 5:54 AM
> > Subject: [PATCH] hv_sock: fix ARM64 support
>
> Typically, for a change to net/, you'd want to add a "net" or "net-next"
> after the "PATCH", i.e.
>
> [PATCH net]
> or
> [PATCH net v2]
>
> See "Documentation/process/maintainer-netdev.rst"
Speaking of Documentation/process/maintainer-netdev.rst:
Reviewer guidance
-----------------
[...]
Reviewers are highly encouraged to do more in-depth review of submissions
and not focus exclusively on process issues, trivial or subjective
matters like code formatting, tags etc.
See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#reviewer-guidance
^ permalink raw reply
* Re: [PATCH v12 05/13] blk-mq: add blk_mq_{online|possible}_queue_affinity
From: Hannes Reinecke @ 2026-04-29 7:15 UTC (permalink / raw)
To: Daniel Wagner, Sebastian Andrzej Siewior
Cc: Aaron Tomlin, axboe, kbusch, hch, sagi, mst, aacraid,
James.Bottomley, martin.petersen, liyihang9, kashyap.desai,
sumit.saxena, shivasharan.srikanteshwara, chandrakanth.patil,
sathya.prakash, sreekanth.reddy, suganath-prabu.subramani,
ranjan.kumar, jinpu.wang, tglx, mingo, peterz, juri.lelli,
vincent.guittot, akpm, maz, ruanjinjie, yphbchou0911, wagi,
frederic, longman, chenridong, kch, ming.lei, tom.leiming, steve,
sean, chjohnst, neelx, mproche, nick.lange, marco.crivellari,
linux-block, linux-kernel, virtualization, linux-nvme, linux-scsi,
megaraidlinux.pdl, mpi3mr-linuxdrv.pdl, MPT-FusionLinux.pdl
In-Reply-To: <c4927a2b-c18d-42ce-8a60-d6d388155671@flourine.local>
On 4/28/26 14:53, Daniel Wagner wrote:
> On Mon, Apr 27, 2026 at 05:34:16PM +0200, Sebastian Andrzej Siewior wrote:
>> Which driver uses cpu_possible_mask? This mask is assigned at boot time
>> once the kernel figured how many CPUs are possible based on ACPI or
>> whatever the system uses. This mask does not change.
>>
>> I only see drivers/scsi/lpfc/lpfc_init.c using it. Looking at
>> cpu_possible_mask might not be the right thing. It is usually the same
>> thing as "online" except on system where ACPI thinks that something
>> could be added via hotplug _or_ if the admin shuts down a CPU via
>> cpuhotplug _or_ boots with less (there a command line option for
>> that).
>
> These HBAs are used on PowerPC which supports lpar (CPUs can be added
> during runtime) I am told it's properly the only driver which is caring
> about this type configuration, thus a bit of an odd ball.
>
>> In case cpu_possible_mask != cpu_online_mask the intention is to
>> allocate memory and setup irqs for the offline CPUs?
>
> I can't answer this. The lpfc driver has several strategies implemented
> how it spreads its resources.
The intention is to setup the driver queue affinity only once (knowing
that all possible CPUs are handled correctly), avoiding a driver
reconfiguration on CPU hotplug.
Not the most efficient one, sure.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
^ permalink raw reply
* [PATCH v2 0/3] drm/virtio: introduce F_BLOB_ALIGNMENT support
From: Sergio Lopez @ 2026-04-28 19:44 UTC (permalink / raw)
To: Chia-I Wu, Jason Wang, Michael S. Tsirkin, Eugenio Pérez,
Xuan Zhuo, linux-kernel, Simona Vetter, Dmitry Osipenko,
Thomas Zimmermann, David Airlie, Gurchetan Singh, Gerd Hoffmann,
virtualization, dri-devel, Maxime Ripard, Maarten Lankhorst
Cc: Sergio Lopez
There's an increasing number of machines supporting multiple page sizes
and on these machines the host and a guest can be running, each one,
with a different page size.
For what pertains to virtio-gpu, this is not a problem if the page size
of the guest happens to be bigger or equal than the host, but will
potentially lead to failures in memory allocations and/or mappings
otherwise.
To deal with this, the virtio-spec was extended to introduce with the
VIRTIO_GPU_F_BLOB_ALIGNMENT feature [1]. If this feature is negotiated,
we must use the "blob_alignment" field in the config to ensure every
CREATE_BLOB and MAP_BLOB is properly aligned before sending it to the
device.
We also introduce the VIRTGPU_PARAM_BLOB_ALIGNMENT parameter to allow
userspace to query the alignment restrictions for blobs.
This supersedes "drm/virtio: introduce the HOST_PAGE_SIZE feature" [2].
[1] https://github.com/oasis-tcs/virtio-spec/commit/f9abfd55cb663837dda1153b826216dcf4d25b84
[2] https://lkml.org/lkml/2024/7/23/438
Changes in v2:
- Rebased.
- Moved blob size alignment validation to verify_blob(), rejecting
misaligned sizes early in the ioctl path instead of checking in
virtio_gpu_cmd_resource_create_blob() and virtio_gpu_cmd_map()
(Dmitry Osipenko).
Sergio Lopez (3):
drm/virtio: support VIRTIO_GPU_F_BLOB_ALIGNMENT
drm/virtio: honor blob_alignment requirements
drm/virtio: add VIRTGPU_PARAM_BLOB_ALIGNMENT to params
drivers/gpu/drm/virtio/virtgpu_drv.c | 1 +
drivers/gpu/drm/virtio/virtgpu_drv.h | 2 ++
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 10 ++++++++++
drivers/gpu/drm/virtio/virtgpu_kms.c | 14 +++++++++++---
include/uapi/drm/virtgpu_drm.h | 1 +
include/uapi/linux/virtio_gpu.h | 9 +++++++++
6 files changed, 34 insertions(+), 3 deletions(-)
--
2.53.0
^ permalink raw reply
* [PATCH v2 1/3] drm/virtio: support VIRTIO_GPU_F_BLOB_ALIGNMENT
From: Sergio Lopez @ 2026-04-28 19:44 UTC (permalink / raw)
To: Chia-I Wu, Jason Wang, Michael S. Tsirkin, Eugenio Pérez,
Xuan Zhuo, linux-kernel, Simona Vetter, Dmitry Osipenko,
Thomas Zimmermann, David Airlie, Gurchetan Singh, Gerd Hoffmann,
virtualization, dri-devel, Maxime Ripard, Maarten Lankhorst
Cc: Sergio Lopez
In-Reply-To: <20260428194450.518296-1-slp@redhat.com>
Support VIRTIO_GPU_F_BLOB_ALIGNMENT, a feature that indicates the device
provides a valid blob_alignment field in its configuration, and that
both RESOURCE_CREATE_BLOB and RESOURCE_MAP_BLOB requests must be aligned
to that value.
Signed-off-by: Sergio Lopez <slp@redhat.com>
---
drivers/gpu/drm/virtio/virtgpu_drv.c | 1 +
drivers/gpu/drm/virtio/virtgpu_drv.h | 2 ++
drivers/gpu/drm/virtio/virtgpu_kms.c | 14 +++++++++++---
include/uapi/linux/virtio_gpu.h | 9 +++++++++
4 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index a5ce96fb8a1d..812ee3f5e4aa 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -163,6 +163,7 @@ static unsigned int features[] = {
VIRTIO_GPU_F_RESOURCE_UUID,
VIRTIO_GPU_F_RESOURCE_BLOB,
VIRTIO_GPU_F_CONTEXT_INIT,
+ VIRTIO_GPU_F_BLOB_ALIGNMENT,
};
static struct virtio_driver virtio_gpu_driver = {
.feature_table = features,
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index f17660a71a3e..d1fa386a5a99 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -257,6 +257,7 @@ struct virtio_gpu_device {
bool has_resource_blob;
bool has_host_visible;
bool has_context_init;
+ bool has_blob_alignment;
struct virtio_shm_region host_visible_region;
struct drm_mm host_visible_mm;
@@ -270,6 +271,7 @@ struct virtio_gpu_device {
uint32_t num_capsets;
uint64_t capset_id_mask;
struct list_head cap_cache;
+ uint32_t blob_alignment;
/* protects uuid state when exporting */
spinlock_t resource_export_lock;
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 80ba69b4860b..cfde9f573df6 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -124,7 +124,7 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
struct virtio_gpu_device *vgdev;
/* this will expand later */
struct virtqueue *vqs[2];
- u32 num_scanouts, num_capsets;
+ u32 num_scanouts, num_capsets, blob_alignment;
int ret = 0;
if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
@@ -198,14 +198,22 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_CONTEXT_INIT))
vgdev->has_context_init = true;
+ if (virtio_has_feature(vgdev->vdev, VIRTIO_GPU_F_BLOB_ALIGNMENT)) {
+ vgdev->has_blob_alignment = true;
+ virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
+ blob_alignment, &blob_alignment);
+ vgdev->blob_alignment = blob_alignment;
+ }
+
DRM_INFO("features: %cvirgl %cedid %cresource_blob %chost_visible",
vgdev->has_virgl_3d ? '+' : '-',
vgdev->has_edid ? '+' : '-',
vgdev->has_resource_blob ? '+' : '-',
vgdev->has_host_visible ? '+' : '-');
- DRM_INFO("features: %ccontext_init\n",
- vgdev->has_context_init ? '+' : '-');
+ DRM_INFO("features: %ccontext_init %cblob_alignment\n",
+ vgdev->has_context_init ? '+' : '-',
+ vgdev->has_blob_alignment ? '+' : '-');
ret = virtio_find_vqs(vgdev->vdev, 2, vqs, vqs_info, NULL);
if (ret) {
diff --git a/include/uapi/linux/virtio_gpu.h b/include/uapi/linux/virtio_gpu.h
index be109777d10d..4f530d90058c 100644
--- a/include/uapi/linux/virtio_gpu.h
+++ b/include/uapi/linux/virtio_gpu.h
@@ -64,6 +64,14 @@
* context_init and multiple timelines
*/
#define VIRTIO_GPU_F_CONTEXT_INIT 4
+/*
+ * The device provides a valid blob_alignment
+ * field in its configuration and both
+ * VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB and
+ * VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB requests
+ * must be aligned to that value.
+ */
+#define VIRTIO_GPU_F_BLOB_ALIGNMENT 5
enum virtio_gpu_ctrl_type {
VIRTIO_GPU_UNDEFINED = 0,
@@ -365,6 +373,7 @@ struct virtio_gpu_config {
__le32 events_clear;
__le32 num_scanouts;
__le32 num_capsets;
+ __le32 blob_alignment;
};
/* simple formats for fbcon/X use */
--
2.53.0
^ permalink raw reply related
* [PATCH v2 2/3] drm/virtio: honor blob_alignment requirements
From: Sergio Lopez @ 2026-04-28 19:44 UTC (permalink / raw)
To: Chia-I Wu, Jason Wang, Michael S. Tsirkin, Eugenio Pérez,
Xuan Zhuo, linux-kernel, Simona Vetter, Dmitry Osipenko,
Thomas Zimmermann, David Airlie, Gurchetan Singh, Gerd Hoffmann,
virtualization, dri-devel, Maxime Ripard, Maarten Lankhorst
Cc: Sergio Lopez
In-Reply-To: <20260428194450.518296-1-slp@redhat.com>
If VIRTIO_GPU_F_BLOB_ALIGNMENT has been negotiated, blob size must be
aligned to blob_alignment. Validate this in verify_blob() so that
invalid requests are rejected early.
Signed-off-by: Sergio Lopez <slp@redhat.com>
---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index c33c057365f8..d0c4edf1eaf4 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -489,6 +489,11 @@ static int verify_blob(struct virtio_gpu_device *vgdev,
params->size = rc_blob->size;
params->blob = true;
params->blob_flags = rc_blob->blob_flags;
+
+ if (vgdev->has_blob_alignment &&
+ !IS_ALIGNED(params->size, vgdev->blob_alignment))
+ return -EINVAL;
+
return 0;
}
--
2.53.0
^ permalink raw reply related
* [PATCH v2 3/3] drm/virtio: add VIRTGPU_PARAM_BLOB_ALIGNMENT to params
From: Sergio Lopez @ 2026-04-28 19:44 UTC (permalink / raw)
To: Chia-I Wu, Jason Wang, Michael S. Tsirkin, Eugenio Pérez,
Xuan Zhuo, linux-kernel, Simona Vetter, Dmitry Osipenko,
Thomas Zimmermann, David Airlie, Gurchetan Singh, Gerd Hoffmann,
virtualization, dri-devel, Maxime Ripard, Maarten Lankhorst
Cc: Sergio Lopez
In-Reply-To: <20260428194450.518296-1-slp@redhat.com>
Add VIRTGPU_PARAM_BLOB_ALIGNMENT as a param that can be read with
VIRTGPU_GETPARAM by userspace applications running in the guest to
obtain the host's page size and find out the right alignment to be used
in shared memory allocations.
Signed-off-by: Sergio Lopez <slp@redhat.com>
---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 +++++
include/uapi/drm/virtgpu_drm.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index d0c4edf1eaf4..146ff25396c2 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -117,6 +117,11 @@ static int virtio_gpu_getparam_ioctl(struct drm_device *dev, void *data,
case VIRTGPU_PARAM_EXPLICIT_DEBUG_NAME:
value = vgdev->has_context_init ? 1 : 0;
break;
+ case VIRTGPU_PARAM_BLOB_ALIGNMENT:
+ if (!vgdev->has_blob_alignment)
+ return -ENOENT;
+ value = vgdev->blob_alignment;
+ break;
default:
return -EINVAL;
}
diff --git a/include/uapi/drm/virtgpu_drm.h b/include/uapi/drm/virtgpu_drm.h
index 9debb320c34b..7d4e4884d942 100644
--- a/include/uapi/drm/virtgpu_drm.h
+++ b/include/uapi/drm/virtgpu_drm.h
@@ -98,6 +98,7 @@ struct drm_virtgpu_execbuffer {
#define VIRTGPU_PARAM_CONTEXT_INIT 6 /* DRM_VIRTGPU_CONTEXT_INIT */
#define VIRTGPU_PARAM_SUPPORTED_CAPSET_IDs 7 /* Bitmask of supported capability set ids */
#define VIRTGPU_PARAM_EXPLICIT_DEBUG_NAME 8 /* Ability to set debug name from userspace */
+#define VIRTGPU_PARAM_BLOB_ALIGNMENT 9 /* Device alignment requirements for blobs */
struct drm_virtgpu_getparam {
__u64 param;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2 1/3] vfio/pci: Set up bar resources and maps in vfio_pci_core_enable()
From: Matt Evans @ 2026-04-29 10:15 UTC (permalink / raw)
To: Alex Williamson
Cc: Kevin Tian, Jason Gunthorpe, Ankit Agrawal, Alistair Popple,
Leon Romanovsky, Kees Cook, Shameer Kolothum, Yishai Hadas,
Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy,
Zhi Wang, kvm, linux-kernel, virtualization
In-Reply-To: <20260424112007.592fd6c0@shazbot.org>
Hi Alex,
On 24/04/2026 18:20, Alex Williamson wrote:
>
> On Fri, 24 Apr 2026 16:15:06 +0100
> Matt Evans <mattev@meta.com> wrote:
>> On 23/04/2026 22:30, Alex Williamson wrote:
>>> On Thu, 23 Apr 2026 11:25:07 -0700
>>> Matt Evans <mattev@meta.com> wrote:
>>>> +
>>>> + if (pci_resource_len(pdev, i) == 0)
>>>> + continue;
>>>> +
>>>> + ret = pci_request_selected_regions(pdev, 1 << bar, "vfio");
>>>> + if (ret) {
>>>> + pci_warn(vdev->pdev, "Failed to reserve region %d\n", bar);
>>>> + continue;
>>>> + }
>>>> + vdev->have_bar_resource[bar] = true;
>>>> +
>>>> + io = pci_iomap(pdev, bar, 0);
>>>> + if (io)
>>>> + vdev->barmap[bar] = io;
>>>> + else
>>>> + pci_warn(vdev->pdev, "Failed to iomap region %d\n", bar);
>>>> + }
>>>> +}
>>>
>>> I see you making the point in the cover letter about the resource
>>> request vs the iomap resource, but we currently handle these together.
>>> If either fails, setup barmap fails and the path returns error. I
>>> don't see any justification for now allowing the request resource to
>>> succeed but the iomap fails.
>>
>> The primary effect was to let consumers see -EBUSY for a resource
>> reservation failure or -ENOMEM for an iomap failure (whether through
>> this patch's vfio_pci_core_setup_barmap() or the next patch's helpers),
>> and that keeps the error signatures identical.
>>
>> A weak secondary effect was that a BAR that gets resource but fails for
>> whatever reason to iomap it can still be used by most clients (assuming
>> the general usage is to mmap). The system's pretty sick if this is the
>> case, so as I say it's weak.
>
> Right, I don't see that's really a necessary add at this point. In
> fact while we expect users to access through the mmap when available,
> we don't actually have a way to specify that mmap works w/o read/write,
> which is effectively what this proposes is a valid state.
>
>>
>> OK, if you prefer the combined approach and don't feel the subsequent
>> single-semantic check helpers (need mapping, need resource) are clearer
>> to read then I'll recombine them, though:
>>
>> - If vfio_pci_core_map_bars() just sets barmap[n] iff both resource
>> acquisition and iomap succeed, then a later check can only return one
>> error from either cause. I'll go with -ENOMEM unless you prefer -EBUSY.
>> Using something else can again make userspace see previously-unseen
>> error values.
>>
>> - IMHO vfio_pci_core_setup_barmap() should still be renamed (in a 2nd
>> patch) since it doesn't do any setting up anymore. Cosmetic, but
>> cleaner to parse when the callers use vfio_pci_core_check_barmap_valid() no?
>
> I'm not sure how important it is to preserve the identical errno, but
> we can actually do that too. Make the enable time "setup" function
> store the ERR_PTR in the barmap and change the current callers from
> "setup" to "get-iomap", where get-iomap is a __must_check return that
> callers test against IS_ERR_OR_NULL().
I like that! (My mental model of ERR_PTR was one of function return
values and must admit it didn't occur to me to stash one longer-term in
barmap.)
> Or maybe better, collapse NULL into -ENODEV so callers only test
> IS_ERR().
>
> There's even one user in vfio_pci_bar_rw() where this provides a minor
> simplification. Likely the others are just wrapping the get-iomap call
> in the ERR/NULL test to get the equivalent behavior. Thoughts? Thanks,
Just implemented a vfio_pci_core_get_iomap() and I think it's a lot
nicer. I also prefer the helper returning the BAR map pointer rather
than the open-coded vdev->barmap[]s. I've moved the helper [back] to a
header, since the additional index checks will then fold out at compile
time in some cases.
Thanks much for the suggestion, posting a v3 shortly.
Matt
^ permalink raw reply
* Re: [PATCH v2 2/3] vfio/pci: Replace vfio_pci_core_setup_barmap() with checks for resource/map
From: Matt Evans @ 2026-04-29 10:17 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Alex Williamson, Kevin Tian, Jason Gunthorpe, Ankit Agrawal,
Alistair Popple, Kees Cook, Shameer Kolothum, Yishai Hadas,
Alexey Kardashevskiy, Eric Auger, Peter Xu, Vivek Kasireddy,
Zhi Wang, kvm, linux-kernel, virtualization
In-Reply-To: <20260426110517.GB440345@unreal>
Hi Leon,
On 26/04/2026 12:05, Leon Romanovsky wrote:
> >
> On Thu, Apr 23, 2026 at 11:25:08AM -0700, Matt Evans wrote:
>> Since "vfio/pci: Set up barmap in vfio_pci_core_enable()", the
>> resource request and iomap for the BARs was performed early, and
>> vfio_pci_core_setup_barmap() now just checks those actions succeeded.
>>
>> There were two types of callers:
>> - Those that need the iomap, because they'll access the BAR
>> - Those that need the resource, because they'll map/export it
>>
>> This replaces vfio_pci_core_setup_barmap() with two helpers,
>> vfio_pci_core_check_barmap_valid() and vfio_pci_core_check_bar_rsrc(),
>> to make it clear which behaviour is required in each caller.
>>
>> Signed-off-by: Matt Evans <mattev@meta.com>
>> ---
>> drivers/vfio/pci/nvgrace-gpu/main.c | 8 +++-----
>> drivers/vfio/pci/vfio_pci_core.c | 5 ++---
>> drivers/vfio/pci/vfio_pci_rdwr.c | 22 ++--------------------
>> drivers/vfio/pci/virtio/legacy_io.c | 4 ++--
>> include/linux/vfio_pci_core.h | 23 ++++++++++++++++++++++-
>> 5 files changed, 31 insertions(+), 31 deletions(-)
>
> <...>
>
>> +/* Returns 0 if vdev->barmap[bar] can be accessed, otherwise errno */
>> +static inline int
>> +vfio_pci_core_check_barmap_valid(struct vfio_pci_core_device *vdev, int bar)
>> +{
>> + if (bar < 0 || bar >= PCI_STD_NUM_BARS)
>> + return -EINVAL;
>> + if (vdev->barmap[bar] == 0)
>
> I understand this line was copied from an earlier implementation, but
> barmap[bar] is a pointer, and it is set to NULL when it is released.
>
> if (!dev->barmap[bar])
> return -ENOMEM;
Absolutely. Fixed in upcoming v3, thank you,
Matt
>
> Thanks
>
>> + return -ENOMEM;
>> + return 0;
>> +}
>> +
>> +/* Returns 0 if BAR has a valid resource reserved for use, otherwise errno */
>> +static inline int vfio_pci_core_check_bar_rsrc(struct vfio_pci_core_device *vdev,
>> + int bar)
>> +{
>> + if (bar < 0 || bar >= PCI_STD_NUM_BARS)
>> + return -EINVAL;
>> + if (!vdev->have_bar_resource[bar])
>> + return -EBUSY;
>> + return 0;
>> +}
>> +
>> static inline bool is_aligned_for_order(struct vm_area_struct *vma,
>> unsigned long addr,
>> unsigned long pfn,
>> --
>> 2.47.3
>>
^ permalink raw reply
* [RFC PATCH] virtio-mmio: add xenbus probing
From: Val Packett @ 2026-04-29 13:52 UTC (permalink / raw)
To: Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez
Cc: Val Packett, Marek Marczykowski-Górecki, Viresh Kumar,
xen-devel, linux-kernel, virtualization
The experimental virtio-mmio support for Xen was initially developed
on aarch64, so device trees were used to configure the mmio devices,
with arbitrary vGIC interrupts used by the hypervisor. On x86_64
however, the only reasonable way to interrupt the guest is over Xen
event channels, which can only be acquired by children of xenbus,
the virtual bus driven by Xen's configuration database, XenStore.
It is also a more convenient and "Xen-ish" way to provision devices.
Implement a xenbus client for virtio-mmio which negotiates an
event channel and provides it as a platform IRQ to the
virtio-mmio driver.
Signed-off-by: Val Packett <val@invisiblethingslab.com>
---
Hi,
I've been working on porting virtio-mmio support from Arm to x86_64,
with the goal of running vhost-user-gpu to power Wayland/GPU integration
for Qubes OS. (I'm aware of various proposals for alternative virtio
transports but virtio-mmio seems to be the only one that *is* upstream
already and just Works..) Setting up virtio-mmio through xenbus, initially
motivated just by event channels being the only real way to get interrupts
working on HVM, turned out to generally be quite pleasant and nice :)
I'd like to get some early feedback for this patch, particularly
the general stuff:
* is this whole thing acceptable in general?
* should it be extracted into a different file?
* (from the Xen side) any input on the xenstore keys, what goes where?
* anything else to keep in mind?
It does seem simple enough, so hopefully this can be done?
The corresponding userspace-side WIP is available at:
https://github.com/QubesOS/xen-vhost-frontend
And the required DMOP for firing the evtchn events will be sent
to xen-devel shortly as well.
Thanks,
~val
---
drivers/virtio/Kconfig | 7 ++
drivers/virtio/virtio_mmio.c | 177 ++++++++++++++++++++++++++++++++++-
2 files changed, 183 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index ce5bc0d9ea28..56bc2b10526b 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -171,6 +171,13 @@ config VIRTIO_MMIO_CMDLINE_DEVICES
If unsure, say 'N'.
+config VIRTIO_MMIO_XENBUS
+ bool "Memory mapped virtio devices parameter parsing"
+ depends on VIRTIO_MMIO && XEN
+ select XEN_XENBUS_FRONTEND
+ help
+ Allow virtio-mmio devices instantiation for Xen guests via xenbus.
+
config VIRTIO_DMA_SHARED_BUFFER
tristate
depends on DMA_SHARED_BUFFER
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 595c2274fbb5..32295284bdbf 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -70,6 +70,11 @@
#include <uapi/linux/virtio_mmio.h>
#include <linux/virtio_ring.h>
+#ifdef CONFIG_VIRTIO_MMIO_XENBUS
+#include <xen/xen.h>
+#include <xen/xenbus.h>
+#include <xen/events.h>
+#endif
/* The alignment to use between consumer and producer parts of vring.
@@ -810,13 +815,183 @@ static struct platform_driver virtio_mmio_driver = {
},
};
+#ifdef CONFIG_VIRTIO_MMIO_XENBUS
+struct virtio_mmio_xen_info {
+ struct resource resources[2];
+ unsigned int evtchn;
+ struct platform_device *pdev;
+};
+
+static int virtio_mmio_xen_probe(struct xenbus_device *dev,
+ const struct xenbus_device_id *id)
+{
+ int err;
+ long long base, size;
+ char *mem;
+ struct virtio_mmio_xen_info *info;
+ struct xenbus_transaction xbt;
+
+ /* TODO: allocate an unused address here and pass it to the host instead */
+ err = xenbus_scanf(XBT_NIL, dev->otherend, "base", "0x%llx",
+ &base);
+ if (err < 0) {
+ xenbus_dev_fatal(dev, err, "reading base");
+ return -EINVAL;
+ }
+
+ mem = xenbus_read(XBT_NIL, dev->otherend, "size", NULL);
+ if (XENBUS_IS_ERR_READ(mem))
+ return PTR_ERR(mem);
+ size = memparse(mem, NULL);
+ kfree(mem);
+
+ info = kzalloc_obj(*info);
+ if (!info) {
+ xenbus_dev_fatal(dev, -ENOMEM, "allocating info structure");
+ return -ENOMEM;
+ }
+
+ info->resources[0].flags = IORESOURCE_MEM;
+ info->resources[0].start = base;
+ info->resources[0].end = base + size - 1;
+
+ err = xenbus_alloc_evtchn(dev, &info->evtchn);
+ if (err) {
+ xenbus_dev_fatal(dev, err, "xenbus_alloc_evtchn");
+ goto error_info;
+ }
+
+ err = bind_evtchn_to_irq(info->evtchn);
+ if (err <= 0) {
+ xenbus_dev_fatal(dev, err, "bind_evtchn_to_irq");
+ goto error_evtchan;
+ }
+
+ info->resources[1].flags = IORESOURCE_IRQ;
+ info->resources[1].start = info->resources[1].end = err;
+
+again:
+ err = xenbus_transaction_start(&xbt);
+ if (err) {
+ xenbus_dev_fatal(dev, err, "starting transaction");
+ goto error_irq;
+ }
+
+ err = xenbus_printf(xbt, dev->nodename, "event-channel", "%u",
+ info->evtchn);
+ if (err) {
+ xenbus_transaction_end(xbt, 1);
+ xenbus_dev_fatal(dev, err, "%s", "writing event-channel");
+ goto error_irq;
+ }
+
+ err = xenbus_transaction_end(xbt, 0);
+ if (err) {
+ if (err == -EAGAIN)
+ goto again;
+ xenbus_dev_fatal(dev, err, "completing transaction");
+ goto error_irq;
+ }
+
+ dev_set_drvdata(&dev->dev, info);
+ xenbus_switch_state(dev, XenbusStateInitialised);
+ return 0;
+
+error_irq:
+ unbind_from_irqhandler(info->resources[1].start, info);
+error_evtchan:
+ xenbus_free_evtchn(dev, info->evtchn);
+error_info:
+ kfree(info);
+
+ return err;
+}
+
+static void virtio_mmio_xen_backend_changed(struct xenbus_device *dev,
+ enum xenbus_state backend_state)
+{
+ struct virtio_mmio_xen_info *info = dev_get_drvdata(&dev->dev);
+
+ switch (backend_state) {
+ case XenbusStateInitialising:
+ case XenbusStateInitWait:
+ case XenbusStateInitialised:
+ case XenbusStateReconfiguring:
+ case XenbusStateReconfigured:
+ case XenbusStateUnknown:
+ break;
+
+ case XenbusStateConnected:
+ if (dev->state != XenbusStateInitialised) {
+ dev_warn(&dev->dev, "state %d on connect", dev->state);
+ break;
+ }
+ info->pdev = platform_device_register_resndata(&dev->dev,
+ "virtio-mmio", PLATFORM_DEVID_AUTO,
+ info->resources, ARRAY_SIZE(info->resources), NULL, 0);
+ xenbus_switch_state(dev, XenbusStateConnected);
+ break;
+
+ case XenbusStateClosed:
+ if (dev->state == XenbusStateClosed)
+ break;
+ fallthrough; /* Missed the backend's Closing state. */
+ case XenbusStateClosing:
+ platform_device_unregister(info->pdev);
+ xenbus_switch_state(dev, XenbusStateClosed);
+ break;
+
+ default:
+ xenbus_dev_fatal(dev, -EINVAL, "saw state %d at frontend",
+ backend_state);
+ break;
+ }
+}
+
+static void virtio_mmio_xen_remove(struct xenbus_device *dev)
+{
+ struct virtio_mmio_xen_info *info = dev_get_drvdata(&dev->dev);
+
+ kfree(info);
+ dev_set_drvdata(&dev->dev, NULL);
+}
+
+static const struct xenbus_device_id virtio_mmio_xen_ids[] = {
+ { "virtio" },
+ { "" },
+};
+
+static struct xenbus_driver virtio_mmio_xen_driver = {
+ .ids = virtio_mmio_xen_ids,
+ .probe = virtio_mmio_xen_probe,
+ .otherend_changed = virtio_mmio_xen_backend_changed,
+ .remove = virtio_mmio_xen_remove,
+};
+#endif
+
static int __init virtio_mmio_init(void)
{
- return platform_driver_register(&virtio_mmio_driver);
+ int ret;
+
+ ret = platform_driver_register(&virtio_mmio_driver);
+ if (ret)
+ return ret;
+
+#ifdef CONFIG_VIRTIO_MMIO_XENBUS
+ if (xen_domain())
+ ret = xenbus_register_frontend(&virtio_mmio_xen_driver);
+#endif
+
+ return ret;
}
static void __exit virtio_mmio_exit(void)
{
+#ifdef CONFIG_VIRTIO_MMIO_XENBUS
+ if (xen_domain())
+ xenbus_unregister_driver(&virtio_mmio_xen_driver);
+#endif
+
platform_driver_unregister(&virtio_mmio_driver);
vm_unregister_cmdline_devices();
}
--
2.53.0
^ permalink raw reply related
* Re: [RFC PATCH] virtio-mmio: add xenbus probing
From: Jürgen Groß @ 2026-04-29 15:35 UTC (permalink / raw)
To: Val Packett, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
Eugenio Pérez
Cc: Marek Marczykowski-Górecki, Viresh Kumar, xen-devel,
linux-kernel, virtualization
In-Reply-To: <20260429141339.74472-1-val@invisiblethingslab.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 9134 bytes --]
Some minor details from the Xen side of things:
On 29.04.26 15:52, Val Packett wrote:
> The experimental virtio-mmio support for Xen was initially developed
> on aarch64, so device trees were used to configure the mmio devices,
> with arbitrary vGIC interrupts used by the hypervisor. On x86_64
> however, the only reasonable way to interrupt the guest is over Xen
> event channels, which can only be acquired by children of xenbus,
More exact: interdomain event channels need to be connected to a xenbus
device. But you are needing those, so for your use case the above statement
is correct.
> the virtual bus driven by Xen's configuration database, XenStore.
> It is also a more convenient and "Xen-ish" way to provision devices.
>
> Implement a xenbus client for virtio-mmio which negotiates an
> event channel and provides it as a platform IRQ to the
> virtio-mmio driver.
>
>
> Signed-off-by: Val Packett <val@invisiblethingslab.com>
> ---
>
> Hi,
>
> I've been working on porting virtio-mmio support from Arm to x86_64,
> with the goal of running vhost-user-gpu to power Wayland/GPU integration
> for Qubes OS. (I'm aware of various proposals for alternative virtio
> transports but virtio-mmio seems to be the only one that *is* upstream
> already and just Works..) Setting up virtio-mmio through xenbus, initially
> motivated just by event channels being the only real way to get interrupts
> working on HVM, turned out to generally be quite pleasant and nice :)
>
> I'd like to get some early feedback for this patch, particularly
> the general stuff:
>
> * is this whole thing acceptable in general?
> * should it be extracted into a different file?
> * (from the Xen side) any input on the xenstore keys, what goes where?
You should add some documentation in the Xen source tree regarding the
Xenstore keys (see docs/misc/xenstore-paths.pandoc there).
> * anything else to keep in mind?
>
> It does seem simple enough, so hopefully this can be done?
>
> The corresponding userspace-side WIP is available at:
> https://github.com/QubesOS/xen-vhost-frontend
>
> And the required DMOP for firing the evtchn events will be sent
> to xen-devel shortly as well.
>
> Thanks,
> ~val
>
> ---
> drivers/virtio/Kconfig | 7 ++
> drivers/virtio/virtio_mmio.c | 177 ++++++++++++++++++++++++++++++++++-
> 2 files changed, 183 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index ce5bc0d9ea28..56bc2b10526b 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -171,6 +171,13 @@ config VIRTIO_MMIO_CMDLINE_DEVICES
>
> If unsure, say 'N'.
>
> +config VIRTIO_MMIO_XENBUS
> + bool "Memory mapped virtio devices parameter parsing"
> + depends on VIRTIO_MMIO && XEN
> + select XEN_XENBUS_FRONTEND
> + help
> + Allow virtio-mmio devices instantiation for Xen guests via xenbus.
> +
> config VIRTIO_DMA_SHARED_BUFFER
> tristate
> depends on DMA_SHARED_BUFFER
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 595c2274fbb5..32295284bdbf 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -70,6 +70,11 @@
> #include <uapi/linux/virtio_mmio.h>
> #include <linux/virtio_ring.h>
>
> +#ifdef CONFIG_VIRTIO_MMIO_XENBUS
> +#include <xen/xen.h>
> +#include <xen/xenbus.h>
> +#include <xen/events.h>
> +#endif
>
>
> /* The alignment to use between consumer and producer parts of vring.
> @@ -810,13 +815,183 @@ static struct platform_driver virtio_mmio_driver = {
> },
> };
>
> +#ifdef CONFIG_VIRTIO_MMIO_XENBUS
> +struct virtio_mmio_xen_info {
> + struct resource resources[2];
> + unsigned int evtchn;
> + struct platform_device *pdev;
> +};
> +
> +static int virtio_mmio_xen_probe(struct xenbus_device *dev,
> + const struct xenbus_device_id *id)
> +{
> + int err;
> + long long base, size;
> + char *mem;
> + struct virtio_mmio_xen_info *info;
> + struct xenbus_transaction xbt;
> +
> + /* TODO: allocate an unused address here and pass it to the host instead */
Indeed.
> + err = xenbus_scanf(XBT_NIL, dev->otherend, "base", "0x%llx",
> + &base);
> + if (err < 0) {
> + xenbus_dev_fatal(dev, err, "reading base");
> + return -EINVAL;
> + }
> +
> + mem = xenbus_read(XBT_NIL, dev->otherend, "size", NULL);
> + if (XENBUS_IS_ERR_READ(mem))
> + return PTR_ERR(mem);
> + size = memparse(mem, NULL);
> + kfree(mem);
> +
> + info = kzalloc_obj(*info);
> + if (!info) {
> + xenbus_dev_fatal(dev, -ENOMEM, "allocating info structure");
> + return -ENOMEM;
> + }
> +
> + info->resources[0].flags = IORESOURCE_MEM;
> + info->resources[0].start = base;
> + info->resources[0].end = base + size - 1;
> +
> + err = xenbus_alloc_evtchn(dev, &info->evtchn);
> + if (err) {
> + xenbus_dev_fatal(dev, err, "xenbus_alloc_evtchn");
> + goto error_info;
> + }
> +
> + err = bind_evtchn_to_irq(info->evtchn);
> + if (err <= 0) {
> + xenbus_dev_fatal(dev, err, "bind_evtchn_to_irq");
> + goto error_evtchan;
> + }
> +
> + info->resources[1].flags = IORESOURCE_IRQ;
> + info->resources[1].start = info->resources[1].end = err;
> +
> +again:
> + err = xenbus_transaction_start(&xbt);
No need to use a Xenstore transaction here. The written node(s) are
regarded to be valid only after calling xenbus_switch_state() to set
the frontend state to XenbusStateInitialised.
> + if (err) {
> + xenbus_dev_fatal(dev, err, "starting transaction");
> + goto error_irq;
> + }
> +
> + err = xenbus_printf(xbt, dev->nodename, "event-channel", "%u",
> + info->evtchn);
With allocation of the base address you'd want to write it to another node,
of course.
> + if (err) {
> + xenbus_transaction_end(xbt, 1);
> + xenbus_dev_fatal(dev, err, "%s", "writing event-channel");
> + goto error_irq;
> + }
> +
> + err = xenbus_transaction_end(xbt, 0);
> + if (err) {
> + if (err == -EAGAIN)
> + goto again;
> + xenbus_dev_fatal(dev, err, "completing transaction");
> + goto error_irq;
> + }
> +
> + dev_set_drvdata(&dev->dev, info);
> + xenbus_switch_state(dev, XenbusStateInitialised);
> + return 0;
> +
> +error_irq:
> + unbind_from_irqhandler(info->resources[1].start, info);
> +error_evtchan:
> + xenbus_free_evtchn(dev, info->evtchn);
> +error_info:
> + kfree(info);
> +
> + return err;
> +}
> +
> +static void virtio_mmio_xen_backend_changed(struct xenbus_device *dev,
> + enum xenbus_state backend_state)
> +{
> + struct virtio_mmio_xen_info *info = dev_get_drvdata(&dev->dev);
> +
> + switch (backend_state) {
> + case XenbusStateInitialising:
> + case XenbusStateInitWait:
> + case XenbusStateInitialised:
> + case XenbusStateReconfiguring:
> + case XenbusStateReconfigured:
> + case XenbusStateUnknown:
> + break;
> +
> + case XenbusStateConnected:
> + if (dev->state != XenbusStateInitialised) {
> + dev_warn(&dev->dev, "state %d on connect", dev->state);
> + break;
> + }
> + info->pdev = platform_device_register_resndata(&dev->dev,
> + "virtio-mmio", PLATFORM_DEVID_AUTO,
> + info->resources, ARRAY_SIZE(info->resources), NULL, 0);
> + xenbus_switch_state(dev, XenbusStateConnected);
> + break;
> +
> + case XenbusStateClosed:
> + if (dev->state == XenbusStateClosed)
> + break;
> + fallthrough; /* Missed the backend's Closing state. */
> + case XenbusStateClosing:
> + platform_device_unregister(info->pdev);
> + xenbus_switch_state(dev, XenbusStateClosed);
> + break;
> +
> + default:
> + xenbus_dev_fatal(dev, -EINVAL, "saw state %d at frontend",
> + backend_state);
> + break;
> + }
> +}
> +
> +static void virtio_mmio_xen_remove(struct xenbus_device *dev)
> +{
> + struct virtio_mmio_xen_info *info = dev_get_drvdata(&dev->dev);
> +
> + kfree(info);
> + dev_set_drvdata(&dev->dev, NULL);
> +}
> +
> +static const struct xenbus_device_id virtio_mmio_xen_ids[] = {
> + { "virtio" },
Please use "virtio-mmio" here, as I could imagine "virtio-pci" devices, too.
Juergen
> + { "" },
> +};
> +
> +static struct xenbus_driver virtio_mmio_xen_driver = {
> + .ids = virtio_mmio_xen_ids,
> + .probe = virtio_mmio_xen_probe,
> + .otherend_changed = virtio_mmio_xen_backend_changed,
> + .remove = virtio_mmio_xen_remove,
> +};
> +#endif
> +
> static int __init virtio_mmio_init(void)
> {
> - return platform_driver_register(&virtio_mmio_driver);
> + int ret;
> +
> + ret = platform_driver_register(&virtio_mmio_driver);
> + if (ret)
> + return ret;
> +
> +#ifdef CONFIG_VIRTIO_MMIO_XENBUS
> + if (xen_domain())
> + ret = xenbus_register_frontend(&virtio_mmio_xen_driver);
> +#endif
> +
> + return ret;
> }
>
> static void __exit virtio_mmio_exit(void)
> {
> +#ifdef CONFIG_VIRTIO_MMIO_XENBUS
> + if (xen_domain())
> + xenbus_unregister_driver(&virtio_mmio_xen_driver);
> +#endif
> +
> platform_driver_unregister(&virtio_mmio_driver);
> vm_unregister_cmdline_devices();
> }
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox