lkml.org 
[lkml]   [2018]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption
    On Tue, Mar 20, 2018 at 05:56:15PM +0000, Sridhar Pitchai wrote:
    > Hi Lorenzo,

    > Are we good with the explanation? Can I send the patch with the
    > updated commit comments?

    Almost.

    [...]

    > Since we have the transparent SRIOV mode now, the short VF device name
    > is no longer needed.

    Can you correlate transparent SRIOV mode to the point you are making
    below ? Please explain what transparent SRIOV mode allows you to remove
    and why. The rest of the explanation seems OK.

    Please follow this email format:

    http://vger.kernel.org/lkml/#s3-9

    Thanks,
    Lorenzo

    > I still do not understand what this means and how it is related to the
    > patch below, it may be clear to you, it is not to me, at all.
    >
    > Sridhar >> the patch below, was introduced to make the device name small, by taking only
    > 16bits of the serial number. Since we are not going to have the serial number
    > updated to the BUS id, this has to be removed.
    >
    > Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")
    >
    > Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
    > Sridhr >> yes
    >
    > I asked you an explicit question. Commit above was added for a reason
    > I assume. This patch implies that kernel has been broken since v4.11
    > which is almost a year ago and nobody every noticed ? Or there are
    > systems where commit above is _necessary_ and this patch would break
    > them ?
    >
    > I want a detailed explanation that highlights *why* it is safe to apply
    > this patch and send it to stable kernels, commit log above won't do.
    >
    > Sridhar>> HyperV provides a unique domain ID for PCI BUS. But it is modified by the child
    > device when it is added. This cannot produce a unique domain ID all the time.
    > Here in the bug, we see the collision between the serial number and already
    > existing PCI bus. The cleaner way is never touch the domain ID provided by
    > hyperV during the PCI bus creation. As long as hyperV make sure it provides a
    > unique domain ID for the PCI for a VM it will not break, and HyperV will
    > guarantees that the domain for the PCI bus for a given VM will be always unique.
    > The original patch was also intending to have a unique domain ID for the PCI
    > bus, by taking the serial number of the device, but it is not sufficient, when
    > the device serial number is number which is the domain ID of the existing PCI
    > bus. With the current kernel we can repro this issue by adding a device with a
    > serial number matching the existing PCI bus domain id. (in this case that
    > happens to be zero).
    >
    >
    > Thanks,
    > Lorenzo
    >
    > Cc: stable@vger.kernel.org
    > Signed-off-by: Sridhar Pitchai <srpitcha@microsoft.com>
    > ---
    > Changes in v3:
    > * fix the commit comment. [KY Srinivasan, Michael Kelley]
    > ---
    > drivers/pci/host/pci-hyperv.c | 11 -----------
    > 1 file changed, 11 deletions(-)
    > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
    > index 2faf38e..ac67e56 100644
    > --- a/drivers/pci/host/pci-hyperv.c
    > +++ b/drivers/pci/host/pci-hyperv.c
    > @@ -1518,17 +1518,6 @@ static struct hv_pci_dev *new_pcichild_device(struct hv_pcibus_device *hbus,
    > get_pcichild(hpdev, hv_pcidev_ref_childlist);
    > spin_lock_irqsave(&hbus->device_list_lock, flags);
    >
    > - /*
    > - * When a device is being added to the bus, we set the PCI domain
    > - * number to be the device serial number, which is non-zero and
    > - * unique on the same VM. The serial numbers start with 1, and
    > - * increase by 1 for each device. So device names including this
    > - * 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.
    > - */
    > - if (list_empty(&hbus->children))
    > - hbus->sysdata.domain = desc->ser;
    > list_add_tail(&hpdev->list_entry, &hbus->children);
    > spin_unlock_irqrestore(&hbus->device_list_lock, flags);
    > return hpdev;
    > --
    > 2.7.4
    >
    >
    >

    \
     
     \ /
      Last update: 2018-03-20 19:32    [W:3.891 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site