All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] "x-disable-pcie" virtio-pci property in compat_props (HW_COMPAT_2_4)
@ 2016-02-09 17:38 Laurent Vivier
  2016-02-10  9:52 ` Marcel Apfelbaum
  0 siblings, 1 reply; 3+ messages in thread
From: Laurent Vivier @ 2016-02-09 17:38 UTC (permalink / raw)
  To: qemu-devel qemu-devel; +Cc: Marcel Apfelbaum, Eduardo Habkost

Hi,

I'm playing with a qemu-2.5.0 and pc-i440fx-2.4 machine type, and
perhaps I don't understand correctly the compat_props machinery but
there is something strange for me:

in qemu-2.5.0, hw/virtio/virtio-pci.c:

   1880     DEFINE_PROP_BIT("x-disable-pcie", VirtIOPCIProxy, flags,
   1881                     VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, false),

So I guess a new default machine (pc-i440fx-2.5) will be defined by
default with "x-disable-pcie" set to "false".

and in qemu-2.5, include/hw/compat.h:

      3
      4 #define HW_COMPAT_2_4 \
      5         {\
..
     14             .driver   = "virtio-pci",\
     15             .property = "x-disable-pcie",\
     16             .value    = "on",\
     17         },{\

So I guess a new machine pc-i440fx-2.4 created with qemu-2.5.0 will have
"x-disable-pcie" set to true (as with qemu-2.4.0 this feature is not
available).

But in this case "x-disable-pcie" is always set to "false".

So the questions are:
- do I understand correctly?
- I check "x-disable-pcie" for "virtio-*-pci" devices (virtio-net-pci,
virtio-balloon-pci, ..."), should they inherit the behavior from their
parent "virtio-pci"? (they should, as virtio-pci is an abstract device type)

My test:

- take qemu-2.5.0 (a8c40fa Update version for v2.5.0 release)
- start it with "-M pc-i440fx-2.5 -device virtio-balloon-pci -S",
  "info qtree":

    bus: pci.0
      type PCI
      dev: virtio-balloon-pci, id ""
...
        x-disable-pcie = false

- start it with "-M pc-i440fx-2.4 -device virtio-balloon-pci -S",
  "info qtree":

    bus: pci.0
      type PCI
      dev: virtio-balloon-pci, id ""
...
        x-disable-pcie = false

Any explanation is very welcome.

Thanks,
Laurent

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [Qemu-devel] "x-disable-pcie" virtio-pci property in compat_props (HW_COMPAT_2_4)
  2016-02-09 17:38 [Qemu-devel] "x-disable-pcie" virtio-pci property in compat_props (HW_COMPAT_2_4) Laurent Vivier
@ 2016-02-10  9:52 ` Marcel Apfelbaum
  2016-02-10 10:09   ` Laurent Vivier
  0 siblings, 1 reply; 3+ messages in thread
From: Marcel Apfelbaum @ 2016-02-10  9:52 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel qemu-devel
  Cc: Jason Wang, Eduardo Habkost, Michael S. Tsirkin

On 02/09/2016 07:38 PM, Laurent Vivier wrote:
> Hi,
>
> I'm playing with a qemu-2.5.0 and pc-i440fx-2.4 machine type, and
> perhaps I don't understand correctly the compat_props machinery but
> there is something strange for me:
>
> in qemu-2.5.0, hw/virtio/virtio-pci.c:
>
>     1880     DEFINE_PROP_BIT("x-disable-pcie", VirtIOPCIProxy, flags,
>     1881                     VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, false),
>
> So I guess a new default machine (pc-i440fx-2.5) will be defined by
> default with "x-disable-pcie" set to "false".

Correct, but we are talking about both pc and q35.
By default, virtio devices *can be* PCIe for all new machines.

>
> and in qemu-2.5, include/hw/compat.h:
>
>        3
>        4 #define HW_COMPAT_2_4 \
>        5         {\
> ..
>       14             .driver   = "virtio-pci",\
>       15             .property = "x-disable-pcie",\
>       16             .value    = "on",\
>       17         },{\
>
> So I guess a new machine pc-i440fx-2.4 created with qemu-2.5.0 will have
> "x-disable-pcie" set to true (as with qemu-2.4.0 this feature is not
> available).

Exactly, machines before 2.5 will not have virtio devices PCIe
Old q35 machines are also included.

>
> But in this case "x-disable-pcie" is always set to "false".

For older machines yes, for newer no.

>
> So the questions are:
> - do I understand correctly?

Does the above respond to your question?

> - I check "x-disable-pcie" for "virtio-*-pci" devices (virtio-net-pci,
> virtio-balloon-pci, ..."), should they inherit the behavior from their
> parent "virtio-pci"? (they should, as virtio-pci is an abstract device type)

Yes, they do inherit this property.

>
> My test:
>
> - take qemu-2.5.0 (a8c40fa Update version for v2.5.0 release)
> - start it with "-M pc-i440fx-2.5 -device virtio-balloon-pci -S",
>    "info qtree":
>
>      bus: pci.0
>        type PCI
>        dev: virtio-balloon-pci, id ""
> ...
>          x-disable-pcie = false

This is correct again.

>
> - start it with "-M pc-i440fx-2.4 -device virtio-balloon-pci -S",
>    "info qtree":
>
>      bus: pci.0
>        type PCI
>        dev: virtio-balloon-pci, id ""
> ...
>          x-disable-pcie = false

This is a bug! Thanks for finding it.
It took a little but I found the root cause.

We have 2 virtio features mapped into the same bit:
- #define VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT 4
- #define VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT 4

Since both properties appear in HW_COMPAT_2_4, but
migrate-extra is the last one used, it wins the race.


To the obvious question of "how did that happen?" I can say we had an unlucky break.
Both Jason and me worked on a new different virtio feature in the same time,
and they were both merged in the same pull request.
We both saw BIT 3 as the last used :)

https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03041.html

I'll send a patch to map virtio pcie feature to a different  bit.
Thanks,
Marcel


>
> Any explanation is very welcome.
>
> Thanks,
> Laurent
>

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [Qemu-devel] "x-disable-pcie" virtio-pci property in compat_props (HW_COMPAT_2_4)
  2016-02-10  9:52 ` Marcel Apfelbaum
@ 2016-02-10 10:09   ` Laurent Vivier
  0 siblings, 0 replies; 3+ messages in thread
From: Laurent Vivier @ 2016-02-10 10:09 UTC (permalink / raw)
  To: Marcel Apfelbaum, qemu-devel qemu-devel
  Cc: Jason Wang, Eduardo Habkost, Michael S. Tsirkin



On 10/02/2016 10:52, Marcel Apfelbaum wrote:
> On 02/09/2016 07:38 PM, Laurent Vivier wrote:
>> Hi,
>>
>> I'm playing with a qemu-2.5.0 and pc-i440fx-2.4 machine type, and
>> perhaps I don't understand correctly the compat_props machinery but
>> there is something strange for me:
>>
>> in qemu-2.5.0, hw/virtio/virtio-pci.c:
>>
>>     1880     DEFINE_PROP_BIT("x-disable-pcie", VirtIOPCIProxy, flags,
>>     1881                     VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT, false),
>>
>> So I guess a new default machine (pc-i440fx-2.5) will be defined by
>> default with "x-disable-pcie" set to "false".
> 
> Correct, but we are talking about both pc and q35.
> By default, virtio devices *can be* PCIe for all new machines.
> 
>>
>> and in qemu-2.5, include/hw/compat.h:
>>
>>        3
>>        4 #define HW_COMPAT_2_4 \
>>        5         {\
>> ..
>>       14             .driver   = "virtio-pci",\
>>       15             .property = "x-disable-pcie",\
>>       16             .value    = "on",\
>>       17         },{\
>>
>> So I guess a new machine pc-i440fx-2.4 created with qemu-2.5.0 will have
>> "x-disable-pcie" set to true (as with qemu-2.4.0 this feature is not
>> available).
> 
> Exactly, machines before 2.5 will not have virtio devices PCIe
> Old q35 machines are also included.
> 
>>
>> But in this case "x-disable-pcie" is always set to "false".
> 
> For older machines yes, for newer no.
> 
>>
>> So the questions are:
>> - do I understand correctly?
> 
> Does the above respond to your question?
> 
>> - I check "x-disable-pcie" for "virtio-*-pci" devices (virtio-net-pci,
>> virtio-balloon-pci, ..."), should they inherit the behavior from their
>> parent "virtio-pci"? (they should, as virtio-pci is an abstract device
>> type)
> 
> Yes, they do inherit this property.
> 
>>
>> My test:
>>
>> - take qemu-2.5.0 (a8c40fa Update version for v2.5.0 release)
>> - start it with "-M pc-i440fx-2.5 -device virtio-balloon-pci -S",
>>    "info qtree":
>>
>>      bus: pci.0
>>        type PCI
>>        dev: virtio-balloon-pci, id ""
>> ...
>>          x-disable-pcie = false
> 
> This is correct again.
> 
>>
>> - start it with "-M pc-i440fx-2.4 -device virtio-balloon-pci -S",
>>    "info qtree":
>>
>>      bus: pci.0
>>        type PCI
>>        dev: virtio-balloon-pci, id ""
>> ...
>>          x-disable-pcie = false
> 
> This is a bug! Thanks for finding it.
> It took a little but I found the root cause.
> 
> We have 2 virtio features mapped into the same bit:
> - #define VIRTIO_PCI_FLAG_MIGRATE_EXTRA_BIT 4
> - #define VIRTIO_PCI_FLAG_DISABLE_PCIE_BIT 4
> 
> Since both properties appear in HW_COMPAT_2_4, but
> migrate-extra is the last one used, it wins the race.
> 
> 
> To the obvious question of "how did that happen?" I can say we had an
> unlucky break.
> Both Jason and me worked on a new different virtio feature in the same
> time,
> and they were both merged in the same pull request.
> We both saw BIT 3 as the last used :)
> 
> https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg03041.html
> 
> I'll send a patch to map virtio pcie feature to a different  bit.
> Thanks,

Thank you Marcel!

You have answered to all my questions and find a fix. This is perfect :)

Laurent

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-02-10 10:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-09 17:38 [Qemu-devel] "x-disable-pcie" virtio-pci property in compat_props (HW_COMPAT_2_4) Laurent Vivier
2016-02-10  9:52 ` Marcel Apfelbaum
2016-02-10 10:09   ` Laurent Vivier

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.