* [PATCH] Path string from the host should not be treated as wchar_t
From: yasuenag @ 2025-06-01 13:45 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui; +Cc: linux-hyperv, Yasumasa Suenaga
From: Yasumasa Suenaga <yasuenag@gmail.com>
hv_fcopy_uio_daemon handles file copy request from the host.
(e.g. Copy-VMFile commandlet)
The request has file path and its name, they would be stored as
__u16 arrays in struct hv_start_fcopy. They are casted to wchar_t*
in fcopyd to convert to UTF-8 string. wchar_t is 32bit in Linux
unlike Windows (16bit), so string conversion would be failed and
the user cannot copy file to Linux guest from Host via fcopyd.
fcopyd converts each characters to char if the value is less
than 0x80. Thus we can convert straightly without wcstombs() call,
it means we are no longer to convert to wchar_t.
Length of path depends on PATH_MAX (Linux) and W_MAX_PATH (Windows),
so this change also addes new check to snprintf() call to make
target path.
---
tools/hv/hv_fcopy_uio_daemon.c | 38 ++++++++++++++--------------------
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
index 0198321d1..049d4fd9c 100644
--- a/tools/hv/hv_fcopy_uio_daemon.c
+++ b/tools/hv/hv_fcopy_uio_daemon.c
@@ -58,12 +58,16 @@ static unsigned long long filesize;
static int hv_fcopy_create_file(char *file_name, char *path_name, __u32 flags)
{
int error = HV_E_FAIL;
+ int ret_snprintf;
char *q, *p;
filesize = 0;
p = path_name;
- snprintf(target_fname, sizeof(target_fname), "%s/%s",
- path_name, file_name);
+ ret_snprintf = snprintf(target_fname, sizeof(target_fname), "%s/%s",
+ path_name, file_name);
+ if (ret_snprintf >= sizeof(target_fname))
+ /* target file name is too long */
+ goto done;
/*
* Check to see if the path is already in place; if not,
@@ -273,6 +277,8 @@ static void wcstoutf8(char *dest, const __u16 *src, size_t dest_size)
while (len < dest_size) {
if (src[len] < 0x80)
dest[len++] = (char)(*src++);
+ else if (src[len] == '0')
+ break;
else
dest[len++] = 'X';
}
@@ -282,27 +288,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);
}
--
2.49.0
^ permalink raw reply related
* Re: [PATCH net,v3] hv_netvsc: fix potential deadlock in netvsc_vf_setxdp()
From: patchwork-bot+netdevbpf @ 2025-05-31 3:10 UTC (permalink / raw)
To: Saurabh Sengar
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, horms, ast, daniel, hawk, john.fastabend, sdf, kuniyu,
ahmed.zaki, aleksander.lobakin, linux-hyperv, netdev,
linux-kernel, bpf, ssengar, stable
In-Reply-To: <1748513910-23963-1-git-send-email-ssengar@linux.microsoft.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 29 May 2025 03:18:30 -0700 you wrote:
> The MANA driver's probe registers netdevice via the following call chain:
>
> mana_probe()
> register_netdev()
> register_netdevice()
>
> register_netdevice() calls notifier callback for netvsc driver,
> holding the netdev mutex via netdev_lock_ops().
>
> [...]
Here is the summary with links:
- [net,v3] hv_netvsc: fix potential deadlock in netvsc_vf_setxdp()
https://git.kernel.org/netdev/net/c/3ec523304976
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v4 0/5] Allow dyn MSI-X vector allocation of MANA
From: Simon Horman @ 2025-05-30 18:07 UTC (permalink / raw)
To: Shradha Gupta
Cc: linux-hyperv, linux-pci, linux-kernel, Nipun Gupta, Yury Norov,
Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen, Kevin Tian,
Long Li, Thomas Gleixner, Bjorn Helgaas, Rob Herring,
Manivannan Sadhasivam, Krzysztof Wilczy???~Dski,
Lorenzo Pieralisi, Dexuan Cui, Wei Liu, Haiyang Zhang,
K. Y. Srinivasan, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Konstantin Taranov, Leon Romanovsky,
Maxim Levitsky, Erni Sri Satya Vennela, Peter Zijlstra, netdev,
linux-rdma, Paul Rosswurm, Shradha Gupta
In-Reply-To: <20250529132845.GE27681@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
On Thu, May 29, 2025 at 06:28:45AM -0700, Shradha Gupta wrote:
> On Wed, May 28, 2025 at 07:55:08PM +0100, Simon Horman wrote:
> > On Tue, May 27, 2025 at 08:57:33AM -0700, Shradha Gupta wrote:
> > > In this patchset we want to enable the MANA driver to be able to
> > > allocate MSI-X vectors in PCI dynamically.
> > >
> > > The first patch exports pci_msix_prepare_desc() in PCI to be able to
> > > correctly prepare descriptors for dynamically added MSI-X vectors.
> > >
> > > The second patch adds the support of dynamic vector allocation in
> > > pci-hyperv PCI controller by enabling the MSI_FLAG_PCI_MSIX_ALLOC_DYN
> > > flag and using the pci_msix_prepare_desc() exported in first patch.
> > >
> > > The third patch adds a detailed description of the irq_setup(), to
> > > help understand the function design better.
> > >
> > > The fourth patch is a preparation patch for mana changes to support
> > > dynamic IRQ allocation. It contains changes in irq_setup() to allow
> > > skipping first sibling CPU sets, in case certain IRQs are already
> > > affinitized to them.
> > >
> > > The fifth patch has the changes in MANA driver to be able to allocate
> > > MSI-X vectors dynamically. If the support does not exist it defaults to
> > > older behavior.
> >
> > Hi Shradha,
> >
> > It's unclear what the target tree for this patch-set is.
> > But if it is net-next, which seems likely given the code under
> > drivers/net/, then:
> >
> > Please include that target in the subject of each patch in the patch-set.
> >
> > Subject: [PATCH v5 net-next 0/5] ...
> >
> > And, moreover, ...
> >
> > ## Form letter - net-next-closed
> >
> > The merge window for v6.16 has begun and therefore net-next is closed
> > for new drivers, features, code refactoring and optimizations. We are
> > currently accepting bug fixes only.
> >
> > Please repost when net-next reopens after June 8th.
> >
> > RFC patches sent for review only are obviously welcome at any time.
>
> Thank you Simon.
>
> While posting this patchset I was a bit confused about what should be
> the target tree. That's why in the cover letter of the V1 for this
> series, I had requested more clarity on the same (since there are patches
> from PCI and net-next both).
>
> In such cases how do we decide which tree to target?
Yes, that isn't entirely clear to me either.
Hopefully the maintainers can negotiate this.
>
> Also, noted about the next merge window for net-next :-)
>
> Regards,
> Shradha.
>
^ permalink raw reply
* Re: [PATCH v3 00/13] KVM: Make irqfd registration globally unique
From: K Prateek Nayak @ 2025-05-30 8:49 UTC (permalink / raw)
To: Sean Christopherson, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Juergen Gross, Stefano Stabellini, Paolo Bonzini,
Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Shuah Khan, Marc Zyngier, Oliver Upton
Cc: linux-kernel, linux-hyperv, xen-devel, kvm, linux-kselftest,
linux-arm-kernel, kvmarm, David Matlack
In-Reply-To: <20250522235223.3178519-1-seanjc@google.com>
Hello Sean,
On 5/23/2025 5:22 AM, Sean Christopherson wrote:
> Non-KVM folks,
>
> I am hoping to route this through the KVM tree (6.17 or later), as the non-KVM
> changes should be glorified nops. Please holler if you object to that idea.
I've tested this series with the selftests and also ran KVM unit test
on top of the specified base and didn't see anything unexpected. Feel
free to include:
Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>
--
Thanks and Regards,
Prateek
^ permalink raw reply
* Re: [PATCH v3 08/13] sched/wait: Add a waitqueue helper for fully exclusive priority waiters
From: K Prateek Nayak @ 2025-05-30 8:45 UTC (permalink / raw)
To: Sean Christopherson, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Juergen Gross, Stefano Stabellini, Paolo Bonzini,
Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Shuah Khan, Marc Zyngier, Oliver Upton
Cc: linux-kernel, linux-hyperv, xen-devel, kvm, linux-kselftest,
linux-arm-kernel, kvmarm, David Matlack
In-Reply-To: <20250522235223.3178519-9-seanjc@google.com>
Hello Sean,
On 5/23/2025 5:22 AM, Sean Christopherson wrote:
> Add a waitqueue helper to add a priority waiter that requires exclusive
> wakeups, i.e. that requires that it be the _only_ priority waiter. The
> API will be used by KVM to ensure that at most one of KVM's irqfds is
> bound to a single eventfd (across the entire kernel).
>
> Open code the helper instead of using __add_wait_queue() so that the
> common path doesn't need to "handle" impossible failures.
>
> Cc: K Prateek Nayak <kprateek.nayak@amd.com>
Feel free to include:
Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
--
Thanks and Regards,
Prateek
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> include/linux/wait.h | 2 ++
> kernel/sched/wait.c | 18 ++++++++++++++++++
> 2 files changed, 20 insertions(+)
> [..snip..]
^ permalink raw reply
* Re: [PATCH v3 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Stanislav Kinsburskii @ 2025-05-29 16:21 UTC (permalink / raw)
To: Naman Jain
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Roman Kisel, Anirudh Rayabharam, Saurabh Sengar, Nuno Das Neves,
ALOK TIWARI, linux-kernel, linux-hyperv
In-Reply-To: <e7763e8c-5f3a-4a37-a696-1b8492d92662@linux.microsoft.com>
On Fri, May 23, 2025 at 02:55:32PM +0530, Naman Jain wrote:
>
>
> On 5/22/2025 11:40 PM, Stanislav Kinsburskii wrote:
> > On Wed, May 21, 2025 at 11:33:29AM +0530, Naman Jain wrote:
> > >
> > >
> > > On 5/21/2025 12:25 AM, Stanislav Kinsburskii wrote:
> > > > On Mon, May 19, 2025 at 10:26:42AM +0530, Naman Jain wrote:
> > > > > Provide an interface for Virtual Machine Monitor like OpenVMM and its
> > > > > use as OpenHCL paravisor to control VTL0 (Virtual trust Level).
> > > > > Expose devices and support IOCTLs for features like VTL creation,
> > > > > VTL0 memory management, context switch, making hypercalls,
> > > > > mapping VTL0 address space to VTL2 userspace, getting new VMBus
> > > > > messages and channel events in VTL2 etc.
> > > > >
> > > > > Co-developed-by: Roman Kisel <romank@linux.microsoft.com>
> > > > > Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> > > > > Co-developed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> > > > > Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> > > > > Reviewed-by: Roman Kisel <romank@linux.microsoft.com>
> > > > > Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
> > > > > Message-ID: <20250512140432.2387503-3-namjain@linux.microsoft.com>
> > > > > Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> > > > > ---
> > > > > drivers/hv/Kconfig | 20 +
> > > > > drivers/hv/Makefile | 7 +-
> > > > > drivers/hv/mshv_vtl.h | 52 +
> > > > > drivers/hv/mshv_vtl_main.c | 1783 +++++++++++++++++++++++++++++++++++
> > > > > include/hyperv/hvgdk_mini.h | 81 ++
> > > > > include/hyperv/hvhdk.h | 1 +
> > > > > include/uapi/linux/mshv.h | 82 ++
> > > > > 7 files changed, 2025 insertions(+), 1 deletion(-)
> > > > > create mode 100644 drivers/hv/mshv_vtl.h
> > > > > create mode 100644 drivers/hv/mshv_vtl_main.c
> > > > >
> > > > > diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> > > > > index eefa0b559b73..21cee5564d70 100644
> > > > > --- a/drivers/hv/Kconfig
> > > > > +++ b/drivers/hv/Kconfig
> > > > > @@ -72,4 +72,24 @@ config MSHV_ROOT
> > > > > If unsure, say N.
> > > > > +config MSHV_VTL
> > > > > + tristate "Microsoft Hyper-V VTL driver"
> > > > > + depends on HYPERV && X86_64
> > > > > + depends on TRANSPARENT_HUGEPAGE
> > > >
> > > > Why does it depend on TRANSPARENT_HUGEPAGE?
> > > >
> > >
> >
> > Let me rephrase: can this driver work without transparent huge pages?
> > If yes, then it shouldn't depend on the option and rather select it.
> > If not, then it should be either fixed to be able to or have an
> > expalanation why this dependecy was introduced.
>
> No, it won't work. Reason being - we are adding support to map VTL0 address
> space to a user-mode process in VTL2. VTL2 for OpenHCL makes use of Huge
> pages to improve performance on VMs having large memory requirements. Thus,
> we need TRANSPARENT_HUGEPAGE.
>
> I will add this as a comment in Kconfig.
>
Then I'd suggest selecting huge pages instead of depending on them: it's
kinda weird that one need to select huge pages first to be able to
even find out, that there is some virtualization driver, depending on it.
Thanks,
Stanislav
^ permalink raw reply
* RE: [EXTERNAL] Re: [PATCH net-next,v6] net: mana: Add handler for hardware servicing events
From: Haiyang Zhang @ 2025-05-29 16:06 UTC (permalink / raw)
To: Paolo Abeni, linux-hyperv@vger.kernel.org, netdev@vger.kernel.org
Cc: Dexuan Cui, stephen@networkplumber.org, KY Srinivasan,
Paul Rosswurm, olaf@aepfle.de, vkuznets@redhat.com,
davem@davemloft.net, wei.liu@kernel.org, edumazet@google.com,
kuba@kernel.org, leon@kernel.org, Long Li,
ssengar@linux.microsoft.com, linux-rdma@vger.kernel.org,
daniel@iogearbox.net, john.fastabend@gmail.com,
bpf@vger.kernel.org, ast@kernel.org, hawk@kernel.org,
tglx@linutronix.de, shradhagupta@linux.microsoft.com,
andrew+netdev@lunn.ch, Konstantin Taranov, horms@kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <21c1b2d9-1b94-4caa-aa68-8abbb6562446@redhat.com>
> -----Original Message-----
> From: Paolo Abeni <pabeni@redhat.com>
> Sent: Thursday, May 29, 2025 2:41 AM
> To: Haiyang Zhang <haiyangz@microsoft.com>; linux-hyperv@vger.kernel.org;
> netdev@vger.kernel.org
> Cc: Dexuan Cui <decui@microsoft.com>; stephen@networkplumber.org; KY
> Srinivasan <kys@microsoft.com>; Paul Rosswurm <paulros@microsoft.com>;
> olaf@aepfle.de; vkuznets@redhat.com; davem@davemloft.net;
> wei.liu@kernel.org; edumazet@google.com; kuba@kernel.org; leon@kernel.org;
> Long Li <longli@microsoft.com>; ssengar@linux.microsoft.com; linux-
> rdma@vger.kernel.org; daniel@iogearbox.net; john.fastabend@gmail.com;
> bpf@vger.kernel.org; ast@kernel.org; hawk@kernel.org; tglx@linutronix.de;
> shradhagupta@linux.microsoft.com; andrew+netdev@lunn.ch; Konstantin
> Taranov <kotaranov@microsoft.com>; horms@kernel.org; linux-
> kernel@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH net-next,v6] net: mana: Add handler for
> hardware servicing events
>
> On 5/27/25 11:42 PM, Haiyang Zhang wrote:
> > To collaborate with hardware servicing events, upon receiving the
> special
> > EQE notification from the HW channel, remove the devices on this bus.
> > Then, after a waiting period based on the device specs, rescan the
> parent
> > bus to recover the devices.
> >
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Reviewed-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> > Reviewed-by: Simon Horman <horms@kernel.org>
>
> ## Form letter - net-next-closed
>
> The merge window for v6.16 has begun and therefore net-next is closed
> for new drivers, features, code refactoring and optimizations. We are
> currently accepting bug fixes only.
>
> Please repost when net-next reopens after June 8th.
>
> RFC patches sent for review only are obviously welcome at any time.
Will re-submit when it reopens.
Thanks,
- Haiyang
^ permalink raw reply
* Re: [PATCH v4 0/5] Allow dyn MSI-X vector allocation of MANA
From: Shradha Gupta @ 2025-05-29 13:28 UTC (permalink / raw)
To: Simon Horman
Cc: linux-hyperv, linux-pci, linux-kernel, Nipun Gupta, Yury Norov,
Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen, Kevin Tian,
Long Li, Thomas Gleixner, Bjorn Helgaas, Rob Herring,
Manivannan Sadhasivam, Krzysztof Wilczy???~Dski,
Lorenzo Pieralisi, Dexuan Cui, Wei Liu, Haiyang Zhang,
K. Y. Srinivasan, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Konstantin Taranov, Leon Romanovsky,
Maxim Levitsky, Erni Sri Satya Vennela, Peter Zijlstra, netdev,
linux-rdma, Paul Rosswurm, Shradha Gupta
In-Reply-To: <20250528185508.GK1484967@horms.kernel.org>
On Wed, May 28, 2025 at 07:55:08PM +0100, Simon Horman wrote:
> On Tue, May 27, 2025 at 08:57:33AM -0700, Shradha Gupta wrote:
> > In this patchset we want to enable the MANA driver to be able to
> > allocate MSI-X vectors in PCI dynamically.
> >
> > The first patch exports pci_msix_prepare_desc() in PCI to be able to
> > correctly prepare descriptors for dynamically added MSI-X vectors.
> >
> > The second patch adds the support of dynamic vector allocation in
> > pci-hyperv PCI controller by enabling the MSI_FLAG_PCI_MSIX_ALLOC_DYN
> > flag and using the pci_msix_prepare_desc() exported in first patch.
> >
> > The third patch adds a detailed description of the irq_setup(), to
> > help understand the function design better.
> >
> > The fourth patch is a preparation patch for mana changes to support
> > dynamic IRQ allocation. It contains changes in irq_setup() to allow
> > skipping first sibling CPU sets, in case certain IRQs are already
> > affinitized to them.
> >
> > The fifth patch has the changes in MANA driver to be able to allocate
> > MSI-X vectors dynamically. If the support does not exist it defaults to
> > older behavior.
>
> Hi Shradha,
>
> It's unclear what the target tree for this patch-set is.
> But if it is net-next, which seems likely given the code under
> drivers/net/, then:
>
> Please include that target in the subject of each patch in the patch-set.
>
> Subject: [PATCH v5 net-next 0/5] ...
>
> And, moreover, ...
>
> ## Form letter - net-next-closed
>
> The merge window for v6.16 has begun and therefore net-next is closed
> for new drivers, features, code refactoring and optimizations. We are
> currently accepting bug fixes only.
>
> Please repost when net-next reopens after June 8th.
>
> RFC patches sent for review only are obviously welcome at any time.
Thank you Simon.
While posting this patchset I was a bit confused about what should be
the target tree. That's why in the cover letter of the V1 for this
series, I had requested more clarity on the same (since there are patches
from PCI and net-next both).
In such cases how do we decide which tree to target?
Also, noted about the next merge window for net-next :-)
Regards,
Shradha.
^ permalink raw reply
* Re: [PATCH v4 5/5] net: mana: Allocate MSI-X vectors dynamically
From: Shradha Gupta @ 2025-05-29 13:20 UTC (permalink / raw)
To: Saurabh Singh Sengar
Cc: Dexuan Cui, Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Konstantin Taranov, Simon Horman, Leon Romanovsky, Maxim Levitsky,
Erni Sri Satya Vennela, Peter Zijlstra, Michael Kelley,
linux-hyperv, linux-pci, linux-kernel, Nipun Gupta, Yury Norov,
Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen, Kevin Tian,
Long Li, Thomas Gleixner, Bjorn Helgaas, Rob Herring,
Manivannan Sadhasivam, Krzysztof Wilczy???~Dski,
Lorenzo Pieralisi, netdev, linux-rdma, Paul Rosswurm,
Shradha Gupta
In-Reply-To: <20250529034520.GA5898@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
On Wed, May 28, 2025 at 08:45:20PM -0700, Saurabh Singh Sengar wrote:
> On Tue, May 27, 2025 at 08:59:03AM -0700, Shradha Gupta wrote:
> > Currently, the MANA driver allocates MSI-X vectors statically based on
> > MANA_MAX_NUM_QUEUES and num_online_cpus() values and in some cases ends
> > up allocating more vectors than it needs. This is because, by this time
> > we do not have a HW channel and do not know how many IRQs should be
> > allocated.
> >
> > To avoid this, we allocate 1 MSI-X vector during the creation of HWC and
> > after getting the value supported by hardware, dynamically add the
> > remaining MSI-X vectors.
> >
> > Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> > Changes in v4:
> > * added BUG_ON at appropriate places
> > * moved xa_destroy to mana_gd_remove()
> > * rearragned the cleanup logic in mana_gd_setup_dyn_irqs()
> > * simplified processing around start_irq_index in mana_gd_setup_irqs()
> > * return 0 instead of return err as appropriate
> > ---
> > Changes in v3:
> > * implemented irq_contexts as xarrays rather than list
> > * split the patch to create a perparation patch around irq_setup()
> > * add log when IRQ allocation/setup for remaining IRQs fails
> > ---
> > Changes in v2:
> > * Use string 'MSI-X vectors' instead of 'pci vectors'
> > * make skip-cpu a bool instead of int
> > * rearrange the comment arout skip_cpu variable appropriately
> > * update the capability bit for driver indicating dynamic IRQ allocation
> > * enforced max line length to 80
> > * enforced RCT convention
> > * initialized gic to NULL, for when there is a possibility of gic
> > not being populated correctly
> > ---
> > .../net/ethernet/microsoft/mana/gdma_main.c | 306 +++++++++++++-----
> > include/net/mana/gdma.h | 8 +-
> > 2 files changed, 235 insertions(+), 79 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > index 763a548c4a2b..98ebecbec9a7 100644
> > --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > @@ -6,6 +6,8 @@
> > #include <linux/pci.h>
> > #include <linux/utsname.h>
> > #include <linux/version.h>
> > +#include <linux/msi.h>
> > +#include <linux/irqdomain.h>
> >
> > #include <net/mana/mana.h>
> >
> > @@ -80,8 +82,15 @@ static int mana_gd_query_max_resources(struct pci_dev *pdev)
> > return err ? err : -EPROTO;
> > }
> >
> > - if (gc->num_msix_usable > resp.max_msix)
> > - gc->num_msix_usable = resp.max_msix;
> > + if (!pci_msix_can_alloc_dyn(pdev)) {
> > + if (gc->num_msix_usable > resp.max_msix)
> > + gc->num_msix_usable = resp.max_msix;
> > + } else {
> > + /* If dynamic allocation is enabled we have already allocated
> > + * hwc msi
> > + */
> > + gc->num_msix_usable = min(resp.max_msix, num_online_cpus() + 1);
> > + }
> >
> > if (gc->num_msix_usable <= 1)
> > return -ENOSPC;
> > @@ -482,7 +491,9 @@ static int mana_gd_register_irq(struct gdma_queue *queue,
> > }
> >
> > queue->eq.msix_index = msi_index;
> > - gic = &gc->irq_contexts[msi_index];
> > + gic = xa_load(&gc->irq_contexts, msi_index);
> > + if (WARN_ON(!gic))
> > + return -EINVAL;
> >
> > spin_lock_irqsave(&gic->lock, flags);
> > list_add_rcu(&queue->entry, &gic->eq_list);
> > @@ -507,7 +518,10 @@ static void mana_gd_deregiser_irq(struct gdma_queue *queue)
> > if (WARN_ON(msix_index >= gc->num_msix_usable))
> > return;
> >
> > - gic = &gc->irq_contexts[msix_index];
> > + gic = xa_load(&gc->irq_contexts, msix_index);
> > + if (WARN_ON(!gic))
> > + return;
> > +
> > spin_lock_irqsave(&gic->lock, flags);
> > list_for_each_entry_rcu(eq, &gic->eq_list, entry) {
> > if (queue == eq) {
> > @@ -1366,47 +1380,113 @@ static int irq_setup(unsigned int *irqs, unsigned int len, int node,
> > return 0;
> > }
> >
> > -static int mana_gd_setup_irqs(struct pci_dev *pdev)
> > +static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> > {
> > struct gdma_context *gc = pci_get_drvdata(pdev);
> > - unsigned int max_queues_per_port;
> > struct gdma_irq_context *gic;
> > - unsigned int max_irqs, cpu;
> > - int start_irq_index = 1;
> > - int nvec, *irqs, irq;
> > - int err, i = 0, j;
> > + bool skip_first_cpu = false;
> > + int *irqs, irq, err, i;
> >
> > cpus_read_lock();
>
> Now that num_online_cpus is moved further down in this new logic,
> do we want to reduce the critical section ?
>
> I don't think we want kmalloc_array to be protected.
>
>
> > - max_queues_per_port = num_online_cpus();
> > - if (max_queues_per_port > MANA_MAX_NUM_QUEUES)
> > - max_queues_per_port = MANA_MAX_NUM_QUEUES;
> >
> > - /* Need 1 interrupt for the Hardware communication Channel (HWC) */
> > - max_irqs = max_queues_per_port + 1;
> > -
> > - nvec = pci_alloc_irq_vectors(pdev, 2, max_irqs, PCI_IRQ_MSIX);
> > - if (nvec < 0) {
> > - cpus_read_unlock();
> > - return nvec;
> > - }
> > - if (nvec <= num_online_cpus())
> > - start_irq_index = 0;
> > -
> > - irqs = kmalloc_array((nvec - start_irq_index), sizeof(int), GFP_KERNEL);
> > + irqs = kmalloc_array(nvec, sizeof(int), GFP_KERNEL);
> > if (!irqs) {
> > err = -ENOMEM;
> > goto free_irq_vector;
> > }
> >
> > - gc->irq_contexts = kcalloc(nvec, sizeof(struct gdma_irq_context),
> > - GFP_KERNEL);
> > - if (!gc->irq_contexts) {
> > + /*
> > + * While processing the next pci irq vector, we start with index 1,
> > + * as IRQ vector at index 0 is already processed for HWC.
> > + * However, the population of irqs array starts with index 0, to be
> > + * further used in irq_setup()
> > + */
> > + for (i = 1; i <= nvec; i++) {
> > + gic = kzalloc(sizeof(*gic), GFP_KERNEL);
> > + if (!gic) {
> > + err = -ENOMEM;
> > + goto free_irq;
> > + }
> > + gic->handler = mana_gd_process_eq_events;
> > + INIT_LIST_HEAD(&gic->eq_list);
> > + spin_lock_init(&gic->lock);
> > +
> > + snprintf(gic->name, MANA_IRQ_NAME_SZ, "mana_q%d@pci:%s",
> > + i - 1, pci_name(pdev));
> > +
> > + /* one pci vector is already allocated for HWC */
> > + irqs[i - 1] = pci_irq_vector(pdev, i);
> > + if (irqs[i - 1] < 0) {
> > + err = irqs[i - 1];
> > + goto free_current_gic;
> > + }
> > +
> > + err = request_irq(irqs[i - 1], mana_gd_intr, 0, gic->name, gic);
> > + if (err)
> > + goto free_current_gic;
> > +
> > + xa_store(&gc->irq_contexts, i, gic, GFP_KERNEL);
> > + }
> > +
> > + /*
> > + * When calling irq_setup() for dynamically added IRQs, if number of
> > + * CPUs is more than or equal to allocated MSI-X, we need to skip the
> > + * first CPU sibling group since they are already affinitized to HWC IRQ
> > + */
> > + if (gc->num_msix_usable <= num_online_cpus())
> > + skip_first_cpu = true;
> > +
> > + err = irq_setup(irqs, nvec, gc->numa_node, skip_first_cpu);
> > + if (err)
> > + goto free_irq;
> > +
> > + cpus_read_unlock();
> > + kfree(irqs);
> > + return 0;
> > +
> > +free_current_gic:
> > + kfree(gic);
> > +free_irq:
> > + for (i -= 1; i > 0; i--) {
> > + irq = pci_irq_vector(pdev, i);
> > + gic = xa_load(&gc->irq_contexts, i);
> > + if (WARN_ON(!gic))
> > + continue;
> > +
> > + irq_update_affinity_hint(irq, NULL);
> > + free_irq(irq, gic);
> > + xa_erase(&gc->irq_contexts, i);
> > + kfree(gic);
> > + }
> > + kfree(irqs);
> > +free_irq_vector:
> > + cpus_read_unlock();
> > + return err;
> > +}
> > +
> > +static int mana_gd_setup_irqs(struct pci_dev *pdev, int nvec)
> > +{
> > + struct gdma_context *gc = pci_get_drvdata(pdev);
> > + struct gdma_irq_context *gic;
> > + int *irqs, *start_irqs, irq;
> > + unsigned int cpu;
> > + int err, i;
> > +
> > + cpus_read_lock();
>
> Same here
>
> > + irqs = kmalloc_array(nvec, sizeof(int), GFP_KERNEL);
> > + if (!irqs) {
> > err = -ENOMEM;
> > - goto free_irq_array;
> > + goto free_irq_vector;
> > }
> >
> > for (i = 0; i < nvec; i++) {
> > - gic = &gc->irq_contexts[i];
> > + gic = kzalloc(sizeof(*gic), GFP_KERNEL);
> > + if (!gic) {
> > + err = -ENOMEM;
> > + goto free_irq;
> > + }
> > +
> > gic->handler = mana_gd_process_eq_events;
> > INIT_LIST_HEAD(&gic->eq_list);
> > spin_lock_init(&gic->lock);
>
> <snip>
>
> - Saurabh
Thanks Saurabh, will get these too
^ permalink raw reply
* Re: [PATCH v4 5/5] net: mana: Allocate MSI-X vectors dynamically
From: Shradha Gupta @ 2025-05-29 13:18 UTC (permalink / raw)
To: Simon Horman
Cc: Dexuan Cui, Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Konstantin Taranov, Leon Romanovsky, Maxim Levitsky,
Erni Sri Satya Vennela, Peter Zijlstra, Michael Kelley,
linux-hyperv, linux-pci, linux-kernel, Nipun Gupta, Yury Norov,
Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen, Kevin Tian,
Long Li, Thomas Gleixner, Bjorn Helgaas, Rob Herring,
Manivannan Sadhasivam, Krzysztof Wilczy???~Dski,
Lorenzo Pieralisi, netdev, linux-rdma, Paul Rosswurm,
Shradha Gupta
In-Reply-To: <20250528185235.GJ1484967@horms.kernel.org>
On Wed, May 28, 2025 at 07:52:35PM +0100, Simon Horman wrote:
> On Tue, May 27, 2025 at 08:59:03AM -0700, Shradha Gupta wrote:
> > Currently, the MANA driver allocates MSI-X vectors statically based on
> > MANA_MAX_NUM_QUEUES and num_online_cpus() values and in some cases ends
> > up allocating more vectors than it needs. This is because, by this time
> > we do not have a HW channel and do not know how many IRQs should be
> > allocated.
> >
> > To avoid this, we allocate 1 MSI-X vector during the creation of HWC and
> > after getting the value supported by hardware, dynamically add the
> > remaining MSI-X vectors.
> >
> > Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
>
> ...
>
> > +static int mana_gd_setup_irqs(struct pci_dev *pdev, int nvec)
> > +{
> > + struct gdma_context *gc = pci_get_drvdata(pdev);
> > + struct gdma_irq_context *gic;
> > + int *irqs, *start_irqs, irq;
> > + unsigned int cpu;
> > + int err, i;
> > +
> > + cpus_read_lock();
> > +
> > + irqs = kmalloc_array(nvec, sizeof(int), GFP_KERNEL);
> > + if (!irqs) {
> > err = -ENOMEM;
> > - goto free_irq_array;
> > + goto free_irq_vector;
> > }
> >
> > for (i = 0; i < nvec; i++) {
> > - gic = &gc->irq_contexts[i];
> > + gic = kzalloc(sizeof(*gic), GFP_KERNEL);
> > + if (!gic) {
> > + err = -ENOMEM;
> > + goto free_irq;
> > + }
> > +
> > gic->handler = mana_gd_process_eq_events;
> > INIT_LIST_HEAD(&gic->eq_list);
> > spin_lock_init(&gic->lock);
> > @@ -1418,69 +1498,128 @@ static int mana_gd_setup_irqs(struct pci_dev *pdev)
> > snprintf(gic->name, MANA_IRQ_NAME_SZ, "mana_q%d@pci:%s",
> > i - 1, pci_name(pdev));
> >
> > - irq = pci_irq_vector(pdev, i);
> > - if (irq < 0) {
> > - err = irq;
> > - goto free_irq;
> > + irqs[i] = pci_irq_vector(pdev, i);
> > + if (irqs[i] < 0) {
> > + err = irqs[i];
> > + goto free_current_gic;
> > }
> >
> > - if (!i) {
> > - err = request_irq(irq, mana_gd_intr, 0, gic->name, gic);
> > - if (err)
> > - goto free_irq;
> > -
> > - /* If number of IRQ is one extra than number of online CPUs,
> > - * then we need to assign IRQ0 (hwc irq) and IRQ1 to
> > - * same CPU.
> > - * Else we will use different CPUs for IRQ0 and IRQ1.
> > - * Also we are using cpumask_local_spread instead of
> > - * cpumask_first for the node, because the node can be
> > - * mem only.
> > - */
> > - if (start_irq_index) {
> > - cpu = cpumask_local_spread(i, gc->numa_node);
> > - irq_set_affinity_and_hint(irq, cpumask_of(cpu));
> > - } else {
> > - irqs[start_irq_index] = irq;
> > - }
> > - } else {
> > - irqs[i - start_irq_index] = irq;
> > - err = request_irq(irqs[i - start_irq_index], mana_gd_intr, 0,
> > - gic->name, gic);
> > - if (err)
> > - goto free_irq;
> > - }
> > + err = request_irq(irqs[i], mana_gd_intr, 0, gic->name, gic);
> > + if (err)
> > + goto free_current_gic;
>
> Jumping to free_current_gic will free start_irqs.
> However, start_irqs isn't initialised until a few lines below.
>
> Flagged by Smatch.
>
Thanks Simon, I'll get this in next version
> > +
> > + xa_store(&gc->irq_contexts, i, gic, GFP_KERNEL);
> > }
> >
> > - err = irq_setup(irqs, nvec - start_irq_index, gc->numa_node, false);
> > + /* If number of IRQ is one extra than number of online CPUs,
> > + * then we need to assign IRQ0 (hwc irq) and IRQ1 to
> > + * same CPU.
> > + * Else we will use different CPUs for IRQ0 and IRQ1.
> > + * Also we are using cpumask_local_spread instead of
> > + * cpumask_first for the node, because the node can be
> > + * mem only.
> > + */
> > + start_irqs = irqs;
> > + if (nvec > num_online_cpus()) {
> > + cpu = cpumask_local_spread(0, gc->numa_node);
> > + irq_set_affinity_and_hint(irqs[0], cpumask_of(cpu));
> > + irqs++;
> > + nvec -= 1;
> > + }
> > +
> > + err = irq_setup(irqs, nvec, gc->numa_node, false);
> > if (err)
> > goto free_irq;
> >
> > - gc->max_num_msix = nvec;
> > - gc->num_msix_usable = nvec;
> > cpus_read_unlock();
> > - kfree(irqs);
> > + kfree(start_irqs);
> > return 0;
> >
> > +free_current_gic:
> > + kfree(gic);
> > free_irq:
> > - for (j = i - 1; j >= 0; j--) {
> > - irq = pci_irq_vector(pdev, j);
> > - gic = &gc->irq_contexts[j];
> > + for (i -= 1; i >= 0; i--) {
> > + irq = pci_irq_vector(pdev, i);
> > + gic = xa_load(&gc->irq_contexts, i);
> > + if (WARN_ON(!gic))
> > + continue;
> >
> > irq_update_affinity_hint(irq, NULL);
> > free_irq(irq, gic);
> > + xa_erase(&gc->irq_contexts, i);
> > + kfree(gic);
> > }
> >
> > - kfree(gc->irq_contexts);
> > - gc->irq_contexts = NULL;
> > -free_irq_array:
> > - kfree(irqs);
> > + kfree(start_irqs);
> > free_irq_vector:
> > cpus_read_unlock();
> > - pci_free_irq_vectors(pdev);
> > return err;
> > }
>
> ...
^ permalink raw reply
* Re: [PATCH v4 5/5] net: mana: Allocate MSI-X vectors dynamically
From: Shradha Gupta @ 2025-05-29 13:17 UTC (permalink / raw)
To: Saurabh Singh Sengar
Cc: Dexuan Cui, Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Konstantin Taranov, Simon Horman, Leon Romanovsky, Maxim Levitsky,
Erni Sri Satya Vennela, Peter Zijlstra, Michael Kelley,
linux-hyperv, linux-pci, linux-kernel, Nipun Gupta, Yury Norov,
Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen, Kevin Tian,
Long Li, Thomas Gleixner, Bjorn Helgaas, Rob Herring,
Manivannan Sadhasivam, Krzysztof Wilczy???~Dski,
Lorenzo Pieralisi, netdev, linux-rdma, Paul Rosswurm,
Shradha Gupta
In-Reply-To: <20250528081638.GA19010@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
On Wed, May 28, 2025 at 01:16:38AM -0700, Saurabh Singh Sengar wrote:
> On Tue, May 27, 2025 at 08:59:03AM -0700, Shradha Gupta wrote:
> > Currently, the MANA driver allocates MSI-X vectors statically based on
> > MANA_MAX_NUM_QUEUES and num_online_cpus() values and in some cases ends
> > up allocating more vectors than it needs. This is because, by this time
> > we do not have a HW channel and do not know how many IRQs should be
> > allocated.
> >
> > To avoid this, we allocate 1 MSI-X vector during the creation of HWC and
> > after getting the value supported by hardware, dynamically add the
> > remaining MSI-X vectors.
> >
> > Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> > Changes in v4:
> > * added BUG_ON at appropriate places
> > * moved xa_destroy to mana_gd_remove()
> > * rearragned the cleanup logic in mana_gd_setup_dyn_irqs()
> > * simplified processing around start_irq_index in mana_gd_setup_irqs()
> > * return 0 instead of return err as appropriate
> > ---
> > Changes in v3:
> > * implemented irq_contexts as xarrays rather than list
> > * split the patch to create a perparation patch around irq_setup()
> > * add log when IRQ allocation/setup for remaining IRQs fails
> > ---
> > Changes in v2:
> > * Use string 'MSI-X vectors' instead of 'pci vectors'
> > * make skip-cpu a bool instead of int
> > * rearrange the comment arout skip_cpu variable appropriately
> > * update the capability bit for driver indicating dynamic IRQ allocation
> > * enforced max line length to 80
> > * enforced RCT convention
> > * initialized gic to NULL, for when there is a possibility of gic
> > not being populated correctly
> > ---
> > .../net/ethernet/microsoft/mana/gdma_main.c | 306 +++++++++++++-----
> > include/net/mana/gdma.h | 8 +-
> > 2 files changed, 235 insertions(+), 79 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > index 763a548c4a2b..98ebecbec9a7 100644
> > --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > @@ -6,6 +6,8 @@
> > #include <linux/pci.h>
> > #include <linux/utsname.h>
> > #include <linux/version.h>
> > +#include <linux/msi.h>
> > +#include <linux/irqdomain.h>
> >
> > #include <net/mana/mana.h>
> >
> > @@ -80,8 +82,15 @@ static int mana_gd_query_max_resources(struct pci_dev *pdev)
> > return err ? err : -EPROTO;
> > }
> >
> > - if (gc->num_msix_usable > resp.max_msix)
> > - gc->num_msix_usable = resp.max_msix;
> > + if (!pci_msix_can_alloc_dyn(pdev)) {
> > + if (gc->num_msix_usable > resp.max_msix)
> > + gc->num_msix_usable = resp.max_msix;
> > + } else {
> > + /* If dynamic allocation is enabled we have already allocated
> > + * hwc msi
> > + */
> > + gc->num_msix_usable = min(resp.max_msix, num_online_cpus() + 1);
> > + }
> >
> > if (gc->num_msix_usable <= 1)
> > return -ENOSPC;
> > @@ -482,7 +491,9 @@ static int mana_gd_register_irq(struct gdma_queue *queue,
> > }
> >
> > queue->eq.msix_index = msi_index;
> > - gic = &gc->irq_contexts[msi_index];
> > + gic = xa_load(&gc->irq_contexts, msi_index);
> > + if (WARN_ON(!gic))
> > + return -EINVAL;
> >
> > spin_lock_irqsave(&gic->lock, flags);
> > list_add_rcu(&queue->entry, &gic->eq_list);
> > @@ -507,7 +518,10 @@ static void mana_gd_deregiser_irq(struct gdma_queue *queue)
> > if (WARN_ON(msix_index >= gc->num_msix_usable))
> > return;
> >
> > - gic = &gc->irq_contexts[msix_index];
> > + gic = xa_load(&gc->irq_contexts, msix_index);
> > + if (WARN_ON(!gic))
> > + return;
> > +
> > spin_lock_irqsave(&gic->lock, flags);
> > list_for_each_entry_rcu(eq, &gic->eq_list, entry) {
> > if (queue == eq) {
> > @@ -1366,47 +1380,113 @@ static int irq_setup(unsigned int *irqs, unsigned int len, int node,
> > return 0;
> > }
> >
> > -static int mana_gd_setup_irqs(struct pci_dev *pdev)
> > +static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> > {
> > struct gdma_context *gc = pci_get_drvdata(pdev);
> > - unsigned int max_queues_per_port;
> > struct gdma_irq_context *gic;
> > - unsigned int max_irqs, cpu;
> > - int start_irq_index = 1;
> > - int nvec, *irqs, irq;
> > - int err, i = 0, j;
> > + bool skip_first_cpu = false;
> > + int *irqs, irq, err, i;
> >
> > cpus_read_lock();
> > - max_queues_per_port = num_online_cpus();
> > - if (max_queues_per_port > MANA_MAX_NUM_QUEUES)
> > - max_queues_per_port = MANA_MAX_NUM_QUEUES;
> >
> > - /* Need 1 interrupt for the Hardware communication Channel (HWC) */
> > - max_irqs = max_queues_per_port + 1;
> > -
> > - nvec = pci_alloc_irq_vectors(pdev, 2, max_irqs, PCI_IRQ_MSIX);
> > - if (nvec < 0) {
> > - cpus_read_unlock();
> > - return nvec;
> > - }
> > - if (nvec <= num_online_cpus())
> > - start_irq_index = 0;
> > -
> > - irqs = kmalloc_array((nvec - start_irq_index), sizeof(int), GFP_KERNEL);
> > + irqs = kmalloc_array(nvec, sizeof(int), GFP_KERNEL);
> > if (!irqs) {
> > err = -ENOMEM;
> > goto free_irq_vector;
> > }
> >
> > - gc->irq_contexts = kcalloc(nvec, sizeof(struct gdma_irq_context),
> > - GFP_KERNEL);
> > - if (!gc->irq_contexts) {
> > + /*
> > + * While processing the next pci irq vector, we start with index 1,
> > + * as IRQ vector at index 0 is already processed for HWC.
> > + * However, the population of irqs array starts with index 0, to be
> > + * further used in irq_setup()
> > + */
> > + for (i = 1; i <= nvec; i++) {
> > + gic = kzalloc(sizeof(*gic), GFP_KERNEL);
> > + if (!gic) {
> > + err = -ENOMEM;
> > + goto free_irq;
> > + }
> > + gic->handler = mana_gd_process_eq_events;
> > + INIT_LIST_HEAD(&gic->eq_list);
> > + spin_lock_init(&gic->lock);
> > +
> > + snprintf(gic->name, MANA_IRQ_NAME_SZ, "mana_q%d@pci:%s",
> > + i - 1, pci_name(pdev));
> > +
> > + /* one pci vector is already allocated for HWC */
> > + irqs[i - 1] = pci_irq_vector(pdev, i);
> > + if (irqs[i - 1] < 0) {
> > + err = irqs[i - 1];
> > + goto free_current_gic;
> > + }
> > +
> > + err = request_irq(irqs[i - 1], mana_gd_intr, 0, gic->name, gic);
> > + if (err)
> > + goto free_current_gic;
> > +
> > + xa_store(&gc->irq_contexts, i, gic, GFP_KERNEL);
> > + }
> > +
> > + /*
> > + * When calling irq_setup() for dynamically added IRQs, if number of
> > + * CPUs is more than or equal to allocated MSI-X, we need to skip the
> > + * first CPU sibling group since they are already affinitized to HWC IRQ
> > + */
> > + if (gc->num_msix_usable <= num_online_cpus())
> > + skip_first_cpu = true;
> > +
> > + err = irq_setup(irqs, nvec, gc->numa_node, skip_first_cpu);
> > + if (err)
> > + goto free_irq;
> > +
> > + cpus_read_unlock();
> > + kfree(irqs);
> > + return 0;
> > +
> > +free_current_gic:
> > + kfree(gic);
> > +free_irq:
> > + for (i -= 1; i > 0; i--) {
> > + irq = pci_irq_vector(pdev, i);
> > + gic = xa_load(&gc->irq_contexts, i);
> > + if (WARN_ON(!gic))
> > + continue;
> > +
> > + irq_update_affinity_hint(irq, NULL);
> > + free_irq(irq, gic);
> > + xa_erase(&gc->irq_contexts, i);
> > + kfree(gic);
> > + }
> > + kfree(irqs);
> > +free_irq_vector:
> > + cpus_read_unlock();
> > + return err;
> > +}
> > +
> > +static int mana_gd_setup_irqs(struct pci_dev *pdev, int nvec)
> > +{
> > + struct gdma_context *gc = pci_get_drvdata(pdev);
> > + struct gdma_irq_context *gic;
> > + int *irqs, *start_irqs, irq;
> > + unsigned int cpu;
> > + int err, i;
> > +
> > + cpus_read_lock();
> > +
> > + irqs = kmalloc_array(nvec, sizeof(int), GFP_KERNEL);
> > + if (!irqs) {
> > err = -ENOMEM;
> > - goto free_irq_array;
> > + goto free_irq_vector;
> > }
> >
> > for (i = 0; i < nvec; i++) {
> > - gic = &gc->irq_contexts[i];
> > + gic = kzalloc(sizeof(*gic), GFP_KERNEL);
> > + if (!gic) {
> > + err = -ENOMEM;
> > + goto free_irq;
> > + }
> > +
> > gic->handler = mana_gd_process_eq_events;
> > INIT_LIST_HEAD(&gic->eq_list);
> > spin_lock_init(&gic->lock);
> > @@ -1418,69 +1498,128 @@ static int mana_gd_setup_irqs(struct pci_dev *pdev)
> > snprintf(gic->name, MANA_IRQ_NAME_SZ, "mana_q%d@pci:%s",
> > i - 1, pci_name(pdev));
> >
> > - irq = pci_irq_vector(pdev, i);
> > - if (irq < 0) {
> > - err = irq;
> > - goto free_irq;
> > + irqs[i] = pci_irq_vector(pdev, i);
> > + if (irqs[i] < 0) {
> > + err = irqs[i];
> > + goto free_current_gic;
> > }
> >
> > - if (!i) {
> > - err = request_irq(irq, mana_gd_intr, 0, gic->name, gic);
> > - if (err)
> > - goto free_irq;
> > -
> > - /* If number of IRQ is one extra than number of online CPUs,
> > - * then we need to assign IRQ0 (hwc irq) and IRQ1 to
> > - * same CPU.
> > - * Else we will use different CPUs for IRQ0 and IRQ1.
> > - * Also we are using cpumask_local_spread instead of
> > - * cpumask_first for the node, because the node can be
> > - * mem only.
> > - */
> > - if (start_irq_index) {
> > - cpu = cpumask_local_spread(i, gc->numa_node);
> > - irq_set_affinity_and_hint(irq, cpumask_of(cpu));
> > - } else {
> > - irqs[start_irq_index] = irq;
> > - }
> > - } else {
> > - irqs[i - start_irq_index] = irq;
> > - err = request_irq(irqs[i - start_irq_index], mana_gd_intr, 0,
> > - gic->name, gic);
> > - if (err)
> > - goto free_irq;
> > - }
> > + err = request_irq(irqs[i], mana_gd_intr, 0, gic->name, gic);
> > + if (err)
> > + goto free_current_gic;
> > +
> > + xa_store(&gc->irq_contexts, i, gic, GFP_KERNEL);
> > }
> >
> > - err = irq_setup(irqs, nvec - start_irq_index, gc->numa_node, false);
> > + /* If number of IRQ is one extra than number of online CPUs,
> > + * then we need to assign IRQ0 (hwc irq) and IRQ1 to
> > + * same CPU.
> > + * Else we will use different CPUs for IRQ0 and IRQ1.
> > + * Also we are using cpumask_local_spread instead of
> > + * cpumask_first for the node, because the node can be
> > + * mem only.
> > + */
> > + start_irqs = irqs;
> > + if (nvec > num_online_cpus()) {
> > + cpu = cpumask_local_spread(0, gc->numa_node);
> > + irq_set_affinity_and_hint(irqs[0], cpumask_of(cpu));
> > + irqs++;
> > + nvec -= 1;
> > + }
> > +
> > + err = irq_setup(irqs, nvec, gc->numa_node, false);
> > if (err)
> > goto free_irq;
> >
> > - gc->max_num_msix = nvec;
> > - gc->num_msix_usable = nvec;
> > cpus_read_unlock();
> > - kfree(irqs);
> > + kfree(start_irqs);
> > return 0;
> >
> > +free_current_gic:
> > + kfree(gic);
> > free_irq:
> > - for (j = i - 1; j >= 0; j--) {
> > - irq = pci_irq_vector(pdev, j);
> > - gic = &gc->irq_contexts[j];
> > + for (i -= 1; i >= 0; i--) {
> > + irq = pci_irq_vector(pdev, i);
> > + gic = xa_load(&gc->irq_contexts, i);
> > + if (WARN_ON(!gic))
> > + continue;
> >
> > irq_update_affinity_hint(irq, NULL);
> > free_irq(irq, gic);
> > + xa_erase(&gc->irq_contexts, i);
> > + kfree(gic);
> > }
> >
> > - kfree(gc->irq_contexts);
> > - gc->irq_contexts = NULL;
> > -free_irq_array:
> > - kfree(irqs);
> > + kfree(start_irqs);
>
> There is a case when start_irqs can be used here uninitialized.
>
> - Saurabh
Thanks Saurabh. I'll get this in the next version
^ permalink raw reply
* Re: [PATCH v3 06/13] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
From: Rob Herring @ 2025-05-29 13:16 UTC (permalink / raw)
To: Ricardo Neri
Cc: Krzysztof Kozlowski, x86, Krzysztof Kozlowski, Conor Dooley,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Michael Kelley, devicetree, Saurabh Sengar, Chris Oo,
linux-hyperv, Kirill A. Shutemov, linux-acpi, linux-kernel,
Ravi V. Shankar, Ricardo Neri
In-Reply-To: <20250524155650.GA16942@ranerica-svr.sc.intel.com>
On Sat, May 24, 2025 at 08:56:50AM -0700, Ricardo Neri wrote:
> On Mon, May 19, 2025 at 10:56:06AM -0700, Ricardo Neri wrote:
> > On Mon, May 19, 2025 at 10:29:37AM -0500, Rob Herring wrote:
> > > On Wed, May 14, 2025 at 08:53:38PM -0700, Ricardo Neri wrote:
> > > > On Wed, May 14, 2025 at 10:42:48AM -0500, Rob Herring wrote:
> > > > > On Tue, May 13, 2025 at 03:14:56PM -0700, Ricardo Neri wrote:
> > > > > > On Mon, May 12, 2025 at 10:32:24AM -0500, Rob Herring wrote:
> > > > > > > On Tue, May 06, 2025 at 08:23:39PM -0700, Ricardo Neri wrote:
> > > > > > > > On Tue, May 06, 2025 at 09:10:22AM +0200, Krzysztof Kozlowski wrote:
> > > > > > > > > On Mon, May 05, 2025 at 10:16:10PM GMT, Ricardo Neri wrote:
> > > > > > > > > > > If this is a device, then compatibles specific to devices. You do not
> > > > > > > > > > > get different rules than all other bindings... or this does not have to
> > > > > > > > > > > be binding at all. Why standard reserved-memory does not work for here?
> > > > > > > > > > >
> > > > > > > > > > > Why do you need compatible in the first place?
> > > > > > > > > >
> > > > > > > > > > Are you suggesting something like this?
> > > > > > > > > >
> > > > > > > > > > reserved-memory {
> > > > > > > > > > # address-cells = <2>;
> > > > > > > > > > # size-cells = <1>;
> > > > > > > > > >
> > > > > > > > > > wakeup_mailbox: wakeupmb@fff000 {
> > > > > > > > > > reg = < 0x0 0xfff000 0x1000>
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > and then reference to the reserved memory using the wakeup_mailbox
> > > > > > > > > > phandle?
> > > > > > > > >
> > > > > > > > > Yes just like every other, typical reserved memory block.
> > > > > > > >
> > > > > > > > Thanks! I will take this approach and drop this patch.
> > > > > > >
> > > > > > > If there is nothing else to this other than the reserved region, then
> > > > > > > don't do this. Keep it like you had. There's no need for 2 nodes.
> > > > > >
> > > > > > Thank you for your feedback!
> > > > > >
> > > > > > I was planning to use one reserved-memory node and inside of it a child
> > > > > > node to with a `reg` property to specify the location and size of the
> > > > > > mailbox. I would reference to that subnode from the kernel code.
> > > > > >
> > > > > > IIUC, the reserved-memory node is only the container and the actual memory
> > > > > > regions are expressed as child nodes.
> > > > > >
> > > > > > I had it like that before, but with a `compatible` property that I did not
> > > > > > need.
> > > > > >
> > > > > > Am I missing anything?
> > > > >
> > > > > Without a compatible, how do you identify which reserved region is the
> > > > > wakeup mailbox?
> > > >
> > > > I thought using a phandle to the wakeup_mailbox. Then I realized that the
> > > > device nodes using the mailbox would be CPUs. They would need a `memory-
> > > > region` property. This does not look right to me.
> > >
> > > That doesn't really make sense unless it's a memory region per CPU.
> >
> > Agreed.
> >
> > >
> > >
> > > > > Before you say node name, those are supposed to be
> > > > > generic though we failed to enforce anything for /reserved-memory child
> > > > > nodes.
> > > >
> > > > I see. Thanks for preventing me from doing this.
> > > >
> > > > Then the `compatible` property seems the way to go after all.
> > > >
> > > > This what motivated this patch in the first place. On further analysis,
> > > > IIUC, defining bindings and schema is not needed, IMO, since the mailbox
> > > > is already defined in the ACPI spec. No need to redefine.
> > >
> > > You lost me...
> > >
> > > You don't need to redefine the layout of the memory region as that's
> > > defined already somewhere,
> >
> > Great!
> >
> > > but you do need to define where it is for DT.
> > > And for that, you need a compatible. Do you know where it is in this
> > > case?
> >
> > The compatible is not defined anywhere yet. Is a DT schema needed to
> > document it? If yes, I am usure what to put in the description. We tried
> > to not redefine the mailbox and refer to the ACPI spec. That was a NAK
> > from Krzysztof [1].
> >
> > [1]. https://lore.kernel.org/r/624e1985-7dd2-4abe-a918-78cb43556967@kernel.org
>
> In summary, documenting the `compatible` property for the mailbox is
> necessary. There is no need to redefine the malbox on a schema but
> referring to the ACPI spec is not acceptable.
There's the whole "DT bindings in ACPI systems" where ACPI tables
contain compatibles and DT properties which I think is what
Krzysztof was objecting to (and I do too). But this is a DT based system
that implements a mailbox region defined in an ACPI spec. That is
perfectly fine to refer to.
>
> What about referring in the schema to the Intel TDX Virtual Firmware Design
> Guide[2]? It describes how firmware should implement the mailbox the section
> 4.3.5.
>
> A mailbox with compatible = "intel,wakeup-mailbox" is implemented after the
> guide that Intel published.
Use whatever you think best describes the programming model of the
region.
Rob
^ permalink raw reply
* Re: [PATCH v4 3/5] net: mana: explain irq_setup() algorithm
From: Shradha Gupta @ 2025-05-29 13:15 UTC (permalink / raw)
To: Yury Norov
Cc: Dexuan Cui, Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Konstantin Taranov, Simon Horman, Leon Romanovsky, Maxim Levitsky,
Erni Sri Satya Vennela, Peter Zijlstra, Michael Kelley,
linux-hyperv, linux-pci, linux-kernel, Nipun Gupta,
Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen, Kevin Tian,
Long Li, Thomas Gleixner, Bjorn Helgaas, Rob Herring,
Manivannan Sadhasivam, Krzysztof Wilczy???~Dski,
Lorenzo Pieralisi, netdev, linux-rdma, Paul Rosswurm,
Shradha Gupta
In-Reply-To: <aDYOFzQrfDFcti-u@yury>
On Tue, May 27, 2025 at 03:10:15PM -0400, Yury Norov wrote:
> So now git will think that you're the author of the patch.
>
> If author and sender are different people, the first line in commit
> message body should state that. In this case, it should be:
>
> From: Yury Norov <yury.norov@gmail.com>
>
> Please consider this one example
>
> https://patchew.org/linux/20250326-fixed-type-genmasks-v8-0-24afed16ca00@wanadoo.fr/20250326-fixed-type-genmasks-v8-6-24afed16ca00@wanadoo.fr/
>
> Thanks,
> Yury
>
Understood, Thank you Yury. I'll make this change in the next version
Regards,
Shradha.
> On Tue, May 27, 2025 at 08:58:25AM -0700, Shradha Gupta wrote:
> > Commit 91bfe210e196 ("net: mana: add a function to spread IRQs per CPUs")
> > added the irq_setup() function that distributes IRQs on CPUs according
> > to a tricky heuristic. The corresponding commit message explains the
> > heuristic.
> >
> > Duplicate it in the source code to make available for readers without
> > digging git in history. Also, add more detailed explanation about how
> > the heuristics is implemented.
> >
> > Signed-off-by: Yury Norov [NVIDIA] <yury.norov@gmail.com>
> > Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> > ---
> > .../net/ethernet/microsoft/mana/gdma_main.c | 41 +++++++++++++++++++
> > 1 file changed, 41 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > index 4ffaf7588885..f9e8d4d1ba3a 100644
> > --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> > @@ -1288,6 +1288,47 @@ void mana_gd_free_res_map(struct gdma_resource *r)
> > r->size = 0;
> > }
> >
> > +/*
> > + * Spread on CPUs with the following heuristics:
> > + *
> > + * 1. No more than one IRQ per CPU, if possible;
> > + * 2. NUMA locality is the second priority;
> > + * 3. Sibling dislocality is the last priority.
> > + *
> > + * Let's consider this topology:
> > + *
> > + * Node 0 1
> > + * Core 0 1 2 3
> > + * CPU 0 1 2 3 4 5 6 7
> > + *
> > + * The most performant IRQ distribution based on the above topology
> > + * and heuristics may look like this:
> > + *
> > + * IRQ Nodes Cores CPUs
> > + * 0 1 0 0-1
> > + * 1 1 1 2-3
> > + * 2 1 0 0-1
> > + * 3 1 1 2-3
> > + * 4 2 2 4-5
> > + * 5 2 3 6-7
> > + * 6 2 2 4-5
> > + * 7 2 3 6-7
> > + *
> > + * The heuristics is implemented as follows.
> > + *
> > + * The outer for_each() loop resets the 'weight' to the actual number
> > + * of CPUs in the hop. Then inner for_each() loop decrements it by the
> > + * number of sibling groups (cores) while assigning first set of IRQs
> > + * to each group. IRQs 0 and 1 above are distributed this way.
> > + *
> > + * Now, because NUMA locality is more important, we should walk the
> > + * same set of siblings and assign 2nd set of IRQs (2 and 3), and it's
> > + * implemented by the medium while() loop. We do like this unless the
> > + * number of IRQs assigned on this hop will not become equal to number
> > + * of CPUs in the hop (weight == 0). Then we switch to the next hop and
> > + * do the same thing.
> > + */
> > +
> > static int irq_setup(unsigned int *irqs, unsigned int len, int node)
> > {
> > const struct cpumask *next, *prev = cpu_none_mask;
> > --
> > 2.34.1
^ permalink raw reply
* [PATCH net,v3] hv_netvsc: fix potential deadlock in netvsc_vf_setxdp()
From: Saurabh Sengar @ 2025-05-29 10:18 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, horms, ast, daniel, hawk, john.fastabend, sdf, kuniyu,
ahmed.zaki, aleksander.lobakin, linux-hyperv, netdev,
linux-kernel, bpf
Cc: ssengar, stable, Saurabh Sengar
The MANA driver's probe registers netdevice via the following call chain:
mana_probe()
register_netdev()
register_netdevice()
register_netdevice() calls notifier callback for netvsc driver,
holding the netdev mutex via netdev_lock_ops().
Further this netvsc notifier callback end up attempting to acquire the
same lock again in dev_xdp_propagate() leading to deadlock.
netvsc_netdev_event()
netvsc_vf_setxdp()
dev_xdp_propagate()
This deadlock was not observed so far because net_shaper_ops was never set,
and thus the lock was effectively a no-op in this case. Fix this by using
netif_xdp_propagate() instead of dev_xdp_propagate() to avoid recursive
locking in this path.
And, since no deadlock is observed on the other path which is via
netvsc_probe, add the lock exclusivly for that path.
Also, clean up the unregistration path by removing the unnecessary call to
netvsc_vf_setxdp(), since unregister_netdevice_many_notify() already
performs this cleanup via dev_xdp_uninstall().
Fixes: 97246d6d21c2 ("net: hold netdev instance lock during ndo_bpf")
Cc: stable@vger.kernel.org
Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
Tested-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: Subbaraya Sundeep <sbhatta@marvell.com>
---
[V3]
- Add the lock for netvsc probe path
[V2]
- Modified commit message
drivers/net/hyperv/netvsc_bpf.c | 2 +-
drivers/net/hyperv/netvsc_drv.c | 4 ++--
net/core/dev.c | 1 +
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
index e01c5997a551..1dd3755d9e6d 100644
--- a/drivers/net/hyperv/netvsc_bpf.c
+++ b/drivers/net/hyperv/netvsc_bpf.c
@@ -183,7 +183,7 @@ int netvsc_vf_setxdp(struct net_device *vf_netdev, struct bpf_prog *prog)
xdp.command = XDP_SETUP_PROG;
xdp.prog = prog;
- ret = dev_xdp_propagate(vf_netdev, &xdp);
+ ret = netif_xdp_propagate(vf_netdev, &xdp);
if (ret && prog)
bpf_prog_put(prog);
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 14a0d04e21ae..c41a025c66f0 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2462,8 +2462,6 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
- netvsc_vf_setxdp(vf_netdev, NULL);
-
reinit_completion(&net_device_ctx->vf_add);
netdev_rx_handler_unregister(vf_netdev);
netdev_upper_dev_unlink(vf_netdev, ndev);
@@ -2631,7 +2629,9 @@ static int netvsc_probe(struct hv_device *dev,
continue;
netvsc_prepare_bonding(vf_netdev);
+ netdev_lock_ops(vf_netdev);
netvsc_register_vf(vf_netdev, VF_REG_IN_PROBE);
+ netdev_unlock_ops(vf_netdev);
__netvsc_vf_setup(net, vf_netdev);
break;
}
diff --git a/net/core/dev.c b/net/core/dev.c
index 2b514d95c528..a388f459a366 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9968,6 +9968,7 @@ int netif_xdp_propagate(struct net_device *dev, struct netdev_bpf *bpf)
return dev->netdev_ops->ndo_bpf(dev, bpf);
}
+EXPORT_SYMBOL_GPL(netif_xdp_propagate);
u32 dev_xdp_prog_id(struct net_device *dev, enum bpf_xdp_mode mode)
{
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2 00/13] objtool: Detect and warn about indirect calls in __nocfi functions
From: Peter Zijlstra @ 2025-05-29 9:30 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Sean Christopherson, H. Peter Anvin, x86, kys, haiyangz, wei.liu,
decui, tglx, mingo, bp, dave.hansen, pbonzini, ardb, kees,
Arnd Bergmann, gregkh, linux-hyperv, linux-kernel, kvm, linux-efi,
samitolvanen, ojeda, xin
In-Reply-To: <20250528163557.GI31726@noisy.programming.kicks-ass.net>
On Wed, May 28, 2025 at 06:35:58PM +0200, Peter Zijlstra wrote:
> On Wed, May 28, 2025 at 06:30:35PM +0200, Peter Zijlstra wrote:
> > On Wed, May 28, 2025 at 09:44:52AM +0200, Peter Zijlstra wrote:
> > > On Tue, May 06, 2025 at 12:18:49PM -0700, Josh Poimboeuf wrote:
> > >
> > > > Weird, I'm not seeing that.
> > >
> > > I Ate'nt Crazeh...
> > >
> > > https://lore.kernel.org/all/202505280410.2qfTQCRt-lkp@intel.com/T/#u
> > >
> > > I'll go poke at it, see if today is the day I can figure out WTF
> > > happens.
> >
> > It manages to trip the CFI_UNDEFINED case in op->dest.reg == cfa->base
> > in update_cfi_state().
> >
> > I figured it ought to tickle the regular 'mov %rbp, %rsp' case above
> > there, but it doesn't, for some reason it has cfa.base == SP at this
> > point.
> >
> > This happens... /me looks in scrollback ... at POP_REGS 'pop
> > %rbp'. ARGH!!
> >
More fun!
> > So the sequence of fail is:
> >
> > push %rbp
> > mov %rsp, %rbp # cfa.base = BP
> >
> > SAVE
sub $0x40,%rsp
and $0xffffffffffffffc0,%rsp
This hits the 'older GCC, drap with frame pointer' case in OP_SRC_AND.
Which means we then hard rely on the frame pointer to get things right.
However, per all the PUSH/POP_REGS nonsense, BP can get clobbered.
Specifically the code between the CALL and POP %rbp below are up in the
air. I don't think it can currently unwind properly there.
> > ...
> > push %rbp
> > ...
> > pop %rbp # cfa.base = SP
>
> This is the POP !drap and dest==base case.
>
> > ...
> > mov %rbp, %rsp # UNDEF
> > nop # FAIL
> > RESTORE
> >
> > Note that the MOV+NOP is the 4 bytes ERETS needs.
^ permalink raw reply
* Re: [PATCH net-next,v6] net: mana: Add handler for hardware servicing events
From: Paolo Abeni @ 2025-05-29 6:41 UTC (permalink / raw)
To: Haiyang Zhang, linux-hyperv, netdev
Cc: decui, stephen, kys, paulros, olaf, vkuznets, davem, wei.liu,
edumazet, kuba, leon, longli, ssengar, linux-rdma, daniel,
john.fastabend, bpf, ast, hawk, tglx, shradhagupta, andrew+netdev,
kotaranov, horms, linux-kernel
In-Reply-To: <1748382166-1886-1-git-send-email-haiyangz@microsoft.com>
On 5/27/25 11:42 PM, Haiyang Zhang wrote:
> To collaborate with hardware servicing events, upon receiving the special
> EQE notification from the HW channel, remove the devices on this bus.
> Then, after a waiting period based on the device specs, rescan the parent
> bus to recover the devices.
>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> Reviewed-by: Simon Horman <horms@kernel.org>
## Form letter - net-next-closed
The merge window for v6.16 has begun and therefore net-next is closed
for new drivers, features, code refactoring and optimizations. We are
currently accepting bug fixes only.
Please repost when net-next reopens after June 8th.
RFC patches sent for review only are obviously welcome at any time.
^ permalink raw reply
* Re: [PATCH] vmbus: retrieve connection-id from device tree
From: Saurabh Singh Sengar @ 2025-05-29 5:43 UTC (permalink / raw)
To: Hardik Garg
Cc: kys, haiyangz, wei.liu, decui, romank, linux-hyperv, linux-kernel,
stable, ssengar, apais
In-Reply-To: <6acee4bf-cb04-43b9-9476-e8d811d26dfd@linux.microsoft.com>
On Wed, May 28, 2025 at 02:33:25PM -0700, 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 device tree where it exists as a property under
> the vmbus node with the compatible ID "microsoft,message-connection-id".
Add Documentation for "microsoft,message-connection-id"
>
> Reading from device tree allows platforms to specify their preferred
> communication channel, making it more flexible. If the property is
> not found in the device tree, use the default connection ID
> (VMBUS_MESSAGE_CONNECTION_ID or VMBUS_MESSAGE_CONNECTION_ID_4
> based on protocol version).
>
> Cc: <stable@kernel.org> # 6.14, 6.12
I don't see it as as bug which needs to be backported on certain branches only.
You can rebase your changes on linux-next or hyperv-next and resend.
Also,
s/device tree/DeviceTree/g
- Saurabh
^ permalink raw reply
* Re: [PATCH v4 2/5] PCI: hv: Allow dynamic MSI-X vector allocation
From: Saurabh Singh Sengar @ 2025-05-29 3:46 UTC (permalink / raw)
To: Shradha Gupta
Cc: Bjorn Helgaas, Rob Herring, Manivannan Sadhasivam,
Krzysztof Wilczy�~Dski, Lorenzo Pieralisi, Dexuan Cui,
Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Michael Kelley,
linux-hyperv, linux-pci, linux-kernel, Nipun Gupta, Yury Norov,
Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen, Kevin Tian,
Long Li, Thomas Gleixner, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Konstantin Taranov,
Simon Horman, Leon Romanovsky, Maxim Levitsky,
Erni Sri Satya Vennela, Peter Zijlstra, netdev, linux-rdma,
Paul Rosswurm, Shradha Gupta
In-Reply-To: <1748361489-25415-1-git-send-email-shradhagupta@linux.microsoft.com>
On Tue, May 27, 2025 at 08:58:09AM -0700, Shradha Gupta wrote:
> Allow dynamic MSI-X vector allocation for pci_hyperv PCI controller
> by adding support for the flag MSI_FLAG_PCI_MSIX_ALLOC_DYN and using
> pci_msix_prepare_desc() to prepare the MSI-X descriptors.
>
> Feature support added for both x86 and ARM64
>
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> Changes in v4:
> * use the same prepare_desc() callback for arm and x86
> ---
> Changes in v3:
> * Add arm64 support
> ---
> Changes in v2:
> * split the patch to keep changes in PCI and pci_hyperv controller
> seperate
> * replace strings "pci vectors" by "MSI-X vectors"
> ---
> drivers/pci/controller/pci-hyperv.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index ac27bda5ba26..0c790f35ad0e 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -2063,6 +2063,7 @@ static struct irq_chip hv_msi_irq_chip = {
> static struct msi_domain_ops hv_msi_ops = {
> .msi_prepare = hv_msi_prepare,
> .msi_free = hv_msi_free,
> + .prepare_desc = pci_msix_prepare_desc,
> };
>
> /**
> @@ -2084,7 +2085,7 @@ static int hv_pcie_init_irq_domain(struct hv_pcibus_device *hbus)
> hbus->msi_info.ops = &hv_msi_ops;
> hbus->msi_info.flags = (MSI_FLAG_USE_DEF_DOM_OPS |
> MSI_FLAG_USE_DEF_CHIP_OPS | MSI_FLAG_MULTI_PCI_MSI |
> - MSI_FLAG_PCI_MSIX);
> + MSI_FLAG_PCI_MSIX | MSI_FLAG_PCI_MSIX_ALLOC_DYN);
> hbus->msi_info.handler = FLOW_HANDLER;
> hbus->msi_info.handler_name = FLOW_NAME;
> hbus->msi_info.data = hbus;
> --
> 2.34.1
>
Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
^ permalink raw reply
* Re: [PATCH v4 1/5] PCI/MSI: Export pci_msix_prepare_desc() for dynamic MSI-X allocations
From: Saurabh Singh Sengar @ 2025-05-29 3:46 UTC (permalink / raw)
To: Shradha Gupta
Cc: Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen,
Thomas Gleixner, Bjorn Helgaas, Michael Kelley, linux-hyperv,
linux-pci, linux-kernel, Nipun Gupta, Yury Norov, Kevin Tian,
Long Li, Rob Herring, Manivannan Sadhasivam,
Krzysztof Wilczy�~Dski, Lorenzo Pieralisi, Dexuan Cui,
Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Konstantin Taranov, Simon Horman, Leon Romanovsky, Maxim Levitsky,
Erni Sri Satya Vennela, Peter Zijlstra, netdev, linux-rdma,
Paul Rosswurm, Shradha Gupta
In-Reply-To: <1748361477-25244-1-git-send-email-shradhagupta@linux.microsoft.com>
On Tue, May 27, 2025 at 08:57:57AM -0700, Shradha Gupta wrote:
> For supporting dynamic MSI-X vector allocation by PCI controllers, enabling
> the flag MSI_FLAG_PCI_MSIX_ALLOC_DYN is not enough, msix_prepare_msi_desc()
> to prepare the MSI descriptor is also needed.
>
> Export pci_msix_prepare_desc() to allow PCI controllers to support dynamic
> MSI-X vector allocation.
>
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> Changes in v3
> * Improved the patch description by removing abbreviations
> ---
> drivers/pci/msi/irqdomain.c | 5 +++--
> include/linux/msi.h | 2 ++
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/msi/irqdomain.c b/drivers/pci/msi/irqdomain.c
> index d7ba8795d60f..43129aa6d6c7 100644
> --- a/drivers/pci/msi/irqdomain.c
> +++ b/drivers/pci/msi/irqdomain.c
> @@ -222,13 +222,14 @@ static void pci_irq_unmask_msix(struct irq_data *data)
> pci_msix_unmask(irq_data_get_msi_desc(data));
> }
>
> -static void pci_msix_prepare_desc(struct irq_domain *domain, msi_alloc_info_t *arg,
> - struct msi_desc *desc)
> +void pci_msix_prepare_desc(struct irq_domain *domain, msi_alloc_info_t *arg,
> + struct msi_desc *desc)
> {
> /* Don't fiddle with preallocated MSI descriptors */
> if (!desc->pci.mask_base)
> msix_prepare_msi_desc(to_pci_dev(desc->dev), desc);
> }
> +EXPORT_SYMBOL_GPL(pci_msix_prepare_desc);
>
> static const struct msi_domain_template pci_msix_template = {
> .chip = {
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index 86e42742fd0f..d5864d5e75c2 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -691,6 +691,8 @@ struct irq_domain *pci_msi_create_irq_domain(struct fwnode_handle *fwnode,
> struct irq_domain *parent);
> u32 pci_msi_domain_get_msi_rid(struct irq_domain *domain, struct pci_dev *pdev);
> struct irq_domain *pci_msi_get_device_domain(struct pci_dev *pdev);
> +void pci_msix_prepare_desc(struct irq_domain *domain, msi_alloc_info_t *arg,
> + struct msi_desc *desc);
> #else /* CONFIG_PCI_MSI */
> static inline struct irq_domain *pci_msi_get_device_domain(struct pci_dev *pdev)
> {
> --
> 2.34.1
>
Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
^ permalink raw reply
* Re: [PATCH v4 5/5] net: mana: Allocate MSI-X vectors dynamically
From: Saurabh Singh Sengar @ 2025-05-29 3:45 UTC (permalink / raw)
To: Shradha Gupta
Cc: Dexuan Cui, Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Konstantin Taranov, Simon Horman, Leon Romanovsky, Maxim Levitsky,
Erni Sri Satya Vennela, Peter Zijlstra, Michael Kelley,
linux-hyperv, linux-pci, linux-kernel, Nipun Gupta, Yury Norov,
Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen, Kevin Tian,
Long Li, Thomas Gleixner, Bjorn Helgaas, Rob Herring,
Manivannan Sadhasivam, Krzysztof Wilczy�~Dski,
Lorenzo Pieralisi, netdev, linux-rdma, Paul Rosswurm,
Shradha Gupta
In-Reply-To: <1748361543-25845-1-git-send-email-shradhagupta@linux.microsoft.com>
On Tue, May 27, 2025 at 08:59:03AM -0700, Shradha Gupta wrote:
> Currently, the MANA driver allocates MSI-X vectors statically based on
> MANA_MAX_NUM_QUEUES and num_online_cpus() values and in some cases ends
> up allocating more vectors than it needs. This is because, by this time
> we do not have a HW channel and do not know how many IRQs should be
> allocated.
>
> To avoid this, we allocate 1 MSI-X vector during the creation of HWC and
> after getting the value supported by hardware, dynamically add the
> remaining MSI-X vectors.
>
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> Changes in v4:
> * added BUG_ON at appropriate places
> * moved xa_destroy to mana_gd_remove()
> * rearragned the cleanup logic in mana_gd_setup_dyn_irqs()
> * simplified processing around start_irq_index in mana_gd_setup_irqs()
> * return 0 instead of return err as appropriate
> ---
> Changes in v3:
> * implemented irq_contexts as xarrays rather than list
> * split the patch to create a perparation patch around irq_setup()
> * add log when IRQ allocation/setup for remaining IRQs fails
> ---
> Changes in v2:
> * Use string 'MSI-X vectors' instead of 'pci vectors'
> * make skip-cpu a bool instead of int
> * rearrange the comment arout skip_cpu variable appropriately
> * update the capability bit for driver indicating dynamic IRQ allocation
> * enforced max line length to 80
> * enforced RCT convention
> * initialized gic to NULL, for when there is a possibility of gic
> not being populated correctly
> ---
> .../net/ethernet/microsoft/mana/gdma_main.c | 306 +++++++++++++-----
> include/net/mana/gdma.h | 8 +-
> 2 files changed, 235 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index 763a548c4a2b..98ebecbec9a7 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -6,6 +6,8 @@
> #include <linux/pci.h>
> #include <linux/utsname.h>
> #include <linux/version.h>
> +#include <linux/msi.h>
> +#include <linux/irqdomain.h>
>
> #include <net/mana/mana.h>
>
> @@ -80,8 +82,15 @@ static int mana_gd_query_max_resources(struct pci_dev *pdev)
> return err ? err : -EPROTO;
> }
>
> - if (gc->num_msix_usable > resp.max_msix)
> - gc->num_msix_usable = resp.max_msix;
> + if (!pci_msix_can_alloc_dyn(pdev)) {
> + if (gc->num_msix_usable > resp.max_msix)
> + gc->num_msix_usable = resp.max_msix;
> + } else {
> + /* If dynamic allocation is enabled we have already allocated
> + * hwc msi
> + */
> + gc->num_msix_usable = min(resp.max_msix, num_online_cpus() + 1);
> + }
>
> if (gc->num_msix_usable <= 1)
> return -ENOSPC;
> @@ -482,7 +491,9 @@ static int mana_gd_register_irq(struct gdma_queue *queue,
> }
>
> queue->eq.msix_index = msi_index;
> - gic = &gc->irq_contexts[msi_index];
> + gic = xa_load(&gc->irq_contexts, msi_index);
> + if (WARN_ON(!gic))
> + return -EINVAL;
>
> spin_lock_irqsave(&gic->lock, flags);
> list_add_rcu(&queue->entry, &gic->eq_list);
> @@ -507,7 +518,10 @@ static void mana_gd_deregiser_irq(struct gdma_queue *queue)
> if (WARN_ON(msix_index >= gc->num_msix_usable))
> return;
>
> - gic = &gc->irq_contexts[msix_index];
> + gic = xa_load(&gc->irq_contexts, msix_index);
> + if (WARN_ON(!gic))
> + return;
> +
> spin_lock_irqsave(&gic->lock, flags);
> list_for_each_entry_rcu(eq, &gic->eq_list, entry) {
> if (queue == eq) {
> @@ -1366,47 +1380,113 @@ static int irq_setup(unsigned int *irqs, unsigned int len, int node,
> return 0;
> }
>
> -static int mana_gd_setup_irqs(struct pci_dev *pdev)
> +static int mana_gd_setup_dyn_irqs(struct pci_dev *pdev, int nvec)
> {
> struct gdma_context *gc = pci_get_drvdata(pdev);
> - unsigned int max_queues_per_port;
> struct gdma_irq_context *gic;
> - unsigned int max_irqs, cpu;
> - int start_irq_index = 1;
> - int nvec, *irqs, irq;
> - int err, i = 0, j;
> + bool skip_first_cpu = false;
> + int *irqs, irq, err, i;
>
> cpus_read_lock();
Now that num_online_cpus is moved further down in this new logic,
do we want to reduce the critical section ?
I don't think we want kmalloc_array to be protected.
> - max_queues_per_port = num_online_cpus();
> - if (max_queues_per_port > MANA_MAX_NUM_QUEUES)
> - max_queues_per_port = MANA_MAX_NUM_QUEUES;
>
> - /* Need 1 interrupt for the Hardware communication Channel (HWC) */
> - max_irqs = max_queues_per_port + 1;
> -
> - nvec = pci_alloc_irq_vectors(pdev, 2, max_irqs, PCI_IRQ_MSIX);
> - if (nvec < 0) {
> - cpus_read_unlock();
> - return nvec;
> - }
> - if (nvec <= num_online_cpus())
> - start_irq_index = 0;
> -
> - irqs = kmalloc_array((nvec - start_irq_index), sizeof(int), GFP_KERNEL);
> + irqs = kmalloc_array(nvec, sizeof(int), GFP_KERNEL);
> if (!irqs) {
> err = -ENOMEM;
> goto free_irq_vector;
> }
>
> - gc->irq_contexts = kcalloc(nvec, sizeof(struct gdma_irq_context),
> - GFP_KERNEL);
> - if (!gc->irq_contexts) {
> + /*
> + * While processing the next pci irq vector, we start with index 1,
> + * as IRQ vector at index 0 is already processed for HWC.
> + * However, the population of irqs array starts with index 0, to be
> + * further used in irq_setup()
> + */
> + for (i = 1; i <= nvec; i++) {
> + gic = kzalloc(sizeof(*gic), GFP_KERNEL);
> + if (!gic) {
> + err = -ENOMEM;
> + goto free_irq;
> + }
> + gic->handler = mana_gd_process_eq_events;
> + INIT_LIST_HEAD(&gic->eq_list);
> + spin_lock_init(&gic->lock);
> +
> + snprintf(gic->name, MANA_IRQ_NAME_SZ, "mana_q%d@pci:%s",
> + i - 1, pci_name(pdev));
> +
> + /* one pci vector is already allocated for HWC */
> + irqs[i - 1] = pci_irq_vector(pdev, i);
> + if (irqs[i - 1] < 0) {
> + err = irqs[i - 1];
> + goto free_current_gic;
> + }
> +
> + err = request_irq(irqs[i - 1], mana_gd_intr, 0, gic->name, gic);
> + if (err)
> + goto free_current_gic;
> +
> + xa_store(&gc->irq_contexts, i, gic, GFP_KERNEL);
> + }
> +
> + /*
> + * When calling irq_setup() for dynamically added IRQs, if number of
> + * CPUs is more than or equal to allocated MSI-X, we need to skip the
> + * first CPU sibling group since they are already affinitized to HWC IRQ
> + */
> + if (gc->num_msix_usable <= num_online_cpus())
> + skip_first_cpu = true;
> +
> + err = irq_setup(irqs, nvec, gc->numa_node, skip_first_cpu);
> + if (err)
> + goto free_irq;
> +
> + cpus_read_unlock();
> + kfree(irqs);
> + return 0;
> +
> +free_current_gic:
> + kfree(gic);
> +free_irq:
> + for (i -= 1; i > 0; i--) {
> + irq = pci_irq_vector(pdev, i);
> + gic = xa_load(&gc->irq_contexts, i);
> + if (WARN_ON(!gic))
> + continue;
> +
> + irq_update_affinity_hint(irq, NULL);
> + free_irq(irq, gic);
> + xa_erase(&gc->irq_contexts, i);
> + kfree(gic);
> + }
> + kfree(irqs);
> +free_irq_vector:
> + cpus_read_unlock();
> + return err;
> +}
> +
> +static int mana_gd_setup_irqs(struct pci_dev *pdev, int nvec)
> +{
> + struct gdma_context *gc = pci_get_drvdata(pdev);
> + struct gdma_irq_context *gic;
> + int *irqs, *start_irqs, irq;
> + unsigned int cpu;
> + int err, i;
> +
> + cpus_read_lock();
Same here
> + irqs = kmalloc_array(nvec, sizeof(int), GFP_KERNEL);
> + if (!irqs) {
> err = -ENOMEM;
> - goto free_irq_array;
> + goto free_irq_vector;
> }
>
> for (i = 0; i < nvec; i++) {
> - gic = &gc->irq_contexts[i];
> + gic = kzalloc(sizeof(*gic), GFP_KERNEL);
> + if (!gic) {
> + err = -ENOMEM;
> + goto free_irq;
> + }
> +
> gic->handler = mana_gd_process_eq_events;
> INIT_LIST_HEAD(&gic->eq_list);
> spin_lock_init(&gic->lock);
<snip>
- Saurabh
^ permalink raw reply
* [PATCH] vmbus: retrieve connection-id from device tree
From: Hardik Garg @ 2025-05-28 21:33 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui
Cc: romank, linux-hyperv, linux-kernel, stable, ssengar, apais
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 device tree where it exists as a property under
the vmbus node with the compatible ID "microsoft,message-connection-id".
Reading from device tree allows platforms to specify their preferred
communication channel, making it more flexible. If the property is
not found in the device tree, use the default connection ID
(VMBUS_MESSAGE_CONNECTION_ID or VMBUS_MESSAGE_CONNECTION_ID_4
based on protocol version).
Cc: <stable@kernel.org> # 6.14, 6.12
Signed-off-by: Hardik Garg <hargar@linux.microsoft.com>
---
drivers/hv/connection.c | 7 +++++--
drivers/hv/vmbus_drv.c | 8 ++++++++
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 8351360bba16..f5acc4685981 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -99,12 +99,15 @@ 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 device tree */
+ 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
* bitwise OR it
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index e3d51a316316..a42ec7d499aa 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2471,10 +2471,18 @@ 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;
hv_dev = &pdev->dev;
+ /* Read connection ID from device tree */
+ 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
* RE: [EXTERNAL] Re: [PATCH net-next,v5] net: mana: Add handler for hardware servicing events
From: Haiyang Zhang @ 2025-05-28 20:29 UTC (permalink / raw)
To: Paolo Abeni, linux-hyperv@vger.kernel.org, netdev@vger.kernel.org
Cc: Dexuan Cui, stephen@networkplumber.org, KY Srinivasan,
Paul Rosswurm, olaf@aepfle.de, vkuznets@redhat.com,
davem@davemloft.net, wei.liu@kernel.org, edumazet@google.com,
kuba@kernel.org, leon@kernel.org, Long Li,
ssengar@linux.microsoft.com, linux-rdma@vger.kernel.org,
daniel@iogearbox.net, john.fastabend@gmail.com,
bpf@vger.kernel.org, ast@kernel.org, hawk@kernel.org,
tglx@linutronix.de, shradhagupta@linux.microsoft.com,
andrew+netdev@lunn.ch, Konstantin Taranov, horms@kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <ea6b8065-76e4-45c8-a51f-858abab4d639@redhat.com>
> -----Original Message-----
> From: Paolo Abeni <pabeni@redhat.com>
> Sent: Tuesday, May 27, 2025 5:29 AM
> To: Haiyang Zhang <haiyangz@microsoft.com>; linux-hyperv@vger.kernel.org;
> netdev@vger.kernel.org
> Cc: Dexuan Cui <decui@microsoft.com>; stephen@networkplumber.org; KY
> Srinivasan <kys@microsoft.com>; Paul Rosswurm <paulros@microsoft.com>;
> olaf@aepfle.de; vkuznets@redhat.com; davem@davemloft.net;
> wei.liu@kernel.org; edumazet@google.com; kuba@kernel.org; leon@kernel.org;
> Long Li <longli@microsoft.com>; ssengar@linux.microsoft.com; linux-
> rdma@vger.kernel.org; daniel@iogearbox.net; john.fastabend@gmail.com;
> bpf@vger.kernel.org; ast@kernel.org; hawk@kernel.org; tglx@linutronix.de;
> shradhagupta@linux.microsoft.com; andrew+netdev@lunn.ch; Konstantin
> Taranov <kotaranov@microsoft.com>; horms@kernel.org; linux-
> kernel@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH net-next,v5] net: mana: Add handler for
> hardware servicing events
>
> On 5/22/25 2:22 AM, Haiyang Zhang wrote:
> > @@ -400,6 +448,33 @@ static void mana_gd_process_eqe(struct gdma_queue
> *eq)
> > eq->eq.callback(eq->eq.context, eq, &event);
> > break;
> >
> > + case GDMA_EQE_HWC_FPGA_RECONFIG:
> > + dev_info(gc->dev, "Recv MANA service type:%d\n", type);
> > +
> > + if (gc->in_service) {
> > + dev_info(gc->dev, "Already in service\n");
> > + break;
> > + }
> > +
> > + if (!try_module_get(THIS_MODULE)) {
> > + dev_info(gc->dev, "Module is unloading\n");
> > + break;
> > + }
> > +
> > + mns_wk = kzalloc(sizeof(*mns_wk), GFP_ATOMIC);
> > + if (!mns_wk) {
> > + module_put(THIS_MODULE);
> > + break;
> > + }
> > +
> > + dev_info(gc->dev, "Start MANA service type:%d\n", type);
> > + gc->in_service = true;
> > + mns_wk->pdev = to_pci_dev(gc->dev);
> > + pci_dev_get(mns_wk->pdev);
>
> Acquiring both the device and the module reference is confusing and
> likely unnecessary. pci_dev_get() should suffice.
>
Thanks for the review. I updated the patch accordingly, and submitted
v6 yesterday.
Thanks,
- Haiyang
^ permalink raw reply
* Re: [PATCH v4 0/5] Allow dyn MSI-X vector allocation of MANA
From: Simon Horman @ 2025-05-28 18:55 UTC (permalink / raw)
To: Shradha Gupta
Cc: linux-hyperv, linux-pci, linux-kernel, Nipun Gupta, Yury Norov,
Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen, Kevin Tian,
Long Li, Thomas Gleixner, Bjorn Helgaas, Rob Herring,
Manivannan Sadhasivam, Krzysztof Wilczy�~Dski,
Lorenzo Pieralisi, Dexuan Cui, Wei Liu, Haiyang Zhang,
K. Y. Srinivasan, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Konstantin Taranov, Leon Romanovsky,
Maxim Levitsky, Erni Sri Satya Vennela, Peter Zijlstra, netdev,
linux-rdma, Paul Rosswurm, Shradha Gupta
In-Reply-To: <1748361453-25096-1-git-send-email-shradhagupta@linux.microsoft.com>
On Tue, May 27, 2025 at 08:57:33AM -0700, Shradha Gupta wrote:
> In this patchset we want to enable the MANA driver to be able to
> allocate MSI-X vectors in PCI dynamically.
>
> The first patch exports pci_msix_prepare_desc() in PCI to be able to
> correctly prepare descriptors for dynamically added MSI-X vectors.
>
> The second patch adds the support of dynamic vector allocation in
> pci-hyperv PCI controller by enabling the MSI_FLAG_PCI_MSIX_ALLOC_DYN
> flag and using the pci_msix_prepare_desc() exported in first patch.
>
> The third patch adds a detailed description of the irq_setup(), to
> help understand the function design better.
>
> The fourth patch is a preparation patch for mana changes to support
> dynamic IRQ allocation. It contains changes in irq_setup() to allow
> skipping first sibling CPU sets, in case certain IRQs are already
> affinitized to them.
>
> The fifth patch has the changes in MANA driver to be able to allocate
> MSI-X vectors dynamically. If the support does not exist it defaults to
> older behavior.
Hi Shradha,
It's unclear what the target tree for this patch-set is.
But if it is net-next, which seems likely given the code under
drivers/net/, then:
Please include that target in the subject of each patch in the patch-set.
Subject: [PATCH v5 net-next 0/5] ...
And, moreover, ...
## Form letter - net-next-closed
The merge window for v6.16 has begun and therefore net-next is closed
for new drivers, features, code refactoring and optimizations. We are
currently accepting bug fixes only.
Please repost when net-next reopens after June 8th.
RFC patches sent for review only are obviously welcome at any time.
^ permalink raw reply
* Re: [PATCH v4 5/5] net: mana: Allocate MSI-X vectors dynamically
From: Simon Horman @ 2025-05-28 18:52 UTC (permalink / raw)
To: Shradha Gupta
Cc: Dexuan Cui, Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Konstantin Taranov, Leon Romanovsky, Maxim Levitsky,
Erni Sri Satya Vennela, Peter Zijlstra, Michael Kelley,
linux-hyperv, linux-pci, linux-kernel, Nipun Gupta, Yury Norov,
Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen, Kevin Tian,
Long Li, Thomas Gleixner, Bjorn Helgaas, Rob Herring,
Manivannan Sadhasivam, Krzysztof Wilczy�~Dski,
Lorenzo Pieralisi, netdev, linux-rdma, Paul Rosswurm,
Shradha Gupta
In-Reply-To: <1748361543-25845-1-git-send-email-shradhagupta@linux.microsoft.com>
On Tue, May 27, 2025 at 08:59:03AM -0700, Shradha Gupta wrote:
> Currently, the MANA driver allocates MSI-X vectors statically based on
> MANA_MAX_NUM_QUEUES and num_online_cpus() values and in some cases ends
> up allocating more vectors than it needs. This is because, by this time
> we do not have a HW channel and do not know how many IRQs should be
> allocated.
>
> To avoid this, we allocate 1 MSI-X vector during the creation of HWC and
> after getting the value supported by hardware, dynamically add the
> remaining MSI-X vectors.
>
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
...
> +static int mana_gd_setup_irqs(struct pci_dev *pdev, int nvec)
> +{
> + struct gdma_context *gc = pci_get_drvdata(pdev);
> + struct gdma_irq_context *gic;
> + int *irqs, *start_irqs, irq;
> + unsigned int cpu;
> + int err, i;
> +
> + cpus_read_lock();
> +
> + irqs = kmalloc_array(nvec, sizeof(int), GFP_KERNEL);
> + if (!irqs) {
> err = -ENOMEM;
> - goto free_irq_array;
> + goto free_irq_vector;
> }
>
> for (i = 0; i < nvec; i++) {
> - gic = &gc->irq_contexts[i];
> + gic = kzalloc(sizeof(*gic), GFP_KERNEL);
> + if (!gic) {
> + err = -ENOMEM;
> + goto free_irq;
> + }
> +
> gic->handler = mana_gd_process_eq_events;
> INIT_LIST_HEAD(&gic->eq_list);
> spin_lock_init(&gic->lock);
> @@ -1418,69 +1498,128 @@ static int mana_gd_setup_irqs(struct pci_dev *pdev)
> snprintf(gic->name, MANA_IRQ_NAME_SZ, "mana_q%d@pci:%s",
> i - 1, pci_name(pdev));
>
> - irq = pci_irq_vector(pdev, i);
> - if (irq < 0) {
> - err = irq;
> - goto free_irq;
> + irqs[i] = pci_irq_vector(pdev, i);
> + if (irqs[i] < 0) {
> + err = irqs[i];
> + goto free_current_gic;
> }
>
> - if (!i) {
> - err = request_irq(irq, mana_gd_intr, 0, gic->name, gic);
> - if (err)
> - goto free_irq;
> -
> - /* If number of IRQ is one extra than number of online CPUs,
> - * then we need to assign IRQ0 (hwc irq) and IRQ1 to
> - * same CPU.
> - * Else we will use different CPUs for IRQ0 and IRQ1.
> - * Also we are using cpumask_local_spread instead of
> - * cpumask_first for the node, because the node can be
> - * mem only.
> - */
> - if (start_irq_index) {
> - cpu = cpumask_local_spread(i, gc->numa_node);
> - irq_set_affinity_and_hint(irq, cpumask_of(cpu));
> - } else {
> - irqs[start_irq_index] = irq;
> - }
> - } else {
> - irqs[i - start_irq_index] = irq;
> - err = request_irq(irqs[i - start_irq_index], mana_gd_intr, 0,
> - gic->name, gic);
> - if (err)
> - goto free_irq;
> - }
> + err = request_irq(irqs[i], mana_gd_intr, 0, gic->name, gic);
> + if (err)
> + goto free_current_gic;
Jumping to free_current_gic will free start_irqs.
However, start_irqs isn't initialised until a few lines below.
Flagged by Smatch.
> +
> + xa_store(&gc->irq_contexts, i, gic, GFP_KERNEL);
> }
>
> - err = irq_setup(irqs, nvec - start_irq_index, gc->numa_node, false);
> + /* If number of IRQ is one extra than number of online CPUs,
> + * then we need to assign IRQ0 (hwc irq) and IRQ1 to
> + * same CPU.
> + * Else we will use different CPUs for IRQ0 and IRQ1.
> + * Also we are using cpumask_local_spread instead of
> + * cpumask_first for the node, because the node can be
> + * mem only.
> + */
> + start_irqs = irqs;
> + if (nvec > num_online_cpus()) {
> + cpu = cpumask_local_spread(0, gc->numa_node);
> + irq_set_affinity_and_hint(irqs[0], cpumask_of(cpu));
> + irqs++;
> + nvec -= 1;
> + }
> +
> + err = irq_setup(irqs, nvec, gc->numa_node, false);
> if (err)
> goto free_irq;
>
> - gc->max_num_msix = nvec;
> - gc->num_msix_usable = nvec;
> cpus_read_unlock();
> - kfree(irqs);
> + kfree(start_irqs);
> return 0;
>
> +free_current_gic:
> + kfree(gic);
> free_irq:
> - for (j = i - 1; j >= 0; j--) {
> - irq = pci_irq_vector(pdev, j);
> - gic = &gc->irq_contexts[j];
> + for (i -= 1; i >= 0; i--) {
> + irq = pci_irq_vector(pdev, i);
> + gic = xa_load(&gc->irq_contexts, i);
> + if (WARN_ON(!gic))
> + continue;
>
> irq_update_affinity_hint(irq, NULL);
> free_irq(irq, gic);
> + xa_erase(&gc->irq_contexts, i);
> + kfree(gic);
> }
>
> - kfree(gc->irq_contexts);
> - gc->irq_contexts = NULL;
> -free_irq_array:
> - kfree(irqs);
> + kfree(start_irqs);
> free_irq_vector:
> cpus_read_unlock();
> - pci_free_irq_vectors(pdev);
> return err;
> }
...
^ permalink raw reply
* Re: [PATCH v2 00/13] objtool: Detect and warn about indirect calls in __nocfi functions
From: Peter Zijlstra @ 2025-05-28 16:35 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Sean Christopherson, H. Peter Anvin, x86, kys, haiyangz, wei.liu,
decui, tglx, mingo, bp, dave.hansen, pbonzini, ardb, kees,
Arnd Bergmann, gregkh, linux-hyperv, linux-kernel, kvm, linux-efi,
samitolvanen, ojeda, xin
In-Reply-To: <20250528163035.GH31726@noisy.programming.kicks-ass.net>
On Wed, May 28, 2025 at 06:30:35PM +0200, Peter Zijlstra wrote:
> On Wed, May 28, 2025 at 09:44:52AM +0200, Peter Zijlstra wrote:
> > On Tue, May 06, 2025 at 12:18:49PM -0700, Josh Poimboeuf wrote:
> >
> > > Weird, I'm not seeing that.
> >
> > I Ate'nt Crazeh...
> >
> > https://lore.kernel.org/all/202505280410.2qfTQCRt-lkp@intel.com/T/#u
> >
> > I'll go poke at it, see if today is the day I can figure out WTF
> > happens.
>
> It manages to trip the CFI_UNDEFINED case in op->dest.reg == cfa->base
> in update_cfi_state().
>
> I figured it ought to tickle the regular 'mov %rbp, %rsp' case above
> there, but it doesn't, for some reason it has cfa.base == SP at this
> point.
>
> This happens... /me looks in scrollback ... at POP_REGS 'pop
> %rbp'. ARGH!!
>
>
> So the sequence of fail is:
>
> push %rbp
> mov %rsp, %rbp # cfa.base = BP
>
> SAVE
> ...
> push %rbp
> ...
> pop %rbp # cfa.base = SP
This is the POP !drap and dest==base case.
> ...
> mov %rbp, %rsp # UNDEF
> nop # FAIL
> RESTORE
>
> Note that the MOV+NOP is the 4 bytes ERETS needs.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox