* 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
* Re: [PATCH v2 00/13] objtool: Detect and warn about indirect calls in __nocfi functions
From: Peter Zijlstra @ 2025-05-28 16: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: <20250528074452.GU39944@noisy.programming.kicks-ass.net>
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
...
mov %rbp, %rsp # UNDEF
nop # FAIL
RESTORE
Note that the MOV+NOP is the 4 bytes ERETS needs.
^ permalink raw reply
* Re: [PATCH v4 5/5] net: mana: Allocate MSI-X vectors dynamically
From: Saurabh Singh Sengar @ 2025-05-28 8:16 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();
> - 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
^ permalink raw reply
* Re: [PATCH v2 00/13] objtool: Detect and warn about indirect calls in __nocfi functions
From: Peter Zijlstra @ 2025-05-28 7:44 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: <vukrlmb4kbpcol6rtest3tsw4y6obopbrwi5hcb5iwzogsopgt@sokysuzxvehi>
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.
^ permalink raw reply
* Re: [PATCH net-next,v2] net: mana: Add support for Multi Vports on Bare metal
From: patchwork-bot+netdevbpf @ 2025-05-28 6:40 UTC (permalink / raw)
To: Haiyang Zhang
Cc: linux-hyperv, netdev, decui, stephen, kys, paulros, olaf,
vkuznets, davem, wei.liu, edumazet, kuba, pabeni, leon, longli,
ssengar, linux-rdma, daniel, john.fastabend, bpf, ast, hawk, tglx,
shradhagupta, andrew+netdev, kotaranov, horms, linux-kernel
In-Reply-To: <1747671636-5810-1-git-send-email-haiyangz@microsoft.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Mon, 19 May 2025 09:20:36 -0700 you wrote:
> To support Multi Vports on Bare metal, increase the device config response
> version. And, skip the register HW vport, and register filter steps, when
> the Bare metal hostmode is set.
>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
> v2:
> Updated comments as suggested by ALOK TIWARI.
> Fixed the version check.
>
> [...]
Here is the summary with links:
- [net-next,v2] net: mana: Add support for Multi Vports on Bare metal
https://git.kernel.org/netdev/net-next/c/290e5d3c49f6
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [EXTERNAL] Re: [PATCH net-next,v2] net: mana: Add support for Multi Vports on Bare metal
From: Paolo Abeni @ 2025-05-28 6:27 UTC (permalink / raw)
To: Haiyang Zhang, Simon Horman
Cc: linux-hyperv@vger.kernel.org, netdev@vger.kernel.org, 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, linux-kernel@vger.kernel.org
In-Reply-To: <MN0PR21MB3437196CFAF4574776862F6ACA99A@MN0PR21MB3437.namprd21.prod.outlook.com>
On 5/22/25 4:51 PM, Haiyang Zhang wrote:
>> -----Original Message-----
>> From: Paolo Abeni <pabeni@redhat.com>
>> Sent: Thursday, May 22, 2025 9:44 AM
>> To: Simon Horman <horms@kernel.org>; Haiyang Zhang
>> <haiyangz@microsoft.com>
>
>>>>>> static int mana_query_device_cfg(struct mana_context *ac, u32
>>>>> proto_major_ver,
>>>>>> u32 proto_minor_ver, u32 proto_micro_ver,
>>>>>> - u16 *max_num_vports)
>>>>>> + u16 *max_num_vports, u8 *bm_hostmode)
>>>>>> {
>>>>>> struct gdma_context *gc = ac->gdma_dev->gdma_context;
>>>>>> struct mana_query_device_cfg_resp resp = {};
>>>>>> @@ -932,7 +932,7 @@ static int mana_query_device_cfg(struct
>> mana_context
>>>>> *ac, u32 proto_major_ver,
>>>>>> mana_gd_init_req_hdr(&req.hdr, MANA_QUERY_DEV_CONFIG,
>>>>>> sizeof(req), sizeof(resp));
>>>>>>
>>>>>> - req.hdr.resp.msg_version = GDMA_MESSAGE_V2;
>>>>>> + req.hdr.resp.msg_version = GDMA_MESSAGE_V3;
>>>>>>
>>>>>> req.proto_major_ver = proto_major_ver;
>>>>>> req.proto_minor_ver = proto_minor_ver;
>>>>>
>>>>>> @@ -956,11 +956,16 @@ static int mana_query_device_cfg(struct
>>>>> mana_context *ac, u32 proto_major_ver,
>>>>>>
>>>>>> *max_num_vports = resp.max_num_vports;
>>>>>>
>>>>>> - if (resp.hdr.response.msg_version == GDMA_MESSAGE_V2)
>>>>>> + if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V2)
>>>>>> gc->adapter_mtu = resp.adapter_mtu;
>>>>>> else
>>>>>> gc->adapter_mtu = ETH_FRAME_LEN;
>>>>>>
>>>>>> + if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V3)
>>>>>> + *bm_hostmode = resp.bm_hostmode;
>>>>>> + else
>>>>>> + *bm_hostmode = 0;
>>>>>
>>>>> Hi,
>>>>>
>>>>> Perhaps not strictly related to this patch, but I see
>>>>> that mana_verify_resp_hdr() is called a few lines above.
>>>>> And that verifies a minimum msg_version. But I do not see
>>>>> any verification of the maximum msg_version supported by the code.
>>>>>
>>>>> I am concerned about a hypothetical scenario where, say the as yet
>> unknown
>>>>> version 5 is sent as the version, and the above behaviour is used,
>> while
>>>>> not being correct.
>>>>>
>>>>> Could you shed some light on this?
>>>>>
>>>>
>>>> In driver, we specify the expected reply msg version is v3 here:
>>>> req.hdr.resp.msg_version = GDMA_MESSAGE_V3;
>>>>
>>>> If the HW side is upgraded, it won't send reply msg version higher
>>>> than expected, which may break the driver.
>>>
>>> Thanks,
>>>
>>> If I understand things correctly the HW side will honour the
>>> req.hdr.resp.msg_version and thus the SW won't receive anything
>>> it doesn't expect. Is that right?
>>
>> @Haiyang, if Simon's interpretation is correct, please change the
>> version checking in the driver from:
>>
>> if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V3)
>>
>> to
>> if (resp.hdr.response.msg_version == GDMA_MESSAGE_V3)
>>
>> As the current code is misleading.
>
> Simon:
> Yes, you are right. So newer HW can support older driver, and vice
> versa.
>
> Paolo:
> The MANA protocol doesn't remove any existing fields during upgrades.
>
> So (resp.hdr.response.msg_version >= GDMA_MESSAGE_V3) will continue
> to work in the future. If we change it to
> (resp.hdr.response.msg_version == GDMA_MESSAGE_V3),
> we will have to remember to update it to something like:
> (resp.hdr.response.msg_version >= GDMA_MESSAGE_V3 &&
> resp.hdr.response.msg_version <= GDMA_MESSAGE_V5),
> if the version is upgraded to v5 in the future. And keep on updating
> the checks on existing fields every time when the version is
> upgraded.
>
> So, can I keep the ">=" condition, to avoid future bug if anyone
> forget to update checks on all existing fields?
Ok, thanks for the clarification. fine by me.
/P
^ permalink raw reply
* [PATCH net-next,v6] net: mana: Add handler for hardware servicing events
From: Haiyang Zhang @ 2025-05-27 21:42 UTC (permalink / raw)
To: linux-hyperv, netdev
Cc: haiyangz, decui, stephen, kys, paulros, olaf, vkuznets, davem,
wei.liu, edumazet, kuba, pabeni, leon, longli, ssengar,
linux-rdma, daniel, john.fastabend, bpf, ast, hawk, tglx,
shradhagupta, andrew+netdev, kotaranov, horms, linux-kernel
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>
---
v6:
Not acquiring module refcnt as suggested by Paolo Abeni.
v5:
Get refcnt of the pdev struct to avoid removal before running the work
as suggested by Jakub Kicinski.
v4:
Renamed EQE type 135 to GDMA_EQE_HWC_RESET_REQUEST, since there can
be multiple cases of this reset request.
v3:
Updated for checkpatch warnings as suggested by Simon Horman.
v2:
Added dev_dbg for service type as suggested by Shradha Gupta.
Added driver cap bit.
---
.../net/ethernet/microsoft/mana/gdma_main.c | 67 +++++++++++++++++++
include/net/mana/gdma.h | 10 ++-
2 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 4ffaf7588885..999cf7f88d5d 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -352,11 +352,58 @@ void mana_gd_ring_cq(struct gdma_queue *cq, u8 arm_bit)
}
EXPORT_SYMBOL_NS(mana_gd_ring_cq, "NET_MANA");
+#define MANA_SERVICE_PERIOD 10
+
+struct mana_serv_work {
+ struct work_struct serv_work;
+ struct pci_dev *pdev;
+};
+
+static void mana_serv_func(struct work_struct *w)
+{
+ struct mana_serv_work *mns_wk;
+ struct pci_bus *bus, *parent;
+ struct pci_dev *pdev;
+
+ mns_wk = container_of(w, struct mana_serv_work, serv_work);
+ pdev = mns_wk->pdev;
+
+ pci_lock_rescan_remove();
+
+ if (!pdev)
+ goto out;
+
+ bus = pdev->bus;
+ if (!bus) {
+ dev_err(&pdev->dev, "MANA service: no bus\n");
+ goto out;
+ }
+
+ parent = bus->parent;
+ if (!parent) {
+ dev_err(&pdev->dev, "MANA service: no parent bus\n");
+ goto out;
+ }
+
+ pci_stop_and_remove_bus_device(bus->self);
+
+ msleep(MANA_SERVICE_PERIOD * 1000);
+
+ pci_rescan_bus(parent);
+
+out:
+ pci_unlock_rescan_remove();
+
+ pci_dev_put(pdev);
+ kfree(mns_wk);
+}
+
static void mana_gd_process_eqe(struct gdma_queue *eq)
{
u32 head = eq->head % (eq->queue_size / GDMA_EQE_SIZE);
struct gdma_context *gc = eq->gdma_dev->gdma_context;
struct gdma_eqe *eq_eqe_ptr = eq->queue_mem_ptr;
+ struct mana_serv_work *mns_wk;
union gdma_eqe_info eqe_info;
enum gdma_eqe_type type;
struct gdma_event event;
@@ -400,6 +447,26 @@ 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;
+ }
+
+ mns_wk = kzalloc(sizeof(*mns_wk), GFP_ATOMIC);
+ if (!mns_wk)
+ 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);
+ INIT_WORK(&mns_wk->serv_work, mana_serv_func);
+ schedule_work(&mns_wk->serv_work);
+ break;
+
default:
break;
}
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 228603bf03f2..150ab3610869 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -58,7 +58,7 @@ enum gdma_eqe_type {
GDMA_EQE_HWC_INIT_EQ_ID_DB = 129,
GDMA_EQE_HWC_INIT_DATA = 130,
GDMA_EQE_HWC_INIT_DONE = 131,
- GDMA_EQE_HWC_SOC_RECONFIG = 132,
+ GDMA_EQE_HWC_FPGA_RECONFIG = 132,
GDMA_EQE_HWC_SOC_RECONFIG_DATA = 133,
GDMA_EQE_RNIC_QP_FATAL = 176,
};
@@ -388,6 +388,8 @@ struct gdma_context {
u32 test_event_eq_id;
bool is_pf;
+ bool in_service;
+
phys_addr_t bar0_pa;
void __iomem *bar0_va;
void __iomem *shm_base;
@@ -558,12 +560,16 @@ enum {
/* Driver can handle holes (zeros) in the device list */
#define GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP BIT(11)
+/* Driver can self reset on FPGA Reconfig EQE notification */
+#define GDMA_DRV_CAP_FLAG_1_HANDLE_RECONFIG_EQE BIT(17)
+
#define GDMA_DRV_CAP_FLAGS1 \
(GDMA_DRV_CAP_FLAG_1_EQ_SHARING_MULTI_VPORT | \
GDMA_DRV_CAP_FLAG_1_NAPI_WKDONE_FIX | \
GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECONFIG | \
GDMA_DRV_CAP_FLAG_1_VARIABLE_INDIRECTION_TABLE_SUPPORT | \
- GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP)
+ GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP | \
+ GDMA_DRV_CAP_FLAG_1_HANDLE_RECONFIG_EQE)
#define GDMA_DRV_CAP_FLAGS2 0
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v4 3/5] net: mana: explain irq_setup() algorithm
From: Yury Norov @ 2025-05-27 19:10 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,
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: <1748361505-25513-1-git-send-email-shradhagupta@linux.microsoft.com>
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
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
* Re: [PATCH net-next v2 8/8] net: core: Convert dev_set_mac_address_user() to use struct sockaddr_storage
From: Kees Cook @ 2025-05-27 18:43 UTC (permalink / raw)
To: Paolo Abeni
Cc: Kuniyuki Iwashima, Willem de Bruijn, Jason Wang, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Simon Horman,
Stanislav Fomichev, Cosmin Ratiu, Vladimir Oltean,
Florian Fainelli, Kory Maincent, Maxim Georgiev, netdev,
Martin K. Petersen, Christoph Hellwig, Sagi Grimberg,
Chaitanya Kulkarni, Mike Christie, Max Gurtovoy,
Maurizio Lombardi, Dmitry Bogdanov, Mingzhe Zou, Christophe Leroy,
Dr. David Alan Gilbert, Gustavo A. R. Silva, Lei Yang,
Ido Schimmel, Samuel Mendoza-Jonas, Paul Fertser, Alexander Aring,
Stefan Schmidt, Miquel Raynal, Hayes Wang, Douglas Anderson,
Grant Grundler, Jay Vosburgh, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Jiri Pirko, Aleksander Jan Bajkowski,
Philipp Hahn, Eric Biggers, Ard Biesheuvel, Al Viro, Ahmed Zaki,
Alexander Lobakin, Xiao Liang, linux-kernel, linux-nvme,
linux-scsi, target-devel, linux-wpan, linux-usb, linux-hyperv,
linux-hardening
In-Reply-To: <e1429351-3c9b-40e0-b50d-de6527d0a05b@redhat.com>
On Tue, May 27, 2025 at 09:02:28AM +0200, Paolo Abeni wrote:
> On 5/21/25 10:46 PM, Kees Cook wrote:
> > diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
> > index fff13a8b48f1..616479e71466 100644
> > --- a/net/core/dev_ioctl.c
> > +++ b/net/core/dev_ioctl.c
> > @@ -572,9 +572,11 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
> > return dev_set_mtu(dev, ifr->ifr_mtu);
> >
> > case SIOCSIFHWADDR:
> > - if (dev->addr_len > sizeof(struct sockaddr))
> > + if (dev->addr_len > sizeof(ifr->ifr_hwaddr))
> > return -EINVAL;
> > - return dev_set_mac_address_user(dev, &ifr->ifr_hwaddr, NULL);
> > + return dev_set_mac_address_user(dev,
> > + (struct sockaddr_storage *)&ifr->ifr_hwaddr,
> > + NULL);
>
> Side note for a possible follow-up: the above pattern is repeated a
> couple of times: IMHO consolidating it into an helper would be nice.
Yeah, I will look at that.
> Also such helper could/should explicitly convert ifr->ifr_hwaddr to
> sockaddr_storage and avoid the cast.
It's UAPI, so it looked verrrry painful to change.
--
Kees Cook
^ permalink raw reply
* [PATCH v4 5/5] net: mana: Allocate MSI-X vectors dynamically
From: Shradha Gupta @ 2025-05-27 15:59 UTC (permalink / raw)
To: 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
Cc: Shradha Gupta, 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: <1748361453-25096-1-git-send-email-shradhagupta@linux.microsoft.com>
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);
free_irq_vector:
cpus_read_unlock();
- pci_free_irq_vectors(pdev);
return err;
}
+static int mana_gd_setup_hwc_irqs(struct pci_dev *pdev)
+{
+ struct gdma_context *gc = pci_get_drvdata(pdev);
+ unsigned int max_irqs, min_irqs;
+ int nvec, err;
+
+ if (pci_msix_can_alloc_dyn(pdev)) {
+ max_irqs = 1;
+ min_irqs = 1;
+ } else {
+ /* Need 1 interrupt for HWC */
+ max_irqs = min(num_online_cpus(), MANA_MAX_NUM_QUEUES) + 1;
+ min_irqs = 2;
+ }
+
+ nvec = pci_alloc_irq_vectors(pdev, min_irqs, max_irqs, PCI_IRQ_MSIX);
+ if (nvec < 0)
+ return nvec;
+
+ err = mana_gd_setup_irqs(pdev, nvec);
+ if (err) {
+ pci_free_irq_vectors(pdev);
+ return err;
+ }
+
+ gc->num_msix_usable = nvec;
+ gc->max_num_msix = nvec;
+
+ return 0;
+}
+
+static int mana_gd_setup_remaining_irqs(struct pci_dev *pdev)
+{
+ struct gdma_context *gc = pci_get_drvdata(pdev);
+ struct msi_map irq_map;
+ int max_irqs, i, err;
+
+ if (!pci_msix_can_alloc_dyn(pdev))
+ /* remain irqs are already allocated with HWC IRQ */
+ return 0;
+
+ /* allocate only remaining IRQs*/
+ max_irqs = gc->num_msix_usable - 1;
+
+ for (i = 1; i <= max_irqs; i++) {
+ irq_map = pci_msix_alloc_irq_at(pdev, i, NULL);
+ if (!irq_map.virq) {
+ err = irq_map.index;
+ /* caller will handle cleaning up all allocated
+ * irqs, after HWC is destroyed
+ */
+ return err;
+ }
+ }
+
+ err = mana_gd_setup_dyn_irqs(pdev, max_irqs);
+ if (err)
+ return err;
+
+ gc->max_num_msix = gc->max_num_msix + max_irqs;
+
+ return 0;
+}
+
static void mana_gd_remove_irqs(struct pci_dev *pdev)
{
struct gdma_context *gc = pci_get_drvdata(pdev);
@@ -1495,19 +1634,21 @@ static void mana_gd_remove_irqs(struct pci_dev *pdev)
if (irq < 0)
continue;
- gic = &gc->irq_contexts[i];
+ gic = xa_load(&gc->irq_contexts, i);
+ if (WARN_ON(!gic))
+ continue;
/* Need to clear the hint before free_irq */
irq_update_affinity_hint(irq, NULL);
free_irq(irq, gic);
+ xa_erase(&gc->irq_contexts, i);
+ kfree(gic);
}
pci_free_irq_vectors(pdev);
gc->max_num_msix = 0;
gc->num_msix_usable = 0;
- kfree(gc->irq_contexts);
- gc->irq_contexts = NULL;
}
static int mana_gd_setup(struct pci_dev *pdev)
@@ -1518,9 +1659,10 @@ static int mana_gd_setup(struct pci_dev *pdev)
mana_gd_init_registers(pdev);
mana_smc_init(&gc->shm_channel, gc->dev, gc->shm_base);
- err = mana_gd_setup_irqs(pdev);
+ err = mana_gd_setup_hwc_irqs(pdev);
if (err) {
- dev_err(gc->dev, "Failed to setup IRQs: %d\n", err);
+ dev_err(gc->dev, "Failed to setup IRQs for HWC creation: %d\n",
+ err);
return err;
}
@@ -1536,6 +1678,12 @@ static int mana_gd_setup(struct pci_dev *pdev)
if (err)
goto destroy_hwc;
+ err = mana_gd_setup_remaining_irqs(pdev);
+ if (err) {
+ dev_err(gc->dev, "Failed to setup remaining IRQs: %d", err);
+ goto destroy_hwc;
+ }
+
err = mana_gd_detect_devices(pdev);
if (err)
goto destroy_hwc;
@@ -1612,6 +1760,7 @@ static int mana_gd_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
gc->is_pf = mana_is_pf(pdev->device);
gc->bar0_va = bar0_va;
gc->dev = &pdev->dev;
+ xa_init(&gc->irq_contexts);
if (gc->is_pf)
gc->mana_pci_debugfs = debugfs_create_dir("0", mana_debugfs_root);
@@ -1640,6 +1789,7 @@ static int mana_gd_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
*/
debugfs_remove_recursive(gc->mana_pci_debugfs);
gc->mana_pci_debugfs = NULL;
+ xa_destroy(&gc->irq_contexts);
pci_iounmap(pdev, bar0_va);
free_gc:
pci_set_drvdata(pdev, NULL);
@@ -1664,6 +1814,8 @@ static void mana_gd_remove(struct pci_dev *pdev)
gc->mana_pci_debugfs = NULL;
+ xa_destroy(&gc->irq_contexts);
+
pci_iounmap(pdev, gc->bar0_va);
vfree(gc);
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 228603bf03f2..f20d1d1ea5e8 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -373,7 +373,7 @@ struct gdma_context {
unsigned int max_num_queues;
unsigned int max_num_msix;
unsigned int num_msix_usable;
- struct gdma_irq_context *irq_contexts;
+ struct xarray irq_contexts;
/* L2 MTU */
u16 adapter_mtu;
@@ -558,12 +558,16 @@ enum {
/* Driver can handle holes (zeros) in the device list */
#define GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP BIT(11)
+/* Driver supports dynamic MSI-X vector allocation */
+#define GDMA_DRV_CAP_FLAG_1_DYNAMIC_IRQ_ALLOC_SUPPORT BIT(13)
+
#define GDMA_DRV_CAP_FLAGS1 \
(GDMA_DRV_CAP_FLAG_1_EQ_SHARING_MULTI_VPORT | \
GDMA_DRV_CAP_FLAG_1_NAPI_WKDONE_FIX | \
GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECONFIG | \
GDMA_DRV_CAP_FLAG_1_VARIABLE_INDIRECTION_TABLE_SUPPORT | \
- GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP)
+ GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP | \
+ GDMA_DRV_CAP_FLAG_1_DYNAMIC_IRQ_ALLOC_SUPPORT)
#define GDMA_DRV_CAP_FLAGS2 0
--
2.34.1
^ permalink raw reply related
* [PATCH v4 4/5] net: mana: Allow irq_setup() to skip cpus for affinity
From: Shradha Gupta @ 2025-05-27 15:58 UTC (permalink / raw)
To: 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
Cc: Shradha Gupta, 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: <1748361453-25096-1-git-send-email-shradhagupta@linux.microsoft.com>
In order to prepare the MANA driver to allocate the MSI-X IRQs
dynamically, we need to enhance irq_setup() to allow skipping
affinitizing IRQs to the first CPU sibling group.
This would be for cases when the number of IRQs is less than or equal
to the number of online CPUs. In such cases for dynamically added IRQs
the first CPU sibling group would already be affinitized with HWC IRQ.
Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: Yury Norov [NVIDIA] <yury.norov@gmail.com>
---
Changes in v4
* fix commit description
* avoided using next_cpumask: label in the irq_setup()
---
drivers/net/ethernet/microsoft/mana/gdma_main.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index f9e8d4d1ba3a..763a548c4a2b 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -1329,7 +1329,8 @@ void mana_gd_free_res_map(struct gdma_resource *r)
* do the same thing.
*/
-static int irq_setup(unsigned int *irqs, unsigned int len, int node)
+static int irq_setup(unsigned int *irqs, unsigned int len, int node,
+ bool skip_first_cpu)
{
const struct cpumask *next, *prev = cpu_none_mask;
cpumask_var_t cpus __free(free_cpumask_var);
@@ -1344,11 +1345,18 @@ static int irq_setup(unsigned int *irqs, unsigned int len, int node)
while (weight > 0) {
cpumask_andnot(cpus, next, prev);
for_each_cpu(cpu, cpus) {
+ cpumask_andnot(cpus, cpus, topology_sibling_cpumask(cpu));
+ --weight;
+
+ if (unlikely(skip_first_cpu)) {
+ skip_first_cpu = false;
+ continue;
+ }
+
if (len-- == 0)
goto done;
+
irq_set_affinity_and_hint(*irqs++, topology_sibling_cpumask(cpu));
- cpumask_andnot(cpus, cpus, topology_sibling_cpumask(cpu));
- --weight;
}
}
prev = next;
@@ -1444,7 +1452,7 @@ static int mana_gd_setup_irqs(struct pci_dev *pdev)
}
}
- err = irq_setup(irqs, (nvec - start_irq_index), gc->numa_node);
+ err = irq_setup(irqs, nvec - start_irq_index, gc->numa_node, false);
if (err)
goto free_irq;
--
2.34.1
^ permalink raw reply related
* [PATCH v4 3/5] net: mana: explain irq_setup() algorithm
From: Shradha Gupta @ 2025-05-27 15:58 UTC (permalink / raw)
To: 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
Cc: Shradha Gupta, 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: <1748361453-25096-1-git-send-email-shradhagupta@linux.microsoft.com>
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 related
* [PATCH v4 2/5] PCI: hv: Allow dynamic MSI-X vector allocation
From: Shradha Gupta @ 2025-05-27 15:58 UTC (permalink / raw)
To: Bjorn Helgaas, Rob Herring, Manivannan Sadhasivam,
Krzysztof Wilczy�~Dski, Lorenzo Pieralisi, Dexuan Cui,
Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Michael Kelley
Cc: Shradha Gupta, 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: <1748361453-25096-1-git-send-email-shradhagupta@linux.microsoft.com>
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
^ permalink raw reply related
* [PATCH v4 1/5] PCI/MSI: Export pci_msix_prepare_desc() for dynamic MSI-X allocations
From: Shradha Gupta @ 2025-05-27 15:57 UTC (permalink / raw)
To: Jason Gunthorpe, Jonathan Cameron, Anna-Maria Behnsen,
Thomas Gleixner, Bjorn Helgaas, Michael Kelley
Cc: Shradha Gupta, 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: <1748361453-25096-1-git-send-email-shradhagupta@linux.microsoft.com>
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
^ permalink raw reply related
* [PATCH v4 0/5] Allow dyn MSI-X vector allocation of MANA
From: Shradha Gupta @ 2025-05-27 15:57 UTC (permalink / raw)
Cc: Shradha Gupta, 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, Simon Horman,
Leon Romanovsky, Maxim Levitsky, Erni Sri Satya Vennela,
Peter Zijlstra, netdev, linux-rdma, Paul Rosswurm, Shradha Gupta
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.
---
Change in v4
* add a patch describing the functionality of irq_setup() through a
comment
* In irq_setup(), avoid using a label next_cpumask:
* modify the changes in MANA patch about restructuring the error
handling path in mana_gd_setup_dyn_irqs()
* modify the mana_gd_setup_irqs() to simplify handling around
start_irq_index
* add warning if an invalid gic is returned
* place the xa_destroy() cleanup in mana_gd_remove
---
Changes in v3
* split the 3rd patch into preparation patch around irq_setup() and
changes in mana driver to allow dynamic IRQ allocation
* Add arm64 support for dynamic MSI-X allocation in pci_hyperv
controller
---
Changes in v2
* split the first patch into two(exporting the preapre_desc
func and using the function and flag in pci-hyperv)
* replace 'pci vectors' by 'MSI-X vectors'
* Change the cover letter description to align with changes made
---
Shradha Gupta (5):
PCI/MSI: Export pci_msix_prepare_desc() for dynamic MSI-X allocations
PCI: hv: Allow dynamic MSI-X vector allocation
net: mana: explain irq_setup() algorithm
net: mana: Allow irq_setup() to skip cpus for affinity
net: mana: Allocate MSI-X vectors dynamically
.../net/ethernet/microsoft/mana/gdma_main.c | 356 ++++++++++++++----
drivers/pci/controller/pci-hyperv.c | 5 +-
drivers/pci/msi/irqdomain.c | 5 +-
include/linux/msi.h | 2 +
include/net/mana/gdma.h | 8 +-
5 files changed, 293 insertions(+), 83 deletions(-)
--
2.34.1
^ permalink raw reply
* Re: [PATCH net-next,v5] net: mana: Add handler for hardware servicing events
From: Paolo Abeni @ 2025-05-27 9:28 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: <1747873343-3118-1-git-send-email-haiyangz@microsoft.com>
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.
/P
^ permalink raw reply
* Re: [PATCH RFC drm-misc-next v2 1/1] drm/hyperv: Add support for drm_panic
From: Jocelyn Falempe @ 2025-05-27 8:11 UTC (permalink / raw)
To: Ryosuke Yasuoka, drawat.floss, maarten.lankhorst, mripard,
tzimmermann, airlied, simona
Cc: linux-hyperv, linux-kernel, dri-devel
In-Reply-To: <20250526090117.80593-2-ryasuoka@redhat.com>
On 26/05/2025 11:01, Ryosuke Yasuoka wrote:
> Add drm_panic module for hyperv drm so that panic screen can be
> displayed on panic.
Thanks, it looks good to me.
Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
>
> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> ---
> drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 36 +++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> index f7d2e973f79e..945b9482bcb3 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
> @@ -17,6 +17,7 @@
> #include <drm/drm_gem_framebuffer_helper.h>
> #include <drm/drm_gem_shmem_helper.h>
> #include <drm/drm_probe_helper.h>
> +#include <drm/drm_panic.h>
> #include <drm/drm_plane.h>
>
> #include "hyperv_drm.h"
> @@ -181,10 +182,45 @@ static void hyperv_plane_atomic_update(struct drm_plane *plane,
> }
> }
>
> +static int hyperv_plane_get_scanout_buffer(struct drm_plane *plane,
> + struct drm_scanout_buffer *sb)
> +{
> + struct hyperv_drm_device *hv = to_hv(plane->dev);
> + struct iosys_map map = IOSYS_MAP_INIT_VADDR_IOMEM(hv->vram);
> +
> + if (plane->state && plane->state->fb) {
> + sb->format = plane->state->fb->format;
> + sb->width = plane->state->fb->width;
> + sb->height = plane->state->fb->height;
> + sb->pitch[0] = plane->state->fb->pitches[0];
> + sb->map[0] = map;
> + return 0;
> + }
> + return -ENODEV;
> +}
> +
> +static void hyperv_plane_panic_flush(struct drm_plane *plane)
> +{
> + struct hyperv_drm_device *hv = to_hv(plane->dev);
> + struct drm_rect rect;
> +
> + if (!plane->state || !plane->state->fb)
> + return;
> +
> + rect.x1 = 0;
> + rect.y1 = 0;
> + rect.x2 = plane->state->fb->width;
> + rect.y2 = plane->state->fb->height;
> +
> + hyperv_update_dirt(hv->hdev, &rect);
> +}
> +
> static const struct drm_plane_helper_funcs hyperv_plane_helper_funcs = {
> DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
> .atomic_check = hyperv_plane_atomic_check,
> .atomic_update = hyperv_plane_atomic_update,
> + .get_scanout_buffer = hyperv_plane_get_scanout_buffer,
> + .panic_flush = hyperv_plane_panic_flush,
> };
>
> static const struct drm_plane_funcs hyperv_plane_funcs = {
^ permalink raw reply
* Re: [PATCH net-next v2 0/8] net: Convert dev_set_mac_address() to struct sockaddr_storage
From: patchwork-bot+netdevbpf @ 2025-05-27 7:10 UTC (permalink / raw)
To: Kees Cook
Cc: kuniyu, willemdebruijn.kernel, martin.petersen, hch, sagi, kch,
davem, edumazet, kuba, pabeni, michael.christie, mgurtovoy,
mlombard, d.bogdanov, mingzhe.zou, christophe.leroy, horms, linux,
gustavoars, andrew+netdev, sdf, cratiu, leiyang, idosch, sam,
fercerpav, alex.aring, stefan, miquel.raynal, hayeswang, dianders,
grundler, jv, kys, haiyangz, wei.liu, decui, jiri, jasowang,
vladimir.oltean, florian.fainelli, kory.maincent, glipus, olek2,
phahn-oss, ebiggers, ardb, viro, ahmed.zaki, aleksander.lobakin,
shaw.leon, linux-kernel, linux-nvme, linux-scsi, target-devel,
netdev, linux-wpan, linux-usb, linux-hyperv, linux-hardening
In-Reply-To: <20250521204310.it.500-kees@kernel.org>
Hello:
This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Wed, 21 May 2025 13:46:08 -0700 you wrote:
> v2:
> - add conversion of dev_set_mac_address_user() (kuniyu)
> - fix missed sockaddr/sockaddr_storage conversion (kuba)
> v1: https://lore.kernel.org/all/20250520222452.work.063-kees@kernel.org/
>
> Hi,
>
> [...]
Here is the summary with links:
- [net-next,v2,1/8] net: core: Convert inet_addr_is_any() to sockaddr_storage
(no matching commit)
- [net-next,v2,2/8] net: core: Switch netif_set_mac_address() to struct sockaddr_storage
https://git.kernel.org/netdev/net-next/c/161972650d67
- [net-next,v2,3/8] net/ncsi: Use struct sockaddr_storage for pending_mac
https://git.kernel.org/netdev/net-next/c/db586cad6f45
- [net-next,v2,4/8] ieee802154: Use struct sockaddr_storage with dev_set_mac_address()
https://git.kernel.org/netdev/net-next/c/7da6117ea144
- [net-next,v2,5/8] net: usb: r8152: Convert to use struct sockaddr_storage internally
https://git.kernel.org/netdev/net-next/c/79deac8d538d
- [net-next,v2,6/8] net: core: Convert dev_set_mac_address() to struct sockaddr_storage
(no matching commit)
- [net-next,v2,7/8] rtnetlink: do_setlink: Use struct sockaddr_storage
https://git.kernel.org/netdev/net-next/c/6b12e0a3c3c9
- [net-next,v2,8/8] net: core: Convert dev_set_mac_address_user() to use struct sockaddr_storage
(no matching commit)
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 net-next v2 8/8] net: core: Convert dev_set_mac_address_user() to use struct sockaddr_storage
From: Paolo Abeni @ 2025-05-27 7:02 UTC (permalink / raw)
To: Kees Cook, Kuniyuki Iwashima
Cc: Willem de Bruijn, Jason Wang, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Simon Horman, Stanislav Fomichev,
Cosmin Ratiu, Vladimir Oltean, Florian Fainelli, Kory Maincent,
Maxim Georgiev, netdev, Martin K. Petersen, Christoph Hellwig,
Sagi Grimberg, Chaitanya Kulkarni, Mike Christie, Max Gurtovoy,
Maurizio Lombardi, Dmitry Bogdanov, Mingzhe Zou, Christophe Leroy,
Dr. David Alan Gilbert, Gustavo A. R. Silva, Lei Yang,
Ido Schimmel, Samuel Mendoza-Jonas, Paul Fertser, Alexander Aring,
Stefan Schmidt, Miquel Raynal, Hayes Wang, Douglas Anderson,
Grant Grundler, Jay Vosburgh, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Jiri Pirko, Aleksander Jan Bajkowski,
Philipp Hahn, Eric Biggers, Ard Biesheuvel, Al Viro, Ahmed Zaki,
Alexander Lobakin, Xiao Liang, linux-kernel, linux-nvme,
linux-scsi, target-devel, linux-wpan, linux-usb, linux-hyperv,
linux-hardening
In-Reply-To: <20250521204619.2301870-8-kees@kernel.org>
On 5/21/25 10:46 PM, Kees Cook wrote:
> diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
> index fff13a8b48f1..616479e71466 100644
> --- a/net/core/dev_ioctl.c
> +++ b/net/core/dev_ioctl.c
> @@ -572,9 +572,11 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
> return dev_set_mtu(dev, ifr->ifr_mtu);
>
> case SIOCSIFHWADDR:
> - if (dev->addr_len > sizeof(struct sockaddr))
> + if (dev->addr_len > sizeof(ifr->ifr_hwaddr))
> return -EINVAL;
> - return dev_set_mac_address_user(dev, &ifr->ifr_hwaddr, NULL);
> + return dev_set_mac_address_user(dev,
> + (struct sockaddr_storage *)&ifr->ifr_hwaddr,
> + NULL);
Side note for a possible follow-up: the above pattern is repeated a
couple of times: IMHO consolidating it into an helper would be nice.
Also such helper could/should explicitly convert ifr->ifr_hwaddr to
sockaddr_storage and avoid the cast.
/P
^ permalink raw reply
* RE: [PATCH hyperv-next v2 1/4] Documentation: hyperv: Confidential VMBus
From: Roman Kisel @ 2025-05-26 16:02 UTC (permalink / raw)
To: mhklinux
Cc: apais, arnd, benhill, bp, bperkins, catalin.marinas, corbet,
dave.hansen, decui, haiyangz, hpa, kys, linux-arch,
linux-arm-kernel, linux-doc, linux-hyperv, linux-kernel, mingo,
romank, sunilmut, tglx, wei.liu, will, x86
In-Reply-To: <SN6PR02MB4157DC69BA25D889CD838D04D49DA@SN6PR02MB4157.namprd02.prod.outlook.com>
> From: Roman Kisel <romank@linux.microsoft.com> Sent: Sunday, May 11, 2025 4:08 PM
>>
>> Define what the confidential VMBus is and describe what advantages
>> it offers on the capable hardware.
>>
>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>> ---
>> Documentation/virt/hyperv/vmbus.rst | 41 +++++++++++++++++++++++++++++
>> 1 file changed, 41 insertions(+)
>>
>> diff --git a/Documentation/virt/hyperv/vmbus.rst
>> b/Documentation/virt/hyperv/vmbus.rst
>> index 1dcef6a7fda3..ca2b948e5070 100644
>> --- a/Documentation/virt/hyperv/vmbus.rst
>> +++ b/Documentation/virt/hyperv/vmbus.rst
>> @@ -324,3 +324,44 @@ rescinded, neither Hyper-V nor Linux retains any state about
>> its previous existence. Such a device might be re-added later,
>> in which case it is treated as an entirely new device. See
>> vmbus_onoffer_rescind().
>> +
>> +Confidential VMBus
>> +------------------
>> +
>> +The confidential VMBus provides the control and data planes where
>> +the guest doesn't talk to either the hypervisor or the host. Instead,
>> +it relies on the trusted paravisor. The hardware (SNP or TDX) encrypts
>> +the guest memory and the register state also measuring the paravisor
>> +image via using the platform security processor to ensure trusted and
>> +confidential computing.
>> +
>> +To support confidential communication with the paravisor, a VMBus client
>> +will first attempt to use regular, non-isolated mechanisms for communication.
>> +To do this, it must:
>> +
>> +* Configure the paravisor SIMP with an encrypted page. The paravisor SIMP is
>> + configured by setting the relevant MSR directly, without using GHCB or tdcall.
>> +
>> +* Enable SINT 2 on both the paravisor and hypervisor, without setting the proxy
>> + flag on the paravisor SINT. Enable interrupts on the paravisor SynIC.
>> +
>> +* Configure both the paravisor and hypervisor event flags page.
>> + Both pages will need to be scanned when VMBus receives a channel interrupt.
>> +
>> +* Send messages to the paravisor by calling HvPostMessage directly, without using
>> + GHCB or tdcall.
>> +
>> +* Set the EOM MSR directly in the paravisor, without using GHCB or tdcall.
>> +
>> +If sending the InitiateContact message using non-isolated HvPostMessage fails,
>> +the client must fall back to using the hypervisor synic, by using the GHCB/tdcall
>> +as appropriate.
>> +
>> +To fall back, the client will have to reconfigure the following:
>> +
>> +* Configure the hypervisor SIMP with a host-visible page.
>> + Since the hypervisor SIMP is not used when in confidential mode,
>> + this can be done up front, or only when needed, whichever makes sense for
>> + the particular implementation.
>> +
>> +* Set the proxy flag on SINT 2 for the paravisor.
>
>I'm assuming there's no public documentation available for how Confidential
>VMBus works. If so, then this documentation needs to take a higher-level
>approach and explain the basic concepts. You've provided some nitty-gritty
>details about how to detect and enable Confidential VMBus, but I think that
>level of detail would be better as comments in the code.
>
>Here's an example of what I envision, with several embedded questions that
>need further explanation. Confidential VMBus is completely new to me, so
>I don't know the answers to the questions. I also think this documentation
>would be better added to the CoCo VM topic instead of the VMBus topic, as
>Confidential VMBus is an extension/enhancement to CoCo VMs that doesn't
>apply to normal VMs.
>
>------------------------------------------
>
>Confidential VMBus is an extension of Confidential Computing (CoCo) VMs
>(a.k.a. "Isolated" VMs in Hyper-V terminology). Without Confidential VMBus,
>guest VMBus device drivers (the "VSC"s in VMBus terminology) communicate
>with VMBus servers (the VSPs) running on the Hyper-V host. The
>communication must be through memory that has been decrypted so the
>host can access it. With Confidential VMBus, one or more of the VSPs reside
>in the trusted paravisor layer in the guest VM. Since the paravisor layer also
>operates in encrypted memory, the memory used for communication with
>such VSPs does not need to be decrypted and thereby exposed to the
>Hyper-V host. The paravisor is responsible for communicating securely
>with the Hyper-V host as necessary. [Does the paravisor do this in a way
>that is better than what the guest can do? This question seems to be core to
>the value prop for Confidential VMBus. I'm not really clear on the value
>prop.]
>
>A guest that is running with a paravisor must determine at runtime if
>Confidential VMBus is supported by the current paravisor. It does so by first
>trying to establish a Confidential VMBus connection with the paravisor using
>standard mechanisms where the memory remains encrypted. If this succeeds,
>then the guest can proceed to use Confidential VMBus. If it fails, then the
>guest must fallback to establishing a non-Confidential VMBus connection with
>the Hyper-V host.
>
>Confidential VMBus is a characteristic of the VMBus connection as a whole,
>and of each VMBus channel that is created. When a Confidential VMBus
>connection is established, the paravisor provides the guest the message-passing
>path that is used for VMBus device creation and deletion, and it provides a
>per-CPU synthetic interrupt controller (SynIC) just like the SyncIC that is
>offered by the Hyper-V host. Each VMBus device that is offered to the guest
>indicates the degree to which it participates in Confidential VMBus. The offer
>indicates if the device uses encrypted ring buffers, and if the device uses
>encrypted memory for DMA that is done outside the ring buffer. [Are these
>two settings independent? Could there be a device that has one set, and the
>other cleared? I'm having trouble understanding what such a mixed state
>would mean.] These settings may be different for different devices using
>the same Confidential VMBus connection.
>
>Because some devices on a Confidential VMBus may require decrypted ring
>buffers and DMA transfers, the guest must interact with two SynICs -- the
>one provided by the paravisor and the one provided by the Hyper-V host
>when Confidential VMBus is not offered. Interrupts are always signaled by
>the paravisor SynIC, but the guest must check for messages and for channel
>interrupts on both SynICs. [This requires some further explanation that I
>don't understand. What governs when a message arrives via the paravisor
>SynIC vs. the hypervisor SynIC, and when a VMBus channel indicates an
>interrupt in the paravisor SynIC event page vs. the hypervisor SynIC event
>page? And from looking at the code, it appears that the RelIDs assigned
>to channels are guaranteed to be unique within the guest VM, and not
>per-SynIC, but it would be good to confirm that.]
>
>[There are probably a few other topics to add a well.]
Michael,
Appreciate your help very much! I'll fill the gaps you've pointed out in
this patch and other ones.
--
Thank you,
Roman
^ permalink raw reply
* [PATCH RFC drm-misc-next v2 1/1] drm/hyperv: Add support for drm_panic
From: Ryosuke Yasuoka @ 2025-05-26 9:01 UTC (permalink / raw)
To: drawat.floss, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, jfalempe
Cc: Ryosuke Yasuoka, linux-hyperv, linux-kernel, dri-devel
In-Reply-To: <20250526090117.80593-1-ryasuoka@redhat.com>
Add drm_panic module for hyperv drm so that panic screen can be
displayed on panic.
Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
---
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 36 +++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
index f7d2e973f79e..945b9482bcb3 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_modeset.c
@@ -17,6 +17,7 @@
#include <drm/drm_gem_framebuffer_helper.h>
#include <drm/drm_gem_shmem_helper.h>
#include <drm/drm_probe_helper.h>
+#include <drm/drm_panic.h>
#include <drm/drm_plane.h>
#include "hyperv_drm.h"
@@ -181,10 +182,45 @@ static void hyperv_plane_atomic_update(struct drm_plane *plane,
}
}
+static int hyperv_plane_get_scanout_buffer(struct drm_plane *plane,
+ struct drm_scanout_buffer *sb)
+{
+ struct hyperv_drm_device *hv = to_hv(plane->dev);
+ struct iosys_map map = IOSYS_MAP_INIT_VADDR_IOMEM(hv->vram);
+
+ if (plane->state && plane->state->fb) {
+ sb->format = plane->state->fb->format;
+ sb->width = plane->state->fb->width;
+ sb->height = plane->state->fb->height;
+ sb->pitch[0] = plane->state->fb->pitches[0];
+ sb->map[0] = map;
+ return 0;
+ }
+ return -ENODEV;
+}
+
+static void hyperv_plane_panic_flush(struct drm_plane *plane)
+{
+ struct hyperv_drm_device *hv = to_hv(plane->dev);
+ struct drm_rect rect;
+
+ if (!plane->state || !plane->state->fb)
+ return;
+
+ rect.x1 = 0;
+ rect.y1 = 0;
+ rect.x2 = plane->state->fb->width;
+ rect.y2 = plane->state->fb->height;
+
+ hyperv_update_dirt(hv->hdev, &rect);
+}
+
static const struct drm_plane_helper_funcs hyperv_plane_helper_funcs = {
DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
.atomic_check = hyperv_plane_atomic_check,
.atomic_update = hyperv_plane_atomic_update,
+ .get_scanout_buffer = hyperv_plane_get_scanout_buffer,
+ .panic_flush = hyperv_plane_panic_flush,
};
static const struct drm_plane_funcs hyperv_plane_funcs = {
--
2.49.0
^ permalink raw reply related
* [PATCH RFC drm-misc-next v2 0/1] Add support for drm_panic
From: Ryosuke Yasuoka @ 2025-05-26 9:01 UTC (permalink / raw)
To: drawat.floss, maarten.lankhorst, mripard, tzimmermann, airlied,
simona, jfalempe
Cc: Ryosuke Yasuoka, linux-hyperv, linux-kernel, dri-devel
This patch adds drm_panic support for hyperv-drm driver. This function
works but it's still needed to brush up. Let me hear your opinions.
Once kernel panic occurs we expect to see a panic screen. However, to
see the screen, I need to close/re-open the graphic console client
window. As the panic screen shows correctly in the small preview
window in Hyper-V manager and debugfs API for drm_panic works correctly,
I think kernel needs to send signal to Hyper-V host that the console
client refreshes, but I have no idea what kind of signal is needed.
This patch is tested on Hyper-V 2022.
v2:
- Re-write codes with regular atomic helper. The driver was implemented
with simple KMS. So replace it with regular atomic helper in [1],
implement this feature on it.
[1] https://lore.kernel.org/all/20250427101825.812766-1-ryasuoka@redhat.com/
v1:
https://lore.kernel.org/lkml/20250402084351.1545536-1-ryasuoka@redhat.com/
Ryosuke Yasuoka (1):
drm/hyperv: Add support for drm_panic
drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 36 +++++++++++++++++++++
1 file changed, 36 insertions(+)
base-commit: c06cb85ad1412c6ff34792b028b2f89495761398
--
2.49.0
^ permalink raw reply
* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: Christoph Hellwig @ 2025-05-26 6:54 UTC (permalink / raw)
To: mhklinux
Cc: simona, deller, haiyangz, kys, wei.liu, decui, akpm, weh,
tzimmermann, hch, dri-devel, linux-fbdev, linux-kernel,
linux-hyperv, linux-mm
In-Reply-To: <20250523161522.409504-4-mhklinux@outlook.com>
On Fri, May 23, 2025 at 09:15:21AM -0700, mhkelley58@gmail.com wrote:
> Commit 37b4837959cb ("video: deferred io with physically contiguous
> memory") from the year 2008 purported to add support for contiguous
> kernel memory framebuffers. The motivating device, sh_mobile_lcdcfb, uses
> dma_alloc_coherent() to allocate framebuffer memory, which is likely to
> use alloc_pages(). It's unclear to me how this commit actually worked at
> the time, unless dma_alloc_coherent() was pulling from a CMA pool instead
> of alloc_pages(). Or perhaps alloc_pages() worked differently or on the
> arm32 architecture on which sh_mobile_lcdcfb is used.
>
> In any case, for x86 and arm64 today, commit 37b4837959cb9 is not
> sufficient to support contiguous kernel memory framebuffers. The problem
> can be seen with the hyperv_fb driver, which may allocate the framebuffer
> memory using vmalloc() or alloc_pages(), depending on the configuration
> of the Hyper-V guest VM (Gen 1 vs. Gen 2) and the size of the framebuffer.
That langugage is far too nice. The existing users of fb_defio are
all gravely broken because they violate the dma API restriction to
not poke into the memory. You can't speculate what you get from
dma_alloc_coherent and it can change behind you all the time.
> Fix this limitation by adding defio support for contiguous kernel memory
> framebuffers. A driver with a framebuffer allocated from contiguous
> kernel memory must set the FBINFO_KMEMFB flag to indicate such.
Honestly, the right thing is to invert the flag. What hypervs is doing
here - take kernel memory in the direct mapping or from vmalloc is fine.
What others drivers do it completely broken crap. So add a flag
FBINFO_BROKEN_CRAP to maybe keep the guessing. Or just disable it
because it's dangerous.
^ permalink raw reply
* Re: [PATCH v3 13/13] KVM: selftests: Add a KVM_IRQFD test to verify uniqueness requirements
From: Sairaj Kodilkar @ 2025-05-26 3:36 UTC (permalink / raw)
To: Sean Christopherson
Cc: 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, linux-kernel, linux-hyperv, xen-devel,
kvm, linux-kselftest, linux-arm-kernel, kvmarm, K Prateek Nayak,
David Matlack
In-Reply-To: <aDB-2lcq4jJm9-OV@google.com>
On 5/23/2025 8:03 PM, Sean Christopherson wrote:
> On Fri, May 23, 2025, Sairaj Kodilkar wrote:
>> On 5/23/2025 5:22 AM, Sean Christopherson wrote:
>>
>>> +
>>> +int main(int argc, char *argv[])
>>> +{
>>> + pthread_t racing_thread;
>>> + int r, i;
>>> +
>>> + /* Create "full" VMs, as KVM_IRQFD requires an in-kernel IRQ chip. */
>>> + vm1 = vm_create(1);
>>> + vm2 = vm_create(1);
>>> +
>>> + WRITE_ONCE(__eventfd, kvm_new_eventfd());
>>> +
>>> + kvm_irqfd(vm1, 10, __eventfd, 0);
>>> +
>>> + r = __kvm_irqfd(vm1, 11, __eventfd, 0);
>>> + TEST_ASSERT(r && errno == EBUSY,
>>> + "Wanted EBUSY, r = %d, errno = %d", r, errno);
>>> +
>>> + r = __kvm_irqfd(vm2, 12, __eventfd, 0);
>>> + TEST_ASSERT(r && errno == EBUSY,
>>> + "Wanted EBUSY, r = %d, errno = %d", r, errno);
>>> +
>>> + kvm_irqfd(vm1, 11, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
>>> + kvm_irqfd(vm1, 12, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
>>> + kvm_irqfd(vm1, 13, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
>>> + kvm_irqfd(vm1, 14, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
>>
>> Hi Sean,
>> I dont see any allocation for the GSI 13 and 14..
>> Is there any reason for the deassigning these two GSIs ?
>
> Yes, KVM's rather bizarre ABI is that DEASSIGN is allowed even if the VM doesn't
> have a corresponding assigned irqfd. The reason I added these early DEASSIGN
> calls is so that there will be an easier-to-debug failure if KVM's behavior
> changes (the racing threads part of the test abuses KVM's ABI). I didn't add a
> comment because the helpers already have comments, but looking at this again, I
> agree that main() needs a better comment.
Makes sense, thanks for the explanation.
Thanks
Sairaj Kodilkar
^ permalink raw reply
* Re: [PATCH v3 06/13] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
From: Ricardo Neri @ 2025-05-24 15:56 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski
Cc: 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: <20250519175606.GA9693@ranerica-svr.sc.intel.com>
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.
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.
Rob, Krzysztof, would that be acceptable?
Thanks and BR,
Ricardo
[2]. https://www.intel.com/content/www/us/en/content-details/733585/intel-tdx-virtual-firmware-design-guide.html
^ 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