From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suravee Suthikulanit Subject: Re: [PATCH 1/1] x86/AMD: Fix setup ssss:bb:dd:f for d0 failed Date: Wed, 7 Aug 2013 14:20:50 -0500 Message-ID: <52029E12.20305@amd.com> References: <1375843208-3165-1-git-send-email-suravee.suthikulpanit@amd.com> <5202147D02000078000E9D00@nat28.tlf.novell.com> <5202685F.8090601@amd.com> <5202871902000078000EA01A@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3370210312363515009==" Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1V79J9-0005Gt-48 for xen-devel@lists.xenproject.org; Wed, 07 Aug 2013 19:22:07 +0000 In-Reply-To: <5202871902000078000EA01A@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel , xiantao.zhang@intel.com List-Id: xen-devel@lists.xenproject.org --===============3370210312363515009== Content-Type: multipart/alternative; boundary="------------020804040206000304060008" --------------020804040206000304060008 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit On 8/7/2013 10:42 AM, Jan Beulich wrote: >>>> On 07.08.13 at 17:31, Suravee Suthikulanit wrote: >> On 8/7/2013 2:33 AM, Jan Beulich wrote: >>>>>> On 07.08.13 at 04:40, wrote: >>>> + /* Filter the bridge devices */ >>>> + if ( (pdev->type == DEV_TYPE_PCIe_BRIDGE) >>>> + || (pdev->type == DEV_TYPE_PCIe2PCI_BRIDGE) >>>> + || (pdev->type == DEV_TYPE_LEGACY_PCI_BRIDGE) >>>> + || (pdev->type == DEV_TYPE_PCI_HOST_BRIDGE) ) >>>> + { >>>> + AMD_IOMMU_DEBUG("Skipping device %04x:%02x:%02x.%u (type %x)\n", >>>> + pdev->seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf), >>>> + pdev->type); >>> I can see why host bridges can be skipped, but is this really also true >>> for other bridge types, most importantly legacy ones? >> I am using the same logic here as in Intel's version in the >> driver/passthrough/vtd/iommu/domain_context_map. > No, not really: That code establishes a mapping for the upstream > bridge of a legacy PCI device when handling that device. Your > proposed code doesn't afaics. (This I understand is because > bridges aren't expected to get assigned to guests, and hence > otherwise the mapping for the bridge would never get established > for a device behind it for other than Dom0.) > > Jan Jan, AFAICT from the domain_context_mapping() below, which get called when the devices are assigned to domains static int domain_context_mapping( struct domain *domain, u8 devfn, const struct pci_dev *pdev) { struct acpi_drhd_unit *drhd; int ret = 0; u8 seg = pdev->seg, bus = pdev->bus, secbus; drhd = acpi_find_matched_drhd_unit(pdev); if ( !drhd ) return -ENODEV; ASSERT(spin_is_locked(&pcidevs_lock)); switch ( pdev->type ) { case DEV_TYPE_PCIe_BRIDGE: case DEV_TYPE_PCIe2PCI_BRIDGE: case DEV_TYPE_LEGACY_PCI_BRIDGE: break; These bridges are actually not mapped(i.e. skipped). That is why I think the logic above is supposed to be doing the same thing. Suravee --------------020804040206000304060008 Content-Type: text/html; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit
On 8/7/2013 10:42 AM, Jan Beulich wrote:
On 07.08.13 at 17:31, Suravee Suthikulanit <suravee.suthikulpanit@amd.com> wrote:

On 8/7/2013 2:33 AM, Jan Beulich wrote:

On 07.08.13 at 04:40, <suravee.suthikulpanit@amd.com> wrote:

+        /* Filter the bridge devices */
+        if ( (pdev->type == DEV_TYPE_PCIe_BRIDGE)
+        ||   (pdev->type == DEV_TYPE_PCIe2PCI_BRIDGE)
+        ||   (pdev->type == DEV_TYPE_LEGACY_PCI_BRIDGE)
+        ||   (pdev->type == DEV_TYPE_PCI_HOST_BRIDGE) )
+        {
+            AMD_IOMMU_DEBUG("Skipping device %04x:%02x:%02x.%u (type %x)\n",
+                        pdev->seg, PCI_BUS(bdf), PCI_SLOT(bdf), PCI_FUNC(bdf),
+                        pdev->type);

I can see why host bridges can be skipped, but is this really also true
for other bridge types, most importantly legacy ones?

I am using the same logic here as in Intel's version in the 
driver/passthrough/vtd/iommu/domain_context_map.

No, not really: That code establishes a mapping for the upstream
bridge of a legacy PCI device when handling that device. Your
proposed code doesn't afaics. (This I understand is because
bridges aren't expected to get assigned to guests, and hence
otherwise the mapping for the bridge would never get established
for a device behind it for other than Dom0.)

Jan
Jan,

AFAICT from the domain_context_mapping() below, which get called when the devices are assigned to domains

static int domain_context_mapping(
    struct domain *domain, u8 devfn, const struct pci_dev *pdev)
{
    struct acpi_drhd_unit *drhd;
    int ret = 0;
    u8 seg = pdev->seg, bus = pdev->bus, secbus;

    drhd = acpi_find_matched_drhd_unit(pdev);
    if ( !drhd )
        return -ENODEV;

    ASSERT(spin_is_locked(&pcidevs_lock));

    switch ( pdev->type )
    {
    case DEV_TYPE_PCIe_BRIDGE:
    case DEV_TYPE_PCIe2PCI_BRIDGE:
    case DEV_TYPE_LEGACY_PCI_BRIDGE:
        break;

These bridges are actually not mapped (i.e. skipped).  That is why I think the logic above is supposed to be doing the same thing.

Suravee
 --------------020804040206000304060008-- --===============3370210312363515009== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============3370210312363515009==--