| Message ID | 1651068453-29588-1-git-send-email-quic_jhugo@quicinc.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: hv: Fix hv_arch_irq_unmask() for multi-MSI | expand |
From: Jeffrey Hugo <quic_jhugo@quicinc.com> Sent: Wednesday, April 27, 2022 7:08 AM > > In the multi-MSI case, hv_arch_irq_unmask() will only operate on the first > MSI of the N allocated. This is because only the first msi_desc is cached > and it is shared by all the MSIs of the multi-MSI block. This means that > hv_arch_irq_unmask() gets the correct address, but the wrong data (always > 0). > > This can break MSIs. > > Lets assume MSI0 is vector 34 on CPU0, and MSI1 is vector 33 on CPU0. > > hv_arch_irq_unmask() is called on MSI0. It uses a hypercall to configure > the MSI address and data (0) to vector 34 of CPU0. This is correct. Then > hv_arch_irq_unmask is called on MSI1. It uses another hypercall to > configure the MSI address and data (0) to vector 33 of CPU0. This is > wrong, and results in both MSI0 and MSI1 being routed to vector 33. Linux > will observe extra instances of MSI1 and no instances of MSI0 despite the > endpoint device behaving correctly. > > For the multi-MSI case, we need unique address and data info for each MSI, > but the cached msi_desc does not provide that. However, that information > can be gotten from the int_desc cached in the chip_data by > compose_msi_msg(). Fix the multi-MSI case to use that cached information > instead. Since hv_set_msi_entry_from_desc() is no longer applicable, > remove it. > > Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> > --- > drivers/pci/controller/pci-hyperv.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c > index 5800ecf..7aea0b7 100644 > --- a/drivers/pci/controller/pci-hyperv.c > +++ b/drivers/pci/controller/pci-hyperv.c > @@ -611,13 +611,6 @@ static unsigned int hv_msi_get_int_vector(struct irq_data > *data) > return cfg->vector; > } > > -static void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry, > - struct msi_desc *msi_desc) > -{ > - msi_entry->address.as_uint32 = msi_desc->msg.address_lo; > - msi_entry->data.as_uint32 = msi_desc->msg.data; > -} > - > static int hv_msi_prepare(struct irq_domain *domain, struct device *dev, > int nvec, msi_alloc_info_t *info) > { > @@ -647,6 +640,7 @@ static void hv_arch_irq_unmask(struct irq_data *data) > { > struct msi_desc *msi_desc = irq_data_get_msi_desc(data); > struct hv_retarget_device_interrupt *params; > + struct tran_int_desc *int_desc; > struct hv_pcibus_device *hbus; > struct cpumask *dest; > cpumask_var_t tmp; > @@ -661,6 +655,7 @@ static void hv_arch_irq_unmask(struct irq_data *data) > pdev = msi_desc_to_pci_dev(msi_desc); > pbus = pdev->bus; > hbus = container_of(pbus->sysdata, struct hv_pcibus_device, sysdata); > + int_desc = data->chip_data; > > spin_lock_irqsave(&hbus->retarget_msi_interrupt_lock, flags); > > @@ -668,7 +663,8 @@ static void hv_arch_irq_unmask(struct irq_data *data) > memset(params, 0, sizeof(*params)); > params->partition_id = HV_PARTITION_ID_SELF; > params->int_entry.source = HV_INTERRUPT_SOURCE_MSI; > - hv_set_msi_entry_from_desc(¶ms->int_entry.msi_entry, msi_desc); > + params->int_entry.msi_entry.address.as_uint32 = int_desc->address & > 0xffffffff; > + params->int_entry.msi_entry.data.as_uint32 = int_desc->data; > params->device_id = (hbus->hdev->dev_instance.b[5] << 24) | > (hbus->hdev->dev_instance.b[4] << 16) | > (hbus->hdev->dev_instance.b[7] << 8) | > -- > 2.7.4 Reviewed-by: Michael Kelley <mikelley@microsoft.com>
On Wed, Apr 27, 2022 at 08:07:33AM -0600, Jeffrey Hugo wrote: > In the multi-MSI case, hv_arch_irq_unmask() will only operate on the first > MSI of the N allocated. This is because only the first msi_desc is cached > and it is shared by all the MSIs of the multi-MSI block. This means that > hv_arch_irq_unmask() gets the correct address, but the wrong data (always > 0). > > This can break MSIs. > > Lets assume MSI0 is vector 34 on CPU0, and MSI1 is vector 33 on CPU0. > > hv_arch_irq_unmask() is called on MSI0. It uses a hypercall to configure > the MSI address and data (0) to vector 34 of CPU0. This is correct. Then > hv_arch_irq_unmask is called on MSI1. It uses another hypercall to > configure the MSI address and data (0) to vector 33 of CPU0. This is > wrong, and results in both MSI0 and MSI1 being routed to vector 33. Linux > will observe extra instances of MSI1 and no instances of MSI0 despite the > endpoint device behaving correctly. > > For the multi-MSI case, we need unique address and data info for each MSI, > but the cached msi_desc does not provide that. However, that information > can be gotten from the int_desc cached in the chip_data by > compose_msi_msg(). Fix the multi-MSI case to use that cached information > instead. Since hv_set_msi_entry_from_desc() is no longer applicable, > remove it. > > Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> > --- > drivers/pci/controller/pci-hyperv.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c > index 5800ecf..7aea0b7 100644 > --- a/drivers/pci/controller/pci-hyperv.c > +++ b/drivers/pci/controller/pci-hyperv.c > @@ -611,13 +611,6 @@ static unsigned int hv_msi_get_int_vector(struct irq_data *data) > return cfg->vector; > } > > -static void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry, > - struct msi_desc *msi_desc) > -{ > - msi_entry->address.as_uint32 = msi_desc->msg.address_lo; > - msi_entry->data.as_uint32 = msi_desc->msg.data; > -} > - Instead of dropping this function, can you change the second argument to take struct tran_int_desc *? This way you can use the same function in hv_compose_msi_msg. Thanks, Wei. > static int hv_msi_prepare(struct irq_domain *domain, struct device *dev, > int nvec, msi_alloc_info_t *info) > { > @@ -647,6 +640,7 @@ static void hv_arch_irq_unmask(struct irq_data *data) > { > struct msi_desc *msi_desc = irq_data_get_msi_desc(data); > struct hv_retarget_device_interrupt *params; > + struct tran_int_desc *int_desc; > struct hv_pcibus_device *hbus; > struct cpumask *dest; > cpumask_var_t tmp; > @@ -661,6 +655,7 @@ static void hv_arch_irq_unmask(struct irq_data *data) > pdev = msi_desc_to_pci_dev(msi_desc); > pbus = pdev->bus; > hbus = container_of(pbus->sysdata, struct hv_pcibus_device, sysdata); > + int_desc = data->chip_data; > > spin_lock_irqsave(&hbus->retarget_msi_interrupt_lock, flags); > > @@ -668,7 +663,8 @@ static void hv_arch_irq_unmask(struct irq_data *data) > memset(params, 0, sizeof(*params)); > params->partition_id = HV_PARTITION_ID_SELF; > params->int_entry.source = HV_INTERRUPT_SOURCE_MSI; > - hv_set_msi_entry_from_desc(¶ms->int_entry.msi_entry, msi_desc); > + params->int_entry.msi_entry.address.as_uint32 = int_desc->address & 0xffffffff; > + params->int_entry.msi_entry.data.as_uint32 = int_desc->data; > params->device_id = (hbus->hdev->dev_instance.b[5] << 24) | > (hbus->hdev->dev_instance.b[4] << 16) | > (hbus->hdev->dev_instance.b[7] << 8) | > -- > 2.7.4 >
On 4/28/2022 8:58 AM, Wei Liu wrote: > On Wed, Apr 27, 2022 at 08:07:33AM -0600, Jeffrey Hugo wrote: >> In the multi-MSI case, hv_arch_irq_unmask() will only operate on the first >> MSI of the N allocated. This is because only the first msi_desc is cached >> and it is shared by all the MSIs of the multi-MSI block. This means that >> hv_arch_irq_unmask() gets the correct address, but the wrong data (always >> 0). >> >> This can break MSIs. >> >> Lets assume MSI0 is vector 34 on CPU0, and MSI1 is vector 33 on CPU0. >> >> hv_arch_irq_unmask() is called on MSI0. It uses a hypercall to configure >> the MSI address and data (0) to vector 34 of CPU0. This is correct. Then >> hv_arch_irq_unmask is called on MSI1. It uses another hypercall to >> configure the MSI address and data (0) to vector 33 of CPU0. This is >> wrong, and results in both MSI0 and MSI1 being routed to vector 33. Linux >> will observe extra instances of MSI1 and no instances of MSI0 despite the >> endpoint device behaving correctly. >> >> For the multi-MSI case, we need unique address and data info for each MSI, >> but the cached msi_desc does not provide that. However, that information >> can be gotten from the int_desc cached in the chip_data by >> compose_msi_msg(). Fix the multi-MSI case to use that cached information >> instead. Since hv_set_msi_entry_from_desc() is no longer applicable, >> remove it. >> >> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> >> --- >> drivers/pci/controller/pci-hyperv.c | 12 ++++-------- >> 1 file changed, 4 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c >> index 5800ecf..7aea0b7 100644 >> --- a/drivers/pci/controller/pci-hyperv.c >> +++ b/drivers/pci/controller/pci-hyperv.c >> @@ -611,13 +611,6 @@ static unsigned int hv_msi_get_int_vector(struct irq_data *data) >> return cfg->vector; >> } >> >> -static void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry, >> - struct msi_desc *msi_desc) >> -{ >> - msi_entry->address.as_uint32 = msi_desc->msg.address_lo; >> - msi_entry->data.as_uint32 = msi_desc->msg.data; >> -} >> - > > Instead of dropping this function, can you change the second argument to > take struct tran_int_desc *? > > This way you can use the same function in hv_compose_msi_msg. I do not see how this could be reused in hv_compose_msi_msg() with the proposed change of the second argument. The hv_msi_entry type is not used in hv_compose_msi_msg(), nor does it look like it is applicable anywhere within the function. What am I missing? > Thanks, > Wei. > >> static int hv_msi_prepare(struct irq_domain *domain, struct device *dev, >> int nvec, msi_alloc_info_t *info) >> { >> @@ -647,6 +640,7 @@ static void hv_arch_irq_unmask(struct irq_data *data) >> { >> struct msi_desc *msi_desc = irq_data_get_msi_desc(data); >> struct hv_retarget_device_interrupt *params; >> + struct tran_int_desc *int_desc; >> struct hv_pcibus_device *hbus; >> struct cpumask *dest; >> cpumask_var_t tmp; >> @@ -661,6 +655,7 @@ static void hv_arch_irq_unmask(struct irq_data *data) >> pdev = msi_desc_to_pci_dev(msi_desc); >> pbus = pdev->bus; >> hbus = container_of(pbus->sysdata, struct hv_pcibus_device, sysdata); >> + int_desc = data->chip_data; >> >> spin_lock_irqsave(&hbus->retarget_msi_interrupt_lock, flags); >> >> @@ -668,7 +663,8 @@ static void hv_arch_irq_unmask(struct irq_data *data) >> memset(params, 0, sizeof(*params)); >> params->partition_id = HV_PARTITION_ID_SELF; >> params->int_entry.source = HV_INTERRUPT_SOURCE_MSI; >> - hv_set_msi_entry_from_desc(¶ms->int_entry.msi_entry, msi_desc); >> + params->int_entry.msi_entry.address.as_uint32 = int_desc->address & 0xffffffff; >> + params->int_entry.msi_entry.data.as_uint32 = int_desc->data; >> params->device_id = (hbus->hdev->dev_instance.b[5] << 24) | >> (hbus->hdev->dev_instance.b[4] << 16) | >> (hbus->hdev->dev_instance.b[7] << 8) | >> -- >> 2.7.4 >>
On Thu, Apr 28, 2022 at 02:58:24PM +0000, Wei Liu wrote: > On Wed, Apr 27, 2022 at 08:07:33AM -0600, Jeffrey Hugo wrote: > > In the multi-MSI case, hv_arch_irq_unmask() will only operate on the first > > MSI of the N allocated. This is because only the first msi_desc is cached > > and it is shared by all the MSIs of the multi-MSI block. This means that > > hv_arch_irq_unmask() gets the correct address, but the wrong data (always > > 0). > > > > This can break MSIs. > > > > Lets assume MSI0 is vector 34 on CPU0, and MSI1 is vector 33 on CPU0. > > > > hv_arch_irq_unmask() is called on MSI0. It uses a hypercall to configure > > the MSI address and data (0) to vector 34 of CPU0. This is correct. Then > > hv_arch_irq_unmask is called on MSI1. It uses another hypercall to > > configure the MSI address and data (0) to vector 33 of CPU0. This is > > wrong, and results in both MSI0 and MSI1 being routed to vector 33. Linux > > will observe extra instances of MSI1 and no instances of MSI0 despite the > > endpoint device behaving correctly. > > > > For the multi-MSI case, we need unique address and data info for each MSI, > > but the cached msi_desc does not provide that. However, that information > > can be gotten from the int_desc cached in the chip_data by > > compose_msi_msg(). Fix the multi-MSI case to use that cached information > > instead. Since hv_set_msi_entry_from_desc() is no longer applicable, > > remove it. > > > > Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> > > --- > > drivers/pci/controller/pci-hyperv.c | 12 ++++-------- > > 1 file changed, 4 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c > > index 5800ecf..7aea0b7 100644 > > --- a/drivers/pci/controller/pci-hyperv.c > > +++ b/drivers/pci/controller/pci-hyperv.c > > @@ -611,13 +611,6 @@ static unsigned int hv_msi_get_int_vector(struct irq_data *data) > > return cfg->vector; > > } > > > > -static void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry, > > - struct msi_desc *msi_desc) > > -{ > > - msi_entry->address.as_uint32 = msi_desc->msg.address_lo; > > - msi_entry->data.as_uint32 = msi_desc->msg.data; > > -} > > - > > Instead of dropping this function, can you change the second argument to > take struct tran_int_desc *? > > This way you can use the same function in hv_compose_msi_msg. > Never mind. I think I mixed things up. Sorry for the noise.
On Thu, Apr 28, 2022 at 09:06:42AM -0600, Jeffrey Hugo wrote: > On 4/28/2022 8:58 AM, Wei Liu wrote: > > On Wed, Apr 27, 2022 at 08:07:33AM -0600, Jeffrey Hugo wrote: > > > In the multi-MSI case, hv_arch_irq_unmask() will only operate on the first > > > MSI of the N allocated. This is because only the first msi_desc is cached > > > and it is shared by all the MSIs of the multi-MSI block. This means that > > > hv_arch_irq_unmask() gets the correct address, but the wrong data (always > > > 0). > > > > > > This can break MSIs. > > > > > > Lets assume MSI0 is vector 34 on CPU0, and MSI1 is vector 33 on CPU0. > > > > > > hv_arch_irq_unmask() is called on MSI0. It uses a hypercall to configure > > > the MSI address and data (0) to vector 34 of CPU0. This is correct. Then > > > hv_arch_irq_unmask is called on MSI1. It uses another hypercall to > > > configure the MSI address and data (0) to vector 33 of CPU0. This is > > > wrong, and results in both MSI0 and MSI1 being routed to vector 33. Linux > > > will observe extra instances of MSI1 and no instances of MSI0 despite the > > > endpoint device behaving correctly. > > > > > > For the multi-MSI case, we need unique address and data info for each MSI, > > > but the cached msi_desc does not provide that. However, that information > > > can be gotten from the int_desc cached in the chip_data by > > > compose_msi_msg(). Fix the multi-MSI case to use that cached information > > > instead. Since hv_set_msi_entry_from_desc() is no longer applicable, > > > remove it. > > > > > > Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> > > > --- > > > drivers/pci/controller/pci-hyperv.c | 12 ++++-------- > > > 1 file changed, 4 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c > > > index 5800ecf..7aea0b7 100644 > > > --- a/drivers/pci/controller/pci-hyperv.c > > > +++ b/drivers/pci/controller/pci-hyperv.c > > > @@ -611,13 +611,6 @@ static unsigned int hv_msi_get_int_vector(struct irq_data *data) > > > return cfg->vector; > > > } > > > -static void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry, > > > - struct msi_desc *msi_desc) > > > -{ > > > - msi_entry->address.as_uint32 = msi_desc->msg.address_lo; > > > - msi_entry->data.as_uint32 = msi_desc->msg.data; > > > -} > > > - > > > > Instead of dropping this function, can you change the second argument to > > take struct tran_int_desc *? > > > > This way you can use the same function in hv_compose_msi_msg. > > I do not see how this could be reused in hv_compose_msi_msg() with the > proposed change of the second argument. The hv_msi_entry type is not used > in hv_compose_msi_msg(), nor does it look like it is applicable anywhere > within the function. > > What am I missing? I mixed up two different types while going through the code -- hv_msi_entry and Linux's own msi_entry type. Sorry for the noise. Wei.
On Wed, Apr 27, 2022 at 05:11:39PM +0000, Michael Kelley (LINUX) wrote: > From: Jeffrey Hugo <quic_jhugo@quicinc.com> Sent: Wednesday, April 27, 2022 7:08 AM [...] > > (hbus->hdev->dev_instance.b[7] << 8) | > > -- > > 2.7.4 > > Reviewed-by: Michael Kelley <mikelley@microsoft.com> > Applied to hyperv-next. Thanks.
On 4/28/2022 9:08 AM, Wei Liu wrote: > On Thu, Apr 28, 2022 at 09:06:42AM -0600, Jeffrey Hugo wrote: >> On 4/28/2022 8:58 AM, Wei Liu wrote: >>> On Wed, Apr 27, 2022 at 08:07:33AM -0600, Jeffrey Hugo wrote: >>>> In the multi-MSI case, hv_arch_irq_unmask() will only operate on the first >>>> MSI of the N allocated. This is because only the first msi_desc is cached >>>> and it is shared by all the MSIs of the multi-MSI block. This means that >>>> hv_arch_irq_unmask() gets the correct address, but the wrong data (always >>>> 0). >>>> >>>> This can break MSIs. >>>> >>>> Lets assume MSI0 is vector 34 on CPU0, and MSI1 is vector 33 on CPU0. >>>> >>>> hv_arch_irq_unmask() is called on MSI0. It uses a hypercall to configure >>>> the MSI address and data (0) to vector 34 of CPU0. This is correct. Then >>>> hv_arch_irq_unmask is called on MSI1. It uses another hypercall to >>>> configure the MSI address and data (0) to vector 33 of CPU0. This is >>>> wrong, and results in both MSI0 and MSI1 being routed to vector 33. Linux >>>> will observe extra instances of MSI1 and no instances of MSI0 despite the >>>> endpoint device behaving correctly. >>>> >>>> For the multi-MSI case, we need unique address and data info for each MSI, >>>> but the cached msi_desc does not provide that. However, that information >>>> can be gotten from the int_desc cached in the chip_data by >>>> compose_msi_msg(). Fix the multi-MSI case to use that cached information >>>> instead. Since hv_set_msi_entry_from_desc() is no longer applicable, >>>> remove it. >>>> >>>> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> >>>> --- >>>> drivers/pci/controller/pci-hyperv.c | 12 ++++-------- >>>> 1 file changed, 4 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c >>>> index 5800ecf..7aea0b7 100644 >>>> --- a/drivers/pci/controller/pci-hyperv.c >>>> +++ b/drivers/pci/controller/pci-hyperv.c >>>> @@ -611,13 +611,6 @@ static unsigned int hv_msi_get_int_vector(struct irq_data *data) >>>> return cfg->vector; >>>> } >>>> -static void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry, >>>> - struct msi_desc *msi_desc) >>>> -{ >>>> - msi_entry->address.as_uint32 = msi_desc->msg.address_lo; >>>> - msi_entry->data.as_uint32 = msi_desc->msg.data; >>>> -} >>>> - >>> >>> Instead of dropping this function, can you change the second argument to >>> take struct tran_int_desc *? >>> >>> This way you can use the same function in hv_compose_msi_msg. >> >> I do not see how this could be reused in hv_compose_msi_msg() with the >> proposed change of the second argument. The hv_msi_entry type is not used >> in hv_compose_msi_msg(), nor does it look like it is applicable anywhere >> within the function. >> >> What am I missing? > > I mixed up two different types while going through the code -- > hv_msi_entry and Linux's own msi_entry type. Sorry for the noise. No problem. Thanks for picking this up.
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c index 5800ecf..7aea0b7 100644 --- a/drivers/pci/controller/pci-hyperv.c +++ b/drivers/pci/controller/pci-hyperv.c @@ -611,13 +611,6 @@ static unsigned int hv_msi_get_int_vector(struct irq_data *data) return cfg->vector; } -static void hv_set_msi_entry_from_desc(union hv_msi_entry *msi_entry, - struct msi_desc *msi_desc) -{ - msi_entry->address.as_uint32 = msi_desc->msg.address_lo; - msi_entry->data.as_uint32 = msi_desc->msg.data; -} - static int hv_msi_prepare(struct irq_domain *domain, struct device *dev, int nvec, msi_alloc_info_t *info) { @@ -647,6 +640,7 @@ static void hv_arch_irq_unmask(struct irq_data *data) { struct msi_desc *msi_desc = irq_data_get_msi_desc(data); struct hv_retarget_device_interrupt *params; + struct tran_int_desc *int_desc; struct hv_pcibus_device *hbus; struct cpumask *dest; cpumask_var_t tmp; @@ -661,6 +655,7 @@ static void hv_arch_irq_unmask(struct irq_data *data) pdev = msi_desc_to_pci_dev(msi_desc); pbus = pdev->bus; hbus = container_of(pbus->sysdata, struct hv_pcibus_device, sysdata); + int_desc = data->chip_data; spin_lock_irqsave(&hbus->retarget_msi_interrupt_lock, flags); @@ -668,7 +663,8 @@ static void hv_arch_irq_unmask(struct irq_data *data) memset(params, 0, sizeof(*params)); params->partition_id = HV_PARTITION_ID_SELF; params->int_entry.source = HV_INTERRUPT_SOURCE_MSI; - hv_set_msi_entry_from_desc(¶ms->int_entry.msi_entry, msi_desc); + params->int_entry.msi_entry.address.as_uint32 = int_desc->address & 0xffffffff; + params->int_entry.msi_entry.data.as_uint32 = int_desc->data; params->device_id = (hbus->hdev->dev_instance.b[5] << 24) | (hbus->hdev->dev_instance.b[4] << 16) | (hbus->hdev->dev_instance.b[7] << 8) |
In the multi-MSI case, hv_arch_irq_unmask() will only operate on the first MSI of the N allocated. This is because only the first msi_desc is cached and it is shared by all the MSIs of the multi-MSI block. This means that hv_arch_irq_unmask() gets the correct address, but the wrong data (always 0). This can break MSIs. Lets assume MSI0 is vector 34 on CPU0, and MSI1 is vector 33 on CPU0. hv_arch_irq_unmask() is called on MSI0. It uses a hypercall to configure the MSI address and data (0) to vector 34 of CPU0. This is correct. Then hv_arch_irq_unmask is called on MSI1. It uses another hypercall to configure the MSI address and data (0) to vector 33 of CPU0. This is wrong, and results in both MSI0 and MSI1 being routed to vector 33. Linux will observe extra instances of MSI1 and no instances of MSI0 despite the endpoint device behaving correctly. For the multi-MSI case, we need unique address and data info for each MSI, but the cached msi_desc does not provide that. However, that information can be gotten from the int_desc cached in the chip_data by compose_msi_msg(). Fix the multi-MSI case to use that cached information instead. Since hv_set_msi_entry_from_desc() is no longer applicable, remove it. Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> --- drivers/pci/controller/pci-hyperv.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)