WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] [IOEMU] Fix wrong INTx for pass-through device

Simon/Tom,
We have 3 optinos:
A. always INTA
B. always physical INTx
C. INTA if virtual function is 0, physical INTx for otherwise.

Difference of option B and C is that guest will see a function 0 on a single function device is linked to a PIN rather than INTA, say assign 0:1a.7 to guest as 0:4.0. Most of OS should handle this. so I'm okay with both.

For option A, I'm not sure the issue for the multiple-function device. Can we assign a multiple function device to a guest as it is? E.g. assign physical 0:a.0/0:a.1to guest as 0:4.0/0:4.1. In this way, with option A, there are performance issue when injecting intr as they share same virtual GSI. If we can't do this now, I think option A is also good. Is any specific reason that we change to C? Does some specific multiple function driver assumes specific pin other than INTA?

BTW, pls. send your patch in attachment as I couldn't get it from your mail:(

Thanks,

Simon Horman wrote:
On Wed, Dec 30, 2009 at 10:20:39AM +0200, Tom Rotenberg wrote:
Hi,

Just tested your patch (without my patch), and it works.
So, now the question is, which approach is better and why? i think
your approach in the patch sounds a little bit better, but i am not
sure why (other than reflecting IntA for multi-function devs).
Can u please explain why?

Hi Tom, Hi Edwin,

As I see things, the problem is that there is a discrepancy in the
way that two different sections of code calculate the same value -
the virtual INTx.

On the one hand there is uint32_t pt_irqpin_reg_init(), which
always uses the hw value.

And on the other hand there is pci_intx() which uses the hw value
if for PCI functions other than zero, and INTA for PCI function zero.
(pci_read_intx() also mangles the value but thats not relevant to the
problem).

Furthermore we can observe that almost always the result of these
two methods is the same, as function zero should use INTA. But some
real HW doesn't, and thats a problem.

The thing that is really annoying here is that there are two bits
of code calculating the same thing. For that reason Ediwin's patch
is nice in that it centralises the code. But otherwise I prefer
Tom's approach of always using the hw value - that just seems nicer
to me. IIRC, the only reason that I put in the use INA for function 0
logic was because prior to multi-function INTA was always used. But
that extra little bit of logic is biting us now.

I propose a) always using the hw value for INTx and b) always calculating
this in the same place.

From: Simon Horman <horms@xxxxxxxxxxxx>


qemu-xen: pass-through: always use hw intx

pass-through: always use hw intx and always get it from the same place

Cc: Tom Rotenberg <tom.rotenberg@xxxxxxxxx>
Cc: Edwin Zhai <edwin.zhai@xxxxxxxxx>
Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx>

Index: ioemu-remote/hw/pass-through.c
===================================================================
--- ioemu-remote.orig/hw/pass-through.c 2009-12-31 13:10:06.000000000 +0900
+++ ioemu-remote/hw/pass-through.c      2009-12-31 13:24:20.000000000 +0900
@@ -2710,7 +2710,7 @@ static uint32_t pt_status_reg_init(struc
 static uint32_t pt_irqpin_reg_init(struct pt_dev *ptdev,
         struct pt_reg_info_tbl *reg, uint32_t real_offset)
 {
-    return ptdev->dev.config[real_offset];
+    return pci_read_intx(ptdev);
 }
/* initialize BAR */
@@ -4471,6 +4471,11 @@ int pt_init(PCIBus *e_bus)
     return 0;
 }
+static uint8_t pci_read_intx(struct pt_dev *ptdev)
+{
+    return pci_read_byte(ptdev->pci_dev, PCI_INTERRUPT_PIN);
+}
+
 /* The PCI Local Bus Specification, Rev. 3.0,
  * Section 6.2.4 Miscellaneous Registers, pp 223
  * outlines 5 valid values for the intertupt pin (intx).
@@ -4499,9 +4504,9 @@ int pt_init(PCIBus *e_bus)
  * 4               | 3   | INTD#
  * any other value | 0   | This should never happen, log error message
  */
-static uint8_t pci_read_intx(struct pt_dev *ptdev)
+uint8_t pci_intx(struct pt_dev *ptdev)
 {
-    uint8_t r_val = pci_read_byte(ptdev->pci_dev, PCI_INTERRUPT_PIN);
+    uint8_t r_val = pci_read_intx(ptdev);
PT_LOG("intx=%i\n", r_val);
     if (r_val < 1 || r_val > 4)
@@ -4517,14 +4522,3 @@ static uint8_t pci_read_intx(struct pt_d
return r_val;
 }
-
-/*
- * For virtual function 0, always use INTA#,
- * otherwise use the hardware value
- */
-uint8_t pci_intx(struct pt_dev *ptdev)
-{
-    if (!PCI_FUNC(ptdev->dev.devfn))
-        return 0;
-    return pci_read_intx(ptdev);
-}


--
best rgds,
edwin


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel