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

To: "Zhai, Edwin" <edwin.zhai@xxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] [IOEMU] Fix wrong INTx for pass-through device
From: Tom Rotenberg <tom.rotenberg@xxxxxxxxx>
Date: Wed, 30 Dec 2009 10:20:39 +0200
Cc: Simon Horman <horms@xxxxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Xen Developers <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 30 Dec 2009 00:21:00 -0800
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=yyqZ0hlHuRVB0PY3zPLn6VsSojMgCV2XNrr4IUwV9n0=; b=svVaJC2dHZTqkdGZ7FBAD14GX1KpAtumBMAcz/5CwOExisU3oXhSdGkU+S+sJ8RdUU rFSVez0CXyWalKxzGY4ClVgHEGBbkCA5OJfVDq7RhqqFj36pBvX4lwpmzVBf+PPquwH9 hV50U0AIrbQ6Gc0jxvAhY2vuMDhyApy5H+11E=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=T9r389oICFJNkUZdBBA2UqR2OWuI3cA2NWfsSgdQ+sf/H/BlkK0dY/9CNDz8FjdchU xdMQZkNZkcRCQekwDl5/zY+BUkSvx3rtOCupoaZqkVYaB/XqE5h3q1FYp02j5kwvKasw GJqe63eAY0AlgNcn7JDpT7HGqT3PAhobhlMjU=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4B39C4DE.8050103@xxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4B38589B.8060307@xxxxxxxxx> <8686c3cd0912280633u27839f56k74abcd73fb041e4@xxxxxxxxxxxxxx> <4B394BCB.30607@xxxxxxxxx> <20091229020128.GG10172@xxxxxxxxxxxx> <8686c3cd0912290051t76c771fdq15314f25287d3973@xxxxxxxxxxxxxx> <4B39C4DE.8050103@xxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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?

Tom

On Tue, Dec 29, 2009 at 10:59 AM, Zhai, Edwin <edwin.zhai@xxxxxxxxx> wrote:
>
>
> Tom Rotenberg wrote:
>>
>> Well, i don't think that your patch will work for all of the cases, as
>> some platforms boot with non multi-function devices using interrupt
>> pin other than INTA... i myself encountered 2 such Lenovo platforms.
>>
>
> In general single function device should be linked to INTA, although most of
> OS can handle otherwise:)
>
> Back to the issue, even this abnormal machine can work with my patch, as our
> policy is handling _virtual_ INTx and providing consistent values to guest
> and xen, and physical INTx doesn't matter here.
>
> Could you pls. test my patch without yours to see if it can work?
> Thanks,
>
>
>> So, i think we will need to use both patches together. What do u say?
>>
>> On Tue, Dec 29, 2009 at 4:01 AM, Simon Horman <horms@xxxxxxxxxxxx> wrote:
>>
>>>
>>> On Tue, Dec 29, 2009 at 08:22:35AM +0800, Zhai, Edwin wrote:
>>>
>>>>
>>>> Your patch can also fix this issue, as using physical pin info for
>>>> both guest and xen.
>>>> Only potential issue is that guest will see a single function device
>>>> is not linked to INTA, say assign 00:1a.7 to guest as 00:4.0. It
>>>> should work, but seems doesn't comply with PCI spec.
>>>>
>>>
>>> I had a vague memory that was the case,
>>> I have been meaning to check the spec.
>>>
>>>
>>>>
>>>> We have 2 options here:
>>>> 1. always use INTA
>>>> 2. Use INTA for virtual function 0, and physical value for others.
>>>> Options 2 is more friendly to multiple-function device assignment,
>>>> and is current used.
>>>>
>>>
>>> 2 seems to be a much better solution to me.
>>>
>>> Tom, could you see if Edwin's patch, without your patch, resolves
>>> the problem that you were seeing?
>>>
>>>
>>>>
>>>> Tom Rotenberg wrote:
>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> I ran into similar problems last week, and i tried the following fix,
>>>>> which looks like it kind-of fixed it, does this do the same fix as
>>>>> your patch?
>>>>>
>>>>> Here is the patch i used:
>>>>>
>>>>> --- a/tools/ioemu/hw/pass-through.c  Sun Dec 27 11:56:08 2009 +0200
>>>>> +++ b/tools/ioemu/hw/pass-through.c  Sun Dec 27 11:56:08 2009 +0200
>>>>> @@ -4209,8 +4209,14 @@
>>>>>  */
>>>>> uint8_t pci_intx(struct pt_dev *ptdev)
>>>>> {
>>>>> +#if 0    /* FIX */
>>>>>     if (!PCI_FUNC(ptdev->dev.devfn))
>>>>>         return 0;
>>>>> +#endif /* FIX */
>>>>> +
>>>>>     return pci_read_intx(ptdev);
>>>>> }
>>>>>
>>>>> Tom
>>>>>
>>>>> On Mon, Dec 28, 2009 at 9:04 AM, Zhai, Edwin <edwin.zhai@xxxxxxxxx>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>> Simon,
>>>>>> For the pass-through device's INTx emulation, we follow the policy: if
>>>>>> virtual function 0, use INTA#, otherwise use hardware value. However,
>>>>>> this
>>>>>> policy only apply when bind_pt_pci_irq to xen, and always use physical
>>>>>> value
>>>>>> when exporting to guest. This discrepancy cause different INTx, thus
>>>>>> different GSI in xen and guest, so that interrupts never got injected
>>>>>> to
>>>>>> guest. E.g. when assigning a USB controller with non-zero
>>>>>> function(00:1d.2)
>>>>>> to guest as 00:4.0, xen will see INTA#, while guest see INTC#.
>>>>>>
>>>>>> This simple patch can fix it. Could you pls. review it?
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> --
>>>>>> best rgds,
>>>>>> edwin
>>>>>>
>>>>>>
>>>>>> Signed-Off-By: Zhai Edwin <edwin.zhai@xxxxxxxxx>
>>>>>>
>>>>>> diff --git a/hw/pass-through.c b/hw/pass-through.c
>>>>>> index e7bd386..a08c0bf 100644
>>>>>> --- a/hw/pass-through.c
>>>>>> +++ b/hw/pass-through.c
>>>>>> @@ -2710,7 +2710,8 @@ static uint32_t pt_status_reg_init(struct pt_dev
>>>>>> *ptdev,
>>>>>> 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];
>>>>>> +    /* Translate xen INTx value to hw */
>>>>>> +    return pci_intx(ptdev) + 1;
>>>>>> }
>>>>>>
>>>>>> /* initialize BAR */
>>>>>>
>>>>>> _______________________________________________
>>>>>> Xen-devel mailing list
>>>>>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>>>>>> http://lists.xensource.com/xen-devel
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>> --
>>>> best rgds,
>>>> edwin
>>>>
>>
>>
>
> --
> best rgds,
> edwin
>
>

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