[Cc stable]
On Thu, Oct 27, 2022 at 01:52:56PM -0700, Dexuan Cui wrote:
The local variable 'vector' must be u32 rather than u8: see the struct hv_msi_desc3.
'vector_count' should be u16 rather than u8: see struct hv_msi_desc, hv_msi_desc2 and hv_msi_desc3.
Dexuan, I think this patch should only be in 5.15, because...
Fixes: a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI")
^^^ this commit is already in 5.15.y (commit id 92dcb50f7f09).
Upstream id e70af8d040d2b7904dca93d942ba23fb722e21b1 Cc: stable@vger.kernel.org # 5.15.x
Regards, Boqun
Signed-off-by: Dexuan Cui decui@microsoft.com Cc: Jeffrey Hugo quic_jhugo@quicinc.com Cc: Carl Vanderlip quic_carlv@quicinc.com
v1 was posted here (sorry, I forgot to follow this up...): https://lwn.net/ml/linux-kernel/20220815185505.7626-1-decui@microsoft.com/
Changes in v2: Added the explicit "(u8)" cast in hv_compose_msi_msg(). Added and improved the comments. Fixed a typo in the subject in v1: s/definiton/definition
drivers/pci/controller/pci-hyperv.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c index e7c6f6629e7c..ba64284eaf9f 100644 --- a/drivers/pci/controller/pci-hyperv.c +++ b/drivers/pci/controller/pci-hyperv.c @@ -1614,7 +1614,7 @@ static void hv_pci_compose_compl(void *context, struct pci_response *resp, static u32 hv_compose_msi_req_v1( struct pci_create_interrupt *int_pkt, const struct cpumask *affinity,
- u32 slot, u8 vector, u8 vector_count)
- u32 slot, u8 vector, u16 vector_count)
{ int_pkt->message_type.type = PCI_CREATE_INTERRUPT_MESSAGE; int_pkt->wslot.slot = slot; @@ -1642,7 +1642,7 @@ static int hv_compose_msi_req_get_cpu(const struct cpumask *affinity) static u32 hv_compose_msi_req_v2( struct pci_create_interrupt2 *int_pkt, const struct cpumask *affinity,
- u32 slot, u8 vector, u8 vector_count)
- u32 slot, u8 vector, u16 vector_count)
{ int cpu; @@ -1661,7 +1661,7 @@ static u32 hv_compose_msi_req_v2( static u32 hv_compose_msi_req_v3( struct pci_create_interrupt3 *int_pkt, const struct cpumask *affinity,
- u32 slot, u32 vector, u8 vector_count)
- u32 slot, u32 vector, u16 vector_count)
{ int cpu; @@ -1701,7 +1701,12 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) struct compose_comp_ctxt comp; struct tran_int_desc *int_desc; struct msi_desc *msi_desc;
- u8 vector, vector_count;
- /*
* vector_count should be u16: see hv_msi_desc, hv_msi_desc2
* and hv_msi_desc3. vector must be u32: see hv_msi_desc3.
*/
- u16 vector_count;
- u32 vector; struct { struct pci_packet pci_pkt; union {
@@ -1767,6 +1772,11 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) vector_count = 1; }
- /*
* hv_compose_msi_req_v1 and v2 are for x86 only, meaning 'vector'
* can't exceed u8. Cast 'vector' down to u8 for v1/v2 explicitly
* for better readability.
memset(&ctxt, 0, sizeof(ctxt)); init_completion(&comp.comp_pkt.host_event); ctxt.pci_pkt.completion_func = hv_pci_compose_compl;*/
@@ -1777,7 +1787,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) size = hv_compose_msi_req_v1(&ctxt.int_pkts.v1, dest, hpdev->desc.win_slot.slot,
vector,
break;(u8)vector, vector_count);
@@ -1786,7 +1796,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) size = hv_compose_msi_req_v2(&ctxt.int_pkts.v2, dest, hpdev->desc.win_slot.slot,
vector,
break;(u8)vector, vector_count);
2.25.1
On Wed, Mar 29, 2023 at 06:56:12PM -0700, Boqun Feng wrote:
[Cc stable]
On Thu, Oct 27, 2022 at 01:52:56PM -0700, Dexuan Cui wrote:
The local variable 'vector' must be u32 rather than u8: see the struct hv_msi_desc3.
'vector_count' should be u16 rather than u8: see struct hv_msi_desc, hv_msi_desc2 and hv_msi_desc3.
Dexuan, I think this patch should only be in 5.15, because...
Sorry, I meant:
"this patch should also be backported in 5.15"
Regards, Boqun
Fixes: a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI")
^^^ this commit is already in 5.15.y (commit id 92dcb50f7f09).
Upstream id e70af8d040d2b7904dca93d942ba23fb722e21b1 Cc: stable@vger.kernel.org # 5.15.x
Regards, Boqun
Signed-off-by: Dexuan Cui decui@microsoft.com Cc: Jeffrey Hugo quic_jhugo@quicinc.com Cc: Carl Vanderlip quic_carlv@quicinc.com
v1 was posted here (sorry, I forgot to follow this up...): https://lwn.net/ml/linux-kernel/20220815185505.7626-1-decui@microsoft.com/
Changes in v2: Added the explicit "(u8)" cast in hv_compose_msi_msg(). Added and improved the comments. Fixed a typo in the subject in v1: s/definiton/definition
drivers/pci/controller/pci-hyperv.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c index e7c6f6629e7c..ba64284eaf9f 100644 --- a/drivers/pci/controller/pci-hyperv.c +++ b/drivers/pci/controller/pci-hyperv.c @@ -1614,7 +1614,7 @@ static void hv_pci_compose_compl(void *context, struct pci_response *resp, static u32 hv_compose_msi_req_v1( struct pci_create_interrupt *int_pkt, const struct cpumask *affinity,
- u32 slot, u8 vector, u8 vector_count)
- u32 slot, u8 vector, u16 vector_count)
{ int_pkt->message_type.type = PCI_CREATE_INTERRUPT_MESSAGE; int_pkt->wslot.slot = slot; @@ -1642,7 +1642,7 @@ static int hv_compose_msi_req_get_cpu(const struct cpumask *affinity) static u32 hv_compose_msi_req_v2( struct pci_create_interrupt2 *int_pkt, const struct cpumask *affinity,
- u32 slot, u8 vector, u8 vector_count)
- u32 slot, u8 vector, u16 vector_count)
{ int cpu; @@ -1661,7 +1661,7 @@ static u32 hv_compose_msi_req_v2( static u32 hv_compose_msi_req_v3( struct pci_create_interrupt3 *int_pkt, const struct cpumask *affinity,
- u32 slot, u32 vector, u8 vector_count)
- u32 slot, u32 vector, u16 vector_count)
{ int cpu; @@ -1701,7 +1701,12 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) struct compose_comp_ctxt comp; struct tran_int_desc *int_desc; struct msi_desc *msi_desc;
- u8 vector, vector_count;
- /*
* vector_count should be u16: see hv_msi_desc, hv_msi_desc2
* and hv_msi_desc3. vector must be u32: see hv_msi_desc3.
*/
- u16 vector_count;
- u32 vector; struct { struct pci_packet pci_pkt; union {
@@ -1767,6 +1772,11 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) vector_count = 1; }
- /*
* hv_compose_msi_req_v1 and v2 are for x86 only, meaning 'vector'
* can't exceed u8. Cast 'vector' down to u8 for v1/v2 explicitly
* for better readability.
memset(&ctxt, 0, sizeof(ctxt)); init_completion(&comp.comp_pkt.host_event); ctxt.pci_pkt.completion_func = hv_pci_compose_compl;*/
@@ -1777,7 +1787,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) size = hv_compose_msi_req_v1(&ctxt.int_pkts.v1, dest, hpdev->desc.win_slot.slot,
vector,
break;(u8)vector, vector_count);
@@ -1786,7 +1796,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) size = hv_compose_msi_req_v2(&ctxt.int_pkts.v2, dest, hpdev->desc.win_slot.slot,
vector,
break;(u8)vector, vector_count);
2.25.1
From: Boqun Feng boqun.feng@gmail.com Sent: Wednesday, March 29, 2023 7:56 PM To: Dexuan Cui decui@microsoft.com ... On Wed, Mar 29, 2023 at 06:56:12PM -0700, Boqun Feng wrote:
[Cc stable]
On Thu, Oct 27, 2022 at 01:52:56PM -0700, Dexuan Cui wrote:
The local variable 'vector' must be u32 rather than u8: see the struct hv_msi_desc3.
'vector_count' should be u16 rather than u8: see struct hv_msi_desc, hv_msi_desc2 and hv_msi_desc3.
Dexuan, I think this patch should only be in 5.15, because...
Sorry, I meant:
"this patch should also be backported in 5.15"
Regards, Boqun
Fixes: a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI")
^^^ this commit is already in 5.15.y (commit id 92dcb50f7f09).
Upstream id e70af8d040d2b7904dca93d942ba23fb722e21b1 Cc: stable@vger.kernel.org # 5.15.x
The faulty commit a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI") is in all the stable branches, even including 4.14.y, so yes, the commit e70af8d040d2 ("PCI: hv: Fix the definition of vector in hv_compose_msi_msg()") should be backported to all the stable branches as well, including v5.15.y, v5.10.y, v5.4.y, v4.19.y, v4.14.y.
e70af8d040d2 has a Fixes tag. Not sure why it's not automatically backported.
On Thu, Mar 30, 2023 at 03:23:45AM +0000, Dexuan Cui wrote:
From: Boqun Feng boqun.feng@gmail.com Sent: Wednesday, March 29, 2023 7:56 PM To: Dexuan Cui decui@microsoft.com ... On Wed, Mar 29, 2023 at 06:56:12PM -0700, Boqun Feng wrote:
[Cc stable]
On Thu, Oct 27, 2022 at 01:52:56PM -0700, Dexuan Cui wrote:
The local variable 'vector' must be u32 rather than u8: see the struct hv_msi_desc3.
'vector_count' should be u16 rather than u8: see struct hv_msi_desc, hv_msi_desc2 and hv_msi_desc3.
Dexuan, I think this patch should only be in 5.15, because...
Sorry, I meant:
"this patch should also be backported in 5.15"
Regards, Boqun
Fixes: a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI")
^^^ this commit is already in 5.15.y (commit id 92dcb50f7f09).
Upstream id e70af8d040d2b7904dca93d942ba23fb722e21b1 Cc: stable@vger.kernel.org # 5.15.x
The faulty commit a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI") is in all the stable branches, even including 4.14.y, so yes, the commit e70af8d040d2 ("PCI: hv: Fix the definition of vector in hv_compose_msi_msg()") should be backported to all the stable branches as well, including v5.15.y, v5.10.y, v5.4.y, v4.19.y, v4.14.y.
e70af8d040d2 has a Fixes tag. Not sure why it's not automatically backported.
Because "Fixes:" is not the flag that we are sure to trigger off of. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly.
That being said, because some subsystem maintainers do NOT put cc: stable in their patches, we do sometimes sweep the tree and try to pick up things with only "Fixes:" but we don't always catch everything.
So if you want to be sure a patch is applied, please always add a cc: stable in the patch.
thanks,
greg k-h
From: Greg Kroah-Hartman gregkh@linuxfoundation.org
... e70af8d040d2 has a Fixes tag. Not sure why it's not automatically
backported.
Because "Fixes:" is not the flag that we are sure to trigger off of. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly.
Thanks, I just read this again to refresh my memory :-) I remember Sasha has an AI algorithm to pick up patches into the stable tree and a "Fixes" tag should be a strong indicator.
I tired to manually cherry-pick e70af8d040d2 to 5.15.y and got some merge conflicts. Probably that's why Sasha's script didn't automatically do the backport. @Carl, @Jeffrey, may I know if you have some cycles to help backport e70af8d040d2?
That being said, because some subsystem maintainers do NOT put cc: stable in their patches, we do sometimes sweep the tree and try to pick up things with only "Fixes:" but we don't always catch everything.
So if you want to be sure a patch is applied, please always add a cc: stable in the patch.
greg k-h
Hi Greg, since you're here, I'd like to ask a question:
If I add the cc: stable line in a patch and use git-send-email to post the patch, git-send-email also posts the patch to the stable list -- is this acceptable? Sometimes a patch may have to undergo multiple revisions, meaning all the discussion emails go to the stable list unnecessarily, and I guess this is not good?
It looks like there is no git-send-email option to exclude an email.
Thanks, Dexuan
On Thu, Mar 30, 2023 at 07:50:11PM +0000, Dexuan Cui wrote:
From: Greg Kroah-Hartman gregkh@linuxfoundation.org
... e70af8d040d2 has a Fixes tag. Not sure why it's not automatically
backported.
Because "Fixes:" is not the flag that we are sure to trigger off of. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly.
Thanks, I just read this again to refresh my memory :-) I remember Sasha has an AI algorithm to pick up patches into the stable tree and a "Fixes" tag should be a strong indicator.
Yes, we have to rely on "hints" like that due to maintainers not wanting to put any cc: stable tags for many subsystems so we have to dig them out somehow.
If I add the cc: stable line in a patch and use git-send-email to post the patch, git-send-email also posts the patch to the stable list -- is this acceptable?
Totally acceptable.
Sometimes a patch may have to undergo multiple revisions, meaning all the discussion emails go to the stable list unnecessarily, and I guess this is not good?
It's not a problem at all, happens all the time and in fact I like it as it gives us a heads-up that a patch is going to be eventually merged for us to handle.
It looks like there is no git-send-email option to exclude an email.
No need to, don't worry about that at all.
thanks,
greg k-h
On 3/30/2023 1:50 PM, Dexuan Cui wrote:
From: Greg Kroah-Hartman gregkh@linuxfoundation.org
... e70af8d040d2 has a Fixes tag. Not sure why it's not automatically
backported.
Because "Fixes:" is not the flag that we are sure to trigger off of. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly.
Thanks, I just read this again to refresh my memory :-) I remember Sasha has an AI algorithm to pick up patches into the stable tree and a "Fixes" tag should be a strong indicator.
I tired to manually cherry-pick e70af8d040d2 to 5.15.y and got some merge conflicts. Probably that's why Sasha's script didn't automatically do the backport. @Carl, @Jeffrey, may I know if you have some cycles to help backport e70af8d040d2?
We might have some cycles this week.
-Jeff
From: Jeffrey Hugo quic_jhugo@quicinc.com Sent: Monday, April 3, 2023 11:51 AM
On 3/30/2023 1:50 PM, Dexuan Cui wrote:
... I tired to manually cherry-pick e70af8d040d2 to 5.15.y and got some merge conflicts. Probably that's why Sasha's script didn't automatically do the backport. @Carl, @Jeffrey, may I know if you have some cycles to help backport e70af8d040d2?
We might have some cycles this week.
-Jeff
Thanks, Jeff. We already have Jose on our team working on the backport. If you think you can do the backport sooner, please let us know so we can avoid duplicate efforts. :-)
On Thu, Mar 30, 2023 at 03:23:45AM +0000, Dexuan Cui wrote:
From: Boqun Feng boqun.feng@gmail.com Sent: Wednesday, March 29, 2023 7:56 PM To: Dexuan Cui decui@microsoft.com ... On Wed, Mar 29, 2023 at 06:56:12PM -0700, Boqun Feng wrote:
[Cc stable]
On Thu, Oct 27, 2022 at 01:52:56PM -0700, Dexuan Cui wrote:
The local variable 'vector' must be u32 rather than u8: see the struct hv_msi_desc3.
'vector_count' should be u16 rather than u8: see struct hv_msi_desc, hv_msi_desc2 and hv_msi_desc3.
Dexuan, I think this patch should only be in 5.15, because...
Sorry, I meant:
"this patch should also be backported in 5.15"
Regards, Boqun
Fixes: a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI")
^^^ this commit is already in 5.15.y (commit id 92dcb50f7f09).
Upstream id e70af8d040d2b7904dca93d942ba23fb722e21b1 Cc: stable@vger.kernel.org # 5.15.x
The faulty commit a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI") is in all the stable branches, even including 4.14.y, so yes, the commit e70af8d040d2 ("PCI: hv: Fix the definition of vector in hv_compose_msi_msg()") should be backported to all the stable branches as well, including v5.15.y, v5.10.y, v5.4.y, v4.19.y, v4.14.y.
e70af8d040d2 has a Fixes tag. Not sure why it's not automatically backported.
Also, the most obvious reason, it does NOT apply there! If this needs to go to 5.15.y and older, please send working backports of it.
thanks,
greg k-h
From: Greg Kroah-Hartman gregkh@linuxfoundation.org Sent: Monday, April 3, 2023 6:08 AM To: Dexuan Cui decui@microsoft.com Cc: Boqun Feng boqun.feng@gmail.com; quic_jhugo@quicinc.com; quic_carlv@quicinc.com; wei.liu@kernel.org; KY Srinivasan kys@microsoft.com; Haiyang Zhang haiyangz@microsoft.com; sthemmin@microsoft.com; lpieralisi@kernel.org; bhelgaas@google.com; linux-hyperv@vger.kernel.org; linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org; Michael Kelley (LINUX) mikelley@microsoft.com; robh@kernel.org; kw@linux.com; helgaas@kernel.org; alex.williamson@redhat.com; stable@vger.kernel.org; Sasha Levin sashal@kernel.org Subject: Re: [PATCH v2] PCI: hv: Fix the definition of vector in hv_compose_msi_msg()
On Thu, Mar 30, 2023 at 03:23:45AM +0000, Dexuan Cui wrote:
From: Boqun Feng boqun.feng@gmail.com Sent: Wednesday, March 29, 2023 7:56 PM To: Dexuan Cui decui@microsoft.com ... On Wed, Mar 29, 2023 at 06:56:12PM -0700, Boqun Feng wrote:
[Cc stable]
On Thu, Oct 27, 2022 at 01:52:56PM -0700, Dexuan Cui wrote:
The local variable 'vector' must be u32 rather than u8: see the struct hv_msi_desc3.
'vector_count' should be u16 rather than u8: see struct hv_msi_desc, hv_msi_desc2 and hv_msi_desc3.
Dexuan, I think this patch should only be in 5.15, because...
Sorry, I meant:
"this patch should also be backported in 5.15"
Regards, Boqun
Fixes: a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI")
^^^ this commit is already in 5.15.y (commit id 92dcb50f7f09).
Upstream id e70af8d040d2b7904dca93d942ba23fb722e21b1 Cc: stable@vger.kernel.org # 5.15.x
The faulty commit a2bad844a67b ("PCI: hv: Fix interrupt mapping for
multi-MSI")
is in all the stable branches, even including 4.14.y, so yes, the commit e70af8d040d2 ("PCI: hv: Fix the definition of vector in
hv_compose_msi_msg()")
should be backported to all the stable branches as well, including v5.15.y, v5.10.y, v5.4.y, v4.19.y, v4.14.y.
e70af8d040d2 has a Fixes tag. Not sure why it's not automatically
backported.
Also, the most obvious reason, it does NOT apply there! If this needs to go to 5.15.y and older, please send working backports of it.
thanks,
greg k-h
I also found that we need to manually rebase the patch for 5.15.y and older. +Jose, who will help to do the backport.
linux-stable-mirror@lists.linaro.org