From mboxrd@z Thu Jan  1 00:00:00 1970
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH 1/2] AMD IOMMU: also spot missing IO-APIC
 entries in IVRS table
Date: Wed, 06 Feb 2013 10:03:38 -0500
Message-ID: <511270CA.6090903@oracle.com>
References: <511262E302000078000BC738@nat28.tlf.novell.com>
	<511264B402000078000BC754@nat28.tlf.novell.com>
	<51126BA3.9020307@oracle.com>
	<51127C2102000078000BC819@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <51127C2102000078000BC819@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xen.org>, Sherry Hurwitz <sherry.hurwitz@amd.com>
List-Id: xen-devel@lists.xenproject.org

On 2/6/2013 9:52 AM, Jan Beulich wrote:
>>>> On 06.02.13 at 15:41, Boris Ostrovsky<boris.ostrovsky@oracle.com>  wrote:
>> On 2/6/2013 8:12 AM, Jan Beulich wrote:
>>
>>>
>>> +    /* Each IO-APIC must have been mentioned in the table. */
>>> +    for ( apic = 0; !error&&   apic<   nr_ioapics; ++apic )
>>> +    {
>>> +        if ( !nr_ioapic_entries[apic] ||
>>> +             ioapic_sbdf[IO_APIC_ID(apic)].pin_setup )
>>> +            continue;
>>> +
>>> +        printk(XENLOG_ERR "IVHD Error: no information for IO-APIC %#x\n",
>>> +               IO_APIC_ID(apic));
>>> +        if ( amd_iommu_perdev_intremap )
>>> +            error = -ENXIO;
>>> +        else
>>> +        {
>>> +            ioapic_sbdf[IO_APIC_ID(apic)].pin_setup = xzalloc_array(
>>> +                unsigned long, BITS_TO_LONGS(nr_ioapic_entries[apic]));
>>> +            if ( !ioapic_sbdf[IO_APIC_ID(apic)].pin_setup )
>>> +            {
>>> +                printk(XENLOG_ERR "IVHD Error: Out of memory\n");
>>> +                error = -ENOMEM;
>>> +            }
>>> +        }
>>> +    }
>>> +
>>>        return error;
>>>    }
>>>
>>
>> Don't we end up with ioapic_sbdf[IO_APIC_ID(apic)].bdf/seg being
>> uninitialized? They are usually set in parse_ivhd_device_special(), at
>> the same time pin_setup is allocated, but with IVRS broken in this way
>> we'll never get there, will we?
>
> Correct. .bdf/.seg being uninitialized is no much of a problem
> when using global intremap tables though.

Since this patch has been tested it clearly must have worked somehow but 
I don't understand where you'd get bdf and seg in 
amd_iommu_ioapic_update_ire() and then manage to find the right IOMMU.

-boris

 > And certainly not on
> a system with just a single IOMMU (as was the case on the
> crashing system). Do you see alternatives? Disable the IOMMU
> always, even if not using global remap tables? That could be
> seen as a regression, as at least global remap tables worked fine
> so far on such systems.