* Re: [PATCH v1 2/4] drm/hyperv: Explicitly set subvendor and subdevice for pci match array
From: Uwe Kleine-König (The Capable Hub) @ 2026-07-02 8:52 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: Dexuan Cui, Long Li, Saurabh Sengar, Maarten Lankhorst,
Maxime Ripard, David Airlie, Simona Vetter, linux-hyperv,
dri-devel, linux-kernel
In-Reply-To: <7a747d47-d275-48ad-a4ea-1e4897df1d28@suse.de>
[-- Attachment #1: Type: text/plain, Size: 1895 bytes --]
Hallo Thomas,
On Thu, Jul 02, 2026 at 08:43:32AM +0200, Thomas Zimmermann wrote:
> Am 01.07.26 um 19:05 schrieb Uwe Kleine-König (The Capable Hub):
> > .subvendor and .subdevice were set to 0 implicitly, so only devices with
> > these two values set to 0 in hardware can probe automatically. Make this
> > requirement explicit.
> >
> > While touching this array item, also make use of the pci macro designed
> > for that case.
> >
> > Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> > ---
> > drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> > index 2e75fb793495..e766d87b7a9d 100644
> > --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> > +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> > @@ -51,8 +51,8 @@ static void hv_drm_pci_remove(struct pci_dev *pdev)
> > static const struct pci_device_id hv_drm_pci_tbl[] = {
> > {
> > - .vendor = PCI_VENDOR_ID_MICROSOFT,
> > - .device = PCI_DEVICE_ID_HYPERV_VIDEO,
> > + PCI_VDEVICE_SUB(MICROSOFT, PCI_DEVICE_ID_HYPERV_VIDEO,
> > + 0, 0),
>
> IDK, but it looks like an oversight to me. Setting the sub-fields to ANY
> seems like the better fix.
That was my initial reflex, too. However while writing the commit log
for that change I noticed that since commit d750785f305e ("Staging: hv:
fix hv_utils module to properly autoload") from 2010 (applied to
v2.6.35-rc4) the driver never worked for hardware with .subvendor != 0
or .subdevice != 0. I cannot believe that something like that is
discovered 16 years later by chance during a rework by someone who
didn't try to run that hardware. And if I understand correctly, this is
emulated hardware and so I guess used quite a lot.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v1 4/4] drm/hyperv: Move MODULE_DEVICE_TABLE to the device_id arrays
From: Thomas Zimmermann @ 2026-07-02 6:45 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub), Dexuan Cui, Long Li,
Saurabh Sengar, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter
Cc: linux-hyperv, dri-devel, linux-kernel
In-Reply-To: <7f9d4a239c76b6bb384048ea5591a21ed87d9b0e.1782925276.git.u.kleine-koenig@baylibre.com>
Am 01.07.26 um 19:05 schrieb Uwe Kleine-König (The Capable Hub):
> It matches the usual coding style to have the MODULE_DEVICE_TABLE macro
> directly after the respective arrays.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index e3f41336a831..6a28048f687b 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -52,6 +52,7 @@ static const struct pci_device_id hv_drm_pci_tbl[] = {
> },
> { /* end of list */ }
> };
> +MODULE_DEVICE_TABLE(pci, hv_drm_pci_tbl);
>
> /*
> * PCI stub to support gen1 VM.
> @@ -219,6 +220,7 @@ static const struct hv_vmbus_device_id hv_drm_vmbus_tbl[] = {
> {HV_SYNTHVID_GUID},
> {}
> };
> +MODULE_DEVICE_TABLE(vmbus, hv_drm_vmbus_tbl);
>
> static struct hv_driver hv_drm_hv_driver = {
> .name = KBUILD_MODNAME,
> @@ -260,8 +262,6 @@ static void __exit hv_drm_exit(void)
> module_init(hv_drm_init);
> module_exit(hv_drm_exit);
>
> -MODULE_DEVICE_TABLE(pci, hv_drm_pci_tbl);
> -MODULE_DEVICE_TABLE(vmbus, hv_drm_vmbus_tbl);
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Deepak Rawat <drawat.floss@gmail.com>");
> MODULE_DESCRIPTION("DRM driver for Hyper-V synthetic video device");
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* Re: [PATCH v1 3/4] drm/hyperv: Drop useless empty remove callback
From: Thomas Zimmermann @ 2026-07-02 6:45 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub), Dexuan Cui, Long Li,
Saurabh Sengar, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter
Cc: linux-hyperv, dri-devel, linux-kernel
In-Reply-To: <8a85b5f4a5ed8ec35b5a213423d4be40e34f9cb9.1782925276.git.u.kleine-koenig@baylibre.com>
Hi
Am 01.07.26 um 19:05 schrieb Uwe Kleine-König (The Capable Hub):
> Having an empty remove callback is equivalent to no remove callback.
> (The only minor difference is that with an empty remove callback
> pm_runtime_get_sync() and pm_runtime_put_noidle() are called.)
>
> Drop this useless function.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> ---
> drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index e766d87b7a9d..e3f41336a831 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -45,10 +45,6 @@ static int hv_drm_pci_probe(struct pci_dev *pdev,
> return 0;
> }
>
> -static void hv_drm_pci_remove(struct pci_dev *pdev)
> -{
It would be better to call drm_dev_unplug() from here. With a bit more
work, the driver can have hot-unplug functionality.
Best regards
Thomas
> -}
> -
> static const struct pci_device_id hv_drm_pci_tbl[] = {
> {
> PCI_VDEVICE_SUB(MICROSOFT, PCI_DEVICE_ID_HYPERV_VIDEO,
> @@ -64,7 +60,6 @@ static struct pci_driver hv_drm_pci_driver = {
> .name = KBUILD_MODNAME,
> .id_table = hv_drm_pci_tbl,
> .probe = hv_drm_pci_probe,
> - .remove = hv_drm_pci_remove,
> };
>
> static int hv_drm_setup_vram(struct hv_drm_device *hv,
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* Re: [PATCH v1 2/4] drm/hyperv: Explicitly set subvendor and subdevice for pci match array
From: Thomas Zimmermann @ 2026-07-02 6:43 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub), Dexuan Cui, Long Li,
Saurabh Sengar, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter
Cc: linux-hyperv, dri-devel, linux-kernel
In-Reply-To: <019450ffb519d02821364afca32b9f48bcd8d2b6.1782925276.git.u.kleine-koenig@baylibre.com>
Hi
Am 01.07.26 um 19:05 schrieb Uwe Kleine-König (The Capable Hub):
> .subvendor and .subdevice were set to 0 implicitly, so only devices with
> these two values set to 0 in hardware can probe automatically. Make this
> requirement explicit.
>
> While touching this array item, also make use of the pci macro designed
> for that case.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
> ---
> drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index 2e75fb793495..e766d87b7a9d 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -51,8 +51,8 @@ static void hv_drm_pci_remove(struct pci_dev *pdev)
>
> static const struct pci_device_id hv_drm_pci_tbl[] = {
> {
> - .vendor = PCI_VENDOR_ID_MICROSOFT,
> - .device = PCI_DEVICE_ID_HYPERV_VIDEO,
> + PCI_VDEVICE_SUB(MICROSOFT, PCI_DEVICE_ID_HYPERV_VIDEO,
> + 0, 0),
IDK, but it looks like an oversight to me. Setting the sub-fields to
ANY seems like the better fix.
Best regards
Thomas
> },
> { /* end of list */ }
> };
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* Re: [PATCH v1 1/4] drm/hyperv: Unregister pci driver in error path before module unload
From: Thomas Zimmermann @ 2026-07-02 6:42 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub), Dexuan Cui, Long Li,
Saurabh Sengar, Maarten Lankhorst, Maxime Ripard, David Airlie,
Simona Vetter, Deepak Rawat
Cc: linux-hyperv, dri-devel, linux-kernel
In-Reply-To: <4b7dbf00ce4ff664b7d5dd74b2f39d8d87c1ade9.1782925276.git.u.kleine-koenig@baylibre.com>
Am 01.07.26 um 19:05 schrieb Uwe Kleine-König (The Capable Hub):
> The pci driver must not kept registered if the module is unloaded after
> vmbus_driver_register() fails. So check the return value of
> vmbus_driver_register() and unregister the pci driver on failure.
>
> Fixes: 76c56a5affeb ("drm/hyperv: Add DRM driver for hyperv synthetic video device")
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
> drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index 20f35c48c0b8..2e75fb793495 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -249,7 +249,11 @@ static int __init hv_drm_init(void)
> if (ret != 0)
> return ret;
>
> - return vmbus_driver_register(&hv_drm_hv_driver);
> + ret = vmbus_driver_register(&hv_drm_hv_driver);
> + if (ret)
> + pci_unregister_driver(&hv_drm_pci_driver);
> +
> + return ret;
> }
>
> static void __exit hv_drm_exit(void)
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* RE: [EXTERNAL] Re: [PATCH net v2 1/2] net: mana: Sync page pool RX frags for CPU
From: Dexuan Cui @ 2026-07-02 4:20 UTC (permalink / raw)
To: Simon Horman
Cc: KY Srinivasan, Haiyang Zhang, wei.liu@kernel.org, Long Li,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, Konstantin Taranov,
ernis@linux.microsoft.com, dipayanroy@linux.microsoft.com,
kees@kernel.org, jacob.e.keller@intel.com,
ssengar@linux.microsoft.com, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-rdma@vger.kernel.org, stable@vger.kernel.org
In-Reply-To: <20260626145048.GB1310988@horms.kernel.org>
> From: Simon Horman <horms@kernel.org>
> Sent: Friday, June 26, 2026 7:51 AM
> ...
> Hi,
>
> I'm sorry to be bothersome but I think that the order of the two patches
> that comprise this series should be reversed. Or if that is not possible,
> go back to a single patch.
Hi Simon,
Thanks for suggesting swapping the order of the 2 patches in v2!
Totally makes sense.
Please review v3 I just posted:
https://lore.kernel.org/netdev/20260702041237.617719-1-decui@microsoft.com/
Thanks,
Dexuan
^ permalink raw reply
* [PATCH net v3 1/2] net: mana: Validate the packet length reported by the NIC
From: Dexuan Cui @ 2026-07-02 4:12 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, pabeni, kotaranov, horms, ernis, dipayanroy, kees,
jacob.e.keller, ssengar, linux-hyperv, netdev, linux-kernel,
linux-rdma
Cc: stable
In-Reply-To: <20260702041237.617719-1-decui@microsoft.com>
Validate the packet length reported in the RX CQE before passing it
to skb processing. The CQE is supplied by the NIC device and should
not be blindly trusted.
Cc: stable@vger.kernel.org
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
Changes since v1:
v1 is split into two patches in the v2.
Add Haiyang's Reviewed-by.
Changes since v2:
Swapped the order of the 2 patches in v2.
No extra change.
drivers/net/ethernet/microsoft/mana/mana_en.c | 23 +++++++++++++++----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index c9b1df1ed109..edc504b2447a 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2170,12 +2170,25 @@ static void mana_process_rx_cqe(struct mana_rxq *rxq, struct mana_cq *cq,
rxbuf_oob = &rxq->rx_oobs[curr];
WARN_ON_ONCE(rxbuf_oob->wqe_inf.wqe_size_in_bu != 1);
- mana_refill_rx_oob(dev, rxq, rxbuf_oob, &old_buf, &old_fp);
+ if (unlikely(pktlen > rxq->datasize)) {
+ /* Increase it even if mana_rx_skb() isn't called. */
+ rxq->rx_cq.work_done++;
- /* Unsuccessful refill will have old_buf == NULL.
- * In this case, mana_rx_skb() will drop the packet.
- */
- mana_rx_skb(old_buf, old_fp, oob, rxq, i);
+ ++ndev->stats.rx_dropped;
+ netdev_warn_once(ndev,
+ "Dropped oversized RX packet: len=%u, datasize=%u\n",
+ pktlen, rxq->datasize);
+
+ /* Reuse the RX buffer since rxbuf_oob is unchanged. */
+ } else {
+
+ mana_refill_rx_oob(dev, rxq, rxbuf_oob, &old_buf, &old_fp);
+
+ /* Unsuccessful refill will have old_buf == NULL.
+ * In this case, mana_rx_skb() will drop the packet.
+ */
+ mana_rx_skb(old_buf, old_fp, oob, rxq, i);
+ }
mana_move_wq_tail(rxq->gdma_rq,
rxbuf_oob->wqe_inf.wqe_size_in_bu);
--
2.34.1
^ permalink raw reply related
* [PATCH net v3 2/2] net: mana: Sync page pool RX frags for CPU
From: Dexuan Cui @ 2026-07-02 4:12 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, pabeni, kotaranov, horms, ernis, dipayanroy, kees,
jacob.e.keller, ssengar, linux-hyperv, netdev, linux-kernel,
linux-rdma
Cc: stable
In-Reply-To: <20260702041237.617719-1-decui@microsoft.com>
MANA allocates RX buffers from page pool fragments when frag_count is
greater than 1. In that case the buffers remain DMA mapped by page pool
and the RX completion path does not call dma_unmap_single(). As a result,
the implicit sync-for-CPU normally performed by dma_unmap_single() is
missing before the packet data is passed to the networking stack.
This breaks RX on configurations which require explicit DMA syncing, for
example when booted with swiotlb=force.
Fix this by recording the page pool page and DMA sync offset when the RX
buffer is allocated, and syncing the received packet range for CPU access
before handing the RX buffer to the stack.
Fixes: 730ff06d3f5c ("net: mana: Use page pool fragments for RX buffers instead of full pages to improve memory efficiency.")
Cc: stable@vger.kernel.org
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
Changes since v1:
v1 is split into two patches in the v2.
Add Haiyang's Reviewed-by.
Changes since v2:
Swapped the order of the 2 patches in v2.
No extra change.
drivers/net/ethernet/microsoft/mana/mana_en.c | 40 +++++++++++++++----
include/net/mana/mana.h | 8 ++++
2 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index edc504b2447a..0b44c51ae6ec 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2044,12 +2044,16 @@ static void mana_rx_skb(void *buf_va, bool from_pool,
}
static void *mana_get_rxfrag(struct mana_rxq *rxq, struct device *dev,
- dma_addr_t *da, bool *from_pool)
+ dma_addr_t *da, bool *from_pool,
+ struct page **pp_page, u32 *dma_sync_offset)
{
struct page *page;
u32 offset;
void *va;
+
*from_pool = false;
+ *pp_page = NULL;
+ *dma_sync_offset = 0;
/* Don't use fragments for jumbo frames or XDP where it's 1 fragment
* per page.
@@ -2087,31 +2091,47 @@ static void *mana_get_rxfrag(struct mana_rxq *rxq, struct device *dev,
va = page_to_virt(page) + offset;
*da = page_pool_get_dma_addr(page) + offset + rxq->headroom;
*from_pool = true;
+ *pp_page = page;
+ *dma_sync_offset = offset + rxq->headroom;
return va;
}
/* Allocate frag for rx buffer, and save the old buf */
static void mana_refill_rx_oob(struct device *dev, struct mana_rxq *rxq,
- struct mana_recv_buf_oob *rxoob, void **old_buf,
- bool *old_fp)
+ struct mana_recv_buf_oob *rxoob, u32 pktlen,
+ void **old_buf, bool *old_fp)
{
+ struct page *pp_page;
+ u32 dma_sync_offset;
bool from_pool;
dma_addr_t da;
void *va;
- va = mana_get_rxfrag(rxq, dev, &da, &from_pool);
+ va = mana_get_rxfrag(rxq, dev, &da, &from_pool, &pp_page,
+ &dma_sync_offset);
if (!va)
return;
- if (!rxoob->from_pool || rxq->frag_count == 1)
+ if (!rxoob->from_pool || rxq->frag_count == 1) {
dma_unmap_single(dev, rxoob->sgl[0].address, rxq->datasize,
DMA_FROM_DEVICE);
+ } else {
+ /* The page pool maps the whole page and only syncs for device
+ * automatically (PP_FLAG_DMA_SYNC_DEV). Sync the received bytes
+ * for the CPU before they are read: this is required if DMA
+ * is incoherent or bounce buffers are used.
+ */
+ page_pool_dma_sync_for_cpu(rxq->page_pool, rxoob->pp_page,
+ rxoob->dma_sync_offset, pktlen);
+ }
*old_buf = rxoob->buf_va;
*old_fp = rxoob->from_pool;
rxoob->buf_va = va;
rxoob->sgl[0].address = da;
rxoob->from_pool = from_pool;
+ rxoob->pp_page = pp_page;
+ rxoob->dma_sync_offset = dma_sync_offset;
}
static void mana_process_rx_cqe(struct mana_rxq *rxq, struct mana_cq *cq,
@@ -2182,7 +2202,8 @@ static void mana_process_rx_cqe(struct mana_rxq *rxq, struct mana_cq *cq,
/* Reuse the RX buffer since rxbuf_oob is unchanged. */
} else {
- mana_refill_rx_oob(dev, rxq, rxbuf_oob, &old_buf, &old_fp);
+ mana_refill_rx_oob(dev, rxq, rxbuf_oob, pktlen,
+ &old_buf, &old_fp);
/* Unsuccessful refill will have old_buf == NULL.
* In this case, mana_rx_skb() will drop the packet.
@@ -2579,6 +2600,8 @@ static int mana_fill_rx_oob(struct mana_recv_buf_oob *rx_oob, u32 mem_key,
struct mana_rxq *rxq, struct device *dev)
{
struct mana_port_context *mpc = netdev_priv(rxq->ndev);
+ struct page *pp_page = NULL;
+ u32 dma_sync_offset = 0;
bool from_pool = false;
dma_addr_t da;
void *va;
@@ -2586,13 +2609,16 @@ static int mana_fill_rx_oob(struct mana_recv_buf_oob *rx_oob, u32 mem_key,
if (mpc->rxbufs_pre)
va = mana_get_rxbuf_pre(rxq, &da);
else
- va = mana_get_rxfrag(rxq, dev, &da, &from_pool);
+ va = mana_get_rxfrag(rxq, dev, &da, &from_pool, &pp_page,
+ &dma_sync_offset);
if (!va)
return -ENOMEM;
rx_oob->buf_va = va;
rx_oob->from_pool = from_pool;
+ rx_oob->pp_page = pp_page;
+ rx_oob->dma_sync_offset = dma_sync_offset;
rx_oob->sgl[0].address = da;
rx_oob->sgl[0].size = rxq->datasize;
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 8f721cd4e4a7..4111b93169d2 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -305,6 +305,14 @@ struct mana_recv_buf_oob {
void *buf_va;
bool from_pool; /* allocated from a page pool */
+ /* head page of the page_pool fragment; valid only when
+ * from_pool && frag_count > 1.
+ */
+ struct page *pp_page;
+ /* Fragment offset plus rxq->headroom, passed to
+ * page_pool_dma_sync_for_cpu().
+ */
+ u32 dma_sync_offset;
/* SGL of the buffer going to be sent as part of the work request. */
u32 num_sge;
--
2.34.1
^ permalink raw reply related
* [PATCH net v3 0/2] Fix MANA RX with bounce buffering
From: Dexuan Cui @ 2026-07-02 4:12 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, pabeni, kotaranov, horms, ernis, dipayanroy, kees,
jacob.e.keller, ssengar, linux-hyperv, netdev, linux-kernel,
linux-rdma
Cc: stable
With swiotlb=force, the MANA NIC fails to work properly due to commit
730ff06d3f5c ("net: mana: Use page pool fragments for RX buffers instead
of full pages to improve memory efficiency.").
This happens because, with the standard MTU=1500, the aforementioned
commit uses page pool frags with PP_FLAG_DMA_MAP, but fails to call
page_pool_dma_sync_for_cpu() to sync the received packet for CPU acces
before handing the RX buffer to the stack.
Here patch #2 adds the required page_pool_dma_sync_for_cpu().
Patch #1 validates the packet length reported by the NIC. With patch #2,
page_pool_dma_sync_for_cpu() uses the packet length, so we don't want
to blindly trust the packet length, just in case.
There is no change between v2 and v3.
v3 just swaps the order of the 2 patches in v2, as suggested by Simon [3].
Please review.
Thanks,
Dexuan
References:
[1] v1: https://lore.kernel.org/netdev/20260618035029.249361-1-decui@microsoft.com/
[2] v2: https://lore.kernel.org/netdev/20260624222605.1794719-1-decui@microsoft.com/
[3] https://lore.kernel.org/netdev/20260626145048.GB1310988@horms.kernel.org/
Dexuan Cui (2):
net: mana: Validate the packet length reported by the NIC
net: mana: Sync page pool RX frags for CPU
drivers/net/ethernet/microsoft/mana/mana_en.c | 61 +++++++++++++++----
include/net/mana/mana.h | 8 +++
2 files changed, 58 insertions(+), 11 deletions(-)
--
2.34.1
^ permalink raw reply
* Re: [PATCH v5 29/51] x86/kvm: Don't disable kvmclock on BSP in syscore_suspend()
From: Sean Christopherson @ 2026-07-01 20:43 UTC (permalink / raw)
To: sashiko-reviews; +Cc: kvm, linux-hyperv
In-Reply-To: <20260701200318.B83061F000E9@smtp.kernel.org>
On Wed, Jul 01, 2026, sashiko-bot@kernel.org wrote:
> [Severity: High]
> Does this change inadvertently leave the BSP's kvmclock enabled when
> powering off the machine for ACPI S4 hibernation?
>
> Looking at the final S4 power-off path in hibernation_platform_enter() in
> kernel/power/hibernate.c, it calls syscore_suspend() but does not call
> save_processor_state(). Because this patch removes the unconditional BSP
> kvmclock_disable() from syscore_suspend() and relies on
> kvm_save_sched_clock_state() (which only fires during image creation),
> kvmclock appears to remain fully enabled during the S4 power-off.
No?
hibernate()
|
-> hibernation_snapshot()
| |
| -> create_image()
| |
| -> save_processor_state()
|
-> power_down()
|
-> hibernation_platform_enter()
I'm not familiar with the /dev/snapshot interface, but I gotta imagine userspace
needs to do SNAPSHOT_CREATE_IMAGE before SNAPSHOT_POWER_OFF if it actually wants
to restore state after S4.
> [Severity: Critical]
> This isn't a bug introduced by this patch, but is there a use-after-free
> risk with the stack-allocated waitqueue head in the async page fault
> handling in arch/x86/kernel/kvm.c?
And I thought I had a problem with scope creep during review...
> When a guest task waits for an async page fault in
> kvm_async_pf_task_wait_schedule(), it waits on a stack-allocated struct
> kvm_task_sleep_node. Concurrently, when a waker CPU executes
> apf_task_wake_one(), it unhashes the node via hlist_del_init().
>
> The waiter checks hlist_unhashed() locklessly. If it observes the unhashed
> state before the waker is finished, the waiter can break its loop, return,
> and destroy the stack frame. The waker might then dereference the
> deallocated waitqueue head to call swake_up_one(), potentially corrupting
> stack memory.
>
> Can this sequence occur during concurrent page fault completions?
No idea, definitely something to sort out at a different time.
^ permalink raw reply
* Re: [PATCH v5 22/51] x86/kvm: Mark TSC as reliable when it's constant and nonstop
From: Sean Christopherson @ 2026-07-01 20:13 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-hyperv, kvm
In-Reply-To: <20260701200321.256EF1F000E9@smtp.kernel.org>
On Wed, Jul 01, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> > index 909d3e5e5bcd5..1cef54e1e7d9e 100644
> > --- a/arch/x86/kernel/kvm.c
> > +++ b/arch/x86/kernel/kvm.c
> [ ... ]
> > @@ -1040,7 +1041,16 @@ static void __init kvm_init_platform(void)
> > }
> > }
> >
> > - kvmclock_init();
> > + /*
> > + * If the TSC counts at a constant frequency across P/T states and in
> > + * deep C-states, treat the TSC reliable, as guaranteed by KVM.
> > + */
> > + tsc_is_reliable = boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
> > + boot_cpu_has(X86_FEATURE_NONSTOP_TSC);
> > + if (tsc_is_reliable)
> > + setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
>
> [Severity: High]
> Does setting X86_FEATURE_TSC_RELIABLE here bypass SMP cross-CPU TSC
> synchronization checks?
No, KVM_FEATURE_CLOCKSOURCE_STABLE_BIT is for kvmclock, not for TSC. The
CONSTANT+NONSTOP combination is KVM's way of guaranteeing cross-CPU synchronization.
> Forcing X86_FEATURE_TSC_RELIABLE based only on CONSTANT_TSC and NONSTOP_TSC,
> without verifying if the hypervisor actually guarantees cross-CPU
> synchronization (e.g., via KVM_FEATURE_CLOCKSOURCE_STABLE_BIT), sets
> tsc_clocksource_reliable = 1.
>
> This bypasses the synchronization check in check_tsc_sync_source():
>
> arch/x86/kernel/tsc_sync.c:check_tsc_sync_source() {
> ...
> if (tsc_store_and_check_tsc_adjust(false) || tsc_clocksource_reliable)
> return;
> ...
> }
>
> Could this cause time to go backwards for guests running on hosts with
> unsynchronized TSCs when threads migrate between vCPUs?
^ permalink raw reply
* Re: [PATCH v5 12/51] x86/acrn: Register TSC/CPU frequency callbacks iff frequency is actually in CPUID
From: Sean Christopherson @ 2026-07-01 20:10 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-hyperv, kvm
In-Reply-To: <20260701195245.467DB1F00A3A@smtp.kernel.org>
On Wed, Jul 01, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
> > index ad8f2da8003b3..dc71a6fdd4613 100644
> > --- a/arch/x86/kernel/cpu/acrn.c
> > +++ b/arch/x86/kernel/cpu/acrn.c
> [ ... ]
> > @@ -26,13 +28,19 @@ static u32 __init acrn_detect(void)
> >
> > static unsigned int __init acrn_get_tsc_khz(void)
> > {
> > - return cpuid_eax(ACRN_CPUID_TIMING_INFO);
> > + return acrn_tsc_khz_cpuid;
> > }
> >
> > static void __init acrn_init_platform(void)
> > {
> > /* Install system interrupt handler for ACRN hypervisor callback */
> > sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_acrn_hv_callback);
> > +
> > + acrn_tsc_khz_cpuid = cpuid_eax(ACRN_CPUID_TIMING_INFO);
>
> [Severity: High]
> This is a pre-existing issue, but does hardcoding ACRN_CPUID_TIMING_INFO to
> 0x40000010 without dynamically applying the base offset break nested setups?
>
> If Linux is booted as an L2 guest with an L0 hypervisor like KVM occupying
> the base 0x40000000 leaf, and L1 ACRN resides at 0x40000100, acrn_detect()
> correctly finds ACRN using cpuid_base_hypervisor() at the 0x40000100 base.
>
> However, acrn_init_platform() hardcodes the fetch:
>
> arch/x86/include/asm/acrn.h:
> #define ACRN_CPUID_TIMING_INFO 0x40000010
>
> Will this bypass the dynamic base detected earlier and read the timing info
> from the wrong hypervisor's CPUID space (e.g., KVM's L0 frequency)?
>
> If nested under a hypervisor that returns garbage for 0x40000010, could
> acrn_tsc_khz_cpuid be populated with a non-zero bogus value and corrupt
> guest timekeeping?
Yes, but it's not at all clear that ACRN supports advertising itself anywhere
but at 0x40000000. Regardless, this can and should be addressed separately.
^ permalink raw reply
* Re: [PATCH v5 06/51] x86/sev: Don't override CPU frequency calibration for SNP's Secure TSC
From: Sean Christopherson @ 2026-07-01 20:09 UTC (permalink / raw)
To: sashiko-reviews; +Cc: kvm, linux-hyperv
In-Reply-To: <20260701195337.2E1F01F000E9@smtp.kernel.org>
On Wed, Jul 01, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The patch removes the `calibrate_cpu` override to allow the CPU base frequency to differ from the TSC frequency, but fails to account for `determine_cpu_tsc_frequencies()` which will forcefully clobber `cpu_khz` if it diverges by more than 10%.
> --
>
> commit 6d3b9e81661ead5f25434666266aec6514dadaa8
> Author: Sean Christopherson <seanjc@google.com>
>
> x86/sev: Don't override CPU frequency calibration for SNP's Secure TSC
>
> This commit removes the override of the kernel's CPU frequency calibration
> routine when registering SNP's Secure TSC. It aims to decouple the CPU base
> frequency from the TSC frequency, as exposing APERF/MPERF requires an
> accurate CPU frequency.
>
> > diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> > --- a/arch/x86/coco/sev/core.c
> > +++ b/arch/x86/coco/sev/core.c
> > @@ -2046,7 +2046,6 @@ void __init snp_secure_tsc_init(void)
> >
> > snp_tsc_freq_khz = SNP_SCALE_TSC_FREQ(tsc_freq_mhz * 1000, secrets->tsc_factor);
> >
> > - x86_platform.calibrate_cpu = securetsc_get_tsc_khz;
>
> [Severity: Medium]
> Will this removal be sufficient to decouple cpu_khz from tsc_khz if they
> diverge significantly?
No, but IMO it's a step in the right direction.
> Looking at determine_cpu_tsc_frequencies() in arch/x86/kernel/tsc.c, there
> is a generic sanity check that enforces a 10% limit on the difference
> between the two frequencies:
>
> arch/x86/kernel/tsc.c:determine_cpu_tsc_frequencies() {
> ...
> else if (abs(cpu_khz - tsc_khz) * 10 > tsc_khz)
> cpu_khz = tsc_khz;
> ...
> }
>
> If the true host CPU frequency diverges from the guest TSC frequency by more
> than 10%, won't this logic forcefully overwrite the correct cpu_khz back to
> tsc_khz, unintentionally undoing the fix for the APERF/MPERF calculation?
Probably, but this series is already boiling a pretty large lake, I think the
ocean can be left for a future cleanup.
^ permalink raw reply
* Re: [PATCH v5 05/51] x86/sev: Mark TSC as reliable when configuring Secure TSC
From: Sean Christopherson @ 2026-07-01 20:07 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-hyperv, kvm
In-Reply-To: <20260701195652.5DEF11F000E9@smtp.kernel.org>
On Wed, Jul 01, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> commit cdbd80de837f6d512a759452a18164ddd83b9b7b
> Author: Sean Christopherson <seanjc@google.com>
>
> x86/sev: Mark TSC as reliable when configuring Secure TSC
>
> This commit moves the setting of the X86_FEATURE_TSC_RELIABLE flag from
> sme_early_init() to snp_secure_tsc_init(). It consolidates the handling of
> known-frequency and reliable TSC flags when overriding the TSC calibration
> routine.
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does the early return in
> arch/x86/kernel/tsc.c:tsc_early_init() allow an untrusted hypervisor to
> bypass Secure TSC initialization?
>
> The tsc_early_init() function checks is_early_uv_system() before initializing
> Secure TSC. The is_early_uv_system() check relies on the presence of
> UV_SYSTEM_TABLE_GUID in the EFI configuration tables.
>
> Because the hypervisor can provide tables to the guest firmware which may
> expose them as EFI configuration tables, an untrusted hypervisor could inject
> this specific GUID. Would this trick the kernel into returning early,
> bypassing both Secure TSC and TDX TSC initialization, and causing the guest
> to fall back to untrusted legacy timers?
I suppose so? But that's a problem best handled in a dedicated, separate patch.
>
> [Severity: High]
> This is also a pre-existing issue, but should the newly updated
> arch/x86/coco/sev/core.c:snp_secure_tsc_init() validate that the calculated
> frequency is non-zero before returning?
>
> If the calculated frequency evaluates to 0, which could happen if the
> hypervisor causes MSR_AMD64_GUEST_TSC_FREQ to read as 0 or if the secrets
> factor is invalid, the function simply returns 0 instead of terminating
> the VM.
The hypervisor can't do that? If it can, that too should be addressed separately.
> The caller tsc_early_init() treats a return value of 0 as a failure to find
> a trusted frequency and gracefully falls back to x86_init.hyper.get_tsc_khz()
> or legacy timers. In a confidential computing threat model, should this
> security-critical initialization fail closed and terminate the VM, rather
> than falling back to the hypervisor-controlled time sources that Secure TSC
> is designed to protect against?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260701193212.749551-1-seanjc@google.com?part=5
^ permalink raw reply
* Re: [PATCH v5 01/51] x86/apic: Provide helpers to set local APIC timer period in hz and khz
From: Sean Christopherson @ 2026-07-01 20:05 UTC (permalink / raw)
To: sashiko-reviews; +Cc: kvm, linux-hyperv
In-Reply-To: <20260701194621.4BD691F000E9@smtp.kernel.org>
On Wed, Jul 01, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> > index aa1e19979aa8e..8d3d930576fd7 100644
> > --- a/arch/x86/kernel/apic/apic.c
> > +++ b/arch/x86/kernel/apic/apic.c
>
> [ ... ]
>
> > @@ -796,6 +796,16 @@ bool __init apic_needs_pit(void)
> > return lapic_timer_period == 0;
> > }
> >
> > +void apic_set_timer_period_khz(u64 period_khz, const char *source)
> > +{
> > + lapic_timer_period = mul_u64_u32_div(period_khz, 1000, HZ);
> > +}
> > +
> > +void apic_set_timer_period_hz(u64 period_hz, const char *source)
> > +{
> > + lapic_timer_period = div_u64(period_hz, HZ);
> > +}
>
> [Severity: Medium]
> It looks like the source parameter is accepted by apic_set_timer_period_khz()
> and apic_set_timer_period_hz() in arch/x86/kernel/apic/apic.c but is never
> used. Was it intended to log the frequency using this source string?
/facepalm, yes.
^ permalink raw reply
* Re: [PATCH v5 22/51] x86/kvm: Mark TSC as reliable when it's constant and nonstop
From: sashiko-bot @ 2026-07-01 20:03 UTC (permalink / raw)
To: Sean Christopherson; +Cc: linux-hyperv, kvm
In-Reply-To: <20260701193212.749551-23-seanjc@google.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditionally forcing X86_FEATURE_TSC_RELIABLE bypasses SMP cross-CPU TSC synchronization checks, which will cause time to go backwards for guests running on hosts with unsynchronized TSCs.
--
commit fb8fd2f8d18aabaf24f424391748d58e9427bdf7
Author: Sean Christopherson <seanjc@google.com>
x86/kvm: Mark TSC as reliable when it's constant and nonstop
This commit marks the TSC as reliable if KVM enumerates it as constant
and nonstop. This aims to prevent the TSC from occasionally being marked
unstable when the host is under moderate or heavy load.
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 909d3e5e5bcd5..1cef54e1e7d9e 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
[ ... ]
> @@ -1040,7 +1041,16 @@ static void __init kvm_init_platform(void)
> }
> }
>
> - kvmclock_init();
> + /*
> + * If the TSC counts at a constant frequency across P/T states and in
> + * deep C-states, treat the TSC reliable, as guaranteed by KVM.
> + */
> + tsc_is_reliable = boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
> + boot_cpu_has(X86_FEATURE_NONSTOP_TSC);
> + if (tsc_is_reliable)
> + setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
[Severity: High]
Does setting X86_FEATURE_TSC_RELIABLE here bypass SMP cross-CPU TSC
synchronization checks?
Forcing X86_FEATURE_TSC_RELIABLE based only on CONSTANT_TSC and NONSTOP_TSC,
without verifying if the hypervisor actually guarantees cross-CPU
synchronization (e.g., via KVM_FEATURE_CLOCKSOURCE_STABLE_BIT), sets
tsc_clocksource_reliable = 1.
This bypasses the synchronization check in check_tsc_sync_source():
arch/x86/kernel/tsc_sync.c:check_tsc_sync_source() {
...
if (tsc_store_and_check_tsc_adjust(false) || tsc_clocksource_reliable)
return;
...
}
Could this cause time to go backwards for guests running on hosts with
unsynchronized TSCs when threads migrate between vCPUs?
> +
> + kvmclock_init(tsc_is_reliable);
> x86_platform.apic_post_init = kvm_apic_init;
>
> /*
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701193212.749551-1-seanjc@google.com?part=22
^ permalink raw reply
* Re: [PATCH v5 29/51] x86/kvm: Don't disable kvmclock on BSP in syscore_suspend()
From: sashiko-bot @ 2026-07-01 20:03 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm, linux-hyperv
In-Reply-To: <20260701193212.749551-30-seanjc@google.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The patch inadvertently leaves the BSP's kvmclock enabled when powering off the machine for ACPI S4 hibernation, leading to potential memory corruption when the VM awakens.
Pre-existing issues:
- [Critical] Stack-allocated waitqueue head use-after-free in async page fault handling.
--
Note: The standard inline diff format has been altered to a free-form
summary due to patch recitation restrictions.
commit 03858970f2d7a88b91bdd5b16c1573166126d3c4
Author: Sean Christopherson <seanjc@google.com>
Subject: x86/kvm: Don't disable kvmclock on BSP in syscore_suspend()
This patch modifies the guest suspend logic so that kvmclock is not disabled
on the BSP during syscore_suspend(), relying instead on the sched_clock
restore callback.
[Severity: High]
Does this change inadvertently leave the BSP's kvmclock enabled when
powering off the machine for ACPI S4 hibernation?
Looking at the final S4 power-off path in hibernation_platform_enter() in
kernel/power/hibernate.c, it calls syscore_suspend() but does not call
save_processor_state(). Because this patch removes the unconditional BSP
kvmclock_disable() from syscore_suspend() and relies on
kvm_save_sched_clock_state() (which only fires during image creation),
kvmclock appears to remain fully enabled during the S4 power-off.
Could this lead to memory corruption when the VM awakens, since the
hypervisor might continue writing to the old kvmclock address?
[Severity: Critical]
This isn't a bug introduced by this patch, but is there a use-after-free
risk with the stack-allocated waitqueue head in the async page fault
handling in arch/x86/kernel/kvm.c?
When a guest task waits for an async page fault in
kvm_async_pf_task_wait_schedule(), it waits on a stack-allocated struct
kvm_task_sleep_node. Concurrently, when a waker CPU executes
apf_task_wake_one(), it unhashes the node via hlist_del_init().
The waiter checks hlist_unhashed() locklessly. If it observes the unhashed
state before the waker is finished, the waiter can break its loop, return,
and destroy the stack frame. The waker might then dereference the
deallocated waitqueue head to call swake_up_one(), potentially corrupting
stack memory.
Can this sequence occur during concurrent page fault completions?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701193212.749551-1-seanjc@google.com?part=29
^ permalink raw reply
* Re: [PATCH v5 05/51] x86/sev: Mark TSC as reliable when configuring Secure TSC
From: sashiko-bot @ 2026-07-01 19:56 UTC (permalink / raw)
To: Sean Christopherson; +Cc: linux-hyperv, kvm
In-Reply-To: <20260701193212.749551-6-seanjc@google.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but a malicious hypervisor can bypass Secure TSC calibration by injecting a fake UV system table, forcing the guest to fall back to untrusted legacy timers.
- [High] This problem wasn't introduced by this patch, but if the calculated Secure TSC frequency is 0, the kernel silently falls back to untrusted timers instead of terminating the VM.
--
Note: Due to system recitation restrictions, this report is provided as a free-form summary without direct diff quoting.
commit cdbd80de837f6d512a759452a18164ddd83b9b7b
Author: Sean Christopherson <seanjc@google.com>
x86/sev: Mark TSC as reliable when configuring Secure TSC
This commit moves the setting of the X86_FEATURE_TSC_RELIABLE flag from
sme_early_init() to snp_secure_tsc_init(). It consolidates the handling of
known-frequency and reliable TSC flags when overriding the TSC calibration
routine.
[Severity: High]
This isn't a bug introduced by this patch, but does the early return in
arch/x86/kernel/tsc.c:tsc_early_init() allow an untrusted hypervisor to
bypass Secure TSC initialization?
The tsc_early_init() function checks is_early_uv_system() before initializing
Secure TSC. The is_early_uv_system() check relies on the presence of
UV_SYSTEM_TABLE_GUID in the EFI configuration tables.
Because the hypervisor can provide tables to the guest firmware which may
expose them as EFI configuration tables, an untrusted hypervisor could inject
this specific GUID. Would this trick the kernel into returning early,
bypassing both Secure TSC and TDX TSC initialization, and causing the guest
to fall back to untrusted legacy timers?
[Severity: High]
This is also a pre-existing issue, but should the newly updated
arch/x86/coco/sev/core.c:snp_secure_tsc_init() validate that the calculated
frequency is non-zero before returning?
If the calculated frequency evaluates to 0, which could happen if the
hypervisor causes MSR_AMD64_GUEST_TSC_FREQ to read as 0 or if the secrets
factor is invalid, the function simply returns 0 instead of terminating
the VM.
The caller tsc_early_init() treats a return value of 0 as a failure to find
a trusted frequency and gracefully falls back to x86_init.hyper.get_tsc_khz()
or legacy timers. In a confidential computing threat model, should this
security-critical initialization fail closed and terminate the VM, rather
than falling back to the hypervisor-controlled time sources that Secure TSC
is designed to protect against?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701193212.749551-1-seanjc@google.com?part=5
^ permalink raw reply
* Re: [PATCH v5 24/51] x86/kvm: Get CPU base frequency from CPUID when it's available
From: sashiko-bot @ 2026-07-01 19:54 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm, linux-hyperv
In-Reply-To: <20260701193212.749551-25-seanjc@google.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The base CPU frequency obtained from CPUID.0x16 is unconditionally clamped back to the TSC frequency if it differs by >10%, defeating the purpose of the patch.
--
commit 1331572140e49219bdcc6ceabc0370555476adde
Author: Sean Christopherson <seanjc@google.com>
x86/kvm: Get CPU base frequency from CPUID when it's available
This commit reads CPUID.0x16 to fetch the CPU base frequency if valid.
It then sets x86_init.hyper.get_cpu_khz to kvm_get_cpu_khz to avoid
assuming the CPU base frequency matches the TSC frequency.
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 1cef54e1e7d9e..6c7011ff7bd13 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
[ ... ]
> @@ -1041,6 +1047,14 @@ static void __init kvm_init_platform(void)
> }
> }
>
> + /*
> + * Prefer CPUID.0x16 over KVM's PV CPUID when possible, as the base CPU
> + * frequency isn't necessarily the same as the TSC frequency.
> + */
> + kvm_cpu_khz_cpuid = __cpu_khz_from_cpuid();
> + if (kvm_cpu_khz_cpuid)
> + x86_init.hyper.get_cpu_khz = kvm_get_cpu_khz;
[Severity: Medium]
Will this new CPU frequency actually be used if it diverges significantly
from the TSC frequency?
Looking at determine_cpu_tsc_frequencies() in arch/x86/kernel/tsc.c, there
is existing logic that unconditionally clamps the CPU frequency back to the
TSC frequency if they differ by more than 10%:
else if (abs(cpu_khz - tsc_khz) * 10 > tsc_khz)
cpu_khz = tsc_khz;
Does this clamping defeat the purpose of this patch for guests running on
hosts where the base CPU frequency and TSC frequency differ by >10%?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701193212.749551-1-seanjc@google.com?part=24
^ permalink raw reply
* Re: [PATCH v5 06/51] x86/sev: Don't override CPU frequency calibration for SNP's Secure TSC
From: sashiko-bot @ 2026-07-01 19:53 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm, linux-hyperv
In-Reply-To: <20260701193212.749551-7-seanjc@google.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch removes the `calibrate_cpu` override to allow the CPU base frequency to differ from the TSC frequency, but fails to account for `determine_cpu_tsc_frequencies()` which will forcefully clobber `cpu_khz` if it diverges by more than 10%.
--
commit 6d3b9e81661ead5f25434666266aec6514dadaa8
Author: Sean Christopherson <seanjc@google.com>
x86/sev: Don't override CPU frequency calibration for SNP's Secure TSC
This commit removes the override of the kernel's CPU frequency calibration
routine when registering SNP's Secure TSC. It aims to decouple the CPU base
frequency from the TSC frequency, as exposing APERF/MPERF requires an
accurate CPU frequency.
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -2046,7 +2046,6 @@ void __init snp_secure_tsc_init(void)
>
> snp_tsc_freq_khz = SNP_SCALE_TSC_FREQ(tsc_freq_mhz * 1000, secrets->tsc_factor);
>
> - x86_platform.calibrate_cpu = securetsc_get_tsc_khz;
[Severity: Medium]
Will this removal be sufficient to decouple cpu_khz from tsc_khz if they
diverge significantly?
Looking at determine_cpu_tsc_frequencies() in arch/x86/kernel/tsc.c, there
is a generic sanity check that enforces a 10% limit on the difference
between the two frequencies:
arch/x86/kernel/tsc.c:determine_cpu_tsc_frequencies() {
...
else if (abs(cpu_khz - tsc_khz) * 10 > tsc_khz)
cpu_khz = tsc_khz;
...
}
If the true host CPU frequency diverges from the guest TSC frequency by more
than 10%, won't this logic forcefully overwrite the correct cpu_khz back to
tsc_khz, unintentionally undoing the fix for the APERF/MPERF calculation?
> x86_platform.calibrate_tsc = securetsc_get_tsc_khz;
>
> early_memunmap(mem, PAGE_SIZE);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701193212.749551-1-seanjc@google.com?part=6
^ permalink raw reply
* Re: [PATCH v5 12/51] x86/acrn: Register TSC/CPU frequency callbacks iff frequency is actually in CPUID
From: sashiko-bot @ 2026-07-01 19:52 UTC (permalink / raw)
To: Sean Christopherson; +Cc: linux-hyperv, kvm
In-Reply-To: <20260701193212.749551-13-seanjc@google.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] ACRN hypervisor integration statically hardcodes its CPUID leaf to 0x40000010 without applying the dynamic base offset, breaking nested setups and potentially causing severe timekeeping corruption.
--
commit 8beac699ed1da5364ebdf7f1cc4475bed5521d60
Author: Sean Christopherson <seanjc@google.com>
x86/acrn: Register TSC/CPU frequency callbacks iff frequency is actually in CPUID
This commit modifies ACRN initialization to only register the TSC and CPU
frequency override callbacks if the hypervisor actually provides a non-zero
frequency in its CPUID leaf. It achieves this by reading the frequency
during platform initialization and caching the result.
> diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
> index ad8f2da8003b3..dc71a6fdd4613 100644
> --- a/arch/x86/kernel/cpu/acrn.c
> +++ b/arch/x86/kernel/cpu/acrn.c
[ ... ]
> @@ -26,13 +28,19 @@ static u32 __init acrn_detect(void)
>
> static unsigned int __init acrn_get_tsc_khz(void)
> {
> - return cpuid_eax(ACRN_CPUID_TIMING_INFO);
> + return acrn_tsc_khz_cpuid;
> }
>
> static void __init acrn_init_platform(void)
> {
> /* Install system interrupt handler for ACRN hypervisor callback */
> sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_acrn_hv_callback);
> +
> + acrn_tsc_khz_cpuid = cpuid_eax(ACRN_CPUID_TIMING_INFO);
[Severity: High]
This is a pre-existing issue, but does hardcoding ACRN_CPUID_TIMING_INFO to
0x40000010 without dynamically applying the base offset break nested setups?
If Linux is booted as an L2 guest with an L0 hypervisor like KVM occupying
the base 0x40000000 leaf, and L1 ACRN resides at 0x40000100, acrn_detect()
correctly finds ACRN using cpuid_base_hypervisor() at the 0x40000100 base.
However, acrn_init_platform() hardcodes the fetch:
arch/x86/include/asm/acrn.h:
#define ACRN_CPUID_TIMING_INFO 0x40000010
Will this bypass the dynamic base detected earlier and read the timing info
from the wrong hypervisor's CPUID space (e.g., KVM's L0 frequency)?
If nested under a hypervisor that returns garbage for 0x40000010, could
acrn_tsc_khz_cpuid be populated with a non-zero bogus value and corrupt
guest timekeeping?
> + if (acrn_tsc_khz_cpuid) {
> + x86_init.hyper.get_tsc_khz = acrn_get_tsc_khz;
> + x86_init.hyper.get_cpu_khz = acrn_get_tsc_khz;
> + }
> }
>
> static bool acrn_x2apic_available(void)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701193212.749551-1-seanjc@google.com?part=12
^ permalink raw reply
* Re: [PATCH v5 17/51] x86/tsc: Fold native_calibrate_cpu() into recalibrate_cpu_khz()
From: sashiko-bot @ 2026-07-01 19:50 UTC (permalink / raw)
To: Sean Christopherson; +Cc: linux-hyperv, kvm
In-Reply-To: <20260701193212.749551-18-seanjc@google.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The refactoring in `recalibrate_cpu_khz()` introduces a race condition where the global variable `cpu_khz` is temporarily set to 0 for up to 150ms during runtime CPU frequency recalibration. This exposes the system to invalid state reads.
--
commit 34cf1b98d726e6c329bdefbd5c9712901d8b4bc8
Author: Sean Christopherson <seanjc@google.com>
x86/tsc: Fold native_calibrate_cpu() into recalibrate_cpu_khz()
This commit folds the implementation of native_calibrate_cpu() directly into
recalibrate_cpu_khz() to remove an extra CONFIG_SMP=n ifdef block. This change
aims to clarify the intentional use of early versus late calibration routines.
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 9764ac758081f..6ed6f8f012ebf 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
[ ... ]
> @@ -944,7 +929,9 @@ void recalibrate_cpu_khz(void)
> if (WARN_ON_ONCE(cpu_feature_enabled(X86_FEATURE_TSC_KNOWN_FREQ)))
> return;
>
> - cpu_khz = native_calibrate_cpu();
> + cpu_khz = native_calibrate_cpu_early();
> + if (!cpu_khz)
> + cpu_khz = native_calibrate_cpu_late();
[Severity: Medium]
Does this expose a race condition by temporarily assigning 0 to the global
cpu_khz variable?
On a UP kernel during cpufreq transitions, if early calibration fails, the
global cpu_khz is directly assigned 0. It remains 0 while
native_calibrate_cpu_late() executes, which can take ~150ms and enables
interrupts.
Could this lead to sysfs readers via arch_freq_get_on_cpu() observing a 0
frequency, or the NMI watchdog being misconfigured with a 0 sample period if
toggled during this window?
The previous implementation used a local variable inside native_calibrate_cpu()
to hold the intermediate state, preventing concurrent readers from seeing an
invalid 0 value before the slow fallback calibration completes.
> tsc_khz = native_calibrate_tsc();
> if (tsc_khz == 0)
> tsc_khz = cpu_khz;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701193212.749551-1-seanjc@google.com?part=17
^ permalink raw reply
* Re: [PATCH v5 02/51] x86/apic: Add CONFIG_X86_LOCAL_APIC=n stubs for apic_set_timer_period_{,k}hz()
From: sashiko-bot @ 2026-07-01 19:48 UTC (permalink / raw)
To: Sean Christopherson; +Cc: linux-hyperv, kvm
In-Reply-To: <20260701193212.749551-3-seanjc@google.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `source` argument in `apic_set_timer_period_hz()` and `apic_set_timer_period_khz()` is completely unused, leading to dead code and lost debug logging.
--
commit a7b2ed4060ee2d44136d35fad056804351b581ca
Author: Sean Christopherson <seanjc@google.com>
x86/apic: Add CONFIG_X86_LOCAL_APIC=n stubs for apic_set_timer_period_{,k}hz()
This commit adds stubs for apic_set_timer_period_hz() and
apic_set_timer_period_khz() when the kernel is built without local APIC
support. It also removes the corresponding #ifdef guards from the calling
code in various x86 subsystems.
> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> index cd84a94688a2f..035998555e992 100644
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -189,6 +189,8 @@ static inline void disable_local_APIC(void) { }
> # define setup_boot_APIC_clock x86_init_noop
> # define setup_secondary_APIC_clock x86_init_noop
> static inline void lapic_update_tsc_freq(void) { }
> +static inline void apic_set_timer_period_hz(u64 period_hz, const char *source) { }
> +static inline void apic_set_timer_period_khz(u64 period_khz, const char *source) { }
[Severity: Medium]
Is it intentional that the source argument is completely unused here in the
stubs?
Looking at the main implementation in arch/x86/kernel/apic/apic.c:
void apic_set_timer_period_khz(u64 period_khz, const char *source)
{
lapic_timer_period = mul_u64_u32_div(period_khz, 1000, HZ);
}
The source parameter is discarded without being used for debug logging. Since
any code calling apic_set_timer_period_hz() or apic_set_timer_period_khz()
passes a string literal describing the source (such as "VMware hypervisor"
or "CPUID 0x15/0x16"), this results in dropped debug information.
Could the source parameter be removed from the API entirely to clean up the
dead code, or should the boot-time logging be restored?
> static inline void init_bsp_APIC(void) { }
> static inline void apic_intr_mode_select(void) { }
> static inline void apic_intr_mode_init(void) { }
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701193212.749551-1-seanjc@google.com?part=2
^ permalink raw reply
* Re: [PATCH v5 01/51] x86/apic: Provide helpers to set local APIC timer period in hz and khz
From: sashiko-bot @ 2026-07-01 19:46 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm, linux-hyperv
In-Reply-To: <20260701193212.749551-2-seanjc@google.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `apic_set_timer_period_khz()` and `apic_set_timer_period_hz()` helper functions accept a `const char *source` argument which is never used, resulting in dead code and a silent loss of boot diagnostic logging on platforms where logging was removed.
--
commit 4fb576b0d909663c9b3e6628dbed45d329c08a5d
Author: Sean Christopherson <seanjc@google.com>
x86/apic: Provide helpers to set local APIC timer period in hz and khz
This commit adds APIs to set the local APIC timer period, abstracting the
HZ math previously open-coded by external callers. It also changes the
lapic_timer_period variable to be local to apic.c.
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index aa1e19979aa8e..8d3d930576fd7 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
[ ... ]
> @@ -796,6 +796,16 @@ bool __init apic_needs_pit(void)
> return lapic_timer_period == 0;
> }
>
> +void apic_set_timer_period_khz(u64 period_khz, const char *source)
> +{
> + lapic_timer_period = mul_u64_u32_div(period_khz, 1000, HZ);
> +}
> +
> +void apic_set_timer_period_hz(u64 period_hz, const char *source)
> +{
> + lapic_timer_period = div_u64(period_hz, HZ);
> +}
[Severity: Medium]
It looks like the source parameter is accepted by apic_set_timer_period_khz()
and apic_set_timer_period_hz() in arch/x86/kernel/apic/apic.c but is never
used. Was it intended to log the frequency using this source string?
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 185d4f677ec0f..87beecec76f04 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -646,10 +646,7 @@ static void __init ms_hyperv_init_platform(void)
> u64 hv_lapic_frequency;
>
> rdmsrq(HV_X64_MSR_APIC_FREQUENCY, hv_lapic_frequency);
> - hv_lapic_frequency = div_u64(hv_lapic_frequency, HZ);
> - lapic_timer_period = hv_lapic_frequency;
> - pr_info("Hyper-V: LAPIC Timer Frequency: %#x\n",
> - lapic_timer_period);
> + apic_set_timer_period_hz(hv_lapic_frequency, "Hyper-V hypervisor");
> }
>
[ ... ]
> diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
> index 34b73573b108d..36f779dd311dc 100644
> --- a/arch/x86/kernel/cpu/vmware.c
> +++ b/arch/x86/kernel/cpu/vmware.c
> @@ -424,9 +424,7 @@ static void __init vmware_platform_setup(void)
>
> #ifdef CONFIG_X86_LOCAL_APIC
> /* Skip lapic calibration since we know the bus frequency. */
> - lapic_timer_period = ecx / HZ;
> - pr_info("Host bus clock speed read from hypervisor : %u Hz\n",
> - ecx);
> + apic_set_timer_period_hz(ecx, "VMware hypervisor");
> #endif
[Severity: Medium]
These changes remove the pr_info() logging of the APIC frequency during early
boot APIC initialization in ms_hyperv_init_platform()
(arch/x86/kernel/cpu/mshyperv.c) and vmware_platform_setup()
(arch/x86/kernel/cpu/vmware.c).
Since the new apic_set_timer_period_hz() helper does not log this information,
does this result in an unintended loss of boot diagnostic logging on these
platforms?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260701193212.749551-1-seanjc@google.com?part=1
^ permalink raw reply
* [PATCH v5 51/51] x86/kvm: Get local APIC bus frequency from PV CPUID Timing Info
From: Sean Christopherson @ 2026-07-01 19:32 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
When running as a KVM guest with PV timing info provided by the host,
stuff the APIC timer period/frequency with the local APIC bus frequency
reported in CPUID.0x40000010.EBX instead of trying to calibrate/guess the
frequency.
See Documentation/virt/kvm/x86/cpuid.rst for details.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/kvm.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index f9a6346077b0..beea0b6aa78e 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -990,7 +990,7 @@ static void __init kvm_init_platform(void)
.mask_lo = (u32)(~(SZ_4G - tolud - 1)) | MTRR_PHYSMASK_V,
.mask_hi = (BIT_ULL(boot_cpu_data.x86_phys_bits) - 1) >> 32,
};
- u32 timing_info_leaf;
+ u32 timing_info_leaf, apic_khz;
bool tsc_is_reliable;
if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT) &&
@@ -1052,6 +1052,11 @@ static void __init kvm_init_platform(void)
x86_init.hyper.get_tsc_khz = kvm_get_tsc_khz;
x86_init.hyper.get_cpu_khz = kvm_get_tsc_khz;
}
+
+ /* The leaf also includes the local APIC bus/timer frequency.*/
+ apic_khz = cpuid_ebx(timing_info_leaf);
+ if (apic_khz)
+ apic_set_timer_period_khz(apic_khz, "KVM hypervisor");
}
/*
--
2.55.0.rc0.799.gd6f94ed593-goog
^ 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