All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcel Apfelbaum <marcel@redhat.com>
To: Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
Cc: qemu-devel@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] virtio-pci: Set the QEMU_PCI_CAP_EXPRESS capability early in its DeviceClass realize method
Date: Wed, 2 Dec 2015 16:00:41 +0200	[thread overview]
Message-ID: <565EF989.9030300@redhat.com> (raw)
In-Reply-To: <20151202153012.08b80fd9@pixies>

On 12/02/2015 03:30 PM, Shmulik Ladkani wrote:
> Hi,
>
> On Wed, 2 Dec 2015 11:51:46 +0200, marcel@redhat.com wrote:
>> 2. We still have pci_is_express returning true, this is error prone because
>> one can use this function assuming the device is express. Maybe we should
>> call it "can_be_express" ?
>>
>> If you think this is good enough, you can simply do the same:
>>     - Instead of replacing the realize method, just advertise it with
>>       "is_express" (meaning it can be express)
>>     - Leave all the conditions as they were in prev patch.
>> As a result, the pci config space will have the right length.
>
> Oh but we can't do so, as the change of config space size is guest
> visible and breaks migration; it must depend on your x-pcie-disable
> flag :)

Indeed, we need at least to condition it on  x-pcie-disable.

>
> As I can't decide what's better, I'm following your initial suggestion
> and submit for maintainers to review.

Sure, and thanks for the patience to get to the bottom of it.

>
> However, do note that there are few more evidence that 'pci_is_express'
> is true while not necessarily placed on a pcie bus:

and this is scary ... we really should call it "pci_can_be_express"

> - pcie_endpoint_cap_init:
>    it tests for 'pci_bus_is_express' although 'dev' is guaranteed to be
>    'pci_is_express' (assertion in pcie_cap_init)
> - 058fdcf 'xhci: add endpoint cap on express bus only'


Thanks,
Marcel

>
> Thanks,
> Shmulik
>

  reply	other threads:[~2015-12-02 14:00 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-01 16:23 [Qemu-devel] [PATCH] virtio-pci: Set the QEMU_PCI_CAP_EXPRESS capability early in its DeviceClass realize method Shmulik Ladkani
2015-12-01 16:36 ` Marcel Apfelbaum
2015-12-01 19:30   ` Shmulik Ladkani
2015-12-01 20:46     ` Marcel Apfelbaum
2015-12-02  8:01       ` Shmulik Ladkani
2015-12-02  9:51         ` Marcel Apfelbaum
2015-12-02 13:30           ` Shmulik Ladkani
2015-12-02 14:00             ` Marcel Apfelbaum [this message]
2015-12-02 14:27               ` Shmulik Ladkani

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=565EF989.9030300@redhat.com \
    --to=marcel@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=shmulik.ladkani@ravellosystems.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.