Hi Daniel,
On 6/27/25 3:15 PM, Daniel P. Berrangé wrote:
On Fri, Jun 27, 2025 at 01:37:55PM +0800, Shaoqin Huang wrote:
Hi Eric,
On 6/26/25 4:01 PM, Eric Auger wrote:
On 6/26/25 4:05 AM, Shaoqin Huang wrote:
Hi Eric,
On 6/23/25 5:20 PM, Eric Auger wrote:
On 6/17/25 5:05 AM, Shaoqin Huang wrote:
Set the "use-legacy-x86-rom" property to false by default, and only set
it to true on x86 since only x86 will need it.
s/compatatibility/compatibility in the title
Ok. Will fix it.
Signed-off-by: Shaoqin Huang <shahu...@redhat.com>
---
hw/display/ramfb-standalone.c | 2 +-
hw/i386/pc_q35.c | 3 +++
hw/vfio/pci.c | 2 +-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/hw/display/ramfb-standalone.c b/hw/display/ramfb-
standalone.c
index af1175bf96..ddbf42f181 100644
--- a/hw/display/ramfb-standalone.c
+++ b/hw/display/ramfb-standalone.c
@@ -63,7 +63,7 @@ static const VMStateDescription ramfb_dev_vmstate = {
static const Property ramfb_properties[] = {
DEFINE_PROP_BOOL("x-migrate", RAMFBStandaloneState, migrate,
true),
- DEFINE_PROP_BOOL("use-legacy-x86-rom", RAMFBStandaloneState,
use_legacy_x86_rom, true),
+ DEFINE_PROP_BOOL("use-legacy-x86-rom", RAMFBStandaloneState,
use_legacy_x86_rom, false),
};
static void ramfb_class_initfn(ObjectClass *klass, void *data)
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index fd96d0345c..f6d89578d0 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -45,6 +45,7 @@
#include "hw/i386/pc.h"
#include "hw/i386/amd_iommu.h"
#include "hw/i386/intel_iommu.h"
+#include "hw/vfio/pci.h"
#include "hw/virtio/virtio-iommu.h"
#include "hw/display/ramfb.h"
#include "hw/ide/pci.h"
@@ -67,6 +68,8 @@
static GlobalProperty pc_q35_compat_defaults[] = {
{ TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "39" },
+ { TYPE_RAMFB_DEVICE, "use-legacy-x86-rom", "true" },
+ { TYPE_VFIO_PCI, "use-legacy-x86-rom", "true" },
this will only keep the legacy behavior along with q35 machine type but
not on other machines being used for x86. what about pc-i440fx? Doesn't
it apply to it as well? Are there other machine types also impacted.
Ok I will also add it with pc-i440fx. I think only q35 and i440fx are
impacted.
Also what about Daniel's comment in v3:
https://lore.kernel.org/all/aeak8utppkhep...@redhat.com/
"For non-x86, historical versioned machine types will need
likely it set to true, in order to avoid the memory layout
being changed IIUC."
Is it actually needed?
If those machine types need to set it to true. I think they can set it
after they have this property.
nope it does not work like that. In case we really need to take care of
this, this must be handled by compats.
If so. Why don't we still keep the "use-legacy-x86-rom" default to true, and
only set it to false to those arch which doesn't need it just like my
original implementation.
Because I don't really know how other arch's memoery layout was impacted by
this property set to false. I think keep their original behavior and only
change it on arm64 is a good idea.
How do you think about it?
No, the default value of the property shoudl reflect the long
term desired behaviour - in this case 'use-legacy-x86-rom = false'.
Ok I understand your thoughts.
We must then reverse this default to 'true' for ALL historical versioned
machine types on ALL architectures, where this device is built, or any
specific machines where we want to keep the historical behaviour going
forward.
But how do we implement that?
Maybe set the 'use-legacy-x86-rom = true' in hw_compat_9_2 ?
I'm not sure about that, could you give some hints about how to reverse
the default to 'true' for ALL historical versioned machine types?
Thanks a lot.
With regards,
Daniel
--
Shaoqin
