Patchwork [-tip/apic,2/2] x86/MSI: Allocate as many multiple-MSIs as requested

login
register
mail settings
Submitter Alexander Gordeev
Date April 29, 2013, 4:33 a.m.
Message ID <eb84d5e5d4af81aa38832c21e49d3276f780a048.1367160713.git.agordeev@redhat.com>
Download mbox | patch
Permalink /patch/240320/
State Superseded
Headers show

Comments

Alexander Gordeev - April 29, 2013, 4:33 a.m.
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 drivers/iommu/irq_remapping.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
Jan Beulich - April 29, 2013, 7:22 a.m.
>>> On 29.04.13 at 06:33, Alexander Gordeev <agordeev@redhat.com> wrote:
> --- a/drivers/iommu/irq_remapping.c
> +++ b/drivers/iommu/irq_remapping.c
> @@ -55,19 +55,19 @@ static int do_setup_msi_irqs(struct pci_dev *dev, int nvec)
>  	unsigned int irq;
>  	struct msi_desc *msidesc;
>  
> -	nvec = __roundup_pow_of_two(nvec);
> -
>  	WARN_ON(!list_is_singular(&dev->msi_list));
>  	msidesc = list_entry(dev->msi_list.next, struct msi_desc, list);
>  	WARN_ON(msidesc->irq);
>  	WARN_ON(msidesc->msi_attrib.multiple);
> +	WARN_ON(msidesc->nvec);
>  
>  	node = dev_to_node(&dev->dev);
>  	irq = __create_irqs(get_nr_irqs_gsi(), nvec, node);
>  	if (irq == 0)
>  		return -ENOSPC;
>  
> -	msidesc->msi_attrib.multiple = ilog2(nvec);
> +	msidesc->nvec = nvec;
> +	msidesc->msi_attrib.multiple = ilog2(__roundup_pow_of_two(nvec));
>  	for (sub_handle = 0; sub_handle < nvec; sub_handle++) {
>  		if (!sub_handle) {
>  			index = msi_alloc_remapped_irq(dev, irq, nvec);

This breaks the interface to IOMMU-specific code: While Intel's
implementation does bump the number of allocated IRTEs to a
power of 2, AMD's doesn't, and hence the tail entries in the block
that don't get allocated here can get used for another device,
thus creating a security hole when both devices aren't owned by
the same guest (with the host being considered a special kind of
guest for this purpose).

IOW, while you can conserve on the number of vectors allocated,
you can't on the IRTEs, and I think this should be taken care of in
the generic IOMMU code, not in the individual vendor
implementations.

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index d56f8c1..315c563 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -55,19 +55,19 @@  static int do_setup_msi_irqs(struct pci_dev *dev, int nvec)
 	unsigned int irq;
 	struct msi_desc *msidesc;
 
-	nvec = __roundup_pow_of_two(nvec);
-
 	WARN_ON(!list_is_singular(&dev->msi_list));
 	msidesc = list_entry(dev->msi_list.next, struct msi_desc, list);
 	WARN_ON(msidesc->irq);
 	WARN_ON(msidesc->msi_attrib.multiple);
+	WARN_ON(msidesc->nvec);
 
 	node = dev_to_node(&dev->dev);
 	irq = __create_irqs(get_nr_irqs_gsi(), nvec, node);
 	if (irq == 0)
 		return -ENOSPC;
 
-	msidesc->msi_attrib.multiple = ilog2(nvec);
+	msidesc->nvec = nvec;
+	msidesc->msi_attrib.multiple = ilog2(__roundup_pow_of_two(nvec));
 	for (sub_handle = 0; sub_handle < nvec; sub_handle++) {
 		if (!sub_handle) {
 			index = msi_alloc_remapped_irq(dev, irq, nvec);
@@ -95,6 +95,7 @@  error:
 	 * IRQs from tearing down again in default_teardown_msi_irqs()
 	 */
 	msidesc->irq = 0;
+	msidesc->nvec = 0;
 	msidesc->msi_attrib.multiple = 0;
 
 	return ret;