All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eduardo Habkost <ehabkost@redhat.com>
To: Greg Kurz <groug@kaod.org>
Cc: qemu-devel@nongnu.org, Halil Pasic <pasic@linux.vnet.ibm.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	qemu-stable@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>,
	Cornelia Huck <cornelia.huck@de.ibm.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [PATCH] virtio: fix HW_COMPAT_2_6 macro for virtio-*-pci drivers
Date: Tue, 6 Dec 2016 17:19:52 -0200	[thread overview]
Message-ID: <20161206191952.GD4027@thinpad.lan.raisama.net> (raw)
In-Reply-To: <148104617887.7699.984921868108236415.stgit@bahia.lab.toulouse-stg.fr.ibm.com>

On Tue, Dec 06, 2016 at 06:50:47PM +0100, Greg Kurz wrote:
> Since commit "9a4c0e220d8a hw/virtio-pci: fix virtio behaviour", passing
> -device virtio-blk-pci.disable-modern=off has no effect on 2.6 machine
> types because the internal virtio-pci.disable-modern=on compat property
> always prevail.
> 
> This should ideally be fixed in the qdev properties core code, but it is
> too late in the QEMU 2.8 schedule. So this patch fixes the issue by setting
> the compat properties for every virtio-*-pci subtypes instead of the base
> virtio-pci type.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

So, it looks like the bug is present in many other cases...

I have hacked QEMU to print a warning in case the driver name in
compat_props refer to an abstract class or a class that have any
subclasses. The results are below:

 apic-common.legacy-instance-id set for abstract class
 apic-common.vapic set for abstract class
 i386-cpu.arat set for abstract class
 i386-cpu.check set for abstract class
 i386-cpu.cpuid-0xb set for abstract class
 i386-cpu.fill-mtrr-mask set for abstract class
 i386-cpu.full-cpuid-auto-level set for abstract class
 i386-cpu.l3-cache set for abstract class
 i386-cpu.pmu set for abstract class
 pci-device.command_serr_enable set for abstract class
 pci-device.rombar set for abstract class
 pci-device.x-pcie-lnksta-dllla set for abstract class
 powerpc64-cpu.pre-2.8-migration set for abstract class
 s390-skeys.migration-enabled set for abstract class
 spapr-pci-host-bridge.ddw set for superclass
 spapr-pci-host-bridge.dynamic-reconfiguration set for superclass
 spapr-pci-host-bridge.mem64_win_size set for superclass
 spapr-pci-host-bridge.mem_win_size set for superclass
 spapr-pci-host-bridge.pre-2.8-migration set for superclass
 usb-device.full-path set for abstract class
 usb-device.msos-desc set for abstract class
 virtio-pci.disable-legacy set for abstract class
 virtio-pci.disable-modern set for abstract class
 virtio-pci.migrate-extra set for abstract class
 virtio-pci.page-per-vq set for abstract class
 virtio-pci.virtio-pci-bus-master-bug-migration set for abstract class
 virtio-pci.x-disable-pcie set for abstract class
 x86_64-cpu.arat set for abstract class
 x86_64-cpu.check set for abstract class
 x86_64-cpu.cpuid-0xb set for abstract class
 x86_64-cpu.fill-mtrr-mask set for abstract class
 x86_64-cpu.full-cpuid-auto-level set for abstract class
 x86_64-cpu.l3-cache set for abstract class
 x86_64-cpu.pmu set for abstract class

I believe the cases where we are likely to cause real-world bugs
are virtio-pci and the *-cpu classes (because -cpu is translated
to -global).

I'm not sure what should be the right fix in 2.8. I am
considering a temporary hack to translate abstract class names in
compat_props to global properties for all subclasses, in case
they refer to an abstract class. This way we fix the bug where
-global doesn't override compat_props properly, but keep the
rules for -global untouched.

-- 
Eduardo

  parent reply	other threads:[~2016-12-06 19:20 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-06 17:50 [Qemu-devel] [PATCH] virtio: fix HW_COMPAT_2_6 macro for virtio-*-pci drivers Greg Kurz
2016-12-06 18:02 ` Cornelia Huck
2016-12-06 18:46 ` Eduardo Habkost
2016-12-06 19:19 ` Eduardo Habkost [this message]
2016-12-06 19:31   ` [Qemu-devel] [RFC for-2.8] machine: Convert abstract typename on compat_props to subclass names Eduardo Habkost
2016-12-07 13:39     ` Greg Kurz
2016-12-07 13:59       ` Eduardo Habkost
2016-12-07 14:58         ` Greg Kurz
2016-12-09 20:06       ` Eduardo Habkost
2016-12-12 11:26         ` Cornelia Huck
2016-12-12 11:39         ` Greg Kurz
2016-12-12 12:25     ` Cornelia Huck
2016-12-12 17:13       ` Halil Pasic
2016-12-12 17:47         ` Eduardo Habkost
2016-12-12 18:00           ` Halil Pasic
2016-12-12 17:36       ` Eduardo Habkost
2016-12-06 19:31 ` [Qemu-devel] [PATCH] virtio: fix HW_COMPAT_2_6 macro for virtio-*-pci drivers Michael S. Tsirkin
2016-12-06 19:33   ` Eduardo Habkost
2016-12-06 19:36     ` Michael S. Tsirkin
2016-12-06 19:40       ` Eduardo Habkost
2016-12-06 19:44         ` Michael S. Tsirkin
2016-12-06 19:48           ` Eduardo Habkost
2016-12-06 20:30             ` Stefan Hajnoczi
2016-12-07  8:36               ` Greg Kurz

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=20161206191952.GD4027@thinpad.lan.raisama.net \
    --to=ehabkost@redhat.com \
    --cc=cornelia.huck@de.ibm.com \
    --cc=groug@kaod.org \
    --cc=mst@redhat.com \
    --cc=pasic@linux.vnet.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=stefanha@redhat.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.