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.
Fixes: a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI")
Signed-off-by: Dexuan Cui <[email protected]>
Cc: Jeffrey Hugo <[email protected]>
Cc: Carl Vanderlip <[email protected]>
---
The patch should be appplied after the earlier patch:
[PATCH] PCI: hv: Only reuse existing IRTE allocation for Multi-MSI
https://lwn.net/ml/linux-kernel/20220804025104.15673-1-decui%40microsoft.com/
drivers/pci/controller/pci-hyperv.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 65d0dab25deb..53580899c859 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, 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(struct cpumask *affinity)
static u32 hv_compose_msi_req_v2(
struct pci_create_interrupt2 *int_pkt, 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, struct cpumask *affinity,
- u32 slot, u32 vector, u8 vector_count)
+ u32 slot, u32 vector, u16 vector_count)
{
int cpu;
@@ -1702,7 +1702,8 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
struct tran_int_desc *int_desc;
struct msi_desc *msi_desc;
bool multi_msi;
- u8 vector, vector_count;
+ u32 vector; /* Must be u32: see the struct hv_msi_desc3 */
+ u16 vector_count;
struct {
struct pci_packet pci_pkt;
union {
--
2.25.1
> From: Bjorn Helgaas <[email protected]>
> Sent: Monday, August 15, 2022 1:36 PM
> To: Dexuan Cui <[email protected]>
>
> s/definiton/definition/ in subject
> (only if you have other occasion to repost this)
Thanks, Bjorn! I suppose Wei can help fix this. :-)
> On Mon, Aug 15, 2022 at 11:55:05AM -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.
> >
> > Fixes: a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI")
> > Signed-off-by: Dexuan Cui <[email protected]>
> > Cc: Jeffrey Hugo <[email protected]>
> > Cc: Carl Vanderlip <[email protected]>
>
> Looks like Wei has been applying most changes to pci-hyperv.c, so I
> assume the same will happen here.
So I interpret this as an ack for Wei to apply the earlier patch
[PATCH] PCI: hv: Only reuse existing IRTE allocation for Multi-MSI
and this patch.
The two small patches are pure Hyper-V specific changes, so IMO it's
better for them to go through Wei's Hyper-V tree rather than the PCI tree.
(It looks like the PCI folks are too busy right now)
> > ---
> >
> > The patch should be appplied after the earlier patch:
> > [PATCH] PCI: hv: Only reuse existing IRTE allocation for Multi-MSI
s/definiton/definition/ in subject
(only if you have other occasion to repost this)
On Mon, Aug 15, 2022 at 11:55:05AM -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.
>
> Fixes: a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI")
> Signed-off-by: Dexuan Cui <[email protected]>
> Cc: Jeffrey Hugo <[email protected]>
> Cc: Carl Vanderlip <[email protected]>
Looks like Wei has been applying most changes to pci-hyperv.c, so I
assume the same will happen here.
> ---
>
> The patch should be appplied after the earlier patch:
> [PATCH] PCI: hv: Only reuse existing IRTE allocation for Multi-MSI
> https://lwn.net/ml/linux-kernel/20220804025104.15673-1-decui%40microsoft.com/
>
> drivers/pci/controller/pci-hyperv.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 65d0dab25deb..53580899c859 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, 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(struct cpumask *affinity)
>
> static u32 hv_compose_msi_req_v2(
> struct pci_create_interrupt2 *int_pkt, 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, struct cpumask *affinity,
> - u32 slot, u32 vector, u8 vector_count)
> + u32 slot, u32 vector, u16 vector_count)
> {
> int cpu;
>
> @@ -1702,7 +1702,8 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> struct tran_int_desc *int_desc;
> struct msi_desc *msi_desc;
> bool multi_msi;
> - u8 vector, vector_count;
> + u32 vector; /* Must be u32: see the struct hv_msi_desc3 */
> + u16 vector_count;
> struct {
> struct pci_packet pci_pkt;
> union {
> --
> 2.25.1
>
On 8/15/2022 12:55 PM, 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.
>
> Fixes: a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI")
> Signed-off-by: Dexuan Cui <[email protected]>
> Cc: Jeffrey Hugo <[email protected]>
> Cc: Carl Vanderlip <[email protected]>
> ---
>
> The patch should be appplied after the earlier patch:
> [PATCH] PCI: hv: Only reuse existing IRTE allocation for Multi-MSI
> https://lwn.net/ml/linux-kernel/20220804025104.15673-1-decui%40microsoft.com/
>
> drivers/pci/controller/pci-hyperv.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 65d0dab25deb..53580899c859 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, 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(struct cpumask *affinity)
>
> static u32 hv_compose_msi_req_v2(
> struct pci_create_interrupt2 *int_pkt, 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, struct cpumask *affinity,
> - u32 slot, u32 vector, u8 vector_count)
> + u32 slot, u32 vector, u16 vector_count)
> {
> int cpu;
>
> @@ -1702,7 +1702,8 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> struct tran_int_desc *int_desc;
> struct msi_desc *msi_desc;
> bool multi_msi;
> - u8 vector, vector_count;
> + u32 vector; /* Must be u32: see the struct hv_msi_desc3 */
Don't you need to cast this down to a u8 for v1 and v2?
Feels like this should be generating a compiler warning...
> From: Jeffrey Hugo <[email protected]>
> Sent: Tuesday, August 16, 2022 9:01 AM
> > ...
> > @@ -1702,7 +1702,8 @@ static void hv_compose_msi_msg(struct irq_data
> *data, struct msi_msg *msg)
> > struct tran_int_desc *int_desc;
> > struct msi_desc *msi_desc;
> > bool multi_msi;
> > - u8 vector, vector_count;
> > + u32 vector; /* Must be u32: see the struct hv_msi_desc3 */
>
> Don't you need to cast this down to a u8 for v1 and v2?
> Feels like this should be generating a compiler warning...
My gcc 9.4.0 didn't generate a warning.
hv_compose_msi_req_v3() is for both ARM64 and x86. In the case of ARM64
the 'vector' can be a u32 integer according to the comment around struct
hv_msi_desc3.
hv_compose_msi_req_v1 and v2 are for x86 only, and the 'vector' can't be
longer than u8. I can post a v2 with the extra changes below:
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 53580899c859..c7fd76bc8b4c 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -1703,7 +1703,7 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
struct msi_desc *msi_desc;
bool multi_msi;
u32 vector; /* Must be u32: see the struct hv_msi_desc3 */
- u16 vector_count;
+ u16 vector_count; /* see hv_msi_desc, hv_msi_desc2 and hv_msi_desc3 */
struct {
struct pci_packet pci_pkt;
union {
@@ -1788,12 +1788,17 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
ctxt.pci_pkt.completion_func = hv_pci_compose_compl;
ctxt.pci_pkt.compl_ctxt = ∁
+ /*
+ * hv_compose_msi_req_v1 and v2 are for x86 only, meaning 'vector'
+ * can't be longer than u8. Cast 'vector' down to u8 explicitly for
+ * better readability.
+ */
switch (hbus->protocol_version) {
case PCI_PROTOCOL_VERSION_1_1:
size = hv_compose_msi_req_v1(&ctxt.int_pkts.v1,
dest,
hpdev->desc.win_slot.slot,
- vector,
+ (u8)vector,
vector_count);
break;
@@ -1802,7 +1807,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,
+ (u8)vector,
vector_count);
break;
Thanks,
-- Dexuan
On Mon, Aug 15, 2022 at 03:35:45PM -0500, Bjorn Helgaas wrote:
> s/definiton/definition/ in subject
> (only if you have other occasion to repost this)
>
> On Mon, Aug 15, 2022 at 11:55:05AM -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.
> >
> > Fixes: a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI")
> > Signed-off-by: Dexuan Cui <[email protected]>
> > Cc: Jeffrey Hugo <[email protected]>
> > Cc: Carl Vanderlip <[email protected]>
>
> Looks like Wei has been applying most changes to pci-hyperv.c, so I
> assume the same will happen here.
I can take care of this one via hyperv-fixes, but ...
>
> > ---
> >
> > The patch should be appplied after the earlier patch:
> > [PATCH] PCI: hv: Only reuse existing IRTE allocation for Multi-MSI
> > https://lwn.net/ml/linux-kernel/20220804025104.15673-1-decui%40microsoft.com/
> >
... this patch looks to be rejected.
Thanks,
Wei.
> From: Wei Liu <[email protected]>
> Sent: Friday, August 19, 2022 8:53 AM
> To: Bjorn Helgaas <[email protected]>
>
> On Mon, Aug 15, 2022 at 03:35:45PM -0500, Bjorn Helgaas wrote:
> > s/definiton/definition/ in subject
> > (only if you have other occasion to repost this)
> >
> > On Mon, Aug 15, 2022 at 11:55:05AM -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.
> > >
> > > Fixes: a2bad844a67b ("PCI: hv: Fix interrupt mapping for multi-MSI")
> > > Signed-off-by: Dexuan Cui <[email protected]>
> > > Cc: Jeffrey Hugo <[email protected]>
> > > Cc: Carl Vanderlip <[email protected]>
> >
> > Looks like Wei has been applying most changes to pci-hyperv.c, so I
> > assume the same will happen here.
>
> I can take care of this one via hyperv-fixes, but ...
Wei, please ignore this patch. I'll post v2 of this patch with v2 of the other patch.
> > > ---
> > >
> > > The patch should be appplied after the earlier patch:
> > > [PATCH] PCI: hv: Only reuse existing IRTE allocation for Multi-MSI
> > >
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.ne
> t%2Fml%2Flinux-kernel%2F20220804025104.15673-1-decui%2540microsoft.co
> m%2F&data=05%7C01%7Cdecui%40microsoft.com%7Cc8ab6638b7d747b
> cddf008da81fadc12%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6
> 37965211688628404%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD
> AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&
> amp;sdata=Rb2MfkSFmJ%2B8ze%2FllN0THBhODCtmnZ8oSMB0EOn20u4%3D&
> amp;reserved=0
> > >
>
> ... this patch looks to be rejected.
Correct. I'm working on a new version.
>
> Thanks,
> Wei.