| Message ID | 1495658355-13103-1-git-send-email-haiyangz@exchange.microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
> -----Original Message----- > From: Haiyang Zhang [mailto:haiyangz@exchange.microsoft.com] > Sent: Wednesday, May 24, 2017 4:39 PM > To: bhelgaas@google.com; linux-pci@vger.kernel.org > Cc: Haiyang Zhang <haiyangz@microsoft.com>; KY Srinivasan > <kys@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>; > olaf@aepfle.de; vkuznets@redhat.com; driverdev- > devel@linuxdriverproject.org; linux-kernel@vger.kernel.org > Subject: [PATCH] pci-hyperv: Use only 16 bit integer for PCI domain > > [This sender failed our fraud detection checks and may not be who they > appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing] > > From: Haiyang Zhang <haiyangz@microsoft.com> > > This patch uses the lower 16 bits of the serial number as PCI > domain, otherwise some drivers may not be able to handle it. > > Besides Nvidia drivers, we also found X.org, and DPDK handle > only 16 bit PCI domain. > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> > --- According to Stephen Hemminger <sthemmin@microsoft.com>, there are additional programs, like X.org, DPDK, are also using 16-bit only PCI domain numbers. So, I'm submitting this patch for re-consideration. Thanks, - Haiyang
On Wed, May 24, 2017 at 01:39:15PM -0700, Haiyang Zhang wrote: > From: Haiyang Zhang <haiyangz@microsoft.com> > > This patch uses the lower 16 bits of the serial number as PCI > domain, otherwise some drivers may not be able to handle it. > > Besides Nvidia drivers, we also found X.org, and DPDK handle > only 16 bit PCI domain. And they will all break behind VMD for example, so NAK. Linux domains are 32 bits.
On Thu, 25 May 2017 14:19:55 +0100 Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote: > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> > > > --- > > > > According to Stephen Hemminger <sthemmin@microsoft.com>, there are > > additional programs, like X.org, DPDK, are also using 16-bit only > > PCI domain numbers. So, I'm submitting this patch for re-consideration. > > The correct way to handle this is to send the needed patches to DPDK and > to X.org both of whom will I am sure be delighted to get it fixed in > their codebase. Both projects have stable ABI requirements. And the lead time to get the change propagated out to applications is long (>5yrs) even longer with the Enterprise distro's. As developers we can all just pass the buck but this doesn't help users in any reasonable time fram. It is not as simple as just making a patch or pull request for their upstream code bases.
[+cc Christoph] On Wed, May 24, 2017 at 01:39:15PM -0700, Haiyang Zhang wrote: > From: Haiyang Zhang <haiyangz@microsoft.com> > > This patch uses the lower 16 bits of the serial number as PCI > domain, otherwise some drivers may not be able to handle it. > > Besides Nvidia drivers, we also found X.org, and DPDK handle > only 16 bit PCI domain. If you've sent patches to X.org and DPDK, please includes URLs to them. Christoph pointed out the conflict with VMD: vmd_find_free_domain() allocates domains starting at 0x10000 to avoid the 16-bit domains returned by ACPI _SEG. I think we need a solution that works for both Nvidia/Hyper-V and VMD. As it is, it looks like this will fix one place but break things elsewhere. If you believe that this will not break VMD, please explain. Bjorn > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> > --- > drivers/pci/host/pci-hyperv.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 8493638..51a815d 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -1335,9 +1335,11 @@ static void put_pcichild(struct hv_pci_dev *hpdev, > * can have shorter names than based on the bus instance UUID. > * Only the first device serial number is used for domain, so the > * domain number will not change after the first device is added. > + * The lower 16 bits of the serial number is used, otherwise some > + * drivers may not be able to handle it. > */ > if (list_empty(&hbus->children)) > - hbus->sysdata.domain = desc->ser; > + hbus->sysdata.domain = desc->ser & 0xFFFF; > list_add_tail(&hpdev->list_entry, &hbus->children); > spin_unlock_irqrestore(&hbus->device_list_lock, flags); > return hpdev; > -- > 1.7.1 >
FYI, I've also got another driver in progress that will need domains assigned outside the ACPI range, so it's not just limited to VMD.
diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c index 8493638..51a815d 100644 --- a/drivers/pci/host/pci-hyperv.c +++ b/drivers/pci/host/pci-hyperv.c @@ -1335,9 +1335,11 @@ static void put_pcichild(struct hv_pci_dev *hpdev, * can have shorter names than based on the bus instance UUID. * Only the first device serial number is used for domain, so the * domain number will not change after the first device is added. + * The lower 16 bits of the serial number is used, otherwise some + * drivers may not be able to handle it. */ if (list_empty(&hbus->children)) - hbus->sysdata.domain = desc->ser; + hbus->sysdata.domain = desc->ser & 0xFFFF; list_add_tail(&hpdev->list_entry, &hbus->children); spin_unlock_irqrestore(&hbus->device_list_lock, flags); return hpdev;