* [PATCH net] net/mlx5: report duplex full when speed is known
From: Li Tian @ 2025-09-13 6:28 UTC (permalink / raw)
To: netdev, linux-hyperv
Cc: linux-kernel, Saeed Mahameed, Tariq Toukan, Mark Bloch,
Leon Romanovsky, linux-rdma, Haiyang Zhang
Prior commit in Fixes, duplex is always reported full as long
as the speed is known. Restore this behavior. Besides, modern
Mellanox doesn't seem to care about half duplex. This change
mitigates duplex unknown issue on Azure Mellanox 5.
Fixes: c268ca6087f55 ("net/mlx5: Expose port speed when possible")
Signed-off-by: Li Tian <litian@redhat.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index d507366d773e..9f35d3b491e0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -1118,9 +1118,11 @@ static void get_link_properties(struct net_device *netdev,
if (info) {
speed = info->speed;
lanes = info->lanes;
- duplex = DUPLEX_FULL;
} else if (data_rate_oper)
speed = 100 * data_rate_oper;
+ if (!speed)
+ goto out;
+ duplex = DUPLEX_FULL;
out:
link_ksettings->base.duplex = duplex;
--
2.50.0
^ permalink raw reply related
* Re: [PATCH net] net/mlx5: report duplex full when speed is known
From: Andrew Lunn @ 2025-09-13 13:47 UTC (permalink / raw)
To: Li Tian
Cc: netdev, linux-hyperv, linux-kernel, Saeed Mahameed, Tariq Toukan,
Mark Bloch, Leon Romanovsky, linux-rdma, Haiyang Zhang
In-Reply-To: <20250913062810.11141-1-litian@redhat.com>
On Sat, Sep 13, 2025 at 02:28:10PM +0800, Li Tian wrote:
> Prior commit in Fixes, duplex is always reported full as long
> as the speed is known. Restore this behavior. Besides, modern
> Mellanox doesn't seem to care about half duplex. This change
> mitigates duplex unknown issue on Azure Mellanox 5.
>
> Fixes: c268ca6087f55 ("net/mlx5: Expose port speed when possible")
I'm confused with your commit message. You say DUPLEX used to be
reported as Full if the speed is known. How does c268ca6087f55 change
this? You don't say in the commit message. Why is Half duplex
important to this fix? I don't see Half anywhere in the code.
Also, what sort of problems do you see with duplex unknown? When
somebody has a problem and is looking to find a patch which might fix
it, seeing a description of the problem fixed in the commit message is
useful.
Andrew
^ permalink raw reply
* [PATCH 1/3] drivers: hv: vmbus: Fix sysfs output format for ring buffer index
From: Alok Tiwari @ 2025-09-13 19:24 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, linux-hyperv; +Cc: alok.a.tiwari, linux-kernel
The sysfs attributes out_read_index and out_write_index in
vmbus_drv.c currently use %d to print outbound.current_read_index
and outbound.current_write_index.
These fields are u32 values, so printing them with %d (signed) is
not logically correct. Update the format specifier to %u to
correctly match their type.
No functional change, only fixes the sysfs output format.
Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
---
drivers/hv/vmbus_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 2ed5a1e89d69..8b58306cb140 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -322,7 +322,7 @@ static ssize_t out_read_index_show(struct device *dev,
&outbound);
if (ret < 0)
return ret;
- return sysfs_emit(buf, "%d\n", outbound.current_read_index);
+ return sysfs_emit(buf, "%u\n", outbound.current_read_index);
}
static DEVICE_ATTR_RO(out_read_index);
@@ -341,7 +341,7 @@ static ssize_t out_write_index_show(struct device *dev,
&outbound);
if (ret < 0)
return ret;
- return sysfs_emit(buf, "%d\n", outbound.current_write_index);
+ return sysfs_emit(buf, "%u\n", outbound.current_write_index);
}
static DEVICE_ATTR_RO(out_write_index);
--
2.50.1
^ permalink raw reply related
* [PATCH 2/3] drivers: hv: vmbus: Fix sscanf format specifier in target_cpu_store()
From: Alok Tiwari @ 2025-09-13 19:24 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, linux-hyperv; +Cc: alok.a.tiwari, linux-kernel
In-Reply-To: <20250913192450.4134957-1-alok.a.tiwari@oracle.com>
The target_cpu_store() function parses the target CPU from the sysfs
buffer using sscanf(). The format string currently uses "%uu", which
is invalid and causes incorrect parsing.
Fix it by using "%u" to correctly read the value.
This only fixes the parsing format.
Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
---
drivers/hv/vmbus_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 8b58306cb140..fbab9f2d7fa6 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1742,7 +1742,7 @@ static ssize_t target_cpu_store(struct vmbus_channel *channel,
u32 target_cpu;
ssize_t ret;
- if (sscanf(buf, "%uu", &target_cpu) != 1)
+ if (sscanf(buf, "%u", &target_cpu) != 1)
return -EIO;
cpus_read_lock();
--
2.50.1
^ permalink raw reply related
* [PATCH 3/3] drivers: hv: vmbus: Fix typos in vmbus_drv.c
From: Alok Tiwari @ 2025-09-13 19:24 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, linux-hyperv; +Cc: alok.a.tiwari, linux-kernel
In-Reply-To: <20250913192450.4134957-1-alok.a.tiwari@oracle.com>
Fix two minor typos in vmbus_drv.c:
- Correct "reponsible" -> "responsible" in a comment.
- Add missing newline in pr_err() message ("channeln" -> "channel\n").
These are cosmetic changes only and do not affect functionality.
Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
---
drivers/hv/vmbus_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index fbab9f2d7fa6..69591dc7bad2 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1947,7 +1947,7 @@ static const struct kobj_type vmbus_chan_ktype = {
* is running.
* For example, HV_NIC device is used either by uio_hv_generic or hv_netvsc at any given point of
* time, and "ring" sysfs is needed only when uio_hv_generic is bound to that device. To avoid
- * exposing the ring buffer by default, this function is reponsible to enable visibility of
+ * exposing the ring buffer by default, this function is responsible to enable visibility of
* ring for userspace to use.
* Note: Race conditions can happen with userspace and it is not encouraged to create new
* use-cases for this. This was added to maintain backward compatibility, while solving
@@ -2110,7 +2110,7 @@ int vmbus_device_register(struct hv_device *child_device_obj)
ret = vmbus_add_channel_kobj(child_device_obj,
child_device_obj->channel);
if (ret) {
- pr_err("Unable to register primary channeln");
+ pr_err("Unable to register primary channel\n");
goto err_kset_unregister;
}
hv_debug_add_dev_dir(child_device_obj);
--
2.50.1
^ permalink raw reply related
* Re: [PATCH net] net/mlx5: report duplex full when speed is known
From: Li Tian @ 2025-09-14 2:22 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, linux-hyperv, linux-kernel, Saeed Mahameed, Tariq Toukan,
Mark Bloch, Leon Romanovsky, linux-rdma, Haiyang Zhang
In-Reply-To: <bacbeaf2-104f-4da5-a66b-b8aee2b2de12@lunn.ch>
On Sat, Sep 13, 2025 at 10:12 PM Andrew Lunn <andrew@lunn.ch> wrote:
> I'm confused with your commit message. You say DUPLEX used to be
> reported as Full if the speed is known. How does c268ca6087f55 change
> this?
Because in some circumstances like Azure, mlx5e_port_ptys2speed (now
mlx5_port_ptys2info) does not return a speed. And thus it relies on
data_rate_oper. It reads to me as long as there's no issue acquiring
the speed duplex should be set to full.
> You don't say in the commit message. Why is Half duplex
> important to this fix? I don't see Half anywhere in the code.
It does not return half duplex at all. It would be unknown. I'm just
saying that half duplex shouldn't exist in modern Mellanox 5.
> Also, what sort of problems do you see with duplex unknown?
There were reports of RHEL seeing duplex unknown in ethtool
on Azure Mellanox 5. The intention is to fix this.
Thanks for reviewing.
Li Tian
^ permalink raw reply
* Re: [PATCH net] net/mlx5: report duplex full when speed is known
From: Andrew Lunn @ 2025-09-14 3:25 UTC (permalink / raw)
To: Li Tian
Cc: netdev, linux-hyperv, linux-kernel, Saeed Mahameed, Tariq Toukan,
Mark Bloch, Leon Romanovsky, linux-rdma, Haiyang Zhang
In-Reply-To: <CAHhBTWvcd45s5P-TfKBVzHy00jofbgoWtX+z3Uaj+5ZEBTNLfQ@mail.gmail.com>
On Sun, Sep 14, 2025 at 10:22:35AM +0800, Li Tian wrote:
> On Sat, Sep 13, 2025 at 10:12 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > I'm confused with your commit message. You say DUPLEX used to be
> > reported as Full if the speed is known. How does c268ca6087f55 change
> > this?
>
> Because in some circumstances like Azure, mlx5e_port_ptys2speed (now
> mlx5_port_ptys2info) does not return a speed.
So it can have link up without a speed? Speed is returned unknown? But
Azure is virtual? So both speed and duplex are both meaningless? There
is no physical communication medium. So unknown is reasonable.
> It does not return half duplex at all. It would be unknown. I'm just
> saying that half duplex shouldn't exist in modern Mellanox 5.
And that is relevant because?
> > Also, what sort of problems do you see with duplex unknown?
>
> There were reports of RHEL seeing duplex unknown in ethtool
> on Azure Mellanox 5. The intention is to fix this.
Just reports of ethtool returning unknown? But nothing actually
broken?
Andrew
^ permalink raw reply
* RE: [PATCH 2/3] drivers: hv: vmbus: Fix sscanf format specifier in target_cpu_store()
From: Michael Kelley @ 2025-09-14 4:27 UTC (permalink / raw)
To: Alok Tiwari, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com,
linux-hyperv@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
In-Reply-To: <20250913192450.4134957-2-alok.a.tiwari@oracle.com>
From: Alok Tiwari <alok.a.tiwari@oracle.com> Sent: Saturday, September 13, 2025 12:25 PM
>
> The target_cpu_store() function parses the target CPU from the sysfs
> buffer using sscanf(). The format string currently uses "%uu", which
> is invalid and causes incorrect parsing.
The %uu format string definitely looks invalid, but I'm not seeing
incorrect parsing. For example, this command works:
# echo 5 >/sys/bus/vmbus/devices/<uuid>/channels/<nn>/cpu
and the target cpu is indeed changed to "5". A two-digit value also
works:
# echo 14 >/sys/bus/vmbus/devices/<uuid>/channels/<nn>/cpu
What are the details of the incorrect parsing that you are seeing?
Michael
>
> Fix it by using "%u" to correctly read the value.
>
> This only fixes the parsing format.
>
> Signed-off-by: Alok Tiwari <alok.a.tiwari@oracle.com>
> ---
> drivers/hv/vmbus_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 8b58306cb140..fbab9f2d7fa6 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1742,7 +1742,7 @@ static ssize_t target_cpu_store(struct vmbus_channel
> *channel,
> u32 target_cpu;
> ssize_t ret;
>
> - if (sscanf(buf, "%uu", &target_cpu) != 1)
> + if (sscanf(buf, "%u", &target_cpu) != 1)
> return -EIO;
>
> cpus_read_lock();
> --
> 2.50.1
>
^ permalink raw reply
* Re: [PATCH net-next] net: mana: Reduce waiting time if HWC not responding
From: patchwork-bot+netdevbpf @ 2025-09-14 19:00 UTC (permalink / raw)
To: Haiyang Zhang
Cc: linux-hyperv, netdev, haiyangz, decui, kys, wei.liu, edumazet,
davem, kuba, pabeni, longli, ssengar, ernis, dipayanroy,
kotaranov, shirazsaleem, andrew+netdev, linux-kernel
In-Reply-To: <1757537841-5063-1-git-send-email-haiyangz@linux.microsoft.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 10 Sep 2025 13:57:21 -0700 you wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> If HW Channel (HWC) is not responding, reduce the waiting time, so further
> steps will fail quickly.
> This will prevent getting stuck for a long time (30 minutes or more), for
> example, during unloading while HWC is not responding.
>
> [...]
Here is the summary with links:
- [net-next] net: mana: Reduce waiting time if HWC not responding
https://git.kernel.org/netdev/net-next/c/c4deabbc1abe
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH 0/2] net: mana: Refactor GF stats handling and add rx_missed_errors counter
From: Erni Sri Satya Vennela @ 2025-09-15 3:58 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, longli, kotaranov, horms, shradhagupta, dipayanroy,
shirazsaleem, ernis, rosenp, linux-hyperv, netdev, linux-kernel,
linux-rdma
Restructure mana_query_gf_stats() to operate on the per-VF mana_context,
instead of per-port statistics. Introduce mana_ethtool_hc_stats to
isolate hardware counter statistics and update the
"ethtool -S <interface>" output to expose all relevant counters while
preserving backward compatibility.
Add support for the standard rx_missed_errors counter by mapping it to
the hardware’s hc_rx_discards_no_wqe metric. Introduce a
dedicated workqueue that refreshes statistics every 2 seconds, ensuring
timely and consistent updates of hardware counters.
Erni Sri Satya Vennela (2):
net: mana: Refactor GF stats to use global mana_context
net: mana: Add standard counter rx_missed_errors
drivers/net/ethernet/microsoft/mana/mana_en.c | 103 ++++++++++++------
.../ethernet/microsoft/mana/mana_ethtool.c | 85 ++++++++-------
include/net/mana/gdma.h | 6 +-
include/net/mana/mana.h | 17 ++-
4 files changed, 131 insertions(+), 80 deletions(-)
--
2.34.1
^ permalink raw reply
* [PATCH 1/2] net: mana: Refactor GF stats to use global mana_context
From: Erni Sri Satya Vennela @ 2025-09-15 3:58 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, longli, kotaranov, horms, shradhagupta, dipayanroy,
shirazsaleem, ernis, rosenp, linux-hyperv, netdev, linux-kernel,
linux-rdma
In-Reply-To: <1757908698-16778-1-git-send-email-ernis@linux.microsoft.com>
Refactor mana_query_gf_stats() to use mana_context instead of per-port,
enabling single query for all VFs. Isolate hardware counter stats by
introducing mana_ethtool_hc_stats in mana_context and update the code
to ensure all stats are properly reported via ethtool -S <interface>,
maintaining consistency with previous behavior.
Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/ethernet/microsoft/mana/mana_en.c | 67 ++++++++-------
.../ethernet/microsoft/mana/mana_ethtool.c | 85 ++++++++++---------
include/net/mana/mana.h | 14 +--
3 files changed, 90 insertions(+), 76 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index f4fc86f20213..787644229897 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2779,11 +2779,12 @@ int mana_config_rss(struct mana_port_context *apc, enum TRI_STATE rx,
return 0;
}
-void mana_query_gf_stats(struct mana_port_context *apc)
+void mana_query_gf_stats(struct mana_context *ac)
{
struct mana_query_gf_stat_resp resp = {};
struct mana_query_gf_stat_req req = {};
- struct net_device *ndev = apc->ndev;
+ struct gdma_context *gc = ac->gdma_dev->gdma_context;
+ struct device *dev = gc->dev;
int err;
mana_gd_init_req_hdr(&req.hdr, MANA_QUERY_GF_STAT,
@@ -2817,52 +2818,52 @@ void mana_query_gf_stats(struct mana_port_context *apc)
STATISTICS_FLAGS_HC_TX_BCAST_BYTES |
STATISTICS_FLAGS_TX_ERRORS_GDMA_ERROR;
- err = mana_send_request(apc->ac, &req, sizeof(req), &resp,
+ err = mana_send_request(ac, &req, sizeof(req), &resp,
sizeof(resp));
if (err) {
- netdev_err(ndev, "Failed to query GF stats: %d\n", err);
+ dev_err(dev, "Failed to query GF stats: %d\n", err);
return;
}
err = mana_verify_resp_hdr(&resp.hdr, MANA_QUERY_GF_STAT,
sizeof(resp));
if (err || resp.hdr.status) {
- netdev_err(ndev, "Failed to query GF stats: %d, 0x%x\n", err,
- resp.hdr.status);
+ dev_err(dev, "Failed to query GF stats: %d, 0x%x\n", err,
+ resp.hdr.status);
return;
}
- apc->eth_stats.hc_rx_discards_no_wqe = resp.rx_discards_nowqe;
- apc->eth_stats.hc_rx_err_vport_disabled = resp.rx_err_vport_disabled;
- apc->eth_stats.hc_rx_bytes = resp.hc_rx_bytes;
- apc->eth_stats.hc_rx_ucast_pkts = resp.hc_rx_ucast_pkts;
- apc->eth_stats.hc_rx_ucast_bytes = resp.hc_rx_ucast_bytes;
- apc->eth_stats.hc_rx_bcast_pkts = resp.hc_rx_bcast_pkts;
- apc->eth_stats.hc_rx_bcast_bytes = resp.hc_rx_bcast_bytes;
- apc->eth_stats.hc_rx_mcast_pkts = resp.hc_rx_mcast_pkts;
- apc->eth_stats.hc_rx_mcast_bytes = resp.hc_rx_mcast_bytes;
- apc->eth_stats.hc_tx_err_gf_disabled = resp.tx_err_gf_disabled;
- apc->eth_stats.hc_tx_err_vport_disabled = resp.tx_err_vport_disabled;
- apc->eth_stats.hc_tx_err_inval_vportoffset_pkt =
+ ac->hc_stats.hc_rx_discards_no_wqe = resp.rx_discards_nowqe;
+ ac->hc_stats.hc_rx_err_vport_disabled = resp.rx_err_vport_disabled;
+ ac->hc_stats.hc_rx_bytes = resp.hc_rx_bytes;
+ ac->hc_stats.hc_rx_ucast_pkts = resp.hc_rx_ucast_pkts;
+ ac->hc_stats.hc_rx_ucast_bytes = resp.hc_rx_ucast_bytes;
+ ac->hc_stats.hc_rx_bcast_pkts = resp.hc_rx_bcast_pkts;
+ ac->hc_stats.hc_rx_bcast_bytes = resp.hc_rx_bcast_bytes;
+ ac->hc_stats.hc_rx_mcast_pkts = resp.hc_rx_mcast_pkts;
+ ac->hc_stats.hc_rx_mcast_bytes = resp.hc_rx_mcast_bytes;
+ ac->hc_stats.hc_tx_err_gf_disabled = resp.tx_err_gf_disabled;
+ ac->hc_stats.hc_tx_err_vport_disabled = resp.tx_err_vport_disabled;
+ ac->hc_stats.hc_tx_err_inval_vportoffset_pkt =
resp.tx_err_inval_vport_offset_pkt;
- apc->eth_stats.hc_tx_err_vlan_enforcement =
+ ac->hc_stats.hc_tx_err_vlan_enforcement =
resp.tx_err_vlan_enforcement;
- apc->eth_stats.hc_tx_err_eth_type_enforcement =
+ ac->hc_stats.hc_tx_err_eth_type_enforcement =
resp.tx_err_ethtype_enforcement;
- apc->eth_stats.hc_tx_err_sa_enforcement = resp.tx_err_SA_enforcement;
- apc->eth_stats.hc_tx_err_sqpdid_enforcement =
+ ac->hc_stats.hc_tx_err_sa_enforcement = resp.tx_err_SA_enforcement;
+ ac->hc_stats.hc_tx_err_sqpdid_enforcement =
resp.tx_err_SQPDID_enforcement;
- apc->eth_stats.hc_tx_err_cqpdid_enforcement =
+ ac->hc_stats.hc_tx_err_cqpdid_enforcement =
resp.tx_err_CQPDID_enforcement;
- apc->eth_stats.hc_tx_err_mtu_violation = resp.tx_err_mtu_violation;
- apc->eth_stats.hc_tx_err_inval_oob = resp.tx_err_inval_oob;
- apc->eth_stats.hc_tx_bytes = resp.hc_tx_bytes;
- apc->eth_stats.hc_tx_ucast_pkts = resp.hc_tx_ucast_pkts;
- apc->eth_stats.hc_tx_ucast_bytes = resp.hc_tx_ucast_bytes;
- apc->eth_stats.hc_tx_bcast_pkts = resp.hc_tx_bcast_pkts;
- apc->eth_stats.hc_tx_bcast_bytes = resp.hc_tx_bcast_bytes;
- apc->eth_stats.hc_tx_mcast_pkts = resp.hc_tx_mcast_pkts;
- apc->eth_stats.hc_tx_mcast_bytes = resp.hc_tx_mcast_bytes;
- apc->eth_stats.hc_tx_err_gdma = resp.tx_err_gdma;
+ ac->hc_stats.hc_tx_err_mtu_violation = resp.tx_err_mtu_violation;
+ ac->hc_stats.hc_tx_err_inval_oob = resp.tx_err_inval_oob;
+ ac->hc_stats.hc_tx_bytes = resp.hc_tx_bytes;
+ ac->hc_stats.hc_tx_ucast_pkts = resp.hc_tx_ucast_pkts;
+ ac->hc_stats.hc_tx_ucast_bytes = resp.hc_tx_ucast_bytes;
+ ac->hc_stats.hc_tx_bcast_pkts = resp.hc_tx_bcast_pkts;
+ ac->hc_stats.hc_tx_bcast_bytes = resp.hc_tx_bcast_bytes;
+ ac->hc_stats.hc_tx_mcast_pkts = resp.hc_tx_mcast_pkts;
+ ac->hc_stats.hc_tx_mcast_bytes = resp.hc_tx_mcast_bytes;
+ ac->hc_stats.hc_tx_err_gdma = resp.tx_err_gdma;
}
void mana_query_phy_stats(struct mana_port_context *apc)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
index a1afa75a9463..3dfd96146424 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
@@ -15,66 +15,69 @@ struct mana_stats_desc {
static const struct mana_stats_desc mana_eth_stats[] = {
{"stop_queue", offsetof(struct mana_ethtool_stats, stop_queue)},
{"wake_queue", offsetof(struct mana_ethtool_stats, wake_queue)},
- {"hc_rx_discards_no_wqe", offsetof(struct mana_ethtool_stats,
+ {"tx_cq_err", offsetof(struct mana_ethtool_stats, tx_cqe_err)},
+ {"tx_cqe_unknown_type", offsetof(struct mana_ethtool_stats,
+ tx_cqe_unknown_type)},
+ {"rx_coalesced_err", offsetof(struct mana_ethtool_stats,
+ rx_coalesced_err)},
+ {"rx_cqe_unknown_type", offsetof(struct mana_ethtool_stats,
+ rx_cqe_unknown_type)},
+};
+
+static const struct mana_stats_desc mana_hc_stats[] = {
+ {"hc_rx_discards_no_wqe", offsetof(struct mana_ethtool_hc_stats,
hc_rx_discards_no_wqe)},
- {"hc_rx_err_vport_disabled", offsetof(struct mana_ethtool_stats,
+ {"hc_rx_err_vport_disabled", offsetof(struct mana_ethtool_hc_stats,
hc_rx_err_vport_disabled)},
- {"hc_rx_bytes", offsetof(struct mana_ethtool_stats, hc_rx_bytes)},
- {"hc_rx_ucast_pkts", offsetof(struct mana_ethtool_stats,
+ {"hc_rx_bytes", offsetof(struct mana_ethtool_hc_stats, hc_rx_bytes)},
+ {"hc_rx_ucast_pkts", offsetof(struct mana_ethtool_hc_stats,
hc_rx_ucast_pkts)},
- {"hc_rx_ucast_bytes", offsetof(struct mana_ethtool_stats,
+ {"hc_rx_ucast_bytes", offsetof(struct mana_ethtool_hc_stats,
hc_rx_ucast_bytes)},
- {"hc_rx_bcast_pkts", offsetof(struct mana_ethtool_stats,
+ {"hc_rx_bcast_pkts", offsetof(struct mana_ethtool_hc_stats,
hc_rx_bcast_pkts)},
- {"hc_rx_bcast_bytes", offsetof(struct mana_ethtool_stats,
+ {"hc_rx_bcast_bytes", offsetof(struct mana_ethtool_hc_stats,
hc_rx_bcast_bytes)},
- {"hc_rx_mcast_pkts", offsetof(struct mana_ethtool_stats,
- hc_rx_mcast_pkts)},
- {"hc_rx_mcast_bytes", offsetof(struct mana_ethtool_stats,
+ {"hc_rx_mcast_pkts", offsetof(struct mana_ethtool_hc_stats,
+ hc_rx_mcast_pkts)},
+ {"hc_rx_mcast_bytes", offsetof(struct mana_ethtool_hc_stats,
hc_rx_mcast_bytes)},
- {"hc_tx_err_gf_disabled", offsetof(struct mana_ethtool_stats,
+ {"hc_tx_err_gf_disabled", offsetof(struct mana_ethtool_hc_stats,
hc_tx_err_gf_disabled)},
- {"hc_tx_err_vport_disabled", offsetof(struct mana_ethtool_stats,
+ {"hc_tx_err_vport_disabled", offsetof(struct mana_ethtool_hc_stats,
hc_tx_err_vport_disabled)},
{"hc_tx_err_inval_vportoffset_pkt",
- offsetof(struct mana_ethtool_stats,
+ offsetof(struct mana_ethtool_hc_stats,
hc_tx_err_inval_vportoffset_pkt)},
- {"hc_tx_err_vlan_enforcement", offsetof(struct mana_ethtool_stats,
+ {"hc_tx_err_vlan_enforcement", offsetof(struct mana_ethtool_hc_stats,
hc_tx_err_vlan_enforcement)},
{"hc_tx_err_eth_type_enforcement",
- offsetof(struct mana_ethtool_stats, hc_tx_err_eth_type_enforcement)},
- {"hc_tx_err_sa_enforcement", offsetof(struct mana_ethtool_stats,
+ offsetof(struct mana_ethtool_hc_stats, hc_tx_err_eth_type_enforcement)},
+ {"hc_tx_err_sa_enforcement", offsetof(struct mana_ethtool_hc_stats,
hc_tx_err_sa_enforcement)},
{"hc_tx_err_sqpdid_enforcement",
- offsetof(struct mana_ethtool_stats, hc_tx_err_sqpdid_enforcement)},
+ offsetof(struct mana_ethtool_hc_stats, hc_tx_err_sqpdid_enforcement)},
{"hc_tx_err_cqpdid_enforcement",
- offsetof(struct mana_ethtool_stats, hc_tx_err_cqpdid_enforcement)},
- {"hc_tx_err_mtu_violation", offsetof(struct mana_ethtool_stats,
+ offsetof(struct mana_ethtool_hc_stats, hc_tx_err_cqpdid_enforcement)},
+ {"hc_tx_err_mtu_violation", offsetof(struct mana_ethtool_hc_stats,
hc_tx_err_mtu_violation)},
- {"hc_tx_err_inval_oob", offsetof(struct mana_ethtool_stats,
+ {"hc_tx_err_inval_oob", offsetof(struct mana_ethtool_hc_stats,
hc_tx_err_inval_oob)},
- {"hc_tx_err_gdma", offsetof(struct mana_ethtool_stats,
+ {"hc_tx_err_gdma", offsetof(struct mana_ethtool_hc_stats,
hc_tx_err_gdma)},
- {"hc_tx_bytes", offsetof(struct mana_ethtool_stats, hc_tx_bytes)},
- {"hc_tx_ucast_pkts", offsetof(struct mana_ethtool_stats,
+ {"hc_tx_bytes", offsetof(struct mana_ethtool_hc_stats, hc_tx_bytes)},
+ {"hc_tx_ucast_pkts", offsetof(struct mana_ethtool_hc_stats,
hc_tx_ucast_pkts)},
- {"hc_tx_ucast_bytes", offsetof(struct mana_ethtool_stats,
+ {"hc_tx_ucast_bytes", offsetof(struct mana_ethtool_hc_stats,
hc_tx_ucast_bytes)},
- {"hc_tx_bcast_pkts", offsetof(struct mana_ethtool_stats,
+ {"hc_tx_bcast_pkts", offsetof(struct mana_ethtool_hc_stats,
hc_tx_bcast_pkts)},
- {"hc_tx_bcast_bytes", offsetof(struct mana_ethtool_stats,
+ {"hc_tx_bcast_bytes", offsetof(struct mana_ethtool_hc_stats,
hc_tx_bcast_bytes)},
- {"hc_tx_mcast_pkts", offsetof(struct mana_ethtool_stats,
+ {"hc_tx_mcast_pkts", offsetof(struct mana_ethtool_hc_stats,
hc_tx_mcast_pkts)},
- {"hc_tx_mcast_bytes", offsetof(struct mana_ethtool_stats,
+ {"hc_tx_mcast_bytes", offsetof(struct mana_ethtool_hc_stats,
hc_tx_mcast_bytes)},
- {"tx_cq_err", offsetof(struct mana_ethtool_stats, tx_cqe_err)},
- {"tx_cqe_unknown_type", offsetof(struct mana_ethtool_stats,
- tx_cqe_unknown_type)},
- {"rx_coalesced_err", offsetof(struct mana_ethtool_stats,
- rx_coalesced_err)},
- {"rx_cqe_unknown_type", offsetof(struct mana_ethtool_stats,
- rx_cqe_unknown_type)},
};
static const struct mana_stats_desc mana_phy_stats[] = {
@@ -138,7 +141,7 @@ static int mana_get_sset_count(struct net_device *ndev, int stringset)
if (stringset != ETH_SS_STATS)
return -EINVAL;
- return ARRAY_SIZE(mana_eth_stats) + ARRAY_SIZE(mana_phy_stats) +
+ return ARRAY_SIZE(mana_eth_stats) + ARRAY_SIZE(mana_phy_stats) + ARRAY_SIZE(mana_hc_stats) +
num_queues * (MANA_STATS_RX_COUNT + MANA_STATS_TX_COUNT);
}
@@ -150,10 +153,12 @@ static void mana_get_strings(struct net_device *ndev, u32 stringset, u8 *data)
if (stringset != ETH_SS_STATS)
return;
-
for (i = 0; i < ARRAY_SIZE(mana_eth_stats); i++)
ethtool_puts(&data, mana_eth_stats[i].name);
+ for (i = 0; i < ARRAY_SIZE(mana_hc_stats); i++)
+ ethtool_puts(&data, mana_hc_stats[i].name);
+
for (i = 0; i < ARRAY_SIZE(mana_phy_stats); i++)
ethtool_puts(&data, mana_phy_stats[i].name);
@@ -186,6 +191,7 @@ static void mana_get_ethtool_stats(struct net_device *ndev,
struct mana_port_context *apc = netdev_priv(ndev);
unsigned int num_queues = apc->num_queues;
void *eth_stats = &apc->eth_stats;
+ void *hc_stats = &apc->ac->hc_stats;
void *phy_stats = &apc->phy_stats;
struct mana_stats_rx *rx_stats;
struct mana_stats_tx *tx_stats;
@@ -208,7 +214,7 @@ static void mana_get_ethtool_stats(struct net_device *ndev,
if (!apc->port_is_up)
return;
/* we call mana function to update stats from GDMA */
- mana_query_gf_stats(apc);
+ mana_query_gf_stats(apc->ac);
/* We call this mana function to get the phy stats from GDMA and includes
* aggregate tx/rx drop counters, Per-TC(Traffic Channel) tx/rx and pause
@@ -219,6 +225,9 @@ static void mana_get_ethtool_stats(struct net_device *ndev,
for (q = 0; q < ARRAY_SIZE(mana_eth_stats); q++)
data[i++] = *(u64 *)(eth_stats + mana_eth_stats[q].offset);
+ for (q = 0; q < ARRAY_SIZE(mana_hc_stats); q++)
+ data[i++] = *(u64 *)(hc_stats + mana_hc_stats[q].offset);
+
for (q = 0; q < ARRAY_SIZE(mana_phy_stats); q++)
data[i++] = *(u64 *)(phy_stats + mana_phy_stats[q].offset);
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 0921485565c0..519c4384c51f 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -375,6 +375,13 @@ struct mana_tx_qp {
struct mana_ethtool_stats {
u64 stop_queue;
u64 wake_queue;
+ u64 tx_cqe_err;
+ u64 tx_cqe_unknown_type;
+ u64 rx_coalesced_err;
+ u64 rx_cqe_unknown_type;
+};
+
+struct mana_ethtool_hc_stats {
u64 hc_rx_discards_no_wqe;
u64 hc_rx_err_vport_disabled;
u64 hc_rx_bytes;
@@ -402,10 +409,6 @@ struct mana_ethtool_stats {
u64 hc_tx_mcast_pkts;
u64 hc_tx_mcast_bytes;
u64 hc_tx_err_gdma;
- u64 tx_cqe_err;
- u64 tx_cqe_unknown_type;
- u64 rx_coalesced_err;
- u64 rx_cqe_unknown_type;
};
struct mana_ethtool_phy_stats {
@@ -473,6 +476,7 @@ struct mana_context {
u16 num_ports;
u8 bm_hostmode;
+ struct mana_ethtool_hc_stats hc_stats;
struct mana_eq *eqs;
struct dentry *mana_eqs_debugfs;
@@ -573,7 +577,7 @@ u32 mana_run_xdp(struct net_device *ndev, struct mana_rxq *rxq,
struct bpf_prog *mana_xdp_get(struct mana_port_context *apc);
void mana_chn_setxdp(struct mana_port_context *apc, struct bpf_prog *prog);
int mana_bpf(struct net_device *ndev, struct netdev_bpf *bpf);
-void mana_query_gf_stats(struct mana_port_context *apc);
+void mana_query_gf_stats(struct mana_context *ac);
int mana_query_link_cfg(struct mana_port_context *apc);
int mana_set_bw_clamp(struct mana_port_context *apc, u32 speed,
int enable_clamping);
--
2.34.1
^ permalink raw reply related
* [PATCH 2/2] net: mana: Add standard counter rx_missed_errors
From: Erni Sri Satya Vennela @ 2025-09-15 3:58 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, longli, kotaranov, horms, shradhagupta, dipayanroy,
shirazsaleem, ernis, rosenp, linux-hyperv, netdev, linux-kernel,
linux-rdma
In-Reply-To: <1757908698-16778-1-git-send-email-ernis@linux.microsoft.com>
Report standard counter stats->rx_missed_errors
using hc_rx_discards_no_wqe from the hardware.
Add a dedicated workqueue to periodically run
mana_query_gf_stats every 2 seconds to get the latest
info in eth_stats and define a driver capability flag
to notify hardware of the periodic queries.
To avoid repeated failures and log flooding, the workqueue
is not rescheduled if mana_query_gf_stats fails.
Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
Reviewed-By: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/ethernet/microsoft/mana/mana_en.c | 38 +++++++++++++++++--
.../ethernet/microsoft/mana/mana_ethtool.c | 2 -
include/net/mana/gdma.h | 6 ++-
include/net/mana/mana.h | 5 ++-
4 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 787644229897..9213ae6ba038 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -494,6 +494,8 @@ static void mana_get_stats64(struct net_device *ndev,
netdev_stats_to_stats64(st, &ndev->stats);
+ st->rx_missed_errors = apc->ac->hc_stats.hc_rx_discards_no_wqe;
+
for (q = 0; q < num_queues; q++) {
rx_stats = &apc->rxqs[q]->stats;
@@ -2779,7 +2781,7 @@ int mana_config_rss(struct mana_port_context *apc, enum TRI_STATE rx,
return 0;
}
-void mana_query_gf_stats(struct mana_context *ac)
+int mana_query_gf_stats(struct mana_context *ac)
{
struct mana_query_gf_stat_resp resp = {};
struct mana_query_gf_stat_req req = {};
@@ -2822,14 +2824,14 @@ void mana_query_gf_stats(struct mana_context *ac)
sizeof(resp));
if (err) {
dev_err(dev, "Failed to query GF stats: %d\n", err);
- return;
+ return err;
}
err = mana_verify_resp_hdr(&resp.hdr, MANA_QUERY_GF_STAT,
sizeof(resp));
if (err || resp.hdr.status) {
dev_err(dev, "Failed to query GF stats: %d, 0x%x\n", err,
resp.hdr.status);
- return;
+ return err;
}
ac->hc_stats.hc_rx_discards_no_wqe = resp.rx_discards_nowqe;
@@ -2864,6 +2866,8 @@ void mana_query_gf_stats(struct mana_context *ac)
ac->hc_stats.hc_tx_mcast_pkts = resp.hc_tx_mcast_pkts;
ac->hc_stats.hc_tx_mcast_bytes = resp.hc_tx_mcast_bytes;
ac->hc_stats.hc_tx_err_gdma = resp.tx_err_gdma;
+
+ return 0;
}
void mana_query_phy_stats(struct mana_port_context *apc)
@@ -3400,6 +3404,19 @@ int mana_rdma_service_event(struct gdma_context *gc, enum gdma_service_type even
return 0;
}
+#define MANA_GF_STATS_PERIOD (2 * HZ)
+
+static void mana_gf_stats_work_handler(struct work_struct *work)
+{
+ struct mana_context *ac =
+ container_of(to_delayed_work(work), struct mana_context, gf_stats_work);
+
+ if (mana_query_gf_stats(ac))
+ return;
+
+ queue_delayed_work(ac->gf_stats_wq, &ac->gf_stats_work, MANA_GF_STATS_PERIOD);
+}
+
int mana_probe(struct gdma_dev *gd, bool resuming)
{
struct gdma_context *gc = gd->gdma_context;
@@ -3488,6 +3505,15 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
}
err = add_adev(gd, "eth");
+ ac->gf_stats_wq = create_singlethread_workqueue("mana_gf_stats");
+ if (!ac->gf_stats_wq) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ INIT_DELAYED_WORK(&ac->gf_stats_work, mana_gf_stats_work_handler);
+ queue_delayed_work(ac->gf_stats_wq, &ac->gf_stats_work, MANA_GF_STATS_PERIOD);
+
out:
if (err) {
mana_remove(gd, false);
@@ -3511,6 +3537,12 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
int err;
int i;
+ if (ac->gf_stats_wq) {
+ cancel_delayed_work_sync(&ac->gf_stats_work);
+ destroy_workqueue(ac->gf_stats_wq);
+ ac->gf_stats_wq = NULL;
+ }
+
/* adev currently doesn't support suspending, always remove it */
if (gd->adev)
remove_adev(gd);
diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
index 3dfd96146424..99e811208683 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
@@ -213,8 +213,6 @@ static void mana_get_ethtool_stats(struct net_device *ndev,
if (!apc->port_is_up)
return;
- /* we call mana function to update stats from GDMA */
- mana_query_gf_stats(apc->ac);
/* We call this mana function to get the phy stats from GDMA and includes
* aggregate tx/rx drop counters, Per-TC(Traffic Channel) tx/rx and pause
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 57df78cfbf82..88a81fb164a0 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -591,6 +591,9 @@ enum {
/* Driver can self reset on FPGA Reconfig EQE notification */
#define GDMA_DRV_CAP_FLAG_1_HANDLE_RECONFIG_EQE BIT(17)
+/* Driver can send HWC periodically to query stats */
+#define GDMA_DRV_CAP_FLAG_1_PERIODIC_STATS_QUERY BIT(21)
+
#define GDMA_DRV_CAP_FLAGS1 \
(GDMA_DRV_CAP_FLAG_1_EQ_SHARING_MULTI_VPORT | \
GDMA_DRV_CAP_FLAG_1_NAPI_WKDONE_FIX | \
@@ -599,7 +602,8 @@ enum {
GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP | \
GDMA_DRV_CAP_FLAG_1_DYNAMIC_IRQ_ALLOC_SUPPORT | \
GDMA_DRV_CAP_FLAG_1_SELF_RESET_ON_EQE | \
- GDMA_DRV_CAP_FLAG_1_HANDLE_RECONFIG_EQE)
+ GDMA_DRV_CAP_FLAG_1_HANDLE_RECONFIG_EQE | \
+ GDMA_DRV_CAP_FLAG_1_PERIODIC_STATS_QUERY)
#define GDMA_DRV_CAP_FLAGS2 0
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 519c4384c51f..74f112bcdc02 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -480,6 +480,9 @@ struct mana_context {
struct mana_eq *eqs;
struct dentry *mana_eqs_debugfs;
+ struct workqueue_struct *gf_stats_wq;
+ struct delayed_work gf_stats_work;
+
struct net_device *ports[MAX_PORTS_IN_MANA_DEV];
};
@@ -577,7 +580,7 @@ u32 mana_run_xdp(struct net_device *ndev, struct mana_rxq *rxq,
struct bpf_prog *mana_xdp_get(struct mana_port_context *apc);
void mana_chn_setxdp(struct mana_port_context *apc, struct bpf_prog *prog);
int mana_bpf(struct net_device *ndev, struct netdev_bpf *bpf);
-void mana_query_gf_stats(struct mana_context *ac);
+int mana_query_gf_stats(struct mana_context *ac);
int mana_query_link_cfg(struct mana_port_context *apc);
int mana_set_bw_clamp(struct mana_port_context *apc, u32 speed,
int enable_clamping);
--
2.34.1
^ permalink raw reply related
* Re: [External] : RE: [PATCH 2/3] drivers: hv: vmbus: Fix sscanf format specifier in target_cpu_store()
From: ALOK TIWARI @ 2025-09-15 13:01 UTC (permalink / raw)
To: Michael Kelley, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com,
linux-hyperv@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
In-Reply-To: <SN6PR02MB41577DBA5D2C981ACEC4D9DED40AA@SN6PR02MB4157.namprd02.prod.outlook.com>
On 9/14/2025 9:57 AM, Michael Kelley wrote:
> From: Alok Tiwari<alok.a.tiwari@oracle.com> Sent: Saturday, September 13, 2025 12:25 PM
>> The target_cpu_store() function parses the target CPU from the sysfs
>> buffer using sscanf(). The format string currently uses "%uu", which
>> is invalid and causes incorrect parsing.
> The %uu format string definitely looks invalid, but I'm not seeing
> incorrect parsing. For example, this command works:
>
> # echo 5 >/sys/bus/vmbus/devices/<uuid>/channels/<nn>/cpu
>
> and the target cpu is indeed changed to "5". A two-digit value also
> works:
>
> # echo 14 >/sys/bus/vmbus/devices/<uuid>/channels/<nn>/cpu
>
> What are the details of the incorrect parsing that you are seeing?
>
> Michael
Thanks Michael,
You are right, The compiler ignores the extra "u", so there is no
incorrect parsing at runtime. My wording was misleading. This change
should be treated as a cleanup/cosmetic change rather than a real fix.
Thanks,
Alok
^ permalink raw reply
* RE: [PATCH v1 3/6] hyperv: Add definitions for hypervisor crash dump support
From: Michael Kelley @ 2025-09-15 17:54 UTC (permalink / raw)
To: Mukesh Rathor, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org,
hpa@zytor.com, arnd@arndb.de
In-Reply-To: <20250910001009.2651481-4-mrathor@linux.microsoft.com>
From: Mukesh Rathor <mrathor@linux.microsoft.com> Sent: Tuesday, September 9, 2025 5:10 PM
>
> Add data structures for hypervisor crash dump support to the hypervisor
> host ABI header file. Details of their usages are in subsequent commits.
>
> Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
> ---
> include/hyperv/hvhdk_mini.h | 55 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
> index 858f6a3925b3..ad9a8048fb4e 100644
> --- a/include/hyperv/hvhdk_mini.h
> +++ b/include/hyperv/hvhdk_mini.h
> @@ -116,6 +116,17 @@ enum hv_system_property {
> /* Add more values when needed */
> HV_SYSTEM_PROPERTY_SCHEDULER_TYPE = 15,
> HV_DYNAMIC_PROCESSOR_FEATURE_PROPERTY = 21,
> + HV_SYSTEM_PROPERTY_CRASHDUMPAREA = 47,
> +};
> +
> +#define HV_PFN_RANGE_PGBITS 24 /* HV_SPA_PAGE_RANGE_ADDITIONAL_PAGES_BITS */
> +union hv_pfn_range { /* HV_SPA_PAGE_RANGE */
> + u64 as_uint64;
> + struct {
> + /* 39:0: base pfn. 63:40: additional pages */
> + u64 base_pfn : 64 - HV_PFN_RANGE_PGBITS;
> + u64 add_pfns : HV_PFN_RANGE_PGBITS;
> + } __packed;
> };
>
> enum hv_dynamic_processor_feature_property {
> @@ -142,6 +153,8 @@ struct hv_output_get_system_property {
> #if IS_ENABLED(CONFIG_X86)
> u64 hv_processor_feature_value;
> #endif
> + union hv_pfn_range hv_cda_info; /* CrashdumpAreaAddress */
> + u64 hv_tramp_pa; /* CrashdumpTrampolineAddress */
> };
> } __packed;
>
> @@ -234,6 +247,48 @@ union hv_gpa_page_access_state {
> u8 as_uint8;
> } __packed;
>
> +enum hv_crashdump_action {
> + HV_CRASHDUMP_NONE = 0,
> + HV_CRASHDUMP_SUSPEND_ALL_VPS,
> + HV_CRASHDUMP_PREPARE_FOR_STATE_SAVE,
> + HV_CRASHDUMP_STATE_SAVED,
> + HV_CRASHDUMP_ENTRY,
> +};
Nit: Since these values are part of the ABI, it's probably better
to assign explicit values to each enum member in order to
ward off any mistaken reordering or additions in the middle
of the list.
> +
> +struct hv_partition_event_root_crashdump_input {
> + u32 crashdump_action; /* enum hv_crashdump_action */
> +} __packed;
> +
> +struct hv_input_disable_hyp_ex { /* HV_X64_INPUT_DISABLE_HYPERVISOR_EX */
> + u64 rip;
> + u64 arg;
> +} __packed;
> +
> +struct hv_crashdump_area { /* HV_CRASHDUMP_AREA */
> + u32 version;
> + union {
> + u32 flags_as_uint32;
> + struct {
> + u32 cda_valid : 1;
> + u32 cda_unused : 31;
> + } __packed;
> + };
> + /* more unused fields */
> +} __packed;
> +
> +union hv_partition_event_input {
> + struct hv_partition_event_root_crashdump_input crashdump_input;
> +};
> +
> +enum hv_partition_event {
> + HV_PARTITION_EVENT_ROOT_CRASHDUMP = 2,
> +};
> +
> +struct hv_input_notify_partition_event {
> + u32 event; /* enum hv_partition_event */
> + union hv_partition_event_input input;
> +} __packed;
> +
> struct hv_lp_startup_status {
> u64 hv_status;
> u64 substatus1;
> --
> 2.36.1.vfs.0.0
>
^ permalink raw reply
* RE: [PATCH v1 4/6] x86/hyperv: Add trampoline asm code to transition from hypervisor
From: Michael Kelley @ 2025-09-15 17:55 UTC (permalink / raw)
To: Mukesh Rathor, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org,
hpa@zytor.com, arnd@arndb.de
In-Reply-To: <20250910001009.2651481-5-mrathor@linux.microsoft.com>
From: Mukesh Rathor <mrathor@linux.microsoft.com> Sent: Tuesday, September 9, 2025 5:10 PM
>
> Introduce a small asm stub to transition from the hypervisor to linux
I'd argue for capitalizing "Linux" here and in other places in commit
text and code comments throughout this patch set.
> upon devirtualization.
In this patch and subsequent patches, you've used the phrase "upon
devirtualization", which seems a little vague to me. Does this mean
"when devirtualization is complete" or perhaps "when the hypervisor
completes devirtualization"? Since there's no spec on any of this,
being as precise as possible will help future readers.
>
> At a high level, during panic of either the hypervisor or the dom0 (aka
> root), the nmi handler asks hypervisor to devirtualize.
Suggest:
At a high level, during panic of either the hypervisor or Linux running
in dom0 (a.k.a. the root partition), the Linux NMI handler asks the
hypervisor to devirtualize.
> As part of that,
> the arguments include an entry point to return back to linux. This asm
> stub implements that entry point.
>
> The stub is entered in protected mode, uses temporary gdt and page table
> to enable long mode and get to kernel entry point which then restores full
> kernel context to resume execution to kexec.
>
> Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
> ---
> arch/x86/hyperv/hv_trampoline.S | 105 ++++++++++++++++++++++++++++++++
> 1 file changed, 105 insertions(+)
> create mode 100644 arch/x86/hyperv/hv_trampoline.S
>
> diff --git a/arch/x86/hyperv/hv_trampoline.S b/arch/x86/hyperv/hv_trampoline.S
> new file mode 100644
> index 000000000000..27a755401a42
> --- /dev/null
> +++ b/arch/x86/hyperv/hv_trampoline.S
> @@ -0,0 +1,105 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * X86 specific Hyper-V kdump/crash related code.
Add a qualification that this is for root partition only, and not for
general guests?
> + *
> + * Copyright (C) 2025, Microsoft, Inc.
> + *
> + */
> +#include <linux/linkage.h>
> +#include <asm/alternative.h>
> +#include <asm/msr.h>
> +#include <asm/processor-flags.h>
> +#include <asm/nospec-branch.h>
> +
> +/*
> + * void noreturn hv_crash_asm32(arg1)
> + * arg1 == edi == 32bit PA of struct hv_crash_trdata
I think this is "struct hv_crash_tramp_data".
> + *
> + * The hypervisor jumps here upon devirtualization in protected mode. This
> + * code gets copied to a page in the low 4G ie, 32bit space so it can run
> + * in the protected mode. Hence we cannot use any compile/link time offsets or
> + * addresses. It restores long mode via temporary gdt and page tables and
> + * eventually jumps to kernel code entry at HV_CRASHDATA_OFFS_C_entry.
> + *
> + * PreCondition (ie, Hypervisor call back ABI):
> + * o CR0 is set to 0x0021: PE(prot mode) and NE are set, paging is disabled
> + * o CR4 is set to 0x0
> + * o IA32_EFER is set to 0x901 (SCE and NXE are set)
> + * o EDI is set to the Arg passed to HVCALL_DISABLE_HYP_EX.
> + * o CS, DS, ES, FS, GS are all initialized with a base of 0 and limit 0xFFFF
> + * o IDTR, TR and GDTR are initialized with a base of 0 and limit of 0xFFFF
> + * o LDTR is initialized as invalid (limit of 0)
> + * o MSR PAT is power on default.
> + * o Other state/registers are cleared. All TLBs flushed.
Clarification about "Other state/registers are cleared": What about
processor features that Linux may have enabled or disabled during its
initial boot? Are those still in the states Linux set? Or are they reset to
power-on defaults? For example, if Linux enabled x2apic, is x2apic
still enabled when the stub is entered?
> + *
> + * See Intel SDM 10.8.5
Hmmm. I downloaded the latest combined SDM, and section 10.8.5
in Volume 3A is about Microcode Update Resources, which doesn't
seem applicable here. Other volumes don't have a section 10.8.5.
> + */
> +
> +#define HV_CRASHDATA_OFFS_TRAMPCR3 0x0 /* 0 */
> +#define HV_CRASHDATA_OFFS_KERNCR3 0x8 /* 8 */
> +#define HV_CRASHDATA_OFFS_GDTRLIMIT 0x12 /* 18 */
> +#define HV_CRASHDATA_OFFS_CS_JMPTGT 0x28 /* 40 */
> +#define HV_CRASHDATA_OFFS_C_entry 0x30 /* 48 */
It seems like these offsets should go in a #include file along
with the definition of struct hv_crash_tramp_data. Then the
BUILD_BUG_ON() calls in hv_crash_setup_trampdata() could
check against these symbolic names instead of hardcoding
numbers that must match these.
> +#define HV_CRASHDATA_TRAMPOLINE_CS 0x8
This #define isn't used anywhere.
> +
> + .text
> + .code32
> +
> +SYM_CODE_START(hv_crash_asm32)
> + UNWIND_HINT_UNDEFINED
> + ANNOTATE_NOENDBR
No ENDBR here, presumably because this function is entered via other
than an indirect CALL or JMP instruction. Right?
> + movl $X86_CR4_PAE, %ecx
> + movl %ecx, %cr4
> +
> + movl %edi, %ebx
> + add $HV_CRASHDATA_OFFS_TRAMPCR3, %ebx
> + movl %cs:(%ebx), %eax
> + movl %eax, %cr3
> +
> + # Setup EFER for long mode now.
> + movl $MSR_EFER, %ecx
> + rdmsr
> + btsl $_EFER_LME, %eax
> + wrmsr
> +
> + # Turn paging on using the temp 32bit trampoline page table.
> + movl %cr0, %eax
> + orl $(X86_CR0_PG), %eax
> + movl %eax, %cr0
> +
> + /* since kernel cr3 could be above 4G, we need to be in the long mode
> + * before we can load 64bits of the kernel cr3. We use a temp gdt for
> + * that with CS.L=1 and CS.D=0 */
> + mov %edi, %eax
> + add $HV_CRASHDATA_OFFS_GDTRLIMIT, %eax
> + lgdtl %cs:(%eax)
> +
> + /* not done yet, restore CS now to switch to CS.L=1 */
> + mov %edi, %eax
> + add $HV_CRASHDATA_OFFS_CS_JMPTGT, %eax
> + ljmp %cs:*(%eax)
> +SYM_CODE_END(hv_crash_asm32)
> +
> + /* we now run in full 64bit IA32-e long mode, CS.L=1 and CS.D=0 */
> + .code64
> + .balign 8
> +SYM_CODE_START(hv_crash_asm64)
> + UNWIND_HINT_UNDEFINED
> + ANNOTATE_NOENDBR
But this *is* entered via an indirect JMP, right? So back to my
earlier question about the state of processor feature enablement.
If Linux enabled IBT, is it still enabled after devirtualization and
the hypervisor invokes this entry point? Linux guests on Hyper-V
have historically not enabled IBT, but patches that enable it are
now in linux-next, and will go into the 6.18 kernel. So maybe
this needs an ENDBR64.
> +SYM_INNER_LABEL(hv_crash_asm64_lbl, SYM_L_GLOBAL)
> + /* restore kernel page tables so we can jump to kernel code */
> + mov %edi, %eax
> + add $HV_CRASHDATA_OFFS_KERNCR3, %eax
> + movq %cs:(%eax), %rbx
> + movq %rbx, %cr3
> +
> + mov %edi, %eax
> + add $HV_CRASHDATA_OFFS_C_entry, %eax
> + movq %cs:(%eax), %rbx
> + ANNOTATE_RETPOLINE_SAFE
> + jmp *%rbx
> +
> + int $3
> +
> +SYM_INNER_LABEL(hv_crash_asm_end, SYM_L_GLOBAL)
> +SYM_CODE_END(hv_crash_asm64)
> --
> 2.36.1.vfs.0.0
>
^ permalink raw reply
* RE: [PATCH v1 5/6] x86/hyperv: Implement hypervisor ram collection into vmcore
From: Michael Kelley @ 2025-09-15 17:55 UTC (permalink / raw)
To: Mukesh Rathor, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org,
hpa@zytor.com, arnd@arndb.de
In-Reply-To: <20250910001009.2651481-6-mrathor@linux.microsoft.com>
From: Mukesh Rathor <mrathor@linux.microsoft.com> Sent: Tuesday, September 9, 2025 5:10 PM
>
> Introduce a new file to implement collection of hypervisor ram into the
s/ram/RAM/ (multiple places)
> vmcore collected by linux. By default, the hypervisor ram is locked, ie,
> protected via hw page table. Hyper-V implements a disable hypercall which
The terminology here is a bit confusing since you have two names for
the same thing: "disable" hypervisor, and "devirtualize". Is it possible to
just use "devirtualize" everywhere, and drop the "disable" terminology?
> essentially devirtualizes the system on the fly. This mechanism makes the
> hypervisor ram accessible to linux. Because the hypervisor ram is already
> mapped into linux address space (as reserved ram),
Is the hypervisor RAM mapped into the VMM process user address space,
or somewhere in the kernel address space? If the latter, where in the kernel
code, or what mechanism, does that? Just curious, as I wasn't aware that
this is happening ....
> it is automatically
> collected into the vmcore without extra work. More details of the
> implementation are available in the file prologue.
>
> Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
> ---
> arch/x86/hyperv/hv_crash.c | 622 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 622 insertions(+)
> create mode 100644 arch/x86/hyperv/hv_crash.c
>
> diff --git a/arch/x86/hyperv/hv_crash.c b/arch/x86/hyperv/hv_crash.c
> new file mode 100644
> index 000000000000..531bac79d598
> --- /dev/null
> +++ b/arch/x86/hyperv/hv_crash.c
> @@ -0,0 +1,622 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * X86 specific Hyper-V kdump/crash support module
> + *
> + * Copyright (C) 2025, Microsoft, Inc.
> + *
> + * This module implements hypervisor ram collection into vmcore for both
> + * cases of the hypervisor crash and linux dom0/root crash.
For a hypervisor crash, does any of this apply to general guest VMs? I'm
thinking it does not. Hypervisor RAM is collected only into the vmcore
for the root partition, right? Maybe some additional clarification could be
added so there's no confusion in this regard.
And what *does* happen to guest VMs after a hypervisor crash?
> + * Hyper-V implements
> + * a devirtualization hypercall with a 32bit protected mode ABI callback. This
> + * mechanism must be used to unlock hypervisor ram. Since the hypervisor ram
> + * is already mapped in linux, it is automatically collected into linux vmcore,
> + * and can be examined by the crash command (raw ram dump) or windbg.
> + *
> + * At a high level:
> + *
> + * Hypervisor Crash:
> + * Upon crash, hypervisor goes into an emergency minimal dispatch loop, a
> + * restrictive mode with very limited hypercall and msr support.
s/msr/MSR/
> + * Each cpu then injects NMIs into dom0/root vcpus.
The "Each cpu" part of this sentence is confusing to me -- which CPUs does
this refer to? Maybe it would be better to say "It then injects an NMI into
each dom0/root partition vCPU." without being specific as to which CPUs do
the injecting since that seems more like a hypervisor implementation detail
that's not relevant here.
> + * A shared page is used to check
> + * by linux in the nmi handler if the hypervisor has crashed. This shared
s/nmi/NMI/ (multiple places)
> + * page is setup in hv_root_crash_init during boot.
> + *
> + * Linux Crash:
> + * In case of linux crash, the callback hv_crash_stop_other_cpus will send
> + * NMIs to all cpus, then proceed to the crash_nmi_callback where it waits
> + * for all cpus to be in NMI.
> + *
> + * NMI Handler (upon quorum):
> + * Eventually, in both cases, all cpus wil end up in the nmi hanlder.
s/hanlder/handler/
And maybe just drop the word "wil" (which is misspelled).
> + * Hyper-V requires the disable hypervisor must be done from the bsp. So
s/bsp/BSP (multiple places)
> + * the bsp nmi handler saves current context, does some fixups and makes
> + * the hypercall to disable the hypervisor, ie, devirtualize. Hypervisor
> + * at that point will suspend all vcpus (except the bsp), unlock all its
> + * ram, and return to linux at the 32bit mode entry RIP.
> + *
> + * Linux 32bit entry trampoline will then restore long mode and call C
> + * function here to restore context and continue execution to crash kexec.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/kexec.h>
> +#include <linux/crash_dump.h>
> +#include <linux/panic.h>
> +#include <asm/apic.h>
> +#include <asm/desc.h>
> +#include <asm/page.h>
> +#include <asm/pgalloc.h>
> +#include <asm/mshyperv.h>
> +#include <asm/nmi.h>
> +#include <asm/idtentry.h>
> +#include <asm/reboot.h>
> +#include <asm/intel_pt.h>
> +
> +int hv_crash_enabled;
Seems like this is conceptually a "bool", not an "int".
> +EXPORT_SYMBOL_GPL(hv_crash_enabled);
> +
> +struct hv_crash_ctxt {
> + ulong rsp;
> + ulong cr0;
> + ulong cr2;
> + ulong cr4;
> + ulong cr8;
> +
> + u16 cs;
> + u16 ss;
> + u16 ds;
> + u16 es;
> + u16 fs;
> + u16 gs;
> +
> + u16 gdt_fill;
> + struct desc_ptr gdtr;
> + char idt_fill[6];
> + struct desc_ptr idtr;
> +
> + u64 gsbase;
> + u64 efer;
> + u64 pat;
> +};
> +static struct hv_crash_ctxt hv_crash_ctxt;
> +
> +/* Shared hypervisor page that contains crash dump area we peek into.
> + * NB: windbg looks for "hv_cda" symbol so don't change it.
> + */
> +static struct hv_crashdump_area *hv_cda;
> +
> +static u32 trampoline_pa, devirt_cr3arg;
> +static atomic_t crash_cpus_wait;
> +static void *hv_crash_ptpgs[4];
> +static int hv_has_crashed, lx_has_crashed;
These are conceptually "bool" as well.
> +
> +/* This cannot be inlined as it needs stack */
> +static noinline __noclone void hv_crash_restore_tss(void)
> +{
> + load_TR_desc();
> +}
> +
> +/* This cannot be inlined as it needs stack */
> +static noinline void hv_crash_clear_kernpt(void)
> +{
> + pgd_t *pgd;
> + p4d_t *p4d;
> +
> + /* Clear entry so it's not confusing to someone looking at the core */
> + pgd = pgd_offset_k(trampoline_pa);
> + p4d = p4d_offset(pgd, trampoline_pa);
> + native_p4d_clear(p4d);
> +}
> +
> +/*
> + * This is the C entry point from the asm glue code after the devirt hypercall.
> + * We enter here in IA32-e long mode, ie, full 64bit mode running on kernel
> + * page tables with our below 4G page identity mapped, but using a temporary
> + * GDT. ds/fs/gs/es are null. ss is not usable. bp is null. stack is not
> + * available. We restore kernel GDT, and rest of the context, and continue
> + * to kexec.
> + */
> +static asmlinkage void __noreturn hv_crash_c_entry(void)
> +{
> + struct hv_crash_ctxt *ctxt = &hv_crash_ctxt;
> +
> + /* first thing, restore kernel gdt */
> + native_load_gdt(&ctxt->gdtr);
> +
> + asm volatile("movw %%ax, %%ss" : : "a"(ctxt->ss));
> + asm volatile("movq %0, %%rsp" : : "m"(ctxt->rsp));
> +
> + asm volatile("movw %%ax, %%ds" : : "a"(ctxt->ds));
> + asm volatile("movw %%ax, %%es" : : "a"(ctxt->es));
> + asm volatile("movw %%ax, %%fs" : : "a"(ctxt->fs));
> + asm volatile("movw %%ax, %%gs" : : "a"(ctxt->gs));
> +
> + native_wrmsrq(MSR_IA32_CR_PAT, ctxt->pat);
> + asm volatile("movq %0, %%cr0" : : "r"(ctxt->cr0));
> +
> + asm volatile("movq %0, %%cr8" : : "r"(ctxt->cr8));
> + asm volatile("movq %0, %%cr4" : : "r"(ctxt->cr4));
> + asm volatile("movq %0, %%cr2" : : "r"(ctxt->cr4));
> +
> + native_load_idt(&ctxt->idtr);
> + native_wrmsrq(MSR_GS_BASE, ctxt->gsbase);
> + native_wrmsrq(MSR_EFER, ctxt->efer);
> +
> + /* restore the original kernel CS now via far return */
> + asm volatile("movzwq %0, %%rax\n\t"
> + "pushq %%rax\n\t"
> + "pushq $1f\n\t"
> + "lretq\n\t"
> + "1:nop\n\t" : : "m"(ctxt->cs) : "rax");
> +
> + /* We are in asmlinkage without stack frame, hence make a C function
> + * call which will buy stack frame to restore the tss or clear PT entry.
> + */
> + hv_crash_restore_tss();
> + hv_crash_clear_kernpt();
> +
> + /* we are now fully in devirtualized normal kernel mode */
> + __crash_kexec(NULL);
The comments for __crash_kexec() say that "panic_cpu" should be set to
the current CPU. I don't see that such is the case here.
> +
> + for (;;)
> + cpu_relax();
Is the intent that __crash_kexec() should never return, on any of the vCPUs,
because devirtualization isn't done unless there's a valid kdump image loaded?
I wonder if
native_wrmsrq(HV_X64_MSR_RESET, 1);
would be better than looping forever in case __crash_kexec() fails
somewhere along the way even if there's a kdump image loaded.
> +}
> +/* Tell gcc we are using lretq long jump in the above function intentionally */
> +STACK_FRAME_NON_STANDARD(hv_crash_c_entry);
> +
> +static void hv_mark_tss_not_busy(void)
> +{
> + struct desc_struct *desc = get_current_gdt_rw();
> + tss_desc tss;
> +
> + memcpy(&tss, &desc[GDT_ENTRY_TSS], sizeof(tss_desc));
> + tss.type = 0x9; /* available 64-bit TSS. 0xB is busy TSS */
> + write_gdt_entry(desc, GDT_ENTRY_TSS, &tss, DESC_TSS);
> +}
> +
> +/* Save essential context */
> +static void hv_hvcrash_ctxt_save(void)
> +{
> + struct hv_crash_ctxt *ctxt = &hv_crash_ctxt;
> +
> + asm volatile("movq %%rsp,%0" : "=m"(ctxt->rsp));
> +
> + ctxt->cr0 = native_read_cr0();
> + ctxt->cr4 = native_read_cr4();
> +
> + asm volatile("movq %%cr2, %0" : "=a"(ctxt->cr2));
> + asm volatile("movq %%cr8, %0" : "=a"(ctxt->cr8));
> +
> + asm volatile("movl %%cs, %%eax" : "=a"(ctxt->cs));
> + asm volatile("movl %%ss, %%eax" : "=a"(ctxt->ss));
> + asm volatile("movl %%ds, %%eax" : "=a"(ctxt->ds));
> + asm volatile("movl %%es, %%eax" : "=a"(ctxt->es));
> + asm volatile("movl %%fs, %%eax" : "=a"(ctxt->fs));
> + asm volatile("movl %%gs, %%eax" : "=a"(ctxt->gs));
> +
> + native_store_gdt(&ctxt->gdtr);
> + store_idt(&ctxt->idtr);
> +
> + ctxt->gsbase = __rdmsr(MSR_GS_BASE);
> + ctxt->efer = __rdmsr(MSR_EFER);
> + ctxt->pat = __rdmsr(MSR_IA32_CR_PAT);
> +}
> +
> +/* Add trampoline page to the kernel pagetable for transition to kernel PT */
> +static void hv_crash_fixup_kernpt(void)
> +{
> + pgd_t *pgd;
> + p4d_t *p4d;
> +
> + pgd = pgd_offset_k(trampoline_pa);
> + p4d = p4d_offset(pgd, trampoline_pa);
> +
> + /* trampoline_pa is below 4G, so no pre-existing entry to clobber */
> + p4d_populate(&init_mm, p4d, (pud_t *)hv_crash_ptpgs[1]);
> + p4d->p4d = p4d->p4d & ~(_PAGE_NX); /* enable execute */
> +}
> +
> +/*
> + * Now that all cpus are in nmi and spinning, we notify the hyp that linux has
> + * crashed and will collect core. This will cause the hyp to quiesce and
> + * suspend all VPs except the bsp. Called if linux crashed and not the hyp.
> + */
> +static void hv_notify_prepare_hyp(void)
> +{
> + u64 status;
> + struct hv_input_notify_partition_event *input;
> + struct hv_partition_event_root_crashdump_input *cda;
> +
> + input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> + cda = &input->input.crashdump_input;
The code ordering here is a bit weird. I'd expect this line to be grouped
with cda->crashdump_action being set.
> + memset(input, 0, sizeof(*input));
> + input->event = HV_PARTITION_EVENT_ROOT_CRASHDUMP;
> +
> + cda->crashdump_action = HV_CRASHDUMP_ENTRY;
> + status = hv_do_hypercall(HVCALL_NOTIFY_PARTITION_EVENT, input, NULL);
> + if (!hv_result_success(status))
> + return;
> +
> + cda->crashdump_action = HV_CRASHDUMP_SUSPEND_ALL_VPS;
> + hv_do_hypercall(HVCALL_NOTIFY_PARTITION_EVENT, input, NULL);
> +}
> +
> +/*
> + * Common function for all cpus before devirtualization.
> + *
> + * Hypervisor crash: all cpus get here in nmi context.
> + * Linux crash: the panicing cpu gets here at base level, all others in nmi
> + * context. Note, panicing cpu may not be the bsp.
> + *
> + * The function is not inlined so it will show on the stack. It is named so
> + * because the crash cmd looks for certain well known function names on the
> + * stack before looking into the cpu saved note in the elf section, and
> + * that work is currently incomplete.
> + *
> + * Notes:
> + * Hypervisor crash:
> + * - the hypervisor is in a very restrictive mode at this point and any
> + * vmexit it cannot handle would result in reboot. For example, console
> + * output from here would result in synic ipi hcall, which would result
> + * in reboot. So, no mumbo jumbo, just get to kexec as quickly as possible.
> + *
> + * Devirtualization is supported from the bsp only.
> + */
> +static noinline __noclone void crash_nmi_callback(struct pt_regs *regs)
> +{
> + struct hv_input_disable_hyp_ex *input;
> + u64 status;
> + int msecs = 1000, ccpu = smp_processor_id();
> +
> + if (ccpu == 0) {
> + /* crash_save_cpu() will be done in the kexec path */
> + cpu_emergency_stop_pt(); /* disable performance trace */
> + atomic_inc(&crash_cpus_wait);
> + } else {
> + crash_save_cpu(regs, ccpu);
> + cpu_emergency_stop_pt(); /* disable performance trace */
> + atomic_inc(&crash_cpus_wait);
> + for (;;); /* cause no vmexits */
> + }
> +
> + while (atomic_read(&crash_cpus_wait) < num_online_cpus() && msecs--)
> + mdelay(1);
> +
> + stop_nmi();
> + if (!hv_has_crashed)
> + hv_notify_prepare_hyp();
> +
> + if (crashing_cpu == -1)
> + crashing_cpu = ccpu; /* crash cmd uses this */
Could just be "crashing_cpu = 0" since only the BSP gets here.
> +
> + hv_hvcrash_ctxt_save();
> + hv_mark_tss_not_busy();
> + hv_crash_fixup_kernpt();
> +
> + input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> + memset(input, 0, sizeof(*input));
> + input->rip = trampoline_pa; /* PA of hv_crash_asm32 */
> + input->arg = devirt_cr3arg; /* PA of trampoline page table L4 */
Is this comment correct? Isn't it the PA of struct hv_crash_tramp_data?
And just for clarification, Hyper-V treats this "arg" value as opaque and does
not access it. It only provides it in EDI when it invokes the trampoline
function, right?
> +
> + status = hv_do_hypercall(HVCALL_DISABLE_HYP_EX, input, NULL);
> +
> + /* Devirt failed, just reboot as things are in very bad state now */
> + native_wrmsrq(HV_X64_MSR_RESET, 1); /* get hv to reboot */
> +}
> +
> +/*
> + * Generic nmi callback handler: could be called without any crash also.
> + * hv crash: hypervisor injects nmi's into all cpus
> + * lx crash: panicing cpu sends nmi to all but self via crash_stop_other_cpus
> + */
> +static int hv_crash_nmi_local(unsigned int cmd, struct pt_regs *regs)
> +{
> + int ccpu = smp_processor_id();
> +
> + if (!hv_has_crashed && hv_cda && hv_cda->cda_valid)
> + hv_has_crashed = 1;
> +
> + if (!hv_has_crashed && !lx_has_crashed)
> + return NMI_DONE; /* ignore the nmi */
> +
> + if (hv_has_crashed) {
> + if (!kexec_crash_loaded() || !hv_crash_enabled) {
> + if (ccpu == 0) {
> + native_wrmsrq(HV_X64_MSR_RESET, 1); /* reboot */
> + } else
> + for (;;); /* cause no vmexits */
> + }
> + }
> +
> + crash_nmi_callback(regs);
> +
> + return NMI_DONE;
crash_nmi_callback() should never return, right? Normally one would
expect to return NMI_HANDLED here, but I guess it doesn't matter
if the return is never executed.
> +}
> +
> +/*
> + * hv_crash_stop_other_cpus() == smp_ops.crash_stop_other_cpus
> + *
> + * On normal linux panic, this is called twice: first from panic and then again
> + * from native_machine_crash_shutdown.
> + *
> + * In case of mshv, 3 ways to get here:
> + * 1. hv crash (only bsp will get here):
> + * BSP : nmi callback -> DisableHv -> hv_crash_asm32 -> hv_crash_c_entry
> + * -> __crash_kexec -> native_machine_crash_shutdown
> + * -> crash_smp_send_stop -> smp_ops.crash_stop_other_cpus
> + * linux panic:
> + * 2. panic cpu x: panic() -> crash_smp_send_stop
> + * -> smp_ops.crash_stop_other_cpus
> + * 3. bsp: native_machine_crash_shutdown -> crash_smp_send_stop
> + *
> + * NB: noclone and non standard stack because of call to crash_setup_regs().
> + */
> +static void __noclone hv_crash_stop_other_cpus(void)
> +{
> + static int crash_stop_done;
> + struct pt_regs lregs;
> + int ccpu = smp_processor_id();
> +
> + if (hv_has_crashed)
> + return; /* all cpus already in nmi handler path */
> +
> + if (!kexec_crash_loaded())
> + return;
If we're in a normal panic path (your Case #2 above) with no kdump kernel
loaded, why leave the other vCPUs running? Seems like that could violate
expectations in vpanic(), where it calls panic_other_cpus_shutdown() and
thereafter assumes other vCPUs are not running.
> +
> + if (crash_stop_done)
> + return;
> + crash_stop_done = 1;
Is crash_stop_done necessary? hv_crash_stop_other_cpus() is called
from crash_smp_send_stop(), which has its own static variable
"cpus_stopped" that does the same thing.
> +
> + /* linux has crashed: hv is healthy, we can ipi safely */
> + lx_has_crashed = 1;
> + wmb(); /* nmi handlers look at lx_has_crashed */
> +
> + apic->send_IPI_allbutself(NMI_VECTOR);
The default .crash_stop_other_cpus function is kdump_nmi_shootdown_cpus().
In addition to sending the NMI IPI, it does disable_local_APIC(). I don't know, but
should disable_local_APIC() be done somewhere here as well?
> +
> + if (crashing_cpu == -1)
> + crashing_cpu = ccpu; /* crash cmd uses this */
> +
> + /* crash_setup_regs() happens in kexec also, but for the kexec cpu which
> + * is the bsp. We could be here on non-bsp cpu, collect regs if so.
> + */
> + if (ccpu)
> + crash_setup_regs(&lregs, NULL);
> +
> + crash_nmi_callback(&lregs);
> +}
> +STACK_FRAME_NON_STANDARD(hv_crash_stop_other_cpus);
> +
> +/* This GDT is accessed in IA32-e compat mode which uses 32bits addresses */
> +struct hv_gdtreg_32 {
> + u16 fill;
> + u16 limit;
> + u32 address;
> +} __packed;
> +
> +/* We need a CS with L bit to goto IA32-e long mode from 32bit compat mode */
> +struct hv_crash_tramp_gdt {
> + u64 null; /* index 0, selector 0, null selector */
> + u64 cs64; /* index 1, selector 8, cs64 selector */
> +} __packed;
> +
> +/* No stack, so jump via far ptr in memory to load the 64bit CS */
> +struct hv_cs_jmptgt {
> + u32 address;
> + u16 csval;
> + u16 fill;
> +} __packed;
> +
> +/* This trampoline data is copied onto the trampoline page after the asm code */
> +struct hv_crash_tramp_data {
> + u64 tramp32_cr3;
> + u64 kernel_cr3;
> + struct hv_gdtreg_32 gdtr32;
> + struct hv_crash_tramp_gdt tramp_gdt;
> + struct hv_cs_jmptgt cs_jmptgt;
> + u64 c_entry_addr;
> +} __packed;
> +
> +/*
> + * Setup a temporary gdt to allow the asm code to switch to the long mode.
> + * Since the asm code is relocated/copied to a below 4G page, it cannot use rip
> + * relative addressing, hence we must use trampoline_pa here. Also, save other
> + * info like jmp and C entry targets for same reasons.
> + *
> + * Returns: 0 on success, -1 on error
> + */
> +static int hv_crash_setup_trampdata(u64 trampoline_va)
> +{
> + int size, offs;
> + void *dest;
> + struct hv_crash_tramp_data *tramp;
> +
> + /* These must match exactly the ones in the corresponding asm file */
> + BUILD_BUG_ON(offsetof(struct hv_crash_tramp_data, tramp32_cr3) != 0);
> + BUILD_BUG_ON(offsetof(struct hv_crash_tramp_data, kernel_cr3) != 8);
> + BUILD_BUG_ON(offsetof(struct hv_crash_tramp_data, gdtr32.limit) != 18);
> + BUILD_BUG_ON(offsetof(struct hv_crash_tramp_data,
> + cs_jmptgt.address) != 40);
It would be nice to pick up the constants from a #include file that is
shared with the asm code in Patch 4 of the series.
> +
> + /* hv_crash_asm_end is beyond last byte by 1 */
> + size = &hv_crash_asm_end - &hv_crash_asm32;
> + if (size + sizeof(struct hv_crash_tramp_data) > PAGE_SIZE) {
> + pr_err("%s: trampoline page overflow\n", __func__);
> + return -1;
> + }
> +
> + dest = (void *)trampoline_va;
> + memcpy(dest, &hv_crash_asm32, size);
> +
> + dest += size;
> + dest = (void *)round_up((ulong)dest, 16);
> + tramp = (struct hv_crash_tramp_data *)dest;
> +
> + /* see MAX_ASID_AVAILABLE in tlb.c: "PCID 0 is reserved for use by
> + * non-PCID-aware users". Build cr3 with pcid 0
> + */
> + tramp->tramp32_cr3 = __sme_pa(hv_crash_ptpgs[0]);
> +
> + /* Note, when restoring X86_CR4_PCIDE, cr3[11:0] must be zero */
> + tramp->kernel_cr3 = __sme_pa(init_mm.pgd);
> +
> + tramp->gdtr32.limit = sizeof(struct hv_crash_tramp_gdt);
> + tramp->gdtr32.address = trampoline_pa +
> + (ulong)&tramp->tramp_gdt - trampoline_va;
> +
> + /* base:0 limit:0xfffff type:b dpl:0 P:1 L:1 D:0 avl:0 G:1 */
> + tramp->tramp_gdt.cs64 = 0x00af9a000000ffff;
> +
> + tramp->cs_jmptgt.csval = 0x8;
> + offs = (ulong)&hv_crash_asm64_lbl - (ulong)&hv_crash_asm32;
> + tramp->cs_jmptgt.address = trampoline_pa + offs;
> +
> + tramp->c_entry_addr = (u64)&hv_crash_c_entry;
> +
> + devirt_cr3arg = trampoline_pa + (ulong)dest - trampoline_va;
> +
> + return 0;
> +}
> +
> +/*
> + * Build 32bit trampoline page table for transition from protected mode
> + * non-paging to long-mode paging. This transition needs pagetables below 4G.
> + */
> +static void hv_crash_build_tramp_pt(void)
> +{
> + p4d_t *p4d;
> + pud_t *pud;
> + pmd_t *pmd;
> + pte_t *pte;
> + u64 pa, addr = trampoline_pa;
> +
> + p4d = hv_crash_ptpgs[0] + pgd_index(addr) * sizeof(p4d);
> + pa = virt_to_phys(hv_crash_ptpgs[1]);
> + set_p4d(p4d, __p4d(_PAGE_TABLE | pa));
> + p4d->p4d &= ~(_PAGE_NX); /* disable no execute */
> +
> + pud = hv_crash_ptpgs[1] + pud_index(addr) * sizeof(pud);
> + pa = virt_to_phys(hv_crash_ptpgs[2]);
> + set_pud(pud, __pud(_PAGE_TABLE | pa));
> +
> + pmd = hv_crash_ptpgs[2] + pmd_index(addr) * sizeof(pmd);
> + pa = virt_to_phys(hv_crash_ptpgs[3]);
> + set_pmd(pmd, __pmd(_PAGE_TABLE | pa));
> +
> + pte = hv_crash_ptpgs[3] + pte_index(addr) * sizeof(pte);
> + set_pte(pte, pfn_pte(addr >> PAGE_SHIFT, PAGE_KERNEL_EXEC));
> +}
> +
> +/*
> + * Setup trampoline for devirtualization:
> + * - a page below 4G, ie 32bit addr containing asm glue code that mshv jmps to
> + * in protected mode.
> + * - 4 pages for a temporary page table that asm code uses to turn paging on
> + * - a temporary gdt to use in the compat mode.
> + *
> + * Returns: 0 on success
> + */
> +static int hv_crash_trampoline_setup(void)
> +{
> + int i, rc, order;
> + struct page *page;
> + u64 trampoline_va;
> + gfp_t flags32 = GFP_KERNEL | GFP_DMA32 | __GFP_ZERO;
> +
> + /* page for 32bit trampoline assembly code + hv_crash_tramp_data */
> + page = alloc_page(flags32);
> + if (page == NULL) {
> + pr_err("%s: failed to alloc asm stub page\n", __func__);
> + return -1;
> + }
> +
> + trampoline_va = (u64)page_to_virt(page);
> + trampoline_pa = (u32)page_to_phys(page);
> +
> + order = 2; /* alloc 2^2 pages */
> + page = alloc_pages(flags32, order);
> + if (page == NULL) {
> + pr_err("%s: failed to alloc pt pages\n", __func__);
> + free_page(trampoline_va);
> + return -1;
> + }
> +
> + for (i = 0; i < 4; i++, page++)
> + hv_crash_ptpgs[i] = page_to_virt(page);
> +
> + hv_crash_build_tramp_pt();
> +
> + rc = hv_crash_setup_trampdata(trampoline_va);
> + if (rc)
> + goto errout;
> +
> + return 0;
> +
> +errout:
> + free_page(trampoline_va);
> + free_pages((ulong)hv_crash_ptpgs[0], order);
> +
> + return rc;
> +}
> +
> +/* Setup for kdump kexec to collect hypervisor ram when running as mshv root */
> +void hv_root_crash_init(void)
> +{
> + int rc;
> + struct hv_input_get_system_property *input;
> + struct hv_output_get_system_property *output;
> + unsigned long flags;
> + u64 status;
> + union hv_pfn_range cda_info;
> +
> + if (pgtable_l5_enabled()) {
> + pr_err("Hyper-V: crash dump not yet supported on 5level PTs\n");
> + return;
> + }
> +
> + rc = register_nmi_handler(NMI_LOCAL, hv_crash_nmi_local, NMI_FLAG_FIRST,
> + "hv_crash_nmi");
> + if (rc) {
> + pr_err("Hyper-V: failed to register crash nmi handler\n");
> + return;
> + }
> +
> + local_irq_save(flags);
> + input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> + output = *this_cpu_ptr(hyperv_pcpu_output_arg);
> +
> + memset(input, 0, sizeof(*input));
> + memset(output, 0, sizeof(*output));
Why zero the output area? This is one of those hypercall things that we're
inconsistent about. A few hypercall call sites zero the output area, and it's
not clear why they do. Hyper-V should be responsible for properly filling in
the output area. Linux should not need to do this zero'ing, unless there's some
known bug in Hyper-V for certain hypercalls, in which case there should be
a code comment stating "why".
> + input->property_id = HV_SYSTEM_PROPERTY_CRASHDUMPAREA;
> +
> + status = hv_do_hypercall(HVCALL_GET_SYSTEM_PROPERTY, input, output);
> + cda_info.as_uint64 = output->hv_cda_info.as_uint64;
> + local_irq_restore(flags);
> +
> + if (!hv_result_success(status)) {
> + pr_err("Hyper-V: %s: property:%d %s\n", __func__,
> + input->property_id, hv_result_to_string(status));
> + goto err_out;
> + }
> +
> + if (cda_info.base_pfn == 0) {
> + pr_err("Hyper-V: hypervisor crash dump area pfn is 0\n");
> + goto err_out;
> + }
> +
> + hv_cda = phys_to_virt(cda_info.base_pfn << PAGE_SHIFT);
Use HV_HYP_PAGE_SHIFT, since PFNs provided by Hyper-V are always in
terms of the Hyper-V page size, which isn't necessarily the guest page size.
Yes, on x86 there's no difference, but for future robustness ....
> +
> + rc = hv_crash_trampoline_setup();
> + if (rc)
> + goto err_out;
> +
> + smp_ops.crash_stop_other_cpus = hv_crash_stop_other_cpus;
> +
> + crash_kexec_post_notifiers = true;
> + hv_crash_enabled = 1;
> + pr_info("Hyper-V: linux and hv kdump support enabled\n");
This message and the message below aren't consistent. One refers
to "hv kdump" and the other to "hyp kdump".
> +
> + return;
> +
> +err_out:
> + unregister_nmi_handler(NMI_LOCAL, "hv_crash_nmi");
> + pr_err("Hyper-V: only linux (but not hyp) kdump support enabled\n");
> +}
> --
> 2.36.1.vfs.0.0
>
^ permalink raw reply
* RE: [PATCH v1 6/6] x86/hyperv: Enable build of hypervisor crashdump collection files
From: Michael Kelley @ 2025-09-15 17:56 UTC (permalink / raw)
To: Mukesh Rathor, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org,
hpa@zytor.com, arnd@arndb.de
In-Reply-To: <20250910001009.2651481-7-mrathor@linux.microsoft.com>
From: Mukesh Rathor <mrathor@linux.microsoft.com> Sent: Tuesday, September 9, 2025 5:10 PM
>
> Enable build of the new files introduced in the earlier commits and add
> call to do the setup during boot.
>
> Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
> ---
> arch/x86/hyperv/Makefile | 6 ++++++
> arch/x86/hyperv/hv_init.c | 1 +
> include/asm-generic/mshyperv.h | 9 +++++++++
> 3 files changed, 16 insertions(+)
>
> diff --git a/arch/x86/hyperv/Makefile b/arch/x86/hyperv/Makefile
> index d55f494f471d..6f5d97cddd80 100644
> --- a/arch/x86/hyperv/Makefile
> +++ b/arch/x86/hyperv/Makefile
> @@ -5,4 +5,10 @@ obj-$(CONFIG_HYPERV_VTL_MODE) += hv_vtl.o
>
> ifdef CONFIG_X86_64
> obj-$(CONFIG_PARAVIRT_SPINLOCKS) += hv_spinlock.o
> +
> + ifdef CONFIG_MSHV_ROOT
> + CFLAGS_REMOVE_hv_trampoline.o += -pg
> + CFLAGS_hv_trampoline.o += -fno-stack-protector
> + obj-$(CONFIG_CRASH_DUMP) += hv_crash.o hv_trampoline.o
> + endif
> endif
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index afdbda2dd7b7..577bbd143527 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -510,6 +510,7 @@ void __init hyperv_init(void)
> memunmap(src);
>
> hv_remap_tsc_clocksource();
> + hv_root_crash_init();
> } else {
> hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg);
> wrmsrq(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index dbd4c2f3aee3..952c221765f5 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -367,6 +367,15 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32
> num_pages);
> int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
> int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags);
>
> +#if CONFIG_CRASH_DUMP
> +void hv_root_crash_init(void);
> +void hv_crash_asm32(void);
> +void hv_crash_asm64_lbl(void);
> +void hv_crash_asm_end(void);
> +#else /* CONFIG_CRASH_DUMP */
> +static inline void hv_root_crash_init(void) {}
> +#endif /* CONFIG_CRASH_DUMP */
> +
The hv_crash_asm* functions are x86 specific. Seems like their
declarations should go in arch/x86/include/asm/mshyperv.h, not in
the architecture-neutral include/asm-generic/mshyperv.h.
> #else /* CONFIG_MSHV_ROOT */
> static inline bool hv_root_partition(void) { return false; }
> static inline bool hv_l1vh_partition(void) { return false; }
> --
> 2.36.1.vfs.0.0
>
^ permalink raw reply
* Re: [PATCH] x86/hyperv: Export hv_hypercall_pg unconditionally
From: Sean Christopherson @ 2025-09-15 21:46 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Naman Jain, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H . Peter Anvin, linux-hyperv, linux-kernel,
mhklinux
In-Reply-To: <20250826120752.GW4067720@noisy.programming.kicks-ass.net>
On Tue, Aug 26, 2025, Peter Zijlstra wrote:
> On Tue, Aug 26, 2025 at 05:00:31PM +0530, Naman Jain wrote:
> >
> >
> > On 8/25/2025 3:12 PM, Peter Zijlstra wrote:
> > > On Mon, Aug 25, 2025 at 11:22:08AM +0530, Naman Jain wrote:
> > > > With commit 0e20f1f4c2cb ("x86/hyperv: Clean up hv_do_hypercall()"),
> > > > config checks were added to conditionally restrict export
> > > > of hv_hypercall_pg symbol at the same time when a usage of that symbol
> > > > was added in mshv_vtl_main.c driver. This results in missing symbol
> > > > warning when mshv_vtl_main is compiled. Change the logic to
> > > > export it unconditionally.
> > > >
> > > > Fixes: 96a1d2495c2f ("Drivers: hv: Introduce mshv_vtl driver")
> > > > Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> > >
> > > Oh gawd, that commit is terrible and adds yet another hypercall
> > > interface.
> > >
> > > I would argue the proper fix is moving the whole of mshv_vtl_return()
> > > into the kernel proper and doing it like hv_std_hypercall() on x86_64.
> >
> > Thanks for the review comments.
> >
> > This is doable, I can move the hypercall part of it to
> > arch/x86/hyperv/hv_init.c if I understand correctly.
> >
> > >
> > > Additionally, how is that function not utterly broken? What happens if
> > > an interrupt or NMI comes in after native_write_cr2() and before the
> > > actual hypercall does VMEXIT and trips a #PF?
> >
> > mshv_vtl driver is used for OpenHCL paravisor. The interrupts are
> > disabled, and NMIs aren't sent to the paravisor by the virt stack.
>
> I do not know what OpenHCL is. Nor is it clear from the code what NMIs
> can't happen.
FWIW, NMIs likely aren't a problem because the NMI handler saves/restores CR2
specifically to guard against #PFs in NMI context clobbering guest state. AMD
CPUs can block NMIs via GIF=0, but blocking NMIs on Intel for the VM-Entry =>
VM-Exit would require worse hacks than saving/restoring CR2 in the NMI handler.
:-(
> Anyway, same can be achieved with breakpoints / kprobes. You can get a trap
> after setting CR2 and scribble it.
Ya, KVM marks everything for vmx_vcpu_enter_exit() to/from VM-Enter/VM-Exit as
noinstr (no instrumentation) to prevent breakpoints/kprobes from clobbering CR2
and other state (and DR7 is zero for good measure).
^ permalink raw reply
* [PATCH v2 0/2] Fix CONFIG_HYPERV and vmbus related anamoly
From: Mukesh Rathor @ 2025-09-15 23:46 UTC (permalink / raw)
To: dri-devel, linux-kernel, linux-input, linux-hyperv, netdev,
linux-pci, linux-scsi, linux-fbdev, linux-arch, virtualization
Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona, jikos,
bentiss, kys, haiyangz, wei.liu, decui, dmitry.torokhov,
andrew+netdev, davem, edumazet, kuba, pabeni, bhelgaas,
James.Bottomley, martin.petersen, gregkh, deller, arnd, sgarzare,
horms
At present, drivers/Makefile will subst =m to =y for CONFIG_HYPERV
for hv subdir. Also, drivers/hv/Makefile replaces =m to =y to build in
hv_common.c that is needed for the drivers. Moreover, vmbus driver is
built if CONFIG_HYPER is set, either loadable or builtin.
This is not a good approach. CONFIG_HYPERV is really an umbrella
config that encompasses builtin code and various other things and not
a dedicated config option for VMBus. VMBus should really have a config
option just like CONFIG_HYPERV_BALLOON etc. This small series introduces
CONFIG_HYPERV_VMBUS to build VMBus driver and make that distinction
explicit. With that CONFIG_HYPERV could be changed to bool.
For now, hv_common.c is left as is to reduce conflicts for upcoming
patches, but once merges are mostly done, that and some others should
be moved to virt/hyperv directory.
V2:
o rebased on hyper-next: commit 553d825fb2f0
("x86/hyperv: Switch to msi_create_parent_irq_domain()")
V1:
o Change subject from hyper-v to "Drivers: hv:"
o Rewrite commit messages paying attention to VMBus and not vmbus
o Change some wordings in Kconfig
o Make new VMBUS config option default to HYPERV option for a smoother
transition
Mukesh Rathor (2):
Driver: hv: Add CONFIG_HYPERV_VMBUS option
Drivers: hv: Make CONFIG_HYPERV bool
drivers/Makefile | 2 +-
drivers/gpu/drm/Kconfig | 2 +-
drivers/hid/Kconfig | 2 +-
drivers/hv/Kconfig | 13 ++++++++++---
drivers/hv/Makefile | 4 ++--
drivers/input/serio/Kconfig | 4 ++--
drivers/net/hyperv/Kconfig | 2 +-
drivers/pci/Kconfig | 2 +-
drivers/scsi/Kconfig | 2 +-
drivers/uio/Kconfig | 2 +-
drivers/video/fbdev/Kconfig | 2 +-
include/asm-generic/mshyperv.h | 8 +++++---
net/vmw_vsock/Kconfig | 2 +-
13 files changed, 28 insertions(+), 19 deletions(-)
--
2.36.1.vfs.0.0
^ permalink raw reply
* [PATCH v2 1/2] Driver: hv: Add CONFIG_HYPERV_VMBUS option
From: Mukesh Rathor @ 2025-09-15 23:46 UTC (permalink / raw)
To: dri-devel, linux-kernel, linux-input, linux-hyperv, netdev,
linux-pci, linux-scsi, linux-fbdev, linux-arch, virtualization
Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona, jikos,
bentiss, kys, haiyangz, wei.liu, decui, dmitry.torokhov,
andrew+netdev, davem, edumazet, kuba, pabeni, bhelgaas,
James.Bottomley, martin.petersen, gregkh, deller, arnd, sgarzare,
horms
In-Reply-To: <20250915234604.3256611-1-mrathor@linux.microsoft.com>
At present VMBus driver is hinged off of CONFIG_HYPERV which entails
lot of builtin code and encompasses too much. It's not always clear
what depends on builtin hv code and what depends on VMBus. Setting
CONFIG_HYPERV as a module and fudging the Makefile to switch to builtin
adds even more confusion. VMBus is an independent module and should have
its own config option. Also, there are scenarios like baremetal dom0/root
where support is built in with CONFIG_HYPERV but without VMBus. Lastly,
there are more features coming down that use CONFIG_HYPERV and add more
dependencies on it.
So, create a fine grained HYPERV_VMBUS option and update Kconfigs for
dependency on VMBus.
Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
---
drivers/gpu/drm/Kconfig | 2 +-
drivers/hid/Kconfig | 2 +-
drivers/hv/Kconfig | 11 +++++++++--
drivers/hv/Makefile | 2 +-
drivers/input/serio/Kconfig | 4 ++--
drivers/net/hyperv/Kconfig | 2 +-
drivers/pci/Kconfig | 2 +-
drivers/scsi/Kconfig | 2 +-
drivers/uio/Kconfig | 2 +-
drivers/video/fbdev/Kconfig | 2 +-
include/asm-generic/mshyperv.h | 8 +++++---
net/vmw_vsock/Kconfig | 2 +-
12 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index f7ea8e895c0c..58f34da061c6 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -398,7 +398,7 @@ source "drivers/gpu/drm/imagination/Kconfig"
config DRM_HYPERV
tristate "DRM Support for Hyper-V synthetic video device"
- depends on DRM && PCI && HYPERV
+ depends on DRM && PCI && HYPERV_VMBUS
select DRM_CLIENT_SELECTION
select DRM_KMS_HELPER
select DRM_GEM_SHMEM_HELPER
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index a57901203aeb..fe3dc8c0db99 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1162,7 +1162,7 @@ config GREENASIA_FF
config HID_HYPERV_MOUSE
tristate "Microsoft Hyper-V mouse driver"
- depends on HYPERV
+ depends on HYPERV_VMBUS
help
Select this option to enable the Hyper-V mouse driver.
diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index e24f6299c376..29f8637f441a 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -45,18 +45,25 @@ config HYPERV_TIMER
config HYPERV_UTILS
tristate "Microsoft Hyper-V Utilities driver"
- depends on HYPERV && CONNECTOR && NLS
+ depends on HYPERV_VMBUS && CONNECTOR && NLS
depends on PTP_1588_CLOCK_OPTIONAL
help
Select this option to enable the Hyper-V Utilities.
config HYPERV_BALLOON
tristate "Microsoft Hyper-V Balloon driver"
- depends on HYPERV
+ depends on HYPERV_VMBUS
select PAGE_REPORTING
help
Select this option to enable Hyper-V Balloon driver.
+config HYPERV_VMBUS
+ tristate "Microsoft Hyper-V VMBus driver"
+ depends on HYPERV
+ default HYPERV
+ help
+ Select this option to enable Hyper-V Vmbus driver.
+
config MSHV_ROOT
tristate "Microsoft Hyper-V root partition support"
depends on HYPERV && (X86_64 || ARM64)
diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index 976189c725dc..4bb41663767d 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_HYPERV) += hv_vmbus.o
+obj-$(CONFIG_HYPERV_VMBUS) += hv_vmbus.o
obj-$(CONFIG_HYPERV_UTILS) += hv_utils.o
obj-$(CONFIG_HYPERV_BALLOON) += hv_balloon.o
obj-$(CONFIG_MSHV_ROOT) += mshv_root.o
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 17edc1597446..c7ef347a4dff 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -276,8 +276,8 @@ config SERIO_OLPC_APSP
config HYPERV_KEYBOARD
tristate "Microsoft Synthetic Keyboard driver"
- depends on HYPERV
- default HYPERV
+ depends on HYPERV_VMBUS
+ default HYPERV_VMBUS
help
Select this option to enable the Hyper-V Keyboard driver.
diff --git a/drivers/net/hyperv/Kconfig b/drivers/net/hyperv/Kconfig
index c8cbd85adcf9..982964c1a9fb 100644
--- a/drivers/net/hyperv/Kconfig
+++ b/drivers/net/hyperv/Kconfig
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
config HYPERV_NET
tristate "Microsoft Hyper-V virtual network driver"
- depends on HYPERV
+ depends on HYPERV_VMBUS
select UCS2_STRING
select NLS
help
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 9a249c65aedc..7065a8e5f9b1 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -221,7 +221,7 @@ config PCI_LABEL
config PCI_HYPERV
tristate "Hyper-V PCI Frontend"
- depends on ((X86 && X86_64) || ARM64) && HYPERV && PCI_MSI && SYSFS
+ depends on ((X86 && X86_64) || ARM64) && HYPERV_VMBUS && PCI_MSI && SYSFS
select PCI_HYPERV_INTERFACE
select IRQ_MSI_LIB
help
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 5522310bab8d..19d0884479a2 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -589,7 +589,7 @@ config XEN_SCSI_FRONTEND
config HYPERV_STORAGE
tristate "Microsoft Hyper-V virtual storage driver"
- depends on SCSI && HYPERV
+ depends on SCSI && HYPERV_VMBUS
depends on m || SCSI_FC_ATTRS != m
default HYPERV
help
diff --git a/drivers/uio/Kconfig b/drivers/uio/Kconfig
index b060dcd7c635..6f86a61231e6 100644
--- a/drivers/uio/Kconfig
+++ b/drivers/uio/Kconfig
@@ -140,7 +140,7 @@ config UIO_MF624
config UIO_HV_GENERIC
tristate "Generic driver for Hyper-V VMBus"
- depends on HYPERV
+ depends on HYPERV_VMBUS
help
Generic driver that you can bind, dynamically, to any
Hyper-V VMBus device. It is useful to provide direct access
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index c21484d15f0c..72c63eaeb983 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -1774,7 +1774,7 @@ config FB_BROADSHEET
config FB_HYPERV
tristate "Microsoft Hyper-V Synthetic Video support"
- depends on FB && HYPERV
+ depends on FB && HYPERV_VMBUS
select DMA_CMA if HAVE_DMA_CONTIGUOUS && CMA
select FB_IOMEM_HELPERS_DEFERRED
help
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index dbd4c2f3aee3..64ba6bc807d9 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -163,6 +163,7 @@ static inline u64 hv_generate_guest_id(u64 kernel_version)
return guest_id;
}
+#if IS_ENABLED(CONFIG_HYPERV_VMBUS)
/* Free the message slot and signal end-of-message if required */
static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
{
@@ -198,6 +199,10 @@ static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
}
}
+extern int vmbus_interrupt;
+extern int vmbus_irq;
+#endif /* CONFIG_HYPERV_VMBUS */
+
int hv_get_hypervisor_version(union hv_hypervisor_version_info *info);
void hv_setup_vmbus_handler(void (*handler)(void));
@@ -211,9 +216,6 @@ void hv_setup_crash_handler(void (*handler)(struct pt_regs *regs));
void hv_remove_crash_handler(void);
void hv_setup_mshv_handler(void (*handler)(void));
-extern int vmbus_interrupt;
-extern int vmbus_irq;
-
#if IS_ENABLED(CONFIG_HYPERV)
/*
* Hypervisor's notion of virtual processor ID is different from
diff --git a/net/vmw_vsock/Kconfig b/net/vmw_vsock/Kconfig
index 56356d2980c8..8e803c4828c4 100644
--- a/net/vmw_vsock/Kconfig
+++ b/net/vmw_vsock/Kconfig
@@ -72,7 +72,7 @@ config VIRTIO_VSOCKETS_COMMON
config HYPERV_VSOCKETS
tristate "Hyper-V transport for Virtual Sockets"
- depends on VSOCKETS && HYPERV
+ depends on VSOCKETS && HYPERV_VMBUS
help
This module implements a Hyper-V transport for Virtual Sockets.
--
2.36.1.vfs.0.0
^ permalink raw reply related
* [PATCH v2 2/2] Drivers: hv: Make CONFIG_HYPERV bool
From: Mukesh Rathor @ 2025-09-15 23:46 UTC (permalink / raw)
To: dri-devel, linux-kernel, linux-input, linux-hyperv, netdev,
linux-pci, linux-scsi, linux-fbdev, linux-arch, virtualization
Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona, jikos,
bentiss, kys, haiyangz, wei.liu, decui, dmitry.torokhov,
andrew+netdev, davem, edumazet, kuba, pabeni, bhelgaas,
James.Bottomley, martin.petersen, gregkh, deller, arnd, sgarzare,
horms
In-Reply-To: <20250915234604.3256611-1-mrathor@linux.microsoft.com>
With CONFIG_HYPERV and CONFIG_HYPERV_VMBUS separated, change CONFIG_HYPERV
to bool from tristate. CONFIG_HYPERV now becomes the core Hyper-V
hypervisor support, such as hypercalls, clocks/timers, Confidential
Computing setup, PCI passthru, etc. that doesn't involve VMBus or VMBus
devices.
Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
---
drivers/Makefile | 2 +-
drivers/hv/Kconfig | 2 +-
drivers/hv/Makefile | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/Makefile b/drivers/Makefile
index b5749cf67044..7ad5744db0b6 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -161,7 +161,7 @@ obj-$(CONFIG_SOUNDWIRE) += soundwire/
# Virtualization drivers
obj-$(CONFIG_VIRT_DRIVERS) += virt/
-obj-$(subst m,y,$(CONFIG_HYPERV)) += hv/
+obj-$(CONFIG_HYPERV) += hv/
obj-$(CONFIG_PM_DEVFREQ) += devfreq/
obj-$(CONFIG_EXTCON) += extcon/
diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index 29f8637f441a..0b8c391a0342 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -3,7 +3,7 @@
menu "Microsoft Hyper-V guest support"
config HYPERV
- tristate "Microsoft Hyper-V client drivers"
+ bool "Microsoft Hyper-V core hypervisor support"
depends on (X86 && X86_LOCAL_APIC && HYPERVISOR_GUEST) \
|| (ARM64 && !CPU_BIG_ENDIAN)
select PARAVIRT
diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index 4bb41663767d..1a1677bf4dac 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -16,5 +16,5 @@ mshv_root-y := mshv_root_main.o mshv_synic.o mshv_eventfd.o mshv_irq.o \
mshv_root_hv_call.o mshv_portid_table.o
# Code that must be built-in
-obj-$(subst m,y,$(CONFIG_HYPERV)) += hv_common.o
+obj-$(CONFIG_HYPERV) += hv_common.o
obj-$(subst m,y,$(CONFIG_MSHV_ROOT)) += hv_proc.o mshv_common.o
--
2.36.1.vfs.0.0
^ permalink raw reply related
* Re: [PATCH v1 3/6] hyperv: Add definitions for hypervisor crash dump support
From: Mukesh R @ 2025-09-16 1:15 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org,
hpa@zytor.com, arnd@arndb.de
In-Reply-To: <SN6PR02MB41577F7E862976DE192DB9C0D415A@SN6PR02MB4157.namprd02.prod.outlook.com>
On 9/15/25 10:54, Michael Kelley wrote:
> From: Mukesh Rathor <mrathor@linux.microsoft.com> Sent: Tuesday, September 9, 2025 5:10 PM
>>
>> Add data structures for hypervisor crash dump support to the hypervisor
>> host ABI header file. Details of their usages are in subsequent commits.
>>
>> Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
>> ---
>> include/hyperv/hvhdk_mini.h | 55 +++++++++++++++++++++++++++++++++++++
>> 1 file changed, 55 insertions(+)
>>
>> diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
>> index 858f6a3925b3..ad9a8048fb4e 100644
>> --- a/include/hyperv/hvhdk_mini.h
>> +++ b/include/hyperv/hvhdk_mini.h
>> @@ -116,6 +116,17 @@ enum hv_system_property {
>> /* Add more values when needed */
>> HV_SYSTEM_PROPERTY_SCHEDULER_TYPE = 15,
>> HV_DYNAMIC_PROCESSOR_FEATURE_PROPERTY = 21,
>> + HV_SYSTEM_PROPERTY_CRASHDUMPAREA = 47,
>> +};
>> +
>> +#define HV_PFN_RANGE_PGBITS 24 /* HV_SPA_PAGE_RANGE_ADDITIONAL_PAGES_BITS */
>> +union hv_pfn_range { /* HV_SPA_PAGE_RANGE */
>> + u64 as_uint64;
>> + struct {
>> + /* 39:0: base pfn. 63:40: additional pages */
>> + u64 base_pfn : 64 - HV_PFN_RANGE_PGBITS;
>> + u64 add_pfns : HV_PFN_RANGE_PGBITS;
>> + } __packed;
>> };
>>
>> enum hv_dynamic_processor_feature_property {
>> @@ -142,6 +153,8 @@ struct hv_output_get_system_property {
>> #if IS_ENABLED(CONFIG_X86)
>> u64 hv_processor_feature_value;
>> #endif
>> + union hv_pfn_range hv_cda_info; /* CrashdumpAreaAddress */
>> + u64 hv_tramp_pa; /* CrashdumpTrampolineAddress */
>> };
>> } __packed;
>>
>> @@ -234,6 +247,48 @@ union hv_gpa_page_access_state {
>> u8 as_uint8;
>> } __packed;
>>
>> +enum hv_crashdump_action {
>> + HV_CRASHDUMP_NONE = 0,
>> + HV_CRASHDUMP_SUSPEND_ALL_VPS,
>> + HV_CRASHDUMP_PREPARE_FOR_STATE_SAVE,
>> + HV_CRASHDUMP_STATE_SAVED,
>> + HV_CRASHDUMP_ENTRY,
>> +};
>
> Nit: Since these values are part of the ABI, it's probably better
> to assign explicit values to each enum member in order to
> ward off any mistaken reordering or additions in the middle
> of the list.
No, like I have mentioned in the past, we are mirroring hyp headers
with the eventual goal of just consuming from there directly.
Each change in ABI header is very carefully examined, we now have
a process for it.
>> +
>> +struct hv_partition_event_root_crashdump_input {
>> + u32 crashdump_action; /* enum hv_crashdump_action */
>> +} __packed;
>> +
>> +struct hv_input_disable_hyp_ex { /* HV_X64_INPUT_DISABLE_HYPERVISOR_EX */
>> + u64 rip;
>> + u64 arg;
>> +} __packed;
>> +
>> +struct hv_crashdump_area { /* HV_CRASHDUMP_AREA */
>> + u32 version;
>> + union {
>> + u32 flags_as_uint32;
>> + struct {
>> + u32 cda_valid : 1;
>> + u32 cda_unused : 31;
>> + } __packed;
>> + };
>> + /* more unused fields */
>> +} __packed;
>> +
>> +union hv_partition_event_input {
>> + struct hv_partition_event_root_crashdump_input crashdump_input;
>> +};
>> +
>> +enum hv_partition_event {
>> + HV_PARTITION_EVENT_ROOT_CRASHDUMP = 2,
>> +};
>> +
>> +struct hv_input_notify_partition_event {
>> + u32 event; /* enum hv_partition_event */
>> + union hv_partition_event_input input;
>> +} __packed;
>> +
>> struct hv_lp_startup_status {
>> u64 hv_status;
>> u64 substatus1;
>> --
>> 2.36.1.vfs.0.0
>>
>
^ permalink raw reply
* Re: [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts
From: Thomas Zimmermann @ 2025-09-16 8:30 UTC (permalink / raw)
To: Michael Kelley, louis.chauvet@bootlin.com, drawat.floss@gmail.com,
hamohammed.sa@gmail.com, melissa.srw@gmail.com, simona@ffwll.ch,
airlied@gmail.com, maarten.lankhorst@linux.intel.com,
ville.syrjala@linux.intel.com, lyude@redhat.com,
javierm@redhat.com
Cc: dri-devel@lists.freedesktop.org, linux-hyperv@vger.kernel.org
In-Reply-To: <BN7PR02MB4148E80C13605F6EAD2B0A03D40FA@BN7PR02MB4148.namprd02.prod.outlook.com>
Hi
Am 09.09.25 um 05:29 schrieb Michael Kelley:
> From: Michael Kelley Sent: Thursday, September 4, 2025 10:36 PM
>> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Thursday, September 4, 2025 7:56 AM
>>> Compositors often depend on vblanks to limit their display-update
>>> rate. Without, they see vblank events ASAP, which breaks the rate-
>>> limit feature. This creates high CPU overhead. It is especially a
>>> problem with virtual devices with fast framebuffer access.
>>>
>>> The series moves vkms' vblank timer to DRM and converts the hyperv
>>> DRM driver. An earlier version of this series contains examples of
>>> other updated drivers. In principle, any DRM driver without vblank
>>> hardware can use the timer.
>> I've tested this patch set in a Hyper-V guest against the linux-next20250829
>> kernel. All looks good. Results and perf are the same as reported here [4].
>> So far I haven't seen the "vblank timer overrun" error, which is consistent
>> with the changes you made since my earlier testing. I'll keep running this
>> test kernel for a while to see if anything anomalous occurs.
> As I continued to run with this patch set, I got a single occurrence of this
> WARN_ON. I can't associate it with any particular action as I didn't notice
> it until well after it occurred.
I've investigated. The stack trace comes from the kernel console's
display update. It runs concurrently to the vblank timeout. What likely
happens here is that the update code reads two values and in between,
the vblank timeout updates them. So the update then compares an old and
a new value; leading to an incorrect result with triggers the warning.
I'll include a fix in the series' next iteration. But I also think that
it's not critical. DRM's vblank helpers can usually deal with such problems.
Best regards
Thomas
>
> [213730.719364] ------------[ cut here ]------------
> [213730.719423] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] drm_WARN_ON(!ktime_compare(*vblank_time, vblank->time))
> [213730.719522] WARNING: drivers/gpu/drm/drm_vblank.c:2309 at drm_crtc_vblank_get_vblank_timeout+0x90/0xb0 [drm], CPU#4: kworker/4:0/7172
> [213730.719871] Modules linked in: nls_iso8859_1(E) dm_multipath(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) binfmt_misc(E) intel_rapl_msr(E) intel_rapl_common(E) rapl(E) hyperv_fb(E) cfbfillrect(E) cfbimgblt(E) fb_io_fops(E) serio_raw(E) cfbcopyarea(E) hv_balloon(E) joydev(E) mac_hid(E) sch_fq_codel(E) msr(E) ramoops(E) reed_solomon(E) efi_pstore(E) autofs4(E) btrfs(E) blake2b_generic(E) raid10(E) raid456(E) async_raid6_recov(E) async_memcpy(E) async_pq(E) async_xor(E) async_tx(E) xor(E) raid6_pq(E) raid1(E) raid0(E) hyperv_drm(E) drm_client_lib(E) drm_shmem_helper(E) syscopyarea(E) sysfillrect(E) sysimgblt(E) fb_sys_fops(E) drm_kms_helper(E) drm(E) drm_panel_orientation_quirks(E) fb(E) hid_generic(E) hid_hyperv(E) lcd(E) hid(E) hv_storvsc(E) ledtrig_backlight(E) hyperv_keyboard(E) hv_netvsc(E) hv_utils(E) scsi_transport_fc(E) ghash_clmulni_intel(E) hv_vmbus(E) aesni_intel(E)
> [213730.720514] CPU: 4 UID: 0 PID: 7172 Comm: kworker/4:0 Kdump: loaded Tainted: G E 6.17.0-rc3-next-20250829+ #3 PREEMPT(voluntary)
> [213730.723220] Tainted: [E]=UNSIGNED_MODULE
> [213730.724452] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v4.1 11/21/2024
> [213730.724993] Workqueue: events drm_fb_helper_damage_work [drm_kms_helper]
> [213730.725491] RIP: 0010:drm_crtc_vblank_get_vblank_timeout+0x90/0xb0 [drm]
> [213730.726082] Code: 8b 7f 08 4c 8b 67 50 4d 85 e4 74 33 e8 99 b6 7f c7 48 c7 c1 60 e8 93 c0 4c 89 e2 48 c7 c7 b5 25 94 c0 48 89 c6 e8 00 06 e3 c6 <0f> 0b eb c0 e8 07 f6 f1 c6 48 89 03 5b 41 5c 5d c3 cc cc cc cc 4c
> [213730.726506] RSP: 0018:ffffbba54e0efc00 EFLAGS: 00010282
> [213730.726692] RAX: 0000000000000000 RBX: ffffbba54e0efc78 RCX: 0000000000000027
> [213730.726899] RDX: ffff954f07d1cec8 RSI: 0000000000000001 RDI: ffff954f07d1cec0
> [213730.727094] RBP: ffffbba54e0efc10 R08: 0000000000000000 R09: ffffbba54e0efa70
> [213730.727280] R10: ffffbba54e0efa68 R11: ffffffff88d4c748 R12: ffff954e010a4cc0
> [213730.727456] R13: 0000000000000000 R14: ffff954e20070d80 R15: ffff954e251002c8
> [213730.727636] FS: 0000000000000000(0000) GS:ffff954f7e938000(0000) knlGS:0000000000000000
> [213730.727834] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [213730.728009] CR2: 00007fe11629d010 CR3: 000000011f843004 CR4: 0000000000b70ef0
> [213730.728186] Call Trace:
> [213730.728359] <TASK>
> [213730.728511] drm_crtc_vblank_helper_get_vblank_timestamp_from_timer+0x15/0x20 [drm_kms_helper]
> [213730.728674] drm_crtc_get_last_vbltimestamp+0x55/0x90 [drm]
> [213730.728864] drm_crtc_next_vblank_start+0x4e/0x90 [drm]
> [213730.729043] drm_atomic_helper_wait_for_fences+0x7c/0x1e0 [drm_kms_helper]
> [213730.729196] drm_atomic_helper_commit+0xa1/0x160 [drm_kms_helper]
> [213730.729335] drm_atomic_commit+0xb0/0xe0 [drm]
> [213730.729481] ? __pfx___drm_printfn_info+0x10/0x10 [drm]
> [213730.729643] drm_atomic_helper_dirtyfb+0x1aa/0x280 [drm_kms_helper]
> [213730.729800] drm_fbdev_shmem_helper_fb_dirty+0x4c/0xb0 [drm_shmem_helper]
> [213730.729939] drm_fb_helper_damage_work+0x8d/0x170 [drm_kms_helper]
> [213730.730071] process_one_work+0x19c/0x3f0
> [213730.730204] worker_thread+0x2c3/0x3d0
> [213730.730332] kthread+0x116/0x230
> [213730.730459] ? __pfx_worker_thread+0x10/0x10
> [213730.730580] ? _raw_spin_unlock_irq+0x12/0x40
> [213730.730744] ? __pfx_kthread+0x10/0x10
> [213730.730898] ret_from_fork+0xec/0x130
> [213730.731027] ? __pfx_kthread+0x10/0x10
> [213730.731152] ret_from_fork_asm+0x1a/0x30
> [213730.731277] </TASK>
> [213730.731396] ---[ end trace 0000000000000000 ]---
>
>> For Patches 1, 2, and 4 of the series on a Hyper-V guest,
>>
>> Tested-by: Michael Kelley <mhklinux@outlook.com>
>>
>> [4] https://lore.kernel.org/dri-devel/20250523161522.409504-1-
>> mhklinux@outlook.com/T/#m2e288dddaf7b3c025bbbf88da4b4c39e7aa950a7
>>
>>> The series has been motivated by a recent discussion about hypervdrm [1]
>>> and other long-standing bug reports. [2][3]
>>>
>>> v3:
>>> - fix deadlock (Ville, Lyude)
>>> v2:
>>> - rework interfaces
>>>
>>> [1] https://lore.kernel.org/dri-devel/20250523161522.409504-1-
>> mhklinux@outlook.com/T/#ma2ebb52b60bfb0325879349377738fadcd7cb7ef
>>> [2] https://bugzilla.suse.com/show_bug.cgi?id=1189174
>>> [3] https://invent.kde.org/plasma/kwin/-/merge_requests/1229#note_284606
>>>
>>> Thomas Zimmermann (4):
>>> drm/vblank: Add vblank timer
>>> drm/vblank: Add CRTC helpers for simple use cases
>>> drm/vkms: Convert to DRM's vblank timer
>>> drm/hypervdrm: Use vblank timer
>>>
>>> Documentation/gpu/drm-kms-helpers.rst | 12 ++
>>> drivers/gpu/drm/Makefile | 3 +-
>>> drivers/gpu/drm/drm_vblank.c | 161 +++++++++++++++++-
>>> drivers/gpu/drm/drm_vblank_helper.c | 176 ++++++++++++++++++++
>>> drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 11 ++
>>> drivers/gpu/drm/vkms/vkms_crtc.c | 83 +--------
>>> drivers/gpu/drm/vkms/vkms_drv.h | 2 -
>>> include/drm/drm_modeset_helper_vtables.h | 12 ++
>>> include/drm/drm_vblank.h | 32 ++++
>>> include/drm/drm_vblank_helper.h | 56 +++++++
>>> 10 files changed, 467 insertions(+), 81 deletions(-)
>>> create mode 100644 drivers/gpu/drm/drm_vblank_helper.c
>>> create mode 100644 include/drm/drm_vblank_helper.h
>>>
>>> --
>>> 2.50.1
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
^ permalink raw reply
* [PATCH v4 0/4] drm: Add vblank timers for devices without interrupts
From: Thomas Zimmermann @ 2025-09-16 8:36 UTC (permalink / raw)
To: louis.chauvet, drawat.floss, hamohammed.sa, melissa.srw, mhklinux,
ptsm, simona, airlied, maarten.lankhorst, ville.syrjala, lyude,
javierm
Cc: dri-devel, linux-hyperv, Thomas Zimmermann
Compositors often depend on vblanks to limit their display-update
rate. Without, they see vblank events ASAP, which breaks the rate-
limit feature. This creates high CPU overhead. It is especially a
problem with virtual devices with fast framebuffer access.
The series moves vkms' vblank timer to DRM and converts the hyperv
DRM driver. An earlier version of this series contains examples of
other updated drivers. In principle, any DRM driver without vblank
hardware can use the timer.
The series has been motivated by a recent discussion about hypervdrm [1]
and other long-standing bug reports. [2][3]
v4:
- fix a race condition in drm_crtc_vblank_get_vblank_timeout() (Michael)
v3:
- fix deadlock (Ville, Lyude)
v2:
- rework interfaces
[1] https://lore.kernel.org/dri-devel/20250523161522.409504-1-mhklinux@outlook.com/T/#ma2ebb52b60bfb0325879349377738fadcd7cb7ef
[2] https://bugzilla.suse.com/show_bug.cgi?id=1189174
[3] https://invent.kde.org/plasma/kwin/-/merge_requests/1229#note_284606
Thomas Zimmermann (4):
drm/vblank: Add vblank timer
drm/vblank: Add CRTC helpers for simple use cases
drm/vkms: Convert to DRM's vblank timer
drm/hypervdrm: Use vblank timer
Documentation/gpu/drm-kms-helpers.rst | 12 ++
drivers/gpu/drm/Makefile | 3 +-
drivers/gpu/drm/drm_vblank.c | 172 ++++++++++++++++++-
drivers/gpu/drm/drm_vblank_helper.c | 176 ++++++++++++++++++++
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 11 ++
drivers/gpu/drm/vkms/vkms_crtc.c | 83 +--------
drivers/gpu/drm/vkms/vkms_drv.h | 2 -
include/drm/drm_modeset_helper_vtables.h | 12 ++
include/drm/drm_vblank.h | 32 ++++
include/drm/drm_vblank_helper.h | 56 +++++++
10 files changed, 478 insertions(+), 81 deletions(-)
create mode 100644 drivers/gpu/drm/drm_vblank_helper.c
create mode 100644 include/drm/drm_vblank_helper.h
--
2.51.0
^ permalink raw reply
* [PATCH v4 2/4] drm/vblank: Add CRTC helpers for simple use cases
From: Thomas Zimmermann @ 2025-09-16 8:36 UTC (permalink / raw)
To: louis.chauvet, drawat.floss, hamohammed.sa, melissa.srw, mhklinux,
ptsm, simona, airlied, maarten.lankhorst, ville.syrjala, lyude,
javierm
Cc: dri-devel, linux-hyperv, Thomas Zimmermann
In-Reply-To: <20250916083816.30275-1-tzimmermann@suse.de>
Implement atomic_flush, atomic_enable and atomic_disable of struct
drm_crtc_helper_funcs for vblank handling. Driver with no further
requirements can use these functions instead of adding their own.
Also simplifies the use of vblank timers.
The code has been adopted from vkms, which added the funtionality
in commit 3a0709928b17 ("drm/vkms: Add vblank events simulated by
hrtimers").
v3:
- mention vkms (Javier)
v2:
- fix docs
Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/gpu/drm/drm_vblank_helper.c | 80 +++++++++++++++++++++++++++++
include/drm/drm_vblank_helper.h | 23 +++++++++
2 files changed, 103 insertions(+)
diff --git a/drivers/gpu/drm/drm_vblank_helper.c b/drivers/gpu/drm/drm_vblank_helper.c
index f94d1e706191..a04a6ba1b0ca 100644
--- a/drivers/gpu/drm/drm_vblank_helper.c
+++ b/drivers/gpu/drm/drm_vblank_helper.c
@@ -1,5 +1,6 @@
// SPDX-License-Identifier: MIT
+#include <drm/drm_atomic.h>
#include <drm/drm_crtc.h>
#include <drm/drm_managed.h>
#include <drm/drm_modeset_helper_vtables.h>
@@ -17,6 +18,12 @@
* Drivers enable support for vblank timers by setting the vblank callbacks
* in struct &drm_crtc_funcs to the helpers provided by this library. The
* initializer macro DRM_CRTC_VBLANK_TIMER_FUNCS does this conveniently.
+ * The driver further has to send the VBLANK event from its atomic_flush
+ * callback and control vblank from the CRTC's atomic_enable and atomic_disable
+ * callbacks. The callbacks are located in struct &drm_crtc_helper_funcs.
+ * The vblank helper library provides implementations of these callbacks
+ * for drivers without further requirements. The initializer macro
+ * DRM_CRTC_HELPER_VBLANK_FUNCS sets them coveniently.
*
* Once the driver enables vblank support with drm_vblank_init(), each
* CRTC's vblank timer fires according to the programmed display mode. By
@@ -25,6 +32,79 @@
* struct &drm_crtc_helper_funcs.handle_vblank_timeout.
*/
+/*
+ * VBLANK helpers
+ */
+
+/**
+ * drm_crtc_vblank_atomic_flush -
+ * Implements struct &drm_crtc_helper_funcs.atomic_flush
+ * @crtc: The CRTC
+ * @state: The atomic state to apply
+ *
+ * The helper drm_crtc_vblank_atomic_flush() implements atomic_flush of
+ * struct drm_crtc_helper_funcs for CRTCs that only need to send out a
+ * VBLANK event.
+ *
+ * See also struct &drm_crtc_helper_funcs.atomic_flush.
+ */
+void drm_crtc_vblank_atomic_flush(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ struct drm_device *dev = crtc->dev;
+ struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+ struct drm_pending_vblank_event *event;
+
+ spin_lock_irq(&dev->event_lock);
+
+ event = crtc_state->event;
+ crtc_state->event = NULL;
+
+ if (event) {
+ if (drm_crtc_vblank_get(crtc) == 0)
+ drm_crtc_arm_vblank_event(crtc, event);
+ else
+ drm_crtc_send_vblank_event(crtc, event);
+ }
+
+ spin_unlock_irq(&dev->event_lock);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_atomic_flush);
+
+/**
+ * drm_crtc_vblank_atomic_enable - Implements struct &drm_crtc_helper_funcs.atomic_enable
+ * @crtc: The CRTC
+ * @state: The atomic state
+ *
+ * The helper drm_crtc_vblank_atomic_enable() implements atomic_enable
+ * of struct drm_crtc_helper_funcs for CRTCs the only need to enable VBLANKs.
+ *
+ * See also struct &drm_crtc_helper_funcs.atomic_enable.
+ */
+void drm_crtc_vblank_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ drm_crtc_vblank_on(crtc);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_atomic_enable);
+
+/**
+ * drm_crtc_vblank_atomic_disable - Implements struct &drm_crtc_helper_funcs.atomic_disable
+ * @crtc: The CRTC
+ * @state: The atomic state
+ *
+ * The helper drm_crtc_vblank_atomic_disable() implements atomic_disable
+ * of struct drm_crtc_helper_funcs for CRTCs the only need to disable VBLANKs.
+ *
+ * See also struct &drm_crtc_funcs.atomic_disable.
+ */
+void drm_crtc_vblank_atomic_disable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state)
+{
+ drm_crtc_vblank_off(crtc);
+}
+EXPORT_SYMBOL(drm_crtc_vblank_atomic_disable);
+
/*
* VBLANK timer
*/
diff --git a/include/drm/drm_vblank_helper.h b/include/drm/drm_vblank_helper.h
index 74a971d0cfba..fcd8a9b35846 100644
--- a/include/drm/drm_vblank_helper.h
+++ b/include/drm/drm_vblank_helper.h
@@ -6,8 +6,31 @@
#include <linux/hrtimer_types.h>
#include <linux/types.h>
+struct drm_atomic_state;
struct drm_crtc;
+/*
+ * VBLANK helpers
+ */
+
+void drm_crtc_vblank_atomic_flush(struct drm_crtc *crtc,
+ struct drm_atomic_state *state);
+void drm_crtc_vblank_atomic_enable(struct drm_crtc *crtc,
+ struct drm_atomic_state *state);
+void drm_crtc_vblank_atomic_disable(struct drm_crtc *crtc,
+ struct drm_atomic_state *crtc_state);
+
+/**
+ * DRM_CRTC_HELPER_VBLANK_FUNCS - Default implementation for VBLANK helpers
+ *
+ * This macro initializes struct &drm_crtc_helper_funcs to default helpers
+ * for VBLANK handling.
+ */
+#define DRM_CRTC_HELPER_VBLANK_FUNCS \
+ .atomic_flush = drm_crtc_vblank_atomic_flush, \
+ .atomic_enable = drm_crtc_vblank_atomic_enable, \
+ .atomic_disable = drm_crtc_vblank_atomic_disable
+
/*
* VBLANK timer
*/
--
2.51.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox