* Re: [PATCH 3/4] x86: hyperv: Expose hv_map_msi_interrupt function
From: Nuno Das Neves @ 2025-06-23 22:13 UTC (permalink / raw)
To: Thomas Gleixner, Michael Kelley, linux-hyperv@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, catalin.marinas@arm.com, will@kernel.org,
mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, lpieralisi@kernel.org, kw@linux.com,
robh@kernel.org, bhelgaas@google.com,
jinankjain@linux.microsoft.com, skinsburskii@linux.microsoft.com,
mrathor@linux.microsoft.com, x86@kernel.org
In-Reply-To: <878qlmqtbn.ffs@tglx>
On 6/20/2025 9:19 AM, Thomas Gleixner wrote:
> On Wed, Jun 18 2025 at 14:08, Nuno Das Neves wrote:
>> On 6/11/2025 4:07 PM, Michael Kelley wrote:
>>> From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Tuesday, June 10, 2025 4:52 PM
>>>> +/**
>>>> + * hv_map_msi_interrupt() - "Map" the MSI IRQ in the hypervisor.
>>>> + * @data: Describes the IRQ
>>>> + * @out_entry: Hypervisor (MSI) interrupt entry (can be NULL)
>>>> + *
>>>> + * Map the IRQ in the hypervisor by issuing a MAP_DEVICE_INTERRUPT hypercall.
>>>> + */
>>>> +int hv_map_msi_interrupt(struct irq_data *data,
>>>> + struct hv_interrupt_entry *out_entry)
>>>> {
>>>> - union hv_device_id device_id = hv_build_pci_dev_id(dev);
>>>> + struct msi_desc *msidesc;
>>>> + struct pci_dev *dev;
>>>> + union hv_device_id device_id;
>>>> + struct hv_interrupt_entry dummy;
>>>> + struct irq_cfg *cfg = irqd_cfg(data);
>>>> + const cpumask_t *affinity;
>>>> + int cpu;
>>>> + u64 res;
>
> https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations
>
>>>>
>>>> - return hv_map_interrupt(device_id, false, cpu, vector, entry);
>>>> + msidesc = irq_data_get_msi_desc(data);
>>>> + dev = msi_desc_to_pci_dev(msidesc);
>>>> + device_id = hv_build_pci_dev_id(dev);
>>>> + affinity = irq_data_get_effective_affinity_mask(data);
>>>> + cpu = cpumask_first_and(affinity, cpu_online_mask);
>>>
>>> Is the cpus_read_lock held at this point? I'm not sure what the
>>> overall calling sequence looks like. If it is not held, the CPU that
>>> is selected could go offline before hv_map_interrupt() is called.
>>> This computation of the target CPU is the same as in the code
>>> before this patch, but that existing code looks like it has the
>>> same problem.
>>>
>>
>> Thanks for pointing it out - It *looks* like the read lock is not held
>> everywhere this could be called, so it could indeed be a problem.
>>
>> I've been thinking about different ways around this but I lack the
>> knowledge to have an informed opinion about it:
>>
>> - We could take the cpu read lock in this function, would that work?
>
> Obviously not.
>
>> - I'm not actually sure why the code is getting the first cpu off the effective
>> affinity mask in the first place. It is possible to get the apic id (and hence
>> the cpu) already associated with the irq, as per e.g. x86_vector_msi_compose_msg()
>> Maybe we could get the cpu that way, assuming that doesn't have a
>> similar issue.
>
> There is no reason to fiddle in the underlying low level data. The
> effective affinity mask is there for a reason.
>
>> - We could just let this race happen, maybe the outcome isn't too catastrophic?
>
> Let's terminate guesswork mode and look at the facts.
>
> The point is that hv_map_msi_interrupt() is called from:
>
> 1) hv_irq_compose_msi_msg()
>
> 2) hv_arch_irq_unmask() (in patch 4/4)
>
> Both functions are interrupt chip callbacks and invoked with the
> interrupt descriptor lock held.
>
> At the point where they are called, the effective affinity mask is valid
> and immutable. Nothing can modify it as any modification requires the
> interrupt descriptor lock to be held. This applies to the CPU hotplug
> machinery too. So this AND cpu_online_mask is a complete pointless
> voodoo exercise.
>
Thanks for explaining.
> Just use:
>
> cpu = cpumask_first(irq_data_get_effective_affinity_mask(data));
>
> and be done with it.
>
> Please fix that first with a seperate patch before moving this code
> around.
Will do!
Nuno
>
> Thanks,
>
> tglx
^ permalink raw reply
* Re: [PATCH net-next v3 1/3] vsock: Add support for SIOCINQ ioctl
From: Stefano Garzarella @ 2025-06-23 17:01 UTC (permalink / raw)
To: Xuewei Niu
Cc: davem, decui, fupan.lfp, haiyangz, jasowang, kvm, kys, leonardi,
linux-hyperv, linux-kernel, mst, netdev, niuxuewei.nxw, pabeni,
stefanha, virtualization, wei.liu, xuanzhuo
In-Reply-To: <20250622135910.1555285-1-niuxuewei.nxw@antgroup.com>
On Sun, Jun 22, 2025 at 09:59:10PM +0800, Xuewei Niu wrote:
>> ACCin hyper-v maintainers and list since I have a question about hyperv
>> transport.
>>
>> On Tue, Jun 17, 2025 at 12:53:44PM +0800, Xuewei Niu wrote:
>> >Add support for SIOCINQ ioctl, indicating the length of bytes unread in the
>> >socket. The value is obtained from `vsock_stream_has_data()`.
>> >
>> >Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
>> >---
>> > net/vmw_vsock/af_vsock.c | 22 ++++++++++++++++++++++
>> > 1 file changed, 22 insertions(+)
>> >
>> >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> >index 2e7a3034e965..bae6b89bb5fb 100644
>> >--- a/net/vmw_vsock/af_vsock.c
>> >+++ b/net/vmw_vsock/af_vsock.c
>> >@@ -1389,6 +1389,28 @@ static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
>> > vsk = vsock_sk(sk);
>> >
>> > switch (cmd) {
>> >+ case SIOCINQ: {
>> >+ ssize_t n_bytes;
>> >+
>> >+ if (!vsk->transport) {
>> >+ ret = -EOPNOTSUPP;
>> >+ break;
>> >+ }
>> >+
>> >+ if (sock_type_connectible(sk->sk_type) &&
>> >+ sk->sk_state == TCP_LISTEN) {
>> >+ ret = -EINVAL;
>> >+ break;
>> >+ }
>> >+
>> >+ n_bytes = vsock_stream_has_data(vsk);
>>
>> Now looks better to me, I just checked transports: vmci and virtio/vhost
>> returns what we want, but for hyperv we have:
>>
>> static s64 hvs_stream_has_data(struct vsock_sock *vsk)
>> {
>> struct hvsock *hvs = vsk->trans;
>> s64 ret;
>>
>> if (hvs->recv_data_len > 0)
>> return 1;
>>
>> @Hyper-v maintainers: do you know why we don't return `recv_data_len`?
>> Do you think we can do that to support this new feature?
>
>Hi Hyper-v maintainers, could you please take a look at this?
>
>Hi Stefano, if no response, can I fix this issue in the next version?
Yep, but let's wait a little bit more.
In that case, please do it in a separate patch (same series is fine)
that we can easily revert/fix if they will find issues later.
Thanks,
Stefano
>
>Thanks,
>Xuewei
>
>> Thanks,
>> Stefano
>>
>> >+ if (n_bytes < 0) {
>> >+ ret = n_bytes;
>> >+ break;
>> >+ }
>> >+ ret = put_user(n_bytes, arg);
>> >+ break;
>> >+ }
>> > case SIOCOUTQ: {
>> > ssize_t n_bytes;
>> >
>> >--
>> >2.34.1
>> >
>
^ permalink raw reply
* [PATCH net-next] net: mana: Fix build errors when CONFIG_NET_SHAPER is disabled
From: Erni Sri Satya Vennela @ 2025-06-23 11:14 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, shradhagupta, longli, kotaranov, lorenzo,
shirazsaleem, ernis, schakrabarti, linux-hyperv, netdev,
linux-kernel
Fix build errors when CONFIG_NET_SHAPER is disabled, including:
drivers/net/ethernet/microsoft/mana/mana_en.c:804:10: error:
'const struct net_device_ops' has no member named 'net_shaper_ops'
804 | .net_shaper_ops = &mana_shaper_ops,
drivers/net/ethernet/microsoft/mana/mana_en.c:804:35: error:
initialization of 'int (*)(struct net_device *, struct neigh_parms *)'
from incompatible pointer type 'const struct net_shaper_ops *'
[-Werror=incompatible-pointer-types]
804 | .net_shaper_ops = &mana_shaper_ops,
Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
Reviewed-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
Fixes: 75cabb46935b ("net: mana: Add support for net_shaper_ops")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202506230625.bfUlqb8o-lkp@intel.com/
---
drivers/net/ethernet/microsoft/mana/mana_en.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 10e766c73fca..a4a18eb02558 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -719,6 +719,7 @@ static int mana_change_mtu(struct net_device *ndev, int new_mtu)
return err;
}
+#if IS_ENABLED(CONFIG_NET_SHAPER)
static int mana_shaper_set(struct net_shaper_binding *binding,
const struct net_shaper *shaper,
struct netlink_ext_ack *extack)
@@ -790,6 +791,7 @@ static const struct net_shaper_ops mana_shaper_ops = {
.delete = mana_shaper_del,
.capabilities = mana_shaper_cap,
};
+#endif
static const struct net_device_ops mana_devops = {
.ndo_open = mana_open,
@@ -801,7 +803,9 @@ static const struct net_device_ops mana_devops = {
.ndo_bpf = mana_bpf,
.ndo_xdp_xmit = mana_xdp_xmit,
.ndo_change_mtu = mana_change_mtu,
+#if IS_ENABLED(CONFIG_NET_SHAPER)
.net_shaper_ops = &mana_shaper_ops,
+#endif
};
static void mana_cleanup_port_context(struct mana_port_context *apc)
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] tools/hv: fcopy: Fix irregularities with size of ring buffer
From: Naman Jain @ 2025-06-23 4:56 UTC (permalink / raw)
To: Olaf Hering
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Michael Kelley, Saurabh Sengar, linux-hyperv
In-Reply-To: <20250621070644.3b4185b6.olaf@aepfle.de>
On 6/21/2025 10:36 AM, Olaf Hering wrote:
> Fri, 20 Jun 2025 12:36:18 +0530 Naman Jain <namjain@linux.microsoft.com>:
>
>> + desc = (unsigned char *)malloc(ring_size * sizeof(unsigned char));
>
> Is this cast required?
>
>
> Olaf
This should not be required, will remove it. Thanks for catching this!
Regards,
Naman
^ permalink raw reply
* Re: [PATCH] tools/hv: fcopy: Fix irregularities with size of ring buffer
From: Naman Jain @ 2025-06-23 4:51 UTC (permalink / raw)
To: Michael Kelley, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Saurabh Sengar
Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
In-Reply-To: <SN6PR02MB41574C54FFDE0D3F3B7A5649D47CA@SN6PR02MB4157.namprd02.prod.outlook.com>
On 6/20/2025 9:35 PM, Michael Kelley wrote:
> From: Naman Jain <namjain@linux.microsoft.com> Sent: Friday, June 20, 2025 12:06 AM
>>
>> Size of ring buffer, as defined in uio_hv_generic driver, is no longer
>> fixed to 16 KB. This creates a problem in fcopy, since this size was
>> hardcoded. With the change in place to make ring sysfs node actually
>> reflect the size of underlying ring buffer, it is safe to get the size
>> of ring sysfs file and use it for ring buffer size in fcopy daemon.
>> Fix the issue of disparity in ring buffer size, by making it dynamic
>> in fcopy uio daemon.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 0315fef2aff9 ("uio_hv_generic: Align ring size to system page")
>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
>> ---
>> tools/hv/hv_fcopy_uio_daemon.c | 65 ++++++++++++++++++++++++++++++----
>> 1 file changed, 58 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
>> index 0198321d14a2..da2b27d6af0e 100644
>> --- a/tools/hv/hv_fcopy_uio_daemon.c
>> +++ b/tools/hv/hv_fcopy_uio_daemon.c
>> @@ -36,6 +36,7 @@
>> #define WIN8_SRV_VERSION (WIN8_SRV_MAJOR << 16 | WIN8_SRV_MINOR)
>>
>> #define FCOPY_UIO "/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/uio"
>> +#define FCOPY_CHANNELS_PATH "/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/channels"
>>
>> #define FCOPY_VER_COUNT 1
>> static const int fcopy_versions[] = {
>> @@ -47,9 +48,51 @@ static const int fw_versions[] = {
>> UTIL_FW_VERSION
>> };
>>
>> -#define HV_RING_SIZE 0x4000 /* 16KB ring buffer size */
>> +#define HV_RING_SIZE_DEFAULT 0x4000 /* 16KB ring buffer size default */
>>
>> -static unsigned char desc[HV_RING_SIZE];
>> +static uint32_t get_ring_buffer_size(void)
>> +{
>> + char ring_path[PATH_MAX];
>> + DIR *dir;
>> + struct dirent *entry;
>> + struct stat st;
>> + uint32_t ring_size = 0;
>> +
>> + /* Find the channel directory */
>> + dir = opendir(FCOPY_CHANNELS_PATH);
>> + if (!dir) {
>> + syslog(LOG_ERR, "Failed to open channels directory, using default ring size");
>
> This is where the previous long discussion about racing with user space
> comes into play. While highly unlikely, it's possible that the "opendir" could fail
> because of racing with the kernel thread that creates the "channels" directory.
> The right thing to do would be to sleep for some period of time, then try
> again. Sleeping for 1 second would be a very generous -- could also go with
> something like 100 milliseconds.
Makes sense, will add that logic.
>
>> + return HV_RING_SIZE_DEFAULT;
>> + }
>> +
>> + while ((entry = readdir(dir)) != NULL) {
>> + if (entry->d_type == DT_DIR && strcmp(entry->d_name, ".") != 0 &&
>> + strcmp(entry->d_name, "..") != 0) {
>> + snprintf(ring_path, sizeof(ring_path), "%s/%s/ring",
>> + FCOPY_CHANNELS_PATH, entry->d_name);
>> +
>> + if (stat(ring_path, &st) == 0) {
>> + /* stat returns size of Tx, Rx rings combined, so take half of it */
>> + ring_size = (uint32_t)st.st_size / 2;
>> + syslog(LOG_INFO, "Ring buffer size from %s: %u bytes",
>> + ring_path, ring_size);
>> + break;
>> + }
>> + }
>> + }
>
> The same race problem could happen with this loop. The "channels" directory
> might have been created, but the entry for the numbered channel might not.
> The loop could exit having found only "." and "..". Again, if no numbered
> channel is found, sleep for a short period of time and try again.
Will cover this too.
>
>> +
>> + closedir(dir);
>> +
>> + if (!ring_size) {
>> + ring_size = HV_RING_SIZE_DEFAULT;
>> + syslog(LOG_ERR, "Could not determine ring size, using default: %u bytes",
>> + HV_RING_SIZE_DEFAULT);
>> + }
>> +
>> + return ring_size;
>> +}
>> +
>> +static unsigned char *desc;
>>
>> static int target_fd;
>> static char target_fname[PATH_MAX];
>> @@ -406,7 +449,8 @@ int main(int argc, char *argv[])
>> int daemonize = 1, long_index = 0, opt, ret = -EINVAL;
>> struct vmbus_br txbr, rxbr;
>> void *ring;
>> - uint32_t len = HV_RING_SIZE;
>> + uint32_t ring_size = get_ring_buffer_size();
>
> Getting the ring buffer size before even the command line options
> are parsed could produce unexpected results. For example, if someone
> just wanted to see the usage (the -h option), they might get
> an error about not being able to get the ring size. I'd suggest doing
> this later, after the /dev/uio<N> entry is successfully opened.
Thanks for pointing this out, I'll take care of it in next version.
Regards,
Naman
>
>> + uint32_t len = ring_size;
>> char uio_name[NAME_MAX] = {0};
>> char uio_dev_path[PATH_MAX] = {0};
>>
>> @@ -416,6 +460,13 @@ int main(int argc, char *argv[])
>> {0, 0, 0, 0 }
>> };
>>
>> + desc = (unsigned char *)malloc(ring_size * sizeof(unsigned char));
>> + if (!desc) {
>> + syslog(LOG_ERR, "malloc failed for desc buffer");
>> + ret = -ENOMEM;
>> + goto exit;
>> + }
>> +
>> while ((opt = getopt_long(argc, argv, "hn", long_options,
>> &long_index)) != -1) {
>> switch (opt) {
>> @@ -448,14 +499,14 @@ int main(int argc, char *argv[])
>> goto exit;
>> }
>>
>> - ring = vmbus_uio_map(&fcopy_fd, HV_RING_SIZE);
>> + ring = vmbus_uio_map(&fcopy_fd, ring_size);
>> if (!ring) {
>> ret = errno;
>> syslog(LOG_ERR, "mmap ringbuffer failed; error: %d %s", ret, strerror(ret));
>> goto close;
>> }
>> - vmbus_br_setup(&txbr, ring, HV_RING_SIZE);
>> - vmbus_br_setup(&rxbr, (char *)ring + HV_RING_SIZE, HV_RING_SIZE);
>> + vmbus_br_setup(&txbr, ring, ring_size);
>> + vmbus_br_setup(&rxbr, (char *)ring + ring_size, ring_size);
>>
>> rxbr.vbr->imask = 0;
>>
>> @@ -472,7 +523,7 @@ int main(int argc, char *argv[])
>> goto close;
>> }
>>
>> - len = HV_RING_SIZE;
>> + len = ring_size;
>> ret = rte_vmbus_chan_recv_raw(&rxbr, desc, &len);
>> if (unlikely(ret <= 0)) {
>> /* This indicates a failure to communicate (or worse) */
>>
>> base-commit: bc6e0ba6c9bafa6241b05524b9829808056ac4ad
>> --
>> 2.34.1
^ permalink raw reply
* Re: [PATCH net-next v3 1/3] vsock: Add support for SIOCINQ ioctl
From: Xuewei Niu @ 2025-06-22 13:59 UTC (permalink / raw)
To: sgarzare
Cc: davem, decui, fupan.lfp, haiyangz, jasowang, kvm, kys, leonardi,
linux-hyperv, linux-kernel, mst, netdev, niuxuewei.nxw,
niuxuewei97, pabeni, stefanha, virtualization, wei.liu, xuanzhuo
In-Reply-To: <y465uw5phymt3gbgdxsxlopeyhcbbherjri6b6etl64qhsc4ud@vc2c45mo5zxw>
> ACCin hyper-v maintainers and list since I have a question about hyperv
> transport.
>
> On Tue, Jun 17, 2025 at 12:53:44PM +0800, Xuewei Niu wrote:
> >Add support for SIOCINQ ioctl, indicating the length of bytes unread in the
> >socket. The value is obtained from `vsock_stream_has_data()`.
> >
> >Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
> >---
> > net/vmw_vsock/af_vsock.c | 22 ++++++++++++++++++++++
> > 1 file changed, 22 insertions(+)
> >
> >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> >index 2e7a3034e965..bae6b89bb5fb 100644
> >--- a/net/vmw_vsock/af_vsock.c
> >+++ b/net/vmw_vsock/af_vsock.c
> >@@ -1389,6 +1389,28 @@ static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
> > vsk = vsock_sk(sk);
> >
> > switch (cmd) {
> >+ case SIOCINQ: {
> >+ ssize_t n_bytes;
> >+
> >+ if (!vsk->transport) {
> >+ ret = -EOPNOTSUPP;
> >+ break;
> >+ }
> >+
> >+ if (sock_type_connectible(sk->sk_type) &&
> >+ sk->sk_state == TCP_LISTEN) {
> >+ ret = -EINVAL;
> >+ break;
> >+ }
> >+
> >+ n_bytes = vsock_stream_has_data(vsk);
>
> Now looks better to me, I just checked transports: vmci and virtio/vhost
> returns what we want, but for hyperv we have:
>
> static s64 hvs_stream_has_data(struct vsock_sock *vsk)
> {
> struct hvsock *hvs = vsk->trans;
> s64 ret;
>
> if (hvs->recv_data_len > 0)
> return 1;
>
> @Hyper-v maintainers: do you know why we don't return `recv_data_len`?
> Do you think we can do that to support this new feature?
Hi Hyper-v maintainers, could you please take a look at this?
Hi Stefano, if no response, can I fix this issue in the next version?
Thanks,
Xuewei
> Thanks,
> Stefano
>
> >+ if (n_bytes < 0) {
> >+ ret = n_bytes;
> >+ break;
> >+ }
> >+ ret = put_user(n_bytes, arg);
> >+ break;
> >+ }
> > case SIOCOUTQ: {
> > ssize_t n_bytes;
> >
> >--
> >2.34.1
> >
^ permalink raw reply
* [QUERY] net: mana: use of sizeof(header) vs sizeof(*header)
From: Alok Tiwari @ 2025-06-21 17:53 UTC (permalink / raw)
To: linux-hyperv, kys, decui, shacharr, haiyangz, shradhagupta,
shirazsaleem, kotaranov
Cc: alok.a.tiwari, davem, edumazet, kuba, pabeni
mana_gd_write_client_oob() in gdma_main.c, I see that the original code
used: -> ptr = wqe_ptr + sizeof(header);
However, since header is a pointer to struct gdma_wqe, would not this
expression evaluate to the size of the pointer itself (typically 8 bytes
on 64-bit systems)
rather than the actual size of the structure?
Should it be the correct expression -> ptr = wqe_ptr + sizeof(*header);
to accurately skip over the size of the structure that header points to?
Even though sizeof(header) and sizeof(*header) may both return 8 in this
case by coincidence.
but that seems unintentional or potentially misleading. Apologies if I
misunderstood that context.
Thanks,
Alok
---
drivers/net/ethernet/microsoft/mana/gdma_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 3504507477c60..df4ee8c23bcbb 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -1086,7 +1086,7 @@ static u32 mana_gd_write_client_oob(const struct gdma_wqe_request *wqe_req,
* to one Basic Unit (i.e. 32 bytes), so the pointer can't go beyond
* the queue memory buffer boundary.
*/
- ptr = wqe_ptr + sizeof(header);
+ ptr = wqe_ptr + sizeof(*header);
if (wqe_req->inline_oob_data && wqe_req->inline_oob_size > 0) {
memcpy(ptr, wqe_req->inline_oob_data, wqe_req->inline_oob_size);
@@ -1096,7 +1096,7 @@ static u32 mana_gd_write_client_oob(const struct gdma_wqe_request *wqe_req,
client_oob_size - wqe_req->inline_oob_size);
}
- return sizeof(header) + client_oob_size;
+ return sizeof(*header) + client_oob_size;
}
static void mana_gd_write_sgl(struct gdma_queue *wq, u8 *wqe_ptr,
--
2.46.0
^ permalink raw reply related
* Re: [PATCH] tools/hv: fcopy: Fix irregularities with size of ring buffer
From: Olaf Hering @ 2025-06-21 5:06 UTC (permalink / raw)
To: Naman Jain
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Michael Kelley, Saurabh Sengar, linux-hyperv
In-Reply-To: <20250620070618.3097-1-namjain@linux.microsoft.com>
[-- Attachment #1: Type: text/plain, Size: 185 bytes --]
Fri, 20 Jun 2025 12:36:18 +0530 Naman Jain <namjain@linux.microsoft.com>:
> + desc = (unsigned char *)malloc(ring_size * sizeof(unsigned char));
Is this cast required?
Olaf
[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] x86: hyperv: Expose hv_map_msi_interrupt function
From: Thomas Gleixner @ 2025-06-20 16:19 UTC (permalink / raw)
To: Nuno Das Neves, Michael Kelley, linux-hyperv@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org
Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
decui@microsoft.com, catalin.marinas@arm.com, will@kernel.org,
mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, lpieralisi@kernel.org, kw@linux.com,
robh@kernel.org, bhelgaas@google.com,
jinankjain@linux.microsoft.com, skinsburskii@linux.microsoft.com,
mrathor@linux.microsoft.com, x86@kernel.org
In-Reply-To: <8f96db3f-fc3b-44b6-ab28-26bca6e2615b@linux.microsoft.com>
On Wed, Jun 18 2025 at 14:08, Nuno Das Neves wrote:
> On 6/11/2025 4:07 PM, Michael Kelley wrote:
>> From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Tuesday, June 10, 2025 4:52 PM
>>> +/**
>>> + * hv_map_msi_interrupt() - "Map" the MSI IRQ in the hypervisor.
>>> + * @data: Describes the IRQ
>>> + * @out_entry: Hypervisor (MSI) interrupt entry (can be NULL)
>>> + *
>>> + * Map the IRQ in the hypervisor by issuing a MAP_DEVICE_INTERRUPT hypercall.
>>> + */
>>> +int hv_map_msi_interrupt(struct irq_data *data,
>>> + struct hv_interrupt_entry *out_entry)
>>> {
>>> - union hv_device_id device_id = hv_build_pci_dev_id(dev);
>>> + struct msi_desc *msidesc;
>>> + struct pci_dev *dev;
>>> + union hv_device_id device_id;
>>> + struct hv_interrupt_entry dummy;
>>> + struct irq_cfg *cfg = irqd_cfg(data);
>>> + const cpumask_t *affinity;
>>> + int cpu;
>>> + u64 res;
https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations
>>>
>>> - return hv_map_interrupt(device_id, false, cpu, vector, entry);
>>> + msidesc = irq_data_get_msi_desc(data);
>>> + dev = msi_desc_to_pci_dev(msidesc);
>>> + device_id = hv_build_pci_dev_id(dev);
>>> + affinity = irq_data_get_effective_affinity_mask(data);
>>> + cpu = cpumask_first_and(affinity, cpu_online_mask);
>>
>> Is the cpus_read_lock held at this point? I'm not sure what the
>> overall calling sequence looks like. If it is not held, the CPU that
>> is selected could go offline before hv_map_interrupt() is called.
>> This computation of the target CPU is the same as in the code
>> before this patch, but that existing code looks like it has the
>> same problem.
>>
>
> Thanks for pointing it out - It *looks* like the read lock is not held
> everywhere this could be called, so it could indeed be a problem.
>
> I've been thinking about different ways around this but I lack the
> knowledge to have an informed opinion about it:
>
> - We could take the cpu read lock in this function, would that work?
Obviously not.
> - I'm not actually sure why the code is getting the first cpu off the effective
> affinity mask in the first place. It is possible to get the apic id (and hence
> the cpu) already associated with the irq, as per e.g. x86_vector_msi_compose_msg()
> Maybe we could get the cpu that way, assuming that doesn't have a
> similar issue.
There is no reason to fiddle in the underlying low level data. The
effective affinity mask is there for a reason.
> - We could just let this race happen, maybe the outcome isn't too catastrophic?
Let's terminate guesswork mode and look at the facts.
The point is that hv_map_msi_interrupt() is called from:
1) hv_irq_compose_msi_msg()
2) hv_arch_irq_unmask() (in patch 4/4)
Both functions are interrupt chip callbacks and invoked with the
interrupt descriptor lock held.
At the point where they are called, the effective affinity mask is valid
and immutable. Nothing can modify it as any modification requires the
interrupt descriptor lock to be held. This applies to the CPU hotplug
machinery too. So this AND cpu_online_mask is a complete pointless
voodoo exercise.
Just use:
cpu = cpumask_first(irq_data_get_effective_affinity_mask(data));
and be done with it.
Please fix that first with a seperate patch before moving this code
around.
Thanks,
tglx
^ permalink raw reply
* RE: [PATCH] tools/hv: fcopy: Fix irregularities with size of ring buffer
From: Michael Kelley @ 2025-06-20 16:05 UTC (permalink / raw)
To: Naman Jain, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Saurabh Sengar
Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
In-Reply-To: <20250620070618.3097-1-namjain@linux.microsoft.com>
From: Naman Jain <namjain@linux.microsoft.com> Sent: Friday, June 20, 2025 12:06 AM
>
> Size of ring buffer, as defined in uio_hv_generic driver, is no longer
> fixed to 16 KB. This creates a problem in fcopy, since this size was
> hardcoded. With the change in place to make ring sysfs node actually
> reflect the size of underlying ring buffer, it is safe to get the size
> of ring sysfs file and use it for ring buffer size in fcopy daemon.
> Fix the issue of disparity in ring buffer size, by making it dynamic
> in fcopy uio daemon.
>
> Cc: stable@vger.kernel.org
> Fixes: 0315fef2aff9 ("uio_hv_generic: Align ring size to system page")
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> ---
> tools/hv/hv_fcopy_uio_daemon.c | 65 ++++++++++++++++++++++++++++++----
> 1 file changed, 58 insertions(+), 7 deletions(-)
>
> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
> index 0198321d14a2..da2b27d6af0e 100644
> --- a/tools/hv/hv_fcopy_uio_daemon.c
> +++ b/tools/hv/hv_fcopy_uio_daemon.c
> @@ -36,6 +36,7 @@
> #define WIN8_SRV_VERSION (WIN8_SRV_MAJOR << 16 | WIN8_SRV_MINOR)
>
> #define FCOPY_UIO "/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/uio"
> +#define FCOPY_CHANNELS_PATH "/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/channels"
>
> #define FCOPY_VER_COUNT 1
> static const int fcopy_versions[] = {
> @@ -47,9 +48,51 @@ static const int fw_versions[] = {
> UTIL_FW_VERSION
> };
>
> -#define HV_RING_SIZE 0x4000 /* 16KB ring buffer size */
> +#define HV_RING_SIZE_DEFAULT 0x4000 /* 16KB ring buffer size default */
>
> -static unsigned char desc[HV_RING_SIZE];
> +static uint32_t get_ring_buffer_size(void)
> +{
> + char ring_path[PATH_MAX];
> + DIR *dir;
> + struct dirent *entry;
> + struct stat st;
> + uint32_t ring_size = 0;
> +
> + /* Find the channel directory */
> + dir = opendir(FCOPY_CHANNELS_PATH);
> + if (!dir) {
> + syslog(LOG_ERR, "Failed to open channels directory, using default ring size");
This is where the previous long discussion about racing with user space
comes into play. While highly unlikely, it's possible that the "opendir" could fail
because of racing with the kernel thread that creates the "channels" directory.
The right thing to do would be to sleep for some period of time, then try
again. Sleeping for 1 second would be a very generous -- could also go with
something like 100 milliseconds.
> + return HV_RING_SIZE_DEFAULT;
> + }
> +
> + while ((entry = readdir(dir)) != NULL) {
> + if (entry->d_type == DT_DIR && strcmp(entry->d_name, ".") != 0 &&
> + strcmp(entry->d_name, "..") != 0) {
> + snprintf(ring_path, sizeof(ring_path), "%s/%s/ring",
> + FCOPY_CHANNELS_PATH, entry->d_name);
> +
> + if (stat(ring_path, &st) == 0) {
> + /* stat returns size of Tx, Rx rings combined, so take half of it */
> + ring_size = (uint32_t)st.st_size / 2;
> + syslog(LOG_INFO, "Ring buffer size from %s: %u bytes",
> + ring_path, ring_size);
> + break;
> + }
> + }
> + }
The same race problem could happen with this loop. The "channels" directory
might have been created, but the entry for the numbered channel might not.
The loop could exit having found only "." and "..". Again, if no numbered
channel is found, sleep for a short period of time and try again.
> +
> + closedir(dir);
> +
> + if (!ring_size) {
> + ring_size = HV_RING_SIZE_DEFAULT;
> + syslog(LOG_ERR, "Could not determine ring size, using default: %u bytes",
> + HV_RING_SIZE_DEFAULT);
> + }
> +
> + return ring_size;
> +}
> +
> +static unsigned char *desc;
>
> static int target_fd;
> static char target_fname[PATH_MAX];
> @@ -406,7 +449,8 @@ int main(int argc, char *argv[])
> int daemonize = 1, long_index = 0, opt, ret = -EINVAL;
> struct vmbus_br txbr, rxbr;
> void *ring;
> - uint32_t len = HV_RING_SIZE;
> + uint32_t ring_size = get_ring_buffer_size();
Getting the ring buffer size before even the command line options
are parsed could produce unexpected results. For example, if someone
just wanted to see the usage (the -h option), they might get
an error about not being able to get the ring size. I'd suggest doing
this later, after the /dev/uio<N> entry is successfully opened.
> + uint32_t len = ring_size;
> char uio_name[NAME_MAX] = {0};
> char uio_dev_path[PATH_MAX] = {0};
>
> @@ -416,6 +460,13 @@ int main(int argc, char *argv[])
> {0, 0, 0, 0 }
> };
>
> + desc = (unsigned char *)malloc(ring_size * sizeof(unsigned char));
> + if (!desc) {
> + syslog(LOG_ERR, "malloc failed for desc buffer");
> + ret = -ENOMEM;
> + goto exit;
> + }
> +
> while ((opt = getopt_long(argc, argv, "hn", long_options,
> &long_index)) != -1) {
> switch (opt) {
> @@ -448,14 +499,14 @@ int main(int argc, char *argv[])
> goto exit;
> }
>
> - ring = vmbus_uio_map(&fcopy_fd, HV_RING_SIZE);
> + ring = vmbus_uio_map(&fcopy_fd, ring_size);
> if (!ring) {
> ret = errno;
> syslog(LOG_ERR, "mmap ringbuffer failed; error: %d %s", ret, strerror(ret));
> goto close;
> }
> - vmbus_br_setup(&txbr, ring, HV_RING_SIZE);
> - vmbus_br_setup(&rxbr, (char *)ring + HV_RING_SIZE, HV_RING_SIZE);
> + vmbus_br_setup(&txbr, ring, ring_size);
> + vmbus_br_setup(&rxbr, (char *)ring + ring_size, ring_size);
>
> rxbr.vbr->imask = 0;
>
> @@ -472,7 +523,7 @@ int main(int argc, char *argv[])
> goto close;
> }
>
> - len = HV_RING_SIZE;
> + len = ring_size;
> ret = rte_vmbus_chan_recv_raw(&rxbr, desc, &len);
> if (unlikely(ret <= 0)) {
> /* This indicates a failure to communicate (or worse) */
>
> base-commit: bc6e0ba6c9bafa6241b05524b9829808056ac4ad
> --
> 2.34.1
^ permalink raw reply
* [RFC PATCH 1/1] KVM: VMX: Use Hyper-V EPT flush for local TLB flushes
From: Jeremi Piotrowski @ 2025-06-20 15:39 UTC (permalink / raw)
To: Sean Christopherson, Vitaly Kuznetsov, Paolo Bonzini, kvm
Cc: Dave Hansen, linux-kernel, alanjiang, chinang.ma,
andrea.pellegrini, Kevin Tian, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, linux-hyperv, Jeremi Piotrowski
In-Reply-To: <cover.1750432368.git.jpiotrowski@linux.microsoft.com>
Use Hyper-V's HvCallFlushGuestPhysicalAddressSpace for local TLB flushes.
This makes any KVM_REQ_TLB_FLUSH_CURRENT (such as on root alloc) visible to
all CPUs which means we no longer need to do a KVM_REQ_TLB_FLUSH on CPU
migration.
The goal is to avoid invept-global in KVM_REQ_TLB_FLUSH. Hyper-V uses a
shadow page table for the nested hypervisor (KVM) and has to invalidate all
EPT roots when invept-global is issued. This has a performance impact on
all nested VMs. KVM issues KVM_REQ_TLB_FLUSH on CPU migration, and under
load the performance hit causes vCPUs to use up more of their slice of CPU
time, leading to more CPU migrations. This has a snowball effect and causes
CPU usage spikes.
By issuing the hypercall we are now guaranteed that any root modification
that requires a local TLB flush becomes visible to all CPUs. The same
hypercall is already used in kvm_arch_flush_remote_tlbs and
kvm_arch_flush_remote_tlbs_range. The KVM expectation is that roots are
flushed locally on alloc and we achieve consistency on migration by
flushing all roots - the new behavior of achieving consistency on alloc on
Hyper-V is a superset of the expected guarantees. This makes the
KVM_REQ_TLB_FLUSH on CPU migration no longer necessary on Hyper-V.
Coincidentally - we now match the behavior of SVM on Hyper-V.
Signed-off-by: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
---
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/vmx/vmx.c | 20 +++++++++++++++++---
arch/x86/kvm/vmx/vmx_onhyperv.h | 6 ++++++
arch/x86/kvm/x86.c | 3 +++
4 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index b4a391929cdb..d3acab19f425 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1077,6 +1077,7 @@ struct kvm_vcpu_arch {
#if IS_ENABLED(CONFIG_HYPERV)
hpa_t hv_root_tdp;
+ bool hv_vmx_use_flush_guest_mapping;
#endif
};
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 4953846cb30d..f537e0df56fc 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1485,8 +1485,12 @@ void vmx_vcpu_load_vmcs(struct kvm_vcpu *vcpu, int cpu)
/*
* Flush all EPTP/VPID contexts, the new pCPU may have stale
* TLB entries from its previous association with the vCPU.
+ * Unless we are running on Hyper-V where we promotes local TLB
+ * flushes to be visible across all CPUs so no need to do again
+ * on migration.
*/
- kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
+ if (!vmx_hv_use_flush_guest_mapping(vcpu))
+ kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
/*
* Linux uses per-cpu TSS and GDT, so set these when switching
@@ -3243,11 +3247,21 @@ void vmx_flush_tlb_current(struct kvm_vcpu *vcpu)
if (!VALID_PAGE(root_hpa))
return;
- if (enable_ept)
+ if (enable_ept) {
+ /*
+ * hyperv_flush_guest_mapping() has the semantics of
+ * invept-single across all pCPUs. This makes root
+ * modifications consistent across pCPUs, so an invept-global
+ * on migration is no longer required.
+ */
+ if (vmx_hv_use_flush_guest_mapping(vcpu))
+ return (void)WARN_ON_ONCE(hyperv_flush_guest_mapping(root_hpa));
+
ept_sync_context(construct_eptp(vcpu, root_hpa,
mmu->root_role.level));
- else
+ } else {
vpid_sync_context(vmx_get_current_vpid(vcpu));
+ }
}
void vmx_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t addr)
diff --git a/arch/x86/kvm/vmx/vmx_onhyperv.h b/arch/x86/kvm/vmx/vmx_onhyperv.h
index cdf8cbb69209..a5c64c90e49e 100644
--- a/arch/x86/kvm/vmx/vmx_onhyperv.h
+++ b/arch/x86/kvm/vmx/vmx_onhyperv.h
@@ -119,6 +119,11 @@ static inline void evmcs_load(u64 phys_addr)
}
void evmcs_sanitize_exec_ctrls(struct vmcs_config *vmcs_conf);
+
+static inline bool vmx_hv_use_flush_guest_mapping(struct kvm_vcpu *vcpu)
+{
+ return vcpu->arch.hv_vmx_use_flush_guest_mapping;
+}
#else /* !IS_ENABLED(CONFIG_HYPERV) */
static __always_inline bool kvm_is_using_evmcs(void) { return false; }
static __always_inline void evmcs_write64(unsigned long field, u64 value) {}
@@ -128,6 +133,7 @@ static __always_inline u64 evmcs_read64(unsigned long field) { return 0; }
static __always_inline u32 evmcs_read32(unsigned long field) { return 0; }
static __always_inline u16 evmcs_read16(unsigned long field) { return 0; }
static inline void evmcs_load(u64 phys_addr) {}
+static inline bool vmx_hv_use_flush_guest_mapping(struct kvm_vcpu *vcpu) { return false; }
#endif /* IS_ENABLED(CONFIG_HYPERV) */
#endif /* __ARCH_X86_KVM_VMX_ONHYPERV_H__ */
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b58a74c1722d..cbde795096a6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -25,6 +25,7 @@
#include "tss.h"
#include "kvm_cache_regs.h"
#include "kvm_emulate.h"
+#include "kvm_onhyperv.h"
#include "mmu/page_track.h"
#include "x86.h"
#include "cpuid.h"
@@ -12390,6 +12391,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
#if IS_ENABLED(CONFIG_HYPERV)
vcpu->arch.hv_root_tdp = INVALID_PAGE;
+ vcpu->arch.hv_vmx_use_flush_guest_mapping =
+ (kvm_x86_ops.flush_remote_tlbs == hv_flush_remote_tlbs);
#endif
r = kvm_x86_call(vcpu_create)(vcpu);
--
2.39.5
^ permalink raw reply related
* [RFC PATCH 0/1] Tweak TLB flushing when VMX is running on Hyper-V
From: Jeremi Piotrowski @ 2025-06-20 15:39 UTC (permalink / raw)
To: Sean Christopherson, Vitaly Kuznetsov, Paolo Bonzini, kvm
Cc: Dave Hansen, linux-kernel, alanjiang, chinang.ma,
andrea.pellegrini, Kevin Tian, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, linux-hyperv, Jeremi Piotrowski
Hi Sean/Parolo/Vitaly,
Wanted to get your opinion on this change. Let me first introduce the scenario:
We have been testing kata containers (containers wrapped in VM) in Azure, and
found some significant issues with TLB flushing. This is a popular workload and
requires launching many nested VMs quickly. When testing on a 64 core Intel VM
(D64s_v5 in case someone is wondering), spinning up some 150-ish nested VMs in
parallel, performance starts getting worse the more nested VMs are already
running, CPU usage spikes to 100% on all cores and doesn't settle even when all
nested VMs boot up. On an idle system a single nested VMs boots within seconds,
but once we have a couple dozen running or so (doing nothing inside), boot time
gets longer and longer for each new nested VM, they start hitting startup
timeout etc. In some cases we never reach the point where all nested VMs are
up and running.
Investigating the issue we found that this can't be reproduced on AMD and on
Intel when EPT is disabled. In both these cases the scenario completes within
20s or so. TPD_MMU or not doesn't make a difference. With EPT=Y the case takes
minutes.Out of curiousity I also ran the test case on an n4-standard-64 VM on
GCP and found that EPT=Y runs in ~30s, while EPT=N runs in ~20s (which I found
slightly interesting).
So that's when we starting looking at the TLB flushing code and found that
INVEPT.global is used on every CPU migration and that it's an expensive
function on Hyper-V. It also has an impact on every running nested VM, so we
end up with lots of INVEPT.global calls - we reach 2000 calls/s before we're
essentially stuck in 100% guest ttime. That's why I'm looking at tweaking the
TLB flushing behavior to avoid it. I came across past discussions on this topic
([^1]) and after some thinking see two options:
1. Do you see a way to optimize this generically to avoid KVM_REQ_TLB_FLUSH on
migration in current KVM? In nested (as in: KVM running nested) I think we
rarely see CPU pinning used the way we it is on baremetal so it's not a rare of
an operation. Much has also changed since [^1] and with kvm_mmu_reset_context()
still being called in many paths we might be over flushing. Perhaps a loop
flushing individual roots with roles that do not have a post_set_xxx hook that
does flushing?
2. We can approach this in a Hyper-V specific way, using the dedicated flush
hypercall, which is what the following RFC patch does. This hypercall acts as a
broadcast INVEPT.single. I believe that using the flush hypercall in
flush_tlb_current() is sufficient to ensure the right semantics and correctness.
The one thing I haven't made up my mind about yet is whether we could still use
a flush of the current root on migration or not - I can imagine at most an
INVEPT.single, I also haven't yet figured out how that could be plumbed in if
it's really necessary (can't put it in KVM_REQ_TLB_FLUSH because that would
break the assumption that it is stronger than KVM_REQ_TLB_FLUSH_CURRENT).
With 2. the performance is comparable to EPT=N on Intel, roughly 20s for the
test scenario.
Let me know what you think about this and if you have any suggestions.
Best wishes,
Jeremi
[^1]: https://lore.kernel.org/kvm/YQljNBBp%2FEousNBk@google.com/
Jeremi Piotrowski (1):
KVM: VMX: Use Hyper-V EPT flush for local TLB flushes
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/vmx/vmx.c | 20 +++++++++++++++++---
arch/x86/kvm/vmx/vmx_onhyperv.h | 6 ++++++
arch/x86/kvm/x86.c | 3 +++
4 files changed, 27 insertions(+), 3 deletions(-)
--
2.39.5
^ permalink raw reply
* Re: [RFC Patch v2 0/4] x86/Hyper-V: Add AMD Secure AVIC for Hyper-V platform
From: Tianyu Lan @ 2025-06-20 15:05 UTC (permalink / raw)
To: Michael Kelley
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, kvijayab@amd.com, Neeraj.Upadhyay@amd.com,
Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <SN6PR02MB41579BCC56F6C966E3E2499CD47CA@SN6PR02MB4157.namprd02.prod.outlook.com>
On Fri, Jun 20, 2025 at 10:17 AM Michael Kelley <mhklinux@outlook.com> wrote:
>
> From: Tianyu Lan <ltykernel@gmail.com> Sent: Friday, June 13, 2025 4:08 AM
> >
> > Secure AVIC is a new hardware feature in the AMD64
> > architecture to allow SEV-SNP guests to prevent the
> > hypervisor from generating unexpected interrupts to
> > a vCPU or otherwise violate architectural assumptions
> > around APIC behavior.
> >
> > Each vCPU has a guest-allocated APIC backing page of
> > size 4K, which maintains APIC state for that vCPU.
> > APIC backing page's ALLOWED_IRR field indicates the
> > interrupt vectors which the guest allows the hypervisor
> > to send.
> >
> > This patchset is to enable the feature for Hyper-V
> > platform. Patch "Expose x2apic_savic_update_vector()"
> > is to expose new fucntion and device driver and arch
> > code may update AVIC backing page ALLOWED_IRR field to
> > allow Hyper-V inject associated vector.
>
> The last sentence above seems to be leftover from v1 of the
> patch set and is no longer accurate. Please update.
Thank you very much, Michael! Will update.
>
> Additional observation: These patches depend on
> CC_ATTR_SNP_SECURE_AVIC, which is not set when operating
> in VTOM mode (i.e., a paravisor is present). So evidently Linux
> on Hyper-V must handle the Secure AVIC only when Linux is
> running as the paravisor in VTL2 (CONFIG_HYPERV_VTL_MODE=y),
> or when running as an SEV-SNP guest with no paravisor. Is
> that correct?
This patchset enables Secure AVIC function for enlightened SEV-SNP guest
which uses c-bit to encrypt/decrypt guest memory.
--
Thanks
Tianyu Lan
^ permalink raw reply
* Re: [PATCH v4 1/1] tools/hv: Fix incorrect file path conversion in fcopy on Linux
From: Naman Jain @ 2025-06-20 10:09 UTC (permalink / raw)
To: yasuenag; +Cc: eahariha, kys, haiyangz, wei.liu, decui, linux-hyperv, ssengar
In-Reply-To: <20250613104648.1212-2-yasuenag@gmail.com>
On 6/13/2025 4:16 PM, yasuenag@gmail.com wrote:
> From: Yasumasa Suenaga <yasuenag@gmail.com>
>
> The hv_fcopy_uio_daemon fails to correctly handle file copy requests
> from Windows hosts (e.g. via Copy-VMFile) due to wchar_t size
> differences between Windows and Linux. On Linux, wchar_t is 32 bit,
> whereas Windows uses 16 bit wide characters.
>
> Currently, the code casts __u16 arrays directly to wchar_t* and
> uses wcstombs(), which leads to corrupted file paths or even crashes.
>
> This patch changes:
Please use imperative mood to write commit msg. Avoid using "This patch
does xyz".
> - Treats file name and path as __u16 arrays, not wchar_t*.
> - Allocates fixed-size buffers (W_MAX_PATH) for converted strings
> instead of using malloc.
> - Adds a check for target path length to prevent snprintf() buffer
> overflow.
>
> This change ensures file transfers from host to Linux guest succeed
> with correctly decoded file names and paths.
Ditto.
Sample commit msg:
The hv_fcopy_uio_daemon fails to correctly handle file copy requests
from Windows hosts (e.g. via Copy-VMFile) due to wchar_t size
differences between Windows and Linux. On Linux, wchar_t is 32 bit,
whereas Windows uses 16 bit wide characters.
Fix this by ensuring that file transfers from host to Linux guest
succeed with correctly decoded file names and paths.
Regards,
Naman
>
> Signed-off-by: Yasumasa Suenaga <yasuenag@gmail.com>
> ---
> tools/hv/hv_fcopy_uio_daemon.c | 37 +++++++++++++---------------------
> 1 file changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
> index 0198321d1..4b09ed6b6 100644
> --- a/tools/hv/hv_fcopy_uio_daemon.c
> +++ b/tools/hv/hv_fcopy_uio_daemon.c
> @@ -62,8 +62,11 @@ static int hv_fcopy_create_file(char *file_name, char *path_name, __u32 flags)
>
> filesize = 0;
> p = path_name;
> - snprintf(target_fname, sizeof(target_fname), "%s/%s",
> - path_name, file_name);
> + if (snprintf(target_fname, sizeof(target_fname), "%s/%s",
> + path_name, file_name) >= sizeof(target_fname)) {
> + syslog(LOG_ERR, "target file name is too long: %s/%s", path_name, file_name);
> + goto done;
> + }
>
> /*
> * Check to see if the path is already in place; if not,
> @@ -270,7 +273,7 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
> {
> size_t len = 0;
>
> - while (len < dest_size) {
> + while (len < dest_size && *src) {
> if (src[len] < 0x80)
> dest[len++] = (char)(*src++);
> else
> @@ -282,27 +285,15 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
>
> static int hv_fcopy_start(struct hv_start_fcopy *smsg_in)
> {
> - setlocale(LC_ALL, "en_US.utf8");
> - size_t file_size, path_size;
> - char *file_name, *path_name;
> - char *in_file_name = (char *)smsg_in->file_name;
> - char *in_path_name = (char *)smsg_in->path_name;
> -
> - file_size = wcstombs(NULL, (const wchar_t *restrict)in_file_name, 0) + 1;
> - path_size = wcstombs(NULL, (const wchar_t *restrict)in_path_name, 0) + 1;
> -
> - file_name = (char *)malloc(file_size * sizeof(char));
> - path_name = (char *)malloc(path_size * sizeof(char));
> -
> - if (!file_name || !path_name) {
> - free(file_name);
> - free(path_name);
> - syslog(LOG_ERR, "Can't allocate memory for file name and/or path name");
> - return HV_E_FAIL;
> - }
> + /*
> + * file_name and path_name should have same length with appropriate
> + * member of hv_start_fcopy.
> + */
> + char file_name[W_MAX_PATH], path_name[W_MAX_PATH];
>
> - wcstoutf8(file_name, (__u16 *)in_file_name, file_size);
> - wcstoutf8(path_name, (__u16 *)in_path_name, path_size);
> + setlocale(LC_ALL, "en_US.utf8");
> + wcstoutf8(file_name, smsg_in->file_name, W_MAX_PATH - 1);
> + wcstoutf8(path_name, smsg_in->path_name, W_MAX_PATH - 1);
>
> return hv_fcopy_create_file(file_name, path_name, smsg_in->copy_flags);
> }
^ permalink raw reply
* Re: [PATCH v4 2/2] vmbus: retrieve connection-id from DeviceTree
From: Krzysztof Kozlowski @ 2025-06-20 7:22 UTC (permalink / raw)
To: Hardik Garg
Cc: kys, haiyangz, wei.liu, decui, robh, krzk+dt, conor+dt,
devicetree, linux-hyperv, linux-kernel, ssengar, hargar, apais
In-Reply-To: <1750374395-14615-3-git-send-email-hargar@linux.microsoft.com>
On Thu, Jun 19, 2025 at 04:06:35PM GMT, Hardik Garg wrote:
> The connection-id determines which hypervisor communication channel the
> guest should use to talk to the VMBus host. This patch adds support to
> read this value from the DeviceTree where it exists as a property under
> the vmbus node with the compatible ID "microsoft,message-connection-id".
> The property name follows the format <vendor>,<field> where
> "vendor": "microsoft" and "field": "message-connection-id"
>
> Reading from DeviceTree allows platforms to specify their preferred
> communication channel, making it more flexible. If the property is
> not found in the DeviceTree, use the default connection ID
> (VMBUS_MESSAGE_CONNECTION_ID or VMBUS_MESSAGE_CONNECTION_ID_4
> based on protocol version).
>
> Signed-off-by: Hardik Garg <hargar@linux.microsoft.com>
> ---
> v3: https://lore.kernel.org/all/6a92ca86-ad6b-4d49-af6e-1ed7651b8ab8@linux.microsoft.com
> v2: https://lore.kernel.org/all/096edaf7-cc90-42b6-aff4-c5f088574e1e@linux.microsoft.com
> v1: https://lore.kernel.org/all/6acee4bf-cb04-43b9-9476-e8d811d26dfd@linux.microsoft.com
> ---
> drivers/hv/connection.c | 6 ++++--
> drivers/hv/vmbus_drv.c | 13 +++++++++++++
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index be490c598785..15d2b652783d 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -99,11 +99,13 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
> if (version >= VERSION_WIN10_V5) {
> msg->msg_sint = VMBUS_MESSAGE_SINT;
> msg->msg_vtl = ms_hyperv.vtl;
> - vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID_4;
> } else {
> msg->interrupt_page = virt_to_phys(vmbus_connection.int_page);
> - vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID;
> }
> + /* Set default connection ID if not provided via DeviceTree */
> + if (!vmbus_connection.msg_conn_id)
Your binding said connection ID 0 is a valid one, so this is wrong. Or
binding is wrong.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: microsoft: Add vmbus message-connection-id property
From: Krzysztof Kozlowski @ 2025-06-20 7:21 UTC (permalink / raw)
To: Hardik Garg
Cc: kys, haiyangz, wei.liu, decui, robh, krzk+dt, conor+dt,
devicetree, linux-hyperv, linux-kernel, ssengar, hargar, apais
In-Reply-To: <1750374395-14615-2-git-send-email-hargar@linux.microsoft.com>
On Thu, Jun 19, 2025 at 04:06:34PM GMT, Hardik Garg wrote:
> Document the microsoft,message-connection-id property for VMBus DeviceTree
> node. This property allows specifying the connection ID used for
What is a connection ID and why it cannot be inferred from existing
system API?
> communication between host and guest.
>
> Signed-off-by: Hardik Garg <hargar@linux.microsoft.com>
> ---
> v3: https://lore.kernel.org/all/6a92ca86-ad6b-4d49-af6e-1ed7651b8ab8@linux.microsoft.com
> v2: https://lore.kernel.org/all/096edaf7-cc90-42b6-aff4-c5f088574e1e@linux.microsoft.com
> v1: https://lore.kernel.org/all/6acee4bf-cb04-43b9-9476-e8d811d26dfd@linux.microsoft.com
> ---
> Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
> index 0bea4f5287ce..615b48bd6a8b 100644
> --- a/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
> +++ b/Documentation/devicetree/bindings/bus/microsoft,vmbus.yaml
> @@ -17,6 +17,14 @@ properties:
> compatible:
> const: microsoft,vmbus
There's a reason why you have here generic property - this is generic
and/or discoverable and/or whatever software interface. Adding now more
properties, just because you made it generic, is not the way.
>
> + microsoft,message-connection-id:
> + description: |
Drop |
> + VMBus message connection ID to be used for communication between host and
> + guest. If not specified, defaults to VMBUS_MESSAGE_CONNECTION_ID_4 (4) for
> + protocol version VERSION_WIN10_V5 and above, or VMBUS_MESSAGE_CONNECTION_ID
> + (1) for older versions.
Missing constraints, defaults, if this stays, but frankly speaking it
looks really not appropriate, considering lack of any explanation in the
binding or in commit msg.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH] tools/hv: fcopy: Fix irregularities with size of ring buffer
From: Naman Jain @ 2025-06-20 7:06 UTC (permalink / raw)
To: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Michael Kelley, Saurabh Sengar
Cc: linux-hyperv, linux-kernel, namjain, stable
Size of ring buffer, as defined in uio_hv_generic driver, is no longer
fixed to 16 KB. This creates a problem in fcopy, since this size was
hardcoded. With the change in place to make ring sysfs node actually
reflect the size of underlying ring buffer, it is safe to get the size
of ring sysfs file and use it for ring buffer size in fcopy daemon.
Fix the issue of disparity in ring buffer size, by making it dynamic
in fcopy uio daemon.
Cc: stable@vger.kernel.org
Fixes: 0315fef2aff9 ("uio_hv_generic: Align ring size to system page")
Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
---
tools/hv/hv_fcopy_uio_daemon.c | 65 ++++++++++++++++++++++++++++++----
1 file changed, 58 insertions(+), 7 deletions(-)
diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
index 0198321d14a2..da2b27d6af0e 100644
--- a/tools/hv/hv_fcopy_uio_daemon.c
+++ b/tools/hv/hv_fcopy_uio_daemon.c
@@ -36,6 +36,7 @@
#define WIN8_SRV_VERSION (WIN8_SRV_MAJOR << 16 | WIN8_SRV_MINOR)
#define FCOPY_UIO "/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/uio"
+#define FCOPY_CHANNELS_PATH "/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/channels"
#define FCOPY_VER_COUNT 1
static const int fcopy_versions[] = {
@@ -47,9 +48,51 @@ static const int fw_versions[] = {
UTIL_FW_VERSION
};
-#define HV_RING_SIZE 0x4000 /* 16KB ring buffer size */
+#define HV_RING_SIZE_DEFAULT 0x4000 /* 16KB ring buffer size default */
-static unsigned char desc[HV_RING_SIZE];
+static uint32_t get_ring_buffer_size(void)
+{
+ char ring_path[PATH_MAX];
+ DIR *dir;
+ struct dirent *entry;
+ struct stat st;
+ uint32_t ring_size = 0;
+
+ /* Find the channel directory */
+ dir = opendir(FCOPY_CHANNELS_PATH);
+ if (!dir) {
+ syslog(LOG_ERR, "Failed to open channels directory, using default ring size");
+ return HV_RING_SIZE_DEFAULT;
+ }
+
+ while ((entry = readdir(dir)) != NULL) {
+ if (entry->d_type == DT_DIR && strcmp(entry->d_name, ".") != 0 &&
+ strcmp(entry->d_name, "..") != 0) {
+ snprintf(ring_path, sizeof(ring_path), "%s/%s/ring",
+ FCOPY_CHANNELS_PATH, entry->d_name);
+
+ if (stat(ring_path, &st) == 0) {
+ /* stat returns size of Tx, Rx rings combined, so take half of it */
+ ring_size = (uint32_t)st.st_size / 2;
+ syslog(LOG_INFO, "Ring buffer size from %s: %u bytes",
+ ring_path, ring_size);
+ break;
+ }
+ }
+ }
+
+ closedir(dir);
+
+ if (!ring_size) {
+ ring_size = HV_RING_SIZE_DEFAULT;
+ syslog(LOG_ERR, "Could not determine ring size, using default: %u bytes",
+ HV_RING_SIZE_DEFAULT);
+ }
+
+ return ring_size;
+}
+
+static unsigned char *desc;
static int target_fd;
static char target_fname[PATH_MAX];
@@ -406,7 +449,8 @@ int main(int argc, char *argv[])
int daemonize = 1, long_index = 0, opt, ret = -EINVAL;
struct vmbus_br txbr, rxbr;
void *ring;
- uint32_t len = HV_RING_SIZE;
+ uint32_t ring_size = get_ring_buffer_size();
+ uint32_t len = ring_size;
char uio_name[NAME_MAX] = {0};
char uio_dev_path[PATH_MAX] = {0};
@@ -416,6 +460,13 @@ int main(int argc, char *argv[])
{0, 0, 0, 0 }
};
+ desc = (unsigned char *)malloc(ring_size * sizeof(unsigned char));
+ if (!desc) {
+ syslog(LOG_ERR, "malloc failed for desc buffer");
+ ret = -ENOMEM;
+ goto exit;
+ }
+
while ((opt = getopt_long(argc, argv, "hn", long_options,
&long_index)) != -1) {
switch (opt) {
@@ -448,14 +499,14 @@ int main(int argc, char *argv[])
goto exit;
}
- ring = vmbus_uio_map(&fcopy_fd, HV_RING_SIZE);
+ ring = vmbus_uio_map(&fcopy_fd, ring_size);
if (!ring) {
ret = errno;
syslog(LOG_ERR, "mmap ringbuffer failed; error: %d %s", ret, strerror(ret));
goto close;
}
- vmbus_br_setup(&txbr, ring, HV_RING_SIZE);
- vmbus_br_setup(&rxbr, (char *)ring + HV_RING_SIZE, HV_RING_SIZE);
+ vmbus_br_setup(&txbr, ring, ring_size);
+ vmbus_br_setup(&rxbr, (char *)ring + ring_size, ring_size);
rxbr.vbr->imask = 0;
@@ -472,7 +523,7 @@ int main(int argc, char *argv[])
goto close;
}
- len = HV_RING_SIZE;
+ len = ring_size;
ret = rte_vmbus_chan_recv_raw(&rxbr, desc, &len);
if (unlikely(ret <= 0)) {
/* This indicates a failure to communicate (or worse) */
base-commit: bc6e0ba6c9bafa6241b05524b9829808056ac4ad
--
2.34.1
^ permalink raw reply related
* RE: [PATCH v4 2/2] vmbus: retrieve connection-id from DeviceTree
From: Michael Kelley @ 2025-06-20 2:36 UTC (permalink / raw)
To: Hardik Garg, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, robh@kernel.org,
krzk+dt@kernel.org, conor+dt@kernel.org
Cc: devicetree@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, ssengar@linux.microsoft.com,
hargar@microsoft.com, apais@microsoft.com
In-Reply-To: <1750374395-14615-3-git-send-email-hargar@linux.microsoft.com>
From: Hardik Garg <hargar@linux.microsoft.com> Sent: Thursday, June 19, 2025 4:07 PM
>
For the patch "Subject:" line, prefer prefix "Drivers: hv: vmbus:" to be consistent
historical precedent. We haven't always been consistent with that precedent,
but I try to call it out when I have the opportunity. :-) If you look at the commit
log for drivers/hv, you'll see that prefix fairly often, though sometimes
shortened to just "Drivers: hv:" when the change is more generically for
Hyper-V and not specifically VMBus.
> The connection-id determines which hypervisor communication channel the
> guest should use to talk to the VMBus host. This patch adds support to
> read this value from the DeviceTree where it exists as a property under
> the vmbus node with the compatible ID "microsoft,message-connection-id".
Avoid wording like "this patch" in commit messages. Commit messages should
be in imperative mood. So something like:
The connection-id determines which hypervisor communication channel the
guest should use to talk to the VMBus host. Add steps to read this value from
the DeviceTree where it exists as a property under the vmbus node with the
compatible ID "microsoft,message-connection-id".
> The property name follows the format <vendor>,<field> where
> "vendor": "microsoft" and "field": "message-connection-id"
>
> Reading from DeviceTree allows platforms to specify their preferred
> communication channel, making it more flexible. If the property is
> not found in the DeviceTree, use the default connection ID
> (VMBUS_MESSAGE_CONNECTION_ID or VMBUS_MESSAGE_CONNECTION_ID_4
> based on protocol version).
>
> Signed-off-by: Hardik Garg <hargar@linux.microsoft.com>
> ---
> v3: https://lore.kernel.org/all/6a92ca86-ad6b-4d49-af6e-1ed7651b8ab8@linux.microsoft.com/
> v2: https://lore.kernel.org/all/096edaf7-cc90-42b6-aff4-c5f088574e1e@linux.microsoft.com/
> v1: https://lore.kernel.org/all/6acee4bf-cb04-43b9-9476-e8d811d26dfd@linux.microsoft.com/
> ---
> drivers/hv/connection.c | 6 ++++--
> drivers/hv/vmbus_drv.c | 13 +++++++++++++
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index be490c598785..15d2b652783d 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -99,11 +99,13 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
> if (version >= VERSION_WIN10_V5) {
> msg->msg_sint = VMBUS_MESSAGE_SINT;
> msg->msg_vtl = ms_hyperv.vtl;
> - vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID_4;
> } else {
> msg->interrupt_page = virt_to_phys(vmbus_connection.int_page);
> - vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID;
> }
> + /* Set default connection ID if not provided via DeviceTree */
> + if (!vmbus_connection.msg_conn_id)
> + vmbus_connection.msg_conn_id = (version >= VERSION_WIN10_V5) ?
> + VMBUS_MESSAGE_CONNECTION_ID_4 : VMBUS_MESSAGE_CONNECTION_ID;
>
> /*
> * shared_gpa_boundary is zero in non-SNP VMs, so it's safe to always
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index c236081d0a87..b78d5499e4bc 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -2541,10 +2541,23 @@ static int vmbus_device_add(struct platform_device *pdev)
> struct of_range range;
> struct of_range_parser parser;
> struct device_node *np = pdev->dev.of_node;
> + unsigned int conn_id;
> int ret;
>
> vmbus_root_device = &pdev->dev;
>
> + /*
> + * Read connection ID from DeviceTree. The property name follows the
> + * format <vendor>,<field> where:
> + * - vendor: "microsoft"
> + * - field: "message-connection-id"
> + */
> + ret = of_property_read_u32(np, "microsoft,message-connection-id", &conn_id);
> + if (!ret) {
> + pr_info("VMBus message connection ID: %u\n", conn_id);
> + vmbus_connection.msg_conn_id = conn_id;
Indentation problem here. Should be a full tab, not 4 spaces.
> + }
> +
> ret = of_range_parser_init(&parser, np);
> if (ret)
> return ret;
> --
> 2.40.4
>
^ permalink raw reply
* RE: [RFC Patch v2 4/4] x86/Hyper-V: Allow Hyper-V to inject Hyper-V vectors
From: Michael Kelley @ 2025-06-20 2:18 UTC (permalink / raw)
To: Tianyu Lan, 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, Neeraj.Upadhyay@amd.com
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20250613110829.122371-5-ltykernel@gmail.com>
From: Tianyu Lan <ltykernel@gmail.com> Sent: Friday, June 13, 2025 4:08 AM
>
> From: Tianyu Lan <tiala@microsoft.com>
Suggested patch Subject line:
x86/hyperv: Allow Hyper-V to inject STIMER0 interrupts
>
> When Secure AVIC is enabled, call Secure AVIC
> function to allow Hyper-V to inject STIMER0 interrupt.
>
> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
> ---
> arch/x86/hyperv/hv_init.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 3d1d3547095a..3b99fffb9ffd 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -132,6 +132,10 @@ static int hv_cpu_init(unsigned int cpu)
> wrmsrq(HV_X64_MSR_VP_ASSIST_PAGE, msr.as_uint64);
> }
>
> + /* Allow Hyper-V stimer vector to be injected from Hypervisor. */
> + if (ms_hyperv.misc_features & HV_STIMER_DIRECT_MODE_AVAILABLE)
> + apic_update_vector(cpu, HYPERV_STIMER0_VECTOR, true);
> +
Should hv_cpu_die() disable the vector so that there is symmetry?
> return hyperv_init_ghcb();
> }
>
> --
> 2.25.1
>
^ permalink raw reply
* RE: [RFC Patch v2 3/4] x86/Hyper-V: Not use auto-eoi when Secure AVIC is available
From: Michael Kelley @ 2025-06-20 2:17 UTC (permalink / raw)
To: Tianyu Lan, 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, kvijayab@amd.com,
Neeraj.Upadhyay@amd.com
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20250613110829.122371-4-ltykernel@gmail.com>
From: Tianyu Lan <ltykernel@gmail.com> Sent: Friday, June 13, 2025 4:08 AM
>
> From: Tianyu Lan <tiala@microsoft.com>
Suggested Subject line:
x86/hyperv: Don't use auto-eoi when Secure AVIC is available
>
> Hyper-V doesn't support auto-eoi with Secure AVIC.
> So Enable HV_DEPRECATING_AEOI_RECOMMENDED flag
> to force to write eoi register after handling
> interrupt.
Wording:
Hyper-V doesn't support auto-eoi with Secure AVIC.
So set the HV_DEPRECATING_AEOI_RECOMMENDED flag
to force writing the EIO register after handling an interrupt.
>
> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
> ---
> arch/x86/kernel/cpu/mshyperv.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index c78f860419d6..8f029650f16c 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -463,6 +463,8 @@ static void __init ms_hyperv_init_platform(void)
> ms_hyperv.max_vp_index, ms_hyperv.max_lp_index);
>
> hv_identify_partition_type();
> + if (cc_platform_has(CC_ATTR_SNP_SECURE_AVIC))
> + ms_hyperv.hints |= HV_DEPRECATING_AEOI_RECOMMENDED;
>
> if (ms_hyperv.hints & HV_X64_HYPERV_NESTED) {
> hv_nested = true;
> --
> 2.25.1
>
^ permalink raw reply
* RE: [RFC Patch v2 2/4] drivers/hv: Allow vmbus message synic interrupt injected from Hyper-V
From: Michael Kelley @ 2025-06-20 2:17 UTC (permalink / raw)
To: Tianyu Lan, 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, kvijayab@amd.com,
Neeraj.Upadhyay@amd.com
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20250613110829.122371-3-ltykernel@gmail.com>
From: Tianyu Lan <ltykernel@gmail.com> Sent: Friday, June 13, 2025 4:08 AM
>
> From: Tianyu Lan <tiala@microsoft.com>
For consistency with existing commits, use "Drivers: hv:" as the Subject prefix.
>
> When Secure AVIC is enabled, Vmbus driver should
s/Vmbus/VMBus/
> call x2apic Secure AVIC interface to allow Hyper-V
> to inject Vmbus message interrupt.
s/Vmbus/VMBus/
>
> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
> ---
> drivers/hv/hv.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 308c8f279df8..f78b46c51d69 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -20,6 +20,7 @@
> #include <linux/interrupt.h>
> #include <clocksource/hyperv_timer.h>
> #include <asm/mshyperv.h>
> +#include <asm/apic.h>
> #include <linux/set_memory.h>
> #include "hyperv_vmbus.h"
>
> @@ -310,6 +311,7 @@ void hv_synic_enable_regs(unsigned int cpu)
> if (vmbus_irq != -1)
> enable_percpu_irq(vmbus_irq, 0);
> shared_sint.as_uint64 = hv_get_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT);
> + apic_update_vector(smp_processor_id(), vmbus_interrupt, true);
hv_synic_enable_regs() has an input parameter "cpu". Use that instead
of smp_processor_id().
Also, apic_update_vector() is an x86/x64 only function. But
hv_synic_enable_regs() is built for ARM64 as well, so there will be
a compile error on ARM64. We've worked hard to avoid adding
#ifdef CONFIG_X86 or #ifdef CONFIG_ARM64 in this source code, so
I don't like the idea of adding #ifdef around the call to
apic_update_vector().
A possible approach would be to create a wrapper function such as
"hv_enable_coco_interrupt()" with the same function signature as
apic_update_vector(). hv_enable_coco_interrupt() would go i
arch/x86/hyperv/hv_apic.c and would call apic_update_vector().
Then also implement a __weak stub in hv_common.c that does nothing.
hv_common.c already has several such stubs as a pattern to follow.
The stub would allow hv_synic_enable_regs() to compile on ARM64
without having to add #ifdef's. And perhaps in the future CoCo VMs
on Hyper-V ARM64 would need their own implementation of
hv_enable_coco_interrupt(), though I'm not familiar enough with the
ARM64 CCA architecture to know for sure.
Since hv_synic_enable_regs() is enabling the vmbus_interrupt
vector, should hv_synic_disable_regs() disable the vector? It seems
like there should be symmetry unless there is a good reason
otherwise.
>
> shared_sint.vector = vmbus_interrupt;
> shared_sint.masked = false;
> --
> 2.25.1
>
^ permalink raw reply
* RE: [RFC Patch v2 1/4] x86/Hyper-V: Not use hv apic driver when Secure AVIC is available
From: Michael Kelley @ 2025-06-20 2:17 UTC (permalink / raw)
To: Tianyu Lan, 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, kvijayab@amd.com,
Neeraj.Upadhyay@amd.com
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20250613110829.122371-2-ltykernel@gmail.com>
From: Tianyu Lan <ltykernel@gmail.com> Sent: Friday, June 13, 2025 4:08 AM
>
> From: Tianyu Lan <tiala@microsoft.com>
The for the patch Subject line, use "x86/hyperv:" as the prefix. Also,
I would suggest better wording. So:
x86/hyperv: Don't use the Hyper-V APIC driver when Secure AVIC is available
>
> When Secure AVIC is available, AMD x2apic Secure
> AVIC driver should be selected and return directly
> in the hv_apic_init().
Better wording:
When Secure AVIC is available, the AMD x2apic Secure
AVIC driver will be selected. In that case, have hv_apic_init()
return immediately without doing anything.
>
> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
> ---
> arch/x86/hyperv/hv_apic.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> index bfde0a3498b9..1c48396e5389 100644
> --- a/arch/x86/hyperv/hv_apic.c
> +++ b/arch/x86/hyperv/hv_apic.c
> @@ -293,6 +293,9 @@ static void hv_send_ipi_self(int vector)
>
> void __init hv_apic_init(void)
> {
> + if (cc_platform_has(CC_ATTR_SNP_SECURE_AVIC))
> + return;
> +
> if (ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) {
> pr_info("Hyper-V: Using IPI hypercalls\n");
> /*
> --
> 2.25.1
>
^ permalink raw reply
* RE: [RFC Patch v2 0/4] x86/Hyper-V: Add AMD Secure AVIC for Hyper-V platform
From: Michael Kelley @ 2025-06-20 2:17 UTC (permalink / raw)
To: Tianyu Lan, 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, kvijayab@amd.com,
Neeraj.Upadhyay@amd.com
Cc: Tianyu Lan, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20250613110829.122371-1-ltykernel@gmail.com>
From: Tianyu Lan <ltykernel@gmail.com> Sent: Friday, June 13, 2025 4:08 AM
>
> Secure AVIC is a new hardware feature in the AMD64
> architecture to allow SEV-SNP guests to prevent the
> hypervisor from generating unexpected interrupts to
> a vCPU or otherwise violate architectural assumptions
> around APIC behavior.
>
> Each vCPU has a guest-allocated APIC backing page of
> size 4K, which maintains APIC state for that vCPU.
> APIC backing page's ALLOWED_IRR field indicates the
> interrupt vectors which the guest allows the hypervisor
> to send.
>
> This patchset is to enable the feature for Hyper-V
> platform. Patch "Expose x2apic_savic_update_vector()"
> is to expose new fucntion and device driver and arch
> code may update AVIC backing page ALLOWED_IRR field to
> allow Hyper-V inject associated vector.
The last sentence above seems to be leftover from v1 of the
patch set and is no longer accurate. Please update.
Additional observation: These patches depend on
CC_ATTR_SNP_SECURE_AVIC, which is not set when operating
in VTOM mode (i.e., a paravisor is present). So evidently Linux
on Hyper-V must handle the Secure AVIC only when Linux is
running as the paravisor in VTL2 (CONFIG_HYPERV_VTL_MODE=y),
or when running as an SEV-SNP guest with no paravisor. Is
that correct?
>
> This patchset is based on the AMD patchset "AMD: Add
> Secure AVIC Guest Support" https://lkml.org/lkml/2025/6/10/1579
>
> Change since v1:
> - Remove the check of Secure AVIC when set APIC backing page
> - Use apic_update_vector() instead of exposing new interface
> from Secure AVIC driver to update APIC backing page and allow
> associated interrupt to be injected by hypervisor.
>
> Tianyu Lan (4):
> x86/Hyper-V: Not use hv apic driver when Secure AVIC is available
> drivers/hv: Allow vmbus message synic interrupt injected from Hyper-V
> x86/Hyper-V: Not use auto-eoi when Secure AVIC is available
> x86/Hyper-V: Allow Hyper-V to inject Hyper-V vectors
>
> arch/x86/hyperv/hv_apic.c | 3 +++
> arch/x86/hyperv/hv_init.c | 4 ++++
> arch/x86/kernel/cpu/mshyperv.c | 2 ++
> drivers/hv/hv.c | 2 ++
> 4 files changed, 11 insertions(+)
>
> --
> 2.25.1
^ permalink raw reply
* Re: [Patch net-next v3] net: mana: Record doorbell physical address in PF mode
From: patchwork-bot+netdevbpf @ 2025-06-19 23:20 UTC (permalink / raw)
To: Long Li
Cc: kys, haiyangz, wei.liu, decui, davem, edumazet, kuba, pabeni,
shradhagupta, horms, kotaranov, schakrabarti, erick.archer,
linux-hyperv, netdev, linux-kernel, linux-rdma, longli
In-Reply-To: <1750210606-12167-1-git-send-email-longli@linuxonhyperv.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Tue, 17 Jun 2025 18:36:46 -0700 you wrote:
> From: Long Li <longli@microsoft.com>
>
> MANA supports RDMA in PF mode. The driver should record the doorbell
> physical address when in PF mode.
>
> The doorbell physical address is used by the RDMA driver to map
> doorbell pages of the device to user-mode applications through RDMA
> verbs interface. In the past, they have been mapped to user-mode while
> the device is in VF mode. With the support for PF mode implemented,
> also expose those pages in PF mode.
>
> [...]
Here is the summary with links:
- [net-next,v3] net: mana: Record doorbell physical address in PF mode
https://git.kernel.org/netdev/net/c/e0fca6f2cebf
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH v4 2/2] vmbus: retrieve connection-id from DeviceTree
From: Hardik Garg @ 2025-06-19 23:06 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, robh, krzk+dt, conor+dt
Cc: devicetree, linux-hyperv, linux-kernel, ssengar, hargar, apais,
Hardik Garg
In-Reply-To: <1750374395-14615-1-git-send-email-hargar@linux.microsoft.com>
The connection-id determines which hypervisor communication channel the
guest should use to talk to the VMBus host. This patch adds support to
read this value from the DeviceTree where it exists as a property under
the vmbus node with the compatible ID "microsoft,message-connection-id".
The property name follows the format <vendor>,<field> where
"vendor": "microsoft" and "field": "message-connection-id"
Reading from DeviceTree allows platforms to specify their preferred
communication channel, making it more flexible. If the property is
not found in the DeviceTree, use the default connection ID
(VMBUS_MESSAGE_CONNECTION_ID or VMBUS_MESSAGE_CONNECTION_ID_4
based on protocol version).
Signed-off-by: Hardik Garg <hargar@linux.microsoft.com>
---
v3: https://lore.kernel.org/all/6a92ca86-ad6b-4d49-af6e-1ed7651b8ab8@linux.microsoft.com
v2: https://lore.kernel.org/all/096edaf7-cc90-42b6-aff4-c5f088574e1e@linux.microsoft.com
v1: https://lore.kernel.org/all/6acee4bf-cb04-43b9-9476-e8d811d26dfd@linux.microsoft.com
---
drivers/hv/connection.c | 6 ++++--
drivers/hv/vmbus_drv.c | 13 +++++++++++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index be490c598785..15d2b652783d 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -99,11 +99,13 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
if (version >= VERSION_WIN10_V5) {
msg->msg_sint = VMBUS_MESSAGE_SINT;
msg->msg_vtl = ms_hyperv.vtl;
- vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID_4;
} else {
msg->interrupt_page = virt_to_phys(vmbus_connection.int_page);
- vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID;
}
+ /* Set default connection ID if not provided via DeviceTree */
+ if (!vmbus_connection.msg_conn_id)
+ vmbus_connection.msg_conn_id = (version >= VERSION_WIN10_V5) ?
+ VMBUS_MESSAGE_CONNECTION_ID_4 : VMBUS_MESSAGE_CONNECTION_ID;
/*
* shared_gpa_boundary is zero in non-SNP VMs, so it's safe to always
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index c236081d0a87..b78d5499e4bc 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2541,10 +2541,23 @@ static int vmbus_device_add(struct platform_device *pdev)
struct of_range range;
struct of_range_parser parser;
struct device_node *np = pdev->dev.of_node;
+ unsigned int conn_id;
int ret;
vmbus_root_device = &pdev->dev;
+ /*
+ * Read connection ID from DeviceTree. The property name follows the
+ * format <vendor>,<field> where:
+ * - vendor: "microsoft"
+ * - field: "message-connection-id"
+ */
+ ret = of_property_read_u32(np, "microsoft,message-connection-id", &conn_id);
+ if (!ret) {
+ pr_info("VMBus message connection ID: %u\n", conn_id);
+ vmbus_connection.msg_conn_id = conn_id;
+ }
+
ret = of_range_parser_init(&parser, np);
if (ret)
return ret;
--
2.40.4
^ 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