linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] hyperv compose_msi_msg fixups
@ 2022-05-09 21:48 Jeffrey Hugo
  2022-05-09 21:48 ` [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in compose_msi_msg() Jeffrey Hugo
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Jeffrey Hugo @ 2022-05-09 21:48 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin, wei.liu, decui, lorenzo.pieralisi, robh,
	kw, bhelgaas
  Cc: jakeo, dazhan, linux-hyperv, linux-pci, linux-kernel, Jeffrey Hugo

While multi-MSI appears to work with pci-hyperv.c, there was a concern about
how linux was doing the ITRE allocations.  Patch 2 addresses the concern.

However, patch 2 exposed an issue with how compose_msi_msg() was freeing a
previous allocation when called for the Nth time.  Imagine a driver using
pci_alloc_irq_vectors() to request 32 MSIs.  This would cause compose_msi_msg()
to be called 32 times, once for each MSI.  With patch 2, MSI0 would allocate
the ITREs needed, and MSI1-31 would use the cached information.  Then the driver
uses request_irq() on MSI1-17.  This would call compose_msi_msg() again on those
MSIs, which would again use the cached information.  Then unmask() would be
called to retarget the MSIs to the right VCPU vectors.  Finally, the driver
calls request_irq() on MSI0.  This would call conpose_msi_msg(), which would
free the block of 32 MSIs, and allocate a new block.  This would undo the
retarget of MSI1-17, and likely leave those MSIs targeting invalid VCPU vectors.
This is addressed by patch 1, which is introduced first to prevent a regression.

Jeffrey Hugo (2):
  PCI: hv: Reuse existing ITRE allocation in compose_msi_msg()
  PCI: hv: Fix interrupt mapping for multi-MSI

 drivers/pci/controller/pci-hyperv.c | 76 ++++++++++++++++++++++++++++---------
 1 file changed, 59 insertions(+), 17 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in compose_msi_msg()
  2022-05-09 21:48 [PATCH 0/2] hyperv compose_msi_msg fixups Jeffrey Hugo
@ 2022-05-09 21:48 ` Jeffrey Hugo
  2022-05-09 23:13   ` Dexuan Cui
  2022-05-09 21:48 ` [PATCH 2/2] PCI: hv: Fix interrupt mapping for multi-MSI Jeffrey Hugo
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Jeffrey Hugo @ 2022-05-09 21:48 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin, wei.liu, decui, lorenzo.pieralisi, robh,
	kw, bhelgaas
  Cc: jakeo, dazhan, linux-hyperv, linux-pci, linux-kernel, Jeffrey Hugo

Currently if compose_msi_msg() is called multiple times, it will free any
previous ITRE allocation, and generate a new allocation.  While nothing
prevents this from occurring, it is extranious when Linux could just reuse
the existing allocation and avoid a bunch of overhead.

However, when future ITRE allocations operate on blocks of MSIs instead of
a single line, freeing the allocation will impact all of the lines.  This
could cause an issue where an allocation of N MSIs occurs, then some of
the lines are retargeted, and finally the allocation is freed/reallocated.
The freeing of the allocation removes all of the configuration for the
entire block, which requires all the lines to be retargeted, which might
not happen since some lines might already be unmasked/active.

Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/pci/controller/pci-hyperv.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index cf2fe575..5e2e637 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -1707,6 +1707,15 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 	u32 size;
 	int ret;
 
+	/* Reuse the previous allocation */
+	if (data->chip_data) {
+		int_desc = data->chip_data;
+		msg->address_hi = int_desc->address >> 32;
+		msg->address_lo = int_desc->address & 0xffffffff;
+		msg->data = int_desc->data;
+		return;
+	}
+
 	pdev = msi_desc_to_pci_dev(irq_data_get_msi_desc(data));
 	dest = irq_data_get_effective_affinity_mask(data);
 	pbus = pdev->bus;
@@ -1716,13 +1725,6 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 	if (!hpdev)
 		goto return_null_message;
 
-	/* Free any previous message that might have already been composed. */
-	if (data->chip_data) {
-		int_desc = data->chip_data;
-		data->chip_data = NULL;
-		hv_int_desc_free(hpdev, int_desc);
-	}
-
 	int_desc = kzalloc(sizeof(*int_desc), GFP_ATOMIC);
 	if (!int_desc)
 		goto drop_reference;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] PCI: hv: Fix interrupt mapping for multi-MSI
  2022-05-09 21:48 [PATCH 0/2] hyperv compose_msi_msg fixups Jeffrey Hugo
  2022-05-09 21:48 ` [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in compose_msi_msg() Jeffrey Hugo
@ 2022-05-09 21:48 ` Jeffrey Hugo
  2022-05-09 23:21   ` Dexuan Cui
  2022-05-09 23:23 ` [PATCH 0/2] hyperv compose_msi_msg fixups Dexuan Cui
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Jeffrey Hugo @ 2022-05-09 21:48 UTC (permalink / raw)
  To: kys, haiyangz, sthemmin, wei.liu, decui, lorenzo.pieralisi, robh,
	kw, bhelgaas
  Cc: jakeo, dazhan, linux-hyperv, linux-pci, linux-kernel, Jeffrey Hugo

According to Dexuan, the hypervisor folks beleive that multi-msi
allocations are not correct.  compose_msi_msg() will allocate multi-msi
one by one.  However, multi-msi is a block of related MSIs, with alignment
requirements.  In order for the hypervisor to allocate properly aligned
and consecutive entries in the IOMMU Interrupt Remapping Table, there
should be a single mapping request that requests all of the multi-msi
vectors in one shot.

Dexuan suggests detecting the multi-msi case and composing a single
request related to the first MSI.  Then for the other MSIs in the same
block, use the cached information.  This appears to be viable, so do it.

Suggested-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/pci/controller/pci-hyperv.c | 60 ++++++++++++++++++++++++++++++-------
 1 file changed, 50 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 5e2e637..a755a0c 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -1525,6 +1525,10 @@ static void hv_int_desc_free(struct hv_pci_dev *hpdev,
 		u8 buffer[sizeof(struct pci_delete_interrupt)];
 	} ctxt;
 
+	if (!int_desc->vector_count) {
+		kfree(int_desc);
+		return;
+	}
 	memset(&ctxt, 0, sizeof(ctxt));
 	int_pkt = (struct pci_delete_interrupt *)&ctxt.pkt.message;
 	int_pkt->message_type.type =
@@ -1609,12 +1613,12 @@ 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)
+	u32 slot, u8 vector, u8 vector_count)
 {
 	int_pkt->message_type.type = PCI_CREATE_INTERRUPT_MESSAGE;
 	int_pkt->wslot.slot = slot;
 	int_pkt->int_desc.vector = vector;
-	int_pkt->int_desc.vector_count = 1;
+	int_pkt->int_desc.vector_count = vector_count;
 	int_pkt->int_desc.delivery_mode = DELIVERY_MODE;
 
 	/*
@@ -1637,14 +1641,14 @@ 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)
+	u32 slot, u8 vector, u8 vector_count)
 {
 	int cpu;
 
 	int_pkt->message_type.type = PCI_CREATE_INTERRUPT_MESSAGE2;
 	int_pkt->wslot.slot = slot;
 	int_pkt->int_desc.vector = vector;
-	int_pkt->int_desc.vector_count = 1;
+	int_pkt->int_desc.vector_count = vector_count;
 	int_pkt->int_desc.delivery_mode = DELIVERY_MODE;
 	cpu = hv_compose_msi_req_get_cpu(affinity);
 	int_pkt->int_desc.processor_array[0] =
@@ -1656,7 +1660,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)
+	u32 slot, u32 vector, u8 vector_count)
 {
 	int cpu;
 
@@ -1664,7 +1668,7 @@ static u32 hv_compose_msi_req_v3(
 	int_pkt->wslot.slot = slot;
 	int_pkt->int_desc.vector = vector;
 	int_pkt->int_desc.reserved = 0;
-	int_pkt->int_desc.vector_count = 1;
+	int_pkt->int_desc.vector_count = vector_count;
 	int_pkt->int_desc.delivery_mode = DELIVERY_MODE;
 	cpu = hv_compose_msi_req_get_cpu(affinity);
 	int_pkt->int_desc.processor_array[0] =
@@ -1695,6 +1699,8 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 	struct cpumask *dest;
 	struct compose_comp_ctxt comp;
 	struct tran_int_desc *int_desc;
+	struct msi_desc *msi_desc;
+	u8 vector, vector_count;
 	struct {
 		struct pci_packet pci_pkt;
 		union {
@@ -1716,7 +1722,8 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 		return;
 	}
 
-	pdev = msi_desc_to_pci_dev(irq_data_get_msi_desc(data));
+	msi_desc  = irq_data_get_msi_desc(data);
+	pdev = msi_desc_to_pci_dev(msi_desc);
 	dest = irq_data_get_effective_affinity_mask(data);
 	pbus = pdev->bus;
 	hbus = container_of(pbus->sysdata, struct hv_pcibus_device, sysdata);
@@ -1729,6 +1736,36 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 	if (!int_desc)
 		goto drop_reference;
 
+	if (!msi_desc->pci.msi_attrib.is_msix && msi_desc->nvec_used > 1) {
+		/*
+		 * if this is not the first MSI of Multi MSI, we already have
+		 * a mapping.  Can exit early.
+		 */
+		if (msi_desc->irq != data->irq) {
+			data->chip_data = int_desc;
+			int_desc->address = msi_desc->msg.address_lo |
+					    (u64)msi_desc->msg.address_hi << 32;
+			int_desc->data = msi_desc->msg.data +
+					 (data->irq - msi_desc->irq);
+			msg->address_hi = msi_desc->msg.address_hi;
+			msg->address_lo = msi_desc->msg.address_lo;
+			msg->data = int_desc->data;
+			put_pcichild(hpdev);
+			return;
+		}
+		/*
+		 * The vector we select here is a dummy value.  The correct
+		 * value gets sent to the hypervisor in unmask().  This needs
+		 * to be aligned with the count, and also not zero.  Multi-msi
+		 * is powers of 2 up to 32, so 32 will always work here.
+		 */
+		vector = 32;
+		vector_count = msi_desc->nvec_used;
+	} else {
+		vector = hv_msi_get_int_vector(data);
+		vector_count = 1;
+	}
+
 	memset(&ctxt, 0, sizeof(ctxt));
 	init_completion(&comp.comp_pkt.host_event);
 	ctxt.pci_pkt.completion_func = hv_pci_compose_compl;
@@ -1739,7 +1776,8 @@ 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,
-					hv_msi_get_int_vector(data));
+					vector,
+					vector_count);
 		break;
 
 	case PCI_PROTOCOL_VERSION_1_2:
@@ -1747,14 +1785,16 @@ 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,
-					hv_msi_get_int_vector(data));
+					vector,
+					vector_count);
 		break;
 
 	case PCI_PROTOCOL_VERSION_1_4:
 		size = hv_compose_msi_req_v3(&ctxt.int_pkts.v3,
 					dest,
 					hpdev->desc.win_slot.slot,
-					hv_msi_get_int_vector(data));
+					vector,
+					vector_count);
 		break;
 
 	default:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* RE: [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in compose_msi_msg()
  2022-05-09 21:48 ` [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in compose_msi_msg() Jeffrey Hugo
@ 2022-05-09 23:13   ` Dexuan Cui
  2022-05-10  2:29     ` Jeffrey Hugo
  0 siblings, 1 reply; 14+ messages in thread
From: Dexuan Cui @ 2022-05-09 23:13 UTC (permalink / raw)
  To: Jeffrey Hugo, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	wei.liu, lorenzo.pieralisi, robh, kw, bhelgaas
  Cc: Jake Oshins, David Zhang, linux-hyperv, linux-pci, linux-kernel

> From: Jeffrey Hugo <quic_jhugo@quicinc.com>
> Sent: Monday, May 9, 2022 2:48 PM
> Subject: [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in

s/ITRE/IRTE. I suppose Wei can help fix this without a v2 :-)

> compose_msi_msg()
> ...
> Currently if compose_msi_msg() is called multiple times, it will free any
> previous ITRE allocation, and generate a new allocation.  While nothing
> prevents this from occurring, it is extranious when Linux could just reuse

s/extranious/extraneous

> the existing allocation and avoid a bunch of overhead.
> 
> However, when future ITRE allocations operate on blocks of MSIs instead of

s/ITRE/IRTE

> a single line, freeing the allocation will impact all of the lines.  This
> could cause an issue where an allocation of N MSIs occurs, then some of
> the lines are retargeted, and finally the allocation is freed/reallocated.
> The freeing of the allocation removes all of the configuration for the
> entire block, which requires all the lines to be retargeted, which might
> not happen since some lines might already be unmasked/active.
> 
> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

Reviewed-by: Dexuan Cui <decui@microsoft.com>
Tested-by: Dexuan Cui <decui@microsoft.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 2/2] PCI: hv: Fix interrupt mapping for multi-MSI
  2022-05-09 21:48 ` [PATCH 2/2] PCI: hv: Fix interrupt mapping for multi-MSI Jeffrey Hugo
@ 2022-05-09 23:21   ` Dexuan Cui
  0 siblings, 0 replies; 14+ messages in thread
From: Dexuan Cui @ 2022-05-09 23:21 UTC (permalink / raw)
  To: Jeffrey Hugo, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	wei.liu, lorenzo.pieralisi, robh, kw, bhelgaas
  Cc: Jake Oshins, David Zhang, linux-hyperv, linux-pci, linux-kernel

> From: Jeffrey Hugo <quic_jhugo@quicinc.com>
> Sent: Monday, May 9, 2022 2:48 PM
> ...
> According to Dexuan, the hypervisor folks beleive that multi-msi
> allocations are not correct.  compose_msi_msg() will allocate multi-msi
> one by one.  However, multi-msi is a block of related MSIs, with alignment
> requirements.  In order for the hypervisor to allocate properly aligned
> and consecutive entries in the IOMMU Interrupt Remapping Table, there
> should be a single mapping request that requests all of the multi-msi
> vectors in one shot.
> 
> Dexuan suggests detecting the multi-msi case and composing a single
> request related to the first MSI.  Then for the other MSIs in the same
> block, use the cached information.  This appears to be viable, so do it.
> 
> Suggested-by: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

Reviewed-by: Dexuan Cui <decui@microsoft.com>

> +	if (!msi_desc->pci.msi_attrib.is_msix && msi_desc->nvec_used > 1) {
> +		/*
> +		 * if this is not the first MSI of Multi MSI, we already have

s/if/If


^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 0/2] hyperv compose_msi_msg fixups
  2022-05-09 21:48 [PATCH 0/2] hyperv compose_msi_msg fixups Jeffrey Hugo
  2022-05-09 21:48 ` [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in compose_msi_msg() Jeffrey Hugo
  2022-05-09 21:48 ` [PATCH 2/2] PCI: hv: Fix interrupt mapping for multi-MSI Jeffrey Hugo
@ 2022-05-09 23:23 ` Dexuan Cui
  2022-05-10  2:31   ` Jeffrey Hugo
  2022-05-10 18:13 ` Michael Kelley (LINUX)
  2022-05-11 14:41 ` Wei Liu
  4 siblings, 1 reply; 14+ messages in thread
From: Dexuan Cui @ 2022-05-09 23:23 UTC (permalink / raw)
  To: Jeffrey Hugo, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	wei.liu, lorenzo.pieralisi, robh, kw, bhelgaas
  Cc: Jake Oshins, David Zhang, linux-hyperv, linux-pci, linux-kernel

> From: Jeffrey Hugo <quic_jhugo@quicinc.com>
> Sent: Monday, May 9, 2022 2:48 PM

Thank you Jeff for working out the patches!

Thanks,
-- Dexuan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in compose_msi_msg()
  2022-05-09 23:13   ` Dexuan Cui
@ 2022-05-10  2:29     ` Jeffrey Hugo
  2022-05-10 10:52       ` Wei Liu
  0 siblings, 1 reply; 14+ messages in thread
From: Jeffrey Hugo @ 2022-05-10  2:29 UTC (permalink / raw)
  To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	wei.liu, lorenzo.pieralisi, robh, kw, bhelgaas
  Cc: Jake Oshins, David Zhang, linux-hyperv, linux-pci, linux-kernel

On 5/9/2022 5:13 PM, Dexuan Cui wrote:
>> From: Jeffrey Hugo <quic_jhugo@quicinc.com>
>> Sent: Monday, May 9, 2022 2:48 PM
>> Subject: [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in
> 
> s/ITRE/IRTE. I suppose Wei can help fix this without a v2 :-)

Thanks for the review.

I have no problem sending out a V2.  Especially since you pointed out my 
mistakes on both patches.  I'll wait a little bit for any additional 
feedback, and then send out a V2.

> 
>> compose_msi_msg()
>> ...
>> Currently if compose_msi_msg() is called multiple times, it will free any
>> previous ITRE allocation, and generate a new allocation.  While nothing
>> prevents this from occurring, it is extranious when Linux could just reuse
> 
> s/extranious/extraneous
> 
>> the existing allocation and avoid a bunch of overhead.
>>
>> However, when future ITRE allocations operate on blocks of MSIs instead of
> 
> s/ITRE/IRTE
> 
>> a single line, freeing the allocation will impact all of the lines.  This
>> could cause an issue where an allocation of N MSIs occurs, then some of
>> the lines are retargeted, and finally the allocation is freed/reallocated.
>> The freeing of the allocation removes all of the configuration for the
>> entire block, which requires all the lines to be retargeted, which might
>> not happen since some lines might already be unmasked/active.
>>
>> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
> 
> Reviewed-by: Dexuan Cui <decui@microsoft.com>
> Tested-by: Dexuan Cui <decui@microsoft.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] hyperv compose_msi_msg fixups
  2022-05-09 23:23 ` [PATCH 0/2] hyperv compose_msi_msg fixups Dexuan Cui
@ 2022-05-10  2:31   ` Jeffrey Hugo
  0 siblings, 0 replies; 14+ messages in thread
From: Jeffrey Hugo @ 2022-05-10  2:31 UTC (permalink / raw)
  To: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	wei.liu, lorenzo.pieralisi, robh, kw, bhelgaas
  Cc: Jake Oshins, David Zhang, linux-hyperv, linux-pci, linux-kernel

On 5/9/2022 5:23 PM, Dexuan Cui wrote:
>> From: Jeffrey Hugo <quic_jhugo@quicinc.com>
>> Sent: Monday, May 9, 2022 2:48 PM
> 
> Thank you Jeff for working out the patches!

Thanks for the feedback and reviews.  This certainly would have been 
more challenging without your assistance.

-Jeff


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in compose_msi_msg()
  2022-05-10  2:29     ` Jeffrey Hugo
@ 2022-05-10 10:52       ` Wei Liu
  0 siblings, 0 replies; 14+ messages in thread
From: Wei Liu @ 2022-05-10 10:52 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: Dexuan Cui, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	wei.liu, lorenzo.pieralisi, robh, kw, bhelgaas, Jake Oshins,
	David Zhang, linux-hyperv, linux-pci, linux-kernel

On Mon, May 09, 2022 at 08:29:19PM -0600, Jeffrey Hugo wrote:
> On 5/9/2022 5:13 PM, Dexuan Cui wrote:
> > > From: Jeffrey Hugo <quic_jhugo@quicinc.com>
> > > Sent: Monday, May 9, 2022 2:48 PM
> > > Subject: [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in
> > 
> > s/ITRE/IRTE. I suppose Wei can help fix this without a v2 :-)
> 
> Thanks for the review.
> 
> I have no problem sending out a V2.  Especially since you pointed out my
> mistakes on both patches.  I'll wait a little bit for any additional
> feedback, and then send out a V2.

Yes please send out v2.

Thanks,
Wei.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 0/2] hyperv compose_msi_msg fixups
  2022-05-09 21:48 [PATCH 0/2] hyperv compose_msi_msg fixups Jeffrey Hugo
                   ` (2 preceding siblings ...)
  2022-05-09 23:23 ` [PATCH 0/2] hyperv compose_msi_msg fixups Dexuan Cui
@ 2022-05-10 18:13 ` Michael Kelley (LINUX)
  2022-05-11 14:41 ` Wei Liu
  4 siblings, 0 replies; 14+ messages in thread
From: Michael Kelley (LINUX) @ 2022-05-10 18:13 UTC (permalink / raw)
  To: Jeffrey Hugo, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
	wei.liu, Dexuan Cui, lorenzo.pieralisi, robh, kw, bhelgaas
  Cc: Jake Oshins, David Zhang, linux-hyperv, linux-pci, linux-kernel

From: Jeffrey Hugo <quic_jhugo@quicinc.com> Sent: Monday, May 9, 2022 2:48 PM
> 
> While multi-MSI appears to work with pci-hyperv.c, there was a concern about
> how linux was doing the ITRE allocations.  Patch 2 addresses the concern.
> 
> However, patch 2 exposed an issue with how compose_msi_msg() was freeing a
> previous allocation when called for the Nth time.  Imagine a driver using
> pci_alloc_irq_vectors() to request 32 MSIs.  This would cause compose_msi_msg()
> to be called 32 times, once for each MSI.  With patch 2, MSI0 would allocate
> the ITREs needed, and MSI1-31 would use the cached information.  Then the driver
> uses request_irq() on MSI1-17.  This would call compose_msi_msg() again on those
> MSIs, which would again use the cached information.  Then unmask() would be
> called to retarget the MSIs to the right VCPU vectors.  Finally, the driver
> calls request_irq() on MSI0.  This would call conpose_msi_msg(), which would
> free the block of 32 MSIs, and allocate a new block.  This would undo the
> retarget of MSI1-17, and likely leave those MSIs targeting invalid VCPU vectors.
> This is addressed by patch 1, which is introduced first to prevent a regression.
> 
> Jeffrey Hugo (2):
>   PCI: hv: Reuse existing ITRE allocation in compose_msi_msg()
>   PCI: hv: Fix interrupt mapping for multi-MSI
> 
>  drivers/pci/controller/pci-hyperv.c | 76 ++++++++++++++++++++++++++++---------
>  1 file changed, 59 insertions(+), 17 deletions(-)
> 
> --
> 2.7.4

I tested these two patches in combination with the earlier two on
5.18-rc6 in an ARM64 VM in Azure.   This was a smoke-test to ensure
everything compiled and that the changes aren't fundamentally broken
on ARM64.   The PCI device in this case is the Mellanox Virtual Function
offered to VMs in Azure, which is MSI-X.   As such, the new MSI "batch"
handling is not tested.

Tested-by: Michael Kelley <mikelley@microsoft.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] hyperv compose_msi_msg fixups
  2022-05-09 21:48 [PATCH 0/2] hyperv compose_msi_msg fixups Jeffrey Hugo
                   ` (3 preceding siblings ...)
  2022-05-10 18:13 ` Michael Kelley (LINUX)
@ 2022-05-11 14:41 ` Wei Liu
  2022-05-11 14:47   ` Jeffrey Hugo
  4 siblings, 1 reply; 14+ messages in thread
From: Wei Liu @ 2022-05-11 14:41 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: kys, haiyangz, sthemmin, wei.liu, decui, lorenzo.pieralisi, robh,
	kw, bhelgaas, jakeo, dazhan, linux-hyperv, linux-pci,
	linux-kernel

On Mon, May 09, 2022 at 03:48:20PM -0600, Jeffrey Hugo wrote:
> While multi-MSI appears to work with pci-hyperv.c, there was a concern about
> how linux was doing the ITRE allocations.  Patch 2 addresses the concern.
> 
> However, patch 2 exposed an issue with how compose_msi_msg() was freeing a
> previous allocation when called for the Nth time.  Imagine a driver using
> pci_alloc_irq_vectors() to request 32 MSIs.  This would cause compose_msi_msg()
> to be called 32 times, once for each MSI.  With patch 2, MSI0 would allocate
> the ITREs needed, and MSI1-31 would use the cached information.  Then the driver
> uses request_irq() on MSI1-17.  This would call compose_msi_msg() again on those
> MSIs, which would again use the cached information.  Then unmask() would be
> called to retarget the MSIs to the right VCPU vectors.  Finally, the driver
> calls request_irq() on MSI0.  This would call conpose_msi_msg(), which would
> free the block of 32 MSIs, and allocate a new block.  This would undo the
> retarget of MSI1-17, and likely leave those MSIs targeting invalid VCPU vectors.
> This is addressed by patch 1, which is introduced first to prevent a regression.
> 
> Jeffrey Hugo (2):
>   PCI: hv: Reuse existing ITRE allocation in compose_msi_msg()
>   PCI: hv: Fix interrupt mapping for multi-MSI
> 

Applied to hyperv-next. Thanks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] hyperv compose_msi_msg fixups
  2022-05-11 14:41 ` Wei Liu
@ 2022-05-11 14:47   ` Jeffrey Hugo
  2022-05-11 15:19     ` Wei Liu
  0 siblings, 1 reply; 14+ messages in thread
From: Jeffrey Hugo @ 2022-05-11 14:47 UTC (permalink / raw)
  To: Wei Liu
  Cc: kys, haiyangz, sthemmin, decui, lorenzo.pieralisi, robh, kw,
	bhelgaas, jakeo, dazhan, linux-hyperv, linux-pci, linux-kernel

On 5/11/2022 8:41 AM, Wei Liu wrote:
> On Mon, May 09, 2022 at 03:48:20PM -0600, Jeffrey Hugo wrote:
>> While multi-MSI appears to work with pci-hyperv.c, there was a concern about
>> how linux was doing the ITRE allocations.  Patch 2 addresses the concern.
>>
>> However, patch 2 exposed an issue with how compose_msi_msg() was freeing a
>> previous allocation when called for the Nth time.  Imagine a driver using
>> pci_alloc_irq_vectors() to request 32 MSIs.  This would cause compose_msi_msg()
>> to be called 32 times, once for each MSI.  With patch 2, MSI0 would allocate
>> the ITREs needed, and MSI1-31 would use the cached information.  Then the driver
>> uses request_irq() on MSI1-17.  This would call compose_msi_msg() again on those
>> MSIs, which would again use the cached information.  Then unmask() would be
>> called to retarget the MSIs to the right VCPU vectors.  Finally, the driver
>> calls request_irq() on MSI0.  This would call conpose_msi_msg(), which would
>> free the block of 32 MSIs, and allocate a new block.  This would undo the
>> retarget of MSI1-17, and likely leave those MSIs targeting invalid VCPU vectors.
>> This is addressed by patch 1, which is introduced first to prevent a regression.
>>
>> Jeffrey Hugo (2):
>>    PCI: hv: Reuse existing ITRE allocation in compose_msi_msg()
>>    PCI: hv: Fix interrupt mapping for multi-MSI
>>
> 
> Applied to hyperv-next. Thanks.

Huh?  I thought you wanted a V2.  I was intending on sending that out today.

-Jeff

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] hyperv compose_msi_msg fixups
  2022-05-11 14:47   ` Jeffrey Hugo
@ 2022-05-11 15:19     ` Wei Liu
  2022-05-11 15:21       ` Jeffrey Hugo
  0 siblings, 1 reply; 14+ messages in thread
From: Wei Liu @ 2022-05-11 15:19 UTC (permalink / raw)
  To: Jeffrey Hugo
  Cc: Wei Liu, kys, haiyangz, sthemmin, decui, lorenzo.pieralisi, robh,
	kw, bhelgaas, jakeo, dazhan, linux-hyperv, linux-pci,
	linux-kernel

On Wed, May 11, 2022 at 08:47:23AM -0600, Jeffrey Hugo wrote:
> On 5/11/2022 8:41 AM, Wei Liu wrote:
> > On Mon, May 09, 2022 at 03:48:20PM -0600, Jeffrey Hugo wrote:
> > > While multi-MSI appears to work with pci-hyperv.c, there was a concern about
> > > how linux was doing the ITRE allocations.  Patch 2 addresses the concern.
> > > 
> > > However, patch 2 exposed an issue with how compose_msi_msg() was freeing a
> > > previous allocation when called for the Nth time.  Imagine a driver using
> > > pci_alloc_irq_vectors() to request 32 MSIs.  This would cause compose_msi_msg()
> > > to be called 32 times, once for each MSI.  With patch 2, MSI0 would allocate
> > > the ITREs needed, and MSI1-31 would use the cached information.  Then the driver
> > > uses request_irq() on MSI1-17.  This would call compose_msi_msg() again on those
> > > MSIs, which would again use the cached information.  Then unmask() would be
> > > called to retarget the MSIs to the right VCPU vectors.  Finally, the driver
> > > calls request_irq() on MSI0.  This would call conpose_msi_msg(), which would
> > > free the block of 32 MSIs, and allocate a new block.  This would undo the
> > > retarget of MSI1-17, and likely leave those MSIs targeting invalid VCPU vectors.
> > > This is addressed by patch 1, which is introduced first to prevent a regression.
> > > 
> > > Jeffrey Hugo (2):
> > >    PCI: hv: Reuse existing ITRE allocation in compose_msi_msg()
> > >    PCI: hv: Fix interrupt mapping for multi-MSI
> > > 
> > 
> > Applied to hyperv-next. Thanks.
> 
> Huh?  I thought you wanted a V2.  I was intending on sending that out today.
> 

Please send them out. I will apply the new version.

Thanks,
Wei.

> -Jeff

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] hyperv compose_msi_msg fixups
  2022-05-11 15:19     ` Wei Liu
@ 2022-05-11 15:21       ` Jeffrey Hugo
  0 siblings, 0 replies; 14+ messages in thread
From: Jeffrey Hugo @ 2022-05-11 15:21 UTC (permalink / raw)
  To: Wei Liu
  Cc: kys, haiyangz, sthemmin, decui, lorenzo.pieralisi, robh, kw,
	bhelgaas, jakeo, dazhan, linux-hyperv, linux-pci, linux-kernel

On 5/11/2022 9:19 AM, Wei Liu wrote:
> On Wed, May 11, 2022 at 08:47:23AM -0600, Jeffrey Hugo wrote:
>> On 5/11/2022 8:41 AM, Wei Liu wrote:
>>> On Mon, May 09, 2022 at 03:48:20PM -0600, Jeffrey Hugo wrote:
>>>> While multi-MSI appears to work with pci-hyperv.c, there was a concern about
>>>> how linux was doing the ITRE allocations.  Patch 2 addresses the concern.
>>>>
>>>> However, patch 2 exposed an issue with how compose_msi_msg() was freeing a
>>>> previous allocation when called for the Nth time.  Imagine a driver using
>>>> pci_alloc_irq_vectors() to request 32 MSIs.  This would cause compose_msi_msg()
>>>> to be called 32 times, once for each MSI.  With patch 2, MSI0 would allocate
>>>> the ITREs needed, and MSI1-31 would use the cached information.  Then the driver
>>>> uses request_irq() on MSI1-17.  This would call compose_msi_msg() again on those
>>>> MSIs, which would again use the cached information.  Then unmask() would be
>>>> called to retarget the MSIs to the right VCPU vectors.  Finally, the driver
>>>> calls request_irq() on MSI0.  This would call conpose_msi_msg(), which would
>>>> free the block of 32 MSIs, and allocate a new block.  This would undo the
>>>> retarget of MSI1-17, and likely leave those MSIs targeting invalid VCPU vectors.
>>>> This is addressed by patch 1, which is introduced first to prevent a regression.
>>>>
>>>> Jeffrey Hugo (2):
>>>>     PCI: hv: Reuse existing ITRE allocation in compose_msi_msg()
>>>>     PCI: hv: Fix interrupt mapping for multi-MSI
>>>>
>>>
>>> Applied to hyperv-next. Thanks.
>>
>> Huh?  I thought you wanted a V2.  I was intending on sending that out today.
>>
> 
> Please send them out. I will apply the new version.

Sure, sending shortly.

-Jeff


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-05-11 15:21 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-09 21:48 [PATCH 0/2] hyperv compose_msi_msg fixups Jeffrey Hugo
2022-05-09 21:48 ` [PATCH 1/2] PCI: hv: Reuse existing ITRE allocation in compose_msi_msg() Jeffrey Hugo
2022-05-09 23:13   ` Dexuan Cui
2022-05-10  2:29     ` Jeffrey Hugo
2022-05-10 10:52       ` Wei Liu
2022-05-09 21:48 ` [PATCH 2/2] PCI: hv: Fix interrupt mapping for multi-MSI Jeffrey Hugo
2022-05-09 23:21   ` Dexuan Cui
2022-05-09 23:23 ` [PATCH 0/2] hyperv compose_msi_msg fixups Dexuan Cui
2022-05-10  2:31   ` Jeffrey Hugo
2022-05-10 18:13 ` Michael Kelley (LINUX)
2022-05-11 14:41 ` Wei Liu
2022-05-11 14:47   ` Jeffrey Hugo
2022-05-11 15:19     ` Wei Liu
2022-05-11 15:21       ` Jeffrey Hugo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).